[llvm-commits] [PATCH] [msan] MemorySanitizer runtime

[Kostya Serebryany]

  Having written part of this myself, looks almost good (modulo few comments below).
  Please make sure lint run is clean.
  I'd really like someone else to review it too, but (unless someone wants to jump in today) it's ok to do the rest of the review after commit
  (pls give the US folks time to respond).


================
Comment at: llvm/projects/compiler-rt/lib/msan/msan.cc:1
@@ +1,2 @@
+//===-- msan_new_delete.cc ------------------------------------------------===//
+//
----------------
check name headers. 

================
Comment at: llvm/projects/compiler-rt/lib/msan/msan.cc:305
@@ +304,3 @@
+int __msan_get_retval_tls_offset() {
+  // volatile here is needed to avoid UB, because the compiler thinks that we are doing address
+  // arithmetics on unrelated pointers, and takes some shortcuts
----------------
80 chars (and check lint, please)

================
Comment at: llvm/projects/compiler-rt/lib/msan/msan_new_delete.cc:18
@@ +17,3 @@
+#include <stddef.h>
+#include <new>
+
----------------
we've got rid of include <new> in asan/tsan, please do the same here. same for stddef

================
Comment at: llvm/projects/compiler-rt/lib/msan/msan_interface.h:1
@@ +1,2 @@
+//===-- msan_interface.h --------------------------------------------------===//
+//
----------------
msan_interface.h whould go into the same place asan asan_interface.h

================
Comment at: llvm/projects/compiler-rt/lib/msan/msan_linux_inl.h:1
@@ +1,2 @@
+//===-- msan_linux_inl.h --  ----------------------------------------------===//
+//
----------------
I don't remember the reason to keep this stuff in header. 
I had one in mind when I did this, but perhaps that reason is gone now.
Can we move this into a msan_linux.cc file? 

================
Comment at: llvm/projects/compiler-rt/lib/msan/msan.cc:237
@@ +236,3 @@
+}
+void __msan_set_expect_umr(int expect_umr) {
+  if (expect_umr) {
----------------
space (I guess lint should catch this)


http://llvm-reviews.chandlerc.com/D191