Request for reviews (M): 7090904: JSR 292: JRuby junit test crashes in PSScavengeRootsClosure::do_oop

[Vladimir Kozlov]

Christian Thalinger wrote:
>> I think you need move the check inside assert:
>>
>> +     if (caller->is_interpreted_frame()) {
>> +       assert(locals < caller->fp() + frame::interpreter_frame_initial_sp_offset, "bad placement");
>> +     }
>> ---
>> +     assert(!caller->is_interpreted_frame() ||
>> +            (locals < caller->fp() + frame::interpreter_frame_initial_sp_offset), "bad placement");
> 
> Why do you think that?  I very easy to read with the if.

I don't trust C++ compilers to eliminate is_interpreted_frame() call in product 
VM even if the body of 'if' statement is empty. You can enclose it with #ifdef 
ASSERT if you want to keep 'if'.

Vladimir

> 
> -- Chris
> 
>>
>> Why assert was not added to sparc?
>>
>> You did not explain changes in bytecodeTracer.cpp and in frame.cpp (frame::describe()). They seems unrelated to this fix.
>>
>> Thanks,
>> Vladimir
>>
>> Christian Thalinger wrote:
>>> http://cr.openjdk.java.net/~twisti/7090904/
>>> 7090904: JSR 292: JRuby junit test crashes in PSScavengeRootsClosure::do_oop
>>> Reviewed-by:
>>> A couple of JRuby junit tests crash in various forms (mostly GC
>>> related) when running on 64-bit Linux.  Although it looks like a GC
>>> related problem it actually is a deoptimization bug.
>>> Tom already had a fix ready but we were unable to write a test case to
>>> prove the correctness of the fix.  An assert added in
>>> AbstractInterpreter::layout_activation shows that the JRuby crashes
>>> are indeed an incarnation of this deoptimization bug.
>>> The bug itself is in the deoptimization logic that calculates the size
>>> of parameter space in the caller frame.  Method handle invokes may
>>> involve fairly arbitrary chains of calls so it's impossible to know
>>> how much actual space the caller has for locals.
>>> The fix is to always assume zero parameters for method handle call
>>> sites.
>>> Extensive testing with JRuby junit tests.
>>> src/share/vm/runtime/deoptimization.cpp
>>> src/cpu/x86/vm/templateInterpreter_x86_32.cpp
>>> src/cpu/x86/vm/templateInterpreter_x86_64.cpp
>>> src/share/vm/interpreter/bytecodeTracer.cpp
>>> src/share/vm/runtime/frame.cpp
>>> src/share/vm/runtime/frame.hpp
>>> src/share/vm/runtime/thread.cpp
>