BAssert.h: use __builtin_trap for BCRASH on clang-cl

[robobun]

Problem

On Windows, WTF::fastMalloc and WTF::fastCompactMalloc compile to a bare jmp mi_malloc with no null check, so on OOM they return nullptr (violating their RETURNS_NONNULL contract) and callers crash downstream with a confusing null dereference. The most frequent symptom is BUN-2Z94: std::_Atomic_storage<unsigned int,4> segfault at 0x0 inside StringImpl::createUninitializedInternalNonEmpty, reached from JSON.parse.

Disassembly of FastMalloc.cpp.obj from the current Windows prebuilt (autobuild-c9296e353e):

WTF::fastCompactMalloc(unsigned __int64):
    jmp  mi_malloc
WTF::fastMalloc(unsigned __int64):
    jmp  mi_malloc
WTF::fastRealloc(void *, unsigned __int64):
    jmp  mi_realloc
WTF::fastCompactZeroedMalloc(unsigned __int64):
    jmp  mi_zalloc

The RELEASE_BASSERT(memory) that is supposed to crash on null is gone.

Cause

BCRASH() is gated on defined(__GNUC__) (BAssert.h:63). clang-cl defines __clang__ and _MSC_VER but not __GNUC__, so it falls into the #else:

#define BCRASH() do { \
    *(int*)0xbbadbeef = 0; \
    ((void(*)())0)(); \
} while (0)

Calling a null function pointer is UB. clang uses it to prove the containing branch unreachable, which backward-propagates to delete the if (!memory) check entirely. Minimal reproducer:

void* f(size_t n) {
    void* m = mi_malloc(n);
    if (!m) { ((void(*)())0)(); }
    return m;
}
// clang-cl /O2 -> jmp mi_malloc   (check deleted)

void* g(size_t n) {
    void* m = mi_malloc(n);
    if (!m) { __builtin_trap(); }
    return m;
}
// clang-cl /O2 -> call mi_malloc; test rax,rax; je .L; ret; .L: ud2

This affects every RELEASE_BASSERT / BASSERT in bmalloc on Windows (about 60 call sites), not just the malloc wrappers.

Fix

Extend the gate to defined(__GNUC__) || defined(__clang__) so clang-cl takes the __builtin_trap() path. The #else fallback is retained for real MSVC (cl.exe), which does not perform this UB-based elimination.

Verification

With this patch, clang-cl --target=x86_64-pc-windows-msvc /O2 /DNDEBUG compiles fastCompactMalloc as:

sub  rsp, 40
call mi_malloc
test rax, rax
je   .LBB0_1
add  rsp, 40
ret
.LBB0_1:
mov  eax, 0xBBADBEEF
mov  dword ptr [rax], 0
ud2

Null check preserved, crash site is the allocator instead of an arbitrary downstream write.

Runtime repro (Bun canary, Windows x64 baseline): loop JSON.parse over 1MB string values until commit limit is hit. Before: Segmentation fault at address 0x0 with a JSONAtomStringCache stack. After: clean crash at 0xBBADBEEF inside fastCompactMalloc.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 4 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 111bc8d3-ff8e-4bdc-a523-b8ae2ad75295

📥 Commits

Reviewing files that changed from the base of the PR and between c9296e3 and 1892d61.

📒 Files selected for processing (1)

• Source/bmalloc/bmalloc/BAssert.h

---

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

LGTM — one-token preprocessor guard widening, well-diagnosed and verified.

What was reviewed:

• Confirmed the only functional change is defined(__GNUC__) → defined(__GNUC__) || defined(__clang__); GCC and non-Windows clang already define __GNUC__, so behavior changes only for clang-cl.
• Checked that __builtin_trap() and BIGNORE_CLANG_STATIC_ANALYZER_WARNINGS_ATTRIBUTE (→ [[clang::suppress]]) are both valid under clang-cl.
• The #else fallback for real MSVC is untouched; #endif comment updated to match.

Extended reasoning...

Overview

Single-file change to Source/bmalloc/bmalloc/BAssert.h. The functional diff is one token: the BCRASH() definition guard changes from #if defined(__GNUC__) to #if defined(__GNUC__) || defined(__clang__), plus an explanatory comment and a matching #endif comment update. This routes clang-cl (which defines __clang__ but not __GNUC__) into the __builtin_trap() branch instead of the ((void(*)())0)() fallback, which clang was treating as pure UB and using to delete the surrounding RELEASE_BASSERT checks at -O2.

Security risks

None introduced. The change strictly restores intended crash-on-assert behavior on Windows/clang-cl; it cannot weaken any check on other platforms since GCC and clang (non-MSVC-driver) already define __GNUC__ and were taking this branch. If anything it improves safety by reinstating ~60 elided RELEASE_BASSERT sites in bmalloc on Windows.

Level of scrutiny

Low. This is a mechanical compiler-detection fix in a preprocessor conditional. The macro body being selected (*(int*)0xbbadbeef = 0; __builtin_trap();) is identical to what every other clang build already uses. __builtin_trap() is supported by clang-cl, and BIGNORE_CLANG_STATIC_ANALYZER_WARNINGS_ATTRIBUTE expands to [[clang::suppress]] under clang, which clang-cl also accepts. The PR description includes disassembly before/after and a runtime repro confirming the fix.

Other factors

No prior reviewer comments to address (only a CodeRabbit rate-limit notice). The bug hunting system found no issues. The change is self-contained, has clear intent tied to a Sentry issue, and follows the same __GNUC__ || __clang__ pattern used elsewhere for clang-cl compatibility.

[github-actions]

Preview Builds

Commit	Release	Date
1892d614	autobuild-preview-pr-316-1892d614	2026-07-22 22:54:26 UTC

[robobun]

Another instance of this, from bun 1.4.0 (WebKit 0f966e8) on Windows x64: Sentry BUN-4RGJ. JSC::Encoder::release -> WTF::memcpySpan -> memcpy, fault address 0x20, on the compile-cache thread (module.enableCompileCache()).

Disassembly of the shipped bun.exe confirms the mechanism. The inlined bmalloc::api::malloc in Encoder::release calls _mi_malloc_generic and uses rax with no null test (RVA 0x23313ae: call _mi_malloc_generic; mov rbx, rax; jmp ...). The faulting instruction is the first aligned store in memcpy (vmovdqa ymmword ptr [rcx], ymm1 at RVA 0x3cef823) with rcx = null rounded up to the next 32-byte boundary, so the destination buffer was null and the page reads had already succeeded.

#512 makes Encoder::release use tryMalloc so the cache is skipped on OOM, but this fix is still needed for every other RELEASE_BASSERT in bmalloc on Windows.