APP-5495: Force GNU build-ids on Linux binaries so daemon pprof dumps can be symbolicated - #15265
Draft
warp-agent-staging[bot] wants to merge 2 commits into
Draft
APP-5495: Force GNU build-ids on Linux binaries so daemon pprof dumps can be symbolicated#15265warp-agent-staging[bot] wants to merge 2 commits into
warp-agent-staging[bot] wants to merge 2 commits into
Conversation
…gs (APP-5495) Root cause: the daemon/CLI binary is cross-compiled for x86_64-unknown-linux-musl with the vendored musl-cross-make toolchain (script/linux/configure_musl_toolchain), whose linker does not enable a GNU build-id note by default (unlike the distro-patched linkers used for the main client's native builds). Without a build-id, jemalloc_pprof heap profiles captured from the daemon have no way to join their mapping table to a debug-info file offline. - Force -Wl,--build-id=sha1 for every Linux link target in .cargo/config.toml, so all Linux binaries we ship carry a build-id. - Log a warning from dump_jemalloc_pprof_bytes() when a captured pprof's mapping table has entries with no build-id, so a future degenerate dump is visible in logs instead of only discoverable by decoding an attachment. - Add unit tests for the minimal protobuf walker backing that check. Co-Authored-By: Warp <agent@warp.dev>
Contributor
Author
|
This PR was generated with Warp. Comment |
Review finding: the previous unit tests validated the pprof mapping walker only against wire data produced by a hand-rolled encoder defined in the same test file, so a shared wrong assumption between encoder and parser could pass all tests while the production diagnostic was still wrong. Replace those fixtures with real jemalloc_pprof 0.8.2 dumps captured from the standalone repro used to establish this PR's root cause: one built natively (has a build-id) and one built with the repo's vendored musl-cross-make toolchain without --build-id (reproducing the daemon's pre-fix degenerate mapping table), plus a truncated fixture for the error path. Also exercise the gzip-decoding entry point (ungzip_and_count_pprof_mappings_missing_build_id) end to end, not just the inner parser. Co-Authored-By: Warp <agent@warp.dev>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Description
Fixes the memory-triage blocker for the Linux remote-server-daemon: its jemalloc heap-profile dumps had a degenerate mapping table (no address range, no GNU build-id), making them impossible to symbolicate offline. See Sentry issue 7259255054, event
2623254a94e8432c860a6e8a0a3000db, and Linear APP-5495.Mechanism (established empirically, not by inference)
Two independent things are going on, and only one of them is actually broken:
memory_start=0,memory_limit=u64::MAX,file_offset=0on everyMappingrecord is intentional upstream behavior, not a bug, and not daemon-specific.jemalloc_pprof/pprof_util0.8.x deliberately zero these fields for every mapping on every platform (see polarsignals/rust-jemalloc-pprof#26, "machine-independent-addrs") — sample addresses are pre-resolved to file-relative offsets before the proto is written, so the address range is redundant by design. I confirmed this by writing a standalone repro that linksjemalloc_pprof 0.8.2(the version this repo actually resolves), dumps a real pprof, and decodes theMappingtable directly: every mapping — main binary and shared libs alike, on a normal native build — hasmemory_start=0/memory_limit=u64::MAX/file_offset=0. This is consistent with APP-5350: the main client's mappings look "fine" only because theirbuild_idis non-empty, not because their address range is real.build_idis empty because it's missing a GNU build-id note, which is a real, daemon-specific bug. The daemon (built as theoz/CLI artifact, viascript/linux/bundle --artifact cli) is cross-compiled forx86_64-unknown-linux-muslusing the vendored musl-cross-make toolchain fromscript/linux/configure_musl_toolchain(cross-tools/musl-cross, tag20250929, GNU ld 2.45). That linker's default is--build-id=none, unlike the Debian/Ubuntu-patchedldused for the main client's native Linux builds (which defaults build-id on). I confirmed this three ways:readelf -nshows only.note.gnu.property); adding-Wl,--build-id=sha1produces one.jemalloc_pprof 0.8.2, same musl-cross toolchain, no extra flags) that linkstikv-jemallocatorand dumps a real pprof. The resultingMappingtable has 4 entries, all with the same filename (the daemon binary itself — as a static musl binary,dl_iterate_phdrsees only itself, and each of its 4PT_LOADsegments becomes oneMapping), all with emptybuild_id— this exactly reproduces the Sentry event's reported pattern. Two of these captured dumps (with and without a build-id) are committed as this PR's test fixtures.-Wl,--build-id=sha1added produces a real, non-empty build-id in everyMappingentry, with the rest of the shape (stillmemory_start=0/memory_limit=u64::MAX) unchanged, confirming the build-id is the only thing that was actually broken.x86_64-unknown-linux-gnu, this repo's actual.cargo/config.toml) already emits a build-id by default, both via a rawccinvocation and via a realcargo testbuild of thewarpcrate.Fix
Added
-Wl,--build-id=sha1for every Linux link target in.cargo/config.toml([target.'cfg(target_os = "linux")']), matching the existing macOS-specificrustflagsentry there. This is the cheapest option from the ticket's suggested list and directly addresses the actual mechanism: it guarantees a build-id note on every Linux binary we ship, including the daemon/CLI, regardless of which linker built it. I did not attempt to "fix" the zeroed address ranges — that's deliberate upstream behavior, and reintroducing real (ASLR-dependent, non-reproducible) addresses into a format that no longer needs them would be a regression, not a fix.Also added a warning (
app/src/profiling.rs::warn_if_pprof_missing_build_ids) so a future degenerate dump — from this path or any other cause — shows up in logs (log::warn!) instead of only being discoverable by decoding an individual Sentry attachment by hand. It walks just enough of the pprofProfileprotobuf to check eachMapping'sbuild_idfield, without a full protobuf/pprof decoder dependency.Changes
.cargo/config.toml: force-Wl,--build-id=sha1on all Linux link targets.app/src/profiling.rs:dump_jemalloc_pprof_bytes()now inspects the dump it produces and logs a warning if anyMappingentries have no build-id; adds the minimal protobuf-walking helpers backing that check.app/src/profiling_tests.rs+app/src/profiling_fixture_*: unit tests for the protobuf walker, backed by realjemalloc_pprof 0.8.2pprof dumps captured from the standalone repro (one with a populated build-id, one without, plus a truncated fixture for the error path) rather than a hand-rolled protobuf encoder, so the tests exercise the actual wire format instead of only the test's own assumptions about it. Both the low-level parser and the gzip-decoding entry point are covered.Verification
Verified:
jemalloc_pprof 0.8.2version this workspace resolves and the exact vendored musl-cross-make toolchain) reproduces the reported degenerate profile shape byte-for-byte (4 identical-filename mappings,memory_start=0/memory_limit=u64::MAX/file_offset=0, emptybuild_id), and confirms-Wl,--build-id=sha1fixes the build-id. Two of its captured dumps are committed as this PR's test fixtures.cargo test -p warp profiling::(default features) andcargo test -p warp --features jemalloc_pprof,heap_usage_tracking profiling::— all 6 unit tests pass in both cases.cargo clippy -p warp --all-targets --tests -- -D warnings(the exact invocationscript/presubmituses for this crate) passes cleanly../script/formatrun; no unrelated files touched.readelf -nthat a realcargo testbuild of thewarpcrate for the nativex86_64-unknown-linux-gnutarget still carries a build-id note after the.cargo/config.tomlchange (no regression there — it already had one).Not verified — one check a human should run before merging: I did not produce the release-equivalent end-to-end proof — building the actual shipped daemon/CLI musl binary and inspecting it directly. A from-scratch build of the full
warpcrate forx86_64-unknown-linux-muslwas not feasible in this environment (no cached build artifacts for that target, a very large dependency graph, and a real risk of the build being killed under memory pressure). Before merging, please run:(adjust the binary path/channel flag for whichever channel you build;
script/linux/bundle --helplists the-c/--channeland profile options.)A passing result shows a
Displaying notes found in: .note.gnu.build-idsection with a non-emptyBuild ID: ...line in thereadelf -noutput — mirroring what this PR's standalone repro already demonstrated with the identical toolchain and linker flag.Linked Issue
Testing
./script/run(Not applicable — this is a headless profiling/build-config fix with no UI surface. See Verification above for what was actually run, and for the one remaining check a human should run before merging.)
Agent Mode
CHANGELOG-NONE