From c6fceaa1f67cb6d8a0b678fe0cc4c9eb8413b12c Mon Sep 17 00:00:00 2001 From: Codrut Stancu Date: Tue, 5 Sep 2023 17:55:15 +0200 Subject: [PATCH 1/8] Rescan lazy set module fields. --- .../svm/core/jdk/RuntimeModuleSupport.java | 7 +- .../oracle/svm/hosted/ModuleLayerFeature.java | 82 +++++++++++-------- 2 files changed, 54 insertions(+), 35 deletions(-) diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/RuntimeModuleSupport.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/RuntimeModuleSupport.java index 0c90464fb5f1..89babece5ed1 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/RuntimeModuleSupport.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/RuntimeModuleSupport.java @@ -24,21 +24,22 @@ */ package com.oracle.svm.core.jdk; +import java.util.function.Function; + import org.graalvm.nativeimage.ImageSingletons; import org.graalvm.nativeimage.Platform; import org.graalvm.nativeimage.Platforms; +import com.oracle.svm.core.BuildPhaseProvider.AfterHostedUniverse; import com.oracle.svm.core.heap.UnknownObjectField; -import java.util.function.Function; - public final class RuntimeModuleSupport { public static RuntimeModuleSupport instance() { return ImageSingletons.lookup(RuntimeModuleSupport.class); } - @UnknownObjectField private ModuleLayer bootLayer; + @UnknownObjectField(availability = AfterHostedUniverse.class) private ModuleLayer bootLayer; @Platforms(Platform.HOSTED_ONLY.class) // private Function hostedToRuntimeModuleMapper; diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ModuleLayerFeature.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ModuleLayerFeature.java index e2a5f40568d1..e013c0d086af 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ModuleLayerFeature.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ModuleLayerFeature.java @@ -70,6 +70,8 @@ import com.oracle.svm.core.jdk.Resources; import com.oracle.svm.core.jdk.RuntimeModuleSupport; import com.oracle.svm.core.util.VMError; +import com.oracle.svm.hosted.FeatureImpl.AfterAnalysisAccessImpl; +import com.oracle.svm.hosted.FeatureImpl.AnalysisAccessBase; import com.oracle.svm.hosted.FeatureImpl.BeforeAnalysisAccessImpl; import com.oracle.svm.util.LogUtils; import com.oracle.svm.util.ModuleSupport; @@ -176,14 +178,14 @@ public void beforeAnalysis(BeforeAnalysisAccess access) { private void scanRuntimeBootLayerPrototype(BeforeAnalysisAccessImpl accessImpl) { Set baseModules = ModuleLayer.boot().modules().stream().map(Module::getName).collect(Collectors.toSet()); Function clf = moduleLayerFeatureUtils::getClassLoaderForBootLayerModule; - ModuleLayer runtimeBootLayer = synthesizeRuntimeModuleLayer(new ArrayList<>(List.of(ModuleLayer.empty())), accessImpl.imageClassLoader, baseModules, Set.of(), clf, null); + ModuleLayer runtimeBootLayer = synthesizeRuntimeModuleLayer(new ArrayList<>(List.of(ModuleLayer.empty())), accessImpl, accessImpl.imageClassLoader, baseModules, Set.of(), clf, null); /* Only scan the value if module support is enabled and bootLayer field is reachable. */ accessImpl.registerReachabilityHandler((a) -> accessImpl.rescanObject(runtimeBootLayer), ReflectionUtil.lookupField(RuntimeModuleSupport.class, "bootLayer")); } @Override public void afterAnalysis(AfterAnalysisAccess access) { - FeatureImpl.AfterAnalysisAccessImpl accessImpl = (FeatureImpl.AfterAnalysisAccessImpl) access; + AfterAnalysisAccessImpl accessImpl = (AfterAnalysisAccessImpl) access; Set runtimeImageNamedModules = accessImpl.getUniverse().getTypes() .stream() @@ -241,8 +243,8 @@ public void afterAnalysis(AfterAnalysisAccess access) { * Ensure that runtime modules have the same relations (i.e., reads, opens and exports) as * the originals. */ - replicateVisibilityModifications(runtimeBootLayer, accessImpl.imageClassLoader, runtimeImageNamedModules); - replicateNativeAccess(runtimeImageNamedModules); + replicateVisibilityModifications(runtimeBootLayer, accessImpl, accessImpl.imageClassLoader, runtimeImageNamedModules); + replicateNativeAccess(accessImpl, runtimeImageNamedModules); } /** @@ -360,7 +362,7 @@ private static Stream extractRequiredModuleNames(Module m) { return Stream.concat(Stream.of(m.getName()), requiredModules); } - private List synthesizeRuntimeModuleLayers(FeatureImpl.AfterAnalysisAccessImpl accessImpl, List hostedModuleLayers, Collection reachableNamedModules, + private List synthesizeRuntimeModuleLayers(AfterAnalysisAccessImpl accessImpl, List hostedModuleLayers, Collection reachableNamedModules, Collection reachableSyntheticModules, Collection rootModuleNames) { /* * A mapping from hosted to runtime module layers. Used when looking up runtime module layer @@ -404,7 +406,7 @@ private List synthesizeRuntimeModuleLayers(FeatureImpl.AfterAnalysi Configuration cf = isBootModuleLayer ? null : hostedModuleLayer.configuration(); Function clf = name -> moduleLayerFeatureUtils.getClassLoaderForModuleInModuleLayer(hostedModuleLayer, name); - ModuleLayer runtimeModuleLayer = synthesizeRuntimeModuleLayer(parents, accessImpl.imageClassLoader, moduleNames, syntheticModules, clf, cf); + ModuleLayer runtimeModuleLayer = synthesizeRuntimeModuleLayer(parents, accessImpl, accessImpl.imageClassLoader, moduleNames, syntheticModules, clf, cf); moduleLayerPairs.put(hostedModuleLayer, runtimeModuleLayer); } @@ -412,8 +414,8 @@ private List synthesizeRuntimeModuleLayers(FeatureImpl.AfterAnalysi return new ArrayList<>(moduleLayerPairs.values()); } - private ModuleLayer synthesizeRuntimeModuleLayer(List parentLayers, ImageClassLoader cl, Set reachableModules, Set syntheticModules, - Function clf, Configuration cfOverride) { + private ModuleLayer synthesizeRuntimeModuleLayer(List parentLayers, AnalysisAccessBase accessImpl, ImageClassLoader cl, Set reachableModules, + Set syntheticModules, Function clf, Configuration cfOverride) { /** * For consistent module lookup we reuse the {@link ModuleFinder}s defined and used in * {@link NativeImageClassLoaderSupport}. @@ -431,20 +433,21 @@ private ModuleLayer synthesizeRuntimeModuleLayer(List parentLayers, ModuleLayer runtimeModuleLayer = null; try { runtimeModuleLayer = moduleLayerFeatureUtils.createNewModuleLayerInstance(runtimeModuleLayerConfiguration); - Map nameToModule = moduleLayerFeatureUtils.synthesizeNameToModule(runtimeModuleLayer, clf); + Map nameToModule = moduleLayerFeatureUtils.synthesizeNameToModule(accessImpl, runtimeModuleLayer, clf); for (Module syntheticModule : syntheticModules) { Module runtimeSyntheticModule = moduleLayerFeatureUtils.getOrCreateRuntimeModuleForHostedModule(syntheticModule); nameToModule.putIfAbsent(runtimeSyntheticModule.getName(), runtimeSyntheticModule); - moduleLayerFeatureUtils.patchModuleLayerField(runtimeSyntheticModule, runtimeModuleLayer); + moduleLayerFeatureUtils.patchModuleLayerField(accessImpl, runtimeSyntheticModule, runtimeModuleLayer); } - patchRuntimeModuleLayer(runtimeModuleLayer, nameToModule, parentLayers); + patchRuntimeModuleLayer(accessImpl, runtimeModuleLayer, nameToModule, parentLayers); + accessImpl.rescanField(runtimeModuleLayer, moduleLayerFeatureUtils.moduleLayerModulesField); return runtimeModuleLayer; } catch (InstantiationException | IllegalAccessException | InvocationTargetException ex) { throw VMError.shouldNotReachHere("Failed to synthesize the runtime module layer: " + runtimeModuleLayer, ex); } } - private void replicateVisibilityModifications(ModuleLayer runtimeBootLayer, ImageClassLoader cl, Set analysisReachableNamedModules) { + private void replicateVisibilityModifications(ModuleLayer runtimeBootLayer, AfterAnalysisAccessImpl accessImpl, ImageClassLoader cl, Set analysisReachableNamedModules) { List applicationModules = findApplicationModules(runtimeBootLayer, cl.applicationModulePath()); Map modulePairs = analysisReachableNamedModules @@ -470,27 +473,27 @@ private void replicateVisibilityModifications(ModuleLayer runtimeBootLayer, Imag } Module runtimeTo = e2.getValue(); if (ModuleLayerFeatureUtils.isModuleSynthetic(hostedFrom) || hostedFrom.canRead(hostedTo)) { - moduleLayerFeatureUtils.addReads(runtimeFrom, runtimeTo); + moduleLayerFeatureUtils.addReads(accessImpl, runtimeFrom, runtimeTo); if (hostedFrom == builderModule) { for (Module appModule : applicationModules) { - moduleLayerFeatureUtils.addReads(appModule, runtimeTo); + moduleLayerFeatureUtils.addReads(accessImpl, appModule, runtimeTo); } } } for (String pn : runtimeFrom.getPackages()) { if (ModuleLayerFeatureUtils.isModuleSynthetic(hostedFrom) || hostedFrom.isOpen(pn, hostedTo)) { - moduleLayerFeatureUtils.addOpens(runtimeFrom, pn, runtimeTo); + moduleLayerFeatureUtils.addOpens(accessImpl, runtimeFrom, pn, runtimeTo); if (hostedTo == builderModule) { for (Module appModule : applicationModules) { - moduleLayerFeatureUtils.addOpens(runtimeFrom, pn, appModule); + moduleLayerFeatureUtils.addOpens(accessImpl, runtimeFrom, pn, appModule); } } } if (ModuleLayerFeatureUtils.isModuleSynthetic(hostedFrom) || hostedFrom.isExported(pn, hostedTo)) { - moduleLayerFeatureUtils.addExports(runtimeFrom, pn, runtimeTo); + moduleLayerFeatureUtils.addExports(accessImpl, runtimeFrom, pn, runtimeTo); if (hostedTo == builderModule) { for (Module appModule : applicationModules) { - moduleLayerFeatureUtils.addExports(runtimeFrom, pn, appModule); + moduleLayerFeatureUtils.addExports(accessImpl, runtimeFrom, pn, appModule); } } } @@ -502,7 +505,7 @@ private void replicateVisibilityModifications(ModuleLayer runtimeBootLayer, Imag } } - private void replicateNativeAccess(Set analysisReachableNamedModules) { + private void replicateNativeAccess(AfterAnalysisAccessImpl accessImpl, Set analysisReachableNamedModules) { if (JavaVersionUtil.JAVA_SPEC < 19) { return; } @@ -518,7 +521,7 @@ private void replicateNativeAccess(Set analysisReachableNamedModules) { Module hosted = modulesPair.getKey(); Module runtime = modulesPair.getValue(); if (moduleLayerFeatureUtils.allowsNativeAccess(hosted)) { - moduleLayerFeatureUtils.setNativeAccess(runtime, true); + moduleLayerFeatureUtils.setNativeAccess(accessImpl, runtime, true); } } @@ -567,10 +570,10 @@ private static Configuration synthesizeRuntimeModuleLayerConfiguration(ModuleFin } } - private void patchRuntimeModuleLayer(ModuleLayer runtimeModuleLayer, Map nameToModule, List parents) { + private void patchRuntimeModuleLayer(AnalysisAccessBase accessImpl, ModuleLayer runtimeModuleLayer, Map nameToModule, List parents) { try { - moduleLayerFeatureUtils.patchModuleLayerNameToModuleField(runtimeModuleLayer, nameToModule); - moduleLayerFeatureUtils.patchModuleLayerParentsField(runtimeModuleLayer, parents); + moduleLayerFeatureUtils.patchModuleLayerNameToModuleField(accessImpl, runtimeModuleLayer, nameToModule); + moduleLayerFeatureUtils.patchModuleLayerParentsField(accessImpl, runtimeModuleLayer, parents); } catch (IllegalAccessException ex) { throw VMError.shouldNotReachHere("Failed to patch the runtime boot module layer.", ex); } @@ -605,6 +608,7 @@ private static final class ModuleLayerFeatureUtils { private final Constructor moduleLayerConstructor; private final Field moduleLayerNameToModuleField; private final Field moduleLayerParentsField; + private final Field moduleLayerModulesField; ModuleLayerFeatureUtils(ImageClassLoader cl) { runtimeModules = new HashMap<>(); @@ -660,6 +664,7 @@ private static final class ModuleLayerFeatureUtils { moduleLayerConstructor = ReflectionUtil.lookupConstructor(ModuleLayer.class, Configuration.class, List.class, Function.class); moduleLayerNameToModuleField = ReflectionUtil.lookupField(ModuleLayer.class, "nameToModule"); moduleLayerParentsField = ReflectionUtil.lookupField(ModuleLayer.class, "parents"); + moduleLayerModulesField = ReflectionUtil.lookupField(ModuleLayer.class, "modules"); } catch (ReflectiveOperationException | NoSuchElementException ex) { throw VMError.shouldNotReachHere("Failed to retrieve fields of the Module/ModuleLayer class.", ex); } @@ -821,7 +826,7 @@ public Module getOrCreateRuntimeModuleForHostedModule(ClassLoader loader, String * and removal of VM state updates (otherwise we would be re-defining modules to the host * VM). */ - Map synthesizeNameToModule(ModuleLayer runtimeModuleLayer, Function clf) + Map synthesizeNameToModule(AnalysisAccessBase access, ModuleLayer runtimeModuleLayer, Function clf) throws IllegalAccessException, InvocationTargetException { Configuration cf = runtimeModuleLayer.configuration(); @@ -840,8 +845,9 @@ Map synthesizeNameToModule(ModuleLayer runtimeModuleLayer, Funct Module m = getOrCreateRuntimeModuleForHostedModule(loader, name, descriptor); if (!descriptor.equals(m.getDescriptor())) { moduleDescriptorField.set(m, descriptor); + access.rescanField(m, moduleDescriptorField); } - patchModuleLayerField(m, runtimeModuleLayer); + patchModuleLayerField(access, m, runtimeModuleLayer); nameToModule.put(name, m); } @@ -868,6 +874,7 @@ Map synthesizeNameToModule(ModuleLayer runtimeModuleLayer, Funct reads.add(allUnnamedModule); } moduleReadsField.set(m, reads); + access.rescanField(m, moduleReadsField); if (!descriptor.isOpen() && !descriptor.isAutomatic()) { if (descriptor.opens().isEmpty()) { @@ -890,6 +897,7 @@ Map synthesizeNameToModule(ModuleLayer runtimeModuleLayer, Funct } } moduleExportedPackagesField.set(m, exportedPackages); + access.rescanField(m, moduleExportedPackagesField); } else { Map> openPackages = new HashMap<>(descriptor.opens().size()); for (ModuleDescriptor.Opens opens : descriptor.opens()) { @@ -937,26 +945,30 @@ Map synthesizeNameToModule(ModuleLayer runtimeModuleLayer, Funct } moduleOpenPackagesField.set(m, openPackages); + access.rescanField(m, moduleOpenPackagesField); moduleExportedPackagesField.set(m, exportedPackages); + access.rescanField(m, moduleExportedPackagesField); } } + access.rescanObject(m); } return nameToModule; } @SuppressWarnings("unchecked") - void addReads(Module module, Module other) throws IllegalAccessException { + void addReads(AfterAnalysisAccessImpl accessImpl, Module module, Module other) throws IllegalAccessException { Set reads = (Set) moduleReadsField.get(module); if (reads == null) { reads = new HashSet<>(1); moduleReadsField.set(module, reads); } reads.add(other == null ? allUnnamedModule : other); + accessImpl.rescanField(module, moduleReadsField); } @SuppressWarnings("unchecked") - void addExports(Module module, String pn, Module other) throws IllegalAccessException { + void addExports(AfterAnalysisAccessImpl accessImpl, Module module, String pn, Module other) throws IllegalAccessException { if (other != null && module.isExported(pn, other)) { return; } @@ -979,10 +991,11 @@ void addExports(Module module, String pn, Module other) throws IllegalAccessExce if (prev != null) { prev.add(other == null ? allUnnamedModule : other); } + accessImpl.rescanField(module, moduleExportedPackagesField); } @SuppressWarnings("unchecked") - void addOpens(Module module, String pn, Module other) throws IllegalAccessException { + void addOpens(AfterAnalysisAccessImpl accessImpl, Module module, String pn, Module other) throws IllegalAccessException { if (other != null && module.isOpen(pn, other)) { return; } @@ -1005,10 +1018,12 @@ void addOpens(Module module, String pn, Module other) throws IllegalAccessExcept if (prev != null) { prev.add(other == null ? allUnnamedModule : other); } + accessImpl.rescanField(module, moduleOpenPackagesField); } - void patchModuleLayerField(Module module, ModuleLayer runtimeBootLayer) throws IllegalAccessException { + void patchModuleLayerField(AnalysisAccessBase accessImpl, Module module, ModuleLayer runtimeBootLayer) throws IllegalAccessException { moduleLayerField.set(module, runtimeBootLayer); + accessImpl.rescanField(module, moduleLayerField); } void patchModuleLoaderField(Module module, ClassLoader loader) throws IllegalAccessException { @@ -1019,12 +1034,14 @@ ModuleLayer createNewModuleLayerInstance(Configuration cf) throws InvocationTarg return moduleLayerConstructor.newInstance(cf, List.of(), null); } - void patchModuleLayerNameToModuleField(ModuleLayer moduleLayer, Map nameToModule) throws IllegalAccessException { + void patchModuleLayerNameToModuleField(AnalysisAccessBase accessImpl, ModuleLayer moduleLayer, Map nameToModule) throws IllegalAccessException { moduleLayerNameToModuleField.set(moduleLayer, nameToModule); + accessImpl.rescanField(moduleLayer, moduleLayerNameToModuleField); } - void patchModuleLayerParentsField(ModuleLayer moduleLayer, List parents) throws IllegalAccessException { + void patchModuleLayerParentsField(AnalysisAccessBase accessImpl, ModuleLayer moduleLayer, List parents) throws IllegalAccessException { moduleLayerParentsField.set(moduleLayer, parents); + accessImpl.rescanField(moduleLayer, moduleLayerParentsField); } ClassLoader getClassLoaderForBootLayerModule(String name) { @@ -1093,10 +1110,11 @@ boolean allowsNativeAccess(Module module) { } - void setNativeAccess(Module module, boolean value) { + void setNativeAccess(AfterAnalysisAccessImpl accessImpl, Module module, boolean value) { assert moduleEnableNativeAccessField != null : "Only available on JDK19+"; try { moduleEnableNativeAccessField.set(module, value); + accessImpl.rescanField(module, moduleEnableNativeAccessField); } catch (IllegalAccessException e) { throw VMError.shouldNotReachHere("Failed to reflectively set Module.enableNativeAccess.", e); } From 919a4bb336d89c287fcd575ac17376d4ef28a28a Mon Sep 17 00:00:00 2001 From: Codrut Stancu Date: Tue, 5 Sep 2023 11:47:21 +0200 Subject: [PATCH 2/8] Rescan MethodHandleImpl.TYPED_COLLECTORS. --- .../oracle/svm/hosted/methodhandles/MethodHandleFeature.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/methodhandles/MethodHandleFeature.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/methodhandles/MethodHandleFeature.java index 36b70f1220f0..6172426ff161 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/methodhandles/MethodHandleFeature.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/methodhandles/MethodHandleFeature.java @@ -94,6 +94,7 @@ public class MethodHandleFeature implements InternalFeature { private Field lambdaFormNFIdentity; private Field lambdaFormNFZero; private Field typedAccessors; + private Field typedCollectors; /** * A new {@link MethodType} interning table which contains only objects that are already part of @@ -123,6 +124,8 @@ public void duringSetup(DuringSetupAccess access) { Class arrayAccessorClass = access.findClassByName("java.lang.invoke.MethodHandleImpl$ArrayAccessor"); typedAccessors = ReflectionUtil.lookupField(arrayAccessorClass, "TYPED_ACCESSORS"); + Class methodHandleImplClass = access.findClassByName("java.lang.invoke.MethodHandleImpl$Makers"); + typedCollectors = ReflectionUtil.lookupField(methodHandleImplClass, "TYPED_COLLECTORS"); if (JavaVersionUtil.JAVA_SPEC >= 22) { try { @@ -384,6 +387,7 @@ public void duringAnalysis(DuringAnalysisAccess a) { access.rescanRoot(lambdaFormNFIdentity); access.rescanRoot(lambdaFormNFZero); access.rescanRoot(typedAccessors); + access.rescanRoot(typedCollectors); access.rescanObject(runtimeMethodTypeInternTable); } From 700ad691b9550ab9eb2911812e3e8941a8c1fea7 Mon Sep 17 00:00:00 2001 From: Codrut Stancu Date: Fri, 8 Sep 2023 00:34:04 +0200 Subject: [PATCH 3/8] Rescan DynamicHub.signature. --- .../oracle/svm/hosted/analysis/DynamicHubInitializer.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/analysis/DynamicHubInitializer.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/analysis/DynamicHubInitializer.java index 273be346a8f6..aaca57d73a02 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/analysis/DynamicHubInitializer.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/analysis/DynamicHubInitializer.java @@ -70,6 +70,7 @@ public class DynamicHubInitializer { private final Field dynamicHubClassInitializationInfoField; private final Field dynamicHubArrayHubField; + private final Field dynamicHubSignatureField; private final Field dynamicHubInterfacesEncodingField; private final Field dynamicHubAnnotationsEnumConstantsReferenceField; @@ -83,6 +84,7 @@ public DynamicHubInitializer(BigBang bb) { dynamicHubClassInitializationInfoField = ReflectionUtil.lookupField(DynamicHub.class, "classInitializationInfo"); dynamicHubArrayHubField = ReflectionUtil.lookupField(DynamicHub.class, "arrayHub"); + dynamicHubSignatureField = ReflectionUtil.lookupField(DynamicHub.class, "signature"); dynamicHubInterfacesEncodingField = ReflectionUtil.lookupField(DynamicHub.class, "interfacesEncoding"); dynamicHubAnnotationsEnumConstantsReferenceField = ReflectionUtil.lookupField(DynamicHub.class, "enumConstantsReference"); } @@ -104,7 +106,7 @@ public void initializeMetaData(ImageHeapScanner heapScanner, AnalysisType type) heapScanner.rescanObject(hub, OtherReason.HUB); buildClassInitializationInfo(heapScanner, type, hub); - fillSignature(type, hub); + fillSignature(heapScanner, type, hub); if (type.getJavaKind() == JavaKind.Object) { if (type.isArray()) { @@ -243,7 +245,7 @@ private ClassInitializationInfo buildRuntimeInitializationInfo(AnalysisType type private static final Method getSignature = ReflectionUtil.lookupMethod(Class.class, "getGenericSignature0"); - private static void fillSignature(AnalysisType type, DynamicHub hub) { + private void fillSignature(ImageHeapScanner heapScanner, AnalysisType type, DynamicHub hub) { AnalysisError.guarantee(hub.getSignature() == null, "Signature already computed for %s.", type.toJavaName(true)); Class javaClass = type.getJavaClass(); String signature; @@ -253,6 +255,7 @@ private static void fillSignature(AnalysisType type, DynamicHub hub) { throw GraalError.shouldNotReachHere(e); // ExcludeFromJacocoGeneratedReport } hub.setSignature(signature); + heapScanner.rescanField(hub, dynamicHubSignatureField); } class InterfacesEncodingKey { From 030de248af7d36c1a0925731f5b79c98284fbdbf Mon Sep 17 00:00:00 2001 From: Codrut Stancu Date: Thu, 7 Sep 2023 15:28:46 +0200 Subject: [PATCH 4/8] Fix heap scanning verification. --- .../oracle/graal/pointsto/ObjectScanner.java | 29 +++++++++++++++++-- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/ObjectScanner.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/ObjectScanner.java index 23962ccfdd6a..0444f13fbe60 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/ObjectScanner.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/ObjectScanner.java @@ -157,6 +157,7 @@ protected void scanField(AnalysisField field, JavaConstant receiver, ScanReason /* The value is not available yet. */ return; } + assert isUnwrapped(receiver); JavaConstant fieldValue = bb.getUniverse().getHeapScanner().readFieldValue(field, receiver); if (fieldValue == null) { StringBuilder backtrace = new StringBuilder(); @@ -186,6 +187,26 @@ protected void scanField(AnalysisField field, JavaConstant receiver, ScanReason } } + /** + * Must unwrap the receiver if it is an ImageHeapConstant to scan the hosted value, if any, for + * verification, otherwise the verification just compares shadow heap with shadow heap for + * embedded roots, which is completely useless. + */ + private static JavaConstant maybeUnwrap(JavaConstant receiver) { + if (receiver instanceof ImageHeapConstant heapConstant && heapConstant.getHostedObject() != null) { + return heapConstant.getHostedObject(); + } + return receiver; + } + + private static boolean isUnwrapped(JavaConstant receiver) { + if (receiver instanceof ImageHeapConstant heapConstant) { + // Non hosted backed ImageHeapConstant is considered unwrapped + return heapConstant.getHostedObject() == null; + } + return true; + } + /** * Scans constant arrays, one element at the time. * @@ -193,6 +214,7 @@ protected void scanField(AnalysisField field, JavaConstant receiver, ScanReason */ protected final void scanArray(JavaConstant array, ScanReason prevReason) { + assert isUnwrapped(array); AnalysisType arrayType = bb.getMetaAccess().lookupJavaType(array); ScanReason reason = new ArrayScan(arrayType, array, prevReason); @@ -246,13 +268,14 @@ public void scanConstant(JavaConstant value, ScanReason reason) { bb.registerTypeAsInHeap(bb.getMetaAccess().lookupJavaType(value), reason); return; } - Object valueObj = (value instanceof ImageHeapConstant) ? value : constantAsObject(bb, value); + JavaConstant unwrappedValue = maybeUnwrap(value); + Object valueObj = unwrappedValue instanceof ImageHeapConstant ? unwrappedValue : constantAsObject(bb, unwrappedValue); if (scannedObjects.putAndAcquire(valueObj) == null) { try { - scanningObserver.forScannedConstant(value, reason); + scanningObserver.forScannedConstant(unwrappedValue, reason); } finally { scannedObjects.release(valueObj); - WorklistEntry worklistEntry = new WorklistEntry(value, reason); + WorklistEntry worklistEntry = new WorklistEntry(unwrappedValue, reason); if (executor != null) { executor.execute(debug -> doScan(worklistEntry)); } else { From 9bf0619042a19d0c676bd1dcde233d48311bb53e Mon Sep 17 00:00:00 2001 From: Codrut Stancu Date: Fri, 8 Sep 2023 00:24:55 +0200 Subject: [PATCH 5/8] Refactor heap verification. --- .../pointsto/AbstractAnalysisEngine.java | 4 +- .../oracle/graal/pointsto/ObjectScanner.java | 24 +- .../pointsto/heap/HeapSnapshotVerifier.java | 225 ++++++++---------- .../graal/pointsto/heap/ImageHeapScanner.java | 8 +- .../svm/hosted/heap/SVMImageHeapVerifier.java | 4 +- 5 files changed, 122 insertions(+), 143 deletions(-) diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/AbstractAnalysisEngine.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/AbstractAnalysisEngine.java index eab389132979..eae468a871e8 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/AbstractAnalysisEngine.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/AbstractAnalysisEngine.java @@ -204,10 +204,10 @@ public void runAnalysis(DebugContext debugContext, Function"; - // Checkstyle: disallow Class.getSimpleName + JavaConstant hosted = constant; + if (constant instanceof ImageHeapConstant heapConstant) { + JavaConstant hostedObject = heapConstant.getHostedObject(); + if (hostedObject == null) { + // Checkstyle: allow Class.getSimpleName + return constant.getClass().getSimpleName() + "<" + type.toJavaName() + ">"; + // Checkstyle: disallow Class.getSimpleName + } + hosted = hostedObject; } - Object obj = constantAsObject(bb, constant); + + Object obj = constantAsObject(bb, hosted); String str = type.toJavaName() + '@' + Integer.toHexString(System.identityHashCode(obj)); if (appendToString) { try { @@ -612,15 +618,21 @@ public String toString() { public static class ArrayScan extends ScanReason { final AnalysisType arrayType; + final int idx; public ArrayScan(AnalysisType arrayType, JavaConstant array, ScanReason previous) { + this(arrayType, array, previous, -1); + } + + public ArrayScan(AnalysisType arrayType, JavaConstant array, ScanReason previous, int idx) { super(previous, array); this.arrayType = arrayType; + this.idx = idx; } @Override public String toString(BigBang bb) { - return "indexing into array " + asString(bb, constant); + return "indexing into array " + asString(bb, constant) + (idx != -1 ? " at index " + idx : ""); } @Override diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/HeapSnapshotVerifier.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/HeapSnapshotVerifier.java index 5d498cab83ae..b4edbc8876c0 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/HeapSnapshotVerifier.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/HeapSnapshotVerifier.java @@ -25,7 +25,6 @@ package com.oracle.graal.pointsto.heap; import static com.oracle.graal.pointsto.ObjectScanner.ScanReason; -import static com.oracle.graal.pointsto.ObjectScanner.asString; import java.util.Objects; import java.util.function.Consumer; @@ -73,8 +72,8 @@ public HeapSnapshotVerifier(BigBang bb, ImageHeap imageHeap, ImageHeapScanner sc verbosity = Options.HeapVerifierVerbosity.getValue(bb.getOptions()); } - public boolean requireAnalysisIteration(CompletionExecutor executor) throws InterruptedException { - info("Verifying the heap snapshot..."); + public boolean checkHeapSnapshot(UniverseMetaAccess metaAccess, CompletionExecutor executor, String phase, boolean forAnalysis) { + info("Verifying the heap snapshot %s%s ...", phase, (forAnalysis ? ", iteration " + iterations : "")); analysisModified = false; heapPatched = false; int reachableTypesBefore = bb.getUniverse().getReachableTypes(); @@ -84,24 +83,33 @@ public boolean requireAnalysisIteration(CompletionExecutor executor) throws Inte executor.start(); scanTypes(objectScanner); objectScanner.scanBootImageHeapRoots(); - executor.complete(); + try { + executor.complete(); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } executor.shutdown(); - int verificationReachableTypes = bb.getUniverse().getReachableTypes() - reachableTypesBefore; if (heapPatched) { info("Heap verification patched the heap snapshot."); } else { info("Heap verification didn't find any heap snapshot modifications."); } - if (verificationReachableTypes > 0) { - info("Heap verification made " + verificationReachableTypes + " new types reachable."); - } else { - info("Heap verification didn't make any new types reachable."); - } - if (analysisModified) { - info("Heap verification modified the analysis state. Executing an additional analysis iteration."); - } else { - info("Heap verification didn't modify the analysis state. Heap state stabilized after " + iterations + " iterations."); - info("Exiting analysis."); + int verificationReachableTypes = bb.getUniverse().getReachableTypes() - reachableTypesBefore; + if (forAnalysis) { + if (verificationReachableTypes > 0) { + info("Heap verification made %s new types reachable.", verificationReachableTypes); + } else { + info("Heap verification didn't make any new types reachable."); + } + if (analysisModified) { + info("Heap verification modified the analysis state. Executing an additional analysis iteration."); + } else { + info("Heap verification didn't modify the analysis state. Heap state stabilized after %s iterations.", iterations); + } + } else if (analysisModified || verificationReachableTypes > 0) { + String error = analysisModified ? "modified the analysis state" : ""; + error += verificationReachableTypes > 0 ? (analysisModified ? " and " : "") + "made " + verificationReachableTypes + " new types reachable" : ""; + throw AnalysisError.shouldNotReachHere("Heap verification " + error + ". This is illegal at this stage."); } return analysisModified || verificationReachableTypes > 0; } @@ -114,7 +122,6 @@ protected void scanTypes(@SuppressWarnings("unused") ObjectScanner objectScanner } public void cleanupAfterAnalysis() { - scannedObjects = null; } protected final class ScanningObserver implements ObjectScanningObserver { @@ -124,119 +131,112 @@ public ScanningObserver() { @Override public boolean forRelocatedPointerFieldValue(JavaConstant receiver, AnalysisField field, JavaConstant fieldValue, ScanReason reason) { - boolean result = scanner.getScanningObserver().forRelocatedPointerFieldValue(receiver, field, fieldValue, reason); - if (result) { - analysisModified = true; + boolean result = false; + ObjectScanningObserver scanningObserver = scanner.getScanningObserver(); + if (scanningObserver != null) { + result = scanningObserver.forRelocatedPointerFieldValue(receiver, field, fieldValue, reason); + if (result) { + analysisModified = true; + } } return result; } @Override public boolean forNullFieldValue(JavaConstant receiver, AnalysisField field, ScanReason reason) { - boolean result = scanner.getScanningObserver().forNullFieldValue(receiver, field, reason); - if (result) { - analysisModified = true; + boolean result = false; + ObjectScanningObserver scanningObserver = scanner.getScanningObserver(); + if (scanningObserver != null) { + result = scanningObserver.forNullFieldValue(receiver, field, reason); + if (result) { + analysisModified = true; + } } return result; } @Override - @SuppressWarnings({"unchecked", "rawtypes"}) public boolean forNonNullFieldValue(JavaConstant receiver, AnalysisField field, JavaConstant fieldValue, ScanReason reason) { + /* + * We don't care if a field in the shadow heap was not yet read, i.e., the future is not + * yet materialized. This can happen with lazy fields that become available but may have + * not yet been consumed. We simply execute the future, then compare the produced value. + */ if (field.isStatic()) { TypeData typeData = field.getDeclaringClass().getOrComputeData(); - Object fieldValueTask = typeData.getFieldValue(field); - if (fieldValueTask instanceof JavaConstant fieldSnapshot) { - verifyStaticFieldValue(typeData, field, maybeUnwrapSnapshot(fieldSnapshot, fieldValue instanceof ImageHeapConstant), fieldValue, reason); - } else if (fieldValueTask instanceof AnalysisFuture) { - AnalysisFuture future = (AnalysisFuture) fieldValueTask; - if (future.isDone()) { - JavaConstant fieldSnapshot = future.guardedGet(); - verifyStaticFieldValue(typeData, field, maybeUnwrapSnapshot(fieldSnapshot, fieldValue instanceof ImageHeapConstant), fieldValue, reason); - } else { - onStaticFieldNotComputed(field, fieldValue, reason); - } - } else { - throw AnalysisError.shouldNotReachHere("Unexpected field value " + fieldValueTask + "."); - } + JavaConstant fieldSnapshot = typeData.readFieldValue(field); + verifyStaticFieldValue(typeData, field, maybeUnwrapSnapshot(fieldSnapshot, fieldValue instanceof ImageHeapConstant), fieldValue, reason); } else { ImageHeapInstance receiverObject = (ImageHeapInstance) getReceiverObject(receiver, reason); - Object fieldValueTask = receiverObject.getFieldValue(field); - if (fieldValueTask instanceof JavaConstant fieldSnapshot) { - verifyInstanceFieldValue(field, receiverObject, maybeUnwrapSnapshot(fieldSnapshot, fieldValue instanceof ImageHeapConstant), fieldValue, reason); - } else if (fieldValueTask instanceof AnalysisFuture) { - AnalysisFuture future = (AnalysisFuture) fieldValueTask; - if (future.isDone()) { - JavaConstant fieldSnapshot = future.guardedGet(); - verifyInstanceFieldValue(field, receiverObject, maybeUnwrapSnapshot(fieldSnapshot, fieldValue instanceof ImageHeapConstant), fieldValue, reason); - } else { - /* - * There may be some instance fields not yet computed because the verifier - * can insert new objects for annotation proxy implementations when scanning - * types. The annotations are set lazily, based on reachability, since we - * only want annotations in the heap that are otherwise marked as used. - */ - Consumer onAnalysisModified = (deepReason) -> onInstanceFieldNotComputed(receiverObject, field, fieldValue, deepReason); - scanner.patchInstanceField(receiverObject, field, fieldValue, reason, onAnalysisModified).ensureDone(); - heapPatched = true; - } - } else { - throw AnalysisError.shouldNotReachHere("Unexpected field value " + fieldValueTask + "."); - } + JavaConstant fieldSnapshot = receiverObject.readFieldValue(field); + verifyInstanceFieldValue(field, receiver, receiverObject, maybeUnwrapSnapshot(fieldSnapshot, fieldValue instanceof ImageHeapConstant), fieldValue, reason); } return false; } private void verifyStaticFieldValue(TypeData typeData, AnalysisField field, JavaConstant fieldSnapshot, JavaConstant fieldValue, ScanReason reason) { if (!Objects.equals(fieldSnapshot, fieldValue)) { - Consumer onAnalysisModified = (deepReason) -> onStaticFieldMismatch(field, fieldSnapshot, fieldValue, deepReason); + String format = "Value mismatch for static field %s %n snapshot: %s %n new value: %s %n"; + Consumer onAnalysisModified = analysisModified(reason, format, field, fieldSnapshot, fieldValue); scanner.patchStaticField(typeData, field, fieldValue, reason, onAnalysisModified).ensureDone(); heapPatched = true; } } - private void verifyInstanceFieldValue(AnalysisField field, ImageHeapInstance receiverObject, JavaConstant fieldSnapshot, JavaConstant fieldValue, ScanReason reason) { + private void verifyInstanceFieldValue(AnalysisField field, JavaConstant receiver, ImageHeapInstance receiverObject, JavaConstant fieldSnapshot, JavaConstant fieldValue, ScanReason reason) { if (!Objects.equals(fieldSnapshot, fieldValue)) { - Consumer onAnalysisModified = (deepReason) -> onInstanceFieldMismatch(receiverObject, field, fieldSnapshot, fieldValue, deepReason); + String format = "Value mismatch for instance field %s of %s %n snapshot: %s %n new value: %s %n"; + Consumer onAnalysisModified = analysisModified(reason, format, field, asString(receiver), fieldSnapshot, fieldValue); scanner.patchInstanceField(receiverObject, field, fieldValue, reason, onAnalysisModified).ensureDone(); heapPatched = true; } } + private Consumer analysisModified(ScanReason reason, String format, Object... args) { + if (printAll()) { + warning(reason, format, args); + return (deepReason) -> analysisModified = true; + } else { + return (deepReason) -> { + analysisModified = true; + if (printWarning()) { + analysisWarning(deepReason, format, args); + } + }; + } + } + @Override public boolean forNullArrayElement(JavaConstant array, AnalysisType arrayType, int elementIndex, ScanReason reason) { - boolean result = scanner.getScanningObserver().forNullArrayElement(array, arrayType, elementIndex, reason); - if (result) { - analysisModified = true; + boolean result = false; + ObjectScanningObserver scanningObserver = scanner.getScanningObserver(); + if (scanningObserver != null) { + result = scanningObserver.forNullArrayElement(array, arrayType, elementIndex, reason); + if (result) { + analysisModified = true; + } } return result; } @Override public boolean forNonNullArrayElement(JavaConstant array, AnalysisType arrayType, JavaConstant elementValue, AnalysisType elementType, int index, ScanReason reason) { + /* + * We don't care if an array element in the shadow heap was not yet read, i.e., the + * future is not yet materialized. This can happen with values originating from lazy + * fields that become available but may have not yet been consumed. We simply execute + * the future, then compare the produced value. + */ ImageHeapObjectArray arrayObject = (ImageHeapObjectArray) getReceiverObject(array, reason); - Object elementValueTask = arrayObject.getElement(index); - if (elementValueTask instanceof JavaConstant elementSnapshot) { - verifyArrayElementValue(elementValue, index, reason, arrayObject, elementSnapshot); - } else if (elementValueTask instanceof AnalysisFuture future) { - if (future.isDone()) { - JavaConstant elementSnapshot = (JavaConstant) future.guardedGet(); - verifyArrayElementValue(elementValue, index, reason, arrayObject, elementSnapshot); - } else { - /* There may be some array elements not yet computed. */ - Consumer onAnalysisModified = (deepReason) -> onArrayElementNotComputed(arrayObject, index, elementValue, deepReason); - scanner.patchArrayElement(arrayObject, index, elementValue, reason, onAnalysisModified).ensureDone(); - heapPatched = true; - } - } else { - throw AnalysisError.shouldNotReachHere("Unexpected element value " + elementValueTask + "."); - } + JavaConstant elementSnapshot = arrayObject.readElementValue(index); + verifyArrayElementValue(elementValue, index, reason, array, arrayObject, elementSnapshot); return false; } - private void verifyArrayElementValue(JavaConstant elementValue, int index, ScanReason reason, ImageHeapObjectArray arrayObject, JavaConstant elementSnapshot) { + private void verifyArrayElementValue(JavaConstant elementValue, int index, ScanReason reason, JavaConstant array, ImageHeapObjectArray arrayObject, JavaConstant elementSnapshot) { if (!Objects.equals(maybeUnwrapSnapshot(elementSnapshot, elementValue instanceof ImageHeapConstant), elementValue)) { - Consumer onAnalysisModified = (deepReason) -> onArrayElementMismatch(elementSnapshot, elementValue, deepReason); + String format = "Value mismatch for array element at index %s of %s %n snapshot: %s %n new value: %s %n"; + Consumer onAnalysisModified = analysisModified(reason, format, index, asString(array), elementSnapshot, elementValue); scanner.patchArrayElement(arrayObject, index, elementValue, reason, onAnalysisModified).ensureDone(); heapPatched = true; } @@ -329,7 +329,9 @@ private void ensureTypeScanned(JavaConstant typeConstant, AnalysisType type, Sca @SuppressWarnings({"unchecked", "rawtypes"}) private void ensureTypeScanned(JavaConstant value, JavaConstant typeConstant, AnalysisType type, ScanReason reason) { - AnalysisError.guarantee(type.isReachable(), "The heap snapshot verifier discovered a type not marked as reachable: %s", type); + if (!type.isReachable()) { + error(reason, "The heap snapshot verifier discovered a type not marked as reachable: %s", type); + } Object task = imageHeap.getSnapshot(typeConstant); /* Make sure the DynamicHub value is scanned. */ if (task == null) { @@ -378,47 +380,6 @@ private void onTaskForClassConstantNotDone(JavaConstant object, AnalysisType typ } } - private void onArrayElementMismatch(JavaConstant elementSnapshot, JavaConstant elementValue, ScanReason reason) { - analysisModified = true; - if (printWarning()) { - analysisWarning(reason, "Value mismatch for array element %n snapshot: %s %n new value: %s %n", elementSnapshot, elementValue); - } - } - - private void onStaticFieldMismatch(AnalysisField field, JavaConstant fieldSnapshot, JavaConstant fieldValue, ScanReason reason) { - analysisModified = true; - if (printWarning()) { - analysisWarning(reason, "Value mismatch for static field %s %n snapshot: %s %n new value: %s %n", field, fieldSnapshot, fieldValue); - } - } - - private void onStaticFieldNotComputed(AnalysisField field, JavaConstant fieldValue, ScanReason reason) { - error(reason, "Snapshot not yet computed for static field %s %n new value: %s %n", field, fieldValue); - } - - private void onInstanceFieldMismatch(ImageHeapInstance receiver, AnalysisField field, JavaConstant fieldSnapshot, JavaConstant fieldValue, ScanReason reason) { - analysisModified = true; - if (printWarning()) { - analysisWarning(reason, "Value mismatch for instance field %s of %s %n snapshot: %s %n new value: %s %n", - field, receiver, fieldSnapshot, fieldValue); - } - } - - private void onInstanceFieldNotComputed(ImageHeapInstance receiver, AnalysisField field, JavaConstant fieldValue, ScanReason reason) { - analysisModified = true; - if (printWarning()) { - analysisWarning(reason, "Snapshot not yet computed for instance field %s of %s %n new value: %s %n", - field, receiver, fieldValue); - } - } - - private void onArrayElementNotComputed(ImageHeapArray array, int index, JavaConstant elementValue, ScanReason reason) { - analysisModified = true; - if (printWarning()) { - analysisWarning(reason, "Snapshot not yet computed for array %s, index %s, %n new value: %s %n", array, index, elementValue); - } - } - private static final int INFO = 1; private static final int WARNING = 2; private static final int ALL = 3; @@ -435,9 +396,9 @@ private boolean printAll() { return verbosity >= ALL; } - private void info(String info) { + private void info(String format, Object... args) { if (printInfo()) { - LogUtils.info(info); + LogUtils.info(String.format(format, args)); } } @@ -458,18 +419,22 @@ private String message(ScanReason reason, String format, Object... args) { } private String message(ScanReason reason, String format, String backtraceHeader, Object... args) { - String message = format(format, args); + String message = format(bb, format, args); StringBuilder objectBacktrace = new StringBuilder(); ObjectScanner.buildObjectBacktrace(bb, reason, objectBacktrace, backtraceHeader); message += objectBacktrace; return message; } - private String format(String msg, Object... args) { + private String asString(JavaConstant array) { + return ObjectScanner.asString(bb, array, false); + } + + public static String format(BigBang bb, String msg, Object... args) { if (args != null) { for (int i = 0; i < args.length; ++i) { if (args[i] instanceof JavaConstant) { - args[i] = asString(bb, (JavaConstant) args[i]); + args[i] = ObjectScanner.asString(bb, (JavaConstant) args[i]); } else if (args[i] instanceof AnalysisField) { args[i] = ((AnalysisField) args[i]).format("%H.%n"); } diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageHeapScanner.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageHeapScanner.java index 4f5fcde13628..95bd6d2ea76e 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageHeapScanner.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageHeapScanner.java @@ -460,16 +460,18 @@ protected void onObjectReachable(ImageHeapConstant imageHeapConstant, ScanReason markTypeInstantiated(objectType, reason); if (imageHeapConstant instanceof ImageHeapObjectArray imageHeapArray) { + AnalysisType arrayType = (AnalysisType) imageHeapArray.getType(metaAccess); for (int idx = 0; idx < imageHeapArray.getLength(); idx++) { JavaConstant elementValue = imageHeapArray.readElementValue(idx); - markReachable(elementValue, reason, onAnalysisModified); - notifyAnalysis(imageHeapArray, (AnalysisType) imageHeapArray.getType(metaAccess), idx, reason, onAnalysisModified, elementValue); + ArrayScan arrayScanReason = new ArrayScan(arrayType, imageHeapArray, reason, idx); + markReachable(elementValue, arrayScanReason, onAnalysisModified); + notifyAnalysis(imageHeapArray, arrayType, idx, arrayScanReason, onAnalysisModified, elementValue); } } else if (imageHeapConstant instanceof ImageHeapInstance imageHeapInstance) { for (ResolvedJavaField javaField : objectType.getInstanceFields(true)) { AnalysisField field = (AnalysisField) javaField; if (field.isRead()) { - updateInstanceField(field, imageHeapInstance, reason, onAnalysisModified); + updateInstanceField(field, imageHeapInstance, new FieldScan(field, imageHeapInstance, reason), onAnalysisModified); } } } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageHeapVerifier.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageHeapVerifier.java index 96ffa7677ba5..a38704f4caa4 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageHeapVerifier.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageHeapVerifier.java @@ -43,8 +43,8 @@ public SVMImageHeapVerifier(BigBang bb, ImageHeap imageHeap, ImageHeapScanner sc } @Override - public boolean requireAnalysisIteration(CompletionExecutor executor) throws InterruptedException { - return super.requireAnalysisIteration(executor) || imageStateModified(); + public boolean checkHeapSnapshot(CompletionExecutor executor, String phase, boolean forAnalysis) { + return super.checkHeapSnapshot(executor, phase, forAnalysis) || imageStateModified(); } /** From 2cf470aa2f13486da012830582654d0d3968dcb4 Mon Sep 17 00:00:00 2001 From: Codrut Stancu Date: Fri, 8 Sep 2023 00:31:17 +0200 Subject: [PATCH 6/8] Verify primitive fields. --- .../com/oracle/graal/pointsto/ObjectScanner.java | 10 ++++++++-- .../graal/pointsto/ObjectScanningObserver.java | 7 +++++++ .../graal/pointsto/heap/HeapSnapshotVerifier.java | 11 ++++++++++- .../graal/pointsto/heap/ImageHeapScanner.java | 13 +++++++++++++ .../oracle/svm/hosted/image/NativeImageHeap.java | 15 +++------------ 5 files changed, 41 insertions(+), 15 deletions(-) diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/ObjectScanner.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/ObjectScanner.java index 504b698202a8..883dda92a75b 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/ObjectScanner.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/ObjectScanner.java @@ -98,7 +98,7 @@ public void scanBootImageHeapRoots(Comparator fieldComparator, Co fields = fieldsList; } for (AnalysisField field : fields) { - if (Modifier.isStatic(field.getModifiers()) && field.getJavaKind() == JavaKind.Object && field.isRead()) { + if (Modifier.isStatic(field.getModifiers()) && field.isRead()) { execute(() -> scanRootField(field)); } } @@ -180,6 +180,8 @@ protected void scanField(AnalysisField field, JavaConstant receiver, ScanReason * referenced elements are being scanned. */ scanConstant(fieldValue, reason); + } else if (fieldValue.getJavaKind().isNumericInteger()) { + scanningObserver.forPrimitiveFieldValue(receiver, field, fieldValue, reason); } } catch (UnsupportedFeatureException ex) { @@ -373,6 +375,10 @@ public static String asString(BigBang bb, JavaConstant constant, boolean appendT hosted = hostedObject; } + if (hosted.getJavaKind().isPrimitive()) { + return hosted.toValueString(); + } + Object obj = constantAsObject(bb, hosted); String str = type.toJavaName() + '@' + Integer.toHexString(System.identityHashCode(obj)); if (appendToString) { @@ -410,7 +416,7 @@ private void doScan(WorklistEntry entry) { /* Scan constant's instance fields. */ for (ResolvedJavaField javaField : type.getInstanceFields(true)) { AnalysisField field = (AnalysisField) javaField; - if (field.getJavaKind() == JavaKind.Object && field.isRead()) { + if (field.isRead()) { assert !Modifier.isStatic(field.getModifiers()); scanField(field, entry.constant, entry.reason); } diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/ObjectScanningObserver.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/ObjectScanningObserver.java index d208dbbd5e21..76cec2f397fb 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/ObjectScanningObserver.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/ObjectScanningObserver.java @@ -51,6 +51,13 @@ default boolean forRelocatedPointerFieldValue(JavaConstant receiver, AnalysisFie return false; } + /** + * Hook for scanned value of primitive field. + */ + default boolean forPrimitiveFieldValue(JavaConstant receiver, AnalysisField field, JavaConstant fieldValue, ScanReason reason) { + return false; + } + /** * Hook for scanned null field value. */ diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/HeapSnapshotVerifier.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/HeapSnapshotVerifier.java index b4edbc8876c0..be9717225597 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/HeapSnapshotVerifier.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/HeapSnapshotVerifier.java @@ -142,6 +142,11 @@ public boolean forRelocatedPointerFieldValue(JavaConstant receiver, AnalysisFiel return result; } + @Override + public boolean forPrimitiveFieldValue(JavaConstant receiver, AnalysisField field, JavaConstant fieldValue, ScanReason reason) { + return verifyFieldValue(receiver, field, fieldValue, reason); + } + @Override public boolean forNullFieldValue(JavaConstant receiver, AnalysisField field, ScanReason reason) { boolean result = false; @@ -157,6 +162,10 @@ public boolean forNullFieldValue(JavaConstant receiver, AnalysisField field, Sca @Override public boolean forNonNullFieldValue(JavaConstant receiver, AnalysisField field, JavaConstant fieldValue, ScanReason reason) { + return verifyFieldValue(receiver, field, fieldValue, reason); + } + + private boolean verifyFieldValue(JavaConstant receiver, AnalysisField field, JavaConstant fieldValue, ScanReason reason) { /* * We don't care if a field in the shadow heap was not yet read, i.e., the future is not * yet materialized. This can happen with lazy fields that become available but may have @@ -403,7 +412,7 @@ private void info(String format, Object... args) { } private void warning(ScanReason reason, String format, Object... args) { - LogUtils.warning(message(reason, format, "Object was reached by", args)); + LogUtils.warning(message(reason, format, "Value was reached by", args)); } private void analysisWarning(ScanReason reason, String format, Object... args) { diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageHeapScanner.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageHeapScanner.java index 95bd6d2ea76e..bf8f16b84a93 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageHeapScanner.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageHeapScanner.java @@ -34,6 +34,7 @@ import org.graalvm.collections.EconomicMap; import org.graalvm.collections.MapCursor; import org.graalvm.compiler.api.replacements.SnippetReflectionProvider; +import org.graalvm.compiler.core.common.SuppressFBWarnings; import org.graalvm.compiler.debug.GraalError; import org.graalvm.word.WordBase; @@ -306,6 +307,9 @@ private Optional maybeReplace(JavaConstant constant, ScanReason re } else if (unwrapped instanceof ImageHeapConstant) { throw GraalError.shouldNotReachHere(formatReason("Double wrapping of constant. Most likely, the reachability analysis code itself is seen as reachable.", reason)); // ExcludeFromJacocoGeneratedReport } + if (unwrapped instanceof String || unwrapped instanceof Enum) { + forceHashCodeComputation(unwrapped); + } /* Run all registered object replacers. */ if (constant.getJavaKind() == JavaKind.Object) { @@ -324,6 +328,15 @@ private Optional maybeReplace(JavaConstant constant, ScanReason re return Optional.empty(); } + /** + * For immutable Strings and other objects in the native image heap, force eager computation of + * the hash field. + */ + @SuppressFBWarnings(value = "RV_RETURN_VALUE_IGNORED", justification = "eager hash field computation") + public static void forceHashCodeComputation(Object object) { + object.hashCode(); + } + JavaConstant onFieldValueReachable(AnalysisField field, JavaConstant fieldValue, ScanReason reason, Consumer onAnalysisModified) { return onFieldValueReachable(field, null, ValueSupplier.eagerValue(fieldValue), reason, onAnalysisModified); } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImageHeap.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImageHeap.java index 84bfa0f1b57d..a55a33ceae8b 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImageHeap.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImageHeap.java @@ -44,7 +44,6 @@ import org.graalvm.compiler.api.replacements.Fold; import org.graalvm.compiler.core.common.CompressEncoding; import org.graalvm.compiler.core.common.NumUtil; -import org.graalvm.compiler.core.common.SuppressFBWarnings; import org.graalvm.compiler.core.common.type.CompressibleConstant; import org.graalvm.compiler.core.common.type.TypedConstant; import org.graalvm.nativeimage.ImageSingletons; @@ -53,6 +52,7 @@ import org.graalvm.word.WordBase; import com.oracle.graal.pointsto.heap.ImageHeapConstant; +import com.oracle.graal.pointsto.heap.ImageHeapScanner; import com.oracle.graal.pointsto.meta.AnalysisUniverse; import com.oracle.graal.pointsto.util.AnalysisError; import com.oracle.svm.core.StaticFieldsSupport; @@ -361,7 +361,7 @@ public void addConstant(final JavaConstant constant, boolean immutableFromParent * initialize the hash field. This is safe because Enum.hashCode() is a final method, * i.e., cannot be overwritten by the user. */ - forceHashCodeComputation(enumConstant); + ImageHeapScanner.forceHashCodeComputation(enumConstant); } final ObjectInfo existing = objects.get(uncompressed); @@ -457,7 +457,7 @@ private boolean assertFillerObjectSizes() { } private void handleImageString(final String str) { - forceHashCodeComputation(str); + ImageHeapScanner.forceHashCodeComputation(str); if (HostedStringDeduplication.isInternedString(str)) { /* The string is interned by the host VM, so it must also be interned in our image. */ assert internedStrings.containsKey(str) || internStringsPhase.isAllowed() : "Should not intern string during phase " + internStringsPhase.toString(); @@ -465,15 +465,6 @@ private void handleImageString(final String str) { } } - /** - * For immutable Strings and other objects in the native image heap, force eager computation of - * the hash field. - */ - @SuppressFBWarnings(value = "RV_RETURN_VALUE_IGNORED", justification = "eager hash field computation") - private static void forceHashCodeComputation(Object object) { - object.hashCode(); - } - /** * It has been determined that an object should be added to the model of the native image heap. * This is the mechanics of recursively adding the object and all its fields and array elements From 6e49210b7b2b2750a78413238d73b84c82a06942 Mon Sep 17 00:00:00 2001 From: Codrut Stancu Date: Wed, 6 Sep 2023 22:23:29 +0200 Subject: [PATCH 7/8] Rescan EconomicMapImpl primitive fields. --- .../src/com/oracle/svm/hosted/heap/SVMImageHeapScanner.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageHeapScanner.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageHeapScanner.java index 1b2725da4a0f..ad29e779b0fc 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageHeapScanner.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageHeapScanner.java @@ -68,6 +68,8 @@ public class SVMImageHeapScanner extends ImageHeapScanner { private final Class economicMapImpl; private final Field economicMapImplEntriesField; private final Field economicMapImplHashArrayField; + private final Field economicMapImplTotalEntriesField; + private final Field economicMapImplDeletedEntriesField; private final ReflectionHostedSupport reflectionSupport; private final Class memberNameClass; private final MethodHandleFeature methodHandleSupport; @@ -82,6 +84,8 @@ public SVMImageHeapScanner(BigBang bb, ImageHeap imageHeap, ImageClassLoader loa economicMapImpl = getClass("org.graalvm.collections.EconomicMapImpl"); economicMapImplEntriesField = ReflectionUtil.lookupField(economicMapImpl, "entries"); economicMapImplHashArrayField = ReflectionUtil.lookupField(economicMapImpl, "hashArray"); + economicMapImplTotalEntriesField = ReflectionUtil.lookupField(economicMapImpl, "totalEntries"); + economicMapImplDeletedEntriesField = ReflectionUtil.lookupField(economicMapImpl, "deletedEntries"); ImageSingletons.add(ImageHeapScanner.class, this); reflectionSupport = ImageSingletons.lookup(ReflectionHostedSupport.class); memberNameClass = getClass("java.lang.invoke.MemberName"); @@ -143,6 +147,8 @@ protected void rescanEconomicMap(EconomicMap map) { if (map.getClass() == economicMapImpl) { rescanField(map, economicMapImplEntriesField); rescanField(map, economicMapImplHashArrayField); + rescanField(map, economicMapImplTotalEntriesField); + rescanField(map, economicMapImplDeletedEntriesField); } } From 59751ac73a230f69fc02b30c18781bfe609332c3 Mon Sep 17 00:00:00 2001 From: Codrut Stancu Date: Thu, 7 Sep 2023 15:16:15 +0200 Subject: [PATCH 8/8] Use custom meta access with the object scanner This allows using the object scanner also after analysis, i.e., with the HostedMetaAccess. --- .../heap/StandaloneHeapSnapshotVerifier.java | 3 +- .../pointsto/AbstractAnalysisEngine.java | 2 +- .../oracle/graal/pointsto/ObjectScanner.java | 7 ++++- .../pointsto/heap/HeapSnapshotVerifier.java | 5 ++-- .../svm/hosted/heap/SVMImageHeapVerifier.java | 28 +++++++++++++++++-- 5 files changed, 38 insertions(+), 7 deletions(-) diff --git a/substratevm/src/com.oracle.graal.pointsto.standalone/src/com/oracle/graal/pointsto/standalone/heap/StandaloneHeapSnapshotVerifier.java b/substratevm/src/com.oracle.graal.pointsto.standalone/src/com/oracle/graal/pointsto/standalone/heap/StandaloneHeapSnapshotVerifier.java index 5a747d086c5a..98926736963a 100644 --- a/substratevm/src/com.oracle.graal.pointsto.standalone/src/com/oracle/graal/pointsto/standalone/heap/StandaloneHeapSnapshotVerifier.java +++ b/substratevm/src/com.oracle.graal.pointsto.standalone/src/com/oracle/graal/pointsto/standalone/heap/StandaloneHeapSnapshotVerifier.java @@ -31,6 +31,7 @@ import com.oracle.graal.pointsto.heap.HeapSnapshotVerifier; import com.oracle.graal.pointsto.heap.ImageHeap; import com.oracle.graal.pointsto.heap.ImageHeapScanner; +import com.oracle.graal.pointsto.infrastructure.UniverseMetaAccess; import com.oracle.graal.pointsto.standalone.StandaloneObjectScanner; import com.oracle.graal.pointsto.util.CompletionExecutor; @@ -40,7 +41,7 @@ public StandaloneHeapSnapshotVerifier(BigBang bb, ImageHeap imageHeap, ImageHeap } @Override - protected ObjectScanner installObjectScanner(CompletionExecutor executor) { + protected ObjectScanner installObjectScanner(UniverseMetaAccess metaAccess, CompletionExecutor executor) { StandaloneImageHeapScanner standaloneImageHeapScanner = (StandaloneImageHeapScanner) this.scanner; return new StandaloneObjectScanner(bb, executor, scannedObjects, new ScanningObserver(), standaloneImageHeapScanner.getShouldScanConstant(), standaloneImageHeapScanner.getShouldScanField()); diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/AbstractAnalysisEngine.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/AbstractAnalysisEngine.java index eae468a871e8..b03dbedd7f02 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/AbstractAnalysisEngine.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/AbstractAnalysisEngine.java @@ -207,7 +207,7 @@ public void runAnalysis(DebugContext debugContext, Function 0; } - protected ObjectScanner installObjectScanner(CompletionExecutor executor) { + protected ObjectScanner installObjectScanner(@SuppressWarnings("unused") UniverseMetaAccess metaAccess, CompletionExecutor executor) { return new ObjectScanner(bb, executor, scannedObjects, new ScanningObserver()); } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageHeapVerifier.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageHeapVerifier.java index a38704f4caa4..a2409d35d082 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageHeapVerifier.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageHeapVerifier.java @@ -28,12 +28,16 @@ import com.oracle.graal.pointsto.BigBang; import com.oracle.graal.pointsto.ObjectScanner; +import com.oracle.graal.pointsto.ObjectScanningObserver; import com.oracle.graal.pointsto.heap.HeapSnapshotVerifier; import com.oracle.graal.pointsto.heap.ImageHeap; import com.oracle.graal.pointsto.heap.ImageHeapScanner; +import com.oracle.graal.pointsto.infrastructure.UniverseMetaAccess; +import com.oracle.graal.pointsto.meta.AnalysisField; import com.oracle.graal.pointsto.meta.AnalysisType; import com.oracle.graal.pointsto.util.CompletionExecutor; import com.oracle.svm.hosted.SVMHost; +import com.oracle.svm.hosted.ameta.AnalysisConstantReflectionProvider; import jdk.vm.ci.meta.JavaConstant; @@ -43,8 +47,8 @@ public SVMImageHeapVerifier(BigBang bb, ImageHeap imageHeap, ImageHeapScanner sc } @Override - public boolean checkHeapSnapshot(CompletionExecutor executor, String phase, boolean forAnalysis) { - return super.checkHeapSnapshot(executor, phase, forAnalysis) || imageStateModified(); + public boolean checkHeapSnapshot(UniverseMetaAccess metaAccess, CompletionExecutor executor, String phase, boolean forAnalysis) { + return super.checkHeapSnapshot(metaAccess, executor, phase, forAnalysis) || imageStateModified(); } /** @@ -75,6 +79,26 @@ private void verifyHub(SVMHost svmHost, ObjectScanner objectScanner, AnalysisTyp objectScanner.scanConstant(hubConstant, ObjectScanner.OtherReason.HUB); } + @Override + protected ObjectScanner installObjectScanner(UniverseMetaAccess metaAccess, CompletionExecutor executor) { + return new VerifierObjectScanner(bb, metaAccess, executor, scannedObjects, new ScanningObserver()); + } + + private static final class VerifierObjectScanner extends ObjectScanner { + private final UniverseMetaAccess metaAccess; + + VerifierObjectScanner(BigBang bb, UniverseMetaAccess metaAccess, CompletionExecutor executor, ReusableSet scannedObjects, ObjectScanningObserver scanningObserver) { + super(bb, executor, scannedObjects, scanningObserver); + this.metaAccess = metaAccess; + } + + @Override + protected JavaConstant readFieldValue(AnalysisField field, JavaConstant receiver) { + AnalysisConstantReflectionProvider constantReflectionProvider = (AnalysisConstantReflectionProvider) bb.getConstantReflectionProvider(); + return constantReflectionProvider.readValue(metaAccess, field, receiver, true); + } + } + private SVMHost svmHost() { return (SVMHost) bb.getHostVM(); }