From a84c86aea25a33ad756ecce4cec7c790af3ff031 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 2 Aug 2026 07:54:51 +0000 Subject: [PATCH 1/4] Thread::getRegisters (Windows): assert on GetThreadContext failure and return only the requested prefix GetThreadContext can fail, leaving CONTEXT (including Rsp/Sp) undefined; the GC caller would then compute a bogus stack range and OOM in growBuffer. Crash immediately with the Win32 error code instead. Only CONTEXT_INTEGER | CONTEXT_CONTROL is requested, so return a prefix that ends after Rip (x64) / Pc (arm64) rather than sizeof(CONTEXT). The rest of the struct is never populated and need not be copied into the conservative root buffer. --- Source/WTF/wtf/win/ThreadingWin.cpp | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/Source/WTF/wtf/win/ThreadingWin.cpp b/Source/WTF/wtf/win/ThreadingWin.cpp index 6fffa48fd0460..dcb86ae3e5bd9 100644 --- a/Source/WTF/wtf/win/ThreadingWin.cpp +++ b/Source/WTF/wtf/win/ThreadingWin.cpp @@ -246,8 +246,24 @@ void Thread::resume(const ThreadSuspendLocker&) size_t Thread::getRegisters(const ThreadSuspendLocker&, PlatformRegisters& registers) { registers.ContextFlags = CONTEXT_INTEGER | CONTEXT_CONTROL; - GetThreadContext(m_handle, ®isters); + if (!GetThreadContext(m_handle, ®isters)) [[unlikely]] { + // On failure the CONTEXT (including the stack pointer) is undefined; the caller + // would compute a bogus stack range and OOM in MachineThreads::growBuffer. Crash + // with the Win32 error instead of wandering off into a huge allocation. + RELEASE_ASSERT_NOT_REACHED(static_cast(GetLastError())); + } + // We only requested CONTEXT_INTEGER | CONTEXT_CONTROL, so only that prefix of CONTEXT + // is populated. Return just that range so the conservative root scan does not copy the + // remainder of the struct (which the caller zero-initializes; see MachineStackMarker). +#if CPU(X86_64) + static_assert(offsetof(CONTEXT, Rax) < offsetof(CONTEXT, Rsp) && offsetof(CONTEXT, Rsp) < offsetof(CONTEXT, Rip)); + return offsetof(CONTEXT, Rip) + sizeof(registers.Rip); +#elif CPU(ARM64) + static_assert(offsetof(CONTEXT, Sp) < offsetof(CONTEXT, Pc)); + return offsetof(CONTEXT, Pc) + sizeof(registers.Pc); +#else return sizeof(CONTEXT); +#endif } Thread& Thread::initializeCurrentTLS() From 6edb56ea51af8b302dd84d8082e77ea451624f33 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 2 Aug 2026 07:55:02 +0000 Subject: [PATCH 2/4] MachineThreads::tryCopyOtherThreadStack: value-initialize PlatformRegisters Any bytes getRegisters() leaves untouched are now zero rather than whatever was on the collector thread's stack (typically stale JSCell* from the previous SlotVisitor::drain), so they no longer scan as false conservative roots. Value-initialization of an aggregate is malloc-free, preserving the no-allocation requirement while the target thread is suspended. --- Source/JavaScriptCore/heap/MachineStackMarker.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Source/JavaScriptCore/heap/MachineStackMarker.cpp b/Source/JavaScriptCore/heap/MachineStackMarker.cpp index 9c95d17c770fd..05e3c172fec67 100644 --- a/Source/JavaScriptCore/heap/MachineStackMarker.cpp +++ b/Source/JavaScriptCore/heap/MachineStackMarker.cpp @@ -118,7 +118,13 @@ static void NODELETE copyMemory(void* dst, const void* src, size_t size) // See: https://bugs.webkit.org/show_bug.cgi?id=146297 void MachineThreads::tryCopyOtherThreadStack(const ThreadSuspendLocker& locker, Thread& thread, void* buffer, size_t capacity, size_t* size) { - PlatformRegisters registers; + // Value-initialize so that any bytes getRegisters() does not populate are zero rather + // than uninitialized collector-thread stack. On Windows in particular, PlatformRegisters + // is the full CONTEXT struct but only the integer/control portion is requested; the + // remainder otherwise carries stale JSCell* from prior SlotVisitor frames and gets + // scanned as false roots. This must stay malloc-free (the target thread is suspended), + // which value-initialization of an aggregate is. + PlatformRegisters registers { }; size_t registersSize = thread.getRegisters(locker, registers); // This is a workaround for . libdispatch recycles work From b39fe7c56f69c45827c8d2aec7050e7391279515 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 2 Aug 2026 07:55:27 +0000 Subject: [PATCH 3/4] Thread::suspend (Windows): retry transient SuspendThread/GetThreadContext failures; crash if a live thread still cannot be suspended during GC SuspendThread and GetThreadContext can both transiently fail while the target is in early start, exit, or certain kernel transitions. Retry the SuspendThread/GetThreadContext pair with a short spin-then-sleep backoff (up to 100 attempts) before returning failure. In MachineThreads::tryCopyOtherThreadStacks, on Windows, if suspend() still fails, RELEASE_ASSERT with the Win32 error instead of silently skipping that thread's stack for the current GC cycle. Any thread still in the iterated set is live at the WTF level (didExit() removes the thread from the group under the same lock we hold before setting m_didExit), so there is no exited-thread case to tolerate here. Skipping a live mutator's stack drops its roots and can sweep objects that are still referenced. --- .../heap/MachineStackMarker.cpp | 10 ++++++ Source/WTF/wtf/win/ThreadingWin.cpp | 35 +++++++++++++------ 2 files changed, 35 insertions(+), 10 deletions(-) diff --git a/Source/JavaScriptCore/heap/MachineStackMarker.cpp b/Source/JavaScriptCore/heap/MachineStackMarker.cpp index 05e3c172fec67..4520ff3dc69f6 100644 --- a/Source/JavaScriptCore/heap/MachineStackMarker.cpp +++ b/Source/JavaScriptCore/heap/MachineStackMarker.cpp @@ -178,6 +178,16 @@ bool MachineThreads::tryCopyOtherThreadStacks(const AbstractLocker& locker, void WTFReportError(__FILE__, __LINE__, WTF_PRETTY_FUNCTION, "JavaScript garbage collection encountered an invalid thread (err 0x%x): Thread [%d/%d: %p].", result.error(), index, threads.size(), thread.ptr()); +#elif OS(WINDOWS) + // Thread::suspend already retries transient SuspendThread / + // GetThreadContext failures. A thread that has finished didExit() is + // no longer in this set (removal happens under the group lock we hold), + // so any thread we iterate is live at the WTF level. If it still cannot + // be suspended, proceeding would skip scanning its stack for this GC + // cycle, dropping its roots; objects it references could then be swept + // while still live. Crash now with a real signature rather than risk a + // use-after-free several GCs later. + RELEASE_ASSERT_NOT_REACHED(static_cast(result.error()), index, threads.size()); #endif } } diff --git a/Source/WTF/wtf/win/ThreadingWin.cpp b/Source/WTF/wtf/win/ThreadingWin.cpp index dcb86ae3e5bd9..6ff493eb60fde 100644 --- a/Source/WTF/wtf/win/ThreadingWin.cpp +++ b/Source/WTF/wtf/win/ThreadingWin.cpp @@ -217,9 +217,6 @@ auto Thread::suspend(const ThreadSuspendLocker&) -> Expected Expected Date: Sun, 2 Aug 2026 07:55:52 +0000 Subject: [PATCH 4/4] RegisterState: use explicit callee-save capture on clang-cl and add Win64 variant Gate the inline-asm capture on COMPILER(GCC_COMPATIBLE) || COMPILER(CLANG) instead of !OS(WINDOWS). clang-cl accepts GNU inline asm (it defines __clang__ but not __GNUC__), so Windows clang-cl builds now take the explicit path instead of falling back to setjmp into a 256-byte jmp_buf that is scanned in full. Add a Win64 x86_64 RegisterState capturing the Windows x64 callee-saved GPRs (rbx, rbp, rdi, rsi, r12-r15; rdi/rsi are callee-saved on Win64 unlike SysV). Add rbp to the existing SysV x86_64 list. The setjmp fallback is kept for MSVC proper. --- Source/JavaScriptCore/heap/RegisterState.h | 36 ++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/Source/JavaScriptCore/heap/RegisterState.h b/Source/JavaScriptCore/heap/RegisterState.h index 3a260ed655466..3b939e0998785 100644 --- a/Source/JavaScriptCore/heap/RegisterState.h +++ b/Source/JavaScriptCore/heap/RegisterState.h @@ -31,7 +31,10 @@ namespace JSC { -#if !OS(WINDOWS) +// clang-cl accepts GNU inline asm but does not define __GNUC__, so COMPILER(CLANG) is +// checked in addition to COMPILER(GCC_COMPATIBLE). MSVC proper falls through to the +// setjmp fallback below. +#if COMPILER(GCC_COMPATIBLE) || COMPILER(CLANG) // ALLOCATE_AND_GET_REGISTER_STATE has to ensure that the GC sees callee-saves. It achieves this by // ensuring that the callee-saves are either spilled to the stack or saved in the RegisterState. The code @@ -55,9 +58,37 @@ struct RegisterState { SAVE_REG(edi, registers.edi); \ SAVE_REG(esi, registers.esi) +#elif CPU(X86_64) && OS(WINDOWS) +// Win64 calling convention: rdi and rsi are callee-saved (unlike SysV). +struct RegisterState { + uint64_t rbx; + uint64_t rbp; + uint64_t rdi; + uint64_t rsi; + uint64_t r12; + uint64_t r13; + uint64_t r14; + uint64_t r15; +}; + +#define SAVE_REG(regname, where) \ + asm volatile ("movq %%" #regname ", %0" : "=m"(where) : : "memory") + +#define ALLOCATE_AND_GET_REGISTER_STATE(registers) \ + RegisterState registers; \ + SAVE_REG(rbx, registers.rbx); \ + SAVE_REG(rbp, registers.rbp); \ + SAVE_REG(rdi, registers.rdi); \ + SAVE_REG(rsi, registers.rsi); \ + SAVE_REG(r12, registers.r12); \ + SAVE_REG(r13, registers.r13); \ + SAVE_REG(r14, registers.r14); \ + SAVE_REG(r15, registers.r15) + #elif CPU(X86_64) struct RegisterState { uint64_t rbx; + uint64_t rbp; uint64_t r12; uint64_t r13; uint64_t r14; @@ -70,6 +101,7 @@ struct RegisterState { #define ALLOCATE_AND_GET_REGISTER_STATE(registers) \ RegisterState registers; \ SAVE_REG(rbx, registers.rbx); \ + SAVE_REG(rbp, registers.rbp); \ SAVE_REG(r12, registers.r12); \ SAVE_REG(r13, registers.r13); \ SAVE_REG(r14, registers.r14); \ @@ -158,7 +190,7 @@ struct RegisterState { SAVE_REG(23, registers.r23) #endif -#endif // !OS(WINDOWS) +#endif // COMPILER(GCC_COMPATIBLE) || COMPILER(CLANG) #ifndef ALLOCATE_AND_GET_REGISTER_STATE