runtime flag for use-after dtor and v simple runtime test

[eugenis]

include/sanitizer/msan_interface.h

96

Make it "const volatile *".

lib/msan/msan_interface_internal.h

120

Don't need this.

test/msan/dtor-member.cc

2

Add -O1 and -O2 runs.

20

return is unnecessary

[nmusgrave]

include/sanitizer/msan_interface.h

96

• why? I was looking at msan_allocated_memory and saw the const volatile void* parameter, but don't understand why its const. Since the memory is being poisoned, shouldn't it -not- be const?
• ...or can the parameter be const since its not the memory at this location, but the shadow memory, that's marked as poisoned?
• also: naming conventions. Shouldn't it be __msan_dtor_callback? That seems to align better with the other msan runtime functions here. If so, I'll change the naming in codegen/

[eugenis]

include/sanitizer/msan_interface.h

96

This only really matters for functions the would be called from the user code. The idea is that even if some memory is "const", we can mark it as uninitialized or not.

The name starts with __sanitizer because it may be implemented in other sanitizers, too. Maybe in the future. ASan can be taught to detect use-after-dtor too, for example, but with some false positives.

[nmusgrave]

• resolved runtime function parameters

[eugenis]

test/msan/dtor-member.cc

10

Please add run lines without (a) compiler flag and (b) runtime flag.

23

If we don't crash on the next line (in the negative test), there will be a second destructor call at the function exit, which invokes undefined behavior. Let's change the code to smth like this:

unsigned long buf[1];
assert(sizeof(Simple) <= sizeof(buf));
Simple *s = new(&buf) Simple();
s->~Simple();

[nmusgrave]

• added tests for without compile and runtime flags

[eugenis]

LGTM

lib/msan/msan_interceptors.cc

1011

FYI, this tag affects the wording of MSan reports. We may want to introduce a new tag, specifically for use-after-dtor. Not in this commit.

test/msan/dtor-member.cc

12

Could just call it "CHECK-NO" and save one line of code.

19

I don't think this include is needed.

37

"interfere"

[nmusgrave]

• modified run prefix

[nmusgrave]

test/msan/dtor-member.cc

20

needed for main- can't have void main function. may rename as add more test cases.

[nmusgrave]

revision alterations have been landed and changes pushed onto trunk

include/sanitizer/msan_interface.h

#endif
 
   /* Returns true if running under a dynamic tool (DynamoRio-based). */
   int  __msan_has_dynamic_component();
 
   /* Tell MSan about newly allocated memory (ex.: custom allocator).
      Memory will be marked uninitialized, with origin at the call site. */
   void __msan_allocated_memory(const volatile void* data, size_t size);
 
+  /* Tell MSan about newly destroyed memory. Mark memory as uninitialized. */
+  void __sanitizer_dtor_callback(const volatile void* data, size_t size);
+
   /* This function may be optionally provided by user and should return
      a string containing Msan runtime options. See msan_flags.h for details. */
   const char* __msan_default_options();
 
   /* Sets the callback to be called right before death on error.
      Passing 0 will unset the callback. */
   void __msan_set_death_callback(void (*callback)(void));
 
 #ifdef __cplusplus
 }  // extern "C"
 #endif
 
 #endif

lib/msan/msan.cc

 void __msan_clear_on_return() {
   __msan_param_tls[0] = 0;
 }
 
 void __msan_partial_poison(const void* data, void* shadow, uptr size) {
   internal_memcpy((void*)MEM_TO_SHADOW((uptr)data), shadow, size);
 }
 
-void __msan_load_unpoisoned(void *src, uptr size, void *dst) {
+void __msan_load_unpoisoned(const void *src, uptr size, void *dst) {
   internal_memcpy(dst, src, size);
   __msan_unpoison(dst, size);
 }
 
 void __msan_set_origin(const void *a, uptr size, u32 origin) {
   if (__msan_get_track_origins()) SetOrigin(a, size, origin);
 }
 

lib/msan/msan_flags.inc

 
 MSAN_FLAG(int, exit_code, 77, "")
 MSAN_FLAG(int, origin_history_size, Origin::kMaxDepth, "")
 MSAN_FLAG(int, origin_history_per_stack_limit, 20000, "")
 MSAN_FLAG(bool, poison_heap_with_zeroes, false, "")
 MSAN_FLAG(bool, poison_stack_with_zeroes, false, "")
 MSAN_FLAG(bool, poison_in_malloc, true, "")
 MSAN_FLAG(bool, poison_in_free, true, "")
+MSAN_FLAG(bool, poison_in_dtor, false, "")
 MSAN_FLAG(bool, report_umrs, true, "")
 MSAN_FLAG(bool, wrap_signals, true, "")
 MSAN_FLAG(bool, print_stats, false, "")
 MSAN_FLAG(bool, halt_on_error, !&__msan_keep_going, "")
 MSAN_FLAG(bool, atexit, false, "")
 MSAN_FLAG(int, store_context_size, 20,
           "Like malloc_context_size, but for uninit stores.")

lib/msan/msan_interceptors.cc

 void __msan_allocated_memory(const void *data, uptr size) {
   GET_MALLOC_STACK_TRACE;
   if (flags()->poison_in_malloc) {
     stack.tag = STACK_TRACE_TAG_POISON;
     PoisonMemory(data, size, &stack);
   }
 }
 
+void __sanitizer_dtor_callback(const void *data, uptr size) {
+  GET_MALLOC_STACK_TRACE;
+  if (flags()->poison_in_dtor) {
+    stack.tag = STACK_TRACE_TAG_POISON;
+    PoisonMemory(data, size, &stack);
+  }
+}
+
 INTERCEPTOR(void *, mmap, void *addr, SIZE_T length, int prot, int flags,
             int fd, OFF_T offset) {
   if (msan_init_is_running)
     return REAL(mmap)(addr, length, prot, flags, fd, offset);
   ENSURE_MSAN_INITED();
   if (addr && !MEM_IS_APP(addr)) {
     if (flags & map_fixed) {
       *__errno_location() = errno_EINVAL;

lib/msan/msan_interface_internal.h

 
 SANITIZER_INTERFACE_ATTRIBUTE
 void __msan_set_keep_going(int keep_going);
 
 SANITIZER_INTERFACE_ATTRIBUTE
 int __msan_set_poison_in_malloc(int do_poison);
 
 SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE
 /* OPTIONAL */ const char* __msan_default_options();
 
 // For testing.
 SANITIZER_INTERFACE_ATTRIBUTE
 void __msan_set_expect_umr(int expect_umr);
 SANITIZER_INTERFACE_ATTRIBUTE
 void __msan_print_shadow(const void *x, uptr size);
 SANITIZER_INTERFACE_ATTRIBUTE
 void __msan_dump_shadow(const void *x, uptr size);
 SANITIZER_INTERFACE_ATTRIBUTE
 int  __msan_has_dynamic_component();
 
 // For testing.
 SANITIZER_INTERFACE_ATTRIBUTE
 u32 __msan_get_umr_origin();
 SANITIZER_INTERFACE_ATTRIBUTE
 void __msan_partial_poison(const void* data, void* shadow, uptr size);
 
 // Tell MSan about newly allocated memory (ex.: custom allocator).
 // Memory will be marked uninitialized, with origin at the call site.
 SANITIZER_INTERFACE_ATTRIBUTE
 void __msan_allocated_memory(const void* data, uptr size);
 
+// Tell MSan about newly destroyed memory. Memory will be marked
+// uninitialized.
+SANITIZER_INTERFACE_ATTRIBUTE
+void __sanitizer_dtor_callback(const void* data, uptr size);
+
 SANITIZER_INTERFACE_ATTRIBUTE
 u16 __sanitizer_unaligned_load16(const uu16 *p);
 
 SANITIZER_INTERFACE_ATTRIBUTE
 u32 __sanitizer_unaligned_load32(const uu32 *p);
 
 SANITIZER_INTERFACE_ATTRIBUTE
 u64 __sanitizer_unaligned_load64(const uu64 *p);

test/msan/dtor-member.cc

+// RUN: %clangxx_msan %s -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 %run %t >%t.out 2>&1
+// RUN: FileCheck %s < %t.out
+
+// RUN: %clangxx_msan %s -O1 -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1 %run %t >%t.out 2>&1
+// RUN: FileCheck %s < %t.out
+
+// RUN: %clangxx_msan %s -O2 -fsanitize=memory -fsanitize-memory-use-after-dtor -o %t && MSAN_OPTIONS=poison_in_dtor=1  %run %t >%t.out 2>&1
+// RUN: FileCheck %s < %t.out
+
+// RUN: %clangxx_msan %s -fsanitize=memory -o %t && MSAN_OPTIONS=poison_in_dtor=1  %run %t >%t.out 2>&1
+// RUN: FileCheck %s --check-prefix=CHECK-NO-FLAG < %t.out
+
+// RUN: %clangxx_msan -fsanitize=memory -fsanitize-memory-use-after-dtor %s -o %t && MSAN_OPTIONS=poison_in_dtor=0 %run %t >%t.out 2>&1
+// RUN: FileCheck %s --check-prefix=CHECK-NO-FLAG < %t.out
+
+#include <sanitizer/msan_interface.h>
+#include <assert.h>
+#include <stdio.h>
+#include <new>
+
+struct Simple {
+  int x_;
+  Simple() {
+    x_ = 5;
+  }
+  ~Simple() { }
+};
+
+int main() {
+  unsigned long buf[1];
+  assert(sizeof(Simple) <= sizeof(buf));
+
+  // The placement new operator forces the object to be constructed in the
+  // memory location &buf. Since objects made in this way must be explicitly
+  // destroyed, there are no implicit calls inserted that would interfere with
+  // test behavior.
+  Simple *s = new(&buf) Simple();
+  s->~Simple();
+
+  if (__msan_test_shadow(s, sizeof(*s)) != -1)
+    printf("s is poisoned\n");
+  else
+    printf("s is not poisoned\n");
+  // CHECK: s is poisoned
+  // CHECK-NO-FLAG: s is not poisoned
+
+  return 0;
+}