From ba8071ec81eb5c39f8ebb869a18181a1ed25c782 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Thu, 12 Sep 2013 18:49:10 +0000 Subject: 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 --- .../clang/Analysis/Analyses/UninitializedValues.h | 22 +++++++- clang/include/clang/Basic/DiagnosticSemaKinds.td | 5 +- clang/lib/Analysis/UninitializedValues.cpp | 18 ++++++- clang/lib/Sema/AnalysisBasedWarnings.cpp | 52 +++++++++++++++---- clang/test/Analysis/uninit-sometimes.cpp | 58 +++++++++++++++++++--- 5 files changed, 133 insertions(+), 22 deletions(-) (limited to 'clang') 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 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, DefaultIgnore; + "switch %3 is taken|" + "its declaration is reached|" + "%3 is called}2">, + InGroup, 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(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(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; + } +} -- cgit v1.1