CVE-2025-43441

Medium CVSS 4.3 webkit Logic Error 🔧 Commit mapped

Overview

Medium

Severity

4.3

CVSS

Exploited ITW

Fixed

Fix Status

DescriptionProcessing maliciously crafted web content may lead to an unexpected process crash

ComponentJSC

Sub-componentJSC Wasm

Bug ClassLogic Error

Tracker298496

Fix commit11eaa3910d44 (WebKit/WebKit) +289/-215

CWECWE-119, CWE-120 (Buffer bounds error, Buffer overflow)

CVSS vectorCVSS:3.1/AV:N/AC:L/PR:N/UI:R/S:U/C:N/I:N/A:L

CISA KEVNot listed

Creditedrheza (@ginggilBesel)

Disclosed2025-11-03

Background

MacroAssembler scratch register: A register the JIT reserves for internal use when lowering a single macro instruction; code must declare AllowMacroScratchRegisterUsage before any macro op is permitted to clobber it.
AllowMacroScratchRegisterUsage / DisallowMacroScratchRegisterUsage: RAII guard objects that toggle, for their lexical scope, whether emitted macro instructions may use (and thus overwrite) the scratch register.
Patchpoint generator: A callback the B3/Air backend invokes at code-emission time to emit custom machine code for a call site, receiving the physical register/stack assignments of its inputs via StackmapGenerationParams.
WebAssembly tail call: A call that reuses the caller’s stack frame instead of pushing a new one; the JIT must shuffle arguments into their final positions and adjust SP without a valid intermediate frame, making register/stack scratch management delicate.
OMG JIT: JavaScriptCore’s Optimizing Maximal-Grade WebAssembly tier that recompiles hot wasm functions through the B3/Air optimizing backend.

Root Cause Analysis

The patch fixes an unsafe use of the MacroAssembler scratch register in the OMG (optimizing) WebAssembly JIT during two related code-generation paths: the indirect-call patchpoint generator in emitIndirectCall and the tail-call setup helper prepareForTailCallImpl (both the 64-bit WasmOMGIRGenerator.cpp and the 32/64-bit WasmOMGIRGenerator32_64.cpp variants). WebKit’s MacroAssembler reserves a dedicated scratch/tmp register whose use is gated by AllowMacroScratchRegisterUsage / DisallowMacroScratchRegisterUsage RAII guards; any macro instruction emitted while AllowMacroScratchRegisterUsage is in scope is permitted to silently clobber that register.

The invariant that was violated is that the scratch register must not be marked allowable while it still holds a live input value. In emitIndirectCall, AllowMacroScratchRegisterUsage was constructed at the very top of the patchpoint generator, so the subsequent storeWasmCalleeToCalleeCallFrame (which may itself materialize the callee value via the scratch register) and the surrounding sequencing could clobber the callee-code pointer held in a param GPR before the farJump consumed it.

The fix moves the AllowMacroScratchRegisterUsage construction to after storeWasmCalleeToCalleeCallFrame, with the comment ‘Allow scratch after the callee is stored, which could be in the scratch register.’ In prepareForTailCallImpl the same class of bug existed across the whole argument-shuffling routine: the old code held a single AllowMacroScratchRegisterUsage over the entire body while it restored callee-saves, spilled/moved tail-call arguments, and finally adjusted SP and jumped, so an argument that happened to live in the scratch (tmp) register could be destroyed by any macro instruction that reached for scratch. The rewrite hoists the frame-size and spill-area computation up front, then wraps the input-live regions in explicit DisallowMacroScratchRegisterUsage scopes (‘Nothing before saving tmp can use the scratch register since it might clobber an input’ and ‘Nothing after restoring tmp can use the scratch register since it might clobber an input’), spills tmp to a dedicated stack slot (renamed tmpSpill -> tmpSpillOffsetRelativeToOriginalSP and clobbersTmp -> tmpNeedsSaving) when it is a live argument, and only re-enables scratch in the narrow window where it is safe. The net effect is that no live wasm argument or callee pointer can be silently overwritten by an internal scratch-register spill during tail-call/indirect-call lowering, which previously produced miscompiled machine code and a crash on the added regression test omg-tail-call-clobber-scratch-register.js.

Key insight

The scratch register is only safe to enable once no input value lives in it: the bug was holding AllowMacroScratchRegisterUsage across regions where the callee pointer or a tail-call argument still occupied the scratch/tmp register, so an internal spill silently destroyed live state.

Attack Path

Deliver crafted wasm module Serve a WebAssembly module whose exported function performs an indirect call or a tail call with an argument layout that places a live value (or the callee-code pointer) into the JIT’s scratch/tmp register, matching the encoding in the regression test.
Force OMG tier-up Drive the function hot (the test uses –jitPolicyScale=0) so it is compiled by the OMG optimizing JIT, exercising emitIndirectCall’s patchpoint generator and prepareForTailCallImpl rather than the interpreter or BBQ tier.
Trigger scratch clobber during lowering During code generation the AllowMacroScratchRegisterUsage window causes a macro instruction (e.g. materializing the callee pointer or an argument move) to overwrite the scratch register while it still holds a live input, producing miscompiled code.
Execute the miscompiled call At runtime the tail/indirect call jumps with a corrupted callee-code pointer or shuffles arguments incorrectly, yielding an unexpected process crash (the observed impact) and, in principle, a corrupted control-flow target.
(Inference) Escalate a controlled clobber If an attacker can influence which value lands in the scratch register and the resulting incorrect jump target or argument, this could in theory be steered beyond a crash; the commit itself only establishes the miscompile-and-crash, so any control-flow-hijack escalation is inferred, not shown.

Impact Assessment

As established by the commit, the primitive is a JIT miscompilation in the wasm OMG tier that clobbers a live register during indirect/tail-call lowering, whose demonstrated effect is an unexpected crash in the WebContent process (the regression test is titled ‘Don’t crash’). Because the clobbered value can be a callee-code pointer used by farJump or a live wasm argument shuffled into the callee frame, there is a plausible but unproven path toward corrupted control flow or argument state; the diff only proves a crash, so treating this as a controlled/uncontrolled crash is the grounded reading and any RCE escalation is inference. It is confined to the WebContent process JS/wasm engine and does not by itself cross the sandbox.

Changed Functions

Function	Change	Notes
`if` Source/JavaScriptCore/wasm/WasmOMGIRGenerator.cpp	modified
`for` Source/JavaScriptCore/wasm/WasmOMGIRGenerator.cpp	modified
`for` Source/JavaScriptCore/wasm/WasmOMGIRGenerator32_64.cpp	modified
`if` Source/JavaScriptCore/wasm/WasmOMGIRGenerator32_64.cpp	modified

Files Changed

JSTests/wasm/stress/omg-tail-call-clobber-scratch-register.js
Source/JavaScriptCore/wasm/WasmOMGIRGenerator.cpp
Source/JavaScriptCore/wasm/WasmOMGIRGenerator32_64.cpp

Audit Directions

Other patchpoint generators enabling scratch too early
In the wasm OMG/BBQ generators grep for ‘AllowMacroScratchRegisterUsage allowScratch(jit)’ placed at the top of a patchpoint setGenerator lambda and check whether any params[…].gpr()/fpr() input is still live (used by a later store/jump) after that point; the fix pattern is to construct the guard only immediately before the instruction that consumes the scratch.
Tail-call and CallFrameShuffler scratch handling
Audit prepareForTailCallImpl analogues and CallFrameShuffler/Air Shuffle code for a single wide AllowMacroScratchRegisterUsage scope surrounding emitRestore(calleeSaves), argument moves and SP adjustment; look for tmp = jit.scratchRegister() where tmp may alias a param GPR without a spill guarded by DisallowMacroScratchRegisterUsage.
scratchRegister() aliasing live inputs
Across MacroAssembler users grep for jit.scratchRegister()/dataTempRegister/addressTempRegister used near loops over params[i].gpr() and verify a spill exists when arg.gpr() == scratch; the renamed tmpNeedsSaving/tmpSpillOffsetRelativeToOriginalSP logic is the reference safe pattern.
Missing Disallow scopes around SP transitions
Search for jit.addPtr(…stackPointerRegister) or farJump sequences that lack a surrounding DisallowMacroScratchRegisterUsage, since adjusting SP or jumping while scratch is enabled and inputs are live is the exact miscompile condition this patch closes.

Patch

view commit

diff --git a/Source/JavaScriptCore/wasm/WasmOMGIRGenerator.cpp b/Source/JavaScriptCore/wasm/WasmOMGIRGenerator.cpp
index 00d08fda5902..f3ed7363c49c 100644
--- a/Source/JavaScriptCore/wasm/WasmOMGIRGenerator.cpp
+++ b/Source/JavaScriptCore/wasm/WasmOMGIRGenerator.cpp
@@ -1847,9 +1847,10 @@ auto OMGIRGenerator::emitIndirectCall(Value* calleeInstance, Value* calleeCode,
         patchpoint->append(boxedCalleeCallee, ValueRep::SomeRegister);
         patchArgsIndex += m_proc.resultCount(patchpoint->type());
         patchpoint->setGenerator([prepareForCall = prepareForCall, patchArgsIndex](CCallHelpers& jit, const B3::StackmapGenerationParams& params) {
-            AllowMacroScratchRegisterUsage allowScratch(jit);
             prepareForCall->run(jit, params);
             jit.storeWasmCalleeToCalleeCallFrame(params[patchArgsIndex + 1].gpr(), sizeof(CallerFrameAndPC) - prologueStackPointerDelta());
+            // Allow scratch after the callee is stored, which could be in the scratch register.
+            AllowMacroScratchRegisterUsage allowScratch(jit);
             jit.farJump(params[patchArgsIndex].gpr(), WasmEntryPtrTag);
         });
         return { };
@@ -5542,33 +5543,74 @@ static inline void prepareForTailCallImpl(unsigned functionIndex, CCallHelpers&
     auto& functionSignature = *signature.as<FunctionSignature>();
     const Checked<int32_t> offsetOfFirstSlotFromFP = WTF::roundUpToMultipleOf<stackAlignmentBytes()>(wasmCallerInfoAsCallee.headerAndArgumentStackSizeInBytes);
     JIT_COMMENT(jit, "Set up tail call, new FP offset from FP: ", newFPOffsetFromFP);
-    AllowMacroScratchRegisterUsage allowScratch(jit);
+
+    const unsigned frameSize = params.code().frameSize();
+    ASSERT(WTF::roundUpToMultipleOf<stackAlignmentBytes()>(frameSize) == frameSize);
+    ASSERT(WTF::roundUpToMultipleOf<stackAlignmentBytes()>(std::abs(newFPOffsetFromFP)) == static_cast<size_t>(std::abs(newFPOffsetFromFP)));
+
+    auto fpOffsetToSPOffset = [frameSize](int32_t offset) {
+        return checkedSum<int>(safeCast<int>(frameSize), offset).value();
+    };
+
+    auto newReturnPCOffset = fpOffsetToSPOffset(checkedSum<intptr_t>(CallFrame::returnPCOffset(), newFPOffsetFromFP).value());
+
+    // We requested some extra stack space below via requestCallArgAreaSize
+    // ... FP [initial safe area][caller stack space ] [callArgSpace                    ] SP ...
+    // becomes
+    // ... FP [safe area growing ->    ] [danger           ] [ scratch                  ] SP ...
+    // This scratch space sits at the very bottom of the stack, near sp.
+    // AirLowerStackArgs takes care of adding callArgSpace to our total caller frame size.
+    // BUT, even though we have this extra space, the new frame might be bigger, so we can't
+    // use the new frame as scratch. The new return pc represents the lowest offset from SP we can use.
+    int spillPointer = 0;
+    const int scratchAreaUpperBound = std::min(
+        safeCast<int>(WTF::roundUpToMultipleOf<stackAlignmentBytes()>(static_cast<int>(wasmCalleeInfoAsCallee.headerAndArgumentStackSizeInBytes))),
+        newReturnPCOffset);
+    auto allocateSpill = [&] (Width width) -> int {
+        int offset = spillPointer;
+        spillPointer += bytesForWidth(width);
+        ASSERT(spillPointer <= scratchAreaUpperBound);
+        ASSERT(offset < scratchAreaUpperBound);
+        return offset;
+    };
+
+    RegisterAtOffsetList calleeSaves = params.code().calleeSaveRegisterAtOffsetList();
 
     // Be careful not to clobber this below.
     // We also need to make sure that we preserve this if it is used by the patchpoint body.
-    bool clobbersTmp = false;
+    AllowMacroScratchRegisterUsage allowScratch(jit);
     auto tmp = jit.scratchRegister();
-    int tmpSpill = 0;
+    bool tmpNeedsSaving = false;
+    int tmpSpillOffsetRelativeToOriginalSP = 0;
 
-    // Set up a valid frame so that we can clobber this one.
-    RegisterAtOffsetList calleeSaves = params.code().calleeSaveRegisterAtOffsetList();
-    jit.emitRestore(calleeSaves);
-
-    for (unsigned i = 0; i < params.size(); ++i) {
-        auto arg = params[i];
-        if (arg.isGPR()) {
-            ASSERT(!calleeSaves.find(arg.gpr()));
-            if (arg.gpr() == tmp)
-                clobbersTmp = true;
-            continue;
-        }
-        if (arg.isFPR()) {
-            ASSERT(!calleeSaves.find(arg.fpr()));
-            continue;
+    // Nothing before saving tmp can use the scratch register since it might clobber an input.
+    {
+        DisallowMacroScratchRegisterUsage disallowScratch(jit);
+
+        // Set up a valid frame so that we can clobber this one.
+        jit.emitRestore(calleeSaves);
+
+        for (unsigned i = 0; i < params.size(); ++i) {
+            auto arg = params[i];
+            if (arg.isGPR()) {
+                ASSERT(!calleeSaves.find(arg.gpr()));
+                if (arg.gpr() == tmp)
+                    tmpNeedsSaving = true;
+                continue;
+            }
+            if (arg.isFPR()) {
+                ASSERT(!calleeSaves.find(arg.fpr()));
+                continue;
+            }
         }
+
+        ASSERT(!calleeSaves.find(tmp));
     }
 
-    ASSERT(!calleeSaves.find(tmp));
+    if (tmpNeedsSaving) {
+        tmpSpillOffsetRelativeToOriginalSP = allocateSpill(WidthPtr);
+        jit.storePtr(tmp, CCallHelpers::Address(MacroAssembler::stackPointerRegister, tmpSpillOffsetRelativeToOriginalSP));
+    }
 
 #if ASSERT_ENABLED
     // Let's make sure we never rely on these slots, so we can use them for scratch in the future.
@@ -5579,14 +5621,6 @@ static inline void prepareForTailCallImpl(unsigned functionIndex, CCallHelpers&
         CCallHelpers::Address(MacroAssembler::framePointerRegister, CallFrameSlot::argumentCountIncludingThis * sizeof(Register)));
 #endif
 
-    const unsigned frameSize = params.code().frameSize();
-    ASSERT(WTF::roundUpToMultipleOf<stackAlignmentBytes()>(frameSize) == frameSize);
-    ASSERT(WTF::roundUpToMultipleOf<stackAlignmentBytes()>(std::abs(newFPOffsetFromFP)) == static_cast<size_t>(std::abs(newFPOffsetFromFP)));
-
-    auto fpOffsetToSPOffset = [frameSize](int32_t offset) {
-        return checkedSum<int>(safeCast<int>(frameSize), offset).value();
-    };
-
     JIT_COMMENT(jit, "Let's use the caller's frame, so that we always have a valid frame.");
     if (WasmOMGIRGeneratorInternal::verboseTailCalls) {
         jit.probeDebugSIMD([frameSize, fpOffsetToSPOffset, newFPOffsetFromFP, signature = Ref<const TypeDefinition>(signature), wasmCalleeInfoAsCallee, firstPatchArg, lastPatchArg, params, functionIndex] (Probe::Context& context) {
@@ -5643,31 +5677,9 @@ static inline void prepareForTailCallImpl(unsigned functionIndex, CCallHelpers&
         });
     }
 
-    auto newReturnPCOffset = fpOffsetToSPOffset(checkedSum<intptr_t>(CallFrame::returnPCOffset(), newFPOffsetFromFP).value());
-
     JIT_COMMENT(jit, "Copy over args if needed into their final position, clobbering everything.");
     // This code has a bunch of overlap with CallFrameShuffler and Shuffle in Air/BBQ
 
-    // We requested some extra stack space below via requestCallArgAreaSize
-    // ... FP [initial safe area][caller stack space ] [callArgSpace                    ] SP ...
-    // becomes
-    // ... FP [safe area growing ->    ] [danger           ] [ scratch                  ] SP ...
-    // This scratch space sits at the very bottom of the stack, near sp.
-    // AirLowerStackArgs takes care of adding callArgSpace to our total caller frame size.
-    // BUT, even though we have this extra space, the new frame might be bigger, so we can't
-    // use the new frame as scratch. The new return pc represents the lowest offset from SP we can use.
-    int spillPointer = 0;
-    const int scratchAreaUpperBound = std::min(
-        safeCast<int>(WTF::roundUpToMultipleOf<stackAlignmentBytes()>(static_cast<int>(wasmCalleeInfoAsCallee.headerAndArgumentStackSizeInBytes))),
-        newReturnPCOffset);
-    auto allocateSpill = [&] (Width width) -> int {
-        int offset = spillPointer;
-        spillPointer += bytesForWidth(width);
-        ASSERT(spillPointer <= scratchAreaUpperBound);
-        ASSERT(offset < scratchAreaUpperBound);
-        return offset;
-    };
-
     auto doMove = [&jit, tmp] (int srcOffset, int dstOffset, Width width) {
         JIT_COMMENT(jit, "Do move ", srcOffset, " -> ", dstOffset);
         auto src = CCallHelpers::Address(MacroAssembler::stackPointerRegister, srcOffset);
@@ -5697,11 +5709,6 @@ static inline void prepareForTailCallImpl(unsigned functionIndex, CCallHelpers&
 
     JIT_COMMENT(jit, "SP[", safeAreaLowerBound, "] to SP[", stackUpperBound, "] form the safe portion of the stack to clobber; Scratches go from SP[0] to SP[", scratchAreaUpperBound, "].");
 
-    if (clobbersTmp) {
-        tmpSpill = allocateSpill(WidthPtr);
-        jit.storePtr(tmp, CCallHelpers::Address(MacroAssembler::stackPointerRegister, tmpSpill));
-    }
-
 #if ASSERT_ENABLED
     // Clobber all safe values to make debugging easier.
     for (int i = safeAreaLowerBound; i < stackUpperBound; i += sizeof(Register)) {
@@ -5734,11 +5741,11 @@ static inline void prepareForTailCallImpl(unsigned functionIndex, CCallHelpers&
             continue;
         }
 
-        auto saveSrc = [tmp, clobbersTmp, tmpSpill, dstType, &allocateSpill, &jit, &fpOffsetToSPOffset](ValueRep src) -> std::tuple<int, Width> {
+        auto saveSrc = [tmp, tmpNeedsSaving, tmpSpillOffsetRelativeToOriginalSP, dstType, &allocateSpill, &jit, &fpOffsetToSPOffset](ValueRep src) -> std::tuple<int, Width> {
             int srcOffset = 0;
-            if (clobbersTmp && src.isGPR() && src.gpr() == tmp) {
+            if (tmpNeedsSaving && src.isGPR() && src.gpr() == tmp) {
                 // Before tmp may have been clobbered, it was spilled to tmpSpill.
-                srcOffset = tmpSpill;
+                srcOffset = tmpSpillOffsetRelativeToOriginalSP;
             } else if (src.isGPR()) {
                 srcOffset = allocateSpill(WidthPtr);
                 jit.storePtr(src.gpr(), CCallHelpers::Address(MacroAssembler::stackPointerRegister, srcOffset));
@@ -5865,51 +5872,64 @@ static inline void prepareForTailCallImpl(unsigned functionIndex, CCallHelpers&
 #endif
 #endif
 
-    jit.addPtr(MacroAssembler::TrustedImm32(newSPAtPrologueOffsetFromSP), MacroAssembler::stackPointerRegister);
+    if (tmpNeedsSaving)
+        jit.loadPtr(CCallHelpers::Address(MacroAssembler::stackPointerRegister, tmpSpillOffsetRelativeToOriginalSP), tmp);
+
+    // Nothing after restoring tmp can use the scratch register since it might clobber an input.
+    {
+        DisallowMacroScratchRegisterUsage disallowScratch(jit);
+
+        jit.addPtr(MacroAssembler::TrustedImm32(newSPAtPrologueOffsetFromSP), MacroAssembler::stackPointerRegister);
 
 #if CPU(X86_64)
-    if (WasmOMGIRGeneratorInternal::verboseTailCalls) {
-        jit.probeDebugSIMD([] (Probe::Context& context) {
-            dataLogLn("return pc on the top of the stack: ", RawHex(*context.gpr<uintptr_t*>(MacroAssembler::stackPointerRegister)), " at ", RawHex(context.gpr<uintptr_t>(MacroAssembler::stackPointerRegister)));
-        });
-    }
+        if (WasmOMGIRGeneratorInternal::verboseTailCalls) {
+            jit.probeDebugSIMD([] (Probe::Context& context) {
+                dataLogLn("return pc on the top of the stack: ", RawHex(*context.gpr<uintptr_t*>(MacroAssembler::stackPointerRegister)), " at ", RawHex(context.gpr<uintptr_t>(MacroAssembler::stackPointerRegister)));
+            });
+        }
 #endif
 
+        JIT_COMMENT(jit, "OK, now we can jump.");
+        if (WasmOMGIRGeneratorInternal::verboseTailCalls) {
+            jit.probeDebugSIMD([wasmCalleeInfoAsCallee] (Probe::Context& context) {
+                dataLogLn("Can now jump: FP: ", RawHex(context.gpr<uintptr_t>(GPRInfo::callFrameRegister)), " SP: ", RawHex(context.gpr<uintptr_t>(MacroAssembler::stackPointerRegister)));
+                auto* newFP = context.gpr<uintptr_t*>(MacroAssembler::stackPointerRegister) - prologueStackPointerDelta() / sizeof(uintptr_t);
+                dataLogLn("New (callee) FP at prologue will be at ", RawPointer(newFP));
+                auto fpl = std::bit_cast<uint64_t*>(newFP);
+                auto fpi = std::bit_cast<uint32_t*>(newFP);
+
+                for (unsigned i = 0; i < wasmCalleeInfoAsCallee.params.size(); ++i) {
+                    auto arg = wasmCalleeInfoAsCallee.params[i];
+                    auto src = arg.location;
+                    dataLog("Arg ", i, " located at ", arg.location, " = ");
+                    if (arg.location.isGPR())
+                        dataLog(context.gpr(arg.location.jsr().payloadGPR()), " / ", (int) context.gpr(arg.location.jsr().payloadGPR()));
+                    else if (arg.location.isFPR() && arg.width <= Width::Width64)
+                        dataLog(context.fpr(arg.location.fpr(), SavedFPWidth::SaveVectors));
+                    else if (arg.location.isFPR())
+                        dataLog(context.vector(arg.location.fpr()));
+                    else
+                        dataLog(fpl[src.offsetFromFP() / sizeof(*fpl)], " / ", fpi[src.offsetFromFP() / sizeof(*fpi)],  " / ", RawHex(fpi[src.offsetFromFP() / sizeof(*fpi)]), " / ", std::bit_cast<double>(fpl[src.offsetFromFP() / sizeof(*fpl)]), " at ", RawPointer(&fpi[src.offsetFromFP() / sizeof(*fpi)]));
+                    dataLogLn();
+                }
+            });
+        }
+
 #if ASSERT_ENABLED
-    for (unsigned i = 2; i < 50; ++i) {
-        // Everthing after sp might be overwritten anyway.
-        jit.storePtr(MacroAssembler::TrustedImm32(0xBFFF), CCallHelpers::Address(MacroAssembler::stackPointerRegister, -i * sizeof(uintptr_t)));
-    }
+        // Everything in the old stack might be overwritten anyway. Clobber for easier debugging.
+        if (tmpNeedsSaving)
+            jit.pushPair(tmp, tmp);
+        jit.move(MacroAssembler::TrustedImm32(0xBFFF), tmp);
+        constexpr int stackSlotsToClobber = 50;
+        constexpr int stackBytesToClobber = stackSlotsToClobber * registerSize();
+        static_assert(!(stackBytesToClobber & (stackAlignmentBytes() - 1)), "Size in bytes to clobber on stack is aligned");
+        for (int i = 0; i < stackSlotsToClobber / 2; ++i)
+            jit.pushPair(tmp, tmp);
+        jit.addPtr(MacroAssembler::TrustedImm32(stackBytesToClobber), MacroAssembler::stackPointerRegister);
+        if (tmpNeedsSaving)
+            jit.popPair(tmp, tmp);
 #endif
-
-    JIT_COMMENT(jit, "OK, now we can jump.");
-    if (WasmOMGIRGeneratorInternal::verboseTailCalls) {
-        jit.probeDebugSIMD([wasmCalleeInfoAsCallee] (Probe::Context& context) {
-            dataLogLn("Can now jump: FP: ", RawHex(context.gpr<uintptr_t>(GPRInfo::callFrameRegister)), " SP: ", RawHex(context.gpr<uintptr_t>(MacroAssembler::stackPointerRegister)));
-            auto* newFP = context.gpr<uintptr_t*>(MacroAssembler::stackPointerRegister) - prologueStackPointerDelta() / sizeof(uintptr_t);
-            dataLogLn("New (callee) FP at prologue will be at ", RawPointer(newFP));
-            auto fpl = std::bit_cast<uint64_t*>(newFP);
-            auto fpi = std::bit_cast<uint32_t*>(newFP);
-
-            for (unsigned i = 0; i < wasmCalleeInfoAsCallee.params.size(); ++i) {
-                auto arg = wasmCalleeInfoAsCallee.params[i];
-                auto src = arg.location;
-                dataLog("Arg ", i, " located at ", arg.location, " = ");
-                if (arg.location.isGPR())
-                    dataLog(context.gpr(arg.location.jsr().payloadGPR()), " / ", (int) context.gpr(arg.location.jsr().payloadGPR()));
-                else if (arg.location.isFPR() && arg.width <= Width::Width64)
-                    dataLog(context.fpr(arg.location.fpr(), SavedFPWidth::SaveVectors));
-                else if (arg.location.isFPR())
-                    dataLog(context.vector(arg.location.fpr()));
-                else
-                    dataLog(fpl[src.offsetFromFP() / sizeof(*fpl)], " / ", fpi[src.offsetFromFP() / sizeof(*fpi)],  " / ", RawHex(fpi[src.offsetFromFP() / sizeof(*fpi)]), " / ", std::bit_cast<double>(fpl[src.offsetFromFP() / sizeof(*fpl)]), " at ", RawPointer(&fpi[src.offsetFromFP() / sizeof(*fpi)]));
-                dataLogLn();
-            }
-        });
     }
-
-    if (clobbersTmp)
-        jit.loadPtr(CCallHelpers::Address(MacroAssembler::stackPointerRegister, tmpSpill), tmp);
 }
 
 // See also: https://leaningtech.com/fantastic-tail-calls-and-how-to-implement-them/, a blog post about contributing this feature.
diff --git a/Source/JavaScriptCore/wasm/WasmOMGIRGenerator32_64.cpp b/Source/JavaScriptCore/wasm/WasmOMGIRGenerator32_64.cpp
index 61a0ee6ef488..ff8e913bf851 100644
--- a/Source/JavaScriptCore/wasm/WasmOMGIRGenerator32_64.cpp
+++ b/Source/JavaScriptCore/wasm/WasmOMGIRGenerator32_64.cpp
@@ -1925,7 +1925,6 @@ auto OMGIRGenerator::emitIndirectCall(Value* calleeInstance, Value* calleeCode,
         }
         patchArgsIndex += m_proc.resultCount(patchpoint->type());
         patchpoint->setGenerator([prepareForCall = prepareForCall, patchArgsIndex](CCallHelpers& jit, const B3::StackmapGenerationParams& params) {
-            AllowMacroScratchRegisterUsage allowScratch(jit);
             prepareForCall->run(jit, params);
             GPRReg callTarget;
             GPRReg callee;
@@ -1938,6 +1937,9 @@ auto OMGIRGenerator::emitIndirectCall(Value* calleeInstance, Value* calleeCode,
 
             jit.storeWasmCalleeToCalleeCallFrame(callee, sizeof(CallerFrameAndPC) - prologueStackPointerDelta());
 
+            // Allow scratch after the callee is stored, which could be in the scratch register.
+            AllowMacroScratchRegisterUsage allowScratch(jit);
+
             if (is32Bit() && params[patchArgsIndex].isStack()) {
                 callTarget = MacroAssembler::addressTempRegister;
                 jit.loadPtr(CCallHelpers::Address(MacroAssembler::stackPointerRegister,
@@ -5693,58 +5695,95 @@ static inline void prepareForTailCallImpl(unsigned functionIndex, CCallHelpers&
     auto& functionSignature = *signature.as<FunctionSignature>();
     const Checked<int32_t> offsetOfFirstSlotFromFP = WTF::roundUpToMultipleOf<stackAlignmentBytes()>(wasmCallerInfoAsCallee.headerAndArgumentStackSizeInBytes);
     JIT_COMMENT(jit, "Set up tail call, new FP offset from FP: ", newFPOffsetFromFP);
-    AllowMacroScratchRegisterUsage allowScratch(jit);
+
+    const unsigned frameSize = params.code().frameSize();
+    ASSERT(WTF::roundUpToMultipleOf<stackAlignmentBytes()>(frameSize + sizeof(CallerFrameAndPC)) == frameSize + sizeof(CallerFrameAndPC));
+
+    auto fpOffsetToSPOffset = [frameSize](int32_t offset) {
+        return checkedSum<int>(safeCast<int>(frameSize), offset).value();
+    };
+
+    auto newReturnPCOffset = fpOffsetToSPOffset(checkedSum<intptr_t>(CallFrame::returnPCOffset(), newFPOffsetFromFP).value());
+
+    // We requested some extra stack space below via requestCallArgAreaSize
+    // ... FP [initial safe area][caller stack space ] [callArgSpace                    ] SP ...
+    // becomes
+    // ... FP [safe area growing ->    ] [danger           ] [ scratch                  ] SP ...
+    // This scratch space sits at the very bottom of the stack, near sp.
+    // AirLowerStackArgs takes care of adding callArgSpace to our total caller frame size.
+    // BUT, even though we have this extra space, the new frame might be bigger, so we can't
+    // use the new frame as scratch. The new return pc represents the lowest offset from SP we can use.
+    int spillPointer = 0;
+    const int scratchAreaUpperBound = std::min(
+        safeCast<int>(WTF::roundUpToMultipleOf<stackAlignmentBytes()>(static_cast<int>(wasmCalleeInfoAsCallee.headerAndArgumentStackSizeInBytes))),
+        newReturnPCOffset);
+    auto allocateSpill = [&] (Width width) -> int {
+        int offset = spillPointer;
+        spillPointer += bytesForWidth(width);
+        ASSERT(spillPointer <= scratchAreaUpperBound);
+        ASSERT(offset < scratchAreaUpperBound);
+        return offset;
+    };
+
+    RegisterAtOffsetList calleeSaves = params.code().calleeSaveRegisterAtOffsetList();
 
     // Be careful not to clobber this below.
     // We also need to make sure that we preserve this if it is used by the patchpoint body.
-    bool clobbersTmp = false;
+    AllowMacroScratchRegisterUsage allowScratch(jit);
     auto tmp = jit.scratchRegister();
-    int tmpSpill = 0;
+    bool tmpNeedsSaving = false;
+    int tmpSpillOffsetRelativeToOriginalSP = 0;
 
     // If we pass a stack location to the patchpoint in arugmentCountIncludingThis, preserve it here.
     bool stackPatchArg[tailCallPatchpointScratchCount] = { false, false };
     int stackPatchArgSpill[tailCallPatchpointScratchCount] = { 0, 0 };
 
-    // Set up a valid frame so that we can clobber this one.
-    RegisterAtOffsetList calleeSaves = params.code().calleeSaveRegisterAtOffsetList();
-    jit.emitRestore(calleeSaves);
-
-    for (unsigned i = 0; i < params.size(); ++i) {
-        auto arg = params[i];
-        if (arg.isGPR()) {
-            ASSERT(!calleeSaves.find(arg.gpr()));
-            if (arg.gpr() == tmp)
-                clobbersTmp = true;
-            continue;
-        }
-        if (arg.isFPR()) {
-            ASSERT(!calleeSaves.find(arg.fpr()));
-            continue;
-        }
-    }
+    // Nothing before saving tmp can use the scratch register since it might clobber an input.
+    {
+        DisallowMacroScratchRegisterUsage disallowScratch(jit);
 
-    for (unsigned i = lastPatchArg; i < params.size(); ++i) {
-        auto arg = params[i];
-        if (arg.isStack()) {
-            int scratch = -1;
-            for (unsigned i = 0; i < tailCallPatchpointScratchCount; ++i) {
-                if (!stackPatchArg[i]) {
-                    scratch = i;
-                    break;
-                }
+        // Set up a valid frame so that we can clobber this one.
+        jit.emitRestore(calleeSaves);
+
+        for (unsigned i = 0; i < params.size(); ++i) {
+            auto arg = params[i];
+            if (arg.isGPR()) {
+                ASSERT(!calleeSaves.find(arg.gpr()));
+                if (arg.gpr() == tmp)
+                    tmpNeedsSaving = true;
+                continue;
             }
... (file diff truncated)

Loading diff…

Regression Test / PoC

shipped with the fix

diff --git a/JSTests/wasm/stress/omg-tail-call-clobber-scratch-register.js b/JSTests/wasm/stress/omg-tail-call-clobber-scratch-register.js
new file mode 100644
index 000000000000..e3878b1353e3
--- /dev/null
+++ b/JSTests/wasm/stress/omg-tail-call-clobber-scratch-register.js
@@ -0,0 +1,33 @@
+//@ runDefault("--jitPolicyScale=0")
+const v39 = new WebAssembly.Instance(new WebAssembly.Module(new Uint8Array([
+    0x00, 0x61, 0x73, 0x6D, 0x01, 0x00, 0x00, 0x00, 0x01, 0x23,
+    0x03, 0x60, 0x00, 0x00, 0x60, 0x09, 0x7E, 0x63, 0x6C, 0x7E,
+    0x7F, 0x63, 0x6F, 0x7E, 0x63, 0x71, 0x63, 0x6F, 0x7E, 0x03,
+    0x7E, 0x63, 0x71, 0x63, 0x72, 0x60, 0x01, 0x63, 0x6C, 0x03,
+    0x7E, 0x63, 0x71, 0x63, 0x72, 0x03, 0x03, 0x02, 0x01, 0x02,
+    0x04, 0x05, 0x01, 0x63, 0x70, 0x00, 0x0A, 0x05, 0x01, 0x00,
+    0x06, 0x01, 0x00, 0x07, 0x11, 0x03, 0x02, 0x77, 0x30, 0x00,
+    0x00, 0x02, 0x77, 0x31, 0x00, 0x01, 0x03, 0x77, 0x74, 0x30,
+    0x01, 0x00, 0x09, 0x09, 0x01, 0x02, 0x00, 0x41, 0x00, 0x0B,
+    0x00, 0x01, 0x00, 0x00, 0x20, 0x19, 0x6D, 0x65, 0x74, 0x61,
+    0x64, 0x61, 0x74, 0x61, 0x2E, 0x63, 0x6F, 0x64, 0x65, 0x2E,
+    0x62, 0x72, 0x61, 0x6E, 0x63, 0x68, 0x5F, 0x68, 0x69, 0x6E,
+    0x74, 0x01, 0x01, 0x01, 0x30, 0x01, 0x01, 0x0A, 0x92, 0x01,
+    0x02, 0x1D, 0x03, 0x01, 0x7B, 0x01, 0x7E, 0x01, 0x63, 0x72,
+    0x20, 0x00, 0xFD, 0x12, 0x21, 0x09, 0x42, 0x07, 0x21, 0x0A,
+    0xD0, 0x72, 0x21, 0x0B, 0x20, 0x0A, 0x20, 0x06, 0x20, 0x0B,
+    0x0B, 0x72, 0x0A, 0x01, 0x7F, 0x01, 0x7F, 0x01, 0x7F, 0x01,
+    0x7F, 0x01, 0x63, 0x72, 0x01, 0x63, 0x71, 0x01, 0x7E, 0x01,
+    0x7E, 0x01, 0x7E, 0x01, 0x7F, 0x41, 0x94, 0xFD, 0x01, 0x21,
+    0x01, 0x03, 0x00, 0x41, 0x01, 0x21, 0x02, 0x20, 0x01, 0x20,
+    0x02, 0x6B, 0x21, 0x03, 0x20, 0x03, 0x21, 0x01, 0x20, 0x01,
+    0x0D, 0x00, 0x0B, 0x41, 0x00, 0x21, 0x04, 0xD0, 0x72, 0x21,
+    0x05, 0xD0, 0x71, 0x21, 0x06, 0x42, 0xFF, 0x00, 0x21, 0x07,
+    0x42, 0xE7, 0xFF, 0xFF, 0xFF, 0x01, 0x21, 0x08, 0x42, 0x09,
+    0x21, 0x09, 0x41, 0x00, 0x21, 0x0A, 0x20, 0x09, 0x20, 0x00,
+    0x20, 0x08, 0x20, 0x04, 0x20, 0x05, 0x20, 0x07, 0x20, 0x06,
+    0x20, 0x05, 0x20, 0x08, 0x20, 0x0A, 0x13, 0x01, 0x00, 0x20,
+    0x09, 0x20, 0x06, 0x20, 0x05, 0x0B,
+])));
+// Don't crash.
+v39.exports.w1(-900879769);

Loading diff…

Original Bug Report

The reporter's bug is still restricted on the tracker.

References

Vendor advisory Fix commit Bug tracker NVD

On This Page