Lower the .bun section alignment on ELF so PT_LOAD segments do not overlap - #40756
Conversation
…erlap The 16KB alignment raised the RW PT_LOAD's p_align to 0x4000 while the other segments stayed at 0x1000, and lld assigned the RW p_vaddr so that round_down(p_vaddr, p_align) overlapped the executable segment's last pages. The kernel ignores p_align at execve, but a loader that honors it (UPX's stub) maps the overlap and corrupts the image. On ELF the section only holds the 8-byte header: exe_format/elf.rs appends the --compile payload at a page-aligned vaddr, so 8-byte alignment suffices. Mach-O keeps 16KB; exe_format/macho.rs expands the __BUN segment in place at that alignment. Fixes #40752
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour. WalkthroughThe change makes ELF blob-header alignment platform-specific. It adds shared ELF64 program-header parsing utilities and uses them in segment-layout and TLS-size regression tests. ChangesELF alignment and validation
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The change is narrowly scoped to ELF alignment behavior, with no actionable merge-blocking risk remaining after normal checks and review. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Full details: Description checkExplanation The description explains the problem, fix, technical background, and verification results in detail. It does not use the template headings "What does this PR do?" and "How did you verify your code works?", but it provides the required information under equivalent sections. Comment |
|
Updated 1:28 AM PT - Aug 28th, 2026
⏳ @autofix-ci[bot], your commit 3781acd is still building in
|
There was a problem hiding this comment.
LGTM — thanks for addressing the earlier feedback (stdout drain, FreeBSD gating, and hoisting the ELF program-header reader into harness.ts).
What was reviewed:
- Confirmed
src/exe_format/elf.rscomputes its own page-alignednew_vaddr(align_up(max_vaddr_end, page_size)) and keepsp_alignunchanged when growing the RW segment, so nothing on the ELF path depends on the old 16KB section alignment;BlobHeader/BLOB_HEADER_ALIGNMENTis otherwise only referenced frommacho.rs, where the 16KB value is preserved. - The
180_000per-test timeout matches existing precedent intest/bundler/bun-build-compile.test.tsandbun-build-api.test.tsfor--compiletests, so I'm no longer flagging it. - Checked the shared
readElf64ProgramHeadersagainst the removed inline parser intls-segment-size.test.ts— behavior-preserving, and the new test's overlap check guards against a vacuous pass withexpect(loads.length).toBeGreaterThan(1).
Extended reasoning...
Overview
The PR lowers BLOB_HEADER_ALIGNMENT from 16 * 1024 to 8 on ELF targets (Linux/FreeBSD) while keeping 16 * 1024 on Darwin, fixing a PT_LOAD segment overlap under strict-p_align loaders like UPX's decompression stub (#40752). Supporting changes hoist preadExact and a new readElf64ProgramHeaders helper into test/harness.ts, refactor tls-segment-size.test.ts to consume the shared helper, and add test/bundler/compile-elf-segment-layout.test.ts asserting the non-overlap invariant on both the bun binary and a fresh --compile output.
Security risks
None. The change adjusts a link-time section alignment constant and adds test infrastructure. No user input handling, auth, crypto, or network paths are touched. Lowering section alignment cannot introduce a memory-safety issue — the struct itself is alignas(BLOB_HEADER_ALIGNMENT) with a single size_t, and 8-byte alignment satisfies size_t's natural alignment.
Level of scrutiny
Moderate. The native change is a single conditional #define, but it affects how every ELF bun binary links, so I verified the load-bearing claim: src/exe_format/elf.rs places the --compile payload at align_up(max_vaddr_end, page_size) and only grows the existing RW segment's filesz/memsz without touching p_align, and a repo-wide grep shows BlobHeader/BLOB_HEADER_ALIGNMENT is otherwise referenced only in macho.rs (which retains 16KB) and StandaloneModuleGraph.rs. Nothing on the ELF path reads the section alignment. The aarch64 note in the PR (segments already at p_align 0x10000) matches elf.rs's page_size table.
Other factors
This is my fifth look at the PR. Commits 6e62802f and e2e36cb3 addressed three of my four prior inline comments (stdout pipe drained, !(isLinux || isFreeBSD) gating, ELF reader deduplicated into harness). The remaining 180_000 timeout follows established precedent in sibling --compile tests (bun-build-compile.test.ts, bun-build-api.test.ts), which is a legitimate outlier per root CLAUDE.md — bun build --compile under debug+ASAN copies the entire multi-hundred-MB binary. The new test uses tempDir, bunEnv, bunExe(), await using, drains all pipes concurrently, asserts stdout/stderr before exit code, and guards against vacuous passes with expect(loads.length).toBeGreaterThan(1). No CODEOWNERS cover the changed paths. Exit reason was dry_streak.
Problem
bun build --compileexecutable fails on linux withSyntaxError: Invalid character: '\0'(Using UPX to compress a single file executable generated with bun 1.4 fails #40752). The uncompressed executable runs..bunsection is declared with 16KB alignment (BLOB_HEADER_ALIGNMENTinsrc/jsc/bindings/c-bindings.cpp). That raises the RWPT_LOAD'sp_alignto 0x4000 while every other segment stays at 0x1000, and lld assigns the RWp_vaddrso thatround_down(p_vaddr, p_align)overlaps the last pages of the executable segment. Linuxexecveignoresp_align, so the plain binary runs. UPX's stub honors it, maps the overlap, and the embedded module source reads back corrupted.Fix
BLOB_HEADER_ALIGNMENTat 16KB on Mach-O, whereexe_format/macho.rsexpands the__BUNsegment in place at that alignment. Lower it to 8 on ELF.exe_format/elf.rsappends the--compilepayload at a page-aligned virtual address it computes itself, so nothing depends on the section alignment.PT_LOADhasp_align0x1000 and the strict mapping ranges no longer intersect. A UPX 5.2.0-packed debug build starts and runs correctly.test/bundler/compile-elf-segment-layout.test.ts(both tests fail on bun 1.4.1 with a 0x3000 overlap). Alsobun-build-compile.test.ts,compile-asset-bunfs.test.ts,tls-segment-size.test.ts, andregression/issue/29290.test.ts.Background
PT_LOADprogram header tells the loader to map a file range atp_vaddr.p_aligndeclares the mapping granularity: a strict loader maps[round_down(p_vaddr, p_align), round_up(p_vaddr + p_memsz, p_align)). A segment'sp_alignis the largest alignment of any section inside it.p_align, which hides the overlap. UPX's decompression stub performs the mapping itself and honorsp_align. The UPX maintainer's analysis is in UPX does not decompress correctly bun bundles with bun 1.4.0 upx/upx#18906..bunsection holds the standalone module graph header. At--compiletime bun appends the payload past every existing mapping and grows the RWPT_LOADto cover it (exe_format/elf.rs).Notes
Layout of the official bun-v1.4.1 linux-x64 binary (
readelf -lW), which--compileinherits:The R E segment's strict mapping ends at
round_up(0x1743200 + 0x31f6ed0, 0x1000) = 0x493b000. The RW segment's strict mapping starts atround_down(0x493b0d0, 0x4000) = 0x4938000. Overlap: 0x3000. Bun 1.4.0 has the same shape with a 0x2000 overlap, and 1.4.0 + UPX 5.2.0 reproduces the reporter's failure reliably.After the fix the debug build links as:
End-to-end check: UPX refuses files over its size cap, and a debug
--compileoutput is 816MB, so the packed run used a stripped copy of the fixed debug bun (484MB).upx -1packed it and the packed binary runs--versionand-ecorrectly. With the old layout the stub maps the RW segment over the executable segment's tail, so any packed binary with this shape is corrupt. The full compiled-app repro needs a release-linked binary.The three
--bytecodetests inbun-build-compile.test.tstime out on this builder with and without the change (checked by stashing the fix and rerunning). They are container-speed failures, not caused by this diff.aarch64 is unaffected: its segments already use
p_align0x10000, which is larger than the old 16KB, so the RWp_aligndoes not change there.no test proof · iteration 1 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/binary/tls-segment-size.test.ts, test/bundler/compile-elf-segment-layout.test.ts