[msan] Fix bad interaction between with-calls mode and chained origin tracking.

Origin history should only be recorded for uninitialized values, because it is
meaningless otherwise. This change moves __msan_chain_origin to the runtime
library side and makes it conditional on the corresponding shadow value.

Previous code was correct, but _very_ inefficient.

llvm-svn: 211700

7 files changed, 65 insertions, 15 deletions

diff --git a/compiler-rt/lib/msan/msan.cc b/compiler-rt/lib/msan/msan.cc
index 84f1bc6ff39..58a5af43c14 100644
--- a/compiler-rt/lib/msan/msan.cc
+++ b/compiler-rt/lib/msan/msan.cc
@@ -317,7 +317,15 @@ MSAN_MAYBE_WARNING(u64, 8)
 
 #define MSAN_MAYBE_STORE_ORIGIN(type, size)                       \
   void __msan_maybe_store_origin_##size(type s, void *p, u32 o) { \
-    if (UNLIKELY(s)) *(u32 *)MEM_TO_ORIGIN((uptr)p &~3UL) = o;    \
+    if (UNLIKELY(s)) {                                            \
+      if (__msan_get_track_origins() > 1) {                       \
+        GET_CALLER_PC_BP_SP;                                      \
+        (void) sp;                                                \
+        GET_STORE_STACK_TRACE_PC_BP(pc, bp);                      \
+        o = ChainOrigin(o, &stack);                               \
+      }                                                           \
+      *(u32 *)MEM_TO_ORIGIN((uptr)p & ~3UL) = o;                  \
+    }                                                             \
   }
 
 MSAN_MAYBE_STORE_ORIGIN(u8, 1)
diff --git a/compiler-rt/test/msan/chained_origin.cc b/compiler-rt/test/msan/chained_origin.cc
index f69de9aba85..336bbd852cb 100644
--- a/compiler-rt/test/msan/chained_origin.cc
+++ b/compiler-rt/test/msan/chained_origin.cc
@@ -6,6 +6,16 @@
 // RUN:     not %run %t >%t.out 2>&1
 // RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-HEAP < %t.out
 
+
+// RUN: %clangxx_msan -mllvm -msan-instrumentation-with-call-threshold=0 -fsanitize-memory-track-origins=2 -m64 -O3 %s -o %t && \
+// RUN:     not %run %t >%t.out 2>&1
+// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-STACK < %t.out
+
+// RUN: %clangxx_msan -mllvm -msan-instrumentation-with-call-threshold=0 -fsanitize-memory-track-origins=2 -DHEAP=1 -m64 -O3 %s -o %t && \
+// RUN:     not %run %t >%t.out 2>&1
+// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-HEAP < %t.out
+
+
 #include <stdio.h>
 
 volatile int x, y;
@@ -38,19 +48,19 @@ int main(int argc, char *argv[]) {
 }
 
 // CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value
-// CHECK: {{#0 .* in main.*chained_origin.cc:37}}
+// CHECK: {{#0 .* in main.*chained_origin.cc:47}}
 
 // CHECK: Uninitialized value was stored to memory at
-// CHECK: {{#0 .* in fn_h.*chained_origin.cc:25}}
-// CHECK: {{#1 .* in main.*chained_origin.cc:36}}
+// CHECK: {{#0 .* in fn_h.*chained_origin.cc:35}}
+// CHECK: {{#1 .* in main.*chained_origin.cc:46}}
 
 // CHECK: Uninitialized value was stored to memory at
-// CHECK: {{#0 .* in fn_g.*chained_origin.cc:15}}
-// CHECK: {{#1 .* in fn_f.*chained_origin.cc:20}}
-// CHECK: {{#2 .* in main.*chained_origin.cc:35}}
+// CHECK: {{#0 .* in fn_g.*chained_origin.cc:25}}
+// CHECK: {{#1 .* in fn_f.*chained_origin.cc:30}}
+// CHECK: {{#2 .* in main.*chained_origin.cc:45}}
 
 // CHECK-STACK: Uninitialized value was created by an allocation of 'z' in the stack frame of function 'main'
-// CHECK-STACK: {{#0 .* in main.*chained_origin.cc:28}}
+// CHECK-STACK: {{#0 .* in main.*chained_origin.cc:38}}
 
 // CHECK-HEAP: Uninitialized value was created by a heap allocation
-// CHECK-HEAP: {{#1 .* in main.*chained_origin.cc:30}}
+// CHECK-HEAP: {{#1 .* in main.*chained_origin.cc:40}}
diff --git a/compiler-rt/test/msan/chained_origin_limits.cc b/compiler-rt/test/msan/chained_origin_limits.cc
index c6f8b626c59..08854f25f71 100644
--- a/compiler-rt/test/msan/chained_origin_limits.cc
+++ b/compiler-rt/test/msan/chained_origin_limits.cc
@@ -11,6 +11,18 @@
 // RUN: MSAN_OPTIONS=origin_history_per_stack_limit=1 not %run %t >%t.out 2>&1
 // RUN: FileCheck %s --check-prefix=CHECK-PER-STACK < %t.out
 
+
+// RUN: %clangxx_msan -mllvm -msan-instrumentation-with-call-threshold=0 -fsanitize-memory-track-origins=2 -m64 -O3 %s -o %t
+
+// RUN: MSAN_OPTIONS=origin_history_size=7 not %run %t >%t.out 2>&1
+// RUN: FileCheck %s --check-prefix=CHECK7 < %t.out
+
+// RUN: MSAN_OPTIONS=origin_history_size=2 not %run %t >%t.out 2>&1
+// RUN: FileCheck %s --check-prefix=CHECK2 < %t.out
+
+// RUN: MSAN_OPTIONS=origin_history_per_stack_limit=1 not %run %t >%t.out 2>&1
+// RUN: FileCheck %s --check-prefix=CHECK-PER-STACK < %t.out
+
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
diff --git a/compiler-rt/test/msan/chained_origin_memcpy.cc b/compiler-rt/test/msan/chained_origin_memcpy.cc
index e56db9c87d6..f4c2f7fcac8 100644
--- a/compiler-rt/test/msan/chained_origin_memcpy.cc
+++ b/compiler-rt/test/msan/chained_origin_memcpy.cc
@@ -6,6 +6,16 @@
 // RUN:     not %run %t >%t.out 2>&1
 // RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-Z2 < %t.out
 
+
+// RUN: %clangxx_msan -mllvm -msan-instrumentation-with-call-threshold=0 -fsanitize-memory-track-origins=2 -m64 -DOFFSET=0 -O3 %s -o %t && \
+// RUN:     not %run %t >%t.out 2>&1
+// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-Z1 < %t.out
+
+// RUN: %clangxx_msan -mllvm -msan-instrumentation-with-call-threshold=0 -fsanitize-memory-track-origins=2 -DOFFSET=10 -m64 -O3 %s -o %t && \
+// RUN:     not %run %t >%t.out 2>&1
+// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-Z2 < %t.out
+
+
 #include <stdio.h>
 #include <string.h>
 
@@ -37,15 +47,15 @@ int main(int argc, char *argv[]) {
 }
 
 // CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value
-// CHECK: {{#0 .* in main .*chained_origin_memcpy.cc:36}}
+// CHECK: {{#0 .* in main .*chained_origin_memcpy.cc:46}}
 
 // CHECK: Uninitialized value was stored to memory at
-// CHECK: {{#1 .* in fn_h.*chained_origin_memcpy.cc:28}}
+// CHECK: {{#1 .* in fn_h.*chained_origin_memcpy.cc:38}}
 
 // CHECK: Uninitialized value was stored to memory at
-// CHECK: {{#0 .* in fn_g.*chained_origin_memcpy.cc:18}}
-// CHECK: {{#1 .* in fn_f.*chained_origin_memcpy.cc:23}}
+// CHECK: {{#0 .* in fn_g.*chained_origin_memcpy.cc:28}}
+// CHECK: {{#1 .* in fn_f.*chained_origin_memcpy.cc:33}}
 
 // CHECK-Z1: Uninitialized value was created by an allocation of 'z1' in the stack frame of function 'main'
 // CHECK-Z2: Uninitialized value was created by an allocation of 'z2' in the stack frame of function 'main'
-// CHECK: {{#0 .* in main.*chained_origin_memcpy.cc:31}}
+// CHECK: {{#0 .* in main.*chained_origin_memcpy.cc:41}}
diff --git a/compiler-rt/test/msan/chained_origin_with_signals.cc b/compiler-rt/test/msan/chained_origin_with_signals.cc
index 5fd497eaedb..ef98385a1e2 100644
--- a/compiler-rt/test/msan/chained_origin_with_signals.cc
+++ b/compiler-rt/test/msan/chained_origin_with_signals.cc
@@ -6,6 +6,10 @@
 // RUN:     not %run %t >%t.out 2>&1
 // RUN: FileCheck %s < %t.out
 
+// RUN: %clangxx_msan -mllvm -msan-instrumentation-with-call-threshold=0 -fsanitize-memory-track-origins=2 -m64 -O3 %s -o %t && \
+// RUN:     not %run %t >%t.out 2>&1
+// RUN: FileCheck %s < %t.out
+
 #include <signal.h>
 #include <stdio.h>
 #include <sys/types.h>
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 75c56c2d430..4ca03238071 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -569,7 +569,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
             ConvertedShadow, IRB.getIntNTy(8 * (1 << SizeIndex)));
         IRB.CreateCall3(Fn, ConvertedShadow2,
                         IRB.CreatePointerCast(Addr, IRB.getInt8PtrTy()),
-                        updateOrigin(Origin, IRB));
+                        Origin);
       } else {
         Value *Cmp = IRB.CreateICmpNE(
             ConvertedShadow, getCleanShadow(ConvertedShadow), "_mscmp");
diff --git a/llvm/test/Instrumentation/MemorySanitizer/instrumentation-with-call-threshold.ll b/llvm/test/Instrumentation/MemorySanitizer/instrumentation-with-call-threshold.ll
index 34988ef6055..beb3c5fad73 100644
--- a/llvm/test/Instrumentation/MemorySanitizer/instrumentation-with-call-threshold.ll
+++ b/llvm/test/Instrumentation/MemorySanitizer/instrumentation-with-call-threshold.ll
@@ -1,7 +1,10 @@
 ; Test -msan-instrumentation-with-call-threshold
+; Test that in with-calls mode there are no calls to __msan_chain_origin - they
+; are done from __msan_maybe_store_origin_*.
 
 ; RUN: opt < %s -msan -msan-check-access-address=0 -msan-instrumentation-with-call-threshold=0 -S | FileCheck %s
 ; RUN: opt < %s -msan -msan-check-access-address=0 -msan-instrumentation-with-call-threshold=0 -msan-track-origins=1 -S | FileCheck -check-prefix=CHECK -check-prefix=CHECK-ORIGINS %s
+; RUN: opt < %s -msan -msan-check-access-address=0 -msan-instrumentation-with-call-threshold=0 -msan-track-origins=2 -S | FileCheck -check-prefix=CHECK -check-prefix=CHECK-ORIGINS %s
 
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
@@ -41,7 +44,10 @@ entry:
 ; CHECK: load {{.*}} @__msan_param_tls
 ; CHECK-ORIGINS: load {{.*}} @__msan_param_origin_tls
 ; CHECK: store
+; CHECK-ORIGINS-NOT: __msan_chain_origin
 ; CHECK-ORIGINS: bitcast i64* {{.*}} to i8*
+; CHECK-ORIGINS-NOT: __msan_chain_origin
 ; CHECK-ORIGINS: call void @__msan_maybe_store_origin_8(
+; CHECK-ORIGINS-NOT: __msan_chain_origin
 ; CHECK: store i64
 ; CHECK: ret void