[PATCH] D108202: [tsan] Add support for disable_sanitizer_instrumentation attribute
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8300d52e8cbf: [tsan] Add support for disable_sanitizer_instrumentation attribute (authored by glider). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108202/new/ https://reviews.llvm.org/D108202 Files: clang/docs/ThreadSanitizer.rst clang/test/CodeGen/sanitize-thread-disable.c llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp Index: llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp === --- llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp +++ llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp @@ -562,6 +562,12 @@ // all. if (F.hasFnAttribute(Attribute::Naked)) return false; + + // __attribute__(disable_sanitizer_instrumentation) prevents all kinds of + // instrumentation. + if (F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation)) +return false; + initialize(*F.getParent()); SmallVector AllLoadsAndStores; SmallVector LocalLoadsAndStores; Index: clang/test/CodeGen/sanitize-thread-disable.c === --- /dev/null +++ clang/test/CodeGen/sanitize-thread-disable.c @@ -0,0 +1,57 @@ +// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s | FileCheck -check-prefixes CHECK,WITHOUT %s +// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s -fsanitize=thread | FileCheck -check-prefixes CHECK,TSAN %s + +#include + +// Instrumented function. +// TSan inserts calls to __tsan_func_entry() and __tsan_func_exit() to prologue/epilogue. +// Non-atomic loads are instrumented with __tsan_readXXX(), atomic loads - with +// __tsan_atomicXXX_load(). +// +// CHECK-LABEL: @instrumented1 +// TSAN: call void @__tsan_func_entry +// WITHOUT-NOT: call void @__tsan_func_entry +// TSAN: call void @__tsan_read4 +// WITHOUT-NOT: call void @__tsan_read4 +// TSAN: call i32 @__tsan_atomic32_load +// WITHOUT-NOT: call i32 @__tsan_atomic32_load +// TSAN: call void @__tsan_func_exit +// WITHOUT-NOT: call void @__tsan_func_exit +// CHECK: ret i32 +int instrumented1(int *a, _Atomic int *b) { + return *a + atomic_load(b); +} + +// Function with no_sanitize("thread"). +// TSan only inserts instrumentation necessary to prevent false positives: calls are inserted for +// function entry/exit and atomics, but not plain memory accesses. +// +// CHECK-LABEL: @no_false_positives1 +// TSAN: call void @__tsan_func_entry +// WITHOUT-NOT: call void @__tsan_func_entry +// TSAN-NOT: call void @__tsan_read4 +// WITHOUT-NOT: call void @__tsan_read4 +// TSAN: call i32 @__tsan_atomic32_load +// WITHOUT-NOT: call i32 @__tsan_atomic32_load +// TSAN: call void @__tsan_func_exit +// WITHOUT-NOT: call void @__tsan_func_exit +// CHECK: ret i32 +__attribute__((no_sanitize("thread"))) int no_false_positives1(int *a, _Atomic int *b) { + return *a + atomic_load(b); +} + +// Function with disable_sanitizer_instrumentation: no instrumentation at all. +// +// CHECK-LABEL: @no_instrumentation1 +// TSAN-NOT: call void @__tsan_func_entry +// WITHOUT-NOT: call void @__tsan_func_entry +// TSAN-NOT: call void @__tsan_read4 +// WITHOUT-NOT: call void @__tsan_read4 +// TSAN-NOT: call i32 @__tsan_atomic32_load +// WITHOUT-NOT: call i32 @__tsan_atomic32_load +// TSAN-NOT: call void @__tsan_func_exit +// WITHOUT-NOT: call void @__tsan_func_exit +// CHECK: ret i32 +__attribute__((disable_sanitizer_instrumentation)) int no_instrumentation1(int *a, _Atomic int *b) { + return *a + atomic_load(b); +} Index: clang/docs/ThreadSanitizer.rst === --- clang/docs/ThreadSanitizer.rst +++ clang/docs/ThreadSanitizer.rst @@ -100,6 +100,16 @@ traces. This attribute may not be supported by other compilers, so we suggest to use it together with ``__has_feature(thread_sanitizer)``. +``__attribute__((disable_sanitizer_instrumentation))`` + + +The ``disable_sanitizer_instrumentation`` attribute can be applied to functions +to prevent all kinds of instrumentation. As a result, it may introduce false +positives and incorrect stack traces. Therefore, it should be used with care, +and only if absolutely required; for example for certain code that cannot +tolerate any instrumentation and resulting side-effects. This attribute +overrides ``no_sanitize("thread")``. + Ignorelist -- Index: llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp === --- llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp +++ llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp @@ -562,6 +562,12 @@ // all. if (F.hasFnAttribute(Attribute::Naked)) return false; + + // __attribute__(disable_sanitizer_instrumentation)
[PATCH] D108202: [tsan] Add support for disable_sanitizer_instrumentation attribute
glider updated this revision to Diff 368056. glider marked 5 inline comments as done. glider added a comment. Addressed Marco's comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108202/new/ https://reviews.llvm.org/D108202 Files: clang/docs/ThreadSanitizer.rst clang/test/CodeGen/sanitize-thread-disable.c llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp Index: llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp === --- llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp +++ llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp @@ -562,6 +562,12 @@ // all. if (F.hasFnAttribute(Attribute::Naked)) return false; + + // __attribute__(disable_sanitizer_instrumentation) prevents all kinds of + // instrumentation. + if (F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation)) +return false; + initialize(*F.getParent()); SmallVector AllLoadsAndStores; SmallVector LocalLoadsAndStores; Index: clang/test/CodeGen/sanitize-thread-disable.c === --- /dev/null +++ clang/test/CodeGen/sanitize-thread-disable.c @@ -0,0 +1,57 @@ +// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s | FileCheck -check-prefixes CHECK,WITHOUT %s +// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - %s -fsanitize=thread | FileCheck -check-prefixes CHECK,TSAN %s + +#include + +// Instrumented function. +// TSan inserts calls to __tsan_func_entry() and __tsan_func_exit() to prologue/epilogue. +// Non-atomic loads are instrumented with __tsan_readXXX(), atomic loads - with +// __tsan_atomicXXX_load(). +// +// CHECK-LABEL: @instrumented1 +// TSAN: call void @__tsan_func_entry +// WITHOUT-NOT: call void @__tsan_func_entry +// TSAN: call void @__tsan_read4 +// WITHOUT-NOT: call void @__tsan_read4 +// TSAN: call i32 @__tsan_atomic32_load +// WITHOUT-NOT: call i32 @__tsan_atomic32_load +// TSAN: call void @__tsan_func_exit +// WITHOUT-NOT: call void @__tsan_func_exit +// CHECK: ret i32 +int instrumented1(int *a, _Atomic int *b) { + return *a + atomic_load(b); +} + +// Function with no_sanitize("thread"). +// TSan only inserts instrumentation necessary to prevent false positives: calls are inserted for +// function entry/exit and atomics, but not plain memory accesses. +// +// CHECK-LABEL: @no_false_positives1 +// TSAN: call void @__tsan_func_entry +// WITHOUT-NOT: call void @__tsan_func_entry +// TSAN-NOT: call void @__tsan_read4 +// WITHOUT-NOT: call void @__tsan_read4 +// TSAN: call i32 @__tsan_atomic32_load +// WITHOUT-NOT: call i32 @__tsan_atomic32_load +// TSAN: call void @__tsan_func_exit +// WITHOUT-NOT: call void @__tsan_func_exit +// CHECK: ret i32 +__attribute__((no_sanitize("thread"))) int no_false_positives1(int *a, _Atomic int *b) { + return *a + atomic_load(b); +} + +// Function with disable_sanitizer_instrumentation: no instrumentation at all. +// +// CHECK-LABEL: @no_instrumentation1 +// TSAN-NOT: call void @__tsan_func_entry +// WITHOUT-NOT: call void @__tsan_func_entry +// TSAN-NOT: call void @__tsan_read4 +// WITHOUT-NOT: call void @__tsan_read4 +// TSAN-NOT: call i32 @__tsan_atomic32_load +// WITHOUT-NOT: call i32 @__tsan_atomic32_load +// TSAN-NOT: call void @__tsan_func_exit +// WITHOUT-NOT: call void @__tsan_func_exit +// CHECK: ret i32 +__attribute__((disable_sanitizer_instrumentation)) int no_instrumentation1(int *a, _Atomic int *b) { + return *a + atomic_load(b); +} Index: clang/docs/ThreadSanitizer.rst === --- clang/docs/ThreadSanitizer.rst +++ clang/docs/ThreadSanitizer.rst @@ -100,6 +100,16 @@ traces. This attribute may not be supported by other compilers, so we suggest to use it together with ``__has_feature(thread_sanitizer)``. +``__attribute__((disable_sanitizer_instrumentation))`` + + +The ``disable_sanitizer_instrumentation`` attribute can be applied to functions +to prevent all kinds of instrumentation. As a result, it may introduce false +positives and incorrect stack traces. Therefore, it should be used with care, +and only if absolutely required; for example for certain code that cannot +tolerate any instrumentation and resulting side-effects. This attribute +overrides ``no_sanitize("thread")``. + Ignorelist -- Index: llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp === --- llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp +++ llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp @@ -562,6 +562,12 @@ // all. if (F.hasFnAttribute(Attribute::Naked)) return false; + + // __attribute__(disable_sanitizer_instrumentation) prevents all kinds of + // instrumentation. + if (F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation)) +
[PATCH] D108202: [tsan] Add support for disable_sanitizer_instrumentation attribute
melver accepted this revision. melver added inline comments. This revision is now accepted and ready to land. Comment at: clang/docs/ThreadSanitizer.rst:106 + +The ``disable_sanitizer_instrumentation`` attribute can be applied to a certain +function to prevent all kinds of instrumentation. This attribute overrides s/to a certain function/to functions/ Comment at: clang/docs/ThreadSanitizer.rst:108 +function to prevent all kinds of instrumentation. This attribute overrides +``no_sanitize("thread")`` and may introduce false positives, so it should +be used with care, e.g. when the user wants to ensure critical code does not It might be clearer to say ``` .. of instrumentation. As a result, it may introduce false positives and incorrect stack traces. Therefore, it should be used with care, and only if absolutely required; for example for certain code that cannot tolerate any instrumentation and resulting side-effects. This attribute overrides ``no_sanitize("thread")``. ``` to deter normal users from using this. Comment at: clang/test/CodeGen/sanitize-thread-disable.c:11 +// +// CHECK: @instrumented1 +// TSAN: call void @__tsan_func_entry Comment at: clang/test/CodeGen/sanitize-thread-disable.c:29 +// +// CHECK: @no_false_positives1 +// TSAN: call void @__tsan_func_entry Comment at: clang/test/CodeGen/sanitize-thread-disable.c:45 +// +// CHECK: @no_instrumentation1 +// TSAN-NOT: call void @__tsan_func_entry Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108202/new/ https://reviews.llvm.org/D108202 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D108202: [tsan] Add support for disable_sanitizer_instrumentation attribute
glider created this revision. glider added a reviewer: melver. Herald added subscribers: jfb, hiraditya. glider requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. Unlike __attribute__((no_sanitize("thread"))), this one will cause TSan to skip the entire function during instrumentation. Depends on https://reviews.llvm.org/D108029 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D108202 Files: clang/docs/ThreadSanitizer.rst clang/test/CodeGen/sanitize-thread-disable.c llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp Index: llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp === --- llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp +++ llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp @@ -563,6 +563,12 @@ // all. if (F.hasFnAttribute(Attribute::Naked)) return false; + + // __attribute__(disable_sanitizer_instrumentation) prevents all kinds of + // instrumentation. + if (F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation)) +return false; + initialize(*F.getParent()); SmallVector AllLoadsAndStores; SmallVector LocalLoadsAndStores; Index: clang/test/CodeGen/sanitize-thread-disable.c === --- /dev/null +++ clang/test/CodeGen/sanitize-thread-disable.c @@ -0,0 +1,57 @@ +// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck -check-prefixes CHECK,WITHOUT %s +// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=thread | FileCheck -check-prefixes CHECK,TSAN %s + +#include + +// Instrumented function. +// TSan inserts calls to __tsan_func_entry() and __tsan_func_exit() to prologue/epilogue. +// Non-atomic loads are instrumented with __tsan_readXXX(), atomic loads - with +// __tsan_atomicXXX_load(). +// +// CHECK: @instrumented1 +// TSAN: call void @__tsan_func_entry +// WITHOUT-NOT: call void @__tsan_func_entry +// TSAN: call void @__tsan_read4 +// WITHOUT-NOT: call void @__tsan_read4 +// TSAN: call i32 @__tsan_atomic32_load +// WITHOUT-NOT: call i32 @__tsan_atomic32_load +// TSAN: call void @__tsan_func_exit +// WITHOUT-NOT: call void @__tsan_func_exit +// CHECK: ret i32 +int instrumented1(int *a, _Atomic int *b) { + return *a + atomic_load(b); +} + +// Function with no_sanitize("thread"). +// TSan only inserts instrumentation necessary to prevent false positives: calls are inserted for +// function entry/exit and atomics, but not plain memory accesses. +// +// CHECK: @no_false_positives1 +// TSAN: call void @__tsan_func_entry +// WITHOUT-NOT: call void @__tsan_func_entry +// TSAN-NOT: call void @__tsan_read4 +// WITHOUT-NOT: call void @__tsan_read4 +// TSAN: call i32 @__tsan_atomic32_load +// WITHOUT-NOT: call i32 @__tsan_atomic32_load +// TSAN: call void @__tsan_func_exit +// WITHOUT-NOT: call void @__tsan_func_exit +// CHECK: ret i32 +__attribute__((no_sanitize("thread"))) int no_false_positives1(int *a, _Atomic int *b) { + return *a + atomic_load(b); +} + +// Function with disable_sanitizer_instrumentation: no instrumentation at all. +// +// CHECK: @no_instrumentation1 +// TSAN-NOT: call void @__tsan_func_entry +// WITHOUT-NOT: call void @__tsan_func_entry +// TSAN-NOT: call void @__tsan_read4 +// WITHOUT-NOT: call void @__tsan_read4 +// TSAN-NOT: call i32 @__tsan_atomic32_load +// WITHOUT-NOT: call i32 @__tsan_atomic32_load +// TSAN-NOT: call void @__tsan_func_exit +// WITHOUT-NOT: call void @__tsan_func_exit +// CHECK: ret i32 +__attribute__((disable_sanitizer_instrumentation)) int no_instrumentation1(int *a, _Atomic int *b) { + return *a + atomic_load(b); +} Index: clang/docs/ThreadSanitizer.rst === --- clang/docs/ThreadSanitizer.rst +++ clang/docs/ThreadSanitizer.rst @@ -100,6 +100,15 @@ traces. This attribute may not be supported by other compilers, so we suggest to use it together with ``__has_feature(thread_sanitizer)``. +``__attribute__((disable_sanitizer_instrumentation))`` + + +The ``disable_sanitizer_instrumentation`` attribute can be applied to a certain +function to prevent all kinds of instrumentation. This attribute overrides +``no_sanitize("thread")`` and may introduce false positives, so it should +be used with care, e.g. when the user wants to ensure critical code does not +have unexpected side effects. + Ignorelist -- Index: llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp === --- llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp +++ llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp @@ -563,6 +563,12 @@ // all. if (F.hasFnAttribute(Attribute::Naked)) return false; + + // __attribute__(disable_sanitizer_instrumentation) prevents all kinds of + // instrumentation. + if