melver created this revision.
melver added reviewers: vitalybuka, morehouse, glider, dvyukov.
Herald added subscribers: dexonsmith, jdoerfert, pengfei, hiraditya.
Herald added a reviewer: aaron.ballman.
melver requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

We really ought to support no_sanitize("coverage") in line with other
sanitizers. This came up again in discussions on the Linux-kernel
mailing lists, because we currently do workarounds using objtool to
remove coverage instrumentation. Since that support is only on x86, to
continue support coverage instrumentation on other architectures, we
must support selectively disabling coverage instrumentation via function
attributes.

Unfortunately, for SanitizeCoverage, it has not been implemented as a
sanitizer via fsanitize= and associated options in Sanitizers.def, but
rolls its own option fsanitize-coverage. This meant that we never got
"automatic" no_sanitize attribute support.

Implement no_sanitize attribute support by special-casing the string
"coverage" in the NoSanitizeAttr implementation. To keep the feature as
unintrusive to existing IR generation as possible, define a new negative
function attribute NoSanitizeCoverage to propagate the information
through to the instrumentation pass.

Fixes: https://bugs.llvm.org/show_bug.cgi?id=49035


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102772

Files:
  clang/docs/SanitizerCoverage.rst
  clang/include/clang/Basic/Attr.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/sanitize-coverage.c
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp

Index: llvm/lib/Transforms/Utils/CodeExtractor.cpp
===================================================================
--- llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -954,6 +954,7 @@
       case Attribute::NonLazyBind:
       case Attribute::NoRedZone:
       case Attribute::NoUnwind:
+      case Attribute::NoSanitizeCoverage:
       case Attribute::NullPointerIsValid:
       case Attribute::OptForFuzzing:
       case Attribute::OptimizeNone:
Index: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
===================================================================
--- llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -621,6 +621,8 @@
     return;
   if (Blocklist && Blocklist->inSection("coverage", "fun", F.getName()))
     return;
+  if (F.hasFnAttribute(Attribute::NoSanitizeCoverage))
+    return;
   if (Options.CoverageType >= SanitizerCoverageOptions::SCK_Edge)
     SplitAllCriticalEdges(F, CriticalEdgeSplittingOptions().setIgnoreUnreachableDests());
   SmallVector<Instruction *, 8> IndirCalls;
Index: llvm/lib/IR/Verifier.cpp
===================================================================
--- llvm/lib/IR/Verifier.cpp
+++ llvm/lib/IR/Verifier.cpp
@@ -1661,6 +1661,7 @@
   case Attribute::NoCfCheck:
   case Attribute::NoUnwind:
   case Attribute::NoInline:
+  case Attribute::NoSanitizeCoverage:
   case Attribute::AlwaysInline:
   case Attribute::OptimizeForSize:
   case Attribute::StackProtect:
Index: llvm/lib/IR/Attributes.cpp
===================================================================
--- llvm/lib/IR/Attributes.cpp
+++ llvm/lib/IR/Attributes.cpp
@@ -442,6 +442,8 @@
     return "noprofile";
   if (hasAttribute(Attribute::NoUnwind))
     return "nounwind";
+  if (hasAttribute(Attribute::NoSanitizeCoverage))
+    return "no_sanitize_coverage";
   if (hasAttribute(Attribute::OptForFuzzing))
     return "optforfuzzing";
   if (hasAttribute(Attribute::OptimizeNone))
Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===================================================================
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -686,6 +686,8 @@
     return bitc::ATTR_KIND_NO_PROFILE;
   case Attribute::NoUnwind:
     return bitc::ATTR_KIND_NO_UNWIND;
+  case Attribute::NoSanitizeCoverage:
+    return bitc::ATTR_KIND_NO_SANITIZE_COVERAGE;
   case Attribute::NullPointerIsValid:
     return bitc::ATTR_KIND_NULL_POINTER_IS_VALID;
   case Attribute::OptForFuzzing:
Index: llvm/lib/Bitcode/Reader/BitcodeReader.cpp
===================================================================
--- llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -1474,6 +1474,8 @@
     return Attribute::NoCfCheck;
   case bitc::ATTR_KIND_NO_UNWIND:
     return Attribute::NoUnwind;
+  case bitc::ATTR_KIND_NO_SANITIZE_COVERAGE:
+    return Attribute::NoSanitizeCoverage;
   case bitc::ATTR_KIND_NULL_POINTER_IS_VALID:
     return Attribute::NullPointerIsValid;
   case bitc::ATTR_KIND_OPT_FOR_FUZZING:
Index: llvm/include/llvm/IR/Attributes.td
===================================================================
--- llvm/include/llvm/IR/Attributes.td
+++ llvm/include/llvm/IR/Attributes.td
@@ -154,6 +154,9 @@
 /// Function doesn't unwind stack.
 def NoUnwind : EnumAttr<"nounwind">;
 
+/// No SanitizeCoverage instrumentation.
+def NoSanitizeCoverage : EnumAttr<"no_sanitize_coverage">;
+
 /// Null pointer in address space zero is valid.
 def NullPointerIsValid : EnumAttr<"null_pointer_is_valid">;
 
Index: llvm/include/llvm/Bitcode/LLVMBitCodes.h
===================================================================
--- llvm/include/llvm/Bitcode/LLVMBitCodes.h
+++ llvm/include/llvm/Bitcode/LLVMBitCodes.h
@@ -666,6 +666,7 @@
   ATTR_KIND_NO_PROFILE = 73,
   ATTR_KIND_VSCALE_RANGE = 74,
   ATTR_KIND_SWIFT_ASYNC = 75,
+  ATTR_KIND_NO_SANITIZE_COVERAGE = 76,
 };
 
 enum ComdatSelectionKindCodes {
Index: clang/test/CodeGen/sanitize-coverage.c
===================================================================
--- clang/test/CodeGen/sanitize-coverage.c
+++ clang/test/CodeGen/sanitize-coverage.c
@@ -19,4 +19,52 @@
   if (n)
     x[n] = 42;
 }
+
+// CHECK-LABEL: define dso_local void @test_no_sanitize_coverage(
+__attribute__((no_sanitize("coverage"))) void test_no_sanitize_coverage(int n) {
+  // CHECK-NOT: call void @__sanitizer_cov_trace_pc
+  // CHECK-NOT: call void @__sanitizer_cov_trace_const_cmp
+  // ASAN-DAG: call void @__asan_report_store
+  // MSAN-DAG: call void @__msan_warning
+  // BOUNDS-DAG: call void @__ubsan_handle_out_of_bounds
+  // TSAN-DAG: call void @__tsan_func_entry
+  // UBSAN-DAG: call void @__ubsan_handle
+  if (n)
+    x[n] = 42;
+}
+
+
+// CHECK-LABEL: define dso_local void @test_no_sanitize_combined(
+__attribute__((no_sanitize("address", "memory", "thread", "bounds", "undefined", "coverage")))
+void test_no_sanitize_combined(int n) {
+  // CHECK-NOT: call void @__sanitizer_cov_trace_pc
+  // CHECK-NOT: call void @__sanitizer_cov_trace_const_cmp
+  // ASAN-NOT: call void @__asan_report_store
+  // MSAN-NOT: call void @__msan_warning
+  // BOUNDS-NOT: call void @__ubsan_handle_out_of_bounds
+  // TSAN-NOT: call void @__tsan_func_entry
+  // UBSAN-NOT: call void @__ubsan_handle
+  if (n)
+    x[n] = 42;
+}
+
+// CHECK-LABEL: define dso_local void @test_no_sanitize_separate(
+__attribute__((no_sanitize("address")))
+__attribute__((no_sanitize("memory")))
+__attribute__((no_sanitize("thread")))
+__attribute__((no_sanitize("bounds")))
+__attribute__((no_sanitize("undefined")))
+__attribute__((no_sanitize("coverage")))
+void test_no_sanitize_separate(int n) {
+  // CHECK-NOT: call void @__sanitizer_cov_trace_pc
+  // CHECK-NOT: call void @__sanitizer_cov_trace_const_cmp
+  // ASAN-NOT: call void @__asan_report_store
+  // MSAN-NOT: call void @__msan_warning
+  // BOUNDS-NOT: call void @__ubsan_handle_out_of_bounds
+  // TSAN-NOT: call void @__tsan_func_entry
+  // UBSAN-NOT: call void @__ubsan_handle
+  if (n)
+    x[n] = 42;
+}
+
 // CHECK-LABEL: declare void
Index: clang/lib/Sema/SemaDeclAttr.cpp
===================================================================
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -7274,7 +7274,8 @@
       return;
 
     if (parseSanitizerValue(SanitizerName, /*AllowGroups=*/true) ==
-        SanitizerMask())
+            SanitizerMask() &&
+        SanitizerName != "coverage")
       S.Diag(LiteralLoc, diag::warn_unknown_sanitizer_ignored) << SanitizerName;
     else if (isGlobalVar(D) && SanitizerName != "address")
       S.Diag(D->getLocation(), diag::err_attribute_wrong_decl_type)
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -734,8 +734,10 @@
   } while (0);
 
   if (D) {
-    // Apply the no_sanitize* attributes to SanOpts.
+    bool NoSanitizeCoverage = false;
+
     for (auto Attr : D->specific_attrs<NoSanitizeAttr>()) {
+      // Apply the no_sanitize* attributes to SanOpts.
       SanitizerMask mask = Attr->getMask();
       SanOpts.Mask &= ~mask;
       if (mask & SanitizerKind::Address)
@@ -746,7 +748,18 @@
         SanOpts.set(SanitizerKind::KernelHWAddress, false);
       if (mask & SanitizerKind::KernelHWAddress)
         SanOpts.set(SanitizerKind::HWAddress, false);
+
+      // SanitizeCoverage is not handled by SanOpts.
+      if (Attr->hasCoverage())
+        NoSanitizeCoverage = true;
     }
+
+    bool HaveSanitizeCoverage =
+        CGM.getCodeGenOpts().SanitizeCoverageType ||
+        CGM.getCodeGenOpts().SanitizeCoverageIndirectCalls ||
+        CGM.getCodeGenOpts().SanitizeCoverageTraceCmp;
+    if (NoSanitizeCoverage && HaveSanitizeCoverage)
+      Fn->addFnAttr(llvm::Attribute::NoSanitizeCoverage);
   }
 
   // Apply sanitizer attributes to the function.
Index: clang/include/clang/Basic/Attr.td
===================================================================
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -2897,6 +2897,14 @@
       }
       return Mask;
     }
+
+    bool hasCoverage() const {
+      for (auto SanitizerName : sanitizers()) {
+        if (SanitizerName == "coverage")
+          return true;
+      }
+      return false;
+    }
   }];
 }
 
Index: clang/docs/SanitizerCoverage.rst
===================================================================
--- clang/docs/SanitizerCoverage.rst
+++ clang/docs/SanitizerCoverage.rst
@@ -312,11 +312,17 @@
   // for every non-constant array index.
   void __sanitizer_cov_trace_gep(uintptr_t Idx);
 
-Partially disabling instrumentation
-===================================
+Disabling instrumentation with ``__attribute__((no_sanitize("coverage")))``
+===========================================================================
+
+It is possible to disable coverage instrumentation for select functions via the
+function attribute ``__attribute__((no_sanitize("coverage")))``.
+
+Disabling instrumentation without source modification
+=====================================================
 
 It is sometimes useful to tell SanitizerCoverage to instrument only a subset of the
-functions in your target.
+functions in your target without modifying source files.
 With ``-fsanitize-coverage-allowlist=allowlist.txt``
 and ``-fsanitize-coverage-blocklist=blocklist.txt``,
 you can specify such a subset through the combination of an allowlist and a blocklist.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to