8294676: [JVMCI] InstalledCode.deoptimize(false) should not touch address field

Doug Simon · Doug Simon · commit b8b9b97a1a3e · 2022-10-01T11:20:46.000Z
Reviewed-by: never
diff --git a/src/hotspot/share/jvmci/jvmciEnv.cpp b/src/hotspot/share/jvmci/jvmciEnv.cpp
@@ -1547,14 +1547,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();
-} else {
-    // We want the nmethod to be deoptimized immediately.
+
+    // 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 {
+    // 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/common/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
@@ -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
@@ -243,8 +243,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
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2021, 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
@@ -27,96 +27,79 @@
  * @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 jdk.test.whitebox.WhiteBox
+ *        jdk.test.whitebox.WhiteBox jdk.test.whitebox.parser.DiagnosticCommand
  * @run driver jdk.test.lib.helpers.ClassFileInstaller jdk.test.whitebox.WhiteBox
- * @run main/othervm -Xbootclasspath/a:.
+ *                                jdk.test.whitebox.parser.DiagnosticCommand
+ * @run junit/othervm -Xbootclasspath/a:.
  *                   -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI
  *                   -XX:+UnlockExperimentalVMOptions -XX:+EnableJVMCI
  *                   compiler.jvmci.compilerToVM.InvalidateInstalledCodeTest
  */
 
 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 jdk.test.whitebox.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<CompileCodeTestCase> 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
@@ -23,20 +23,21 @@
 
 /**
  * @test
+ * @library /
  * @requires vm.jvmci
  * @modules 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
  *          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
@@ -23,20 +23,21 @@
 
 /**
  * @test
+ * @library /
  * @requires vm.jvmci
  * @modules 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
  *          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
@@ -23,20 +23,21 @@
 
 /**
  * @test
+ * @library /
  * @requires vm.jvmci
  * @modules 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
  *          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;

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`/*`
`2`		`- * Copyright (c) 2015, 2019, Oracle and/or its affiliates. All rights reserved.`
	`2`	`+ * Copyright (c) 2015, 2022, Oracle and/or its affiliates. All rights reserved.`
`3`	`3`	`* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.`
`4`	`4`	`*`
`5`	`5`	`* This code is free software; you can redistribute it and/or modify it`
`@@ -243,8 +243,8 @@ public static void reprofile(HotSpotResolvedJavaMethod method) {`
`243`	`243`	`CTVM.reprofile((HotSpotResolvedJavaMethodImpl)method);`
`244`	`244`	`}`
`245`	`245`
`246`		`- public static void invalidateHotSpotNmethod(HotSpotNmethod nmethodMirror) {`
`247`		`- CTVM.invalidateHotSpotNmethod(nmethodMirror, true);`
	`246`	`+ public static void invalidateHotSpotNmethod(HotSpotNmethod nmethodMirror, boolean deoptimize) {`
	`247`	`+ CTVM.invalidateHotSpotNmethod(nmethodMirror, deoptimize);`
`248`	`248`	`}`
`249`	`249`
`250`	`250`	`public static long[] collectCounters() {`