build: update to ianlancetaylor/libbacktrace@b9e40069c0, make detection more robust, refactor and add platform-specific checks, add linker and compiler flags

[kwvg]

Additional Information

• ianlancetaylor/libbacktrace@b9e40069c0 includes fixes relevant to us, specifically 3fda5a87, d48f8403 as they relate to handling of Mach-O

• On BSDs, libexecinfo (source) is a distinct library (unlike Linux, where it is bundled as part of glibc) that needs to be obtained in order for stacktraces to work, we will now explicitly specify we want to link against this dependency when we configure the build.

• libbacktrace is known to have issues with DWARF 5 parsing (see ianlancetaylor/libbacktrace#158) and we are therefore adapting the fix implemented in ElementsProject/lightning#8431 to ensure that we emit DWARF 4 binaries that should work with libbacktrace.

• It appears that until this PR, some portions of our stacktrace code weren't being built on Windows which revealed some compile errors (build), they have been resolved in a separate commit.

• We need to make sure that we build with -export-dynamic (or -rdynamic on certain compilers, source) to ensure symbol data is preserved for generating backtraces.

Breaking Changes

None expected.

Checklist

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas (note: N/A)
• I have added or updated relevant unit/integration/functional/e2e tests (note: N/A)
• I have made corresponding changes to the documentation (note: N/A)
• I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

[github-actions]

⚠️ Potential Merge Conflicts Detected

This PR has potential conflicts with the following open PRs:

• backport: merge bitcoin#31100, #31626, #31840, #30584, #31982, #32086, #31998, #32505, #32568, #32690, #32731, #32716, #32837, #32266, #30095, #30137, #33580, partial bitcoin#30454 (build backports: part 4) #6919 - backport: merge bitcoin#31100, #31626, #31840, #30584, #31982, #32086, #31998, #32505, #32568, #32690, #32731, #32716, #32837, #32266, #30095, #30137, #33580, partial bitcoin#30454 (build backports: part 4)
• feat(wallet): external signer (hardware signer) #6019 - feat(wallet): external signer (hardware signer)

Please coordinate with the authors of these PRs to avoid merge conflicts.

[kwvg]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Walkthrough

The patch centralizes and OS-awares backtrace handling in configure.ac: it replaces the prior early stacktrace block with detection that sets BACKTRACE_FLAGS, BACKTRACE_LDFLAGS, and BACKTRACE_LIBS (and exports them via AC_SUBST), adds TARGET_OS=bsd detection, and restores AM_CONDITIONAL/AC_DEFINE paths for ENABLE_STACKTRACES and ENABLE_CRASH_HOOKS (plus CRASH_HOOKS_WRAPPED_CXX_ABI). configure output now shows BACKTRACE_FLAGS in CFLAGS/CXXFLAGS and BACKTRACE_LDFLAGS in LDFLAGS. depends/packages/backtrace.mk updates the backtrace package version and SHA256. Makefiles switch BACKTRACE_LIB -> BACKTRACE_LIBS. src/stacktraces.cpp changes Windows debug-path existence logic and removes SymInitialize from GetStackFrames.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor Configure as configure.ac
    participant Host as Host OS probe
    participant Header as header checks (backtrace/execinfo)
    participant LibProbe as library/flag detection (libbacktrace/libexecinfo/dbghelp)
    participant Subst as AC_SUBST / AM_CONDITIONAL
    participant Make as Makefiles (src/*)
    rect rgb(240,248,255)
      note left of Host: detect TARGET_OS (windows/darwin/bsd/other)
      Configure->>Host: determine OS
      Host-->>Configure: OS id
    end
    Configure->>Header: test for required headers
    Header-->>Configure: present / absent
    alt headers present
        Configure->>LibProbe: platform-specific probe & flags
        LibProbe-->>Configure: populate BACKTRACE_FLAGS / BACKTRACE_LDFLAGS / BACKTRACE_LIBS
    else headers missing or probe failed
        Configure->>Configure: disable ENABLE_STACKTRACES / emit warning
    end
    Configure->>Subst: export BACKTRACE_* and ENABLE_* conditionals
    Subst-->>Make: Makefiles consume $(BACKTRACE_FLAGS|LDFLAGS|LIBS)
    note right of Make: multiple src/Makefile.* updated to use $(BACKTRACE_LIBS)

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

• inspect configure.ac: OS branches (Windows dbghelp, Darwin DWARF/standalone-debug probing, BSD libexecinfo), correct flag/linker probe logic, and configure output changes;
• verify AC_SUBST / AM_CONDITIONAL and AC_DEFINE emissions for BACKTRACE_FLAGS, BACKTRACE_LDFLAGS, BACKTRACE_LIBS, ENABLE_STACKTRACES, ENABLE_CRASH_HOOKS, CRASH_HOOKS_WRAPPED_CXX_ABI;
• confirm Makefile edits across src/Makefile.am, src/Makefile.bench.include, src/Makefile.qt.include, src/Makefile.qttest.include, src/Makefile.test.include (no leftover BACKTRACE_LIB references, correct variable scope placement in CFLAGS/CXXFLAGS/LDFLAGS);
• check depends/packages/backtrace.mk version and sha256 change;
• review src/stacktraces.cpp Windows changes (absolute-path debug-file check and removal of SymInitialize) for functional impact.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main changes: updating libbacktrace, improving detection robustness, adding platform-specific checks, and introducing compiler/linker flags.
Description check	✅ Passed	The description comprehensively covers the PR objectives, including the libbacktrace update, BSD libexecinfo linking, DWARF 4 compatibility, Windows fixes, and symbol preservation requirements.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 40a9943 and 4504c49a0470fcd6426a70ab81b3a49b205f5723.

📒 Files selected for processing (7)

• configure.ac (4 hunks)
• depends/packages/backtrace.mk (1 hunks)
• src/Makefile.am (6 hunks)
• src/Makefile.bench.include (1 hunks)
• src/Makefile.qt.include (1 hunks)
• src/Makefile.qttest.include (1 hunks)
• src/Makefile.test.include (2 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

depends/**

📄 CodeRabbit inference engine (CLAUDE.md)

Unless specifically prompted, avoid making changes to the depends directory (dependency build system)

Files:

• depends/packages/backtrace.mk

🧠 Learnings (7)

📓 Common learnings

Learnt from: kwvg
Repo: dashpay/dash PR: 6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.

Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.cpp:247-250
Timestamp: 2025-07-29T14:32:48.369Z
Learning: In PR #6761, kwvg acknowledged a null pointer check issue in ChainLockSigner::Cleanup() method but deferred it to follow-up, consistent with the pattern of avoiding scope creep in refactoring PRs.

Learnt from: knst
Repo: dashpay/dash PR: 6692
File: src/llmq/blockprocessor.cpp:217-224
Timestamp: 2025-08-19T14:57:31.801Z
Learning: In PR #6692, knst acknowledged a null pointer dereference issue in ProcessBlock() method where LookupBlockIndex may return nullptr but is passed to gsl::not_null, and created follow-up PR #6789 to address it, consistent with avoiding scope creep in performance-focused PRs.

Learnt from: kwvg
Repo: dashpay/dash PR: 6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.

Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/wallet/wallet.cpp:0-0
Timestamp: 2025-07-29T14:33:01.040Z
Learning: In refactoring PRs like #6761, kwvg acknowledges code safety improvements (like null pointer checks and unused parameter warnings) but prefers to defer them to follow-up PRs to maintain focus on the primary refactoring objectives, avoiding scope creep.

Learnt from: knst
Repo: dashpay/dash PR: 6916
File: src/univalue/include/univalue.h:81-88
Timestamp: 2025-10-25T07:08:51.918Z
Learning: For backport PRs from bitcoin/bitcoin, bitcoin-core/gui, etc., backported changes should match the original upstream PRs even if they appear strange, modify vendored code, or seem to violate coding guidelines. Still flag genuine issues like bugs, undefined behavior, crashes, compilation errors, or linter failures.

Learnt from: knst
Repo: dashpay/dash PR: 6883
File: src/rpc/rawtransaction.cpp:1088-1125
Timestamp: 2025-10-13T12:37:12.357Z
Learning: In backport pull requests (especially from Bitcoin Core), treat "moved" or refactored code as out-of-scope for content-level review. Focus validation on verifying that code is moved correctly: no fields added, no fields removed, no fields reordered, and no unexpected changes beyond whitespace adjustments. Pre-existing issues in the upstream code should be preserved to maintain fidelity to the original implementation.

Learnt from: UdjinM6
Repo: dashpay/dash PR: 6786
File: ci/test/04_install.sh:99-101
Timestamp: 2025-08-01T07:46:37.840Z
Learning: In backport PRs like #6786, UdjinM6 prefers to defer non-critical fixes (such as shell command expansion issues) to separate commits/PRs to maintain focus on the primary backport objectives, consistent with the project's pattern of avoiding scope creep.

📚 Learning: 2025-07-20T18:42:49.794Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp} : Unit tests for C++ code should be placed in src/test/, src/wallet/test/, or src/qt/test/ and use Boost::Test or Qt 5 for GUI tests

Applied to files:

• src/Makefile.qttest.include

📚 Learning: 2025-08-11T17:16:36.654Z

Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6804
File: src/qt/proposalwizard.cpp:40-42
Timestamp: 2025-08-11T17:16:36.654Z
Learning: In the Dash repository, when a PR adds new files that are not from Bitcoin backports, these files must be added to the list in test/util/data/non-backported.txt. This applies to newly created files like qt/proposalwizard.{h,cpp} and forms/proposalwizard.ui. Limited exemptions may exist for subtrees and similar cases.

Applied to files:

• src/Makefile.qttest.include
• src/Makefile.qt.include
• src/Makefile.test.include
• src/Makefile.am

📚 Learning: 2025-07-23T09:30:34.631Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.h:5-6
Timestamp: 2025-07-23T09:30:34.631Z
Learning: Dash Core uses BITCOIN_ prefix for header guards as the standard convention, inherited from Bitcoin Core. Only a few BLS-specific files in src/bls/ use DASH_ prefix. The vast majority of files (385+) use BITCOIN_ prefix.

Applied to files:

• src/Makefile.qttest.include
• src/Makefile.qt.include
• src/Makefile.test.include
• src/Makefile.am

📚 Learning: 2025-07-20T18:42:49.794Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/bench/**/*.{cpp,h,cc,cxx,hpp} : Performance benchmarks should be placed in src/bench/ and use nanobench

Applied to files:

• src/Makefile.bench.include

📚 Learning: 2025-01-06T09:51:03.167Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6516
File: depends/patches/gmp/include_ldflags_in_configure.patch:557-621
Timestamp: 2025-01-06T09:51:03.167Z
Learning: The `GMP_GCC_ARM_UMODSI` macro checks only the compiler version, and `GMP_GCC_MIPS_O32` relies on the `-mabi=32` flag. Therefore, `$LDFLAGS` is irrelevant to these tests.

Applied to files:

• configure.ac
• src/Makefile.am

📚 Learning: 2025-10-05T20:38:28.457Z

Learnt from: knst
Repo: dashpay/dash PR: 6871
File: contrib/guix/libexec/build.sh:358-360
Timestamp: 2025-10-05T20:38:28.457Z
Learning: In the Dash repository, when backporting code from Bitcoin Core, typos and minor issues in comments should be kept as-is to reduce merge conflicts in future backports, even if they remain unfixed in Bitcoin Core's master branch.

Applied to files:

• src/Makefile.am

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: linux64_ubsan-build / Build source
• GitHub Check: linux64_nowallet-build / Build source
• GitHub Check: x86_64-w64-mingw32 / Build depends
• GitHub Check: x86_64-pc-linux-gnu_multiprocess / Build depends

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4504c49a0470fcd6426a70ab81b3a49b205f5723 and 7fc4ff927b50d60170d387c48297b0b578852a96.

📒 Files selected for processing (8)

• configure.ac (4 hunks)
• depends/packages/backtrace.mk (1 hunks)
• src/Makefile.am (6 hunks)
• src/Makefile.bench.include (1 hunks)
• src/Makefile.qt.include (1 hunks)
• src/Makefile.qttest.include (1 hunks)
• src/Makefile.test.include (2 hunks)
• src/stacktraces.cpp (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (3)

• src/Makefile.qt.include
• src/Makefile.am
• src/Makefile.bench.include

🧰 Additional context used

📓 Path-based instructions (2)

src/**/*.{cpp,h,cc,cxx,hpp}

📄 CodeRabbit inference engine (CLAUDE.md)

src/**/*.{cpp,h,cc,cxx,hpp}: Dash Core C++ codebase must be written in C++20 and require at least Clang 16 or GCC 11.1
Dash uses unordered_lru_cache for efficient caching with LRU eviction

Files:

• src/stacktraces.cpp

depends/**

📄 CodeRabbit inference engine (CLAUDE.md)

Unless specifically prompted, avoid making changes to the depends directory (dependency build system)

Files:

• depends/packages/backtrace.mk

🧠 Learnings (16)

📓 Common learnings

Learnt from: knst
Repo: dashpay/dash PR: 6692
File: src/llmq/blockprocessor.cpp:217-224
Timestamp: 2025-08-19T14:57:31.801Z
Learning: In PR #6692, knst acknowledged a null pointer dereference issue in ProcessBlock() method where LookupBlockIndex may return nullptr but is passed to gsl::not_null, and created follow-up PR #6789 to address it, consistent with avoiding scope creep in performance-focused PRs.

Learnt from: kwvg
Repo: dashpay/dash PR: 6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.

Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.cpp:247-250
Timestamp: 2025-07-29T14:32:48.369Z
Learning: In PR #6761, kwvg acknowledged a null pointer check issue in ChainLockSigner::Cleanup() method but deferred it to follow-up, consistent with the pattern of avoiding scope creep in refactoring PRs.

Learnt from: knst
Repo: dashpay/dash PR: 6916
File: src/univalue/include/univalue.h:81-88
Timestamp: 2025-10-25T07:08:51.918Z
Learning: For backport PRs from bitcoin/bitcoin, bitcoin-core/gui, etc., backported changes should match the original upstream PRs even if they appear strange, modify vendored code, or seem to violate coding guidelines. Still flag genuine issues like bugs, undefined behavior, crashes, compilation errors, or linter failures.

Learnt from: kwvg
Repo: dashpay/dash PR: 6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.

Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/wallet/wallet.cpp:0-0
Timestamp: 2025-07-29T14:33:01.040Z
Learning: In refactoring PRs like #6761, kwvg acknowledges code safety improvements (like null pointer checks and unused parameter warnings) but prefers to defer them to follow-up PRs to maintain focus on the primary refactoring objectives, avoiding scope creep.

Learnt from: UdjinM6
Repo: dashpay/dash PR: 6786
File: ci/test/04_install.sh:99-101
Timestamp: 2025-08-01T07:46:37.840Z
Learning: In backport PRs like #6786, UdjinM6 prefers to defer non-critical fixes (such as shell command expansion issues) to separate commits/PRs to maintain focus on the primary backport objectives, consistent with the project's pattern of avoiding scope creep.

📚 Learning: 2025-06-09T16:43:20.996Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.

Applied to files:

• configure.ac

📚 Learning: 2025-10-05T20:38:28.457Z

Learnt from: knst
Repo: dashpay/dash PR: 6871
File: contrib/guix/libexec/build.sh:358-360
Timestamp: 2025-10-05T20:38:28.457Z
Learning: In the Dash repository, when backporting code from Bitcoin Core, typos and minor issues in comments should be kept as-is to reduce merge conflicts in future backports, even if they remain unfixed in Bitcoin Core's master branch.

Applied to files:

• configure.ac

📚 Learning: 2025-08-01T07:46:37.840Z

Learnt from: UdjinM6
Repo: dashpay/dash PR: 6786
File: ci/test/04_install.sh:99-101
Timestamp: 2025-08-01T07:46:37.840Z
Learning: In backport PRs like #6786, UdjinM6 prefers to defer non-critical fixes (such as shell command expansion issues) to separate commits/PRs to maintain focus on the primary backport objectives, consistent with the project's pattern of avoiding scope creep.

Applied to files:

• configure.ac

📚 Learning: 2025-07-29T14:32:48.369Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.cpp:247-250
Timestamp: 2025-07-29T14:32:48.369Z
Learning: In PR #6761, kwvg acknowledged a null pointer check issue in ChainLockSigner::Cleanup() method but deferred it to follow-up, consistent with the pattern of avoiding scope creep in refactoring PRs.

Applied to files:

• configure.ac

📚 Learning: 2025-11-04T18:24:27.241Z

Learnt from: UdjinM6
Repo: dashpay/dash PR: 6933
File: src/llmq/utils.cpp:284-298
Timestamp: 2025-11-04T18:24:27.241Z
Learning: In consensus-critical code (such as quorum formation, block validation, or deployment activation logic), do not suggest changes to the logic itself even if the implementation appears theoretically incorrect or off-by-one. Consensus rules, once deployed on the Dash network, must be preserved exactly to avoid network forks. Refactoring PRs should maintain perfect behavioral equivalence. Only suggest logic changes if explicitly accompanied by a DIP (Dash Improvement Proposal) or if the maintainer indicates the consensus rule needs to be changed with appropriate activation logic.

Applied to files:

• configure.ac

📚 Learning: 2025-10-13T12:37:12.357Z

Learnt from: knst
Repo: dashpay/dash PR: 6883
File: src/rpc/rawtransaction.cpp:1088-1125
Timestamp: 2025-10-13T12:37:12.357Z
Learning: In backport pull requests (especially from Bitcoin Core), treat "moved" or refactored code as out-of-scope for content-level review. Focus validation on verifying that code is moved correctly: no fields added, no fields removed, no fields reordered, and no unexpected changes beyond whitespace adjustments. Pre-existing issues in the upstream code should be preserved to maintain fidelity to the original implementation.

Applied to files:

• configure.ac

📚 Learning: 2025-10-25T07:08:51.918Z

Learnt from: knst
Repo: dashpay/dash PR: 6916
File: src/univalue/include/univalue.h:81-88
Timestamp: 2025-10-25T07:08:51.918Z
Learning: For backport PRs from bitcoin/bitcoin, bitcoin-core/gui, etc., backported changes should match the original upstream PRs even if they appear strange, modify vendored code, or seem to violate coding guidelines. Still flag genuine issues like bugs, undefined behavior, crashes, compilation errors, or linter failures.

Applied to files:

• configure.ac

📚 Learning: 2025-07-29T14:33:01.040Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/wallet/wallet.cpp:0-0
Timestamp: 2025-07-29T14:33:01.040Z
Learning: In refactoring PRs like #6761, kwvg acknowledges code safety improvements (like null pointer checks and unused parameter warnings) but prefers to defer them to follow-up PRs to maintain focus on the primary refactoring objectives, avoiding scope creep.

Applied to files:

• configure.ac

📚 Learning: 2025-08-11T17:16:36.654Z

Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6804
File: src/qt/proposalwizard.cpp:40-42
Timestamp: 2025-08-11T17:16:36.654Z
Learning: In the Dash repository, when a PR adds new files that are not from Bitcoin backports, these files must be added to the list in test/util/data/non-backported.txt. This applies to newly created files like qt/proposalwizard.{h,cpp} and forms/proposalwizard.ui. Limited exemptions may exist for subtrees and similar cases.

Applied to files:

• configure.ac
• src/Makefile.qttest.include
• src/Makefile.test.include

📚 Learning: 2025-08-19T14:57:31.801Z

Learnt from: knst
Repo: dashpay/dash PR: 6692
File: src/llmq/blockprocessor.cpp:217-224
Timestamp: 2025-08-19T14:57:31.801Z
Learning: In PR #6692, knst acknowledged a null pointer dereference issue in ProcessBlock() method where LookupBlockIndex may return nullptr but is passed to gsl::not_null, and created follow-up PR #6789 to address it, consistent with avoiding scope creep in performance-focused PRs.

Applied to files:

• configure.ac

📚 Learning: 2025-07-23T09:28:32.783Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.cpp:15-250
Timestamp: 2025-07-23T09:28:32.783Z
Learning: In refactoring PRs like #6761, kwvg prefers to defer code formatting fixes to separate follow-up PRs when formatting is not the primary objective, to maintain focus on the structural changes and avoid scope creep.

Applied to files:

• configure.ac

📚 Learning: 2025-07-17T15:48:29.418Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6752
File: src/wallet/load.cpp:164-164
Timestamp: 2025-07-17T15:48:29.418Z
Learning: In Dash Core, kwvg prefers using assert() statements to document expected behavior in code paths, especially when the called function also has internal asserts. This is used as a defensive programming pattern rather than requiring conditional error handling.

Applied to files:

• configure.ac

📚 Learning: 2025-01-06T09:51:03.167Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6516
File: depends/patches/gmp/include_ldflags_in_configure.patch:557-621
Timestamp: 2025-01-06T09:51:03.167Z
Learning: The `GMP_GCC_ARM_UMODSI` macro checks only the compiler version, and `GMP_GCC_MIPS_O32` relies on the `-mabi=32` flag. Therefore, `$LDFLAGS` is irrelevant to these tests.

Applied to files:

• configure.ac

📚 Learning: 2025-07-20T18:42:49.794Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-07-20T18:42:49.794Z
Learning: Applies to src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp} : Unit tests for C++ code should be placed in src/test/, src/wallet/test/, or src/qt/test/ and use Boost::Test or Qt 5 for GUI tests

Applied to files:

• src/Makefile.qttest.include

📚 Learning: 2025-07-23T09:30:34.631Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.h:5-6
Timestamp: 2025-07-23T09:30:34.631Z
Learning: Dash Core uses BITCOIN_ prefix for header guards as the standard convention, inherited from Bitcoin Core. Only a few BLS-specific files in src/bls/ use DASH_ prefix. The vast majority of files (385+) use BITCOIN_ prefix.

Applied to files:

• src/Makefile.qttest.include
• src/Makefile.test.include

🧬 Code graph analysis (1)

src/stacktraces.cpp (1)

src/fs.h (2)

• absolute (81-84)
• PathFromString (173-180)

🪛 GitHub Actions: Clang Diff Format Check

src/stacktraces.cpp

[error] 131-131: Clang-format differences detected for src/stacktraces.cpp. The diff shows formatting changes; run the clang-format tool or the provided script to fix formatting issues. The step exited with code 1 due to differences.

🔇 Additional comments (8)

depends/packages/backtrace.mk (1)

2-6: LGTM! Dependency version and hash updated correctly.

The version and SHA256 hash updates align with the PR objective to update libbacktrace to commit b9e40069c0.

src/Makefile.qttest.include (1)

62-62: LGTM! Variable rename is consistent.

The change from $(BACKTRACE_LIB) to $(BACKTRACE_LIBS) aligns with the broader build system refactoring to use pluralized variable names for backtrace libraries.

src/Makefile.test.include (1)

63-63: LGTM! Consistent variable rename.

Both occurrences of the variable rename from $(BACKTRACE_LIB) to $(BACKTRACE_LIBS) are consistent with the project-wide build system updates.

Also applies to: 259-259

configure.ac (5)

948-950: LGTM! BSD target OS detection added.

The addition of the BSD case in the host detection switch correctly sets TARGET_OS=bsd for BSD systems, enabling platform-specific backtrace handling later in the configuration.

---

1227-1282: LGTM! Comprehensive and robust backtrace detection logic.

The detection logic is well-structured with proper OS-specific handling:

• Windows: Links dbghelp and backtrace libraries
• Darwin: Validates DWARF compatibility flags with proper fallback
• BSD: Locates libexecinfo with explicit error handling
• Others: Ensures symbol export flags are supported

The error handling correctly distinguishes between explicit --enable-stacktraces=yes (which errors out if requirements aren't met) and auto-detection (which warns and disables gracefully).

---

1284-1301: LGTM! Conditionals correctly evaluated after detection.

The AM_CONDITIONAL and AC_DEFINE calls are properly placed after all detection logic completes, ensuring they reflect the final state of enable_stacktraces and related variables. The crash hooks are also correctly disabled when stacktraces are unavailable.

---

2036-2038: LGTM! New backtrace variables properly exported.

The AC_SUBST declarations for BACKTRACE_FLAGS, BACKTRACE_LDFLAGS, and BACKTRACE_LIBS correctly export these variables for downstream Makefile consumption.

---

2133-2137: LGTM! Build configuration output includes backtrace flags.

The configure output messages correctly display the backtrace-related flags in CFLAGS, CXXFLAGS, and LDFLAGS, improving visibility of the build configuration.

[UdjinM6]

utACK 7fc4ff927b50d60170d387c48297b0b578852a96

[coderabbitai]

Actionable comments posted: 0

♻️ Duplicate comments (1)

src/stacktraces.cpp (1)

134-134: Fix persistent clang-format issue to unblock pipeline.

The pipeline continues to report clang-format differences on this line despite past attempts to address it. The line likely needs to be broken across multiple lines to comply with project formatting standards.

Apply formatting to resolve the pipeline failure:

-    static const char* exeFileNamePtr = fs::exists(fs::absolute(fs::PathFromString(debugFileName))) ? debugFileName.c_str() : g_exeFileName.c_str();
+    static const char* exeFileNamePtr =
+        fs::exists(fs::absolute(fs::PathFromString(debugFileName))) ? debugFileName.c_str()
+                                                                     : g_exeFileName.c_str();

🧹 Nitpick comments (1)

configure.ac (1)

1235-1283: Consider refactoring to avoid partial variable state.

The current implementation may leave BACKTRACE_LIBS and related variables partially filled if stacktraces are disabled by later checks. While the conditional usage in Makefiles prevents actual problems, it would be cleaner to use temporary variables during detection and only assign to BACKTRACE_* at the end if enable_stacktraces remains "yes".

This is a minor code quality suggestion and not a blocking issue.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7fc4ff927b50d60170d387c48297b0b578852a96 and 0d1d6d3.

📒 Files selected for processing (3)

• configure.ac (4 hunks)
• src/Makefile.am (6 hunks)
• src/stacktraces.cpp (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• src/Makefile.am

🧰 Additional context used

📓 Path-based instructions (1)

src/**/*.{cpp,h,cc,cxx,hpp}

📄 CodeRabbit inference engine (CLAUDE.md)

src/**/*.{cpp,h,cc,cxx,hpp}: Dash Core C++ codebase must be written in C++20 and require at least Clang 16 or GCC 11.1
Dash uses unordered_lru_cache for efficient caching with LRU eviction

Files:

• src/stacktraces.cpp

🧠 Learnings (14)

📓 Common learnings

Learnt from: knst
Repo: dashpay/dash PR: 6692
File: src/llmq/blockprocessor.cpp:217-224
Timestamp: 2025-08-19T14:57:31.801Z
Learning: In PR #6692, knst acknowledged a null pointer dereference issue in ProcessBlock() method where LookupBlockIndex may return nullptr but is passed to gsl::not_null, and created follow-up PR #6789 to address it, consistent with avoiding scope creep in performance-focused PRs.

Learnt from: kwvg
Repo: dashpay/dash PR: 6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.

Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.cpp:247-250
Timestamp: 2025-07-29T14:32:48.369Z
Learning: In PR #6761, kwvg acknowledged a null pointer check issue in ChainLockSigner::Cleanup() method but deferred it to follow-up, consistent with the pattern of avoiding scope creep in refactoring PRs.

Learnt from: kwvg
Repo: dashpay/dash PR: 6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.

Learnt from: knst
Repo: dashpay/dash PR: 6916
File: src/univalue/include/univalue.h:81-88
Timestamp: 2025-10-25T07:08:51.918Z
Learning: For backport PRs from bitcoin/bitcoin, bitcoin-core/gui, etc., backported changes should match the original upstream PRs even if they appear strange, modify vendored code, or seem to violate coding guidelines. Still flag genuine issues like bugs, undefined behavior, crashes, compilation errors, or linter failures.

Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/wallet/wallet.cpp:0-0
Timestamp: 2025-07-29T14:33:01.040Z
Learning: In refactoring PRs like #6761, kwvg acknowledges code safety improvements (like null pointer checks and unused parameter warnings) but prefers to defer them to follow-up PRs to maintain focus on the primary refactoring objectives, avoiding scope creep.

Learnt from: UdjinM6
Repo: dashpay/dash PR: 6786
File: ci/test/04_install.sh:99-101
Timestamp: 2025-08-01T07:46:37.840Z
Learning: In backport PRs like #6786, UdjinM6 prefers to defer non-critical fixes (such as shell command expansion issues) to separate commits/PRs to maintain focus on the primary backport objectives, consistent with the project's pattern of avoiding scope creep.

Learnt from: knst
Repo: dashpay/dash PR: 6883
File: src/rpc/rawtransaction.cpp:1088-1125
Timestamp: 2025-10-13T12:37:12.357Z
Learning: In backport pull requests (especially from Bitcoin Core), treat "moved" or refactored code as out-of-scope for content-level review. Focus validation on verifying that code is moved correctly: no fields added, no fields removed, no fields reordered, and no unexpected changes beyond whitespace adjustments. Pre-existing issues in the upstream code should be preserved to maintain fidelity to the original implementation.

📚 Learning: 2025-06-09T16:43:20.996Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.

Applied to files:

• configure.ac

📚 Learning: 2025-10-05T20:38:28.457Z

Learnt from: knst
Repo: dashpay/dash PR: 6871
File: contrib/guix/libexec/build.sh:358-360
Timestamp: 2025-10-05T20:38:28.457Z
Learning: In the Dash repository, when backporting code from Bitcoin Core, typos and minor issues in comments should be kept as-is to reduce merge conflicts in future backports, even if they remain unfixed in Bitcoin Core's master branch.

Applied to files:

• configure.ac

📚 Learning: 2025-08-01T07:46:37.840Z

Learnt from: UdjinM6
Repo: dashpay/dash PR: 6786
File: ci/test/04_install.sh:99-101
Timestamp: 2025-08-01T07:46:37.840Z
Learning: In backport PRs like #6786, UdjinM6 prefers to defer non-critical fixes (such as shell command expansion issues) to separate commits/PRs to maintain focus on the primary backport objectives, consistent with the project's pattern of avoiding scope creep.

Applied to files:

• configure.ac

📚 Learning: 2025-07-29T14:32:48.369Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.cpp:247-250
Timestamp: 2025-07-29T14:32:48.369Z
Learning: In PR #6761, kwvg acknowledged a null pointer check issue in ChainLockSigner::Cleanup() method but deferred it to follow-up, consistent with the pattern of avoiding scope creep in refactoring PRs.

Applied to files:

• configure.ac

📚 Learning: 2025-11-04T18:24:27.241Z

Learnt from: UdjinM6
Repo: dashpay/dash PR: 6933
File: src/llmq/utils.cpp:284-298
Timestamp: 2025-11-04T18:24:27.241Z
Learning: In consensus-critical code (such as quorum formation, block validation, or deployment activation logic), do not suggest changes to the logic itself even if the implementation appears theoretically incorrect or off-by-one. Consensus rules, once deployed on the Dash network, must be preserved exactly to avoid network forks. Refactoring PRs should maintain perfect behavioral equivalence. Only suggest logic changes if explicitly accompanied by a DIP (Dash Improvement Proposal) or if the maintainer indicates the consensus rule needs to be changed with appropriate activation logic.

Applied to files:

• configure.ac

📚 Learning: 2025-10-13T12:37:12.357Z

Learnt from: knst
Repo: dashpay/dash PR: 6883
File: src/rpc/rawtransaction.cpp:1088-1125
Timestamp: 2025-10-13T12:37:12.357Z
Learning: In backport pull requests (especially from Bitcoin Core), treat "moved" or refactored code as out-of-scope for content-level review. Focus validation on verifying that code is moved correctly: no fields added, no fields removed, no fields reordered, and no unexpected changes beyond whitespace adjustments. Pre-existing issues in the upstream code should be preserved to maintain fidelity to the original implementation.

Applied to files:

• configure.ac

📚 Learning: 2025-10-25T07:08:51.918Z

Learnt from: knst
Repo: dashpay/dash PR: 6916
File: src/univalue/include/univalue.h:81-88
Timestamp: 2025-10-25T07:08:51.918Z
Learning: For backport PRs from bitcoin/bitcoin, bitcoin-core/gui, etc., backported changes should match the original upstream PRs even if they appear strange, modify vendored code, or seem to violate coding guidelines. Still flag genuine issues like bugs, undefined behavior, crashes, compilation errors, or linter failures.

Applied to files:

• configure.ac

📚 Learning: 2025-07-29T14:33:01.040Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/wallet/wallet.cpp:0-0
Timestamp: 2025-07-29T14:33:01.040Z
Learning: In refactoring PRs like #6761, kwvg acknowledges code safety improvements (like null pointer checks and unused parameter warnings) but prefers to defer them to follow-up PRs to maintain focus on the primary refactoring objectives, avoiding scope creep.

Applied to files:

• configure.ac

📚 Learning: 2025-08-11T17:16:36.654Z

Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6804
File: src/qt/proposalwizard.cpp:40-42
Timestamp: 2025-08-11T17:16:36.654Z
Learning: In the Dash repository, when a PR adds new files that are not from Bitcoin backports, these files must be added to the list in test/util/data/non-backported.txt. This applies to newly created files like qt/proposalwizard.{h,cpp} and forms/proposalwizard.ui. Limited exemptions may exist for subtrees and similar cases.

Applied to files:

• configure.ac

📚 Learning: 2025-08-19T14:57:31.801Z

Learnt from: knst
Repo: dashpay/dash PR: 6692
File: src/llmq/blockprocessor.cpp:217-224
Timestamp: 2025-08-19T14:57:31.801Z
Learning: In PR #6692, knst acknowledged a null pointer dereference issue in ProcessBlock() method where LookupBlockIndex may return nullptr but is passed to gsl::not_null, and created follow-up PR #6789 to address it, consistent with avoiding scope creep in performance-focused PRs.

Applied to files:

• configure.ac

📚 Learning: 2025-07-23T09:28:32.783Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.cpp:15-250
Timestamp: 2025-07-23T09:28:32.783Z
Learning: In refactoring PRs like #6761, kwvg prefers to defer code formatting fixes to separate follow-up PRs when formatting is not the primary objective, to maintain focus on the structural changes and avoid scope creep.

Applied to files:

• configure.ac

📚 Learning: 2025-07-17T15:48:29.418Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6752
File: src/wallet/load.cpp:164-164
Timestamp: 2025-07-17T15:48:29.418Z
Learning: In Dash Core, kwvg prefers using assert() statements to document expected behavior in code paths, especially when the called function also has internal asserts. This is used as a defensive programming pattern rather than requiring conditional error handling.

Applied to files:

• configure.ac

📚 Learning: 2025-01-06T09:51:03.167Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6516
File: depends/patches/gmp/include_ldflags_in_configure.patch:557-621
Timestamp: 2025-01-06T09:51:03.167Z
Learning: The `GMP_GCC_ARM_UMODSI` macro checks only the compiler version, and `GMP_GCC_MIPS_O32` relies on the `-mabi=32` flag. Therefore, `$LDFLAGS` is irrelevant to these tests.

Applied to files:

• configure.ac

🧬 Code graph analysis (1)

src/stacktraces.cpp (1)

src/fs.h (2)

• absolute (81-84)
• PathFromString (173-180)

🪛 GitHub Actions: Clang Diff Format Check

src/stacktraces.cpp

[error] 131-135: Clang-format differences detected. Run clang-format (or the provided clang-format-diff.py script) to format the file according to project style.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)

• GitHub Check: linux64_multiprocess-build / Build source
• GitHub Check: mac-build / Build source
• GitHub Check: arm-linux-build / Build source
• GitHub Check: linux64_nowallet-build / Build source
• GitHub Check: linux64_tsan-build / Build source
• GitHub Check: win64-build / Build source
• GitHub Check: linux64_sqlite-build / Build source
• GitHub Check: linux64_fuzz-build / Build source
• GitHub Check: linux64_ubsan-build / Build source
• GitHub Check: linux64-build / Build source

🔇 Additional comments (6)

configure.ac (6)

948-950: LGTM: BSD target detection added.

The new case branch correctly identifies BSD systems and sets TARGET_OS accordingly for use in platform-specific backtrace configuration.

---

1227-1234: LGTM: Backtrace detection initialization.

The initialization and Windows-specific handling are correct. The Windows path appropriately links against dbghelp for stack unwinding.

---

1264-1283: LGTM: DWARF and platform-specific compiler flags.

The enforcement of DWARF 4 (to avoid libbacktrace DWARF 5 parsing issues) and Darwin-specific -fno-standalone-debug flag are correctly implemented with appropriate fallbacks.

---

1293-1310: LGTM: Conditional setup is correctly ordered.

The detection logic runs first, then AM_CONDITIONAL and AC_DEFINE macros are set based on the final detection results. The dependency of crash hooks on stacktraces (line 1297) is appropriate.

---

2045-2047: LGTM: Variable substitution.

The new backtrace variables are properly exported via AC_SUBST for use in downstream Makefiles.

---

2142-2146: LGTM: Configure output display.

The backtrace flags are appropriately displayed in the configure summary output for visibility.

[PastaPastaPasta]

utACK 0d1d6d3

[UdjinM6]

utACK 0d1d6d3