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

[Aleksey Shlyapnikov via Phabricator via llvm-commits]

alekseyshl added inline comments.


================
Comment at: lib/asan/asan_allocator.cc:806
+      return nullptr;
+    } else {
+      // Allocate a size of 1 if we shouldn't free() on Realloc to 0
----------------
No need in else block after return, less nesting is better.


================
Comment at: test/asan/TestCases/realloc.cc:5
+// RUN: %env_asan_opts=free_on_realloc_zero=true %run %t 2>&1 | FileCheck %s
+// RUN: %env_asan_opts=free_on_realloc_zero=false %run %t 2>&1 | FileCheck %s -check-prefix=NO-FREE -allow-empty
+
----------------
--allow-empty --check-prefix=NO-FREE

to match other tests


================
Comment at: test/asan/TestCases/realloc.cc:5
+// RUN: %env_asan_opts=free_on_realloc_zero=true %run %t 2>&1 | FileCheck %s
+// RUN: %env_asan_opts=free_on_realloc_zero=false %run %t 2>&1 | FileCheck %s -check-prefix=NO-FREE -allow-empty
+
----------------
alekseyshl wrote:
> --allow-empty --check-prefix=NO-FREE
> 
> to match other tests
Why do you need --allow-empty? Isn't it always supposed to print  "Allocated something on realloc(p, 0)"?


================
Comment at: test/asan/TestCases/realloc.cc:11
+int main(int argc, char **argv) {
+  void *p = malloc(argc);
+  p = realloc(p, 0);
----------------
Why argc and not the particular number of bytes?


================
Comment at: test/asan/TestCases/realloc.cc:13
+  p = realloc(p, 0);
+  if (p)
+    // NO-FREE: Allocated something on realloc(p, 0)
----------------
if (p) {
} else {
}


https://reviews.llvm.org/D31295