Bug 295660 – Instantiating an extended class inside overwritten Array.prototype[Symbol.iterator] throws RangeError

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Meghan Denny]

Code to reproduce

```js
Array.prototype[Symbol.iterator] = function () {
  class A {}
  class B extends A {}
  new B();
};
for (const _ of []) {
}
```

Expected Behavior

```
❯ ~/WebKit/WebKitBuild/Debug/bin/jsc test.js
Exception: TypeError: Iterator result interface is not an object.
global [e-mail]:6:14
```

Actual Behavior

```
❯ ~/WebKit/WebKitBuild/Debug/bin/jsc test.js
Exception: RangeError: Maximum call stack size exceeded.
B@
@test.js:4:8
B@
@test.js:4:8
B@
@test.js:4:8
B@
@test.js:4:8
...
```

Observed in

jsc shell @master
Safari Version 18.5 (20621.2.5.11.8)

[Radar WebKit Bug Importer]

<rdar://problem/155999441>

[Itsuru Nomaki]

# About

This bug appears to be caused by behavior that is inconsitsent with the spec note in the runtime semantics for class definition evaluation:

> NOTE: This branch behaves similarly to constructor(...args) { super(...args); }. The most notable distinction is that while the aforementioned ECMAScript source text observably calls the @@iterator method on %Array.prototype%, this function does not.
https://tc39.es/proposal-class-brand-check/#sec-runtime-semantics-classdefinitionevaluation

In this testcase, overriding `Array.prototype[Symbol.iterator]` causes re-entrancy through the implicit derived constructor path, eventually leading to unbounded recursion.

However, JSC currently synthesizes `(function (...args) { super(...args); })` for default derived constructors internally (at `builtins/BuiltinExecutables.cpp:L60 / defaultConstructorSourceCode()`), and compiles it through the normal rest/spread-based path rather than using a dedicated non-observable forwarding path.

As a result, the synthesized constructor materializes rest arguments and performs spread before `super_construct_varargs`, which makes the operation observably consult `Array.prototype[Symbol.iterator]`.

The following dumped bytecode shows the relevant part of the synthesized default derived constructor:

Environment:

* Revision: `ef14489fb3cc`
* Machine: MacBook Air (M2, 2022), 16 GB RAM

```text
bb#1
Predecessors: [ ]
[   0] enter              
[   1] mov                dst:loc5, src:callee
[   4] mov                dst:loc6, src:this
[   7] mov                dst:this, src:<JSValue()>(const0)
[  10] mov                dst:loc7, src:<JSValue()>(const0)
[  13] create_rest        dst:loc7, numParametersToSkip:0
[  16] get_prototype_of   dst:loc8, value:callee, valueProfile:1
[  20] mov                dst:loc11, src:loc7
[  23] spread             dst:loc11, argument:loc11
[  26] mov                dst:loc12, src:loc6
[  29] super_construct_varargs dst:loc8, callee:loc8, thisValue:loc12, arguments:loc11, firstFree:loc13, firstVarArg:0, valueProfile:2
[  38] is_empty           dst:loc13, operand:this
[  41] jtrue              condition:loc13, targetLabel:6(->47)
Successors: [ #3 #2 ]
```

# Fixing Plans

There seem to be two possible directions for fixing this issue.

### 1. Stop synthesizing the source text for default derived constructors

One possible fix is to stop synthesizing source text such as `(function (...args) { super(...args); })` for default derived constructors, introduce an explicit `isDefaultDerivedConstructor` flag in `UnlinkedFunctionExecutable`, and add a dedicated bytecode-generation path in `BytecodeGenerator` for this case.

This direction appears more principled, because it avoids modeling the implicit derived constructor as ordinary rest/spread-based source code in the first place. In particular, it would make it possible to implement a non-observable forwarding path that does not go through `create_rest` / `spread`.

However, this approach seems to have a relatively large implementation surface. It would affect the executable/function metadata layer in addition to bytecode generation. Also, at least from my current investigation, it is not yet clear how the actual arguments should be forwarded to `super` without using rest/spread while still keeping the implementation self-contained and consistent with the existing machinery.

### 2. Restrict the fix to `emitBytecode`

Another possible fix is to keep the current synthesized source text, but special-case the default derived constructor path during bytecode generation, for example around `FunctionCallValueNode::emitBytecode`, so that the synthesized `super(...args)` does not get compiled through the normal observable rest/spread path.

This direction appears smaller in scope, since it would localize the fix to bytecode generation and avoid broader changes to constructor synthesis or executable metadata.

The downside is that the synthesized source code itself would remain in the program, even though it would no longer accurately describe the effective execution path. That may be confusing for future maintenance. In addition, I do not yet have a clear way to obtain the actual argument count at runtime from this path without introducing further machinery, so even this seemingly smaller approach still has an unresolved implementation issue.

This is my first time investigating and proposing a change in JSC, so my understanding of some parts of the codebase may still be incomplete. I would greatly appreciate any feedback, corrections, or guidance on the analysis and the proposed directions above.

[Itsuru Nomaki]

Pull Request: https://github.com/WebKit/WebKit/pull/63689

[EWS]

Committed 313130@main (cf863e88b16a): <https://commits.webkit.org/313130@main>

Reviewed commits have been landed. Closing PR #63689 and removing active labels.