[clang] 05ee1f4 - Revert "[asan] Add support for disable_sanitizer_instrumentation attribute"

Alexander Potapenko via cfe-commits cfe-commits at lists.llvm.org
Tue Feb 15 06:05:02 PST 2022


Author: Alexander Potapenko
Date: 2022-02-15T15:04:53+01:00
New Revision: 05ee1f4af8972f021917fb1f96624050fc6268bd

URL: https://github.com/llvm/llvm-project/commit/05ee1f4af8972f021917fb1f96624050fc6268bd
DIFF: https://github.com/llvm/llvm-project/commit/05ee1f4af8972f021917fb1f96624050fc6268bd.diff

LOG: Revert "[asan] Add support for disable_sanitizer_instrumentation attribute"

This reverts commit dd145f953db3dafbc019f1d3783bb4f09a28af92.

https://reviews.llvm.org/D119726, like https://reviews.llvm.org/D114421,
still causes TSan to fail, see https://lab.llvm.org/buildbot/#/builders/70/builds/18020

Differential Revision: https://reviews.llvm.org/D119838

Added: 
    

Modified: 
    clang/docs/AddressSanitizer.rst
    clang/lib/CodeGen/CodeGenFunction.cpp
    clang/lib/CodeGen/SanitizerMetadata.cpp
    clang/test/CodeGen/address-safety-attr-flavors.cpp
    clang/test/CodeGen/asan-globals.cpp
    llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

Removed: 
    llvm/test/Instrumentation/AddressSanitizer/asan-disable-sanitizer-instrumentation.ll


################################################################################
diff  --git a/clang/docs/AddressSanitizer.rst b/clang/docs/AddressSanitizer.rst
index fe5f683580a46..06b53e2e5da0b 100644
--- a/clang/docs/AddressSanitizer.rst
+++ b/clang/docs/AddressSanitizer.rst
@@ -229,12 +229,6 @@ compilers, so we suggest to use it together with
 The same attribute used on a global variable prevents AddressSanitizer
 from adding redzones around it and detecting out of bounds accesses.
 
-
-AddressSanitizer also supports
-``__attribute__((disable_sanitizer_instrumentation))``. This attribute
-works similar to ``__attribute__((no_sanitize("address")))``, but it also
-prevents instrumentation performed by other sanitizers.
-
 Suppressing Errors in Recompiled Code (Ignorelist)
 --------------------------------------------------
 

diff  --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp
index bd29842afbaa9..a7f62b4a4e30a 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -383,6 +383,9 @@ void CodeGenFunction::FinishFunction(SourceLocation EndLoc) {
                        "__cyg_profile_func_exit");
   }
 
+  if (ShouldSkipSanitizerInstrumentation())
+    CurFn->addFnAttr(llvm::Attribute::DisableSanitizerInstrumentation);
+
   // Emit debug descriptor for function end.
   if (CGDebugInfo *DI = getDebugInfo())
     DI->EmitFunctionEnd(Builder, CurFn);
@@ -764,22 +767,18 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, QualType RetTy,
       Fn->addFnAttr(llvm::Attribute::NoSanitizeCoverage);
   }
 
-  if (ShouldSkipSanitizerInstrumentation()) {
-    CurFn->addFnAttr(llvm::Attribute::DisableSanitizerInstrumentation);
-  } else {
-    // Apply sanitizer attributes to the function.
-    if (SanOpts.hasOneOf(SanitizerKind::Address | SanitizerKind::KernelAddress))
-      Fn->addFnAttr(llvm::Attribute::SanitizeAddress);
-    if (SanOpts.hasOneOf(SanitizerKind::HWAddress |
-                         SanitizerKind::KernelHWAddress))
-      Fn->addFnAttr(llvm::Attribute::SanitizeHWAddress);
-    if (SanOpts.has(SanitizerKind::MemTag))
-      Fn->addFnAttr(llvm::Attribute::SanitizeMemTag);
-    if (SanOpts.has(SanitizerKind::Thread))
-      Fn->addFnAttr(llvm::Attribute::SanitizeThread);
-    if (SanOpts.hasOneOf(SanitizerKind::Memory | SanitizerKind::KernelMemory))
-      Fn->addFnAttr(llvm::Attribute::SanitizeMemory);
-  }
+  // Apply sanitizer attributes to the function.
+  if (SanOpts.hasOneOf(SanitizerKind::Address | SanitizerKind::KernelAddress))
+    Fn->addFnAttr(llvm::Attribute::SanitizeAddress);
+  if (SanOpts.hasOneOf(SanitizerKind::HWAddress |
+                       SanitizerKind::KernelHWAddress))
+    Fn->addFnAttr(llvm::Attribute::SanitizeHWAddress);
+  if (SanOpts.has(SanitizerKind::MemTag))
+    Fn->addFnAttr(llvm::Attribute::SanitizeMemTag);
+  if (SanOpts.has(SanitizerKind::Thread))
+    Fn->addFnAttr(llvm::Attribute::SanitizeThread);
+  if (SanOpts.hasOneOf(SanitizerKind::Memory | SanitizerKind::KernelMemory))
+    Fn->addFnAttr(llvm::Attribute::SanitizeMemory);
   if (SanOpts.has(SanitizerKind::SafeStack))
     Fn->addFnAttr(llvm::Attribute::SafeStack);
   if (SanOpts.has(SanitizerKind::ShadowCallStack))

diff  --git a/clang/lib/CodeGen/SanitizerMetadata.cpp b/clang/lib/CodeGen/SanitizerMetadata.cpp
index 9e26d242d3a7e..009965a36c396 100644
--- a/clang/lib/CodeGen/SanitizerMetadata.cpp
+++ b/clang/lib/CodeGen/SanitizerMetadata.cpp
@@ -73,8 +73,6 @@ void SanitizerMetadata::reportGlobalToASan(llvm::GlobalVariable *GV,
   for (auto Attr : D.specific_attrs<NoSanitizeAttr>())
     if (Attr->getMask() & SanitizerKind::Address)
       IsExcluded = true;
-  if (D.hasAttr<DisableSanitizerInstrumentationAttr>())
-    IsExcluded = true;
   reportGlobalToASan(GV, D.getLocation(), OS.str(), D.getType(), IsDynInit,
                      IsExcluded);
 }

diff  --git a/clang/test/CodeGen/address-safety-attr-flavors.cpp b/clang/test/CodeGen/address-safety-attr-flavors.cpp
index ef815555059db..e6d17ed2da340 100644
--- a/clang/test/CodeGen/address-safety-attr-flavors.cpp
+++ b/clang/test/CodeGen/address-safety-attr-flavors.cpp
@@ -73,12 +73,3 @@ __attribute__((no_sanitize("kernel-hwaddress"))) int NoSanitizeKernelHWAddress()
 // CHECK-KASAN: {{Function Attrs: mustprogress noinline nounwind sanitize_address$}}
 // CHECK-HWASAN: {{Function Attrs: mustprogress noinline nounwind$}}
 // CHECK-KHWASAN: {{Function Attrs: mustprogress noinline nounwind$}}
-
-__attribute__((disable_sanitizer_instrumentation)) int DisableSanitizerInstrumentation() {
-  return 0;
-}
-// CHECK-NOASAN: {{Function Attrs: disable_sanitizer_instrumentation mustprogress noinline nounwind$}}
-// CHECK-ASAN: {{Function Attrs: disable_sanitizer_instrumentation mustprogress noinline nounwind$}}
-// CHECK-KASAN: {{Function Attrs: disable_sanitizer_instrumentation mustprogress noinline nounwind$}}
-// CHECK-HWASAN: {{Function Attrs: disable_sanitizer_instrumentation mustprogress noinline nounwind$}}
-// CHECK-KHWASAN: {{Function Attrs: disable_sanitizer_instrumentation mustprogress noinline nounwind$}}

diff  --git a/clang/test/CodeGen/asan-globals.cpp b/clang/test/CodeGen/asan-globals.cpp
index 0b17038a5f15a..2cea167d0ea59 100644
--- a/clang/test/CodeGen/asan-globals.cpp
+++ b/clang/test/CodeGen/asan-globals.cpp
@@ -10,7 +10,6 @@
 int global;
 int dyn_init_global = global;
 int __attribute__((no_sanitize("address"))) attributed_global;
-int __attribute__((disable_sanitizer_instrumentation)) disable_instrumentation_global;
 int ignorelisted_global;
 
 int __attribute__((section("__DATA, __common"))) sectioned_global; // KASAN - ignore globals in a section
@@ -51,33 +50,31 @@ void func() {
 // UWTABLE: attributes #[[#ATTR]] = { nounwind uwtable }
 // UWTABLE: ![[#]] = !{i32 7, !"uwtable", i32 2}
 
-// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], ![[DISABLE_INSTR_GLOBAL:[0-9]+]], ![[IGNORELISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], ![[SPECIAL_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
+// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], ![[IGNORELISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], ![[SPECIAL_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
 // CHECK: ![[EXTRA_GLOBAL]] = !{{{.*}} ![[EXTRA_GLOBAL_LOC:[0-9]+]], !"extra_global", i1 false, i1 false}
 // CHECK: ![[EXTRA_GLOBAL_LOC]] = !{!"{{.*}}extra-source.cpp", i32 1, i32 5}
 // CHECK: ![[GLOBAL]] = !{{{.*}} ![[GLOBAL_LOC:[0-9]+]], !"global", i1 false, i1 false}
 // CHECK: ![[GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 10, i32 5}
 // CHECK: ![[DYN_INIT_GLOBAL]] = !{{{.*}} ![[DYN_INIT_LOC:[0-9]+]], !"dyn_init_global", i1 true, i1 false}
 // CHECK: ![[DYN_INIT_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 11, i32 5}
-// CHECK: ![[ATTR_GLOBAL]] = !{{{.*attributed_global.*}}, null, null, i1 false, i1 true}
-// CHECK: ![[DISABLE_INSTR_GLOBAL]] = !{{{.*disable_instrumentation_global.*}}, null, null, i1 false, i1 true}
-// CHECK: ![[IGNORELISTED_GLOBAL]] = !{{{.*ignorelisted_global.*}}, null, null, i1 false, i1 true}
+// CHECK: ![[ATTR_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
+// CHECK: ![[IGNORELISTED_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
 // CHECK: ![[SECTIONED_GLOBAL]] = !{{{.*}} ![[SECTIONED_GLOBAL_LOC:[0-9]+]], !"sectioned_global", i1 false, i1 false}
-// CHECK: ![[SECTIONED_GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 16, i32 50}
+// CHECK: ![[SECTIONED_GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 15, i32 50}
 // CHECK: ![[SPECIAL_GLOBAL]] = !{{{.*}} ![[SPECIAL_GLOBAL_LOC:[0-9]+]], !"__special_global", i1 false, i1 false}
-// CHECK: ![[SPECIAL_GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 18, i32 5}
+// CHECK: ![[SPECIAL_GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 17, i32 5}
 // CHECK: ![[STATIC_VAR]] = !{{{.*}} ![[STATIC_LOC:[0-9]+]], !"static_var", i1 false, i1 false}
-// CHECK: ![[STATIC_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 22, i32 14}
+// CHECK: ![[STATIC_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 21, i32 14}
 // CHECK: ![[LITERAL]] = !{{{.*}} ![[LITERAL_LOC:[0-9]+]], !"<string literal>", i1 false, i1 false}
-// CHECK: ![[LITERAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 23, i32 25}
+// CHECK: ![[LITERAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 22, i32 25}
 
-// IGNORELIST-SRC: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], ![[DISABLE_INSTR_GLOBAL:[0-9]+]], ![[IGNORELISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], ![[SPECIAL_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
+// IGNORELIST-SRC: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], ![[IGNORELISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], ![[SPECIAL_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
 // IGNORELIST-SRC: ![[EXTRA_GLOBAL]] = !{{{.*}} ![[EXTRA_GLOBAL_LOC:[0-9]+]], !"extra_global", i1 false, i1 false}
 // IGNORELIST-SRC: ![[EXTRA_GLOBAL_LOC]] = !{!"{{.*}}extra-source.cpp", i32 1, i32 5}
 // IGNORELIST-SRC: ![[GLOBAL]] = !{{{.*}} null, null, i1 false, i1 true}
 // IGNORELIST-SRC: ![[DYN_INIT_GLOBAL]] = !{{{.*}} null, null, i1 true, i1 true}
-// IGNORELIST-SRC: ![[ATTR_GLOBAL]] = !{{{.*attributed_global.*}}, null, null, i1 false, i1 true}
-// IGNORELIST-SRC: ![[DISABLE_INSTR_GLOBAL]] = !{{{.*disable_instrumentation_global.*}}, null, null, i1 false, i1 true}
-// IGNORELIST-SRC: ![[IGNORELISTED_GLOBAL]] = !{{{.*ignorelisted_global.*}}, null, null, i1 false, i1 true}
+// IGNORELIST-SRC: ![[ATTR_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
+// IGNORELIST-SRC: ![[IGNORELISTED_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
 // IGNORELIST-SRC: ![[SECTIONED_GLOBAL]] = !{{{.*}} null, null, i1 false, i1 true}
 // IGNORELIST-SRC: ![[SPECIAL_GLOBAL]] = !{{{.*}} null, null, i1 false, i1 true}
 // IGNORELIST-SRC: ![[STATIC_VAR]] = !{{{.*}} null, null, i1 false, i1 true}

diff  --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index a8d67c755799d..8f94172a6402d 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -2888,9 +2888,6 @@ bool AddressSanitizer::instrumentFunction(Function &F,
   // Leave if the function doesn't need instrumentation.
   if (!F.hasFnAttribute(Attribute::SanitizeAddress)) return FunctionModified;
 
-  if (F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation))
-    return FunctionModified;
-
   LLVM_DEBUG(dbgs() << "ASAN instrumenting:\n" << F << "\n");
 
   initializeCallbacks(*F.getParent());

diff  --git a/llvm/test/Instrumentation/AddressSanitizer/asan-disable-sanitizer-instrumentation.ll b/llvm/test/Instrumentation/AddressSanitizer/asan-disable-sanitizer-instrumentation.ll
deleted file mode 100644
index ffd94889f97a6..0000000000000
--- a/llvm/test/Instrumentation/AddressSanitizer/asan-disable-sanitizer-instrumentation.ll
+++ /dev/null
@@ -1,47 +0,0 @@
-; This test checks that we are not instrumenting sanitizer code.
-; RUN: opt < %s -passes='asan-pipeline' -S | FileCheck %s
-
-target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-unknown-linux-gnu"
-
-; Function with sanitize_address is instrumented.
-; Function Attrs: nounwind uwtable
-define void @instr_sa(i32* %a) sanitize_address {
-entry:
-  %tmp1 = load i32, i32* %a, align 4
-  %tmp2 = add i32 %tmp1,  1
-  store i32 %tmp2, i32* %a, align 4
-  ret void
-}
-
-; CHECK-LABEL: @instr_sa
-; CHECK: call void @__asan_report_load
-
-
-; Function with disable_sanitizer_instrumentation is not instrumented.
-; Function Attrs: nounwind uwtable
-define void @noinstr_dsi(i32* %a) disable_sanitizer_instrumentation {
-entry:
-  %tmp1 = load i32, i32* %a, align 4
-  %tmp2 = add i32 %tmp1,  1
-  store i32 %tmp2, i32* %a, align 4
-  ret void
-}
-
-; CHECK-LABEL: @noinstr_dsi
-; CHECK-NOT: call void @__asan_report_load
-
-
-; disable_sanitizer_instrumentation takes precedence over sanitize_address.
-; Function Attrs: nounwind uwtable
-define void @noinstr_dsi_sa(i32* %a) disable_sanitizer_instrumentation sanitize_address {
-entry:
-  %tmp1 = load i32, i32* %a, align 4
-  %tmp2 = add i32 %tmp1,  1
-  store i32 %tmp2, i32* %a, align 4
-  ret void
-}
-
-; CHECK-LABEL: @noinstr_dsi_sa
-; CHECK-NOT: call void @__asan_report_load
-


        


More information about the cfe-commits mailing list