Code Review for 7009361

Prepared by:never on Fri Apr 29 15:29:00 PDT 2011
Workspace:/net/smite.us.oracle.com/export/ws/indy
Compare against: ssh://hg.openjdk.java.net/jdk7/hotspot-comp-gate/hotspot
Summary of changes: 510 lines changed: 343 ins; 131 del; 36 mod; 32672 unchg
Patch of changes: 7009361.patch
Author comments:
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.

Bug id: 7009361 JSR 292 Invalid value on stack on solaris-sparc with -Xcomp
Legend: Modified file
Deleted file
New file

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/share/vm/runtime/deoptimization.cpp

24 lines changed: 21 ins; 1 del; 2 mod; 1925 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/cpu/sparc/vm/templateInterpreter_sparc.cpp

27 lines changed: 0 ins; 14 del; 13 mod; 2002 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/cpu/sparc/vm/methodHandles_sparc.cpp

4 lines changed: 2 ins; 0 del; 2 mod; 1028 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/share/vm/runtime/thread.hpp

6 lines changed: 6 ins; 0 del; 0 mod; 1853 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/share/vm/runtime/thread.cpp

20 lines changed: 20 ins; 0 del; 0 mod; 4310 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/share/vm/utilities/debug.cpp

12 lines changed: 12 ins; 0 del; 0 mod; 898 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/share/vm/runtime/frame.hpp

41 lines changed: 41 ins; 0 del; 0 mod; 505 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/share/vm/runtime/frame.cpp

134 lines changed: 134 ins; 0 del; 0 mod; 1321 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/cpu/x86/vm/frame_x86.cpp

20 lines changed: 20 ins; 0 del; 0 mod; 671 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/cpu/sparc/vm/frame_sparc.cpp

31 lines changed: 31 ins; 0 del; 0 mod; 808 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/cpu/x86/vm/methodHandles_x86.cpp

43 lines changed: 5 ins; 27 del; 11 mod; 1182 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/cpu/x86/vm/assembler_x86.hpp

8 lines changed: 8 ins; 0 del; 0 mod; 2370 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/cpu/x86/vm/assembler_x86.cpp

37 lines changed: 37 ins; 0 del; 0 mod; 9504 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/cpu/x86/vm/interp_masm_x86_32.hpp

6 lines changed: 0 ins; 6 del; 0 mod; 231 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/cpu/x86/vm/interp_masm_x86_32.cpp

26 lines changed: 0 ins; 26 del; 0 mod; 1404 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/cpu/x86/vm/interp_masm_x86_64.hpp

7 lines changed: 0 ins; 7 del; 0 mod; 246 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/cpu/x86/vm/interp_masm_x86_64.cpp

50 lines changed: 0 ins; 50 del; 0 mod; 1462 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/share/vm/opto/loopPredicate.cpp

14 lines changed: 6 ins; 0 del; 8 mod; 952 unchg

This code review page was prepared using /never/bin/webrev (vers 23.18-hg-never).