[GR-52211] Re-enable should verify framestates for runtime compiled m…

…ethods.

PullRequest: graal/17045

sdk/mx.sdk/mx_sdk_vm_impl.py

@@ -1369,6 +1369,8 @@ def contents(self):
             ] + svm_experimental_options([
                 '-H:+AssertInitializationSpecifiedForAllClasses',
                 '-H:+EnforceMaxRuntimeCompileMethods',
+                '-H:+VerifyRuntimeCompilationFrameStates',
+                '-H:+GuaranteeSubstrateTypesLinked',
             ])
             if _debug_images():
                 build_args += ['-ea', '-O0',] + svm_experimental_options(['-H:+PreserveFramePointer', '-H:-DeleteLocalSymbols'])

substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/runtimecompilation/RuntimeCompilationFeature.java

@@ -222,6 +222,7 @@ enum InlineDecision {
     private final Set<SubstrateMethod> substrateAnalysisMethods = ConcurrentHashMap.newKeySet();
     private final Map<AnalysisMethod, String> invalidForRuntimeCompilation = new ConcurrentHashMap<>();
     private final Set<RuntimeCompilationCandidate> runtimeCompilationCandidates = ConcurrentHashMap.newKeySet();
+    private final Set<AnalysisMethod> runtimeCompilationsFailedDuringParsing = ConcurrentHashMap.newKeySet();
     private CallTreeInfo callTreeMetadata = null;
     private HostedProviders analysisProviders = null;
     private AllowInliningPredicate allowInliningPredicate = (builder, target) -> AllowInliningPredicate.InlineDecision.INLINING_DISALLOWED;
@@ -513,6 +514,12 @@ private void checkMaxRuntimeCompiledMethods(CallTreeInfo callTreeInfo) {
     public void afterAnalysis(Feature.AfterAnalysisAccess access) {
         VMError.guarantee(callTreeMetadata == null);
         callTreeMetadata = CallTreeInfo.create(((FeatureImpl.AfterAnalysisAccessImpl) access).getUniverse(), invalidForRuntimeCompilation);
+        if (!runtimeCompilationsFailedDuringParsing.isEmpty()) {
+            System.out.println("PermanentBailouts seen while parsing runtime compiled methods. One reason for this can be encountering invalid frame states.");
+            for (AnalysisMethod failedMethod : runtimeCompilationsFailedDuringParsing) {
+                printFailingRuntimeMethodTrace(callTreeMetadata, failedMethod, failedMethod);
+            }
+        }
 
         ProgressReporter.singleton().setNumRuntimeCompiledMethods(callTreeMetadata.runtimeCompilations().size());
         // after analysis has completed we must ensure no new SubstrateTypes are introduced
@@ -596,20 +603,24 @@ public void reportAnalysisError(AnalysisUniverse aUniverse, Throwable t) {
                         } else if (errorMethod.isDeoptTarget()) {
                             failingRuntimeMethod = errorMethod.getMultiMethod(RUNTIME_COMPILED_METHOD);
                         }
-                        System.out.println("Parsing failed on a special method version: " + errorMethod.format("%H.%n"));
-                        System.out.println("Method reachability trace");
-                        if (failingRuntimeMethod != null) {
-                            Arrays.stream(CallTreeInfo.getCallTrace(treeInfo, failingRuntimeMethod, registeredRuntimeCompilations)).forEach(System.out::println);
-                        } else {
-                            System.out.println("trace unavailable");
-                        }
+                        printFailingRuntimeMethodTrace(treeInfo, failingRuntimeMethod, errorMethod);
                         System.out.println("error: " + e.getMessage());
                     }
                 }
             }
         }
     }
 
+    private void printFailingRuntimeMethodTrace(CallTreeInfo treeInfo, AnalysisMethod failingRuntimeMethod, AnalysisMethod errorMethod) {
+        System.out.println("Parsing failed on a special method version: " + errorMethod.format("%H.%n"));
+        System.out.println("Method reachability trace");
+        if (failingRuntimeMethod != null) {
+            Arrays.stream(CallTreeInfo.getCallTrace(treeInfo, failingRuntimeMethod, registeredRuntimeCompilations)).forEach(System.out::println);
+        } else {
+            System.out.println("trace unavailable");
+        }
+    }
+
     private class RuntimeCompilationParsingSupport implements SVMParsingSupport {
         RuntimeCompilationInlineBeforeAnalysisPolicy runtimeInlineBeforeAnalysisPolicy = null;
 
@@ -665,6 +676,7 @@ private Object parseRuntimeCompiledMethod(BigBang bb, DebugContext debug, Analys
             if (graph == null) {
                 if (!method.hasBytecodes()) {
                     recordFailed(method);
+                    runtimeCompilationsFailedDuringParsing.add(method);
                     return HostVM.PARSING_FAILED;
                 }
 
@@ -684,6 +696,7 @@ private Object parseRuntimeCompiledMethod(BigBang bb, DebugContext debug, Analys
                     } catch (PermanentBailoutException ex) {
                         bb.getUnsupportedFeatures().addMessage(method.format("%H.%n(%p)"), method, ex.getLocalizedMessage(), null, ex);
                         recordFailed(method);
+                        runtimeCompilationsFailedDuringParsing.add(method);
                         return HostVM.PARSING_FAILED;
                     }
                 }

substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/runtimecompilation/RuntimeCompiledMethodSupport.java

@@ -118,6 +118,9 @@ public class RuntimeCompiledMethodSupport {
     public static class Options {
         @Option(help = "Remove Deopt(Entries,Anchors,Proxies) determined to be unneeded after the runtime compiled graphs have been finalized.")//
         public static final HostedOptionKey<Boolean> RemoveUnneededDeoptSupport = new HostedOptionKey<>(true);
+
+        @Option(help = "Verify runtime compilation framestates during bytecode parsing.")//
+        public static final HostedOptionKey<Boolean> VerifyRuntimeCompilationFrameStates = new HostedOptionKey<>(false);
     }
 
     private record CompilationState(
@@ -462,14 +465,7 @@ protected boolean tryInvocationPlugin(CallTargetNode.InvokeKind invokeKind, Valu
 
         @Override
         protected boolean shouldVerifyFrameStates() {
-            /*
-             * (GR-46115) Ideally we should verify frame states in methods registered for runtime
-             * compilations, as well as any other methods that can deoptimize. Because runtime
-             * compiled methods can pull in almost arbitrary code, this means most frame states
-             * should be verified. We currently use illegal states as placeholders in many places,
-             * so this cannot be enabled at the moment.
-             */
-            return false;
+            return Options.VerifyRuntimeCompilationFrameStates.getValue();
         }
     }