Skip to content

Commit 2cdaa1a

Browse files
woesswoess
committed
[GR-63039] Iterator helpers close receiver on argument validation failure.
PullRequest: js/3451
2 parents cecd9f4 + 6a1bf36 commit 2cdaa1a

File tree

4 files changed

+288
-125
lines changed

4 files changed

+288
-125
lines changed

‎graal-js/mx.graal-js/mx_graal_js.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@
4747
TEST262_REPO = "https://" + "github.com/tc39/test262.git"
4848

4949
# Git revision of Test262 to checkout
50-
TEST262_REV = "82ebbb4060b0a6696f87e79d643dfcb4db0827cd"
50+
TEST262_REV = "48bb2621838bac390f38f4fdf0735e5cbecfaed5"
5151

5252
# Git repository of V8
5353
TESTV8_REPO = "https://" + "github.com/v8/v8.git"

‎graal-js/src/com.oracle.truffle.js.test/js/GR-63039.js

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
/*
2+
* Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* Licensed under the Universal Permissive License v 1.0 as shown at http://oss.oracle.com/licenses/upl.
6+
*/
7+
8+
/**
9+
* Iterator helpers should only close the receiver if it is an Object.
10+
*
11+
* @option iterator-helpers
12+
*/
13+
14+
load("./assert.js")
15+
16+
let returnCalled = 0;
17+
Number.prototype.return = function() { returnCalled++ };
18+
19+
for (const iteratorMethod of [
20+
Iterator.prototype.drop,
21+
Iterator.prototype.take,
22+
]) {
23+
returnCalled = 0;
24+
assertThrows(() => iteratorMethod.call(42), TypeError, "not an Object");
25+
assertSame(0, returnCalled);
26+
assertThrows(() => iteratorMethod.call(new Number(42)), RangeError);
27+
assertSame(1, returnCalled);
28+
}
29+
30+
for (const iteratorMethod of [
31+
Iterator.prototype.filter,
32+
Iterator.prototype.map,
33+
Iterator.prototype.flatMap,
34+
Iterator.prototype.reduce,
35+
Iterator.prototype.find,
36+
Iterator.prototype.some,
37+
Iterator.prototype.every,
38+
Iterator.prototype.forEach,
39+
]) {
40+
returnCalled = 0;
41+
assertThrows(() => iteratorMethod.call(42, function() {}), TypeError, "not an Object");
42+
assertSame(0, returnCalled);
43+
assertThrows(() => iteratorMethod.call(42), TypeError, "not an Object");
44+
assertSame(0, returnCalled);
45+
assertThrows(() => iteratorMethod.call(new Number(42)), TypeError, "not a function");
46+
assertSame(1, returnCalled);
47+
}

‎graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/builtins/IteratorPrototypeBuiltins.java

Lines changed: 76 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,7 @@ protected final IteratorRecord getIteratorDirect(Object thisObj) {
336336
protected abstract static class IteratorMethodWithCallableNode extends IteratorMethodNode {
337337

338338
@Child private IsCallableNode isCallableNode;
339+
@Child private IteratorCloseNode iteratorCloseNode;
339340

340341
protected IteratorMethodWithCallableNode(JSContext context, JSBuiltin builtin) {
341342
super(context, builtin);
@@ -345,6 +346,19 @@ protected IteratorMethodWithCallableNode(JSContext context, JSBuiltin builtin) {
345346
public final boolean isCallable(Object fn) {
346347
return isCallableNode.executeBoolean(fn);
347348
}
349+
350+
/**
351+
* Closes the iterator on argument validation failure.
352+
*
353+
* @param iterator the receiver iterator object
354+
*/
355+
protected final void iteratorCloseDirectAbrupt(Object iterator) {
356+
if (iteratorCloseNode == null) {
357+
CompilerDirectives.transferToInterpreterAndInvalidate();
358+
iteratorCloseNode = insert(IteratorCloseNode.create(getContext()));
359+
}
360+
iteratorCloseNode.executeAbrupt(iterator);
361+
}
348362
}
349363

350364
protected abstract static class IteratorFromGeneratorNode<T extends IteratorArgs> extends IteratorMethodWithCallableNode {
@@ -500,8 +514,13 @@ public JSDynamicObject map(Object thisObj, Object mapper) {
500514
}
501515

502516
@Specialization(guards = "!isCallable(mapper)")
503-
public Object unsupported(@SuppressWarnings("unused") Object thisObj, @SuppressWarnings("unused") Object mapper) {
504-
throw Errors.createTypeErrorCallableExpected();
517+
public Object unsupported(Object thisObj, Object mapper,
518+
@Cached IsObjectNode isObjectNode) {
519+
if (!isObjectNode.executeBoolean(thisObj)) {
520+
throw Errors.createTypeErrorNotAnObject(thisObj, this);
521+
}
522+
iteratorCloseDirectAbrupt(thisObj);
523+
throw Errors.createTypeErrorNotAFunction(mapper);
505524
}
506525

507526
protected abstract static class IteratorMapNextNode extends IteratorFromGeneratorImplNode<IteratorMapArgs> {
@@ -568,8 +587,13 @@ public JSDynamicObject filter(Object thisObj, Object filterer) {
568587
}
569588

570589
@Specialization(guards = "!isCallable(filterer)")
571-
public Object unsupported(@SuppressWarnings("unused") Object thisObj, @SuppressWarnings("unused") Object filterer) {
572-
throw Errors.createTypeErrorCallableExpected();
590+
public Object unsupported(Object thisObj, Object filterer,
591+
@Cached IsObjectNode isObjectNode) {
592+
if (!isObjectNode.executeBoolean(thisObj)) {
593+
throw Errors.createTypeErrorNotAnObject(thisObj, this);
594+
}
595+
iteratorCloseDirectAbrupt(thisObj);
596+
throw Errors.createTypeErrorNotAFunction(filterer);
573597
}
574598

575599
protected abstract static class IteratorFilterNextNode extends IteratorFromGeneratorImplNode<IteratorFilterArgs> {
@@ -647,22 +671,27 @@ public IteratorTakeArgs(IteratorRecord target, double limit) {
647671
public JSDynamicObject take(Object thisObj, Object limit,
648672
@Cached IsObjectNode isObjectNode,
649673
@Cached InlinedBranchProfile errorBranch) {
650-
651674
if (!isObjectNode.executeBoolean(thisObj)) {
652675
errorBranch.enter(this);
653676
throw Errors.createTypeErrorNotAnObject(thisObj, this);
654677
}
655678

656-
Number numLimit = toNumberNode.executeNumber(limit);
657-
if (JSRuntime.isNaN(numLimit)) {
658-
errorBranch.enter(this);
659-
throw Errors.createRangeError("NaN is not allowed", this);
660-
}
679+
double integerLimit;
680+
try {
681+
Number numLimit = toNumberNode.executeNumber(limit);
682+
if (JSRuntime.isNaN(numLimit)) {
683+
errorBranch.enter(this);
684+
throw Errors.createRangeError("NaN is not allowed", this);
685+
}
661686

662-
double integerLimit = JSRuntime.doubleValue(toIntegerOrInfinityNode.executeNumber(numLimit));
663-
if (integerLimit < 0) {
664-
errorBranch.enter(this);
665-
throw Errors.createRangeErrorIndexNegative(this);
687+
integerLimit = JSRuntime.doubleValue(toIntegerOrInfinityNode.executeNumber(numLimit));
688+
if (integerLimit < 0) {
689+
errorBranch.enter(this);
690+
throw Errors.createRangeErrorIndexNegative(this);
691+
}
692+
} catch (AbstractTruffleException e) {
693+
iteratorCloseDirectAbrupt(thisObj);
694+
throw e;
666695
}
667696

668697
IteratorRecord iterated = getIteratorDirect(thisObj);
@@ -733,22 +762,27 @@ public IteratorDropArgs(IteratorRecord target, double limit) {
733762
public JSDynamicObject drop(Object thisObj, Object limit,
734763
@Cached IsObjectNode isObjectNode,
735764
@Cached InlinedBranchProfile errorBranch) {
736-
737765
if (!isObjectNode.executeBoolean(thisObj)) {
738766
errorBranch.enter(this);
739767
throw Errors.createTypeErrorNotAnObject(thisObj, this);
740768
}
741769

742-
Number numLimit = toNumberNode.executeNumber(limit);
743-
if (JSRuntime.isNaN(numLimit)) {
744-
errorBranch.enter(this);
745-
throw Errors.createRangeError("NaN is not allowed", this);
746-
}
770+
double integerLimit;
771+
try {
772+
Number numLimit = toNumberNode.executeNumber(limit);
773+
if (JSRuntime.isNaN(numLimit)) {
774+
errorBranch.enter(this);
775+
throw Errors.createRangeError("NaN is not allowed", this);
776+
}
747777

748-
double integerLimit = JSRuntime.doubleValue(toIntegerOrInfinityNode.executeNumber(numLimit));
749-
if (integerLimit < 0) {
750-
errorBranch.enter(this);
751-
throw Errors.createRangeErrorIndexNegative(this);
778+
integerLimit = JSRuntime.doubleValue(toIntegerOrInfinityNode.executeNumber(numLimit));
779+
if (integerLimit < 0) {
780+
errorBranch.enter(this);
781+
throw Errors.createRangeErrorIndexNegative(this);
782+
}
783+
} catch (AbstractTruffleException e) {
784+
iteratorCloseDirectAbrupt(thisObj);
785+
throw e;
752786
}
753787

754788
IteratorRecord iterated = getIteratorDirect(thisObj);
@@ -818,8 +852,13 @@ public JSDynamicObject flatMap(Object thisObj, Object mapper) {
818852
}
819853

820854
@Specialization(guards = "!isCallable(mapper)")
821-
public Object unsupported(@SuppressWarnings("unused") Object thisObj, @SuppressWarnings("unused") Object mapper) {
822-
throw Errors.createTypeErrorCallableExpected();
855+
public Object unsupported(Object thisObj, Object mapper,
856+
@Cached IsObjectNode isObjectNode) {
857+
if (!isObjectNode.executeBoolean(thisObj)) {
858+
throw Errors.createTypeErrorNotAnObject(thisObj, this);
859+
}
860+
iteratorCloseDirectAbrupt(thisObj);
861+
throw Errors.createTypeErrorNotAFunction(mapper);
823862
}
824863

825864
protected abstract static class IteratorFlatMapNextNode extends IteratorFromGeneratorNode.IteratorFromGeneratorImplNode<IteratorFlatMapArgs> {
@@ -966,7 +1005,11 @@ protected Object compatible(Object thisObj, Object fn,
9661005
}
9671006

9681007
@Specialization(guards = "!isCallable(fn)")
969-
protected void incompatible(@SuppressWarnings("unused") Object thisObj, Object fn) {
1008+
protected void incompatible(Object thisObj, Object fn) {
1009+
if (!isObjectNode.executeBoolean(thisObj)) {
1010+
throw Errors.createTypeErrorNotAnObject(thisObj, this);
1011+
}
1012+
iteratorCloseDirectAbrupt(thisObj);
9701013
throw Errors.createTypeErrorNotAFunction(fn);
9711014
}
9721015

@@ -1131,7 +1174,12 @@ protected Object reduce(Object thisObj, Object reducer, Object[] args,
11311174
}
11321175

11331176
@Specialization(guards = "!isCallable(reducer)")
1134-
protected void incompatible(@SuppressWarnings("unused") Object thisObj, Object reducer, @SuppressWarnings("unused") Object[] args) {
1177+
protected void incompatible(Object thisObj, Object reducer, @SuppressWarnings("unused") Object[] args,
1178+
@Cached IsObjectNode isObjectNode) {
1179+
if (!isObjectNode.executeBoolean(thisObj)) {
1180+
throw Errors.createTypeErrorNotAnObject(thisObj, this);
1181+
}
1182+
iteratorCloseDirectAbrupt(thisObj);
11351183
throw Errors.createTypeErrorNotAFunction(reducer);
11361184
}
11371185

0 commit comments

Comments
 (0)