[nfc][ubsan-minimal] Refactor error reporting to use a single function

[kstoimenov]

This refactoring will allow to make this function weak later on so that it could be overloaded by a client. See #119242.

[llvmbot]

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Kirill Stoimenov (kstoimenov)

Changes

This refactoring will allow to make this function weak later on so that it could be overloaded by a client.

---

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

2 Files Affected:

• (modified) compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp (+49-44)
• (added) compiler-rt/test/ubsan_minimal/TestCases/override-callback.c (+15)

diff --git a/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp b/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
index 53a3273e4e6980..54554e5b915b1f 100644
--- a/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
+++ b/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
@@ -1,5 +1,6 @@
 #include "sanitizer_common/sanitizer_atomic.h"
 
+#include <stdio.h>
 #include <stdlib.h>
 #include <stdint.h>
 #include <string.h>
@@ -20,12 +21,41 @@ static __sanitizer::atomic_uintptr_t caller_pcs[kMaxCallerPcs];
 // that "too many errors" has already been reported.
 static __sanitizer::atomic_uint32_t caller_pcs_sz;
 
-__attribute__((noinline)) static bool report_this_error(uintptr_t caller) {
+#define MSG_PREFIX "ubsan: "
+#define MSG_SUFFIX " by 0x"
+
+static char *append_str(const char *s, char *buf, const char *end) {
+  for (const char *p = s; (buf < end) && (*p != '\0'); ++p, ++buf) *buf = *p;
+  return buf;
+}
+
+static char *append_hex(uintptr_t d, char *buf, const char *end) {
+  // Print the address by nibbles.
+  for (unsigned shift = sizeof(uintptr_t) * 8; shift && buf < end;) {
+    shift -= 4;
+    unsigned nibble = (d >> shift) & 0xf;
+    *(buf++) = nibble < 10 ? nibble + '0' : nibble - 10 + 'a';
+  }
+  return buf;
+}
+
+#if defined(__ANDROID__)
+extern "C" __attribute__((weak)) void android_set_abort_message(const char *);
+static void abort_with_message(const char *msg) {
+  if (&android_set_abort_message) android_set_abort_message(msg);
+  abort();
+}
+#else
+static void abort_with_message(const char *) { abort(); }
+#endif
+
+SANITIZER_INTERFACE_WEAK_DEF(void, __ubsan_report_error, const char *msg,
+                             uintptr_t caller, int abort) {
   if (caller == 0)
-    return false;
+    return;
   while (true) {
     unsigned sz = __sanitizer::atomic_load_relaxed(&caller_pcs_sz);
-    if (sz > kMaxCallerPcs) return false;  // early exit
+    if (sz > kMaxCallerPcs) return;  // early exit
     // when sz==kMaxCallerPcs print "too many errors", but only when cmpxchg
     // succeeds in order to not print it multiple times.
     if (sz > 0 && sz < kMaxCallerPcs) {
@@ -33,7 +63,7 @@ __attribute__((noinline)) static bool report_this_error(uintptr_t caller) {
       for (unsigned i = 0; i < sz; ++i) {
         p = __sanitizer::atomic_load_relaxed(&caller_pcs[i]);
         if (p == 0) break;  // Concurrent update.
-        if (p == caller) return false;
+        if (p == caller) return;
       }
       if (p == 0) continue;  // FIXME: yield?
     }
@@ -44,35 +74,24 @@ __attribute__((noinline)) static bool report_this_error(uintptr_t caller) {
 
     if (sz == kMaxCallerPcs) {
       message("ubsan: too many errors\n");
-      return false;
+      return;
     }
     __sanitizer::atomic_store_relaxed(&caller_pcs[sz], caller);
-    return true;
-  }
-}
 
-__attribute__((noinline)) static void decorate_msg(char *buf,
-                                                   uintptr_t caller) {
-  // print the address by nibbles
-  for (unsigned shift = sizeof(uintptr_t) * 8; shift;) {
-    shift -= 4;
-    unsigned nibble = (caller >> shift) & 0xf;
-    *(buf++) = nibble < 10 ? nibble + '0' : nibble - 10 + 'a';
-  }
-  // finish the message
-  buf[0] = '\n';
-  buf[1] = '\0';
-}
+    char msg_buf[128] = MSG_PREFIX;
+    const char *end = msg_buf + sizeof(msg_buf);
+    char *p = append_str(msg, msg_buf + sizeof(MSG_PREFIX) - 1, end);
+    p = append_str(MSG_SUFFIX, p, end);
+    p = append_hex(caller, p, end);
+    if (p < end) *p++ = '\n';
 
-#if defined(__ANDROID__)
-extern "C" __attribute__((weak)) void android_set_abort_message(const char *);
-static void abort_with_message(const char *msg) {
-  if (&android_set_abort_message) android_set_abort_message(msg);
-  abort();
+    // Zero terminate.
+    if (p == end) --p;
+    *p = '\0';
+    message(msg_buf);
+    if (abort) abort_with_message(msg_buf);                                 \
+  }
 }
-#else
-static void abort_with_message(const char *) { abort(); }
-#endif
 
 #if SANITIZER_DEBUG
 namespace __sanitizer {
@@ -89,28 +108,14 @@ void NORETURN CheckFailed(const char *file, int, const char *cond, u64, u64) {
 
 #define INTERFACE extern "C" __attribute__((visibility("default")))
 
-// How many chars we need to reserve to print an address.
-constexpr unsigned kAddrBuf = SANITIZER_WORDSIZE / 4;
-#define MSG_TMPL(msg) "ubsan: " msg " by 0x"
-#define MSG_TMPL_END(buf, msg) (buf + sizeof(MSG_TMPL(msg)) - 1)
-// Reserve an additional byte for '\n'.
-#define MSG_BUF_LEN(msg) (sizeof(MSG_TMPL(msg)) + kAddrBuf + 1)
-
 #define HANDLER_RECOVER(name, msg)                               \
   INTERFACE void __ubsan_handle_##name##_minimal() {             \
-    uintptr_t caller = GET_CALLER_PC();                  \
-    if (!report_this_error(caller)) return;                      \
-    char msg_buf[MSG_BUF_LEN(msg)] = MSG_TMPL(msg);              \
-    decorate_msg(MSG_TMPL_END(msg_buf, msg), caller);            \
-    message(msg_buf);                                            \
+    __ubsan_report_error(msg, GET_CALLER_PC(), 0);               \
   }
 
 #define HANDLER_NORECOVER(name, msg)                             \
   INTERFACE void __ubsan_handle_##name##_minimal_abort() {       \
-    char msg_buf[MSG_BUF_LEN(msg)] = MSG_TMPL(msg);              \
-    decorate_msg(MSG_TMPL_END(msg_buf, msg), GET_CALLER_PC());   \
-    message(msg_buf);                                            \
-    abort_with_message(msg_buf);                                 \
+    __ubsan_report_error(msg, GET_CALLER_PC(), 1);               \
   }
 
 #define HANDLER(name, msg)                                       \
diff --git a/compiler-rt/test/ubsan_minimal/TestCases/override-callback.c b/compiler-rt/test/ubsan_minimal/TestCases/override-callback.c
new file mode 100644
index 00000000000000..52e02f2046c63e
--- /dev/null
+++ b/compiler-rt/test/ubsan_minimal/TestCases/override-callback.c
@@ -0,0 +1,15 @@
+// RUN: %clang -fsanitize=implicit-integer-sign-change %s -o %t && %run %t 0 2>&1 | FileCheck %s
+#include <stdio.h>
+#include <stdint.h>
+#include <stdlib.h>
+
+static int Result;
+
+void __ubsan_report_error(const char *msg, uintptr_t caller, int abort) {
+  fprintf(stderr, "CUSTOM_CALLBACK: %s %d\n", msg, abort);
+}
+
+int main(int argc, const char** argv) {
+  int32_t t0 = (~((uint32_t)0));
+// CHECK: CUSTOM_CALLBACK: implicit-conversion 0
+}

[github-actions]

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