[asan] Allocator support for Fuchsia

[mcgrathr]

Split out from D35865.

[cryptoad]

lib/asan/asan_allocator.h

122 ↗

(On Diff #109219)

Random question:
SANITIZER_CAN_USE_ALLOCATOR64 is false for __aarch64__ by default (https://github.com/llvm-mirror/compiler-rt/blob/master/lib/sanitizer_common/sanitizer_platform.h#L193) except for Android.
I didn't see any change for Fuchsia there (I might have missed it), and as I understand Fuchsia supports AArch64.
Would setting SANITIZER_CAN_USE_ALLOCATOR64 for AArch64 in sanitizer_platform.h make sense as well?

[mcgrathr]

Make sure SANITIZER_CAN_USE_ALLOCATOR64 is 1 on aarch64.

[alekseyshl]

lib/asan/asan_malloc_linux.cc

87 ↗

(On Diff #109248)

How about minimizing the ifdefed section and go with this:

INLINE bool MaybeInDlsym() {
#if SANITIZER_FUCHSIA
  return false;
#else
  return asan_init_is_running;
#endif
}

Wouldn't the compiler be smart enough to optimize out all that unused stuff? And we will have to do the same for IsInDlsymAllocPool to help it to figure that out.

[mcgrathr]

Minimal #if'ing out for alloc-inside-dlsym support, let the compiler optimize it away.

[vitalybuka]

lib/asan/asan_malloc_linux.cc

56 ↗

(On Diff #109395)

Why do you need to ifdef this?
I'd expect that asan_init_is_running will be false on fuchsia anyway

[mcgrathr]

lib/asan/asan_malloc_linux.cc

56 ↗

(On Diff #109395)

It will always be false at runtime, but the compiler can't tell that statically.
I want the dead code (and its static data bloat) to be compiled out, which it is with this.

[vitalybuka]

lib/asan/asan_malloc_linux.cc

56 ↗

(On Diff #109395)

could you please remove ifdef then? We don't like them only when no other options.
We can add them later if it cause real problems.

56 ↗

(On Diff #109395)

actually just undo MaybeInDlsym change

[mcgrathr]

lib/asan/asan_malloc_linux.cc

56 ↗

(On Diff #109395)

It sounds like you want all the dead code and data bloat to be there.
It won't cause problems other than marginal slowdown and memory bloat.
But I don't see a good reason to leave it there when eliminating it is easy and noninvasive.
I have a new version where there is no #if but only a single if (constant) that will cause everything to be compiled away.
It required a semantic change to IsInDlsymAllocPool but one that is harmless and lets the compiler grok that it can compile it away.

[mcgrathr]

No more #if to get the alloc-from-dlsym hacks compiled away, but they still get compiled away.

[alekseyshl]

lib/asan/asan_malloc_linux.cc

46 ↗

(On Diff #109441)

Make it inline.

48 ↗

(On Diff #109441)

UNLIKELY is not required here.

[mcgrathr]

lib/asan/asan_malloc_linux.cc

46 ↗

(On Diff #109441)

Done, though the compiler does inline it without the keyword.

[mcgrathr]

Added INLINE, removed UNLIKELY.

[mcgrathr]

This is ready to land now.
Please land it for me!

Thanks,
Roland

[vitalybuka]

lib/asan/asan_malloc_linux.cc

35 ↗

(On Diff #109441)

Why sizeof is gone here?

[mcgrathr]

lib/asan/asan_malloc_linux.cc

35 ↗

(On Diff #109441)

Previously it was sizeof(alloc_memory_for_dlsym), the static size of the static buffer.
I changed it to allocated_for_dlsym, which is the dynamic count of how much of the static buffer has been used.

The reason for the change is that in the SANITIZER_FUCHSIA case, the compiler can see statically that nothing ever sets allocated_for_dlsym and so it will always be zero and hence this comparison is always false and it can optimize away all the related dead code entirely.

In theory the change is harmless because the offset of any allocated block will be less than the current count.
However, I failed to notice that the count in allocated_for_dlsym is of sizeof(uptr) units while the comparison is being made against a byte count. Hence I introduced https://bugs.llvm.org/show_bug.cgi?id=34085 with the change. I'll do a follow-up change that gets the comparison arithmetic right, so that bug won't happen but Fuchsia will get back the dead code elimination I was going for with the change.

compiler-rt/trunk/lib/asan/asan_allocator.h

 };
 
 struct AsanMapUnmapCallback {
   void OnMap(uptr p, uptr size) const;
   void OnUnmap(uptr p, uptr size) const;
 };
 
 #if SANITIZER_CAN_USE_ALLOCATOR64
-# if defined(__powerpc64__)
+# if SANITIZER_FUCHSIA
+const uptr kAllocatorSpace = ~(uptr)0;
+const uptr kAllocatorSize  =  0x40000000000ULL;  // 4T.
+typedef DefaultSizeClassMap SizeClassMap;
+# elif defined(__powerpc64__)
 const uptr kAllocatorSpace =  0xa0000000000ULL;
 const uptr kAllocatorSize  =  0x20000000000ULL;  // 2T.
 typedef DefaultSizeClassMap SizeClassMap;
 # elif defined(__aarch64__) && SANITIZER_ANDROID
 const uptr kAllocatorSpace =  0x3000000000ULL;
 const uptr kAllocatorSize  =  0x2000000000ULL;  // 128G.
 typedef VeryCompactSizeClassMap SizeClassMap;
 # elif defined(__aarch64__)

compiler-rt/trunk/lib/asan/asan_malloc_linux.cc

 // This file is a part of AddressSanitizer, an address sanity checker.
 //
 // Linux-specific malloc interception.
 // We simply define functions like malloc, free, realloc, etc.
 // They will replace the corresponding libc functions automagically.
 //===----------------------------------------------------------------------===//
 
 #include "sanitizer_common/sanitizer_platform.h"
-#if SANITIZER_FREEBSD || SANITIZER_LINUX
+#if SANITIZER_FREEBSD || SANITIZER_FUCHSIA || SANITIZER_LINUX
 
 #include "sanitizer_common/sanitizer_tls_get_addr.h"
 #include "asan_allocator.h"
 #include "asan_interceptors.h"
 #include "asan_internal.h"
 #include "asan_stack.h"
 
 // ---------------------- Replacement functions ---------------- {{{1
 using namespace __asan;  // NOLINT
 
 static uptr allocated_for_dlsym;
 static const uptr kDlsymAllocPoolSize = 1024;
 static uptr alloc_memory_for_dlsym[kDlsymAllocPoolSize];
 
-static bool IsInDlsymAllocPool(const void *ptr) {
+static INLINE bool IsInDlsymAllocPool(const void *ptr) {
   uptr off = (uptr)ptr - (uptr)alloc_memory_for_dlsym;
-  return off < sizeof(alloc_memory_for_dlsym);
+  return off < allocated_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;
 }
 
+static INLINE bool MaybeInDlsym() {
+  // Fuchsia doesn't use dlsym-based interceptors.
+  return !SANITIZER_FUCHSIA && asan_init_is_running;
+}
+
+static void *ReallocFromLocalPool(void *ptr, uptr size) {
+  const uptr offset = (uptr)ptr - (uptr)alloc_memory_for_dlsym;
+  const uptr copy_size = Min(size, kDlsymAllocPoolSize - offset);
+  void *new_ptr;
+  if (UNLIKELY(MaybeInDlsym())) {
+    new_ptr = AllocateFromLocalPool(size);
+  } else {
+    ENSURE_ASAN_INITED();
+    GET_STACK_TRACE_MALLOC;
+    new_ptr = asan_malloc(size, &stack);
+  }
+  internal_memcpy(new_ptr, ptr, copy_size);
+  return new_ptr;
+}
+
 INTERCEPTOR(void, free, void *ptr) {
   GET_STACK_TRACE_FREE;
   if (UNLIKELY(IsInDlsymAllocPool(ptr)))
     return;
   asan_free(ptr, &stack, FROM_MALLOC);
 }
 
 #if SANITIZER_INTERCEPT_CFREE
 INTERCEPTOR(void, cfree, void *ptr) {
   GET_STACK_TRACE_FREE;
   if (UNLIKELY(IsInDlsymAllocPool(ptr)))
     return;
   asan_free(ptr, &stack, FROM_MALLOC);
 }
 #endif // SANITIZER_INTERCEPT_CFREE
 
 INTERCEPTOR(void*, malloc, uptr size) {
-  if (UNLIKELY(asan_init_is_running))
+  if (UNLIKELY(MaybeInDlsym()))
     // Hack: dlsym calls malloc before REAL(malloc) is retrieved from dlsym.
     return AllocateFromLocalPool(size);
   ENSURE_ASAN_INITED();
   GET_STACK_TRACE_MALLOC;
   return asan_malloc(size, &stack);
 }
 
 INTERCEPTOR(void*, calloc, uptr nmemb, uptr size) {
-  if (UNLIKELY(asan_init_is_running))
+  if (UNLIKELY(MaybeInDlsym()))
     // Hack: dlsym calls calloc before REAL(calloc) is retrieved from dlsym.
     return AllocateFromLocalPool(nmemb * size);
   ENSURE_ASAN_INITED();
   GET_STACK_TRACE_MALLOC;
   return asan_calloc(nmemb, size, &stack);
 }
 
 INTERCEPTOR(void*, realloc, void *ptr, uptr size) {
-  if (UNLIKELY(IsInDlsymAllocPool(ptr))) {
-    const uptr offset = (uptr)ptr - (uptr)alloc_memory_for_dlsym;
-    const uptr copy_size = Min(size, kDlsymAllocPoolSize - offset);
-    void *new_ptr;
-    if (UNLIKELY(asan_init_is_running)) {
-      new_ptr = AllocateFromLocalPool(size);
-    } else {
-      ENSURE_ASAN_INITED();
-      GET_STACK_TRACE_MALLOC;
-      new_ptr = asan_malloc(size, &stack);
-    }
-    internal_memcpy(new_ptr, ptr, copy_size);
-    return new_ptr;
-  }
-  if (UNLIKELY(asan_init_is_running))
+  if (UNLIKELY(IsInDlsymAllocPool(ptr)))
+    return ReallocFromLocalPool(ptr, size);
+  if (UNLIKELY(MaybeInDlsym()))
     return AllocateFromLocalPool(size);
   ENSURE_ASAN_INITED();
   GET_STACK_TRACE_MALLOC;
   return asan_realloc(ptr, size, &stack);
 }
 
 #if SANITIZER_INTERCEPT_MEMALIGN
 INTERCEPTOR(void*, memalign, uptr boundary, uptr size) {

 #else  // SANITIZER_ANDROID
 
 namespace __asan {
 void ReplaceSystemMalloc() {
 }
 }  // namespace __asan
 #endif  // SANITIZER_ANDROID
 
-#endif  // SANITIZER_FREEBSD || SANITIZER_LINUX
+#endif  // SANITIZER_FREEBSD || SANITIZER_FUCHSIA || SANITIZER_LINUX

compiler-rt/trunk/lib/sanitizer_common/sanitizer_platform.h

 #endif
 
 // By default we allow to use SizeClassAllocator64 on 64-bit platform.
 // But in some cases (e.g. AArch64's 39-bit address space) SizeClassAllocator64
 // does not work well and we need to fallback to SizeClassAllocator32.
 // For such platforms build this code with -DSANITIZER_CAN_USE_ALLOCATOR64=0 or
 // change the definition of SANITIZER_CAN_USE_ALLOCATOR64 here.
 #ifndef SANITIZER_CAN_USE_ALLOCATOR64
-# if SANITIZER_ANDROID && defined(__aarch64__)
+# if (SANITIZER_ANDROID && defined(__aarch64__)) || SANITIZER_FUCHSIA
 #  define SANITIZER_CAN_USE_ALLOCATOR64 1
 # elif defined(__mips64) || defined(__aarch64__)
 #  define SANITIZER_CAN_USE_ALLOCATOR64 0
 # else
 #  define SANITIZER_CAN_USE_ALLOCATOR64 (SANITIZER_WORDSIZE == 64)
 # endif
 #endif