On Thu, Feb 6, 2014 at 6:49 PM, Subramanya Sastry <sss.li...@gmail.com>wrote:
>
>
>
> On Thu, Feb 6, 2014 at 5:46 PM, Charles Oliver Nutter <head...@headius.com
> > wrote:
>
> I also ran into an issue with how heap variable assignment is being
>> optimized:
>>
>> for the closure passed to foo: a = 'here'; foo { a = 'bar'; puts a }
>>
>> Linearized instructions for JIT:
>> 0 thread_poll
>> 1 line_num(0)
>> 2 %t_a_1 = "bar"
>> 3 %cl_1_0 = call_1o(FUNCTIONAL, 'puts', %self, [%t_a_1]){1O}
>> 4 store_lvar(%t_a_1, -e_CLOSURE_1, a(1:0))
>> 5 return(%cl_1_0)
>> 6 %cl_1_2 = recv_jruby_exc
>> 7 store_lvar(%t_a_1, -e_CLOSURE_1, a(1:0))
>> 8 runtime_helper(catchUncaughtBreakInLambdas, [%cl_1_2])
>>
>> The exception table here handles exceptions from 0 to 5 by branching to
>> 6. However, 7 attempts to load a value first assigned within that catch
>> range. When I try to compile this to JVM bytecode, it fails because there's
>> no guarantee that %t_a_1 has been assigned before 7 runs.
>>
>
> This looks like a bug in the AddLocalVarLoadStoreInstrs pass ... It
> should have dumped dirty vars at all sites that could raise excs. -- in
> this case before the puts.
> Will take a look in the coming days.
>
> We should also think about adding additional tests beyond rubyspecs to
> stress test some of this ... given that the rubyspec run is back to old
> state (10 fails, 3 errors) with this pass enabled.
>
This bug is now fixed (
https://github.com/jruby/jruby/commit/f5facaa1be5656da02e18131aca90cdc8ef0d94f
)
-S.
