[compiler-rt] e5e0f23 - [nfc][ubsan-minimal] Refactor error reporting to use a single function (#119920)

via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 13 13:43:11 PST 2024


Author: Kirill Stoimenov
Date: 2024-12-13T13:43:07-08:00
New Revision: e5e0f23ae8e97eb910cb8ae42373f354eee496c7

URL: https://github.com/llvm/llvm-project/commit/e5e0f23ae8e97eb910cb8ae42373f354eee496c7
DIFF: https://github.com/llvm/llvm-project/commit/e5e0f23ae8e97eb910cb8ae42373f354eee496c7.diff

LOG: [nfc][ubsan-minimal] Refactor error reporting to use a single function (#119920)

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

Added: 
    

Modified: 
    compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp b/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
index 53a3273e4e6980..cd0b38618c5cb2 100644
--- a/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
+++ b/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
@@ -20,12 +20,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) {
+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;
+}
+
+static void format_msg(const char *kind, uintptr_t caller, char *buf,
+                       const char *end) {
+  buf = append_str("ubsan: ", buf, end);
+  buf = append_str(kind, buf, end);
+  buf = append_str(" by 0x", buf, end);
+  buf = append_hex(caller, buf, end);
+  buf = append_str("\n", buf, end);
+  if (buf == end)
+    --buf; // Make sure we don't cause a buffer overflow.
+  *buf = '\0';
+}
+
+static void report_error(const char *kind, uintptr_t caller) {
   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 +62,8 @@ __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,34 +74,27 @@ __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';
+    char msg_buf[128];
+    format_msg(kind, caller, msg_buf, msg_buf + sizeof(msg_buf));
+    message(msg_buf);
   }
-  // finish the message
-  buf[0] = '\n';
-  buf[1] = '\0';
 }
 
 #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);
+static void abort_with_message(const char *kind, uintptr_t caller) {
+  char msg_buf[128];
+  format_msg(kind, caller, msg_buf, msg_buf + sizeof(msg_buf));
+  if (&android_set_abort_message)
+    android_set_abort_message(msg_buf);
   abort();
 }
 #else
-static void abort_with_message(const char *) { abort(); }
+static void abort_with_message(const char *kind, uintptr_t caller) { abort(); }
 #endif
 
 #if SANITIZER_DEBUG
@@ -89,33 +112,21 @@ 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);                                            \
+#define HANDLER_RECOVER(name, kind)                                            \
+  INTERFACE void __ubsan_handle_##name##_minimal() {                           \
+    report_error(kind, GET_CALLER_PC());                                       \
   }
 
-#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);                                 \
+#define HANDLER_NORECOVER(name, kind)                                          \
+  INTERFACE void __ubsan_handle_##name##_minimal_abort() {                     \
+    uintptr_t caller = GET_CALLER_PC();                                        \
+    report_error(kind, caller);                                                \
+    abort_with_message(kind, caller);                                          \
   }
 
-#define HANDLER(name, msg)                                       \
-  HANDLER_RECOVER(name, msg)                                     \
-  HANDLER_NORECOVER(name, msg)
+#define HANDLER(name, kind)                                                    \
+  HANDLER_RECOVER(name, kind)                                                  \
+  HANDLER_NORECOVER(name, kind)
 
 HANDLER(type_mismatch, "type-mismatch")
 HANDLER(alignment_assumption, "alignment-assumption")


        


More information about the llvm-commits mailing list