Fix PR 9626 (duplicated self-init warnings under -Wuninitialized) with numerous CFG and UninitializedValues analysis changes:

1) Change the CFG to include the DeclStmt for conditional variables, instead of using the condition itself as a faux DeclStmt.
2) Update ExprEngine (the static analyzer) to understand (1), so not to regress.
3) Update UninitializedValues.cpp to initialize all tracked variables to Uninitialized at the start of the function/method.
4) Only use the SelfReferenceChecker (SemaDecl.cpp) on global variables, leaving the dataflow analysis to handle other cases.

The combination of (1) and (3) allows the dataflow-based -Wuninitialized to find self-init problems when the initializer
contained control-flow.

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

5 files changed, 58 insertions, 69 deletions

diff --git a/lib/Analysis/CFG.cpp b/lib/Analysis/CFG.cpp
index e91de0366c..b5930cdccf 100644
--- a/lib/Analysis/CFG.cpp
+++ b/lib/Analysis/CFG.cpp
@@ -393,11 +393,11 @@ private:
   void addLocalScopeAndDtors(Stmt* S);
 
   // Interface to CFGBlock - adding CFGElements.
-  void appendStmt(CFGBlock *B, Stmt *S) {
+  void appendStmt(CFGBlock *B, const Stmt *S) {
     if (alwaysAdd(S))
       cachedEntry->second = B;
 
-    B->appendStmt(S, cfg->getBumpVectorContext());
+    B->appendStmt(const_cast<Stmt*>(S), cfg->getBumpVectorContext());
   }
   void appendInitializer(CFGBlock *B, CXXCtorInitializer *I) {
     B->appendInitializer(I, cfg->getBumpVectorContext());
@@ -1495,7 +1495,7 @@ CFGBlock* CFGBuilder::VisitIfStmt(IfStmt* I) {
   if (VarDecl *VD = I->getConditionVariable()) {
     if (Expr *Init = VD->getInit()) {
       autoCreateBlock();
-      appendStmt(Block, I);
+      appendStmt(Block, I->getConditionVariableDeclStmt());
       addStmt(Init);
     }
   }
@@ -1633,7 +1633,7 @@ CFGBlock* CFGBuilder::VisitForStmt(ForStmt* F) {
     if (VarDecl *VD = F->getConditionVariable()) {
       if (Expr *Init = VD->getInit()) {
         autoCreateBlock();
-        appendStmt(Block, F);
+        appendStmt(Block, F->getConditionVariableDeclStmt());
         EntryConditionBlock = addStmt(Init);
         assert(Block == EntryConditionBlock);
       }
@@ -1917,7 +1917,7 @@ CFGBlock* CFGBuilder::VisitWhileStmt(WhileStmt* W) {
     if (VarDecl *VD = W->getConditionVariable()) {
       if (Expr *Init = VD->getInit()) {
         autoCreateBlock();
-        appendStmt(Block, W);
+        appendStmt(Block, W->getConditionVariableDeclStmt());        
         EntryConditionBlock = addStmt(Init);
         assert(Block == EntryConditionBlock);
       }
@@ -2279,7 +2279,7 @@ CFGBlock* CFGBuilder::VisitSwitchStmt(SwitchStmt* Terminator) {
   if (VarDecl *VD = Terminator->getConditionVariable()) {
     if (Expr *Init = VD->getInit()) {
       autoCreateBlock();
-      appendStmt(Block, Terminator);
+      appendStmt(Block, Terminator->getConditionVariableDeclStmt());
       addStmt(Init);
     }
   }
diff --git a/lib/Analysis/UninitializedValues.cpp b/lib/Analysis/UninitializedValues.cpp
index f775cc5b50..3dde41f227 100644
--- a/lib/Analysis/UninitializedValues.cpp
+++ b/lib/Analysis/UninitializedValues.cpp
@@ -138,6 +138,8 @@ public:
   CFGBlockValues(const CFG &cfg);
   ~CFGBlockValues();
   
+  unsigned getNumEntries() const { return declToIndex.size(); }
+  
   void computeSetOfDeclarations(const DeclContext &dc);  
   ValueVector &getValueVector(const CFGBlock *block,
                                 const CFGBlock *dstBlock);
@@ -470,7 +472,6 @@ void TransferFunctions::VisitDeclStmt(DeclStmt *ds) {
        DI != DE; ++DI) {
     if (VarDecl *vd = dyn_cast<VarDecl>(*DI)) {
       if (isTrackedVar(vd)) {
-        vals[vd] = Uninitialized;
         if (Stmt *init = vd->getInit()) {
           Visit(init);
           vals[vd] = Initialized;
@@ -669,9 +670,23 @@ void clang::runUninitializedVariablesAnalysis(const DeclContext &dc,
   vals.computeSetOfDeclarations(dc);
   if (vals.hasNoDeclarations())
     return;
+
+  // Mark all variables uninitialized at the entry.
+  const CFGBlock &entry = cfg.getEntry();
+  for (CFGBlock::const_succ_iterator i = entry.succ_begin(), 
+        e = entry.succ_end(); i != e; ++i) {
+    if (const CFGBlock *succ = *i) {
+      ValueVector &vec = vals.getValueVector(&entry, succ);
+      const unsigned n = vals.getNumEntries();
+      for (unsigned j = 0; j < n ; ++j) {
+        vec[j] = Uninitialized;
+      }
+    }
+  }
+
+  // Proceed with the workist.
   DataflowWorklist worklist(cfg);
   llvm::BitVector previouslyVisited(cfg.getNumBlockIDs());
-  
   worklist.enqueueSuccessors(&cfg.getEntry());
   llvm::BitVector wasAnalyzed(cfg.getNumBlockIDs(), false);
 
diff --git a/lib/Sema/AnalysisBasedWarnings.cpp b/lib/Sema/AnalysisBasedWarnings.cpp
index 3467dae4ba..ecfa821358 100644
--- a/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/lib/Sema/AnalysisBasedWarnings.cpp
@@ -470,14 +470,25 @@ public:
       for (UsesVec::iterator vi = vec->begin(), ve = vec->end(); vi != ve; ++vi)
       {
         const bool isAlwaysUninit = vi->second;
+        bool showDefinition = true;
+
         if (const DeclRefExpr *dr = dyn_cast<DeclRefExpr>(vi->first)) {
-          S.Diag(dr->getLocStart(),
-                 isAlwaysUninit ?
-                  (isSelfInit(S.Context, vd, dr) 
-                    ? diag::warn_uninit_self_reference_in_init
-                    : diag::warn_uninit_var)
-                  : diag::warn_maybe_uninit_var)
-            << vd->getDeclName() << dr->getSourceRange();          
+          if (isAlwaysUninit) {
+            if (isSelfInit(S.Context, vd, dr)) {
+              S.Diag(dr->getLocStart(), 
+                     diag::warn_uninit_self_reference_in_init)
+              << vd->getDeclName() << vd->getLocation() << dr->getSourceRange();             
+              showDefinition = false;
+            }
+            else {
+              S.Diag(dr->getLocStart(), diag::warn_uninit_var)
+                << vd->getDeclName() << dr->getSourceRange();          
+            }
+          }
+          else {
+            S.Diag(dr->getLocStart(), diag::warn_maybe_uninit_var)
+              << vd->getDeclName() << dr->getSourceRange();          
+          }          
         }
         else {
           const BlockExpr *be = cast<BlockExpr>(vi->first);
@@ -488,8 +499,9 @@ public:
         }
         
         // Report where the variable was declared.
-        S.Diag(vd->getLocStart(), diag::note_uninit_var_def)
-          << vd->getDeclName();
+        if (showDefinition)
+          S.Diag(vd->getLocStart(), diag::note_uninit_var_def)
+            << vd->getDeclName();
 
         // Only report the fixit once.
         if (fixitIssued)
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index e52082bbbf..8bf6c84c0b 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -4715,7 +4715,16 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init,
   if (RealDecl == 0 || RealDecl->isInvalidDecl())
     return;
 
-  SelfReferenceChecker(*this, RealDecl).VisitExpr(Init);
+  // Check for self-references within variable initializers.
+  if (VarDecl *vd = dyn_cast<VarDecl>(RealDecl)) {
+    // Variables declared within a function/method body are handled
+    // by a dataflow analysis.
+    if (!vd->hasLocalStorage() && !vd->isStaticLocal())
+      SelfReferenceChecker(*this, RealDecl).VisitExpr(Init);    
+  }
+  else {
+    SelfReferenceChecker(*this, RealDecl).VisitExpr(Init);
+  }
 
   if (CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(RealDecl)) {
     // With declarators parsed the way they are, the parser cannot
diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 3826a12226..a422428b1b 100644
--- a/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -459,11 +459,15 @@ void ExprEngine::Visit(const Stmt* S, ExplodedNode* Pred,
     case Stmt::ContinueStmtClass:
     case Stmt::DefaultStmtClass:
     case Stmt::DoStmtClass:
+    case Stmt::ForStmtClass:
     case Stmt::GotoStmtClass:
+    case Stmt::IfStmtClass:
     case Stmt::IndirectGotoStmtClass:
     case Stmt::LabelStmtClass:
     case Stmt::NoStmtClass:
     case Stmt::NullStmtClass:
+    case Stmt::SwitchStmtClass:
+    case Stmt::WhileStmtClass:
       llvm_unreachable("Stmt should not be in analyzer evaluation loop");
       break;
 
@@ -620,12 +624,6 @@ void ExprEngine::Visit(const Stmt* S, ExplodedNode* Pred,
       VisitDeclStmt(cast<DeclStmt>(S), Pred, Dst);
       break;
 
-    case Stmt::ForStmtClass:
-      // This case isn't for branch processing, but for handling the
-      // initialization of a condition variable.
-      VisitCondInit(cast<ForStmt>(S)->getConditionVariable(), S, Pred, Dst);
-      break;
-
     case Stmt::ImplicitCastExprClass:
     case Stmt::CStyleCastExprClass:
     case Stmt::CXXStaticCastExprClass:
@@ -638,12 +636,6 @@ void ExprEngine::Visit(const Stmt* S, ExplodedNode* Pred,
       break;
     }
 
-    case Stmt::IfStmtClass:
-      // This case isn't for branch processing, but for handling the
-      // initialization of a condition variable.
-      VisitCondInit(cast<IfStmt>(S)->getConditionVariable(), S, Pred, Dst);
-      break;
-
     case Stmt::InitListExprClass:
       VisitInitListExpr(cast<InitListExpr>(S), Pred, Dst);
       break;
@@ -713,12 +705,6 @@ void ExprEngine::Visit(const Stmt* S, ExplodedNode* Pred,
       return;
     }
 
-    case Stmt::SwitchStmtClass:
-      // This case isn't for branch processing, but for handling the
-      // initialization of a condition variable.
-      VisitCondInit(cast<SwitchStmt>(S)->getConditionVariable(), S, Pred, Dst);
-      break;
-
     case Stmt::UnaryOperatorClass: {
       const UnaryOperator *U = cast<UnaryOperator>(S);
       if (AMgr.shouldEagerlyAssume()&&(U->getOpcode() == UO_LNot)) {
@@ -730,12 +716,6 @@ void ExprEngine::Visit(const Stmt* S, ExplodedNode* Pred,
         VisitUnaryOperator(U, Pred, Dst);
       break;
     }
-
-    case Stmt::WhileStmtClass:
-      // This case isn't for branch processing, but for handling the
-      // initialization of a condition variable.
-      VisitCondInit(cast<WhileStmt>(S)->getConditionVariable(), S, Pred, Dst);
-      break;
   }
 }
 
@@ -2308,33 +2288,6 @@ void ExprEngine::VisitDeclStmt(const DeclStmt *DS, ExplodedNode *Pred,
   }
 }
 
-void ExprEngine::VisitCondInit(const VarDecl *VD, const Stmt *S,
-                                 ExplodedNode *Pred, ExplodedNodeSet& Dst) {
-
-  const Expr* InitEx = VD->getInit();
-  ExplodedNodeSet Tmp;
-  Visit(InitEx, Pred, Tmp);
-
-  for (ExplodedNodeSet::iterator I=Tmp.begin(), E=Tmp.end(); I!=E; ++I) {
-    ExplodedNode *N = *I;
-    const GRState *state = GetState(N);
-
-    const LocationContext *LC = N->getLocationContext();
-    SVal InitVal = state->getSVal(InitEx);
-
-    // Recover some path-sensitivity if a scalar value evaluated to
-    // UnknownVal.
-    if (InitVal.isUnknown() ||
-        !getConstraintManager().canReasonAbout(InitVal)) {
-      InitVal = svalBuilder.getConjuredSymbolVal(NULL, InitEx,
-                                            Builder->getCurrentBlockCount());
-    }
-
-    evalBind(Dst, S, N, state,
-             loc::MemRegionVal(state->getRegion(VD, LC)), InitVal, true);
-  }
-}
-
 namespace {
   // This class is used by VisitInitListExpr as an item in a worklist
   // for processing the values contained in an InitListExpr.