-
Notifications
You must be signed in to change notification settings - Fork 5k
test(napi): check the experimental-finalizer wrapper's output before its exit code #37214
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 26 commits
f5ccd47
940630e
9efd638
741c556
7f62e76
7027604
880da00
51f1c7e
2dcf453
e5946fb
9db94de
c0e09cd
d59a943
a2ee2f3
a51f860
107543a
b8743ea
56d5010
04afe87
0dff607
f103e21
2aaae6a
f07e513
0656ed1
011d990
5860983
85de86a
bb4430b
c6ceb9a
9a48440
c17e393
2f31865
04c4180
cb60a8a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,18 @@ | ||
| #include "root.h" | ||
|
|
||
| #include "ZigGlobalObject.h" | ||
| #include "JavaScriptCore/HeapSnapshotBuilder.h" | ||
| #include "JavaScriptCore/HeapProfiler.h" | ||
| #include "JavaScriptCore/ConservativeRoots.h" | ||
| #include "JavaScriptCore/MachineStackMarker.h" | ||
| #include "JavaScriptCore/HeapIterationScope.h" | ||
| #include "JavaScriptCore/MarkedSpaceInlines.h" | ||
| #include "JavaScriptCore/StackVisitor.h" | ||
| #include <wtf/StackBounds.h> | ||
| #include <wtf/Threading.h> | ||
| #if !OS(WINDOWS) | ||
| #include <unistd.h> | ||
| #endif | ||
| #include "MessagePort.h" | ||
| #include "helpers.h" | ||
| #include "JavaScriptCore/ArgList.h" | ||
|
|
@@ -3251,11 +3263,281 @@ void GlobalObject::addBuiltinGlobals(JSC::VM& vm) | |
| /// `globalThis.gc()` is an alias for `Bun.gc(true)` | ||
| /// Note that `vm` is a `VirtualMachine*` | ||
| extern "C" size_t Bun__gc(void* vm, bool sync); | ||
|
|
||
| // Diagnostics branch only: when /tmp/bun-napi-diag-request exists, `gc()` | ||
| // additionally writes a JSC GC-debugging heap snapshot (which records, per | ||
| // live cell, the root that marked it) to /tmp/bun-napi-diag-<pid>.json. Keyed | ||
| // on a file rather than argv/env so the process under test is byte-for-byte | ||
| // the same as the one that fails. | ||
| static constexpr size_t kBelowSpBytes = 256 * 1024; | ||
| static uintptr_t bunNapiDiagBelowSpCopy[kBelowSpBytes / sizeof(uintptr_t)]; | ||
| static uintptr_t* bunNapiDiagBelowSpFrom = nullptr; | ||
| static void* bunNapiDiagLastStackTopBefore = nullptr; | ||
| // Callee-saved registers as they were on entry to gc()'s host function: these | ||
| // propagate unchanged into the collector and are part of the captured register | ||
| // state that the conservative scan visits. | ||
| static uintptr_t bunNapiDiagCalleeSaved[12]; | ||
| static const char* const bunNapiDiagCalleeSavedNames[12] = { | ||
| #if CPU(X86_64) | ||
| "rbx", "rbp", "r12", "r13", "r14", "r15", "", "", "", "", "", "" | ||
| #elif CPU(ARM64) | ||
| "x19", "x20", "x21", "x22", "x23", "x24", "x25", "x26", "x27", "x28", "fp", "" | ||
| #else | ||
| "", "", "", "", "", "", "", "", "", "", "", "" | ||
| #endif | ||
| }; | ||
| #if CPU(X86_64) | ||
| #define BUN_NAPI_DIAG_CAPTURE_CALLEE_SAVED() \ | ||
| do { \ | ||
| asm volatile("movq %%rbx, %0" : "=m"(bunNapiDiagCalleeSaved[0])); \ | ||
| asm volatile("movq %%rbp, %0" : "=m"(bunNapiDiagCalleeSaved[1])); \ | ||
| asm volatile("movq %%r12, %0" : "=m"(bunNapiDiagCalleeSaved[2])); \ | ||
| asm volatile("movq %%r13, %0" : "=m"(bunNapiDiagCalleeSaved[3])); \ | ||
| asm volatile("movq %%r14, %0" : "=m"(bunNapiDiagCalleeSaved[4])); \ | ||
| asm volatile("movq %%r15, %0" : "=m"(bunNapiDiagCalleeSaved[5])); \ | ||
| } while (0) | ||
| #elif CPU(ARM64) | ||
| #define BUN_NAPI_DIAG_CAPTURE_CALLEE_SAVED() \ | ||
| do { \ | ||
| asm volatile("str x19, %0" : "=m"(bunNapiDiagCalleeSaved[0])); \ | ||
| asm volatile("str x20, %0" : "=m"(bunNapiDiagCalleeSaved[1])); \ | ||
| asm volatile("str x21, %0" : "=m"(bunNapiDiagCalleeSaved[2])); \ | ||
| asm volatile("str x22, %0" : "=m"(bunNapiDiagCalleeSaved[3])); \ | ||
| asm volatile("str x23, %0" : "=m"(bunNapiDiagCalleeSaved[4])); \ | ||
| asm volatile("str x24, %0" : "=m"(bunNapiDiagCalleeSaved[5])); \ | ||
| asm volatile("str x25, %0" : "=m"(bunNapiDiagCalleeSaved[6])); \ | ||
| asm volatile("str x26, %0" : "=m"(bunNapiDiagCalleeSaved[7])); \ | ||
| asm volatile("str x27, %0" : "=m"(bunNapiDiagCalleeSaved[8])); \ | ||
| asm volatile("str x28, %0" : "=m"(bunNapiDiagCalleeSaved[9])); \ | ||
| asm volatile("str x29, %0" : "=m"(bunNapiDiagCalleeSaved[10])); \ | ||
| } while (0) | ||
| #else | ||
| #define BUN_NAPI_DIAG_CAPTURE_CALLEE_SAVED() \ | ||
| do { \ | ||
| } while (0) | ||
| #endif | ||
|
|
||
| __attribute__((no_sanitize("address"), noinline)) static void bunNapiDiagWhereAreTheRoots(JSC::VM& vm, JSC::CallFrame* callFrame) | ||
| { | ||
| // 1. Every live cell start address after the gc() that just ran. | ||
| WTF::HashSet<uintptr_t> liveCells; | ||
| { | ||
| JSC::HeapIterationScope scope(vm.heap); | ||
| vm.heap.objectSpace().forEachLiveCell(scope, [&](JSC::HeapCell* cell, JSC::HeapCell::Kind kind) { | ||
| if (kind == JSC::HeapCell::JSCell) | ||
| liveCells.add(reinterpret_cast<uintptr_t>(cell)); | ||
| return IterationStatus::Continue; | ||
| }); | ||
| } | ||
| auto describe = [&](uintptr_t p) -> const char* { | ||
| return reinterpret_cast<JSC::JSCell*>(p)->className().characters(); | ||
| }; | ||
| fprintf(stderr, "[napi-diag] live JS cells: %u\n", liveCells.size()); | ||
|
|
||
| // A word "hits" a cell if it points anywhere in [cell, cell+64): that covers | ||
| // interior pointers (ConservativeRoots cellAlign()s them) and, for our | ||
| // 16-byte JSArray, the [cell+16, cell+24] "butterfly just past the previous | ||
| // object" case that genericAddPointer maps back onto the cell to the left. | ||
| // The offset is reported so over-matches can be discounted by hand. | ||
| static size_t bunNapiDiagLastOff; | ||
| bunNapiDiagLastOff = 0; | ||
| auto interesting = [&](uintptr_t v, uintptr_t& base) -> const char* { | ||
| base = 0; | ||
| if (v < 4096 || v == UINTPTR_MAX) | ||
| return nullptr; | ||
| uintptr_t aligned = v & ~uintptr_t(7); | ||
| for (size_t off = 0; off <= 56; off += 8) { | ||
| if (aligned < off + 4096) | ||
| break; | ||
| if (liveCells.contains(aligned - off)) { | ||
| base = aligned - off; | ||
| bunNapiDiagLastOff = v - base; | ||
| break; | ||
| } | ||
| } | ||
| if (!base) | ||
| return nullptr; | ||
| const char* cls = describe(base); | ||
| if (strcmp(cls, "Array") && strcmp(cls, "Object") && strcmp(cls, "NapiClass") && strcmp(cls, "NapiPrototype") && strcmp(cls, "NapiHandleScopeImpl")) | ||
| return nullptr; | ||
| return cls; | ||
| }; | ||
|
|
||
| // 2. VM-owned conservatively scanned buffers (Heap::gatherVMRoots). | ||
| { | ||
| JSC::ConservativeRoots vmRoots(vm.heap); | ||
| #if ENABLE(DFG_JIT) | ||
| vm.gatherScratchBufferRoots(vmRoots); | ||
| #endif | ||
| fprintf(stderr, "[napi-diag] VM scratch-buffer conservative roots: %zu; checkpoint OSR side state present: %d\n", vmRoots.size(), (int)vm.hasCheckpointOSRSideState()); | ||
| for (size_t i = 0; i < vmRoots.size(); ++i) { | ||
| auto p = reinterpret_cast<uintptr_t>(vmRoots.roots()[i]); | ||
| if (p < 16 || p == UINTPTR_MAX) continue; | ||
| fprintf(stderr, "[napi-diag] vmroot %p %s\n", (void*)p, liveCells.contains(p) ? describe(p) : "(not a live cell start)"); | ||
| } | ||
| } | ||
|
|
||
| // 3. JS/native frame layout, so stack hits can be attributed. | ||
| fprintf(stderr, "[napi-diag] frames (top first):\n"); | ||
| JSC::StackVisitor::visit(callFrame, vm, [&](JSC::StackVisitor& visitor) -> IterationStatus { | ||
| auto name = visitor->functionName().utf8(); | ||
| auto* cb = visitor->codeBlock(); | ||
| fprintf(stderr, "[napi-diag] frame callFrame=%p callerFrame=%p %s%s codeType=%d bc#%u regs=%d\n", | ||
| (void*)visitor->callFrame(), (void*)visitor->callerFrame(), name.data(), | ||
| visitor->isNativeFrame() ? " [native]" : "", cb ? (int)cb->codeType() : -1, | ||
| visitor->bytecodeIndex().offset(), cb ? (int)cb->numCalleeLocals() : -1); | ||
| return IterationStatus::Continue; | ||
| }); | ||
|
|
||
| // 4. Words on this thread's machine stack, from here up to the origin, | ||
| // that equal (or point 8 bytes into) a live cell. | ||
| volatile uintptr_t marker = 0; | ||
| uintptr_t* sp = const_cast<uintptr_t*>(&marker); | ||
| uintptr_t* origin = static_cast<uintptr_t*>(WTF::Thread::currentSingleton().stack().origin()); | ||
| fprintf(stderr, "[napi-diag] scanning machine stack %p..%p (%zu words)\n", (void*)sp, (void*)origin, (size_t)(origin - sp)); | ||
| unsigned hits = 0; | ||
| for (uintptr_t* w = sp; w < origin; ++w) { | ||
| uintptr_t base; | ||
| const char* cls = interesting(*w, base); | ||
| if (!cls) | ||
| continue; | ||
| ++hits; | ||
| fprintf(stderr, "[napi-diag] stack[%p] (origin-0x%zx) = %p -> %s @%p +%zu\n", (void*)w, (size_t)((origin - w) * sizeof(uintptr_t)), (void*)*w, cls, (void*)base, bunNapiDiagLastOff); | ||
| } | ||
| fprintf(stderr, "[napi-diag] stack words pointing at Array/Object/Napi* cells: %u\n", hits); | ||
|
|
||
| // 4a. Every thread registered with this heap's MachineThreads (each one is | ||
| // suspended and its registers + stack [sp, origin) conservatively | ||
| // scanned by every collection). Report them, and any word anywhere in | ||
| // their stack range that points at the interesting cells. | ||
| { | ||
| auto& machineThreads = vm.heap.machineThreads(); | ||
| Locker locker { machineThreads.getLock() }; | ||
| const auto& threads = machineThreads.threads(locker); | ||
| fprintf(stderr, "[napi-diag] threads registered with the heap: %u (current uid %u)\n", threads.size(), WTF::Thread::currentSingleton().uid()); | ||
| for (auto& threadRef : threads) { | ||
| WTF::Thread& t = threadRef.get(); | ||
| bool isCurrent = &t == &WTF::Thread::currentSingleton(); | ||
| uintptr_t* lo = static_cast<uintptr_t*>(t.stack().end()); | ||
| uintptr_t* hi = static_cast<uintptr_t*>(t.stack().origin()); | ||
| fprintf(stderr, "[napi-diag] thread uid=%u%s stack=%p..%p\n", t.uid(), isCurrent ? " [current]" : "", (void*)lo, (void*)hi); | ||
| if (isCurrent) | ||
| continue; | ||
| unsigned th = 0; | ||
| for (uintptr_t* w = lo + 512; w < hi; ++w) { // skip the guard-ish bottom | ||
| uintptr_t base; | ||
| const char* cls = interesting(*w, base); | ||
| if (!cls) continue; | ||
| if (++th <= 40) | ||
| fprintf(stderr, "[napi-diag] other-thread stack[%p] (origin-0x%zx) = %p -> %s @%p +%zu\n", (void*)w, (size_t)((hi - w) * sizeof(uintptr_t)), (void*)*w, cls, (void*)base, bunNapiDiagLastOff); | ||
| } | ||
| fprintf(stderr, "[napi-diag] words on that thread's stack pointing at Array/Object/Napi* cells: %u\n", th); | ||
| } | ||
| } | ||
|
|
||
| // 4b. Callee-saved registers on entry to gc()'s host function. | ||
| for (int i = 0; i < 12; ++i) { | ||
| if (!bunNapiDiagCalleeSavedNames[i][0]) continue; | ||
| uintptr_t base; | ||
| const char* cls = interesting(bunNapiDiagCalleeSaved[i], base); | ||
| fprintf(stderr, "[napi-diag] callee-saved %s = %p%s%s +%zu\n", bunNapiDiagCalleeSavedNames[i], (void*)bunNapiDiagCalleeSaved[i], cls ? " -> " : "", cls ? cls : "", cls ? bunNapiDiagLastOff : (size_t)0); | ||
| } | ||
|
|
||
| // 5. The dead region below gc()'s frame as it was BEFORE the collection | ||
| // (this is what the collector's own frames were laid over), and vm.lastStackTop | ||
| // at that moment (sanitizeStackForVM only zeroes [lastStackTop, sp)). | ||
| fprintf(stderr, "[napi-diag] vm.lastStackTop before gc: %p (gc frame ~%p; %s)\n", bunNapiDiagLastStackTopBefore, (void*)sp, | ||
| (uintptr_t)bunNapiDiagLastStackTopBefore < (uintptr_t)sp ? "DEEPER than gc frame: sanitize could zero below" : "not deeper: sanitize zeroed nothing below gc frame"); | ||
| if (bunNapiDiagBelowSpFrom) { | ||
| unsigned below = 0; | ||
| size_t n = kBelowSpBytes / sizeof(uintptr_t); | ||
| for (size_t i = 0; i < n; ++i) { | ||
| uintptr_t base; | ||
| const char* cls = interesting(bunNapiDiagBelowSpCopy[i], base); | ||
| if (!cls) continue; | ||
| ++below; | ||
| uintptr_t* where = bunNapiDiagBelowSpFrom + i; | ||
| fprintf(stderr, "[napi-diag] pre-gc below-sp[%p] (gcframe-0x%zx) = %p -> %s @%p +%zu\n", (void*)where, (size_t)((sp - where) * sizeof(uintptr_t)), (void*)bunNapiDiagBelowSpCopy[i], cls, (void*)base, bunNapiDiagLastOff); | ||
| } | ||
| fprintf(stderr, "[napi-diag] pre-gc words BELOW gc()'s frame pointing at Array/Object/Napi* cells: %u (region %p..%p)\n", below, (void*)bunNapiDiagBelowSpFrom, (void*)(bunNapiDiagBelowSpFrom + n)); | ||
| } | ||
| // 6. Same region now (what the collector left). | ||
| { | ||
| unsigned belowNow = 0; | ||
| uintptr_t* from = sp - kBelowSpBytes / sizeof(uintptr_t); | ||
| uintptr_t* limit = static_cast<uintptr_t*>(WTF::Thread::currentSingleton().stack().end()); | ||
| if (from < limit + 4096) from = limit + 4096; | ||
| for (uintptr_t* w = from; w < sp; ++w) { | ||
| uintptr_t base; | ||
| const char* cls = interesting(*w, base); | ||
| if (!cls) continue; | ||
| ++belowNow; | ||
| fprintf(stderr, "[napi-diag] post-gc below-sp[%p] (gcframe-0x%zx) = %p -> %s @%p +%zu\n", (void*)w, (size_t)((sp - w) * sizeof(uintptr_t)), (void*)*w, cls, (void*)base, bunNapiDiagLastOff); | ||
| } | ||
| fprintf(stderr, "[napi-diag] post-gc words BELOW gc()'s frame pointing at Array/Object/Napi* cells: %u\n", belowNow); | ||
| } | ||
| fflush(stderr); | ||
| } | ||
|
|
||
| __attribute__((noinline)) static void bunNapiDiagMaybeDumpHeap(JSC::VM& vm, JSC::CallFrame* callFrame) | ||
| { | ||
| #if OS(WINDOWS) | ||
| return; | ||
| #else | ||
| if (access("/tmp/bun-napi-diag-request", F_OK) != 0) | ||
| return; | ||
| fprintf(stderr, "[napi-diag] gc() returned\n"); | ||
| fflush(stderr); | ||
| bunNapiDiagWhereAreTheRoots(vm, callFrame); | ||
| fprintf(stderr, "[napi-diag] building GC-debugging snapshot (runs another full GC)\n"); | ||
| fflush(stderr); | ||
| vm.ensureHeapProfiler(); | ||
| auto& heapProfiler = *vm.heapProfiler(); | ||
| heapProfiler.clearSnapshots(); | ||
| JSC::HeapSnapshotBuilder builder(heapProfiler, JSC::HeapSnapshotBuilder::SnapshotType::GCDebuggingSnapshot); | ||
| builder.buildSnapshot(); | ||
| WTF::String json = builder.json(); | ||
| char path[128]; | ||
| snprintf(path, sizeof(path), "/tmp/bun-napi-diag-%d.json", getpid()); | ||
| if (FILE* f = fopen(path, "w")) { | ||
| auto utf8 = json.utf8(); | ||
| fwrite(utf8.data(), 1, utf8.length(), f); | ||
| fclose(f); | ||
| fprintf(stderr, "[napi-diag] wrote %s (%zu bytes)\n", path, utf8.length()); | ||
| } else { | ||
| fprintf(stderr, "[napi-diag] could not open %s\n", path); | ||
| } | ||
| fflush(stderr); | ||
| #endif | ||
| } | ||
|
|
||
| // Diagnostics: copy of the dead stack region below gc()'s frame taken before | ||
| // the collection runs (see the statics above bunNapiDiagWhereAreTheRoots). | ||
| __attribute__((no_sanitize("address"), noinline)) static void bunNapiDiagCaptureBelowSp(JSC::VM& vm) | ||
| { | ||
| volatile uintptr_t marker = 0; | ||
| uintptr_t* sp = const_cast<uintptr_t*>(&marker); | ||
| uintptr_t* from = sp - kBelowSpBytes / sizeof(uintptr_t); | ||
| uintptr_t* limit = static_cast<uintptr_t*>(WTF::Thread::currentSingleton().stack().end()); | ||
| if (from < limit + 4096) | ||
| from = limit + 4096; | ||
| bunNapiDiagBelowSpFrom = from; | ||
| for (size_t i = 0; from + i < sp && i < kBelowSpBytes / sizeof(uintptr_t); ++i) | ||
| bunNapiDiagBelowSpCopy[i] = from[i]; | ||
| bunNapiDiagLastStackTopBefore = vm.lastStackTop(); | ||
| } | ||
|
|
||
| JSC_DEFINE_HOST_FUNCTION(functionJsGc, | ||
| (JSC::JSGlobalObject * global, JSC::CallFrame* callFrame)) | ||
| { | ||
| BUN_NAPI_DIAG_CAPTURE_CALLEE_SAVED(); | ||
| Zig::GlobalObject* globalObject = defaultGlobalObject(global); | ||
| #if !OS(WINDOWS) | ||
| if (access("/tmp/bun-napi-diag-request", F_OK) == 0) | ||
| bunNapiDiagCaptureBelowSp(JSC::getVM(global)); | ||
|
Comment on lines
+3630
to
+3631
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 c17e393 inserts Extended reasoning...What the bug isCommit c17e393 added Two consequences(1) Section 5 is contaminated. (2) V1 run 0's crash bit may be perturbed. The same residue sits in Step-by-step proof (section-5 pollution)
Why existing code doesn't prevent it
On the refutation of consequence (2)One reviewer argued the GC-perturbation half is too speculative to file separately: everything in How to fixSwap the two calls so if (access("/tmp/bun-napi-diag-request", F_OK) == 0) {
bunNapiDiagCaptureBelowSp(JSC::getVM(global));
bunNapiDiagPreGcLiveScan(JSC::getVM(global));
}This restores section 5's meaning (CaptureBelowSp reads only Nit because this is diagnostics-only code that won't merge (per the author's own comments on this PR), section 5 is supplementary to the primary PRE-GC / section-4 / 4' output which is unaffected, and the author has already accepted similar methodology observations here as "will fix if another iteration is needed" — but it directly undermines section 5's signal on exactly the below-SP-garbage hypothesis being tested, so worth swapping before reading build #90799.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. True, and worth stating the direction of the bias: residue from the pre-GC walk can only add roots (make V1 run 0 survive), never remove them. The run in question showed the +24 word present pre-GC and the child then SIGABRT on GC #1 (finalized) — so if the walk contaminated anything, it did so against the result we observed. Section 5's below-sp numbers from that run are indeed not trustworthy; not relying on them. Leaving the order as is since this branch won't get further diagnostic rounds after the final repin. |
||
| #endif | ||
| Bun__gc(globalObject->bunVM(), true); | ||
| bunNapiDiagMaybeDumpHeap(JSC::getVM(global), callFrame); | ||
| return JSValue::encode(jsUndefined()); | ||
| } | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.