Add free_on_realloc_zero=true flag for compatibility with allocators which allow a realloc(p, 0) and don't free the pointer.

[filcab]

Summary

I know of two implementations that do this (ASan is not protecting against accessing the returned memory for now, just like malloc(0)):
SIE libc on the PS4
dlmalloc has a flag for this

This allows us to properly support this behaviour.

[vsk]

Ah, so is it also allowed to access P[0] under ASan after doing P = malloc(0)? If so, I can't spot any issues here.

[kcc]

I am horrified by the amount of flags we already have, and would like to reduce it, not increase it.
Can you please explain in more detaild why anyone would need this behavior and how this behavior matches the C/C++ standard behavior?

[kcc]

The standard says the behaviour is implementation defined, IIRC (I can quote later).

Yes, please!

I can keep this patch private if it's too much of a problem.

It's not too much of a problem, and I *am* interested to have reasonable patches upstream.
Let's see what the standard says.

[kcc]

Ok, let's do it. But I'd like to choose a better flag name.

lib/asan/asan_allocator.cc

803 ↗

(On Diff #92834)

maybe "free_and return_null_on_realloc_zero"?

Os some other name with the opposite meaning (and default=false):

allocator_returns_non_null_on_realloc_zero?

[alekseyshl]

lib/asan/asan_allocator.cc

806 ↗

(On Diff #92834)

No need in else block after return, less nesting is better.

test/asan/TestCases/realloc.cc

5 ↗

(On Diff #92834)

--allow-empty --check-prefix=NO-FREE

to match other tests

5 ↗

(On Diff #92834)

Why do you need --allow-empty? Isn't it always supposed to print "Allocated something on realloc(p, 0)"?

11 ↗

(On Diff #92834)

Why argc and not the particular number of bytes?

13 ↗

(On Diff #92834)

if (p) {
} else {
}

[filcab]

Address revision comments.

[filcab]

Address review comments

lib/asan/asan_allocator.cc

803 ↗

(On Diff #92834)

I phrased it in terms of the allocator, and explicitly mentioned is returns null new proposal for name (it's a bit long, but I can't shorten it and still mention all those points): allocator_frees_and_returns_null_on_realloc_zero

806 ↗

(On Diff #92834)

Thanks. Done!

test/asan/TestCases/realloc.cc

5 ↗

(On Diff #92834)

Was from a previous iteration of the test. Removed.

11 ↗

(On Diff #92834)

It was originally to try to avoid clang optimizing away the memory allocations. Which doesn't make sense since I'm passing -O0. Doesn't hurt, though. Let me know if you'd rather have a constant anyway.

13 ↗

(On Diff #92834)

Added braces.

[alekseyshl]

test/asan/TestCases/realloc.cc

11 ↗

(On Diff #92834)

Yes, please.

asm volatile("" : : "r"(p) : "memory"); after malloc should prevent optimizations.

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

 void *asan_calloc(uptr nmemb, uptr size, BufferedStackTrace *stack) {
   return instance.Calloc(nmemb, size, stack);
 }
 
 void *asan_realloc(void *p, uptr size, BufferedStackTrace *stack) {
   if (!p)
     return instance.Allocate(size, 8, stack, FROM_MALLOC, true);
   if (size == 0) {
+    if (flags()->allocator_frees_and_returns_null_on_realloc_zero) {
       instance.Deallocate(p, 0, stack, FROM_MALLOC);
       return nullptr;
     }
+    // Allocate a size of 1 if we shouldn't free() on Realloc to 0
+    size = 1;
+  }
   return instance.Reallocate(p, size, stack);
 }
 
 void *asan_valloc(uptr size, BufferedStackTrace *stack) {
   return instance.Allocate(size, GetPageSizeCached(), stack, FROM_MALLOC, true);
 }
 
 void *asan_pvalloc(uptr size, BufferedStackTrace *stack) {

compiler-rt/trunk/lib/asan/asan_flags.inc

ASAN_FLAG(bool, dump_registers, true,
           "If true, dump values of CPU registers when SEGV happens. Only "
           "available on OS X for now.")
 ASAN_FLAG(const char *, suppressions, "", "Suppressions file name.")
 ASAN_FLAG(bool, halt_on_error, true,
           "Crash the program after printing the first error report "
           "(WARNING: USE AT YOUR OWN RISK!)")
 ASAN_FLAG(bool, use_odr_indicator, false,
           "Use special ODR indicator symbol for ODR violation detection")
+ASAN_FLAG(bool, allocator_frees_and_returns_null_on_realloc_zero, true,
+          "realloc(p, 0) is equivalent to free(p) by default (Same as the "
+          "POSIX standard). If set to false, realloc(p, 0) will return a "
+          "pointer to an allocated space which can not be used.")

compiler-rt/trunk/test/asan/TestCases/realloc.cc

+// RUN: %clangxx_asan -O0 %s -o %t
+// Default is true (free on realloc to 0 size)
+// RUN: %run %t 2>&1 | FileCheck %s
+// RUN: %env_asan_opts=allocator_frees_and_returns_null_on_realloc_zero=true %run %t 2>&1 | FileCheck %s
+// RUN: %env_asan_opts=allocator_frees_and_returns_null_on_realloc_zero=false %run %t 2>&1 | FileCheck %s --check-prefix=NO-FREE
+
+#include <stdio.h>
+#include <stdlib.h>
+
+int main() {
+  void *p = malloc(42);
+  p = realloc(p, 0);
+  if (p) {
+    // NO-FREE: Allocated something on realloc(p, 0)
+    fprintf(stderr, "Allocated something on realloc(p, 0)\n");
+  } else {
+    // CHECK: realloc(p, 0) returned nullptr
+    fprintf(stderr, "realloc(p, 0) returned nullptr\n");
+  }
+  free(p);
+}