[msan] Fix origin of deallocated memory.

MSan poisons deallocated memory but it used to give it an invalid origin value,
resulting in confusing reports. This change associates deallocation stack trace
with such memory.

Note that MSan does not have quarantine, and use-after-free detection is very
limited.


git-svn-id: https://llvm.org/svn/llvm-project/compiler-rt/trunk@190781 91177308-0d34-0410-b5e6-96231b3b80d8

7 files changed, 64 insertions, 17 deletions

diff --git a/lib/msan/lit_tests/use-after-free.cc b/lib/msan/lit_tests/use-after-free.cc
new file mode 100644
index 000000000..ac47c0233
--- /dev/null
+++ b/lib/msan/lit_tests/use-after-free.cc
@@ -0,0 +1,34 @@
+// RUN: %clangxx_msan -m64 -O0 %s -o %t && not %t >%t.out 2>&1
+// RUN: FileCheck %s < %t.out
+// RUN: %clangxx_msan -m64 -O1 %s -o %t && not %t >%t.out 2>&1
+// RUN: FileCheck %s < %t.out
+// RUN: %clangxx_msan -m64 -O2 %s -o %t && not %t >%t.out 2>&1
+// RUN: FileCheck %s < %t.out
+// RUN: %clangxx_msan -m64 -O3 %s -o %t && not %t >%t.out 2>&1
+// RUN: FileCheck %s < %t.out
+
+// RUN: %clangxx_msan -fsanitize-memory-track-origins -m64 -O0 %s -o %t && not %t >%t.out 2>&1
+// RUN: FileCheck %s < %t.out && FileCheck %s --check-prefix=CHECK-ORIGINS < %t.out
+// RUN: %clangxx_msan -fsanitize-memory-track-origins -m64 -O1 %s -o %t && not %t >%t.out 2>&1
+// RUN: FileCheck %s < %t.out && FileCheck %s --check-prefix=CHECK-ORIGINS < %t.out
+// RUN: %clangxx_msan -fsanitize-memory-track-origins -m64 -O2 %s -o %t && not %t >%t.out 2>&1
+// RUN: FileCheck %s < %t.out && FileCheck %s --check-prefix=CHECK-ORIGINS < %t.out
+// RUN: %clangxx_msan -fsanitize-memory-track-origins -m64 -O3 %s -o %t && not %t >%t.out 2>&1
+// RUN: FileCheck %s < %t.out && FileCheck %s --check-prefix=CHECK-ORIGINS < %t.out
+
+#include <stdlib.h>
+int main(int argc, char **argv) {
+  int *volatile p = (int *)malloc(sizeof(int));
+  *p = 42;
+  free(p);
+
+  if (*p)
+    exit(0);
+  // CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value
+  // CHECK: {{#0 0x.* in main .*use-after-free.cc:}}[[@LINE-3]]
+
+  // CHECK-ORIGINS: Uninitialized value was created by a heap allocation
+  // CHECK-ORIGINS: {{#0 0x.* in .*free}}
+  // CHECK-ORIGINS: {{#1 0x.* in main .*use-after-free.cc:}}[[@LINE-9]]
+  return 0;
+}
diff --git a/lib/msan/msan.cc b/lib/msan/msan.cc
index 0a4af7cdd..2b9324aa3 100644
--- a/lib/msan/msan.cc
+++ b/lib/msan/msan.cc
@@ -122,6 +122,7 @@ static void ParseFlagsFromString(Flags *f, const char *str) {
   ParseFlag(str, &f->poison_heap_with_zeroes, "poison_heap_with_zeroes");
   ParseFlag(str, &f->poison_stack_with_zeroes, "poison_stack_with_zeroes");
   ParseFlag(str, &f->poison_in_malloc, "poison_in_malloc");
+  ParseFlag(str, &f->poison_in_malloc, "poison_in_free");
   ParseFlag(str, &f->exit_code, "exit_code");
   if (f->exit_code < 0 || f->exit_code > 127) {
     Printf("Exit code not in [0, 128) range: %d\n", f->exit_code);
@@ -153,6 +154,7 @@ static void InitializeFlags(Flags *f, const char *options) {
   f->poison_heap_with_zeroes = false;
   f->poison_stack_with_zeroes = false;
   f->poison_in_malloc = true;
+  f->poison_in_free = true;
   f->exit_code = 77;
   f->report_umrs = true;
   f->verbosity = 0;
diff --git a/lib/msan/msan.h b/lib/msan/msan.h
index 16dfc0cf6..8c24de647 100644
--- a/lib/msan/msan.h
+++ b/lib/msan/msan.h
@@ -46,7 +46,7 @@ void InitializeInterceptors();
 
 void *MsanReallocate(StackTrace *stack, void *oldp, uptr size,
                      uptr alignment, bool zeroise);
-void MsanDeallocate(void *ptr);
+void MsanDeallocate(StackTrace *stack, void *ptr);
 void InstallTrapHandler();
 void InstallAtExitHandler();
 void ReplaceOperatorsNewAndDelete();
diff --git a/lib/msan/msan_allocator.cc b/lib/msan/msan_allocator.cc
index 050a55c50..0fe1297a8 100644
--- a/lib/msan/msan_allocator.cc
+++ b/lib/msan/msan_allocator.cc
@@ -51,21 +51,23 @@ static void *MsanAllocate(StackTrace *stack, uptr size,
   void *res = allocator.Allocate(&cache, size, alignment, false);
   Metadata *meta = reinterpret_cast<Metadata*>(allocator.GetMetaData(res));
   meta->requested_size = size;
-  if (zeroise)
+  if (zeroise) {
     __msan_clear_and_unpoison(res, size);
-  else if (flags()->poison_in_malloc)
+  } else if (flags()->poison_in_malloc) {
     __msan_poison(res, size);
-  if (__msan_get_track_origins()) {
-    u32 stack_id = StackDepotPut(stack->trace, stack->size);
-    CHECK(stack_id);
-    CHECK_EQ((stack_id >> 31), 0);  // Higher bit is occupied by stack origins.
-    __msan_set_origin(res, size, stack_id);
+    if (__msan_get_track_origins()) {
+      u32 stack_id = StackDepotPut(stack->trace, stack->size);
+      CHECK(stack_id);
+      CHECK_EQ((stack_id >> 31),
+               0);  // Higher bit is occupied by stack origins.
+      __msan_set_origin(res, size, stack_id);
+    }
   }
   MSAN_MALLOC_HOOK(res, size);
   return res;
 }
 
-void MsanDeallocate(void *p) {
+void MsanDeallocate(StackTrace *stack, void *p) {
   CHECK(p);
   Init();
   MSAN_FREE_HOOK(p);
@@ -74,9 +76,16 @@ void MsanDeallocate(void *p) {
   meta->requested_size = 0;
   // This memory will not be reused by anyone else, so we are free to keep it
   // poisoned.
-  __msan_poison(p, size);
-  if (__msan_get_track_origins())
-    __msan_set_origin(p, size, -1);
+  if (flags()->poison_in_free) {
+    __msan_poison(p, size);
+    if (__msan_get_track_origins()) {
+      u32 stack_id = StackDepotPut(stack->trace, stack->size);
+      CHECK(stack_id);
+      CHECK_EQ((stack_id >> 31),
+               0);  // Higher bit is occupied by stack origins.
+      __msan_set_origin(p, size, stack_id);
+    }
+  }
   allocator.Deallocate(&cache, p);
 }
 
@@ -85,7 +94,7 @@ void *MsanReallocate(StackTrace *stack, void *old_p, uptr new_size,
   if (!old_p)
     return MsanAllocate(stack, new_size, alignment, zeroise);
   if (!new_size) {
-    MsanDeallocate(old_p);
+    MsanDeallocate(stack, old_p);
     return 0;
   }
   Metadata *meta = reinterpret_cast<Metadata*>(allocator.GetMetaData(old_p));
@@ -103,7 +112,7 @@ void *MsanReallocate(StackTrace *stack, void *old_p, uptr new_size,
   // Printf("realloc: old_size %zd new_size %zd\n", old_size, new_size);
   if (new_p)
     __msan_memcpy(new_p, old_p, memcpy_size);
-  MsanDeallocate(old_p);
+  MsanDeallocate(stack, old_p);
   return new_p;
 }
 
diff --git a/lib/msan/msan_flags.h b/lib/msan/msan_flags.h
index f05263247..318ea7cf4 100644
--- a/lib/msan/msan_flags.h
+++ b/lib/msan/msan_flags.h
@@ -23,6 +23,7 @@ struct Flags {
   bool poison_heap_with_zeroes;  // default: false
   bool poison_stack_with_zeroes;  // default: false
   bool poison_in_malloc;  // default: true
+  bool poison_in_free;  // default: true
   bool report_umrs;
   bool wrap_signals;
   bool halt_on_error;
diff --git a/lib/msan/msan_interceptors.cc b/lib/msan/msan_interceptors.cc
index 0431fed6c..d15618316 100644
--- a/lib/msan/msan_interceptors.cc
+++ b/lib/msan/msan_interceptors.cc
@@ -165,9 +165,9 @@ INTERCEPTOR(void *, pvalloc, SIZE_T size) {
 }
 
 INTERCEPTOR(void, free, void *ptr) {
-  ENSURE_MSAN_INITED();
+  GET_MALLOC_STACK_TRACE;
   if (ptr == 0) return;
-  MsanDeallocate(ptr);
+  MsanDeallocate(&stack, ptr);
 }
 
 INTERCEPTOR(SIZE_T, strlen, const char *s) {
diff --git a/lib/msan/msan_new_delete.cc b/lib/msan/msan_new_delete.cc
index 88d4364f6..17687ddfc 100644
--- a/lib/msan/msan_new_delete.cc
+++ b/lib/msan/msan_new_delete.cc
@@ -43,7 +43,8 @@ void *operator new(size_t size, std::nothrow_t const&) { OPERATOR_NEW_BODY; }
 void *operator new[](size_t size, std::nothrow_t const&) { OPERATOR_NEW_BODY; }
 
 #define OPERATOR_DELETE_BODY \
-  if (ptr) MsanDeallocate(ptr)
+  GET_MALLOC_STACK_TRACE; \
+  if (ptr) MsanDeallocate(&stack, ptr)
 
 void operator delete(void *ptr) { OPERATOR_DELETE_BODY; }
 void operator delete[](void *ptr) { OPERATOR_DELETE_BODY; }