nemanjai created this revision. nemanjai added reviewers: eugenis, vitalybuka, philip.pfaffe, leonardchan, PowerPC. Herald added projects: clang, Sanitizers. Herald added subscribers: Sanitizers, cfe-commits.
There are a number of test cases that fail when clang is built to use NPM by default. This is due to a number of issues. One of those issues is that when we construct the memory sanitizer instrumentation pass with the NPM, we do not pass in the pertinent flags. This patch focuses on that issue. For testing, this patch simply duplicates the tests that fail due to this issue and adds `-fexperimental-new-pass-manager` to the compile step. Once the NPM is the default, we can presumably remove those test cases. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D77249 Files: clang/lib/CodeGen/BackendUtil.cpp compiler-rt/test/msan/chained_origin_empty_stack_npm.cpp compiler-rt/test/msan/chained_origin_limits_npm.cpp compiler-rt/test/msan/chained_origin_memcpy_npm.cpp compiler-rt/test/msan/chained_origin_npm.cpp compiler-rt/test/msan/select_float_origin_npm.cpp
Index: compiler-rt/test/msan/select_float_origin_npm.cpp =================================================================== --- /dev/null +++ compiler-rt/test/msan/select_float_origin_npm.cpp @@ -0,0 +1,29 @@ +// FIXME: This test case is a copy of select_float_origin.cpp but uses +// the new pass manager. Once the NPM becomes the default, we can simply remove + +// Regression test for origin propagation in "select i1, float, float". +// https://code.google.com/p/memory-sanitizer/issues/detail?id=78 + +// RUN: %clangxx_msan -O2 -fsanitize-memory-track-origins %s \ +// RUN: -fexperimental-new-pass-manager -o %t && not %run %t >%t.out 2>&1 +// RUN: FileCheck %s < %t.out + +// RUN: %clangxx_msan -O2 -fsanitize-memory-track-origins=2 %s \ +// RUN: -fexperimental-new-pass-manager -o %t && not %run %t >%t.out 2>&1 +// RUN: FileCheck %s < %t.out + +#include <stdio.h> +#include <sanitizer/msan_interface.h> + +int main() { + volatile bool b = true; + float x, y; + __msan_allocated_memory(&x, sizeof(x)); + __msan_allocated_memory(&y, sizeof(y)); + float z = b ? x : y; + if (z > 0) printf(".\n"); + // CHECK: Memory was marked as uninitialized + // CHECK: {{#0 0x.* in .*__msan_allocated_memory}} + // CHECK: {{#1 0x.* in main .*select_float_origin_npm.cpp:}}[[@LINE-6]] + return 0; +} Index: compiler-rt/test/msan/chained_origin_npm.cpp =================================================================== --- /dev/null +++ compiler-rt/test/msan/chained_origin_npm.cpp @@ -0,0 +1,74 @@ +// FIXME: This test case is a copy of chained_origin.cpp but uses the new pass +// manager. Once the NPM becomes the default, we can simply remove this test. +// RUN: %clangxx_msan -fsanitize-memory-track-origins=2 -O3 \ +// RUN: -fexperimental-new-pass-manager %s -o %t && not %run %t >%t.out 2>&1 +// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-%short-stack \ +// RUN: --check-prefix=CHECK-STACK < %t.out + +// RUN: %clangxx_msan -fsanitize-memory-track-origins=2 -DHEAP=1 -O3 \ +// RUN: -fexperimental-new-pass-manager %s -o %t && not %run %t >%t.out 2>&1 +// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-%short-stack \ +// RUN: --check-prefix=CHECK-HEAP < %t.out + + +// RUN: %clangxx_msan -mllvm -msan-instrumentation-with-call-threshold=0 \ +// RUN: -fsanitize-memory-track-origins=2 -fexperimental-new-pass-manager \ +// RUN: -O3 %s -o %t && not %run %t >%t.out 2>&1 +// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-%short-stack \ +// RUN: --check-prefix=CHECK-STACK < %t.out + +// RUN: %clangxx_msan -mllvm -msan-instrumentation-with-call-threshold=0 \ +// RUN: -fsanitize-memory-track-origins=2 -fexperimental-new-pass-manager \ +// RUN: -DHEAP=1 -O3 %s -o %t && 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; + +__attribute__((noinline)) +void fn_g(int a) { + x = a; +} + +__attribute__((noinline)) +void fn_f(int a) { + fn_g(a); +} + +__attribute__((noinline)) +void fn_h() { + y = x; +} + +int main(int argc, char *argv[]) { +#ifdef HEAP + int * volatile zz = new int; + int z = *zz; +#else + int volatile z; +#endif + fn_f(z); + fn_h(); + return y; +} + +// CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value +// CHECK: {{#0 .* in main.*chained_origin_npm.cpp:}}[[@LINE-4]] + +// CHECK: Uninitialized value was stored to memory at +// CHECK-FULL-STACK: {{#0 .* in fn_h.*chained_origin_npm.cpp:}}[[@LINE-19]] +// CHECK-FULL-STACK: {{#1 .* in main.*chained_origin_npm.cpp:}}[[@LINE-9]] +// CHECK-SHORT-STACK: {{#0 .* in fn_h.*chained_origin_npm.cpp:}}[[@LINE-21]] + +// CHECK: Uninitialized value was stored to memory at +// CHECK-FULL-STACK: {{#0 .* in fn_g.*chained_origin_npm.cpp:}}[[@LINE-34]] +// CHECK-FULL-STACK: {{#1 .* in fn_f.*chained_origin_npm.cpp:}}[[@LINE-30]] +// CHECK-FULL-STACK: {{#2 .* in main.*chained_origin_npm.cpp:}}[[@LINE-16]] +// CHECK-SHORT-STACK: {{#0 .* in fn_g.*chained_origin_npm.cpp:}}[[@LINE-37]] + +// 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_npm.cpp:}}[[@LINE-27]] + +// CHECK-HEAP: Uninitialized value was created by a heap allocation +// CHECK-HEAP: {{#1 .* in main.*chained_origin_npm.cpp:}}[[@LINE-28]] Index: compiler-rt/test/msan/chained_origin_memcpy_npm.cpp =================================================================== --- /dev/null +++ compiler-rt/test/msan/chained_origin_memcpy_npm.cpp @@ -0,0 +1,71 @@ +// FIXME: This test case is a copy of chained_origin_memcpy.cpp but uses +// the new pass manager. Once the NPM becomes the default, we can simply remove + +// RUN: %clangxx_msan -fsanitize-memory-track-origins=2 -DOFFSET=0 -O3 \ +// RUN: -fexperimental-new-pass-manager %s -o %t && not %run %t >%t.out 2>&1 +// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-Z1 \ +// RUN: --check-prefix=CHECK-%short-stack < %t.out + +// RUN: %clangxx_msan -fsanitize-memory-track-origins=2 -DOFFSET=10 -O3 %s \ +// RUN: -fexperimental-new-pass-manager -o %t && not %run %t >%t.out 2>&1 +// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-Z2 \ +// RUN: --check-prefix=CHECK-%short-stack < %t.out + + +// RUN: %clangxx_msan -mllvm -msan-instrumentation-with-call-threshold=0 \ +// RUN: -fexperimental-new-pass-manager -fsanitize-memory-track-origins=2 \ +// RUN: -DOFFSET=0 -O3 %s -o %t && \not %run %t >%t.out 2>&1 +// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-Z1 \ +// RUN: --check-prefix=CHECK-%short-stack < %t.out + +// RUN: %clangxx_msan -mllvm -msan-instrumentation-with-call-threshold=0 \ +// RUN: -fexperimental-new-pass-manager -fsanitize-memory-track-origins=2 \ +// RUN: -DOFFSET=10 -O3 %s -o %t && not %run %t >%t.out 2>&1 +// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-Z2 \ +// RUN: --check-prefix=CHECK-%short-stack < %t.out + +#include <stdio.h> +#include <string.h> + +int xx[10000]; +int yy[10000]; +volatile int idx = 30; + +__attribute__((noinline)) +void fn_g(int a, int b) { + xx[idx] = a; xx[idx + 10] = b; +} + +__attribute__((noinline)) +void fn_f(int a, int b) { + fn_g(a, b); +} + +__attribute__((noinline)) +void fn_h() { + memcpy(&yy, &xx, sizeof(xx)); +} + +int main(int argc, char *argv[]) { + int volatile z1; + int volatile z2; + fn_f(z1, z2); + fn_h(); + return yy[idx + OFFSET]; +} + +// CHECK: WARNING: MemorySanitizer: use-of-uninitialized-value +// CHECK: {{#0 .* in main .*chained_origin_memcpy_npm.cpp:}}[[@LINE-4]] + +// CHECK: Uninitialized value was stored to memory at +// CHECK-FULL-STACK: {{#1 .* in fn_h.*chained_origin_memcpy_npm.cpp:}}[[@LINE-15]] +// CHECK-SHORT-STACK: {{#0 .* in __msan_memcpy.*msan_interceptors.cpp:}} + +// CHECK: Uninitialized value was stored to memory at +// CHECK-FULL-STACK: {{#0 .* in fn_g.*chained_origin_memcpy_npm.cpp:}}[[@LINE-29]] +// CHECK-FULL-STACK: {{#1 .* in fn_f.*chained_origin_memcpy_npm.cpp:}}[[@LINE-25]] +// CHECK-SHORT-STACK: {{#0 .* in fn_g.*chained_origin_memcpy_npm.cpp:}}[[@LINE-31]] + +// 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_npm.cpp:}}[[@LINE-22]] Index: compiler-rt/test/msan/chained_origin_limits_npm.cpp =================================================================== --- /dev/null +++ compiler-rt/test/msan/chained_origin_limits_npm.cpp @@ -0,0 +1,193 @@ +// FIXME: This test case is a copy of chained_origin_limits.cpp but uses +// the new pass manager. Once the NPM becomes the default, we can simply remove +// This test program creates a very large number of unique histories. + +// Heap origin. +// RUN: %clangxx_msan -fsanitize-memory-track-origins=2 \ +// RUN: -fexperimental-new-pass-manager -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 --check-prefix=CHECK-%short-stack < %t.out + +// RUN: MSAN_OPTIONS=origin_history_size=7,origin_history_per_stack_limit=0 not %run %t >%t.out 2>&1 +// RUN: FileCheck %s --check-prefix=CHECK7 < %t.out + +// Stack origin. +// RUN: %clangxx_msan -DSTACK -fsanitize-memory-track-origins=2 -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 --check-prefix=CHECK-%short-stack < %t.out + +// RUN: MSAN_OPTIONS=origin_history_size=7,origin_history_per_stack_limit=0 not %run %t >%t.out 2>&1 +// RUN: FileCheck %s --check-prefix=CHECK7 < %t.out + + +// Heap origin, with calls. +// RUN: %clangxx_msan -mllvm -msan-instrumentation-with-call-threshold=0 \ +// RUN: -fexperimental-new-pass-manager -fsanitize-memory-track-origins=2 \ +// RUN: -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 --check-prefix=CHECK-%short-stack < %t.out + +// RUN: MSAN_OPTIONS=origin_history_size=7,origin_history_per_stack_limit=0 not %run %t >%t.out 2>&1 +// RUN: FileCheck %s --check-prefix=CHECK7 < %t.out + + +// Stack origin, with calls. +// RUN: %clangxx_msan -DSTACK -fexperimental-new-pass-manager \ +// RUN: -mllvm -msan-instrumentation-with-call-threshold=0 \ +// RUN: -fsanitize-memory-track-origins=2 -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 --check-prefix=CHECK-%short-stack < %t.out + +// RUN: MSAN_OPTIONS=origin_history_size=7,origin_history_per_stack_limit=0 not %run %t >%t.out 2>&1 +// RUN: FileCheck %s --check-prefix=CHECK7 < %t.out + +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <unistd.h> + +static char *buf, *cur, *end; +void init() { + buf = new char[1000]; +#ifdef STACK + char stackbuf[1000]; + char *volatile p = stackbuf; + memcpy(buf, p, 1000); +#endif + cur = buf; + end = buf + 1000; +} + +void line_flush() { + char *p; + for (p = cur - 1; p >= buf; --p) + if (*p == '\n') + break; + if (p >= buf) { + size_t write_sz = p - buf + 1; + // write(2, buf, write_sz); + memmove(buf, p + 1, end - p - 1); + cur -= write_sz; + } +} + +void buffered_write(const char *p, size_t sz) { + while (sz > 0) { + size_t copy_sz = end - cur; + if (sz < copy_sz) copy_sz = sz; + memcpy(cur, p, copy_sz); + cur += copy_sz; + sz -= copy_sz; + line_flush(); + } +} + +void fn1() { + buffered_write("a\n", 2); +} + +void fn2() { + buffered_write("a\n", 2); +} + +void fn3() { + buffered_write("a\n", 2); +} + +int main(void) { + init(); + for (int i = 0; i < 2000; ++i) { + fn1(); + fn2(); + fn3(); + } + return buf[50]; +} + +// CHECK7: WARNING: MemorySanitizer: use-of-uninitialized-value +// CHECK7-NOT: Uninitialized value was stored to memory at +// CHECK7: Uninitialized value was stored to memory at +// CHECK7-NOT: Uninitialized value was stored to memory at +// CHECK7: Uninitialized value was stored to memory at +// CHECK7-NOT: Uninitialized value was stored to memory at +// CHECK7: Uninitialized value was stored to memory at +// CHECK7-NOT: Uninitialized value was stored to memory at +// CHECK7: Uninitialized value was stored to memory at +// CHECK7-NOT: Uninitialized value was stored to memory at +// CHECK7: Uninitialized value was stored to memory at +// CHECK7-NOT: Uninitialized value was stored to memory at +// CHECK7: Uninitialized value was stored to memory at +// CHECK7-NOT: Uninitialized value was stored to memory at +// CHECK7: Uninitialized value was created + +// CHECK2: WARNING: MemorySanitizer: use-of-uninitialized-value +// CHECK2-NOT: Uninitialized value was stored to memory at +// CHECK2: Uninitialized value was stored to memory at +// CHECK2-NOT: Uninitialized value was stored to memory at +// CHECK2: Uninitialized value was created + +// For architectures with short stack all the stacks in the chain are same +// because the stack trace does not contain frames upto the functions fn1, fn2, +// fn3 from where the uninitialized stores actually originate. Since we report +// uninitialized value store once for each stack frame +// (origin_history_per_stack_limit = 1) we expect only one instance of +// "Uninitialized value was stored to memory at". + +// CHECK-PER-STACK: WARNING: MemorySanitizer: use-of-uninitialized-value +// CHECK-PER-STACK: Uninitialized value was stored to memory at +// CHECK-SHORT-STACK: in __msan_memmove +// CHECK-FULL-STACK: in fn3 +// CHECK-FULL-STACK: Uninitialized value was stored to memory at +// CHECK-FULL-STACK: in fn2 +// CHECK-FULL-STACK: Uninitialized value was stored to memory at +// CHECK-FULL-STACK: in fn1 +// CHECK-PER-STACK: Uninitialized value was created + +// CHECK-UNLIMITED: WARNING: MemorySanitizer: use-of-uninitialized-value +// CHECK-UNLIMITED: Uninitialized value was stored to memory at +// CHECK-UNLIMITED: Uninitialized value was stored to memory at +// CHECK-UNLIMITED: Uninitialized value was stored to memory at +// CHECK-UNLIMITED: Uninitialized value was stored to memory at +// CHECK-UNLIMITED: Uninitialized value was stored to memory at +// CHECK-UNLIMITED: Uninitialized value was stored to memory at +// CHECK-UNLIMITED: Uninitialized value was stored to memory at +// CHECK-UNLIMITED: Uninitialized value was stored to memory at +// CHECK-UNLIMITED: Uninitialized value was stored to memory at +// CHECK-UNLIMITED: Uninitialized value was stored to memory at +// CHECK-UNLIMITED: Uninitialized value was stored to memory at +// CHECK-UNLIMITED: Uninitialized value was stored to memory at +// CHECK-UNLIMITED: Uninitialized value was stored to memory at +// CHECK-UNLIMITED: Uninitialized value was stored to memory at +// CHECK-UNLIMITED: Uninitialized value was stored to memory at +// CHECK-UNLIMITED: Uninitialized value was stored to memory at +// CHECK-UNLIMITED: Uninitialized value was stored to memory at +// CHECK-UNLIMITED: Uninitialized value was stored to memory at +// CHECK-UNLIMITED: Uninitialized value was created Index: compiler-rt/test/msan/chained_origin_empty_stack_npm.cpp =================================================================== --- /dev/null +++ compiler-rt/test/msan/chained_origin_empty_stack_npm.cpp @@ -0,0 +1,38 @@ +// FIXME: This test case is a copy of chained_origin_empty_stack.cpp but uses +// the new pass manager. Once the NPM becomes the default, we can simply remove +// this test. +// RUN: %clangxx_msan -fsanitize-memory-track-origins=2 \ +// RUN: -fexperimental-new-pass-manager -O3 %s -o %t && \ +// RUN: MSAN_OPTIONS=store_context_size=1 not %run %t 2>&1 | FileCheck %s + +// Test that stack trace for the intermediate store is not empty. + +// CHECK: MemorySanitizer: use-of-uninitialized-value +// CHECK: #0 {{.*}} in main + +// CHECK: Uninitialized value was stored to memory at +// CHECK: #0 {{.*}} in fn_g +// CHECK-NOT: #1 + +// CHECK: Uninitialized value was created by an allocation of 'z' in the stack frame of function 'main' +// CHECK: #0 {{.*}} in main + +#include <stdio.h> + +volatile int x; + +__attribute__((noinline)) +void fn_g(int a) { + x = a; +} + +__attribute__((noinline)) +void fn_f(int a) { + fn_g(a); +} + +int main(int argc, char *argv[]) { + int volatile z; + fn_f(z); + return x; +} Index: clang/lib/CodeGen/BackendUtil.cpp =================================================================== --- clang/lib/CodeGen/BackendUtil.cpp +++ clang/lib/CodeGen/BackendUtil.cpp @@ -1233,12 +1233,17 @@ FPM.addPass(BoundsCheckingPass()); }); if (LangOpts.Sanitize.has(SanitizerKind::Memory)) { - PB.registerPipelineStartEPCallback([](ModulePassManager &MPM) { - MPM.addPass(MemorySanitizerPass({})); - }); + int TrackOrigins = CodeGenOpts.SanitizeMemoryTrackOrigins; + bool Recover = CodeGenOpts.SanitizeRecover.has(SanitizerKind::Memory); + PB.registerPipelineStartEPCallback( + [this, TrackOrigins, Recover](ModulePassManager &MPM) { + MPM.addPass(MemorySanitizerPass({TrackOrigins, Recover, false})); + }); PB.registerOptimizerLastEPCallback( - [](FunctionPassManager &FPM, PassBuilder::OptimizationLevel Level) { - FPM.addPass(MemorySanitizerPass({})); + [this, TrackOrigins, + Recover](FunctionPassManager &FPM, + PassBuilder::OptimizationLevel Level) { + FPM.addPass(MemorySanitizerPass({TrackOrigins, Recover, false})); }); } if (LangOpts.Sanitize.has(SanitizerKind::Thread)) {
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
