[llvm] r303698 - Disable coverage opt-out for strong postdominator blocks.

[George Karpenkov via llvm-commits]

Author: george.karpenkov
Date: Tue May 23 16:58:54 2017
New Revision: 303698

URL: http://llvm.org/viewvc/llvm-project?rev=303698&view=rev
Log:
Disable coverage opt-out for strong postdominator blocks.

Coverage instrumentation has an optimization not to instrument extra
blocks, if the pass is already "accounted for" by a
successor/predecessor basic block.
However (https://github.com/google/sanitizers/issues/783) this
reasoning may become circular, which stops valid paths from having
coverage.
In the worst case this can cause fuzzing to stop working entirely.

This change simplifies logic to something which trivially can not have
such circular reasoning, as losing valid paths does not seem like a
good trade-off for a ~15% decrease in the # of instrumented basic blocks.

Added:
    llvm/trunk/test/Instrumentation/SanitizerCoverage/chains.ll
Modified:
    llvm/trunk/lib/Transforms/Instrumentation/SanitizerCoverage.cpp

Modified: llvm/trunk/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Instrumentation/SanitizerCoverage.cpp?rev=303698&r1=303697&r2=303698&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Instrumentation/SanitizerCoverage.cpp (original)
+++ llvm/trunk/lib/Transforms/Instrumentation/SanitizerCoverage.cpp Tue May 23 16:58:54 2017
@@ -31,7 +31,6 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Analysis/EHPersonalities.h"
-#include "llvm/Analysis/PostDominators.h"
 #include "llvm/IR/CFG.h"
 #include "llvm/IR/CallSite.h"
 #include "llvm/IR/DataLayout.h"
@@ -169,7 +168,6 @@ public:
 
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.addRequired<DominatorTreeWrapperPass>();
-    AU.addRequired<PostDominatorTreeWrapperPass>();
   }
 
 private:
@@ -367,23 +365,8 @@ static bool isFullDominator(const BasicB
   return true;
 }
 
-// True if block has predecessors and it postdominates all of them.
-static bool isFullPostDominator(const BasicBlock *BB,
-                                const PostDominatorTree *PDT) {
-  if (pred_begin(BB) == pred_end(BB))
-    return false;
-
-  for (const BasicBlock *PRED : make_range(pred_begin(BB), pred_end(BB))) {
-    if (!PDT->dominates(BB, PRED))
-      return false;
-  }
-
-  return true;
-}
-
 static bool shouldInstrumentBlock(const Function &F, const BasicBlock *BB,
                                   const DominatorTree *DT,
-                                  const PostDominatorTree *PDT,
                                   const SanitizerCoverageOptions &Options) {
   // Don't insert coverage for unreachable blocks: we will never call
   // __sanitizer_cov() for them, so counting them in
@@ -401,7 +384,7 @@ static bool shouldInstrumentBlock(const
   if (Options.NoPrune || &F.getEntryBlock() == BB)
     return true;
 
-  return !(isFullDominator(BB, DT) || isFullPostDominator(BB, PDT));
+  return !isFullDominator(BB, DT);
 }
 
 bool SanitizerCoverageModule::runOnFunction(Function &F) {
@@ -433,11 +416,9 @@ bool SanitizerCoverageModule::runOnFunct
 
   const DominatorTree *DT =
       &getAnalysis<DominatorTreeWrapperPass>(F).getDomTree();
-  const PostDominatorTree *PDT =
-      &getAnalysis<PostDominatorTreeWrapperPass>(F).getPostDomTree();
 
   for (auto &BB : F) {
-    if (shouldInstrumentBlock(F, &BB, DT, PDT, Options))
+    if (shouldInstrumentBlock(F, &BB, DT, Options))
       BlocksToInstrument.push_back(&BB);
     for (auto &Inst : BB) {
       if (Options.IndirectCalls) {
@@ -719,7 +700,6 @@ INITIALIZE_PASS_BEGIN(SanitizerCoverageM
                       "ModulePass",
                       false, false)
 INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass)
-INITIALIZE_PASS_DEPENDENCY(PostDominatorTreeWrapperPass)
 INITIALIZE_PASS_END(SanitizerCoverageModule, "sancov",
                     "SanitizerCoverage: TODO."
                     "ModulePass",

Added: llvm/trunk/test/Instrumentation/SanitizerCoverage/chains.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Instrumentation/SanitizerCoverage/chains.ll?rev=303698&view=auto
==============================================================================
--- llvm/trunk/test/Instrumentation/SanitizerCoverage/chains.ll (added)
+++ llvm/trunk/test/Instrumentation/SanitizerCoverage/chains.ll Tue May 23 16:58:54 2017
@@ -0,0 +1,27 @@
+; RUN: opt < %s -sancov -sanitizer-coverage-level=4 -sanitizer-coverage-trace-pc -sanitizer-coverage-prune-blocks=1  -S | FileCheck %s
+
+define i32 @blah(i32) #0 {
+  %2 = icmp sgt i32 %0, 1
+  br i1 %2, label %branch, label %exit
+; CHECK: call void @__sanitizer_cov_trace_pc()
+branch:
+  br label %pos2
+; CHECK-LABEL: branch:
+; CHECK-NOT: call void @__sanitizer_cov_trace_pc()
+pos2:
+  br label %pos3
+; CHECK-LABEL: pos2:
+; CHECK-NOT: call void @__sanitizer_cov_trace_pc()
+pos3:
+  br label %pos4
+; CHECK-LABEL: pos3:
+; CHECK-NOT: call void @__sanitizer_cov_trace_pc()
+pos4:
+  ret i32 0
+; CHECK-LABEL: pos4:
+; CHECK: call void @__sanitizer_cov_trace_pc()
+exit:
+  ret i32 0
+; CHECK-LABEL: exit:
+; CHECK: call void @__sanitizer_cov_trace_pc()
+}