On Tue, Oct 18, 2011 at 19:47, Ted Kremenek <[email protected]> wrote: > Hi Matt, > > How about doing: > > if (!vec->empty() && hasSelfInit && hasAlwaysUninitializedUse(vec)) > DiagnoseUninitializedUse(S, vd, vd->getInit()->IgnoreParenCasts(), > true, /* alwaysReportSelfInit */ true); > > In this case, we only warn at the initialization when the value is always > unconditionally uninitialized. That seems reasonable since it is the root > cause. We then let the other cases be handled by the other logic. > > What do you think?
Sounds good, I'll commit something to that effect tomorrow (unless you get to it first). > > On Oct 18, 2011, at 7:00 PM, Matt Beaumont-Gay wrote: > >> Hi Ted, >> >> On Thu, Oct 13, 2011 at 11:50, Ted Kremenek <[email protected]> wrote: >>> Author: kremenek >>> Date: Thu Oct 13 13:50:06 2011 >>> New Revision: 141881 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=141881&view=rev >>> Log: >>> 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 <rdar://problem/9259237>. >> >> When this new logic fires, it makes no distinction between >> conditionally uninitialized and always uninitialized uses. (This is >> important to me because we build with -Wuninitialized >> -Wnoconditional-uninitialized.) As a quick hack, the attached patch >> downgrades the warning to "may be uninitialized" when appropriate. The >> diagnostic is pretty bad, though, since it points at the declaration >> and not at the site of maybe-uninitialized use, so the patch needs >> some more work. Do you have any suggestions? >> >> -Matt >> >>> >>> Modified: >>> cfe/trunk/include/clang/Analysis/Analyses/UninitializedValues.h >>> cfe/trunk/lib/Analysis/UninitializedValues.cpp >>> cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp >>> cfe/trunk/test/Sema/uninit-variables.c >>> >>> Modified: cfe/trunk/include/clang/Analysis/Analyses/UninitializedValues.h >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/UninitializedValues.h?rev=141881&r1=141880&r2=141881&view=diff >>> ============================================================================== >>> --- cfe/trunk/include/clang/Analysis/Analyses/UninitializedValues.h >>> (original) >>> +++ cfe/trunk/include/clang/Analysis/Analyses/UninitializedValues.h Thu Oct >>> 13 13:50:06 2011 >>> @@ -27,10 +27,16 @@ >>> 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 { >>> >>> Modified: cfe/trunk/lib/Analysis/UninitializedValues.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/UninitializedValues.cpp?rev=141881&r1=141880&r2=141881&view=diff >>> ============================================================================== >>> --- cfe/trunk/lib/Analysis/UninitializedValues.cpp (original) >>> +++ cfe/trunk/lib/Analysis/UninitializedValues.cpp Thu Oct 13 13:50:06 2011 >>> @@ -484,11 +484,17 @@ >>> 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; >>> } >>> } >>> >>> Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=141881&r1=141880&r2=141881&view=diff >>> ============================================================================== >>> --- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original) >>> +++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Thu Oct 13 13:50:06 2011 >>> @@ -470,7 +470,8 @@ >>> /// 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<DeclRefExpr>(E)) { >>> @@ -490,7 +491,7 @@ >>> // 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 @@ >>> class UninitValsDiagReporter : public UninitVariablesHandler { >>> Sema &S; >>> typedef SmallVector<UninitUse, 2> UsesVec; >>> - typedef llvm::DenseMap<const VarDecl *, UsesVec*> UsesMap; >>> + typedef llvm::DenseMap<const VarDecl *, std::pair<UsesVec*, bool> > >>> UsesMap; >>> UsesMap *uses; >>> >>> public: >>> @@ -549,17 +550,26 @@ >>> ~UninitValsDiagReporter() { >>> flushDiagnostics(); >>> } >>> - >>> - void handleUseOfUninitVariable(const Expr *ex, const VarDecl *vd, >>> - bool isAlwaysUninit) { >>> + >>> + std::pair<UsesVec*, bool> &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 @@ >>> >>> for (UsesMap::iterator i = uses->begin(), e = uses->end(); i != e; ++i) >>> { >>> const VarDecl *vd = i->first; >>> - UsesVec *vec = i->second; >>> + const UsesMap::mapped_type &V = 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; >>> - } >>> + 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; >>> >>> Modified: cfe/trunk/test/Sema/uninit-variables.c >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/uninit-variables.c?rev=141881&r1=141880&r2=141881&view=diff >>> ============================================================================== >>> --- cfe/trunk/test/Sema/uninit-variables.c (original) >>> +++ cfe/trunk/test/Sema/uninit-variables.c Thu Oct 13 13:50:06 2011 >>> @@ -94,10 +94,15 @@ >>> 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. >>> >>> >>> _______________________________________________ >>> cfe-commits mailing list >>> [email protected] >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>> >> <self-init.patch> > > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
