From 01ad089bc379bfce98047c6f7be002efa3a40636 Mon Sep 17 00:00:00 2001 From: Richard Trieu Date: Fri, 8 Aug 2014 22:41:43 +0000 Subject: [PATCH] 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@215251 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Sema/SemaChecking.cpp | 22 +++++++++- test/SemaCXX/warn-bool-conversion.cpp | 27 +++++++++++++ test/SemaCXX/warn-tautological-compare.cpp | 40 +++++++++++++++++++ .../warn-tautological-undefined-compare.cpp | 28 +++++++++++++ .../warn-undefined-bool-conversion.cpp | 24 +++++++++++ 5 files changed, 140 insertions(+), 1 deletion(-) diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index 0483341cef..7bc3ba7feb 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -6340,6 +6340,22 @@ static bool CheckForReference(Sema &SemaRef, const Expr *E, 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(Expr *E, 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; diff --git a/test/SemaCXX/warn-bool-conversion.cpp b/test/SemaCXX/warn-bool-conversion.cpp index b4628947f0..b0c8d0d19d 100644 --- a/test/SemaCXX/warn-bool-conversion.cpp +++ b/test/SemaCXX/warn-bool-conversion.cpp @@ -118,3 +118,30 @@ namespace Pointer { // expected-warning@-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@-1{{address of array 'array' will always evaluate to 'true'}} + + assert(fun); + assert(fun && "expecting null pointer"); + // expected-warning@-1{{address of function 'fun' will always evaluate to 'true'}} + // expected-note@-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@-1{{address of 'x' will always evaluate to 'true'}} + } +} diff --git a/test/SemaCXX/warn-tautological-compare.cpp b/test/SemaCXX/warn-tautological-compare.cpp index b44f3f9d8f..7d5b4b14e9 100644 --- a/test/SemaCXX/warn-tautological-compare.cpp +++ b/test/SemaCXX/warn-tautological-compare.cpp @@ -136,3 +136,43 @@ namespace PointerCompare { // expected-warning@-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@-1{{comparison of array 'array' equal to a null pointer is always false}} + assert(array != 0); + // expected-warning@-1{{comparison of array 'array' not equal to a null pointer is always true}} + assert(array == 0 && "expecting null pointer"); + // expected-warning@-1{{comparison of array 'array' equal to a null pointer is always false}} + assert(array != 0 && "expecting non-null pointer"); + // expected-warning@-1{{comparison of array 'array' not equal to a null pointer is always true}} + + assert(fun == 0); + // expected-warning@-1{{comparison of function 'fun' equal to a null pointer is always false}} + // expected-note@-2{{prefix with the address-of operator to silence this warning}} + assert(fun != 0); + // expected-warning@-1{{comparison of function 'fun' not equal to a null pointer is always true}} + // expected-note@-2{{prefix with the address-of operator to silence this warning}} + assert(fun == 0 && "expecting null pointer"); + // expected-warning@-1{{comparison of function 'fun' equal to a null pointer is always false}} + // expected-note@-2{{prefix with the address-of operator to silence this warning}} + assert(fun != 0 && "expecting non-null pointer"); + // expected-warning@-1{{comparison of function 'fun' not equal to a null pointer is always true}} + // expected-note@-2{{prefix with the address-of operator to silence this warning}} + + assert(&x == 0); + // expected-warning@-1{{comparison of address of 'x' equal to a null pointer is always false}} + assert(&x != 0); + // expected-warning@-1{{comparison of address of 'x' not equal to a null pointer is always true}} + assert(&x == 0 && "expecting null pointer"); + // expected-warning@-1{{comparison of address of 'x' equal to a null pointer is always false}} + assert(&x != 0 && "expecting non-null pointer"); + // expected-warning@-1{{comparison of address of 'x' not equal to a null pointer is always true}} + } +} diff --git a/test/SemaCXX/warn-tautological-undefined-compare.cpp b/test/SemaCXX/warn-tautological-undefined-compare.cpp index ce05bfc14d..9321c0edbd 100644 --- a/test/SemaCXX/warn-tautological-undefined-compare.cpp +++ b/test/SemaCXX/warn-tautological-undefined-compare.cpp @@ -110,3 +110,31 @@ namespace function_return_reference { // expected-warning@-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@-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@-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@-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@-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@-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@-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@-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@-1{{'this' pointer cannot be null in well-defined C++ code; comparison may be assumed to always evaluate to false}} + } + }; +} diff --git a/test/SemaCXX/warn-undefined-bool-conversion.cpp b/test/SemaCXX/warn-undefined-bool-conversion.cpp index 1f8baa0e8d..24099f79ad 100644 --- a/test/SemaCXX/warn-undefined-bool-conversion.cpp +++ b/test/SemaCXX/warn-undefined-bool-conversion.cpp @@ -95,3 +95,27 @@ namespace function_return_reference { // expected-warning@-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@-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@-1{{'this' pointer cannot be null in well-defined C++ code; pointer may be assumed to always convert to true}} + } + }; +} -- 2.40.0