depends: add new LLVM debug macro #29781

fanquake · 2024-04-01T17:01:33Z

LIBCXX_HARDENING_MODE is the new macro, the previous one was removed in LLVM 18.

See https://libcxx.llvm.org/Hardening.html.

Required before google/oss-fuzz#11725 will do anything (with the bump to 18.x).

Seems reasonable to do now that almost all our test infra is using LLVM 18.

DrahtBot · 2024-04-01T17:01:35Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	theuni

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

theuni · 2024-04-01T17:39:02Z

I don't see any harm in keeping both?

fanquake · 2024-04-01T17:41:26Z

IIRC usage was turned into a compile error (maybe only intermittently, will check again).

fanquake · 2024-04-03T10:08:11Z

IIRC usage was turned into a compile error (maybe only intermittently, will check again).

I think I was thinking of something else here. Have changed this to just adding the new macro.

depends/hosts/linux.mk

theuni · 2024-04-05T16:55:34Z

Trying to make sense of all of this, I found this RFC helpful:

LLVM 18: first release that supports hardening modes and ways to enable them as described in the RFC.

The safe mode (available since the LLVM 15 release) is still supported; the release notes will mention that projects using the safe mode have to transition to use the hardened mode or the debug-lite mode instead (debug-lite is the rough equivalent of the old safe mode).

A few checks that used to be in the safe mode might become excluded (internally, safe will be mapped to debug-lite). In LLVM 17, the safe mode contains every check that isn’t explicitly marked as debug-only, but finer-grained categorization might allow trimming it down further.

The safe mode will no longer use __libcpp_verbose_abort when a check fails (__builtin_trap will be used instead). Overriding __libcpp_verbose_abort will no longer have an effect on the behavior of the safemode.

The meaning of the debug mode will change. The legacy debug mode has been removed in LLVM 17 1. The new debug mode that is part of hardening will be enabled using the mechanisms explained in the RFC and will function differently (e.g. it won’t require a global database).

LLVM 19: the safe mode will be deprecated. The LIBCXX_ENABLE_ASSERTIONS CMake variable and the _LIBCPP_ENABLE_ASSERTIONS macro will be deprecated (with a warning) and users will be given a message to migrate to the hardened mode or the debug-lite mode instead.

LLVM 20: the safe mode will be removed along with the associated macros and the CMake variable.

theuni · 2024-04-05T17:05:29Z

Oh, heh, I missed that @maflcko had already linked that. See also here for an RFC about the deprecation.

I think I'm inclined to agree that it's not worth supporting the old mode. It seems it's old and janky and deprecated for good reason.

fanquake · 2024-04-05T17:10:05Z

Dropped the older define.

theuni · 2024-04-05T20:33:54Z

utACK. Should darwin get this too though since it also uses libc++ ?

fanquake · 2024-04-07T09:46:03Z

utACK. Should darwin get this too though since it also uses libc++ ?

Maybe we could look into this once we switch the Darwin cross builds to use LLVM 18 (currently 17)? At this point the newest Apple Clang is still also based on LLVM 16.

`LIBCPP_HARDENING_MODE` is the new macro, the previous one was removed in LLVM 18. See https://libcxx.llvm.org/Hardening.html.

theuni

ACK 5efebc0

maflcko · 2024-04-09T07:41:11Z

My understanding is that the previous macro does not exist in llvm 18 (what this CI task is using). (git grep LIBCPP_ENABLE_DEBUG_MODE upstream_push/release/18.x is empty).

It could make sense to double check that this can still catch issues.

# Conflicts: # src/kernel/chainparams.cpp

maflcko · 2024-04-23T16:12:16Z

Looks like it worked in https://github.com/bitcoin/bitcoin/runs/24156835563

/msan/cxx_build/include/c++/v1/string_view:399: assertion __pos < size() failed: string_view[] index out of bounds

5efebc0 depends: add the new LLVM debug macro (fanquake) Pull request description: `LIBCXX_HARDENING_MODE` is the new macro, the previous one was removed in LLVM 18. See https://libcxx.llvm.org/Hardening.html. Required before google/oss-fuzz#11725 will do anything (with the bump to 18.x). Seems reasonable to do now that almost all our test infra is using LLVM 18. ACKs for top commit: theuni: ACK 5efebc0 Tree-SHA512: 43078eeb5940c55ef4f95c72682f8a372dcd3eb97956b3114149c16d9f59b067a999b2aab7f34ffb57eab191524514408e2bba154ff4a6ea0cd6ec4d119c5d18

700b8c5 Merge bitcoin#29658: Bugfix: GUI: Help messages already have a trailing newline, so don't add an extra one (merge-script) a0cd305 Merge bitcoin#29948: test: add missing comparison of node1's mempool in MempoolPackagesTest (Ava Chow) f1907ea Merge bitcoin#29984: net: Replace ifname check with IFF_LOOPBACK in Discover (merge-script) daa6eee Merge bitcoin#29960: depends: pass verbose through to cmake based makefiles (merge-script) 61a5832 Merge bitcoin#29907: test: Fix `test/streams_tests.cpp` compilation on SunOS / illumos (merge-script) 23f25a9 Merge bitcoin#29872: test: Add missing Assert(mock_time_in >= 0s) to SetMockTime (merge-script) a7daee7 Merge bitcoin#29689: lint: scripted-diff verification also requires GNU grep (Ava Chow) 3df1ca1 Merge bitcoin#29953: doc: Bash is needed in gen_id and is not installed on FreeBSD by default (merge-script) b53b854 Merge bitcoin#29850: net: Decrease nMaxIPs when learning from DNS seeds (Ava Chow) c4a147c Merge bitcoin#28340: security: restrict abis in bitcoind.service (Ryan Ofsky) acfdf9e Merge bitcoin#28373: doc: Add example of mixing private and public keys in descriptors (Ava Chow) 51bc8bd Merge bitcoin#29859: build: Fix false positive `CHECK_ATOMIC` test (merge-script) bb4102c Merge bitcoin#29893: test: fix intermittent failure in p2p_compactblocks_hb.py (glozow) 4ecb761 Merge bitcoin#29786: Drop Windows Socket dependency for `randomenv.cpp` (fanquake) 1a8e805 Merge bitcoin#29498: test: Update --tmpdir doc string to say directory must not exist (fanquake) 81ca71c Merge bitcoin#29781: depends: add new LLVM debug macro (fanquake) 5ce92ca Merge bitcoin#29527: depends: add -g to DEBUG=1 flags (fanquake) Pull request description: ## Issue being fixed or feature implemented Trivial backports ## What was done? ## How Has This Been Tested? built locally ## Breaking Changes ## Checklist: _Go over all the following points, and put an `x` in all the boxes that apply._ - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 700b8c5 but pls confirm Guix is happy now before merging kwvg: utACK 700b8c5 Tree-SHA512: 9722979c4f0589cb02bfeaf39373713372a4aa1c8c5a55aa5b4d33388f73ef19231de3963b80ffaad25fbe0db90a133de5080baac691daed41b86a762b867b2d

DrahtBot added the Build system label Apr 1, 2024

fanquake mentioned this pull request Apr 1, 2024

bitcoin-core: Enable libc++ "Debug" hardening mode google/oss-fuzz#11725

Merged

DrahtBot added the CI failed label Apr 2, 2024

fanquake force-pushed the llvm_18_hardening_mode branch from 26ea15e to 3fb8cb4 Compare April 3, 2024 10:07

fanquake changed the title ~~depends: switch to new LLVM debug macro~~ depends: add new LLVM debug macro Apr 3, 2024

fanquake marked this pull request as ready for review April 3, 2024 10:08

maflcko reviewed Apr 3, 2024

View reviewed changes

depends/hosts/linux.mk Outdated Show resolved Hide resolved

theuni mentioned this pull request Apr 3, 2024

Compilation failure with -O0 + -fsanitize=address due to inline asm #29801

Closed

1 task

DrahtBot removed the CI failed label Apr 4, 2024

fanquake force-pushed the llvm_18_hardening_mode branch from 3fb8cb4 to e6d2c9d Compare April 5, 2024 17:08

depends: add the new LLVM debug macro

5efebc0

`LIBCPP_HARDENING_MODE` is the new macro, the previous one was removed in LLVM 18. See https://libcxx.llvm.org/Hardening.html.

fanquake force-pushed the llvm_18_hardening_mode branch from e6d2c9d to 5efebc0 Compare April 7, 2024 09:49

theuni approved these changes Apr 8, 2024

View reviewed changes

fanquake merged commit 9dfe435 into bitcoin:master Apr 9, 2024

fanquake deleted the llvm_18_hardening_mode branch April 9, 2024 07:34

Pttn added a commit to RiecoinTeam/Riecoin that referenced this pull request Apr 13, 2024

Merge bitcoin#29781

461e5cb

# Conflicts: # src/kernel/chainparams.cpp

bitcoin locked and limited conversation to collaborators Apr 23, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

depends: add new LLVM debug macro #29781

depends: add new LLVM debug macro #29781

Uh oh!

fanquake commented Apr 1, 2024

Uh oh!

DrahtBot commented Apr 1, 2024 •

edited

Loading

Uh oh!

theuni commented Apr 1, 2024

Uh oh!

fanquake commented Apr 1, 2024

Uh oh!

fanquake commented Apr 3, 2024

Uh oh!

Uh oh!

theuni commented Apr 5, 2024

Uh oh!

theuni commented Apr 5, 2024

Uh oh!

fanquake commented Apr 5, 2024

Uh oh!

theuni commented Apr 5, 2024

Uh oh!

fanquake commented Apr 7, 2024

Uh oh!

theuni left a comment

Uh oh!

maflcko commented Apr 9, 2024 •

edited

Loading

Uh oh!

maflcko commented Apr 23, 2024 •

edited

Loading

Uh oh!

Uh oh!

depends: add new LLVM debug macro #29781

depends: add new LLVM debug macro #29781

Uh oh!

Conversation

fanquake commented Apr 1, 2024

Uh oh!

DrahtBot commented Apr 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage

Reviews

Uh oh!

theuni commented Apr 1, 2024

Uh oh!

fanquake commented Apr 1, 2024

Uh oh!

fanquake commented Apr 3, 2024

Uh oh!

Uh oh!

theuni commented Apr 5, 2024

Uh oh!

theuni commented Apr 5, 2024

Uh oh!

fanquake commented Apr 5, 2024

Uh oh!

theuni commented Apr 5, 2024

Uh oh!

fanquake commented Apr 7, 2024

Uh oh!

theuni left a comment

Choose a reason for hiding this comment

Uh oh!

maflcko commented Apr 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maflcko commented Apr 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

DrahtBot commented Apr 1, 2024 •

edited

Loading

maflcko commented Apr 9, 2024 •

edited

Loading

maflcko commented Apr 23, 2024 •

edited

Loading