From dc33627d6ff524074848c3744b23c312e1b07a18 Mon Sep 17 00:00:00 2001 From: Codrut Stancu Date: Wed, 19 Oct 2022 01:05:21 +0200 Subject: [PATCH 1/4] Refactor parsing context. --- .../graal/pointsto/PointsToAnalysis.java | 4 +-- .../graal/pointsto/api/PointstoOptions.java | 3 ++ .../graal/pointsto/flow/MethodTypeFlow.java | 20 ++++------- .../graal/pointsto/meta/AnalysisMethod.java | 24 ++++++++++++- .../pointsto/meta/PointsToAnalysisMethod.java | 6 ++-- .../graal/pointsto/reports/ReportUtils.java | 20 +++++++++-- .../ReachabilityAnalysisMethod.java | 34 +++++++------------ 7 files changed, 68 insertions(+), 43 deletions(-) diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/PointsToAnalysis.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/PointsToAnalysis.java index 108f5d12d25a..c2171cab4256 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/PointsToAnalysis.java +++ b/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 diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/api/PointstoOptions.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/api/PointstoOptions.java index 0f2a1f9859d5..4b2ac3919dfa 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/api/PointstoOptions.java +++ b/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 TrackAccessChain = new OptionKey<>(false); + @Option(help = "Limit the parsing context depth. Default value is arbitrary set at 100.")// + public static final OptionKey ParsingContextMaxDepth = new OptionKey<>(100); + @Option(help = "Track the input for type flows.")// public static final OptionKey TrackInputFlows = new OptionKey<>(false); diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/MethodTypeFlow.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/MethodTypeFlow.java index 43295274a73d..a07f2d485ac3 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/MethodTypeFlow.java +++ b/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 { 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 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 diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java index 6f96b0435844..f3e4ecdd585e 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java @@ -55,6 +55,7 @@ import com.oracle.graal.pointsto.infrastructure.OriginalMethodProvider; import com.oracle.graal.pointsto.infrastructure.WrappedJavaMethod; import com.oracle.graal.pointsto.infrastructure.WrappedSignature; +import com.oracle.graal.pointsto.reports.ReportUtils; import com.oracle.graal.pointsto.results.StaticAnalysisResults; import com.oracle.graal.pointsto.util.AnalysisError; import com.oracle.graal.pointsto.util.AtomicUtils; @@ -100,6 +101,7 @@ public abstract class AnalysisMethod extends AnalysisElement implements WrappedJ private final String qualifiedName; private final AnalysisType declaringClass; + private final int parsingContextMaxDepth; /** Virtually invoked method registered as root. */ @SuppressWarnings("unused") private volatile int isVirtualRootMethod; @@ -164,6 +166,7 @@ public AnalysisMethod(AnalysisUniverse universe, ResolvedJavaMethod wrapped) { this.qualifiedName = format("%H.%n(%P)"); registerSignatureTypes(); + parsingContextMaxDepth = PointstoOptions.ParsingContextMaxDepth.getValue(universe.hostVM.options()); } /** @@ -224,10 +227,29 @@ public void cleanupAfterAnalysis() { */ public abstract Iterable getInvokes(); + /** + * @return the position of the invocation that triggered parsing for this method, or null + */ + public abstract BytecodePosition getParsingReason(); + /** * @return the parsing context in which given method was parsed */ - public abstract StackTraceElement[] getParsingContext(); + public final StackTraceElement[] getParsingContext() { + List trace = new ArrayList<>(); + BytecodePosition curr = getParsingReason(); + + while (curr != null) { + if (trace.size() > parsingContextMaxDepth) { + trace.add(ReportUtils.truncatedStackTraceSentinel(this)); + break; + } + AnalysisMethod caller = (AnalysisMethod) curr.getMethod(); + trace.add(caller.asStackTraceElement(curr.getBCI())); + curr = caller.getParsingReason(); + } + return trace.toArray(new StackTraceElement[0]); + } public int getId() { return id; diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/PointsToAnalysisMethod.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/PointsToAnalysisMethod.java index 2d8bcbc505e3..e4d10009bf1f 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/PointsToAnalysisMethod.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/PointsToAnalysisMethod.java @@ -97,7 +97,7 @@ public List getInvokeLocations() { for (InvokeTypeFlow invoke : implementationInvokedBy.keySet()) { if (InvokeTypeFlow.isContextInsensitiveVirtualInvoke(invoke)) { locations.addAll(((AbstractVirtualInvokeTypeFlow) invoke).getInvokeLocations()); - } else { + } else if (invoke.getSource() != null) { locations.add(invoke.getSource()); } } @@ -110,8 +110,8 @@ public Iterable getInvokes() { } @Override - public StackTraceElement[] getParsingContext() { - return getTypeFlow().getParsingContext(); + public BytecodePosition getParsingReason() { + return typeFlow.getParsingReason(); } public InvokeTypeFlow initAndGetContextInsensitiveInvoke(PointsToAnalysis bb, BytecodePosition originalLocation, boolean isSpecial) { diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/reports/ReportUtils.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/reports/ReportUtils.java index cb31f7a5e5ee..4eabdb77d05e 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/reports/ReportUtils.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/reports/ReportUtils.java @@ -232,8 +232,14 @@ public static String parsingContext(AnalysisMethod method, int bci, String inden /* Include target method first. */ msg.append(String.format("%n%sat %s", indent, method.asStackTraceElement(bci))); /* Then add the parsing context. */ - for (StackTraceElement e : parsingContext) { - msg.append(String.format("%n%sat %s", indent, e)); + for (int i = 0; i < parsingContext.length; i++) { + StackTraceElement e = parsingContext[i]; + if (isStackTraceTruncationSentinel(e)) { + msg.append(String.format("%n%s", e.getClassName())); + assert i == parsingContext.length - 1; + } else { + msg.append(String.format("%n%sat %s", indent, e)); + } } msg.append(String.format("%n")); } else { @@ -242,6 +248,16 @@ public static String parsingContext(AnalysisMethod method, int bci, String inden return msg.toString(); } + private static final String stackTraceTruncationSentinel = "WARNING: Parsing context is truncated because its depth exceeds a reasonable limit for "; + + private static boolean isStackTraceTruncationSentinel(StackTraceElement element) { + return element.getClassName().startsWith(stackTraceTruncationSentinel); + } + + public static StackTraceElement truncatedStackTraceSentinel(AnalysisMethod method) { + return new StackTraceElement(stackTraceTruncationSentinel + method.format("%H.%n(%p)"), "", null, -1); + } + public static String typePropagationTrace(PointsToAnalysis bb, TypeFlow flow, AnalysisType type) { return typePropagationTrace(bb, flow, type, " "); } diff --git a/substratevm/src/com.oracle.graal.reachability/src/com/oracle/graal/reachability/ReachabilityAnalysisMethod.java b/substratevm/src/com.oracle.graal.reachability/src/com/oracle/graal/reachability/ReachabilityAnalysisMethod.java index 0d65128260e4..784f5279a203 100644 --- a/substratevm/src/com.oracle.graal.reachability/src/com/oracle/graal/reachability/ReachabilityAnalysisMethod.java +++ b/substratevm/src/com.oracle.graal.reachability/src/com/oracle/graal/reachability/ReachabilityAnalysisMethod.java @@ -24,23 +24,25 @@ */ package com.oracle.graal.reachability; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; + +import org.graalvm.compiler.debug.GraalError; +import org.graalvm.compiler.nodes.GraphEncoder; +import org.graalvm.compiler.nodes.Invoke; +import org.graalvm.compiler.nodes.StructuredGraph; + import com.oracle.graal.pointsto.flow.AnalysisParsedGraph; import com.oracle.graal.pointsto.meta.AnalysisMethod; import com.oracle.graal.pointsto.meta.AnalysisUniverse; import com.oracle.graal.pointsto.meta.InvokeInfo; import com.oracle.graal.pointsto.phases.InlineBeforeAnalysis; import com.oracle.graal.pointsto.util.AnalysisError; + import jdk.vm.ci.code.BytecodePosition; import jdk.vm.ci.meta.ResolvedJavaMethod; -import org.graalvm.compiler.debug.GraalError; -import org.graalvm.compiler.nodes.GraphEncoder; -import org.graalvm.compiler.nodes.Invoke; -import org.graalvm.compiler.nodes.StructuredGraph; - -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.List; /** * Reachability specific extension of AnalysisMethod. Contains mainly information necessary to @@ -83,18 +85,8 @@ public Collection getInvokes() { } @Override - public StackTraceElement[] getParsingContext() { - List parsingContext = new ArrayList<>(); - ReachabilityAnalysisMethod curr = this; - - /* Defend against cycles in the parsing context. GR-35744 should fix this properly. */ - int maxSize = 100; - - while (curr != null && parsingContext.size() < maxSize) { - parsingContext.add(curr.asStackTraceElement(curr.reason.getBCI())); - curr = ((ReachabilityAnalysisMethod) curr.reason.getMethod()); - } - return parsingContext.toArray(new StackTraceElement[parsingContext.size()]); + public BytecodePosition getParsingReason() { + return reason; } public void setReason(BytecodePosition reason) { From 44f87bd65b40dc18c2c1458db5bb0a9276a11cd6 Mon Sep 17 00:00:00 2001 From: Codrut Stancu Date: Mon, 10 Oct 2022 22:10:33 +0200 Subject: [PATCH 2/4] Enhance reason backtrace. --- .../pointsto/AbstractAnalysisEngine.java | 28 +++++ .../oracle/graal/pointsto/ObjectScanner.java | 119 +++++++++++++++++- .../graal/pointsto/ReachabilityAnalysis.java | 4 +- .../pointsto/flow/MethodTypeFlowBuilder.java | 102 ++++++--------- .../graal/pointsto/heap/ImageHeapScanner.java | 5 +- .../graal/pointsto/meta/AnalysisField.java | 37 ++++-- .../graal/pointsto/reports/ReportUtils.java | 16 +-- .../graal/pointsto/util/AnalysisError.java | 2 +- .../graal/pointsto/util/AtomicUtils.java | 9 ++ .../DirectMethodProcessingHandler.java | 17 +-- .../MethodSummaryBasedHandler.java | 2 +- .../image/DisallowedImageHeapObjects.java | 4 +- .../com/oracle/svm/hosted/FeatureImpl.java | 8 +- .../svm/hosted/NativeImageGenerator.java | 2 +- .../svm/hosted/SecurityServicesFeature.java | 2 +- .../hosted/SubstrateDiagnosticFeature.java | 4 +- .../flow/SVMMethodTypeFlowBuilder.java | 13 +- .../svm/hosted/jni/JNIAccessFeature.java | 2 +- .../phases/ConstantFoldLoadFieldPlugin.java | 30 ++++- .../HostedTruffleConstantFieldProvider.java | 2 +- 20 files changed, 286 insertions(+), 122 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 e33971427379..03767f57fd57 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 @@ -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,26 @@ 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. + * + * 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) { + int bci = BytecodeFrame.UNKNOWN_BCI; + if (node instanceof DeoptBciSupplier) { + bci = ((DeoptBciSupplier) node).bci(); + } + ResolvedJavaMethod method = node.graph().method(); + position = new BytecodePosition(null, method, bci); + } + return position; + } + } 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 3d95d0b85564..10e3e45083fe 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 @@ -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 ""; + } + } + + @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) { diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/ReachabilityAnalysis.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/ReachabilityAnalysis.java index 3c41ca02e918..9d7a061b4e73 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/ReachabilityAnalysis.java +++ b/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) { diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/MethodTypeFlowBuilder.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/MethodTypeFlowBuilder.java index 11e28920c6cb..61f361fa5f92 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/MethodTypeFlowBuilder.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/MethodTypeFlowBuilder.java @@ -48,7 +48,6 @@ import org.graalvm.compiler.nodes.BeginNode; import org.graalvm.compiler.nodes.CallTargetNode.InvokeKind; import org.graalvm.compiler.nodes.ConstantNode; -import org.graalvm.compiler.nodes.DeoptBciSupplier; import org.graalvm.compiler.nodes.EndNode; import org.graalvm.compiler.nodes.FixedGuardNode; import org.graalvm.compiler.nodes.FixedNode; @@ -111,6 +110,7 @@ import org.graalvm.compiler.virtual.phases.ea.PartialEscapePhase; import org.graalvm.compiler.word.WordCastNode; +import com.oracle.graal.pointsto.AbstractAnalysisEngine; import com.oracle.graal.pointsto.PointsToAnalysis; import com.oracle.graal.pointsto.flow.LoadFieldTypeFlow.LoadInstanceFieldTypeFlow; import com.oracle.graal.pointsto.flow.LoadFieldTypeFlow.LoadStaticFieldTypeFlow; @@ -135,7 +135,6 @@ import com.oracle.graal.pointsto.typestate.TypeState; import com.oracle.graal.pointsto.util.AnalysisError; -import jdk.vm.ci.code.BytecodeFrame; import jdk.vm.ci.code.BytecodePosition; import jdk.vm.ci.meta.Constant; import jdk.vm.ci.meta.JavaConstant; @@ -277,7 +276,7 @@ public void registerUsedElements(boolean registerEmbeddedRoots) { } else if (n instanceof LoadFieldNode) { LoadFieldNode node = (LoadFieldNode) n; AnalysisField field = (AnalysisField) node.field(); - field.registerAsRead(method); + field.registerAsRead(AbstractAnalysisEngine.sourcePosition(node)); } else if (n instanceof StoreFieldNode) { StoreFieldNode node = (StoreFieldNode) n; @@ -324,7 +323,7 @@ public void registerUsedElements(boolean registerEmbeddedRoots) { private void registerEmbeddedRoot(ConstantNode cn) { JavaConstant root = cn.asJavaConstant(); if (bb.scanningPolicy().trackConstant(bb, root)) { - bb.getUniverse().registerEmbeddedRoot(root, sourcePosition(cn)); + bb.getUniverse().registerEmbeddedRoot(root, AbstractAnalysisEngine.sourcePosition(cn)); } } @@ -375,11 +374,11 @@ protected void apply() { FormalParamTypeFlow parameter; if (!isStatic && index == 0) { AnalysisType paramType = method.getDeclaringClass(); - parameter = new FormalReceiverTypeFlow(sourcePosition(node), paramType); + parameter = new FormalReceiverTypeFlow(AbstractAnalysisEngine.sourcePosition(node), paramType); } else { int offset = isStatic ? 0 : 1; AnalysisType paramType = (AnalysisType) method.getSignature().getParameterType(index - offset, method.getDeclaringClass()); - parameter = new FormalParamTypeFlow(sourcePosition(node), paramType, index); + parameter = new FormalParamTypeFlow(AbstractAnalysisEngine.sourcePosition(node), paramType, index); } flowsGraph.setParameter(index, parameter); return parameter; @@ -395,7 +394,7 @@ protected void apply() { AnalysisType type = (AnalysisType) StampTool.typeOrNull(node); TypeFlowBuilder boxBuilder = TypeFlowBuilder.create(bb, node, BoxTypeFlow.class, () -> { - BoxTypeFlow boxFlow = new BoxTypeFlow(sourcePosition(node), type); + BoxTypeFlow boxFlow = new BoxTypeFlow(AbstractAnalysisEngine.sourcePosition(node), type); flowsGraph.addMiscEntryFlow(boxFlow); return boxFlow; }); @@ -415,7 +414,7 @@ protected void apply() { // do nothing } else if (node.asJavaConstant().isNull()) { TypeFlowBuilder sourceBuilder = TypeFlowBuilder.create(bb, node, ConstantTypeFlow.class, () -> { - ConstantTypeFlow constantSource = new ConstantTypeFlow(sourcePosition(node), null, TypeState.forNull()); + ConstantTypeFlow constantSource = new ConstantTypeFlow(AbstractAnalysisEngine.sourcePosition(node), null, TypeState.forNull()); flowsGraph.addMiscEntryFlow(constantSource); return constantSource; }); @@ -431,7 +430,7 @@ protected void apply() { assert type.isInstantiated(); TypeFlowBuilder sourceBuilder = TypeFlowBuilder.create(bb, node, ConstantTypeFlow.class, () -> { JavaConstant heapConstant = bb.getUniverse().getHeapScanner().toImageHeapObject(node.asJavaConstant()); - ConstantTypeFlow constantSource = new ConstantTypeFlow(sourcePosition(node), type, TypeState.forConstant(this.bb, heapConstant, type)); + ConstantTypeFlow constantSource = new ConstantTypeFlow(AbstractAnalysisEngine.sourcePosition(node), type, TypeState.forConstant(this.bb, heapConstant, type)); flowsGraph.addMiscEntryFlow(constantSource); return constantSource; }); @@ -528,7 +527,7 @@ public TypeFlowBuilder lookup(ValueNode n) { */ result = TypeFlowBuilder.create(bb, node, SourceTypeFlow.class, () -> { AnalysisType type = (AnalysisType) stamp.type(); - SourceTypeFlow src = new SourceTypeFlow(sourcePosition(node), type, !stamp.nonNull()); + SourceTypeFlow src = new SourceTypeFlow(AbstractAnalysisEngine.sourcePosition(node), type, !stamp.nonNull()); flowsGraph.addMiscEntryFlow(src); return src; }); @@ -540,7 +539,7 @@ public TypeFlowBuilder lookup(ValueNode n) { */ AnalysisType type = (AnalysisType) (stamp.type() == null ? bb.getObjectType() : stamp.type()); result = TypeFlowBuilder.create(bb, node, TypeFlow.class, () -> { - TypeFlow proxy = bb.analysisPolicy().proxy(sourcePosition(node), type.getTypeFlow(bb, true)); + TypeFlow proxy = bb.analysisPolicy().proxy(AbstractAnalysisEngine.sourcePosition(node), type.getTypeFlow(bb, true)); flowsGraph.addMiscEntryFlow(proxy); return proxy; }); @@ -585,7 +584,7 @@ public boolean merge(AbstractMergeNode merge, List withStates) } else if (mergeFlow != newFlow) { if (newFlow == oldFlow) { newFlow = TypeFlowBuilder.create(bb, merge, MergeTypeFlow.class, () -> { - MergeTypeFlow newMergeFlow = new MergeTypeFlow(sourcePosition(merge)); + MergeTypeFlow newMergeFlow = new MergeTypeFlow(AbstractAnalysisEngine.sourcePosition(merge)); flowsGraph.addMiscEntryFlow(newMergeFlow); return newMergeFlow; }); @@ -631,7 +630,7 @@ private TypeFlowBuilder uniqueReturnFlowBuilder(ReturnNode node) { AnalysisType returnType = (AnalysisType) method.getSignature().getReturnType(method.getDeclaringClass()); if (returnType.getJavaKind() == JavaKind.Object) { returnBuilder = TypeFlowBuilder.create(bb, node, FormalReturnTypeFlow.class, () -> { - FormalReturnTypeFlow resultFlow = new FormalReturnTypeFlow(sourcePosition(node), returnType); + FormalReturnTypeFlow resultFlow = new FormalReturnTypeFlow(AbstractAnalysisEngine.sourcePosition(node), returnType); flowsGraph.setReturnFlow(resultFlow); return resultFlow; }); @@ -653,7 +652,7 @@ private TypeFlowBuilder uniqueInstanceOfFlow(InstanceOfNode node, AnalysisTyp Object key = StaticAnalysisResultsBuilder.uniqueKey(node); return instanceOfFlows.computeIfAbsent(key, (bciKey) -> { TypeFlowBuilder instanceOfBuilder = TypeFlowBuilder.create(bb, node, InstanceOfTypeFlow.class, () -> { - InstanceOfTypeFlow instanceOf = new InstanceOfTypeFlow(sourcePosition(node), declaredType); + InstanceOfTypeFlow instanceOf = new InstanceOfTypeFlow(AbstractAnalysisEngine.sourcePosition(node), declaredType); flowsGraph.addInstanceOf(key, instanceOf); return instanceOf; }); @@ -669,7 +668,7 @@ private void handleCondition(ValueNode source, LogicNode condition, boolean isTr ValueNode object = nullCheck.getValue(); TypeFlowBuilder inputBuilder = state.lookup(object); TypeFlowBuilder nullCheckBuilder = TypeFlowBuilder.create(bb, source, NullCheckTypeFlow.class, () -> { - NullCheckTypeFlow nullCheckFlow = new NullCheckTypeFlow(sourcePosition(source), inputBuilder.get().getDeclaredType(), !isTrue); + NullCheckTypeFlow nullCheckFlow = new NullCheckTypeFlow(AbstractAnalysisEngine.sourcePosition(source), inputBuilder.get().getDeclaredType(), !isTrue); flowsGraph.addNodeFlow(bb, source, nullCheckFlow); return nullCheckFlow; }); @@ -692,8 +691,8 @@ private void handleCondition(ValueNode source, LogicNode condition, boolean isTr * is unique. */ TypeFlowBuilder objectBuilder = state.lookup(object); - BytecodePosition instanceOfPosition = sourcePosition(instanceOf); - if (!bb.strengthenGraalGraphs() && instanceOfPosition != null && instanceOfPosition.getBCI() >= 0) { + BytecodePosition instanceOfPosition = AbstractAnalysisEngine.sourcePosition(instanceOf); + if (!bb.strengthenGraalGraphs() && instanceOfPosition.getBCI() >= 0) { /* * An InstanceOf with negative BCI is not useful. This can happen for example * for instanceof bytecodes for exception unwind. However, the filtering below @@ -715,7 +714,7 @@ private void handleCondition(ValueNode source, LogicNode condition, boolean isTr * of objectFlow (which is context sensitive for exactly our condition). */ TypeFlowBuilder filterBuilder = TypeFlowBuilder.create(bb, source, FilterTypeFlow.class, () -> { - FilterTypeFlow filterFlow = new FilterTypeFlow(sourcePosition(source), type, typeReference.isExact(), isTrue, !isTrue ^ instanceOf.allowsNull()); + FilterTypeFlow filterFlow = new FilterTypeFlow(AbstractAnalysisEngine.sourcePosition(source), type, typeReference.isExact(), isTrue, !isTrue ^ instanceOf.allowsNull()); flowsGraph.addNodeFlow(bb, source, filterFlow); return filterFlow; }); @@ -750,7 +749,7 @@ protected void node(FixedNode n) { for (PhiNode phi : merge.phis()) { if (phi.getStackKind() == JavaKind.Object) { TypeFlowBuilder newFlowBuilder = TypeFlowBuilder.create(bb, merge, MergeTypeFlow.class, () -> { - MergeTypeFlow newFlow = new MergeTypeFlow(sourcePosition(merge)); + MergeTypeFlow newFlow = new MergeTypeFlow(AbstractAnalysisEngine.sourcePosition(merge)); flowsGraph.addMiscEntryFlow(newFlow); return newFlow; }); @@ -778,7 +777,7 @@ protected void node(FixedNode n) { ExceptionObjectNode node = (ExceptionObjectNode) n; TypeFlowBuilder exceptionObjectBuilder = TypeFlowBuilder.create(bb, node, TypeFlow.class, () -> { TypeFlow input = ((AnalysisType) StampTool.typeOrNull(node)).getTypeFlow(bb, false); - TypeFlow exceptionObjectFlow = bb.analysisPolicy().proxy(sourcePosition(node), input); + TypeFlow exceptionObjectFlow = bb.analysisPolicy().proxy(AbstractAnalysisEngine.sourcePosition(node), input); flowsGraph.addMiscEntryFlow(exceptionObjectFlow); return exceptionObjectFlow; }); @@ -836,7 +835,7 @@ protected void node(FixedNode n) { } AnalysisType nonNullInstanceType = Optional.ofNullable(instanceType).orElseGet(bb::getObjectType); TypeFlowBuilder dynamicNewInstanceBuilder = TypeFlowBuilder.create(bb, node, DynamicNewInstanceTypeFlow.class, () -> { - DynamicNewInstanceTypeFlow newInstanceTypeFlow = new DynamicNewInstanceTypeFlow(sourcePosition(node), instanceTypeBuilder.get(), nonNullInstanceType); + DynamicNewInstanceTypeFlow newInstanceTypeFlow = new DynamicNewInstanceTypeFlow(AbstractAnalysisEngine.sourcePosition(node), instanceTypeBuilder.get(), nonNullInstanceType); flowsGraph.addMiscEntryFlow(newInstanceTypeFlow); return newInstanceTypeFlow; }); @@ -862,7 +861,7 @@ protected void node(FixedNode n) { AnalysisType arrayType = bb.getObjectType(); TypeFlowBuilder dynamicNewArrayBuilder = TypeFlowBuilder.create(bb, node, DynamicNewInstanceTypeFlow.class, () -> { - DynamicNewInstanceTypeFlow newArrayTypeFlow = new DynamicNewInstanceTypeFlow(sourcePosition(node), arrayType.getTypeFlow(bb, false), arrayType); + DynamicNewInstanceTypeFlow newArrayTypeFlow = new DynamicNewInstanceTypeFlow(AbstractAnalysisEngine.sourcePosition(node), arrayType.getTypeFlow(bb, false), arrayType); flowsGraph.addMiscEntryFlow(newArrayTypeFlow); return newArrayTypeFlow; }); @@ -874,7 +873,7 @@ protected void node(FixedNode n) { assert type.isInstantiated(); TypeFlowBuilder newArrayBuilder = TypeFlowBuilder.create(bb, node, NewInstanceTypeFlow.class, () -> { - NewInstanceTypeFlow newArray = new NewInstanceTypeFlow(sourcePosition(node), type); + NewInstanceTypeFlow newArray = new NewInstanceTypeFlow(AbstractAnalysisEngine.sourcePosition(node), type); flowsGraph.addMiscEntryFlow(newArray); return newArray; }); @@ -887,7 +886,7 @@ protected void node(FixedNode n) { assert field.isAccessed(); if (node.getStackKind() == JavaKind.Object) { TypeFlowBuilder loadFieldBuilder; - BytecodePosition loadLocation = sourcePosition(node); + BytecodePosition loadLocation = AbstractAnalysisEngine.sourcePosition(node); if (node.isStatic()) { loadFieldBuilder = TypeFlowBuilder.create(bb, node, LoadStaticFieldTypeFlow.class, () -> { FieldTypeFlow fieldFlow = field.getStaticFieldFlow(); @@ -921,7 +920,7 @@ protected void node(FixedNode n) { AnalysisType nonNullArrayType = Optional.ofNullable(arrayType).orElseGet(bb::getObjectArrayType); TypeFlowBuilder loadIndexedBuilder = TypeFlowBuilder.create(bb, node, LoadIndexedTypeFlow.class, () -> { - LoadIndexedTypeFlow loadIndexedFlow = new LoadIndexedTypeFlow(sourcePosition(node), nonNullArrayType, arrayBuilder.get()); + LoadIndexedTypeFlow loadIndexedFlow = new LoadIndexedTypeFlow(AbstractAnalysisEngine.sourcePosition(node), nonNullArrayType, arrayBuilder.get()); flowsGraph.addNodeFlow(bb, node, loadIndexedFlow); return loadIndexedFlow; }); @@ -952,7 +951,7 @@ protected void node(FixedNode n) { TypeFlowBuilder objectBuilder = state.lookup(node.object()); TypeFlowBuilder unsafeLoadBuilder = TypeFlowBuilder.create(bb, node, UnsafePartitionLoadTypeFlow.class, () -> { - UnsafePartitionLoadTypeFlow loadTypeFlow = new UnsafePartitionLoadTypeFlow(sourcePosition(node), objectType, componentType, objectBuilder.get(), + UnsafePartitionLoadTypeFlow loadTypeFlow = new UnsafePartitionLoadTypeFlow(AbstractAnalysisEngine.sourcePosition(node), objectType, componentType, objectBuilder.get(), node.unsafePartitionKind(), partitionType); flowsGraph.addMiscEntryFlow(loadTypeFlow); return loadTypeFlow; @@ -983,7 +982,8 @@ protected void node(FixedNode n) { TypeFlowBuilder valueBuilder = state.lookup(node.value()); TypeFlowBuilder unsafeStoreBuilder = TypeFlowBuilder.create(bb, node, UnsafePartitionStoreTypeFlow.class, () -> { - UnsafePartitionStoreTypeFlow storeTypeFlow = new UnsafePartitionStoreTypeFlow(sourcePosition(node), objectType, componentType, objectBuilder.get(), valueBuilder.get(), + UnsafePartitionStoreTypeFlow storeTypeFlow = new UnsafePartitionStoreTypeFlow(AbstractAnalysisEngine.sourcePosition(node), objectType, componentType, objectBuilder.get(), + valueBuilder.get(), node.partitionKind(), partitionType); flowsGraph.addMiscEntryFlow(storeTypeFlow); return storeTypeFlow; @@ -1003,7 +1003,7 @@ protected void node(FixedNode n) { AnalysisType objectType = (AnalysisType) StampTool.typeOrNull(node.object()); TypeFlowBuilder objectBuilder = state.lookup(node.object()); TypeFlowBuilder loadBuilder; - BytecodePosition loadLocation = sourcePosition(node); + BytecodePosition loadLocation = AbstractAnalysisEngine.sourcePosition(node); if (objectType != null && objectType.isArray() && objectType.getComponentType().getJavaKind() == JavaKind.Object) { /* * Unsafe load from an array object is essentially an array load since we @@ -1041,7 +1041,7 @@ protected void node(FixedNode n) { TypeFlowBuilder objectBuilder = state.lookup(node.object()); TypeFlowBuilder valueBuilder = state.lookup(node.value()); TypeFlowBuilder storeBuilder; - BytecodePosition storeLocation = sourcePosition(node); + BytecodePosition storeLocation = AbstractAnalysisEngine.sourcePosition(node); if (objectType != null && objectType.isArray() && objectType.getComponentType().getJavaKind() == JavaKind.Object) { /* * Unsafe store to an array object is essentially an array store since we @@ -1081,7 +1081,7 @@ protected void node(FixedNode n) { TypeFlowBuilder objectBuilder = state.lookup(object); TypeFlowBuilder newValueBuilder = state.lookup(newValue); TypeFlowBuilder storeBuilder; - BytecodePosition storeLocation = sourcePosition(node); + BytecodePosition storeLocation = AbstractAnalysisEngine.sourcePosition(node); if (objectType != null && objectType.isArray() && objectType.getComponentType().getJavaKind() == JavaKind.Object) { /* * Unsafe compare and swap is essentially unsafe store and unsafe store to @@ -1134,7 +1134,7 @@ protected void node(FixedNode n) { AnalysisType type = (AnalysisType) StampTool.typeOrNull(node.asNode()); TypeFlowBuilder arrayCopyBuilder = TypeFlowBuilder.create(bb, node, ArrayCopyTypeFlow.class, () -> { - ArrayCopyTypeFlow arrayCopyFlow = new ArrayCopyTypeFlow(sourcePosition(node.asNode()), type, srcBuilder.get(), dstBuilder.get()); + ArrayCopyTypeFlow arrayCopyFlow = new ArrayCopyTypeFlow(AbstractAnalysisEngine.sourcePosition(node.asNode()), type, srcBuilder.get(), dstBuilder.get()); flowsGraph.addMiscEntryFlow(arrayCopyFlow); return arrayCopyFlow; }); @@ -1160,7 +1160,7 @@ protected void node(FixedNode n) { /* Word-to-object: Any object can flow out from a low level memory read. */ TypeFlowBuilder wordToObjectBuilder = TypeFlowBuilder.create(bb, node, TypeFlow.class, () -> { /* Use the all-instantiated type flow. */ - TypeFlow objectFlow = bb.analysisPolicy().proxy(sourcePosition(node), bb.getAllInstantiatedTypeFlow()); + TypeFlow objectFlow = bb.analysisPolicy().proxy(AbstractAnalysisEngine.sourcePosition(node), bb.getAllInstantiatedTypeFlow()); flowsGraph.addMiscEntryFlow(objectFlow); return objectFlow; }); @@ -1185,7 +1185,7 @@ protected void node(FixedNode n) { AnalysisType inputType = Optional.ofNullable((AnalysisType) StampTool.typeOrNull(node.getObject())).orElseGet(bb::getObjectType); TypeFlowBuilder cloneBuilder = TypeFlowBuilder.create(bb, node, CloneTypeFlow.class, () -> { - CloneTypeFlow cloneFlow = new CloneTypeFlow(sourcePosition(node.asNode()), inputType, inputBuilder.get()); + CloneTypeFlow cloneFlow = new CloneTypeFlow(AbstractAnalysisEngine.sourcePosition(node.asNode()), inputType, inputBuilder.get()); flowsGraph.addMiscEntryFlow(cloneFlow); return cloneFlow; }); @@ -1196,7 +1196,7 @@ protected void node(FixedNode n) { TypeFlowBuilder objectBuilder = state.lookup(node.object()); TypeFlowBuilder monitorEntryBuilder = TypeFlowBuilder.create(bb, node, MonitorEnterTypeFlow.class, () -> { - MonitorEnterTypeFlow monitorEntryFlow = new MonitorEnterTypeFlow(sourcePosition(node), bb); + MonitorEnterTypeFlow monitorEntryFlow = new MonitorEnterTypeFlow(AbstractAnalysisEngine.sourcePosition(node), bb); flowsGraph.addMiscEntryFlow(monitorEntryFlow); return monitorEntryFlow; }); @@ -1246,7 +1246,7 @@ private void modelUnsafeReadAndWriteFlow(ValueNode node, ValueNode object, Value TypeFlowBuilder storeBuilder; TypeFlowBuilder loadBuilder; - BytecodePosition location = sourcePosition(node); + BytecodePosition location = AbstractAnalysisEngine.sourcePosition(node); if (objectType != null && objectType.isArray() && objectType.getComponentType().getJavaKind() == JavaKind.Object) { /* * Atomic read and write is essentially unsafe store and unsafe store to an @@ -1314,7 +1314,7 @@ protected void processMethodInvocation(TypeFlowsOfNodes state, ValueNode invoke, protected void processMethodInvocation(TypeFlowsOfNodes state, ValueNode invoke, InvokeKind invokeKind, PointsToAnalysisMethod targetMethod, NodeInputList arguments, boolean installResult) { // check if the call is allowed - bb.isCallAllowed(bb, method, targetMethod, sourcePosition(invoke)); + bb.isCallAllowed(bb, method, targetMethod, AbstractAnalysisEngine.sourcePosition(invoke)); /* * Collect the parameters builders into an array so that we don't capture the `state` @@ -1370,7 +1370,7 @@ protected void processMethodInvocation(TypeFlowsOfNodes state, ValueNode invoke, * The invokeLocation is used for all sorts of call stack printing (for error messages * and diagnostics), so we must have a non-null BytecodePosition. */ - BytecodePosition invokeLocation = sourcePosition(invoke); + BytecodePosition invokeLocation = AbstractAnalysisEngine.sourcePosition(invoke); InvokeTypeFlow invokeFlow; switch (invokeKind) { case Static: @@ -1425,7 +1425,7 @@ protected void processMethodInvocation(TypeFlowsOfNodes state, ValueNode invoke, * filter to avoid loosing precision. */ TypeFlowBuilder filterBuilder = TypeFlowBuilder.create(bb, invoke, FilterTypeFlow.class, () -> { - FilterTypeFlow filterFlow = new FilterTypeFlow(sourcePosition(invoke), (AnalysisType) stamp.type(), stamp.isExactType(), true, true); + FilterTypeFlow filterFlow = new FilterTypeFlow(AbstractAnalysisEngine.sourcePosition(invoke), (AnalysisType) stamp.type(), stamp.isExactType(), true, true); flowsGraph.addMiscEntryFlow(filterFlow); return filterFlow; }); @@ -1500,7 +1500,7 @@ protected void processNewInstance(ValueNode node, AnalysisType type, TypeFlowsOf assert type.isInstantiated(); TypeFlowBuilder newInstanceBuilder = TypeFlowBuilder.create(bb, node, NewInstanceTypeFlow.class, () -> { - NewInstanceTypeFlow newInstance = new NewInstanceTypeFlow(sourcePosition(node), type); + NewInstanceTypeFlow newInstance = new NewInstanceTypeFlow(AbstractAnalysisEngine.sourcePosition(node), type); flowsGraph.addMiscEntryFlow(newInstance); return newInstance; }); @@ -1517,7 +1517,7 @@ protected void processStoreField(ValueNode node, AnalysisField field, ValueNode TypeFlowBuilder valueBuilder = state.lookup(value); TypeFlowBuilder storeFieldBuilder; - BytecodePosition storeLocation = sourcePosition(node); + BytecodePosition storeLocation = AbstractAnalysisEngine.sourcePosition(node); if (field.isStatic()) { storeFieldBuilder = TypeFlowBuilder.create(bb, node, StoreFieldTypeFlow.class, () -> { FieldTypeFlow fieldFlow = field.getStaticFieldFlow(); @@ -1552,7 +1552,7 @@ private void processStoreIndexed(ValueNode node, ValueNode array, ValueNode valu TypeFlowBuilder arrayBuilder = state.lookup(array); TypeFlowBuilder valueBuilder = state.lookup(value); TypeFlowBuilder storeIndexedBuilder = TypeFlowBuilder.create(bb, node, StoreIndexedTypeFlow.class, () -> { - StoreIndexedTypeFlow storeIndexedFlow = new StoreIndexedTypeFlow(sourcePosition(node), nonNullArrayType, arrayBuilder.get(), valueBuilder.get()); + StoreIndexedTypeFlow storeIndexedFlow = new StoreIndexedTypeFlow(AbstractAnalysisEngine.sourcePosition(node), nonNullArrayType, arrayBuilder.get(), valueBuilder.get()); flowsGraph.addMiscEntryFlow(storeIndexedFlow); return storeIndexedFlow; }); @@ -1568,24 +1568,4 @@ private void processStoreIndexed(ValueNode node, ValueNode array, ValueNode valu protected void checkUnsafeOffset(@SuppressWarnings("unused") ValueNode base, @SuppressWarnings("unused") ValueNode offset) { } - /** - * 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. - * - * This is necessary because {@link Node#getNodeSourcePosition()} doesn't always provide a - * position, like for example for generated factory methods in FactoryThrowMethodHolder. - */ - protected BytecodePosition sourcePosition(ValueNode node) { - BytecodePosition position = node.getNodeSourcePosition(); - if (position == null) { - int bci = BytecodeFrame.UNKNOWN_BCI; - if (node instanceof DeoptBciSupplier) { - bci = ((DeoptBciSupplier) node).bci(); - } - position = new BytecodePosition(null, method, bci); - } - return position; - } - } 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 161a4166ee7e..36ce8f35803e 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 @@ -255,10 +255,10 @@ protected void scanImageHeapObject(ImageHeapConstant object, ScanReason reason, /* We are about to query the type's fields, the type must be marked as reachable. */ markTypeInstantiated(type); for (AnalysisField field : type.getInstanceFields(true)) { - final ScanReason fieldReason = new FieldScan(field, object, reason); if (field.isRead() && isValueAvailable(field)) { final JavaConstant fieldValue = instance.readFieldValue(field); /* If field is read scan its value immediately. */ + final ScanReason fieldReason = new FieldScan(field, object, reason); onFieldValueReachable(field, instance, fieldValue, fieldReason, onAnalysisModified); } else { /* @@ -297,6 +297,7 @@ protected void scanImageHeapObject(ImageHeapConstant object, ScanReason reason, // imageHeap.setValue(value, (ImageHeapObject) scannedFieldValue); // @formatter:on + final ScanReason fieldReason = new FieldScan(field, object, reason); onFieldValueReachable(field, instance, originalFieldValue, fieldReason, onAnalysisModified); /* Re-install the original constant value. */ instance.setFieldValue(field, originalFieldValue); @@ -362,7 +363,6 @@ protected ImageHeapConstant createImageHeapObject(JavaConstant constant, ScanRea AnalysisField[] instanceFields = type.getInstanceFields(true); newImageHeapConstant = new ImageHeapInstance(type, constant, instanceFields.length); for (AnalysisField field : instanceFields) { - ScanReason fieldReason = new FieldScan(field, constant, reason); ValueSupplier rawFieldValue; try { rawFieldValue = readHostedFieldValue(field, universe.toHosted(constant)); @@ -372,6 +372,7 @@ protected ImageHeapConstant createImageHeapObject(JavaConstant constant, ScanRea } ImageHeapInstance finalObject = (ImageHeapInstance) newImageHeapConstant; finalObject.setFieldTask(field, new AnalysisFuture<>(() -> { + ScanReason fieldReason = new FieldScan(field, constant, reason); JavaConstant value = onFieldValueReachable(field, finalObject, rawFieldValue, fieldReason, onAnalysisModified); finalObject.setFieldValue(field, value); return value; diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisField.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisField.java index 857337022e26..594cf2c5b483 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisField.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisField.java @@ -49,6 +49,7 @@ import com.oracle.svm.util.AnnotationWrapper; import com.oracle.svm.util.UnsafePartitionKind; +import jdk.vm.ci.code.BytecodePosition; import jdk.vm.ci.meta.JavaKind; import jdk.vm.ci.meta.ResolvedJavaField; import jdk.vm.ci.meta.ResolvedJavaType; @@ -62,8 +63,8 @@ public abstract class AnalysisField extends AnalysisElement implements WrappedJa private static final AtomicIntegerFieldUpdater isAccessedUpdater = AtomicIntegerFieldUpdater .newUpdater(AnalysisField.class, "isAccessed"); - private static final AtomicIntegerFieldUpdater isReadUpdater = AtomicIntegerFieldUpdater - .newUpdater(AnalysisField.class, "isRead"); + private static final AtomicReferenceFieldUpdater isReadUpdater = AtomicReferenceFieldUpdater + .newUpdater(AnalysisField.class, Object.class, "isRead"); private static final AtomicIntegerFieldUpdater isWrittenUpdater = AtomicIntegerFieldUpdater .newUpdater(AnalysisField.class, "isWritten"); @@ -94,7 +95,8 @@ public abstract class AnalysisField extends AnalysisElement implements WrappedJa private ContextInsensitiveFieldTypeFlow instanceFieldFlow; @SuppressWarnings("unused") private volatile int isAccessed; - @SuppressWarnings("unused") private volatile int isRead; + /** Contains the {@link BytecodePosition} of the read or a reason object. */ + @SuppressWarnings("unused") private volatile Object isRead; @SuppressWarnings("unused") private volatile int isWritten; @SuppressWarnings("unused") private volatile int isFolded; @@ -110,7 +112,7 @@ public abstract class AnalysisField extends AnalysisElement implements WrappedJa */ private boolean canBeNull; - private ConcurrentMap readBy; + private ConcurrentMap readBy; private ConcurrentMap writtenBy; protected TypeState instanceFieldTypeState; @@ -189,7 +191,7 @@ public void intersectAccessInfos(AnalysisField other) { this.canBeNull = this.canBeNull && other.canBeNull; isWrittenUpdater.set(this, this.isWritten & other.isWritten); isFoldedUpdater.set(this, this.isFolded & other.isFolded); - isReadUpdater.set(this, this.isRead & other.isRead); + isReadUpdater.set(this, this.isRead != null & other.isRead != null ? this.isRead : null); notifyUpdateAccessInfo(); } @@ -198,7 +200,7 @@ public void clearAccessInfos() { this.canBeNull = true; isWrittenUpdater.set(this, 0); isFoldedUpdater.set(this, 0); - isReadUpdater.set(this, 0); + isReadUpdater.set(this, null); notifyUpdateAccessInfo(); } @@ -271,11 +273,16 @@ public boolean registerAsAccessed() { return firstAttempt; } - public boolean registerAsRead(AnalysisMethod method) { - boolean firstAttempt = AtomicUtils.atomicMark(this, isReadUpdater); + /** + * @param reason the {@link BytecodePosition} where a load from this field is seen, or a + * {@link String} describing why this field was manually marked as read + */ + public boolean registerAsRead(Object reason) { + assert reason != null && (!(reason instanceof String) || !reason.equals("")) : "Registering a field as read needs to provide a non-empty reason."; + boolean firstAttempt = AtomicUtils.atomicSet(this, reason, isReadUpdater); notifyUpdateAccessInfo(); - if (readBy != null && method != null) { - readBy.put(method, Boolean.TRUE); + if (readBy != null) { + readBy.put(reason, Boolean.TRUE); } if (firstAttempt) { onReachable(); @@ -373,8 +380,8 @@ public boolean hasUnsafeFrozenTypeState() { return AtomicUtils.isSet(this, unsafeFrozenTypeStateUpdater); } - public Set getReadBy() { - return readBy.keySet(); + public Object getReadBy() { + return isReadUpdater.get(this); } /** @@ -405,6 +412,10 @@ public boolean isAccessed() { (AtomicUtils.isSet(this, isWrittenUpdater) && (Modifier.isVolatile(getModifiers()) || getStorageKind() == JavaKind.Object)); } + private boolean isReadSet() { + return AtomicUtils.isSet(this, isReadUpdater); + } + public boolean isRead() { return AtomicUtils.isSet(this, isAccessedUpdater) || AtomicUtils.isSet(this, isReadUpdater); } @@ -498,7 +509,7 @@ public AnnotatedElement getAnnotationRoot() { @Override public String toString() { - return "AnalysisField<" + format("%h.%n") + " accessed: " + isAccessed + " reads: " + isRead + " written: " + isWritten + " folded: " + isFolded + ">"; + return "AnalysisField<" + format("%h.%n") + " accessed: " + isAccessed + " reads: " + isReadSet() + " written: " + isWritten + " folded: " + isFolded + ">"; } public void markAsUsedInComparison() { diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/reports/ReportUtils.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/reports/ReportUtils.java index 4eabdb77d05e..5a6dc666a5ad 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/reports/ReportUtils.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/reports/ReportUtils.java @@ -45,8 +45,8 @@ 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.meta.InvokeInfo; + import jdk.vm.ci.code.BytecodePosition; import jdk.vm.ci.common.JVMCIError; import jdk.vm.ci.meta.ResolvedJavaMethod; @@ -214,23 +214,25 @@ private static void reportImpl(String description, Path folder, String fileName, } public static String parsingContext(AnalysisMethod method) { - return parsingContext(method, 0, " "); + return parsingContext(method, 0, " ", false); } public static String parsingContext(AnalysisMethod method, String indent) { - return parsingContext(method, 0, indent); + return parsingContext(method, 0, indent, false); } public static String parsingContext(BytecodePosition context) { - return parsingContext((AnalysisMethod) context.getMethod(), context.getBCI(), " "); + return parsingContext((AnalysisMethod) context.getMethod(), context.getBCI(), " ", true); } - public static String parsingContext(AnalysisMethod method, int bci, String indent) { + public static String parsingContext(AnalysisMethod method, int bci, String indent, boolean includeTarget) { StringBuilder msg = new StringBuilder(); StackTraceElement[] parsingContext = method.getParsingContext(); if (parsingContext.length > 0) { - /* Include target method first. */ - msg.append(String.format("%n%sat %s", indent, method.asStackTraceElement(bci))); + if (includeTarget) { + /* Include target method first. */ + msg.append(String.format("%n%sat %s", indent, method.asStackTraceElement(bci))); + } /* Then add the parsing context. */ for (int i = 0; i < parsingContext.length; i++) { StackTraceElement e = parsingContext[i]; diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/util/AnalysisError.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/util/AnalysisError.java index 06b8a1724d3a..b605f4240144 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/util/AnalysisError.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/util/AnalysisError.java @@ -98,7 +98,7 @@ public AnalysisMethod getMethod() { } private static String message(AnalysisMethod method) { - String msg = String.format("Error encountered while parsing %s %n", method.format("%H.%n(%P)")); + String msg = String.format("Error encountered while parsing %s %n", method.asStackTraceElement(0)); msg += "Parsing context:" + ReportUtils.parsingContext(method); return msg; } diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/util/AtomicUtils.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/util/AtomicUtils.java index 0ed817f6234e..adcfa684d735 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/util/AtomicUtils.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/util/AtomicUtils.java @@ -27,11 +27,20 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicIntegerFieldUpdater; import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; import java.util.function.Consumer; import java.util.function.Supplier; public class AtomicUtils { + public static boolean atomicSet(T holder, V value, AtomicReferenceFieldUpdater updater) { + return updater.compareAndSet(holder, null, value); + } + + public static boolean isSet(T holder, AtomicReferenceFieldUpdater updater) { + return updater.get(holder) != null; + } + /** * Atomically set the field to 1 if the current value is 0. * diff --git a/substratevm/src/com.oracle.graal.reachability/src/com/oracle/graal/reachability/DirectMethodProcessingHandler.java b/substratevm/src/com.oracle.graal.reachability/src/com/oracle/graal/reachability/DirectMethodProcessingHandler.java index f8ac9238975b..84c1906c9f1b 100644 --- a/substratevm/src/com.oracle.graal.reachability/src/com/oracle/graal/reachability/DirectMethodProcessingHandler.java +++ b/substratevm/src/com.oracle.graal.reachability/src/com/oracle/graal/reachability/DirectMethodProcessingHandler.java @@ -24,10 +24,9 @@ */ package com.oracle.graal.reachability; -import com.oracle.graal.pointsto.meta.AnalysisMethod; -import jdk.vm.ci.meta.JavaConstant; -import jdk.vm.ci.meta.ResolvedJavaMethod; -import jdk.vm.ci.meta.ResolvedJavaType; +import java.lang.reflect.Modifier; +import java.util.Optional; + import org.graalvm.compiler.core.common.spi.ForeignCallDescriptor; import org.graalvm.compiler.core.common.spi.ForeignCallSignature; import org.graalvm.compiler.graph.Node; @@ -50,8 +49,12 @@ import org.graalvm.compiler.replacements.nodes.UnaryMathIntrinsicNode; import org.graalvm.nativeimage.AnnotationAccess; -import java.lang.reflect.Modifier; -import java.util.Optional; +import com.oracle.graal.pointsto.AbstractAnalysisEngine; +import com.oracle.graal.pointsto.meta.AnalysisMethod; + +import jdk.vm.ci.meta.JavaConstant; +import jdk.vm.ci.meta.ResolvedJavaMethod; +import jdk.vm.ci.meta.ResolvedJavaType; /** * This handler walks the structured graphs of methods and directly calls back into the @@ -120,7 +123,7 @@ private static void analyzeStructuredGraph(ReachabilityAnalysisEngine bb, Reacha bb.markTypeReachable((ReachabilityAnalysisType) node.type().getType()); } else if (n instanceof LoadFieldNode) { LoadFieldNode node = (LoadFieldNode) n; - bb.markFieldRead((ReachabilityAnalysisField) node.field()); + bb.markFieldRead((ReachabilityAnalysisField) node.field(), AbstractAnalysisEngine.sourcePosition(node)); } else if (n instanceof StoreFieldNode) { StoreFieldNode node = (StoreFieldNode) n; bb.markFieldWritten((ReachabilityAnalysisField) node.field()); diff --git a/substratevm/src/com.oracle.graal.reachability/src/com/oracle/graal/reachability/MethodSummaryBasedHandler.java b/substratevm/src/com.oracle.graal.reachability/src/com/oracle/graal/reachability/MethodSummaryBasedHandler.java index a7ea23603a44..0057f23bcf91 100644 --- a/substratevm/src/com.oracle.graal.reachability/src/com/oracle/graal/reachability/MethodSummaryBasedHandler.java +++ b/substratevm/src/com.oracle.graal.reachability/src/com/oracle/graal/reachability/MethodSummaryBasedHandler.java @@ -87,7 +87,7 @@ private static void processSummary(ReachabilityAnalysisEngine bb, ReachabilityAn bb.markTypeInstantiated(type); } for (AnalysisField field : summary.readFields) { - bb.markFieldRead(field); + bb.markFieldRead(field, method); bb.markTypeReachable(field.getType()); } for (AnalysisField field : summary.writtenFields) { diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/image/DisallowedImageHeapObjects.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/image/DisallowedImageHeapObjects.java index e86e9131a3ea..69c9c8ddd616 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/image/DisallowedImageHeapObjects.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/image/DisallowedImageHeapObjects.java @@ -111,8 +111,8 @@ public static void check(Object obj, DisallowedObjectReporter reporter) { */ if (buffer.capacity() != 0 || getFileDescriptor(buffer) != null) { throw reporter.raise("Detected a direct/mapped ByteBuffer in the image heap. " + - "A direct ByteBuffer has a pointer to unmanaged C memory, and C memory from the image generator is not available at image runtime." + - "A mapped ByteBuffer references a file descriptor, which is no longer open and mapped at run time. ", + "A direct ByteBuffer has a pointer to unmanaged C memory, and C memory from the image generator is not available at image runtime. " + + "A mapped ByteBuffer references a file descriptor, which is no longer open and mapped at run time.", buffer, "Try avoiding to initialize the class that caused initialization of the MappedByteBuffer."); } } else if (obj instanceof Buffer && ((Buffer) obj).isDirect()) { diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/FeatureImpl.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/FeatureImpl.java index c319c20bde20..c942ed499d28 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/FeatureImpl.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/FeatureImpl.java @@ -367,13 +367,13 @@ public void registerAsAccessed(AnalysisField aField) { bb.markFieldAccessed(aField); } - public void registerAsRead(Field field) { + public void registerAsRead(Field field, Object reason) { getMetaAccess().lookupJavaType(field.getDeclaringClass()).registerAsReachable(); - registerAsRead(getMetaAccess().lookupJavaField(field)); + registerAsRead(getMetaAccess().lookupJavaField(field), reason); } - public void registerAsRead(AnalysisField aField) { - bb.markFieldRead(aField); + public void registerAsRead(AnalysisField aField, Object reason) { + bb.markFieldRead(aField, reason); } @Override diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java index 07cd1c91352d..198083671fcc 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java @@ -1238,7 +1238,7 @@ public static void registerGraphBuilderPlugins(FeatureHandler featureHandler, Ru ((AnalysisType) resolvedJavaType).registerAsReachable(); } plugins.appendNodePlugin(new EarlyConstantFoldLoadFieldPlugin(providers.getMetaAccess(), providers.getSnippetReflection())); - plugins.appendNodePlugin(new ConstantFoldLoadFieldPlugin()); + plugins.appendNodePlugin(new ConstantFoldLoadFieldPlugin(reason)); plugins.appendNodePlugin(new CInterfaceInvocationPlugin(providers.getMetaAccess(), providers.getWordTypes(), nativeLibs)); plugins.appendNodePlugin(new LocalizationFeature.CharsetNodePlugin()); diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/SecurityServicesFeature.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/SecurityServicesFeature.java index 21818c2ebbfb..2d7accdf3088 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/SecurityServicesFeature.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/SecurityServicesFeature.java @@ -971,7 +971,7 @@ private static TracingAutoCloseable trace(DuringAnalysisAccess a, Object trigger Method method = (Method) trigger; DuringAnalysisAccessImpl access = (DuringAnalysisAccessImpl) a; AnalysisMethod analysisMethod = access.getMetaAccess().lookupJavaMethod(method); - String msg = String.format("Service factory method %s is reachable.%n", analysisMethod.format("%H.%n(%P)")); + String msg = String.format("Service factory method %s is reachable.%n", analysisMethod.asStackTraceElement(0)); msg += String.format("%sAnalysis parsing context: %s", indent, ReportUtils.parsingContext(analysisMethod, indent + " ")); msg += String.format("%sReachability of %s service type API triggers registration of following services:%n", indent, serviceType); return msg; diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/SubstrateDiagnosticFeature.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/SubstrateDiagnosticFeature.java index d7d5bb95a8fa..d24ef25b448f 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/SubstrateDiagnosticFeature.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/SubstrateDiagnosticFeature.java @@ -34,8 +34,8 @@ import com.oracle.svm.core.SubstrateDiagnostics.DiagnosticThunkRegistry; import com.oracle.svm.core.SubstrateDiagnostics.FatalErrorState; import com.oracle.svm.core.SubstrateOptions; -import com.oracle.svm.core.feature.InternalFeature; import com.oracle.svm.core.feature.AutomaticallyRegisteredFeature; +import com.oracle.svm.core.feature.InternalFeature; import com.oracle.svm.core.util.VMError; import com.oracle.svm.hosted.FeatureImpl.BeforeAnalysisAccessImpl; @@ -61,7 +61,7 @@ private static void registerOptionAsRead(BeforeAnalysisAccessImpl accessImpl, Cl try { Field javaField = clazz.getField(fieldName); AnalysisField analysisField = accessImpl.getMetaAccess().lookupJavaField(javaField); - accessImpl.registerAsRead(analysisField); + accessImpl.registerAsRead(analysisField, "it is a runtime option field"); } catch (NoSuchFieldException | SecurityException e) { throw VMError.shouldNotReachHere(e); } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/analysis/flow/SVMMethodTypeFlowBuilder.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/analysis/flow/SVMMethodTypeFlowBuilder.java index dfcb80e918c0..091b0bbc30f8 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/analysis/flow/SVMMethodTypeFlowBuilder.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/analysis/flow/SVMMethodTypeFlowBuilder.java @@ -36,6 +36,7 @@ import org.graalvm.compiler.nodes.ValueNode; import org.graalvm.compiler.nodes.java.LoadFieldNode; +import com.oracle.graal.pointsto.AbstractAnalysisEngine; import com.oracle.graal.pointsto.PointsToAnalysis; import com.oracle.graal.pointsto.flow.MethodTypeFlowBuilder; import com.oracle.graal.pointsto.flow.TypeFlow; @@ -146,7 +147,7 @@ protected void checkUnsafeOffset(ValueNode base, ValueNode offsetNode) { * if it was properly intercepted or not is LoadFieldNode. */ - BytecodePosition pos = sourcePosition(offsetNode); + BytecodePosition pos = AbstractAnalysisEngine.sourcePosition(offsetNode); if (offsetNode instanceof LoadFieldNode) { LoadFieldNode offsetLoadNode = (LoadFieldNode) offsetNode; AnalysisField field = (AnalysisField) offsetLoadNode.field(); @@ -156,17 +157,13 @@ protected void checkUnsafeOffset(ValueNode base, ValueNode offsetNode) { !(field.wrapped instanceof ComputedValueField) && !(base.isConstant() && base.asConstant().isDefaultForKind())) { String message = String.format("Field %s is used as an offset in an unsafe operation, but no value recomputation found.%n Wrapped field: %s", field, field.wrapped); - if (pos != null) { - message += String.format("%n Location: %s", pos); - } + message += String.format("%n Location: %s", pos); UnsafeOffsetError.report(message); } } else if (NativeImageOptions.ReportUnsafeOffsetWarnings.getValue()) { String message = "Offset used in an unsafe operation. Cannot determine if the offset value is recomputed."; message += String.format("%nNode class: %s", offsetNode.getClass().getName()); - if (pos != null) { - message += String.format("%n Location: %s", pos); - } + message += String.format("%n Location: %s", pos); if (NativeImageOptions.UnsafeOffsetWarningsAreFatal.getValue()) { UnsafeOffsetError.report(message); } else { @@ -205,7 +202,7 @@ private void storeVMThreadLocal(TypeFlowsOfNodes state, ValueNode storeNode, Val AnalysisType valueType = (AnalysisType) (valueStamp.type() == null ? bb.getObjectType() : valueStamp.type()); TypeFlowBuilder storeBuilder = TypeFlowBuilder.create(bb, storeNode, TypeFlow.class, () -> { - TypeFlow proxy = bb.analysisPolicy().proxy(sourcePosition(storeNode), valueType.getTypeFlow(bb, false)); + TypeFlow proxy = bb.analysisPolicy().proxy(AbstractAnalysisEngine.sourcePosition(storeNode), valueType.getTypeFlow(bb, false)); flowsGraph.addMiscEntryFlow(proxy); return proxy; }); diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/jni/JNIAccessFeature.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/jni/JNIAccessFeature.java index 905296dc7994..2bbd7449a62f 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/jni/JNIAccessFeature.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/jni/JNIAccessFeature.java @@ -418,7 +418,7 @@ private static void addField(Field reflField, boolean writable, DuringAnalysisAc AnalysisField field = access.getMetaAccess().lookupJavaField(reflField); jniClass.addFieldIfAbsent(field.getName(), name -> new JNIAccessibleField(jniClass, field.getJavaKind(), field.getModifiers())); field.registerAsJNIAccessed(); - field.registerAsRead(null); + field.registerAsRead("it is registered for JNI access"); if (writable) { field.registerAsWritten(null); AnalysisType fieldType = field.getType(); diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/ConstantFoldLoadFieldPlugin.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/ConstantFoldLoadFieldPlugin.java index e4fe8a2e2732..9026409daa70 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/ConstantFoldLoadFieldPlugin.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/ConstantFoldLoadFieldPlugin.java @@ -30,10 +30,23 @@ import org.graalvm.compiler.nodes.graphbuilderconf.NodePlugin; import org.graalvm.compiler.nodes.util.ConstantFoldUtil; +import com.oracle.graal.pointsto.ObjectScanner; +import com.oracle.graal.pointsto.constraints.UnsupportedFeatureException; +import com.oracle.graal.pointsto.meta.AnalysisField; +import com.oracle.graal.pointsto.meta.AnalysisMetaAccess; +import com.oracle.graal.pointsto.meta.AnalysisMethod; +import com.oracle.svm.core.ParsingReason; +import com.oracle.svm.core.graal.nodes.LoweredDeadEndNode; + import jdk.vm.ci.meta.JavaConstant; import jdk.vm.ci.meta.ResolvedJavaField; public final class ConstantFoldLoadFieldPlugin implements NodePlugin { + private final ParsingReason reason; + + public ConstantFoldLoadFieldPlugin(ParsingReason reason) { + this.reason = reason; + } @Override public boolean handleLoadField(GraphBuilderContext b, ValueNode receiver, ResolvedJavaField field) { @@ -49,8 +62,21 @@ public boolean handleLoadStaticField(GraphBuilderContext b, ResolvedJavaField st return tryConstantFold(b, staticField, null); } - private static boolean tryConstantFold(GraphBuilderContext b, ResolvedJavaField field, JavaConstant receiver) { - ConstantNode result = ConstantFoldUtil.tryConstantFold(b.getConstantFieldProvider(), b.getConstantReflection(), b.getMetaAccess(), field, receiver, b.getOptions()); + private boolean tryConstantFold(GraphBuilderContext b, ResolvedJavaField field, JavaConstant receiver) { + ConstantNode result; + try { + result = ConstantFoldUtil.tryConstantFold(b.getConstantFieldProvider(), b.getConstantReflection(), b.getMetaAccess(), field, receiver, b.getOptions()); + } catch (UnsupportedFeatureException e) { + if (reason == ParsingReason.PointsToAnalysis) { + AnalysisMetaAccess metaAccess = (AnalysisMetaAccess) b.getMetaAccess(); + ObjectScanner.unsupportedFeatureDuringFieldFolding(metaAccess.getUniverse().getBigbang(), (AnalysisField) field, receiver, e, (AnalysisMethod) b.getMethod(), b.bci()); + // kill control flow, the image build fails anyway + b.add(new LoweredDeadEndNode()); + return true; + } else { + throw e; + } + } if (result != null) { assert result.asJavaConstant() != null; diff --git a/substratevm/src/com.oracle.svm.truffle/src/com/oracle/svm/truffle/HostedTruffleConstantFieldProvider.java b/substratevm/src/com.oracle.svm.truffle/src/com/oracle/svm/truffle/HostedTruffleConstantFieldProvider.java index d6ab0811bc46..8f3949143650 100644 --- a/substratevm/src/com.oracle.svm.truffle/src/com/oracle/svm/truffle/HostedTruffleConstantFieldProvider.java +++ b/substratevm/src/com.oracle.svm.truffle/src/com/oracle/svm/truffle/HostedTruffleConstantFieldProvider.java @@ -59,7 +59,7 @@ public T readConstantField(ResolvedJavaField field, ConstantFieldTool too * during static analysis. So the runtime graph can be the only place where a read * occurs, therefore we explicitly mark the field as read. */ - ((AnalysisField) field).registerAsRead(null); + ((AnalysisField) field).registerAsRead("it is annotated with " + CompilationFinal.class.getName()); } return null; } From b03a7dc09fb3ecc9c6ddd4a5667a6795289ddb90 Mon Sep 17 00:00:00 2001 From: Codrut Stancu Date: Tue, 18 Oct 2022 22:06:05 +0200 Subject: [PATCH 3/4] Reconstruct invoke source position for macro node and inlined invokes. --- .../pointsto/AbstractAnalysisEngine.java | 18 +++-- .../pointsto/flow/MethodTypeFlowBuilder.java | 72 +++++++++++++++---- .../graal/pointsto/reports/ReportUtils.java | 26 ++++--- 3 files changed, 85 insertions(+), 31 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 03767f57fd57..a1b85dfa6b26 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 @@ -318,21 +318,25 @@ public Replacements getReplacements() { * 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. - * + *

* 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) { - int bci = BytecodeFrame.UNKNOWN_BCI; - if (node instanceof DeoptBciSupplier) { - bci = ((DeoptBciSupplier) node).bci(); - } - ResolvedJavaMethod method = node.graph().method(); - position = new BytecodePosition(null, method, bci); + 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); + } + } diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/MethodTypeFlowBuilder.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/MethodTypeFlowBuilder.java index 61f361fa5f92..d9f1647704aa 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/MethodTypeFlowBuilder.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/MethodTypeFlowBuilder.java @@ -43,6 +43,7 @@ import org.graalvm.compiler.graph.Node.NodeIntrinsic; import org.graalvm.compiler.graph.NodeBitMap; import org.graalvm.compiler.graph.NodeInputList; +import org.graalvm.compiler.graph.NodeSourcePosition; import org.graalvm.compiler.nodes.AbstractEndNode; import org.graalvm.compiler.nodes.AbstractMergeNode; import org.graalvm.compiler.nodes.BeginNode; @@ -109,6 +110,7 @@ import org.graalvm.compiler.replacements.nodes.UnaryMathIntrinsicNode; import org.graalvm.compiler.virtual.phases.ea.PartialEscapePhase; import org.graalvm.compiler.word.WordCastNode; +import org.graalvm.nativeimage.AnnotationAccess; import com.oracle.graal.pointsto.AbstractAnalysisEngine; import com.oracle.graal.pointsto.PointsToAnalysis; @@ -140,7 +142,6 @@ import jdk.vm.ci.meta.JavaConstant; import jdk.vm.ci.meta.JavaKind; import jdk.vm.ci.meta.VMConstant; -import org.graalvm.nativeimage.AnnotationAccess; public class MethodTypeFlowBuilder { @@ -1176,7 +1177,7 @@ protected void node(FixedNode n) { invoke.stateAfter(), invoke.stateAfter().outerFrameState()); MethodCallTargetNode target = (MethodCallTargetNode) invoke.callTarget(); - processMethodInvocation(state, invoke.asFixedNode(), target.invokeKind(), (PointsToAnalysisMethod) target.targetMethod(), target.arguments()); + processMethodInvocation(state, invoke, target.invokeKind(), (PointsToAnalysisMethod) target.targetMethod(), target.arguments()); } } else if (n instanceof ObjectClone) { @@ -1304,17 +1305,67 @@ protected boolean delegateNodeProcessing(FixedNode n, TypeFlowsOfNodes state) { } protected void processMacroInvokable(TypeFlowsOfNodes state, MacroInvokable macro, boolean installResult) { - processMethodInvocation(state, macro.asNode(), macro.getInvokeKind(), (PointsToAnalysisMethod) macro.getTargetMethod(), macro.getArguments(), installResult); + ValueNode macroNode = macro.asNode(); + BytecodePosition invokePosition = getInvokePosition(macro, macroNode); + processMethodInvocation(state, macroNode, macro.getInvokeKind(), (PointsToAnalysisMethod) macro.getTargetMethod(), macro.getArguments(), installResult, invokePosition); } - protected void processMethodInvocation(TypeFlowsOfNodes state, ValueNode invoke, InvokeKind invokeKind, PointsToAnalysisMethod targetMethod, NodeInputList arguments) { - processMethodInvocation(state, invoke, invokeKind, targetMethod, arguments, true); + /* Reconstruct the macro node invoke position, avoiding cycles in the parsing backtrace. */ + private BytecodePosition getInvokePosition(MacroInvokable macro, ValueNode macroNode) { + BytecodePosition invokePosition = null; + NodeSourcePosition position = macroNode.getNodeSourcePosition(); + if (position != null) { + /* + * BytecodeParser.applyInvocationPlugin() gives the macro nodes a position in the target + * method. We pop it here because the invoke flow needs a position in the caller, i.e., + * the currently parsed method. + */ + assert position.getMethod().equals(macro.getTargetMethod()) : "Unexpected macro node source position: " + macro + " at " + position; + invokePosition = position.getCaller(); + } + if (invokePosition == null) { + invokePosition = AbstractAnalysisEngine.syntheticSourcePosition(macroNode, method); + } + return invokePosition; + } + + protected void processMethodInvocation(TypeFlowsOfNodes state, Invoke invoke, InvokeKind invokeKind, PointsToAnalysisMethod targetMethod, NodeInputList arguments) { + FixedNode invokeNode = invoke.asFixedNode(); + BytecodePosition invokePosition = getInvokePosition(invokeNode); + processMethodInvocation(state, invokeNode, invokeKind, targetMethod, arguments, true, invokePosition); + } + + /* Get a reasonable position for inlined invokes, avoiding cycles in the parsing backtrace. */ + private BytecodePosition getInvokePosition(FixedNode invokeNode) { + BytecodePosition invokePosition = invokeNode.getNodeSourcePosition(); + /* Get the outermost caller position for inlined invokes. */ + while (invokePosition != null && invokePosition.getCaller() != null) { + /* + * Invokes coming from recursive inlined methods can lead to cycles when reporting the + * parsing backtrace if we use the reported position directly. For inlined nodes Graal + * reports the original position (in the original method before inlining) and sets its + * caller to the inline location. So by using the outermost caller we simply use a + * version of the graph preprocessed by inline-before-analysis, whose shape may differ + * from the original source code. In some cases this will lead to stack traces with + * missing frames, but it is always correct. + */ + invokePosition = invokePosition.getCaller(); + } + + if (invokePosition == null) { + /* + * The invokePosition is used for all sorts of call stack printing (for error messages + * and diagnostics), so we must have a non-null BytecodePosition. + */ + invokePosition = AbstractAnalysisEngine.syntheticSourcePosition(invokeNode, method); + } + return invokePosition; } protected void processMethodInvocation(TypeFlowsOfNodes state, ValueNode invoke, InvokeKind invokeKind, PointsToAnalysisMethod targetMethod, NodeInputList arguments, - boolean installResult) { + boolean installResult, BytecodePosition invokeLocation) { // check if the call is allowed - bb.isCallAllowed(bb, method, targetMethod, AbstractAnalysisEngine.sourcePosition(invoke)); + bb.isCallAllowed(bb, method, targetMethod, invokeLocation); /* * Collect the parameters builders into an array so that we don't capture the `state` @@ -1366,11 +1417,6 @@ protected void processMethodInvocation(TypeFlowsOfNodes state, ValueNode invoke, } } - /* - * The invokeLocation is used for all sorts of call stack printing (for error messages - * and diagnostics), so we must have a non-null BytecodePosition. - */ - BytecodePosition invokeLocation = AbstractAnalysisEngine.sourcePosition(invoke); InvokeTypeFlow invokeFlow; switch (invokeKind) { case Static: @@ -1425,7 +1471,7 @@ protected void processMethodInvocation(TypeFlowsOfNodes state, ValueNode invoke, * filter to avoid loosing precision. */ TypeFlowBuilder filterBuilder = TypeFlowBuilder.create(bb, invoke, FilterTypeFlow.class, () -> { - FilterTypeFlow filterFlow = new FilterTypeFlow(AbstractAnalysisEngine.sourcePosition(invoke), (AnalysisType) stamp.type(), stamp.isExactType(), true, true); + FilterTypeFlow filterFlow = new FilterTypeFlow(invokeLocation, (AnalysisType) stamp.type(), stamp.isExactType(), true, true); flowsGraph.addMiscEntryFlow(filterFlow); return filterFlow; }); diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/reports/ReportUtils.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/reports/ReportUtils.java index 5a6dc666a5ad..243e056d162a 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/reports/ReportUtils.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/reports/ReportUtils.java @@ -233,23 +233,27 @@ public static String parsingContext(AnalysisMethod method, int bci, String inden /* Include target method first. */ msg.append(String.format("%n%sat %s", indent, method.asStackTraceElement(bci))); } - /* Then add the parsing context. */ - for (int i = 0; i < parsingContext.length; i++) { - StackTraceElement e = parsingContext[i]; - if (isStackTraceTruncationSentinel(e)) { - msg.append(String.format("%n%s", e.getClassName())); - assert i == parsingContext.length - 1; - } else { - msg.append(String.format("%n%sat %s", indent, e)); - } - } - msg.append(String.format("%n")); + formatParsingContext(parsingContext, indent, msg); } else { msg.append(String.format(" %n")); } return msg.toString(); } + public static void formatParsingContext(StackTraceElement[] parsingContext, String indent, StringBuilder msg) { + /* Then add the parsing context. */ + for (int i = 0; i < parsingContext.length; i++) { + StackTraceElement e = parsingContext[i]; + if (isStackTraceTruncationSentinel(e)) { + msg.append(String.format("%n%s", e.getClassName())); + assert i == parsingContext.length - 1; + } else { + msg.append(String.format("%n%sat %s", indent, e)); + } + } + msg.append(String.format("%n")); + } + private static final String stackTraceTruncationSentinel = "WARNING: Parsing context is truncated because its depth exceeds a reasonable limit for "; private static boolean isStackTraceTruncationSentinel(StackTraceElement element) { From f65d849a7247121a650d6802af6747b742919940 Mon Sep 17 00:00:00 2001 From: Codrut Stancu Date: Thu, 13 Oct 2022 13:14:46 +0200 Subject: [PATCH 4/4] Fix GraphKit invoke source position. --- .../graalvm/compiler/replacements/GraphKit.java | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/compiler/src/org.graalvm.compiler.replacements/src/org/graalvm/compiler/replacements/GraphKit.java b/compiler/src/org.graalvm.compiler.replacements/src/org/graalvm/compiler/replacements/GraphKit.java index 8c4d2380a395..e3e416619181 100644 --- a/compiler/src/org.graalvm.compiler.replacements/src/org/graalvm/compiler/replacements/GraphKit.java +++ b/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();