[PATCH] D81699: MemorySanitizer: Add option to insert init checks at call site

[Alexander Potapenko via Phabricator via llvm-commits]

glider added a comment.

Wrapping all variables in the existing tests in PartInit<> doesn't look good.
I believe instead of doing this you need to:

- change existing tests to also run with eager checks, adding expected MSan outputs as needed. Do not scatter PartInit<> across all tests.
- add more tests that use the partinit attribute to cover new functionality.

I also don't quite understand how this partinit thing works with programs written in C. We probably need a test for it as well.



================
Comment at: compiler-rt/test/msan/in-struct-padding.cpp:4
+// RUN: %clangxx_msan -O3 %s -o %t && %run %t
+
+#include <stdint.h>
----------------
It's unclear what this test is doing. Please consider adding comments.


================
Comment at: compiler-rt/test/msan/param_tls_limit.cpp:39
 
-
 template<int N>
----------------
Please don't introduce unnecessary diff.


================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:3471
                                        kShadowTLSAlignment);
+        
         Constant *Cst = dyn_cast<Constant>(ArgShadow);
----------------
Please remove unrelated diff from the patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81699/new/

https://reviews.llvm.org/D81699