From 7df7d98edf3feddd3d98cd9f053674c972cb3034 Mon Sep 17 00:00:00 2001 From: Sacha Coppey Date: Mon, 20 Jan 2025 14:04:14 +0100 Subject: [PATCH] Ensure objects are not modified while graphs are persisted --- .../jdk/graal/compiler/util/ObjectCopier.java | 6 +++ .../imagelayer/LoadImageSingletonFeature.java | 52 +++++++++++-------- .../imagelayer/SVMImageLayerSnapshotUtil.java | 12 +++++ 3 files changed, 47 insertions(+), 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 index 2838cd3298a3..8cf2829b49d3 100644 --- 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/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/LoadImageSingletonFeature.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/LoadImageSingletonFeature.java index 5e960747628e..92b2aa2e4536 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/LoadImageSingletonFeature.java +++ b/substratevm/src/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, SlotInfo> currentKeyToSlotInfoMap; + Map, 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, LoadImageSingletonDataImpl> layerKeyToSingletonDataMap = new ConcurrentHashMap<>(); boolean sealedSingletonLookup = false; - private CGlobalData singletonTableStart; + CGlobalData 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/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerSnapshotUtil.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerSnapshotUtil.java index 7b60b0d7d5ab..10ae960c3ee8 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/SVMImageLayerSnapshotUtil.java +++ b/substratevm/src/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 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 {