[cfe-commits] r141881 - in /cfe/trunk: include/clang/Analysis/Analyses/UninitializedValues.h lib/Analysis/UninitializedValues.cpp lib/Sema/AnalysisBasedWarnings.cpp test/Sema/uninit-variables.c
Matt Beaumont-Gay
matthewbg at google.com
Tue Oct 18 19:00:18 PDT 2011
Hi Ted,
On Thu, Oct 13, 2011 at 11:50, Ted Kremenek <kremenek at apple.com> 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: self-init.patch
Type: text/x-patch
Size: 1902 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20111018/2c8ca58d/attachment.bin>
More information about the cfe-commits
mailing list