Skip to content

Remove dead code from the glibc wraps, #[uws_callback], and two re-export lists - #47156

Open
robobun wants to merge 1 commit into
mainfrom
robobun/e500bc05/dead-code-sweep
Open

robobun wants to merge 1 commit into
mainfrom
robobun/e500bc05/dead-code-sweep

Conversation

@robobun

@robobun robobun commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator

Problem

  • --wrap=mknod and --wrap=fstatat guard glibc symbols that nothing calls. libarchive was the one caller of both, and Replace libarchive with a tar reader and writer in safe Rust #46638 removed libarchive. The shims __wrap_mknod and __wrap_fstatat (src/jsc/bindings/workaround-missing-symbols.cpp) stayed.
  • Three smaller items have no user: the thunk = "name" option of #[uws_callback], two crate-root re-exports, and two members of the NativePtr type.

Fix

  • Delete them: 28 lines in 6 files. The Notes give the proof for each item.
  • Correct because no link input refers to mknod or fstatat. The debug binary of this branch and the canary bun-profile for linux x64 and aarch64 import neither.
  • Verified: bun bd, bun run rust:check-all (12 targets), cargo check --workspace --all-targets and objdump -T. The Notes list the tests.
  • Self-reviewed: 40 concerns raised, 36 addressed. Most led to a cut of the draft from 16 files to 6. The Notes list the four rejected.

Background

  • Bun links with -Wl,--wrap=<symbol> for each glibc symbol newer than 2.17. __wrap_<symbol> calls an older versioned symbol. test/js/bun/symbols.test.ts fails when the binary imports a newer one.
  • #[uws_callback] generates an extern "C" thunk for a method. All 45 uses pass export, which names the symbol.
  • No other design was weighed: a deletion has one place.

Downsides

Behaviour change: none

Notes

Removed

  • "mknod" and "fstatat" in the -Wl,--wrap= list of scripts/build/flags.ts. __wrap_mknod, __wrap_fstatat, the __xmknod and __fxstatat declarations with their BUN_SYMVER pins, and _MKNOD_VER.
    • git grep -w finds no other mknod. The only other fstatat is bun_sys::fstatat, which is a raw syscall on Linux and Android (src/sys/lib.rs).
    • llvm-nm -u over the 1,489 link inputs of the debug build finds no undefined mknod or fstatat. Rust std still refers to fstatat64, so that wrap stays.
    • objdump -T build/debug/bun-debug on this branch shows __fxstat, __fxstat64 and __fxstatat64, and no fstatat, __fxstatat, mknod or __xmknod.
    • The canary bun-profile of main 11c3671, for linux x64 and for linux aarch64, has no __wrap_mknod and no __wrap_fstatat symbol, and imports neither __xmknod nor __fxstatat.
  • thunk = "name" of #[uws_callback] (src/jsc_macros/lib.rs). The one use, on Subprocess::handle_abort_signal, went in Bun.ModuleGraph: a context per graph for timers and I/O, per-graph CommonJS #42590.
  • The root re-exports bun_tar::Reader and bun_archive::Plucker. Every user writes reader::Reader and archiver::Plucker. Plucker was unused before Replace libarchive with a tar reader and writer in safe Rust #46638 too. Both items stay pub in their modules. cargo mordant does not report an unused pub use path, so these two are a hand-found sample and not the whole class.
  • NativePtr.onClose and NativePtr.onDrain (src/js/internal/streams/native-readable.ts). Nothing reads or writes them.

Open pull requests

Found dead in this run, and left alone

How the candidates were found

Rejected from the self-review

  • No pull request for this run. The two wraps are new since Replace libarchive with a tar reader and writer in safe Rust #46638 and no other pull request removes them.
  • A wider cut into BunBuiltinNames.h and builtins.d.ts. Open pull requests hold those lines.
  • A lint for names in BunBuiltinNames.h that nothing reads, and a refile of the open sweeps that conflict with main. Each is a separate change.

Tests, debug build of this branch

  • cargo clippy -p bun_jsc_macros -p bun_tar -p bun_archive --no-deps: no finding.
  • test/js/bun/archive.test.ts, test/js/node/stream/node-stream.test.js, test/js/node/http/node-http-server-abort-events.test.ts: all pass.
  • test/js/bun/http/bun-serve-ssl.test.ts: 39 of 41 pass in the full file. The other two reach their 5 s limit on this machine, and pass when they run alone.
  • test/js/bun/symbols.test.ts fails on the local debug build for seven symbols that this change does not touch (hypot, hypotf, arc4random, pidfd_spawnp, pidfd_getpid, __isoc23_strtoul, __isoc23_strtoull). Its list has no mknod or fstatat.

…export lists

- `--wrap=mknod` and `--wrap=fstatat`, with `__wrap_mknod`, `__wrap_fstatat`
  and their version pins. libarchive was the one caller of both symbols, and
  #46638 removed libarchive.
- The `thunk = "name"` option of `#[uws_callback]`. #42590 removed its one use.
- The root re-exports `bun_tar::Reader` and `bun_archive::Plucker`. Every user
  names the defining module.
- The `onClose` and `onDrain` members of the `NativePtr` type.
@robobun

robobun commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator Author

@robobun

robobun commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: ready for review.

  • Scope: 28 lines removed in 6 files. No behaviour change.
  • How the two glibc wraps were checked: llvm-nm -u over the link inputs of the debug build finds no undefined mknod or fstatat. objdump -T of the debug binary of this branch, and of the canary bun-profile for linux x64 and linux aarch64 (main 11c3671), shows no mknod, __xmknod, fstatat or __fxstatat import.
  • Checks on this head: bun bd, bun run rust:check-all (12 targets), cargo check --workspace --all-targets.
  • CI, build 134341: 197 jobs pass. test/js/bun/symbols.test.ts, the glibc 2.17 check, passes on the release binaries for linux x64 and linux aarch64.
  • The red jobs are the five test-aot-drawn jobs, which are allowed to fail. The same tests (test/aot/perf/generated-code-of-idioms, test/aot/minifying-first-saves-no-code, test/aot/programs-that-ask-the-shell) are red on main in builds 134067 and 134092. This diff does not touch the AOT compiler.
  • The four macOS test jobs are in the queue since 07:42 UTC on 2026-10-28. At 11:10 UTC no agent had started them.

@coderabbitai

coderabbitai Bot commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 11c3671 and 70fa650.

Files selected for processing: 6
  • scripts/build/flags.ts
  • src/archive/lib.rs
  • src/js/internal/streams/native-readable.ts
  • src/jsc/bindings/workaround-missing-symbols.cpp
  • src/jsc_macros/lib.rs
  • src/tar/lib.rs
Files with no reviewable changes: 2
  • src/js/internal/streams/native-readable.ts
  • scripts/build/flags.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

Walkthrough

The change removes selected Linux linker wrappers, two crate-root re-exports, two internal NativePtr callback declarations, and the uws_callback thunk-name override.

Changes

Linux glibc wrappers

Layer / File(s) Summary
Update GNU Linux wrapper bindings
scripts/build/flags.ts, src/jsc/bindings/workaround-missing-symbols.cpp
The wrapper list and stat compatibility block remove fstatat and mknod wrappers. The fstatat64 wrapper remains.

uws_callback thunk parsing

Layer / File(s) Summary
Fix thunk-name generation
src/jsc_macros/lib.rs
The macro documentation and parser remove the thunk override. Generated thunk identifiers use the __<method>_c form.

Archive crate exports

Layer / File(s) Summary
Update archive crate exports
src/archive/lib.rs
The crate root continues to re-export Context and ExtractOptions but no longer re-exports Plucker.

NativePtr callbacks

Layer / File(s) Summary
Remove NativePtr callback declarations
src/js/internal/streams/native-readable.ts
The internal NativePtr interface no longer declares the onClose and onDrain callbacks.

Tar crate exports

Layer / File(s) Summary
Update tar crate exports
src/tar/lib.rs
The crate root continues to re-export Charset, Dialect, Entry, Event, and Options but no longer re-exports Reader.

Suggested reviewers: dylan-conway

Priority: Low

Merge Risk: Minimal · up to 70fa6

The reviewed dead-code removals have no established merge-blocking impact.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly summarizes the main changes: removal of dead glibc wrappers, the unused #[uws_callback] option, and two re-export lists.
Description check Passed The description explains the problem, fix, scope, behavior impact, verification steps, test results, and known unrelated failures. It does not use the exact template headings, but it provides the requ…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description explains the problem, fix, scope, behavior impact, verification steps, test results, and known unrelated failures. It does not use the exact template headings, but it provides the required information in equivalent sections.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — a pure dead-code deletion; each removed item has no remaining reference in the tree.

What was reviewed:

  • The fstatat/mknod wrap removal: no C/C++ source calls either; the only libc::fstatat calls in src/sys/lib.rs are #[cfg(not(linux))], and the Linux path goes through rustix's linux_raw backend (no use-libc feature), so nothing links against glibc's 2.33 fstatat. The fstatat64 wrap that Rust std needs stays.
  • thunk = on #[uws_callback]: no use anywhere in Rust sources, so dropping the parser arm cannot break an in-tree caller.
  • bun_tar::Reader / bun_archive::Plucker: every consumer imports via reader::/archiver::; no crate-root path or glob import exists.
  • NativePtr.onClose/onDrain: no read or write in native-readable.ts; no stale symbol lists in verify-binary.ts or elsewhere name the deleted thunks.
Extended reasoning...

The change is a single commit deleting 28 lines across six files: two glibc --wrap entries plus their C++ thunks and extern/symver declarations, one unused attribute option of a proc macro, two crate-root re-exports, and two unused TypeScript interface members. It touches no security-sensitive surface; the only correctness exposure is the Linux glibc symbol-version floor, which I verified is unaffected by grepping all C/C++ and Rust callers and confirming the remaining libc::fstatat calls are cfg-gated off Linux, with CI's verify-binary.ts ceiling scan as a backstop. No CODEOWNERS entry covers the changed files, there are no third-party reviews or objections, and the bug hunt ran dry, so approval is warranted.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants