8158850: [JVMCI] be more precise when enforcing OopMapValue encoding limitations
authornever
Wed, 22 Jun 2016 22:39:32 +0000
changeset 39441 7464b1552bf7
parent 39436 f07320a4e634
child 39442 ca66cfb4e62b
8158850: [JVMCI] be more precise when enforcing OopMapValue encoding limitations Reviewed-by: kvn
hotspot/src/share/vm/jvmci/jvmciCodeInstaller.cpp
hotspot/src/share/vm/jvmci/jvmciCompilerToVM.cpp
hotspot/src/share/vm/jvmci/jvmciCompilerToVM.hpp
hotspot/src/share/vm/jvmci/vmStructs_jvmci.cpp
hotspot/test/compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/CodeInstallationTest.java
hotspot/test/compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/DebugInfoTest.java
hotspot/test/compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/MaxOopMapStackOffsetTest.java
hotspot/test/compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/TestAssembler.java
hotspot/test/compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/TestHotSpotVMConfig.java
hotspot/test/compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/sparc/SPARCTestAssembler.java
--- a/hotspot/src/share/vm/jvmci/jvmciCodeInstaller.cpp	Wed Jun 22 21:13:52 2016 +0000
+++ b/hotspot/src/share/vm/jvmci/jvmciCodeInstaller.cpp	Wed Jun 22 22:39:32 2016 +0000
@@ -91,7 +91,19 @@
   } else {
     // stack slot
     if (offset % 4 == 0) {
-      return VMRegImpl::stack2reg(offset / 4);
+      VMReg vmReg = VMRegImpl::stack2reg(offset / 4);
+      if (!OopMapValue::legal_vm_reg_name(vmReg)) {
+        // This restriction only applies to VMRegs that are used in OopMap but
+        // since that's the only use of VMRegs it's simplest to put this test
+        // here.  This test should also be equivalent legal_vm_reg_name but JVMCI
+        // clients can use max_oop_map_stack_stack_offset to detect this problem
+        // directly.  The asserts just ensure that the tests are in agreement.
+        assert(offset > CompilerToVM::Data::max_oop_map_stack_offset(), "illegal VMReg");
+        JVMCI_ERROR_NULL("stack offset %d is too large to be encoded in OopMap (max %d)",
+                         offset, CompilerToVM::Data::max_oop_map_stack_offset());
+      }
+      assert(OopMapValue::legal_vm_reg_name(vmReg), "illegal VMReg");
+      return vmReg;
     } else {
       JVMCI_ERROR_NULL("unaligned stack offset %d in oop map", offset);
     }
--- a/hotspot/src/share/vm/jvmci/jvmciCompilerToVM.cpp	Wed Jun 22 21:13:52 2016 +0000
+++ b/hotspot/src/share/vm/jvmci/jvmciCompilerToVM.cpp	Wed Jun 22 22:39:32 2016 +0000
@@ -113,6 +113,7 @@
 bool       CompilerToVM::Data::_supports_inline_contig_alloc;
 HeapWord** CompilerToVM::Data::_heap_end_addr;
 HeapWord** CompilerToVM::Data::_heap_top_addr;
+int CompilerToVM::Data::_max_oop_map_stack_offset;
 
 jbyte* CompilerToVM::Data::cardtable_start_address;
 int CompilerToVM::Data::cardtable_shift;
@@ -154,6 +155,11 @@
   _heap_end_addr = _supports_inline_contig_alloc ? Universe::heap()->end_addr() : (HeapWord**) -1;
   _heap_top_addr = _supports_inline_contig_alloc ? Universe::heap()->top_addr() : (HeapWord**) -1;
 
+  _max_oop_map_stack_offset = (OopMapValue::register_mask - VMRegImpl::stack2reg(0)->value()) * VMRegImpl::stack_slot_size;
+  int max_oop_map_stack_index = _max_oop_map_stack_offset / VMRegImpl::stack_slot_size;
+  assert(OopMapValue::legal_vm_reg_name(VMRegImpl::stack2reg(max_oop_map_stack_index)), "should be valid");
+  assert(!OopMapValue::legal_vm_reg_name(VMRegImpl::stack2reg(max_oop_map_stack_index + 1)), "should be invalid");
+
   BarrierSet* bs = Universe::heap()->barrier_set();
   switch (bs->kind()) {
   case BarrierSet::CardTableModRef:
--- a/hotspot/src/share/vm/jvmci/jvmciCompilerToVM.hpp	Wed Jun 22 21:13:52 2016 +0000
+++ b/hotspot/src/share/vm/jvmci/jvmciCompilerToVM.hpp	Wed Jun 22 22:39:32 2016 +0000
@@ -59,6 +59,7 @@
     static bool _supports_inline_contig_alloc;
     static HeapWord** _heap_end_addr;
     static HeapWord** _heap_top_addr;
+    static int _max_oop_map_stack_offset;
 
     static jbyte* cardtable_start_address;
     static int cardtable_shift;
@@ -75,6 +76,11 @@
 
    public:
     static void initialize();
+
+    static int max_oop_map_stack_offset() {
+      assert(_max_oop_map_stack_offset > 0, "must be initialized");
+      return Data::_max_oop_map_stack_offset;
+    }
   };
 
  public:
--- a/hotspot/src/share/vm/jvmci/vmStructs_jvmci.cpp	Wed Jun 22 21:13:52 2016 +0000
+++ b/hotspot/src/share/vm/jvmci/vmStructs_jvmci.cpp	Wed Jun 22 22:39:32 2016 +0000
@@ -71,6 +71,8 @@
   static_field(CompilerToVM::Data,             _heap_end_addr,                         HeapWord**)                                   \
   static_field(CompilerToVM::Data,             _heap_top_addr,                         HeapWord**)                                   \
                                                                                                                                      \
+  static_field(CompilerToVM::Data,             _max_oop_map_stack_offset,              int)                                          \
+                                                                                                                                     \
   static_field(CompilerToVM::Data,             cardtable_start_address,                jbyte*)                                       \
   static_field(CompilerToVM::Data,             cardtable_shift,                        int)                                          \
                                                                                                                                      \
--- a/hotspot/test/compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/CodeInstallationTest.java	Wed Jun 22 21:13:52 2016 +0000
+++ b/hotspot/test/compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/CodeInstallationTest.java	Wed Jun 22 22:39:32 2016 +0000
@@ -89,17 +89,17 @@
     }
 
     protected void test(TestCompiler compiler, Method method, Object... args) {
-        HotSpotResolvedJavaMethod resolvedMethod = (HotSpotResolvedJavaMethod) metaAccess.lookupJavaMethod(method);
-        TestAssembler asm = createAssembler();
+        try {
+            HotSpotResolvedJavaMethod resolvedMethod = (HotSpotResolvedJavaMethod) metaAccess.lookupJavaMethod(method);
+            TestAssembler asm = createAssembler();
 
-        asm.emitPrologue();
-        compiler.compile(asm);
-        asm.emitEpilogue();
+            asm.emitPrologue();
+            compiler.compile(asm);
+            asm.emitEpilogue();
 
-        HotSpotCompiledCode code = asm.finish(resolvedMethod);
-        InstalledCode installed = codeCache.addCode(resolvedMethod, code, null, null);
+            HotSpotCompiledCode code = asm.finish(resolvedMethod);
+            InstalledCode installed = codeCache.addCode(resolvedMethod, code, null, null);
 
-        try {
             Object expected = method.invoke(null, args);
             Object actual = installed.executeVarargs(args);
             Assert.assertEquals(expected, actual);
--- a/hotspot/test/compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/DebugInfoTest.java	Wed Jun 22 21:13:52 2016 +0000
+++ b/hotspot/test/compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/DebugInfoTest.java	Wed Jun 22 22:39:32 2016 +0000
@@ -44,6 +44,10 @@
     }
 
     protected void test(DebugInfoCompiler compiler, Method method, int bci, JavaKind... slotKinds) {
+        test(compiler, method, bci, new Location[0], new Location[0], new int[0], slotKinds);
+    }
+
+    protected void test(DebugInfoCompiler compiler, Method method, int bci, Location[] objects, Location[] derivedBase, int[] sizeInBytes, JavaKind... slotKinds) {
         ResolvedJavaMethod resolvedMethod = metaAccess.lookupJavaMethod(method);
 
         int numLocals = resolvedMethod.getMaxLocals();
@@ -54,7 +58,7 @@
 
             BytecodeFrame frame = new BytecodeFrame(null, resolvedMethod, bci, false, false, values, slotKinds, numLocals, numStack, 0);
             DebugInfo info = new DebugInfo(frame, vobjs);
-            info.setReferenceMap(new HotSpotReferenceMap(new Location[0], new Location[0], new int[0], 8));
+            info.setReferenceMap(new HotSpotReferenceMap(objects, derivedBase, sizeInBytes, 8));
 
             asm.emitTrap(info);
         }, method);
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/MaxOopMapStackOffsetTest.java	Wed Jun 22 22:39:32 2016 +0000
@@ -0,0 +1,90 @@
+/*
+ * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * @test
+ * @requires (os.simpleArch == "x64" | os.simpleArch == "sparcv9") & os.arch != "aarch64"
+ * @library /
+ * @modules jdk.vm.ci/jdk.vm.ci.hotspot
+ *          jdk.vm.ci/jdk.vm.ci.meta
+ *          jdk.vm.ci/jdk.vm.ci.code
+ *          jdk.vm.ci/jdk.vm.ci.code.site
+ *          jdk.vm.ci/jdk.vm.ci.common
+ *          jdk.vm.ci/jdk.vm.ci.runtime
+ *          jdk.vm.ci/jdk.vm.ci.amd64
+ *          jdk.vm.ci/jdk.vm.ci.sparc
+ * @compile CodeInstallationTest.java DebugInfoTest.java TestAssembler.java TestHotSpotVMConfig.java amd64/AMD64TestAssembler.java sparc/SPARCTestAssembler.java
+ * @run junit/othervm -XX:+UnlockExperimentalVMOptions -XX:+EnableJVMCI jdk.vm.ci.code.test.MaxOopMapStackOffsetTest
+ */
+
+package jdk.vm.ci.code.test;
+
+import org.junit.Test;
+
+import jdk.vm.ci.code.Location;
+import jdk.vm.ci.code.Register;
+import jdk.vm.ci.common.JVMCIError;
+import jdk.vm.ci.meta.JavaConstant;
+import jdk.vm.ci.meta.JavaKind;
+
+public class MaxOopMapStackOffsetTest extends DebugInfoTest {
+
+    public static int pass() {
+        return 42;
+    }
+
+    public static int fail() {
+        return 42;
+    }
+
+    private void test(String name, int offset) {
+        Location location = Location.stack(offset);
+               DebugInfoCompiler compiler = (asm, values) -> {
+            asm.growFrame(offset);
+            Register v = asm.emitLoadInt(0);
+            asm.emitIntToStack(v);
+            values[0] = JavaConstant.forInt(42);
+            return null;
+        };
+        test(compiler, getMethod(name), 2, new Location[]{location}, new Location[1], new int[]{4}, JavaKind.Int);
+    }
+
+    private int maxOffset() {
+        return config.maxOopMapStackOffset;
+    }
+
+    private int wordSize() {
+        return config.heapWordSize;
+    }
+
+    @Test(expected = JVMCIError.class)
+    public void failTooLargeOffset() {
+        // This should throw a JVMCIError during installation because the offset is too large.
+        test("fail", maxOffset() + wordSize());
+    }
+
+    @Test
+    public void passWithLargeOffset() {
+        test("pass", maxOffset());
+    }
+}
--- a/hotspot/test/compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/TestAssembler.java	Wed Jun 22 21:13:52 2016 +0000
+++ b/hotspot/test/compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/TestAssembler.java	Wed Jun 22 22:39:32 2016 +0000
@@ -233,7 +233,12 @@
     }
 
     protected StackSlot newStackSlot(PlatformKind kind) {
-        curStackSlot += kind.getSizeInBytes();
+        growFrame(kind.getSizeInBytes());
+        return StackSlot.get(new TestValueKind(kind), -curStackSlot, true);
+    }
+
+    protected void growFrame(int sizeInBytes) {
+        curStackSlot += sizeInBytes;
         if (curStackSlot > frameSize) {
             int newFrameSize = curStackSlot;
             if (newFrameSize % stackAlignment != 0) {
@@ -242,7 +247,6 @@
             emitGrowStack(newFrameSize - frameSize);
             frameSize = newFrameSize;
         }
-        return StackSlot.get(new TestValueKind(kind), -curStackSlot, true);
     }
 
     protected void setDeoptRescueSlot(StackSlot deoptRescue) {
--- a/hotspot/test/compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/TestHotSpotVMConfig.java	Wed Jun 22 21:13:52 2016 +0000
+++ b/hotspot/test/compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/TestHotSpotVMConfig.java	Wed Jun 22 22:39:32 2016 +0000
@@ -44,4 +44,7 @@
 
     public final int MARKID_DEOPT_HANDLER_ENTRY = getConstant("CodeInstaller::DEOPT_HANDLER_ENTRY", Integer.class);
     public final long handleDeoptStub = getFieldValue("CompilerToVM::Data::SharedRuntime_deopt_blob_unpack", Long.class, "address");
+
+    public final int maxOopMapStackOffset = getFieldValue("CompilerToVM::Data::_max_oop_map_stack_offset", Integer.class, "int");
+    public final int heapWordSize = getConstant("HeapWordSize", Integer.class);
 }
--- a/hotspot/test/compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/sparc/SPARCTestAssembler.java	Wed Jun 22 21:13:52 2016 +0000
+++ b/hotspot/test/compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/sparc/SPARCTestAssembler.java	Wed Jun 22 22:39:32 2016 +0000
@@ -50,6 +50,7 @@
     }
 
     private void emitOp2(Register rd, int op2, int imm22) {
+        assert isSimm(imm22, 22);
         code.emitInt((0b00 << 30) | (rd.encoding << 25) | (op2 << 22) | imm22);
     }
 
@@ -58,6 +59,7 @@
     }
 
     private void emitOp3(int op, Register rd, int op3, Register rs1, int simm13) {
+        assert isSimm(simm13, 13);
         code.emitInt((op << 30) | (rd.encoding << 25) | (op3 << 19) | (rs1.encoding << 14) | (1 << 13) | (simm13 & MASK13));
     }
 
@@ -65,6 +67,27 @@
         code.emitInt(1 << 24);
     }
 
+    /**
+     * Minimum value for signed immediate ranges.
+     */
+    public static long minSimm(long nbits) {
+        return -(1L << (nbits - 1));
+    }
+
+    /**
+     * Maximum value for signed immediate ranges.
+     */
+    public static long maxSimm(long nbits) {
+        return (1L << (nbits - 1)) - 1;
+    }
+
+    /**
+     * Test if imm is within signed immediate range for nbits.
+     */
+    public static boolean isSimm(long imm, int nbits) {
+        return minSimm(nbits) <= imm && imm <= maxSimm(nbits);
+    }
+
     @Override
     public void emitPrologue() {
         // SAVE sp, -128, sp
@@ -87,7 +110,13 @@
 
     @Override
     public void emitGrowStack(int size) {
-        emitOp3(0b10, SPARC.sp, 0b000100, SPARC.sp, size); // SUB sp, size, sp
+        frameSize += size;
+        if (isSimm(size, 13)) {
+            emitOp3(0b10, SPARC.sp, 0b000100, SPARC.sp, size); // SUB sp, size, sp
+        } else {
+            Register r = emitLoadInt(size);
+            emitOp3(0b10, SPARC.sp, 0b000100, SPARC.sp, r); // SUB sp, size, sp
+        }
     }
 
     @Override
@@ -103,6 +132,11 @@
     @Override
     public Register emitLoadInt(int c) {
         Register ret = newRegister();
+        loadIntToRegister(c, ret);
+        return ret;
+    }
+
+    private void loadIntToRegister(int c, Register ret) {
         int hi = c >>> 10;
         int lo = c & ((1 << 10) - 1);
         if (hi == 0) {
@@ -113,19 +147,28 @@
                 emitOp3(0b10, ret, 0b000010, ret, lo);  // OR ret, lo, ret
             }
         }
-        return ret;
     }
 
     @Override
     public Register emitLoadLong(long c) {
+        Register ret = newRegister();
+        emitLoadLongToRegister(c, ret);
+        return ret;
+    }
+
+    private void loadLongToRegister(long c, Register ret) {
+        DataSectionReference ref = new DataSectionReference();
+        data.align(8);
+        ref.setOffset(data.position());
+        data.emitLong(c);
+        emitLoadPointerToRegister(ref, ret);
+    }
+
+    public void emitLoadLongToRegister(long c, Register r) {
         if ((c & 0xFFFF_FFFFL) == c) {
-            return emitLoadInt((int) c);
+            loadIntToRegister((int) c, r);
         } else {
-            DataSectionReference ref = new DataSectionReference();
-            data.align(8);
-            ref.setOffset(data.position());
-            data.emitLong(c);
-            return emitLoadPointer(ref);
+            loadLongToRegister(c, r);
         }
     }
 
@@ -168,10 +211,14 @@
     @Override
     public Register emitLoadPointer(DataSectionReference ref) {
         Register ret = newRegister();
+        emitLoadPointerToRegister(ref, ret);
+        return ret;
+    }
+
+    private void emitLoadPointerToRegister(DataSectionReference ref, Register ret) {
         recordDataPatchInCode(ref);
         emitPatchableSethi(ret, true);
         emitOp3(0b11, ret, 0b001011, ret, 0); // LDX [ret+0], ret
-        return ret;
     }
 
     @Override
@@ -194,15 +241,15 @@
     public StackSlot emitIntToStack(Register a) {
         StackSlot ret = newStackSlot(SPARCKind.WORD);
         // STW a, [fp+offset]
-        emitOp3(0b11, a, 0b000100, SPARC.fp, ret.getRawOffset() + SPARC.STACK_BIAS);
+        emitStore(0b000100, a, ret);
         return ret;
     }
 
     @Override
     public StackSlot emitLongToStack(Register a) {
         StackSlot ret = newStackSlot(SPARCKind.XWORD);
-        // STX a, [fp+offset]
-        emitOp3(0b11, a, 0b001110, SPARC.fp, ret.getRawOffset() + SPARC.STACK_BIAS);
+        // STX a, [sp+offset]
+        emitStore(0b001110, a, ret);
         return ret;
     }
 
@@ -210,7 +257,7 @@
     public StackSlot emitFloatToStack(Register a) {
         StackSlot ret = newStackSlot(SPARCKind.SINGLE);
         // STF a, [fp+offset]
-        emitOp3(0b11, a, 0b100100, SPARC.fp, ret.getRawOffset() + SPARC.STACK_BIAS);
+        emitStore(0b100100, a, ret);
         return ret;
     }
 
@@ -218,7 +265,7 @@
     public StackSlot emitPointerToStack(Register a) {
         StackSlot ret = newStackSlot(SPARCKind.XWORD);
         // STX a, [fp+offset]
-        emitOp3(0b11, a, 0b001110, SPARC.fp, ret.getRawOffset() + SPARC.STACK_BIAS);
+        emitStore(0b001110, a, ret);
         return ret;
     }
 
@@ -226,10 +273,24 @@
     public StackSlot emitNarrowPointerToStack(Register a) {
         StackSlot ret = newStackSlot(SPARCKind.WORD);
         // STW a, [fp+offset]
-        emitOp3(0b11, a, 0b000100, SPARC.fp, ret.getRawOffset() + SPARC.STACK_BIAS);
+        emitStore(0b000100, a, ret);
         return ret;
     }
 
+    private void emitStore(int op3, Register a, StackSlot ret) {
+        int offset = ret.getRawOffset() + SPARC.STACK_BIAS;
+        if (isSimm(offset, 13)) {
+            // op3 a, [sp+offset]
+            emitOp3(0b11, a, op3, SPARC.fp, offset);
+        } else {
+            assert a != SPARC.g3;
+            Register r = SPARC.g3;
+            loadLongToRegister(offset, r);
+            // op3 a, [sp+g3]
+            emitOp3(0b11, a, op3, SPARC.fp, r);
+        }
+    }
+
     @Override
     public Register emitUncompressPointer(Register compressed, long base, int shift) {
         Register ret;