From 9ed503430bcc13d58266351ece0d024ea320d11c Mon Sep 17 00:00:00 2001 From: Andreas Woess Date: Thu, 19 Sep 2024 17:45:27 +0200 Subject: [PATCH 1/2] [GR-58286] Fix const binding increment, decrement, and compound assignment. --- .../truffle/js/parser/GraalJSTranslator.java | 13 ++++-- .../js/const_reassign_ident_global.js | 40 +++++++++++++++++++ 2 files changed, 50 insertions(+), 3 deletions(-) create mode 100644 graal-js/src/com.oracle.truffle.js.test/js/const_reassign_ident_global.js diff --git a/graal-js/src/com.oracle.truffle.js.parser/src/com/oracle/truffle/js/parser/GraalJSTranslator.java b/graal-js/src/com.oracle.truffle.js.parser/src/com/oracle/truffle/js/parser/GraalJSTranslator.java index 4935a04a28f..c89584872aa 100644 --- a/graal-js/src/com.oracle.truffle.js.parser/src/com/oracle/truffle/js/parser/GraalJSTranslator.java +++ b/graal-js/src/com.oracle.truffle.js.parser/src/com/oracle/truffle/js/parser/GraalJSTranslator.java @@ -2885,15 +2885,19 @@ 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) { + if (constAssignment) { + rhs = checkMutableBinding(rhs, scopeVar.getName()); + } return scopeVar.createWriteNode(rhs); } else { if (isLogicalOp(binaryOp)) { assert !convertLHSToNumeric && !returnOldValue; + 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); @@ -2913,6 +2917,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()); diff --git a/graal-js/src/com.oracle.truffle.js.test/js/const_reassign_ident_global.js b/graal-js/src/com.oracle.truffle.js.test/js/const_reassign_ident_global.js new file mode 100644 index 00000000000..0725d761079 --- /dev/null +++ b/graal-js/src/com.oracle.truffle.js.test/js/const_reassign_ident_global.js @@ -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."); +} From 19b3734c541e090a4bbf177b68e5e2d5b2b15de4 Mon Sep 17 00:00:00 2001 From: Andreas Woess Date: Thu, 19 Sep 2024 18:33:49 +0200 Subject: [PATCH 2/2] Remove constant numeric unit node. --- .../truffle/js/parser/GraalJSTranslator.java | 13 ++++++---- .../oracle/truffle/js/nodes/NodeFactory.java | 15 +++++++---- .../js/nodes/access/JSConstantNode.java | 26 ------------------- .../js/nodes/access/LocalVarIncNode.java | 11 +++----- .../truffle/js/nodes/binary/JSAddNode.java | 4 --- .../js/nodes/binary/JSSubtractNode.java | 4 --- 6 files changed, 22 insertions(+), 51 deletions(-) diff --git a/graal-js/src/com.oracle.truffle.js.parser/src/com/oracle/truffle/js/parser/GraalJSTranslator.java b/graal-js/src/com.oracle.truffle.js.parser/src/com/oracle/truffle/js/parser/GraalJSTranslator.java index c89584872aa..9aac1ea7d38 100644 --- a/graal-js/src/com.oracle.truffle.js.parser/src/com/oracle/truffle/js/parser/GraalJSTranslator.java +++ b/graal-js/src/com.oracle.truffle.js.parser/src/com/oracle/truffle/js/parser/GraalJSTranslator.java @@ -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) { @@ -2888,13 +2888,14 @@ private JavaScriptNode transformAssignmentIdent(IdentNode identNode, JavaScriptN 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()); } @@ -2943,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; @@ -2956,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); @@ -2988,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: @@ -3018,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); diff --git a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/NodeFactory.java b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/NodeFactory.java index 43a575379a5..5601d81651a 100644 --- a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/NodeFactory.java +++ b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/NodeFactory.java @@ -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; @@ -280,7 +281,9 @@ public enum BinaryOperation { INSTANCEOF, IN, DUAL, - NULLISH_COALESCING + NULLISH_COALESCING, + INCREMENT, + DECREMENT, } public enum UnaryOperation { @@ -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(); } @@ -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); } diff --git a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/access/JSConstantNode.java b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/access/JSConstantNode.java index aa814836f92..1369ab705bd 100644 --- a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/access/JSConstantNode.java +++ b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/access/JSConstantNode.java @@ -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; @@ -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); } @@ -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; diff --git a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/access/LocalVarIncNode.java b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/access/LocalVarIncNode.java index f6aae4ab8a1..d5f5e5febd7 100644 --- a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/access/LocalVarIncNode.java +++ b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/access/LocalVarIncNode.java @@ -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 @@ -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; @@ -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); diff --git a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/binary/JSAddNode.java b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/binary/JSAddNode.java index 131c6e82f2c..f14d08995d5 100644 --- a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/binary/JSAddNode.java +++ b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/binary/JSAddNode.java @@ -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; @@ -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); diff --git a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/binary/JSSubtractNode.java b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/binary/JSSubtractNode.java index 8dec38847cc..38ad60ddc80 100644 --- a/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/binary/JSSubtractNode.java +++ b/graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/binary/JSSubtractNode.java @@ -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; @@ -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); }