review for 7009361: JSR 292 Invalid value on stack on solaris-sparc with -Xcomp

[Tom Rodriguez]

On Apr 29, 2011, at 11:05 PM, Vladimir Kozlov wrote:

> Looks fine as far as I understand it and thank you for fixing loop predicates output.
> One thing which was not clear is registers usage in 32-bit code in trace_method_handle() in methodHandles_x86.cpp. Yes, from caller of trace_method_handle() you can find it out but in trace_method_handle() it looks like mess.

What would you like to see?

tom


> 
> Vladimir
> 
> On 4/29/11 3:29 PM, Tom Rodriguez wrote:
>> http://cr.openjdk.java.net/~never/7009361
>> 
>> 7009361: JSR 292 Invalid value on stack on solaris-sparc with -Xcomp
>> Reviewed-by:
>> 
>> In the invokedynamic world the signature of the method at an invoke
>> bytecode might note be the same as the signature of the callee you
>> finally end up in.  This all works correctly during normal execution
>> but it can break the logic that deopt uses to layout the frames.  In
>> particular on sparc we attempt to place the locals in the same
>> location that the interpreter execution would have produced by using
>> the callers expression stack address and callee->size_of_parameters()
>> to figure out where the top of the live stack is.  If the size of the
>> parameters at the original call site is less than the size of the
>> parameters of the actual callee then the locals will end up top of the
>> callers live expression stack.  The x86 version of this code places
>> the locals at the bottom of the frame which keeps this from happening
>> and I've modified sparc to do the same.  There's no reason they need
>> to be in the same location.
>> 
>> The other potential problem is that deopt also has logic that makes
>> sure the existing caller is enlarged enough to account for the
>> difference between the callee parameters and the callee locals.  In
>> the current world we don't really need to enlarge this space because
>> the method handle machinery also operates like the interpreter so it
>> extends the caller frame when injecting extra arguments, thus making
>> sure there's really enough space when we deopt.  Since it doesn't have
>> to work this way I decided to fix this logic to grab the caller notion
>> of the number of arguments and correct the last frame adjust using
>> this value.
>> 
>> Additionally the TraceMethodHandles logic was very broken in x86 so I
>> fixed it.  It was mainly broken because some of the
>> trace_method_handle calls are generated using an
>> InterpreterMacroAssembler which has extra asserts in call_VM_leaf_base
>> that don't really apply for the method handle adapters.  There were
>> also problems with the number of arguments being passed in 64 bit.  I
>> ended up moving super_call_VM_leaf up into MacroAssembler since
>> there's no way to figure out that you are using an
>> InterpreterMacroAssembler versus a MacroAssembler.
>> 
>> To debug this I added a new printing function,
>> JavaThread::print_frame_layout, that prints an annotated view of the
>> stack memory similar to what the SA's memory viewer does.  It also
>> includes some logic to check that the space owned by a frame isn't
>> claimed by another frame.  That actually detects the original bug
>> immediately.  It's not as full featured as it might be but it reports
>> everything you need to know about interpreter frames.
>> 
>> I also made a small change in loopPredicate because the ttyLocker was
>> producing spurious output in the log all the time.
>> 
>> Tested with original failing test from test and DeoptimizeALot testing
>> on sparc.
>>