[tsan] Return 0 from malloc_size for non-malloc'd pointers

[kubamracek]

Summary

In http://reviews.llvm.org/D19100, I introduced a bug: On OS X, existing programs rely on malloc_size() to detect whether a pointer comes from heap memory (malloc_size returns non-zero) or not. We have to distinguish between a zero-sized allocation (where we need to return 1 from malloc_size, due to other binary compatibility reasons, see http://reviews.llvm.org/D19100), and pointers that are not returned from malloc at all.

[aizatsky]

lib/tsan/rtl/tsan_mman.cc

167

this changes behavior on all platforms, not only Mac, right? Is it intended?

[kubamracek]

lib/tsan/rtl/tsan_mman.cc

167

Correct, this changes all platforms. It is intended, I should have said that in the original description. At least on Linux, malloc_usable_size also returns non-zero value for a zero-sized allocation, I guess other platforms behave the same. Running under TSan shouldn't change that to avoid breaking compatibility.

lib/tsan/rtl/tsan_malloc_mac.cc

#define COMMON_MALLOC_VALLOC(size)                          \
   SCOPED_INTERCEPTOR_RAW(valloc, size);                     \
   void *p = user_alloc(thr, pc, size, GetPageSizeCached())
 #define COMMON_MALLOC_FREE(ptr)      \
   if (cur_thread()->in_symbolizer)   \
     return REAL(free)(ptr);          \
   SCOPED_INTERCEPTOR_RAW(free, ptr); \
   user_free(thr, pc, ptr)
 #define COMMON_MALLOC_SIZE(ptr) \
-  uptr size = user_alloc_usable_size(ptr); \
-  if (size == 0) size = 1;
+  uptr size = user_alloc_usable_size(ptr);
 #define COMMON_MALLOC_FILL_STATS(zone, stats)
 #define COMMON_MALLOC_REPORT_UNKNOWN_REALLOC(ptr, zone_ptr, zone_name) \
   (void)zone_name; \
   Report("mz_realloc(%p) -- attempting to realloc unallocated memory.\n", ptr);
 #define COMMON_MALLOC_NAMESPACE __tsan
 
 #include "sanitizer_common/sanitizer_malloc_mac.inc"
 
 #endif

lib/tsan/rtl/tsan_mman.cc

if (p)
     user_free(thr, pc, p);
   return p2;
 }
 
 uptr user_alloc_usable_size(const void *p) {
   if (p == 0)
     return 0;
   MBlock *b = ctx->metamap.GetBlock((uptr)p);
-  return b ? b->siz : 0;
+  if (!b)
+    return 0;  // Not a valid pointer.
+  if (b->siz == 0)
+    return 1;  // Zero-sized allocations are actually 1 byte.
+  return b->siz;
 }
 
 void invoke_malloc_hook(void *ptr, uptr size) {
   ThreadState *thr = cur_thread();
   if (ctx == 0 || !ctx->initialized || thr->ignore_interceptors)
     return;
   __sanitizer_malloc_hook(ptr, size);
 }

test/tsan/Darwin/malloc_size.mm

 // Test that malloc_zone_from_ptr returns a valid zone for a 0-sized allocation.
 
 // RUN: %clang_tsan %s -o %t -framework Foundation
 // RUN: %run %t 2>&1 | FileCheck %s
 
 #import <Foundation/Foundation.h>
 #include <malloc/malloc.h>
 
-int main() {
-  void *p = malloc(0);
-
-  size_t s = malloc_size(p);
-  printf("size = 0x%zx\n", s);
+int some_global;
 
+void describe_zone(void *p) {
   malloc_zone_t *z = malloc_zone_from_ptr(p);
-  if (z)
-    printf("z = %p\n", z);
-  else
-    printf("no zone\n");
+  if (z) {
+    fprintf(stderr, "zone = %p\n", z);
+  } else {
+    fprintf(stderr, "zone = no zone\n");
+  }
 }
 
-// CHECK: z = 0x{{[0-9a-f]+}}
-// CHECK-NOT: no zone
+int main() {
+  void *p;
+  size_t s;
+
+  p = malloc(0x40);
+  s = malloc_size(p);
+  fprintf(stderr, "size = 0x%zx\n", s);
+  // CHECK: size = 0x40
+  describe_zone(p);
+  // CHECK: zone = 0x{{[0-9a-f]+}}
+
+  p = malloc(0);
+  s = malloc_size(p);
+  fprintf(stderr, "size = 0x%zx\n", s);
+  // CHECK: size = 0x1
+  describe_zone(p);
+  // CHECK: zone = 0x{{[0-9a-f]+}}
+
+  p = &some_global;
+  s = malloc_size(p);
+  fprintf(stderr, "size = 0x%zx\n", s);
+  // CHECK: size = 0x0
+  describe_zone(p);
+  // CHECK: zone = no zone
+
+  p = mmap(0, 0x1000, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANON, -1, 0);
+  if (!p) {
+    fprintf(stderr, "mmap failed\n");
+    exit(1);
+  }
+  s = malloc_size(p);
+  fprintf(stderr, "size = 0x%zx\n", s);
+  // CHECK: size = 0x0
+  describe_zone(p);
+  // CHECK: zone = no zone
+}