]> granicus.if.org Git - clang/commitdiff
[analyzer][IDF] Add a control dependency calculator + a new debug checker
authorKristof Umann <dkszelethus@gmail.com>
Fri, 5 Jul 2019 12:17:44 +0000 (12:17 +0000)
committerKristof Umann <dkszelethus@gmail.com>
Fri, 5 Jul 2019 12:17:44 +0000 (12:17 +0000)
I intend to improve the analyzer's bug reports by tracking condition
expressions.

01 bool b = messyComputation();
02 int i = 0;
03 if (b) // control dependency of the bug site, let's explain why we assume val
04        // to be true
05   10 / i; // warn: division by zero

I'll detail this heuristic in the followup patch, strictly related to this one
however:

* Create the new ControlDependencyCalculator class that uses llvm::IDFCalculator
  to (lazily) calculate control dependencies for Clang's CFG.
* A new debug checker debug.DumpControlDependencies is added for lit tests
* Add unittests

Differential Revision: https://reviews.llvm.org/D62619

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@365197 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/Analysis/Analyses/Dominators.h
include/clang/Analysis/CFG.h
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
test/Analysis/domtest.c
test/Analysis/domtest.cpp
unittests/Analysis/CFGDominatorTree.cpp

index bdd48198d0d59ae638f605be228c319e946ed554..bc672eb7d52f35cb17ebf1750b1db0bdd23ab88c 100644 (file)
@@ -18,6 +18,7 @@
 #include "llvm/ADT/DepthFirstIterator.h"
 #include "llvm/ADT/GraphTraits.h"
 #include "llvm/ADT/iterator.h"
+#include "llvm/Support/GenericIteratedDominanceFrontier.h"
 #include "llvm/Support/GenericDomTree.h"
 #include "llvm/Support/GenericDomTreeConstruction.h"
 #include "llvm/Support/raw_ostream.h"
@@ -44,12 +45,17 @@ class CFGDominatorTreeImpl : public ManagedAnalysis {
 public:
   using DominatorTreeBase = llvm::DominatorTreeBase<CFGBlock, IsPostDom>;
 
-  DominatorTreeBase DT;
-
   CFGDominatorTreeImpl() = default;
+
+  CFGDominatorTreeImpl(CFG *cfg) {
+    buildDominatorTree(cfg);
+  }
+
   ~CFGDominatorTreeImpl() override = default;
 
-  DominatorTreeBase& getBase() { return *DT; }
+  DominatorTreeBase &getBase() { return DT; }
+
+  CFG *getCFG() { return cfg; }
 
   /// \returns the root CFGBlock of the dominators tree.
   CFGBlock *getRoot() const {
@@ -172,11 +178,96 @@ public:
 
 private:
   CFG *cfg;
+  DominatorTreeBase DT;
 };
 
 using CFGDomTree = CFGDominatorTreeImpl</*IsPostDom*/ false>;
 using CFGPostDomTree = CFGDominatorTreeImpl</*IsPostDom*/ true>;
 
+} // end of namespace clang
+
+namespace llvm {
+namespace IDFCalculatorDetail {
+
+/// Specialize ChildrenGetterTy to skip nullpointer successors.
+template <bool IsPostDom>
+struct ChildrenGetterTy<clang::CFGBlock, IsPostDom> {
+  using NodeRef = typename GraphTraits<clang::CFGBlock>::NodeRef;
+  using ChildrenTy = SmallVector<NodeRef, 8>;
+
+  ChildrenTy get(const NodeRef &N) {
+    using OrderedNodeTy =
+        typename IDFCalculatorBase<clang::CFGBlock, IsPostDom>::OrderedNodeTy;
+
+    auto Children = children<OrderedNodeTy>(N);
+    ChildrenTy Ret{Children.begin(), Children.end()};
+    Ret.erase(std::remove(Ret.begin(), Ret.end(), nullptr), Ret.end());
+    return Ret;
+  }
+};
+
+} // end of namespace IDFCalculatorDetail
+} // end of namespace llvm
+
+namespace clang {
+
+class ControlDependencyCalculator : public ManagedAnalysis {
+  using IDFCalculator = llvm::IDFCalculatorBase<CFGBlock, /*IsPostDom=*/true>;
+  using CFGBlockVector = llvm::SmallVector<CFGBlock *, 4>;
+  using CFGBlockSet = llvm::SmallPtrSet<CFGBlock *, 4>;
+
+  CFGPostDomTree PostDomTree;
+  IDFCalculator IDFCalc;
+
+  llvm::DenseMap<CFGBlock *, CFGBlockVector> ControlDepenencyMap;
+
+public:
+  ControlDependencyCalculator(CFG *cfg)
+    : PostDomTree(cfg), IDFCalc(PostDomTree.getBase()) {}
+
+  const CFGPostDomTree &getCFGPostDomTree() const { return PostDomTree; }
+
+  // Lazily retrieves the set of control dependencies to \p A.
+  const CFGBlockVector &getControlDependencies(CFGBlock *A) {
+    auto It = ControlDepenencyMap.find(A);
+    if (It == ControlDepenencyMap.end()) {
+      CFGBlockSet DefiningBlock = {A};
+      IDFCalc.setDefiningBlocks(DefiningBlock);
+
+      CFGBlockVector ControlDependencies;
+      IDFCalc.calculate(ControlDependencies);
+
+      It = ControlDepenencyMap.insert({A, ControlDependencies}).first;
+    }
+
+    assert(It != ControlDepenencyMap.end());
+    return It->second;
+  }
+
+  /// Whether \p A is control dependent on \p B.
+  bool isControlDependent(CFGBlock *A, CFGBlock *B) {
+    return llvm::is_contained(getControlDependencies(A), B);
+  }
+
+  // Dumps immediate control dependencies for each block.
+  LLVM_DUMP_METHOD void dump() {
+    CFG *cfg = PostDomTree.getCFG();
+    llvm::errs() << "Control dependencies (Node#,Dependency#):\n";
+    for (CFGBlock *BB : *cfg) {
+
+      assert(BB &&
+             "LLVM's Dominator tree builder uses nullpointers to signify the "
+             "virtual root!");
+
+      for (CFGBlock *isControlDependency : getControlDependencies(BB))
+        llvm::errs() << "(" << BB->getBlockID()
+                     << ","
+                     << isControlDependency->getBlockID()
+                     << ")\n";
+    }
+  }
+};
+
 } // namespace clang
 
 namespace llvm {
index c2f49be123f83498df139a79d7be88da48807c36..277b2292e5eac9b9c9dec94758313df530b8fdb4 100644 (file)
@@ -1285,6 +1285,9 @@ template <> struct GraphTraits< ::clang::CFGBlock *> {
   static ChildIteratorType child_end(NodeRef N) { return N->succ_end(); }
 };
 
+template <> struct GraphTraits<clang::CFGBlock>
+    : GraphTraits<clang::CFGBlock *> {};
+
 template <> struct GraphTraits< const ::clang::CFGBlock *> {
   using NodeRef = const ::clang::CFGBlock *;
   using ChildIteratorType = ::clang::CFGBlock::const_succ_iterator;
@@ -1294,6 +1297,9 @@ template <> struct GraphTraits< const ::clang::CFGBlock *> {
   static ChildIteratorType child_end(NodeRef N) { return N->succ_end(); }
 };
 
+template <> struct GraphTraits<const clang::CFGBlock>
+    : GraphTraits<clang::CFGBlock *> {};
+
 template <> struct GraphTraits<Inverse< ::clang::CFGBlock *>> {
   using NodeRef = ::clang::CFGBlock *;
   using ChildIteratorType = ::clang::CFGBlock::const_pred_iterator;
@@ -1306,6 +1312,9 @@ template <> struct GraphTraits<Inverse< ::clang::CFGBlock *>> {
   static ChildIteratorType child_end(NodeRef N) { return N->pred_end(); }
 };
 
+template <> struct GraphTraits<Inverse<clang::CFGBlock>>
+    : GraphTraits<clang::CFGBlock *> {};
+
 template <> struct GraphTraits<Inverse<const ::clang::CFGBlock *>> {
   using NodeRef = const ::clang::CFGBlock *;
   using ChildIteratorType = ::clang::CFGBlock::const_pred_iterator;
@@ -1318,6 +1327,9 @@ template <> struct GraphTraits<Inverse<const ::clang::CFGBlock *>> {
   static ChildIteratorType child_end(NodeRef N) { return N->pred_end(); }
 };
 
+template <> struct GraphTraits<const Inverse<clang::CFGBlock>>
+    : GraphTraits<clang::CFGBlock *> {};
+
 // Traits for: CFG
 
 template <> struct GraphTraits< ::clang::CFG* >
index 8e0122bcc83796a1aba9952a0b9ed9f221b5fc0c..95ea2d5234ea5f81291e97b5653db67d71ae913f 100644 (file)
@@ -1237,6 +1237,10 @@ def PostDominatorsTreeDumper : Checker<"DumpPostDominators">,
   HelpText<"Print the post dominance tree for a given CFG">,
   Documentation<NotDocumented>;
 
+def ControlDependencyTreeDumper : Checker<"DumpControlDependencies">,
+  HelpText<"Print the post control dependency tree for a given CFG">,
+  Documentation<NotDocumented>;
+
 def LiveVariablesDumper : Checker<"DumpLiveVars">,
   HelpText<"Print results of live variable analysis">,
   Documentation<NotDocumented>;
index 6d32b9191419d0a1fe5b8875071aecc2a622e078..bb9e8110b647915a8ac0cb86e2e372c89d77e218 100644 (file)
@@ -35,9 +35,9 @@ public:
   void checkASTCodeBody(const Decl *D, AnalysisManager& mgr,
                         BugReporter &BR) const {
     if (AnalysisDeclContext *AC = mgr.getAnalysisDeclContext(D)) {
-      CFGDomTree dom;
-      dom.buildDominatorTree(AC->getCFG());
-      dom.dump();
+      CFGDomTree Dom;
+      Dom.buildDominatorTree(AC->getCFG());
+      Dom.dump();
     }
   }
 };
@@ -61,9 +61,9 @@ public:
   void checkASTCodeBody(const Decl *D, AnalysisManager& mgr,
                         BugReporter &BR) const {
     if (AnalysisDeclContext *AC = mgr.getAnalysisDeclContext(D)) {
-      CFGPostDomTree dom;
-      dom.buildDominatorTree(AC->getCFG());
-      dom.dump();
+      CFGPostDomTree Dom;
+      Dom.buildDominatorTree(AC->getCFG());
+      Dom.dump();
     }
   }
 };
@@ -77,6 +77,31 @@ bool ento::shouldRegisterPostDominatorsTreeDumper(const LangOptions &LO) {
   return true;
 }
 
+//===----------------------------------------------------------------------===//
+// ControlDependencyTreeDumper
+//===----------------------------------------------------------------------===//
+
+namespace {
+class ControlDependencyTreeDumper : public Checker<check::ASTCodeBody> {
+public:
+  void checkASTCodeBody(const Decl *D, AnalysisManager& mgr,
+                        BugReporter &BR) const {
+    if (AnalysisDeclContext *AC = mgr.getAnalysisDeclContext(D)) {
+      ControlDependencyCalculator Dom(AC->getCFG());
+      Dom.dump();
+    }
+  }
+};
+}
+
+void ento::registerControlDependencyTreeDumper(CheckerManager &mgr) {
+  mgr.registerChecker<ControlDependencyTreeDumper>();
+}
+
+bool ento::shouldRegisterControlDependencyTreeDumper(const LangOptions &LO) {
+  return true;
+}
+
 //===----------------------------------------------------------------------===//
 // LiveVariablesDumper
 //===----------------------------------------------------------------------===//
index 91cd7de420fa0427006eebbfdf1844db773b6d50..b642bd35319cf26ec75dd98ac2710223fa1c9763 100644 (file)
@@ -1,6 +1,7 @@
 // RUN: %clang_analyze_cc1 %s \
 // RUN:   -analyzer-checker=debug.DumpDominators \
 // RUN:   -analyzer-checker=debug.DumpPostDominators \
+// RUN:   -analyzer-checker=debug.DumpControlDependencies \
 // RUN:   2>&1 | FileCheck %s
 
 // Test the DominatorsTree implementation with various control flows
@@ -32,7 +33,16 @@ int test1()
 //                          V
 //                         [B1] -> [B0 (EXIT)]
 
-// CHECK:      Immediate dominance tree (Node#,IDom#):
+// CHECK:      Control dependencies (Node#,Dependency#):
+// CHECK-NEXT: (2,7)
+// CHECK-NEXT: (3,7)
+// CHECK-NEXT: (4,6)
+// CHECK-NEXT: (4,7)
+// CHECK-NEXT: (5,6)
+// CHECK-NEXT: (5,7)
+// CHECK-NEXT: (6,7)
+// CHECK-NEXT: (7,7)
+// CHECK-NEXT: Immediate dominance tree (Node#,IDom#):
 // CHECK-NEXT: (0,1)
 // CHECK-NEXT: (1,7)
 // CHECK-NEXT: (2,3)
@@ -80,7 +90,15 @@ int test2()
 //                  /               V
 // [B7 (ENTRY)] -> [B6] -> [B5] -> [B1] -> [B0 (EXIT)]
 
-// CHECK:      Immediate dominance tree (Node#,IDom#):
+// CHECK:      Control dependencies (Node#,Dependency#):
+// CHECK-NEXT: (2,4)
+// CHECK-NEXT: (2,6)
+// CHECK-NEXT: (3,4)
+// CHECK-NEXT: (3,6)
+// CHECK-NEXT: (4,6)
+// CHECK-NEXT: (4,4)
+// CHECK-NEXT: (5,6)
+// CHECK-NEXT: Immediate dominance tree (Node#,IDom#):
 // CHECK-NEXT: (0,1)
 // CHECK-NEXT: (1,6)
 // CHECK-NEXT: (2,3)
@@ -117,17 +135,29 @@ int test3()
   return 0;
 }
 
-//                            <-------------
-//                           /              \
-//                           |        ---> [B2]
-//                           |       /
-// [B8 (ENTRY)] -> [B7] -> [B6] -> [B5] -> [B4] -> [B3]
-//                   \       |       \              /
-//                    \      |        <-------------
+//                           <- [B2] <-
+//                          /          \
+// [B8 (ENTRY)] -> [B7] -> [B6] ---> [B5] -> [B4] -> [B3]
+//                   \       |         \              /
+//                    \      |          <-------------
 //                     \      \
 //                      --------> [B1] -> [B0 (EXIT)]
 
-// CHECK:      Immediate dominance tree (Node#,IDom#):
+// CHECK:      Control dependencies (Node#,Dependency#):
+// CHECK-NEXT: (2,6)
+// CHECK-NEXT: (2,7)
+// CHECK-NEXT: (3,5)
+// CHECK-NEXT: (3,6)
+// CHECK-NEXT: (3,7)
+// CHECK-NEXT: (4,5)
+// CHECK-NEXT: (4,6)
+// CHECK-NEXT: (4,7)
+// CHECK-NEXT: (5,6)
+// CHECK-NEXT: (5,5)
+// CHECK-NEXT: (5,7)
+// CHECK-NEXT: (6,7)
+// CHECK-NEXT: (6,6)
+// CHECK-NEXT: Immediate dominance tree (Node#,IDom#):
 // CHECK-NEXT: (0,1)
 // CHECK-NEXT: (1,7)
 // CHECK-NEXT: (2,5)
@@ -176,7 +206,29 @@ int test4()
 //                              \
 //                               -> [B1] -> [B0 (EXIT)]
 
-// CHECK:      Immediate dominance tree (Node#,IDom#):
+// CHECK:      Control dependencies (Node#,Dependency#):
+// CHECK-NEXT: (2,10)
+// CHECK-NEXT: (3,5)
+// CHECK-NEXT: (3,9)
+// CHECK-NEXT: (3,10)
+// CHECK-NEXT: (4,5)
+// CHECK-NEXT: (4,9)
+// CHECK-NEXT: (4,10)
+// CHECK-NEXT: (5,9)
+// CHECK-NEXT: (5,5)
+// CHECK-NEXT: (5,10)
+// CHECK-NEXT: (6,8)
+// CHECK-NEXT: (6,9)
+// CHECK-NEXT: (6,10)
+// CHECK-NEXT: (7,8)
+// CHECK-NEXT: (7,9)
+// CHECK-NEXT: (7,10)
+// CHECK-NEXT: (8,9)
+// CHECK-NEXT: (8,8)
+// CHECK-NEXT: (8,10)
+// CHECK-NEXT: (9,10)
+// CHECK-NEXT: (10,10)
+// CHECK-NEXT: Immediate dominance tree (Node#,IDom#):
 // CHECK-NEXT: (0,1)
 // CHECK-NEXT: (1,10)
 // CHECK-NEXT: (2,9)
@@ -242,7 +294,23 @@ int test5()
 //                            V     [B4] ----------------->       /
 //                          [B2]--------------------------------->
 
-// CHECK:      Immediate dominance tree (Node#,IDom#):
+// CHECK:      Control dependencies (Node#,Dependency#):
+// CHECK-NEXT: (2,10)
+// CHECK-NEXT: (3,10)
+// CHECK-NEXT: (4,9)
+// CHECK-NEXT: (4,10)
+// CHECK-NEXT: (5,9)
+// CHECK-NEXT: (5,10)
+// CHECK-NEXT: (6,8)
+// CHECK-NEXT: (6,9)
+// CHECK-NEXT: (6,10)
+// CHECK-NEXT: (7,8)
+// CHECK-NEXT: (7,9)
+// CHECK-NEXT: (7,10)
+// CHECK-NEXT: (8,9)
+// CHECK-NEXT: (8,10)
+// CHECK-NEXT: (9,10)
+// CHECK-NEXT: Immediate dominance tree (Node#,IDom#):
 // CHECK-NEXT: (0,1)
 // CHECK-NEXT: (1,10)
 // CHECK-NEXT: (2,10)
index d57f94e44bbc6c07ba50913215066d12d6de741a..078117ef85dc15c8d27c3cbfbc6eac58d9beaade 100644 (file)
@@ -1,6 +1,7 @@
 // RUN: %clang_analyze_cc1 %s \
 // RUN:   -analyzer-checker=debug.DumpDominators \
 // RUN:   -analyzer-checker=debug.DumpPostDominators \
+// RUN:   -analyzer-checker=debug.DumpControlDependencies \
 // RUN:   2>&1 | FileCheck %s
 
 bool coin();
@@ -20,7 +21,8 @@ void f() {
 
 //  [B3 (ENTRY)]  -> [B1] -> [B2] -> [B0 (EXIT)]
 
-// CHECK:      Immediate dominance tree (Node#,IDom#):
+// CHECK:      Control dependencies (Node#,Dependency#):
+// CHECK-NEXT: Immediate dominance tree (Node#,IDom#):
 // CHECK-NEXT: (0,2)
 // CHECK-NEXT: (1,3)
 // CHECK-NEXT: (2,1)
@@ -42,13 +44,18 @@ void funcWithBranch() {
   }
 }
 
-//                            ----> [B2] ---->
-//                           /                \
-// [B5 (ENTRY)] -> [B4] -> [B3] -----------> [B1]
-//                   \                       /
-//                    ----> [B0 (EXIT)] <----
+//                  1st if  2nd if
+//  [B5 (ENTRY)]  -> [B4] -> [B3] -> [B2] -> [B1] -> [B0 (EXIT)]
+//                    \        \              /         /
+//                     \        ------------->         /
+//                      ------------------------------>
 
-// CHECK:      Immediate dominance tree (Node#,IDom#):
+// CHECK:      Control dependencies (Node#,Dependency#):
+// CHECK-NEXT: (1,4)
+// CHECK-NEXT: (2,3)
+// CHECK-NEXT: (2,4)
+// CHECK-NEXT: (3,4)
+// CHECK-NEXT: Immediate dominance tree (Node#,IDom#):
 // CHECK-NEXT: (0,4)
 // CHECK-NEXT: (1,3)
 // CHECK-NEXT: (2,3)
index 1df8c2f7d05d5f23fb4a4ef31af625f0676b28b3..8cbd72c94e67701d8b8fd8d32fe74a7c7c4ec8db 100644 (file)
@@ -98,6 +98,97 @@ TEST(CFGDominatorTree, DomTree) {
   EXPECT_TRUE(PostDom.dominates(nullptr, ExitBlock));
 }
 
+TEST(CFGDominatorTree, ControlDependency) {
+  const char *Code = R"(bool coin();
+
+                        void funcWithBranch() {
+                          int x = 0;
+                          if (coin()) {
+                            if (coin()) {
+                              x = 5;
+                            }
+                            int j = 10 / x;
+                            (void)j;
+                          }
+                        };)";
+  BuildResult Result = BuildCFG(Code);
+  EXPECT_EQ(BuildResult::BuiltCFG, Result.getStatus());
+
+  //                  1st if  2nd if
+  //  [B5 (ENTRY)]  -> [B4] -> [B3] -> [B2] -> [B1] -> [B0 (EXIT)]
+  //                    \        \              /         /
+  //                     \        ------------->         /
+  //                      ------------------------------>
+
+  CFG *cfg = Result.getCFG();
+
+  // Sanity checks.
+  EXPECT_EQ(cfg->size(), 6u);
+
+  CFGBlock *ExitBlock = *cfg->begin();
+  EXPECT_EQ(ExitBlock, &cfg->getExit());
+
+  CFGBlock *NullDerefBlock = *(cfg->begin() + 1);
+
+  CFGBlock *SecondThenBlock = *(cfg->begin() + 2);
+
+  CFGBlock *SecondIfBlock = *(cfg->begin() + 3);
+
+  CFGBlock *FirstIfBlock = *(cfg->begin() + 4);
+
+  CFGBlock *EntryBlock = *(cfg->begin() + 5);
+  EXPECT_EQ(EntryBlock, &cfg->getEntry());
+
+  ControlDependencyCalculator Control(cfg);
+
+  EXPECT_TRUE(Control.isControlDependent(SecondThenBlock, SecondIfBlock));
+  EXPECT_TRUE(Control.isControlDependent(SecondIfBlock, FirstIfBlock));
+  EXPECT_FALSE(Control.isControlDependent(NullDerefBlock, SecondIfBlock));
+}
+
+TEST(CFGDominatorTree, ControlDependencyWithLoops) {
+  const char *Code = R"(int test3() {
+                          int x,y,z;
+
+                          x = y = z = 1;
+                          if (x > 0) {
+                            while (x >= 0){
+                              while (y >= x) {
+                                x = x-1;
+                                y = y/2;
+                              }
+                            }
+                          }
+                          z = y;
+
+                          return 0;
+                        })";
+  BuildResult Result = BuildCFG(Code);
+  EXPECT_EQ(BuildResult::BuiltCFG, Result.getStatus());
+
+  //                           <- [B2] <-
+  //                          /          \
+  // [B8 (ENTRY)] -> [B7] -> [B6] ---> [B5] -> [B4] -> [B3]
+  //                   \       |         \              /
+  //                    \      |          <-------------
+  //                     \      \
+  //                      --------> [B1] -> [B0 (EXIT)]
+
+  CFG *cfg = Result.getCFG();
+
+  ControlDependencyCalculator Control(cfg);
+
+  auto GetBlock = [cfg] (unsigned Index) -> CFGBlock * {
+    assert(Index < cfg->size());
+    return *(cfg->begin() + Index);
+  };
+
+  // While not immediately obvious, the second block in fact post dominates the
+  // fifth, hence B5 is not a control dependency of 2.
+  EXPECT_FALSE(Control.isControlDependent(GetBlock(5), GetBlock(2)));
+}
+
+
 } // namespace
 } // namespace analysis
 } // namespace clang