Skip to content

Commit d56c395

Browse files
author
Christian Wimmer
committed
Add missing exception edge for array allocations
1 parent be5cf4d commit d56c395

File tree

16 files changed

+240
-52
lines changed

16 files changed

+240
-52
lines changed

compiler/src/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/meta/DefaultHotSpotLoweringProvider.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -760,6 +760,7 @@ static final class Exceptions {
760760
cachedExceptions.put(BytecodeExceptionKind.OUT_OF_BOUNDS, clearStackTrace(new ArrayIndexOutOfBoundsException()));
761761
cachedExceptions.put(BytecodeExceptionKind.CLASS_CAST, clearStackTrace(new ClassCastException()));
762762
cachedExceptions.put(BytecodeExceptionKind.ARRAY_STORE, clearStackTrace(new ArrayStoreException()));
763+
cachedExceptions.put(BytecodeExceptionKind.NEGATIVE_ARRAY_SIZE, clearStackTrace(new NegativeArraySizeException()));
763764
cachedExceptions.put(BytecodeExceptionKind.DIVISION_BY_ZERO, clearStackTrace(new ArithmeticException()));
764765
cachedExceptions.put(BytecodeExceptionKind.ILLEGAL_ARGUMENT_EXCEPTION_ARGUMENT_IS_NOT_AN_ARRAY,
765766
clearStackTrace(new IllegalArgumentException(BytecodeExceptionKind.ILLEGAL_ARGUMENT_EXCEPTION_ARGUMENT_IS_NOT_AN_ARRAY.getExceptionMessage())));
@@ -780,6 +781,7 @@ public static final class RuntimeCalls {
780781
runtimeCalls.put(BytecodeExceptionKind.CLASS_CAST, new ForeignCallSignature("createClassCastException", ClassCastException.class, Object.class, KlassPointer.class));
781782
runtimeCalls.put(BytecodeExceptionKind.NULL_POINTER, new ForeignCallSignature("createNullPointerException", NullPointerException.class));
782783
runtimeCalls.put(BytecodeExceptionKind.OUT_OF_BOUNDS, new ForeignCallSignature("createOutOfBoundsException", ArrayIndexOutOfBoundsException.class, int.class, int.class));
784+
runtimeCalls.put(BytecodeExceptionKind.NEGATIVE_ARRAY_SIZE, new ForeignCallSignature("createNegativeArraySizeException", NegativeArraySizeException.class, int.class));
783785
runtimeCalls.put(BytecodeExceptionKind.DIVISION_BY_ZERO, new ForeignCallSignature("createDivisionByZeroException", ArithmeticException.class));
784786
runtimeCalls.put(BytecodeExceptionKind.INTEGER_EXACT_OVERFLOW, new ForeignCallSignature("createIntegerExactOverflowException", ArithmeticException.class));
785787
runtimeCalls.put(BytecodeExceptionKind.LONG_EXACT_OVERFLOW, new ForeignCallSignature("createLongExactOverflowException", ArithmeticException.class));

compiler/src/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/meta/HotSpotHostForeignCallsProvider.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@
113113
import org.graalvm.compiler.hotspot.stubs.IllegalArgumentExceptionArgumentIsNotAnArrayStub;
114114
import org.graalvm.compiler.hotspot.stubs.IntegerExactOverflowExceptionStub;
115115
import org.graalvm.compiler.hotspot.stubs.LongExactOverflowExceptionStub;
116+
import org.graalvm.compiler.hotspot.stubs.NegativeArraySizeExceptionStub;
116117
import org.graalvm.compiler.hotspot.stubs.NullPointerExceptionStub;
117118
import org.graalvm.compiler.hotspot.stubs.OutOfBoundsExceptionStub;
118119
import org.graalvm.compiler.hotspot.stubs.Stub;
@@ -477,6 +478,8 @@ public void initialize(HotSpotProviders providers, OptionValues options) {
477478
registerStubCall(exceptionRuntimeCalls.get(BytecodeExceptionKind.NULL_POINTER), SAFEPOINT, NOT_REEXECUTABLE, DESTROYS_ALL_CALLER_SAVE_REGISTERS, any())));
478479
link(new OutOfBoundsExceptionStub(options, providers,
479480
registerStubCall(exceptionRuntimeCalls.get(BytecodeExceptionKind.OUT_OF_BOUNDS), SAFEPOINT, NOT_REEXECUTABLE, DESTROYS_ALL_CALLER_SAVE_REGISTERS, any())));
481+
link(new NegativeArraySizeExceptionStub(options, providers,
482+
registerStubCall(exceptionRuntimeCalls.get(BytecodeExceptionKind.NEGATIVE_ARRAY_SIZE), SAFEPOINT, NOT_REEXECUTABLE, DESTROYS_ALL_CALLER_SAVE_REGISTERS, any())));
480483
link(new DivisionByZeroExceptionStub(options, providers,
481484
registerStubCall(exceptionRuntimeCalls.get(BytecodeExceptionKind.DIVISION_BY_ZERO), SAFEPOINT, NOT_REEXECUTABLE, DESTROYS_ALL_CALLER_SAVE_REGISTERS, any())));
482485
link(new IntegerExactOverflowExceptionStub(options, providers,

compiler/src/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/replacements/ClassGetHubNode.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,12 @@ public Node canonical(CanonicalizerTool tool) {
128128
@NodeIntrinsic
129129
public static native KlassPointer readClass(Class<?> clazzNonNull);
130130

131+
public static KlassPointer piCastNonNull(KlassPointer object, GuardingNode anchor) {
132+
return intrinsifiedPiNode(object, anchor, PiNode.INTRINSIFY_OP_NON_NULL);
133+
}
134+
131135
@NodeIntrinsic(PiNode.class)
132-
public static native KlassPointer piCastNonNull(Object object, GuardingNode anchor);
136+
private static native KlassPointer intrinsifiedPiNode(KlassPointer object, GuardingNode anchor, @ConstantNodeParameter int intrinsifyOp);
133137

134138
@Override
135139
public ValueNode getValue() {
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
/*
2+
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation. Oracle designates this
8+
* particular file as subject to the "Classpath" exception as provided
9+
* by Oracle in the LICENSE file that accompanied this code.
10+
*
11+
* This code is distributed in the hope that it will be useful, but WITHOUT
12+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
13+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
14+
* version 2 for more details (a copy is included in the LICENSE file that
15+
* accompanied this code).
16+
*
17+
* You should have received a copy of the GNU General Public License version
18+
* 2 along with this work; if not, write to the Free Software Foundation,
19+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
20+
*
21+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
22+
* or visit www.oracle.com if you need additional information or have any
23+
* questions.
24+
*/
25+
package org.graalvm.compiler.hotspot.stubs;
26+
27+
import static org.graalvm.compiler.hotspot.stubs.StubUtil.printNumber;
28+
29+
import org.graalvm.compiler.api.replacements.Snippet;
30+
import org.graalvm.compiler.api.replacements.Snippet.ConstantParameter;
31+
import org.graalvm.compiler.debug.GraalError;
32+
import org.graalvm.compiler.hotspot.HotSpotForeignCallLinkage;
33+
import org.graalvm.compiler.hotspot.meta.HotSpotProviders;
34+
import org.graalvm.compiler.hotspot.nodes.AllocaNode;
35+
import org.graalvm.compiler.hotspot.replacements.HotSpotReplacementsUtil;
36+
import org.graalvm.compiler.options.OptionValues;
37+
import org.graalvm.compiler.serviceprovider.JavaVersionUtil;
38+
import org.graalvm.compiler.word.Word;
39+
40+
import jdk.vm.ci.code.Register;
41+
42+
/**
43+
* Stub to allocate a {@link NegativeArraySizeException} thrown by a bytecode when the length of an
44+
* array allocation is negative.
45+
*/
46+
public class NegativeArraySizeExceptionStub extends CreateExceptionStub {
47+
public NegativeArraySizeExceptionStub(OptionValues options, HotSpotProviders providers, HotSpotForeignCallLinkage linkage) {
48+
super("createNegativeArraySizeException", options, providers, linkage);
49+
}
50+
51+
private static final boolean PRINT_LENGTH_IN_EXCEPTION = JavaVersionUtil.JAVA_SPEC >= 11;
52+
53+
@Override
54+
protected Object getConstantParameterValue(int index, String name) {
55+
switch (index) {
56+
case 1:
57+
return providers.getRegisters().getThreadRegister();
58+
case 2:
59+
// required bytes for maximum length + nullbyte
60+
return OutOfBoundsExceptionStub.MAX_INT_STRING_SIZE + 1;
61+
case 3:
62+
return PRINT_LENGTH_IN_EXCEPTION;
63+
default:
64+
throw GraalError.shouldNotReachHere("unknown parameter " + name + " at index " + index);
65+
}
66+
}
67+
68+
@Snippet
69+
private static Object createNegativeArraySizeException(int length, @ConstantParameter Register threadRegister, @ConstantParameter int bufferSizeInBytes,
70+
@ConstantParameter boolean printLengthInException) {
71+
if (printLengthInException) {
72+
Word buffer = AllocaNode.alloca(bufferSizeInBytes, HotSpotReplacementsUtil.wordSize());
73+
Word ptr = printNumber(buffer, length);
74+
ptr.writeByte(0, (byte) 0);
75+
return createException(threadRegister, NegativeArraySizeException.class, buffer);
76+
} else {
77+
return createException(threadRegister, NegativeArraySizeException.class);
78+
}
79+
}
80+
}

compiler/src/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/stubs/OutOfBoundsExceptionStub.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public OutOfBoundsExceptionStub(OptionValues options, HotSpotProviders providers
5050

5151
// JDK-8201593: Print array length in ArrayIndexOutOfBoundsException.
5252
private static final boolean PRINT_LENGTH_IN_EXCEPTION = JavaVersionUtil.JAVA_SPEC >= 11;
53-
private static final int MAX_INT_STRING_SIZE = Integer.toString(Integer.MIN_VALUE).length();
53+
static final int MAX_INT_STRING_SIZE = Integer.toString(Integer.MIN_VALUE).length();
5454
private static final String STR_INDEX = "Index ";
5555
private static final String STR_OUTOFBOUNDSFORLENGTH = " out of bounds for length ";
5656

compiler/src/org.graalvm.compiler.java/src/org/graalvm/compiler/java/BytecodeParser.java

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4626,7 +4626,7 @@ private static Class<?> arrayTypeCodeToClass(int code) {
46264626

46274627
private void genNewPrimitiveArray(int typeCode) {
46284628
ResolvedJavaType elementType = getMetaAccess().lookupJavaType(arrayTypeCodeToClass(typeCode));
4629-
ValueNode length = frameState.pop(JavaKind.Int);
4629+
ValueNode length = maybeEmitExplicitNegativeArraySizeCheck(frameState.pop(JavaKind.Int));
46304630

46314631
for (NodePlugin plugin : graphBuilderConfig.getPlugins().getNodePlugins()) {
46324632
if (plugin.handleNewArray(this, elementType, length)) {
@@ -4639,14 +4639,10 @@ private void genNewPrimitiveArray(int typeCode) {
46394639

46404640
private void genNewObjectArray(int cpi) {
46414641
JavaType type = lookupType(cpi, ANEWARRAY);
4642-
genNewObjectArray(type);
4643-
}
4644-
4645-
private void genNewObjectArray(JavaType type) {
46464642
if (typeIsResolved(type)) {
46474643
genNewObjectArray((ResolvedJavaType) type);
46484644
} else {
4649-
ValueNode length = frameState.pop(JavaKind.Int);
4645+
ValueNode length = maybeEmitExplicitNegativeArraySizeCheck(frameState.pop(JavaKind.Int));
46504646
handleUnresolvedNewObjectArray(type, length);
46514647
}
46524648
}
@@ -4658,7 +4654,7 @@ private void genNewObjectArray(ResolvedJavaType resolvedType) {
46584654
classInitializationPlugin.apply(this, resolvedType.getArrayClass(), this::createCurrentFrameState);
46594655
}
46604656

4661-
ValueNode length = frameState.pop(JavaKind.Int);
4657+
ValueNode length = maybeEmitExplicitNegativeArraySizeCheck(frameState.pop(JavaKind.Int));
46624658
for (NodePlugin plugin : graphBuilderConfig.getPlugins().getNodePlugins()) {
46634659
if (plugin.handleNewArray(this, resolvedType, length)) {
46644660
return;
@@ -4672,15 +4668,11 @@ private void genNewMultiArray(int cpi) {
46724668
JavaType type = lookupType(cpi, MULTIANEWARRAY);
46734669
int rank = getStream().readUByte(bci() + 3);
46744670
ValueNode[] dims = new ValueNode[rank];
4675-
genNewMultiArray(type, rank, dims);
4676-
}
4677-
4678-
private void genNewMultiArray(JavaType type, int rank, ValueNode[] dims) {
46794671
if (typeIsResolved(type)) {
46804672
genNewMultiArray((ResolvedJavaType) type, rank, dims);
46814673
} else {
46824674
for (int i = rank - 1; i >= 0; i--) {
4683-
dims[i] = frameState.pop(JavaKind.Int);
4675+
dims[i] = maybeEmitExplicitNegativeArraySizeCheck(frameState.pop(JavaKind.Int));
46844676
}
46854677
handleUnresolvedNewMultiArray(type, dims);
46864678
}
@@ -4694,7 +4686,7 @@ private void genNewMultiArray(ResolvedJavaType resolvedType, int rank, ValueNode
46944686
}
46954687

46964688
for (int i = rank - 1; i >= 0; i--) {
4697-
dims[i] = frameState.pop(JavaKind.Int);
4689+
dims[i] = maybeEmitExplicitNegativeArraySizeCheck(frameState.pop(JavaKind.Int));
46984690
}
46994691

47004692
for (NodePlugin plugin : graphBuilderConfig.getPlugins().getNodePlugins()) {

compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/PiNode.java

Lines changed: 43 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.graalvm.compiler.core.common.type.StampFactory;
3434
import org.graalvm.compiler.core.common.type.TypeReference;
3535
import org.graalvm.compiler.debug.DebugContext;
36+
import org.graalvm.compiler.debug.GraalError;
3637
import org.graalvm.compiler.graph.Node;
3738
import org.graalvm.compiler.graph.Node.NodeIntrinsicFactory;
3839
import org.graalvm.compiler.graph.NodeClass;
@@ -125,13 +126,29 @@ public static ValueNode create(ValueNode object, ValueNode guard) {
125126
return new PiNode(object, stamp, guard);
126127
}
127128

128-
public static boolean intrinsify(GraphBuilderContext b, ValueNode object, ValueNode guard) {
129-
Stamp stamp = AbstractPointerStamp.pointerNonNull(object.stamp(NodeView.DEFAULT));
130-
ValueNode value = canonical(object, stamp, (GuardingNode) guard, null);
129+
public static final int INTRINSIFY_OP_NON_NULL = 1;
130+
public static final int INTRINSIFY_OP_POSITIVE_INT = 2;
131+
132+
public static boolean intrinsify(GraphBuilderContext b, ValueNode input, ValueNode guard, int intrinsifyOp) {
133+
Stamp stamp;
134+
JavaKind pushKind;
135+
switch (intrinsifyOp) {
136+
case INTRINSIFY_OP_NON_NULL:
137+
stamp = AbstractPointerStamp.pointerNonNull(input.stamp(NodeView.DEFAULT));
138+
pushKind = JavaKind.Object;
139+
break;
140+
case INTRINSIFY_OP_POSITIVE_INT:
141+
stamp = StampFactory.positiveInt();
142+
pushKind = JavaKind.Int;
143+
break;
144+
default:
145+
throw GraalError.shouldNotReachHere();
146+
}
147+
ValueNode value = canonical(input, stamp, (GuardingNode) guard, null);
131148
if (value == null) {
132-
value = new PiNode(object, stamp, guard);
149+
value = new PiNode(input, stamp, guard);
133150
}
134-
b.push(JavaKind.Object, b.append(value));
151+
b.push(pushKind, b.append(value));
135152
return true;
136153
}
137154

@@ -278,19 +295,38 @@ public static Class<?> asNonNullObject(Object object) {
278295
@NodeIntrinsic(PiNode.Placeholder.class)
279296
public static native Object piCastToSnippetReplaceeStamp(Object object);
280297

298+
/**
299+
* Changes the stamp of a primitive value and ensures the newly stamped value is positive and
300+
* does not float above a given guard.
301+
*/
302+
public static int piCastPositive(int value, GuardingNode guard) {
303+
return intrinsified(value, guard, INTRINSIFY_OP_POSITIVE_INT);
304+
}
305+
306+
@NodeIntrinsic
307+
private static native int intrinsified(int value, GuardingNode guard, @ConstantNodeParameter int intrinsifyOp);
308+
281309
/**
282310
* Changes the stamp of an object and ensures the newly stamped value is non-null and does not
283311
* float above a given guard.
284312
*/
313+
public static Object piCastNonNull(Object object, GuardingNode guard) {
314+
return intrinsified(object, guard, INTRINSIFY_OP_NON_NULL);
315+
}
316+
285317
@NodeIntrinsic
286-
public static native Object piCastNonNull(Object object, GuardingNode guard);
318+
private static native Object intrinsified(Object object, GuardingNode guard, @ConstantNodeParameter int intrinsifyOp);
287319

288320
/**
289321
* Changes the stamp of an object and ensures the newly stamped value is non-null and does not
290322
* float above a given guard.
291323
*/
324+
public static Class<?> piCastNonNullClass(Class<?> type, GuardingNode guard) {
325+
return intrinsified(type, guard, INTRINSIFY_OP_NON_NULL);
326+
}
327+
292328
@NodeIntrinsic
293-
public static native Class<?> piCastNonNullClass(Class<?> type, GuardingNode guard);
329+
private static native Class<?> intrinsified(Class<?> object, GuardingNode guard, @ConstantNodeParameter int intrinsifyOp);
294330

295331
/**
296332
* Changes the stamp of an object to represent a given type and to indicate that the object is

compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/extended/BytecodeExceptionNode.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,12 @@ public enum BytecodeExceptionKind {
122122
*/
123123
ILLEGAL_ARGUMENT_EXCEPTION_ARGUMENT_IS_NOT_AN_ARRAY(0, IllegalArgumentException.class, "Argument is not an array"),
124124

125+
/**
126+
* Represents a {@link NegativeArraySizeException} with one required int argument for the
127+
* length of the array.
128+
*/
129+
NEGATIVE_ARRAY_SIZE(1, NegativeArraySizeException.class),
130+
125131
/**
126132
* Represents a {@link ArithmeticException}, with the exception message indicating a
127133
* division by zero. No arguments are allowed.
@@ -157,6 +163,10 @@ public enum BytecodeExceptionKind {
157163
public String getExceptionMessage() {
158164
return exceptionMessage;
159165
}
166+
167+
public int getNumArguments() {
168+
return numArguments;
169+
}
160170
}
161171

162172
public static final NodeClass<BytecodeExceptionNode> TYPE = NodeClass.create(BytecodeExceptionNode.class);

compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/graphbuilderconf/GraphBuilderContext.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
import org.graalvm.compiler.nodes.StructuredGraph;
5757
import org.graalvm.compiler.nodes.ValueNode;
5858
import org.graalvm.compiler.nodes.calc.IntegerEqualsNode;
59+
import org.graalvm.compiler.nodes.calc.IntegerLessThanNode;
5960
import org.graalvm.compiler.nodes.calc.IsNullNode;
6061
import org.graalvm.compiler.nodes.calc.NarrowNode;
6162
import org.graalvm.compiler.nodes.calc.SignExtendNode;
@@ -333,6 +334,29 @@ default ValueNode nullCheckedValue(ValueNode value, DeoptimizationAction action)
333334
return value;
334335
}
335336

337+
/**
338+
* When {@link #needsExplicitException} is true, the method returns a node with a stamp that is
339+
* always positive and emits code that trows a {@link NegativeArraySizeException} for a negative
340+
* length.
341+
*/
342+
default ValueNode maybeEmitExplicitNegativeArraySizeCheck(ValueNode arrayLength, BytecodeExceptionKind exceptionKind) {
343+
if (!needsExplicitException() || ((IntegerStamp) arrayLength.stamp(NodeView.DEFAULT)).isPositive()) {
344+
return arrayLength;
345+
}
346+
ConstantNode zero = ConstantNode.defaultForKind(arrayLength.getStackKind());
347+
LogicNode condition = append(IntegerLessThanNode.create(getConstantReflection(), getMetaAccess(), getOptions(), null, arrayLength, zero, NodeView.DEFAULT));
348+
ValueNode[] arguments = exceptionKind.getNumArguments() == 1 ? new ValueNode[]{arrayLength} : new ValueNode[0];
349+
GuardingNode guardingNode = emitBytecodeExceptionCheck(condition, false, exceptionKind, arguments);
350+
if (guardingNode == null) {
351+
return arrayLength;
352+
}
353+
return append(PiNode.create(arrayLength, StampFactory.positiveInt(), guardingNode.asNode()));
354+
}
355+
356+
default ValueNode maybeEmitExplicitNegativeArraySizeCheck(ValueNode arrayLength) {
357+
return maybeEmitExplicitNegativeArraySizeCheck(arrayLength, BytecodeExceptionKind.NEGATIVE_ARRAY_SIZE);
358+
}
359+
336360
default GuardingNode maybeEmitExplicitDivisionByZeroCheck(ValueNode divisor) {
337361
if (!needsExplicitException() || !((IntegerStamp) divisor.stamp(NodeView.DEFAULT)).contains(0)) {
338362
return null;

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -450,7 +450,9 @@ private static void registerArrayPlugins(InvocationPlugins plugins, Replacements
450450
r.register2("newInstance", Class.class, int.class, new InvocationPlugin() {
451451
@Override
452452
public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Receiver unused, ValueNode componentType, ValueNode length) {
453-
b.addPush(JavaKind.Object, new DynamicNewArrayNode(componentType, length, true));
453+
ValueNode componentTypeNonNull = b.nullCheckedValue(componentType);
454+
ValueNode lengthPositive = b.maybeEmitExplicitNegativeArraySizeCheck(length);
455+
b.addPush(JavaKind.Object, new DynamicNewArrayNode(componentTypeNonNull, lengthPositive, true));
454456
return true;
455457
}
456458
});

0 commit comments

Comments
 (0)