Enable bun:ffi (TinyCC) on Windows ARM64 - #33696
Conversation
|
Found 1 issue this PR may fix:
🤖 Generated with Claude Code |
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
|
Updated 11:34 PM PT - Jul 19th, 2026
✅ @robobun, your commit e83d3d00209058071a0075b5e3cfc2b3e749bdc4 passed in 🧪 To try this PR locally: bunx bun-pr 33696That installs a local version of the PR into your bun-33696 --bun |
0241464 to
cc3e711
Compare
eef6e54 to
f5e2f90
Compare
|
Cleaned this up so it's landable — it had drifted 3 days behind main and was still carrying the two temporary diagnostic commits, which were ~all of the red on it. What changed (head
The branch is now exactly: TinyCC → |
WalkthroughTinyCC is enabled on Windows ARM64, its pinned commit and runtime TLS patch are updated, x86_64 variadic argument handling is added, and FFI tests remove Windows ARM64 exclusions while covering variadic calls and thread-local storage errors. ChangesTinyCC platform and FFI support
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/build/config.ts`:
- Around line 866-868: Confirm that the pending Windows ARM64 CI acceptance run
passes before merging the change to the tinycc default in the build
configuration; do not land the new TinyCC/FFI path until this platform-specific
validation succeeds.
In `@src/runtime/ffi/libtcc1.c`:
- Around line 608-611: Shorten the comment above the libtcc1 implementation to
no more than three lines while preserving the key context: why __va_arg is
present, that Bun replaces libtcc1, its x86_64 SysV scope, and why extern
abort() is intentionally omitted.
In `@test/js/bun/ffi/cc.test.ts`:
- Around line 618-621: The explanatory comment in the TinyCC TLS test exceeds
the three-line limit; condense it to at most three lines while retaining the key
point that Local-Exec TLS is invalid for in-memory relocation and must be
rejected to prevent corruption.
- Around line 624-626: Update the TLS fixture setup in the parameterized test to
use disposable temp directory handling: replace tempDirWithFiles() with using
and tempDir(), then create/write tls.c within that directory while preserving
the existing fixture contents and test behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 98d027c2-2024-440e-a28e-e01d3d81b0f8
📒 Files selected for processing (14)
scripts/build/buildOptionsRs.tsscripts/build/config.tsscripts/build/deps/tinycc.tsscripts/build/source.tssrc/runtime/ffi/libtcc1.csrc/tcc_sys/tcc.rstest/js/bun/ffi/cc.test.tstest/js/bun/ffi/ffi-error-messages.test.tstest/js/bun/ffi/ffi-viewSource-non-object.test.tstest/js/bun/ffi/ffi.test.jstest/js/node/fs/cp.test.tstest/js/node/fs/fs-writeSync-stdio-windows.test.tstest/js/node/process/process.test.jstest/napi/napi-value-ffi.test.ts
💤 Files with no reviewable changes (1)
- scripts/build/buildOptionsRs.ts
|
CI verdict for the cleaned head (
So: Suggested order from here: land #33653 (this PR is a superset of it) or land this and close #33653 into it — either way this is what resolves #28055. #29476 duplicates the enablement with neither TinyCC fix and should close as superseded. |
f5e2f90 to
fc228dc
Compare
|
Rebased onto current
Fail-before: the canary bun on the same box throws Also confirmed no regression on Windows x64 (26 pass / 0 fail across the same files) and Linux x64. Swept Addressed the three comment-length / |
|
Adversarial review pass (on both this PR's diff and the full oven-sh/tinycc fork delta vs upstream), with the follow-ups applied at This PR survived every correctness probe; the one surviving concern was the #33653 stacking, now moot (#33653 and #29476 are closed as superseded). TinyCC fork delta surfaced two substantive items:
Re-verified on Windows 11 ARM64 at |
|
Second adversarial pass, on oven-sh/tinycc#3 itself: approve-with-changes with one survivor. Placing the SHF_TLS scan after Fixed by keeping the scan at its original position before
|
There was a problem hiding this comment.
No bugs found, but this enables a native JIT path (TinyCC) on a new platform along with a vendored-dep bump and a tccrun.c patch, so it warrants a human sign-off.
What was reviewed:
- The three-way
cfg.tinycc/ENABLE_TINYCC/tcc_externs!predicate stays in sync after dropping the windows-aarch64 arm. __va_arginlibtcc1.cmatches upstreamlib/va_list.csemantics; theabort()→null-write substitution is intentional and gated to x86_64 SysV.- The SHF_TLS scan patch reuses locals
i/salready in scope intcc_relocate_exand sits beforepe_output_file/tcc_add_runtimeper the follow-up commit. - Test un-gating: no remaining
isFFIUnavailablereferences were left dangling after import removals.
Extended reasoning...
Overview
This PR enables bun:ffi on Windows ARM64 by (1) bumping the vendored TinyCC pin to oven-sh/tinycc@8a6cbc12 (which carries an LLP64 immediate-encoding fix in the arm64 backend), (2) removing the windows && arm64 exclusion from cfg.tinycc in config.ts, the generated ENABLE_TINYCC cfg predicate in buildOptionsRs.ts, and the tcc_externs! stub cfg in src/tcc_sys/tcc.rs, (3) adding a patches/tinycc/tccrun-shf-tls.patch that widens the run-time TLS guard to scan all SHF_TLS sections, (4) porting TinyCC's __va_arg into src/runtime/ffi/libtcc1.c (needed after the pin bump moved it out of tccdefs.h), and (5) un-gating ~8 test files that were skipped on windows-arm64 and adding new coverage for varargs, long-double varargs, and TLS rejection.
Security risks
FFI is inherently security-sensitive — it JIT-compiles user-provided C and executes it in-process. The specific risk surface this PR touches is TinyCC's arm64 codegen on an LLP64 host: the fork-side fix (not in this diff, but pulled in via the commit bump) changes how 64-bit immediates are classified for ADD/SUB encoding. A miscompile there produced wrong values and wild pointers pre-fix; the fix has been validated on real windows-11-aarch64 hardware in CI (build #71302) and locally at fc228dc3/fb473c6b. The TLS patch is defensive (rejects more inputs, not fewer). The __va_arg addition is a near-verbatim copy of upstream lib/va_list.c and is only reachable on x86_64 SysV targets. No new attack surface is introduced beyond what bun:ffi already exposes on other platforms.
Level of scrutiny
High. This is a platform enablement for a native-code JIT path, coupled with a vendored dependency bump whose actual code change (the LLP64 fix in arm64-gen.c) lives in a separate repo and is only referenced by commit hash. The build-config changes and test un-gating are mechanical, but the correctness of the whole PR hinges on the fork-side TinyCC fix and on the __va_arg port — both of which are C-ABI-level code where subtle mistakes manifest as memory corruption. Jarred-Sumner has been actively shepherding this (rebasing, sweeping for stragglers, validating CI), and the adversarial review pass in the timeline already caught and fixed two follow-ups (SHF_TLS scan scope, .tbss test coverage), but a maintainer approval is appropriate for a change of this scope.
Other factors
CI on the windows-11-aarch64 lane is green per the timeline (build #71302 and local re-verification at fb473c6b). All CodeRabbit nits (comment length, tempDir disposal) were addressed and threads resolved. #33653 and #29476 are closed as superseded. The process.versions.tinycc assertion in process.test.js matches the new pin. No outstanding unresolved review comments.
Merges ~82 upstream commits (Jan-Jun 2026) into oven-sh/tinycc, keeping the fork's macOS framework/dlsym and parser patches. Notable upstream fixes for bun:ffi: x86_64 REX prefix miscompile when materializing 0 into r8-r15 (5th/6th integer args), an OOB read in tccrun after relocate with -g, and arm64 long double comparison/negation fixes. The fork additionally makes tcc_relocate flush the instruction cache on arm64 Windows. Two upstream behavior changes needed handling here: - __va_arg is no longer inlined by tcc's preprocessor and now lives in libtcc1, which Bun replaces with src/runtime/ffi/libtcc1.c; ship the function there (extern-free: the trampoline TCC states are -nostdlib). Without it, any va_arg in cc()-compiled C fails to relocate. - Upstream now accepts _Thread_local/__thread and emits Local-Exec TLS, which is meaningless for in-memory relocation and aliases the host's thread block; the fork makes tcc_relocate reject TLS objects. The fork also drops its Windows-ARM64 patches in favor of upstream's own arm64-PE backend, and fixes an uninitialized pstrcat buffer in the macOS framework path.
Upstream TinyCC now has an arm64-PE backend (merged in the previous commit), so drop the windows-arm64 exclusion from cfg.tinycc, the generated ENABLE_TINYCC constant, and the tcc_sys link stubs, and un-skip the FFI tests that were gated on the platform. The fork also gained the FlushInstructionCache call TinyCC's in-memory relocation needs on arm64 Windows. Verified by cross-compiling bun.exe for aarch64-pc-windows-msvc from Linux (TinyCC's arm64+PE objects compile with clang-cl and the binary links with the real libtcc symbols); the windows-aarch64 CI lanes now run the bun:ffi test suites. Known parity notes, unchanged by this commit: cc() code with a stack frame over 4 KB needs __chkstk, which Bun does not provide on any Windows target, and long double helpers are not provided on any arm64 target.
On Windows, TinyCC itself is compiled by an LLP64 toolchain, and its arm64 backend used 'unsigned long' masks to decide whether a 64-bit constant fits an add/sub immediate: (1ll << 49) was encoded as 'add ..., #0'. That constant is JSC's DoubleEncodeOffset, so every double crossing bun:ffi's JIT'd trampolines on Windows ARM64 lost the NaN-boxing offset (values off by one exponent step) and pointer arguments decoded to wild addresses (segfaults). Fixed in oven-sh/tinycc mob @ 8a6cbc12 by using uint64_t-typed masks.
Apply the SHF_TLS section scan from oven-sh/tinycc#3 as a build patch so the guard also catches per-symbol .tdata.foo/.tbss.foo sections from linked object files (gcc -fdata-sections). The previous two-pointer check only saw the canonical .tdata/.tbss sections that tcc's own codegen writes into. Extend the cc.test.ts TLS matrix to include uninitialized thread-locals, which land in .tbss rather than .tdata, so both arms of the guard are exercised.
Placing the scan after tcc_add_runtime also swept tcc's own support objects, which under the documented libtcc1-usegcc=yes knob makes every "tcc -b -run" fail on a TLS-free program (gcc-built bcheck.o carries a __thread no_checking in .tbss). User inputs are added via tcc_add_file before tcc_relocate runs, so the original position already covers them. Matches oven-sh/tinycc#3 at e2643dcc.
The SHF_TLS section-scan guard is now in the fork's mob branch, so drop the build-time patch that carried it.
3379760 to
e83d3d0
Compare
There was a problem hiding this comment.
I didn't find any bugs, but this enables the TinyCC JIT on a new platform, bumps the vendored TinyCC pin, and adds a new __va_arg runtime helper — worth a human look before landing.
What was reviewed:
- The three-way
cfg.tinycc/ENABLE_TINYCC/tcc_externs!predicate stays consistent (all now!(android || freebsd)). - The new
__va_arginlibtcc1.cmatches upstream TinyCC'slib/va_list.cminus theabort()reference; correctly gated to__x86_64__ && !_WIN32. - Un-gated tests: each removed
isFFIUnavailableguard was purely the win-arm64 exclusion; no other condition was dropped. process.versions.tinyccassertion andTINYCC_COMMITagree;patches/tinycc/now holds onlytcc.h.patch(the SHF_TLS patch was upstreamed into the new pin).
Extended reasoning...
Overview
This PR enables bun:ffi (TinyCC) on Windows ARM64. It touches four layers that must agree: the build-config default (config.ts), the generated Rust constant (buildOptionsRs.ts), the Rust FFI link stubs (tcc_sys/tcc.rs), and the dep enable predicate (deps/tinycc.ts). It also bumps the vendored TinyCC commit to 05f0fafa (carrying the LLP64 arm64-immediate fix and the SHF_TLS run-time TLS guard fix), adds an x86_64-SysV __va_arg implementation to Bun's libtcc1.c (needed because the newer TinyCC no longer inlines it in tccdefs.h), and un-skips ~10 FFI test gates plus adds new variadic/TLS coverage in cc.test.ts.
Security risks
FFI JIT-compiles and executes arbitrary user C at runtime. Enabling it on a new target is inherently security-relevant: a codegen bug (like the LLP64 immediate miscompile this PR fixes in the fork) can turn into memory corruption or type confusion in the host process. The new __va_arg runtime helper is compiled by TinyCC into every cc() on x86_64 SysV; it does raw pointer arithmetic on the caller's register-save area. It is a near-verbatim copy of upstream's lib/va_list.c, and the new tests exercise integer/float/struct/overflow-area paths, but it is still new code on a hot boundary.
Level of scrutiny
High. Per the repo's own guidance, this hits both Dependencies & vendoring (TinyCC pin bump) and Cross-platform (enabling a platform-gated native code path). The change is not mechanical — the correctness of the enablement rests on a codegen fix in an external fork, and the __va_arg addition affects every non-Windows x64 cc() call, not just the newly-enabled platform.
Other factors
The PR has been actively shepherded by a maintainer (rebased, CI-verified on real windows-11-aarch64 hardware, adversarial review passes on both the diff and the fork delta), and all CodeRabbit threads are resolved. Test coverage for the new code is good (variadic ints/doubles/structs/long-double, TLS rejection across .tdata/.tbss). Nothing looks wrong to me, but the combination of a vendored-dep bump, a new native runtime helper, and a platform enablement is exactly the shape that warrants a human sign-off rather than a bot approval.
…sts on arm64 The node directory iterator now asks NtQueryDirectoryFile for FileFullDirectoryInformation, whose EaSize field is the reparse tag of a reparse-point entry, and hands attributes and tag to the cp walkers. A walked reparse entry is classified from the listing alone and the result travels with the CpSingleTask, so the copy neither re-stats nor opens it; the path-based lookup remains for the top-level operand and for listings without a tag. readdir ignores the extra field. bun:ffi works on Windows arm64 since #33696, so the tests no longer skip there, and the link cases now check the link flavor and target.
Summary
Supersedes #33653 (the TinyCC upgrade; the first commit here is that PR's content) and #29476. Enables
bun:ffion Windows ARM64: drops the windows-arm64 exclusion fromcfg.tinycc, the generatedENABLE_TINYCCconstant, and thetcc_syslink stubs, un-skips the FFI tests that were gated on the platform, and bumps the TinyCC pin tooven-sh/tinycc@05f0fafa(which is8a6cbc12plus oven-sh/tinycc#3).The windows-arm64 bug this uncovered (root-caused and fixed in the TinyCC fork)
Enabling the platform made the windows-11-aarch64 CI lane run the FFI suites on real hardware for the first time, which exposed wrong doubles (
sum(0.5…9.5)= 46 instead of 50) and segfaults in every JSCallback test. Root cause, proven with an in-CI probe that dumped the JIT machine code from the runner:TinyCC's arm64 backend decides whether a 64-bit constant fits an ADD/SUB immediate with
!(val & ~0xffful). On an LLP64 host (which is exactly how Bun builds TinyCC intobun.exeon Windows via clang-cl)unsigned longis 32 bits, so~0xfffulzero-extends to0x00000000fffff000, and any constant with no bits in [12,32) "fits".1ll << 49is JSC'sDoubleEncodeOffset, used by everybun:ffitrampoline, and it compiled toadd xN, xN, #0:… ldur x0,[x29,#-8]; add x0,x0,#0 …… mov x30,#0x2000000000000; add x0,x0,x30 …So every double crossing the FFI boundary lost the NaN-boxing offset (JS saw
bits − 2^49: 12.25→11.25, 50→46) and pointer arguments decoded to wild addresses (the segfaults). LP64-built TinyCC (Linux/macOS) is unaffected; the fix produces byte-identical output there, which is why the bug only ever existed on Windows ARM64. Fixed in the fork (arm64_gen_opic,arm64_check_offset,arm64_sym) by usinguint64_t-typed masks; TinyCC's own test suite passes, and oven-sh/tinycc#3 adds regression cases totests2/73_arm64. This also affects any natively-built windows-arm64 TinyCC upstream.Known parity notes (pre-existing, unchanged)
cc()code with a >4 KB stack frame needs__chkstk, which Bun doesn't provide on any Windows target.long doublesoft-float helpers aren't provided on any arm64 target.Test plan
bun.exefrom Linux links with TinyCC enabledbun:ffisuites green with the LLP64 fix (build #71302, and re-verified locally on Windows 11 ARM64 ate83d3d00)Fixes #28055