[sancov] Introduce optional callback for stack-depth tracking #138323
+408
−15
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-clang-codegen Author: Kees Cook (kees) ChangesNormally -fsanitize-coverage=stack-depth inserts inline arithmetic to update thread_local __sancov_lowest_stack. To support stack depth tracking in the Linux kernel, which does not implement traditional thread_local storage, provide the option to call a function instead. This matches the existing "stackleak" implementation that is supported in Linux via a GCC plugin. To make this coverage more performant, a minimum estimated stack depth can be chosen to enable the callback mode, skipping instrumentation of functions with smaller stacks. With -fsanitize-coverage-stack-depth-callback-min set greater than 0, the __sanitize_cov_stack_depth() callback will be injected when the estimated stack depth is greater than or equal to the given minimum. Full diff: https://github.com/llvm/llvm-project/pull/138323.diff 7 Files Affected:
diff --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def
index 927972015c3dc..452b1e325afb2 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -305,6 +305,7 @@ CODEGENOPT(SanitizeCoveragePCTable, 1, 0) ///< Create a PC Table.
CODEGENOPT(SanitizeCoverageControlFlow, 1, 0) ///< Collect control flow
CODEGENOPT(SanitizeCoverageNoPrune, 1, 0) ///< Disable coverage pruning.
CODEGENOPT(SanitizeCoverageStackDepth, 1, 0) ///< Enable max stack depth tracing
+VALUE_CODEGENOPT(SanitizeCoverageStackDepthCallbackMin , 32, 0) ///< Enable stack depth tracing callbacks.
CODEGENOPT(SanitizeCoverageTraceLoads, 1, 0) ///< Enable tracing of loads.
CODEGENOPT(SanitizeCoverageTraceStores, 1, 0) ///< Enable tracing of stores.
CODEGENOPT(SanitizeBinaryMetadataCovered, 1, 0) ///< Emit PCs for covered functions.
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 561b0498c549c..5823899bdb558 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2361,6 +2361,11 @@ def fsanitize_coverage_ignorelist : Joined<["-"], "fsanitize-coverage-ignorelist
HelpText<"Disable sanitizer coverage instrumentation for modules and functions "
"that match the provided special case list, even the allowed ones">,
MarshallingInfoStringVector<CodeGenOpts<"SanitizeCoverageIgnorelistFiles">>;
+def fsanitize_coverage_stack_depth_callback_min_EQ :
+ Joined<["-"], "fsanitize-coverage-stack-depth-callback-min=">,
+ Group<f_clang_Group>, MetaVarName<"<M>">,
+ HelpText<"Use callback for max stack depth tracing with minimum stack depth M">,
+ MarshallingInfoInt<CodeGenOpts<"SanitizeCoverageStackDepthCallbackMin">>;
def fexperimental_sanitize_metadata_EQ : CommaJoined<["-"], "fexperimental-sanitize-metadata=">,
Group<f_Group>,
HelpText<"Specify the type of metadata to emit for binary analysis sanitizers">;
diff --git a/clang/include/clang/Driver/SanitizerArgs.h b/clang/include/clang/Driver/SanitizerArgs.h
index 528e3b400f3dc..1213cd8dcd3b6 100644
--- a/clang/include/clang/Driver/SanitizerArgs.h
+++ b/clang/include/clang/Driver/SanitizerArgs.h
@@ -34,6 +34,7 @@ class SanitizerArgs {
std::vector<std::string> CoverageIgnorelistFiles;
std::vector<std::string> BinaryMetadataIgnorelistFiles;
int CoverageFeatures = 0;
+ int StackDepthCallbackMin = 0;
int BinaryMetadataFeatures = 0;
int OverflowPatternExclusions = 0;
int MsanTrackOrigins = 0;
diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp
index c9ceb49ce5ceb..42c59377688b2 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -255,6 +255,7 @@ getSancovOptsFromCGOpts(const CodeGenOptions &CGOpts) {
Opts.InlineBoolFlag = CGOpts.SanitizeCoverageInlineBoolFlag;
Opts.PCTable = CGOpts.SanitizeCoveragePCTable;
Opts.StackDepth = CGOpts.SanitizeCoverageStackDepth;
+ Opts.StackDepthCallbackMin = CGOpts.SanitizeCoverageStackDepthCallbackMin;
Opts.TraceLoads = CGOpts.SanitizeCoverageTraceLoads;
Opts.TraceStores = CGOpts.SanitizeCoverageTraceStores;
Opts.CollectControlFlow = CGOpts.SanitizeCoverageControlFlow;
diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp
index ff08bffdbde1f..414f68e4423e8 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -751,6 +751,16 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
options::OPT_fno_sanitize_ignorelist,
clang::diag::err_drv_malformed_sanitizer_ignorelist, DiagnoseErrors);
+ // Verify that -fsanitize-coverage-stack-depth-callback-min is >= 0.
+ if (Arg *A = Args.getLastArg(options::OPT_fsanitize_coverage_stack_depth_callback_min_EQ)) {
+ StringRef S = A->getValue();
+ if (S.getAsInteger(0, StackDepthCallbackMin) || StackDepthCallbackMin < 0) {
+ if (DiagnoseErrors)
+ D.Diag(clang::diag::err_drv_invalid_value)
+ << A->getAsString(Args) << S;
+ }
+ }
+
// Parse -f[no-]sanitize-memory-track-origins[=level] options.
if (AllAddedKinds & SanitizerKind::Memory) {
if (Arg *A =
@@ -1269,6 +1279,10 @@ void SanitizerArgs::addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args,
addSpecialCaseListOpt(Args, CmdArgs, "-fsanitize-coverage-ignorelist=",
CoverageIgnorelistFiles);
+ if (StackDepthCallbackMin)
+ CmdArgs.push_back(Args.MakeArgString("-fsanitize-coverage-stack-depth-callback-min=" +
+ Twine(StackDepthCallbackMin)));
+
if (!GPUSanitize) {
// Translate available BinaryMetadataFeatures to corresponding clang-cc1
// flags. Does not depend on any other sanitizers. Unsupported on GPUs.
diff --git a/llvm/include/llvm/Transforms/Utils/Instrumentation.h b/llvm/include/llvm/Transforms/Utils/Instrumentation.h
index 0e2c0d9bfa605..0b2ccf6180e1c 100644
--- a/llvm/include/llvm/Transforms/Utils/Instrumentation.h
+++ b/llvm/include/llvm/Transforms/Utils/Instrumentation.h
@@ -158,6 +158,7 @@ struct SanitizerCoverageOptions {
bool PCTable = false;
bool NoPrune = false;
bool StackDepth = false;
+ int StackDepthCallbackMin = 0;
bool TraceLoads = false;
bool TraceStores = false;
bool CollectControlFlow = false;
diff --git a/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp b/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
index e52269637b92d..b8212b42b9d0c 100644
--- a/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ b/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -33,6 +33,7 @@
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/SpecialCaseList.h"
#include "llvm/Support/VirtualFileSystem.h"
+#include "llvm/Support/raw_ostream.h"
#include "llvm/TargetParser/Triple.h"
#include "llvm/Transforms/Utils/BasicBlockUtils.h"
#include "llvm/Transforms/Utils/ModuleUtils.h"
@@ -86,6 +87,7 @@ const char SanCovPCsSectionName[] = "sancov_pcs";
const char SanCovCFsSectionName[] = "sancov_cfs";
const char SanCovCallbackGateSectionName[] = "sancov_gate";
+const char SanCovStackDepthCallbackName[] = "__sanitizer_cov_stack_depth";
const char SanCovLowestStackName[] = "__sancov_lowest_stack";
const char SanCovCallbackGateName[] = "__sancov_should_track";
@@ -152,6 +154,12 @@ static cl::opt<bool> ClStackDepth("sanitizer-coverage-stack-depth",
cl::desc("max stack depth tracing"),
cl::Hidden);
+static cl::opt<int> ClStackDepthCallbackMin(
+ "sanitizer-coverage-stack-depth-callback-min",
+ cl::desc("max stack depth tracing should use callback and only when "
+ "stack depth more than specified"),
+ cl::Hidden);
+
static cl::opt<bool>
ClCollectCF("sanitizer-coverage-control-flow",
cl::desc("collect control flow for each function"), cl::Hidden);
@@ -202,6 +210,8 @@ SanitizerCoverageOptions OverrideFromCL(SanitizerCoverageOptions Options) {
Options.PCTable |= ClCreatePCTable;
Options.NoPrune |= !ClPruneBlocks;
Options.StackDepth |= ClStackDepth;
+ Options.StackDepthCallbackMin = std::max(Options.StackDepthCallbackMin,
+ ClStackDepthCallbackMin.getValue());
Options.TraceLoads |= ClLoadTracing;
Options.TraceStores |= ClStoreTracing;
Options.GatedCallbacks |= ClGatedCallbacks;
@@ -271,6 +281,7 @@ class ModuleSanitizerCoverage {
DomTreeCallback DTCallback;
PostDomTreeCallback PDTCallback;
+ FunctionCallee SanCovStackDepthCallback;
FunctionCallee SanCovTracePCIndir;
FunctionCallee SanCovTracePC, SanCovTracePCGuard;
std::array<FunctionCallee, 4> SanCovTraceCmpFunction;
@@ -514,6 +525,8 @@ bool ModuleSanitizerCoverage::instrumentModule() {
SanCovTracePCGuard =
M.getOrInsertFunction(SanCovTracePCGuardName, VoidTy, PtrTy);
+ SanCovStackDepthCallback = M.getOrInsertFunction(SanCovStackDepthCallbackName, VoidTy);
+
for (auto &F : M)
instrumentFunction(F);
@@ -1078,22 +1091,44 @@ void ModuleSanitizerCoverage::InjectCoverageAtBlock(Function &F, BasicBlock &BB,
Store->setNoSanitizeMetadata();
}
if (Options.StackDepth && IsEntryBB && !IsLeafFunc) {
- // Check stack depth. If it's the deepest so far, record it.
Module *M = F.getParent();
- auto FrameAddrPtr = IRB.CreateIntrinsic(
- Intrinsic::frameaddress,
- IRB.getPtrTy(M->getDataLayout().getAllocaAddrSpace()),
- {Constant::getNullValue(Int32Ty)});
- auto FrameAddrInt = IRB.CreatePtrToInt(FrameAddrPtr, IntptrTy);
- auto LowestStack = IRB.CreateLoad(IntptrTy, SanCovLowestStack);
- auto IsStackLower = IRB.CreateICmpULT(FrameAddrInt, LowestStack);
- auto ThenTerm = SplitBlockAndInsertIfThen(
- IsStackLower, &*IP, false,
- MDBuilder(IRB.getContext()).createUnlikelyBranchWeights());
- IRBuilder<> ThenIRB(ThenTerm);
- auto Store = ThenIRB.CreateStore(FrameAddrInt, SanCovLowestStack);
- LowestStack->setNoSanitizeMetadata();
- Store->setNoSanitizeMetadata();
+ if (Options.StackDepthCallbackMin) {
+ // In callback mode, only add call when stack depth reaches minimum.
+ const DataLayout &DL = M->getDataLayout();
+ uint32_t EstimatedStackSize = 0;
+
+ // Make an estimate on the stack usage.
+ for (auto &I : F.getEntryBlock()) {
+ if (auto *AI = dyn_cast<AllocaInst>(&I)) {
+ if (AI->isStaticAlloca()) {
+ uint32_t TypeSize = DL.getTypeAllocSize(AI->getAllocatedType());
+ EstimatedStackSize += TypeSize;
+ } else {
+ // Over-estimate dynamic sizes.
+ EstimatedStackSize += 4096;
+ }
+ }
+ }
+
+ if (EstimatedStackSize >= Options.StackDepthCallbackMin)
+ IRB.CreateCall(SanCovStackDepthCallback)->setCannotMerge();
+ } else {
+ // Check stack depth. If it's the deepest so far, record it.
+ auto FrameAddrPtr = IRB.CreateIntrinsic(
+ Intrinsic::frameaddress,
+ IRB.getPtrTy(M->getDataLayout().getAllocaAddrSpace()),
+ {Constant::getNullValue(Int32Ty)});
+ auto FrameAddrInt = IRB.CreatePtrToInt(FrameAddrPtr, IntptrTy);
+ auto LowestStack = IRB.CreateLoad(IntptrTy, SanCovLowestStack);
+ auto IsStackLower = IRB.CreateICmpULT(FrameAddrInt, LowestStack);
+ auto ThenTerm = SplitBlockAndInsertIfThen(
+ IsStackLower, &*IP, false,
+ MDBuilder(IRB.getContext()).createUnlikelyBranchWeights());
+ IRBuilder<> ThenIRB(ThenTerm);
+ auto Store = ThenIRB.CreateStore(FrameAddrInt, SanCovLowestStack);
+ LowestStack->setNoSanitizeMetadata();
+ Store->setNoSanitizeMetadata();
+ }
}
}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Whoops, yes, I've fixed these now. |
intel-lab-lkp
pushed a commit
to intel-lab-lkp/linux
that referenced
this pull request
May 2, 2025
Wire up stackleak to Clang's proposed[1] stack depth tracking callback option. While __noinstr already contained __no_sanitize_coverage, it was still needed for __init and __head section markings. This is needed to make sure the callback is not executed in unsupported contexts. Link: llvm/llvm-project#138323 [1] Signed-off-by: Kees Cook <kees@kernel.org>
lougovsk
pushed a commit
to lougovsk/linux
that referenced
this pull request
May 2, 2025
Wire up stackleak to Clang's proposed[1] stack depth tracking callback option. While __noinstr already contained __no_sanitize_coverage, it was still needed for __init and __head section markings. This is needed to make sure the callback is not executed in unsupported contexts. Link: llvm/llvm-project#138323 [1] Signed-off-by: Kees Cook <kees@kernel.org> Message-Id: <20250502190129.246328-4-kees@kernel.org>
linux-riscv-bot
pushed a commit
to linux-riscv/linux
that referenced
this pull request
May 2, 2025
Wire up stackleak to Clang's proposed[1] stack depth tracking callback option. While __noinstr already contained __no_sanitize_coverage, it was still needed for __init and __head section markings. This is needed to make sure the callback is not executed in unsupported contexts. Link: llvm/llvm-project#138323 [1] Signed-off-by: Kees Cook <kees@kernel.org> Signed-off-by: Linux RISC-V bot <linux.riscv.bot@gmail.com>
melver
requested changes
May 4, 2025
Oh, yes, I will add those. Thanks! |
kees
force-pushed
the
stackleak
branch
2 times, most recently
from
729d249 to
027ff82
Compare
melver
reviewed
May 5, 2025
There was a problem hiding this comment.
Generally LGTM - but let's also wait for others to comment.
Documentation of this feature is lacking (and afaik also wasn't added in https://reviews.llvm.org/D36839). Given this will be used in the kernel, some kind of official documentation might be good to have.
Btw, if you want a runtime test, it's possible to add one similar to the one in compiler-rt/trunk/test/sanitizer_common/TestCases/sanitizer_coverage_stack_depth.cc.
melver
approved these changes
May 5, 2025
kees
force-pushed
the
stackleak
branch
2 times, most recently
from
18bc755 to
084b20d
Compare
|
Okay, a couple small clean-ups, and I've also solved the lack of dynamic stack size tracking. |
vitalybuka
reviewed
May 6, 2025
| dyn_cast<ConstantInt>(AI->getArraySize())) | ||
| Bytes *= arraySize->getZExtValue(); | ||
| } | ||
| EstimatedStackSize += Bytes; |
There was a problem hiding this comment.
why do we need to do this accounting and not just use something like __builtin_frame_address ?
There was a problem hiding this comment.
I think you're asking why I can't use Intrinsic::frameaddress in the callback case? (This is what the __sancov_lowest_stack mode does.) At this point the actual size of the stack space is unknown (since it hasn't gone through CodeGen). We need to know this early to minimize the number of places where instrumentation is happening. So to estimate how much space is in use, this manually adds it up to decide whether or not to insert the callback call. (In the __sancov_lowest_stack mode, the instrumentation is always added.)
There was a problem hiding this comment.
I've added a more detailed comment to the accounting loop (and moved the insertion calculation there since it's only needed for the callback case). And I've tweaked the documentation a bit more with a short example.
Normally -fsanitize-coverage=stack-depth inserts inline arithmetic to update thread_local __sancov_lowest_stack. To support stack depth tracking in the Linux kernel, which does not implement traditional thread_local storage, provide the option to call a function instead. This matches the existing "stackleak" implementation that is supported in Linux via a GCC plugin. To make this coverage more performant, a minimum estimated stack depth can be chosen to enable the callback mode, skipping instrumentation of functions with smaller stacks. With -fsanitize-coverage-stack-depth-callback-min set greater than 0, the __sanitize_cov_stack_depth() callback will be injected when the estimated stack depth is greater than or equal to the given minimum.
Add documentation to detail the behaviors of the stack-depth tracer, including the new -fsanitize-coverage-stack-depth-callback-min=N argument.
melver
reviewed
May 6, 2025
| // If dynamic alloca found, always add call. | ||
| bool dynamic_alloca = false; | ||
| // Find an insertion point after last "alloca". | ||
| llvm::Instruction *InsertBefore = NULL; |
| // In callback mode, only add call when stack depth reaches minimum. | ||
| uint32_t EstimatedStackSize = 0; | ||
| // If dynamic alloca found, always add call. | ||
| bool dynamic_alloca = false; |
|
|
||
| The maximum depth of a function call graph is stored in the thread-local | ||
| ``__sancov_lowest_stack`` variable. Instrumentation is inserted in every | ||
| non-leaf function to check the stack pointer against this variable, |
There was a problem hiding this comment.
You write "stack pointer" here, but in the below it's "frame pointer", which are different.
| if (stack < __sancov_lowest_stack) | ||
| __sancov_lowest_stack = stack; | ||
|
|
||
| If ``-fsanitize-coverage-stack-depth-callback-min=N`` is also used, the |
There was a problem hiding this comment.
"If -fsanitize-coverage-stack-depth-callback-min=N (where N > 0) is also used ..." ?
Because right now it's unclear what behaviour is supposed to be if N==0. Per current implementation N==0 disables this feature.
|
|
||
| .. code-block:: c++ | ||
|
|
||
| thread_local uintptr_t __sancov_lowest_stack; |
There was a problem hiding this comment.
extern ?
Because the compiler doesn't define the TLS variable.
| case of a dynamically sized stack, the callback is unconditionally added. | ||
|
|
||
| The callback takes no arguments and is responsible for determining the | ||
| stack pointer and doing any needed comparisons and storage. A roughtly |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Normally -fsanitize-coverage=stack-depth inserts inline arithmetic to update thread_local __sancov_lowest_stack. To support stack depth tracking in the Linux kernel, which does not implement traditional thread_local storage, provide the option to call a function instead.
This matches the existing "stackleak" implementation that is supported in Linux via a GCC plugin. To make this coverage more performant, a minimum estimated stack depth can be chosen to enable the callback mode, skipping instrumentation of functions with smaller stacks.
With -fsanitize-coverage-stack-depth-callback-min set greater than 0, the __sanitize_cov_stack_depth() callback will be injected when the estimated stack depth is greater than or equal to the given minimum.