[NFC][compiler-rt][hwasan] Move allocation functions into their own file

[leonardchan]

Summary

This removes the __sanitizer_* allocation function definitions from hwasan_interceptors.cpp and moves them into their own file. This way implementations that do not use interceptors at all can just ignore (almost) everything in hwasan_interceptors.cpp.

Also remove some unused headers in hwasan_interceptors.cpp after the move.

[eugenis]

s/vfork/fork/ in the description

I'm afraid removing the fork interceptor will break Android. This is a pretty uncommon condition, but still. It is interesting to note that ASan does not have this fork protection, see https://github.com/google/sanitizers/issues/774 for some history.

I wonder if this could be replaced with pthread_atfork. The order of callback execution seems right, as long as hwasan is registered first. Or, if you want to make progress, add SANITIZER_ANDROID ifdef somewhere.

[mcgrathr]

I think probably the better refactoring here would be to split this file in two: one containing just the allocation functions; and a second with everything else.
On Fuchsia, we'll use the allocation functions but none of the rest of it. So the second whole file could just be under #if !SANITIZER_FUCHSIA.

[leonardchan]

In D103564#2795287, @mcgrathr wrote:

I think probably the better refactoring here would be to split this file in two: one containing just the allocation functions; and a second with everything else.
On Fuchsia, we'll use the allocation functions but none of the rest of it. So the second whole file could just be under #if !SANITIZER_FUCHSIA.

I don't think we can move the __sanitizer_* allocation functions since they're used for aliases for their libc equivalents which require definitions in the same TU:

/usr/local/google/home/leonardchan/llvm-monorepo/llvm-project-4/compiler-rt/lib/hwasan/hwasan_interceptors.cpp:59:1: error: alias must point to a defined variable or function
INTERCEPTOR_ALIAS(void *, aligned_alloc, SIZE_T alignment, SIZE_T size);
^
/usr/local/google/home/leonardchan/llvm-monorepo/llvm-project-4/compiler-rt/lib/hwasan/hwasan_interceptors.cpp:53:7: note: expanded from macro 'INTERCEPTOR_ALIAS'
      ALIAS("__sanitizer_" #FN);                                             \
      ^

Perhaps it might just be easier to just wrap the part of hwasan_interceptors.cpp that don't include the allocation functions with #if !SANITIZER_FUCHSIA, or as @eugenis suggested wrap the fork interceptor with #if SANITIZER_ANDROID.

[mcgrathr]

In D103564#2797657, @leonardchan wrote:

I don't think we can move the __sanitizer_* allocation functions since they're used for aliases for their libc equivalents which require definitions in the same TU:

What I suggested is moving *everything* related to the allocation functions into a separate file.

[leonardchan]

Followed up with Roland's suggestion and moved everything relating to the allocation functions.

[mcgrathr]

compiler-rt/lib/hwasan/hwasan_interceptors.cpp

171

blank lines around #else and #endif lines. But here I think you just omit this. We can define the empty function in a fuchsia-specific file.

[vitalybuka]

compiler-rt/lib/hwasan/hwasan_interceptors.cpp

171

+1 to "empty function in a fuchsia-specific file"

[leonardchan]

compiler-rt/lib/hwasan/hwasan_interceptors.cpp

171

Will add in an eventual hwasan_fuchsia.cpp.

[thakis]

this breaks tests: http://45.33.8.238/linux/48401/step_10.txt

// CHECK: #0 {{.*}} in {{.*}}free{{.*}} {{.*}}hwasan_interceptors.cpp

vs

13: #0 0x5647a467e9a0 in free ../../compiler-rt/lib/hwasan/hwasan_allocation_functions.cpp:75:3

need to update the path in that test

[leonardchan]

In D103564#2806292, @thakis wrote:

this breaks tests: http://45.33.8.238/linux/48401/step_10.txt

// CHECK: #0 {{.*}} in {{.*}}free{{.*}} {{.*}}hwasan_interceptors.cpp

vs

13: #0 0x5647a467e9a0 in free ../../compiler-rt/lib/hwasan/hwasan_allocation_functions.cpp:75:3

need to update the path in that test

Fixed in a9ea0a6a77b30305bfbe1b06c30bf6136f64c1ad.

compiler-rt/lib/hwasan/CMakeLists.txt

 include_directories(..)
 
 # Runtime library sources and build flags.
 set(HWASAN_RTL_SOURCES
   hwasan.cpp
   hwasan_allocator.cpp
+  hwasan_allocation_functions.cpp
   hwasan_dynamic_shadow.cpp
   hwasan_exceptions.cpp
   hwasan_globals.cpp
   hwasan_interceptors.cpp
   hwasan_interceptors_vfork.S
   hwasan_linux.cpp
   hwasan_memintrinsics.cpp
   hwasan_poisoning.cpp

compiler-rt/lib/hwasan/hwasan.h

struct __hw_jmp_buf_struct {
   __hw_register_buf __jmpbuf; // Calling environment.
   int __mask_was_saved;       // Saved the signal mask?
   __hw_sigset_t __saved_mask; // Saved signal mask.
 };
 typedef struct __hw_jmp_buf_struct __hw_jmp_buf[1];
 typedef struct __hw_jmp_buf_struct __hw_sigjmp_buf[1];
 #endif // HWASAN_WITH_INTERCEPTORS && __aarch64__
 
+#define ENSURE_HWASAN_INITED()      \
+  do {                              \
+    CHECK(!hwasan_init_is_running); \
+    if (!hwasan_inited) {           \
+      __hwasan_init();              \
+    }                               \
+  } while (0)
+
 #endif  // HWASAN_H

compiler-rt/lib/hwasan/hwasan_allocation_functions.cpp

+//===-- hwasan_allocation_functions.cpp -----------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file is a part of HWAddressSanitizer.
+//
+// Definitions for __sanitizer allocation functions.
+//
+//===----------------------------------------------------------------------===//
+
+#include "hwasan.h"
+#include "interception/interception.h"
+#include "sanitizer_common/sanitizer_allocator_interface.h"
+#include "sanitizer_common/sanitizer_tls_get_addr.h"
+
+using namespace __hwasan;
+
+static uptr allocated_for_dlsym;
+static const uptr kDlsymAllocPoolSize = 1024;
+static uptr alloc_memory_for_dlsym[kDlsymAllocPoolSize];
+
+static bool IsInDlsymAllocPool(const void *ptr) {
+  uptr off = (uptr)ptr - (uptr)alloc_memory_for_dlsym;
+  return off < sizeof(alloc_memory_for_dlsym);
+}
+
+static void *AllocateFromLocalPool(uptr size_in_bytes) {
+  uptr size_in_words = RoundUpTo(size_in_bytes, kWordSize) / kWordSize;
+  void *mem = (void *)&alloc_memory_for_dlsym[allocated_for_dlsym];
+  allocated_for_dlsym += size_in_words;
+  CHECK_LT(allocated_for_dlsym, kDlsymAllocPoolSize);
+  return mem;
+}
+
+int __sanitizer_posix_memalign(void **memptr, uptr alignment, uptr size) {
+  GET_MALLOC_STACK_TRACE;
+  CHECK_NE(memptr, 0);
+  int res = hwasan_posix_memalign(memptr, alignment, size, &stack);
+  return res;
+}
+
+void *__sanitizer_memalign(uptr alignment, uptr size) {
+  GET_MALLOC_STACK_TRACE;
+  return hwasan_memalign(alignment, size, &stack);
+}
+
+void *__sanitizer_aligned_alloc(uptr alignment, uptr size) {
+  GET_MALLOC_STACK_TRACE;
+  return hwasan_aligned_alloc(alignment, size, &stack);
+}
+
+void *__sanitizer___libc_memalign(uptr alignment, uptr size) {
+  GET_MALLOC_STACK_TRACE;
+  void *ptr = hwasan_memalign(alignment, size, &stack);
+  if (ptr)
+    DTLS_on_libc_memalign(ptr, size);
+  return ptr;
+}
+
+void *__sanitizer_valloc(uptr size) {
+  GET_MALLOC_STACK_TRACE;
+  return hwasan_valloc(size, &stack);
+}
+
+void *__sanitizer_pvalloc(uptr size) {
+  GET_MALLOC_STACK_TRACE;
+  return hwasan_pvalloc(size, &stack);
+}
+
+void __sanitizer_free(void *ptr) {
+  GET_MALLOC_STACK_TRACE;
+  if (!ptr || UNLIKELY(IsInDlsymAllocPool(ptr)))
+    return;
+  hwasan_free(ptr, &stack);
+}
+
+void __sanitizer_cfree(void *ptr) {
+  GET_MALLOC_STACK_TRACE;
+  if (!ptr || UNLIKELY(IsInDlsymAllocPool(ptr)))
+    return;
+  hwasan_free(ptr, &stack);
+}
+
+uptr __sanitizer_malloc_usable_size(const void *ptr) {
+  return __sanitizer_get_allocated_size(ptr);
+}
+
+struct __sanitizer_struct_mallinfo __sanitizer_mallinfo() {
+  __sanitizer_struct_mallinfo sret;
+  internal_memset(&sret, 0, sizeof(sret));
+  return sret;
+}
+
+int __sanitizer_mallopt(int cmd, int value) { return 0; }
+
+void __sanitizer_malloc_stats(void) {
+  // FIXME: implement, but don't call REAL(malloc_stats)!
+}
+
+void *__sanitizer_calloc(uptr nmemb, uptr size) {
+  GET_MALLOC_STACK_TRACE;
+  if (UNLIKELY(!hwasan_inited))
+    // Hack: dlsym calls calloc before REAL(calloc) is retrieved from dlsym.
+    return AllocateFromLocalPool(nmemb * size);
+  return hwasan_calloc(nmemb, size, &stack);
+}
+
+void *__sanitizer_realloc(void *ptr, uptr size) {
+  GET_MALLOC_STACK_TRACE;
+  if (UNLIKELY(IsInDlsymAllocPool(ptr))) {
+    uptr offset = (uptr)ptr - (uptr)alloc_memory_for_dlsym;
+    uptr copy_size = Min(size, kDlsymAllocPoolSize - offset);
+    void *new_ptr;
+    if (UNLIKELY(!hwasan_inited)) {
+      new_ptr = AllocateFromLocalPool(copy_size);
+    } else {
+      copy_size = size;
+      new_ptr = hwasan_malloc(copy_size, &stack);
+    }
+    internal_memcpy(new_ptr, ptr, copy_size);
+    return new_ptr;
+  }
+  return hwasan_realloc(ptr, size, &stack);
+}
+
+void *__sanitizer_reallocarray(void *ptr, uptr nmemb, uptr size) {
+  GET_MALLOC_STACK_TRACE;
+  return hwasan_reallocarray(ptr, nmemb, size, &stack);
+}
+
+void *__sanitizer_malloc(uptr size) {
+  GET_MALLOC_STACK_TRACE;
+  if (UNLIKELY(!hwasan_init_is_running))
+    ENSURE_HWASAN_INITED();
+  if (UNLIKELY(!hwasan_inited))
+    // Hack: dlsym calls malloc before REAL(malloc) is retrieved from dlsym.
+    return AllocateFromLocalPool(size);
+  return hwasan_malloc(size, &stack);
+}
+
+#if HWASAN_WITH_INTERCEPTORS
+#  define INTERCEPTOR_ALIAS(RET, FN, ARGS...)                                 \
+    extern "C" SANITIZER_INTERFACE_ATTRIBUTE RET WRAP(FN)(ARGS)               \
+        ALIAS("__sanitizer_" #FN);                                            \
+    extern "C" SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE RET FN( \
+        ARGS) ALIAS("__sanitizer_" #FN)
+
+INTERCEPTOR_ALIAS(int, posix_memalign, void **memptr, SIZE_T alignment,
+                  SIZE_T size);
+INTERCEPTOR_ALIAS(void *, aligned_alloc, SIZE_T alignment, SIZE_T size);
+INTERCEPTOR_ALIAS(void *, __libc_memalign, SIZE_T alignment, SIZE_T size);
+INTERCEPTOR_ALIAS(void *, valloc, SIZE_T size);
+INTERCEPTOR_ALIAS(void, free, void *ptr);
+INTERCEPTOR_ALIAS(uptr, malloc_usable_size, const void *ptr);
+INTERCEPTOR_ALIAS(void *, calloc, SIZE_T nmemb, SIZE_T size);
+INTERCEPTOR_ALIAS(void *, realloc, void *ptr, SIZE_T size);
+INTERCEPTOR_ALIAS(void *, reallocarray, void *ptr, SIZE_T nmemb, SIZE_T size);
+INTERCEPTOR_ALIAS(void *, malloc, SIZE_T size);
+
+#  if !SANITIZER_FREEBSD && !SANITIZER_NETBSD
+INTERCEPTOR_ALIAS(void *, memalign, SIZE_T alignment, SIZE_T size);
+INTERCEPTOR_ALIAS(void *, pvalloc, SIZE_T size);
+INTERCEPTOR_ALIAS(void, cfree, void *ptr);
+INTERCEPTOR_ALIAS(__sanitizer_struct_mallinfo, mallinfo);
+INTERCEPTOR_ALIAS(int, mallopt, int cmd, int value);
+INTERCEPTOR_ALIAS(void, malloc_stats, void);
+#  endif
+#endif  // #if HWASAN_WITH_INTERCEPTORS

compiler-rt/lib/hwasan/hwasan_interceptors.cpp

 // Interceptors for standard library functions.
 //
 // FIXME: move as many interceptors as possible into
 // sanitizer_common/sanitizer_common_interceptors.h
 //===----------------------------------------------------------------------===//
 
 #include "interception/interception.h"
 #include "hwasan.h"
-#include "hwasan_allocator.h"
-#include "hwasan_mapping.h"
 #include "hwasan_thread.h"
-#include "hwasan_poisoning.h"
-#include "hwasan_report.h"
-#include "sanitizer_common/sanitizer_platform_limits_posix.h"
-#include "sanitizer_common/sanitizer_allocator.h"
-#include "sanitizer_common/sanitizer_allocator_interface.h"
-#include "sanitizer_common/sanitizer_allocator_internal.h"
-#include "sanitizer_common/sanitizer_atomic.h"
-#include "sanitizer_common/sanitizer_common.h"
-#include "sanitizer_common/sanitizer_errno.h"
 #include "sanitizer_common/sanitizer_stackdepot.h"
-#include "sanitizer_common/sanitizer_libc.h"
-#include "sanitizer_common/sanitizer_linux.h"
-#include "sanitizer_common/sanitizer_tls_get_addr.h"
-
-#include <stdarg.h>
-// ACHTUNG! No other system header includes in this file.
-// Ideally, we should get rid of stdarg.h as well.
 
-using namespace __hwasan;
-
-using __sanitizer::memory_order;
-using __sanitizer::atomic_load;
-using __sanitizer::atomic_store;
-using __sanitizer::atomic_uintptr_t;
-
-static uptr allocated_for_dlsym;
-static const uptr kDlsymAllocPoolSize = 1024;
-static uptr alloc_memory_for_dlsym[kDlsymAllocPoolSize];
-
-static bool IsInDlsymAllocPool(const void *ptr) {
-  uptr off = (uptr)ptr - (uptr)alloc_memory_for_dlsym;
-  return off < sizeof(alloc_memory_for_dlsym);
-}
-
-static void *AllocateFromLocalPool(uptr size_in_bytes) {
-  uptr size_in_words = RoundUpTo(size_in_bytes, kWordSize) / kWordSize;
-  void *mem = (void *)&alloc_memory_for_dlsym[allocated_for_dlsym];
-  allocated_for_dlsym += size_in_words;
-  CHECK_LT(allocated_for_dlsym, kDlsymAllocPoolSize);
-  return mem;
-}
-
-#define ENSURE_HWASAN_INITED() do { \
-  CHECK(!hwasan_init_is_running); \
-  if (!hwasan_inited) { \
-    __hwasan_init(); \
-  } \
-} while (0)
-
-
-int __sanitizer_posix_memalign(void **memptr, uptr alignment, uptr size) {
-  GET_MALLOC_STACK_TRACE;
-  CHECK_NE(memptr, 0);
-  int res = hwasan_posix_memalign(memptr, alignment, size, &stack);
-  return res;
-}
-
-void * __sanitizer_memalign(uptr alignment, uptr size) {
-  GET_MALLOC_STACK_TRACE;
-  return hwasan_memalign(alignment, size, &stack);
-}
-
-void * __sanitizer_aligned_alloc(uptr alignment, uptr size) {
-  GET_MALLOC_STACK_TRACE;
-  return hwasan_aligned_alloc(alignment, size, &stack);
-}
-
-void * __sanitizer___libc_memalign(uptr alignment, uptr size) {
-  GET_MALLOC_STACK_TRACE;
-  void *ptr = hwasan_memalign(alignment, size, &stack);
-  if (ptr)
-    DTLS_on_libc_memalign(ptr, size);
-  return ptr;
-}
-
-void * __sanitizer_valloc(uptr size) {
-  GET_MALLOC_STACK_TRACE;
-  return hwasan_valloc(size, &stack);
-}
-
-void * __sanitizer_pvalloc(uptr size) {
-  GET_MALLOC_STACK_TRACE;
-  return hwasan_pvalloc(size, &stack);
-}
-
-void __sanitizer_free(void *ptr) {
-  GET_MALLOC_STACK_TRACE;
-  if (!ptr || UNLIKELY(IsInDlsymAllocPool(ptr))) return;
-  hwasan_free(ptr, &stack);
-}
-
-void __sanitizer_cfree(void *ptr) {
-  GET_MALLOC_STACK_TRACE;
-  if (!ptr || UNLIKELY(IsInDlsymAllocPool(ptr))) return;
-  hwasan_free(ptr, &stack);
-}
-
-uptr __sanitizer_malloc_usable_size(const void *ptr) {
-  return __sanitizer_get_allocated_size(ptr);
-}
+#if !SANITIZER_FUCHSIA
 
-struct __sanitizer_struct_mallinfo __sanitizer_mallinfo() {
-  __sanitizer_struct_mallinfo sret;
-  internal_memset(&sret, 0, sizeof(sret));
-  return sret;
-}
-
-int __sanitizer_mallopt(int cmd, int value) {
-  return 0;
-}
-
-void __sanitizer_malloc_stats(void) {
-  // FIXME: implement, but don't call REAL(malloc_stats)!
-}
-
-void * __sanitizer_calloc(uptr nmemb, uptr size) {
-  GET_MALLOC_STACK_TRACE;
-  if (UNLIKELY(!hwasan_inited))
-    // Hack: dlsym calls calloc before REAL(calloc) is retrieved from dlsym.
-    return AllocateFromLocalPool(nmemb * size);
-  return hwasan_calloc(nmemb, size, &stack);
-}
-
-void * __sanitizer_realloc(void *ptr, uptr size) {
-  GET_MALLOC_STACK_TRACE;
-  if (UNLIKELY(IsInDlsymAllocPool(ptr))) {
-    uptr offset = (uptr)ptr - (uptr)alloc_memory_for_dlsym;
-    uptr copy_size = Min(size, kDlsymAllocPoolSize - offset);
-    void *new_ptr;
-    if (UNLIKELY(!hwasan_inited)) {
-      new_ptr = AllocateFromLocalPool(copy_size);
-    } else {
-      copy_size = size;
-      new_ptr = hwasan_malloc(copy_size, &stack);
-    }
-    internal_memcpy(new_ptr, ptr, copy_size);
-    return new_ptr;
-  }
-  return hwasan_realloc(ptr, size, &stack);
-}
-
-void * __sanitizer_reallocarray(void *ptr, uptr nmemb, uptr size) {
-  GET_MALLOC_STACK_TRACE;
-  return hwasan_reallocarray(ptr, nmemb, size, &stack);
-}
-
-void * __sanitizer_malloc(uptr size) {
-  GET_MALLOC_STACK_TRACE;
-  if (UNLIKELY(!hwasan_init_is_running))
-    ENSURE_HWASAN_INITED();
-  if (UNLIKELY(!hwasan_inited))
-    // Hack: dlsym calls malloc before REAL(malloc) is retrieved from dlsym.
-    return AllocateFromLocalPool(size);
-  return hwasan_malloc(size, &stack);
-}
+using namespace __hwasan;
 
 #if HWASAN_WITH_INTERCEPTORS
-#define INTERCEPTOR_ALIAS(RET, FN, ARGS...)                                  \
-  extern "C" SANITIZER_INTERFACE_ATTRIBUTE RET WRAP(FN)(ARGS)                \
-      ALIAS("__sanitizer_" #FN);                                             \
-  extern "C" SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE RET FN(  \
-      ARGS) ALIAS("__sanitizer_" #FN)
-
-INTERCEPTOR_ALIAS(int, posix_memalign, void **memptr, SIZE_T alignment,
-                  SIZE_T size);
-INTERCEPTOR_ALIAS(void *, aligned_alloc, SIZE_T alignment, SIZE_T size);
-INTERCEPTOR_ALIAS(void *, __libc_memalign, SIZE_T alignment, SIZE_T size);
-INTERCEPTOR_ALIAS(void *, valloc, SIZE_T size);
-INTERCEPTOR_ALIAS(void, free, void *ptr);
-INTERCEPTOR_ALIAS(uptr, malloc_usable_size, const void *ptr);
-INTERCEPTOR_ALIAS(void *, calloc, SIZE_T nmemb, SIZE_T size);
-INTERCEPTOR_ALIAS(void *, realloc, void *ptr, SIZE_T size);
-INTERCEPTOR_ALIAS(void *, reallocarray, void *ptr, SIZE_T nmemb, SIZE_T size);
-INTERCEPTOR_ALIAS(void *, malloc, SIZE_T size);
-
-#if !SANITIZER_FREEBSD && !SANITIZER_NETBSD
-INTERCEPTOR_ALIAS(void *, memalign, SIZE_T alignment, SIZE_T size);
-INTERCEPTOR_ALIAS(void *, pvalloc, SIZE_T size);
-INTERCEPTOR_ALIAS(void, cfree, void *ptr);
-INTERCEPTOR_ALIAS(__sanitizer_struct_mallinfo, mallinfo);
-INTERCEPTOR_ALIAS(int, mallopt, int cmd, int value);
-INTERCEPTOR_ALIAS(void, malloc_stats, void);
-#endif
 
 struct ThreadStartArg {
   thread_callback_t callback;
   void *param;
 };
 
 static void *HwasanThreadStartFunc(void *arg) {
   __hwasan_thread_enter();
#if defined(__linux__)
   INTERCEPT_FUNCTION(vfork);
 #endif  // __linux__
   INTERCEPT_FUNCTION(pthread_create);
 #endif
 
   inited = 1;
 }
 } // namespace __hwasan
+
+#endif  // #if !SANITIZER_FUCHSIA