JDK-7009361 : JSR 292 Invalid value on stack on solaris-sparc with -Xcomp
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: hs20,7
  • Priority: P3
  • Status: Closed
  • Resolution: Fixed
  • OS: generic,solaris
  • CPU: generic,sparc
  • Submitted: 2010-12-28
  • Updated: 2018-03-26
  • Resolved: 2011-05-16
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 7 Other
7Fixed hs21Fixed
Related Reports
Duplicate :  
Duplicate :  
Relates :  
Relates :  
Relates :  
Relates :  
Description
The following code prints invalid value in assertEquals() on solaris-sparc with -Xcomp option:

public class T {
    static void assertEquals(Object exp, Object act) {
        System.out.println(exp + " vs " + act);
    }

    static MethodHandle asList;

    static void test() {
        MethodHandle makeEmptyList = MethodHandles.constant(List.class, Arrays.asList());
        MethodHandle asList = lookup().findStatic(Arrays.class, "asList", methodType(List.class, Object[].class));
        T.asList = asList;

        MethodHandle collectingTypeHandler = lookup()
                .findStatic(lookup().lookupClass(), "collectingTypeHandler",
                            methodType(MethodHandle.class, MethodHandle.class, MethodType.class));

        MethodHandle makeAnyList = makeEmptyList.withTypeHandler(collectingTypeHandler);

        assertEquals("TEST>[two, too]<TEST", makeAnyList.invokeGeneric("two", "too").toString());
    }

    static MethodHandle collectingTypeHandler(MethodHandle base, MethodType newType) {
        return asList.asCollector(Object[].class, newType.parameterCount()).asType(newType);
    }
}

With -Xcomp, it prints:

(String,String)Object vs [two, too]

With -Xint it prints:

TEST>[two, too]<TEST vs [two, too]

Comments
EVALUATION 7009361: JSR 292 Invalid value on stack on solaris-sparc with -Xcomp Reviewed-by: kvn, twisti 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.
03-05-2011

EVALUATION http://hg.openjdk.java.net/jdk7/hotspot-comp/hotspot/rev/2e038ad0c1d0
03-05-2011

EVALUATION So I've tracked this bug down to bad frame layout during deopt for an uncommon trap that results in the locals of the deopt'ed callee overwriting the live expression stack of the caller. I think this bug may have some big implications for the deopt code since the invokedynamic machinery breaks some assumptions of deopt. My modified test case looks like this: MethodHandle asList = publicLookup() .findStatic(Test.class, "asList", methodType(List.class, Object[].class)) .asVarargsCollector(Object[].class); assertEquals(0xfffffff1, 0xfffffff3, asList.invokeGeneric().toString(), 0xfffffff7, 0xffffffff); and the bug is that the first two arguments are corrupted by the execution of the asList part. This frame is interpreted and the asList call, which is an invokevirtual, jumps through a few method handle adapters before returning a value. In particular it calls F2::invoke_F2 which uncommon traps almost immediately. The problem arises when deopt is building the interpreter frame and is deciding where to put the locals for the frame. Prior to invoke dynamic the signature of the callers callsite and the callee were the same so deopt could compute the location of the beginning of the locals area by adding callee->parameter_size() to the callers expression stack pointer. But that no longer works because of method handles. The call site may start out with one signature and end up somewhere with a different signature which is what occurs here. When it starts the invoke chain there are 3 values on stack, 0xfffffff1 and 0xfffffff3, plus asList as the argument to invokevirtual. However invoke_F2 takes three arguments and has three locals so instead of extending the callers expression stack by 2 it thinks there's already space in the caller and overwrites the expression stack with it's current locals. This particular test doesn't fail on x86 but I think that's just because the layout has a bit more slop and the way the computations is done in layout_activation put us in empty space. I think a test case where the number of locals in the uncommon trapping callee were much larger would probably fail too. I can't quite figure out how to write that though. Any simple ways to create a situation like this? So I think to fix this in a systematic way we need to change all the code that uses callee->parameter_size() to use the parameter_size of the caller method. Alternatively I guess we could stop putting the locals in the caller frame when we deopt or we could enlarge the caller by the whole num_locals amount. That's what we do on sparc when we're deoptimizing a frame and the caller is compiled.
21-04-2011