From: Ted Kremenek Date: Thu, 13 Oct 2011 18:50:06 +0000 (+0000) Subject: Tweak -Wuninitialized's handling of 'int x = x' to report that as the root cause... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=9e7617220135a6f6226cf09cb242cc1b905aedb4;p=clang Tweak -Wuninitialized's handling of 'int x = x' to report that as the root cause of an uninitialized variable IFF there are other uses of that uninitialized variable. Fixes . git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@141881 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Analysis/Analyses/UninitializedValues.h b/include/clang/Analysis/Analyses/UninitializedValues.h index badb493a9d..e2e4f35043 100644 --- a/include/clang/Analysis/Analyses/UninitializedValues.h +++ b/include/clang/Analysis/Analyses/UninitializedValues.h @@ -27,10 +27,16 @@ class UninitVariablesHandler { public: UninitVariablesHandler() {} virtual ~UninitVariablesHandler(); - + + /// Called when the uninitialized variable is used at the given expression. virtual void handleUseOfUninitVariable(const Expr *ex, const VarDecl *vd, bool isAlwaysUninit) {} + + /// Called when the uninitialized variable analysis detects the + /// idiom 'int x = x'. All other uses of 'x' within the initializer + /// are handled by handleUseOfUninitVariable. + virtual void handleSelfInit(const VarDecl *vd) {} }; struct UninitVariablesAnalysisStats { diff --git a/lib/Analysis/UninitializedValues.cpp b/lib/Analysis/UninitializedValues.cpp index 8d48fffe9f..9e98560b65 100644 --- a/lib/Analysis/UninitializedValues.cpp +++ b/lib/Analysis/UninitializedValues.cpp @@ -484,11 +484,17 @@ void TransferFunctions::VisitDeclStmt(DeclStmt *ds) { vals[vd] = Uninitialized; lastLoad = 0; lastDR = 0; + if (handler) + handler->handleSelfInit(vd); return; } } // All other cases: treat the new variable as initialized. + // This is a minor optimization to reduce the propagation + // of the analysis, since we will have already reported + // the use of the uninitialized value (which visiting the + // initializer). vals[vd] = Initialized; } } diff --git a/lib/Sema/AnalysisBasedWarnings.cpp b/lib/Sema/AnalysisBasedWarnings.cpp index f461772ba7..babb8af18b 100644 --- a/lib/Sema/AnalysisBasedWarnings.cpp +++ b/lib/Sema/AnalysisBasedWarnings.cpp @@ -470,7 +470,8 @@ static bool SuggestInitializationFixit(Sema &S, const VarDecl *VD) { /// as a warning. If a pariticular use is one we omit warnings for, returns /// false. static bool DiagnoseUninitializedUse(Sema &S, const VarDecl *VD, - const Expr *E, bool isAlwaysUninit) { + const Expr *E, bool isAlwaysUninit, + bool alwaysReportSelfInit = false) { bool isSelfInit = false; if (const DeclRefExpr *DRE = dyn_cast(E)) { @@ -490,7 +491,7 @@ static bool DiagnoseUninitializedUse(Sema &S, const VarDecl *VD, // TODO: Should we suppress maybe-uninitialized warnings for // variables initialized in this way? if (const Expr *Initializer = VD->getInit()) { - if (DRE == Initializer->IgnoreParenImpCasts()) + if (!alwaysReportSelfInit && DRE == Initializer->IgnoreParenImpCasts()) return false; ContainsReference CR(S.Context, DRE); @@ -541,7 +542,7 @@ struct SLocSort { class UninitValsDiagReporter : public UninitVariablesHandler { Sema &S; typedef SmallVector UsesVec; - typedef llvm::DenseMap UsesMap; + typedef llvm::DenseMap > UsesMap; UsesMap *uses; public: @@ -549,17 +550,26 @@ public: ~UninitValsDiagReporter() { flushDiagnostics(); } - - void handleUseOfUninitVariable(const Expr *ex, const VarDecl *vd, - bool isAlwaysUninit) { + + std::pair &getUses(const VarDecl *vd) { if (!uses) uses = new UsesMap(); - - UsesVec *&vec = (*uses)[vd]; + + UsesMap::mapped_type &V = (*uses)[vd]; + UsesVec *&vec = V.first; if (!vec) vec = new UsesVec(); - vec->push_back(std::make_pair(ex, isAlwaysUninit)); + return V; + } + + void handleUseOfUninitVariable(const Expr *ex, const VarDecl *vd, + bool isAlwaysUninit) { + getUses(vd).first->push_back(std::make_pair(ex, isAlwaysUninit)); + } + + void handleSelfInit(const VarDecl *vd) { + getUses(vd).second = true; } void flushDiagnostics() { @@ -568,22 +578,34 @@ public: for (UsesMap::iterator i = uses->begin(), e = uses->end(); i != e; ++i) { const VarDecl *vd = i->first; - UsesVec *vec = i->second; - - // Sort the uses by their SourceLocations. While not strictly - // guaranteed to produce them in line/column order, this will provide - // a stable ordering. - std::sort(vec->begin(), vec->end(), SLocSort()); - - for (UsesVec::iterator vi = vec->begin(), ve = vec->end(); vi != ve; - ++vi) { - if (DiagnoseUninitializedUse(S, vd, vi->first, - /*isAlwaysUninit=*/vi->second)) - // Skip further diagnostics for this variable. We try to warn only on - // the first point at which a variable is used uninitialized. - break; + const UsesMap::mapped_type &V = i->second; + + UsesVec *vec = V.first; + bool hasSelfInit = V.second; + + // Specially handle the case where we have uses of an uninitialized + // variable, but the root cause is an idiomatic self-init. We want + // to report the diagnostic at the self-init since that is the root cause. + if (!vec->empty() && hasSelfInit) + DiagnoseUninitializedUse(S, vd, vd->getInit()->IgnoreParenCasts(), + true, /* alwaysReportSelfInit */ true); + else { + // Sort the uses by their SourceLocations. While not strictly + // guaranteed to produce them in line/column order, this will provide + // a stable ordering. + std::sort(vec->begin(), vec->end(), SLocSort()); + + for (UsesVec::iterator vi = vec->begin(), ve = vec->end(); vi != ve; + ++vi) { + if (DiagnoseUninitializedUse(S, vd, vi->first, + /*isAlwaysUninit=*/vi->second)) + // Skip further diagnostics for this variable. We try to warn only + // on the first point at which a variable is used uninitialized. + break; + } } - + + // Release the uses vector. delete vec; } delete uses; diff --git a/test/Sema/uninit-variables.c b/test/Sema/uninit-variables.c index f716124bcb..49af4f3226 100644 --- a/test/Sema/uninit-variables.c +++ b/test/Sema/uninit-variables.c @@ -94,10 +94,15 @@ void test14() { for (;;) {} } -int test15() { - int x = x; // no-warning: signals intended lack of initialization. \ - // expected-note{{variable 'x' is declared here}} - return x; // expected-warning{{variable 'x' is uninitialized when used here}} +void test15() { + int x = x; // no-warning: signals intended lack of initialization. +} + +int test15b() { + // Warn here with the self-init, since it does result in a use of + // an unintialized variable and this is the root cause. + int x = x; // expected-warning {{variable 'x' is uninitialized when used within its own initialization}} + return x; } // Don't warn in the following example; shows dataflow confluence.