Skip to content

Commit 12ecbc9

Browse files
author
Christian Wimmer
committed
Add missing exception edge for array allocations
1 parent 55ab7d5 commit 12ecbc9

File tree

12 files changed

+150
-44
lines changed

12 files changed

+150
-44
lines changed

‎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() {

‎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: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,11 @@ 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}. No arguments are allowed.
127+
*/
128+
NEGATIVE_ARRAY_SIZE(0, NegativeArraySizeException.class),
129+
125130
/**
126131
* Represents a {@link ArithmeticException}, with the exception message indicating a
127132
* division by zero. No arguments are allowed.

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

Lines changed: 16 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,21 @@ 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) {
343+
if (!needsExplicitException() || ((IntegerStamp) arrayLength.stamp(NodeView.DEFAULT)).isPositive()) {
344+
return arrayLength;
345+
}
346+
ConstantNode zero = add(ConstantNode.defaultForKind(arrayLength.getStackKind()));
347+
LogicNode condition = add(IntegerLessThanNode.create(getConstantReflection(), getMetaAccess(), getOptions(), null, arrayLength, zero, NodeView.DEFAULT));
348+
GuardingNode guardingNode = emitBytecodeExceptionCheck(condition, false, BytecodeExceptionKind.NEGATIVE_ARRAY_SIZE);
349+
return getGraph().addOrUniqueWithInputs(PiNode.create(arrayLength, StampFactory.positiveInt(), guardingNode.asNode()));
350+
}
351+
336352
default GuardingNode maybeEmitExplicitDivisionByZeroCheck(ValueNode divisor) {
337353
if (!needsExplicitException() || !((IntegerStamp) divisor.stamp(NodeView.DEFAULT)).contains(0)) {
338354
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
});

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/snippets/NonSnippetLowerings.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ protected NonSnippetLowerings(RuntimeConfiguration runtimeConfig, Predicate<Reso
140140
getCachedExceptionDescriptors.put(BytecodeExceptionKind.ARRAY_STORE, ImplicitExceptions.GET_CACHED_ARRAY_STORE_EXCEPTION);
141141
getCachedExceptionDescriptors.put(BytecodeExceptionKind.ILLEGAL_ARGUMENT_EXCEPTION_NEGATIVE_LENGTH, ImplicitExceptions.GET_CACHED_ILLEGAL_ARGUMENT_EXCEPTION);
142142
getCachedExceptionDescriptors.put(BytecodeExceptionKind.ILLEGAL_ARGUMENT_EXCEPTION_ARGUMENT_IS_NOT_AN_ARRAY, ImplicitExceptions.GET_CACHED_ILLEGAL_ARGUMENT_EXCEPTION);
143+
getCachedExceptionDescriptors.put(BytecodeExceptionKind.NEGATIVE_ARRAY_SIZE, ImplicitExceptions.GET_CACHED_NEGATIVE_ARRAY_SIZE_EXCEPTION);
143144
getCachedExceptionDescriptors.put(BytecodeExceptionKind.DIVISION_BY_ZERO, ImplicitExceptions.GET_CACHED_ARITHMETIC_EXCEPTION);
144145
getCachedExceptionDescriptors.put(BytecodeExceptionKind.ASSERTION_ERROR_NULLARY, ImplicitExceptions.GET_CACHED_ASSERTION_ERROR);
145146
getCachedExceptionDescriptors.put(BytecodeExceptionKind.ASSERTION_ERROR_OBJECT, ImplicitExceptions.GET_CACHED_ASSERTION_ERROR);
@@ -152,6 +153,7 @@ protected NonSnippetLowerings(RuntimeConfiguration runtimeConfig, Predicate<Reso
152153
createExceptionDescriptors.put(BytecodeExceptionKind.ARRAY_STORE, ImplicitExceptions.CREATE_ARRAY_STORE_EXCEPTION);
153154
createExceptionDescriptors.put(BytecodeExceptionKind.ILLEGAL_ARGUMENT_EXCEPTION_NEGATIVE_LENGTH, ImplicitExceptions.CREATE_ILLEGAL_ARGUMENT_EXCEPTION);
154155
createExceptionDescriptors.put(BytecodeExceptionKind.ILLEGAL_ARGUMENT_EXCEPTION_ARGUMENT_IS_NOT_AN_ARRAY, ImplicitExceptions.CREATE_ILLEGAL_ARGUMENT_EXCEPTION);
156+
createExceptionDescriptors.put(BytecodeExceptionKind.NEGATIVE_ARRAY_SIZE, ImplicitExceptions.CREATE_NEGATIVE_ARRAY_SIZE_EXCEPTION);
155157
createExceptionDescriptors.put(BytecodeExceptionKind.DIVISION_BY_ZERO, ImplicitExceptions.CREATE_DIVISION_BY_ZERO_EXCEPTION);
156158
createExceptionDescriptors.put(BytecodeExceptionKind.ASSERTION_ERROR_NULLARY, ImplicitExceptions.CREATE_ASSERTION_ERROR_NULLARY);
157159
createExceptionDescriptors.put(BytecodeExceptionKind.ASSERTION_ERROR_OBJECT, ImplicitExceptions.CREATE_ASSERTION_ERROR_OBJECT);
@@ -164,6 +166,7 @@ protected NonSnippetLowerings(RuntimeConfiguration runtimeConfig, Predicate<Reso
164166
throwCachedExceptionDescriptors.put(BytecodeExceptionKind.ARRAY_STORE, ImplicitExceptions.THROW_CACHED_ARRAY_STORE_EXCEPTION);
165167
throwCachedExceptionDescriptors.put(BytecodeExceptionKind.ILLEGAL_ARGUMENT_EXCEPTION_NEGATIVE_LENGTH, ImplicitExceptions.THROW_CACHED_ILLEGAL_ARGUMENT_EXCEPTION);
166168
throwCachedExceptionDescriptors.put(BytecodeExceptionKind.ILLEGAL_ARGUMENT_EXCEPTION_ARGUMENT_IS_NOT_AN_ARRAY, ImplicitExceptions.THROW_CACHED_ILLEGAL_ARGUMENT_EXCEPTION);
169+
throwCachedExceptionDescriptors.put(BytecodeExceptionKind.NEGATIVE_ARRAY_SIZE, ImplicitExceptions.THROW_CACHED_NEGATIVE_ARRAY_SIZE_EXCEPTION);
167170
throwCachedExceptionDescriptors.put(BytecodeExceptionKind.DIVISION_BY_ZERO, ImplicitExceptions.THROW_CACHED_ARITHMETIC_EXCEPTION);
168171
throwCachedExceptionDescriptors.put(BytecodeExceptionKind.ASSERTION_ERROR_NULLARY, ImplicitExceptions.THROW_CACHED_ASSERTION_ERROR);
169172
throwCachedExceptionDescriptors.put(BytecodeExceptionKind.ASSERTION_ERROR_OBJECT, ImplicitExceptions.THROW_CACHED_ASSERTION_ERROR);
@@ -176,6 +179,7 @@ protected NonSnippetLowerings(RuntimeConfiguration runtimeConfig, Predicate<Reso
176179
throwNewExceptionDescriptors.put(BytecodeExceptionKind.ARRAY_STORE, ImplicitExceptions.THROW_NEW_ARRAY_STORE_EXCEPTION_WITH_ARGS);
177180
throwNewExceptionDescriptors.put(BytecodeExceptionKind.ILLEGAL_ARGUMENT_EXCEPTION_NEGATIVE_LENGTH, ImplicitExceptions.THROW_NEW_ILLEGAL_ARGUMENT_EXCEPTION_WITH_ARGS);
178181
throwNewExceptionDescriptors.put(BytecodeExceptionKind.ILLEGAL_ARGUMENT_EXCEPTION_ARGUMENT_IS_NOT_AN_ARRAY, ImplicitExceptions.THROW_NEW_ILLEGAL_ARGUMENT_EXCEPTION_WITH_ARGS);
182+
throwNewExceptionDescriptors.put(BytecodeExceptionKind.NEGATIVE_ARRAY_SIZE, ImplicitExceptions.THROW_NEW_NEGATIVE_ARRAY_SIZE_EXCEPTION);
179183
throwNewExceptionDescriptors.put(BytecodeExceptionKind.DIVISION_BY_ZERO, ImplicitExceptions.THROW_NEW_DIVISION_BY_ZERO_EXCEPTION);
180184
throwNewExceptionDescriptors.put(BytecodeExceptionKind.ASSERTION_ERROR_NULLARY, ImplicitExceptions.THROW_NEW_ASSERTION_ERROR_NULLARY);
181185
throwNewExceptionDescriptors.put(BytecodeExceptionKind.ASSERTION_ERROR_OBJECT, ImplicitExceptions.THROW_NEW_ASSERTION_ERROR_OBJECT);

0 commit comments

Comments
 (0)