[compiler-rt] [ubsan-minimal] Switch to weak symbols for callbacks to allow overriding in client code (PR #119242)

Kirill Stoimenov via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 12 15:49:30 PST 2024


https://github.com/kstoimenov updated https://github.com/llvm/llvm-project/pull/119242

>From 15acabf5add7fd15cf58ca2963694e0357e5377a Mon Sep 17 00:00:00 2001
From: Kirill Stoimenov <kstoimenov at google.com>
Date: Fri, 6 Dec 2024 22:05:59 +0000
Subject: [PATCH 1/5] [UBSAN-MINRT] Switch to weak symbols for callbacks to
 allow overriding in client code.

---
 compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp b/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
index 53a3273e4e6980..aa91a66cb6d32e 100644
--- a/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
+++ b/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
@@ -97,8 +97,9 @@ constexpr unsigned kAddrBuf = SANITIZER_WORDSIZE / 4;
 #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();                  \
+  SANITIZER_INTERFACE_WEAK_DEF(                                  \
+    void, __ubsan_handle_##name##_minimal, void) {               \
+    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);            \

>From 6c8ac3b6a5665733d9adc70b88ed34405b3272bc Mon Sep 17 00:00:00 2001
From: Kirill Stoimenov <kstoimenov at google.com>
Date: Mon, 9 Dec 2024 18:21:40 +0000
Subject: [PATCH 2/5] Added a test.

---
 .../ubsan_minimal/TestCases/override-callback.c    | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
 create mode 100644 compiler-rt/test/ubsan_minimal/TestCases/override-callback.c

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..2893b5df8644ac
--- /dev/null
+++ b/compiler-rt/test/ubsan_minimal/TestCases/override-callback.c
@@ -0,0 +1,14 @@
+// RUN: %clang -fsanitize=implicit-integer-sign-change %s -o %t && %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK
+
+#include <stdio.h>
+#include <stdint.h>
+
+void __ubsan_handle_implicit_conversion_minimal() {
+  printf("CUSTOM_CALLBACK\n");
+}
+
+int main() {
+  int32_t t0 = (~((uint32_t)0));
+// CHECK: CUSTOM_CALLBACK
+  return 0;
+}

>From 4551d25414f1ceb5623cf840f12a4c19aaafb6b8 Mon Sep 17 00:00:00 2001
From: Kirill Stoimenov <kstoimenov at google.com>
Date: Tue, 10 Dec 2024 23:00:53 +0000
Subject: [PATCH 3/5] Addressed comments.

---
 .../ubsan_minimal/ubsan_minimal_handlers.cpp  |  9 +++---
 .../TestCases/override-callback.c             | 31 +++++++++++++++----
 2 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp b/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
index aa91a66cb6d32e..3ada75cbe55be0 100644
--- a/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
+++ b/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
@@ -20,7 +20,8 @@ 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) {
+SANITIZER_INTERFACE_WEAK_DEF(int, __sanitizer_report_ubsan_error,
+                             uintptr_t caller, const char* name) {
   if (caller == 0)
     return false;
   while (true) {
@@ -97,10 +98,10 @@ constexpr unsigned kAddrBuf = SANITIZER_WORDSIZE / 4;
 #define MSG_BUF_LEN(msg) (sizeof(MSG_TMPL(msg)) + kAddrBuf + 1)
 
 #define HANDLER_RECOVER(name, msg)                               \
-  SANITIZER_INTERFACE_WEAK_DEF(                                  \
-    void, __ubsan_handle_##name##_minimal, void) {               \
+  INTERFACE void __ubsan_handle_##name##_minimal() {             \
     uintptr_t caller = GET_CALLER_PC();                          \
-    if (!report_this_error(caller)) return;                      \
+    if (!__sanitizer_report_ubsan_error(caller, #name))          \
+      return;                                                    \
     char msg_buf[MSG_BUF_LEN(msg)] = MSG_TMPL(msg);              \
     decorate_msg(MSG_TMPL_END(msg_buf, msg), caller);            \
     message(msg_buf);                                            \
diff --git a/compiler-rt/test/ubsan_minimal/TestCases/override-callback.c b/compiler-rt/test/ubsan_minimal/TestCases/override-callback.c
index 2893b5df8644ac..9c9b1d7b6ad9eb 100644
--- a/compiler-rt/test/ubsan_minimal/TestCases/override-callback.c
+++ b/compiler-rt/test/ubsan_minimal/TestCases/override-callback.c
@@ -1,14 +1,33 @@
-// RUN: %clang -fsanitize=implicit-integer-sign-change %s -o %t && %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK
-
+// RUN: %clang -fsanitize=implicit-integer-sign-change %s -o %t && %run %t 0 2>&1 | \
+// RUN:   FileCheck %s --check-prefixes=CHECK_NO_MESSAGE
+// RUN: %clang -fsanitize=implicit-integer-sign-change %s -o %t && %run %t 1 2>&1 | \
+// RUN:   FileCheck %s --check-prefixes=CHECK_MESSAGE
 #include <stdio.h>
 #include <stdint.h>
+#include <stdlib.h>
+
+static int Result;
 
-void __ubsan_handle_implicit_conversion_minimal() {
-  printf("CUSTOM_CALLBACK\n");
+int __sanitizer_report_ubsan_error(uintptr_t caller, const char* name) {
+  fprintf(stderr, "CUSTOM_CALLBACK\n");
+  return Result;
 }
 
-int main() {
+void test_message() {
   int32_t t0 = (~((uint32_t)0));
-// CHECK: CUSTOM_CALLBACK
+// CHECK_MESSAGE: CUSTOM_CALLBACK
+// CHECK_MESSAGE: ubsan: implicit-conversion
+}
+
+void test_no_message() {
+  int32_t t0 = (~((uint32_t)0));
+// CHECK_NO_MESSAGE: CUSTOM_CALLBACK
+// CCHECK_NO_MESSAGE-NOT: ubsan: implicit-conversion
+}
+
+int main(int argc, const char** argv) {
+  Result = atoi(argv[1]);
+  if (Result) test_message();
+  else test_no_message();
   return 0;
 }

>From 056ea1a868eb67011a59aef61533cd24c11359ad Mon Sep 17 00:00:00 2001
From: Kirill Stoimenov <kstoimenov at google.com>
Date: Wed, 11 Dec 2024 01:19:25 +0000
Subject: [PATCH 4/5] Make formater happy.

---
 .../test/ubsan_minimal/TestCases/override-callback.c        | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/compiler-rt/test/ubsan_minimal/TestCases/override-callback.c b/compiler-rt/test/ubsan_minimal/TestCases/override-callback.c
index 9c9b1d7b6ad9eb..6fbff7967a5f6d 100644
--- a/compiler-rt/test/ubsan_minimal/TestCases/override-callback.c
+++ b/compiler-rt/test/ubsan_minimal/TestCases/override-callback.c
@@ -27,7 +27,9 @@ void test_no_message() {
 
 int main(int argc, const char** argv) {
   Result = atoi(argv[1]);
-  if (Result) test_message();
-  else test_no_message();
+  if (Result)
+    test_message();
+  else
+    test_no_message();
   return 0;
 }

>From 334a4f8227c70bced6bfe1d5fd9f28835f46db6a Mon Sep 17 00:00:00 2001
From: Kirill Stoimenov <kstoimenov at google.com>
Date: Thu, 12 Dec 2024 23:48:57 +0000
Subject: [PATCH 5/5] Addressed comments.

---
 .../ubsan_minimal/ubsan_minimal_handlers.cpp  | 23 +++++++-------
 .../TestCases/override-callback.c             | 31 ++++---------------
 2 files changed, 17 insertions(+), 37 deletions(-)

diff --git a/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp b/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
index 3ada75cbe55be0..bf46068ce897db 100644
--- a/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
+++ b/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
@@ -20,13 +20,13 @@ static __sanitizer::atomic_uintptr_t caller_pcs[kMaxCallerPcs];
 // that "too many errors" has already been reported.
 static __sanitizer::atomic_uint32_t caller_pcs_sz;
 
-SANITIZER_INTERFACE_WEAK_DEF(int, __sanitizer_report_ubsan_error,
-                             uintptr_t caller, const char* name) {
+SANITIZER_INTERFACE_WEAK_DEF(void, __ubsan_report_error, uintptr_t caller,
+                             const char *msg, const char *decorated_msg) {
   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) {
@@ -34,7 +34,7 @@ SANITIZER_INTERFACE_WEAK_DEF(int, __sanitizer_report_ubsan_error,
       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?
     }
@@ -45,10 +45,10 @@ SANITIZER_INTERFACE_WEAK_DEF(int, __sanitizer_report_ubsan_error,
 
     if (sz == kMaxCallerPcs) {
       message("ubsan: too many errors\n");
-      return false;
+      return;
     }
     __sanitizer::atomic_store_relaxed(&caller_pcs[sz], caller);
-    return true;
+    message(decorated_msg);
   }
 }
 
@@ -100,18 +100,17 @@ constexpr unsigned kAddrBuf = SANITIZER_WORDSIZE / 4;
 #define HANDLER_RECOVER(name, msg)                               \
   INTERFACE void __ubsan_handle_##name##_minimal() {             \
     uintptr_t caller = GET_CALLER_PC();                          \
-    if (!__sanitizer_report_ubsan_error(caller, #name))          \
-      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(caller, msg, msg_buf);                  \
   }
 
 #define HANDLER_NORECOVER(name, msg)                             \
   INTERFACE void __ubsan_handle_##name##_minimal_abort() {       \
+    uintptr_t caller = GET_CALLER_PC();                          \
     char msg_buf[MSG_BUF_LEN(msg)] = MSG_TMPL(msg);              \
-    decorate_msg(MSG_TMPL_END(msg_buf, msg), GET_CALLER_PC());   \
-    message(msg_buf);                                            \
+    decorate_msg(MSG_TMPL_END(msg_buf, msg), caller);            \
+    __ubsan_report_error(caller, #msg, msg_buf);                 \
     abort_with_message(msg_buf);                                 \
   }
 
diff --git a/compiler-rt/test/ubsan_minimal/TestCases/override-callback.c b/compiler-rt/test/ubsan_minimal/TestCases/override-callback.c
index 6fbff7967a5f6d..4d2718585ea248 100644
--- a/compiler-rt/test/ubsan_minimal/TestCases/override-callback.c
+++ b/compiler-rt/test/ubsan_minimal/TestCases/override-callback.c
@@ -1,35 +1,16 @@
-// RUN: %clang -fsanitize=implicit-integer-sign-change %s -o %t && %run %t 0 2>&1 | \
-// RUN:   FileCheck %s --check-prefixes=CHECK_NO_MESSAGE
-// RUN: %clang -fsanitize=implicit-integer-sign-change %s -o %t && %run %t 1 2>&1 | \
-// RUN:   FileCheck %s --check-prefixes=CHECK_MESSAGE
+// 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;
 
-int __sanitizer_report_ubsan_error(uintptr_t caller, const char* name) {
-  fprintf(stderr, "CUSTOM_CALLBACK\n");
-  return Result;
-}
-
-void test_message() {
-  int32_t t0 = (~((uint32_t)0));
-// CHECK_MESSAGE: CUSTOM_CALLBACK
-// CHECK_MESSAGE: ubsan: implicit-conversion
-}
-
-void test_no_message() {
-  int32_t t0 = (~((uint32_t)0));
-// CHECK_NO_MESSAGE: CUSTOM_CALLBACK
-// CCHECK_NO_MESSAGE-NOT: ubsan: implicit-conversion
+void __ubsan_report_error(uintptr_t caller, const char *msg,
+                                   const char *decorated_msg) {
+  fprintf(stderr, "CUSTOM_CALLBACK: %s\n", msg);
 }
 
 int main(int argc, const char** argv) {
-  Result = atoi(argv[1]);
-  if (Result)
-    test_message();
-  else
-    test_no_message();
-  return 0;
+  int32_t t0 = (~((uint32_t)0));
+// CHECK: CUSTOM_CALLBACK: implicit-conversion
 }



More information about the llvm-commits mailing list