Skip to content

Commit efaeba3

Browse files
tkrodrigueztkrodriguez
committed
[GR-12844] SVM error improvements.
PullRequest: graal/2582
2 parents 7368915 + c8ec163 commit efaeba3

File tree

25 files changed

+179
-108
lines changed

25 files changed

+179
-108
lines changed

‎compiler/src/org.graalvm.compiler.phases.common/src/org/graalvm/compiler/phases/common/inlining/InliningUtil.java

-34
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
import static jdk.vm.ci.meta.DeoptimizationReason.NullCheckException;
2929
import static org.graalvm.compiler.core.common.GraalOptions.HotSpotPrintInlining;
3030

31-
import java.lang.reflect.Constructor;
3231
import java.util.ArrayDeque;
3332
import java.util.ArrayList;
3433
import java.util.List;
@@ -49,7 +48,6 @@
4948
import org.graalvm.compiler.debug.DebugCloseable;
5049
import org.graalvm.compiler.debug.DebugContext;
5150
import org.graalvm.compiler.debug.GraalError;
52-
import org.graalvm.compiler.graph.GraalGraphError;
5351
import org.graalvm.compiler.graph.Graph.DuplicationReplacement;
5452
import org.graalvm.compiler.graph.Graph.Mark;
5553
import org.graalvm.compiler.graph.Graph.NodeEventScope;
@@ -63,14 +61,12 @@
6361
import org.graalvm.compiler.nodes.AbstractEndNode;
6462
import org.graalvm.compiler.nodes.AbstractMergeNode;
6563
import org.graalvm.compiler.nodes.BeginNode;
66-
import org.graalvm.compiler.nodes.CallTargetNode;
6764
import org.graalvm.compiler.nodes.CallTargetNode.InvokeKind;
6865
import org.graalvm.compiler.nodes.DeoptimizeNode;
6966
import org.graalvm.compiler.nodes.DeoptimizingGuard;
7067
import org.graalvm.compiler.nodes.EndNode;
7168
import org.graalvm.compiler.nodes.FixedGuardNode;
7269
import org.graalvm.compiler.nodes.FixedNode;
73-
import org.graalvm.compiler.nodes.FixedWithNextNode;
7470
import org.graalvm.compiler.nodes.FrameState;
7571
import org.graalvm.compiler.nodes.InliningLog;
7672
import org.graalvm.compiler.nodes.Invoke;
@@ -1017,36 +1013,6 @@ public static StructuredGraph getIntrinsicGraph(Replacements replacements, Resol
10171013
return replacements.getSubstitution(target, invokeBci, trackNodeSourcePosition, replaceePosition);
10181014
}
10191015

1020-
public static FixedWithNextNode inlineMacroNode(Invoke invoke, ResolvedJavaMethod concrete, Class<? extends FixedWithNextNode> macroNodeClass) throws GraalError {
1021-
StructuredGraph graph = invoke.asNode().graph();
1022-
if (!concrete.equals(((MethodCallTargetNode) invoke.callTarget()).targetMethod())) {
1023-
assert ((MethodCallTargetNode) invoke.callTarget()).invokeKind().hasReceiver();
1024-
InliningUtil.replaceInvokeCallTarget(invoke, graph, InvokeKind.Special, concrete);
1025-
}
1026-
1027-
FixedWithNextNode macroNode = createMacroNodeInstance(macroNodeClass, invoke);
1028-
1029-
CallTargetNode callTarget = invoke.callTarget();
1030-
if (invoke instanceof InvokeNode) {
1031-
graph.replaceFixedWithFixed((InvokeNode) invoke, graph.add(macroNode));
1032-
} else {
1033-
InvokeWithExceptionNode invokeWithException = (InvokeWithExceptionNode) invoke;
1034-
invokeWithException.killExceptionEdge();
1035-
graph.replaceSplitWithFixed(invokeWithException, graph.add(macroNode), invokeWithException.next());
1036-
}
1037-
GraphUtil.killWithUnusedFloatingInputs(callTarget);
1038-
return macroNode;
1039-
}
1040-
1041-
private static FixedWithNextNode createMacroNodeInstance(Class<? extends FixedWithNextNode> macroNodeClass, Invoke invoke) throws GraalError {
1042-
try {
1043-
Constructor<?> cons = macroNodeClass.getDeclaredConstructor(Invoke.class);
1044-
return (FixedWithNextNode) cons.newInstance(invoke);
1045-
} catch (ReflectiveOperationException | IllegalArgumentException | SecurityException e) {
1046-
throw new GraalGraphError(e).addContext(invoke.asNode()).addContext("macroSubstitution", macroNodeClass);
1047-
}
1048-
}
1049-
10501016
/**
10511017
* This method exclude InstrumentationNode from inlining heuristics.
10521018
*/

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

+10-3
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,11 @@ protected final void scanField(AnalysisField field, JavaConstant receiver, Objec
114114
JavaConstant fieldValue = bb.getConstantReflectionProvider().readFieldValue(field, receiver);
115115

116116
if (fieldValue == null) {
117+
StringBuilder backtrace = new StringBuilder();
118+
buildObjectBacktrace(reason, backtrace);
117119
throw AnalysisError.shouldNotReachHere("Could not find field " + field.format("%H.%n") +
118-
(receiver == null ? "" : " on " + bb.getSnippetReflectionProvider().asObject(Object.class, receiver).getClass()));
120+
(receiver == null ? "" : " on " + bb.getSnippetReflectionProvider().asObject(Object.class, receiver).getClass()) +
121+
System.lineSeparator() + backtrace.toString());
119122
}
120123

121124
if (fieldValue.getJavaKind() == JavaKind.Object && bb.getHostVM().isRelocatedPointer(bb.getSnippetReflectionProvider().asObject(Object.class, fieldValue))) {
@@ -229,6 +232,11 @@ public final void scanConstant(JavaConstant value, Object reason) {
229232

230233
private void unsupportedFeature(String key, String message, Object entry) {
231234
StringBuilder objectBacktrace = new StringBuilder();
235+
AnalysisMethod method = buildObjectBacktrace(entry, objectBacktrace);
236+
bb.getUnsupportedFeatures().addMessage(key, method, message, objectBacktrace.toString());
237+
}
238+
239+
private AnalysisMethod buildObjectBacktrace(Object entry, StringBuilder objectBacktrace) {
232240
Object cur = entry;
233241
AnalysisMethod method = null;
234242

@@ -246,8 +254,7 @@ private void unsupportedFeature(String key, String message, Object entry) {
246254
} else {
247255
objectBacktrace.append("\t[unknown] ").append(cur.toString());
248256
}
249-
250-
bb.getUnsupportedFeatures().addMessage(key, method, message, objectBacktrace.toString());
257+
return method;
251258
}
252259

253260
/**

‎substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/constraints/UnsupportedFeatures.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ static class Data implements Comparable<Data> {
5050
Data(String key, AnalysisMethod method, String message, String trace, Throwable originalException) {
5151
this.key = key;
5252
this.method = method;
53-
this.message = message;
53+
this.message = message != null ? message : "";
5454
this.trace = trace;
5555
this.originalException = originalException;
5656
}

‎substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/MethodTypeFlowBuilder.java

+7-7
Original file line numberDiff line numberDiff line change
@@ -954,7 +954,7 @@ protected void node(FixedNode n) {
954954
AnalysisUnsafePartitionLoadNode node = (AnalysisUnsafePartitionLoadNode) n;
955955
assert node.object().getStackKind() == JavaKind.Object;
956956

957-
checkUnsafeOffset(node.offset());
957+
checkUnsafeOffset(node.object(), node.offset());
958958

959959
AnalysisType partitionType = (AnalysisType) node.partitionType();
960960

@@ -979,7 +979,7 @@ protected void node(FixedNode n) {
979979
assert node.object().getStackKind() == JavaKind.Object;
980980
assert node.value().getStackKind() == JavaKind.Object;
981981

982-
checkUnsafeOffset(node.offset());
982+
checkUnsafeOffset(node.object(), node.offset());
983983

984984
AnalysisType partitionType = (AnalysisType) node.partitionType();
985985

@@ -1009,7 +1009,7 @@ protected void node(FixedNode n) {
10091009
} else if (n instanceof RawLoadNode) {
10101010
RawLoadNode node = (RawLoadNode) n;
10111011

1012-
checkUnsafeOffset(node.offset());
1012+
checkUnsafeOffset(node.object(), node.offset());
10131013

10141014
if (node.object().getStackKind() == JavaKind.Object && node.getStackKind() == JavaKind.Object) {
10151015
AnalysisType objectType = (AnalysisType) StampTool.typeOrNull(node.object());
@@ -1042,7 +1042,7 @@ protected void node(FixedNode n) {
10421042
} else if (n instanceof RawStoreNode) {
10431043
RawStoreNode node = (RawStoreNode) n;
10441044

1045-
checkUnsafeOffset(node.offset());
1045+
checkUnsafeOffset(node.object(), node.offset());
10461046

10471047
if (node.object().getStackKind() == JavaKind.Object && node.value().getStackKind() == JavaKind.Object) {
10481048
AnalysisType objectType = (AnalysisType) StampTool.typeOrNull(node.object());
@@ -1076,7 +1076,7 @@ protected void node(FixedNode n) {
10761076
}
10771077
} else if (n instanceof UnsafeCompareAndSwapNode) {
10781078
UnsafeCompareAndSwapNode node = (UnsafeCompareAndSwapNode) n;
1079-
checkUnsafeOffset(node.offset());
1079+
checkUnsafeOffset(node.object(), node.offset());
10801080
if (node.object().getStackKind() == JavaKind.Object && node.newValue().getStackKind() == JavaKind.Object) {
10811081
AnalysisType objectType = (AnalysisType) StampTool.typeOrNull(node.object());
10821082
TypeFlowBuilder<?> objectBuilder = state.lookup(node.object());
@@ -1111,7 +1111,7 @@ protected void node(FixedNode n) {
11111111

11121112
} else if (n instanceof AtomicReadAndWriteNode) {
11131113
AtomicReadAndWriteNode node = (AtomicReadAndWriteNode) n;
1114-
checkUnsafeOffset(node.offset());
1114+
checkUnsafeOffset(node.object(), node.offset());
11151115
if (node.object().getStackKind() == JavaKind.Object && node.newValue().getStackKind() == JavaKind.Object) {
11161116

11171117
AnalysisType objectType = (AnalysisType) StampTool.typeOrNull(node.object());
@@ -1496,7 +1496,7 @@ protected NewInstanceTypeFlow createNewArrayTypeFlow(NewArrayNode node, Analysis
14961496
}
14971497

14981498
/** Hook for unsafe offset value checks. */
1499-
protected void checkUnsafeOffset(@SuppressWarnings("unused") ValueNode offset) {
1499+
protected void checkUnsafeOffset(@SuppressWarnings("unused") ValueNode base, @SuppressWarnings("unused") ValueNode offset) {
15001500
}
15011501

15021502
}

‎substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisUniverse.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,8 @@ public class AnalysisUniverse implements Universe {
8989

9090
/**
9191
* True if the analysis has converged and the analysis data is valid. This is similar to
92-
* {@sealed} but in contrast to {@sealed}, the analysis data can be set to invalid again, e.g.
93-
* if features modify the universe.
92+
* {@link #sealed} but in contrast to {@link #sealed}, the analysis data can be set to invalid
93+
* again, e.g. if features modify the universe.
9494
*/
9595
boolean analysisDataValid;
9696

‎substratevm/src/com.oracle.svm.core.graal/src/com/oracle/svm/core/graal/meta/RuntimeConfiguration.java

+3
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@
4242

4343
import jdk.vm.ci.meta.ResolvedJavaMethod;
4444

45+
/**
46+
* Configuration used by Graal at runtime to compile and install code in the same runtime.
47+
*/
4548
public class RuntimeConfiguration {
4649

4750
private final Providers providers;

‎substratevm/src/com.oracle.svm.core.graal/src/com/oracle/svm/core/graal/meta/SubstrateReplacements.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ protected EncodedGraph lookupEncodedGraph(ResolvedJavaMethod lookupMethod, Resol
212212
@Platforms(Platform.HOSTED_ONLY.class)
213213
@Override
214214
public void registerSnippet(ResolvedJavaMethod method, boolean trackNodeSourcePosition) {
215-
assert method.getAnnotation(Snippet.class) != null : "Snippet must be annotated with @" + Snippet.class.getSimpleName();
215+
assert method.getAnnotation(Snippet.class) != null : "Snippet must be annotated with @" + Snippet.class.getSimpleName() + " " + method;
216216
assert method.hasBytecodes() : "Snippet must not be abstract or native";
217217
assert builder.graphs.get(method) == null : "snippet registered twice: " + method.getName();
218218

@@ -223,7 +223,7 @@ public void registerSnippet(ResolvedJavaMethod method, boolean trackNodeSourcePo
223223
for (MethodCallTargetNode callTarget : graph.getNodes(MethodCallTargetNode.TYPE)) {
224224
ResolvedJavaMethod callee = callTarget.targetMethod();
225225
if (!builder.delayedInvocationPluginMethods.contains(callee)) {
226-
throw shouldNotReachHere("method " + callee.getName() + " not inlined in snippet " + method.getName() + " (maybe not final?)");
226+
throw shouldNotReachHere("method " + callee.format("%h.%n") + " not inlined in snippet " + method.format("%h.%n") + " (maybe not final?)");
227227
}
228228
}
229229

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java

+3
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,9 @@ protected void onValueUpdate(EconomicMap<OptionKey<?>, Object> values, Boolean o
166166
@Option(help = "Resources describing program elements to be made accessible via JNI (see JNIConfigurationFiles).", type = OptionType.User)//
167167
public static final HostedOptionKey<String[]> JNIConfigurationResources = new HostedOptionKey<>(null);
168168

169+
@Option(help = "Report information about known JNI elements when lookup fails", type = OptionType.User)//
170+
public static final HostedOptionKey<Boolean> JNIVerboseLookupErrors = new HostedOptionKey<>(false);
171+
169172
/*
170173
* Object and array allocation options.
171174
*/

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/util/UserError.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ protected UserException(String msg) {
4949
}
5050

5151
protected UserException(Iterable<String> messages) {
52-
super(String.join("\n", messages));
52+
super(String.join(System.lineSeparator(), messages));
5353
this.messages = messages;
5454
}
5555

‎substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/GraalSubstitutions.java

+1-17
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
import java.util.Map;
3737
import java.util.concurrent.ConcurrentHashMap;
3838

39-
import jdk.vm.ci.meta.MetaAccessProvider;
4039
import org.graalvm.collections.EconomicMap;
4140
import org.graalvm.collections.EconomicSet;
4241
import org.graalvm.collections.Equivalence;
@@ -55,7 +54,6 @@
5554
import org.graalvm.compiler.lir.LIRInstructionClass;
5655
import org.graalvm.compiler.lir.gen.ArithmeticLIRGeneratorTool;
5756
import org.graalvm.compiler.lir.phases.LIRPhase;
58-
import org.graalvm.compiler.nodes.FixedWithNextNode;
5957
import org.graalvm.compiler.nodes.Invoke;
6058
import org.graalvm.compiler.nodes.StructuredGraph;
6159
import org.graalvm.compiler.nodes.spi.NodeLIRBuilderTool;
@@ -78,6 +76,7 @@
7876
import com.oracle.svm.graal.hosted.GraalFeature;
7977
import com.oracle.svm.graal.meta.SubstrateMethod;
8078

79+
import jdk.vm.ci.meta.MetaAccessProvider;
8180
import jdk.vm.ci.meta.ResolvedJavaField;
8281
import jdk.vm.ci.meta.ResolvedJavaMethod;
8382

@@ -238,21 +237,6 @@ private void toString(StringBuilder str, int i) {
238237
}
239238
}
240239

241-
@TargetClass(value = org.graalvm.compiler.phases.common.inlining.InliningUtil.class, onlyWith = GraalFeature.IsEnabled.class)
242-
final class Target_org_graalvm_compiler_phases_common_inlining_InliningUtil {
243-
244-
/**
245-
* Creates a macro node.
246-
*
247-
* @param macroNodeClass The class of the macro node to create
248-
* @param invoke The parameter to the constructor
249-
*/
250-
@Substitute
251-
private static FixedWithNextNode createMacroNodeInstance(Class<? extends FixedWithNextNode> macroNodeClass, Invoke invoke) {
252-
throw VMError.shouldNotReachHere("unknown macro node class: " + macroNodeClass.getName());
253-
}
254-
}
255-
256240
@TargetClass(value = org.graalvm.compiler.debug.KeyRegistry.class, onlyWith = GraalFeature.IsEnabled.class)
257241
final class Target_org_graalvm_compiler_debug_KeyRegistry {
258242

‎substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/FieldsOffsetsFeature.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ private static Object replaceFieldsOffsets(Object source) {
129129
* All instances of Fields must have been registered before, otherwise we miss the
130130
* substitution of its offsets array.
131131
*/
132-
assert !ImageSingletons.lookup(FieldsOffsetsReplacements.class).newValuesAvailable || getReplacements().containsKey(((Fields) source).getOffsets());
132+
assert !ImageSingletons.lookup(FieldsOffsetsReplacements.class).newValuesAvailable || getReplacements().containsKey(((Fields) source).getOffsets()) : source;
133133

134134
} else if (source instanceof long[]) {
135135
FieldsOffsetsReplacement replacement = getReplacements().get(source);

‎substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/GraalFeature.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -134,10 +134,10 @@
134134

135135
/**
136136
* The main handler for running Graal in the Substrate VM at run time. This feature (and features it
137-
* depends on like {@link FieldsOffsetsFeature}) encode Graal graphs for runtime compilation, ensure
138-
* that all the {@link SubstrateType}, {@link SubstrateMethod}, {@link SubstrateField} are created
139-
* by the {@link GraalObjectReplacer} and put in the image. Data that is prepared during image
140-
* generation and used at run time is stored in the {@link GraalSupport}.
137+
* depends on like {@link FieldsOffsetsFeature}) encodes Graal graphs for runtime compilation,
138+
* ensures that all required {@link SubstrateType}, {@link SubstrateMethod}, {@link SubstrateField}
139+
* objects are created by {@link GraalObjectReplacer} and added to the image. Data that is prepared
140+
* during image generation and used at run time is stored in {@link GraalSupport}.
141141
*/
142142
public final class GraalFeature implements Feature {
143143

‎substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/GraalObjectReplacer.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@
7575
import jdk.vm.ci.meta.Signature;
7676

7777
/**
78-
* Replaces graal related objects during analysis in the universe.
78+
* Replaces Graal related objects during analysis in the universe.
7979
*
8080
* It is mainly used to replace the Hosted* meta data with the Substrate* meta data.
8181
*/
@@ -295,8 +295,10 @@ public SubstrateType createType(JavaType original) {
295295
private static AnalysisType toAnalysisType(JavaType original) {
296296
if (original instanceof HostedType) {
297297
return ((HostedType) original).getWrapped();
298-
} else {
298+
} else if (original instanceof AnalysisType) {
299299
return (AnalysisType) original;
300+
} else {
301+
throw new InternalError("unexpected type " + original);
300302
}
301303
}
302304

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -712,7 +712,7 @@ private void doRun(Map<Method, CEntryPointData> entryPoints, Method mainEntryPoi
712712
if (!config.getAndResetRequireAnalysisIteration()) {
713713
if (numTypes != aUniverse.getTypes().size() || numMethods != aUniverse.getMethods().size() || numFields != aUniverse.getFields().size()) {
714714
throw UserError.abort(
715-
"When a feature makes more types, methods, of fields reachable, it must require another analysis iteration via DuringAnalysisAccess.requireAnalysisIteration()");
715+
"When a feature makes more types, methods, or fields reachable, it must require another analysis iteration via DuringAnalysisAccess.requireAnalysisIteration()");
716716
}
717717
break;
718718
}

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGeneratorRunner.java

+23-4
Original file line numberDiff line numberDiff line change
@@ -287,19 +287,19 @@ private int buildImage(String[] arguments, String[] classpath, ClassLoader class
287287
e.getReason().ifPresent(NativeImageGeneratorRunner::info);
288288
return 0;
289289
} catch (UserException e) {
290-
e.getMessages().iterator().forEachRemaining(NativeImageGeneratorRunner::reportUserError);
290+
reportUserError(e);
291291
return -1;
292292
} catch (AnalysisError e) {
293-
NativeImageGeneratorRunner.reportUserError(e.getMessage());
293+
reportUserError(e);
294294
return -1;
295295
} catch (ParallelExecutionException pee) {
296296
boolean hasUserError = false;
297297
for (Throwable exception : pee.getExceptions()) {
298298
if (exception instanceof UserException) {
299-
((UserException) exception).getMessages().iterator().forEachRemaining(NativeImageGeneratorRunner::reportUserError);
299+
reportUserError(exception);
300300
hasUserError = true;
301301
} else if (exception instanceof AnalysisError) {
302-
NativeImageGeneratorRunner.reportUserError(exception.getMessage());
302+
reportUserError(exception);
303303
hasUserError = true;
304304
}
305305
}
@@ -361,6 +361,25 @@ public static void reportUserError(String msg) {
361361
System.err.println("error: " + msg);
362362
}
363363

364+
/**
365+
* Function for reporting all fatal errors in SVM.
366+
*
367+
* @param e error message that is printed.
368+
*/
369+
public static void reportUserError(Throwable e) {
370+
if (e instanceof UserException) {
371+
UserException ue = (UserException) e;
372+
for (String message : ue.getMessages()) {
373+
reportUserError(message);
374+
}
375+
} else {
376+
reportUserError(e.getMessage());
377+
}
378+
if (NativeImageOptions.ReportExceptionStackTraces.getValue()) {
379+
e.printStackTrace();
380+
}
381+
}
382+
364383
/**
365384
* Report an informational message in SVM.
366385
*

0 commit comments

Comments
 (0)