Skip to content

[GR-34403] Add missing exception edge for array allocations. #3883

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,7 @@ static final class Exceptions {
cachedExceptions.put(BytecodeExceptionKind.OUT_OF_BOUNDS, clearStackTrace(new ArrayIndexOutOfBoundsException()));
cachedExceptions.put(BytecodeExceptionKind.CLASS_CAST, clearStackTrace(new ClassCastException()));
cachedExceptions.put(BytecodeExceptionKind.ARRAY_STORE, clearStackTrace(new ArrayStoreException()));
cachedExceptions.put(BytecodeExceptionKind.NEGATIVE_ARRAY_SIZE, clearStackTrace(new NegativeArraySizeException()));
cachedExceptions.put(BytecodeExceptionKind.DIVISION_BY_ZERO, clearStackTrace(new ArithmeticException()));
cachedExceptions.put(BytecodeExceptionKind.ILLEGAL_ARGUMENT_EXCEPTION_ARGUMENT_IS_NOT_AN_ARRAY,
clearStackTrace(new IllegalArgumentException(BytecodeExceptionKind.ILLEGAL_ARGUMENT_EXCEPTION_ARGUMENT_IS_NOT_AN_ARRAY.getExceptionMessage())));
Expand All @@ -780,6 +781,7 @@ public static final class RuntimeCalls {
runtimeCalls.put(BytecodeExceptionKind.CLASS_CAST, new ForeignCallSignature("createClassCastException", ClassCastException.class, Object.class, KlassPointer.class));
runtimeCalls.put(BytecodeExceptionKind.NULL_POINTER, new ForeignCallSignature("createNullPointerException", NullPointerException.class));
runtimeCalls.put(BytecodeExceptionKind.OUT_OF_BOUNDS, new ForeignCallSignature("createOutOfBoundsException", ArrayIndexOutOfBoundsException.class, int.class, int.class));
runtimeCalls.put(BytecodeExceptionKind.NEGATIVE_ARRAY_SIZE, new ForeignCallSignature("createNegativeArraySizeException", NegativeArraySizeException.class, int.class));
runtimeCalls.put(BytecodeExceptionKind.DIVISION_BY_ZERO, new ForeignCallSignature("createDivisionByZeroException", ArithmeticException.class));
runtimeCalls.put(BytecodeExceptionKind.INTEGER_EXACT_OVERFLOW, new ForeignCallSignature("createIntegerExactOverflowException", ArithmeticException.class));
runtimeCalls.put(BytecodeExceptionKind.LONG_EXACT_OVERFLOW, new ForeignCallSignature("createLongExactOverflowException", ArithmeticException.class));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@
import org.graalvm.compiler.hotspot.stubs.IllegalArgumentExceptionArgumentIsNotAnArrayStub;
import org.graalvm.compiler.hotspot.stubs.IntegerExactOverflowExceptionStub;
import org.graalvm.compiler.hotspot.stubs.LongExactOverflowExceptionStub;
import org.graalvm.compiler.hotspot.stubs.NegativeArraySizeExceptionStub;
import org.graalvm.compiler.hotspot.stubs.NullPointerExceptionStub;
import org.graalvm.compiler.hotspot.stubs.OutOfBoundsExceptionStub;
import org.graalvm.compiler.hotspot.stubs.Stub;
Expand Down Expand Up @@ -477,6 +478,8 @@ public void initialize(HotSpotProviders providers, OptionValues options) {
registerStubCall(exceptionRuntimeCalls.get(BytecodeExceptionKind.NULL_POINTER), SAFEPOINT, NOT_REEXECUTABLE, DESTROYS_ALL_CALLER_SAVE_REGISTERS, any())));
link(new OutOfBoundsExceptionStub(options, providers,
registerStubCall(exceptionRuntimeCalls.get(BytecodeExceptionKind.OUT_OF_BOUNDS), SAFEPOINT, NOT_REEXECUTABLE, DESTROYS_ALL_CALLER_SAVE_REGISTERS, any())));
link(new NegativeArraySizeExceptionStub(options, providers,
registerStubCall(exceptionRuntimeCalls.get(BytecodeExceptionKind.NEGATIVE_ARRAY_SIZE), SAFEPOINT, NOT_REEXECUTABLE, DESTROYS_ALL_CALLER_SAVE_REGISTERS, any())));
link(new DivisionByZeroExceptionStub(options, providers,
registerStubCall(exceptionRuntimeCalls.get(BytecodeExceptionKind.DIVISION_BY_ZERO), SAFEPOINT, NOT_REEXECUTABLE, DESTROYS_ALL_CALLER_SAVE_REGISTERS, any())));
link(new IntegerExactOverflowExceptionStub(options, providers,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@
import org.graalvm.compiler.graph.Node;
import org.graalvm.compiler.graph.Node.NodeIntrinsicFactory;
import org.graalvm.compiler.graph.NodeClass;
import org.graalvm.compiler.nodes.spi.Canonicalizable;
import org.graalvm.compiler.nodes.spi.CanonicalizerTool;
import org.graalvm.compiler.hotspot.nodes.type.KlassPointerStamp;
import org.graalvm.compiler.hotspot.word.KlassPointer;
import org.graalvm.compiler.nodeinfo.NodeInfo;
Expand All @@ -49,6 +47,8 @@
import org.graalvm.compiler.nodes.graphbuilderconf.GraphBuilderContext;
import org.graalvm.compiler.nodes.memory.ReadNode;
import org.graalvm.compiler.nodes.memory.address.AddressNode;
import org.graalvm.compiler.nodes.spi.Canonicalizable;
import org.graalvm.compiler.nodes.spi.CanonicalizerTool;
import org.graalvm.compiler.nodes.spi.Lowerable;
import org.graalvm.word.LocationIdentity;

Expand Down Expand Up @@ -129,11 +129,11 @@ public Node canonical(CanonicalizerTool tool) {
public static native KlassPointer readClass(Class<?> clazzNonNull);

public static KlassPointer piCastNonNull(KlassPointer object, GuardingNode anchor) {
return intrinsifiedPiNode(object, anchor, PiNode.INTRINSIFY_OP_NON_NULL);
return intrinsifiedPiNode(object, anchor, PiNode.IntrinsifyOp.NON_NULL);
}

@NodeIntrinsic(PiNode.class)
private static native KlassPointer intrinsifiedPiNode(KlassPointer object, GuardingNode anchor, @ConstantNodeParameter int intrinsifyOp);
private static native KlassPointer intrinsifiedPiNode(KlassPointer object, GuardingNode anchor, @ConstantNodeParameter PiNode.IntrinsifyOp intrinsifyOp);

@Override
public ValueNode getValue() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation. Oracle designates this
* particular file as subject to the "Classpath" exception as provided
* by Oracle in the LICENSE file that accompanied this code.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
package org.graalvm.compiler.hotspot.stubs;

import static org.graalvm.compiler.hotspot.stubs.StubUtil.printNumber;

import org.graalvm.compiler.api.replacements.Snippet;
import org.graalvm.compiler.api.replacements.Snippet.ConstantParameter;
import org.graalvm.compiler.debug.GraalError;
import org.graalvm.compiler.hotspot.HotSpotForeignCallLinkage;
import org.graalvm.compiler.hotspot.meta.HotSpotProviders;
import org.graalvm.compiler.hotspot.nodes.AllocaNode;
import org.graalvm.compiler.hotspot.replacements.HotSpotReplacementsUtil;
import org.graalvm.compiler.options.OptionValues;
import org.graalvm.compiler.serviceprovider.JavaVersionUtil;
import org.graalvm.compiler.word.Word;

import jdk.vm.ci.code.Register;

/**
* Stub to allocate a {@link NegativeArraySizeException} thrown by a bytecode when the length of an
* array allocation is negative.
*/
public class NegativeArraySizeExceptionStub extends CreateExceptionStub {
public NegativeArraySizeExceptionStub(OptionValues options, HotSpotProviders providers, HotSpotForeignCallLinkage linkage) {
super("createNegativeArraySizeException", options, providers, linkage);
}

private static final boolean PRINT_LENGTH_IN_EXCEPTION = JavaVersionUtil.JAVA_SPEC >= 11;

@Override
protected Object getConstantParameterValue(int index, String name) {
switch (index) {
case 1:
return providers.getRegisters().getThreadRegister();
case 2:
// required bytes for maximum length + nullbyte
return OutOfBoundsExceptionStub.MAX_INT_STRING_SIZE + 1;
case 3:
return PRINT_LENGTH_IN_EXCEPTION;
default:
throw GraalError.shouldNotReachHere("unknown parameter " + name + " at index " + index);
}
}

@Snippet
private static Object createNegativeArraySizeException(int length, @ConstantParameter Register threadRegister, @ConstantParameter int bufferSizeInBytes,
@ConstantParameter boolean printLengthInException) {
if (printLengthInException) {
Word buffer = AllocaNode.alloca(bufferSizeInBytes, HotSpotReplacementsUtil.wordSize());
Word ptr = printNumber(buffer, length);
ptr.writeByte(0, (byte) 0);
return createException(threadRegister, NegativeArraySizeException.class, buffer);
} else {
return createException(threadRegister, NegativeArraySizeException.class);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public OutOfBoundsExceptionStub(OptionValues options, HotSpotProviders providers

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4644,7 +4644,7 @@ private static Class<?> arrayTypeCodeToClass(int code) {

private void genNewPrimitiveArray(int typeCode) {
ResolvedJavaType elementType = getMetaAccess().lookupJavaType(arrayTypeCodeToClass(typeCode));
ValueNode length = frameState.pop(JavaKind.Int);
ValueNode length = maybeEmitExplicitNegativeArraySizeCheck(frameState.pop(JavaKind.Int));

for (NodePlugin plugin : graphBuilderConfig.getPlugins().getNodePlugins()) {
if (plugin.handleNewArray(this, elementType, length)) {
Expand All @@ -4657,14 +4657,10 @@ private void genNewPrimitiveArray(int typeCode) {

private void genNewObjectArray(int cpi) {
JavaType type = lookupType(cpi, ANEWARRAY);
genNewObjectArray(type);
}

private void genNewObjectArray(JavaType type) {
if (typeIsResolved(type)) {
genNewObjectArray((ResolvedJavaType) type);
} else {
ValueNode length = frameState.pop(JavaKind.Int);
ValueNode length = maybeEmitExplicitNegativeArraySizeCheck(frameState.pop(JavaKind.Int));
handleUnresolvedNewObjectArray(type, length);
}
}
Expand All @@ -4676,7 +4672,7 @@ private void genNewObjectArray(ResolvedJavaType resolvedType) {
classInitializationPlugin.apply(this, resolvedType.getArrayClass(), this::createCurrentFrameState);
}

ValueNode length = frameState.pop(JavaKind.Int);
ValueNode length = maybeEmitExplicitNegativeArraySizeCheck(frameState.pop(JavaKind.Int));
for (NodePlugin plugin : graphBuilderConfig.getPlugins().getNodePlugins()) {
if (plugin.handleNewArray(this, resolvedType, length)) {
return;
Expand All @@ -4690,15 +4686,11 @@ private void genNewMultiArray(int cpi) {
JavaType type = lookupType(cpi, MULTIANEWARRAY);
int rank = getStream().readUByte(bci() + 3);
ValueNode[] dims = new ValueNode[rank];
genNewMultiArray(type, rank, dims);
}

private void genNewMultiArray(JavaType type, int rank, ValueNode[] dims) {
if (typeIsResolved(type)) {
genNewMultiArray((ResolvedJavaType) type, rank, dims);
} else {
for (int i = rank - 1; i >= 0; i--) {
dims[i] = frameState.pop(JavaKind.Int);
dims[i] = maybeEmitExplicitNegativeArraySizeCheck(frameState.pop(JavaKind.Int));
}
handleUnresolvedNewMultiArray(type, dims);
}
Expand All @@ -4712,7 +4704,7 @@ private void genNewMultiArray(ResolvedJavaType resolvedType, int rank, ValueNode
}

for (int i = rank - 1; i >= 0; i--) {
dims[i] = frameState.pop(JavaKind.Int);
dims[i] = maybeEmitExplicitNegativeArraySizeCheck(frameState.pop(JavaKind.Int));
}

for (NodePlugin plugin : graphBuilderConfig.getPlugins().getNodePlugins()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,18 +126,20 @@ public static ValueNode create(ValueNode object, ValueNode guard) {
return new PiNode(object, stamp, guard);
}

public static final int INTRINSIFY_OP_NON_NULL = 1;
public static final int INTRINSIFY_OP_POSITIVE_INT = 2;
public enum IntrinsifyOp {
NON_NULL,
POSITIVE_INT
}

public static boolean intrinsify(GraphBuilderContext b, ValueNode input, ValueNode guard, int intrinsifyOp) {
public static boolean intrinsify(GraphBuilderContext b, ValueNode input, ValueNode guard, IntrinsifyOp intrinsifyOp) {
Stamp piStamp;
JavaKind pushKind;
switch (intrinsifyOp) {
case INTRINSIFY_OP_NON_NULL:
case NON_NULL:
piStamp = AbstractPointerStamp.pointerNonNull(input.stamp(NodeView.DEFAULT));
pushKind = JavaKind.Object;
break;
case INTRINSIFY_OP_POSITIVE_INT:
case POSITIVE_INT:
piStamp = StampFactory.positiveInt();
pushKind = JavaKind.Int;
break;
Expand Down Expand Up @@ -324,33 +326,33 @@ public static Class<?> asNonNullObject(Object object) {
* {@code guard}
*/
public static int piCastPositive(int value, GuardingNode guard) {
return intrinsified(value, guard, INTRINSIFY_OP_POSITIVE_INT);
return intrinsified(value, guard, IntrinsifyOp.POSITIVE_INT);
}

@NodeIntrinsic
private static native int intrinsified(int value, GuardingNode guard, @ConstantNodeParameter int intrinsifyOp);
private static native int intrinsified(int value, GuardingNode guard, @ConstantNodeParameter IntrinsifyOp intrinsifyOp);

/**
* Changes the stamp of an object and ensures the newly stamped value is non-null and does not
* float above a given guard.
*/
public static Object piCastNonNull(Object object, GuardingNode guard) {
return intrinsified(object, guard, INTRINSIFY_OP_NON_NULL);
return intrinsified(object, guard, IntrinsifyOp.NON_NULL);
}

@NodeIntrinsic
private static native Object intrinsified(Object object, GuardingNode guard, @ConstantNodeParameter int intrinsifyOp);
private static native Object intrinsified(Object object, GuardingNode guard, @ConstantNodeParameter IntrinsifyOp intrinsifyOp);

/**
* Changes the stamp of an object and ensures the newly stamped value is non-null and does not
* float above a given guard.
*/
public static Class<?> piCastNonNullClass(Class<?> type, GuardingNode guard) {
return intrinsified(type, guard, INTRINSIFY_OP_NON_NULL);
return intrinsified(type, guard, IntrinsifyOp.NON_NULL);
}

@NodeIntrinsic
private static native Class<?> intrinsified(Class<?> object, GuardingNode guard, @ConstantNodeParameter int intrinsifyOp);
private static native Class<?> intrinsified(Class<?> object, GuardingNode guard, @ConstantNodeParameter IntrinsifyOp intrinsifyOp);

/**
* Changes the stamp of an object to represent a given type and to indicate that the object is
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,12 @@ public enum BytecodeExceptionKind {
*/
ILLEGAL_ARGUMENT_EXCEPTION_ARGUMENT_IS_NOT_AN_ARRAY(0, IllegalArgumentException.class, "Argument is not an array"),

/**
* Represents a {@link NegativeArraySizeException} with one required int argument for the
* length of the array.
*/
NEGATIVE_ARRAY_SIZE(1, NegativeArraySizeException.class),

/**
* Represents a {@link ArithmeticException}, with the exception message indicating a
* division by zero. No arguments are allowed.
Expand Down Expand Up @@ -157,6 +163,10 @@ public enum BytecodeExceptionKind {
public String getExceptionMessage() {
return exceptionMessage;
}

public int getNumArguments() {
return numArguments;
}
}

public static final NodeClass<BytecodeExceptionNode> TYPE = NodeClass.create(BytecodeExceptionNode.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
import org.graalvm.compiler.nodes.StructuredGraph;
import org.graalvm.compiler.nodes.ValueNode;
import org.graalvm.compiler.nodes.calc.IntegerEqualsNode;
import org.graalvm.compiler.nodes.calc.IntegerLessThanNode;
import org.graalvm.compiler.nodes.calc.IsNullNode;
import org.graalvm.compiler.nodes.calc.NarrowNode;
import org.graalvm.compiler.nodes.calc.SignExtendNode;
Expand Down Expand Up @@ -334,6 +335,28 @@ default ValueNode nullCheckedValue(ValueNode value, DeoptimizationAction action)
return value;
}

/**
* When {@link #needsExplicitException} is true, the method returns a node with a stamp that is
* always positive and emits code that throws the provided exceptionKind for a negative length.
*/
default ValueNode maybeEmitExplicitNegativeArraySizeCheck(ValueNode arrayLength, BytecodeExceptionKind exceptionKind) {
if (!needsExplicitException() || ((IntegerStamp) arrayLength.stamp(NodeView.DEFAULT)).isPositive()) {
return arrayLength;
}
ConstantNode zero = ConstantNode.defaultForKind(arrayLength.getStackKind());
LogicNode condition = append(IntegerLessThanNode.create(getConstantReflection(), getMetaAccess(), getOptions(), null, arrayLength, zero, NodeView.DEFAULT));
ValueNode[] arguments = exceptionKind.getNumArguments() == 1 ? new ValueNode[]{arrayLength} : new ValueNode[0];
GuardingNode guardingNode = emitBytecodeExceptionCheck(condition, false, exceptionKind, arguments);
if (guardingNode == null) {
return arrayLength;
}
return append(PiNode.create(arrayLength, StampFactory.positiveInt(), guardingNode.asNode()));
}

default ValueNode maybeEmitExplicitNegativeArraySizeCheck(ValueNode arrayLength) {
return maybeEmitExplicitNegativeArraySizeCheck(arrayLength, BytecodeExceptionKind.NEGATIVE_ARRAY_SIZE);
}

default GuardingNode maybeEmitExplicitDivisionByZeroCheck(ValueNode divisor) {
if (!needsExplicitException() || !((IntegerStamp) divisor.stamp(NodeView.DEFAULT)).contains(0)) {
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,9 @@ private static void registerArrayPlugins(InvocationPlugins plugins, Replacements
r.register2("newInstance", Class.class, int.class, new InvocationPlugin() {
@Override
public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Receiver unused, ValueNode componentType, ValueNode length) {
b.addPush(JavaKind.Object, new DynamicNewArrayNode(componentType, length, true));
ValueNode componentTypeNonNull = b.nullCheckedValue(componentType);
ValueNode lengthPositive = b.maybeEmitExplicitNegativeArraySizeCheck(length);
b.addPush(JavaKind.Object, new DynamicNewArrayNode(componentTypeNonNull, lengthPositive, true));
return true;
}
});
Expand Down
Loading