From 92dae093371fd9240c401d4cd888b6402233b627 Mon Sep 17 00:00:00 2001 From: Doug Simon Date: Fri, 30 Sep 2022 14:27:52 +0200 Subject: [PATCH] InstalledCode::invalidate(false) should not clear address field --- src/hotspot/share/jvmci/jvmciEnv.cpp | 14 ++-- .../{errors => common}/CodeInstallerTest.java | 49 +++++++++-- .../jdk/vm/ci/hotspot/CompilerToVMHelper.java | 6 +- .../InvalidateInstalledCodeTest.java | 84 ++++++++----------- .../errors/TestInvalidCompilationResult.java | 3 +- .../jvmci/errors/TestInvalidDebugInfo.java | 3 +- .../jvmci/errors/TestInvalidOopMap.java | 3 +- 7 files changed, 93 insertions(+), 69 deletions(-) rename test/hotspot/jtreg/compiler/jvmci/{errors => common}/CodeInstallerTest.java (62%) diff --git a/src/hotspot/share/jvmci/jvmciEnv.cpp b/src/hotspot/share/jvmci/jvmciEnv.cpp index f3c4fea9137..93c963b9da7 100644 --- a/src/hotspot/share/jvmci/jvmciEnv.cpp +++ b/src/hotspot/share/jvmci/jvmciEnv.cpp @@ -1551,14 +1551,18 @@ void JVMCIEnv::invalidate_nmethod_mirror(JVMCIObject mirror, bool deoptimize, JV if (!deoptimize) { // Prevent future executions of the nmethod but let current executions complete. nm->make_not_entrant(); + + // Do not clear the address field here as the Java code may still + // want to later call this method with deoptimize == true. That requires + // the address field to still be pointing at the nmethod. } else if (nm->is_alive()) { - // We want the nmethod to be deoptimized immediately. + // Deoptimize the nmethod immediately. Deoptimization::deoptimize_all_marked(nm); - } - // A HotSpotNmethod instance can only reference a single nmethod - // during its lifetime so simply clear it here. - set_InstalledCode_address(mirror, 0); + // A HotSpotNmethod instance can only reference a single nmethod + // during its lifetime so simply clear it here. + set_InstalledCode_address(mirror, 0); + } } Klass* JVMCIEnv::asKlass(JVMCIObject obj) { diff --git a/test/hotspot/jtreg/compiler/jvmci/errors/CodeInstallerTest.java b/test/hotspot/jtreg/compiler/jvmci/common/CodeInstallerTest.java similarity index 62% rename from test/hotspot/jtreg/compiler/jvmci/errors/CodeInstallerTest.java rename to test/hotspot/jtreg/compiler/jvmci/common/CodeInstallerTest.java index 43a8d3a46e3..c2f55aea39e 100644 --- a/test/hotspot/jtreg/compiler/jvmci/errors/CodeInstallerTest.java +++ b/test/hotspot/jtreg/compiler/jvmci/common/CodeInstallerTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2022, 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 @@ -21,7 +21,7 @@ * questions. */ -package compiler.jvmci.errors; +package compiler.jvmci.common; import jdk.vm.ci.code.Architecture; import jdk.vm.ci.code.CodeCacheProvider; @@ -30,6 +30,7 @@ import jdk.vm.ci.code.StackSlot; import jdk.vm.ci.code.site.DataPatch; import jdk.vm.ci.code.site.Site; +import jdk.vm.ci.code.InstalledCode; import jdk.vm.ci.hotspot.HotSpotCompiledCode; import jdk.vm.ci.hotspot.HotSpotCompiledCode.Comment; import jdk.vm.ci.hotspot.HotSpotCompiledNmethod; @@ -39,6 +40,7 @@ import jdk.vm.ci.meta.MetaAccessProvider; import jdk.vm.ci.meta.PlatformKind; import jdk.vm.ci.meta.ResolvedJavaMethod; +import jdk.vm.ci.meta.SpeculationLog; import jdk.vm.ci.runtime.JVMCI; import jdk.vm.ci.runtime.JVMCIBackend; import org.junit.Assert; @@ -74,11 +76,44 @@ protected CodeInstallerTest() { dummyMethod = (HotSpotResolvedJavaMethod) metaAccess.lookupJavaMethod(method); } - protected void installEmptyCode(Site[] sites, Assumption[] assumptions, Comment[] comments, int dataSectionAlignment, DataPatch[] dataSectionPatches, StackSlot deoptRescueSlot) { - HotSpotCompiledCode code = new HotSpotCompiledNmethod("dummyMethod", new byte[0], 0, sites, assumptions, new ResolvedJavaMethod[]{dummyMethod}, comments, new byte[8], dataSectionAlignment, - dataSectionPatches, false, 0, deoptRescueSlot, - dummyMethod, 0, 1, 0L, false); - codeCache.addCode(dummyMethod, code, null, null); + protected InstalledCode installEmptyCode(Site[] sites, + Assumption[] assumptions, + Comment[] comments, + int dataSectionAlignment, + DataPatch[] dataSectionPatches, + StackSlot deoptRescueSlot) { + ResolvedJavaMethod[] methods = {dummyMethod}; + byte[] targetCode = {0}; + int targetCodeSize = targetCode.length; + boolean isImmutablePIC = false; + int totalFrameSize = 0; + int entryBCI = 0; + int id = 1; + long compileState = 0L; + boolean hasUnsafeAccess = false; + + HotSpotCompiledCode code = + new HotSpotCompiledNmethod("dummyMethod", + targetCode, + targetCodeSize, + sites, + assumptions, + methods, + comments, + new byte[8], + dataSectionAlignment, + dataSectionPatches, + isImmutablePIC, + totalFrameSize, + deoptRescueSlot, + dummyMethod, + entryBCI, + id, + compileState, + hasUnsafeAccess); + SpeculationLog log = null; + InstalledCode installedCode = null; + return codeCache.addCode(dummyMethod, code, log, installedCode); } protected Register getRegister(PlatformKind kind, int index) { diff --git a/test/hotspot/jtreg/compiler/jvmci/common/patches/jdk.internal.vm.ci/jdk/vm/ci/hotspot/CompilerToVMHelper.java b/test/hotspot/jtreg/compiler/jvmci/common/patches/jdk.internal.vm.ci/jdk/vm/ci/hotspot/CompilerToVMHelper.java index a85a73ebe71..cf06fa5c9b1 100644 --- a/test/hotspot/jtreg/compiler/jvmci/common/patches/jdk.internal.vm.ci/jdk/vm/ci/hotspot/CompilerToVMHelper.java +++ b/test/hotspot/jtreg/compiler/jvmci/common/patches/jdk.internal.vm.ci/jdk/vm/ci/hotspot/CompilerToVMHelper.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2022, 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 @@ -242,8 +242,8 @@ public static void reprofile(HotSpotResolvedJavaMethod method) { CTVM.reprofile((HotSpotResolvedJavaMethodImpl)method); } - public static void invalidateHotSpotNmethod(HotSpotNmethod nmethodMirror) { - CTVM.invalidateHotSpotNmethod(nmethodMirror, true); + public static void invalidateHotSpotNmethod(HotSpotNmethod nmethodMirror, boolean deoptimize) { + CTVM.invalidateHotSpotNmethod(nmethodMirror, deoptimize); } public static long[] collectCounters() { diff --git a/test/hotspot/jtreg/compiler/jvmci/compilerToVM/InvalidateInstalledCodeTest.java b/test/hotspot/jtreg/compiler/jvmci/compilerToVM/InvalidateInstalledCodeTest.java index e9b6b1bf51a..2a624d3b290 100644 --- a/test/hotspot/jtreg/compiler/jvmci/compilerToVM/InvalidateInstalledCodeTest.java +++ b/test/hotspot/jtreg/compiler/jvmci/compilerToVM/InvalidateInstalledCodeTest.java @@ -27,20 +27,19 @@ * @requires vm.jvmci * @library /test/lib / * @library ../common/patches - * @ignore 8249621 - * @ignore 8163894 * @modules java.base/jdk.internal.misc * @modules java.base/jdk.internal.org.objectweb.asm * java.base/jdk.internal.org.objectweb.asm.tree * jdk.internal.vm.ci/jdk.vm.ci.hotspot * jdk.internal.vm.ci/jdk.vm.ci.code + * jdk.internal.vm.ci/jdk.vm.ci.code.site + * jdk.internal.vm.ci/jdk.vm.ci.meta * jdk.internal.vm.ci/jdk.vm.ci.runtime * * @build jdk.internal.vm.ci/jdk.vm.ci.hotspot.CompilerToVMHelper - * @build compiler.jvmci.compilerToVM.InvalidateInstalledCodeTest * @build sun.hotspot.WhiteBox * @run driver jdk.test.lib.helpers.ClassFileInstaller sun.hotspot.WhiteBox - * @run main/othervm -Xbootclasspath/a:. + * @run junit/othervm -Xbootclasspath/a:. * -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI * -XX:+UnlockExperimentalVMOptions -XX:+EnableJVMCI * compiler.jvmci.compilerToVM.InvalidateInstalledCodeTest @@ -48,75 +47,58 @@ package compiler.jvmci.compilerToVM; +import compiler.jvmci.common.CodeInstallerTest; import compiler.jvmci.common.CTVMUtilities; import jdk.test.lib.Asserts; import jdk.test.lib.Utils; -import jdk.vm.ci.code.CodeCacheProvider; -import jdk.vm.ci.code.CompilationResult; import jdk.vm.ci.code.InstalledCode; +import jdk.vm.ci.code.site.Site; +import jdk.vm.ci.code.site.DataPatch; import jdk.vm.ci.hotspot.CompilerToVMHelper; -import jdk.vm.ci.hotspot.HotSpotCompilationRequest; import jdk.vm.ci.hotspot.HotSpotJVMCIRuntime; import jdk.vm.ci.hotspot.HotSpotResolvedJavaMethod; -import sun.hotspot.code.NMethod; +import jdk.vm.ci.hotspot.HotSpotCompiledCode.Comment; +import jdk.vm.ci.hotspot.HotSpotNmethod; +import jdk.vm.ci.meta.Assumptions.Assumption; import java.util.List; +import org.junit.Test; -public class InvalidateInstalledCodeTest { - private static final CodeCacheProvider CACHE_PROVIDER - = HotSpotJVMCIRuntime.runtime().getHostJVMCIBackend() - .getCodeCache(); +public class InvalidateInstalledCodeTest extends CodeInstallerTest { - public static void main(String[] args) { - InvalidateInstalledCodeTest test - = new InvalidateInstalledCodeTest(); + @Test + public void testInvalidation() { List testCases = CompileCodeTestCase.generate(/* bci = */ 0); testCases.addAll(CompileCodeTestCase.generate(/* bci = */ -1)); - testCases.forEach(test::check); - test.checkNull(); + testCases.forEach(t -> check(t)); + checkNull(); } private void checkNull() { Utils.runAndCheckException( - () -> CompilerToVMHelper.invalidateInstalledCode(null), + () -> CompilerToVMHelper.invalidateHotSpotNmethod(null, true), NullPointerException.class); } private void check(CompileCodeTestCase testCase) { - System.out.println(testCase); - HotSpotResolvedJavaMethod javaMethod - = CTVMUtilities.getResolvedMethod(testCase.executable); - HotSpotCompilationRequest compRequest = new HotSpotCompilationRequest( - javaMethod, testCase.bci, /* jvmciEnv = */ 0L); - String name = testCase.executable.getName(); - CompilationResult compResult = new CompilationResult(name); - // to pass sanity check of default -1 - compResult.setTotalFrameSize(0); - compResult.close(); - InstalledCode installedCode = CACHE_PROVIDER.installCode( - compRequest, compResult, - new InstalledCode(name), /* speculationLog = */ null, - /* isDefault = */ false); - Asserts.assertTrue(installedCode.isValid(), testCase - + " : code is invalid even before invalidation"); + HotSpotResolvedJavaMethod javaMethod = CTVMUtilities.getResolvedMethod(testCase.executable); + HotSpotNmethod nmethod = (HotSpotNmethod) installEmptyCode(new Site[0], new Assumption[0], + new Comment[0], 8, new DataPatch[0], null); - NMethod beforeInvalidation = testCase.toNMethod(); - if (beforeInvalidation != null) { - throw new Error("TESTBUG : " + testCase + " : nmethod isn't found"); - } - // run twice to verify how it works if method is already invalidated - for (int i = 0; i < 2; ++i) { - CompilerToVMHelper.invalidateInstalledCode(installedCode); - Asserts.assertFalse(installedCode.isValid(), testCase - + " : code is valid after invalidation, i = " + i); - NMethod afterInvalidation = testCase.toNMethod(); - if (afterInvalidation != null) { - System.err.println("before: " + beforeInvalidation); - System.err.println("after: " + afterInvalidation); - throw new AssertionError(testCase - + " : method hasn't been invalidated, i = " + i); - } - } + Asserts.assertTrue(nmethod.isValid(), testCase + " : code is invalid even before invalidation"); + + Asserts.assertTrue(nmethod.isValid(), testCase + " : code is not valid, i = " + nmethod); + Asserts.assertTrue(nmethod.isAlive(), testCase + " : code is not alive, i = " + nmethod); + + // Make nmethod non-entrant but still alive + CompilerToVMHelper.invalidateHotSpotNmethod(nmethod, false); + Asserts.assertFalse(nmethod.isValid(), testCase + " : code is valid, i = " + nmethod); + Asserts.assertTrue(nmethod.isAlive(), testCase + " : code is not alive, i = " + nmethod); + + // Deoptimize the nmethod and cut the link to it from the HotSpotNmethod + CompilerToVMHelper.invalidateHotSpotNmethod(nmethod, true); + Asserts.assertFalse(nmethod.isValid(), testCase + " : code is valid, i = " + nmethod); + Asserts.assertFalse(nmethod.isAlive(), testCase + " : code is alive, i = " + nmethod); } } diff --git a/test/hotspot/jtreg/compiler/jvmci/errors/TestInvalidCompilationResult.java b/test/hotspot/jtreg/compiler/jvmci/errors/TestInvalidCompilationResult.java index 1bed3513997..40f27e65895 100644 --- a/test/hotspot/jtreg/compiler/jvmci/errors/TestInvalidCompilationResult.java +++ b/test/hotspot/jtreg/compiler/jvmci/errors/TestInvalidCompilationResult.java @@ -23,6 +23,7 @@ /** * @test + * @library / * @requires vm.jvmci * @modules jdk.internal.vm.ci/jdk.vm.ci.hotspot * jdk.internal.vm.ci/jdk.vm.ci.code @@ -30,13 +31,13 @@ * jdk.internal.vm.ci/jdk.vm.ci.meta * jdk.internal.vm.ci/jdk.vm.ci.runtime * jdk.internal.vm.ci/jdk.vm.ci.common - * @compile CodeInstallerTest.java * @run junit/othervm -da:jdk.vm.ci... -XX:+UnlockExperimentalVMOptions -XX:+EnableJVMCI * -XX:-UseJVMCICompiler compiler.jvmci.errors.TestInvalidCompilationResult */ package compiler.jvmci.errors; +import compiler.jvmci.common.CodeInstallerTest; import jdk.vm.ci.code.StackSlot; import jdk.vm.ci.code.site.ConstantReference; import jdk.vm.ci.code.site.DataPatch; diff --git a/test/hotspot/jtreg/compiler/jvmci/errors/TestInvalidDebugInfo.java b/test/hotspot/jtreg/compiler/jvmci/errors/TestInvalidDebugInfo.java index 858550e6e3b..f25422a4b2a 100644 --- a/test/hotspot/jtreg/compiler/jvmci/errors/TestInvalidDebugInfo.java +++ b/test/hotspot/jtreg/compiler/jvmci/errors/TestInvalidDebugInfo.java @@ -23,6 +23,7 @@ /** * @test + * @library / * @requires vm.jvmci * @modules jdk.internal.vm.ci/jdk.vm.ci.hotspot * jdk.internal.vm.ci/jdk.vm.ci.code @@ -30,13 +31,13 @@ * jdk.internal.vm.ci/jdk.vm.ci.meta * jdk.internal.vm.ci/jdk.vm.ci.runtime * jdk.internal.vm.ci/jdk.vm.ci.common - * @compile CodeInstallerTest.java * @run junit/othervm -da:jdk.vm.ci... -XX:+UnlockExperimentalVMOptions -XX:+EnableJVMCI * -XX:-UseJVMCICompiler compiler.jvmci.errors.TestInvalidDebugInfo */ package compiler.jvmci.errors; +import compiler.jvmci.common.CodeInstallerTest; import jdk.vm.ci.code.Architecture; import jdk.vm.ci.code.BytecodeFrame; import jdk.vm.ci.code.DebugInfo; diff --git a/test/hotspot/jtreg/compiler/jvmci/errors/TestInvalidOopMap.java b/test/hotspot/jtreg/compiler/jvmci/errors/TestInvalidOopMap.java index 9d766909eb3..9f7df442133 100644 --- a/test/hotspot/jtreg/compiler/jvmci/errors/TestInvalidOopMap.java +++ b/test/hotspot/jtreg/compiler/jvmci/errors/TestInvalidOopMap.java @@ -23,6 +23,7 @@ /** * @test + * @library / * @requires vm.jvmci * @modules jdk.internal.vm.ci/jdk.vm.ci.hotspot * jdk.internal.vm.ci/jdk.vm.ci.code @@ -30,13 +31,13 @@ * jdk.internal.vm.ci/jdk.vm.ci.meta * jdk.internal.vm.ci/jdk.vm.ci.runtime * jdk.internal.vm.ci/jdk.vm.ci.common - * @compile CodeInstallerTest.java * @run junit/othervm -da:jdk.vm.ci... -XX:+UnlockExperimentalVMOptions -XX:+EnableJVMCI * -XX:-UseJVMCICompiler compiler.jvmci.errors.TestInvalidOopMap */ package compiler.jvmci.errors; +import compiler.jvmci.common.CodeInstallerTest; import jdk.vm.ci.code.BytecodePosition; import jdk.vm.ci.code.DebugInfo; import jdk.vm.ci.code.Location;