[compiler-rt] [nfc][ubsan-minimal] Refactor error reporting to use a single function (PR #119920)
Kirill Stoimenov via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 13 13:13:57 PST 2024
https://github.com/kstoimenov updated https://github.com/llvm/llvm-project/pull/119920
>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 01/15] [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 02/15] 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 03/15] 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 04/15] 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 05/15] 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
}
>From 753e228246b2cb0411d1e92a331d23afe35829a4 Mon Sep 17 00:00:00 2001
From: Kirill Stoimenov <kstoimenov at google.com>
Date: Thu, 12 Dec 2024 23:58:09 +0000
Subject: [PATCH 06/15] Addressed more comments.
---
compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp | 8 ++++----
.../test/ubsan_minimal/TestCases/override-callback.c | 4 ++--
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp b/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
index bf46068ce897db..31fced98a9b605 100644
--- a/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
+++ b/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
@@ -20,8 +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;
-SANITIZER_INTERFACE_WEAK_DEF(void, __ubsan_report_error, uintptr_t caller,
- const char *msg, const char *decorated_msg) {
+SANITIZER_INTERFACE_WEAK_DEF(void, __ubsan_report_error, const char *msg,
+ uintptr_t caller, const char *decorated_msg) {
if (caller == 0)
return;
while (true) {
@@ -102,7 +102,7 @@ constexpr unsigned kAddrBuf = SANITIZER_WORDSIZE / 4;
uintptr_t caller = GET_CALLER_PC(); \
char msg_buf[MSG_BUF_LEN(msg)] = MSG_TMPL(msg); \
decorate_msg(MSG_TMPL_END(msg_buf, msg), caller); \
- __ubsan_report_error(caller, msg, msg_buf); \
+ __ubsan_report_error(msg, caller, msg_buf); \
}
#define HANDLER_NORECOVER(name, msg) \
@@ -110,7 +110,7 @@ constexpr unsigned kAddrBuf = SANITIZER_WORDSIZE / 4;
uintptr_t caller = GET_CALLER_PC(); \
char msg_buf[MSG_BUF_LEN(msg)] = MSG_TMPL(msg); \
decorate_msg(MSG_TMPL_END(msg_buf, msg), caller); \
- __ubsan_report_error(caller, #msg, msg_buf); \
+ __ubsan_report_error(msg, caller, 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 4d2718585ea248..7f81d93d5c8608 100644
--- a/compiler-rt/test/ubsan_minimal/TestCases/override-callback.c
+++ b/compiler-rt/test/ubsan_minimal/TestCases/override-callback.c
@@ -5,8 +5,8 @@
static int Result;
-void __ubsan_report_error(uintptr_t caller, const char *msg,
- const char *decorated_msg) {
+void __ubsan_report_error(const char *msg, uintptr_t caller,
+ const char *decorated_msg) {
fprintf(stderr, "CUSTOM_CALLBACK: %s\n", msg);
}
>From f32656fa5ff69058ed73883a8e30d5d65169dd03 Mon Sep 17 00:00:00 2001
From: Kirill Stoimenov <kstoimenov at google.com>
Date: Fri, 13 Dec 2024 19:38:43 +0000
Subject: [PATCH 07/15] Refactored the code to avoid using compile-time macros.
---
.../ubsan_minimal/ubsan_minimal_handlers.cpp | 82 ++++++++++---------
1 file changed, 44 insertions(+), 38 deletions(-)
diff --git a/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp b/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
index 31fced98a9b605..03172da6fb4b39 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,8 +21,36 @@ static __sanitizer::atomic_uintptr_t caller_pcs[kMaxCallerPcs];
// that "too many errors" has already been reported.
static __sanitizer::atomic_uint32_t caller_pcs_sz;
+#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, const char *decorated_msg) {
+ uintptr_t caller, int abort) {
if (caller == 0)
return;
while (true) {
@@ -48,32 +77,21 @@ SANITIZER_INTERFACE_WEAK_DEF(void, __ubsan_report_error, const char *msg,
return;
}
__sanitizer::atomic_store_relaxed(&caller_pcs[sz], caller);
- message(decorated_msg);
- }
-}
-__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 {
@@ -90,28 +108,16 @@ 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(); \
- char msg_buf[MSG_BUF_LEN(msg)] = MSG_TMPL(msg); \
- decorate_msg(MSG_TMPL_END(msg_buf, msg), caller); \
- __ubsan_report_error(msg, caller, msg_buf); \
+ __ubsan_report_error(msg, caller, 0); \
}
#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), caller); \
- __ubsan_report_error(msg, caller, msg_buf); \
- abort_with_message(msg_buf); \
+ __ubsan_report_error(msg, caller, 1); \
}
#define HANDLER(name, msg) \
>From 54361b984076b7b42ab073d798271b0df506c6ad Mon Sep 17 00:00:00 2001
From: Kirill Stoimenov <kstoimenov at google.com>
Date: Fri, 13 Dec 2024 19:45:22 +0000
Subject: [PATCH 08/15] Updated the test.
---
.../test/ubsan_minimal/TestCases/override-callback.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/compiler-rt/test/ubsan_minimal/TestCases/override-callback.c b/compiler-rt/test/ubsan_minimal/TestCases/override-callback.c
index 7f81d93d5c8608..52e02f2046c63e 100644
--- a/compiler-rt/test/ubsan_minimal/TestCases/override-callback.c
+++ b/compiler-rt/test/ubsan_minimal/TestCases/override-callback.c
@@ -5,12 +5,11 @@
static int Result;
-void __ubsan_report_error(const char *msg, uintptr_t caller,
- const char *decorated_msg) {
- fprintf(stderr, "CUSTOM_CALLBACK: %s\n", msg);
+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
+// CHECK: CUSTOM_CALLBACK: implicit-conversion 0
}
>From 66e7129af12f72d011b7ede521a5ee766ef7a707 Mon Sep 17 00:00:00 2001
From: Kirill Stoimenov <kstoimenov at google.com>
Date: Fri, 13 Dec 2024 19:47:32 +0000
Subject: [PATCH 09/15] Removed a local variable.
---
compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp b/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
index 03172da6fb4b39..54554e5b915b1f 100644
--- a/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
+++ b/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
@@ -110,14 +110,12 @@ void NORETURN CheckFailed(const char *file, int, const char *cond, u64, u64) {
#define HANDLER_RECOVER(name, msg) \
INTERFACE void __ubsan_handle_##name##_minimal() { \
- uintptr_t caller = GET_CALLER_PC(); \
- __ubsan_report_error(msg, caller, 0); \
+ __ubsan_report_error(msg, GET_CALLER_PC(), 0); \
}
#define HANDLER_NORECOVER(name, msg) \
INTERFACE void __ubsan_handle_##name##_minimal_abort() { \
- uintptr_t caller = GET_CALLER_PC(); \
- __ubsan_report_error(msg, caller, 1); \
+ __ubsan_report_error(msg, GET_CALLER_PC(), 1); \
}
#define HANDLER(name, msg) \
>From 438d3d5d114526fd653555823e79da983397569d Mon Sep 17 00:00:00 2001
From: Kirill Stoimenov <kstoimenov at google.com>
Date: Fri, 13 Dec 2024 20:08:44 +0000
Subject: [PATCH 10/15] Revert the actual changes.
---
.../lib/ubsan_minimal/ubsan_minimal_handlers.cpp | 6 +++---
.../ubsan_minimal/TestCases/override-callback.c | 15 ---------------
2 files changed, 3 insertions(+), 18 deletions(-)
delete mode 100644 compiler-rt/test/ubsan_minimal/TestCases/override-callback.c
diff --git a/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp b/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
index 54554e5b915b1f..d5a9ee5370516d 100644
--- a/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
+++ b/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
@@ -49,7 +49,7 @@ static void abort_with_message(const char *msg) {
static void abort_with_message(const char *) { abort(); }
#endif
-SANITIZER_INTERFACE_WEAK_DEF(void, __ubsan_report_error, const char *msg,
+__attribute__((noinline)) static void report_error(const char *msg,
uintptr_t caller, int abort) {
if (caller == 0)
return;
@@ -110,12 +110,12 @@ void NORETURN CheckFailed(const char *file, int, const char *cond, u64, u64) {
#define HANDLER_RECOVER(name, msg) \
INTERFACE void __ubsan_handle_##name##_minimal() { \
- __ubsan_report_error(msg, GET_CALLER_PC(), 0); \
+ report_error(msg, GET_CALLER_PC(), 0); \
}
#define HANDLER_NORECOVER(name, msg) \
INTERFACE void __ubsan_handle_##name##_minimal_abort() { \
- __ubsan_report_error(msg, GET_CALLER_PC(), 1); \
+ 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
deleted file mode 100644
index 52e02f2046c63e..00000000000000
--- a/compiler-rt/test/ubsan_minimal/TestCases/override-callback.c
+++ /dev/null
@@ -1,15 +0,0 @@
-// 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
-}
>From e26ffe04b5aaa50a74f29ef4f4e6d8cec16d0cee Mon Sep 17 00:00:00 2001
From: Kirill Stoimenov <kstoimenov at google.com>
Date: Fri, 13 Dec 2024 20:09:47 +0000
Subject: [PATCH 11/15] No need for noinline anymore.
---
compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp | 3 +--
1 file changed, 1 insertion(+), 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 d5a9ee5370516d..56e97be163e504 100644
--- a/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
+++ b/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
@@ -49,8 +49,7 @@ static void abort_with_message(const char *msg) {
static void abort_with_message(const char *) { abort(); }
#endif
-__attribute__((noinline)) static void report_error(const char *msg,
- uintptr_t caller, int abort) {
+static void report_error(const char *msg, uintptr_t caller, int abort) {
if (caller == 0)
return;
while (true) {
>From b029cd84f98aaa041204ae3703a99d09469587b9 Mon Sep 17 00:00:00 2001
From: Kirill Stoimenov <kstoimenov at google.com>
Date: Fri, 13 Dec 2024 20:49:28 +0000
Subject: [PATCH 12/15] Refactored the code.
---
.../ubsan_minimal/ubsan_minimal_handlers.cpp | 64 ++++++++++---------
1 file changed, 35 insertions(+), 29 deletions(-)
diff --git a/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp b/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
index 56e97be163e504..e60c8844520fcc 100644
--- a/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
+++ b/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
@@ -39,17 +39,18 @@ static char *append_hex(uintptr_t d, char *buf, const char *end) {
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();
+static void format_msg(const char *kind, uintptr_t caller, char *buf,
+ const char *end) {
+ buf = append_str(MSG_PREFIX, buf, end);
+ buf = append_str(kind, buf, end);
+ buf = append_str(MSG_SUFFIX, 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';
}
-#else
-static void abort_with_message(const char *) { abort(); }
-#endif
-static void report_error(const char *msg, uintptr_t caller, int abort) {
+static void report_error(const char *kind, uintptr_t caller) {
if (caller == 0)
return;
while (true) {
@@ -77,21 +78,24 @@ static void report_error(const char *msg, uintptr_t caller, int abort) {
}
__sanitizer::atomic_store_relaxed(&caller_pcs[sz], caller);
- 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';
-
- // Zero terminate.
- if (p == end) --p;
- *p = '\0';
+ char msg_buf[128];
+ format_msg(kind, caller, msg_buf, msg_buf + sizeof(msg_buf));
message(msg_buf);
- if (abort) abort_with_message(msg_buf); \
}
}
+#if defined(__ANDROID__)
+extern "C" __attribute__((weak)) void android_set_abort_message(const char *);
+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 *kind, uintptr_t caller) { abort(); }
+#endif
+
#if SANITIZER_DEBUG
namespace __sanitizer {
// The DCHECK macro needs this symbol to be defined.
@@ -107,19 +111,21 @@ void NORETURN CheckFailed(const char *file, int, const char *cond, u64, u64) {
#define INTERFACE extern "C" __attribute__((visibility("default")))
-#define HANDLER_RECOVER(name, msg) \
- INTERFACE void __ubsan_handle_##name##_minimal() { \
- report_error(msg, GET_CALLER_PC(), 0); \
+#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() { \
- report_error(msg, GET_CALLER_PC(), 1); \
+#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")
>From 102a3beac7519c91e7799d87bac12bf7205bb474 Mon Sep 17 00:00:00 2001
From: Kirill Stoimenov <kstoimenov at google.com>
Date: Fri, 13 Dec 2024 20:56:59 +0000
Subject: [PATCH 13/15] Removed defines.
---
compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp b/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
index e60c8844520fcc..43a4ad6b3fdd8a 100644
--- a/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
+++ b/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
@@ -21,9 +21,6 @@ static __sanitizer::atomic_uintptr_t caller_pcs[kMaxCallerPcs];
// that "too many errors" has already been reported.
static __sanitizer::atomic_uint32_t caller_pcs_sz;
-#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;
@@ -41,9 +38,9 @@ static char *append_hex(uintptr_t d, char *buf, const char *end) {
static void format_msg(const char *kind, uintptr_t caller, char *buf,
const char *end) {
- buf = append_str(MSG_PREFIX, buf, end);
+ buf = append_str("ubsan: ", buf, end);
buf = append_str(kind, buf, end);
- buf = append_str(MSG_SUFFIX, 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.
>From 8e53a8a091f47579d778949cda50a8256408f702 Mon Sep 17 00:00:00 2001
From: Kirill Stoimenov <kstoimenov at google.com>
Date: Fri, 13 Dec 2024 20:57:37 +0000
Subject: [PATCH 14/15] Removed stdio.h.
---
compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp b/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
index 43a4ad6b3fdd8a..914c2116a0b668 100644
--- a/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
+++ b/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
@@ -1,6 +1,5 @@
#include "sanitizer_common/sanitizer_atomic.h"
-#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
#include <string.h>
>From 054c94fe325d231620b131bea2dcd52d00113747 Mon Sep 17 00:00:00 2001
From: Kirill Stoimenov <kstoimenov at google.com>
Date: Fri, 13 Dec 2024 21:13:29 +0000
Subject: [PATCH 15/15] clang-format
---
.../ubsan_minimal/ubsan_minimal_handlers.cpp | 35 +++++++++++--------
1 file changed, 20 insertions(+), 15 deletions(-)
diff --git a/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp b/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
index 914c2116a0b668..cd0b38618c5cb2 100644
--- a/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
+++ b/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
@@ -21,7 +21,8 @@ static __sanitizer::atomic_uintptr_t caller_pcs[kMaxCallerPcs];
static __sanitizer::atomic_uint32_t caller_pcs_sz;
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;
+ for (const char *p = s; (buf < end) && (*p != '\0'); ++p, ++buf)
+ *buf = *p;
return buf;
}
@@ -42,7 +43,8 @@ static void format_msg(const char *kind, uintptr_t caller, char *buf,
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.
+ if (buf == end)
+ --buf; // Make sure we don't cause a buffer overflow.
*buf = '\0';
}
@@ -51,7 +53,8 @@ static void report_error(const char *kind, uintptr_t caller) {
return;
while (true) {
unsigned sz = __sanitizer::atomic_load_relaxed(&caller_pcs_sz);
- if (sz > kMaxCallerPcs) return; // 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) {
@@ -59,7 +62,8 @@ static void report_error(const char *kind, 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;
+ if (p == caller)
+ return;
}
if (p == 0) continue; // FIXME: yield?
}
@@ -85,7 +89,8 @@ extern "C" __attribute__((weak)) void android_set_abort_message(const char *);
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);
+ if (&android_set_abort_message)
+ android_set_abort_message(msg_buf);
abort();
}
#else
@@ -107,20 +112,20 @@ void NORETURN CheckFailed(const char *file, int, const char *cond, u64, u64) {
#define INTERFACE extern "C" __attribute__((visibility("default")))
-#define HANDLER_RECOVER(name, kind) \
- INTERFACE void __ubsan_handle_##name##_minimal() { \
- report_error(kind, GET_CALLER_PC()); \
+#define HANDLER_RECOVER(name, kind) \
+ INTERFACE void __ubsan_handle_##name##_minimal() { \
+ report_error(kind, GET_CALLER_PC()); \
}
-#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_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, kind) \
- HANDLER_RECOVER(name, kind) \
+#define HANDLER(name, kind) \
+ HANDLER_RECOVER(name, kind) \
HANDLER_NORECOVER(name, kind)
HANDLER(type_mismatch, "type-mismatch")
More information about the llvm-commits
mailing list