Merge bitcoin/bitcoin#28865: test: migrate to some per-symbol ubsan suppressions#999
Merge bitcoin/bitcoin#28865: test: migrate to some per-symbol ubsan suppressions#999DashCoreAutoGuix wants to merge 1 commit into
Conversation
fd30e96 test: migrate to some per-symbol ubsan suppressions (fanquake) Pull request description: Now that the symbolizer should be hanging around (bitcoin#28814), migrate some file-wide suppressions to be symbol specific. Should assist in catching new issues that may otherwise go unnoticed due to file-wide suppression. Only tested (so far) on aarch64 using the native ASAN & FUZZ CI. ACKs for top commit: maflcko: lgtm ACK fd30e96 dergoegge: utACK fd30e96 (if CI is green) Tree-SHA512: fbc44464d22813969dd4d1cdeab00042fa45f0af9bf1aed4fd3b688dc7b3c377a7c0f5f0c0a37ba65b649cfb5c7ff8ab2774500fe182d702c4340ca19f08479f
WalkthroughSanitizer suppression files were updated by replacing broad file-level suppressions with more granular function- or symbol-level suppressions. This refinement applies to suppressions for unsigned-integer-overflow, implicit-integer-sign-change, and shift-base issues, targeting specific methods, functions, or variables instead of entire files. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**⚙️ CodeRabbit Configuration File
Files:
🧠 Learnings (10)📓 Common learnings📚 Learning: during dash backport verification of bitcoin core commit 06d469c, scope creep was detected when a...Applied to files:
📚 Learning: during multiple verification attempts of bitcoin core commit 06d469c backport to dash pr #566, da...Applied to files:
📚 Learning: dashcoreautoguix successfully fixed scope creep in bitcoin core commit fcdb39d backport by removi...Applied to files:
📚 Learning: univalue (src/univalue) is no longer a vendored dependency in dash as of pr dashpay#6775, so modifications ...Applied to files:
📚 Learning: applies to src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp} : unit tests should be placed in s...Applied to files:
📚 Learning: dashcoreautoguix successfully completed a complex bitcoin core backport (pr bitcoin#29412) for block mutati...Applied to files:
📚 Learning: in dash backports from bitcoin core, including necessary compilation fixes (such as api compatibilit...Applied to files:
📚 Learning: applies to src/bench/**/*.{cpp,h,cc,cxx,hpp} : performance benchmarks should be placed in src/bench/...Applied to files:
📚 Learning: applies to src/{masternode,evo}/**/*.{cpp,h,cc,cxx,hpp} : masternode lists use immutable data struct...Applied to files:
🔇 Additional comments (4)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Backport Verification SuccessfulNo issues found! This backport is complete and faithful to the original Bitcoin change. Original Bitcoin commit: Verification Details:
Reviewer Feedback Analysis:PastaPastaPasta comments reviewed: 0 Technical Analysis:Change Summary: Migrates UBSan suppressions from file-wide to per-symbol specificity
This PR is ready for merge. ✅ |
|
Critical Issues Found: 🚫 CI Status: 2 jobs failing (threshold: max 1 allowed) ❌ Validation FAILED: 0% size ratio - missing Bitcoin changes entirely
Missing Changes:
Required Actions:
The backport appears incomplete and needs manual review and proper implementation of Bitcoin bitcoin#28865. |
Backports bitcoin#28865
Original commit: a73715e
Backported from Bitcoin Core v0.27
Summary by CodeRabbit