I discovered a problem during deoptimisation at a safepoint which
leads to a SIGBUS on 64bit-SPARC. The problem was introduced by the
change "6420645: Create a vm that uses compressed oops for up to 32gb
heapsizes" which has been submitted by you. The problem is easily
reproducible with the attached test program. Just run:

java -d64 -server -showversion -Xcomp -Xbatch
"-XX:CompileCommand=compileonly DeoptTest
deopt_compiledframe_at_safepoint" -XX:+PrintCompilation DeoptTest

and you will get a VM crash like:

CompilerOracle: compileonly DeoptTest.deopt_compiledframe_at_safepoint
java version "1.6.0_14"
Java(TM) SE Runtime Environment (build 1.6.0_14-b08)
Java HotSpot(TM) 64-Bit Server VM (build 14.0-b16, compiled mode)

  1 b DeoptTest::deopt_compiledframe_at_safepoint (220 bytes)
  1 made not entrant DeoptTest::deopt_compiledframe_at_safepoint (220 bytes)
#
# A fatal error has been detected by the Java Runtime Environment:
#
# SIGBUS (0xa) at pc=0xffffffff7e37514c, pid=9314, tid=15
#
# JRE version: 6.0_14-b08
# Java VM: Java HotSpot(TM) 64-Bit Server VM (14.0-b16 compiled mode
solaris-sparc )
# Problematic frame:
# V [libjvm.so+0x77514c]

As noticed before, the error is a regression of change 6420645. It
doesn't happen with earlier versions of the HotSpot. For example 6u13
with HS 11 runs the test just fine:

CompilerOracle: compileonly DeoptTest.deopt_compiledframe_at_safepoint
java version "1.6.0_13"
Java(TM) SE Runtime Environment (build 1.6.0_13-b03)
Java HotSpot(TM) 64-Bit Server VM (build 11.3-b02, compiled mode)

  1 b DeoptTest::deopt_compiledframe_at_safepoint (220 bytes)
  1 made not entrant DeoptTest::deopt_compiledframe_at_safepoint (220 bytes)
OK


Notice that the problem is still present in the HS head revsion. I've
tried with 7-ea b70:

java version "1.7.0-ea"
Java(TM) SE Runtime Environment (build 1.7.0-ea-b70)
Java HotSpot(TM) 64-Bit Server VM (build 16.0-b07, compiled mode)

  1 b DeoptTest::deopt_compiledframe_at_safepoint (220 bytes)
  1 made not entrant DeoptTest::deopt_compiledframe_at_safepoint (220 bytes)
#
# A fatal error has been detected by the Java Runtime Environment:
#
# SIGBUS (0xa) at pc=0xffffffff7e38c7fc, pid=10714, tid=15
#
# JRE version: 7.0-b70
# Java VM: Java HotSpot(TM) 64-Bit Server VM (16.0-b07 compiled mode
solaris-sparc )
# Problematic frame:
# V [libjvm.so+0x78c7fc]


The problem is caused be the following changes in frame.cpp:


--- a/src/share/vm/runtime/frame.cpp Sat Dec 01 00:00:00 2007 +0000
+++ b/src/share/vm/runtime/frame.cpp Sun Apr 13 17:43:42 2008 -0400
@@ -1153,9 +1153,8 @@ oop* frame::oopmapreg_to_location(VMReg
     // If it is passed in a register, it got spilled in the stub frame.
     return (oop *)reg_map->location(reg);
   } else {
- int sp_offset_in_stack_slots = reg->reg2stack();
- int sp_offset = sp_offset_in_stack_slots >> (LogBytesPerWord -
LogBytesPerInt);
- return (oop *)&unextended_sp()[sp_offset];
+ int sp_offset_in_bytes = reg->reg2stack() * VMRegImpl::stack_slot_size;
+ return (oop*)(((address)unextended_sp()) + sp_offset_in_bytes);
   }
 }

and the fact that reg->reg2stack() returns odd values for float
registers >= F32. This finally leads to a BUS error due to an
unaligned double read when the location of the register is accessed
through the reg_map during deoptimisation in
StackValue::create_stack_value(). In the old implementation, this was
hidden by the right shift in frame::oopmapreg_to_location() which
mapped F32 and F33 to the same stack offset.

The problem can be easily solved by switching back to the old
implementation of frame::oopmapreg_to_location(), but I assume there
was a rational behind the change and that the new implementation is
probably necessary for compressed oops (at least that's what the whole
change was all about). So I dug a little further and found that in my
opinion the root cause of the whole problem is the strange numbering
of the 16 upper double registers in sparc.ad. They are defined as
follows:

reg_def R_D32x(SOC, SOC, Op_RegD,255, F32->as_VMReg());
reg_def R_D32 (SOC, SOC, Op_RegD, 1, F32->as_VMReg()->next());
reg_def R_D34x(SOC, SOC, Op_RegD,255, F34->as_VMReg());
reg_def R_D34 (SOC, SOC, Op_RegD, 3, F34->as_VMReg()->next());
...
reg_def R_D62x(SOC, SOC, Op_RegD,255, F62->as_VMReg());
reg_def R_D62 (SOC, SOC, Op_RegD, 31, F62->as_VMReg()->next());

This maps the invalid half (R_D32x, R_D34x, ..) of the double
registers F32-F62 to even VMReg numbers (96, 98, ..) and the valid
part (R_D32, R_D34, ..) to odd VMReg numbers (97, 99, ..). Later on,
when the locals array for the safepoint is constructed in
Compile::FillLocArray(), the call to OptoReg::as_VMReg(regnum) for a
valid, even double register >= F32 (e.g. 96) returns the invalid, odd
part (e.g. 97). This odd VMReg number is than stored in the Location
part of the local and leads to the undesired behaviour in the new
implementation of frame::oopmapreg_to_location() as described before.

I don't know why this strange encoding has been chosen for the 16
upper double registers in sparc.ad, but changing it to:

reg_def R_D32x(SOC, SOC, Op_RegD,255, F32->as_VMReg()->next());
reg_def R_D32 (SOC, SOC, Op_RegD, 1, F32->as_VMReg());
reg_def R_D34x(SOC, SOC, Op_RegD,255, F34->as_VMReg()->next());
reg_def R_D34 (SOC, SOC, Op_RegD, 3, F34->as_VMReg());
...
reg_def R_D62x(SOC, SOC, Op_RegD,255, F62->as_VMReg()->next());
reg_def R_D62 (SOC, SOC, Op_RegD, 31, F62->as_VMReg());

which seems more natural to me, solved the SIGBUS issue and didn't
revealed any other problems in the tests which I run so far.

Could you please comment on the proposed solution of changing the
VMReg numbering of F32-F62 or advice a better solution if you think
that the proposed one will not work in the general case?

Thank you and best regards,
Volker

PS: while I was hunting the error, I also stumbled across the
following code in RegisterSaver::save_live_registers():

  // Save all the FP registers
  int offset = d00_offset;
  for( int i=0; i<64; i+=2 ) {
    FloatRegister f = as_FloatRegister(i);
    __ stf(FloatRegisterImpl::D, f, SP, offset+STACK_BIAS);
    map->set_callee_saved(VMRegImpl::stack2reg(offset>>2), f->as_VMReg());
    if (true) {
      map->set_callee_saved(VMRegImpl::stack2reg((offset +
sizeof(float))>>2), f->as_VMReg()->next());
    }
    offset += sizeof(double);
  }

In my opinion, this could be changed to:

  // Save all the FP registers
  int offset = d00_offset;
  for( int i=0; i<64; i+=2 ) {
    FloatRegister f = as_FloatRegister(i);
    __ stf(FloatRegisterImpl::D, f, SP, offset+STACK_BIAS);
    map->set_callee_saved(VMRegImpl::stack2reg(offset>>2), f->as_VMReg());
    if (i < 32) { // VS 2009-08-31: the 16 upper double registers
can't be used as floats anyway
      map->set_callee_saved(VMRegImpl::stack2reg((offset +
sizeof(float))>>2), f->as_VMReg()->next());
    }
    offset += sizeof(double);
  }

because the 16 upper double registers can't be used as floats anyway.
Again, I didn't found any regression in my few tests. What do you
think?



/*

This test provokes a deoptimisation at a safepoint.

It achieves this by compiling the method 'deopt_compiledframe_at_safepoint'
before its first usage (with '-Xcomp -Xbatch') at a point in time when a call
to the virtual method A::doSomething() from within
'deopt_compiledframe_at_safepoint' can be optimised to a static call because
class A has no descendants. Later, when deopt_compiledframe_at_safepoint() is
running, class B which extends A and overrides the virtual method
"doSomething()", is loaded asynchronously in another thread. This makes the
compiled code of 'deopt_compiledframe_at_safepoint' invalid and triggers a
deoptimisation of the frame where 'deopt_compiledframe_at_safepoint' is running
in a loop. The deoptimisation leads to a SIGBUS due to a regression introduced
by the change: "6420645: Create a vm that uses compressed oops for up to 32gb
heapsizes" (http://hg.openjdk.java.net/jdk7/jdk7/hotspot/rev/ba764ed4b6f2).

Run: java -d64 -server -showversion -Xcomp -Xbatch "-XX:CompileCommand=compileonly DeoptTest deopt_compiledframe_at_safepoint" -XX:+PrintCompilation DeoptTest

Author: Volker H. Simonis

*/


class A {
  public int doSomething() {
    return 0;
  }
}

class B extends A {
  public B() {}
  // override 'A::doSomething()'
  public int doSomething() {
    return 1;
  }
}

class G {
  public static volatile A a = new A();

  // Change 'a' to point to a 'B' object
  public static void setAtoB() {
    try {
      a = (A) ClassLoader.
        getSystemClassLoader().
        loadClass("B").
        getConstructor(new Class[] {}).
        newInstance(new Object[] {});
    }
    catch (Exception e) {
      System.out.println(e);
    }
  }
}

public class DeoptTest {
  
  public static volatile boolean is_in_loop = false;
  public static volatile boolean stop_while_loop = false;

  public static double deopt_compiledframe_at_safepoint() {
    // This will be an optimised static call to A::doSomething() until we load "B"
    int i = G.a.doSomething();

    // Need more than 16 'double' locals in this frame
    double local1 = 1;
    double local2 = 2;
    double local3 = 3;
    double local4 = 4;
    double local5 = 5;
    double local6 = 6;
    double local7 = 7;
    double local8 = 8;

    long k = 0;
    // Once we load "B", this method will be made 'not entrant' and deoptimised
    // at the safepoint which is at the end of this loop.
    while (!stop_while_loop) {
      if (k == 1) local1 += i;
      if (k == 2) local2 += i;
      if (k == 3) local3 += i;
      if (k == 4) local4 += i;
      if (k == 5) local5 += i;
      if (k == 6) local6 += i;
      if (k == 7) local7 += i;
      if (k == 8) local8 += i;

      // Tell the world that we're now running wild in the loop
      if (k++ == 20000) is_in_loop = true;
    }

    return
      local1 + local2 + local3 + local4 +
      local5 + local6 + local7 + local8 + i;
  }

  public static void main(String[] args) {

    // Just to resolve G before we compile deopt_compiledframe_at_safepoint()
    G g = new G();

    new Thread() {
      public void run() {
        // Run deopt_compiledframe_at_safepoint() in another thread..
        double retVal = deopt_compiledframe_at_safepoint();
        System.out.println(retVal == 36 ? "OK" : "ERROR : " + retVal);
      }
    }.start();

    while (!is_in_loop) { /* wait until the loop is running */ }

    // And load class 'B' asynchronously
    G.setAtoB();

    // Now stop the loop
    stop_while_loop = true;
  }
}