]> granicus.if.org Git - clang/commitdiff
Add preliminary support for enhancing null-pointer dereference diagnostics.
authorTed Kremenek <kremenek@apple.com>
Thu, 7 May 2009 00:45:33 +0000 (00:45 +0000)
committerTed Kremenek <kremenek@apple.com>
Thu, 7 May 2009 00:45:33 +0000 (00:45 +0000)
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@71135 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/Analysis/PathSensitive/BugReporter.h
lib/Analysis/BugReporter.cpp
lib/Analysis/GRExprEngineInternalChecks.cpp

index 99cbfd844b7a64c3880eafe7e56cbc1d175759e9..90fd9d8ebf45e7b7d304bea47a6d9504ea530f16 100644 (file)
@@ -50,7 +50,7 @@ public:
   virtual ~BugReporterVisitor();
   virtual PathDiagnosticPiece* VisitNode(const ExplodedNode<GRState>* N,
                                          const ExplodedNode<GRState>* PrevN,
-                                         BugReporterContext& BR) = 0;
+                                         BugReporterContext& BRC) = 0;
   
   virtual bool isOwnedByReporterContext() { return true; }
 };
@@ -115,7 +115,7 @@ public:
   }
   
   // FIXME: Perhaps move this into a subclass.
-  virtual PathDiagnosticPiece* getEndPath(BugReporterContext& BR,
+  virtual PathDiagnosticPiece* getEndPath(BugReporterContext& BRC,
                                           const ExplodedNode<GRState>* N);
   
   /// getLocation - Return the "definitive" location of the reported bug.
@@ -130,15 +130,10 @@ public:
 
   virtual PathDiagnosticPiece* VisitNode(const ExplodedNode<GRState>* N,
                                          const ExplodedNode<GRState>* PrevN,
-                                         BugReporterContext& BR);  
-
-  /*
-  virtual PathDiagnosticPiece* VisitNode(const ExplodedNode<GRState>* N,
-                                         const ExplodedNode<GRState>* PrevN,
-                                         const ExplodedGraph<GRState>& G,
-                                         BugReporterContext& BR,
-                                         NodeResolver& NR);
-   */
+                                         BugReporterContext& BR);
+  
+  virtual void registerInitialVisitors(BugReporterContext& BRC,
+                                       const ExplodedNode<GRState>* N) {}
 };
 
 //===----------------------------------------------------------------------===//
@@ -424,6 +419,10 @@ public:
     return BR.getStateManager();
   }
   
+  ValueManager& getValueManager() {
+    return getStateManager().getValueManager();
+  }
+  
   ASTContext& getASTContext() {
     return BR.getContext();
   }
index 36dc6b75eeba0f7389e5478164b3205bf2acb623..23de85709ad8d59dafc4327d08250212fd5a71fd 100644 (file)
@@ -710,10 +710,12 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD,
       }
     }
     
-    for (BugReporterContext::visitor_iterator I = PDB.visitor_begin(),
-         E = PDB.visitor_end(); I!=E; ++I) {
-      if (PathDiagnosticPiece* p = (*I)->VisitNode(N, NextNode, PDB))
-        PD.push_front(p);
+    if (NextNode) {
+      for (BugReporterContext::visitor_iterator I = PDB.visitor_begin(),
+           E = PDB.visitor_end(); I!=E; ++I) {
+        if (PathDiagnosticPiece* p = (*I)->VisitNode(N, NextNode, PDB))
+          PD.push_front(p);
+      }
     }
     
     if (const PostStmt* PS = dyn_cast<PostStmt>(&P)) {      
@@ -1052,53 +1054,58 @@ static void GenerateExtensivePathDiagnostic(PathDiagnostic& PD,
     NextNode = GetPredecessorNode(N);
     ProgramPoint P = N->getLocation();
 
-    // Block edges.
-    if (const BlockEdge *BE = dyn_cast<BlockEdge>(&P)) {
-      const CFGBlock &Blk = *BE->getSrc();
-      const Stmt *Term = Blk.getTerminator();
-      
-      if (Term)
-        EB.addContext(Term);
-
-      // Are we jumping to the head of a loop?  Add a special diagnostic.
-      if (const Stmt *Loop = BE->getSrc()->getLoopTarget()) {
-        
-        PathDiagnosticLocation L(Loop, PDB.getSourceManager());
-        PathDiagnosticEventPiece *p =
-          new PathDiagnosticEventPiece(L,
-                                       "Looping back to the head of the loop");
+    do {
+      // Block edges.
+      if (const BlockEdge *BE = dyn_cast<BlockEdge>(&P)) {
+        const CFGBlock &Blk = *BE->getSrc();
+        const Stmt *Term = Blk.getTerminator();
         
-        EB.addEdge(p->getLocation(), true);
-        PD.push_front(p);
-        
-        if (!Term) {
-          const CompoundStmt *CS = NULL;
-          if (const ForStmt *FS = dyn_cast<ForStmt>(Loop))
-            CS = dyn_cast<CompoundStmt>(FS->getBody());
-          else if (const WhileStmt *WS = dyn_cast<WhileStmt>(Loop))
-            CS = dyn_cast<CompoundStmt>(WS->getBody());
-                   
-          if (CS)
-            EB.rawAddEdge(PathDiagnosticLocation(CS->getRBracLoc(),
-                                                 PDB.getSourceManager()));
+        if (Term)
+          EB.addContext(Term);
+
+        // Are we jumping to the head of a loop?  Add a special diagnostic.
+        if (const Stmt *Loop = BE->getSrc()->getLoopTarget()) {
+          
+          PathDiagnosticLocation L(Loop, PDB.getSourceManager());
+          PathDiagnosticEventPiece *p =
+            new PathDiagnosticEventPiece(L,
+                                         "Looping back to the head of the loop");
+          
+          EB.addEdge(p->getLocation(), true);
+          PD.push_front(p);
+          
+          if (!Term) {
+            const CompoundStmt *CS = NULL;
+            if (const ForStmt *FS = dyn_cast<ForStmt>(Loop))
+              CS = dyn_cast<CompoundStmt>(FS->getBody());
+            else if (const WhileStmt *WS = dyn_cast<WhileStmt>(Loop))
+              CS = dyn_cast<CompoundStmt>(WS->getBody());
+                     
+            if (CS)
+              EB.rawAddEdge(PathDiagnosticLocation(CS->getRBracLoc(),
+                                                   PDB.getSourceManager()));
+          }
         }
+              
+        break;
       }
-            
-      continue;
-    }
 
-    if (const BlockEntrance *BE = dyn_cast<BlockEntrance>(&P)) {      
-      if (const Stmt* S = BE->getFirstStmt()) {
-       if (IsControlFlowExpr(S)) {
-         // Add the proper context for '&&', '||', and '?'.
-         EB.addContext(S);
-       }
-       else
-         EB.addExtendedContext(PDB.getEnclosingStmtLocation(S).asStmt());
-      }
+      if (const BlockEntrance *BE = dyn_cast<BlockEntrance>(&P)) {      
+        if (const Stmt* S = BE->getFirstStmt()) {
+         if (IsControlFlowExpr(S)) {
+           // Add the proper context for '&&', '||', and '?'.
+           EB.addContext(S);
+         }
+         else
+           EB.addExtendedContext(PDB.getEnclosingStmtLocation(S).asStmt());
+        }
 
+        break;
+      }
+    } while (0);
+    
+    if (!NextNode)
       continue;
-    }
     
     for (BugReporterContext::visitor_iterator I = PDB.visitor_begin(),
          E = PDB.visitor_end(); I!=E; ++I) {
@@ -1506,7 +1513,8 @@ void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD,
     PD.push_back(Piece);
   else
     return;
-
+  
+  R->registerInitialVisitors(PDB, N);
   
   switch (PDB.getGenerationScheme()) {
     case PathDiagnosticClient::Extensive:
index a40eff8fc42c9c5d8bdfc80cf05e091da0c441d1..541282da3e5941e752b59e39af34acab9de0471f 100644 (file)
@@ -14,6 +14,7 @@
 
 #include "clang/Analysis/PathSensitive/BugReporter.h"
 #include "clang/Analysis/PathSensitive/GRExprEngine.h"
+#include "clang/Analysis/PathDiagnostic.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/raw_ostream.h"
@@ -34,11 +35,29 @@ ExplodedNode<GRState>* GetNode(GRExprEngine::undef_arg_iterator I) {
   return I->first;
 }
 
+//===----------------------------------------------------------------------===//
+// Forward declarations for bug reporter visitors.
+//===----------------------------------------------------------------------===//
+
+static void registerTrackNullValue(BugReporterContext& BRC,
+                                   const ExplodedNode<GRState>* N);
+
 //===----------------------------------------------------------------------===//
 // Bug Descriptions.
 //===----------------------------------------------------------------------===//
 
 namespace {
+
+class VISIBILITY_HIDDEN BuiltinBugReport : public BugReport {
+public:
+  BuiltinBugReport(BugType& bt, const char* desc,
+                   const ExplodedNode<GRState> *n)
+  : BugReport(bt, desc, n) {}
+  
+  void registerInitialVisitors(BugReporterContext& BRC,
+                               const ExplodedNode<GRState>* N);
+};  
+  
 class VISIBILITY_HIDDEN BuiltinBug : public BugType {
   GRExprEngine &Eng;
 protected:
@@ -54,13 +73,25 @@ public:
 
   void FlushReports(BugReporter& BR) { FlushReportsImpl(BR, Eng); }
   
-  template <typename ITER>
-  void Emit(BugReporter& BR, ITER I, ITER E) {
-    for (; I != E; ++I) BR.EmitReport(new BugReport(*this, desc.c_str(),
-                                                     GetNode(I)));
-  }  
+  virtual void registerInitialVisitors(BugReporterContext& BRC,
+                                       const ExplodedNode<GRState>* N,
+                                       BuiltinBugReport *R) {}
+  
+  template <typename ITER> void Emit(BugReporter& BR, ITER I, ITER E);
 };
   
+  
+template <typename ITER>
+void BuiltinBug::Emit(BugReporter& BR, ITER I, ITER E) {
+  for (; I != E; ++I) BR.EmitReport(new BuiltinBugReport(*this, desc.c_str(),
+                                                         GetNode(I)));
+}  
+
+void BuiltinBugReport::registerInitialVisitors(BugReporterContext& BRC,
+                                               const ExplodedNode<GRState>* N) {
+  static_cast<BuiltinBug&>(getBugType()).registerInitialVisitors(BRC, N, this);
+}  
+  
 class VISIBILITY_HIDDEN NullDeref : public BuiltinBug {
 public:
   NullDeref(GRExprEngine* eng)
@@ -69,6 +100,12 @@ public:
   void FlushReportsImpl(BugReporter& BR, GRExprEngine& Eng) {
     Emit(BR, Eng.null_derefs_begin(), Eng.null_derefs_end());
   }
+  
+  void registerInitialVisitors(BugReporterContext& BRC,
+                               const ExplodedNode<GRState>* N,
+                               BuiltinBugReport *R) {
+    registerTrackNullValue(BRC, N);
+  }
 };
   
 class VISIBILITY_HIDDEN NilReceiverStructRet : public BugType {
@@ -483,6 +520,124 @@ public:
 };
 } // end anonymous namespace
 
+//===----------------------------------------------------------------------===//
+// Definitions for bug reporter visitors.
+//===----------------------------------------------------------------------===//
+
+namespace {
+class VISIBILITY_HIDDEN TrackConstraintBRVisitor : public BugReporterVisitor {
+  SVal Constraint;
+  const bool Assumption;
+  bool isSatisfied;
+public:
+  TrackConstraintBRVisitor(SVal constraint, bool assumption)
+    : Constraint(constraint), Assumption(assumption), isSatisfied(false) {}
+    
+  PathDiagnosticPiece* VisitNode(const ExplodedNode<GRState>* N,
+                                 const ExplodedNode<GRState>* PrevN,
+                                 BugReporterContext& BRC) {
+    if (isSatisfied)
+      return NULL;
+    
+    // Check if in the previous state it was feasible for this constraint
+    // to *not* be true.
+    
+    GRStateManager &StateMgr = BRC.getStateManager();
+    bool isFeasible = false;    
+    if (StateMgr.Assume(PrevN->getState(), Constraint, !Assumption,
+                        isFeasible)) {
+      assert(isFeasible); // Eventually we don't need 'isFeasible'.
+
+      isSatisfied = true;
+      
+      // As a sanity check, make sure that the negation of the constraint
+      // was infeasible in the current state.  If it is feasible, we somehow
+      // missed the transition point.
+      isFeasible = false;
+      if (StateMgr.Assume(N->getState(), Constraint, !Assumption,
+                          isFeasible)) {
+        assert(isFeasible);
+        return NULL;
+      }
+      
+      // We found the transition point for the constraint.  We now need to
+      // pretty-print the constraint. (work-in-progress)      
+      std::string sbuf;
+      llvm::raw_string_ostream os(sbuf);
+      
+      if (isa<Loc>(Constraint)) {
+        os << "Assuming pointer value is ";
+        os << (Assumption ? "non-NULL" : "NULL");
+      }
+      
+      if (os.str().empty())
+        return NULL;
+      
+      // FIXME: Refactor this into BugReporterContext.
+      Stmt *S = 0;      
+      ProgramPoint P = N->getLocation();
+      
+      if (BlockEdge *BE = dyn_cast<BlockEdge>(&P)) {
+        CFGBlock *BSrc = BE->getSrc();
+        S = BSrc->getTerminatorCondition();
+      }
+      else if (PostStmt *PS = dyn_cast<PostStmt>(&P)) {
+        S = PS->getStmt();
+      }
+       
+      if (!S)
+        return NULL;
+      
+      // Construct a new PathDiagnosticPiece.
+      PathDiagnosticLocation L(S, BRC.getSourceManager());
+      return new PathDiagnosticEventPiece(L, os.str());
+    }
+    
+    return NULL;
+  }  
+};
+} // end anonymous namespace
+
+static void registerTrackConstraint(BugReporterContext& BRC, SVal Constraint,
+                                    bool Assumption) {
+  BRC.addVisitor(new TrackConstraintBRVisitor(Constraint, Assumption));  
+}
+
+static void registerTrackNullValue(BugReporterContext& BRC,
+                                   const ExplodedNode<GRState>* N) {
+  
+  ProgramPoint P = N->getLocation();
+  PostStmt *PS = dyn_cast<PostStmt>(&P);
+
+  if (!PS)
+    return;
+  
+  Stmt *S = PS->getStmt();
+  
+  if (ArraySubscriptExpr *AE = dyn_cast<ArraySubscriptExpr>(S)) {
+    S = AE->getBase();
+  }
+  
+  SVal V = BRC.getStateManager().GetSValAsScalarOrLoc(N->getState(), S);
+  
+  // Uncomment this to find cases where we aren't properly getting the
+  // base value that was dereferenced.
+  // assert(!V.isUnknownOrUndef());
+  
+  // For now just track when a symbolic value became null.
+  if (loc::MemRegionVal *L = dyn_cast<loc::MemRegionVal>(&V)) {
+    const SubRegion *R = cast<SubRegion>(L->getRegion());
+    while (R && !isa<SymbolicRegion>(R)) {
+      R = dyn_cast<SubRegion>(R->getSuperRegion());
+    }
+    
+    if (R) {
+      assert(isa<SymbolicRegion>(R));
+      registerTrackConstraint(BRC, loc::MemRegionVal(R), false);
+    }
+  }
+}
+
 //===----------------------------------------------------------------------===//
 // Check registration.
 //===----------------------------------------------------------------------===//