8158850: [JVMCI] be more precise when enforcing OopMapValue encoding limitations
Reviewed-by: kvn
--- 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;