r215251 - Extend tautological pointer compare and pointer to bool conversion warnings to
Nico Weber
thakis at chromium.org
Thu Aug 14 10:48:01 PDT 2014
I guess the idea is to disable this warning on debug builds then, right?
assert(&a != NULL) or assert(this) are things that make sense to write in
debug builds: The comparison won't be optimized away, and it helps catch
bugs where code accidentally calls a non-virtual method on a NULL this, or
where code as a reference to a NULL pointer.
On Fri, Aug 8, 2014 at 3:41 PM, Richard Trieu <rtrieu at google.com> wrote:
> Author: rtrieu
> Date: Fri Aug 8 17:41:43 2014
> New Revision: 215251
>
> URL: http://llvm.org/viewvc/llvm-project?rev=215251&view=rev
> Log:
> Extend tautological pointer compare and pointer to bool conversion
> warnings to
> macro arguments.
>
> Previously, these warnings skipped any code in a macro expansion. Preform
> an
> additional check and warn when the expression and context locations are
> both
> in the macro argument.
>
> The most obvious case not caught is passing a pointer directly to a macro,
> i.e 'assert(&array)' but 'assert(&array && "valid array")' is caught.
> This is
> because macro arguments are not typed and the conversion happens inside the
> macro.
>
>
> Modified:
> cfe/trunk/lib/Sema/SemaChecking.cpp
> cfe/trunk/test/SemaCXX/warn-bool-conversion.cpp
> cfe/trunk/test/SemaCXX/warn-tautological-compare.cpp
> cfe/trunk/test/SemaCXX/warn-tautological-undefined-compare.cpp
> cfe/trunk/test/SemaCXX/warn-undefined-bool-conversion.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=215251&r1=215250&r2=215251&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Aug 8 17:41:43 2014
> @@ -6340,6 +6340,22 @@ static bool CheckForReference(Sema &Sema
> return true;
> }
>
> +// Returns true if the SourceLocation is expanded from any macro body.
> +// Returns false if the SourceLocation is invalid, is from not in a macro
> +// expansion, or is from expanded from a top-level macro argument.
> +static bool IsInAnyMacroBody(const SourceManager &SM, SourceLocation Loc)
> {
> + if (Loc.isInvalid())
> + return false;
> +
> + while (Loc.isMacroID()) {
> + if (SM.isMacroBodyExpansion(Loc))
> + return true;
> + Loc = SM.getImmediateMacroCallerLoc(Loc);
> + }
> +
> + return false;
> +}
> +
> /// \brief Diagnose pointers that are always non-null.
> /// \param E the expression containing the pointer
> /// \param NullKind NPCK_NotNull if E is a cast to bool, otherwise, E is
> @@ -6353,8 +6369,12 @@ void Sema::DiagnoseAlwaysNonNullPointer(
> return;
>
> // Don't warn inside macros.
> - if (E->getExprLoc().isMacroID())
> + if (E->getExprLoc().isMacroID()) {
> + const SourceManager &SM = getSourceManager();
> + if (IsInAnyMacroBody(SM, E->getExprLoc()) ||
> + IsInAnyMacroBody(SM, Range.getBegin()))
> return;
> + }
> E = E->IgnoreImpCasts();
>
> const bool IsCompare = NullKind != Expr::NPCK_NotNull;
>
> Modified: cfe/trunk/test/SemaCXX/warn-bool-conversion.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-bool-conversion.cpp?rev=215251&r1=215250&r2=215251&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/warn-bool-conversion.cpp (original)
> +++ cfe/trunk/test/SemaCXX/warn-bool-conversion.cpp Fri Aug 8 17:41:43
> 2014
> @@ -118,3 +118,30 @@ namespace Pointer {
> // expected-warning at -1{{address of 'S::a' will always evaluate to
> 'true'}}
> }
> }
> +
> +namespace macros {
> + #define assert(x) if (x) {}
> + #define zero_on_null(x) ((x) ? *(x) : 0)
> +
> + int array[5];
> + void fun();
> + int x;
> +
> + void test() {
> + assert(array);
> + assert(array && "expecting null pointer");
> + // expected-warning at -1{{address of array 'array' will always
> evaluate to 'true'}}
> +
> + assert(fun);
> + assert(fun && "expecting null pointer");
> + // expected-warning at -1{{address of function 'fun' will always
> evaluate to 'true'}}
> + // expected-note at -2 {{prefix with the address-of operator to silence
> this warning}}
> +
> + // TODO: warn on assert(&x) while not warning on zero_on_null(&x)
> + zero_on_null(&x);
> + assert(zero_on_null(&x));
> + assert(&x);
> + assert(&x && "expecting null pointer");
> + // expected-warning at -1{{address of 'x' will always evaluate to
> 'true'}}
> + }
> +}
>
> Modified: cfe/trunk/test/SemaCXX/warn-tautological-compare.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-tautological-compare.cpp?rev=215251&r1=215250&r2=215251&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/warn-tautological-compare.cpp (original)
> +++ cfe/trunk/test/SemaCXX/warn-tautological-compare.cpp Fri Aug 8
> 17:41:43 2014
> @@ -136,3 +136,43 @@ namespace PointerCompare {
> // expected-warning at -1{{comparison of address of 'S::a' equal to a
> null pointer is always false}}
> }
> }
> +
> +namespace macros {
> + #define assert(x) if (x) {}
> + int array[5];
> + void fun();
> + int x;
> +
> + void test() {
> + assert(array == 0);
> + // expected-warning at -1{{comparison of array 'array' equal to a null
> pointer is always false}}
> + assert(array != 0);
> + // expected-warning at -1{{comparison of array 'array' not equal to a
> null pointer is always true}}
> + assert(array == 0 && "expecting null pointer");
> + // expected-warning at -1{{comparison of array 'array' equal to a null
> pointer is always false}}
> + assert(array != 0 && "expecting non-null pointer");
> + // expected-warning at -1{{comparison of array 'array' not equal to a
> null pointer is always true}}
> +
> + assert(fun == 0);
> + // expected-warning at -1{{comparison of function 'fun' equal to a null
> pointer is always false}}
> + // expected-note at -2{{prefix with the address-of operator to silence
> this warning}}
> + assert(fun != 0);
> + // expected-warning at -1{{comparison of function 'fun' not equal to a
> null pointer is always true}}
> + // expected-note at -2{{prefix with the address-of operator to silence
> this warning}}
> + assert(fun == 0 && "expecting null pointer");
> + // expected-warning at -1{{comparison of function 'fun' equal to a null
> pointer is always false}}
> + // expected-note at -2{{prefix with the address-of operator to silence
> this warning}}
> + assert(fun != 0 && "expecting non-null pointer");
> + // expected-warning at -1{{comparison of function 'fun' not equal to a
> null pointer is always true}}
> + // expected-note at -2{{prefix with the address-of operator to silence
> this warning}}
> +
> + assert(&x == 0);
> + // expected-warning at -1{{comparison of address of 'x' equal to a null
> pointer is always false}}
> + assert(&x != 0);
> + // expected-warning at -1{{comparison of address of 'x' not equal to a
> null pointer is always true}}
> + assert(&x == 0 && "expecting null pointer");
> + // expected-warning at -1{{comparison of address of 'x' equal to a null
> pointer is always false}}
> + assert(&x != 0 && "expecting non-null pointer");
> + // expected-warning at -1{{comparison of address of 'x' not equal to a
> null pointer is always true}}
> + }
> +}
>
> Modified: cfe/trunk/test/SemaCXX/warn-tautological-undefined-compare.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-tautological-undefined-compare.cpp?rev=215251&r1=215250&r2=215251&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/warn-tautological-undefined-compare.cpp
> (original)
> +++ cfe/trunk/test/SemaCXX/warn-tautological-undefined-compare.cpp Fri
> Aug 8 17:41:43 2014
> @@ -110,3 +110,31 @@ namespace function_return_reference {
> // expected-warning at -1{{reference cannot be bound to dereferenced
> null pointer in well-defined C++ code; comparison may be assumed to always
> evaluate to true}}
> }
> }
> +
> +namespace macros {
> + #define assert(x) if (x) {}
> +
> + void test(int &x) {
> + assert(&x != 0);
> + // expected-warning at -1{{reference cannot be bound to dereferenced
> null pointer in well-defined C++ code; comparison may be assumed to always
> evaluate to true}}
> + assert(&x == 0);
> + // expected-warning at -1{{reference cannot be bound to dereferenced
> null pointer in well-defined C++ code; comparison may be assumed to always
> evaluate to false}}
> + assert(&x != 0 && "Expecting valid reference");
> + // expected-warning at -1{{reference cannot be bound to dereferenced
> null pointer in well-defined C++ code; comparison may be assumed to always
> evaluate to true}}
> + assert(&x == 0 && "Expecting invalid reference");
> + // expected-warning at -1{{reference cannot be bound to dereferenced
> null pointer in well-defined C++ code; comparison may be assumed to always
> evaluate to false}}
> + }
> +
> + class S {
> + void test() {
> + assert(this != 0);
> + // expected-warning at -1{{'this' pointer cannot be null in
> well-defined C++ code; comparison may be assumed to always evaluate to
> true}}
> + assert(this == 0);
> + // expected-warning at -1{{'this' pointer cannot be null in
> well-defined C++ code; comparison may be assumed to always evaluate to
> false}}
> + assert(this != 0 && "Expecting valid reference");
> + // expected-warning at -1{{'this' pointer cannot be null in
> well-defined C++ code; comparison may be assumed to always evaluate to
> true}}
> + assert(this == 0 && "Expecting invalid reference");
> + // expected-warning at -1{{'this' pointer cannot be null in
> well-defined C++ code; comparison may be assumed to always evaluate to
> false}}
> + }
> + };
> +}
>
> Modified: cfe/trunk/test/SemaCXX/warn-undefined-bool-conversion.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-undefined-bool-conversion.cpp?rev=215251&r1=215250&r2=215251&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/warn-undefined-bool-conversion.cpp (original)
> +++ cfe/trunk/test/SemaCXX/warn-undefined-bool-conversion.cpp Fri Aug 8
> 17:41:43 2014
> @@ -95,3 +95,27 @@ namespace function_return_reference {
> // expected-warning at -1{{reference cannot be bound to dereferenced
> null pointer in well-defined C++ code; pointer may be assumed to always
> convert to true}}
> }
> }
> +
> +namespace macros {
> + #define assert(x) if (x) {}
> + #define zero_on_null(x) ((x) ? *(x) : 0)
> +
> + void test(int &x) {
> + // TODO: warn on assert(&x) but not on zero_on_null(&x)
> + zero_on_null(&x);
> + assert(zero_on_null(&x));
> + assert(&x);
> +
> + assert(&x && "Expecting valid reference");
> + // expected-warning at -1{{reference cannot be bound to dereferenced
> null pointer in well-defined C++ code; pointer may be assumed to always
> convert to true}}
> + }
> +
> + class S {
> + void test() {
> + assert(this);
> +
> + assert(this && "Expecting invalid reference");
> + // expected-warning at -1{{'this' pointer cannot be null in
> well-defined C++ code; pointer may be assumed to always convert to true}}
> + }
> + };
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140814/8a744bf1/attachment.html>
More information about the cfe-commits
mailing list