[clang] [llvm] [SanitizerCoverage] Add gated tracing callbacks support to trace-cmp (PR #113227)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 22 10:24:27 PDT 2024
https://github.com/thetruestblue updated https://github.com/llvm/llvm-project/pull/113227
>From 53b316d74683064f2db88ec401f6c3018ee6896a Mon Sep 17 00:00:00 2001
From: thetruestblue <92476612+thetruestblue at users.noreply.github.com>
Date: Mon, 21 Oct 2024 11:06:02 -0700
Subject: [PATCH] [SanitizerCoverage] Add gated tracing callbacks support to
trace-cmp
The option -sanitizer-coverage-gated-trace-callbacks gates the invocation of the
trace-pc-guard callbacks based on the value of a global variable, which is stored
in a specific section.
In this commit, we extend this feature to trace-cmp and gate the cmp callbacks to
the same variable used for trace-pc-guard.
Update SanitizerCoverage doc with this flag.
rdar://135404160
Patch by: Andrea Fioraldi
---
clang/docs/SanitizerCoverage.rst | 14 +++
.../sanitize-coverage-gated-callbacks.c | 13 ++-
.../Instrumentation/SanitizerCoverage.cpp | 87 ++++++++++++-------
3 files changed, 83 insertions(+), 31 deletions(-)
diff --git a/clang/docs/SanitizerCoverage.rst b/clang/docs/SanitizerCoverage.rst
index 45ad03cb43774c..6ea1d14829005c 100644
--- a/clang/docs/SanitizerCoverage.rst
+++ b/clang/docs/SanitizerCoverage.rst
@@ -385,6 +385,20 @@ Users need to implement a single function to capture the CF table at startup:
// the collected control flow.
}
+Gated Trace Callbacks
+=====================
+
+Gate the invocation of the tracing callbacks with
+``-sanitizer-coverage-gated-trace-callbacks``.
+
+When this option is enabled, the instrumentation will not call into the
+runtime-provided callbacks for tracing, thus only incurring in a trivial
+branch without going through a function call.
+
+It is up to the runtime to toggle the value of the global variable in order to
+enable tracing.
+
+This option is only supported for trace-pc-guard and trace-cmp.
Disabling instrumentation with ``__attribute__((no_sanitize("coverage")))``
===========================================================================
diff --git a/clang/test/CodeGen/sanitize-coverage-gated-callbacks.c b/clang/test/CodeGen/sanitize-coverage-gated-callbacks.c
index 9a00d91d5ad086..e226591d80d077 100644
--- a/clang/test/CodeGen/sanitize-coverage-gated-callbacks.c
+++ b/clang/test/CodeGen/sanitize-coverage-gated-callbacks.c
@@ -1,5 +1,7 @@
// RUN: %clang %s -target arm64-apple-darwin -emit-llvm -S -fsanitize-coverage=trace-pc-guard -mllvm -sanitizer-coverage-gated-trace-callbacks=1 -o - | FileCheck %s --check-prefixes=CHECK,GATED
// RUN: %clang %s -target arm64-apple-darwin -emit-llvm -S -fsanitize-coverage=trace-pc-guard -mllvm -sanitizer-coverage-gated-trace-callbacks=0 -o - | FileCheck %s --check-prefixes=CHECK,PLAIN
+// RUN: %clang %s -target arm64-apple-darwin -emit-llvm -S -fsanitize-coverage=trace-pc-guard,trace-cmp -mllvm -sanitizer-coverage-gated-trace-callbacks=1 -o - | FileCheck %s --check-prefixes=CHECK,GATED,GATEDCMP
+// RUN: %clang %s -target arm64-apple-darwin -emit-llvm -S -fsanitize-coverage=trace-pc-guard,trace-cmp -mllvm -sanitizer-coverage-gated-trace-callbacks=0 -o - | FileCheck %s --check-prefixes=CHECK,PLAIN,PLAINCMP
// RUN: not %clang %s -target arm64-apple-darwin -emit-llvm -S -fsanitize-coverage=trace-pc -mllvm -sanitizer-coverage-gated-trace-callbacks=1 -o /dev/null 2>&1 | FileCheck %s --check-prefixes=INCOMPATIBLE
// RUN: not %clang %s -target arm64-apple-darwin -emit-llvm -S -fsanitize-coverage=inline-8bit-counters -mllvm -sanitizer-coverage-gated-trace-callbacks=1 -o /dev/null 2>&1 | FileCheck %s --check-prefixes=INCOMPATIBLE
// RUN: not %clang %s -target arm64-apple-darwin -emit-llvm -S -fsanitize-coverage=inline-bool-flag -mllvm -sanitizer-coverage-gated-trace-callbacks=1 -o /dev/null 2>&1 | FileCheck %s --check-prefixes=INCOMPATIBLE
@@ -9,7 +11,7 @@
// PLAIN-NOT: section "__DATA,__sancov_gate"
// Produce an error for all incompatible sanitizer coverage modes.
-// INCOMPATIBLE: error: 'sanitizer-coverage-gated-trace-callbacks' is only supported with trace-pc-guard
+// INCOMPATIBLE: error: 'sanitizer-coverage-gated-trace-callbacks' is only supported with trace-pc-guard or trace-cmp
int x[10];
@@ -23,6 +25,11 @@ void foo(int n, int m) {
// GATED-NEXT: br i1 [[CMP]], label %[[L_TRUE:.*]], label %[[L_FALSE:.*]], !prof [[WEIGHTS:!.+]]
// GATED: [[L_TRUE]]:
// GATED-NEXT: call void @__sanitizer_cov_trace_pc_guard
+ // COM: Check the trace-cmp instrumentation of the if (n) branch
+ // GATEDCMP: [[OPERAND:%.*]] = load i32, {{.*}}
+ // GATEDCMP-NEXT: br i1 [[CMP]], label %[[L_TRUE_1:.*]], label %[[L_FALSE_1:.*]]
+ // GATEDCMP: [[L_TRUE_1]]:
+ // GATEDCMP-NEXT: call void @__sanitizer_cov_trace_const_cmp4(i32 0, i32 [[OPERAND]])
// GATED: br i1 [[CMP]], label %[[L_TRUE_2:.*]], label %[[L_FALSE_2:.*]]
// GATED: [[L_TRUE_2]]:
// GATED-NEXT: call void @__sanitizer_cov_trace_pc_guard
@@ -33,10 +40,12 @@ void foo(int n, int m) {
// PLAIN-NOT: __sancov_should_track
// But we should still be emitting the calls to the callback.
// PLAIN: call void @__sanitizer_cov_trace_pc_guard
+ // PLAINCMP: [[OPERAND:%.*]] = load i32, {{.*}}
+ // PLAINCMP-NEXT: call void @__sanitizer_cov_trace_const_cmp4(i32 0, i32 [[OPERAND]])
if (n) {
x[n] = 42;
if (m) {
x[m] = 41;
}
}
-}
+}
\ No newline at end of file
diff --git a/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp b/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
index f7461127ec51cf..72e6a1c2f11ade 100644
--- a/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ b/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -159,8 +159,8 @@ static cl::opt<bool>
static cl::opt<bool> ClGatedCallbacks(
"sanitizer-coverage-gated-trace-callbacks",
- cl::desc("Gate the invocation of the tracing callbacks on a global "
- "variable. Currently only supported for trace-pc-guard."),
+ cl::desc("Gate the invocation of the tracing callbacks on a global variable"
+ ". Currently only supported for trace-pc-guard and trace-cmp."),
cl::Hidden, cl::init(false));
namespace {
@@ -235,7 +235,8 @@ class ModuleSanitizerCoverage {
void instrumentFunction(Function &F);
void InjectCoverageForIndirectCalls(Function &F,
ArrayRef<Instruction *> IndirCalls);
- void InjectTraceForCmp(Function &F, ArrayRef<Instruction *> CmpTraceTargets);
+ void InjectTraceForCmp(Function &F, ArrayRef<Instruction *> CmpTraceTargets,
+ Value *&FunctionGateCmp);
void InjectTraceForDiv(Function &F,
ArrayRef<BinaryOperator *> DivTraceTargets);
void InjectTraceForGep(Function &F,
@@ -243,14 +244,17 @@ class ModuleSanitizerCoverage {
void InjectTraceForLoadsAndStores(Function &F, ArrayRef<LoadInst *> Loads,
ArrayRef<StoreInst *> Stores);
void InjectTraceForSwitch(Function &F,
- ArrayRef<Instruction *> SwitchTraceTargets);
+ ArrayRef<Instruction *> SwitchTraceTargets,
+ Value *&FunctionGateCmp);
bool InjectCoverage(Function &F, ArrayRef<BasicBlock *> AllBlocks,
- bool IsLeafFunc = true);
+ Value *&FunctionGateCmp, bool IsLeafFunc = true);
GlobalVariable *CreateFunctionLocalArrayInSection(size_t NumElements,
Function &F, Type *Ty,
const char *Section);
GlobalVariable *CreatePCArray(Function &F, ArrayRef<BasicBlock *> AllBlocks);
void CreateFunctionLocalArrays(Function &F, ArrayRef<BasicBlock *> AllBlocks);
+ Instruction *CreateGateBranch(Function &F, Value *&FunctionGateCmp,
+ Instruction *I);
Value *CreateFunctionLocalGateCmp(IRBuilder<> &IRB);
void InjectCoverageAtBlock(Function &F, BasicBlock &BB, size_t Idx,
Value *&FunctionGateCmp, bool IsLeafFunc = true);
@@ -493,9 +497,9 @@ bool ModuleSanitizerCoverage::instrumentModule() {
SanCovLowestStack->setInitializer(Constant::getAllOnesValue(IntptrTy));
if (Options.GatedCallbacks) {
- if (!Options.TracePCGuard) {
+ if (!Options.TracePCGuard && !Options.TraceCmp) {
C->emitError(StringRef("'") + ClGatedCallbacks.ArgStr +
- "' is only supported with trace-pc-guard");
+ "' is only supported with trace-pc-guard or trace-cmp");
return true;
}
@@ -724,10 +728,11 @@ void ModuleSanitizerCoverage::instrumentFunction(Function &F) {
if (Options.CollectControlFlow)
createFunctionControlFlow(F);
- InjectCoverage(F, BlocksToInstrument, IsLeafFunc);
+ Value *FunctionGateCmp = nullptr;
+ InjectCoverage(F, BlocksToInstrument, FunctionGateCmp, IsLeafFunc);
InjectCoverageForIndirectCalls(F, IndirCalls);
- InjectTraceForCmp(F, CmpTraceTargets);
- InjectTraceForSwitch(F, SwitchTraceTargets);
+ InjectTraceForCmp(F, CmpTraceTargets, FunctionGateCmp);
+ InjectTraceForSwitch(F, SwitchTraceTargets, FunctionGateCmp);
InjectTraceForDiv(F, DivTraceTargets);
InjectTraceForGep(F, GepTraceTargets);
InjectTraceForLoadsAndStores(F, Loads, Stores);
@@ -816,12 +821,30 @@ Value *ModuleSanitizerCoverage::CreateFunctionLocalGateCmp(IRBuilder<> &IRB) {
return Cmp;
}
+Instruction *ModuleSanitizerCoverage::CreateGateBranch(Function &F,
+ Value *&FunctionGateCmp,
+ Instruction *IP) {
+ if (!FunctionGateCmp) {
+ // Create this in the entry block
+ BasicBlock &BB = F.getEntryBlock();
+ BasicBlock::iterator IP = BB.getFirstInsertionPt();
+ IP = PrepareToSplitEntryBlock(BB, IP);
+ IRBuilder<> EntryIRB(&*IP);
+ FunctionGateCmp = CreateFunctionLocalGateCmp(EntryIRB);
+ }
+ // Set the branch weights in order to minimize the price paid when the
+ // gate is turned off, allowing the default enablement of this
+ // instrumentation with as little of a performance cost as possible
+ auto Weights = MDBuilder(*C).createBranchWeights(1, 100000);
+ return SplitBlockAndInsertIfThen(FunctionGateCmp, IP, false, Weights);
+}
+
bool ModuleSanitizerCoverage::InjectCoverage(Function &F,
ArrayRef<BasicBlock *> AllBlocks,
+ Value *&FunctionGateCmp,
bool IsLeafFunc) {
if (AllBlocks.empty()) return false;
CreateFunctionLocalArrays(F, AllBlocks);
- Value *FunctionGateCmp = nullptr;
for (size_t i = 0, N = AllBlocks.size(); i < N; i++)
InjectCoverageAtBlock(F, *AllBlocks[i], i, FunctionGateCmp, IsLeafFunc);
return true;
@@ -855,7 +878,8 @@ void ModuleSanitizerCoverage::InjectCoverageForIndirectCalls(
// {NumCases, ValueSizeInBits, Case0Value, Case1Value, Case2Value, ... })
void ModuleSanitizerCoverage::InjectTraceForSwitch(
- Function &, ArrayRef<Instruction *> SwitchTraceTargets) {
+ Function &F, ArrayRef<Instruction *> SwitchTraceTargets,
+ Value *&FunctionGateCmp) {
for (auto *I : SwitchTraceTargets) {
if (SwitchInst *SI = dyn_cast<SwitchInst>(I)) {
InstrumentationIRBuilder IRB(I);
@@ -886,7 +910,13 @@ void ModuleSanitizerCoverage::InjectTraceForSwitch(
*CurModule, ArrayOfInt64Ty, false, GlobalVariable::InternalLinkage,
ConstantArray::get(ArrayOfInt64Ty, Initializers),
"__sancov_gen_cov_switch_values");
- IRB.CreateCall(SanCovTraceSwitchFunction, {Cond, GV});
+ if (Options.GatedCallbacks) {
+ auto GateBranch = CreateGateBranch(F, FunctionGateCmp, I);
+ IRBuilder<> GateIRB(GateBranch);
+ GateIRB.CreateCall(SanCovTraceSwitchFunction, {Cond, GV});
+ } else {
+ IRB.CreateCall(SanCovTraceSwitchFunction, {Cond, GV});
+ }
}
}
}
@@ -950,7 +980,8 @@ void ModuleSanitizerCoverage::InjectTraceForLoadsAndStores(
}
void ModuleSanitizerCoverage::InjectTraceForCmp(
- Function &, ArrayRef<Instruction *> CmpTraceTargets) {
+ Function &F, ArrayRef<Instruction *> CmpTraceTargets,
+ Value *&FunctionGateCmp) {
for (auto *I : CmpTraceTargets) {
if (ICmpInst *ICMP = dyn_cast<ICmpInst>(I)) {
InstrumentationIRBuilder IRB(ICMP);
@@ -978,8 +1009,15 @@ void ModuleSanitizerCoverage::InjectTraceForCmp(
}
auto Ty = Type::getIntNTy(*C, TypeSize);
- IRB.CreateCall(CallbackFunc, {IRB.CreateIntCast(A0, Ty, true),
- IRB.CreateIntCast(A1, Ty, true)});
+ if (Options.GatedCallbacks) {
+ auto GateBranch = CreateGateBranch(F, FunctionGateCmp, I);
+ IRBuilder<> GateIRB(GateBranch);
+ GateIRB.CreateCall(CallbackFunc, {GateIRB.CreateIntCast(A0, Ty, true),
+ GateIRB.CreateIntCast(A1, Ty, true)});
+ } else {
+ IRB.CreateCall(CallbackFunc, {IRB.CreateIntCast(A0, Ty, true),
+ IRB.CreateIntCast(A1, Ty, true)});
+ }
}
}
}
@@ -1013,19 +1051,10 @@ void ModuleSanitizerCoverage::InjectCoverageAtBlock(Function &F, BasicBlock &BB,
ConstantInt::get(IntptrTy, Idx * 4)),
PtrTy);
if (Options.GatedCallbacks) {
- if (!FunctionGateCmp) {
- // Create this in the entry block
- assert(IsEntryBB);
- FunctionGateCmp = CreateFunctionLocalGateCmp(IRB);
- }
- // Set the branch weights in order to minimize the price paid when the
- // gate is turned off, allowing the default enablement of this
- // instrumentation with as little of a performance cost as possible
- auto Weights = MDBuilder(*C).createBranchWeights(1, 100000);
- auto ThenTerm =
- SplitBlockAndInsertIfThen(FunctionGateCmp, &*IP, false, Weights);
- IRBuilder<> ThenIRB(ThenTerm);
- ThenIRB.CreateCall(SanCovTracePCGuard, GuardPtr)->setCannotMerge();
+ Instruction *I = &*IP;
+ auto GateBranch = CreateGateBranch(F, FunctionGateCmp, I);
+ IRBuilder<> GateIRB(GateBranch);
+ GateIRB.CreateCall(SanCovTracePCGuard, GuardPtr)->setCannotMerge();
} else {
IRB.CreateCall(SanCovTracePCGuard, GuardPtr)->setCannotMerge();
}
More information about the cfe-commits
mailing list