From e8e95c57dc5599fc1d389bdce250c3a6cfad46b2 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Thu, 11 Jan 2024 17:05:58 -0800 Subject: [PATCH 01/19] [libc++][hardening] Rework how the assertion handler can be overridden. Previously there were two ways to override the verbose abort function which gets called when a hardening assertion is triggered: - compile-time: define the `_LIBCPP_VERBOSE_ABORT` macro; - link-time: provide a definition of `__libcpp_verbose_abort` function. This patch adds a new configure-time approach: a vendor can provide a path to a custom header file whose contents will get injected into the build by CMake. The header must provide a definition of the `_LIBCPP_ASSERTION_HANDLER` macro which is what will get called should a hardening assertion fail. As of this patch, overriding `_LIBCPP_VERBOSE_ABORT` will still work, but the previous mechanisms will be effectively removed in a follow-up patch, making the configure-time mechanism the sole way of overriding the default handler. Note that `_LIBCPP_ASSERTION_HANDLER` only gets invoked when a hardening assertion fails. It does not affect other cases where `_LIBCPP_VERBOSE_ABORT` is currently used (e.g. when an exception is thrown in the `-fno-exceptions` mode). The library provides a default version of the custom header file that will get used if it's not overridden by the vendor. That allows us to always test the override mechanism and reduces the difference in configuration between the pristine version of the library and a platform-specific version. --- libcxx/CMakeLists.txt | 7 +++++++ libcxx/include/CMakeLists.txt | 9 +++++++++ libcxx/include/__assert | 4 ++-- libcxx/include/__assertion_handler.in | 13 +++++++++++++ libcxx/test/libcxx/lint/lint_headers.sh.py | 1 + libcxx/utils/generate_iwyu_mapping.py | 2 ++ libcxx/utils/libcxx/header_information.py | 3 +++ libcxx/vendor/llvm/default_assertion_handler.in | 14 ++++++++++++++ 8 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 libcxx/include/__assertion_handler.in create mode 100644 libcxx/vendor/llvm/default_assertion_handler.in diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt index 75cb63222da35..b09836a6ab69c 100644 --- a/libcxx/CMakeLists.txt +++ b/libcxx/CMakeLists.txt @@ -69,6 +69,13 @@ 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() +set(LIBCXX_ASSERTION_HANDLER_FILE + "${CMAKE_CURRENT_SOURCE_DIR}/vendor/llvm/default_assertion_handler.in" + CACHE STRING + "Specify the path to a header that contains a custom implementation of the + assertion handler that gets invoked when a hardening assertion fails. If + provided, the contents of this header will get injected into the library code + and override the default assertion handler.") 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 diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt index 0fe3ab44d2466..efbce0fee847d 100644 --- a/libcxx/include/CMakeLists.txt +++ b/libcxx/include/CMakeLists.txt @@ -1019,9 +1019,12 @@ foreach(feature LIBCXX_ENABLE_FILESYSTEM LIBCXX_ENABLE_LOCALIZATION LIBCXX_ENABL endforeach() configure_file("__config_site.in" "${LIBCXX_GENERATED_INCLUDE_TARGET_DIR}/__config_site" @ONLY) +file(READ ${LIBCXX_ASSERTION_HANDLER_FILE} _LIBCPP_ASSERTION_HANDLER_OVERRIDE) +configure_file("__assertion_handler.in" "${LIBCXX_GENERATED_INCLUDE_TARGET_DIR}/__assertion_handler" @ONLY) configure_file("module.modulemap.in" "${LIBCXX_GENERATED_INCLUDE_DIR}/module.modulemap" @ONLY) set(_all_includes "${LIBCXX_GENERATED_INCLUDE_TARGET_DIR}/__config_site" + "${LIBCXX_GENERATED_INCLUDE_TARGET_DIR}/__assertion_handler" "${LIBCXX_GENERATED_INCLUDE_DIR}/module.modulemap") foreach(f ${files}) set(src "${CMAKE_CURRENT_SOURCE_DIR}/${f}") @@ -1059,6 +1062,12 @@ if (LIBCXX_INSTALL_HEADERS) PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ COMPONENT cxx-headers) + # Install the generated __assertion_handler file to the per-target include dir. + install(FILES "${LIBCXX_GENERATED_INCLUDE_TARGET_DIR}/__assertion_handler" + DESTINATION "${LIBCXX_INSTALL_INCLUDE_TARGET_DIR}" + PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ + COMPONENT cxx-headers) + # Install the generated modulemap file to the generic include dir. install(FILES "${LIBCXX_GENERATED_INCLUDE_DIR}/module.modulemap" DESTINATION "${LIBCXX_INSTALL_INCLUDE_DIR}" diff --git a/libcxx/include/__assert b/libcxx/include/__assert index d4af7e6c7192a..3c49c96f65a3e 100644 --- a/libcxx/include/__assert +++ b/libcxx/include/__assert @@ -10,8 +10,8 @@ #ifndef _LIBCPP___ASSERT #define _LIBCPP___ASSERT +#include <__assertion_handler> #include <__config> -#include <__verbose_abort> #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) # pragma GCC system_header @@ -20,7 +20,7 @@ #define _LIBCPP_ASSERT(expression, message) \ (__builtin_expect(static_cast(expression), 1) \ ? (void)0 \ - : _LIBCPP_VERBOSE_ABORT( \ + : _LIBCPP_ASSERTION_HANDLER( \ "%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 diff --git a/libcxx/include/__assertion_handler.in b/libcxx/include/__assertion_handler.in new file mode 100644 index 0000000000000..59342a3b0be89 --- /dev/null +++ b/libcxx/include/__assertion_handler.in @@ -0,0 +1,13 @@ +// -*- C++ -*- +#ifndef _LIBCPP___ASSERTION_HANDLER +#define _LIBCPP___ASSERTION_HANDLER + +#include <__config> + +#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) +# pragma GCC system_header +#endif + +@_LIBCPP_ASSERTION_HANDLER_OVERRIDE@ + +#endif // _LIBCPP___ASSERTION_HANDLER diff --git a/libcxx/test/libcxx/lint/lint_headers.sh.py b/libcxx/test/libcxx/lint/lint_headers.sh.py index ab237c968da7e..aee10e13b9560 100644 --- a/libcxx/test/libcxx/lint/lint_headers.sh.py +++ b/libcxx/test/libcxx/lint/lint_headers.sh.py @@ -14,6 +14,7 @@ def exclude_from_consideration(path): or path.endswith(".modulemap.in") or os.path.basename(path) == "__config" or os.path.basename(path) == "__config_site.in" + or os.path.basename(path) == "__assertion_handler.in" or os.path.basename(path) == "libcxx.imp" or os.path.basename(path).startswith("__pstl") or not os.path.isfile(path) # TODO: Remove once PSTL integration is finished diff --git a/libcxx/utils/generate_iwyu_mapping.py b/libcxx/utils/generate_iwyu_mapping.py index 343538a6cae48..d5d1577e4a2ad 100644 --- a/libcxx/utils/generate_iwyu_mapping.py +++ b/libcxx/utils/generate_iwyu_mapping.py @@ -43,6 +43,8 @@ def generate_map(include): public = [] if i == "__assert": continue + elif i == "__assertion_handler.in": + continue elif i == "__availability": continue elif i == "__bit_reference": diff --git a/libcxx/utils/libcxx/header_information.py b/libcxx/utils/libcxx/header_information.py index 54e18b5ea533d..326931b0081c0 100644 --- a/libcxx/utils/libcxx/header_information.py +++ b/libcxx/utils/libcxx/header_information.py @@ -168,6 +168,9 @@ def is_modulemap_header(header): if header == "__config_site": return False + if header == "__assertion_handler": + return False + # exclude libc++abi files if header in ["cxxabi.h", "__cxxabi_config.h"]: return False diff --git a/libcxx/vendor/llvm/default_assertion_handler.in b/libcxx/vendor/llvm/default_assertion_handler.in new file mode 100644 index 0000000000000..2422b6dadbe64 --- /dev/null +++ b/libcxx/vendor/llvm/default_assertion_handler.in @@ -0,0 +1,14 @@ +// -*- C++ -*- +#include <__config> +#include <__verbose_abort> + +#if _LIBCPP_HARDENING_MODE == _LIBCPP_HARDENING_MODE_DEBUG + +#define _LIBCPP_ASSERTION_HANDLER(...) _LIBCPP_VERBOSE_ABORT(__VA_ARGS__) + +#else + +// TODO(hardening): in production, trap rather than abort. +#define _LIBCPP_ASSERTION_HANDLER(...) _LIBCPP_VERBOSE_ABORT(__VA_ARGS__) + +#endif // #if _LIBCPP_HARDENING_MODE == _LIBCPP_HARDENING_MODE_DEBUG From 86fc352301f8d6503fb3388fd5288a30f1ff6011 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Thu, 11 Jan 2024 23:28:42 -0800 Subject: [PATCH 02/19] Pass the original error message as the first argument. --- libcxx/include/__assert | 1 + libcxx/vendor/llvm/default_assertion_handler.in | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/libcxx/include/__assert b/libcxx/include/__assert index 3c49c96f65a3e..a56e098b52328 100644 --- a/libcxx/include/__assert +++ b/libcxx/include/__assert @@ -21,6 +21,7 @@ (__builtin_expect(static_cast(expression), 1) \ ? (void)0 \ : _LIBCPP_ASSERTION_HANDLER( \ + message, \ "%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 diff --git a/libcxx/vendor/llvm/default_assertion_handler.in b/libcxx/vendor/llvm/default_assertion_handler.in index 2422b6dadbe64..b72b0be5b5f4b 100644 --- a/libcxx/vendor/llvm/default_assertion_handler.in +++ b/libcxx/vendor/llvm/default_assertion_handler.in @@ -4,11 +4,11 @@ #if _LIBCPP_HARDENING_MODE == _LIBCPP_HARDENING_MODE_DEBUG -#define _LIBCPP_ASSERTION_HANDLER(...) _LIBCPP_VERBOSE_ABORT(__VA_ARGS__) +#define _LIBCPP_ASSERTION_HANDLER(error_message, ...) ((void)error_message, _LIBCPP_VERBOSE_ABORT(__VA_ARGS__)) #else // TODO(hardening): in production, trap rather than abort. -#define _LIBCPP_ASSERTION_HANDLER(...) _LIBCPP_VERBOSE_ABORT(__VA_ARGS__) +#define _LIBCPP_ASSERTION_HANDLER(error_message, ...) ((void)error_message, _LIBCPP_VERBOSE_ABORT(__VA_ARGS__)) #endif // #if _LIBCPP_HARDENING_MODE == _LIBCPP_HARDENING_MODE_DEBUG From 216605f0cf09c7b27361e9e8d61942c595e32eb5 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Fri, 12 Jan 2024 11:09:31 -0800 Subject: [PATCH 03/19] Update the documentation about overriding the default assertion handler. --- libcxx/docs/UsingLibcxx.rst | 82 ++++++++++++++----------------------- 1 file changed, 31 insertions(+), 51 deletions(-) diff --git a/libcxx/docs/UsingLibcxx.rst b/libcxx/docs/UsingLibcxx.rst index e1bbf39b9634a..91d5b148bc12a 100644 --- a/libcxx/docs/UsingLibcxx.rst +++ b/libcxx/docs/UsingLibcxx.rst @@ -158,57 +158,37 @@ provided by the static or shared library, so it is only available when deploying the compiled library is sufficiently recent. On older platforms, the program will terminate in an unspecified unsuccessful manner, but the quality of diagnostics won't be great. -However, users can also override that mechanism at two different levels. First, the mechanism can be -overridden at compile time by defining the ``_LIBCPP_VERBOSE_ABORT(format, args...)`` variadic macro. -When that macro is defined, it will be called with a format string as the first argument, followed by -a series of arguments to format using printf-style formatting. Compile-time customization may be -useful to get precise control over code generation, however it is also inconvenient to use in -some cases. Indeed, compile-time customization of the verbose termination function requires that all -translation units be compiled with a consistent definition for ``_LIBCPP_VERBOSE_ABORT`` to avoid ODR -violations, which can add complexity in the build system of users. - -Otherwise, if compile-time customization is not necessary, link-time customization of the handler is also -possible, similarly to how replacing ``operator new`` works. This mechanism trades off fine-grained control -over the call site where the termination is initiated in exchange for better ergonomics. Link-time -customization is done by simply defining the following function in exactly one translation unit of your -program: - -.. code-block:: cpp - - void __libcpp_verbose_abort(char const* format, ...) - -This mechanism is similar to how one can replace the default definition of ``operator new`` -and ``operator delete``. For example: - -.. code-block:: cpp - - // In HelloWorldHandler.cpp - #include // must include any libc++ header before defining the function (C compatibility headers excluded) - - void std::__libcpp_verbose_abort(char const* format, ...) { - std::va_list list; - va_start(list, format); - std::vfprintf(stderr, format, list); - va_end(list); - - std::abort(); - } - - // In HelloWorld.cpp - #include - - int main() { - std::vector v; - int& x = v[0]; // Your termination function will be called here if hardening is enabled. - } - -Also note that the verbose termination function should never return. Since assertions in libc++ -catch undefined behavior, your code will proceed with undefined behavior if your function is called -and does return. - -Furthermore, exceptions should not be thrown from the function. Indeed, many functions in the -library are ``noexcept``, and any exception thrown from the termination function will result -in ``std::terminate`` being called. +However, vendors can also override that mechanism at CMake configuration time. When a hardening +assertion fails, the library invokes the ``_LIBCPP_ASSERTION_HANDLER`` macro. A vendor may provide +a header that contains a custom definition of this macro and specify the path to the header via the +``LIBCXX_ASSERTION_HANDLER_FILE`` CMake variable. If provided, the contents of this header will be +injected into library configuration headers, replacing the default implementation. The header must not +include any standard library headers (directly or transitively) because doing so will almost always +create a circular dependency. + +``_LIBCPP_ASSERTION_HANDLER(error_message, format, args...)`` is a variadic macro that takes the +following parameters: + +* ``error_message`` -- the original error message that explains the hardening failure. In general, it + does not contain information about the source location that triggered the failure. +* ``format`` -- a printf-style format string that contains a general description of the failure with + placeholders for the error message as well as details about the source location. +* ``args...`` -- arguments to substitute in the ``format`` string. The exact order and meaning of the + arguments is unspecified and subject to change (but is always in sync with the format string). Note + that for convenience, ``args`` contain the error message as well. + +Programs that wish to terminate as fast as possible may use the ``error_message`` parameter that +doesn't require any formatting. Programs that prefer having more information about the failure (such as +the filename and the line number of the code that triggered the assertion) should use the printf-style +formatting with ``format`` and ``args...``. + +When a hardening assertion fails, it means that the program is about to invoke undefined behavior. For +this reason, the custom assertion handler is generally expected to terminate the program. If a custom +assertion handler decides to avoid doing so (e.g. it chooses to log and continue instead), it does so +at its own risk -- this approach should only be used in non-production builds and with an understanding +of potential consequences. Furthermore, the custom assertion handler should not throw any exceptions as +it may be invoked from standard library functions that are marked ``noexcept`` (so throwing will result +in ``std::terminate`` being called). Libc++ Configuration Macros =========================== From 692aac0ff4ae89820d625ef29c95e55fff2dbbb3 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Fri, 12 Jan 2024 11:30:59 -0800 Subject: [PATCH 04/19] Update the release notes --- libcxx/docs/ReleaseNotes/18.rst | 4 ++++ libcxx/docs/UsingLibcxx.rst | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/libcxx/docs/ReleaseNotes/18.rst b/libcxx/docs/ReleaseNotes/18.rst index 882f53b8d9f83..da190aa8d3f64 100644 --- a/libcxx/docs/ReleaseNotes/18.rst +++ b/libcxx/docs/ReleaseNotes/18.rst @@ -107,6 +107,10 @@ Deprecations and Removals macro is provided to restore the previous behavior, and it will be supported in the LLVM 18 release only. In LLVM 19 and beyond, ``_LIBCPP_ENABLE_NARROWING_CONVERSIONS_IN_VARIANT`` will not be honored anymore. +- The only supported way to customize the assertion handler that gets invoked when a hardening assertion fails + is now by setting the ``LIBCXX_ASSERTION_HANDLER_FILE`` CMake variable and providing a custom header. See + the documentation on overriding the default assertion handler for details. + - The ``_LIBCPP_AVAILABILITY_CUSTOM_VERBOSE_ABORT_PROVIDED`` macro is not honored anymore in LLVM 18. Please see the updated documentation about the hardening modes in libc++ and in particular the ``_LIBCPP_VERBOSE_ABORT`` macro for details. diff --git a/libcxx/docs/UsingLibcxx.rst b/libcxx/docs/UsingLibcxx.rst index 91d5b148bc12a..5c04cc4f7a808 100644 --- a/libcxx/docs/UsingLibcxx.rst +++ b/libcxx/docs/UsingLibcxx.rst @@ -146,9 +146,9 @@ IWYU, you should run the tool like so: If you would prefer to not use that flag, then you can replace ``/path/to/include-what-you-use/share/libcxx.imp`` file with the libc++-provided ``libcxx.imp`` file. -.. _termination-handler: +.. _assertion-handler: -Overriding the default termination handler +Overriding the default assertion handler ========================================== When the library wants to terminate due to an unforeseen condition (such as a hardening assertion From 8981ef9e8f5eeebd605cbde24d8d0764b5187ed8 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Fri, 12 Jan 2024 11:36:34 -0800 Subject: [PATCH 05/19] Add license to the new headers --- libcxx/include/__assertion_handler.in | 8 ++++++++ libcxx/vendor/llvm/default_assertion_handler.in | 8 ++++++++ 2 files changed, 16 insertions(+) diff --git a/libcxx/include/__assertion_handler.in b/libcxx/include/__assertion_handler.in index 59342a3b0be89..bf4afc31b6e7e 100644 --- a/libcxx/include/__assertion_handler.in +++ b/libcxx/include/__assertion_handler.in @@ -1,4 +1,12 @@ // -*- C++ -*- +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + #ifndef _LIBCPP___ASSERTION_HANDLER #define _LIBCPP___ASSERTION_HANDLER diff --git a/libcxx/vendor/llvm/default_assertion_handler.in b/libcxx/vendor/llvm/default_assertion_handler.in index b72b0be5b5f4b..9162a19aaf064 100644 --- a/libcxx/vendor/llvm/default_assertion_handler.in +++ b/libcxx/vendor/llvm/default_assertion_handler.in @@ -1,4 +1,12 @@ // -*- C++ -*- +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + #include <__config> #include <__verbose_abort> From 50282f37da27983c4cffd3df9ce1611f61450097 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Fri, 12 Jan 2024 17:56:00 -0800 Subject: [PATCH 06/19] Partially address feedback, update how matching the error message works in tests. --- libcxx/docs/BuildingLibcxx.rst | 39 ++++++++++++++++ libcxx/docs/UsingLibcxx.rst | 44 ------------------- libcxx/include/__assert | 5 +-- libcxx/test/support/check_assertion.h | 30 ++++++++----- .../vendor/llvm/default_assertion_handler.in | 10 +---- 5 files changed, 60 insertions(+), 68 deletions(-) diff --git a/libcxx/docs/BuildingLibcxx.rst b/libcxx/docs/BuildingLibcxx.rst index 0b4d420528651..edc7d0163527d 100644 --- a/libcxx/docs/BuildingLibcxx.rst +++ b/libcxx/docs/BuildingLibcxx.rst @@ -473,6 +473,45 @@ LLVM-specific options others. +.. _assertion-handler: + +Overriding the default assertion handler +========================================== + +When the library wants to terminate due to an unforeseen condition (such as +a hardening assertion failure), the program is aborted through a special verbose +termination function. The library provides a default function that prints an +error message and calls ``std::abort()``. Note that this function is provided by +the static or shared library, so it is only available when deploying to +a platform where the compiled library is sufficiently recent. On older +platforms, the program will terminate in an unspecified unsuccessful manner, but +the quality of diagnostics won't be great. + +However, vendors can also override that mechanism at CMake configuration time. +When a hardening assertion fails, the library invokes the +``_LIBCPP_ASSERTION_HANDLER`` macro. A vendor may provide a header that contains +a custom definition of this macro and specify the path to the header via the +``LIBCXX_ASSERTION_HANDLER_FILE`` CMake variable. If provided, the contents of +this header will be injected into library configuration headers, replacing the +default implementation. The header must not include any standard library headers +(directly or transitively) because doing so will almost always create a circular +dependency. + +The ``_LIBCPP_ASSERTION_HANDLER(message)`` macro takes a single parameter that +contains an error message explaining the hardening failure and some details +about the source location that triggered it. + +When a hardening assertion fails, it means that the program is about to invoke +library undefined behavior. For this reason, the custom assertion handler is +generally expected to terminate the program. If a custom assertion handler +decides to avoid doing so (e.g. it chooses to log and continue instead), it does +so at its own risk -- this approach should only be used in non-production builds +and with an understanding of potential consequences. Furthermore, the custom +assertion handler should not throw any exceptions as it may be invoked from +standard library functions that are marked ``noexcept`` (so throwing will result +in ``std::terminate`` being called). + + Using Alternate ABI libraries ============================= diff --git a/libcxx/docs/UsingLibcxx.rst b/libcxx/docs/UsingLibcxx.rst index 5c04cc4f7a808..917f86b9aef97 100644 --- a/libcxx/docs/UsingLibcxx.rst +++ b/libcxx/docs/UsingLibcxx.rst @@ -146,50 +146,6 @@ IWYU, you should run the tool like so: If you would prefer to not use that flag, then you can replace ``/path/to/include-what-you-use/share/libcxx.imp`` file with the libc++-provided ``libcxx.imp`` file. -.. _assertion-handler: - -Overriding the default assertion handler -========================================== - -When the library wants to terminate due to an unforeseen condition (such as a hardening assertion -failure), the program is aborted through a special verbose termination function. The library provides -a default function that prints an error message and calls ``std::abort()``. Note that this function is -provided by the static or shared library, so it is only available when deploying to a platform where -the compiled library is sufficiently recent. On older platforms, the program will terminate in an -unspecified unsuccessful manner, but the quality of diagnostics won't be great. - -However, vendors can also override that mechanism at CMake configuration time. When a hardening -assertion fails, the library invokes the ``_LIBCPP_ASSERTION_HANDLER`` macro. A vendor may provide -a header that contains a custom definition of this macro and specify the path to the header via the -``LIBCXX_ASSERTION_HANDLER_FILE`` CMake variable. If provided, the contents of this header will be -injected into library configuration headers, replacing the default implementation. The header must not -include any standard library headers (directly or transitively) because doing so will almost always -create a circular dependency. - -``_LIBCPP_ASSERTION_HANDLER(error_message, format, args...)`` is a variadic macro that takes the -following parameters: - -* ``error_message`` -- the original error message that explains the hardening failure. In general, it - does not contain information about the source location that triggered the failure. -* ``format`` -- a printf-style format string that contains a general description of the failure with - placeholders for the error message as well as details about the source location. -* ``args...`` -- arguments to substitute in the ``format`` string. The exact order and meaning of the - arguments is unspecified and subject to change (but is always in sync with the format string). Note - that for convenience, ``args`` contain the error message as well. - -Programs that wish to terminate as fast as possible may use the ``error_message`` parameter that -doesn't require any formatting. Programs that prefer having more information about the failure (such as -the filename and the line number of the code that triggered the assertion) should use the printf-style -formatting with ``format`` and ``args...``. - -When a hardening assertion fails, it means that the program is about to invoke undefined behavior. For -this reason, the custom assertion handler is generally expected to terminate the program. If a custom -assertion handler decides to avoid doing so (e.g. it chooses to log and continue instead), it does so -at its own risk -- this approach should only be used in non-production builds and with an understanding -of potential consequences. Furthermore, the custom assertion handler should not throw any exceptions as -it may be invoked from standard library functions that are marked ``noexcept`` (so throwing will result -in ``std::terminate`` being called). - Libc++ Configuration Macros =========================== diff --git a/libcxx/include/__assert b/libcxx/include/__assert index a56e098b52328..0293c77f5b941 100644 --- a/libcxx/include/__assert +++ b/libcxx/include/__assert @@ -20,9 +20,8 @@ #define _LIBCPP_ASSERT(expression, message) \ (__builtin_expect(static_cast(expression), 1) \ ? (void)0 \ - : _LIBCPP_ASSERTION_HANDLER( \ - message, \ - "%s:%d: assertion %s failed: %s\n", __builtin_FILE(), __builtin_LINE(), #expression, message)) + : _LIBCPP_ASSERTION_HANDLER(__FILE__ ":" _LIBCPP_TOSTRING(__LINE__) ": assertion " _LIBCPP_TOSTRING( \ + expression) " failed: " message "\n")) // 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. diff --git a/libcxx/test/support/check_assertion.h b/libcxx/test/support/check_assertion.h index 98dd95b11556e..f3aeb95d99ba7 100644 --- a/libcxx/test/support/check_assertion.h +++ b/libcxx/test/support/check_assertion.h @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -235,18 +236,23 @@ struct DeathTest { }; #ifdef _LIBCPP_VERSION -void std::__libcpp_verbose_abort(char const* format, ...) { - // Extract information from the error message. This has to stay synchronized with - // how we format assertions in the library. - va_list list; - va_start(list, format); - char const* file = va_arg(list, char const*); - int line = va_arg(list, int); - char const* expression = va_arg(list, char const*); (void)expression; - char const* message = va_arg(list, char const*); - va_end(list); - - if (GlobalMatcher().Matches(file, line, message)) { +void std::__libcpp_verbose_abort(char const* message, ...) { + // Extract information from the error message. This has to stay synchronized with how we format assertions in the + // library. Note that the variadic arguments to `__libcpp_verbose_abort` are empty but maintained for backward + // compatibility reasons. + std::regex message_format(R"(.*):(\d+): assertion (.*) failed: (.*)"); + + std::cmatch match_result; + bool has_match = std::regex_match(message, match_result, message_format); + assert(has_match); + assert(match_result.size() == 5); + + std::string file = match_result[1]; + int line = std::stoi(match_result[2]); + // Omitting `expression` in `match_result[3]` + std::string failure_reason = match_result[4]; + + if (GlobalMatcher().Matches(file.c_str(), line, failure_reason.c_str())) { std::exit(DeathTest::RK_MatchFound); } std::exit(DeathTest::RK_MatchFailure); diff --git a/libcxx/vendor/llvm/default_assertion_handler.in b/libcxx/vendor/llvm/default_assertion_handler.in index 9162a19aaf064..7f48107358c25 100644 --- a/libcxx/vendor/llvm/default_assertion_handler.in +++ b/libcxx/vendor/llvm/default_assertion_handler.in @@ -10,13 +10,5 @@ #include <__config> #include <__verbose_abort> -#if _LIBCPP_HARDENING_MODE == _LIBCPP_HARDENING_MODE_DEBUG - -#define _LIBCPP_ASSERTION_HANDLER(error_message, ...) ((void)error_message, _LIBCPP_VERBOSE_ABORT(__VA_ARGS__)) - -#else - // TODO(hardening): in production, trap rather than abort. -#define _LIBCPP_ASSERTION_HANDLER(error_message, ...) ((void)error_message, _LIBCPP_VERBOSE_ABORT(__VA_ARGS__)) - -#endif // #if _LIBCPP_HARDENING_MODE == _LIBCPP_HARDENING_MODE_DEBUG +#define _LIBCPP_ASSERTION_HANDLER(message) _LIBCPP_VERBOSE_ABORT(message) From deae2b81bfd1c14af7aebca9a2099dac32fbca94 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Fri, 12 Jan 2024 22:31:18 -0800 Subject: [PATCH 07/19] Finish addressing feedback --- libcxx/CMakeLists.txt | 4 ++-- libcxx/docs/BuildingLibcxx.rst | 21 ++++++++++++------- libcxx/include/CMakeLists.txt | 7 +++---- libcxx/include/__assert | 2 +- libcxx/include/__assertion_handler.in | 21 ------------------- .../vendor/llvm/default_assertion_handler.in | 9 ++++++++ 6 files changed, 29 insertions(+), 35 deletions(-) delete mode 100644 libcxx/include/__assertion_handler.in diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt index b09836a6ab69c..a4046da847dd8 100644 --- a/libcxx/CMakeLists.txt +++ b/libcxx/CMakeLists.txt @@ -74,8 +74,8 @@ set(LIBCXX_ASSERTION_HANDLER_FILE CACHE STRING "Specify the path to a header that contains a custom implementation of the assertion handler that gets invoked when a hardening assertion fails. If - provided, the contents of this header will get injected into the library code - and override the default assertion handler.") + provided, this header will be included by the library, replacing the + default assertion handler.") 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 diff --git a/libcxx/docs/BuildingLibcxx.rst b/libcxx/docs/BuildingLibcxx.rst index edc7d0163527d..dedf0a516bbb7 100644 --- a/libcxx/docs/BuildingLibcxx.rst +++ b/libcxx/docs/BuildingLibcxx.rst @@ -409,6 +409,15 @@ libc++ Feature Options Use the specified GCC toolchain and standard library when building the native stdlib benchmark tests. +.. option:: LIBCXX_ASSERTION_HANDLER_FILE:PATH + + **Default**:: ``"${CMAKE_CURRENT_SOURCE_DIR}/vendor/llvm/default_assertion_handler.in"`` + + Specify the path to a header that contains a custom implementation of the + assertion handler that gets invoked when a hardening assertion fails. If + provided, this header will be included by the library, replacing the + default assertion handler. + libc++ ABI Feature Options -------------------------- @@ -491,13 +500,11 @@ However, vendors can also override that mechanism at CMake configuration time. When a hardening assertion fails, the library invokes the ``_LIBCPP_ASSERTION_HANDLER`` macro. A vendor may provide a header that contains a custom definition of this macro and specify the path to the header via the -``LIBCXX_ASSERTION_HANDLER_FILE`` CMake variable. If provided, the contents of -this header will be injected into library configuration headers, replacing the -default implementation. The header must not include any standard library headers -(directly or transitively) because doing so will almost always create a circular -dependency. - -The ``_LIBCPP_ASSERTION_HANDLER(message)`` macro takes a single parameter that +``LIBCXX_ASSERTION_HANDLER_FILE`` CMake variable. If provided, this header will +be included by the library and replace the default implementation. The header +must not include any standard library headers (directly or transitively) because +doing so will almost always create a circular dependency. The +``_LIBCPP_ASSERTION_HANDLER(message)`` macro takes a single parameter that contains an error message explaining the hardening failure and some details about the source location that triggered it. diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt index efbce0fee847d..519573aad666e 100644 --- a/libcxx/include/CMakeLists.txt +++ b/libcxx/include/CMakeLists.txt @@ -1019,13 +1019,12 @@ foreach(feature LIBCXX_ENABLE_FILESYSTEM LIBCXX_ENABLE_LOCALIZATION LIBCXX_ENABL endforeach() configure_file("__config_site.in" "${LIBCXX_GENERATED_INCLUDE_TARGET_DIR}/__config_site" @ONLY) -file(READ ${LIBCXX_ASSERTION_HANDLER_FILE} _LIBCPP_ASSERTION_HANDLER_OVERRIDE) -configure_file("__assertion_handler.in" "${LIBCXX_GENERATED_INCLUDE_TARGET_DIR}/__assertion_handler" @ONLY) configure_file("module.modulemap.in" "${LIBCXX_GENERATED_INCLUDE_DIR}/module.modulemap" @ONLY) +configure_file("${LIBCXX_ASSERTION_HANDLER_FILE}" "${LIBCXX_GENERATED_INCLUDE_DIR}/__assertion_handler" @ONLY) set(_all_includes "${LIBCXX_GENERATED_INCLUDE_TARGET_DIR}/__config_site" - "${LIBCXX_GENERATED_INCLUDE_TARGET_DIR}/__assertion_handler" - "${LIBCXX_GENERATED_INCLUDE_DIR}/module.modulemap") + "${LIBCXX_GENERATED_INCLUDE_DIR}/module.modulemap" + "${LIBCXX_GENERATED_INCLUDE_TARGET_DIR}/__assertion_handler") foreach(f ${files}) set(src "${CMAKE_CURRENT_SOURCE_DIR}/${f}") set(dst "${LIBCXX_GENERATED_INCLUDE_DIR}/${f}") diff --git a/libcxx/include/__assert b/libcxx/include/__assert index 0293c77f5b941..eb862b5369b25 100644 --- a/libcxx/include/__assert +++ b/libcxx/include/__assert @@ -10,7 +10,7 @@ #ifndef _LIBCPP___ASSERT #define _LIBCPP___ASSERT -#include <__assertion_handler> +#include <__assertion_handler> // Note: this include is generated by CMake and is potentially vendor-provided. #include <__config> #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) diff --git a/libcxx/include/__assertion_handler.in b/libcxx/include/__assertion_handler.in deleted file mode 100644 index bf4afc31b6e7e..0000000000000 --- a/libcxx/include/__assertion_handler.in +++ /dev/null @@ -1,21 +0,0 @@ -// -*- C++ -*- -//===----------------------------------------------------------------------===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#ifndef _LIBCPP___ASSERTION_HANDLER -#define _LIBCPP___ASSERTION_HANDLER - -#include <__config> - -#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) -# pragma GCC system_header -#endif - -@_LIBCPP_ASSERTION_HANDLER_OVERRIDE@ - -#endif // _LIBCPP___ASSERTION_HANDLER diff --git a/libcxx/vendor/llvm/default_assertion_handler.in b/libcxx/vendor/llvm/default_assertion_handler.in index 7f48107358c25..ca2542aed6ebb 100644 --- a/libcxx/vendor/llvm/default_assertion_handler.in +++ b/libcxx/vendor/llvm/default_assertion_handler.in @@ -7,8 +7,17 @@ // //===----------------------------------------------------------------------===// +#ifndef _LIBCPP___ASSERTION_HANDLER +#define _LIBCPP___ASSERTION_HANDLER + #include <__config> #include <__verbose_abort> +#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) +# pragma GCC system_header +#endif + // TODO(hardening): in production, trap rather than abort. #define _LIBCPP_ASSERTION_HANDLER(message) _LIBCPP_VERBOSE_ABORT(message) + +#endif // _LIBCPP___ASSERTION_HANDLER From d38fe7ee2a9c025bf638edded3cc37ad18c2cd65 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Fri, 12 Jan 2024 22:46:28 -0800 Subject: [PATCH 08/19] - Use the `COPYONLY` option; - Reformat tests; - Remove the header from test scripts in cases where it's no longer relevant. --- libcxx/include/CMakeLists.txt | 2 +- libcxx/test/libcxx/lint/lint_headers.sh.py | 1 - libcxx/test/support/check_assertion.h | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt index 519573aad666e..0ab39874f0cab 100644 --- a/libcxx/include/CMakeLists.txt +++ b/libcxx/include/CMakeLists.txt @@ -1020,7 +1020,7 @@ endforeach() configure_file("__config_site.in" "${LIBCXX_GENERATED_INCLUDE_TARGET_DIR}/__config_site" @ONLY) configure_file("module.modulemap.in" "${LIBCXX_GENERATED_INCLUDE_DIR}/module.modulemap" @ONLY) -configure_file("${LIBCXX_ASSERTION_HANDLER_FILE}" "${LIBCXX_GENERATED_INCLUDE_DIR}/__assertion_handler" @ONLY) +configure_file("${LIBCXX_ASSERTION_HANDLER_FILE}" "${LIBCXX_GENERATED_INCLUDE_DIR}/__assertion_handler" COPYONLY @ONLY) set(_all_includes "${LIBCXX_GENERATED_INCLUDE_TARGET_DIR}/__config_site" "${LIBCXX_GENERATED_INCLUDE_DIR}/module.modulemap" diff --git a/libcxx/test/libcxx/lint/lint_headers.sh.py b/libcxx/test/libcxx/lint/lint_headers.sh.py index aee10e13b9560..ab237c968da7e 100644 --- a/libcxx/test/libcxx/lint/lint_headers.sh.py +++ b/libcxx/test/libcxx/lint/lint_headers.sh.py @@ -14,7 +14,6 @@ def exclude_from_consideration(path): or path.endswith(".modulemap.in") or os.path.basename(path) == "__config" or os.path.basename(path) == "__config_site.in" - or os.path.basename(path) == "__assertion_handler.in" or os.path.basename(path) == "libcxx.imp" or os.path.basename(path).startswith("__pstl") or not os.path.isfile(path) # TODO: Remove once PSTL integration is finished diff --git a/libcxx/test/support/check_assertion.h b/libcxx/test/support/check_assertion.h index f3aeb95d99ba7..d8df46a44a488 100644 --- a/libcxx/test/support/check_assertion.h +++ b/libcxx/test/support/check_assertion.h @@ -248,7 +248,7 @@ void std::__libcpp_verbose_abort(char const* message, ...) { assert(match_result.size() == 5); std::string file = match_result[1]; - int line = std::stoi(match_result[2]); + int line = std::stoi(match_result[2]); // Omitting `expression` in `match_result[3]` std::string failure_reason = match_result[4]; From dc9d771f595fb061f68861ad5202249459664ee0 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Fri, 12 Jan 2024 22:48:58 -0800 Subject: [PATCH 09/19] Remove the new header from another script --- libcxx/utils/generate_iwyu_mapping.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/libcxx/utils/generate_iwyu_mapping.py b/libcxx/utils/generate_iwyu_mapping.py index d5d1577e4a2ad..343538a6cae48 100644 --- a/libcxx/utils/generate_iwyu_mapping.py +++ b/libcxx/utils/generate_iwyu_mapping.py @@ -43,8 +43,6 @@ def generate_map(include): public = [] if i == "__assert": continue - elif i == "__assertion_handler.in": - continue elif i == "__availability": continue elif i == "__bit_reference": From d83f5a13b84331d7cb5fb4c5defb80be364db8c9 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Tue, 16 Jan 2024 14:45:45 -0800 Subject: [PATCH 10/19] Try to fix the bootstrapping CI job (install the header consistently into the generic include directory). --- libcxx/include/CMakeLists.txt | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt index 0ab39874f0cab..9fdf978a89d7e 100644 --- a/libcxx/include/CMakeLists.txt +++ b/libcxx/include/CMakeLists.txt @@ -1020,11 +1020,11 @@ endforeach() configure_file("__config_site.in" "${LIBCXX_GENERATED_INCLUDE_TARGET_DIR}/__config_site" @ONLY) configure_file("module.modulemap.in" "${LIBCXX_GENERATED_INCLUDE_DIR}/module.modulemap" @ONLY) -configure_file("${LIBCXX_ASSERTION_HANDLER_FILE}" "${LIBCXX_GENERATED_INCLUDE_DIR}/__assertion_handler" COPYONLY @ONLY) +configure_file("${LIBCXX_ASSERTION_HANDLER_FILE}" "${LIBCXX_GENERATED_INCLUDE_DIR}/__assertion_handler" COPYONLY) set(_all_includes "${LIBCXX_GENERATED_INCLUDE_TARGET_DIR}/__config_site" "${LIBCXX_GENERATED_INCLUDE_DIR}/module.modulemap" - "${LIBCXX_GENERATED_INCLUDE_TARGET_DIR}/__assertion_handler") + "${LIBCXX_GENERATED_INCLUDE_DIR}/__assertion_handler") foreach(f ${files}) set(src "${CMAKE_CURRENT_SOURCE_DIR}/${f}") set(dst "${LIBCXX_GENERATED_INCLUDE_DIR}/${f}") @@ -1061,9 +1061,9 @@ if (LIBCXX_INSTALL_HEADERS) PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ COMPONENT cxx-headers) - # Install the generated __assertion_handler file to the per-target include dir. - install(FILES "${LIBCXX_GENERATED_INCLUDE_TARGET_DIR}/__assertion_handler" - DESTINATION "${LIBCXX_INSTALL_INCLUDE_TARGET_DIR}" + # Install the generated __assertion_handler file to the generic include dir. + install(FILES "${LIBCXX_GENERATED_INCLUDE_DIR}/__assertion_handler" + DESTINATION "${LIBCXX_INSTALL_INCLUDE_DIR}" PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ COMPONENT cxx-headers) From 48db22a1201d97874920afb8b946b6f9f3a669b0 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Tue, 16 Jan 2024 17:19:16 -0800 Subject: [PATCH 11/19] Fix the regex --- libcxx/test/support/check_assertion.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcxx/test/support/check_assertion.h b/libcxx/test/support/check_assertion.h index d8df46a44a488..8c08fbb3cd29f 100644 --- a/libcxx/test/support/check_assertion.h +++ b/libcxx/test/support/check_assertion.h @@ -240,7 +240,7 @@ void std::__libcpp_verbose_abort(char const* message, ...) { // Extract information from the error message. This has to stay synchronized with how we format assertions in the // library. Note that the variadic arguments to `__libcpp_verbose_abort` are empty but maintained for backward // compatibility reasons. - std::regex message_format(R"(.*):(\d+): assertion (.*) failed: (.*)"); + std::regex message_format("(.*):(\\d+): assertion (.*) failed: (.*)\\n"); std::cmatch match_result; bool has_match = std::regex_match(message, match_result, message_format); From 230de80d045cdabd29942a57667ceb965f537455 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Tue, 16 Jan 2024 17:47:44 -0800 Subject: [PATCH 12/19] Workarounds for the `%` character in asserted expressions. --- libcxx/include/__memory/assume_aligned.h | 9 ++++++++- libcxx/src/include/to_chars_floating_point.h | 18 ++++++++++++++++-- libcxx/src/memory_resource.cpp | 11 ++++++++++- 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/libcxx/include/__memory/assume_aligned.h b/libcxx/include/__memory/assume_aligned.h index c66fb49ebb3c0..d188dfe78032d 100644 --- a/libcxx/include/__memory/assume_aligned.h +++ b/libcxx/include/__memory/assume_aligned.h @@ -29,7 +29,14 @@ _LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 _Tp* __ass if (__libcpp_is_constant_evaluated()) { return __ptr; } else { - _LIBCPP_ASSERT_UNCATEGORIZED(reinterpret_cast(__ptr) % _Np == 0, "Alignment assumption is violated"); + { + // Note: the `%` symbol is used to introduce a conversion specifier in printf format syntax and thus cannot appear + // in an expression passed to a hardening assertion (because that expression might end up being passed to + // a printf-style formatting function). As a workaround, wrap the expression in a lambda. + auto __is_aligned = [&] { return reinterpret_cast(__ptr) % _Np == 0; }; + (void)__is_aligned; // Prevent "maybe unused" warnings in modes which don't enable the assertion. + _LIBCPP_ASSERT_UNCATEGORIZED(__is_aligned(), "Alignment assumption is violated"); + } return static_cast<_Tp*>(__builtin_assume_aligned(__ptr, _Np)); } } diff --git a/libcxx/src/include/to_chars_floating_point.h b/libcxx/src/include/to_chars_floating_point.h index e4715d10d97da..35ca00ed70595 100644 --- a/libcxx/src/include/to_chars_floating_point.h +++ b/libcxx/src/include/to_chars_floating_point.h @@ -289,7 +289,14 @@ to_chars_result _Floating_to_chars_hex_precision( for (;;) { _LIBCPP_ASSERT_INTERNAL(_Number_of_bits_remaining >= 4, ""); - _LIBCPP_ASSERT_INTERNAL(_Number_of_bits_remaining % 4 == 0, ""); + { + // Note: the `%` symbol is used to introduce a conversion specifier in printf format syntax and thus + // cannot appear in an expression passed to a hardening assertion (because that expression might end up + // being passed to a printf-style formatting function). As a workaround, wrap the expression in a lambda. + auto __is_aligned = [&] { return _Number_of_bits_remaining % 4 == 0; }; + (void)__is_aligned; // Prevent "maybe unused" warnings in modes which don't enable the assertion. + _LIBCPP_ASSERT_INTERNAL(__is_aligned(), ""); + } _Number_of_bits_remaining -= 4; const uint32_t _Nibble = static_cast(_Adjusted_mantissa >> _Number_of_bits_remaining); @@ -416,7 +423,14 @@ to_chars_result _Floating_to_chars_hex_shortest( // _Number_of_bits_remaining. do { _LIBCPP_ASSERT_INTERNAL(_Number_of_bits_remaining >= 4, ""); - _LIBCPP_ASSERT_INTERNAL(_Number_of_bits_remaining % 4 == 0, ""); + { + // Note: the `%` symbol is used to introduce a conversion specifier in printf format syntax and thus + // cannot appear in an expression passed to a hardening assertion (because that expression might end up + // being passed to a printf-style formatting function). As a workaround, wrap the expression in a lambda. + auto __is_aligned = [&] { return _Number_of_bits_remaining % 4 == 0; }; + (void)__is_aligned; // Prevent "maybe unused" warnings in modes which don't enable the assertion. + _LIBCPP_ASSERT_INTERNAL(__is_aligned(), ""); + } _Number_of_bits_remaining -= 4; const uint32_t _Nibble = static_cast(_Adjusted_mantissa >> _Number_of_bits_remaining); diff --git a/libcxx/src/memory_resource.cpp b/libcxx/src/memory_resource.cpp index 42c366893f736..b46b8d8314811 100644 --- a/libcxx/src/memory_resource.cpp +++ b/libcxx/src/memory_resource.cpp @@ -230,7 +230,16 @@ class unsynchronized_pool_resource::__fixed_pool { } void* __allocate_in_new_chunk(memory_resource* upstream, size_t block_size, size_t chunk_size) { - _LIBCPP_ASSERT_INTERNAL(chunk_size % block_size == 0, ""); + { + // Note: the `%` symbol is used to introduce a conversion specifier in printf format syntax and thus cannot appear + // in an expression passed to a hardening assertion (because that expression might end up being passed to + // a printf-style formatting function). As a workaround, wrap the expression in a lambda. + auto __is_aligned = [&] { + return chunk_size % block_size == 0; + }; + (void)__is_aligned; // Prevent "maybe unused" warnings in modes which don't enable the assertion. + _LIBCPP_ASSERT_INTERNAL(__is_aligned(), ""); + } static_assert(__default_alignment >= alignof(std::max_align_t), ""); static_assert(__default_alignment >= alignof(__chunk_footer), ""); static_assert(__default_alignment >= alignof(__vacancy_header), ""); From 5dd75886766ce10ed75a5ab1e93f61875598e0d6 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Tue, 16 Jan 2024 17:53:55 -0800 Subject: [PATCH 13/19] Formatting --- libcxx/src/memory_resource.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/libcxx/src/memory_resource.cpp b/libcxx/src/memory_resource.cpp index b46b8d8314811..84bdbe5088a60 100644 --- a/libcxx/src/memory_resource.cpp +++ b/libcxx/src/memory_resource.cpp @@ -234,9 +234,7 @@ class unsynchronized_pool_resource::__fixed_pool { // Note: the `%` symbol is used to introduce a conversion specifier in printf format syntax and thus cannot appear // in an expression passed to a hardening assertion (because that expression might end up being passed to // a printf-style formatting function). As a workaround, wrap the expression in a lambda. - auto __is_aligned = [&] { - return chunk_size % block_size == 0; - }; + auto __is_aligned = [&] { return chunk_size % block_size == 0; }; (void)__is_aligned; // Prevent "maybe unused" warnings in modes which don't enable the assertion. _LIBCPP_ASSERT_INTERNAL(__is_aligned(), ""); } From b6bcc169a23c95677cd2a6eda9d5fcec2ffcefa0 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Tue, 16 Jan 2024 18:20:33 -0800 Subject: [PATCH 14/19] Fix the CI. --- libcxx/include/__memory/assume_aligned.h | 7 ++++--- libcxx/src/include/to_chars_floating_point.h | 14 ++++++++------ libcxx/src/memory_resource.cpp | 7 ++++--- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/libcxx/include/__memory/assume_aligned.h b/libcxx/include/__memory/assume_aligned.h index d188dfe78032d..aeb6de77f2a57 100644 --- a/libcxx/include/__memory/assume_aligned.h +++ b/libcxx/include/__memory/assume_aligned.h @@ -32,10 +32,11 @@ _LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 _Tp* __ass { // Note: the `%` symbol is used to introduce a conversion specifier in printf format syntax and thus cannot appear // in an expression passed to a hardening assertion (because that expression might end up being passed to - // a printf-style formatting function). As a workaround, wrap the expression in a lambda. - auto __is_aligned = [&] { return reinterpret_cast(__ptr) % _Np == 0; }; + // a printf-style formatting function). As a workaround, use an intermediate variable (lambdas aren't supported in + // C++03). + bool __is_aligned = reinterpret_cast(__ptr) % _Np == 0; (void)__is_aligned; // Prevent "maybe unused" warnings in modes which don't enable the assertion. - _LIBCPP_ASSERT_UNCATEGORIZED(__is_aligned(), "Alignment assumption is violated"); + _LIBCPP_ASSERT_UNCATEGORIZED(__is_aligned, "Alignment assumption is violated"); } return static_cast<_Tp*>(__builtin_assume_aligned(__ptr, _Np)); } diff --git a/libcxx/src/include/to_chars_floating_point.h b/libcxx/src/include/to_chars_floating_point.h index 35ca00ed70595..40179fc616c97 100644 --- a/libcxx/src/include/to_chars_floating_point.h +++ b/libcxx/src/include/to_chars_floating_point.h @@ -292,10 +292,11 @@ to_chars_result _Floating_to_chars_hex_precision( { // Note: the `%` symbol is used to introduce a conversion specifier in printf format syntax and thus // cannot appear in an expression passed to a hardening assertion (because that expression might end up - // being passed to a printf-style formatting function). As a workaround, wrap the expression in a lambda. - auto __is_aligned = [&] { return _Number_of_bits_remaining % 4 == 0; }; + // being passed to a printf-style formatting function). As a workaround, use an intermediate variable + // (lambdas aren't supported in C++03). + bool __is_aligned = _Number_of_bits_remaining % 4 == 0; (void)__is_aligned; // Prevent "maybe unused" warnings in modes which don't enable the assertion. - _LIBCPP_ASSERT_INTERNAL(__is_aligned(), ""); + _LIBCPP_ASSERT_INTERNAL(__is_aligned, ""); } _Number_of_bits_remaining -= 4; @@ -426,10 +427,11 @@ to_chars_result _Floating_to_chars_hex_shortest( { // Note: the `%` symbol is used to introduce a conversion specifier in printf format syntax and thus // cannot appear in an expression passed to a hardening assertion (because that expression might end up - // being passed to a printf-style formatting function). As a workaround, wrap the expression in a lambda. - auto __is_aligned = [&] { return _Number_of_bits_remaining % 4 == 0; }; + // being passed to a printf-style formatting function). As a workaround, use an intermediate variable + // (lambdas aren't supported in C++03). + bool __is_aligned = _Number_of_bits_remaining % 4 == 0; (void)__is_aligned; // Prevent "maybe unused" warnings in modes which don't enable the assertion. - _LIBCPP_ASSERT_INTERNAL(__is_aligned(), ""); + _LIBCPP_ASSERT_INTERNAL(__is_aligned, ""); } _Number_of_bits_remaining -= 4; diff --git a/libcxx/src/memory_resource.cpp b/libcxx/src/memory_resource.cpp index 84bdbe5088a60..adc4747dc0135 100644 --- a/libcxx/src/memory_resource.cpp +++ b/libcxx/src/memory_resource.cpp @@ -233,10 +233,11 @@ class unsynchronized_pool_resource::__fixed_pool { { // Note: the `%` symbol is used to introduce a conversion specifier in printf format syntax and thus cannot appear // in an expression passed to a hardening assertion (because that expression might end up being passed to - // a printf-style formatting function). As a workaround, wrap the expression in a lambda. - auto __is_aligned = [&] { return chunk_size % block_size == 0; }; + // a printf-style formatting function). As a workaround, use an intermediate variable (lambdas aren't supported in + // C++03). + bool __is_aligned = chunk_size % block_size == 0; (void)__is_aligned; // Prevent "maybe unused" warnings in modes which don't enable the assertion. - _LIBCPP_ASSERT_INTERNAL(__is_aligned(), ""); + _LIBCPP_ASSERT_INTERNAL(__is_aligned, ""); } static_assert(__default_alignment >= alignof(std::max_align_t), ""); static_assert(__default_alignment >= alignof(__chunk_footer), ""); From 1f5cab99f2cd66620ee966fe9779522cd9c290c2 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Tue, 16 Jan 2024 23:25:52 -0800 Subject: [PATCH 15/19] Mark the tests failing in `no-localization` configuration because of the new regex include as unsupported --- .../enabling_assertions_enables_extensive_mode.pass.cpp | 4 ++-- .../assertions/modes/override_with_debug_mode.pass.cpp | 6 ++---- .../assertions/modes/override_with_extensive_mode.pass.cpp | 6 ++---- .../assertions/modes/override_with_fast_mode.pass.cpp | 6 ++---- .../assertions/modes/override_with_unchecked_mode.pass.cpp | 6 ++---- .../containers/sequences/deque/asan_caterpillar.pass.cpp | 4 ++-- .../alg.fill/pstl.exception_handling.pass.cpp | 3 ++- .../alg.move/pstl.exception_handling.pass.cpp | 3 ++- .../alg.replace/pstl.exception_handling.pass.cpp | 3 ++- .../alg.rotate/pstl.exception_handling.pass.cpp | 3 ++- .../alg.transform/pstl.exception_handling.pass.cpp | 3 ++- .../alg.all_of/pstl.exception_handling.pass.cpp | 3 ++- .../alg.any_of/pstl.exception_handling.pass.cpp | 3 ++- .../alg.equal/pstl.exception_handling.pass.cpp | 3 ++- .../alg.find/pstl.exception_handling.pass.cpp | 3 ++- .../alg.foreach/pstl.exception_handling.pass.cpp | 3 ++- .../alg.none_of/pstl.exception_handling.pass.cpp | 3 ++- .../alg.sorting/alg.merge/pstl.exception_handling.pass.cpp | 3 ++- .../alg.sort/stable.sort/pstl.exception_handling.pass.cpp | 3 ++- .../numeric.ops/reduce/pstl.exception_handling.pass.cpp | 3 ++- .../transform.reduce/pstl.exception_handling.pass.cpp | 3 ++- 21 files changed, 42 insertions(+), 35 deletions(-) diff --git a/libcxx/test/libcxx/assertions/modes/enabling_assertions_enables_extensive_mode.pass.cpp b/libcxx/test/libcxx/assertions/modes/enabling_assertions_enables_extensive_mode.pass.cpp index be08159d561bb..a40ae84fa8e85 100644 --- a/libcxx/test/libcxx/assertions/modes/enabling_assertions_enables_extensive_mode.pass.cpp +++ b/libcxx/test/libcxx/assertions/modes/enabling_assertions_enables_extensive_mode.pass.cpp @@ -10,8 +10,8 @@ // This test ensures that enabling assertions with the legacy `_LIBCPP_ENABLE_ASSERTIONS` now enables the extensive // hardening mode. -// `check_assertion.h` is only available starting from C++11 and requires Unix headers. -// UNSUPPORTED: c++03, !has-unix-headers +// `check_assertion.h` is only available starting from C++11 and requires Unix headers and regex support. +// UNSUPPORTED: c++03, !has-unix-headers, no-localization // The ability to set a custom abort message is required to compare the assertion message. // XFAIL: availability-verbose_abort-missing // Note that GCC doesn't support `-Wno-macro-redefined`. diff --git a/libcxx/test/libcxx/assertions/modes/override_with_debug_mode.pass.cpp b/libcxx/test/libcxx/assertions/modes/override_with_debug_mode.pass.cpp index 665babf59c259..f49ad490780c3 100644 --- a/libcxx/test/libcxx/assertions/modes/override_with_debug_mode.pass.cpp +++ b/libcxx/test/libcxx/assertions/modes/override_with_debug_mode.pass.cpp @@ -8,10 +8,8 @@ // This test ensures that we can override any hardening mode with the debug mode on a per-TU basis. -// `check_assertion.h` is only available starting from C++11. -// UNSUPPORTED: c++03 -// `check_assertion.h` requires Unix headers. -// REQUIRES: has-unix-headers +// `check_assertion.h` is only available starting from C++11 and requires Unix headers and regex support. +// UNSUPPORTED: c++03, !has-unix-headers, no-localization // The ability to set a custom abort message is required to compare the assertion message. // XFAIL: availability-verbose_abort-missing // ADDITIONAL_COMPILE_FLAGS: -U_LIBCPP_HARDENING_MODE -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_DEBUG diff --git a/libcxx/test/libcxx/assertions/modes/override_with_extensive_mode.pass.cpp b/libcxx/test/libcxx/assertions/modes/override_with_extensive_mode.pass.cpp index e2b0ea7208118..f5323c671c9b1 100644 --- a/libcxx/test/libcxx/assertions/modes/override_with_extensive_mode.pass.cpp +++ b/libcxx/test/libcxx/assertions/modes/override_with_extensive_mode.pass.cpp @@ -8,10 +8,8 @@ // This test ensures that we can override any hardening mode with the extensive hardening mode on a per-TU basis. -// `check_assertion.h` is only available starting from C++11. -// UNSUPPORTED: c++03 -// `check_assertion.h` requires Unix headers. -// REQUIRES: has-unix-headers +// `check_assertion.h` is only available starting from C++11 and requires Unix headers and regex support. +// UNSUPPORTED: c++03, !has-unix-headers, no-localization // The ability to set a custom abort message is required to compare the assertion message. // XFAIL: availability-verbose_abort-missing // ADDITIONAL_COMPILE_FLAGS: -U_LIBCPP_HARDENING_MODE -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_EXTENSIVE diff --git a/libcxx/test/libcxx/assertions/modes/override_with_fast_mode.pass.cpp b/libcxx/test/libcxx/assertions/modes/override_with_fast_mode.pass.cpp index b38afed17e567..5ee22cc45f3bf 100644 --- a/libcxx/test/libcxx/assertions/modes/override_with_fast_mode.pass.cpp +++ b/libcxx/test/libcxx/assertions/modes/override_with_fast_mode.pass.cpp @@ -8,10 +8,8 @@ // This test ensures that we can override any hardening mode with the fast mode on a per-TU basis. -// `check_assertion.h` is only available starting from C++11. -// UNSUPPORTED: c++03 -// `check_assertion.h` requires Unix headers. -// REQUIRES: has-unix-headers +// `check_assertion.h` is only available starting from C++11 and requires Unix headers and regex support. +// UNSUPPORTED: c++03, !has-unix-headers, no-localization // The ability to set a custom abort message is required to compare the assertion message. // XFAIL: availability-verbose_abort-missing // ADDITIONAL_COMPILE_FLAGS: -U_LIBCPP_HARDENING_MODE -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_FAST diff --git a/libcxx/test/libcxx/assertions/modes/override_with_unchecked_mode.pass.cpp b/libcxx/test/libcxx/assertions/modes/override_with_unchecked_mode.pass.cpp index 65707a9863d07..8ea8b731ec4d3 100644 --- a/libcxx/test/libcxx/assertions/modes/override_with_unchecked_mode.pass.cpp +++ b/libcxx/test/libcxx/assertions/modes/override_with_unchecked_mode.pass.cpp @@ -8,10 +8,8 @@ // This test ensures that we can override any hardening mode with the unchecked mode on a per-TU basis. -// `check_assertion.h` is only available starting from C++11. -// UNSUPPORTED: c++03 -// `check_assertion.h` requires Unix headers. -// REQUIRES: has-unix-headers +// `check_assertion.h` is only available starting from C++11 and requires Unix headers and regex support. +// UNSUPPORTED: c++03, !has-unix-headers, no-localization // ADDITIONAL_COMPILE_FLAGS: -U_LIBCPP_HARDENING_MODE -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_NONE #include diff --git a/libcxx/test/libcxx/containers/sequences/deque/asan_caterpillar.pass.cpp b/libcxx/test/libcxx/containers/sequences/deque/asan_caterpillar.pass.cpp index b99b2737bce02..d3c6e0493ee28 100644 --- a/libcxx/test/libcxx/containers/sequences/deque/asan_caterpillar.pass.cpp +++ b/libcxx/test/libcxx/containers/sequences/deque/asan_caterpillar.pass.cpp @@ -11,8 +11,8 @@ // Regression test to error in deque::__annotate_from_to in deque, // with origin in deque::__add_back_capacity. -// REQUIRES: has-unix-headers -// UNSUPPORTED: c++03 +// `check_assertion.h` is only available starting from C++11 and requires Unix headers and regex support. +// UNSUPPORTED: c++03, !has-unix-headers, no-localization #include #include diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/pstl.exception_handling.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/pstl.exception_handling.pass.cpp index f7279655cb129..dda642be85bc0 100644 --- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/pstl.exception_handling.pass.cpp +++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/pstl.exception_handling.pass.cpp @@ -8,7 +8,8 @@ // UNSUPPORTED: c++03, c++11, c++14 // UNSUPPORTED: no-exceptions -// REQUIRES: has-unix-headers +// `check_assertion.h` requires Unix headers and regex support. +// UNSUPPORTED: !has-unix-headers, no-localization // UNSUPPORTED: libcpp-has-no-incomplete-pstl diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/pstl.exception_handling.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/pstl.exception_handling.pass.cpp index ffe4f1866ebe6..bb8ab42172226 100644 --- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/pstl.exception_handling.pass.cpp +++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.move/pstl.exception_handling.pass.cpp @@ -8,7 +8,8 @@ // UNSUPPORTED: c++03, c++11, c++14 // UNSUPPORTED: no-exceptions -// REQUIRES: has-unix-headers +// `check_assertion.h` requires Unix headers and regex support. +// UNSUPPORTED: !has-unix-headers, no-localization // UNSUPPORTED: libcpp-has-no-incomplete-pstl diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.replace/pstl.exception_handling.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.replace/pstl.exception_handling.pass.cpp index 98fde95fb0e44..c02496bef4212 100644 --- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.replace/pstl.exception_handling.pass.cpp +++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.replace/pstl.exception_handling.pass.cpp @@ -8,7 +8,8 @@ // UNSUPPORTED: c++03, c++11, c++14 // UNSUPPORTED: no-exceptions -// REQUIRES: has-unix-headers +// `check_assertion.h` requires Unix headers and regex support. +// UNSUPPORTED: !has-unix-headers, no-localization // UNSUPPORTED: libcpp-has-no-incomplete-pstl diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.rotate/pstl.exception_handling.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.rotate/pstl.exception_handling.pass.cpp index d0b77661334d7..88d177a6e39f4 100644 --- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.rotate/pstl.exception_handling.pass.cpp +++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.rotate/pstl.exception_handling.pass.cpp @@ -8,7 +8,8 @@ // UNSUPPORTED: c++03, c++11, c++14 // UNSUPPORTED: no-exceptions -// REQUIRES: has-unix-headers +// `check_assertion.h` requires Unix headers and regex support. +// UNSUPPORTED: !has-unix-headers, no-localization // UNSUPPORTED: libcpp-has-no-incomplete-pstl diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.transform/pstl.exception_handling.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.transform/pstl.exception_handling.pass.cpp index 6c7149f4048ad..439204060e189 100644 --- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.transform/pstl.exception_handling.pass.cpp +++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.transform/pstl.exception_handling.pass.cpp @@ -8,7 +8,8 @@ // UNSUPPORTED: c++03, c++11, c++14 // UNSUPPORTED: no-exceptions -// REQUIRES: has-unix-headers +// `check_assertion.h` requires Unix headers and regex support. +// UNSUPPORTED: !has-unix-headers, no-localization // UNSUPPORTED: libcpp-has-no-incomplete-pstl diff --git a/libcxx/test/std/algorithms/alg.nonmodifying/alg.all_of/pstl.exception_handling.pass.cpp b/libcxx/test/std/algorithms/alg.nonmodifying/alg.all_of/pstl.exception_handling.pass.cpp index ae5063c5264ea..d1c031bdd97a2 100644 --- a/libcxx/test/std/algorithms/alg.nonmodifying/alg.all_of/pstl.exception_handling.pass.cpp +++ b/libcxx/test/std/algorithms/alg.nonmodifying/alg.all_of/pstl.exception_handling.pass.cpp @@ -8,7 +8,8 @@ // UNSUPPORTED: c++03, c++11, c++14 // UNSUPPORTED: no-exceptions -// REQUIRES: has-unix-headers +// `check_assertion.h` requires Unix headers and regex support. +// UNSUPPORTED: !has-unix-headers, no-localization // UNSUPPORTED: libcpp-has-no-incomplete-pstl diff --git a/libcxx/test/std/algorithms/alg.nonmodifying/alg.any_of/pstl.exception_handling.pass.cpp b/libcxx/test/std/algorithms/alg.nonmodifying/alg.any_of/pstl.exception_handling.pass.cpp index 167c4ac4df95e..58fe79b34c008 100644 --- a/libcxx/test/std/algorithms/alg.nonmodifying/alg.any_of/pstl.exception_handling.pass.cpp +++ b/libcxx/test/std/algorithms/alg.nonmodifying/alg.any_of/pstl.exception_handling.pass.cpp @@ -8,7 +8,8 @@ // UNSUPPORTED: c++03, c++11, c++14 // UNSUPPORTED: no-exceptions -// REQUIRES: has-unix-headers +// `check_assertion.h` requires Unix headers and regex support. +// UNSUPPORTED: !has-unix-headers, no-localization // UNSUPPORTED: libcpp-has-no-incomplete-pstl diff --git a/libcxx/test/std/algorithms/alg.nonmodifying/alg.equal/pstl.exception_handling.pass.cpp b/libcxx/test/std/algorithms/alg.nonmodifying/alg.equal/pstl.exception_handling.pass.cpp index 4c2f9946165e7..1bcd858f3c02d 100644 --- a/libcxx/test/std/algorithms/alg.nonmodifying/alg.equal/pstl.exception_handling.pass.cpp +++ b/libcxx/test/std/algorithms/alg.nonmodifying/alg.equal/pstl.exception_handling.pass.cpp @@ -8,7 +8,8 @@ // UNSUPPORTED: c++03, c++11, c++14 // UNSUPPORTED: no-exceptions -// REQUIRES: has-unix-headers +// `check_assertion.h` requires Unix headers and regex support. +// UNSUPPORTED: !has-unix-headers, no-localization // UNSUPPORTED: libcpp-has-no-incomplete-pstl diff --git a/libcxx/test/std/algorithms/alg.nonmodifying/alg.find/pstl.exception_handling.pass.cpp b/libcxx/test/std/algorithms/alg.nonmodifying/alg.find/pstl.exception_handling.pass.cpp index 06b0810c257b3..b0ee4f8d062ef 100644 --- a/libcxx/test/std/algorithms/alg.nonmodifying/alg.find/pstl.exception_handling.pass.cpp +++ b/libcxx/test/std/algorithms/alg.nonmodifying/alg.find/pstl.exception_handling.pass.cpp @@ -8,7 +8,8 @@ // UNSUPPORTED: c++03, c++11, c++14 // UNSUPPORTED: no-exceptions -// REQUIRES: has-unix-headers +// `check_assertion.h` requires Unix headers and regex support. +// UNSUPPORTED: !has-unix-headers, no-localization // UNSUPPORTED: libcpp-has-no-incomplete-pstl diff --git a/libcxx/test/std/algorithms/alg.nonmodifying/alg.foreach/pstl.exception_handling.pass.cpp b/libcxx/test/std/algorithms/alg.nonmodifying/alg.foreach/pstl.exception_handling.pass.cpp index ab15bdf5d8b99..a63276f1e025d 100644 --- a/libcxx/test/std/algorithms/alg.nonmodifying/alg.foreach/pstl.exception_handling.pass.cpp +++ b/libcxx/test/std/algorithms/alg.nonmodifying/alg.foreach/pstl.exception_handling.pass.cpp @@ -8,7 +8,8 @@ // UNSUPPORTED: c++03, c++11, c++14 // UNSUPPORTED: no-exceptions -// REQUIRES: has-unix-headers +// `check_assertion.h` requires Unix headers and regex support. +// UNSUPPORTED: !has-unix-headers, no-localization // UNSUPPORTED: libcpp-has-no-incomplete-pstl diff --git a/libcxx/test/std/algorithms/alg.nonmodifying/alg.none_of/pstl.exception_handling.pass.cpp b/libcxx/test/std/algorithms/alg.nonmodifying/alg.none_of/pstl.exception_handling.pass.cpp index 9a6d8d636200c..26e6fea5904fe 100644 --- a/libcxx/test/std/algorithms/alg.nonmodifying/alg.none_of/pstl.exception_handling.pass.cpp +++ b/libcxx/test/std/algorithms/alg.nonmodifying/alg.none_of/pstl.exception_handling.pass.cpp @@ -8,7 +8,8 @@ // UNSUPPORTED: c++03, c++11, c++14 // UNSUPPORTED: no-exceptions -// REQUIRES: has-unix-headers +// `check_assertion.h` requires Unix headers and regex support. +// UNSUPPORTED: !has-unix-headers, no-localization // UNSUPPORTED: libcpp-has-no-incomplete-pstl diff --git a/libcxx/test/std/algorithms/alg.sorting/alg.merge/pstl.exception_handling.pass.cpp b/libcxx/test/std/algorithms/alg.sorting/alg.merge/pstl.exception_handling.pass.cpp index b56959809829b..b48a5a9fa2b7d 100644 --- a/libcxx/test/std/algorithms/alg.sorting/alg.merge/pstl.exception_handling.pass.cpp +++ b/libcxx/test/std/algorithms/alg.sorting/alg.merge/pstl.exception_handling.pass.cpp @@ -8,7 +8,8 @@ // UNSUPPORTED: c++03, c++11, c++14 // UNSUPPORTED: no-exceptions -// REQUIRES: has-unix-headers +// `check_assertion.h` requires Unix headers and regex support. +// UNSUPPORTED: !has-unix-headers, no-localization // UNSUPPORTED: libcpp-has-no-incomplete-pstl diff --git a/libcxx/test/std/algorithms/alg.sorting/alg.sort/stable.sort/pstl.exception_handling.pass.cpp b/libcxx/test/std/algorithms/alg.sorting/alg.sort/stable.sort/pstl.exception_handling.pass.cpp index 43ee937a8a185..1dc603cfaa554 100644 --- a/libcxx/test/std/algorithms/alg.sorting/alg.sort/stable.sort/pstl.exception_handling.pass.cpp +++ b/libcxx/test/std/algorithms/alg.sorting/alg.sort/stable.sort/pstl.exception_handling.pass.cpp @@ -8,7 +8,8 @@ // UNSUPPORTED: c++03, c++11, c++14 // UNSUPPORTED: no-exceptions -// REQUIRES: has-unix-headers +// `check_assertion.h` requires Unix headers and regex support. +// UNSUPPORTED: !has-unix-headers, no-localization // UNSUPPORTED: libcpp-has-no-incomplete-pstl diff --git a/libcxx/test/std/algorithms/numeric.ops/reduce/pstl.exception_handling.pass.cpp b/libcxx/test/std/algorithms/numeric.ops/reduce/pstl.exception_handling.pass.cpp index 65a112774ff76..d52889b1be147 100644 --- a/libcxx/test/std/algorithms/numeric.ops/reduce/pstl.exception_handling.pass.cpp +++ b/libcxx/test/std/algorithms/numeric.ops/reduce/pstl.exception_handling.pass.cpp @@ -8,7 +8,8 @@ // UNSUPPORTED: c++03, c++11, c++14 // UNSUPPORTED: no-exceptions -// REQUIRES: has-unix-headers +// `check_assertion.h` requires Unix headers and regex support. +// UNSUPPORTED: !has-unix-headers, no-localization // UNSUPPORTED: libcpp-has-no-incomplete-pstl diff --git a/libcxx/test/std/algorithms/numeric.ops/transform.reduce/pstl.exception_handling.pass.cpp b/libcxx/test/std/algorithms/numeric.ops/transform.reduce/pstl.exception_handling.pass.cpp index d2df251d5c8ad..5ac04334f0005 100644 --- a/libcxx/test/std/algorithms/numeric.ops/transform.reduce/pstl.exception_handling.pass.cpp +++ b/libcxx/test/std/algorithms/numeric.ops/transform.reduce/pstl.exception_handling.pass.cpp @@ -8,7 +8,8 @@ // UNSUPPORTED: c++03, c++11, c++14 // UNSUPPORTED: no-exceptions -// REQUIRES: has-unix-headers +// `check_assertion.h` requires Unix headers and regex support. +// UNSUPPORTED: !has-unix-headers, no-localization // UNSUPPORTED: libcpp-has-no-incomplete-pstl From 0bef0fb7dd80c7949f797ed425f978a4404cc556 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Wed, 17 Jan 2024 01:31:49 -0800 Subject: [PATCH 16/19] Poke CI --- libcxx/test/support/check_assertion.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcxx/test/support/check_assertion.h b/libcxx/test/support/check_assertion.h index 8c08fbb3cd29f..0c6609ecd7aeb 100644 --- a/libcxx/test/support/check_assertion.h +++ b/libcxx/test/support/check_assertion.h @@ -239,7 +239,7 @@ struct DeathTest { void std::__libcpp_verbose_abort(char const* message, ...) { // Extract information from the error message. This has to stay synchronized with how we format assertions in the // library. Note that the variadic arguments to `__libcpp_verbose_abort` are empty but maintained for backward - // compatibility reasons. + // compatibility reasons std::regex message_format("(.*):(\\d+): assertion (.*) failed: (.*)\\n"); std::cmatch match_result; From cfa7d3431e57d0e4f90b9daaffb04032aaebdbfa Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Wed, 17 Jan 2024 01:31:58 -0800 Subject: [PATCH 17/19] Poke CI --- libcxx/test/support/check_assertion.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcxx/test/support/check_assertion.h b/libcxx/test/support/check_assertion.h index 0c6609ecd7aeb..8c08fbb3cd29f 100644 --- a/libcxx/test/support/check_assertion.h +++ b/libcxx/test/support/check_assertion.h @@ -239,7 +239,7 @@ struct DeathTest { void std::__libcpp_verbose_abort(char const* message, ...) { // Extract information from the error message. This has to stay synchronized with how we format assertions in the // library. Note that the variadic arguments to `__libcpp_verbose_abort` are empty but maintained for backward - // compatibility reasons + // compatibility reasons. std::regex message_format("(.*):(\\d+): assertion (.*) failed: (.*)\\n"); std::cmatch match_result; From 6cb68e64bf388fdf0afc83b2c5fdcc3ff79d3755 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Wed, 17 Jan 2024 13:17:56 -0800 Subject: [PATCH 18/19] Better way to work around the `%` issue --- libcxx/include/__memory/assume_aligned.h | 10 +--------- libcxx/src/include/to_chars_floating_point.h | 20 ++----------------- libcxx/src/memory_resource.cpp | 10 +--------- .../vendor/llvm/default_assertion_handler.in | 2 +- 4 files changed, 5 insertions(+), 37 deletions(-) diff --git a/libcxx/include/__memory/assume_aligned.h b/libcxx/include/__memory/assume_aligned.h index aeb6de77f2a57..c66fb49ebb3c0 100644 --- a/libcxx/include/__memory/assume_aligned.h +++ b/libcxx/include/__memory/assume_aligned.h @@ -29,15 +29,7 @@ _LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 _Tp* __ass if (__libcpp_is_constant_evaluated()) { return __ptr; } else { - { - // Note: the `%` symbol is used to introduce a conversion specifier in printf format syntax and thus cannot appear - // in an expression passed to a hardening assertion (because that expression might end up being passed to - // a printf-style formatting function). As a workaround, use an intermediate variable (lambdas aren't supported in - // C++03). - bool __is_aligned = reinterpret_cast(__ptr) % _Np == 0; - (void)__is_aligned; // Prevent "maybe unused" warnings in modes which don't enable the assertion. - _LIBCPP_ASSERT_UNCATEGORIZED(__is_aligned, "Alignment assumption is violated"); - } + _LIBCPP_ASSERT_UNCATEGORIZED(reinterpret_cast(__ptr) % _Np == 0, "Alignment assumption is violated"); return static_cast<_Tp*>(__builtin_assume_aligned(__ptr, _Np)); } } diff --git a/libcxx/src/include/to_chars_floating_point.h b/libcxx/src/include/to_chars_floating_point.h index 40179fc616c97..e4715d10d97da 100644 --- a/libcxx/src/include/to_chars_floating_point.h +++ b/libcxx/src/include/to_chars_floating_point.h @@ -289,15 +289,7 @@ to_chars_result _Floating_to_chars_hex_precision( for (;;) { _LIBCPP_ASSERT_INTERNAL(_Number_of_bits_remaining >= 4, ""); - { - // Note: the `%` symbol is used to introduce a conversion specifier in printf format syntax and thus - // cannot appear in an expression passed to a hardening assertion (because that expression might end up - // being passed to a printf-style formatting function). As a workaround, use an intermediate variable - // (lambdas aren't supported in C++03). - bool __is_aligned = _Number_of_bits_remaining % 4 == 0; - (void)__is_aligned; // Prevent "maybe unused" warnings in modes which don't enable the assertion. - _LIBCPP_ASSERT_INTERNAL(__is_aligned, ""); - } + _LIBCPP_ASSERT_INTERNAL(_Number_of_bits_remaining % 4 == 0, ""); _Number_of_bits_remaining -= 4; const uint32_t _Nibble = static_cast(_Adjusted_mantissa >> _Number_of_bits_remaining); @@ -424,15 +416,7 @@ to_chars_result _Floating_to_chars_hex_shortest( // _Number_of_bits_remaining. do { _LIBCPP_ASSERT_INTERNAL(_Number_of_bits_remaining >= 4, ""); - { - // Note: the `%` symbol is used to introduce a conversion specifier in printf format syntax and thus - // cannot appear in an expression passed to a hardening assertion (because that expression might end up - // being passed to a printf-style formatting function). As a workaround, use an intermediate variable - // (lambdas aren't supported in C++03). - bool __is_aligned = _Number_of_bits_remaining % 4 == 0; - (void)__is_aligned; // Prevent "maybe unused" warnings in modes which don't enable the assertion. - _LIBCPP_ASSERT_INTERNAL(__is_aligned, ""); - } + _LIBCPP_ASSERT_INTERNAL(_Number_of_bits_remaining % 4 == 0, ""); _Number_of_bits_remaining -= 4; const uint32_t _Nibble = static_cast(_Adjusted_mantissa >> _Number_of_bits_remaining); diff --git a/libcxx/src/memory_resource.cpp b/libcxx/src/memory_resource.cpp index adc4747dc0135..42c366893f736 100644 --- a/libcxx/src/memory_resource.cpp +++ b/libcxx/src/memory_resource.cpp @@ -230,15 +230,7 @@ class unsynchronized_pool_resource::__fixed_pool { } void* __allocate_in_new_chunk(memory_resource* upstream, size_t block_size, size_t chunk_size) { - { - // Note: the `%` symbol is used to introduce a conversion specifier in printf format syntax and thus cannot appear - // in an expression passed to a hardening assertion (because that expression might end up being passed to - // a printf-style formatting function). As a workaround, use an intermediate variable (lambdas aren't supported in - // C++03). - bool __is_aligned = chunk_size % block_size == 0; - (void)__is_aligned; // Prevent "maybe unused" warnings in modes which don't enable the assertion. - _LIBCPP_ASSERT_INTERNAL(__is_aligned, ""); - } + _LIBCPP_ASSERT_INTERNAL(chunk_size % block_size == 0, ""); static_assert(__default_alignment >= alignof(std::max_align_t), ""); static_assert(__default_alignment >= alignof(__chunk_footer), ""); static_assert(__default_alignment >= alignof(__vacancy_header), ""); diff --git a/libcxx/vendor/llvm/default_assertion_handler.in b/libcxx/vendor/llvm/default_assertion_handler.in index ca2542aed6ebb..111d305a16f7c 100644 --- a/libcxx/vendor/llvm/default_assertion_handler.in +++ b/libcxx/vendor/llvm/default_assertion_handler.in @@ -18,6 +18,6 @@ #endif // TODO(hardening): in production, trap rather than abort. -#define _LIBCPP_ASSERTION_HANDLER(message) _LIBCPP_VERBOSE_ABORT(message) +#define _LIBCPP_ASSERTION_HANDLER(message) _LIBCPP_VERBOSE_ABORT("%s", message) #endif // _LIBCPP___ASSERTION_HANDLER From 8cc2fffd609334176e4598173ce9e6f606ef6d72 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Wed, 17 Jan 2024 15:32:37 -0800 Subject: [PATCH 19/19] Update `check_assertion` to match 2-argument verbose abort --- libcxx/test/support/check_assertion.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/libcxx/test/support/check_assertion.h b/libcxx/test/support/check_assertion.h index 8c08fbb3cd29f..65c93eded971d 100644 --- a/libcxx/test/support/check_assertion.h +++ b/libcxx/test/support/check_assertion.h @@ -236,10 +236,13 @@ struct DeathTest { }; #ifdef _LIBCPP_VERSION -void std::__libcpp_verbose_abort(char const* message, ...) { +void std::__libcpp_verbose_abort(char const* printf_format, ...) { // Extract information from the error message. This has to stay synchronized with how we format assertions in the - // library. Note that the variadic arguments to `__libcpp_verbose_abort` are empty but maintained for backward - // compatibility reasons. + // library. + va_list args; + va_start(args, printf_format); + char const* message = va_arg(args, char const*); + std::regex message_format("(.*):(\\d+): assertion (.*) failed: (.*)\\n"); std::cmatch match_result;