From: Evgeniy Stepanov Date: Wed, 25 Jun 2014 14:41:57 +0000 (+0000) Subject: [msan] Fix bad interaction between with-calls mode and chained origin tracking. X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=b163f0276f19ec207b3dafba398b0f273787c570;p=platform%2Fupstream%2Fllvm.git [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 --- diff --git a/compiler-rt/lib/msan/msan.cc b/compiler-rt/lib/msan/msan.cc index 84f1bc6..58a5af4 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 f69de9a..336bbd8 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 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 c6f8b62..08854f2 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 #include #include diff --git a/compiler-rt/test/msan/chained_origin_memcpy.cc b/compiler-rt/test/msan/chained_origin_memcpy.cc index e56db9c..f4c2f7f 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 #include @@ -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 5fd497e..ef98385 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 #include #include diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp index 75c56c2..4ca0323 100644 --- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp @@ -569,7 +569,7 @@ struct MemorySanitizerVisitor : public InstVisitor { 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 34988ef..beb3c5f 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