[PATCH] D108202: [tsan] Add support for disable_sanitizer_instrumentation attribute

2021-08-23 Thread Alexander Potapenko via Phabricator via cfe-commits
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

2021-08-23 Thread Alexander Potapenko via Phabricator via cfe-commits
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

2021-08-17 Thread Marco Elver via Phabricator via cfe-commits
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

2021-08-17 Thread Alexander Potapenko via Phabricator via cfe-commits
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