[SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

[morehouse]

Summary

• Don't sanitize __sancov_lowest_stack.
• Don't instrument leaf functions.
• Add CoverageStackDepth to Fuzzer and FuzzerNoLink.
• Only enable on Linux.

[kcc]

Did you check this on something other than the unit tests?
E.g. a couple of benchmarks from fuzzer-test-suite?

llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp

177

we already have a linear scan in SanitizerCoverageModule::runOnFunction -- don't introduce a second one.
Also, there is InvokeInst in addition to CallInst, see the loop in runOnFunction.

You can simply extend the loop in runOnFunction to set a flag if the function has non-intrin calls/ invokes

[morehouse]

In D37156#852780, @kcc wrote:

Did you check this on something other than the unit tests?
E.g. a couple of benchmarks from fuzzer-test-suite?

Just tested on the proj4 and lcms benchmarks and no issues came up.

[morehouse]

• Use existing linear scan, and check for InvokeInst.

[kcc]

LGTM

[morehouse]

Turns out I should have been testing the benchmarks with FUZZING_ENGINE=fsanitize_fuzzer. My mistake.

After adding the weak reference to SanitizerCoverage.cpp, both lcms and proj4 build with fsanitize_fuzzer.

[morehouse]

• Add weak reference in SanitizerCoverage.cpp

[morehouse]

Full diff.

[morehouse]

• Add weak definition of __sancov_lowest_stack to runtime.

[kcc]

+George, in case he knows about attribute((tls_model("initial-exec"))) on Mac

compiler-rt/lib/sanitizer_common/sanitizer_coverage_libcdep_new.cc

218

I wonder if this going to work on Mac.

[george.karpenkov]

I don't, but I can check whether tests pass.

[george.karpenkov]

@kcc I've disabled the relevant test on Mac in r311916, please revert my change once this CR goes through.

[morehouse]

• Disable stack depth tracking on Mac.

[kcc]

clang/lib/Driver/SanitizerArgs.cpp

297

please use if(SomeCondition) instead of #if

In general: 99% of cases where you may want to use #if -- you shouldn't

compiler-rt/lib/sanitizer_common/sanitizer_coverage_libcdep_new.cc

20

no standard hearder in this files. Just use the 'uptr' type.

[morehouse]

• Eliminate "#if".
• Replace uintptr_t with uptr.

[kcc]

LGTM

[morehouse]

• Only enable stack depth tracking on Linux.
• Ignore __sancov_lowest_stack in interface symbols tests.

clang/lib/Driver/SanitizerArgs.cpp

if (Arg->getOption().matches(options::OPT_fsanitize_EQ)) {
       // group expansion.
       Add &= ~InvalidTrappingKinds;
       Add &= Supported;
 
       if (Add & Fuzzer)
         Add |= FuzzerNoLink;
 
       // Enable coverage if the fuzzing flag is set.
-      if (Add & FuzzerNoLink)
+      if (Add & FuzzerNoLink) {
         CoverageFeatures |= CoverageTracePCGuard | CoverageIndirCall |
                             CoverageTraceCmp | CoveragePCTable;
+        // Due to TLS differences, stack depth tracking is disabled on Mac.
+        if (!TC.getTriple().isOSDarwin())
+          CoverageFeatures |= CoverageStackDepth;
+      }
 
       Kinds |= Add;
     } else if (Arg->getOption().matches(options::OPT_fno_sanitize_EQ)) {
       Arg->claim();
       SanitizerMask Remove = parseArgValues(D, Arg, true);
       AllRemove |= expandSanitizerGroups(Remove);
     }
   }

compiler-rt/lib/sanitizer_common/sanitizer_coverage_libcdep_new.cc

 
 #if !SANITIZER_FUCHSIA
 #include "sancov_flags.h"
 #include "sanitizer_allocator_internal.h"
 #include "sanitizer_atomic.h"
 #include "sanitizer_common.h"
 #include "sanitizer_file.h"
 #include "sanitizer_symbolizer.h"
 
 using namespace __sanitizer;
 
 using AddressRange = LoadedModule::AddressRange;
 
 namespace __sancov {
 namespace {
 
 static const u64 Magic64 = 0xC0BFFFFFFFFFFF64ULL;

 SANITIZER_INTERFACE_WEAK_DEF(void, __sanitizer_cov_trace_switch, void) {}
 SANITIZER_INTERFACE_WEAK_DEF(void, __sanitizer_cov_trace_div4, void) {}
 SANITIZER_INTERFACE_WEAK_DEF(void, __sanitizer_cov_trace_div8, void) {}
 SANITIZER_INTERFACE_WEAK_DEF(void, __sanitizer_cov_trace_gep, void) {}
 SANITIZER_INTERFACE_WEAK_DEF(void, __sanitizer_cov_trace_pc_indir, void) {}
 SANITIZER_INTERFACE_WEAK_DEF(void, __sanitizer_cov_8bit_counters_init, void) {}
 SANITIZER_INTERFACE_WEAK_DEF(void, __sanitizer_cov_pcs_init, void) {}
 }  // extern "C"
+// Weak definition for code instrumented with -fsanitize-coverage=stack-depth
+// and later linked with code containing a strong definition.
+// E.g., -fsanitize=fuzzer-no-link
+SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE
+SANITIZER_TLS_INITIAL_EXEC_ATTRIBUTE uptr __sancov_lowest_stack;
 
 #endif  // !SANITIZER_FUCHSIA

compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h

 #elif SANITIZER_GO
 # define SANITIZER_INTERFACE_ATTRIBUTE
 # define SANITIZER_WEAK_ATTRIBUTE
 #else
 # define SANITIZER_INTERFACE_ATTRIBUTE __attribute__((visibility("default")))
 # define SANITIZER_WEAK_ATTRIBUTE  __attribute__((weak))
 #endif
 
+// Mac handles TLS differently
+#if SANITIZER_MAC
+# define SANITIZER_TLS_INITIAL_EXEC_ATTRIBUTE
+#else
+# define SANITIZER_TLS_INITIAL_EXEC_ATTRIBUTE \
+    __attribute((tls_model("initial-exec"))) thread_local
+#endif
+
 //--------------------------- WEAK FUNCTIONS ---------------------------------//
 // When working with weak functions, to simplify the code and make it more
 // portable, when possible define a default implementation using this macro:
 //
 // SANITIZER_INTERFACE_WEAK_DEF(<return_type>, <name>, <parameter list>)
 //
 // For example:
 //   SANITIZER_INTERFACE_WEAK_DEF(bool, compare, int a, int b) { return a > b; }

compiler-rt/test/fuzzer/deep-recursion.test

 # Test that we can find a stack overflow
 REQUIRES: linux
-RUN: %cpp_compiler -fsanitize-coverage=stack-depth %S/DeepRecursionTest.cpp -o %t
+RUN: %cpp_compiler %S/DeepRecursionTest.cpp -o %t
 RUN: not %t -seed=1 -runs=100000000 2>&1 | FileCheck %s
 CHECK: ERROR: libFuzzer: deadly signal

llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp

 #include "llvm/IR/Constant.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/InlineAsm.h"
+#include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/MDBuilder.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/Type.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
SanitizerCoverageOptions OverrideFromCL(SanitizerCoverageOptions Options) {
   Options.NoPrune |= !ClPruneBlocks;
   Options.StackDepth |= ClStackDepth;
   if (!Options.TracePCGuard && !Options.TracePC &&
       !Options.Inline8bitCounters && !Options.StackDepth)
     Options.TracePCGuard = true; // TracePCGuard is default.
   return Options;
 }
 
 class SanitizerCoverageModule : public ModulePass {
 public:
   SanitizerCoverageModule(
       const SanitizerCoverageOptions &Options = SanitizerCoverageOptions())
       : ModulePass(ID), Options(OverrideFromCL(Options)) {
     initializeSanitizerCoverageModulePass(*PassRegistry::getPassRegistry());
   }
   bool runOnModule(Module &M) override;
   bool runOnFunction(Function &F);
void InjectCoverageForIndirectCalls(Function &F,
                                       ArrayRef<Instruction *> IndirCalls);
   void InjectTraceForCmp(Function &F, ArrayRef<Instruction *> CmpTraceTargets);
   void InjectTraceForDiv(Function &F,
                          ArrayRef<BinaryOperator *> DivTraceTargets);
   void InjectTraceForGep(Function &F,
                          ArrayRef<GetElementPtrInst *> GepTraceTargets);
   void InjectTraceForSwitch(Function &F,
                             ArrayRef<Instruction *> SwitchTraceTargets);
-  bool InjectCoverage(Function &F, ArrayRef<BasicBlock *> AllBlocks);
+  bool InjectCoverage(Function &F, ArrayRef<BasicBlock *> AllBlocks,
+                      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);
-  void InjectCoverageAtBlock(Function &F, BasicBlock &BB, size_t Idx);
+  void InjectCoverageAtBlock(Function &F, BasicBlock &BB, size_t Idx,
+                             bool IsLeafFunc = true);
   Function *CreateInitCallsForSections(Module &M, const char *InitFunctionName,
                                        Type *Ty, const char *Section);
   std::pair<GlobalVariable *, GlobalVariable *>
   CreateSecStartEnd(Module &M, const char *Section, Type *Ty);
 
   void SetNoSanitizeMetadata(Instruction *I) {
     I->setMetadata(I->getModule()->getMDKindID("nosanitize"),
                    MDNode::get(*C, None));
bool SanitizerCoverageModule::runOnFunction(Function &F) {
   SmallVector<Instruction *, 8> SwitchTraceTargets;
   SmallVector<BinaryOperator *, 8> DivTraceTargets;
   SmallVector<GetElementPtrInst *, 8> GepTraceTargets;
 
   const DominatorTree *DT =
       &getAnalysis<DominatorTreeWrapperPass>(F).getDomTree();
   const PostDominatorTree *PDT =
       &getAnalysis<PostDominatorTreeWrapperPass>(F).getPostDomTree();
+  bool IsLeafFunc = true;
 
   for (auto &BB : F) {
     if (shouldInstrumentBlock(F, &BB, DT, PDT, Options))
       BlocksToInstrument.push_back(&BB);
     for (auto &Inst : BB) {
       if (Options.IndirectCalls) {
         CallSite CS(&Inst);
         if (CS && !CS.getCalledFunction())
           IndirCalls.push_back(&Inst);
       }
       if (Options.TraceCmp) {
         if (isa<ICmpInst>(&Inst))
           CmpTraceTargets.push_back(&Inst);
         if (isa<SwitchInst>(&Inst))
           SwitchTraceTargets.push_back(&Inst);
       }
       if (Options.TraceDiv)
         if (BinaryOperator *BO = dyn_cast<BinaryOperator>(&Inst))
           if (BO->getOpcode() == Instruction::SDiv ||
               BO->getOpcode() == Instruction::UDiv)
             DivTraceTargets.push_back(BO);
       if (Options.TraceGep)
         if (GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(&Inst))
           GepTraceTargets.push_back(GEP);
+      if (Options.StackDepth)
+        if (isa<InvokeInst>(Inst) ||
+            (isa<CallInst>(Inst) && !isa<IntrinsicInst>(Inst)))
+          IsLeafFunc = false;
     }
   }
 
-  InjectCoverage(F, BlocksToInstrument);
+  InjectCoverage(F, BlocksToInstrument, IsLeafFunc);
   InjectCoverageForIndirectCalls(F, IndirCalls);
   InjectTraceForCmp(F, CmpTraceTargets);
   InjectTraceForSwitch(F, SwitchTraceTargets);
   InjectTraceForDiv(F, DivTraceTargets);
   InjectTraceForGep(F, GepTraceTargets);
   return true;
 }
 
void SanitizerCoverageModule::CreateFunctionLocalArrays(
   }
 
   // We don't reference these arrays directly in any of our runtime functions,
   // so we need to prevent them from being dead stripped.
   appendToUsed(*F.getParent(), LocalArrays);
 }
 
 bool SanitizerCoverageModule::InjectCoverage(Function &F,
-                                             ArrayRef<BasicBlock *> AllBlocks) {
+                                             ArrayRef<BasicBlock *> AllBlocks,
+                                             bool IsLeafFunc) {
   if (AllBlocks.empty()) return false;
   CreateFunctionLocalArrays(F, AllBlocks);
   for (size_t i = 0, N = AllBlocks.size(); i < N; i++)
-    InjectCoverageAtBlock(F, *AllBlocks[i], i);
+    InjectCoverageAtBlock(F, *AllBlocks[i], i, IsLeafFunc);
   return true;
 }
 
 // On every indirect call we call a run-time function
 // __sanitizer_cov_indir_call* with two parameters:
 //   - callee address,
 //   - global cache array that contains CacheSize pointers (zero-initialized).
 //     The cache is used to speed up recording the caller-callee pairs.
if (ICmpInst *ICMP = dyn_cast<ICmpInst>(I)) {
       auto Ty = Type::getIntNTy(*C, TypeSize);
       IRB.CreateCall(CallbackFunc, {IRB.CreateIntCast(A0, Ty, true),
               IRB.CreateIntCast(A1, Ty, true)});
     }
   }
 }
 
 void SanitizerCoverageModule::InjectCoverageAtBlock(Function &F, BasicBlock &BB,
-                                                    size_t Idx) {
+                                                    size_t Idx,
+                                                    bool IsLeafFunc) {
   BasicBlock::iterator IP = BB.getFirstInsertionPt();
   bool IsEntryBB = &BB == &F.getEntryBlock();
   DebugLoc EntryLoc;
   if (IsEntryBB) {
     if (auto SP = F.getSubprogram())
       EntryLoc = DebugLoc::get(SP->getScopeLine(), 0, SP);
     // Keep static allocas and llvm.localescape calls in the entry block.  Even
     // if we aren't splitting the block, it's nice for allocas to be before
auto CounterPtr = IRB.CreateGEP(
         Function8bitCounterArray,
         {ConstantInt::get(IntptrTy, 0), ConstantInt::get(IntptrTy, Idx)});
     auto Load = IRB.CreateLoad(CounterPtr);
     auto Inc = IRB.CreateAdd(Load, ConstantInt::get(Int8Ty, 1));
     auto Store = IRB.CreateStore(Inc, CounterPtr);
     SetNoSanitizeMetadata(Load);
     SetNoSanitizeMetadata(Store);
   }
-  if (Options.StackDepth && IsEntryBB) {
+  if (Options.StackDepth && IsEntryBB && !IsLeafFunc) {
     // Check stack depth.  If it's the deepest so far, record it.
     Function *GetFrameAddr =
         Intrinsic::getDeclaration(F.getParent(), Intrinsic::frameaddress);
     auto FrameAddrPtr =
         IRB.CreateCall(GetFrameAddr, {Constant::getNullValue(Int32Ty)});
     auto FrameAddrInt = IRB.CreatePtrToInt(FrameAddrPtr, IntptrTy);
     auto LowestStack = IRB.CreateLoad(SanCovLowestStack);
     auto IsStackLower = IRB.CreateICmpULT(FrameAddrInt, LowestStack);
     auto ThenTerm = SplitBlockAndInsertIfThen(IsStackLower, &*IP, false);
     IRBuilder<> ThenIRB(ThenTerm);
-    ThenIRB.CreateStore(FrameAddrInt, SanCovLowestStack);
+    auto Store = ThenIRB.CreateStore(FrameAddrInt, SanCovLowestStack);
+    SetNoSanitizeMetadata(LowestStack);
+    SetNoSanitizeMetadata(Store);
   }
 }
 
 std::string
 SanitizerCoverageModule::getSectionName(const std::string &Section) const {
   if (TargetTriple.getObjectFormat() == Triple::COFF)
     return ".SCOV$M";
   if (TargetTriple.isOSBinFormatMachO())

llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll

 ; This check verifies that stack depth instrumentation works correctly.
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=1 \
-; RUN:     -sanitizer-coverage-stack-depth -S | FileCheck %s --enable-var-scope
+; RUN:     -sanitizer-coverage-stack-depth -S | FileCheck %s
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=3 \
 ; RUN:     -sanitizer-coverage-stack-depth -sanitizer-coverage-trace-pc-guard \
-; RUN:     -S | FileCheck %s --enable-var-scope
+; RUN:     -S | FileCheck %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 ; CHECK: @__sancov_lowest_stack = thread_local(initialexec) global i64 -1
 @__sancov_lowest_stack = thread_local global i64 0, align 8
 
 define i32 @foo() {
 entry:
 ; CHECK-LABEL: define i32 @foo
-; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0)
-; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType:i[0-9]+]]
-; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack
-; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]]
-; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label
-; CHECK: <label>:[[ifLabel]]:
-; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack
+; CHECK-NOT: call i8* @llvm.frameaddress(i32 0)
+; CHECK-NOT: @__sancov_lowest_stack
 ; CHECK: ret i32 7
 
   ret i32 7
 }
 
 define i32 @bar() {
 entry:
 ; CHECK-LABEL: define i32 @bar
 ; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0)
-; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType]]
-; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack
-; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]]
+; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[intType:i[0-9]+]]
+; CHECK: [[lowest:%[^ \t]+]] = load [[intType]], [[intType]]* @__sancov_lowest_stack
+; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[intType]] [[frameInt]], [[lowest]]
 ; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label
 ; CHECK: <label>:[[ifLabel]]:
-; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack
+; CHECK: store [[intType]] [[frameInt]], [[intType]]* @__sancov_lowest_stack
 ; CHECK: %call = call i32 @foo()
 ; CHECK: ret i32 %call
 
   %call = call i32 @foo()
   ret i32 %call
 }
 
 define weak_odr hidden i64* @_ZTW21__sancov_lowest_stack() {
   ret i64* @__sancov_lowest_stack
 }