[asan] Add support for disable_sanitizer_instrumentation attribute

[glider]

Summary

For ASan this will effectively serve as a synonym for
attribute((no_sanitize("address")))

[melver]

Thanks!

[glider]

Vitaly, Evgenii, can one of you please take a look?

[glider]

Given this is a minor change that only affects users of disable_sanitizer_instrumentation, I'm inclined towards landing it.

[eugenis]

LGTM

clang/test/CodeGen/asan-globals.cpp

61

Does this test rely on the metadata being in the same order as the global declarations in the source? Feels brittle and hard to understand - could you check if the regexps could be made more precise?

[vitalybuka]

Should we have in AddressSanitizer.cpp the following for consistency with other sanitizers?
if (F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation))

return false;

[glider]

Addressed the comments

[glider]

Updated asan-globals.cpp

[glider]

Should we have in AddressSanitizer.cpp the following for consistency with other sanitizers?

Good catch, without that disable_sanitizer_instrumentation could not override sanitize_address.
I fixed this and added a test.

clang/test/CodeGen/asan-globals.cpp

61

llvm.asan.globals above actually look as follows: !llvm.asan.globals = !{!0, !2, !4, !6, !7, !8, !9, !11, !13, !15}

, so the placeholder variables (e.g. ATTR_GLOBAL and DISABLE_INSTR_GLOBAL) actually match just the metadata node numbers.
I replaced the {{.*}} with variable/file names where applicable, but we'll still depend on the order of globals.

[glider]

Rebase.

[glider]

Fix a windows test failure: https://buildkite.com/llvm-project/premerge-checks/builds/69318#0b873cc6-bf07-482a-ac37-e960566ac8a3

[melver]

I think we dropped the ball on this - was it ever re-reverted?
Is it still worth trying to land this?

clang/docs/AddressSanitizer.rst

 `no_address_safety_analysis`) to disable instrumentation of a
 particular function. This attribute may not be supported by other
 compilers, so we suggest to use it together with
 ``__has_feature(address_sanitizer)``.
 
 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)
 --------------------------------------------------
 
 AddressSanitizer supports ``src`` and ``fun`` entity types in
 :doc:`SanitizerSpecialCaseList`, that can be used to suppress error reports
 in the specified source files or functions. Additionally, AddressSanitizer
 introduces ``global`` and ``type`` entity types that can be used to
 suppress error reports for out-of-bound access to globals with certain

clang/lib/CodeGen/CodeGenFunction.cpp

void CodeGenFunction::FinishFunction(SourceLocation EndLoc) {
   if (ShouldInstrumentFunction()) {
     if (CGM.getCodeGenOpts().InstrumentFunctions)
       CurFn->addFnAttr("instrument-function-exit", "__cyg_profile_func_exit");
     if (CGM.getCodeGenOpts().InstrumentFunctionsAfterInlining)
       CurFn->addFnAttr("instrument-function-exit-inlined",
                        "__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);
 
   // Reset the debug location to that of the simple 'return' expression, if any
   // rather than that of the end of the function's scope '}'.
   ApplyDebugLocation AL(*this, Loc);
   EmitFunctionEpilog(*CurFnInfo, EmitRetDbgLoc, EndLoc);
for (auto Attr : D->specific_attrs<NoSanitizeAttr>()) {
       if (Attr->hasCoverage())
         NoSanitizeCoverage = true;
     }
 
     if (NoSanitizeCoverage && CGM.getCodeGenOpts().hasSanitizeCoverage())
       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))
+    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))
     Fn->addFnAttr(llvm::Attribute::ShadowCallStack);
 
   // Apply fuzzing attribute to the function.
   if (SanOpts.hasOneOf(SanitizerKind::Fuzzer | SanitizerKind::FuzzerNoLink))
     Fn->addFnAttr(llvm::Attribute::OptForFuzzing);

clang/lib/CodeGen/SanitizerMetadata.cpp

void SanitizerMetadata::reportGlobalToASan(llvm::GlobalVariable *GV,
   std::string QualName;
   llvm::raw_string_ostream OS(QualName);
   D.printQualifiedName(OS);
 
   bool IsExcluded = false;
   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);
 }
 
 void SanitizerMetadata::disableSanitizerForGlobal(llvm::GlobalVariable *GV) {
   // For now, just make sure the global is not modified by the ASan
   // instrumentation.
   if (isAsanHwasanOrMemTag(CGM.getLangOpts().Sanitize))

clang/test/CodeGen/address-safety-attr-flavors.cpp

 __attribute__((no_sanitize("kernel-hwaddress"))) int NoSanitizeKernelHWAddress() {
   return 0;
 }
 // CHECK-NOASAN: {{Function Attrs: mustprogress noinline nounwind$}}
 // CHECK-ASAN: {{Function Attrs: mustprogress noinline nounwind sanitize_address$}}
 // 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$}}

clang/test/CodeGen/asan-globals.cpp

 // RUN: echo "int extra_global;" > %t.extra-source.cpp
 // RUN: echo "global:*blacklisted_global*" > %t.blacklist
 // RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=address -fsanitize-blacklist=%t.blacklist -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,ASAN
 // RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=kernel-address -fsanitize-blacklist=%t.blacklist -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,KASAN
 // The blacklist file uses regexps, so Windows path backslashes.
 // RUN: echo "src:%s" | sed -e 's/\\/\\\\/g' > %t.blacklist-src
 // RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=address -fsanitize-blacklist=%t.blacklist-src -emit-llvm -o - %s | FileCheck %s --check-prefix=BLACKLIST-SRC
 // RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=kernel-address -fsanitize-blacklist=%t.blacklist-src -emit-llvm -o - %s | FileCheck %s --check-prefix=BLACKLIST-SRC
 
 int global;
 int dyn_init_global = global;
 int __attribute__((no_sanitize("address"))) attributed_global;
+int __attribute__((disable_sanitizer_instrumentation)) disable_instrumentation_global;
 int blacklisted_global;
 
 int __attribute__((section("__DATA, __common"))) sectioned_global; // KASAN - ignore globals in a section
 extern "C" {
 int __special_global; // KASAN - ignore globals with __-prefix
 }
 
 void func() {

 
 /// If -fasynchronous-unwind-tables, set the module flag "uwtable". ctor/dtor
 /// will thus get the uwtable attribute.
 // RUN: %clang_cc1 -emit-llvm -fsanitize=address -funwind-tables=2 -o - %s | FileCheck %s --check-prefixes=UWTABLE
 // UWTABLE: define internal void @asan.module_dtor() #[[#ATTR:]] {
 // UWTABLE: attributes #[[#ATTR]] = { nounwind uwtable }
 // UWTABLE: ![[#]] = !{i32 7, !"uwtable", i32 1}
 
-// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], ![[BLACKLISTED_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]+]], ![[DISABLE_INSTR_GLOBAL:[0-9]+]], ![[BLACKLISTED_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]] = !{{{.*}}, null, null, i1 false, i1 true}
-// CHECK: ![[BLACKLISTED_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
+// 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: ![[BLACKLISTED_GLOBAL]] = !{{{.*@blacklisted_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 15, i32 50}
+// CHECK: ![[SECTIONED_GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 16, i32 50}
 // CHECK: ![[SPECIAL_GLOBAL]] = !{{{.*}} ![[SPECIAL_GLOBAL_LOC:[0-9]+]], !"__special_global", i1 false, i1 false}
-// CHECK: ![[SPECIAL_GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 17, i32 5}
+// CHECK: ![[SPECIAL_GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 18, i32 5}
 // CHECK: ![[STATIC_VAR]] = !{{{.*}} ![[STATIC_LOC:[0-9]+]], !"static_var", i1 false, i1 false}
-// CHECK: ![[STATIC_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 21, i32 14}
+// CHECK: ![[STATIC_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 22, i32 14}
 // CHECK: ![[LITERAL]] = !{{{.*}} ![[LITERAL_LOC:[0-9]+]], !"<string literal>", i1 false, i1 false}
-// CHECK: ![[LITERAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 22, i32 25}
+// CHECK: ![[LITERAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 23, i32 25}
 
-// BLACKLIST-SRC: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], ![[BLACKLISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], ![[SPECIAL_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
+// BLACKLIST-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]+]], ![[BLACKLISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], ![[SPECIAL_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
 // BLACKLIST-SRC: ![[EXTRA_GLOBAL]] = !{{{.*}} ![[EXTRA_GLOBAL_LOC:[0-9]+]], !"extra_global", i1 false, i1 false}
 // BLACKLIST-SRC: ![[EXTRA_GLOBAL_LOC]] = !{!"{{.*}}extra-source.cpp", i32 1, i32 5}
 // BLACKLIST-SRC: ![[GLOBAL]] = !{{{.*}} null, null, i1 false, i1 true}
 // BLACKLIST-SRC: ![[DYN_INIT_GLOBAL]] = !{{{.*}} null, null, i1 true, i1 true}
-// BLACKLIST-SRC: ![[ATTR_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
-// BLACKLIST-SRC: ![[BLACKLISTED_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
+// BLACKLIST-SRC: ![[ATTR_GLOBAL]] = !{{{.*@attributed_global.*}}, null, null, i1 false, i1 true}
+// BLACKLIST-SRC: ![[DISABLE_INSTR_GLOBAL]] = !{{{.*@disable_instrumentation_global.*}}, null, null, i1 false, i1 true}
+// BLACKLIST-SRC: ![[BLACKLISTED_GLOBAL]] = !{{{.*@blacklisted_global.*}}, null, null, i1 false, i1 true}
 // BLACKLIST-SRC: ![[SECTIONED_GLOBAL]] = !{{{.*}} null, null, i1 false, i1 true}
 // BLACKLIST-SRC: ![[SPECIAL_GLOBAL]] = !{{{.*}} null, null, i1 false, i1 true}
 // BLACKLIST-SRC: ![[STATIC_VAR]] = !{{{.*}} null, null, i1 false, i1 true}
 // BLACKLIST-SRC: ![[LITERAL]] = !{{{.*}} null, null, i1 false, i1 true}

llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

bool AddressSanitizer::instrumentFunction(Function &F,
   // This function needs to be called even if the function body is not
   // instrumented.
   if (maybeInsertAsanInitAtFunctionEntry(F))
     FunctionModified = true;
 
   // 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());
 
   FunctionStateRAII CleanupObj(this);
 
   FunctionModified |= maybeInsertDynamicShadowAtFunctionEntry(F);
 

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

+; 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
+