[llvm-commits] [PATCH] MemorySanitizer instrumentation pass.

[Evgeniy Stepanov]

  InstVisitor changes are sent as a patch to llvm-commits.
  I'll split varargs logic to a helper later.


================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:129
@@ +128,3 @@
+
+  TargetData *TD;
+  LLVMContext *C;
----------------
Chandler Carruth wrote:
> DataLayout and DL now...
Rebased onto the current ToT.

================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:198-199
@@ +197,4 @@
+
+  // Insert a call to __msan_init/__msan_track_origins into the module's CTORs.
+  IRBuilder<> IRB(*C);
+  appendToGlobalCtors(M, cast<Function>(M.getOrInsertFunction(
----------------
Chandler Carruth wrote:
> If you bild this 3 statemetns earlier you can use its integer type building facilities.
I'm not sure what you mean.

================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:208-214
@@ +207,9 @@
+  // Create the callback.
+  // FIXME: this function should have "Cold" calling conv,
+  // which is not yet implemented. Alternatively, we may use llvm.trap.
+  if (ClUseTrap) {
+    // WarningFn = Intrinsic::getDeclaration(&M, Intrinsic::trap);
+    // We use inline asm because Intrinsic::trap is treated as never return.
+    WarningFn = InlineAsm::get(FunctionType::get(Type::getVoidTy(*C), false),
+                                  StringRef("ud2"), StringRef(""), true);
+  } else {
----------------
Chandler Carruth wrote:
> I don't understand any of this. Why aren't we using trap? Why aren't we following the pattern of ASan? What is the goal of UseTrap?
Unlike ASan, we want MSan reports to be non-fatal. Normal call spills scratch registers in the caller, etc - it's way too slow (AFAIR, it's ~20% slower than ud2 for very cold calls).


================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:225-226
@@ +224,4 @@
+    IRB.getVoidTy(), IRB.getInt8PtrTy(), IntptrTy, NULL);
+  MemmoveFn = M.getOrInsertFunction("memmove",
+    IRB.getInt8PtrTy(), IRB.getInt8PtrTy(), IRB.getInt8PtrTy(), IntptrTy, NULL);
+  // Create globals.
----------------
Chandler Carruth wrote:
> Shouldn't this use the LLVM intrinsic?
The point here is to replace all memmove intrinsics with function calls. See the comment to handleMemmove.


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