This is an archive of the discontinued LLVM Phabricator instance.

[libc++][hardening] Deprecate `_LIBCPP_ENABLE_ASSERTIONS`.
ClosedPublic

Authored by var-const on Jul 11 2023, 11:51 AM.

Details

Reviewers
ldionne
Group Reviewers
Restricted Project
Commits
rGf0dfe682bca0: [libc++][hardening] Deprecate `_LIBCPP_ENABLE_ASSERTIONS`.
Summary

_LIBCPP_ENABLE_ASSERTIONS was used to enable the "safe" mode in
libc++. Libc++ now provides the hardened mode and the debug mode that
replace the safe mode.

For backward compatibility, enabling _LIBCPP_ENABLE_ASSERTIONS now
enables the hardened mode. Note that the hardened mode provides
a narrower set of checks than the previous "safe" mode (only
security-critical checks that are performant enough to be used in
production).

Diff Detail

Event Timeline

var-const created this revision.Jul 11 2023, 11:51 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 11 2023, 11:51 AM
Herald added a subscriber: arichardson. · View Herald Transcript
var-const requested review of this revision.Jul 11 2023, 11:51 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 11 2023, 11:51 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne added inline comments.
libcxx/CMakeLists.txt
71

I would remove that option, since it's only used by vendors. I think this is what we want to do:

# TODO(LLVM 18): Remove this after branching for LLVM 17, this is a simple courtesy for vendors to be notified about this change.
if (LIBCXX_ENABLE_ASSERTIONS)
  message(FATAL_ERROR "LIBCXX_ENABLE_ASSERTIONS has been replaced by LIBCXX_HARDENING_MODE=hardened")
endif()

Then we also release note (for vendors) that we removed LIBCXX_ENABLE_ASSERTIONS.

The reason for the temporary FATAL_ERROR is that I want to avoid the case where a vendor is currently specifying LIBCXX_ENABLE_ASSERTIONS and then suddenly after this change they would build the library in unchecked mode, silently.

ldionne added inline comments.Jul 11 2023, 12:40 PM
libcxx/CMakeLists.txt
46

Also we should update the documentation and tests.

602–606

Let's remove LIBCXX_DEBUG_BUILD in a pre-requisite patch. I don't think that's needed anymore, since we use our own internal assertion mechanism.

I think the only place where you'll still want to have a similar check is in cxx_link_system_libraries where you can do something like

if (CMAKE_BUILD_TYPE STREQUAL "Debug")
  set(LIB_SUFFIX "d")
else()
  set(LIB_SUFFIX "")
endif()
libcxx/include/__assert
37–38

I think what I would do instead is this:

// Always define _LIBCPP_ASSERT to _LIBCPP_VERBOSE_ABORT(whatever)
// Always define _SOME_MACRO_THAT_MIGHT_EXPAND_TO_BUILTIN_ASSUME_ONE_DAY

#if _LIBCPP_ENABLE_HARDENED_MODE
    // TODO(hardening): Once we categorize assertions, only enable the ones that we really want here.
#  define _LIBCPP_ASSERT_UNCATEGORIZED(expression, message) _LIBCPP_ASSERT(expression, message)
#elif _LIBCPP_ENABLE_DEBUG_MODE
#  define _LIBCPP_ASSERT_UNCATEGORIZED(expression, message) _LIBCPP_ASSERT(expression, message)
#else
#  define _LIBCPP_ASSERT_UNCATEGORIZED(expression, message) _SOME_MACRO_THAT_MIGHT_EXPAND_TO_BUILTIN_ASSUME_ONE_DAY(...)
#endif
libcxx/include/__config
211–212

We can add a TODO(LLVM 18) here to remove this I guess.

216–218

_LIBCPP_ENABLE_ASSERTIONS_DEFAULT wouldn't be needed anymore since we're removing the CMake flag.

libcxx/utils/ci/run-buildbot
384

buildkite-pipeline.yml changes?

491

Let's make this apple-system-backdeployment-hardened-* and make the corresponding changes so this uses the hardened mode instead.

var-const marked 7 inline comments as done.

Address feedback

var-const added inline comments.Jul 12 2023, 11:32 AM
libcxx/CMakeLists.txt
602–606
libcxx/include/__assert
37–38

Done. I moved the part that deals with _LIBCPP_ASSERT_UNCATEGORIZED to __config, please let me know what you think.

libcxx/utils/ci/run-buildbot
384

Thanks!

491

Done. Do we also need a new configuration for the debug mode?

Documentation changes.

Add a release note.

ldionne added inline comments.Jul 12 2023, 1:04 PM
libcxx/CMakeLists.txt
71

I think it would be easier to just remove the whole thing here, since we're not really retaining for backwards compat.

libcxx/cmake/caches/AIX.cmake
9 ↗(On Diff #539677)

Since this is the default, I think I would just remove it. I think this was probably a leftover from copying the Apple cache, and the reason why it's in the Apple cache is that I wasn't sure what the default was (this went through various states, all of which were more complicated than the other).

libcxx/cmake/caches/Apple.cmake
5 ↗(On Diff #539677)

I would remove since this is default.

15 ↗(On Diff #539677)

This is a bit of a can of worms. I think I would avoid touching this in this patch and handle it as a cleanup later. While LIBCXX_ENABLE_ASSERTIONS is related to the hardened mode, LIBCXXABI_ENABLE_ASSERTIONS is not (for now) -- yes that is confusing.

libcxx/cmake/caches/FreeBSD.cmake
4 ↗(On Diff #539677)

Same here.

libcxx/docs/HardenedMode.rst
23 ↗(On Diff #539677)
27 ↗(On Diff #539677)
29 ↗(On Diff #539677)
33–34 ↗(On Diff #539677)
36–37 ↗(On Diff #539677)
43–44 ↗(On Diff #539677)
libcxx/docs/ReleaseNotes.rst
94 ↗(On Diff #539677)

TODO in a follow-up: Rename the document from HardenedMode.rst to Hardening.rst?

libcxx/docs/UsingLibcxx.rst
146 ↗(On Diff #539677)
193 ↗(On Diff #539677)
208 ↗(On Diff #539677)
libcxx/include/__config
260

I think we might need to move the categorization to __assert, otherwise we're depending on _LIBCPP_ASSERT and _LIBCPP_ASSUME that are defined in __assert (and there's a circular dep if you try to include __assert from __config).

libcxx/utils/ci/buildkite-pipeline.yml
932–933 ↗(On Diff #539677)
ldionne added inline comments.Jul 12 2023, 1:18 PM
libcxx/utils/libcxx/test/params.py
278

Existing tests that -D_LIBCPP_ENABLE_ASSERTIONS could now be moved to // UNSUPPORTED: !libcpp-has-hardened-mode && !libcpp-has-debug-mode or similar.

var-const marked 18 inline comments as done.Jul 12 2023, 6:28 PM
var-const added inline comments.
libcxx/include/__config
260

I think it should be fine. This seems to work across all compilers (https://godbolt.org/z/o6dq7MaoT) and this part of the example in the Standard:

#define m(a)    a(w)
#define w       0,1
// (http://eel.is/c++draft/cpp.rescan)

also seems to confirm that we can rely on the other macro being substituted during the rescan.

var-const updated this revision to Diff 539812.Jul 12 2023, 6:28 PM
var-const marked an inline comment as done.

Address feedback.

var-const retitled this revision from DRAFT [libc++][hardening] Deprecate `_LIBCPP_ENABLE_ASSERTIONS`. to [libc++][hardening] Deprecate `_LIBCPP_ENABLE_ASSERTIONS`..Jul 13 2023, 10:08 AM

Fix the CI.

Nice, thanks! This LGTM except for the _LIBCPP_DEBUG_STRICT_WEAK_ORDERING_CHECK test, which I have to investigate a bit more.

libcxx/include/__config
219–221

I think we should keep (or add) a test that you get the hardened mode when you define _LIBCPP_ENABLE_ASSERTIONS. If we mess that up, it could be very embarrassing in the upcoming release.

libcxx/test/libcxx/algorithms/alg.sorting/assert.sort.invalid_comparator.pass.cpp
13 ↗(On Diff #540130)

This test isn't related to the legacy debug mode, so we don't want to disable it. I'm not sure what's the right call here cause I missed the review where they added -D_LIBCPP_DEBUG_STRICT_WEAK_ORDERING_CHECK, but naively I'd say that we should remove -D_LIBCPP_DEBUG_STRICT_WEAK_ORDERING_CHECK and instead add // UNSUPPORTED: !libcpp-has-hardened-mode && !libcpp-has-debug-mode. I'll dig a bit more.

For now, to unblock you, I think you can just add

// UNSUPPORTED: !libcpp-has-hardened-mode && !libcpp-has-debug-mode

like you did everywhere around, and still keep ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DEBUG_STRICT_WEAK_ORDERING_CHECK below. I'll follow up on D150264 and we can fix this test separately.

libcxx/test/libcxx/assertions/assertions_disabled.pass.cpp
1 ↗(On Diff #540130)

Can you confirm that we have tests to ensure that -D_LIBCPP_ENABLE_HARDENED_MODE=0 or -D_LIBCPP_ENABLE_DEBUG_MODE=0 has a similar effect?

libcxx/test/support/test.support/test_check_assertion.pass.cpp
10 ↗(On Diff #540130)

Maybe

// UNSUPPORTED: c++03
// UNSUPPORTED: !libcpp-has-hardened-mode && !libcpp-has-debug-mode

since you do it consistently everywhere else?

ldionne accepted this revision.Jul 13 2023, 12:45 PM
This revision is now accepted and ready to land.Jul 13 2023, 12:45 PM
var-const updated this revision to Diff 540231.Jul 13 2023, 4:59 PM
var-const marked 4 inline comments as done.

Address feedback.

libcxx/include/__config
219–221

Added libcxx/test/libcxx/assertions/modes/enabling_assertions_enables_hardened_mode.pass.cp.

libcxx/test/libcxx/algorithms/alg.sorting/assert.sort.invalid_comparator.pass.cpp
9 ↗(On Diff #540130)

Question unrelated to this patch: is this necessary? (Since the test is under test/libcxx)

13 ↗(On Diff #540130)

Thanks!

libcxx/test/libcxx/assertions/assertions_disabled.pass.cpp
1 ↗(On Diff #540130)

This should be covered by test/libcxx/assertions/modes/debug_mode_disabled_in_tu.pass.cpp and test/libcxx/assertions/modes/hardened_mode_disabled_in_tu.pass.cpp.

Fix the CI.

var-const marked an inline comment as done.Jul 20 2023, 5:43 PM
var-const added inline comments.
libcxx/docs/ReleaseNotes.rst
94 ↗(On Diff #539677)

Done: e52f0e7746c8f2e8c0328054cc0934d4710e372d

Looks like this change didn't update https://github.com/llvm/llvm-project/commit/d8e9a1a12557cd5e80c3d713d9cbbba0f7406882 ? At least for me this seems to be causing hard failures when building like this:

CC=clang CXX=clang++ cmake -G Ninja -DLLVM_ENABLE_WERROR=true -DLIBCXX_ENABLE_WERROR=true -DLLVM_BUILD_EXAMPLES=true -DCMAKE_BUILD_TYPE=Debug -DLLVM_ENABLE_ASSERTIONS=true -DLLVM_USE_SPLIT_DWARF=true -DLLVM_OPTIMIZED_TABLEGEN=true  -DLLVM_ENABLE_LLD=true -DLLVM_ENABLE_PROJECTS='llvm;clang;clang-tools-extra;lld;lldb;cross-project-tests' -DLLVM_ENABLE_RUNTIMES='libcxxabi;libcxx;compiler-rt'  -DLLDB_ENABLE_PYTHON=On  ~/dev/llvm/src/llvm
ninja check-all
usr/local/google/home/blaikie/dev/llvm/build/default/./bin/clang++ --target=x86_64-unknown-linux-gnu -DLIBCXX_BUILDING_LIBCXXABI -D_GLIBCXX_ASSERTIONS -D_LIBCPP_BUILDING_LIBRARY -D_LIBCPP_ENABLE_ASSERTIONS -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_LINK_PTHREAD_LIB -D_LIBCPP_LINK_RT_LIB -D_LIBCPP_REMOVE_TRANSITIVE_INCLUDES -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/usr/local/google/home/blaikie/dev/llvm/src/libcxx/src -I/usr/local/google/home/blaikie/dev/llvm/build/default/include/c++/v1 -I/usr/local/google/home/blaikie/dev/llvm/build/default/include/x86_64-unknown-linux-gnu/c++/v1 -I/usr/local/google/home/blaikie/dev/llvm/src/libcxxabi/include -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -g -fPIC -gsplit-dwarf -faligned-allocation -nostdinc++ -fvisibility-inlines-hidden -fvisibility=hidden -Wall -Wextra -Wnewline-eof -Wshadow -Wwrite-strings -Wno-unused-parameter -Wno-long-long -Werror=return-type -Wextra-semi -Wundef -Wunused-template -Wformat-nonliteral -Wno-user-defined-literals -Wno-covered-switch-default -Wno-suggest-override -Werror -std=c++20 -MD -MT libcxx/src/CMakeFiles/cxx_shared.dir/new_handler.cpp.o -MF libcxx/src/CMakeFiles/cxx_shared.dir/new_handler.cpp.o.d -o libcxx/src/CMakeFiles/cxx_shared.dir/new_handler.cpp.o -c /usr/local/google/home/blaikie/dev/llvm/src/libcxx/src/new_handler.cpp
In file included from /usr/local/google/home/blaikie/dev/llvm/src/libcxx/src/new_handler.cpp:9:
In file included from /usr/local/google/home/blaikie/dev/llvm/build/default/include/c++/v1/new:89:
In file included from /usr/local/google/home/blaikie/dev/llvm/build/default/include/c++/v1/__assert:13:
/usr/local/google/home/blaikie/dev/llvm/build/default/include/c++/v1/__config:215:6: error: "_LIBCPP_ENABLE_ASSERTIONS is deprecated, please use _LIBCPP_ENABLE_HARDENED_MODE instead." [-Werror,-W#warnings]
  215 | #    warning "_LIBCPP_ENABLE_ASSERTIONS is deprecated, please use _LIBCPP_ENABLE_HARDENED_MODE instead."
      |      ^

is something in the way I'm doing this not supported? If not, how has this not impacted other people in the last few weeks?

cjdb added a subscriber: cjdb.Jul 26 2023, 12:22 PM

Looks like this change didn't update https://github.com/llvm/llvm-project/commit/d8e9a1a12557cd5e80c3d713d9cbbba0f7406882 ? At least for me this seems to be causing hard failures when building like this:

CC=clang CXX=clang++ cmake -G Ninja -DLLVM_ENABLE_WERROR=true -DLIBCXX_ENABLE_WERROR=true -DLLVM_BUILD_EXAMPLES=true -DCMAKE_BUILD_TYPE=Debug -DLLVM_ENABLE_ASSERTIONS=true -DLLVM_USE_SPLIT_DWARF=true -DLLVM_OPTIMIZED_TABLEGEN=true  -DLLVM_ENABLE_LLD=true -DLLVM_ENABLE_PROJECTS='llvm;clang;clang-tools-extra;lld;lldb;cross-project-tests' -DLLVM_ENABLE_RUNTIMES='libcxxabi;libcxx;compiler-rt'  -DLLDB_ENABLE_PYTHON=On  ~/dev/llvm/src/llvm
ninja check-all
usr/local/google/home/blaikie/dev/llvm/build/default/./bin/clang++ --target=x86_64-unknown-linux-gnu -DLIBCXX_BUILDING_LIBCXXABI -D_GLIBCXX_ASSERTIONS -D_LIBCPP_BUILDING_LIBRARY -D_LIBCPP_ENABLE_ASSERTIONS -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_LINK_PTHREAD_LIB -D_LIBCPP_LINK_RT_LIB -D_LIBCPP_REMOVE_TRANSITIVE_INCLUDES -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/usr/local/google/home/blaikie/dev/llvm/src/libcxx/src -I/usr/local/google/home/blaikie/dev/llvm/build/default/include/c++/v1 -I/usr/local/google/home/blaikie/dev/llvm/build/default/include/x86_64-unknown-linux-gnu/c++/v1 -I/usr/local/google/home/blaikie/dev/llvm/src/libcxxabi/include -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -g -fPIC -gsplit-dwarf -faligned-allocation -nostdinc++ -fvisibility-inlines-hidden -fvisibility=hidden -Wall -Wextra -Wnewline-eof -Wshadow -Wwrite-strings -Wno-unused-parameter -Wno-long-long -Werror=return-type -Wextra-semi -Wundef -Wunused-template -Wformat-nonliteral -Wno-user-defined-literals -Wno-covered-switch-default -Wno-suggest-override -Werror -std=c++20 -MD -MT libcxx/src/CMakeFiles/cxx_shared.dir/new_handler.cpp.o -MF libcxx/src/CMakeFiles/cxx_shared.dir/new_handler.cpp.o.d -o libcxx/src/CMakeFiles/cxx_shared.dir/new_handler.cpp.o -c /usr/local/google/home/blaikie/dev/llvm/src/libcxx/src/new_handler.cpp
In file included from /usr/local/google/home/blaikie/dev/llvm/src/libcxx/src/new_handler.cpp:9:
In file included from /usr/local/google/home/blaikie/dev/llvm/build/default/include/c++/v1/new:89:
In file included from /usr/local/google/home/blaikie/dev/llvm/build/default/include/c++/v1/__assert:13:
/usr/local/google/home/blaikie/dev/llvm/build/default/include/c++/v1/__config:215:6: error: "_LIBCPP_ENABLE_ASSERTIONS is deprecated, please use _LIBCPP_ENABLE_HARDENED_MODE instead." [-Werror,-W#warnings]
  215 | #    warning "_LIBCPP_ENABLE_ASSERTIONS is deprecated, please use _LIBCPP_ENABLE_HARDENED_MODE instead."
      |      ^

is something in the way I'm doing this not supported? If not, how has this not impacted other people in the last few weeks?

I also see this when building LLVM with libc++.

var-const marked an inline comment as done.Jul 26 2023, 2:36 PM

Looks like this change didn't update https://github.com/llvm/llvm-project/commit/d8e9a1a12557cd5e80c3d713d9cbbba0f7406882 ? At least for me this seems to be causing hard failures when building like this:

Thank you for the heads-up! I'm trying the command you posted locally, but yes, I'm pretty sure that HandleLLVMOptions.cmake needs to be updated like you mentioned. Not sure why this hasn't come up sooner -- perhaps other build configurations don't use -Werror?

I can work on the fix, or feel free to replace the _LIBCPP_ENABLE_ASSERTIONS macro with _LIBCPP_ENABLE_HARDENED_MODE if that unblocks you faster.

@dblaikie @cjdb Created https://reviews.llvm.org/D156377. I'm still waiting for my local build to finish (my machine is pretty slow, unfortunately, so this will take a while), but it should be the fix. Can you help me find a reviewer for this? Thanks!

Ok, I verified locally that the fix in https://reviews.llvm.org/D156377 makes the build command succeed. I'll land the fix once the CI is green and ask for a cherry-pick for the LLVM 17 release.

Ok, I verified locally that the fix in https://reviews.llvm.org/D156377 makes the build command succeed. I'll land the fix once the CI is green and ask for a cherry-pick for the LLVM 17 release.

Awesome - thanks for the fix!

(anyone have any idea why this issue wasn't discovered sooner? are other libc++ developers doing something different from what I did? @ldionne?)

Ok, I verified locally that the fix in https://reviews.llvm.org/D156377 makes the build command succeed. I'll land the fix once the CI is green and ask for a cherry-pick for the LLVM 17 release.

Awesome - thanks for the fix!

(anyone have any idea why this issue wasn't discovered sooner? are other libc++ developers doing something different from what I did? @ldionne?)

Most libc++ developers (maybe even all right now) don't compile their own toolchain, especially not with assertions enabled.

Awesome - thanks for the fix!

(anyone have any idea why this issue wasn't discovered sooner? are other libc++ developers doing something different from what I did? @ldionne?)

Louis is on vacation, but to echo what Nikolas said, libc++ developers normally don't build llvm or clang, just libc++ and a few closely related targets. Not sure why other people building LLVM haven't encountered this, but perhaps they either don't build libc++ and/or don't use -Werror?

Ok, I verified locally that the fix in https://reviews.llvm.org/D156377 makes the build command succeed. I'll land the fix once the CI is green and ask for a cherry-pick for the LLVM 17 release.

Awesome - thanks for the fix!

(anyone have any idea why this issue wasn't discovered sooner? are other libc++ developers doing something different from what I did? @ldionne?)

Debian provides nightly builds of Clang so I don't build Clang myself. And it seems we don't test this in the CI.

Most libc++ developers (maybe even all right now) don't compile their own toolchain, especially not with assertions enabled.

Louis is on vacation, but to echo what Nikolas said, libc++ developers normally don't build llvm or clang, just libc++ and a few closely related targets. Not sure why other people building LLVM haven't encountered this, but perhaps they either don't build libc++ and/or don't use -Werror?

Debian provides nightly builds of Clang so I don't build Clang myself. And it seems we don't test this in the CI.

Thanks for the perspectives - yeah, I guess as LLVM's gotten larger people have become more specialized/it's not uncommon to be just building one subproject like that. Surprised it didn't turn up on some buildbot then, I guess...

Thanks all!

thakis added a subscriber: thakis.EditedAug 9 2023, 6:51 AM

We are currently shipping the old _LIBCPP_ENABLE_ASSERTION mode. Is there a list of checks that we'll lose when we switch to this new hardened mode?

Also, it's a bit unfortunate that the Windows build of libc++ got broken before this landed (by D153709) and only fixed after this landed (by D155185). That means we can't update to the revision before this change here, then turn switch to hardened mode independent of updating libc++, and then keep updating. It'd be nice to have a temporary opt-out to make it possible to update libc++ in one change and then switch to this new mode in a separate change without updating libc++.

Currently, we'd have to pull in over 100 libc++ revisions at once, including this change (which as far as I understand causes us to lose a not-further-described list of checks).

thakis added a comment.EditedAug 9 2023, 12:19 PM

We are currently shipping the old _LIBCPP_ENABLE_ASSERTION mode. Is there a list of checks that we'll lose when we switch to this new hardened mode?

I tried looking at this, here's what I came up with: https://bugs.chromium.org/p/chromium/issues/detail?id=1465186#c3

In summary, if I understood correctly, most asserts got replaced with _LIBCPP_ASSERT_UNCATEGORIZED() which is disabled in hardened mode.

So to get what we had, we want to ship debug mode. However, debug mode also enables __debug_less and __debug_three_way_comp, and it comes with an ominous comment in __config that says that debug mode "may affect the complexity of algorithms" (which we don't want, and which I think currently also doesn't happen), and it sounds like it's something you don't consider to be shippable.

Maybe _LIBCPP_ASSERT_UNCATEGORIZED and friends could only be defined if they aren't already defined, then we could map all of them to _LIBCPP_ASSERT and not define _LIBCPP_ENABLE_DEBUG_MODE? Then I think we'd have what we used to have and what we currently ship.

Does that sound correct?

(For example, one thing that the old asserts caught and that our tests verify libc++ catches but which isn't caught in hardened mode is string_view(ptr, -1).)

[...]

Maybe _LIBCPP_ASSERT_UNCATEGORIZED and friends could only be defined if they aren't already defined, then we could map all of them to _LIBCPP_ASSERT and not define _LIBCPP_ENABLE_DEBUG_MODE? Then I think we'd have what we used to have and what we currently ship.

Does that sound correct?

(For example, one thing that the old asserts caught and that our tests verify libc++ catches but which isn't caught in hardened mode is string_view(ptr, -1).)

Just to give an update, we talked about it with @var-const and @thakis just now and we plan to add a new mode that contains the barebones hardening assertions, and also other assertions that are not expensive but also not the strict minimum security-wise. That's the mode that Chromium would want to use, since it'd be very close to the old "safe" mode.

[...]

Maybe _LIBCPP_ASSERT_UNCATEGORIZED and friends could only be defined if they aren't already defined, then we could map all of them to _LIBCPP_ASSERT and not define _LIBCPP_ENABLE_DEBUG_MODE? Then I think we'd have what we used to have and what we currently ship.

Does that sound correct?

(For example, one thing that the old asserts caught and that our tests verify libc++ catches but which isn't caught in hardened mode is string_view(ptr, -1).)

Just to give an update, we talked about it with @var-const and @thakis just now and we plan to add a new mode that contains the barebones hardening assertions, and also other assertions that are not expensive but also not the strict minimum security-wise. That's the mode that Chromium would want to use, since it'd be very close to the old "safe" mode.

@thakis @ldionne For easy reference, the patch that's intended to address this is https://reviews.llvm.org/D158823

libcxx/CMakeLists.txt