review for 7009361: JSR 292 Invalid value on stack on solaris-sparc with -Xcomp
tom.rodriguez at oracle.com
Mon May 2 10:06:13 PDT 2011
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?
> On 4/29/11 3:29 PM, Tom Rodriguez wrote:
>> 7009361: JSR 292 Invalid value on stack on solaris-sparc with -Xcomp
>> 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.
More information about the hotspot-compiler-dev