From: Richard Smith Date: Fri, 13 Jul 2012 23:33:44 +0000 (+0000) Subject: PR13360: When deciding the earliest point which inevitably leads to an X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=558e8872b364b43ab9f201dd6b2df9a5b74b0542;p=clang PR13360: When deciding the earliest point which inevitably leads to an uninitialized variable use, walk back over branches where we've reached all the non-null successors, not just cases where we've reached all successors. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@160206 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Analysis/UninitializedValues.cpp b/lib/Analysis/UninitializedValues.cpp index 1f97c58946..c561b601a7 100644 --- a/lib/Analysis/UninitializedValues.cpp +++ b/lib/Analysis/UninitializedValues.cpp @@ -16,6 +16,7 @@ #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/PackedVector.h" #include "llvm/ADT/DenseMap.h" +#include "clang/AST/ASTContext.h" #include "clang/AST/Decl.h" #include "clang/Analysis/CFG.h" #include "clang/Analysis/AnalysisContext.h" @@ -25,6 +26,8 @@ using namespace clang; +#define DEBUG_LOGGING 0 + static bool isTrackedVar(const VarDecl *vd, const DeclContext *dc) { if (vd->isLocalVarDecl() && !vd->hasGlobalStorage() && !vd->isExceptionVariable() && @@ -221,24 +224,15 @@ BVPair &CFGBlockValues::getValueVectors(const clang::CFGBlock *block, return vals[idx]; } -#if 0 +#if DEBUG_LOGGING static void printVector(const CFGBlock *block, ValueVector &bv, unsigned num) { - llvm::errs() << block->getBlockID() << " :"; for (unsigned i = 0; i < bv.size(); ++i) { llvm::errs() << ' ' << bv[i]; } llvm::errs() << " : " << num << '\n'; } - -static void printVector(const char *name, ValueVector const &bv) { - llvm::errs() << name << " : "; - for (unsigned i = 0; i < bv.size(); ++i) { - llvm::errs() << ' ' << bv[i]; - } - llvm::errs() << "\n"; -} #endif void CFGBlockValues::setAllScratchValues(Value V) { @@ -259,7 +253,7 @@ bool CFGBlockValues::updateValueVectorWithScratch(const CFGBlock *block) { bool changed = (dst != scratch); if (changed) dst = scratch; -#if 0 +#if DEBUG_LOGGING printVector(block, scratch, 0); #endif return changed; @@ -272,7 +266,7 @@ bool CFGBlockValues::updateValueVectors(const CFGBlock *block, *newVals.second != *vals.second; *vals.first = *newVals.first; *vals.second = *newVals.second; -#if 0 +#if DEBUG_LOGGING printVector(block, *vals.first, 1); printVector(block, *vals.second, 2); #endif @@ -463,7 +457,18 @@ public: // This block initializes the variable. continue; - if (++SuccsVisited[Pred->getBlockID()] == Pred->succ_size()) + unsigned &SV = SuccsVisited[Pred->getBlockID()]; + if (!SV) { + // When visiting the first successor of a block, mark all NULL + // successors as having been visited. + for (CFGBlock::const_succ_iterator SI = Pred->succ_begin(), + SE = Pred->succ_end(); + SI != SE; ++SI) + if (!*SI) + ++SV; + } + + if (++SV == Pred->succ_size()) // All paths from this block lead to the use and don't initialize the // variable. Queue.push_back(Pred); @@ -831,7 +836,7 @@ void clang::runUninitializedVariablesAnalysis( vals.computeSetOfDeclarations(dc); if (vals.hasNoDeclarations()) return; -#if 0 +#if DEBUG_LOGGING cfg.dump(dc.getParentASTContext().getLangOpts(), true); #endif diff --git a/test/Analysis/uninit-sometimes.cpp b/test/Analysis/uninit-sometimes.cpp index 9667e2a65c..7825e87346 100644 --- a/test/Analysis/uninit-sometimes.cpp +++ b/test/Analysis/uninit-sometimes.cpp @@ -356,16 +356,32 @@ 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; + if (0) + foo: x = 0; + if (x) + goto foo; +} -void test_null_pred_succ() { + +void foo(); +int PR13360(bool b) { int x; // expected-note {{variable}} - if (0) // expected-warning {{whenever}} expected-note {{remove}} - foo: x = 0; - if (x) // expected-note {{uninitialized use}} - goto foo; + if (b) { // expected-warning {{variable 'x' is used uninitialized whenever 'if' condition is true}} expected-note {{remove}} + do { + foo(); + } while (0); + } else { + x = 1; + } + return x; // expected-note {{uninitialized use occurs here}} } -// CHECK: fix-it:"{{.*}}":{364:3-365:5}:"" -// CHECK: fix-it:"{{.*}}":{363:8-363:8}:" = 0" +// CHECK: fix-it:"{{.*}}":{376:3-380:10}:"" +// CHECK: fix-it:"{{.*}}":{375:8-375:8}:" = 0"