PR16054: Slight strengthening for -Wsometimes-uninitialized: if we use a

variable uninitialized every time we reach its (reachable) declaration, or
every time we call the surrounding function, promote the warning from
-Wmaybe-uninitialized to -Wsometimes-uninitialized.

This is still slightly weaker than desired: we should, in general, warn
if a use is uninitialized the first time it is evaluated.

llvm-svn: 190623

5 files changed, 133 insertions, 22 deletions

diff --git a/clang/include/clang/Analysis/Analyses/UninitializedValues.h b/clang/include/clang/Analysis/Analyses/UninitializedValues.h
index e8810c3..188722d 100644
--- a/clang/include/clang/Analysis/Analyses/UninitializedValues.h
+++ b/clang/include/clang/Analysis/Analyses/UninitializedValues.h
@@ -38,6 +38,12 @@ private:
   /// The expression which uses this variable.
   const Expr *User;
 
+  /// Is this use uninitialized whenever the function is called?
+  bool UninitAfterCall;
+
+  /// Is this use uninitialized whenever the variable declaration is reached?
+  bool UninitAfterDecl;
+
   /// Does this use always see an uninitialized value?
   bool AlwaysUninit;
 
@@ -46,13 +52,17 @@ private:
   SmallVector<Branch, 2> UninitBranches;
 
 public:
-  UninitUse(const Expr *User, bool AlwaysUninit) :
-    User(User), AlwaysUninit(AlwaysUninit) {}
+  UninitUse(const Expr *User, bool AlwaysUninit)
+      : User(User), UninitAfterCall(false), UninitAfterDecl(false),
+        AlwaysUninit(AlwaysUninit) {}
 
   void addUninitBranch(Branch B) {
     UninitBranches.push_back(B);
   }
 
+  void setUninitAfterCall() { UninitAfterCall = true; }
+  void setUninitAfterDecl() { UninitAfterDecl = true; }
+
   /// Get the expression containing the uninitialized use.
   const Expr *getUser() const { return User; }
 
@@ -62,6 +72,12 @@ public:
     Maybe,
     /// The use is uninitialized whenever a certain branch is taken.
     Sometimes,
+    /// The use is uninitialized the first time it is reached after we reach
+    /// the variable's declaration.
+    AfterDecl,
+    /// The use is uninitialized the first time it is reached after the function
+    /// is called.
+    AfterCall,
     /// The use is always uninitialized.
     Always
   };
@@ -69,6 +85,8 @@ public:
   /// Get the kind of uninitialized use.
   Kind getKind() const {
     return AlwaysUninit ? Always :
+           UninitAfterCall ? AfterCall :
+           UninitAfterDecl ? AfterDecl :
            !branch_empty() ? Sometimes : Maybe;
   }
 
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index ee82137..2932fe1 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1388,7 +1388,10 @@ def warn_sometimes_uninit_var : Warning<
   "%select{'%3' condition is %select{true|false}4|"
   "'%3' loop %select{is entered|exits because its condition is false}4|"
   "'%3' loop %select{condition is true|exits because its condition is false}4|"
-  "switch %3 is taken}2">, InGroup<UninitializedSometimes>, DefaultIgnore;
+  "switch %3 is taken|"
+  "its declaration is reached|"
+  "%3 is called}2">,
+  InGroup<UninitializedSometimes>, DefaultIgnore;
 def warn_maybe_uninit_var : Warning<
   "variable %0 may be uninitialized when "
   "%select{used here|captured by block}1">,
diff --git a/clang/lib/Analysis/UninitializedValues.cpp b/clang/lib/Analysis/UninitializedValues.cpp
index 6c6804fe..332c02c 100644
--- a/clang/lib/Analysis/UninitializedValues.cpp
+++ b/clang/lib/Analysis/UninitializedValues.cpp
@@ -527,12 +527,28 @@ public:
     SuccsVisited[block->getBlockID()] = block->succ_size();
     while (!Queue.empty()) {
       const CFGBlock *B = Queue.pop_back_val();
+
+      // If the use is always reached from the entry block, make a note of that.
+      if (B == &cfg.getEntry())
+        Use.setUninitAfterCall();
+
       for (CFGBlock::const_pred_iterator I = B->pred_begin(), E = B->pred_end();
            I != E; ++I) {
         const CFGBlock *Pred = *I;
-        if (vals.getValue(Pred, B, vd) == Initialized)
+        Value AtPredExit = vals.getValue(Pred, B, vd);
+        if (AtPredExit == Initialized)
           // This block initializes the variable.
           continue;
+        if (AtPredExit == MayUninitialized &&
+            vals.getValue(B, 0, vd) == Uninitialized) {
+          // This block declares the variable (uninitialized), and is reachable
+          // from a block that initializes the variable. We can't guarantee to
+          // give an earlier location for the diagnostic (and it appears that
+          // this code is intended to be reachable) so give a diagnostic here
+          // and go no further down this path.
+          Use.setUninitAfterDecl();
+          continue;
+        }
 
         unsigned &SV = SuccsVisited[Pred->getBlockID()];
         if (!SV) {
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 599c486..7893a45 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -493,6 +493,31 @@ static void DiagUninitUse(Sema &S, const VarDecl *VD, const UninitUse &Use,
                           bool IsCapturedByBlock) {
   bool Diagnosed = false;
 
+  switch (Use.getKind()) {
+  case UninitUse::Always:
+    S.Diag(Use.getUser()->getLocStart(), diag::warn_uninit_var)
+        << VD->getDeclName() << IsCapturedByBlock
+        << Use.getUser()->getSourceRange();
+    return;
+
+  case UninitUse::AfterDecl:
+  case UninitUse::AfterCall:
+    S.Diag(VD->getLocation(), diag::warn_sometimes_uninit_var)
+      << VD->getDeclName() << IsCapturedByBlock
+      << (Use.getKind() == UninitUse::AfterDecl ? 4 : 5)
+      << const_cast<DeclContext*>(VD->getLexicalDeclContext())
+      << VD->getSourceRange();
+    S.Diag(Use.getUser()->getLocStart(), diag::note_uninit_var_use)
+      << IsCapturedByBlock << Use.getUser()->getSourceRange();
+    return;
+
+  case UninitUse::Maybe:
+  case UninitUse::Sometimes:
+    // Carry on to report sometimes-uninitialized branches, if possible,
+    // or a 'may be used uninitialized' diagnostic otherwise.
+    break;
+  }
+
   // Diagnose each branch which leads to a sometimes-uninitialized use.
   for (UninitUse::branch_iterator I = Use.branch_begin(), E = Use.branch_end();
        I != E; ++I) {
@@ -515,14 +540,10 @@ static void DiagUninitUse(Sema &S, const VarDecl *VD, const UninitUse &Use,
                                   : (I->Output ? "1" : "0");
     FixItHint Fixit1, Fixit2;
 
-    switch (Term->getStmtClass()) {
+    switch (Term ? Term->getStmtClass() : Stmt::DeclStmtClass) {
     default:
       // Don't know how to report this. Just fall back to 'may be used
-      // uninitialized'. This happens for range-based for, which the user
-      // can't explicitly fix.
-      // FIXME: This also happens if the first use of a variable is always
-      // uninitialized, eg "for (int n; n < 10; ++n)". We should report that
-      // with the 'is uninitialized' diagnostic.
+      // uninitialized'. FIXME: Can this happen?
       continue;
 
     // "condition is true / condition is false".
@@ -583,6 +604,17 @@ static void DiagUninitUse(Sema &S, const VarDecl *VD, const UninitUse &Use,
       else
         Fixit1 = FixItHint::CreateReplacement(Range, FixitStr);
       break;
+    case Stmt::CXXForRangeStmtClass:
+      if (I->Output == 1) {
+        // The use occurs if a range-based for loop's body never executes.
+        // That may be impossible, and there's no syntactic fix for this,
+        // so treat it as a 'may be uninitialized' case.
+        continue;
+      }
+      DiagKind = 1;
+      Str = "for";
+      Range = cast<CXXForRangeStmt>(Term)->getRangeInit()->getSourceRange();
+      break;
 
     // "condition is true / loop is exited".
     case Stmt::DoStmtClass:
@@ -619,9 +651,7 @@ static void DiagUninitUse(Sema &S, const VarDecl *VD, const UninitUse &Use,
   }
 
   if (!Diagnosed)
-    S.Diag(Use.getUser()->getLocStart(),
-           Use.getKind() == UninitUse::Always ? diag::warn_uninit_var
-                                              : diag::warn_maybe_uninit_var)
+    S.Diag(Use.getUser()->getLocStart(), diag::warn_maybe_uninit_var)
         << VD->getDeclName() << IsCapturedByBlock
         << Use.getUser()->getSourceRange();
 }
@@ -1233,7 +1263,9 @@ public:
 private:
   static bool hasAlwaysUninitializedUse(const UsesVec* vec) {
   for (UsesVec::const_iterator i = vec->begin(), e = vec->end(); i != e; ++i) {
-    if (i->getKind() == UninitUse::Always) {
+    if (i->getKind() == UninitUse::Always ||
+        i->getKind() == UninitUse::AfterCall ||
+        i->getKind() == UninitUse::AfterDecl) {
       return true;
     }
   }
diff --git a/clang/test/Analysis/uninit-sometimes.cpp b/clang/test/Analysis/uninit-sometimes.cpp
index 015b675..425d304 100644
--- a/clang/test/Analysis/uninit-sometimes.cpp
+++ b/clang/test/Analysis/uninit-sometimes.cpp
@@ -145,13 +145,13 @@ int test_for_range_false(int k) {
 
 int test_for_range_true(int k) {
   int arr[3] = { 1, 2, 3 };
-  int x;
-  for (int &a : arr) { // no-warning
+  int x; // expected-note {{variable}}
+  for (int &a : arr) { // expected-warning {{variable 'x' is used uninitialized whenever 'for' loop is entered}}
     goto label;
   }
   x = 0;
 label:
-  return x;
+  return x; // expected-note {{uninitialized use}}
 }
 
 
@@ -356,14 +356,14 @@ int test_no_false_positive_2() {
 }
 
 
-// FIXME: In this case, the variable is used uninitialized whenever the
-// function's entry block is reached. Produce a diagnostic saying that
-// the variable is uninitialized the first time it is used.
+
+
+
 void test_null_pred_succ() {
-  int x;
+  int x; // expected-note {{variable}} expected-warning {{used uninitialized whenever function 'test_null_pred_succ' is called}}
   if (0)
     foo: x = 0;
-  if (x)
+  if (x) // expected-note {{use}}
     goto foo;
 }
 
@@ -385,3 +385,45 @@ int PR13360(bool b) {
 
 // CHECK: fix-it:"{{.*}}":{376:3-380:10}:""
 // CHECK: fix-it:"{{.*}}":{375:8-375:8}:" = 0"
+
+void test_jump_init() {
+goto later;
+  int x; // expected-note {{variable}} expected-warning {{used uninitialized whenever function 'test_jump_init'}}
+later:
+  while (x) x = 0; // expected-note {{use}}
+}
+
+void PR16054() {
+  int x; // expected-note {{variable}} expected-warning {{used uninitialized whenever function 'PR16054}}
+  while (x != 0) { // expected-note {{use}}
+    (void)&x;
+  }
+}
+
+void test_loop_uninit() {
+  for (int n = 0; n < 10; ++n) {
+    int k; // expected-warning {{variable 'k' is used uninitialized whenever its declaration is reached}} expected-note {{variable}}
+    do {
+      k = k + 1; // expected-note {{use}}
+    } while (k != 5);
+  }
+}
+
+// FIXME: We should warn here, because the variable is used uninitialized
+// the first time we encounter the use.
+void test_loop_with_assignment() {
+  double d;
+  for (int n = 0; n < 10; ++n) {
+    d = d + n;
+  }
+}
+
+// FIXME: We should warn here, because the variable is used uninitialized
+// the first time we encounter the use.
+void test_loop_with_ref_bind() {
+  double d;
+  for (int n = 0; n < 10; ++n) {
+    d += n;
+    const double &r = d;
+  }
+}