[libcxx-commits] [libcxx] [libc++][hardening] Rework how the assertion handler can be overridden. (PR #77883)

[Konstantin Varlamov via libcxx-commits]

================
@@ -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 <version> // 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 <vector>
-
-  int main() {
-    std::vector<int> 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
----------------
var-const wrote:

Done. (This required tweaking how we parse the assertion message in tests a little bit, but that was pretty minor)

I was originally a little concerned that it would make every assertion message slightly larger (since e.g. the `assertion failed:` part is now "inlined" into every message), but got convinced otherwise during the review:
- it only matters when the string literal is actually referenced by the program. With the new approach we're aiming for with `__builtin_verbose_trap`, the string will end up not being referenced by the application and will simply not be stored in the binary (only in the debug metadata), making this a non-issue for all projects using the default assertion handler;
- even if a project overrides the default handler _and_ references the string, it looks like switching to passing a single parameter to the macro instead of variadics results in noticeably less code being generated (well, the difference is minor, but it affects every single assertion so it adds up). While I haven't measured, it looks like it should offset if not exceed the extra size coming from the string (which is already pretty small). Moreover, for projects using the default handler, this is strictly a binary size improvement (no extra instructions needed for passing variadic arguments).

So all in all, I really like this suggestion! Not doing runtime formatting also removes one more potential point of failure.

https://github.com/llvm/llvm-project/pull/77883