[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
Ted Kremenek
kremenek at apple.com
Tue Oct 18 19:47:55 PDT 2011
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?
On Oct 18, 2011, at 7:00 PM, Matt Beaumont-Gay wrote:
> 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
>>
> <self-init.patch>
More information about the cfe-commits
mailing list