[GR-41279] Enhance reason backtrace.

PullRequest: graal/12900

Author: cstancu

compiler/src/org.graalvm.compiler.replacements/src/org/graalvm/compiler/replacements/GraphKit.java

@@ -239,13 +239,22 @@ public ResolvedJavaMethod findMethod(Class<?> declaringClass, String name, Class
         }
     }
 
+    private NodeSourcePosition invokePosition(int invokeBci) {
+        if (graph.trackNodeSourcePosition()) {
+            NodeSourcePosition currentPosition = graph.currentNodeSourcePosition();
+            assert currentPosition.getCaller() == null : "The GraphKit currentPosition should be a top level position.";
+            return NodeSourcePosition.substitution(currentPosition.getMethod(), invokeBci);
+        }
+        return null;
+    }
+
     /**
      * Creates and appends an {@link InvokeNode} for a call to a given method with a given set of
      * arguments.
      */
     @SuppressWarnings("try")
     public InvokeNode createInvoke(ResolvedJavaMethod method, InvokeKind invokeKind, FrameStateBuilder frameStateBuilder, int bci, ValueNode... args) {
-        try (DebugCloseable context = graph.withNodeSourcePosition(NodeSourcePosition.substitution(graph.currentNodeSourcePosition(), method))) {
+        try (DebugCloseable context = graph.withNodeSourcePosition(invokePosition(bci))) {
             assert method.isStatic() == (invokeKind == InvokeKind.Static);
             Signature signature = method.getSignature();
             JavaType returnType = signature.getReturnType(null);
@@ -282,7 +291,7 @@ public InvokeNode createIntrinsicInvoke(ResolvedJavaMethod method, ValueNode...
     @SuppressWarnings("try")
     public InvokeWithExceptionNode createInvokeWithExceptionAndUnwind(ResolvedJavaMethod method, InvokeKind invokeKind,
                     FrameStateBuilder frameStateBuilder, int invokeBci, ValueNode... args) {
-        try (DebugCloseable context = graph.withNodeSourcePosition(NodeSourcePosition.substitution(graph.currentNodeSourcePosition(), method))) {
+        try (DebugCloseable context = graph.withNodeSourcePosition(invokePosition(invokeBci))) {
             InvokeWithExceptionNode result = startInvokeWithException(method, invokeKind, frameStateBuilder, invokeBci, args);
             exceptionPart();
             ExceptionObjectNode exception = exceptionObject();
@@ -294,7 +303,7 @@ public InvokeWithExceptionNode createInvokeWithExceptionAndUnwind(ResolvedJavaMe
 
     @SuppressWarnings("try")
     public InvokeWithExceptionNode createInvokeWithExceptionAndUnwind(MethodCallTargetNode callTarget, FrameStateBuilder frameStateBuilder, int invokeBci) {
-        try (DebugCloseable context = graph.withNodeSourcePosition(NodeSourcePosition.substitution(graph.currentNodeSourcePosition(), callTarget.targetMethod()))) {
+        try (DebugCloseable context = graph.withNodeSourcePosition(invokePosition(invokeBci))) {
             InvokeWithExceptionNode result = startInvokeWithException(callTarget, frameStateBuilder, invokeBci);
             exceptionPart();
             ExceptionObjectNode exception = exceptionObject();

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/AbstractAnalysisEngine.java

@@ -35,6 +35,9 @@
 import org.graalvm.compiler.debug.DebugContext.Builder;
 import org.graalvm.compiler.debug.DebugHandlersFactory;
 import org.graalvm.compiler.debug.Indent;
+import org.graalvm.compiler.graph.Node;
+import org.graalvm.compiler.nodes.DeoptBciSupplier;
+import org.graalvm.compiler.nodes.ValueNode;
 import org.graalvm.compiler.nodes.spi.Replacements;
 import org.graalvm.compiler.options.OptionValues;
 import org.graalvm.compiler.printer.GraalDebugHandlersFactory;
@@ -55,7 +58,10 @@
 import com.oracle.graal.pointsto.util.Timer;
 import com.oracle.graal.pointsto.util.TimerCollection;
 
+import jdk.vm.ci.code.BytecodeFrame;
+import jdk.vm.ci.code.BytecodePosition;
 import jdk.vm.ci.meta.ConstantReflectionProvider;
+import jdk.vm.ci.meta.ResolvedJavaMethod;
 
 /**
  * This abstract class is shared between Reachability and Points-to. It contains generic methods
@@ -307,4 +313,30 @@ public final boolean executorIsStarted() {
     public Replacements getReplacements() {
         return replacements;
     }
+
+    /**
+     * Provide a non-null position. Some flows like newInstance and invoke require a non-null
+     * position, for others is just better. The constructed position is best-effort, i.e., it
+     * contains at least the method, and a BCI only if the node provides it.
+     * <p>
+     * This is necessary because {@link Node#getNodeSourcePosition()} doesn't always provide a
+     * position, like for example for generated factory methods in FactoryThrowMethodHolder.
+     */
+    public static BytecodePosition sourcePosition(ValueNode node) {
+        BytecodePosition position = node.getNodeSourcePosition();
+        if (position == null) {
+            position = syntheticSourcePosition(node, node.graph().method());
+        }
+        return position;
+    }
+
+    /** Creates a synthetic position for the node in the given method. */
+    public static BytecodePosition syntheticSourcePosition(ValueNode node, ResolvedJavaMethod method) {
+        int bci = BytecodeFrame.UNKNOWN_BCI;
+        if (node instanceof DeoptBciSupplier) {
+            bci = ((DeoptBciSupplier) node).bci();
+        }
+        return new BytecodePosition(null, method, bci);
+    }
+
 }

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/ObjectScanner.java

@@ -37,17 +37,21 @@
 import org.graalvm.word.WordBase;
 
 import com.oracle.graal.pointsto.constraints.UnsupportedFeatureException;
+import com.oracle.graal.pointsto.heap.HeapSnapshotVerifier;
 import com.oracle.graal.pointsto.heap.ImageHeapArray;
 import com.oracle.graal.pointsto.heap.ImageHeapConstant;
+import com.oracle.graal.pointsto.heap.ImageHeapScanner;
 import com.oracle.graal.pointsto.meta.AnalysisField;
 import com.oracle.graal.pointsto.meta.AnalysisMethod;
 import com.oracle.graal.pointsto.meta.AnalysisType;
+import com.oracle.graal.pointsto.reports.ReportUtils;
 import com.oracle.graal.pointsto.util.AnalysisError;
 import com.oracle.graal.pointsto.util.CompletionExecutor;
 
 import jdk.vm.ci.code.BytecodePosition;
 import jdk.vm.ci.meta.JavaConstant;
 import jdk.vm.ci.meta.JavaKind;
+import jdk.vm.ci.meta.ResolvedJavaMethod;
 
 /**
  * Provides functionality for scanning constant objects.
@@ -267,7 +271,7 @@ public final void scanConstant(JavaConstant value, ScanReason reason) {
      * message if the constant is reachable from multiple places.
      */
     public static void unsupportedFeatureDuringConstantScan(BigBang bb, JavaConstant constant, UnsupportedFeatureException e, ScanReason reason) {
-        unsupportedFeature(bb, String.valueOf(constant.hashCode()), e.getMessage(), reason);
+        unsupportedFeature(bb, String.valueOf(receiverHashCode(constant)), e.getMessage(), reason);
     }
 
     /**
@@ -276,7 +280,27 @@ public static void unsupportedFeatureDuringConstantScan(BigBang bb, JavaConstant
      * heap scanning and the heap verification would scan a field that contains an illegal value.
      */
     public static void unsupportedFeatureDuringFieldScan(BigBang bb, AnalysisField field, JavaConstant receiver, UnsupportedFeatureException e, ScanReason reason) {
-        unsupportedFeature(bb, (receiver != null ? receiver.hashCode() + "_" : "") + field.format("%H.%n"), e.getMessage(), reason);
+        unsupportedFeature(bb, (receiver != null ? receiverHashCode(receiver) + "_" : "") + field.format("%H.%n"), e.getMessage(), reason);
+    }
+
+    public static void unsupportedFeatureDuringFieldFolding(BigBang bb, AnalysisField field, JavaConstant receiver, UnsupportedFeatureException e, AnalysisMethod parsedMethod, int bci) {
+        ScanReason reason = new FieldConstantFold(field, parsedMethod, bci, receiver, new MethodParsing(parsedMethod));
+        unsupportedFeature(bb, (receiver != null ? receiverHashCode(receiver) + "_" : "") + field.format("%H.%n"), e.getMessage(), reason);
+    }
+
+    /**
+     * The {@link ImageHeapScanner} may find issue when scanning the {@link ImageHeapConstant}
+     * whereas the {@link HeapSnapshotVerifier} may find issues when scanning the original hosted
+     * objects. Use a consistent hash code as a key to map them to the same error message.
+     */
+    private static int receiverHashCode(JavaConstant receiver) {
+        if (receiver instanceof ImageHeapConstant) {
+            JavaConstant hostedObject = ((ImageHeapConstant) receiver).getHostedObject();
+            if (hostedObject != null) {
+                return hostedObject.hashCode();
+            }
+        }
+        return receiver.hashCode();
     }
 
     public static void unsupportedFeature(BigBang bb, String key, String message, ScanReason reason) {
@@ -311,13 +335,28 @@ public static AnalysisMethod buildObjectBacktrace(BigBang bb, ScanReason reason,
     }
 
     static String asString(BigBang bb, ScanReason reason) {
-        if (reason instanceof FieldScan) {
+        if (reason instanceof MethodParsing) {
+            MethodParsing mp = (MethodParsing) reason;
+            String str = String.format("parsing method %s reachable via the parsing context", mp.getMethod().asStackTraceElement(0));
+            str += ReportUtils.parsingContext(mp.getMethod(), indent + indent);
+            return str;
+        } else if (reason instanceof FieldConstantFold) {
+            FieldConstantFold fieldFold = (FieldConstantFold) reason;
+            StackTraceElement location = fieldFold.parsedMethod.asStackTraceElement(fieldFold.bci);
+            if (fieldFold.field.isStatic()) {
+                return "trying to constant fold static field " + reason + "\n    at " + location;
+            } else {
+                /* Instance field scans must have a receiver, hence the 'of'. */
+                return "trying to constant fold field " + reason + " of constant \n    " + asString(bb, reason.constant) + "\n    at " + location;
+            }
+        } else if (reason instanceof FieldScan) {
             FieldScan fieldScan = (FieldScan) reason;
             if (fieldScan.field.isStatic()) {
-                return "reading static field " + reason;
+                return "reading static field " + reason + "\n    at " + fieldScan.location();
             } else {
                 /* Instance field scans must have a receiver, hence the 'of'. */
                 return "reading field " + reason + " of constant \n    " + asString(bb, reason.constant);
+                // + "\n at " + location;
             }
         } else if (reason instanceof EmbeddedRootScan) {
             return "scanning root " + asString(bb, reason.constant) + " embedded in \n    " + reason;
@@ -472,25 +511,93 @@ public String toString() {
     public static class FieldScan extends ScanReason {
         final AnalysisField field;
 
+        private static ScanReason previous(AnalysisField field) {
+            assert field.isStatic() && field.isRead();
+            Object readBy = field.getReadBy();
+            if (readBy instanceof BytecodePosition) {
+                ResolvedJavaMethod readingMethod = ((BytecodePosition) readBy).getMethod();
+                return new MethodParsing((AnalysisMethod) readingMethod);
+            } else if (readBy instanceof AnalysisMethod) {
+                return new MethodParsing((AnalysisMethod) readBy);
+            } else {
+                return new OtherReason("registered as read because: " + readBy);
+            }
+        }
+
         public FieldScan(AnalysisField field) {
-            this(field, null, null);
+            this(field, null, previous(field));
         }
 
         public FieldScan(AnalysisField field, JavaConstant receiver, ScanReason previous) {
             super(previous, receiver);
             this.field = field;
+            assert field.isRead();
         }
 
         public AnalysisField getField() {
             return field;
         }
 
+        public String location() {
+            Object readBy = field.getReadBy();
+            if (readBy instanceof BytecodePosition) {
+                BytecodePosition position = (BytecodePosition) readBy;
+                return position.getMethod().asStackTraceElement(position.getBCI()).toString();
+            } else if (readBy instanceof AnalysisMethod) {
+                return ((AnalysisMethod) readBy).asStackTraceElement(0).toString();
+            } else {
+                return "<unknown-location>";
+            }
+        }
+
+        @Override
+        public String toString() {
+            return field.format("%H.%n");
+        }
+    }
+
+    public static class FieldConstantFold extends ScanReason {
+        final AnalysisField field;
+        private final AnalysisMethod parsedMethod;
+        private final int bci;
+
+        public FieldConstantFold(AnalysisField field, AnalysisMethod parsedMethod, int bci, JavaConstant receiver, ScanReason previous) {
+            super(previous, receiver);
+            this.field = field;
+            this.parsedMethod = parsedMethod;
+            this.bci = bci;
+        }
+
         @Override
         public String toString() {
             return field.format("%H.%n");
         }
     }
 
+    public static class MethodParsing extends ScanReason {
+        final AnalysisMethod method;
+
+        public MethodParsing(AnalysisMethod method) {
+            this(method, null);
+        }
+
+        public MethodParsing(AnalysisMethod method, ScanReason previous) {
+            super(previous, null);
+            this.method = method;
+        }
+
+        public AnalysisMethod getMethod() {
+            return method;
+        }
+
+        @Override
+        public String toString() {
+            String str = String.format("Parsing method %s %n", method.asStackTraceElement(0));
+            str += "Parsing context:" + ReportUtils.parsingContext(method);
+            return str;
+        }
+    }
+
     public static class ArrayScan extends ScanReason {
         final AnalysisType arrayType;
 
@@ -509,7 +616,7 @@ public static class EmbeddedRootScan extends ScanReason {
         final BytecodePosition position;
 
         public EmbeddedRootScan(BytecodePosition nodeSourcePosition, JavaConstant root) {
-            this(nodeSourcePosition, root, null);
+            this(nodeSourcePosition, root, new MethodParsing((AnalysisMethod) nodeSourcePosition.getMethod()));
         }
 
         public EmbeddedRootScan(BytecodePosition nodeSourcePosition, JavaConstant root, ScanReason previous) {

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/PointsToAnalysis.java

@@ -81,7 +81,6 @@
 import com.oracle.svm.util.ClassUtil;
 import com.oracle.svm.util.ImageGeneratorThreadMarker;
 
-import jdk.vm.ci.code.BytecodePosition;
 import jdk.vm.ci.meta.JavaKind;
 import jdk.vm.ci.meta.JavaType;
 
@@ -347,13 +346,12 @@ public AnalysisMethod addRootMethod(AnalysisMethod aMethod, boolean invokeSpecia
                  * the callee is linked and registered as implementation-invoked.
                  */
                 postTask(() -> {
-                    BytecodePosition location = new BytecodePosition(null, pointsToMethod, 0);
                     if (invokeSpecial) {
                         pointsToMethod.registerAsDirectRootMethod();
                     } else {
                         pointsToMethod.registerAsVirtualRootMethod();
                     }
-                    InvokeTypeFlow invoke = pointsToMethod.initAndGetContextInsensitiveInvoke(PointsToAnalysis.this, location, invokeSpecial);
+                    InvokeTypeFlow invoke = pointsToMethod.initAndGetContextInsensitiveInvoke(PointsToAnalysis.this, null, invokeSpecial);
                     /*
                      * Initialize the type flow of the invoke's actual parameters with the
                      * corresponding parameter declared type. Thus, when the invoke links callees it

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/ReachabilityAnalysis.java

@@ -109,8 +109,8 @@ default void markFieldAccessed(AnalysisField field) {
         field.registerAsAccessed();
     }
 
-    default void markFieldRead(AnalysisField field) {
-        field.registerAsRead(null);
+    default void markFieldRead(AnalysisField field, Object reason) {
+        field.registerAsRead(reason);
     }
 
     default void markFieldWritten(AnalysisField field) {

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/api/PointstoOptions.java

@@ -89,6 +89,9 @@ public class PointstoOptions {
     @Option(help = "Track the callers for methods and accessing methods for fields.")//
     public static final OptionKey<Boolean> TrackAccessChain = new OptionKey<>(false);
 
+    @Option(help = "Limit the parsing context depth. Default value is arbitrary set at 100.")//
+    public static final OptionKey<Integer> ParsingContextMaxDepth = new OptionKey<>(100);
+
     @Option(help = "Track the input for type flows.")//
     public static final OptionKey<Boolean> TrackInputFlows = new OptionKey<>(false);
 

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/MethodTypeFlow.java

@@ -26,7 +26,6 @@
 
 import static jdk.vm.ci.common.JVMCIError.shouldNotReachHere;
 
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
@@ -44,6 +43,8 @@
 import com.oracle.graal.pointsto.typestate.TypeState;
 import com.oracle.graal.pointsto.util.AnalysisError;
 
+import jdk.vm.ci.code.BytecodePosition;
+
 public class MethodTypeFlow extends TypeFlow<AnalysisMethod> {
 
     protected final PointsToAnalysisMethod method;
@@ -87,6 +88,9 @@ protected void ensureFlowsGraphCreated(PointsToAnalysis bb, InvokeTypeFlow reaso
     /* All threads that try to parse the current method synchronize and only the first parses. */
     private synchronized void createFlowsGraph(PointsToAnalysis bb, InvokeTypeFlow reason) {
         if (flowsGraph == null) {
+            AnalysisError.guarantee(reason == null || reason.getSource() == null ||
+                            !reason.getSource().getMethod().equals(method), "Parsing reason cannot be in the target method itself " + method.format("%H.%n"));
+
             parsingReason = reason;
             try {
                 MethodTypeFlowBuilder builder = bb.createMethodTypeFlowBuilder(bb, method);
@@ -163,18 +167,8 @@ public int getReturnedParameterIndex() {
         return returnedParameterIndex;
     }
 
-    public StackTraceElement[] getParsingContext() {
-        List<StackTraceElement> parsingContext = new ArrayList<>();
-        InvokeTypeFlow invokeFlow = parsingReason;
-
-        /* Defend against cycles in the parsing context. GR-35744 should fix this properly. */
-        int maxSize = 100;
-
-        while (invokeFlow != null && parsingContext.size() < maxSize) {
-            parsingContext.add(invokeFlow.getSource().getMethod().asStackTraceElement(invokeFlow.getSource().getBCI()));
-            invokeFlow = ((PointsToAnalysisMethod) invokeFlow.getSource().getMethod()).getTypeFlow().parsingReason;
-        }
-        return parsingContext.toArray(new StackTraceElement[0]);
+    public BytecodePosition getParsingReason() {
+        return parsingReason != null ? parsingReason.getSource() : null;
     }
 
     @Override