[llvm-commits] [PATCH] [msan] Heuristic handling of unknown intrinsics

[Evgeniy Stepanov]

================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:1242
@@ +1241,3 @@
+        } else {
+          // Two pointer operands? Meh.
+          visitInstruction(I);
----------------
Timur Iskhodzhanov wrote:
> Do you know which intrinsics have this behavior?
> Can we put an assertion here?
gcread/gcwrite, init_trampoline/adjust_trampoline, and some ARM NEON instructions.
In general, MSan lets everything unusual silently fallback to visitInstruction(), until it starts causing problems. visitInstruction adds checks for all operands, so, hopefully, the issue will be easy to locate.


================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:1247
@@ +1246,3 @@
+      } else if (!MemAccessType) {
+        MemAccessType = Op->getType();
+      }
----------------
Timur Iskhodzhanov wrote:
> This implies that the first non-pointer operand defines the access type.
> Are there intrinsics where this gives us wrong results (i.e. intrinsics with more than one non-pointer operands)?
Seems like it will work for all SSE intrinsic, and will cause false negatives with some NEON loads.

================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:1294
@@ +1293,3 @@
+        } else {
+          assert(Shadow);
+          Value *S = convertToShadowTyNoVec(Shadow, IRB);
----------------
Timur Iskhodzhanov wrote:
> Nonsense assert or you can move it up (outside if's)
That's actually a bug :)


================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:1289
@@ +1288,3 @@
+        Shadow = IRB.CreateOr(Shadow, OpShadow, "_msprop");
+      if (ClTrackOrigins) {
+        Value* OpOrigin = getOrigin(Op);
----------------
Timur Iskhodzhanov wrote:
> Do you have code like this (OR'ing N values, their shadows and origins) reused in many places?
> Maybe extract into a function?
It's kind of different in all cases.


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