[libc++][hardening][NFC] Introduce _LIBCPP_VERBOSE_TRAP macro.

[var-const]

Split out the calls to __builtin_verbose_trap into a separate header.
This is just a refactoring to make the code a bit more structured.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Konstantin Varlamov (var-const)

Changes

Split out the calls to __builtin_verbose_trap into a separate header.
This is just a refactoring to make the code a bit more structured.

---

Full diff: https://github.com/llvm/llvm-project/pull/148262.diff

3 Files Affected:

• (modified) libcxx/include/CMakeLists.txt (+1)
• (added) libcxx/include/__verbose_trap (+37)
• (modified) libcxx/vendor/llvm/default_assertion_handler.in (+2-12)

diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt
index c8e6d28584623..f79edc9e32599 100644
--- a/libcxx/include/CMakeLists.txt
+++ b/libcxx/include/CMakeLists.txt
@@ -944,6 +944,7 @@ set(files
   __vector/vector_bool.h
   __vector/vector_bool_formatter.h
   __verbose_abort
+  __verbose_trap
   algorithm
   any
   array
diff --git a/libcxx/include/__verbose_trap b/libcxx/include/__verbose_trap
new file mode 100644
index 0000000000000..db77b29e69062
--- /dev/null
+++ b/libcxx/include/__verbose_trap
@@ -0,0 +1,37 @@
+// -*- 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___VERBOSE_TRAP
+#define _LIBCPP___VERBOSE_TRAP
+
+#include <__config>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#  pragma GCC system_header
+#endif
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+#  if __has_builtin(__builtin_verbose_trap)
+// AppleClang shipped a slightly different version of __builtin_verbose_trap from the upstream
+// version before upstream Clang actually got the builtin.
+// TODO: Remove once AppleClang supports the two-arguments version of the builtin.
+#    if defined(_LIBCPP_APPLE_CLANG_VER) && _LIBCPP_APPLE_CLANG_VER < 1700
+#      define _LIBCPP_VERBOSE_TRAP(message) __builtin_verbose_trap(message)
+#    else
+#      define _LIBCPP_VERBOSE_TRAP(message) __builtin_verbose_trap("libc++", message)
+#    endif
+#  else
+#    define _LIBCPP_VERBOSE_TRAP(message) ((void)message, __builtin_trap())
+#  endif
+
+
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // _LIBCPP___VERBOSE_TRAP
diff --git a/libcxx/vendor/llvm/default_assertion_handler.in b/libcxx/vendor/llvm/default_assertion_handler.in
index 1d6b21fc6bb45..90b202a2dae57 100644
--- a/libcxx/vendor/llvm/default_assertion_handler.in
+++ b/libcxx/vendor/llvm/default_assertion_handler.in
@@ -15,6 +15,7 @@
 #  include <__cxx03/__verbose_abort>
 #else
 #  include <__config>
+#  include <__verbose_trap>
 #  include <__verbose_abort>
 #endif
 
@@ -28,18 +29,7 @@
 
 #else
 
-#  if __has_builtin(__builtin_verbose_trap)
-// AppleClang shipped a slightly different version of __builtin_verbose_trap from the upstream
-// version before upstream Clang actually got the builtin.
-// TODO: Remove once AppleClang supports the two-arguments version of the builtin.
-#    if defined(_LIBCPP_APPLE_CLANG_VER) && _LIBCPP_APPLE_CLANG_VER < 1700
-#      define _LIBCPP_ASSERTION_HANDLER(message) __builtin_verbose_trap(message)
-#    else
-#      define _LIBCPP_ASSERTION_HANDLER(message) __builtin_verbose_trap("libc++", message)
-#    endif
-#  else
-#    define _LIBCPP_ASSERTION_HANDLER(message) ((void)message, __builtin_trap())
-#  endif
+#  define _LIBCPP_ASSERTION_HANDLER(message) _LIBCPP_VERBOSE_TRAP(message)
 
 #endif // _LIBCPP_HARDENING_MODE == _LIBCPP_HARDENING_MODE_DEBUG
 

[var-const]

Question: what do we do about the frozen headers?

[ldionne]

This is a good question, and actually it's not easy to answer IMO. CCing @philnik777 .

I see a few options:

1. Introduce <__cxx03/__verbose_trap>. This seems to go against the notion of having "frozen headers".
2. Define _LIBCPP_VERBOSE_TRAP inline inside default_assertion_handler.in inside this #if block. This is equivalent to (1) except we don't introduce a new header under __cxx03.
3. Always use the non-C++03 headers from default_assertion_handler.in. So this means get rid of the #if and always include <__config> and <__verbose_abort>, never their <__cxx03/...> counterparts. This would be doable since <__verbose_abort> doesn't actually need C++11, but it goes against the principle of the split and may cause other conflicts if we somehow end up including both <__config> and <__cxx03/__config>.
4. Have two copies of default_assertion_handler.in, one normal and one for the frozen headers. This is a bit tricky since default_assertion_handler.in is something we set up from CMake and expect vendors to be able to override.

I think (1) is the best outcome here. We'll have to support these hardening improvements in frozen-C++03 anyways otherwise this'll be a regression in functionality.

[var-const]

Thanks for the comprehensive overview of alternatives, this is super useful!

From what I gather, the frozen headers are still undergoing development, so them being completely frozen is the end goal rather than a description of the current state. If that perspective is valid, I also think that option (1) is the best, or at least the lesser evil out of the available options.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[ldionne]

This LGTM but we need to agree on what to do with the frozen headers, and get CI green.

[ldionne]

This is a good question, and actually it's not easy to answer IMO. CCing @philnik777 .

I see a few options:

1. Introduce <__cxx03/__verbose_trap>. This seems to go against the notion of having "frozen headers".
2. Define _LIBCPP_VERBOSE_TRAP inline inside default_assertion_handler.in inside this #if block. This is equivalent to (1) except we don't introduce a new header under __cxx03.
3. Always use the non-C++03 headers from default_assertion_handler.in. So this means get rid of the #if and always include <__config> and <__verbose_abort>, never their <__cxx03/...> counterparts. This would be doable since <__verbose_abort> doesn't actually need C++11, but it goes against the principle of the split and may cause other conflicts if we somehow end up including both <__config> and <__cxx03/__config>.
4. Have two copies of default_assertion_handler.in, one normal and one for the frozen headers. This is a bit tricky since default_assertion_handler.in is something we set up from CMake and expect vendors to be able to override.

I think (1) is the best outcome here. We'll have to support these hardening improvements in frozen-C++03 anyways otherwise this'll be a regression in functionality.

[ldionne]

LGTM. We can discuss alternatives for the C++03 frozen header handling but I'd like to get this shipped in time for tomorrow's branch.