[GR-59742] Re-enable @Uninterruptible and @RestrictHeapAccess for open world and layered images.

PullRequest: graal/21439

Author: cstancu

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateDiagnostics.java

@@ -96,6 +96,7 @@
 import com.oracle.svm.core.threadlocal.VMThreadLocalInfos;
 import com.oracle.svm.core.util.CounterSupport;
 import com.oracle.svm.core.util.ImageHeapList;
+import com.oracle.svm.core.util.RuntimeImageHeapList;
 import com.oracle.svm.core.util.TimeUtils;
 import com.oracle.svm.core.util.VMError;
 
@@ -1352,7 +1353,12 @@ int size() {
         }
 
         DiagnosticThunk getThunk(int index) {
-            return thunks.get(index);
+            /*
+             * Use an explicit cast to aid open-world analysis. Otherwise, this may trigger false
+             * positive violations of @RestrictHeapAccess since some implementations of List.get()
+             * can allocate.
+             */
+            return ((RuntimeImageHeapList<DiagnosticThunk>) thunks).get(index);
         }
 
         int getInitialInvocationCount(int index) {

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/sampler/AbstractJfrExecutionSampler.java

@@ -170,6 +170,7 @@ protected static boolean isExecutionSamplingAllowedInCurrentThread() {
 
     protected abstract void updateInterval();
 
+    @Uninterruptible(reason = "Prevent VM operations that modify the recurring callbacks.")
     protected abstract void uninstall(IsolateThread thread);
 
     @Uninterruptible(reason = "The method executes during signal handling.", callerMustBe = true)

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/util/ImageHeapList.java

@@ -35,7 +35,6 @@
 import org.graalvm.nativeimage.Platforms;
 
 import com.oracle.svm.core.BuildPhaseProvider;
-import com.oracle.svm.core.Uninterruptible;
 
 /**
  * A list that is filled at image build time while the static analysis is running, and then read at
@@ -80,7 +79,7 @@ private ImageHeapList() {
     public static final class HostedImageHeapList<E> extends AbstractList<E> {
         private final Comparator<E> comparator;
         private final List<E> hostedList;
-        public final RuntimeImageHeapList<E> runtimeList;
+        private final RuntimeImageHeapList<E> runtimeList;
         /**
          * Used to signal if this list has been modified. If true, the change should be propagated
          * from the hosted list to the runtime list by calling {@link #update()}. This variable
@@ -96,6 +95,10 @@ private HostedImageHeapList(Class<E> elementClass, Comparator<E> comparator) {
             this.runtimeList = new RuntimeImageHeapList<>((E[]) Array.newInstance(elementClass, 0));
         }
 
+        public RuntimeImageHeapList<E> getRuntimeList() {
+            return runtimeList;
+        }
+
         public synchronized boolean needsUpdate() {
             return modified;
         }
@@ -158,25 +161,3 @@ private static UnsupportedOperationException notSupported() {
         }
     }
 }
-
-final class RuntimeImageHeapList<E> extends AbstractList<E> {
-
-    E[] elementData;
-
-    @Platforms(Platform.HOSTED_ONLY.class)
-    RuntimeImageHeapList(E[] elementData) {
-        this.elementData = elementData;
-    }
-
-    @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
-    @Override
-    public E get(int index) {
-        return elementData[index];
-    }
-
-    @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
-    @Override
-    public int size() {
-        return elementData.length;
-    }
-}

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/util/RuntimeImageHeapList.java

@@ -0,0 +1,57 @@
+/*
+ * Copyright (c) 2024, 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.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * 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.
+ */
+package com.oracle.svm.core.util;
+
+import java.util.AbstractList;
+
+import org.graalvm.nativeimage.Platform;
+import org.graalvm.nativeimage.Platforms;
+
+import com.oracle.svm.core.Uninterruptible;
+
+/**
+ * The immutable runtime list view for an {@link ImageHeapList}.
+ */
+public final class RuntimeImageHeapList<E> extends AbstractList<E> {
+
+    E[] elementData;
+
+    @Platforms(Platform.HOSTED_ONLY.class)
+    RuntimeImageHeapList(E[] elementData) {
+        this.elementData = elementData;
+    }
+
+    @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
+    @Override
+    public E get(int index) {
+        return elementData[index];
+    }
+
+    @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
+    @Override
+    public int size() {
+        return elementData.length;
+    }
+}

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/CompileQueue.java

@@ -42,9 +42,11 @@
 
 import com.oracle.graal.pointsto.api.PointstoOptions;
 import com.oracle.graal.pointsto.flow.AnalysisParsedGraph;
+import com.oracle.graal.pointsto.infrastructure.OriginalClassProvider;
 import com.oracle.graal.pointsto.meta.HostedProviders;
 import com.oracle.graal.pointsto.util.CompletionExecutor;
 import com.oracle.graal.pointsto.util.CompletionExecutor.DebugContextRunnable;
+import com.oracle.graal.pointsto.util.GraalAccess;
 import com.oracle.svm.common.meta.MultiMethod;
 import com.oracle.svm.core.SubstrateOptions;
 import com.oracle.svm.core.Uninterruptible;
@@ -137,6 +139,7 @@
 import jdk.vm.ci.code.site.Infopoint;
 import jdk.vm.ci.meta.MetaAccessProvider;
 import jdk.vm.ci.meta.ResolvedJavaMethod;
+import jdk.vm.ci.meta.ResolvedJavaType;
 import jdk.vm.ci.meta.VMConstant;
 
 public class CompileQueue {
@@ -182,6 +185,9 @@ protected PhaseSuite<HighTierContext> getAfterParseSuite() {
 
     private final boolean printMethodHistogram = NativeImageOptions.PrintMethodHistogram.getValue();
     private final boolean optionAOTTrivialInline = SubstrateOptions.AOTTrivialInline.getValue();
+    private final boolean allowFoldMethods = NativeImageOptions.AllowFoldMethods.getValue();
+
+    private final ResolvedJavaType generatedFoldInvocationPluginType;
 
     public record UnpublishedTrivialMethods(CompilationGraph unpublishedGraph, boolean newlyTrivial) {
     }
@@ -387,6 +393,7 @@ public CompileQueue(DebugContext debug, FeatureHandler featureHandler, HostedUni
         this.defaultParseHooks = new ParseHooks(this);
 
         callForReplacements(debug, runtimeConfig);
+        generatedFoldInvocationPluginType = GraalAccess.getOriginalProviders().getMetaAccess().lookupJavaType(GeneratedFoldInvocationPlugin.class);
     }
 
     protected AnalysisToHostedGraphTransplanter createGraphTransplanter() {
@@ -409,9 +416,7 @@ public void finish(DebugContext debug) {
                 parseAll();
             }
 
-            // GR-59742 re-enable for open type world and layered images
-            if (SubstrateOptions.useClosedTypeWorld() && !ImageLayerBuildingSupport.buildingImageLayer() &&
-                            !PointstoOptions.UseExperimentalReachabilityAnalysis.getValue(universe.hostVM().options())) {
+            if (!PointstoOptions.UseExperimentalReachabilityAnalysis.getValue(universe.hostVM().options())) {
                 /*
                  * Reachability Analysis creates call graphs with more edges compared to the
                  * Points-to Analysis, therefore the annotations would have to be added to a lot
@@ -1005,17 +1010,21 @@ protected void ensureParsed(HostedMethod method, HostedMethod callerMethod, Comp
             return;
         }
 
-        if (!(NativeImageOptions.AllowFoldMethods.getValue() || method.getAnnotation(Fold.class) == null ||
-                        (callerMethod != null && metaAccess.lookupJavaType(GeneratedFoldInvocationPlugin.class).isAssignableFrom(callerMethod.getDeclaringClass())))) {
-            throw VMError.shouldNotReachHere("Parsing method annotated with @" + Fold.class.getSimpleName() + ": " +
-                            method.format("%H.%n(%p)") +
-                            ". Make sure you have used Graal annotation processors on the parent-project of the method's declaring class.");
+        if (!allowFoldMethods && method.getAnnotation(Fold.class) != null && !isFoldInvocationPluginMethod(callerMethod)) {
+            throw VMError.shouldNotReachHere("Parsing method annotated with @%s: %s. " +
+                            "This could happen if either: the Graal annotation processor was not executed on the parent-project of the method's declaring class, " +
+                            "the arguments passed to the method were not compile-time constants, or the plugin was disabled by the corresponding %s.",
+                            Fold.class.getSimpleName(), method.format("%H.%n(%p)"), GraphBuilderContext.class.getSimpleName());
         }
         if (!method.compilationInfo.inParseQueue.getAndSet(true)) {
             executor.execute(new ParseTask(method, reason));
         }
     }
 
+    private boolean isFoldInvocationPluginMethod(HostedMethod method) {
+        return method != null && generatedFoldInvocationPluginType.isAssignableFrom(OriginalClassProvider.getOriginalType(method.getDeclaringClass()));
+    }
+
     protected final void doParse(DebugContext debug, ParseTask task) {
         HostedMethod method = task.method;
         if (HostedImageLayerBuildingSupport.buildingExtensionLayer()) {

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/ImageHeapCollectionFeature.java

@@ -68,7 +68,7 @@ private Object replaceHostedWithRuntime(Object obj) {
             } else {
                 allLists.add(hostedImageHeapList);
             }
-            return hostedImageHeapList.runtimeList;
+            return hostedImageHeapList.getRuntimeList();
         }
         return obj;
     }
@@ -101,7 +101,7 @@ public void duringAnalysis(DuringAnalysisAccess a) {
         allLists.parallelStream().forEach(hostedImageHeapList -> {
             if (hostedImageHeapList.needsUpdate()) {
                 hostedImageHeapList.update();
-                objectsToRescan.add(hostedImageHeapList.runtimeList);
+                objectsToRescan.add(hostedImageHeapList.getRuntimeList());
             }
         });
         if (!objectsToRescan.isEmpty()) {
@@ -135,7 +135,7 @@ public void afterImageWrite(AfterImageWriteAccess access) {
         for (var hostedImageHeapList : allLists) {
             if (hostedImageHeapList.needsUpdate()) {
                 throw VMError.shouldNotReachHere("ImageHeapList modified after static analysis:%n%s%n%s",
-                                hostedImageHeapList, hostedImageHeapList.runtimeList);
+                                hostedImageHeapList, hostedImageHeapList.getRuntimeList());
 
             }
         }