-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix: detached debug symbols from guix #6971
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: detached debug symbols from guix #6971
Conversation
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
|
Guix Automation has failed due to the HEAD commit not being signed by an authorized core-team member. Please rebase and sign or push a new empty signed commit to allow Guix build to happen. |
e83b604 to
ef44418
Compare
|
Guix Automation has began to build this PR tagged as v23.0.0-devpr6971.ef444188. A new comment will be made when the image is pushed. |
- Added compression for DWARF sections in debug files. - Implemented creation of .build-id tree for automatic discovery by perf. - Added verification checks for build-ids and debug links in ELF files.
|
Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v23.0.0-devpr6971.ef444188. The image should be on dockerhub soon. |
|
Guix Automation has began to build this PR tagged as v23.0.0-devpr6971.38934351. A new comment will be made when the image is pushed. |
|
for the first commit; --enable-debug guix builds compared to latest nightly have only 80bytes bigger shipped binary, and 64k bigger dash-qt; Negligible. |
|
Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v23.0.0-devpr6971.38934351. The image should be on dockerhub soon. |
|
Guix Automation has began to build this PR tagged as v23.0.0-devpr6971.badabf97. A new comment will be made when the image is pushed. |
|
Second commit also appears to cause no increase in shipped binaries (or minimal like 80 bytes etc) |
|
Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v23.0.0-devpr6971.badabf97. The image should be on dockerhub soon. |
|
Guix Automation has began to build this PR tagged as v23.0.0-devpr6971.48446f9d. A new comment will be made when the image is pushed. |
|
Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v23.0.0-devpr6971.48446f9d. The image should be on dockerhub soon. |
|
Guix Automation has began to build this PR tagged as v23.0.0-devpr6971.857f860a. A new comment will be made when the image is pushed. |
|
Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v23.0.0-devpr6971.857f860a. The image should be on dockerhub soon. |
WalkthroughThis pull request adds Linux-specific build-id linking and refines debug symbol handling. configure.ac appends Sequence Diagram(s)sequenceDiagram
participant Builder as Builder (guix build)
participant Configure as configure.ac
participant Linker as Linker
participant Objcopy as objcopy
participant FS as Filesystem
participant Tar as Tarball packaging
Note over Configure,Linker: Linux link-time flag selection
Configure->>Linker: Add -Wl,--build-id=sha1 (when TARGET_OS=linux)
Linker-->>Builder: ELF binaries with Build ID
Builder->>Objcopy: Extract debug info -> produce .dbg
Objcopy->>Objcopy: --strip-debug --strip-unneeded (split-debug)
Objcopy->>Objcopy: Compress DWARF sections in .dbg
Objcopy-->>FS: .dbg files + stripped binaries
Builder->>Builder: Verify each ELF has Build ID and .gnu_debuglink
alt All verified
Builder->>FS: Create .build-id tree from .dbg files (perf discovery)
Builder->>Tar: Package main tarball (exclude .dbg + build-id tree)
Builder->>Tar: Package debug tarball (include .dbg + build-id tree)
else Any missing
Builder-->>Builder: Fail build with error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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. Comment |
Fix verification block to properly capture exit status and prevent build from continuing on failure. Change objcopy command to use && instead of semicolon to ensure chmod only runs if mv succeeds. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
contrib/guix/libexec/build.sh (1)
345-366: Minor: Silent failures in .build-id tree creation aren't caught by verification.Lines 350–366 create the .build-id tree for perf auto-discovery. If
mkdir -porcpfail on lines 360–361, the loop continues silently. The verification phase (lines 367–396) only checks that binaries have a.gnu_debuglinksection; it doesn't verify that the corresponding.debugfile was successfully placed in the.build-idtree.If the tree creation fails, perf still won't discover symbols, but the build succeeds. This is likely acceptable since the debug tarball can be extracted manually, but consider adding explicit error handling (e.g.,
cp ... || { echo "Error copying ..."; exit 1; }) if robustness is critical.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
contrib/guix/libexec/build.sh(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: kwvg
Repo: dashpay/dash PR: 6754
File: contrib/containers/guix/docker-compose.yml:18-19
Timestamp: 2025-07-14T10:11:05.011Z
Learning: In the Guix build process for Dash Core, the `guix.sigs` directory requires write access as signatures are written to it during the build process, and `dash-detached-sigs` may be updated with `git pull` operations, so both directories need rw permissions in the Docker volume mounts.
🔇 Additional comments (3)
contrib/guix/libexec/build.sh (3)
368-396: Verification block now correctly captures exit status.The previous review flagged that the verification block ran in a subshell (due to the pipe) and the exit status wasn't properly propagated. This has been fixed: the exit status is now correctly captured via
verification_status=$?after the command substitution, and the script exits withexit 1if verification fails. Good work addressing the prior concern.
348-348: LGTM: Objcopy error handling is correct.The
&&chaining betweenobjcopy,mv, andchmodensures that if any step fails, subsequent steps don't run. The previous review concern about error handling has been addressed.
435-446: Tarball separation logic is sound.The main tarball (lines 436–440) correctly excludes
*.dbgfiles and the/usr/lib/debugtree. The debug tarball (lines 442–446) correctly includes both via the find predicate\( -name "*.dbg" -o -path "${DISTNAME}/usr/lib/debug/*" \). The split is clean and deterministic.
|
Guix Automation has began to build this PR tagged as v23.0.0-devpr6971.81cd2400. A new comment will be made when the image is pushed. |
|
Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v23.0.0-devpr6971.81cd2400. The image should be on dockerhub soon. |
UdjinM6
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK 81cd240
knst
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 81cd240
c359fb2 ci: bump to Clang 19 (Kittywhiskers Van Gogh) 9c9e876 partial bitcoin#33185: update time-machine to 5cb84f2013c5b1e48a7d0e617032266f1e6059e2 (Kittywhiskers Van Gogh) 7fef63d merge bitcoin#31529: latest 2.31 glibc (Kittywhiskers Van Gogh) de1b467 merge bitcoin#30730: Bump time machine to 53396a22afc04536ddf75d8f82ad2eafa5082725 (Kittywhiskers Van Gogh) Pull request description: ## Additional Information Spun-off from [dash#6998](#6988) to remedy CI failures. Potential alternative to [dash#6991](#6991) as Clang 19 does not seem to exhibit failures observed with Clang 18 ([comment](#6991 (comment))) caused by change in compile flags in [dash#6966](#6966). **Note:** ~~The base of this pull request does not use latest `develop` (86e84d7) due to build issues potentially related to [dash#6971](#6971) (see below) and uses a slightly out-of-date base to sidestep the issue. The build issue was found to be present independent of this PR using the Guix container.~~ Fixed with [dash#6997](#6997). <details> <summary>Build error:</summary> ``` make[2]: Nothing to be done for 'install-exec-am'. /home/ubuntu/.guix-profile/bin/mkdir -p '/distsrc-base/distsrc-23.0.0-rc.3-453-g86e84d701641-x86_64-linux-gnu/installed/dashcore-23.0.0-rc.3-453-g86e84d701641//lib/pkgconfig' /home/ubuntu/.guix-profile/bin/install -c -m 644 libdashconsensus.pc '/distsrc-base/distsrc-23.0.0-rc.3-453-g86e84d701641-x86_64-linux-gnu/installed/dashcore-23.0.0-rc.3-453-g86e84d701641//lib/pkgconfig' make[2]: Leaving directory '/distsrc-base/distsrc-23.0.0-rc.3-453-g86e84d701641-x86_64-linux-gnu' make[1]: Leaving directory '/distsrc-base/distsrc-23.0.0-rc.3-453-g86e84d701641-x86_64-linux-gnu' objcopy: Unable to recognise the format of the input file `dashcore-23.0.0-rc.3-453-g86e84d701641/bin/dash-tx.dbg' objcopy: Unable to recognise the format of the input file `dashcore-23.0.0-rc.3-453-g86e84d701641/lib/libdashconsensus.so.0.0.0.dbg' objcopy: Unable to recognise the format of the input file `dashcore-23.0.0-rc.3-453-g86e84d701641/bin/dash-cli.dbg' objcopy: Unable to recognise the format of the input file `dashcore-23.0.0-rc.3-453-g86e84d701641/bin/dash-wallet.dbg' objcopy: Unable to recognise the format of the input file `dashcore-23.0.0-rc.3-453-g86e84d701641/bin/dash-qt.dbg' objcopy: Unable to recognise the format of the input file `dashcore-23.0.0-rc.3-453-g86e84d701641/bin/test_dash.dbg' objcopy: Unable to recognise the format of the input file `dashcore-23.0.0-rc.3-453-g86e84d701641/bin/dashd.dbg' ``` </details> ## Breaking Changes None expected. ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [x] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation **(note: N/A)** - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK c359fb2 Tree-SHA512: 69576f6e6c0c101be434b273e9a84d1ab9f9341734241e794585615f194b453d2983dce077f9a2efa04b93bfba2374274aad0f90a1a07aed4ef87ff3f22ddd59
Issue being fixed or feature implemented
The current debug symbols shipped are not sufficient for using tools like perf
What was done?
See changes in commits; debug artifacts are now ~1.2GB per, much bigger sadly.
How Has This Been Tested?
Following process worked:
✅ Perf + Docker + External Debug Tarball: Full Symbolization Checklist
tar -xvf dashcore-*.debug.tar.gz
You should now have:
• dashd.dbg binaries
• /usr/lib/debug/.build-id/.../*.debug files
⸻
sudo readelf -n /proc/$(pidof dashd)/root/usr/local/bin/dashd | grep 'Build ID'
Example output:
Build ID: 4bec1dd76cd952657cbb3e8d3b9086bdfb651904
⸻
The .debug file must live at:
/usr/lib/debug/.build-id/4b/ec1dd76c...debug
So copy them:
sudo rsync -a dashcore-*/usr/lib/debug/.build-id/ /usr/lib/debug/.build-id/
⸻
Perf expects:
/usr/lib/debug/.build-id/4b/
/usr/lib/debug/.build-id/4b/.debug
Create the symlink if needed:
sudo ln -sf
/usr/lib/debug/.build-id/4b/.debug
/usr/lib/debug/.build-id/4b/
You already did this step.
⸻
Perf must see the exact same ELF that produced the build-id.
sudo mkdir -p /usr/local/bin
sudo cp /proc/$(pidof dashd)/root/usr/local/bin/dashd /usr/local/bin/dashd
⸻
sudo perf buildid-cache -a /usr/local/bin/dashd
⸻
Record:
sudo perf record -F 99 -g --call-graph dwarf -p "$(pidof dashd)" -- sleep 30
Report with source lines:
sudo perf report --stdio -g srcline
You should now see:
• full symbol names
• file paths
• line numbers
• normal call chains
⸻
🎯 Summary
Whenever doing perf profiling inside Docker using external debug symbols:
1. Extract debug tarball
2. Find running binary’s build-id
3. Place matching .debug file under /usr/lib/debug/.build-id/
4. Create build-id symlink
5. Copy the running ELF binary to the host at the same path
6. Add binary to perf’s build-id cache
7. Run perf again — you now get full symbols
Breaking Changes
Checklist:
Go over all the following points, and put an
xin all the boxes that apply.