[libc++] Generalize the customizeable assertion handler

Instead of taking a fixed set of arguments, use variadics so that
we can pass arbitrary arguments to the handler. This is the first
step towards using the handler to handle other non-assertion-related
failures, like std::unreachable and an exception being thrown in
-fno-exceptions mode, which would improve user experience by including
additional information in crashes (right now, we call abort() without
additional information).

Differential Revision: https://reviews.llvm.org/D130507

diff --git a/libcxx/docs/UsingLibcxx.rst b/libcxx/docs/UsingLibcxx.rst
index aa58120..40a8d9a 100644 (file)
--- a/libcxx/docs/UsingLibcxx.rst
+++ b/libcxx/docs/UsingLibcxx.rst
@@ -163,7 +163,7 @@ Replacing the default assertion handler is done by defining the following functi
 
 .. code-block:: cpp
 
-  void __libcpp_assertion_handler(char const* file, int line, char const* expression, char const* message)
+  void __libcpp_assertion_handler(char const* format, ...)
 
 This mechanism is similar to how one can replace the default definition of ``operator new``
 and ``operator delete``. For example:
@@ -173,8 +173,12 @@ and ``operator delete``. For example:
   // In HelloWorldHandler.cpp
   #include <version> // must include any libc++ header before defining the handler (C compatibility headers excluded)
 
-  void std::__libcpp_assertion_handler(char const* file, int line, char const* expression, char const* message) {
-    std::printf("Assertion %s failed at %s:%d, more info: %s", expression, file, line, message);
+  void std::__libcpp_assertion_handler(char const* format, ...) {
+    va_list list;
+    va_start(list, format);
+    std::vfprintf(stderr, format, list);
+    va_end(list);
+
     std::abort();
   }
 

diff --git a/libcxx/include/__assert b/libcxx/include/__assert
index 84ddcd2..82db2cf 100644 (file)
--- a/libcxx/include/__assert
+++ b/libcxx/include/__assert
@@ -45,7 +45,7 @@
 # define _LIBCPP_ASSERT(expression, message)                                        \
     (__builtin_expect(static_cast<bool>(expression), 1) ?                           \
       (void)0 :                                                                     \
-      ::std::__libcpp_assertion_handler(__FILE__, __LINE__, #expression, message))
+      ::std::__libcpp_assertion_handler("%s:%d: assertion %s failed: %s", __FILE__, __LINE__, #expression, message))
 #elif !defined(_LIBCPP_ASSERTIONS_DISABLE_ASSUME) && __has_builtin(__builtin_assume)
 # define _LIBCPP_ASSERT(expression, message)                                        \
     (_LIBCPP_DIAGNOSTIC_PUSH                                                        \
@@ -58,8 +58,8 @@
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-_LIBCPP_OVERRIDABLE_FUNC_VIS _LIBCPP_AVAILABILITY_ASSERTION_HANDLER
-void __libcpp_assertion_handler(char const* __file, int __line, char const* __expression, char const* __message);
+_LIBCPP_OVERRIDABLE_FUNC_VIS _LIBCPP_AVAILABILITY_ASSERTION_HANDLER _LIBCPP_ATTRIBUTE_FORMAT(__printf__, 1, 2)
+void __libcpp_assertion_handler(const char *__format, ...);
 
 _LIBCPP_END_NAMESPACE_STD
 

diff --git a/libcxx/lib/abi/CHANGELOG.TXT b/libcxx/lib/abi/CHANGELOG.TXT
index 034f2e4..877ba8d 100644 (file)
--- a/libcxx/lib/abi/CHANGELOG.TXT
+++ b/libcxx/lib/abi/CHANGELOG.TXT
@@ -64,7 +64,7 @@ Version 15.0
   Symbol removed: _ZTSNSt3__18__c_nodeE
   Symbol removed: _ZTVNSt3__18__c_nodeE
 
-* b0fd9497af6d - [libc++] Add a lightweight overridable assertion handler
+* b0fd9497af6d and XXXXXXX - [libc++] Add a lightweight overridable assertion handler
 
   This patch adds a lightweight assertion handler mechanism that can be
   overriden at link-time in a fashion similar to `operator new`. A default
@@ -73,7 +73,7 @@ Version 15.0
 
   All platforms
   -------------
-  Symbol added: _ZNSt3__126__libcpp_assertion_handlerEPKciS1_S1_
+  Symbol added: _ZNSt3__126__libcpp_assertion_handlerEPKcz
 
 ------------
 Version 14.0

diff --git a/libcxx/lib/abi/arm64-apple-darwin.libcxxabi.v1.stable.exceptions.nonew.abilist b/libcxx/lib/abi/arm64-apple-darwin.libcxxabi.v1.stable.exceptions.nonew.abilist
index f1cb3c7..dd2438d 100644 (file)
--- a/libcxx/lib/abi/arm64-apple-darwin.libcxxabi.v1.stable.exceptions.nonew.abilist
+++ b/libcxx/lib/abi/arm64-apple-darwin.libcxxabi.v1.stable.exceptions.nonew.abilist
@@ -1534,7 +1534,7 @@
 {'is_defined': True, 'name': '__ZNSt3__123__libcpp_atomic_monitorEPVKNS_17__cxx_atomic_implIxNS_22__cxx_atomic_base_implIxEEEE', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__123__libcpp_atomic_monitorEPVKv', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__125notify_all_at_thread_exitERNS_18condition_variableENS_11unique_lockINS_5mutexEEE', 'type': 'FUNC'}
-{'is_defined': True, 'name': '__ZNSt3__126__libcpp_assertion_handlerEPKciS1_S1_', 'type': 'FUNC'}
+{'is_defined': True, 'name': '__ZNSt3__126__libcpp_assertion_handlerEPKcz', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__127__insertion_sort_incompleteIRNS_6__lessIaaEEPaEEbT0_S5_T_', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__127__insertion_sort_incompleteIRNS_6__lessIccEEPcEEbT0_S5_T_', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__127__insertion_sort_incompleteIRNS_6__lessIddEEPdEEbT0_S5_T_', 'type': 'FUNC'}

diff --git a/libcxx/lib/abi/powerpc-ibm-aix.libcxxabi.v1.stable.exceptions.nonew.abilist b/libcxx/lib/abi/powerpc-ibm-aix.libcxxabi.v1.stable.exceptions.nonew.abilist
index 69d1f7e..c662644 100644 (file)
--- a/libcxx/lib/abi/powerpc-ibm-aix.libcxxabi.v1.stable.exceptions.nonew.abilist
+++ b/libcxx/lib/abi/powerpc-ibm-aix.libcxxabi.v1.stable.exceptions.nonew.abilist
@@ -1678,7 +1678,7 @@
 {'import_export': 'wEXP', 'is_defined': True, 'name': '_ZNSt3__118basic_stringstreamIcNS_11char_traitsIcEENS_9allocatorIcEEEaSEOS5_', 'storage_mapping_class': 'DS', 'type': 'FUNC'}
 {'import_export': 'wEXP', 'is_defined': True, 'name': '_ZNSt3__119basic_istringstreamIcNS_11char_traitsIcEENS_9allocatorIcEEEaSEOS5_', 'storage_mapping_class': 'DS', 'type': 'FUNC'}
 {'import_export': 'wEXP', 'is_defined': True, 'name': '_ZNSt3__119basic_ostringstreamIcNS_11char_traitsIcEENS_9allocatorIcEEEaSEOS5_', 'storage_mapping_class': 'DS', 'type': 'FUNC'}
-{'import_export': 'wEXP', 'is_defined': True, 'name': '_ZNSt3__126__libcpp_assertion_handlerEPKciS1_S1_', 'storage_mapping_class': 'DS', 'type': 'FUNC'}
+{'import_export': 'wEXP', 'is_defined': True, 'name': '_ZNSt3__126__libcpp_assertion_handlerEPKcz', 'storage_mapping_class': 'DS', 'type': 'FUNC'}
 {'import_export': 'wEXP', 'is_defined': True, 'name': '_ZNSt3__127__insertion_sort_incompleteIRNS_6__lessIaaEEPaEEbT0_S5_T_', 'storage_mapping_class': 'DS', 'type': 'FUNC'}
 {'import_export': 'wEXP', 'is_defined': True, 'name': '_ZNSt3__127__insertion_sort_incompleteIRNS_6__lessIccEEPcEEbT0_S5_T_', 'storage_mapping_class': 'DS', 'type': 'FUNC'}
 {'import_export': 'wEXP', 'is_defined': True, 'name': '_ZNSt3__127__insertion_sort_incompleteIRNS_6__lessIddEEPdEEbT0_S5_T_', 'storage_mapping_class': 'DS', 'type': 'FUNC'}

diff --git a/libcxx/lib/abi/powerpc64-ibm-aix.libcxxabi.v1.stable.exceptions.nonew.abilist b/libcxx/lib/abi/powerpc64-ibm-aix.libcxxabi.v1.stable.exceptions.nonew.abilist
index b17b3bb..d730eae 100644 (file)
--- a/libcxx/lib/abi/powerpc64-ibm-aix.libcxxabi.v1.stable.exceptions.nonew.abilist
+++ b/libcxx/lib/abi/powerpc64-ibm-aix.libcxxabi.v1.stable.exceptions.nonew.abilist
@@ -1678,7 +1678,7 @@
 {'import_export': 'wEXP', 'is_defined': True, 'name': '_ZNSt3__118basic_stringstreamIcNS_11char_traitsIcEENS_9allocatorIcEEEaSEOS5_', 'storage_mapping_class': 'DS', 'type': 'FUNC'}
 {'import_export': 'wEXP', 'is_defined': True, 'name': '_ZNSt3__119basic_istringstreamIcNS_11char_traitsIcEENS_9allocatorIcEEEaSEOS5_', 'storage_mapping_class': 'DS', 'type': 'FUNC'}
 {'import_export': 'wEXP', 'is_defined': True, 'name': '_ZNSt3__119basic_ostringstreamIcNS_11char_traitsIcEENS_9allocatorIcEEEaSEOS5_', 'storage_mapping_class': 'DS', 'type': 'FUNC'}
-{'import_export': 'wEXP', 'is_defined': True, 'name': '_ZNSt3__126__libcpp_assertion_handlerEPKciS1_S1_', 'storage_mapping_class': 'DS', 'type': 'FUNC'}
+{'import_export': 'wEXP', 'is_defined': True, 'name': '_ZNSt3__126__libcpp_assertion_handlerEPKcz', 'storage_mapping_class': 'DS', 'type': 'FUNC'}
 {'import_export': 'wEXP', 'is_defined': True, 'name': '_ZNSt3__127__insertion_sort_incompleteIRNS_6__lessIaaEEPaEEbT0_S5_T_', 'storage_mapping_class': 'DS', 'type': 'FUNC'}
 {'import_export': 'wEXP', 'is_defined': True, 'name': '_ZNSt3__127__insertion_sort_incompleteIRNS_6__lessIccEEPcEEbT0_S5_T_', 'storage_mapping_class': 'DS', 'type': 'FUNC'}
 {'import_export': 'wEXP', 'is_defined': True, 'name': '_ZNSt3__127__insertion_sort_incompleteIRNS_6__lessIddEEPdEEbT0_S5_T_', 'storage_mapping_class': 'DS', 'type': 'FUNC'}

diff --git a/libcxx/lib/abi/x86_64-apple-darwin.libcxxabi.v1.stable.exceptions.nonew.abilist b/libcxx/lib/abi/x86_64-apple-darwin.libcxxabi.v1.stable.exceptions.nonew.abilist
index 5abc715..bc81e61 100644 (file)
--- a/libcxx/lib/abi/x86_64-apple-darwin.libcxxabi.v1.stable.exceptions.nonew.abilist
+++ b/libcxx/lib/abi/x86_64-apple-darwin.libcxxabi.v1.stable.exceptions.nonew.abilist
@@ -1534,7 +1534,7 @@
 {'is_defined': True, 'name': '__ZNSt3__123__libcpp_atomic_monitorEPVKNS_17__cxx_atomic_implIxNS_22__cxx_atomic_base_implIxEEEE', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__123__libcpp_atomic_monitorEPVKv', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__125notify_all_at_thread_exitERNS_18condition_variableENS_11unique_lockINS_5mutexEEE', 'type': 'FUNC'}
-{'is_defined': True, 'name': '__ZNSt3__126__libcpp_assertion_handlerEPKciS1_S1_', 'type': 'FUNC'}
+{'is_defined': True, 'name': '__ZNSt3__126__libcpp_assertion_handlerEPKcz', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__127__insertion_sort_incompleteIRNS_6__lessIaaEEPaEEbT0_S5_T_', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__127__insertion_sort_incompleteIRNS_6__lessIccEEPcEEbT0_S5_T_', 'type': 'FUNC'}
 {'is_defined': True, 'name': '__ZNSt3__127__insertion_sort_incompleteIRNS_6__lessIddEEPdEEbT0_S5_T_', 'type': 'FUNC'}

diff --git a/libcxx/lib/abi/x86_64-unknown-linux-gnu.libcxxabi.v1.stable.exceptions.nonew.abilist b/libcxx/lib/abi/x86_64-unknown-linux-gnu.libcxxabi.v1.stable.exceptions.nonew.abilist
index 9fb4b7f..1437c1c 100644 (file)
--- a/libcxx/lib/abi/x86_64-unknown-linux-gnu.libcxxabi.v1.stable.exceptions.nonew.abilist
+++ b/libcxx/lib/abi/x86_64-unknown-linux-gnu.libcxxabi.v1.stable.exceptions.nonew.abilist
@@ -1225,7 +1225,7 @@
 {'is_defined': True, 'name': '_ZNSt3__123__libcpp_atomic_monitorEPVKNS_17__cxx_atomic_implIiNS_22__cxx_atomic_base_implIiEEEE', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNSt3__123__libcpp_atomic_monitorEPVKv', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNSt3__125notify_all_at_thread_exitERNS_18condition_variableENS_11unique_lockINS_5mutexEEE', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNSt3__126__libcpp_assertion_handlerEPKciS1_S1_', 'type': 'FUNC'}
+{'is_defined': True, 'name': '_ZNSt3__126__libcpp_assertion_handlerEPKcz', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNSt3__127__insertion_sort_incompleteIRNS_6__lessIaaEEPaEEbT0_S5_T_', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNSt3__127__insertion_sort_incompleteIRNS_6__lessIccEEPcEEbT0_S5_T_', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNSt3__127__insertion_sort_incompleteIRNS_6__lessIddEEPdEEbT0_S5_T_', 'type': 'FUNC'}

diff --git a/libcxx/lib/abi/x86_64-unknown-linux-gnu.libcxxabi.v1.stable.noexceptions.nonew.abilist b/libcxx/lib/abi/x86_64-unknown-linux-gnu.libcxxabi.v1.stable.noexceptions.nonew.abilist
index de08e52..1348fc6 100644 (file)
--- a/libcxx/lib/abi/x86_64-unknown-linux-gnu.libcxxabi.v1.stable.noexceptions.nonew.abilist
+++ b/libcxx/lib/abi/x86_64-unknown-linux-gnu.libcxxabi.v1.stable.noexceptions.nonew.abilist
@@ -1197,7 +1197,7 @@
 {'is_defined': True, 'name': '_ZNSt3__123__libcpp_atomic_monitorEPVKNS_17__cxx_atomic_implIiNS_22__cxx_atomic_base_implIiEEEE', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNSt3__123__libcpp_atomic_monitorEPVKv', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNSt3__125notify_all_at_thread_exitERNS_18condition_variableENS_11unique_lockINS_5mutexEEE', 'type': 'FUNC'}
-{'is_defined': True, 'name': '_ZNSt3__126__libcpp_assertion_handlerEPKciS1_S1_', 'type': 'FUNC'}
+{'is_defined': True, 'name': '_ZNSt3__126__libcpp_assertion_handlerEPKcz', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNSt3__127__insertion_sort_incompleteIRNS_6__lessIaaEEPaEEbT0_S5_T_', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNSt3__127__insertion_sort_incompleteIRNS_6__lessIccEEPcEEbT0_S5_T_', 'type': 'FUNC'}
 {'is_defined': True, 'name': '_ZNSt3__127__insertion_sort_incompleteIRNS_6__lessIddEEPdEEbT0_S5_T_', 'type': 'FUNC'}

diff --git a/libcxx/src/assert.cpp b/libcxx/src/assert.cpp
index 5445980..c218645 100644 (file)
--- a/libcxx/src/assert.cpp
+++ b/libcxx/src/assert.cpp
@@ -8,14 +8,57 @@
 
 #include <__assert>
 #include <__config>
+#include <cstdarg>
 #include <cstdio>
 #include <cstdlib>
 
+#ifdef __BIONIC__
+#  include <android/api-level.h>
+#  include <syslog.h>
+extern "C" void android_set_abort_message(const char* msg);
+#endif
+
+#if defined(__APPLE__) && __has_include(<CrashReporterClient.h>)
+#  include <CrashReporterClient.h>
+#endif
+
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 _LIBCPP_WEAK
-void __libcpp_assertion_handler(char const* __file, int __line, char const* __expression, char const* __message) {
-  std::fprintf(stderr, "%s:%d: libc++ assertion '%s' failed. %s\n", __file, __line, __expression, __message);
+void __libcpp_assertion_handler(char const* format, ...) {
+  // Write message to stderr. We do this before formatting into a
+  // buffer so that we still get some information out if that fails.
+  {
+    va_list list;
+    va_start(list, format);
+    std::vfprintf(stderr, format, list);
+    va_end(list);
+  }
+
+  // Format the arguments into an allocated buffer for CrashReport & friends.
+  // We leak the buffer on purpose, since we're about to abort() anyway.
+  char* buffer; (void)buffer;
+  va_list list;
+  va_start(list, format);
+
+#if defined(__APPLE__) && __has_include(<CrashReporterClient.h>)
+  // Note that we should technically synchronize accesses here (by e.g. taking a lock),
+  // however concretely we're only setting a pointer, so the likelihood of a race here
+  // is low.
+  vasprintf(&buffer, format, list);
+  CRSetCrashLogMessage(buffer);
+#elif defined(__BIONIC__)
+  // Show error in tombstone.
+  vasprintf(&buffer, format, list);
+  android_set_abort_message(buffer);
+
+  // Show error in logcat.
+  openlog("libc++", 0, 0);
+  syslog(LOG_CRIT, "%s", buffer);
+  closelog();
+#endif
+  va_end(list);
+
   std::abort();
 }
 

diff --git a/libcxx/test/libcxx/assertions/customize_handler.backdeployment.pass.cpp b/libcxx/test/libcxx/assertions/customize_handler.backdeployment.pass.cpp
index fda41a5..b1b5e0c 100644 (file)
--- a/libcxx/test/libcxx/assertions/customize_handler.backdeployment.pass.cpp
+++ b/libcxx/test/libcxx/assertions/customize_handler.backdeployment.pass.cpp
@@ -17,7 +17,7 @@
 #include <cassert>
 
 bool handler_called = false;
-void std::__libcpp_assertion_handler(char const*, int, char const*, char const*) {
+void std::__libcpp_assertion_handler(char const*, ...) {
   handler_called = true;
 }
 

diff --git a/libcxx/test/libcxx/assertions/customize_handler.pass.cpp b/libcxx/test/libcxx/assertions/customize_handler.pass.cpp
index 54c0d16..e6faf1b 100644 (file)
--- a/libcxx/test/libcxx/assertions/customize_handler.pass.cpp
+++ b/libcxx/test/libcxx/assertions/customize_handler.pass.cpp
@@ -17,7 +17,7 @@
 #include <cassert>
 
 bool handler_called = false;
-void std::__libcpp_assertion_handler(char const*, int, char const*, char const*) {
+void std::__libcpp_assertion_handler(char const*, ...) {
   handler_called = true;
 }
 

diff --git a/libcxx/test/libcxx/assertions/debug_mode_compatibility.pass.cpp b/libcxx/test/libcxx/assertions/debug_mode_compatibility.pass.cpp
index a4c61a6..6cee46a 100644 (file)
--- a/libcxx/test/libcxx/assertions/debug_mode_compatibility.pass.cpp
+++ b/libcxx/test/libcxx/assertions/debug_mode_compatibility.pass.cpp
@@ -19,7 +19,7 @@
 #include <cassert>
 
 bool handler_called = false;
-void std::__libcpp_assertion_handler(char const*, int, char const*, char const*) {
+void std::__libcpp_assertion_handler(char const*, ...) {
   handler_called = true;
 }
 

diff --git a/libcxx/test/support/check_assertion.h b/libcxx/test/support/check_assertion.h
index 30067b4..789823a 100644 (file)
--- a/libcxx/test/support/check_assertion.h
+++ b/libcxx/test/support/check_assertion.h
@@ -10,6 +10,7 @@
 #define TEST_SUPPORT_CHECK_ASSERTION_H
 
 #include <cassert>
+#include <cstdarg>
 #include <cstddef>
 #include <cstdio>
 #include <cstdlib>
@@ -235,8 +236,19 @@ private:
   std::string stderr_from_child_;
 };
 
-void std::__libcpp_assertion_handler(char const* file, int line, char const* /*expression*/, char const* message) {
+void std::__libcpp_assertion_handler(char const* format, ...) {
   assert(!GlobalMatcher().empty());
+
+  // 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)) {
     std::exit(DeathTest::RK_MatchFound);
   }