Skip to content

Commit f958a63

Browse files
committed
Fix canonicalization of always-null guarded values.
1 parent 28d3c7e commit f958a63

File tree

1 file changed

+20
-3
lines changed

1 file changed

+20
-3
lines changed

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/phases/common/CanonicalizerPhase.java

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import org.graalvm.collections.EconomicSet;
3939
import org.graalvm.collections.Pair;
4040

41+
import jdk.graal.compiler.core.common.type.AbstractObjectStamp;
4142
import jdk.graal.compiler.core.common.type.Stamp;
4243
import jdk.graal.compiler.debug.Assertions;
4344
import jdk.graal.compiler.debug.CounterKey;
@@ -77,6 +78,7 @@
7778
import jdk.graal.compiler.nodes.calc.ConditionalNode;
7879
import jdk.graal.compiler.nodes.calc.FloatingNode;
7980
import jdk.graal.compiler.nodes.cfg.ControlFlowGraph;
81+
import jdk.graal.compiler.nodes.extended.GuardedNode;
8082
import jdk.graal.compiler.nodes.extended.GuardingNode;
8183
import jdk.graal.compiler.nodes.loop.InductionVariable;
8284
import jdk.graal.compiler.nodes.spi.Canonicalizable;
@@ -615,11 +617,16 @@ private boolean processNode(Node node, Tool tool, NodeBitMap singleShotList) {
615617
if (tryCanonicalize(node, nodeClass, tool)) {
616618
return true;
617619
}
618-
if (node instanceof ValueNode) {
619-
ValueNode valueNode = (ValueNode) node;
620+
if (node instanceof ValueNode valueNode) {
620621
boolean improvedStamp = tryInferStamp(valueNode, tool);
621622
Constant constant = valueNode.stamp(NodeView.DEFAULT).asConstant();
622-
if (constant != null && !(node instanceof ConstantNode)) {
623+
if (constant != null && !(node instanceof ConstantNode) && !isAnchoredNullConstant(valueNode)) {
624+
/*
625+
* Do not fold always-null Pis to null constants. Doing so could lead to dependent
626+
* reads floating above safety checks, potentially causing runtime segfaults. By
627+
* contrast, folding integer constants that could cause out-of-bounds reads is safe
628+
* as JVMCI detects these and fails gracefully.
629+
*/
623630
ConstantNode stampConstant = ConstantNode.forConstant(valueNode.stamp(NodeView.DEFAULT), constant, tool.context.getMetaAccess(), graph);
624631
valueNode.replaceAtUsages(stampConstant, InputType.Value);
625632
GraphUtil.tryKillUnused(valueNode);
@@ -640,6 +647,16 @@ private boolean processNode(Node node, Tool tool, NodeBitMap singleShotList) {
640647
return false;
641648
}
642649

650+
/**
651+
* Tests if {@code node} is an always-null value that is anchored.
652+
*/
653+
private static boolean isAnchoredNullConstant(ValueNode node) {
654+
if (node instanceof GuardedNode guarded && guarded.getGuard() != null) {
655+
return node.stamp(NodeView.DEFAULT).isObjectStamp() && ((AbstractObjectStamp) node.stamp(NodeView.DEFAULT)).alwaysNull();
656+
}
657+
return false;
658+
}
659+
643660
public static boolean gvn(Node node, NodeClass<?> nodeClass) {
644661
if (nodeClass.valueNumberable()) {
645662
Node newNode = node.graph().findDuplicate(node);

0 commit comments

Comments
 (0)