[cfe-commits] r128894 - in /cfe/trunk: lib/Sema/AnalysisBasedWarnings.cpp test/Sema/uninit-variables.c test/SemaCXX/uninitialized.cpp
Ted Kremenek
kremenek at apple.com
Tue Apr 5 11:11:43 PDT 2011
Hi Chandler,
The only problem with this approach is that it won't flag:
int foo() {
int x = x; // don't warn here
return x; // warn here
}
The problem is that the analysis currently treats 'x' to be initialized, regardless of whatever is in the initializer. This is done to halt the propagation of uninitialized values warnings, and only report the earliest instance. The real fix probably needs to be done in the analysis (UninitializedValues.cpp), not in the error reporting side in AnalysisBasedWarnings.cpp.
As an aside, with respect to test cases, whenever I add something to an analysis to suppress a warning in a specific case, I always try to add a matching "positive" case (where a warning is present) to match the negative case. If we just test the negative case, then we aren't verifying that the analysis still actually works for the other cases.
Cheers,
Ted
On Apr 5, 2011, at 10:41 AM, Chandler Carruth wrote:
> Author: chandlerc
> Date: Tue Apr 5 12:41:31 2011
> New Revision: 128894
>
> URL: http://llvm.org/viewvc/llvm-project?rev=128894&view=rev
> Log:
> Fix PR9624 by explicitly disabling uninitialized warnings for direct self-init:
>
> int x = x;
>
> GCC disables its warnings on this construct as a way of indicating that
> the programmer intentionally wants the variable to be uninitialized.
> Only the warning on the initializer is turned off in this iteration.
>
> This makes the code a lot more ugly, but starts commenting the
> surprising behavior here. This is a WIP, I want to refactor it
> substantially for clarity, and to determine whether subsequent warnings
> should be suppressed or not.
>
> Modified:
> cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
> cfe/trunk/test/Sema/uninit-variables.c
> cfe/trunk/test/SemaCXX/uninitialized.cpp
>
> Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=128894&r1=128893&r2=128894&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original)
> +++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Tue Apr 5 12:41:31 2011
> @@ -410,16 +410,6 @@
> };
> }
>
> -static bool isSelfInit(ASTContext &Context,
> - const VarDecl *VD, const DeclRefExpr *DR) {
> - if (const Expr *E = VD->getInit()) {
> - ContainsReference CR(Context, DR);
> - CR.Visit(const_cast<Expr*>(E));
> - return CR.doesContainReference();
> - }
> - return false;
> -}
> -
> typedef std::pair<const Expr*, bool> UninitUse;
>
> namespace {
> @@ -473,17 +463,37 @@
> for (UsesVec::iterator vi = vec->begin(), ve = vec->end(); vi != ve; ++vi)
> {
> const bool isAlwaysUninit = vi->second;
> - bool showDefinition = true;
> + bool isSelfInit = false;
>
> if (const DeclRefExpr *dr = dyn_cast<DeclRefExpr>(vi->first)) {
> if (isAlwaysUninit) {
> - if (isSelfInit(S.Context, vd, dr)) {
> + // Inspect the initializer of the variable declaration which is
> + // being referenced prior to its initialization. We emit
> + // specialized diagnostics for self-initialization, and we
> + // specifically avoid warning about self references which take the
> + // form of:
> + //
> + // int x = x;
> + //
> + // This is used to indicate to GCC that 'x' is intentionally left
> + // uninitialized. Proven code paths which access 'x' in
> + // an uninitialized state after this will still warn.
> + //
> + // TODO: Should we suppress maybe-uninitialized warnings for
> + // variables initialized in this way?
> + if (const Expr *E = vd->getInit()) {
> + if (dr == E->IgnoreParenImpCasts())
> + continue;
> +
> + ContainsReference CR(S.Context, dr);
> + CR.Visit(const_cast<Expr*>(E));
> + isSelfInit = CR.doesContainReference();
> + }
> + if (isSelfInit) {
> S.Diag(dr->getLocStart(),
> diag::warn_uninit_self_reference_in_init)
> << vd->getDeclName() << vd->getLocation() << dr->getSourceRange();
> - showDefinition = false;
> - }
> - else {
> + } else {
> S.Diag(dr->getLocStart(), diag::warn_uninit_var)
> << vd->getDeclName() << dr->getSourceRange();
> }
> @@ -501,8 +511,9 @@
> << vd->getDeclName();
> }
>
> - // Report where the variable was declared.
> - if (showDefinition)
> + // Report where the variable was declared when the use wasn't within
> + // the initializer of that declaration.
> + if (!isSelfInit)
> S.Diag(vd->getLocStart(), diag::note_uninit_var_def)
> << vd->getDeclName();
>
>
> Modified: cfe/trunk/test/Sema/uninit-variables.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/uninit-variables.c?rev=128894&r1=128893&r2=128894&view=diff
> ==============================================================================
> --- cfe/trunk/test/Sema/uninit-variables.c (original)
> +++ cfe/trunk/test/Sema/uninit-variables.c Tue Apr 5 12:41:31 2011
> @@ -92,7 +92,7 @@
> }
>
> void test15() {
> - int x = x; // expected-warning{{variable 'x' is uninitialized when used within its own initialization}}
> + int x = x; // no-warning: signals intended lack of initialization.
> }
>
> // Don't warn in the following example; shows dataflow confluence.
>
> Modified: cfe/trunk/test/SemaCXX/uninitialized.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/uninitialized.cpp?rev=128894&r1=128893&r2=128894&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/uninitialized.cpp (original)
> +++ cfe/trunk/test/SemaCXX/uninitialized.cpp Tue Apr 5 12:41:31 2011
> @@ -7,7 +7,7 @@
>
> // Test self-references within initializers which are guaranteed to be
> // uninitialized.
> -int a = a; // FIXME: This doesn't warn!? Seems it doesn't cast 'a' to an RValue.
> +int a = a; // no-warning: used to signal intended lack of initialization.
> int b = b + 1; // expected-warning {{variable 'b' is uninitialized when used within its own initialization}}
> int c = (c + c); // expected-warning 2 {{variable 'c' is uninitialized when used within its own initialization}}
> void test() {
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list