depends: Remove `_LIBCPP_DEBUG` from depends DEBUG mode

fanquake commented at 2:17 pm on April 11, 2023: member

It was deprecated in LLVM 15, turned into compile-time error in LLVM 16:

0In file included from /usr/lib/llvm-16/bin/../include/c++/v1/cassert:19:
1/usr/lib/llvm-16/bin/../include/c++/v1/__assert:22:5: error: "Defining _LIBCPP_DEBUG is not supported anymore.
2Please use _LIBCPP_ENABLE_DEBUG_MODE instead."
3    ^
41 error generated.

and has been removed entirely in LLVM 17 (main): https://github.com/llvm/llvm-project/commit/ff573a42cd1f1d05508f165dc3e645a0ec17edb5.

Building libc++ in debug mode, will also automatically set _LIBCPP_ENABLE_DEBUG_MODE (the new define), so adding it to depends doesn’t seem useful, and would just result in redefinition errors.

I’m wondering if as a followup, we could enable a DEBUG build of libc++ in our MSAN CI job? i.e https://github.com/fanquake/bitcoin/tree/msan_with_enable_debug_mode.

Somewhat related to https://github.com/google/oss-fuzz/pull/9828, where it looks like we’ll have to sort out getting a DEBUG build of LLVM, and can drop the commentary about re-enabling DEBUG=1.

DrahtBot commented at 2:17 pm on April 11, 2023: contributor

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
Approach ACK	MarcoFalke

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

DrahtBot added the label Build system on Apr 11, 2023

in depends/hosts/linux.mk:20 in 5de5899c16 outdated

16@@ -17,7 +17,7 @@ linux_release_CXXFLAGS=$(linux_release_CFLAGS)
17 linux_debug_CFLAGS=-O1
18 linux_debug_CXXFLAGS=$(linux_debug_CFLAGS)
19 
20-linux_debug_CPPFLAGS=-D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC -D_LIBCPP_DEBUG=1
21+linux_debug_CPPFLAGS=-D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC

maflcko commented at 2:41 pm on April 11, 2023:

0linux_debug_CPPFLAGS=-D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC -D_LIBCPP_ENABLE_ASSERTIONS=1

Any reason to not enable assertions in the headers?

… most of the code in libc++ is in the headers, so the user-selected value for _LIBCPP_ENABLE_ASSERTIONS (if any) will usually be respected. https://releases.llvm.org/16.0.0/projects/libcxx/docs/UsingLibcxx.html#assertions-mode

fanquake commented at 4:13 pm on April 11, 2023:

Any reason to not enable assertions in the headers?

Given that this shouldn’t conflict with also turning them on at compile time, i.e also as part of #27448, I don’t think so.

Should we also do _GLIBCXX_ASSERTIONS?

Undefined by default. When defined, enables extra error checking in the form of precondition assertions, such as bounds checking in strings and null pointer checks when dereferencing smart pointers.

maflcko commented at 4:19 pm on April 11, 2023:

_GLIBCXX_ASSERTIONS is enabled by _GLIBCXX_DEBUG

fanquake commented at 4:26 pm on April 11, 2023:

Yea I should have read another 2 lines down

fanquake commented at 4:29 pm on April 11, 2023:

Added now.

maflcko commented at 3:03 pm on April 11, 2023: member

I’m wondering if as a followup, we could enable a DEBUG build of libc++ in our MSAN CI job?

sgtm

maflcko approved

fanquake referenced this in commit 619cb86957 on Apr 11, 2023

fanquake force-pushed on Apr 11, 2023

fanquake force-pushed on Apr 13, 2023

fanquake requested review from dergoegge on Apr 13, 2023

jonathanmetzman referenced this in commit df76eb3f27 on Apr 14, 2023

depends: Remove _LIBCPP_DEBUG from depends DEBUG mode

It was deprecated in LLVM 15, turned into a compile-time error in LLVM 16:
```bash
In file included from /usr/lib/llvm-16/bin/../include/c++/v1/cassert:19:
/usr/lib/llvm-16/bin/../include/c++/v1/__assert:22:5: error: "Defining _LIBCPP_DEBUG is not supported anymore.
Please use _LIBCPP_ENABLE_DEBUG_MODE instead."
    ^
1 error generated.
```

and has been removed entirely in LLVM 17 (main),
https://github.com/llvm/llvm-project/commit/ff573a42cd1f1d05508f165dc3e645a0ec17edb5.

Building libc++ in debug mode, will also automatically set
`_LIBCPP_ENABLE_DEBUG_MODE` (the new define), so adding it to depends
doesn't seem useful, and would just result in redefinition errors.

I'm wondering if as a followup, we could enable a DEBUG build of libc++
in our MSAN CI job?

Somewhat related to https://github.com/google/oss-fuzz/pull/9828, where
it looks like we'll have to sort out getting a DEBUG build of LLVM.

cf266b2270

depends: add _LIBCPP_ENABLE_ASSERTIONS to DEBUG mode

See
https://releases.llvm.org/16.0.0/projects/libcxx/docs/UsingLibcxx.html#assertions-mode
for more info.

bc4fd49d09

fanquake force-pushed on Apr 18, 2023

maflcko commented at 8:49 am on April 19, 2023: member

This should also fix a segfault with DEBUG=1. To reproduce on a fresh install of Ubuntu Jammy:

export DEBIAN_FRONTEND=noninteractive && apt update && apt install curl wget htop git vim ccache -y && git clone https://github.com/bitcoin/bitcoin.git --depth=1 ./bitcoin-core && cd bitcoin-core && apt install build-essential libtool autotools-dev automake pkg-config bsdmainutils python3-zmq make automake cmake curl g++-multilib libtool binutils bsdmainutils pkg-config python3 patch bison clang-14 llvm-14 libc++abi-14-dev libc++-14-dev libunwind-14-dev -y
( cd depends && make DEBUG=1 NO_QT=1 NO_WALLET=1 NO_ZMQ=1 NO_UPNP=1 NO_NATPMP=1 CC=clang-14 CXX='clang++-14 -stdlib=libc++' -j9 ) && ./autogen.sh && CONFIG_SITE="$PWD/depends/x86_64-pc-linux-gnu/share/config.site" ./configure && make clean && make -j $(nproc) check

on master -> segfault

on this pull -> no segfault

lgtm Approach ACK bc4fd49d09dec3791b0acd4ada285b2287361d14

fanquake merged this on Apr 19, 2023

fanquake closed this on Apr 19, 2023

fanquake deleted the branch on Apr 19, 2023

sidhujag referenced this in commit dcc7090fd3 on Apr 19, 2023

fanquake referenced this in commit d26a71a94a on Apr 19, 2023

sidhujag referenced this in commit d3e82cf873 on Apr 21, 2023

bitcoin locked this on Apr 18, 2024

depends: Remove _LIBCPP_DEBUG from depends DEBUG mode #27447

Reviews

depends: Remove `_LIBCPP_DEBUG` from depends DEBUG mode #27447