[GR-61459] Ensure objects are not modified while graphs are persisted

PullRequest: graal/19840
oracle · Jan 21, 2025 · 9bd367e · 9bd367e
2 parents e358584 + 7df7d98
commit 9bd367e
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 23 deletions.
diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/util/ObjectCopier.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/util/ObjectCopier.java
@@ -787,6 +787,7 @@ void makeId(Object obj, ObjectPath objectPath) {
                 checkIllegalValue(LocationIdentity.class, obj, objectPath, "must come from a static field");
                 checkIllegalValue(HashSet.class, obj, objectPath, "hashes are typically not stable across VM executions");
 
+                prepareObject(obj);
                 makeStringId(clazz.getName(), objectPath);
                 ClassInfo classInfo = makeClassInfo(clazz, objectPath);
                 classInfo.fields().forEach((fieldDesc, f) -> {
@@ -799,6 +800,11 @@ void makeId(Object obj, ObjectPath objectPath) {
             }
         }
 
+        @SuppressWarnings("unused")
+        protected void prepareObject(Object obj) {
+            /* Hook to prepare special objects */
+        }
+
         private ClassInfo makeClassInfo(Class<?> clazz, ObjectPath objectPath) {
             try {
                 return classInfos.computeIfAbsent(clazz, this::makeClassInfo);

diff --git a/...com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/LoadImageSingletonFeature.java b/...com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/LoadImageSingletonFeature.java
@@ -25,6 +25,7 @@
 package com.oracle.svm.hosted.imagelayer;
 
 import static com.oracle.svm.hosted.imagelayer.LoadImageSingletonFeature.CROSS_LAYER_SINGLETON_TABLE_SYMBOL;
+import static com.oracle.svm.hosted.imagelayer.LoadImageSingletonFeature.getCrossLayerSingletonMappingInfo;
 
 import java.util.ArrayList;
 import java.util.Comparator;
@@ -102,7 +103,7 @@
 public class LoadImageSingletonFeature implements InternalFeature, FeatureSingleton, UnsavedSingleton {
     public static final String CROSS_LAYER_SINGLETON_TABLE_SYMBOL = "__layered_singleton_table_start";
 
-    private static CrossLayerSingletonMappingInfo getCrossLayerSingletonMappingInfo() {
+    static CrossLayerSingletonMappingInfo getCrossLayerSingletonMappingInfo() {
         return (CrossLayerSingletonMappingInfo) ImageSingletons.lookup(LoadImageSingletonFactory.class);
     }
 
@@ -454,7 +455,7 @@ class CrossLayerSingletonMappingInfo extends LoadImageSingletonFactory implement
     /**
      * Map of all slot infos (past & present). Is created in {@link #assignSlots}.
      */
-    private Map<Class<?>, SlotInfo> currentKeyToSlotInfoMap;
+    Map<Class<?>, SlotInfo> currentKeyToSlotInfoMap;
 
     /**
      * Map of constant identifiers for MultiLayer objects installed in this layer.
@@ -467,7 +468,7 @@ class CrossLayerSingletonMappingInfo extends LoadImageSingletonFactory implement
     private final Map<Class<?>, LoadImageSingletonDataImpl> layerKeyToSingletonDataMap = new ConcurrentHashMap<>();
 
     boolean sealedSingletonLookup = false;
-    private CGlobalData<Pointer> singletonTableStart;
+    CGlobalData<Pointer> singletonTableStart;
     int referenceSize = 0;
 
     @Override
@@ -524,7 +525,7 @@ private LoadImageSingletonData getImageSingletonInfo(Class<?> keyClass, SlotReco
             return result;
         }
 
-        if (result.kind == SlotRecordKind.MULTI_LAYERED_SINGLETON) {
+        if (result.getKind() == SlotRecordKind.MULTI_LAYERED_SINGLETON) {
             if (!ImageSingletons.contains(keyClass)) {
                 /*
                  * If the singleton doesn't exist, ensure this is allowed.
@@ -580,7 +581,7 @@ void assignSlots(HostedMetaAccess metaAccess) {
                      * singleton a slot now.
                      */
                     slotAssignment = nextFreeSlot++;
-                    slotInfo = new SlotInfo(keyClass, slotAssignment, info.kind);
+                    slotInfo = new SlotInfo(keyClass, slotAssignment, info.getKind());
                 }
                 var prior = currentKeyToSlotInfoMap.put(keyClass, slotInfo);
                 assert prior == null : prior;
@@ -689,28 +690,33 @@ public static Object createFromLoader(ImageSingletonLoader loader) {
 
         return new CrossLayerSingletonMappingInfo(Map.copyOf(keyClassToSlotInfoMap), Map.copyOf(keyClassToObjectIDListMap));
     }
+}
 
-    class LoadImageSingletonDataImpl implements LoadImageSingletonData {
+class LoadImageSingletonDataImpl implements LoadImageSingletonFactory.LoadImageSingletonData {
 
-        private final Class<?> key;
-        private final SlotRecordKind kind;
+    private final Class<?> key;
+    private final SlotRecordKind kind;
 
-        LoadImageSingletonDataImpl(Class<?> key, SlotRecordKind kind) {
-            this.key = key;
-            this.kind = kind;
-        }
+    LoadImageSingletonDataImpl(Class<?> key, SlotRecordKind kind) {
+        this.key = key;
+        this.kind = kind;
+    }
 
-        @Override
-        public Class<?> getLoadType() {
-            return kind == SlotRecordKind.APPLICATION_LAYER_SINGLETON ? key : key.arrayType();
-        }
+    public SlotRecordKind getKind() {
+        return kind;
+    }
 
-        @Override
-        public SingletonAccessInfo getAccessInfo() {
-            assert singletonTableStart != null;
-            CGlobalDataInfo cglobal = CGlobalDataFeature.singleton().registerAsAccessedOrGet(singletonTableStart);
-            int slotNum = currentKeyToSlotInfoMap.get(key).slotNum();
-            return new SingletonAccessInfo(cglobal, slotNum * referenceSize);
-        }
+    @Override
+    public Class<?> getLoadType() {
+        return kind == SlotRecordKind.APPLICATION_LAYER_SINGLETON ? key : key.arrayType();
+    }
+
+    @Override
+    public LoadImageSingletonFactory.SingletonAccessInfo getAccessInfo() {
+        CrossLayerSingletonMappingInfo singleton = getCrossLayerSingletonMappingInfo();
+        assert singleton.singletonTableStart != null;
+        CGlobalDataInfo cglobal = CGlobalDataFeature.singleton().registerAsAccessedOrGet(singleton.singletonTableStart);
+        int slotNum = singleton.currentKeyToSlotInfoMap.get(key).slotNum();
+        return new LoadImageSingletonFactory.SingletonAccessInfo(cglobal, slotNum * singleton.referenceSize);
     }
 }
diff --git a/...com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerSnapshotUtil.java b/...com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerSnapshotUtil.java
@@ -80,6 +80,7 @@
 import com.oracle.svm.util.ReflectionUtil;
 
 import jdk.graal.compiler.api.replacements.SnippetReflectionProvider;
+import jdk.graal.compiler.debug.CounterKey;
 import jdk.graal.compiler.nodes.EncodedGraph;
 import jdk.graal.compiler.nodes.FieldLocationIdentity;
 import jdk.graal.compiler.util.ObjectCopier;
@@ -338,6 +339,17 @@ public SVMGraphEncoder(Map<Object, Field> externalValues) {
             addBuiltin(new FastThreadLocalLocationIdentityBuiltIn());
             addBuiltin(new VMThreadLocalInfoBuiltIn());
         }
+
+        @Override
+        protected void prepareObject(Object obj) {
+            if (obj instanceof CounterKey counterKey) {
+                /*
+                 * The name needs to be cached before we persist the graph to avoid modifying the
+                 * field during the encoding.
+                 */
+                counterKey.getName();
+            }
+        }
     }
 
     public abstract static class AbstractSVMGraphDecoder extends ObjectCopier.Decoder {