From 0ffe3489e06c045626aab3d3c17dac5d24744c5a Mon Sep 17 00:00:00 2001 From: Codrut Stancu Date: Tue, 2 Sep 2025 14:16:39 +0200 Subject: [PATCH] Fix error in bad toString reporting. --- .../oracle/graal/pointsto/ObjectScanner.java | 2 +- .../graal/pointsto/meta/AnalysisElement.java | 55 +++++++++---------- .../graal/pointsto/meta/AnalysisType.java | 13 +++-- .../substitutions/GraalSubstitutions.java | 10 ++++ 4 files changed, 44 insertions(+), 36 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 7017aa4d6fbe..894bb46e9dd7 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 @@ -747,7 +747,7 @@ public AnalysisMethod getMethod() { @Override public String toString(BigBang bb) { - return "scanning root " + asString(bb, constant) + " embedded in" + System.lineSeparator() + INDENTATION_AFTER_NEWLINE + asStackTraceElement(); + return "scanning root constant " + asString(bb, constant) + " embedded in" + System.lineSeparator() + INDENTATION_AFTER_NEWLINE + asStackTraceElement(); } @Override diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisElement.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisElement.java index 015a07852c36..a7d16307ea50 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisElement.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisElement.java @@ -38,7 +38,6 @@ import java.util.function.BiConsumer; import java.util.function.Consumer; -import com.oracle.graal.pointsto.util.AtomicUtils; import org.graalvm.nativeimage.hosted.Feature.DuringAnalysisAccess; import com.oracle.graal.pointsto.BigBang; @@ -47,6 +46,7 @@ import com.oracle.graal.pointsto.reports.ReportUtils; import com.oracle.graal.pointsto.util.AnalysisError; import com.oracle.graal.pointsto.util.AnalysisFuture; +import com.oracle.graal.pointsto.util.AtomicUtils; import com.oracle.graal.pointsto.util.ConcurrentLightHashSet; import jdk.graal.compiler.debug.GraalError; @@ -309,8 +309,7 @@ private void build() { while (!reasonStack.isEmpty()) { boolean expanded; Object top = reasonStack.peekLast(); - if (top instanceof CompoundReason) { - CompoundReason compoundReason = (CompoundReason) top; + if (top instanceof CompoundReason compoundReason) { if (compoundReason.isFirst()) { compoundReason.storeCurrentIndent(indent); } @@ -378,23 +377,20 @@ private boolean processReason(Object current, String prefix) { if (current instanceof String) { reasonStr = "str: " + current; - } else if (current instanceof AnalysisMethod) { - AnalysisMethod method = (AnalysisMethod) current; - reasonStr = "at " + method.format("%f method %H.%n(%p)") + ", " + methodReasonStr(method); - expanded = methodReason((AnalysisMethod) current); + } else if (current instanceof AnalysisMethod method) { + reasonStr = "at " + method.format("%f method %H.%n(%p)") + " " + methodReasonStr(method); + expanded = methodReason(method); - } else if (current instanceof AnalysisField) { - AnalysisField field = (AnalysisField) current; + } else if (current instanceof AnalysisField field) { reasonStr = "field " + field.format("%H.%n") + " " + fieldReasonStr(field); expanded = fieldReason(field); - } else if (current instanceof AnalysisType) { - AnalysisType type = (AnalysisType) current; - reasonStr = "type " + (type).toJavaName() + " " + typeReasonStr(type); + } else if (current instanceof AnalysisType type) { + reasonStr = "type " + type.toJavaName() + " " + typeReasonStr(type); expanded = typeReason(type); - } else if (current instanceof ResolvedJavaMethod) { - reasonStr = ((ResolvedJavaMethod) current).format("%f method %H.%n"); + } else if (current instanceof ResolvedJavaMethod method) { + reasonStr = method.format("%f method %H.%n"); } else if (current instanceof ResolvedJavaField field) { /* @@ -408,26 +404,23 @@ private boolean processReason(Object current, String prefix) { if (analysisField != null) { return processReason(analysisField, prefix); } else { - reasonStr = "field " + ((ResolvedJavaField) current).format("%H.%n"); + reasonStr = "field " + field.format("%H.%n"); } } else if (current instanceof ResolvedJavaType) { reasonStr = "type " + ((ResolvedJavaType) current).getName(); - } else if (current instanceof BytecodePosition) { - BytecodePosition position = (BytecodePosition) current; + } else if (current instanceof BytecodePosition position) { ResolvedJavaMethod method = position.getMethod(); reasonStr = "at " + method.format("%f") + " method " + method.asStackTraceElement(position.getBCI()) + ", " + methodReasonStr(method); expanded = methodReason(position.getMethod()); - } else if (current instanceof MethodParsing) { - MethodParsing methodParsing = (MethodParsing) current; + } else if (current instanceof MethodParsing methodParsing) { AnalysisMethod method = methodParsing.getMethod(); reasonStr = "at " + method.format("%f method %H.%n(%p)") + ", " + methodReasonStr(method); expanded = methodReason(methodParsing.getMethod()); - } else if (current instanceof ObjectScanner.ScanReason) { - ObjectScanner.ScanReason scanReason = (ObjectScanner.ScanReason) current; + } else if (current instanceof ObjectScanner.ScanReason scanReason) { reasonStr = scanReason.toString(bb); expanded = maybeExpandReasonStack(scanReason.getPrevious()); @@ -447,6 +440,8 @@ private void print(String prefix, String reasonStr) { private boolean typeReason(AnalysisType type) { if (type.isInstantiated()) { return maybeExpandReasonStack(type.getInstantiatedReason()); + } else if (type.isAnySubtypeInstantiated()) { + return maybeExpandReasonStack(type.getAnyInstantiatedSubtype()); } else { return maybeExpandReasonStack(type.getReachableReason()); } @@ -455,6 +450,8 @@ private boolean typeReason(AnalysisType type) { private static String typeReasonStr(AnalysisType type) { if (type.isInstantiated()) { return "is marked as instantiated"; + } else if (type.isAnySubtypeInstantiated()) { + return "has a subtype marked as instantiated"; } return "is reachable"; } @@ -489,8 +486,7 @@ private static String fieldReasonStr(AnalysisField field) { } private boolean methodReason(ResolvedJavaMethod method) { - if (method instanceof AnalysisMethod) { - AnalysisMethod aMethod = (AnalysisMethod) method; + if (method instanceof AnalysisMethod aMethod) { if (aMethod.isSimplyImplementationInvoked()) { if (aMethod.isStatic()) { return maybeExpandReasonStack(aMethod.getImplementationInvokedReason()); @@ -522,29 +518,28 @@ private boolean maybeExpandReasonStack(Object reason) { } private static String methodReasonStr(ResolvedJavaMethod method) { - if (method instanceof AnalysisMethod) { - AnalysisMethod aMethod = (AnalysisMethod) method; + if (method instanceof AnalysisMethod aMethod) { if (aMethod.isSimplyImplementationInvoked()) { if (aMethod.isStatic()) { - return "implementation invoked"; + return "is implementation invoked"; } else { /* For virtual methods we follow back type reachability. */ AnalysisType declaringClass = aMethod.getDeclaringClass(); assert declaringClass.isInstantiated() || declaringClass.isAbstract() || (declaringClass.isInterface() && aMethod.isDefault()) || declaringClass.isReachable() : declaringClass + " is not reachable"; - return "implementation invoked"; + return "is implementation invoked"; } } else if (aMethod.isInlined()) { if (aMethod.isStatic()) { - return "inlined"; + return "is inlined"; } else { AnalysisType declaringClass = aMethod.getDeclaringClass(); assert declaringClass.isInstantiated() || declaringClass.isAbstract() || (declaringClass.isInterface() && aMethod.isDefault()) || declaringClass.isReachable() : declaringClass + " is not reachable"; - return "inlined"; + return "is inlined"; } } else if (aMethod.isIntrinsicMethod()) { - return "intrinsified"; + return "is intrinsified"; } } return ""; diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java index 2ee4350ac4bb..8968c9d664d7 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java @@ -34,7 +34,6 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; -import java.util.concurrent.atomic.AtomicIntegerFieldUpdater; import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; import java.util.function.Consumer; import java.util.function.Function; @@ -100,8 +99,8 @@ public abstract class AnalysisType extends AnalysisElement implements WrappedJav private static final AtomicReferenceFieldUpdater isReachableUpdater = AtomicReferenceFieldUpdater .newUpdater(AnalysisType.class, Object.class, "isReachable"); - private static final AtomicIntegerFieldUpdater isAnySubtypeInstantiatedUpdater = AtomicIntegerFieldUpdater - .newUpdater(AnalysisType.class, "isAnySubtypeInstantiated"); + private static final AtomicReferenceFieldUpdater isAnySubtypeInstantiatedUpdater = AtomicReferenceFieldUpdater + .newUpdater(AnalysisType.class, Object.class, "isAnySubtypeInstantiated"); static final AtomicReferenceFieldUpdater overrideableMethodsUpdater = AtomicReferenceFieldUpdater .newUpdater(AnalysisType.class, Object.class, "overrideableMethods"); @@ -121,7 +120,7 @@ public abstract class AnalysisType extends AnalysisElement implements WrappedJav /** Can be allocated via Unsafe or JNI, i.e., without executing a constructor. */ @SuppressWarnings("unused") private volatile Object isUnsafeAllocated; @SuppressWarnings("unused") private volatile Object isReachable; - @SuppressWarnings("unused") private volatile int isAnySubtypeInstantiated; + @SuppressWarnings("unused") private volatile Object isAnySubtypeInstantiated; private boolean reachabilityListenerNotified; private boolean unsafeFieldsRecomputed; @@ -533,7 +532,7 @@ public boolean registerAsInstantiated(Object reason) { protected void onInstantiated() { assert !isWordType() : Assertions.errorMessage("Word types cannot be instantiated", this); - forAllSuperTypes(superType -> AtomicUtils.atomicMark(superType, isAnySubtypeInstantiatedUpdater)); + forAllSuperTypes(superType -> AtomicUtils.atomicSet(superType, this, isAnySubtypeInstantiatedUpdater)); universe.onTypeInstantiated(this); notifyInstantiatedCallbacks(); @@ -836,6 +835,10 @@ public Object getInstantiatedReason() { return isInstantiated; } + public Object getAnyInstantiatedSubtype() { + return isAnySubtypeInstantiated; + } + public boolean isUnsafeAllocated() { return AtomicUtils.isSet(this, isUnsafeAllocatedUpdater); } diff --git a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/substitutions/GraalSubstitutions.java b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/substitutions/GraalSubstitutions.java index d0b1ccfb320a..908bd3f2d1df 100644 --- a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/substitutions/GraalSubstitutions.java +++ b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/substitutions/GraalSubstitutions.java @@ -32,6 +32,7 @@ import java.io.PrintStream; import java.util.ArrayList; import java.util.List; +import java.util.Set; import org.graalvm.collections.EconomicMap; import org.graalvm.collections.EconomicSet; @@ -373,6 +374,15 @@ static void set(Target_jdk_vm_ci_code_TargetDescription receiver, boolean value) } } +@TargetClass(className = "jdk.graal.compiler.graphio.GraphProtocol") +final class Target_jdk_graal_compiler_graphio_GraphProtocol { + + /** GraphProtocol.badToString can capture hosted only types such as ImageHeapInstance. */ + @Alias// + @RecomputeFieldValue(kind = RecomputeFieldValue.Kind.Reset) // + private static Set> badToString; +} + /** Dummy class to have a class with the file's name. Do not remove. */ public final class GraalSubstitutions { // Dummy