From: George Burgess IV Date: Mon, 5 Aug 2019 22:15:40 +0000 (+0000) Subject: [Sema] Add -Wpointer-compare X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=73d8bf5ccf3b5712929b5b19e22e5a5df7912750;p=clang [Sema] Add -Wpointer-compare This patch adds a warning that diagnoses comparisons of pointers to '\0'. This is often indicative of a bug (e.g. the user might've forgotten to dereference the pointer). Patch by Elaina Guan! Differential Revision: https://reviews.llvm.org/D65595 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@367940 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index cf036b5760..a60d548334 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -3296,6 +3296,10 @@ def warn_impcast_bool_to_null_pointer : Warning< def warn_non_literal_null_pointer : Warning< "expression which evaluates to zero treated as a null pointer constant of " "type %0">, InGroup; +def warn_pointer_compare : Warning< + "comparing a pointer to a null character constant; did you mean " + "to compare to %select{NULL|(void *)0}0?">, + InGroup>; def warn_impcast_null_pointer_to_integer : Warning< "implicit conversion of %select{NULL|nullptr}0 constant to %1">, InGroup; diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index a709ebb249..a617e71999 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -10022,6 +10022,7 @@ public: QualType CheckShiftOperands( // C99 6.5.7 ExprResult &LHS, ExprResult &RHS, SourceLocation Loc, BinaryOperatorKind Opc, bool IsCompAssign = false); + void CheckPtrComparisonWithNullChar(ExprResult &E, ExprResult &NullE); QualType CheckCompareOperands( // C99 6.5.8/9 ExprResult &LHS, ExprResult &RHS, SourceLocation Loc, BinaryOperatorKind Opc); diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index 52fa932eee..b407fb3c13 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -10443,6 +10443,32 @@ static QualType checkArithmeticOrEnumeralCompare(Sema &S, ExprResult &LHS, return S.Context.getLogicalOperationType(); } +void Sema::CheckPtrComparisonWithNullChar(ExprResult &E, ExprResult &NullE) { + if (!NullE.get()->getType()->isAnyPointerType()) + return; + int NullValue = PP.isMacroDefined("NULL") ? 0 : 1; + if (!E.get()->getType()->isAnyPointerType() && + E.get()->isNullPointerConstant(Context, + Expr::NPC_ValueDependentIsNotNull) == + Expr::NPCK_ZeroExpression) { + if (const auto *CL = dyn_cast(E.get())) { + if (CL->getValue() == 0) + Diag(E.get()->getExprLoc(), diag::warn_pointer_compare) + << NullValue + << FixItHint::CreateReplacement(E.get()->getExprLoc(), + NullValue ? "NULL" : "(void *)0"); + } else if (const auto *CE = dyn_cast(E.get())) { + TypeSourceInfo *TI = CE->getTypeInfoAsWritten(); + QualType T = Context.getCanonicalType(TI->getType()).getUnqualifiedType(); + if (T == Context.CharTy) + Diag(E.get()->getExprLoc(), diag::warn_pointer_compare) + << NullValue + << FixItHint::CreateReplacement(E.get()->getExprLoc(), + NullValue ? "NULL" : "(void *)0"); + } + } +} + // C99 6.5.8, C++ [expr.rel] QualType Sema::CheckCompareOperands(ExprResult &LHS, ExprResult &RHS, SourceLocation Loc, @@ -10476,6 +10502,10 @@ QualType Sema::CheckCompareOperands(ExprResult &LHS, ExprResult &RHS, } checkArithmeticNull(*this, LHS, RHS, Loc, /*IsCompare=*/true); + if (!getLangOpts().CPlusPlus && BinaryOperator::isEqualityOp(Opc)) { + CheckPtrComparisonWithNullChar(LHS, RHS); + CheckPtrComparisonWithNullChar(RHS, LHS); + } // Handle vector comparisons separately. if (LHS.get()->getType()->isVectorType() || diff --git a/test/Sema/warn-nullchar-nullptr.c b/test/Sema/warn-nullchar-nullptr.c new file mode 100644 index 0000000000..36a126a043 --- /dev/null +++ b/test/Sema/warn-nullchar-nullptr.c @@ -0,0 +1,49 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -Wall %s + +int test1(int *a) { + return a == '\0'; // expected-warning {{comparing a pointer to a null character constant; did you mean to compare to (void *)0?}} +} + +int test2(int *a) { + return '\0' == a; // expected-warning {{comparing a pointer to a null character constant; did you mean to compare to (void *)0?}} +} + +int test3(int *a) { + return a == L'\0'; // expected-warning {{comparing a pointer to a null character constant; did you mean to compare to (void *)0?}} +} + +int test4(int *a) { + return a == u'\0'; // expected-warning {{comparing a pointer to a null character constant; did you mean to compare to (void *)0?}} +} + +int test5(int *a) { + return a == U'\0'; // expected-warning {{comparing a pointer to a null character constant; did you mean to compare to (void *)0?}} +} + +int test6(int *a) { + return a == (char)0; // expected-warning {{comparing a pointer to a null character constant; did you mean to compare to (void *)0?}} +} + +typedef char my_char; +int test7(int *a) { + return a == (my_char)0; + // expected-warning@-1 {{comparing a pointer to a null character constant; did you mean to compare to (void *)0?}} +} + +int test8(int *a) { + return a != '\0'; // expected-warning {{comparing a pointer to a null character constant; did you mean to compare to (void *)0?}} +} + +#define NULL (void *)0 +int test9(int *a) { + return a == '\0'; // expected-warning {{comparing a pointer to a null character constant; did you mean to compare to NULL?}} +} + +#define MYCHAR char +int test10(int *a) { + return a == (MYCHAR)0; // expected-warning {{comparing a pointer to a null character constant; did you mean to compare to NULL?}} +} + +int test11(int *a) { + return a > '\0'; +}