Update TinyCC to latest upstream - #33653
Conversation
|
Updated 1:05 PM PT - Jul 7th, 2026
@Jarred-Sumner, your commit 0bf5d06 is building: |
WalkthroughUpdates TinyCC platform gating and dependency metadata, adds x86_64 SysV ChangesTinyCC gating, helper, and tests
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@test/js/bun/ffi/cc.test.ts`:
- Around line 591-604: Replace the temporary directory setup in the cc() TLS
test to use tempDir with automatic disposal instead of tempDirWithFiles, since
each synchronous it.each iteration only needs the directory briefly and should
clean up after itself. Update the thread-local storage test block in cc.test.ts
to create the tls.c file via tempDir from harness and keep the existing cc()
assertion/toThrow behavior tied to the bump symbol and the
_Thread_local/__thread cases.
🪄 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: cd80ceb6-62c1-4e55-b2ac-347c397d5e9a
📒 Files selected for processing (3)
scripts/build/deps/tinycc.tssrc/runtime/ffi/libtcc1.ctest/js/bun/ffi/cc.test.ts
| describe.skipIf(isASAN || isFFIUnavailable)("thread-local storage inside cc()-compiled C", () => { | ||
| it.each(["_Thread_local", "__thread"])("%s is a compile error", keyword => { | ||
| const dir = tempDirWithFiles(`bun-ffi-cc-tls`, { | ||
| "tls.c": `${keyword} int bun_test_tls_counter = 0;\nint bump(void) { return ++bun_test_tls_counter; }\n`, | ||
| }); | ||
| expect(() => { | ||
| cc({ | ||
| source: path.join(dir, "tls.c"), | ||
| symbols: { bump: { args: [], returns: "int" } }, | ||
| }); | ||
| }).toThrow(/thread-local storage is not supported/); | ||
| }); | ||
| }); | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use tempDir (with automatic disposal) instead of tempDirWithFiles.
The directory only needs to live for the duration of each synchronous it.each iteration, so tempDirWithFiles (no cleanup) leaves temp directories behind on every run — unlike the varargs test above it, which correctly uses using dir = tempDir(...).
As per path instructions, test/**/*.test.ts: "Use port: 0, normalizeBunSnapshot, tempDir from harness, and assert stdout before exitCode when writing subprocess tests."
🧹 Proposed fix
it.each(["_Thread_local", "__thread"])("%s is a compile error", keyword => {
- const dir = tempDirWithFiles(`bun-ffi-cc-tls`, {
+ using dir = tempDir(`bun-ffi-cc-tls`, {
"tls.c": `${keyword} int bun_test_tls_counter = 0;\nint bump(void) { return ++bun_test_tls_counter; }\n`,
});
expect(() => {
cc({
- source: path.join(dir, "tls.c"),
+ source: path.join(String(dir), "tls.c"),
symbols: { bump: { args: [], returns: "int" } },
});
}).toThrow(/thread-local storage is not supported/);
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| describe.skipIf(isASAN || isFFIUnavailable)("thread-local storage inside cc()-compiled C", () => { | |
| it.each(["_Thread_local", "__thread"])("%s is a compile error", keyword => { | |
| const dir = tempDirWithFiles(`bun-ffi-cc-tls`, { | |
| "tls.c": `${keyword} int bun_test_tls_counter = 0;\nint bump(void) { return ++bun_test_tls_counter; }\n`, | |
| }); | |
| expect(() => { | |
| cc({ | |
| source: path.join(dir, "tls.c"), | |
| symbols: { bump: { args: [], returns: "int" } }, | |
| }); | |
| }).toThrow(/thread-local storage is not supported/); | |
| }); | |
| }); | |
| describe.skipIf(isASAN || isFFIUnavailable)("thread-local storage inside cc()-compiled C", () => { | |
| it.each(["_Thread_local", "__thread"])("%s is a compile error", keyword => { | |
| using dir = tempDir(`bun-ffi-cc-tls`, { | |
| "tls.c": `${keyword} int bun_test_tls_counter = 0;\nint bump(void) { return ++bun_test_tls_counter; }\n`, | |
| }); | |
| expect(() => { | |
| cc({ | |
| source: path.join(String(dir), "tls.c"), | |
| symbols: { bump: { args: [], returns: "int" } }, | |
| }); | |
| }).toThrow(/thread-local storage is not supported/); | |
| }); | |
| }); |
🤖 Prompt for 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.
In `@test/js/bun/ffi/cc.test.ts` around lines 591 - 604, Replace the temporary
directory setup in the cc() TLS test to use tempDir with automatic disposal
instead of tempDirWithFiles, since each synchronous it.each iteration only needs
the directory briefly and should clean up after itself. Update the thread-local
storage test block in cc.test.ts to create the tls.c file via tempDir from
harness and keep the existing cc() assertion/toThrow behavior tied to the bump
symbol and the _Thread_local/__thread cases.
Source: Path instructions
58ad912 to
0f56ad6
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/runtime/ffi/libtcc1.c`:
- Around line 608-611: The block comment in libtcc1.c is too long and needs to
be trimmed to 3 lines or fewer. Condense the explanation around __va_arg, Bun’s
replacement of libtcc1, and the no-extern-abort deviation into a shorter comment
near the existing libtcc1/va_arg logic, keeping only the essential context and
moving any extra detail to docs if needed.
In `@test/js/bun/ffi/cc.test.ts`:
- Line 464: The subprocess-spawning tests in cc.test.ts should be made
concurrent instead of sequential. Update the affected `it(...)` cases (including
the `va_arg over ints, doubles, and the stack overflow area` test and the other
listed `it` blocks) to use `it.concurrent` or group them under
`describe.concurrent`, since they are independent and each uses its own temp
directory. Keep the test bodies unchanged and only switch the test declarations
to the concurrent variant.
🪄 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: e615bd26-f9e3-4294-8625-63f911cfe829
📒 Files selected for processing (3)
scripts/build/deps/tinycc.tssrc/runtime/ffi/libtcc1.ctest/js/bun/ffi/cc.test.ts
| /* TinyCC's lib/va_list.c: __va_arg is no longer inlined by tccdefs.h, and Bun | ||
| replaces libtcc1 with this file. Only referenced by x86_64 SysV codegen. | ||
| Deviation from upstream: no extern abort() — Bun never injects that symbol, | ||
| so referencing it would make every cc() fail with an unresolved reference. */ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Comment exceeds 3-line guideline.
This block comment spans 4 lines. As per coding guidelines: "Keep code comments to 3 lines max - Comments must be concise. If the code needs more explanation than that, it belongs in docs."
✏️ Proposed trim
-/* TinyCC's lib/va_list.c: __va_arg is no longer inlined by tccdefs.h, and Bun
- replaces libtcc1 with this file. Only referenced by x86_64 SysV codegen.
- Deviation from upstream: no extern abort() — Bun never injects that symbol,
- so referencing it would make every cc() fail with an unresolved reference. */
+/* TinyCC's lib/va_list.c, referenced only by x86_64 SysV codegen. Deviation
+ from upstream: no extern abort() (Bun never injects that symbol, so any
+ reference would make every cc() fail to link). */📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /* TinyCC's lib/va_list.c: __va_arg is no longer inlined by tccdefs.h, and Bun | |
| replaces libtcc1 with this file. Only referenced by x86_64 SysV codegen. | |
| Deviation from upstream: no extern abort() — Bun never injects that symbol, | |
| so referencing it would make every cc() fail with an unresolved reference. */ | |
| /* TinyCC's lib/va_list.c, referenced only by x86_64 SysV codegen. Deviation | |
| from upstream: no extern abort() (Bun never injects that symbol, so any | |
| reference would make every cc() fail to link). */ |
🤖 Prompt for 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.
In `@src/runtime/ffi/libtcc1.c` around lines 608 - 611, The block comment in
libtcc1.c is too long and needs to be trimmed to 3 lines or fewer. Condense the
explanation around __va_arg, Bun’s replacement of libtcc1, and the
no-extern-abort deviation into a shorter comment near the existing
libtcc1/va_arg logic, keeping only the essential context and moving any extra
detail to docs if needed.
Source: Coding guidelines
| // libtcc1 to provide; Bun replaces libtcc1 with src/runtime/ffi/libtcc1.c. | ||
| // TinyCC's setjmp/longjmp error handling conflicts with ASan. | ||
| describe.skipIf(isASAN || isFFIUnavailable)("variadic functions inside cc()-compiled C", () => { | ||
| it("va_arg over ints, doubles, and the stack overflow area", async () => { |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
Consider it.concurrent for subprocess-spawning tests.
These tests spawn a Bun.spawn subprocess and are independent of each other (unique temp dirs). As per path instructions: "Prefer concurrent tests over sequential tests: When multiple tests in the same file spawn processes or write files, make them concurrent with test.concurrent or describe.concurrent unless it's very difficult to make them concurrent."
Also applies to: 549-556, 579-579, 607-614
🤖 Prompt for 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.
In `@test/js/bun/ffi/cc.test.ts` at line 464, The subprocess-spawning tests in
cc.test.ts should be made concurrent instead of sequential. Update the affected
`it(...)` cases (including the `va_arg over ints, doubles, and the stack
overflow area` test and the other listed `it` blocks) to use `it.concurrent` or
group them under `describe.concurrent`, since they are independent and each uses
its own temp directory. Keep the test bodies unchanged and only switch the test
declarations to the concurrent variant.
Source: Path instructions
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
CI triage for build #69884 (current commit): every
One CI finding in the first build was real and is fixed in the current commit: the new long-double varargs test failed on linux-aarch64 because |
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.
0f56ad6 to
c42d302
Compare
Jarred-Sumner
changed the title
|
Correction to the triage above: the |
c42d302 to
0bf5d06
Compare
Jarred-Sumner
changed the title
| * Disabled on windows-arm64 (upstream tinycc now has an arm64-pe backend, | ||
| * but bun:ffi has never been enabled there — see cfg.tinycc in config.ts). |
There was a problem hiding this comment.
🔴 The PR title, description (section 2), and the 18:37 follow-up comment all say this push enables bun:ffi on Windows ARM64, but that commit isn't in the diff — only 4 files are touched, config.ts:869 still gates tinycc off for (windows && arm64), cc.test.ts:8 still sets isFFIUnavailable = isWindows && isArm64, and this very comment is being updated to say "Disabled on windows-arm64 … bun:ffi has never been enabled there". Looks like the second commit was never pushed (or got dropped) — either push it or drop the claim from the title/description before merge.
Extended reasoning...
What the discrepancy is
The PR title is "Update TinyCC to latest upstream; enable bun:ffi on Windows ARM64", the description's section 2 describes concrete enablement changes ("Drops the windows-arm64 exclusion from cfg.tinycc, the generated ENABLE_TINYCC constant, and the tcc_sys link stubs, and un-skips the FFI tests…"), and Jarred's own follow-up comment at 2026-07-07T18:37 says "the current push also enables bun:ffi on Windows ARM64". CodeRabbit's walkthrough likewise lists scripts/build/config.ts, scripts/build/buildOptionsRs.ts, src/tcc_sys/tcc.rs, ffi.test.js, ffi-error-messages.test.ts, cp.test.ts, fs-writeSync-stdio-windows.test.ts, and napi-value-ffi.test.ts as changed. None of that is in the diff being reviewed.
What's actually in the diff
The changed-files list contains exactly four files: scripts/build/deps/tinycc.ts, src/runtime/ffi/libtcc1.c, test/js/bun/ffi/cc.test.ts, and test/js/node/process/process.test.js. git log on the branch shows only one commit (0bf5d067 "Update TinyCC to upstream tip"). Checked against the repo at HEAD:
scripts/build/config.ts:869still readsconst tinycc = partial.tinycc ?? !((windows && arm64) || abi === "android" || freebsd);— the windows-arm64 exclusion is intact.test/js/bun/ffi/cc.test.ts:8still hasconst isFFIUnavailable = isWindows && isArm64;.- No changes to
buildOptionsRs.ts,src/tcc_sys/tcc.rs,ffi.test.js,ffi-error-messages.test.ts,ffi-viewSource-non-object.test.ts,cp.test.ts,fs-writeSync-stdio-windows.test.ts, ornapi-value-ffi.test.ts.
Most tellingly, the one hunk this PR does apply to tinycc.ts:5-6 updates the doc comment to say "Disabled on windows-arm64 (upstream tinycc now has an arm64-pe backend, but bun:ffi has never been enabled there — see cfg.tinycc in config.ts)" — i.e. the change itself documents that it remains disabled. That's internally consistent with a first-commit-only push and directly contradicts the title.
Why this matters
This is not merely a stale description. The 18:37 comment explicitly says "the current push also enables bun:ffi on Windows ARM64", so the author believes the enablement commit is on the branch. It isn't — the second commit appears to have been lost in a rebase, force-push, or was authored but never pushed. Merging as-is would (a) record in git history that Windows ARM64 FFI was enabled when the code path is still compiled out, and (b) silently drop work the author intended to ship.
Step-by-step proof
- PR diff → 4 files; none is
config.ts,buildOptionsRs.ts,tcc.rs, or any of the un-skipped test files. git logon HEAD → only0bf5d067"Update TinyCC to upstream tip"; no second commit.scripts/build/config.ts:869→!((windows && arm64) || …)still excludes windows-arm64.scripts/build/deps/tinycc.ts:5-6in the diff → new comment literally says "Disabled on windows-arm64 … bun:ffi has never been enabled there".test/js/bun/ffi/cc.test.ts:8→isFFIUnavailable = isWindows && isArm64unchanged.- Therefore: on a windows-arm64 build with this PR applied,
cfg.tinyccis stillfalse, TinyCC is not built, and everybun:ffitest is still skipped — the PR does not do what the title claims.
Fix
Either push the missing second commit (the one touching config.ts / buildOptionsRs.ts / tcc_sys/tcc.rs / the FFI + fs test skips), or — if it's intentionally being deferred to a follow-up — retitle the PR and drop section 2 from the description so the merge commit accurately reflects what shipped.
|
Heads-up on impact: your So this PR, once merged, is what unblocks #29476 and therefore #28055 ( |
|
Correcting my earlier comment: this PR's pin ( |
|
Superseded by #33696, which carries this PR's commit plus the windows-arm64 enablement on top. The review notes here (libtcc1.c comment length, TLS test |
## Summary Supersedes #33653 (the TinyCC upgrade; the first commit here is that PR's content) and #29476. Enables `bun:ffi` on Windows ARM64: drops the windows-arm64 exclusion from `cfg.tinycc`, the generated `ENABLE_TINYCC` constant, and the `tcc_sys` link stubs, un-skips the FFI tests that were gated on the platform, and bumps the TinyCC pin to [`oven-sh/tinycc@05f0fafa`](oven-sh/tinycc@05f0faf) (which is [`8a6cbc12`](oven-sh/tinycc@8a6cbc1) plus 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 into `bun.exe` on Windows via clang-cl) `unsigned long` is 32 bits, so `~0xffful` zero-extends to `0x00000000fffff000`, and any constant with no bits in [12,32) "fits". `1ll << 49` is JSC's `DoubleEncodeOffset`, used by every `bun:ffi` trampoline, and it compiled to `add xN, xN, #0`: - machine code on the CI runner: `… ldur x0,[x29,#-8]; add x0,x0,#0 …` - same source, LP64-built tcc: `… 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 using `uint64_t`-typed masks; TinyCC's own test suite passes, and oven-sh/tinycc#3 adds regression cases to `tests2/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 double` soft-float helpers aren't provided on any arm64 target. ## Test plan - [x] Linux/macOS/Windows-x64 behavior unchanged (same suites as #33653; LP64 tcc output byte-identical across the fix) - [x] Cross-built windows-arm64 `bun.exe` from Linux links with TinyCC enabled - [x] windows-11-aarch64 CI lane: `bun:ffi` suites green with the LLP64 fix (build #71302, and re-verified locally on Windows 11 ARM64 at `e83d3d00`) Fixes #28055 --------- Co-authored-by: robobun <117481402+robobun@users.noreply.github.com>
2 participants
Summary
Updates the vendored TinyCC (the C compiler behind
bun:ffi'scc()/ JIT trampolines) to the tip of upstream TinyCC, merged into oven-sh/tinyccmob@c22ca054. The previous pin was from January 2026; this brings in ~82 upstream commits.Fork maintenance (already pushed to
oven-sh/tinyccmob):mob@a338258d(2026-06-13). Fork patches upstream has since implemented itself were dropped in favor of the upstream version (all of the Windows-ARM64/PE backend work,configurehost_cc, CI job, test skips, libtcc1 aarch64 guard).-framework, framework header/library search,dlsym(RTLD_SELF)fallback, cached xcode-select SDK lookup) plus the enum-attribute /__attribute__((deprecated))parser patches. Fixed three latent bugs in them while there (uninitializedpstrcatbuffer,boolwithout<stdbool.h>,parse_attributeon an uninitializedAttributeDef).tcc_relocatenow rejects thread-local variables (upstream newly accepts_Thread_local/__threadand emits Local-Exec TLS, which has no meaning for an in-memory image — the generated thread-pointer-relative accesses land inside the host process's own TLS block, i.e. they would silently corrupt Bun's thread-locals; previously this was a compile error, and it is again), andtcc_relocateflushes the instruction cache afterVirtualProtecton arm64 Windows (groundwork for enabling bun:ffi there).Notable upstream fixes we pick up: an x86_64 miscompile (missing REX prefix when materializing 0 into r8–r15, which are the 5th/6th integer argument registers), an out-of-bounds read in
tccrunafter relocate with-g, arm64 long-double comparison fixes, an arm64 inline assembler, Win32 compile-lock init made thread-safe, and Win32 JIT run memory moved from the CRT heap to a dedicatedVirtualAllocregion withRtlAddFunctionTablefailure now surfaced as an error (6728a64f+601a0882). The last one is the root cause of the Windows x64bun:ffisegfaults insideRtlAddFunctionTable/JSFFIFunction::trampolinetracked as Sentry BUN-2V2K and BUN-3K30; see #32013 for the full analysis. Supersedes #32013. Should address #31941 (not auto-closing; the link is causal, not an observed before/after).Bun-side changes needed by the bump:
src/runtime/ffi/libtcc1.c: upstream moved__va_argout of the preprocessor predefs into libtcc1, which Bun replaces with this file — so ship__va_arghere. Without it, anyva_argincc()-compiled C on x86_64 fails to relocate (unresolved reference to '__va_arg'). The function is upstream's, minus theextern abort()(Bun's trampoline TCC states are-nostdlib, so an extern there breaks everycc()call).process.versions.tinyccpin updated inprocess.test.js.long doubleva_argpaths, and a test that_Thread_local/__threadproduce a compile error.Not changed here: bun:ffi is still disabled on Windows ARM64. Upstream now has an arm64-PE backend, but running Bun's FFI suites on real windows-arm64 hardware (see the follow-up PR) shows it still miscompiles variadic doubles and crashes when JIT code calls back into the host, so enabling stays split out until those are fixed in the fork.
Test plan
bun bd --asan=off test test/js/bun/ffi/cc.test.ts test/js/bun/ffi/ffi.test.js test/js/bun/ffi/ffi-error-messages.test.ts— all pass (thedlopenmatrix runs against/tmp/bun-ffi-test.so)libtcc1.cchange (unresolved reference to '__va_arg') and passes with it; also passes on the previous TinyCCtcc_relocateguard (TinyCC silently accepts TLS) and passes with it./configure && make && make testpasses on the merged fork tree (linux-x64)vendor/tinyccfetch of the pinned commit → full debug build → the above suites