Skip to content

Commit

Permalink
[GR-58286] JSConstantNumericUnitNode should never be executed.
Browse files Browse the repository at this point in the history
PullRequest: js/3265
  • Loading branch information
woess committed Sep 20, 2024
2 parents 9e3bac7 + 19b3734 commit 94fc310
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2445,9 +2445,9 @@ private JavaScriptNode enterUnaryIncDecNode(UnaryNode unaryNode) {
}
}

BinaryOperation operation = unaryNode.tokenType() == TokenType.INCPREFIX || unaryNode.tokenType() == TokenType.INCPOSTFIX ? BinaryOperation.ADD : BinaryOperation.SUBTRACT;
BinaryOperation operation = unaryNode.tokenType() == TokenType.INCPREFIX || unaryNode.tokenType() == TokenType.INCPOSTFIX ? BinaryOperation.INCREMENT : BinaryOperation.DECREMENT;
boolean isPostfix = unaryNode.tokenType() == TokenType.INCPOSTFIX || unaryNode.tokenType() == TokenType.DECPOSTFIX;
return tagExpression(transformCompoundAssignment(unaryNode, unaryNode.getExpression(), factory.createConstantNumericUnit(), operation, isPostfix, true), unaryNode);
return tagExpression(transformCompoundAssignment(unaryNode, unaryNode.getExpression(), null, operation, isPostfix, true), unaryNode);
}

private static UnaryOperation tokenTypeToUnaryOperation(TokenType tokenType) {
Expand Down Expand Up @@ -2885,15 +2885,20 @@ private JavaScriptNode transformAssignmentIdent(IdentNode identNode, JavaScriptN

// if scopeVar is const, the assignment will never succeed and is only there to perform
// the temporal dead zone check and throw a ReferenceError instead of a TypeError
if (!initializationAssignment && scopeVar.isConst()) {
rhs = checkMutableBinding(rhs, scopeVar.getName());
}
boolean constAssignment = !initializationAssignment && scopeVar.isConst();

if (binaryOp == null) {
assert assignedValue != null;
if (constAssignment) {
rhs = checkMutableBinding(rhs, scopeVar.getName());
}
return scopeVar.createWriteNode(rhs);
} else {
if (isLogicalOp(binaryOp)) {
assert !convertLHSToNumeric && !returnOldValue;
assert !convertLHSToNumeric && !returnOldValue && assignedValue != null;
if (constAssignment) {
rhs = checkMutableBinding(rhs, scopeVar.getName());
}
JavaScriptNode readNode = tagExpression(scopeVar.createReadNode(), identNode);
JavaScriptNode writeNode = scopeVar.createWriteNode(rhs);
return factory.createBinary(context, binaryOp, readNode, writeNode);
Expand All @@ -2913,6 +2918,9 @@ private JavaScriptNode transformAssignmentIdent(IdentNode identNode, JavaScriptN
readNode = prevValueTemp.createWriteNode(readNode);
}
JavaScriptNode binOpNode = tagExpression(factory.createBinary(context, binaryOp, readNode, rhs), identNode);
if (constAssignment) {
binOpNode = checkMutableBinding(binOpNode, scopeVar.getName());
}
JavaScriptNode writeNode = pair.getSecond().apply(binOpNode);
if (returnOldValue) {
return factory.createDual(context, writeNode, prevValueTemp.createReadNode());
Expand All @@ -2936,6 +2944,7 @@ private JavaScriptNode transformPropertyAssignment(AccessNode accessNode, JavaSc
JavaScriptNode target = transform(accessNode.getBase());

if (binaryOp == null) {
assert assignedValue != null;
assignedNode = createWriteProperty(accessNode, target, assignedValue);
} else {
JavaScriptNode target1;
Expand All @@ -2949,7 +2958,7 @@ private JavaScriptNode transformPropertyAssignment(AccessNode accessNode, JavaSc
target2 = targetTemp.createReadNode();
}
if (isLogicalOp(binaryOp)) {
assert !convertToNumeric && !returnOldValue;
assert !convertToNumeric && !returnOldValue && assignedValue != null;
JavaScriptNode readNode = tagExpression(createReadProperty(accessNode, target1), accessNode);
JavaScriptNode writeNode = createWriteProperty(accessNode, target2, assignedValue);
assignedNode = factory.createBinary(context, binaryOp, readNode, writeNode);
Expand Down Expand Up @@ -2981,6 +2990,7 @@ private JavaScriptNode transformIndexAssignment(IndexNode indexNode, JavaScriptN
JavaScriptNode elem = transform(indexNode.getIndex());

if (binaryOp == null) {
assert assignedValue != null;
assignedNode = factory.createWriteElementNode(target, elem, assignedValue, context, environment.isStrictMode());
} else {
// Evaluation order:
Expand Down Expand Up @@ -3011,7 +3021,7 @@ private JavaScriptNode transformIndexAssignment(IndexNode indexNode, JavaScriptN
}

if (isLogicalOp(binaryOp)) {
assert !convertToNumeric && !returnOldValue;
assert !convertToNumeric && !returnOldValue && assignedValue != null;
JavaScriptNode readNode = tagExpression(factory.createReadElementNode(context, target1, keyTemp.createWriteNode(elem)), indexNode);
JavaScriptNode writeNode = factory.createCompoundWriteElementNode(target2, readIndex, assignedValue, null, context, environment.isStrictMode());
assignedNode = factory.createBinary(context, binaryOp, readNode, writeNode);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* Licensed under the Universal Permissive License v 1.0 as shown at http://oss.oracle.com/licenses/upl.
*/

/**
* Tests correct immutable binding reassignment handling for `const` variables in the global scope
* via increment (++), decrement (--), and compound assignment operators.
*/

const i = 4; try { i++; fail(); } catch (e) { if (!(e instanceof TypeError)) throw e; }
const j = 4; try { ++j; fail(); } catch (e) { if (!(e instanceof TypeError)) throw e; }
const k = 4; try { k--; fail(); } catch (e) { if (!(e instanceof TypeError)) throw e; }
const l = 4; try { --l; fail(); } catch (e) { if (!(e instanceof TypeError)) throw e; }

// If binding is immutable, compound assignment should fail only after the binary operator (including both its operands) has been executed.
let side_effects = 0;
const m = {toString() { side_effects += 1; return "a"; }};
try {
m += {toString() { side_effects += 2; return "b"; }};
fail();
} catch (e) {
if (!(e instanceof TypeError)) throw e;
}
if (side_effects !== 3) throw new Error(`${side_effects}`);

// TypeError due to mixing BigInt and Number is thrown first.
const n = 1n;
try {
n += 1;
fail();
} catch (e) {
if (!(e instanceof TypeError && e.message.includes("BigInt"))) throw e;
}

function fail() {
throw new Error("Expected a TypeError, but no error was thrown.");
}
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@
import com.oracle.truffle.js.nodes.binary.InNode;
import com.oracle.truffle.js.nodes.binary.InstanceofNode;
import com.oracle.truffle.js.nodes.binary.JSAddNode;
import com.oracle.truffle.js.nodes.binary.JSAddSubNumericUnitNode;
import com.oracle.truffle.js.nodes.binary.JSAndNode;
import com.oracle.truffle.js.nodes.binary.JSBitwiseAndNode;
import com.oracle.truffle.js.nodes.binary.JSBitwiseOrNode;
Expand Down Expand Up @@ -280,7 +281,9 @@ public enum BinaryOperation {
INSTANCEOF,
IN,
DUAL,
NULLISH_COALESCING
NULLISH_COALESCING,
INCREMENT,
DECREMENT,
}

public enum UnaryOperation {
Expand Down Expand Up @@ -395,6 +398,12 @@ public JavaScriptNode createBinary(JSContext context, BinaryOperation operation,
return InNode.create(context, left, right);
case DUAL:
return DualNode.create(left, right);
case INCREMENT:
assert right == null;
return JSAddSubNumericUnitNode.create(left, true, false);
case DECREMENT:
assert right == null;
return JSAddSubNumericUnitNode.create(left, false, false);
default:
throw new IllegalArgumentException();
}
Expand Down Expand Up @@ -453,10 +462,6 @@ public JavaScriptNode createConstantSafeInteger(long value) {
return JSConstantNode.createSafeInteger(SafeInteger.valueOf(value));
}

public JavaScriptNode createConstantNumericUnit() {
return JSConstantNode.createConstantNumericUnit();
}

public JavaScriptNode createConstantDouble(double value) {
return JSConstantNode.createDouble(value);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@
import com.oracle.truffle.js.nodes.instrumentation.JSTags.LiteralTag;
import com.oracle.truffle.js.nodes.instrumentation.NodeObjectDescriptor;
import com.oracle.truffle.js.runtime.BigInt;
import com.oracle.truffle.js.runtime.Errors;
import com.oracle.truffle.js.runtime.JSRuntime;
import com.oracle.truffle.js.runtime.SafeInteger;
import com.oracle.truffle.js.runtime.Strings;
Expand Down Expand Up @@ -150,10 +149,6 @@ public static JSConstantNode createDouble(double value) {
return new JSConstantDoubleNode(value);
}

public static JSConstantNode createConstantNumericUnit() {
return new JSConstantNumericUnitNode();
}

public static JSConstantNode createBoolean(boolean value) {
return new JSConstantBooleanNode(value);
}
Expand Down Expand Up @@ -223,27 +218,6 @@ public Object getValue() {
}
}

public static final class JSConstantNumericUnitNode extends JSConstantNode {

private JSConstantNumericUnitNode() {
}

@Override
public boolean isInstrumentable() {
return false;
}

@Override
public Object execute(VirtualFrame frame) {
throw Errors.shouldNotReachHere();
}

@Override
public Object getValue() {
throw Errors.shouldNotReachHere();
}
}

public static final class JSConstantBigIntNode extends JSConstantNode {
private final BigInt bigIntValue;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* The Universal Permissive License (UPL), Version 1.0
Expand Down Expand Up @@ -56,8 +56,7 @@
import com.oracle.truffle.api.strings.TruffleString;
import com.oracle.truffle.js.nodes.JSFrameSlot;
import com.oracle.truffle.js.nodes.JavaScriptNode;
import com.oracle.truffle.js.nodes.binary.JSAddNode;
import com.oracle.truffle.js.nodes.binary.JSSubtractNode;
import com.oracle.truffle.js.nodes.binary.JSAddSubNumericUnitNode;
import com.oracle.truffle.js.nodes.cast.JSToNumericNode;
import com.oracle.truffle.js.nodes.instrumentation.JSTags.ReadVariableTag;
import com.oracle.truffle.js.nodes.instrumentation.JSTags.WriteVariableTag;
Expand Down Expand Up @@ -223,19 +222,17 @@ abstract class LocalVarOpMaterializedNode extends LocalVarIncNode {
convertOld = cloneUninitialized(JSWriteFrameSlotNode.create(from.getSlotIndex(), from.getIdentifier(), scopeFrameNode, convert, hasTemporalDeadZone), materializedTags);

JavaScriptNode readTmp = JSReadFrameSlotNode.create(from.getSlotIndex(), from.getIdentifier(), scopeFrameNode, hasTemporalDeadZone);
JavaScriptNode one = JSConstantNode.createConstantNumericUnit();
JavaScriptNode opNode;
if (from.op instanceof DecOp) {
opNode = JSSubtractNode.create(readTmp, one);
opNode = JSAddSubNumericUnitNode.create(readTmp, false, false);
} else {
opNode = JSAddNode.create(readTmp, one);
opNode = JSAddSubNumericUnitNode.create(readTmp, true, false);
}
/*
* Have to transfer source sections before cloning and materialization. Some nodes might
* become instrumentable by this operation.
*/
transferSourceSectionAddExpressionTag(from, readTmp);
transferSourceSectionAddExpressionTag(from, one);
transferSourceSectionAddExpressionTag(from, opNode);
this.writeNew = cloneUninitialized(JSWriteFrameSlotNode.create(from.getSlotIndex(), from.getIdentifier(), scopeFrameNode, opNode, hasTemporalDeadZone), materializedTags);
transferSourceSectionAddExpressionTag(from, writeNew);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@
import com.oracle.truffle.js.nodes.access.JSConstantNode;
import com.oracle.truffle.js.nodes.access.JSConstantNode.JSConstantDoubleNode;
import com.oracle.truffle.js.nodes.access.JSConstantNode.JSConstantIntegerNode;
import com.oracle.truffle.js.nodes.access.JSConstantNode.JSConstantNumericUnitNode;
import com.oracle.truffle.js.nodes.access.JSConstantNode.JSConstantStringNode;
import com.oracle.truffle.js.nodes.cast.JSDoubleToStringNode;
import com.oracle.truffle.js.nodes.cast.JSToNumericNode;
Expand All @@ -85,9 +84,6 @@ protected JSAddNode(boolean truncate, JavaScriptNode left, JavaScriptNode right)
}

public static JavaScriptNode create(JavaScriptNode left, JavaScriptNode right, boolean truncate) {
if (right instanceof JSConstantNumericUnitNode) {
return JSAddSubNumericUnitNode.create(left, true, truncate);
}
if (JSConfig.UseSuperOperations) {
if (left instanceof JSConstantIntegerNode && right instanceof JSConstantIntegerNode) {
int leftValue = ((JSConstantIntegerNode) left).executeInt(null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@
import com.oracle.truffle.api.strings.TruffleString;
import com.oracle.truffle.js.nodes.JavaScriptNode;
import com.oracle.truffle.js.nodes.Truncatable;
import com.oracle.truffle.js.nodes.access.JSConstantNode.JSConstantNumericUnitNode;
import com.oracle.truffle.js.nodes.cast.JSToNumericNode;
import com.oracle.truffle.js.runtime.BigInt;
import com.oracle.truffle.js.runtime.Strings;
Expand All @@ -71,9 +70,6 @@ protected JSSubtractNode(boolean truncate, JavaScriptNode left, JavaScriptNode r
}

public static JavaScriptNode create(JavaScriptNode left, JavaScriptNode right, boolean truncate) {
if (right instanceof JSConstantNumericUnitNode) {
return JSAddSubNumericUnitNode.create(left, false, truncate);
}
return JSSubtractNodeGen.create(truncate, left, right);
}

Expand Down

0 comments on commit 94fc310

Please sign in to comment.