# HG changeset patch # User never # Date 1466635172 0 # Node ID 7464b1552bf7176564b42311d8f0ad8d6f2b054b # Parent f07320a4e6348324b2841746b2d428860baed069 8158850: [JVMCI] be more precise when enforcing OopMapValue encoding limitations Reviewed-by: kvn diff -r f07320a4e634 -r 7464b1552bf7 hotspot/src/share/vm/jvmci/jvmciCodeInstaller.cpp --- 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); } diff -r f07320a4e634 -r 7464b1552bf7 hotspot/src/share/vm/jvmci/jvmciCompilerToVM.cpp --- 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: diff -r f07320a4e634 -r 7464b1552bf7 hotspot/src/share/vm/jvmci/jvmciCompilerToVM.hpp --- 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: diff -r f07320a4e634 -r 7464b1552bf7 hotspot/src/share/vm/jvmci/vmStructs_jvmci.cpp --- 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) \ \ diff -r f07320a4e634 -r 7464b1552bf7 hotspot/test/compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/CodeInstallationTest.java --- 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); diff -r f07320a4e634 -r 7464b1552bf7 hotspot/test/compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/DebugInfoTest.java --- 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); diff -r f07320a4e634 -r 7464b1552bf7 hotspot/test/compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/MaxOopMapStackOffsetTest.java --- /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()); + } +} diff -r f07320a4e634 -r 7464b1552bf7 hotspot/test/compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/TestAssembler.java --- 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) { diff -r f07320a4e634 -r 7464b1552bf7 hotspot/test/compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/TestHotSpotVMConfig.java --- 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); } diff -r f07320a4e634 -r 7464b1552bf7 hotspot/test/compiler/jvmci/jdk.vm.ci.code.test/src/jdk/vm/ci/code/test/sparc/SPARCTestAssembler.java --- 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;