[analyzer] Use the common evalBind infrastructure for initializers.

This allows checkers (like the MallocChecker) to process the effects of the
bind. Previously, using a memory-allocating function (like strdup()) in an
initializer would result in a leak warning.

This does bend the expectations of checkBind a bit; since there is no
assignment expression, the statement being used is the initializer value.
In most cases this shouldn't matter because we'll use a PostInitializer
program point (rather than PostStmt) for any checker-generated nodes, though
we /will/ generate a PostStore node referencing the internal statement.
(In theory this could have funny effects if someone actually does an
assignment within an initializer; in practice, that seems like it would be
very rare.)

<rdar://problem/12171711>

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

6 files changed, 52 insertions, 33 deletions

diff --git a/include/clang/StaticAnalyzer/Core/CheckerManager.h b/include/clang/StaticAnalyzer/Core/CheckerManager.h
index e11b6d518a..9826e5f41a 100644
--- a/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ b/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -258,7 +258,7 @@ public:
                           const ExplodedNodeSet &Src,
                           SVal location, SVal val,
                           const Stmt *S, ExprEngine &Eng,
-                          ProgramPoint::Kind PointKind);
+                          const ProgramPoint &PP);
 
   /// \brief Run checkers for end of analysis.
   void runCheckersForEndAnalysis(ExplodedGraph &G, BugReporter &BR,
diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index 158a903a68..0daf1527b0 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -434,7 +434,8 @@ protected:
   /// evalBind - Handle the semantics of binding a value to a specific location.
   ///  This method is used by evalStore, VisitDeclStmt, and others.
   void evalBind(ExplodedNodeSet &Dst, const Stmt *StoreE, ExplodedNode *Pred,
-                SVal location, SVal Val, bool atDeclInit = false);
+                SVal location, SVal Val, bool atDeclInit = false,
+                const ProgramPoint *PP = 0);
 
 public:
   // FIXME: 'tag' should be removed, and a LocationContext should be used
diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index 1dbd8f0379..956174457b 100644
--- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -32,7 +32,11 @@ using namespace ento;
 const Stmt *bugreporter::GetDerefExpr(const ExplodedNode *N) {
   // Pattern match for a few useful cases (do something smarter later):
   //   a[0], p->f, *p
-  const Stmt *S = N->getLocationAs<PostStmt>()->getStmt();
+  const PostStmt *Loc = N->getLocationAs<PostStmt>();
+  if (!Loc)
+    return 0;
+
+  const Stmt *S = Loc->getStmt();
 
   while (true) {
     if (const BinaryOperator *B = dyn_cast<BinaryOperator>(S)) {
diff --git a/lib/StaticAnalyzer/Core/CheckerManager.cpp b/lib/StaticAnalyzer/Core/CheckerManager.cpp
index c7866559c6..1c4458a098 100644
--- a/lib/StaticAnalyzer/Core/CheckerManager.cpp
+++ b/lib/StaticAnalyzer/Core/CheckerManager.cpp
@@ -314,20 +314,19 @@ namespace {
     SVal Val;
     const Stmt *S;
     ExprEngine &Eng;
-    ProgramPoint::Kind PointKind;
+    const ProgramPoint &PP;
 
     CheckersTy::const_iterator checkers_begin() { return Checkers.begin(); }
     CheckersTy::const_iterator checkers_end() { return Checkers.end(); }
 
     CheckBindContext(const CheckersTy &checkers,
                      SVal loc, SVal val, const Stmt *s, ExprEngine &eng,
-                     ProgramPoint::Kind PK)
-      : Checkers(checkers), Loc(loc), Val(val), S(s), Eng(eng), PointKind(PK) {}
+                     const ProgramPoint &pp)
+      : Checkers(checkers), Loc(loc), Val(val), S(s), Eng(eng), PP(pp) {}
 
     void runChecker(CheckerManager::CheckBindFunc checkFn,
                     NodeBuilder &Bldr, ExplodedNode *Pred) {
-      const ProgramPoint &L = ProgramPoint::getProgramPoint(S, PointKind,
-                                Pred->getLocationContext(), checkFn.Checker);
+      const ProgramPoint &L = PP.withTag(checkFn.Checker);
       CheckerContext C(Bldr, Eng, Pred, L);
 
       checkFn(Loc, Val, S, C);
@@ -340,8 +339,8 @@ void CheckerManager::runCheckersForBind(ExplodedNodeSet &Dst,
                                         const ExplodedNodeSet &Src,
                                         SVal location, SVal val,
                                         const Stmt *S, ExprEngine &Eng,
-                                        ProgramPoint::Kind PointKind) {
-  CheckBindContext C(BindCheckers, location, val, S, Eng, PointKind);
+                                        const ProgramPoint &PP) {
+  CheckBindContext C(BindCheckers, location, val, S, Eng, PP);
   expandGraphWithCheckers(C, Dst, Src);
 }
 
diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 6fb9116dbb..d6bcfe4eb2 100644
--- a/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -354,11 +354,6 @@ void ExprEngine::ProcessStmt(const CFGStmt S,
 
 void ExprEngine::ProcessInitializer(const CFGInitializer Init,
                                     ExplodedNode *Pred) {
-  ExplodedNodeSet Dst;
-  NodeBuilder Bldr(Pred, Dst, *currBldrCtx);
-
-  ProgramStateRef State = Pred->getState();
-
   const CXXCtorInitializer *BMI = Init.getInitializer();
 
   PrettyStackTraceLoc CrashInfo(getContext().getSourceManager(),
@@ -370,13 +365,19 @@ void ExprEngine::ProcessInitializer(const CFGInitializer Init,
                            cast<StackFrameContext>(Pred->getLocationContext());
   const CXXConstructorDecl *decl =
                            cast<CXXConstructorDecl>(stackFrame->getDecl());
+
+  ProgramStateRef State = Pred->getState();
   SVal thisVal = State->getSVal(svalBuilder.getCXXThis(decl, stackFrame));
 
+  PostInitializer PP(BMI, stackFrame);
+  ExplodedNodeSet Tmp(Pred);
+
   // Evaluate the initializer, if necessary
   if (BMI->isAnyMemberInitializer()) {
     // Constructors build the object directly in the field,
     // but non-objects must be copied in from the initializer.
-    if (!isa<CXXConstructExpr>(BMI->getInit())) {
+    const Expr *Init = BMI->getInit();
+    if (!isa<CXXConstructExpr>(Init)) {
       SVal FieldLoc;
       if (BMI->isIndirectMemberInitializer())
         FieldLoc = State->getLValue(BMI->getIndirectMember(), thisVal);
@@ -384,19 +385,23 @@ void ExprEngine::ProcessInitializer(const CFGInitializer Init,
         FieldLoc = State->getLValue(BMI->getMember(), thisVal);
 
       SVal InitVal = State->getSVal(BMI->getInit(), stackFrame);
-      State = State->bindLoc(FieldLoc, InitVal);
+
+      Tmp.clear();
+      evalBind(Tmp, Init, Pred, FieldLoc, InitVal, /*isInit=*/true, &PP);
     }
   } else {
     assert(BMI->isBaseInitializer() || BMI->isDelegatingInitializer());
     // We already did all the work when visiting the CXXConstructExpr.
   }
 
-  // Construct a PostInitializer node whether the state changed or not,
+  // Construct PostInitializer nodes whether the state changed or not,
   // so that the diagnostics don't get confused.
-  PostInitializer PP(BMI, stackFrame);
-  // Builder automatically add the generated node to the deferred set,
-  // which are processed in the builder's dtor.
-  Bldr.generateNode(PP, State, Pred);
+  ExplodedNodeSet Dst;
+  NodeBuilder Bldr(Tmp, Dst, *currBldrCtx);
+  for (ExplodedNodeSet::iterator I = Tmp.begin(), E = Tmp.end(); I != E; ++I) {
+    ExplodedNode *N = *I;
+    Bldr.generateNode(PP, N->getState(), N);
+  }
 
   // Enqueue the new nodes onto the work list.
   Engine.enqueue(Dst, currBldrCtx->getBlock(), currStmtIdx);
@@ -1541,13 +1546,17 @@ void ExprEngine::VisitMemberExpr(const MemberExpr *M, ExplodedNode *Pred,
 void ExprEngine::evalBind(ExplodedNodeSet &Dst, const Stmt *StoreE,
                           ExplodedNode *Pred,
                           SVal location, SVal Val,
-                          bool atDeclInit) {
+                          bool atDeclInit, const ProgramPoint *PP) {
+
+  const LocationContext *LC = Pred->getLocationContext();
+  PostStmt PS(StoreE, LC);
+  if (!PP)
+    PP = &PS;
 
   // Do a previsit of the bind.
   ExplodedNodeSet CheckedSet;
   getCheckerManager().runCheckersForBind(CheckedSet, Pred, location, Val,
-                                         StoreE, *this,
-                                         ProgramPoint::PostStmtKind);
+                                         StoreE, *this, *PP);
 
   // If the location is not a 'Loc', it will already be handled by
   // the checkers.  There is nothing left to do.
@@ -1559,7 +1568,6 @@ void ExprEngine::evalBind(ExplodedNodeSet &Dst, const Stmt *StoreE,
   ExplodedNodeSet TmpDst;
   StmtNodeBuilder Bldr(CheckedSet, TmpDst, *currBldrCtx);
 
-  const LocationContext *LC = Pred->getLocationContext();
   for (ExplodedNodeSet::iterator I = CheckedSet.begin(), E = CheckedSet.end();
        I!=E; ++I) {
     ExplodedNode *PredI = *I;
@@ -1575,6 +1583,7 @@ void ExprEngine::evalBind(ExplodedNodeSet &Dst, const Stmt *StoreE,
     if (loc::MemRegionVal *LocRegVal = dyn_cast<loc::MemRegionVal>(&location)) {
       LocReg = LocRegVal->getRegion();
     }
+    
     const ProgramPoint L = PostStore(StoreE, LC, LocReg, 0);
     Bldr.generateNode(L, state, PredI);
   }
diff --git a/test/Analysis/initializer.cpp b/test/Analysis/initializer.cpp
index 4f800d5d43..8785a002c6 100644
--- a/test/Analysis/initializer.cpp
+++ b/test/Analysis/initializer.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-ipa=inlining -cfg-add-implicit-dtors -std=c++11 -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-ipa=inlining -cfg-add-implicit-dtors -std=c++11 -verify %s
 
 // We don't inline constructors unless we have destructors turned on.
 
@@ -57,10 +57,6 @@ void testDelegatingConstructor() {
 }
 
 
-// ------------------------------------
-// False negatives
-// ------------------------------------
-
 struct RefWrapper {
   RefWrapper(int *p) : x(*p) {}
   RefWrapper(int &r) : x(r) {}
@@ -69,10 +65,20 @@ struct RefWrapper {
 
 void testReferenceMember() {
   int *p = 0;
-  RefWrapper X(p); // should warn in the constructor
+  RefWrapper X(p); // expected-warning@61 {{Dereference of null pointer}}
 }
 
 void testReferenceMember2() {
   int *p = 0;
-  RefWrapper X(*p); // should warn here
+  // FIXME: We should warn here, since we're creating the reference here.
+  RefWrapper X(*p); // expected-warning@62 {{Dereference of null pointer}}
 }
+
+
+extern "C" char *strdup(const char *);
+
+class StringWrapper {
+  char *str;
+public:
+  StringWrapper(const char *input) : str(strdup(input)) {} // no-warning
+};