Skip to content

Conversation

fanquake
Copy link
Member

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

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): llvm/llvm-project@ff573a4.

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 google/oss-fuzz#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
Copy link
Contributor

DrahtBot commented Apr 11, 2023

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.

@maflcko
Copy link
Member

maflcko commented Apr 11, 2023

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

sgtm

fanquake added a commit to fanquake/oss-fuzz that referenced this pull request Apr 11, 2023
We'll be removing the `_LIBCPP_DEBUG` (which has been deprecated/removed
by LLVM), downstream in bitcoin/bitcoin#27447.

So remove the comment about re-enabling DEBUG=1, as that will no-longer
do anything for the builds here.

We could follow up with getting a Debug Mode build of libc++ available in the
oss-fuzz environment.
@fanquake fanquake force-pushed the remove_libccp_debug_mode branch 2 times, most recently from 3da1320 to 625140e Compare April 13, 2023 09:43
@fanquake fanquake requested a review from dergoegge April 13, 2023 09:45
jonathanmetzman pushed a commit to google/oss-fuzz that referenced this pull request Apr 14, 2023
We'll be removing the `_LIBCPP_DEBUG` (which has been deprecated/removed
by LLVM), downstream in bitcoin/bitcoin#27447.

So remove the comment about re-enabling DEBUG=1, as that will no-longer
do anything for the builds here.

We could follow up with getting a Debug Mode build of libc++ available
in the
oss-fuzz environment.
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),
llvm/llvm-project@ff573a4.

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 google/oss-fuzz#9828, where
it looks like we'll have to sort out getting a DEBUG build of LLVM.
@fanquake fanquake force-pushed the remove_libccp_debug_mode branch from 625140e to bc4fd49 Compare April 18, 2023 09:45
@maflcko
Copy link
Member

maflcko commented Apr 19, 2023

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 bc4fd49

@fanquake fanquake merged commit d908877 into bitcoin:master Apr 19, 2023
@fanquake fanquake deleted the remove_libccp_debug_mode branch April 19, 2023 10:54
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 19, 2023
…UG mode

bc4fd49 depends: add _LIBCPP_ENABLE_ASSERTIONS to DEBUG mode (fanquake)
cf266b2 depends: Remove _LIBCPP_DEBUG from depends DEBUG mode (fanquake)

Pull request description:

  It was deprecated in LLVM 15, turned into 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): llvm/llvm-project@ff573a4.

  [Building libc++ in debug mode](https://releases.llvm.org/16.0.0/projects/libcxx/docs/DesignDocs/DebugMode.html), 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 google/oss-fuzz#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.

ACKs for top commit:
  MarcoFalke:
    lgtm Approach ACK bc4fd49

Tree-SHA512: 9c0f48fc428278fbf34fbb8f81e761e232506d7ab28e971cb9a9b9a81d549b4d8bbe51e2f7608d56e489428679231da5b7431443849b238a8a993ad241740282
fanquake added a commit that referenced this pull request Apr 19, 2023
4de9c2a ci: build libc++ with assertions in MSAN jobs (fanquake)
23b8b20 ci: build libc++ in DEBUG mode in MSAN jobs (fanquake)

Pull request description:

  Followup to #27447.

  See https://releases.llvm.org/16.0.0/projects/libcxx/docs/DesignDocs/DebugMode.html:
  > Libc++ provides a debug mode that enables special debugging checks meant to detect incorrect usage of the standard library. These checks are disabled by default, but they can be enabled by vendors when building the library by using LIBCXX_ENABLE_DEBUG_MODE.

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 4de9c2a

Tree-SHA512: 788c7f031ccf7a6ac96a87758e57f604cf4d9db0144f0ecc4931823111d2396e64ab260825d74f06b2770d0ac3e4e2c21c46f4def046cf3e1a44d705921ab6d2
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 21, 2023
4de9c2a ci: build libc++ with assertions in MSAN jobs (fanquake)
23b8b20 ci: build libc++ in DEBUG mode in MSAN jobs (fanquake)

Pull request description:

  Followup to bitcoin#27447.

  See https://releases.llvm.org/16.0.0/projects/libcxx/docs/DesignDocs/DebugMode.html:
  > Libc++ provides a debug mode that enables special debugging checks meant to detect incorrect usage of the standard library. These checks are disabled by default, but they can be enabled by vendors when building the library by using LIBCXX_ENABLE_DEBUG_MODE.

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 4de9c2a

Tree-SHA512: 788c7f031ccf7a6ac96a87758e57f604cf4d9db0144f0ecc4931823111d2396e64ab260825d74f06b2770d0ac3e4e2c21c46f4def046cf3e1a44d705921ab6d2
@bitcoin bitcoin locked and limited conversation to collaborators Apr 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants