[SanitizeCoverage] Add support for NoSanitizeCoverage function attribute

[melver]

Summary

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

[aaron.ballman]

clang/docs/SanitizerCoverage.rst

319

I would expect to see this documentation also in AttrDocs.td within the NoSanitizeDocs.

clang/include/clang/Basic/Attr.td

2902–2906

[melver]

Address Aaron's comments

[melver]

Test that always_inline works as expected with no_sanitize("coverage")

[aaron.ballman]

The clang parts look good to me, but someone else should do the final sign-off.

clang/lib/Sema/SemaDeclAttr.cpp

7276–7279

The formatting looks rather off here to my eyes, but if this is what clang-format produces for you, then I'm fine with it.

[melver]

clang/lib/Sema/SemaDeclAttr.cpp

7276–7279

Yeah, I had something else here, but then clang-format suggested this and I don't like it either.

Could remove the /*AllowGroups=*/ bit, for the sake of formatting better?

[aaron.ballman]

clang/lib/Sema/SemaDeclAttr.cpp

7276–7279

I'd rather have the comment + ugly formatting than pretty formatting + mysterious boolean literal, so I'd say let's just leave it as-is.

[melver]

Rest of long-tail of IR changes related to introducing a new attributes...

[morehouse]

LGTM

[vitalybuka]

clang/lib/CodeGen/CodeGenFunction.cpp

757–760

consider adding method into CodeGenOptions and replace here and and the rest
ideally in a separate patch

clang/test/CodeGen/sanitize-coverage.c

23–87

you are adding tests for unrelated code.
Could you please land is as a separate NFC patch first?

llvm/lib/AsmParser/LLLexer.cpp

674 ↗

(On Diff #346577)

looking at the rest, seems "nosanitize_coverage" follows pattern better :)

[melver]

clang/test/CodeGen/sanitize-coverage.c

23–87

Which bits in particular? Just the

static inline __attribute__((__always_inline__)) void always_inlined_fn(int n) {
  if (n)
    x[n] = 42;
}
// CHECK-LABEL: define dso_local void @test_always_inline(
void test_always_inline(int n) {
  // CHECK-DAG: call void @__sanitizer_cov_trace_pc
  // CHECK-DAG: call void @__sanitizer_cov_trace_const_cmp
  always_inlined_fn(n);
}

part?

[melver]

• Refactor test and SanitizeCoverage* checking.
• s/no_sanitize_coverage/nosanitize_coverage/

Thanks!

[melver]

clang/lib/CodeGen/CodeGenFunction.cpp

757–760

https://reviews.llvm.org/D102927

clang/test/CodeGen/sanitize-coverage.c

23–87

https://reviews.llvm.org/D102929

llvm/lib/AsmParser/LLLexer.cpp

674 ↗

(On Diff #346577)

I wasn't sure myself, it's a bit inconsistent with "no_sanitize", but I've changed it. ;-)

[melver]

Please have a look at the other 2 patches which are now a dependency for this. Once you're happy with all 3 I'll push them all together.
Thanks!

clang/docs/SanitizerCoverage.rst

.. code-block:: c++
   // Val is the second argument of division.
   void __sanitizer_cov_trace_div4(uint32_t Val);
   void __sanitizer_cov_trace_div8(uint64_t Val);
 
   // Called before a GetElemementPtr (GEP) instruction
   // 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.
 
 SanitizerCoverage will only instrument functions that satisfy two conditions.
 First, the function should belong to a source file with a path that is both allowlisted
 and not blocklisted.
 Second, the function should have a mangled name that is both allowlisted and not blocklisted.

clang/include/clang/Basic/Attr.td

SanitizerMask getMask() const {
       SanitizerMask Mask;
       for (auto SanitizerName : sanitizers()) {
         SanitizerMask ParsedMask =
             parseSanitizerValue(SanitizerName, /*AllowGroups=*/true);
         Mask |= expandSanitizerGroups(ParsedMask);
       }
       return Mask;
     }
+
+    bool hasCoverage() const {
+      return llvm::is_contained(sanitizers(), "coverage");
+    }
   }];
 }
 
 // Attributes to disable a specific sanitizer. No new sanitizers should be added
 // to this list; the no_sanitize attribute should be extended instead.
 def NoSanitizeSpecific : InheritableAttr {
   let Spellings = [GCC<"no_address_safety_analysis">,
                    GCC<"no_sanitize_address">,
                    GCC<"no_sanitize_thread">,
                    Clang<"no_sanitize_memory">];
   let Subjects = SubjectList<[Function, GlobalVar], ErrorDiag>;

clang/include/clang/Basic/AttrDocs.td

``unknown``, ``consumed``, or ``unconsumed``.
   }];
 }
 
 def NoSanitizeDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
 Use the ``no_sanitize`` attribute on a function or a global variable
 declaration to specify that a particular instrumentation or set of
-instrumentations should not be applied. The attribute takes a list of
-string literals, which have the same meaning as values accepted by the
-``-fno-sanitize=`` flag. For example,
-``__attribute__((no_sanitize("address", "thread")))`` specifies that
-AddressSanitizer and ThreadSanitizer should not be applied to the
-function or variable.
+instrumentations should not be applied.
+
+The attribute takes a list of string literals with the following accepted
+values:
+* all values accepted by ``-fno-sanitize=``;
+* ``coverage``, to disable SanitizerCoverage instrumentation.
+
+For example, ``__attribute__((no_sanitize("address", "thread")))`` specifies
+that AddressSanitizer and ThreadSanitizer should not be applied to the function
+or variable. Using ``__attribute__((no_sanitize("coverage")))`` specifies that
+SanitizerCoverage should not be applied to the function.
 
 See :ref:`Controlling Code Generation <controlling-code-generation>` for a
 full list of supported sanitizer flags.
   }];
 }
 
 def NoSanitizeAddressDocs : Documentation {
   let Category = DocCatFunction;

clang/lib/CodeGen/CodeGenFunction.cpp

if (SanOpts.has(SanitizerKind::ID))                                          \
     if (CGM.isInNoSanitizeList(SanitizerKind::ID, Fn, Loc))                    \
       SanOpts.set(SanitizerKind::ID, false);
 
 #include "clang/Basic/Sanitizers.def"
 #undef SANITIZER
   } 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)
         SanOpts.set(SanitizerKind::KernelAddress, false);
       if (mask & SanitizerKind::KernelAddress)
         SanOpts.set(SanitizerKind::Address, false);
       if (mask & SanitizerKind::HWAddress)
         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.
   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))

clang/lib/Sema/SemaDeclAttr.cpp

static void handleNoSanitizeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
 
   for (unsigned I = 0, E = AL.getNumArgs(); I != E; ++I) {
     StringRef SanitizerName;
     SourceLocation LiteralLoc;
 
     if (!S.checkStringLiteralArgumentAttr(AL, I, SanitizerName, &LiteralLoc))
       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)
           << AL << ExpectedFunctionOrMethod;
     Sanitizers.push_back(SanitizerName);
   }
 
   D->addAttr(::new (S.Context) NoSanitizeAttr(S.Context, AL, Sanitizers.data(),
                                               Sanitizers.size()));

clang/test/CodeGen/sanitize-coverage.c

void foo(int n) {
   // 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_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

llvm/include/llvm/Bitcode/LLVMBitCodes.h

enum AttributeKindCodes {
   ATTR_KIND_NOUNDEF = 68,
   ATTR_KIND_BYREF = 69,
   ATTR_KIND_MUSTPROGRESS = 70,
   ATTR_KIND_NO_CALLBACK = 71,
   ATTR_KIND_HOT = 72,
   ATTR_KIND_NO_PROFILE = 73,
   ATTR_KIND_VSCALE_RANGE = 74,
   ATTR_KIND_SWIFT_ASYNC = 75,
+  ATTR_KIND_NO_SANITIZE_COVERAGE = 76,
 };
 
 enum ComdatSelectionKindCodes {
   COMDAT_SELECTION_KIND_ANY = 1,
   COMDAT_SELECTION_KIND_EXACT_MATCH = 2,
   COMDAT_SELECTION_KIND_LARGEST = 3,
   COMDAT_SELECTION_KIND_NO_DUPLICATES = 4,
   COMDAT_SELECTION_KIND_SAME_SIZE = 5,

llvm/include/llvm/IR/Attributes.td

 def NoCfCheck : EnumAttr<"nocf_check">;
 
 /// Function should not be instrumented.
 def NoProfile : EnumAttr<"noprofile">;
 
 /// 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">;
 
 /// Select optimizations for best fuzzing signal.
 def OptForFuzzing : EnumAttr<"optforfuzzing">;
 
 /// opt_size.
 def OptimizeForSize : EnumAttr<"optsize">;

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

static Attribute::AttrKind getAttrFromCode(uint64_t Code) {
   case bitc::ATTR_KIND_NO_RETURN:
     return Attribute::NoReturn;
   case bitc::ATTR_KIND_NOSYNC:
     return Attribute::NoSync;
   case bitc::ATTR_KIND_NOCF_CHECK:
     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:
     return Attribute::OptForFuzzing;
   case bitc::ATTR_KIND_OPTIMIZE_FOR_SIZE:
     return Attribute::OptimizeForSize;
   case bitc::ATTR_KIND_OPTIMIZE_NONE:
     return Attribute::OptimizeNone;

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

static uint64_t getAttrKindEncoding(Attribute::AttrKind Kind) {
   case Attribute::NoSync:
     return bitc::ATTR_KIND_NOSYNC;
   case Attribute::NoCfCheck:
     return bitc::ATTR_KIND_NOCF_CHECK;
   case Attribute::NoProfile:
     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:
     return bitc::ATTR_KIND_OPT_FOR_FUZZING;
   case Attribute::OptimizeForSize:
     return bitc::ATTR_KIND_OPTIMIZE_FOR_SIZE;
   case Attribute::OptimizeNone:
     return bitc::ATTR_KIND_OPTIMIZE_NONE;

llvm/lib/IR/Attributes.cpp

std::string Attribute::getAsString(bool InAttrGrp) const {
   if (hasAttribute(Attribute::NoCfCheck))
     return "nocf_check";
   if (hasAttribute(Attribute::NoRecurse))
     return "norecurse";
   if (hasAttribute(Attribute::NoProfile))
     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))
     return "optnone";
   if (hasAttribute(Attribute::OptimizeForSize))
     return "optsize";
   if (hasAttribute(Attribute::ReadNone))
     return "readnone";

llvm/lib/IR/Verifier.cpp

static bool isFuncOnlyAttr(Attribute::AttrKind Kind) {
   case Attribute::NoMerge:
   case Attribute::NoReturn:
   case Attribute::NoSync:
   case Attribute::WillReturn:
   case Attribute::NoCallback:
   case Attribute::NoCfCheck:
   case Attribute::NoUnwind:
   case Attribute::NoInline:
+  case Attribute::NoSanitizeCoverage:
   case Attribute::AlwaysInline:
   case Attribute::OptimizeForSize:
   case Attribute::StackProtect:
   case Attribute::StackProtectReq:
   case Attribute::StackProtectStrong:
   case Attribute::SafeStack:
   case Attribute::ShadowCallStack:
   case Attribute::NoRedZone:

llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp

void ModuleSanitizerCoverage::instrumentFunction(
   // FIXME: Remove this when SEH no longer uses landingpad pattern matching.
   if (F.hasPersonalityFn() &&
       isAsynchronousEHPersonality(classifyEHPersonality(F.getPersonalityFn())))
     return;
   if (Allowlist && !Allowlist->inSection("coverage", "fun", F.getName()))
     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;
   SmallVector<BasicBlock *, 16> BlocksToInstrument;
   SmallVector<Instruction *, 8> CmpTraceTargets;
   SmallVector<Instruction *, 8> SwitchTraceTargets;
   SmallVector<BinaryOperator *, 8> DivTraceTargets;
   SmallVector<GetElementPtrInst *, 8> GepTraceTargets;

llvm/lib/Transforms/Utils/CodeExtractor.cpp

if (Attr.isStringAttribute()) {
       case Attribute::NoCallback:
       case Attribute::NoDuplicate:
       case Attribute::NoFree:
       case Attribute::NoImplicitFloat:
       case Attribute::NoInline:
       case Attribute::NonLazyBind:
       case Attribute::NoRedZone:
       case Attribute::NoUnwind:
+      case Attribute::NoSanitizeCoverage:
       case Attribute::NullPointerIsValid:
       case Attribute::OptForFuzzing:
       case Attribute::OptimizeNone:
       case Attribute::OptimizeForSize:
       case Attribute::SafeStack:
       case Attribute::ShadowCallStack:
       case Attribute::SanitizeAddress:
       case Attribute::SanitizeMemory: