build: make the declared -g1 debug-info floor effective, drop -gdwarf-4 pin - #7542
build: make the declared -g1 debug-info floor effective, drop -gdwarf-4 pin#7542PastaPastaPasta wants to merge 4 commits into
Conversation
Potential PR merge conflictsThis is advisory only. It does not block CI, but it marks PRs that will likely need a rebase depending on merge order. If this PR merges firstThese open PRs will likely need a rebase: |
|
🔍 Review in progress — actively reviewing now (commit 74fcc11) |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe build adds the Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
32dea36 to
4f50eb1
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f50eb1467
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export RUN_FUNCTIONAL_TESTS=false | ||
| export GOAL="install" | ||
| export BITCOIN_CONFIG="--enable-reduce-exports" | ||
| export BITCOIN_CONFIG="--enable-reduce-exports CFLAGS='-g1 -O2' CXXFLAGS='-g1 -O2'" |
There was a problem hiding this comment.
In the checked .github/workflows/build.yml pipeline, the standard linux64 source job is mapped by ci/dash/matrix.sh:21-22 to 00_setup_env_native_qt5.sh, but that file remains unchanged and its BITCOIN_CONFIG still lets Autoconf supply full -g -O2 debug information. Consequently, the primary linux64 ccache continues storing the large DWARF objects this commit is intended to eliminate; apply the same CFLAGS/CXXFLAGS override to the native Qt5 environment.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Outdated — this reviewed a superseded version of the branch. The current shape adds no per-job env flags at all: with the flag-ordering fix and the -gdwarf-4 removal, linux64 (and every job without an explicit -g) gets -g1 from configure's own floor. The premise also doesn't hold in CI: depends' config.site always sets CXXFLAGS (host -O2), so autoconf's -g -O2 default never applies to these jobs.
🤖 Posted autonomously by Claude on behalf of pasta.
BACKTRACE_FLAGS carries -gdwarf-4, which pins the DWARF version for libbacktrace but, being a member of the -g option family, also implies debug level 2. Placed last in AM_CFLAGS/AM_CXXFLAGS it silently overrode every debug level requested earlier on the command line: the '-g1 -fno-omit-frame-pointer' floor that configure.ac declares for crash-info stacktraces ('We always enable at least -g1') was escalated to full -g2 in every build, and --enable-debug's -g3 was clamped down to -g2. Measured on src/validation.cpp with gcc and clang: '-g1 ... -gdwarf-4 -O2' produces a 9.7 MB object, '-gdwarf-4 ... -g1 -O2' produces 2.5 MB.
Move BACKTRACE_FLAGS to the front so the DWARF version is pinned first and the debug level is decided by what follows: DEBUG_*FLAGS' -g1 floor, --enable-debug's -g3, or the user's own CFLAGS/CXXFLAGS, which automake always places last. Builds that pass an explicit -g (e.g. Guix releases with '-O2 -g') are unchanged. The main effect is that builds without a -g in their flags now get the documented -g1 instead of an accidental -g2, cutting roughly 90% of DWARF from ~1000 objects per CI target.
The pin was added when libbacktrace could not parse DWARF 5 and modern compilers had started emitting it by default. The libbacktrace pinned in depends (b9e40069, 2025-11-06) has had DWARF 5 support since 2021 and is what dashd statically links, so runtime crash symbolization always uses a DWARF 5 capable reader. All CI and release compilers (gcc 11+, clang 14+) default to DWARF 5. Dropping the pin lets the compiler use its default debug format and shrinks debug info a further ~10-25% on top of the -g1 level fix (measured at -g1 -O2: g++ 67.3 KB to 61.5 KB, clang++ 39.3 KB to 30.0 KB for a representative object). The valgrind CI jobs keep their own explicit -gdwarf-4 in their environment files, which is a separate valgrind-compatibility concern.
4f50eb1 to
9f85e36
Compare
PastaPastaPasta
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f85e36583
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # (DEBUG_*FLAGS' -g1 floor, --enable-debug's -g3, or the user's CXXFLAGS) must | ||
| # come later to take effect. | ||
| AM_CFLAGS = $(BACKTRACE_FLAGS) $(DEBUG_CFLAGS) | ||
| AM_CXXFLAGS = $(BACKTRACE_FLAGS) $(DEBUG_CXXFLAGS) $(HARDENED_CXXFLAGS) $(WARN_CXXFLAGS) $(NOWARN_CXXFLAGS) $(ERROR_CXXFLAGS) $(SANITIZER_CXXFLAGS) $(CORE_CXXFLAGS) |
There was a problem hiding this comment.
Checked .github/workflows/build.yml through ci/dash/matrix.sh: Automake appends user CXXFLAGS after AM_CXXFLAGS, so this reordering cannot override Autoconf's default -g -O2. The aarch64, fuzz, nowallet, sqlite, and UBSan environment scripts still set neither CFLAGS nor CXXFLAGS, leaving those source jobs with full -g2 DWARF; TSan's C objects also remain at -g2 because its config only overrides CXXFLAGS. Add explicit -g1 flags to these target configurations, apart from the intentionally unchanged linux64 canary.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The premise is incorrect for CI builds: autoconf's -g -O2 default only fires when CXXFLAGS is unset, and depends' config.site unconditionally sets CXXFLAGS (prepending the host -O2) for every CI job. With no -g in user flags, the floor's -g1 is the last debug-level flag on the line and wins — verified empirically in fork run 30873992664's per-target ccache stats. The TSan C-objects point is moot on the current branch: tsan's explicit CXXFLAGS was removed entirely, so both C and C++ objects take the floor.
🤖 Posted autonomously by Claude on behalf of pasta.
c6bd54c to
9f85e36
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6bd54cc9d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # (DEBUG_*FLAGS' -g1 floor, --enable-debug's -g3, or the user's CXXFLAGS) must | ||
| # come later to take effect. | ||
| AM_CFLAGS = $(BACKTRACE_FLAGS) $(DEBUG_CFLAGS) | ||
| AM_CXXFLAGS = $(BACKTRACE_FLAGS) $(DEBUG_CXXFLAGS) $(HARDENED_CXXFLAGS) $(WARN_CXXFLAGS) $(NOWARN_CXXFLAGS) $(ERROR_CXXFLAGS) $(SANITIZER_CXXFLAGS) $(CORE_CXXFLAGS) |
There was a problem hiding this comment.
For Guix macOS releases, contrib/guix/libexec/build.sh:225 explicitly unsets HOST_CFLAGS before deriving HOST_CXXFLAGS, while depends/hosts/darwin.mk:83-84 contributes only -O2. Previously the later -gdwarf-4 implicitly selected debug level 2, but after this change the final debug-level flag is DEBUG_CXXFLAGS's -g1. Consequently, make osx_debug packages dSYMs without the variable and type information expected from release debug artifacts; explicitly retain -g/-g2 for the Darwin Guix release path while allowing CI builds to use -g1.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid catch — fixed in 74fcc11. The guix darwin case now requests -g explicitly instead of relying on the removed -gdwarf-4 escalation, so make osx_debug dSYMs keep full variable/type DWARF, matching the linux/mingw releases' explicit -O2 -g.
🤖 Posted autonomously by Claude on behalf of pasta.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@configure.ac`:
- Around line 386-397: Update the GCC and GXX debug-flag setup around
DEBUG_CFLAGS and DEBUG_CXXFLAGS to use CPPFLAGS_overridden and
CXXFLAGS_overridden: when the corresponding flags were not explicitly supplied,
remove Autoconf’s default debug option before applying -g$with_debug_info, while
preserving user-provided flags and optimization settings. Ensure
--with-debug-info=0 disables debug information rather than allowing a default -g
to remain.
In `@doc/developer-notes.md`:
- Line 356: Update the debugging-build note to document that --enable-debug
attempts -O0 -g3 first and falls back to -g when -g3 is unavailable, while
retaining the mention of the associated debug macros.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c1163958-de1e-4d69-875a-3bd297e6b2aa
📒 Files selected for processing (4)
ci/test/00_setup_env_native_tsan.shconfigure.acdoc/developer-notes.mdsrc/Makefile.am
🚧 Files skipped from review as they are similar to previous changes (1)
- ci/test/00_setup_env_native_tsan.sh
The depends config.site prepends the host release flags (-O2) to user CXXFLAGS, so this job has always built at -O2 and the trailing '-g' only escalated debug info to level 2 -- at ~540 MB compressed the largest ccache entry of any target. ThreadSanitizer reports only consume function names and line tables. With the -g1 floor now effective, removing the override lets tsan pick up the same '-O2 -g1' as every other job instead of remaining a special case. A fork CI run of the full tsan test suite at debug level 1 with -O2 passed (run 30867089833).
9f85e36 to
11b1815
Compare
The darwin case unsets HOST_CFLAGS, so the mac release build's debug level was never requested explicitly: it came from the main build's since-removed -gdwarf-4 backtrace flag escalating configure's -g1 floor to -g2. Without it the shipped dSYM debug artifacts (make osx_debug) would silently drop variable and type DWARF while linux and mingw releases keep theirs via their explicit '-O2 -g'. Request -g explicitly for darwin instead. Optimization (-O2) and target flags still come from depends' config.site, whose flags are prepended to user CFLAGS/CXXFLAGS, so the user-supplied -g lands last and selects level 2 exactly as before.
2 participants
Issue being fixed or feature implemented
CI ccache entries are enormous — ~490 MB compressed per target, ~3.3 GB per develop push — which permanently fills the repository's 10 GB Actions cache quota and causes entries to be evicted within hours of being saved (run 30848679596 found no usable ccache for 4 of 9 targets and spent 27–37 min recompiling each). Since June 2026
pull_request_targetruns have read-only cache tokens, so PR CI inherits every one of those misses.The root cause is a flag-interaction bug, not a missing setting.
configure.acalready declares the right debug level — "We always enable at least -g1 debug info to support proper stacktraces in crash infos" — but it never takes effect:BACKTRACE_FLAGScarries-gdwarf-4, added when libbacktrace could not parse DWARF 5. Being a-g-family flag, it also implies debug level 2.src/Makefile.amplaced$(BACKTRACE_FLAGS)after$(DEBUG_CXXFLAGS), so the-gdwarf-4silently escalated the declared-g1floor to full-g2in every build — and clamped--enable-debug's-g3down to-g2as well.Full
-g2DWARF is ~90% of every object in this template-heavy codebase. Measured onsrc/validation.cpp(clang,-O2): 9.7 MB with today's effective flags, 2.5 MB at-g1, 0.65 MB with no debug info. Nothing in CI consumes more than function names and line tables — crash-hook backtraces and sanitizer reports are fully served by-g1(verified: reports keep fullfunction file:lineframes).What was done?
Three commits:
build: emit BACKTRACE_FLAGS before debug-level flags— reorderAM_CFLAGS/AM_CXXFLAGSso the DWARF-version flag comes first and the debug level is decided by what follows: the-g1floor,--enable-debug's-g3, or the user's own flags (which automake always places last). Kept as defense-in-depth even after commit 2: any future-g-family flag inBACKTRACE_FLAGSwould otherwise reintroduce the bug.build: drop the -gdwarf-4 pin from stacktrace flags— the pin's reason is gone: the libbacktrace pinned in depends (b9e40069, 2025-11-06) has had DWARF 5 support since 2021 and is whatdashdstatically links, and every CI/release compiler (gcc 11+, clang 14+) defaults to DWARF 5. Using the compiler default shrinks debug info a further ~10–25% at-g1(g++ 67→62 KB, clang++ 39→30 KB on a representative object). The valgrind CI jobs keep their own-gdwarf-4env pins — a separate valgrind-compatibility concern.ci: drop the tsan job's explicit CXXFLAGS— the dependsconfig.siteprepends the host release flags (-O2) to userCXXFLAGS, so this job has always built at-O2; its trailing'-g'only escalated debug info to level 2 (at ~540 MB compressed, the largest ccache entry of any target). Removing the override lets tsan pick up the same-O2+-g1floor as every other job instead of remaining a special case. A fork run of the full tsan suite at this debug level passed (30867089833); its timings also confirmed the debug level has no wall-clock effect on the tsan tests (unit 355s vs 349s, functional 39 min vs 37.5 min).guix: keep full debug info in darwin release dSYMs— the guix darwin case unsetsHOST_CFLAGS, so the mac release build's debug level was never requested explicitly: it came from the same-gdwarf-4escalation this PR removes, and the shipped dSYM debug artifacts (make osx_debug) would have silently dropped from-g2to-g1while linux/mingw releases keep-g2via their explicit-O2 -g. Request-gexplicitly for darwin instead (optimization and target flags still come from depends'config.site). Credit: flagged by Codex review.No other env-file changes are needed: with the floor effective, every job whose flags lack a
-g(linux64, sqlite, nowallet, ubsan, fuzz, aarch64, mac) lands on-g1automatically, and dev builds get the same documented behavior instead of an accidental-g2.Expected cache effect: ~490 MB → roughly 130–170 MB compressed per affected target, taking a develop push's ccache footprint from ~3.3 GB to about 1 GB, which stops the quota-eviction cycle.
How Has This Been Tested?
-g1flag shape (30867089833): every build and test job green, including the tsan, ubsan, and multiprocess functional suites. Cold-g1builds were consistently faster than cold-g2baselines on the same runner class (linux64 1318s vs 1629s, sqlite 1152s vs 1366s, ubsan 2038s vs 2223s).-g1 … -gdwarf-4⇒ 9.7 MB object;-gdwarf-4 … -g1⇒ 2.5 MB).-g1: UBSAN (print_stacktrace=1) and TSAN reports keep full function + file:line frames.b9e40069) linked into a multi-TU harness callingbacktrace_full()at-g1 -O2: symbolized output (function + file:line, including cross-TU frames) is byte-identical between-gdwarf-4and-gdwarf-5builds on Linux (g++ 13 and clang++ 18, binary versions confirmed via readelf) and on native macOS (Apple clang 21 with dSYM, versions confirmed via dwarfdump: 0x0004 vs 0x0005).Breaking Changes
None for releases: linux and mingw pass an explicit
-O2 -gwhich still lands last, and darwin now requests-gexplicitly (commit 4), so all release binaries and dSYM artifacts keep full-g2(now DWARF 5, which all release tooling supports).Developer-facing note: builds against a depends prefix without explicit flags previously got an accidental
-g2and will now get the documented-g1. Developers who want full debug info passCXXFLAGS='-g -O2'(user flags always win) or use--enable-debug, which now correctly yields-g3instead of being clamped to-g2.Checklist: