8356318: Unexpected VerifyError in AOT training run

iklam · iklam · commit 40f696dbe49f · 2025-05-07T17:36:37.000Z
Reviewed-by: shade, kvn
diff --git a/src/hotspot/share/classfile/verifier.cpp b/src/hotspot/share/classfile/verifier.cpp
@@ -223,12 +223,9 @@ bool Verifier::verify(InstanceKlass* klass, bool should_verify_class, TRAPS) {
     split_verifier.verify_class(THREAD);
     exception_name = split_verifier.result();
 
-    // If dumping static archive then don't fall back to the old verifier on
-    // verification failure. If a class fails verification with the split verifier,
-    // it might fail the CDS runtime verifier constraint check. In that case, we
-    // don't want to share the class. We only archive classes that pass the split
-    // verifier.
-    bool can_failover = !CDSConfig::is_dumping_static_archive() &&
+    // If dumping {classic, final} static archive, don't bother to run the old verifier, as
+    // the class will be excluded from the archive anyway.
+    bool can_failover = !(CDSConfig::is_dumping_classic_static_archive() || CDSConfig::is_dumping_final_static_archive()) &&
       klass->major_version() < NOFAILOVER_MAJOR_VERSION;
 
     if (can_failover && !HAS_PENDING_EXCEPTION &&  // Split verifier doesn't set PENDING_EXCEPTION for failure
@@ -237,9 +234,10 @@ bool Verifier::verify(InstanceKlass* klass, bool should_verify_class, TRAPS) {
       log_info(verification)("Fail over class verification to old verifier for: %s", klass->external_name());
       log_info(class, init)("Fail over class verification to old verifier for: %s", klass->external_name());
 #if INCLUDE_CDS
-      // Exclude any classes that fail over during dynamic dumping
-      if (CDSConfig::is_dumping_dynamic_archive()) {
-        SystemDictionaryShared::warn_excluded(klass, "Failed over class verification while dynamic dumping");
+      // Exclude any classes that are verified with the old verifier, as the old verifier
+      // doesn't call SystemDictionaryShared::add_verification_constraint()
+      if (CDSConfig::is_dumping_archive()) {
+        SystemDictionaryShared::warn_excluded(klass, "Verified with old verifier");
         SystemDictionaryShared::set_excluded(klass);
       }
 #endif
diff --git a/test/hotspot/jtreg/runtime/cds/appcds/aotCache/VerifierFailOver.java b/test/hotspot/jtreg/runtime/cds/appcds/aotCache/VerifierFailOver.java
@@ -0,0 +1,65 @@
+/*
+ * Copyright (c) 2025, 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
+ * @summary Sanity test for AOTCache
+ * @requires vm.cds.supports.aot.class.linking
+ * @comment work around JDK-8345635
+ * @requires !vm.jvmci.enabled
+ * @library /test/lib
+ * @build VerifierFailOver_Helper
+ * @build VerifierFailOver
+ * @run driver jdk.test.lib.helpers.ClassFileInstaller -jar app.jar VerifierFailOverApp VerifierFailOver_Helper
+ * @run driver VerifierFailOver
+ */
+
+import jdk.test.lib.cds.SimpleCDSAppTester;
+import jdk.test.lib.process.OutputAnalyzer;
+
+public class VerifierFailOver {
+    public static void main(String... args) throws Exception {
+        SimpleCDSAppTester.of("VerifierFailOver")
+            .addVmArgs("-Xlog:cds+class=debug")
+            .classpath("app.jar")
+            .appCommandLine("VerifierFailOverApp")
+            .setTrainingChecker((OutputAnalyzer out) -> {
+                    out.shouldContain("Skipping VerifierFailOver_Helper: Verified with old verifier");
+                })
+            .setAssemblyChecker((OutputAnalyzer out) -> {
+                    // classes verified with fail-over mode should not be cached.
+                    out.shouldMatch("class.* klasses.* VerifierFailOverApp");
+                    out.shouldNotMatch("class.* klasses.* VerifierFailOver_Helper");
+                })
+            .runAOTWorkflow();
+    }
+}
+
+class VerifierFailOverApp {
+    public static void main(String[] args) throws Throwable {
+        Class goodClass = Class.forName("VerifierFailOver_Helper");
+        Object obj = goodClass.newInstance();
+        System.out.println("Successfully loaded: " + obj.getClass().getName());
+    }
+}
diff --git a/test/hotspot/jtreg/runtime/cds/appcds/aotCache/VerifierFailOver_Helper.jcod b/test/hotspot/jtreg/runtime/cds/appcds/aotCache/VerifierFailOver_Helper.jcod
@@ -0,0 +1,96 @@
+/*
+ * Copyright (c) 2025, 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.
+ *
+ */
+
+// This class should be rejected by the new verifier, but will be accepted by the old verifier.
+
+class VerifierFailOver_Helper {
+  0xCAFEBABE;
+  0;
+  50;
+  [] { // Constant Pool
+    ; // first element is empty
+    Method #3 #5; // #1    
+    Utf8 "StackMapTable"; // #2    
+    class #4; // #3    
+    Utf8 "java/lang/Object"; // #4    
+    NameAndType #12 #7; // #5    
+    Utf8 "SourceFile"; // #6    
+    Utf8 "()V"; // #7    
+    class #9; // #8    
+    Utf8 "VerifierFailOver_Helper"; // #9    
+    Utf8 "Code"; // #10    
+    Utf8 "VerifierFailOver_Helper.jasm"; // #11    
+    Utf8 "<init>"; // #12    
+    Utf8 "stackmap"; // #13
+  } // Constant Pool
+
+  0x0000; // access
+  #8;// this_cpx
+  #3;// super_cpx
+
+  [] { // Interfaces
+  } // Interfaces
+
+  [] { // fields
+  } // fields
+
+  [] { // methods
+    { // Member
+      0x0001; // access
+      #12; // name_cpx
+      #7; // sig_cpx
+      [] { // Attributes
+        Attr(#10) { // Code
+          1; // max_stack
+          1; // max_locals
+          Bytes[]{
+            0x2AB70001B100B1;
+          };
+          [] { // Traps
+          } // end Traps
+          [] { // Attributes
+/* right:
+            Attr(#2) { // StackMap
+end right */
+// wrong:
+            Attr(#13) { // stackmap
+// end wrong
+              [] { // 
+                255b,  5, []{I}, []{};
+              }
+            } // end StackMap
+          } // Attributes
+        } // end Code
+      } // Attributes
+    } // Member
+  } // methods
+
+  [] { // Attributes
+    Attr(#6) { // SourceFile
+      #11;
+    } // end SourceFile
+  } // Attributes
+} // end class stackmap00303m1n
+
+
diff --git a/test/lib/jdk/test/lib/cds/SimpleCDSAppTester.java b/test/lib/jdk/test/lib/cds/SimpleCDSAppTester.java
@@ -51,6 +51,7 @@
  */
 public class SimpleCDSAppTester {
     private String name;
+    private BiConsumer<OutputAnalyzer, RunMode> trainingChecker;
     private BiConsumer<OutputAnalyzer, RunMode> assemblyChecker;
     private BiConsumer<OutputAnalyzer, RunMode> productionChecker;
     private String classpath;
@@ -100,6 +101,11 @@ public SimpleCDSAppTester appCommandLine(String... args) {
         return this;
     }
 
+    public SimpleCDSAppTester setTrainingChecker(BiConsumer<OutputAnalyzer, RunMode> checker) {
+        this.trainingChecker = checker;
+        return this;
+    }
+
     public SimpleCDSAppTester setAssemblyChecker(BiConsumer<OutputAnalyzer, RunMode> checker) {
         this.assemblyChecker = checker;
         return this;
@@ -110,6 +116,12 @@ public SimpleCDSAppTester setProductionChecker(BiConsumer<OutputAnalyzer, RunMod
         return this;
     }
 
+    public SimpleCDSAppTester setTrainingChecker(Consumer<OutputAnalyzer> checker) {
+        this.trainingChecker = (OutputAnalyzer out, RunMode runMode) -> {
+            checker.accept(out);
+        };
+        return this;
+    }
 
     public SimpleCDSAppTester setAssemblyChecker(Consumer<OutputAnalyzer> checker) {
         this.assemblyChecker = (OutputAnalyzer out, RunMode runMode) -> {
@@ -152,7 +164,11 @@ public String[] appCommandLine(RunMode runMode) {
 
         @Override
         public void checkExecution(OutputAnalyzer out, RunMode runMode) throws Exception {
-            if (isDumping(runMode) && runMode != RunMode.TRAINING) {
+            if (runMode == RunMode.TRAINING) {
+                if (trainingChecker != null) {
+                    trainingChecker.accept(out, runMode);
+                }
+            } else if (isDumping(runMode)) {
                 if (assemblyChecker != null) {
                     assemblyChecker.accept(out, runMode);
                 }
