depends: Remove _LIBCPP_DEBUG from depends DEBUG mode#27447
Merged
fanquake merged 2 commits intobitcoin:masterfrom Apr 19, 2023
Merged
depends: Remove _LIBCPP_DEBUG from depends DEBUG mode#27447fanquake merged 2 commits intobitcoin:masterfrom
_LIBCPP_DEBUG from depends DEBUG mode#27447fanquake merged 2 commits intobitcoin:masterfrom
Conversation
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. |
maflcko
reviewed
Apr 11, 2023
Member
sgtm |
maflcko
approved these changes
Apr 11, 2023
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
force-pushed
the
remove_libccp_debug_mode
branch
2 times, most recently
from
3da1320 to
625140e
Compare
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
force-pushed
the
remove_libccp_debug_mode
branch
from
625140e to
bc4fd49
Compare
Member
|
This should also fix a segfault with
on master -> segfault on this pull -> no segfault lgtm Approach ACK bc4fd49 |
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
It was deprecated in LLVM 15, turned into compile-time error in LLVM 16:
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 dependsdoesn'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.