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

[var-const]

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).

[ldionne]

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]

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]

Address feedback

[var-const]

libcxx/CMakeLists.txt

602–606

Done: https://reviews.llvm.org/D155038

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?

[var-const]

Documentation changes.

[var-const]

Add a release note.

[ldionne]

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]

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]

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]

Address feedback.

[var-const]

Fix the CI.

[ldionne]

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?

[var-const]

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.

[var-const]

Fix the CI.

[var-const]

libcxx/docs/ReleaseNotes.rst

94 ↗

(On Diff #539677)

Done: e52f0e7746c8f2e8c0328054cc0934d4710e372d

[dblaikie]

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]

In D154997#4536344, @dblaikie wrote:

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]

In D154997#4536344, @dblaikie wrote:

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.

[var-const]

@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!

[var-const]

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.

[dblaikie]

In D154997#4537052, @var-const wrote:

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?)

[philnik]

In D154997#4537400, @dblaikie wrote:

In D154997#4537052, @var-const wrote:

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.

[var-const]

In D154997#4537400, @dblaikie wrote:

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?

[Mordante]

In D154997#4537400, @dblaikie wrote:

In D154997#4537052, @var-const wrote:

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.

[dblaikie]

In D154997#4537439, @philnik wrote:

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

In D154997#4537661, @var-const wrote:

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?

In D154997#4539515, @Mordante wrote:

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]

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]

In D154997#4572972, @thakis wrote:

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).)

[ldionne]

In D154997#4574222, @thakis wrote:

[...]

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.

[var-const]

In D154997#4577893, @ldionne wrote:

In D154997#4574222, @thakis wrote:

[...]

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

 endif()
 
 if (MSVC)
   set(LIBCXX_TARGETING_MSVC ON)
   message(STATUS "Configuring for MSVC")
 else()
   set(LIBCXX_TARGETING_MSVC OFF)
 endif()
 
 #===============================================================================
 # Setup CMake Options
 #===============================================================================
 include(CMakeDependentOption)
 include(HandleCompilerRT)
 
 # Basic options ---------------------------------------------------------------
-option(LIBCXX_ENABLE_ASSERTIONS
-  "Enable assertions inside the compiled library, and at the same time make it the
-   default when compiling user code. Note that assertions can be enabled or disabled
-   by users in their own code regardless of this option." OFF)
 option(LIBCXX_ENABLE_SHARED "Build libc++ as a shared library." ON)
 option(LIBCXX_ENABLE_STATIC "Build libc++ as a static library." ON)
 option(LIBCXX_ENABLE_FILESYSTEM
   "Whether to include support for parts of the library that rely on a filesystem being
    available on the platform. This includes things like most parts of <filesystem> and
    others like <fstream>" ON)
 option(LIBCXX_INCLUDE_TESTS "Build the libc++ tests." ${LLVM_INCLUDE_TESTS})
 set(LIBCXX_SUPPORTED_HARDENING_MODES unchecked hardened debug)
 set(LIBCXX_HARDENING_MODE "unchecked" CACHE STRING
   "Specify the default hardening mode to use. This mode will be used inside the
    compiled library and will be the default when compiling user code. Note that
    users can override this setting in their own code. This does not affect the
    ABI. Supported values are ${LIBCXX_SUPPORTED_HARDENING_MODES}.")
 if (NOT "${LIBCXX_HARDENING_MODE}" IN_LIST LIBCXX_SUPPORTED_HARDENING_MODES)
   message(FATAL_ERROR
     "Unsupported hardening mode: '${LIBCXX_HARDENING_MODE}'. Supported values are ${LIBCXX_SUPPORTED_HARDENING_MODES}.")
 endif()
+option(LIBCXX_ENABLE_ASSERTIONS
+  "This is a deprecated option that is only provided for backward compatibility.
+  Enabling this option is equivalent to setting LIBCXX_HARDENING_MODE to
+  "hardened"." OFF)
 option(LIBCXX_ENABLE_RANDOM_DEVICE
   "Whether to include support for std::random_device in the library. Disabling
    this can be useful when building the library for platforms that don't have
    a source of randomness, such as some embedded platforms. When this is not
    supported, most of <random> will still be available, but std::random_device
    will not." ON)
 option(LIBCXX_ENABLE_LOCALIZATION
   "Whether to include support for localization in the library. Disabling
if (APPLE)
     add_link_flags("-undefined dynamic_lookup")
   else()
     # Relax this restriction from HandleLLVMOptions
     string(REPLACE "-Wl,-z,defs" "" CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS}")
   endif()
 endif()
 
 # Assertion flags =============================================================
 define_if(LIBCXX_DEBUG_BUILD -D_DEBUG)
-if (LIBCXX_ENABLE_ASSERTIONS AND NOT LIBCXX_DEBUG_BUILD)
+if (NOT LIBCXX_HARDENING_MODE STREQUAL "unchecked" AND NOT LIBCXX_DEBUG_BUILD)
   # MSVC doesn't like _DEBUG on release builds. See PR 4379.
   define_if_not(LIBCXX_TARGETING_MSVC -D_DEBUG)
 endif()
 
 # Modules flags ===============================================================
 # FIXME The libc++ sources are fundamentally non-modular. They need special
 # versions of the headers in order to provide C++03 and legacy ABI definitions.
 # NOTE: The public headers can be used with modules in all other contexts.
 function(cxx_add_module_flags target)
   if (LLVM_ENABLE_MODULES)
     # Ignore that the rest of the modules flags are now unused.

libcxx/cmake/caches/Generic-assertions.cmake

-set(LIBCXX_ENABLE_ASSERTIONS ON CACHE BOOL "")
-set(LIBCXXABI_ENABLE_ASSERTIONS ON CACHE BOOL "")

libcxx/docs/BuildingLibcxx.rst

.. option:: LIBCXX_INSTALL_LIBRARY:BOOL
   Toggle the installation of the library portion of libc++.
 
 .. option:: LIBCXX_INSTALL_HEADERS:BOOL
 
   **Default**: ``ON``
 
   Toggle the installation of the libc++ headers.
 
-.. option:: LIBCXX_ENABLE_ASSERTIONS:BOOL
-
-  **Default**: ``OFF``
-
-  Build libc++ with assertions enabled in the compiled library, and enable assertions
-  by default when building user code as well. Assertions can be turned off by users
-  by defining ``_LIBCPP_ENABLE_ASSERTIONS=0``. For details, see
-  :ref:`the documentation <assertions-mode>`.
-
 .. option:: LIBCXX_ENABLE_SHARED:BOOL
 
   **Default**: ``ON``
 
   Build libc++ as a shared library. Either `LIBCXX_ENABLE_SHARED` or
   `LIBCXX_ENABLE_STATIC` has to be enabled.
 
 .. option:: LIBCXX_ENABLE_STATIC:BOOL

libcxx/include/__assert

 
 #include <__config>
 #include <__verbose_abort>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
 #endif
 
-#ifndef _LIBCPP_ENABLE_ASSERTIONS
-#  define _LIBCPP_ENABLE_ASSERTIONS _LIBCPP_ENABLE_ASSERTIONS_DEFAULT
-#endif
-
-#if _LIBCPP_ENABLE_ASSERTIONS != 0 && _LIBCPP_ENABLE_ASSERTIONS != 1
-#  error "_LIBCPP_ENABLE_ASSERTIONS must be set to 0 or 1"
-#endif
-
-#if _LIBCPP_ENABLE_ASSERTIONS
+#if _LIBCPP_ENABLE_HARDENED_MODE || _LIBCPP_ENABLE_DEBUG_MODE
 #  define _LIBCPP_ASSERT(expression, message)                                                                          \
     (__builtin_expect(static_cast<bool>(expression), 1)                                                                \
          ? (void)0                                                                                                     \
          : _LIBCPP_VERBOSE_ABORT(                                                                                      \
                "%s:%d: assertion %s failed: %s\n", __builtin_FILE(), __builtin_LINE(), #expression, message))
 // TODO: __builtin_assume can currently inhibit optimizations. Until this has been fixed and we can add
 //       assumptions without a clear optimization intent, disable that to avoid worsening the code generation.
 //       See https://discourse.llvm.org/t/llvm-assume-blocks-optimization/71609 for a discussion.
 #elif 0 && __has_builtin(__builtin_assume)
 #  define _LIBCPP_ASSERT(expression, message)                                                                          \
     (_LIBCPP_DIAGNOSTIC_PUSH _LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wassume")                                              \
          __builtin_assume(static_cast<bool>(expression)) _LIBCPP_DIAGNOSTIC_POP)
 #else
 #  define _LIBCPP_ASSERT(expression, message) ((void)0)
 #endif
 
+// TODO(hardening): only enable uncategorized assertions in the debug mode.
 #define _LIBCPP_ASSERT_UNCATEGORIZED(expression, message) _LIBCPP_ASSERT(expression, message)
 
 #endif // _LIBCPP___ASSERT

libcxx/include/__config

 // - `string_view`;
 // - `array`.
 // #define _LIBCPP_ABI_BOUNDED_ITERATORS
 
 // } ABI
 
 // HARDENING {
 
+// This is for backward compatibility -- make enabling `_LIBCPP_ENABLE_ASSERTIONS` (which predates hardening modes)
+// equivalent to setting the hardened mode.
+#    ifdef(_LIBCPP_ENABLE_ASSERTIONS)
+#      warning "_LIBCPP_ENABLE_ASSERTIONS is deprecated, please use _LIBCPP_ENABLE_HARDENED_MODE instead."
+#    endif
+#    ifndef _LIBCPP_ENABLE_ASSERTIONS
+#      define _LIBCPP_ENABLE_ASSERTIONS _LIBCPP_ENABLE_ASSERTIONS_DEFAULT
+#    endif
+#    if _LIBCPP_ENABLE_ASSERTIONS != 0 && _LIBCPP_ENABLE_ASSERTIONS != 1
+#      error "_LIBCPP_ENABLE_ASSERTIONS must be set to 0 or 1"
+#    endif
+#    if _LIBCPP_ENABLE_ASSERTIONS
+#      define _LIBCPP_ENABLE_HARDENED_MODE 1
+#    endif
+
 // Enables the hardened mode which consists of all checks intended to be used in production. Hardened mode prioritizes
 // security-critical checks that can be done with relatively little overhead in constant time. Mutually exclusive with
 // `_LIBCPP_ENABLE_DEBUG_MODE`.
 //
 // #define _LIBCPP_ENABLE_HARDENED_MODE 1
 
 // Enables the debug mode which contains all the checks from the hardened mode and additionally more expensive checks
 // that may affect the complexity of algorithms. The debug mode is intended to be used for testing, not in production.

 #  if _LIBCPP_ENABLE_DEBUG_MODE != 0 && _LIBCPP_ENABLE_DEBUG_MODE != 1
 #    error "_LIBCPP_ENABLE_DEBUG_MODE must be set to 0 or 1."
 #  endif
 
 #  if _LIBCPP_ENABLE_HARDENED_MODE && _LIBCPP_ENABLE_DEBUG_MODE
 #    error "Only one of _LIBCPP_ENABLE_HARDENED_MODE and _LIBCPP_ENABLE_DEBUG_MODE can be enabled."
 #  endif
 
 // Hardened mode checks.
 #  if _LIBCPP_ENABLE_HARDENED_MODE
 
-// Automatically enable assertions in hardened mode (unless the user explicitly turned them off).
-#    ifndef _LIBCPP_ENABLE_ASSERTIONS
-#      define _LIBCPP_ENABLE_ASSERTIONS 1
-#    endif
-
 // TODO(hardening): more checks to be added here...
 
 // Debug mode checks.
 #  elif _LIBCPP_ENABLE_DEBUG_MODE
 
-// Automatically enable assertions in debug mode (unless the user explicitly turned them off).
-#    ifndef _LIBCPP_ENABLE_ASSERTIONS
-#      define _LIBCPP_ENABLE_ASSERTIONS 1
-#    endif
-
 // TODO(hardening): more checks to be added here...
 
 // Disable all checks if neither the hardened mode nor the debug mode is enabled.
 #  else
 
 // TODO: more checks to be added here...
 
 #  endif // _LIBCPP_ENABLE_HARDENED_MODE

libcxx/utils/ci/run-buildbot

 generic-merged)
     clean
     generate-cmake -C "${MONOREPO_ROOT}/libcxx/cmake/caches/Generic-merged.cmake" \
                    -DLIBCXX_TEST_CONFIG="llvm-libc++-shared.cfg.in" \
                    -DLIBCXXABI_TEST_CONFIG="llvm-libc++abi-merged.cfg.in" \
                    -DLIBUNWIND_TEST_CONFIG="llvm-libunwind-merged.cfg.in"
     check-runtimes
 ;;
-generic-assertions)
-    clean
-    generate-cmake -C "${MONOREPO_ROOT}/libcxx/cmake/caches/Generic-assertions.cmake"
-    check-runtimes
-    check-abi-list
-;;
 generic-hardened-mode)
     clean
     generate-cmake -C "${MONOREPO_ROOT}/libcxx/cmake/caches/Generic-hardened-mode.cmake"
     check-runtimes
     check-abi-list
 ;;
 generic-debug-mode)
     clean
xcrun --sdk macosx                                              \
             --install-dir ${INSTALL_DIR}                            \
             --symbols-dir "${BUILD_DIR}/symbols"                    \
             --architectures "${arch}"                               \
             --version "999.99"
 
     # TODO: It would be better to run the tests against the fake-installed version of libc++ instead
     xcrun --sdk macosx ninja -vC "${BUILD_DIR}/${arch}" check-cxx check-cxxabi check-cxx-abilist
 ;;
 apple-system-backdeployment-assertions-*)
     clean
 
     if [[ "${OSX_ROOTS}" == "" ]]; then
         echo "--- Downloading previous macOS dylibs"
         PREVIOUS_DYLIBS_URL="https://dl.dropboxusercontent.com/s/gmcfxwgl9f9n6pu/libcxx-roots.tar.gz"
         OSX_ROOTS="${BUILD_DIR}/macos-roots"
         mkdir -p "${OSX_ROOTS}"
         curl "${PREVIOUS_DYLIBS_URL}" | tar -xz --strip-components=1 -C "${OSX_ROOTS}"

libcxx/utils/libcxx/test/params.py

Parameter(
             AddCompileFlag("-D_LIBCPP_ENABLE_EXPERIMENTAL"),
         ]
         if experimental
         else [
             AddFeature("libcpp-has-no-incomplete-pstl"),
         ],
     ),
     Parameter(
         name="long_tests",
         choices=[True, False],
         type=bool,
         default=True,
         help="Whether to enable tests that take longer to run. This can be useful when running on a very slow device.",
         actions=lambda enabled: [] if not enabled else [AddFeature("long_tests")],
     ),
     Parameter(
-        name="enable_assertions",
-        choices=[True, False],
-        type=bool,
-        default=False,
-        help="Whether to enable assertions when compiling the test suite. This is only meaningful when "
-        "running the tests against libc++.",
-        actions=lambda assertions: [] if not assertions else [
-            AddCompileFlag("-D_LIBCPP_ENABLE_ASSERTIONS=1"),
-            AddFeature("libcpp-has-assertions"),
-        ],
-    ),
-    Parameter(
         name="hardening_mode",
         choices=["unchecked", "hardened", "debug"],
         type=str,
         default="unchecked",
         help="Whether to enable the hardened mode or the debug mode when compiling the test suite. This is only "
         "meaningful when running the tests against libc++.",
         actions=lambda hardening_mode: filter(
             None,