Skip to content

Commit ca2131a

Browse files
tkrodrigueztkrodriguez
committed
[JDK-8343624] BoxPlugin should respect OOME in try
PullRequest: graal/19266
2 parents e1d99ae + c182016 commit ca2131a

File tree

22 files changed

+90
-68
lines changed

22 files changed

+90
-68
lines changed

‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/SubprocessTest.java‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,17 +102,28 @@ private static List<String> filter(List<String> args, Predicate<String> vmArgsFi
102102
return result;
103103
}
104104

105+
public boolean isRecursiveLaunch() {
106+
return isRecursiveLaunch(getClass());
107+
}
108+
109+
private static boolean isRecursiveLaunch(Class<? extends GraalCompilerTest> testClass) {
110+
return Boolean.getBoolean(getRecursionPropName(testClass));
111+
}
112+
113+
private static String getRecursionPropName(Class<? extends GraalCompilerTest> testClass) {
114+
return "test." + testClass.getName() + ".subprocess";
115+
}
116+
105117
public static SubprocessUtil.Subprocess launchSubprocess(Predicate<List<String>> testPredicate, Predicate<String> vmArgsFilter, boolean expectNormalExit,
106118
Class<? extends GraalCompilerTest> testClass, String testSelector, Runnable runnable, String... args)
107119
throws InterruptedException, IOException {
108-
String recursionPropName = "test." + testClass.getName() + ".subprocess";
109-
if (Boolean.getBoolean(recursionPropName)) {
120+
if (isRecursiveLaunch(testClass)) {
110121
runnable.run();
111122
return null;
112123
} else {
113124
List<String> vmArgs = withoutDebuggerArguments(getVMCommandLine());
114125
vmArgs.add(SubprocessUtil.PACKAGE_OPENING_OPTIONS);
115-
vmArgs.add("-D" + recursionPropName + "=true");
126+
vmArgs.add("-D" + getRecursionPropName(testClass) + "=true");
116127
vmArgs.addAll(Arrays.asList(args));
117128
if (vmArgsFilter != null) {
118129
vmArgs = filter(vmArgs, vmArgsFilter);
@@ -147,4 +158,5 @@ public static SubprocessUtil.Subprocess launchSubprocess(Predicate<List<String>>
147158
return proc;
148159
}
149160
}
161+
150162
}

‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/replacements/test/MethodSubstitutionTest.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
import jdk.graal.compiler.phases.common.MidTierLoweringPhase;
3737
import jdk.graal.compiler.phases.tiers.HighTierContext;
3838
import jdk.graal.compiler.replacements.nodes.MacroNode;
39-
4039
import jdk.vm.ci.code.InstalledCode;
4140
import jdk.vm.ci.meta.ResolvedJavaMethod;
4241

@@ -129,7 +128,7 @@ protected void testSubstitution(String testMethodName, Class<?> intrinsicClass,
129128
StructuredGraph graph = testGraph(testMethodName);
130129

131130
// Check to see if the resulting graph contains the expected node
132-
StructuredGraph replacement = getReplacements().getInlineSubstitution(realMethod, 0, Invoke.InlineControl.Normal, false, null, graph.allowAssumptions(), graph.getOptions());
131+
StructuredGraph replacement = getReplacements().getInlineSubstitution(realMethod, 0, false, Invoke.InlineControl.Normal, false, null, graph.allowAssumptions(), graph.getOptions());
133132
if (replacement == null && !optional) {
134133
assertInGraph(graph, intrinsicClass);
135134
}

‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/replacements/test/StandardMethodSubstitutionsTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ public void testSubstitution(String testMethodName, Class<?> holder, String meth
151151
StructuredGraph graph = testGraph(testMethodName);
152152

153153
// Check to see if the resulting graph contains the expected node
154-
StructuredGraph replacement = getReplacements().getInlineSubstitution(realJavaMethod, 0, Invoke.InlineControl.Normal, false, null, graph.allowAssumptions(), graph.getOptions());
154+
StructuredGraph replacement = getReplacements().getInlineSubstitution(realJavaMethod, 0, false, Invoke.InlineControl.Normal, false, null, graph.allowAssumptions(), graph.getOptions());
155155
if (replacement == null && !optional) {
156156
assertInGraph(graph, intrinsicClasses);
157157
}

‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/replacements/test/StringCompareToTest.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,16 +28,16 @@
2828

2929
import java.util.List;
3030

31+
import org.junit.Assert;
32+
import org.junit.Assume;
33+
import org.junit.Test;
34+
3135
import jdk.graal.compiler.graph.Node;
3236
import jdk.graal.compiler.nodes.Invoke;
3337
import jdk.graal.compiler.nodes.StructuredGraph;
3438
import jdk.graal.compiler.options.OptionValues;
3539
import jdk.graal.compiler.replacements.nodes.ArrayCompareToNode;
3640
import jdk.graal.compiler.serviceprovider.GraalServices;
37-
import org.junit.Assert;
38-
import org.junit.Assume;
39-
import org.junit.Test;
40-
4141
import jdk.vm.ci.aarch64.AArch64;
4242
import jdk.vm.ci.amd64.AMD64;
4343
import jdk.vm.ci.meta.ResolvedJavaMethod;
@@ -84,7 +84,7 @@ protected void initSubstitution(ResolvedJavaMethod theRealMethod,
8484
StructuredGraph graph = testGraph(testMethod.getName());
8585

8686
// Check to see if the resulting graph contains the expected node
87-
StructuredGraph replacement = getReplacements().getInlineSubstitution(realMethod, 0, Invoke.InlineControl.Normal, false, null, graph.allowAssumptions(), graph.getOptions());
87+
StructuredGraph replacement = getReplacements().getInlineSubstitution(realMethod, 0, false, Invoke.InlineControl.Normal, false, null, graph.allowAssumptions(), graph.getOptions());
8888
if (replacement == null) {
8989
assertInGraph(graph, expectedNode);
9090
}

‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/replacements/test/StringCompressInflateTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,7 @@ private class TestMethods {
381381
}
382382

383383
StructuredGraph replacementGraph() {
384-
return getReplacements().getInlineSubstitution(javamethod, 0, Invoke.InlineControl.Normal, false, null, testgraph.allowAssumptions(), getInitialOptions());
384+
return getReplacements().getInlineSubstitution(javamethod, 0, false, Invoke.InlineControl.Normal, false, null, testgraph.allowAssumptions(), getInitialOptions());
385385
}
386386

387387
StructuredGraph testMethodGraph() {

‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/replacements/test/StringSubstitutionTestBase.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,11 @@
2424
*/
2525
package jdk.graal.compiler.replacements.test;
2626

27-
import jdk.graal.compiler.nodes.Invoke;
28-
import jdk.graal.compiler.nodes.StructuredGraph;
2927
import org.junit.Assert;
3028
import org.junit.Assume;
3129

30+
import jdk.graal.compiler.nodes.Invoke;
31+
import jdk.graal.compiler.nodes.StructuredGraph;
3232
import jdk.vm.ci.aarch64.AArch64;
3333
import jdk.vm.ci.amd64.AMD64;
3434
import jdk.vm.ci.code.InstalledCode;
@@ -72,7 +72,7 @@ protected void initSubstitution(ResolvedJavaMethod theRealMethod,
7272
StructuredGraph graph = testGraph(testMethod.getName());
7373

7474
// Check to see if the resulting graph contains the expected node
75-
StructuredGraph replacement = getReplacements().getInlineSubstitution(realMethod, 0, Invoke.InlineControl.Normal, false, null, graph.allowAssumptions(), graph.getOptions());
75+
StructuredGraph replacement = getReplacements().getInlineSubstitution(realMethod, 0, false, Invoke.InlineControl.Normal, false, null, graph.allowAssumptions(), graph.getOptions());
7676
if (replacement == null) {
7777
assertInGraph(graph, expectedNode);
7878
}

‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/replacements/test/StringSubstitutionsTest.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,11 @@
2424
*/
2525
package jdk.graal.compiler.replacements.test;
2626

27+
import org.junit.Test;
28+
2729
import jdk.graal.compiler.nodes.Invoke;
2830
import jdk.graal.compiler.nodes.StructuredGraph;
2931
import jdk.graal.compiler.replacements.nodes.ArrayEqualsNode;
30-
import org.junit.Test;
31-
3232
import jdk.vm.ci.code.InstalledCode;
3333
import jdk.vm.ci.meta.ResolvedJavaMethod;
3434

@@ -43,7 +43,7 @@ public void testSubstitution(String testMethodName, Class<?> intrinsicClass, Cla
4343
StructuredGraph graph = testGraph(testMethodName);
4444

4545
// Check to see if the resulting graph contains the expected node
46-
StructuredGraph replacement = getReplacements().getInlineSubstitution(realMethod, 0, Invoke.InlineControl.Normal, false, null, graph.allowAssumptions(), graph.getOptions());
46+
StructuredGraph replacement = getReplacements().getInlineSubstitution(realMethod, 0, false, Invoke.InlineControl.Normal, false, null, graph.allowAssumptions(), graph.getOptions());
4747
if (replacement == null && !optional) {
4848
assertInGraph(graph, intrinsicClass);
4949
}

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/HotSpotReplacementsImpl.java‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -132,19 +132,28 @@ public void notifyNotInlined(GraphBuilderContext b, ResolvedJavaMethod method, I
132132

133133
public static class HotSpotIntrinsicGraphBuilder extends IntrinsicGraphBuilder {
134134

135-
public HotSpotIntrinsicGraphBuilder(OptionValues options, DebugContext debug, CoreProviders providers, Bytecode code, int invokeBci, AllowAssumptions allowAssumptions) {
135+
private final boolean isInOOMETry;
136+
137+
public HotSpotIntrinsicGraphBuilder(OptionValues options, DebugContext debug, CoreProviders providers, Bytecode code, int invokeBci, AllowAssumptions allowAssumptions, boolean isInOOMETry) {
136138
super(options, debug, providers, code, invokeBci, allowAssumptions);
139+
this.isInOOMETry = isInOOMETry;
137140
}
138141

139142
public HotSpotIntrinsicGraphBuilder(OptionValues options, DebugContext debug, CoreProviders providers, Bytecode code, int invokeBci, AllowAssumptions allowAssumptions,
140143
GraphBuilderConfiguration graphBuilderConfig) {
141144
super(options, debug, providers, code, invokeBci, allowAssumptions, graphBuilderConfig);
145+
this.isInOOMETry = false;
142146
}
143147

144148
@Override
145149
public GuardingNode intrinsicRangeCheck(LogicNode condition, boolean negated) {
146150
return HotSpotBytecodeParser.doIntrinsicRangeCheck(this, condition, negated);
147151
}
152+
153+
@Override
154+
public boolean currentBlockCatchesOOME() {
155+
return isInOOMETry;
156+
}
148157
}
149158

150159
@Override
@@ -154,8 +163,8 @@ public boolean hasSubstitution(ResolvedJavaMethod method, OptionValues options)
154163
}
155164

156165
@Override
157-
public StructuredGraph getInlineSubstitution(ResolvedJavaMethod method, int invokeBci, Invoke.InlineControl inlineControl, boolean trackNodeSourcePosition, NodeSourcePosition replaceePosition,
158-
AllowAssumptions allowAssumptions, OptionValues options) {
166+
public StructuredGraph getInlineSubstitution(ResolvedJavaMethod method, int invokeBci, boolean isInOOMETry, Invoke.InlineControl inlineControl, boolean trackNodeSourcePosition,
167+
NodeSourcePosition replaceePosition, AllowAssumptions allowAssumptions, OptionValues options) {
159168
assert invokeBci >= 0 : method;
160169
if (!inlineControl.allowSubstitution()) {
161170
return null;
@@ -165,7 +174,7 @@ public StructuredGraph getInlineSubstitution(ResolvedJavaMethod method, int invo
165174
if (plugin != null) {
166175
Bytecode code = new ResolvedJavaMethodBytecode(method);
167176
try (DebugContext debug = openSnippetDebugContext("Substitution_", method, options)) {
168-
result = new HotSpotIntrinsicGraphBuilder(options, debug, providers, code, invokeBci, allowAssumptions).buildGraph(plugin);
177+
result = new HotSpotIntrinsicGraphBuilder(options, debug, providers, code, invokeBci, allowAssumptions, isInOOMETry).buildGraph(plugin);
169178
}
170179
} else {
171180
result = null;

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/meta/HotSpotGraphBuilderPlugins.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
import java.math.BigInteger;
6262
import java.util.zip.CRC32;
6363

64+
import org.graalvm.nativeimage.ImageInfo;
6465
import org.graalvm.word.LocationIdentity;
6566

6667
import jdk.graal.compiler.api.directives.GraalDirectives;
@@ -190,7 +191,6 @@
190191
import jdk.vm.ci.meta.SpeculationLog;
191192
import jdk.vm.ci.meta.SpeculationLog.SpeculationReason;
192193
import jdk.vm.ci.meta.UnresolvedJavaType;
193-
import org.graalvm.nativeimage.ImageInfo;
194194

195195
/**
196196
* Defines the {@link Plugins} used when running on HotSpot.
@@ -543,7 +543,7 @@ public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Rec
543543
*/
544544
b.add(new KlassFullyInitializedCheckNode(clazzLegal));
545545

546-
if (b.currentBlockCatchesOOM()) {
546+
if (b.currentBlockCatchesOOME()) {
547547
b.addPush(JavaKind.Object, new DynamicNewInstanceWithExceptionNode(clazzLegal, true));
548548
} else {
549549
b.addPush(JavaKind.Object, new DynamicNewInstanceNode(clazzLegal, true));

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/java/BytecodeParser.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,6 @@
334334
import jdk.graal.compiler.nodes.IfNode;
335335
import jdk.graal.compiler.nodes.InliningLog;
336336
import jdk.graal.compiler.nodes.InliningLog.PlaceholderInvokable;
337-
import jdk.graal.compiler.nodes.LoopBeginNode.SafepointState;
338337
import jdk.graal.compiler.nodes.Invokable;
339338
import jdk.graal.compiler.nodes.Invoke;
340339
import jdk.graal.compiler.nodes.InvokeNode;
@@ -343,6 +342,7 @@
343342
import jdk.graal.compiler.nodes.LogicNegationNode;
344343
import jdk.graal.compiler.nodes.LogicNode;
345344
import jdk.graal.compiler.nodes.LoopBeginNode;
345+
import jdk.graal.compiler.nodes.LoopBeginNode.SafepointState;
346346
import jdk.graal.compiler.nodes.LoopEndNode;
347347
import jdk.graal.compiler.nodes.LoopExitNode;
348348
import jdk.graal.compiler.nodes.MergeNode;
@@ -2162,7 +2162,7 @@ protected Invokable appendInvoke(InvokeKind initialInvokeKind, ResolvedJavaMetho
21622162
if (referencedType != null) {
21632163
invoke.callTarget().setReferencedType(referencedType);
21642164
}
2165-
if (currentBlockCatchesOOM()) {
2165+
if (currentBlockCatchesOOME()) {
21662166
invoke.setInOOMETry(true);
21672167
}
21682168
return invoke;
@@ -2778,7 +2778,7 @@ protected void parseAndInlineCallee(ResolvedJavaMethod targetMethod, ValueNode[]
27782778
: (calleeIntrinsicContext != null ? new IntrinsicScope(this, targetMethod, args)
27792779
: new InliningScope(this, targetMethod, args))) {
27802780
BytecodeParser parser = graphBuilderInstance.createBytecodeParser(graph, this, targetMethod, INVOCATION_ENTRY_BCI, calleeIntrinsicContext);
2781-
if (currentBlockCatchesOOM()) {
2781+
if (currentBlockCatchesOOME()) {
27822782
parser.calleeInOOMEBlock = true;
27832783
}
27842784
boolean targetIsSubstitution = parsingIntrinsic();
@@ -5003,7 +5003,7 @@ protected void genNewInstance(ResolvedJavaType resolvedType) {
50035003
* failures.
50045004
*/
50055005
@Override
5006-
public boolean currentBlockCatchesOOM() {
5006+
public boolean currentBlockCatchesOOME() {
50075007
if (disableExplicitAllocationExceptionEdges) {
50085008
return false;
50095009
}
@@ -5024,7 +5024,7 @@ public boolean currentBlockCatchesOOM() {
50245024
}
50255025

50265026
private void createNewInstance(ResolvedJavaType resolvedType) {
5027-
if (currentBlockCatchesOOM()) {
5027+
if (currentBlockCatchesOOME()) {
50285028
NewInstanceWithExceptionNode ni = new NewInstanceWithExceptionNode(resolvedType, true);
50295029
frameState.push(JavaKind.Object, append(ni));
50305030
setStateAfter(ni);
@@ -5034,7 +5034,7 @@ private void createNewInstance(ResolvedJavaType resolvedType) {
50345034
}
50355035

50365036
private void createNewArray(ResolvedJavaType resolvedType, ValueNode length) {
5037-
if (currentBlockCatchesOOM()) {
5037+
if (currentBlockCatchesOOME()) {
50385038
NewArrayWithExceptionNode nawe = new NewArrayWithExceptionNode(resolvedType, length, true);
50395039
frameState.push(JavaKind.Object, append(nawe));
50405040
setStateAfter(nawe);
@@ -5044,7 +5044,7 @@ private void createNewArray(ResolvedJavaType resolvedType, ValueNode length) {
50445044
}
50455045

50465046
private void generateNewMultIArray(ResolvedJavaType resolvedType, ValueNode[] dims) {
5047-
if (currentBlockCatchesOOM()) {
5047+
if (currentBlockCatchesOOME()) {
50485048
NewMultiArrayWithExceptionNode nmanwe = new NewMultiArrayWithExceptionNode(resolvedType, dims);
50495049
frameState.push(JavaKind.Object, append(nmanwe));
50505050
setStateAfter(nmanwe);

0 commit comments

Comments
 (0)