[compiler-rt] Add aligned_alloc to macOS tsan interceptors (PR #79198)

Chris Apple via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 23 11:58:32 PST 2024


https://github.com/cjappl created https://github.com/llvm/llvm-project/pull/79198

Directly following on the heels of the suggestion given in #77543 

Following the example in the tsan libdispatch interceptors (https://github.com/llvm/llvm-project/blob/f7669ba3d9443bc95dd63fa25beea13e6265fdc5/compiler-rt/lib/tsan/rtl/tsan_interceptors_libdispatch.cpp#L226-L247)

This PR forward declares `aligned_alloc` on systems that may not have it (like pre-10.15 OSX). There is a unit test provided to ensure this works as intended, which passes for me when I compile both with 

SANITIZER_MIN_OSX_VERSION:STRING=10.10
and 
SANITIZER_MIN_OSX_VERSION:STRING=14.0

This test is almost an exact copy of another tsan test, found [here](https://github.com/llvm/llvm-project/blob/main/compiler-rt/test/tsan/free_race.c).

To facilitate this in a more reusable way, a small refactor was done:
1. Removed SANITIZER_WEAK_IMPORT from [compiler-rt/lib/tsan/rtl/tsan_dispatch_defs.h](https://github.com/llvm/llvm-project/compare/main...cjappl:llvm-project:add_aligned_alloc?expand=1#diff-8178a9a7b2839ad0a02ed04254e674686e73750fbd7d274a6abf41cb3b80b83b). This was placed in sanitizer_common so it can be accessible by more sanitizers in the future.




>From 5da5f35088e950b7bd648a92e2538f24f8da4358 Mon Sep 17 00:00:00 2001
From: Chris Apple <14171107+cjappl at users.noreply.github.com>
Date: Tue, 23 Jan 2024 10:20:36 -0800
Subject: [PATCH 1/5] Initial working version

---
 .../lib/tsan/rtl/tsan_interceptors_posix.cpp  |  7 +++-
 compiler-rt/test/tsan/free_race.c             | 40 ++++++++++++-------
 2 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
index a9f6673ac44e90e..1365478e37eea7a 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
@@ -815,14 +815,19 @@ TSAN_INTERCEPTOR(void*, memalign, uptr align, uptr sz) {
 #define TSAN_MAYBE_INTERCEPT_MEMALIGN
 #endif
 
-#if !SANITIZER_APPLE
+#if !SANITIZER_APPLE || defined(__MAC_10_16)
+# define SANITIZER_WEAK_IMPORT extern "C" __attribute((weak_import))
+SANITIZER_WEAK_IMPORT void *aligned_alloc(SIZE_T __alignment, SIZE_T __size);
+
 TSAN_INTERCEPTOR(void*, aligned_alloc, uptr align, uptr sz) {
   if (in_symbolizer())
     return InternalAlloc(sz, nullptr, align);
   SCOPED_INTERCEPTOR_RAW(aligned_alloc, align, sz);
   return user_aligned_alloc(thr, pc, align, sz);
 }
+#endif
 
+#if !SANITIZER_APPLE
 TSAN_INTERCEPTOR(void*, valloc, uptr sz) {
   if (in_symbolizer())
     return InternalAlloc(sz, nullptr, GetPageSizeCached());
diff --git a/compiler-rt/test/tsan/free_race.c b/compiler-rt/test/tsan/free_race.c
index af86b447a07cdbd..095b3efe24758be 100644
--- a/compiler-rt/test/tsan/free_race.c
+++ b/compiler-rt/test/tsan/free_race.c
@@ -1,9 +1,18 @@
-// RUN: %clang_tsan -O1 %s -o %t
-// RUN: %deflake %run %t | FileCheck %s --check-prefix=CHECK-NOZUPP
-// RUN: %env_tsan_opts=suppressions='%s.supp':print_suppressions=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-SUPP
+// RUN: %clang_tsan -O1 %s -o %t -undefined dynamic_lookup 
+// RUN: %deflake %run %t | FileCheck %s 
 
 #include "test.h"
 
+#ifdef __APPLE__
+#include <os/availability.h>
+
+// Allow compilation with pre-aligned-alloc SDKs
+API_AVAILABLE(macos(10.15), ios(13.0), tvos(13.0), watchos(6.0))
+void *aligned_alloc(size_t alignment, size_t size);
+#else
+#include <stdlib.h>
+#endif
+
 int *mem;
 pthread_mutex_t mtx;
 
@@ -24,8 +33,13 @@ __attribute__((noinline)) void *Thread2(void *x) {
 }
 
 int main() {
+  if (aligned_alloc == NULL) {
+    fprintf(stderr, "Done.\n");
+    return 0;
+  }
+
   barrier_init(&barrier, 2);
-  mem = (int*)malloc(100);
+  mem = (int*)aligned_alloc(8, 8);
   pthread_mutex_init(&mtx, 0);
   pthread_t t;
   pthread_create(&t, NULL, Thread1, NULL);
@@ -35,13 +49,11 @@ int main() {
   return 0;
 }
 
-// CHECK-NOZUPP: WARNING: ThreadSanitizer: heap-use-after-free
-// CHECK-NOZUPP:   Write of size 4 at {{.*}} by main thread{{.*}}:
-// CHECK-NOZUPP:     #0 Thread2
-// CHECK-NOZUPP:     #1 main
-// CHECK-NOZUPP:   Previous write of size 8 at {{.*}} by thread T1{{.*}}:
-// CHECK-NOZUPP:     #0 free
-// CHECK-NOZUPP:     #{{(1|2)}} Thread1
-// CHECK-NOZUPP: SUMMARY: ThreadSanitizer: heap-use-after-free{{.*}}Thread2
-// CHECK-SUPP:   ThreadSanitizer: Matched 1 suppressions
-// CHECK-SUPP:    1 race:^Thread2$
+// CHECK: WARNING: ThreadSanitizer: heap-use-after-free
+// CHECK:   Write of size 4 at {{.*}} by main thread{{.*}}:
+// CHECK:     #0 Thread2
+// CHECK:     #1 main
+// CHECK:   Previous write of size 8 at {{.*}} by thread T1{{.*}}:
+// CHECK:     #0 free
+// CHECK:     #{{(1|2)}} Thread1
+// CHECK: SUMMARY: ThreadSanitizer: heap-use-after-free{{.*}}Thread2

>From fc27e77c7ae77c0556d213cd91a552a34cf7cbe4 Mon Sep 17 00:00:00 2001
From: Chris Apple <14171107+cjappl at users.noreply.github.com>
Date: Tue, 23 Jan 2024 10:35:22 -0800
Subject: [PATCH 2/5] Refactor, moving WEAK_IMPORT up to common defs

---
 .../lib/sanitizer_common/sanitizer_internal_defs.h  | 13 ++++++++++---
 compiler-rt/lib/tsan/rtl/tsan_dispatch_defs.h       |  7 -------
 .../lib/tsan/rtl/tsan_interceptors_posix.cpp        |  2 +-
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h b/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h
index 3809669dd48bb3b..86437aaf4457e30 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h
@@ -33,15 +33,22 @@
 # define SANITIZER_INTERFACE_ATTRIBUTE __declspec(dllimport)
 #else
 # define SANITIZER_INTERFACE_ATTRIBUTE __declspec(dllexport)
-#endif
+#endif // SANITIZER_IMPORT_INTERFACE
 # define SANITIZER_WEAK_ATTRIBUTE
+#define  SANITIZER_WEAK_IMPORT
 #elif SANITIZER_GO
 # define SANITIZER_INTERFACE_ATTRIBUTE
 # define SANITIZER_WEAK_ATTRIBUTE
-#else
+# define SANITIZER_WEAK_IMPORT
+#else // NOT SANITIZER_WINDOWS
 # define SANITIZER_INTERFACE_ATTRIBUTE __attribute__((visibility("default")))
 # define SANITIZER_WEAK_ATTRIBUTE  __attribute__((weak))
-#endif
+#if SANITIZER_APPLE
+# define SANITIZER_WEAK_IMPORT extern "C" __attribute((weak_import))
+#else // NOT SANITIZER_APPLE
+# define SANITIZER_WEAK_IMPORT extern "C" SANITIZER_WEAK_ATTRIBUTE
+#endif // SANITIZER_APPLE
+#endif // SANITIZER_WINDOWS
 
 //--------------------------- WEAK FUNCTIONS ---------------------------------//
 // When working with weak functions, to simplify the code and make it more
diff --git a/compiler-rt/lib/tsan/rtl/tsan_dispatch_defs.h b/compiler-rt/lib/tsan/rtl/tsan_dispatch_defs.h
index 54c0b0ba4b409a4..8d38beb0b0a2092 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_dispatch_defs.h
+++ b/compiler-rt/lib/tsan/rtl/tsan_dispatch_defs.h
@@ -56,13 +56,6 @@ extern const dispatch_block_t _dispatch_data_destructor_munmap;
 # define DISPATCH_NOESCAPE
 #endif
 
-#if SANITIZER_APPLE
-# define SANITIZER_WEAK_IMPORT extern "C" __attribute((weak_import))
-#else
-# define SANITIZER_WEAK_IMPORT extern "C" __attribute((weak))
-#endif
-
-
 // Data types used in dispatch APIs
 typedef unsigned long size_t;
 typedef unsigned long uintptr_t;
diff --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
index 1365478e37eea7a..6499097a5d1afaa 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
@@ -14,6 +14,7 @@
 
 #include "sanitizer_common/sanitizer_atomic.h"
 #include "sanitizer_common/sanitizer_errno.h"
+#include "sanitizer_common/sanitizer_internal_defs.h"
 #include "sanitizer_common/sanitizer_libc.h"
 #include "sanitizer_common/sanitizer_linux.h"
 #include "sanitizer_common/sanitizer_platform_limits_netbsd.h"
@@ -816,7 +817,6 @@ TSAN_INTERCEPTOR(void*, memalign, uptr align, uptr sz) {
 #endif
 
 #if !SANITIZER_APPLE || defined(__MAC_10_16)
-# define SANITIZER_WEAK_IMPORT extern "C" __attribute((weak_import))
 SANITIZER_WEAK_IMPORT void *aligned_alloc(SIZE_T __alignment, SIZE_T __size);
 
 TSAN_INTERCEPTOR(void*, aligned_alloc, uptr align, uptr sz) {

>From 9e49edb9549f0bc5a4d4a78ef127cd5324a79edc Mon Sep 17 00:00:00 2001
From: Chris Apple <14171107+cjappl at users.noreply.github.com>
Date: Tue, 23 Jan 2024 10:37:13 -0800
Subject: [PATCH 3/5] Introduce new test file

---
 compiler-rt/test/tsan/free_race.c             | 40 +++++--------
 .../test/tsan/free_race_aligned_alloc.c       | 59 +++++++++++++++++++
 2 files changed, 73 insertions(+), 26 deletions(-)
 create mode 100644 compiler-rt/test/tsan/free_race_aligned_alloc.c

diff --git a/compiler-rt/test/tsan/free_race.c b/compiler-rt/test/tsan/free_race.c
index 095b3efe24758be..af86b447a07cdbd 100644
--- a/compiler-rt/test/tsan/free_race.c
+++ b/compiler-rt/test/tsan/free_race.c
@@ -1,18 +1,9 @@
-// RUN: %clang_tsan -O1 %s -o %t -undefined dynamic_lookup 
-// RUN: %deflake %run %t | FileCheck %s 
+// RUN: %clang_tsan -O1 %s -o %t
+// RUN: %deflake %run %t | FileCheck %s --check-prefix=CHECK-NOZUPP
+// RUN: %env_tsan_opts=suppressions='%s.supp':print_suppressions=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-SUPP
 
 #include "test.h"
 
-#ifdef __APPLE__
-#include <os/availability.h>
-
-// Allow compilation with pre-aligned-alloc SDKs
-API_AVAILABLE(macos(10.15), ios(13.0), tvos(13.0), watchos(6.0))
-void *aligned_alloc(size_t alignment, size_t size);
-#else
-#include <stdlib.h>
-#endif
-
 int *mem;
 pthread_mutex_t mtx;
 
@@ -33,13 +24,8 @@ __attribute__((noinline)) void *Thread2(void *x) {
 }
 
 int main() {
-  if (aligned_alloc == NULL) {
-    fprintf(stderr, "Done.\n");
-    return 0;
-  }
-
   barrier_init(&barrier, 2);
-  mem = (int*)aligned_alloc(8, 8);
+  mem = (int*)malloc(100);
   pthread_mutex_init(&mtx, 0);
   pthread_t t;
   pthread_create(&t, NULL, Thread1, NULL);
@@ -49,11 +35,13 @@ int main() {
   return 0;
 }
 
-// CHECK: WARNING: ThreadSanitizer: heap-use-after-free
-// CHECK:   Write of size 4 at {{.*}} by main thread{{.*}}:
-// CHECK:     #0 Thread2
-// CHECK:     #1 main
-// CHECK:   Previous write of size 8 at {{.*}} by thread T1{{.*}}:
-// CHECK:     #0 free
-// CHECK:     #{{(1|2)}} Thread1
-// CHECK: SUMMARY: ThreadSanitizer: heap-use-after-free{{.*}}Thread2
+// CHECK-NOZUPP: WARNING: ThreadSanitizer: heap-use-after-free
+// CHECK-NOZUPP:   Write of size 4 at {{.*}} by main thread{{.*}}:
+// CHECK-NOZUPP:     #0 Thread2
+// CHECK-NOZUPP:     #1 main
+// CHECK-NOZUPP:   Previous write of size 8 at {{.*}} by thread T1{{.*}}:
+// CHECK-NOZUPP:     #0 free
+// CHECK-NOZUPP:     #{{(1|2)}} Thread1
+// CHECK-NOZUPP: SUMMARY: ThreadSanitizer: heap-use-after-free{{.*}}Thread2
+// CHECK-SUPP:   ThreadSanitizer: Matched 1 suppressions
+// CHECK-SUPP:    1 race:^Thread2$
diff --git a/compiler-rt/test/tsan/free_race_aligned_alloc.c b/compiler-rt/test/tsan/free_race_aligned_alloc.c
new file mode 100644
index 000000000000000..095b3efe24758be
--- /dev/null
+++ b/compiler-rt/test/tsan/free_race_aligned_alloc.c
@@ -0,0 +1,59 @@
+// RUN: %clang_tsan -O1 %s -o %t -undefined dynamic_lookup 
+// RUN: %deflake %run %t | FileCheck %s 
+
+#include "test.h"
+
+#ifdef __APPLE__
+#include <os/availability.h>
+
+// Allow compilation with pre-aligned-alloc SDKs
+API_AVAILABLE(macos(10.15), ios(13.0), tvos(13.0), watchos(6.0))
+void *aligned_alloc(size_t alignment, size_t size);
+#else
+#include <stdlib.h>
+#endif
+
+int *mem;
+pthread_mutex_t mtx;
+
+void *Thread1(void *x) {
+  pthread_mutex_lock(&mtx);
+  free(mem);
+  pthread_mutex_unlock(&mtx);
+  barrier_wait(&barrier);
+  return NULL;
+}
+
+__attribute__((noinline)) void *Thread2(void *x) {
+  barrier_wait(&barrier);
+  pthread_mutex_lock(&mtx);
+  mem[0] = 42;
+  pthread_mutex_unlock(&mtx);
+  return NULL;
+}
+
+int main() {
+  if (aligned_alloc == NULL) {
+    fprintf(stderr, "Done.\n");
+    return 0;
+  }
+
+  barrier_init(&barrier, 2);
+  mem = (int*)aligned_alloc(8, 8);
+  pthread_mutex_init(&mtx, 0);
+  pthread_t t;
+  pthread_create(&t, NULL, Thread1, NULL);
+  Thread2(0);
+  pthread_join(t, NULL);
+  pthread_mutex_destroy(&mtx);
+  return 0;
+}
+
+// CHECK: WARNING: ThreadSanitizer: heap-use-after-free
+// CHECK:   Write of size 4 at {{.*}} by main thread{{.*}}:
+// CHECK:     #0 Thread2
+// CHECK:     #1 main
+// CHECK:   Previous write of size 8 at {{.*}} by thread T1{{.*}}:
+// CHECK:     #0 free
+// CHECK:     #{{(1|2)}} Thread1
+// CHECK: SUMMARY: ThreadSanitizer: heap-use-after-free{{.*}}Thread2

>From 0c7fed9f0768024f8a39944b76251db635445a70 Mon Sep 17 00:00:00 2001
From: Chris Apple <14171107+cjappl at users.noreply.github.com>
Date: Tue, 23 Jan 2024 11:41:48 -0800
Subject: [PATCH 4/5] Aligned alloc test cleaned up

---
 .../test/tsan/free_race_aligned_alloc.c       | 25 +++++++++++--------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/compiler-rt/test/tsan/free_race_aligned_alloc.c b/compiler-rt/test/tsan/free_race_aligned_alloc.c
index 095b3efe24758be..5e9c496925cb199 100644
--- a/compiler-rt/test/tsan/free_race_aligned_alloc.c
+++ b/compiler-rt/test/tsan/free_race_aligned_alloc.c
@@ -1,18 +1,21 @@
-// RUN: %clang_tsan -O1 %s -o %t -undefined dynamic_lookup 
+// RUN: %clang_tsan -O1 %s -o %t -undefined dynamic_lookup
 // RUN: %deflake %run %t | FileCheck %s 
 
 #include "test.h"
 
-#ifdef __APPLE__
-#include <os/availability.h>
+#include <stdlib.h>
+
+#if defined(__cplusplus) && (__cplusplus >= 201703L)
+#define HAVE_ALIGNED_ALLOC 1
+#endif
 
-// Allow compilation with pre-aligned-alloc SDKs
-API_AVAILABLE(macos(10.15), ios(13.0), tvos(13.0), watchos(6.0))
-void *aligned_alloc(size_t alignment, size_t size);
+#if defined(__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__) && \
+    __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ < 101500
+#define HAVE_ALIGNED_ALLOC 0 
 #else
-#include <stdlib.h>
 #endif
 
+
 int *mem;
 pthread_mutex_t mtx;
 
@@ -33,13 +36,13 @@ __attribute__((noinline)) void *Thread2(void *x) {
 }
 
 int main() {
-  if (aligned_alloc == NULL) {
-    fprintf(stderr, "Done.\n");
-    return 0;
-  }
 
   barrier_init(&barrier, 2);
+#if HAVE_ALIGNED_ALLOC
   mem = (int*)aligned_alloc(8, 8);
+#else
+  mem = (int*)malloc(8);
+#endif
   pthread_mutex_init(&mtx, 0);
   pthread_t t;
   pthread_create(&t, NULL, Thread1, NULL);

>From f4e1f4149d3a4b3babcd15ee2d326c195fed7888 Mon Sep 17 00:00:00 2001
From: Chris Apple <14171107+cjappl at users.noreply.github.com>
Date: Tue, 23 Jan 2024 11:47:47 -0800
Subject: [PATCH 5/5] Clean up for PR

---
 .../lib/sanitizer_common/sanitizer_internal_defs.h       | 2 +-
 compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp     | 9 +++++++--
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h b/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h
index 86437aaf4457e30..75fda42a480a783 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h
@@ -35,7 +35,7 @@
 # define SANITIZER_INTERFACE_ATTRIBUTE __declspec(dllexport)
 #endif // SANITIZER_IMPORT_INTERFACE
 # define SANITIZER_WEAK_ATTRIBUTE
-#define  SANITIZER_WEAK_IMPORT
+# define  SANITIZER_WEAK_IMPORT
 #elif SANITIZER_GO
 # define SANITIZER_INTERFACE_ATTRIBUTE
 # define SANITIZER_WEAK_ATTRIBUTE
diff --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
index 6499097a5d1afaa..1e27807e26a498b 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
@@ -816,9 +816,14 @@ TSAN_INTERCEPTOR(void*, memalign, uptr align, uptr sz) {
 #define TSAN_MAYBE_INTERCEPT_MEMALIGN
 #endif
 
-#if !SANITIZER_APPLE || defined(__MAC_10_16)
+// aligned_alloc was introduced in macOS 10.15
+// Linking will fail when using an older SDK
+#if !SANITIZER_APPLE || defined(__MAC_10_15)
+// macOS 10.15 is greater than our minimal deployment target.  To ensure we
+// generate a weak reference so the TSan dylib continues to work on older
+// systems, we need to forward declare the intercepted function as "weak
+// imports".   
 SANITIZER_WEAK_IMPORT void *aligned_alloc(SIZE_T __alignment, SIZE_T __size);
-
 TSAN_INTERCEPTOR(void*, aligned_alloc, uptr align, uptr sz) {
   if (in_symbolizer())
     return InternalAlloc(sz, nullptr, align);



More information about the llvm-commits mailing list