diff --git a/JSTests/BUN-TEST-DIFFERENCES.md b/JSTests/BUN-TEST-DIFFERENCES.md index a25802d7fe587..eb9b274973074 100644 --- a/JSTests/BUN-TEST-DIFFERENCES.md +++ b/JSTests/BUN-TEST-DIFFERENCES.md @@ -235,6 +235,7 @@ Not upstream's: a sync neither brings nor changes them. Most are named `stress/b | `stress/inline-cache-stub-freed-during-call-to-custom-setter.js` | an inline cache stub that has called native code stays until the call returns, also when the callee signed the return address that it saved (`JITStubRoutineSet::mark`). The setter of `$vm`'s `customAccessor` is compiled with `pac-ret` on ARM64 Linux in every build. On a processor without pointer authentication nothing is signed and the test passes whatever the code does | | `stress/bun-return-address-of-function-is-signed.js` | with `ENABLE(RETURN_ADDRESS_SIGNING)` a function signs the return address that it saves in LLInt, Baseline, the DFG and the FTL alike: `$vm.returnAddressOfCallerIsSigned()` signs the address again and compares. Elsewhere, and on a processor without pointer authentication (`$vm.functionsSignReturnAddress()`), the test ends at once | | `stress/bun-return-address-with-wrong-signature-is-not-returned-to.js` | with `ENABLE(RETURN_ADDRESS_SIGNING)` a function does not return once a bit of the signature of its saved return address was flipped, in each of the four tiers: the run must crash. Where nothing is signed the test crashes by itself | +| `stress/dfg-arrow-function-with-super-call-is-compiled-once.js` | the DFG compiles an arrow function that calls `super()` once, and the check for a second `super()` still throws in its code: the load of `this` from the scope in front of that check gives the empty value each time, which no value profile records (`op_get_from_scope` in `DFGByteCodeParser.cpp`). The same recompile loop is in WebKit/WebKit: once a sync brings a fix from there, this row and the fork's line go | | the fr-CA block of `stress/intl-date-pattern-includes-literal-text.js` (the file is upstream's) | keeps the file's coverage on CLDR 49, where fr's pattern no longer has literal text: an `h` inside literal text is not mistaken for the hour field | | the last block of `stress/regexp-test-sticky-first-char-filter.js` (the file is upstream's; nothing of upstream's text changes) | the first-character filter of a sticky `test()` comes from the first sets of the alternatives (`yarr/YarrFirstFollowSets.*`): behind a leading lookbehind, forward reference or lookahead, and in groups nested more than eight deep, a byte that passes the filter still reaches the matcher | diff --git a/JSTests/stress/dfg-arrow-function-with-super-call-is-compiled-once.js b/JSTests/stress/dfg-arrow-function-with-super-call-is-compiled-once.js new file mode 100644 index 0000000000000..7211b41871894 --- /dev/null +++ b/JSTests/stress/dfg-arrow-function-with-super-call-is-compiled-once.js @@ -0,0 +1,78 @@ +//@ skip if not $jitTests +//@ runDefault("--useConcurrentJIT=0") + +// An arrow function that calls super() first reads |this| of its derived constructor from the scope, to see that no +// super() came before. The value is empty each time, and a value profile does not record the empty value. The DFG takes +// such a load for a load of the empty value, not for code that did not run yet: its code for the arrow function stays. + +function shouldBe(actual, expected, name) +{ + if (actual !== expected) + throw new Error(`${name}: expected ${String(expected)} but got ${String(actual)}`); +} + +class Base { + constructor() + { + this.own = 1; + } +} + +let initialize; +class Derived extends Base { + constructor() + { + initialize = () => { super(); }; + noInline(initialize); + initialize(); + } +} +noInline(Derived); + +// The shape of a helper that a compiler makes for a lowered class. +let initializeAndUse; +class DerivedWithHelper extends Base { + constructor() + { + initializeAndUse = (...args) => { super(...args); this.field = 2; return this; }; + noInline(initializeAndUse); + initializeAndUse(); + } +} +noInline(DerivedWithHelper); + +// The check for a second super() still throws in the optimized code. +let second; +class DerivedTwice extends Base { + constructor(again) + { + second = () => { super(); }; + noInline(second); + second(); + if (again) + second(); + } +} +noInline(DerivedTwice); + +for (let i = 0; i < testLoopCount; ++i) { + shouldBe(new Derived().own, 1, "Derived"); + shouldBe(new DerivedWithHelper().field, 2, "DerivedWithHelper"); + shouldBe(new DerivedTwice(false).own, 1, "DerivedTwice"); +} + +for (let i = 0; i < 10; ++i) { + let error = null; + try { + new DerivedTwice(true); + } catch (e) { + error = e; + } + shouldBe(error instanceof ReferenceError && error.message, "'super()' can't be called more than once in a constructor.", "a second super()"); +} + +for (const arrowFunction of [initialize, initializeAndUse, second]) { + const compiles = numberOfDFGCompiles(arrowFunction); + if (compiles > 1) + throw new Error(`the DFG compiled ${arrowFunction.name} ${compiles} times`); +} diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp index 836a268e6dfcb..c94de6649a742 100644 --- a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp +++ b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp @@ -10928,11 +10928,13 @@ void ByteCodeParser::parseBlock(unsigned limit) } SpeculatedType prediction = SpecNone; - if (bytecode.m_getPutInfo.resolveType() == ResolvedClosureVar) { + if (bytecode.m_getPutInfo.resolveType() == ResolvedClosureVar || uid == m_vm->propertyNames->builtinNames().thisPrivateName().impl()) { // ResolvedClosureVar is not used normally. It is very special internal ResolveType, mainly used for generators and private fields. // In these variables, it can happen that we use JSEmpty as a result of op_get_from_scope (which becomes a TDZ error in normal ClosureVar). // And this JSEmpty is still legit. The problem is that ValueProfile never tells about JSEmpty since it sees no value is stored when JSEmpty // is stored. We workaround this very special internal use case by explicitly setting SpecEmpty when ValueProfile tells this is SpecNone. + // |this| of a derived constructor is such a variable too: an arrow function that calls super() reads it from the scope first, + // to see that no super() came before, and the value is JSEmpty each time. prediction = getPredictionWithoutOSRExit(); if (prediction == SpecNone) prediction = SpecEmpty;