r367940 - [Sema] Add -Wpointer-compare

David Blaikie via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 13 16:14:02 PDT 2019


This seems a bit narrow - could it be generalized to all cases of '\0' as a
null pointer?

On Mon, Aug 5, 2019 at 3:14 PM George Burgess IV via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Author: gbiv
> Date: Mon Aug  5 15:15:40 2019
> New Revision: 367940
>
> URL: http://llvm.org/viewvc/llvm-project?rev=367940&view=rev
> Log:
> [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
>
> Added:
>     cfe/trunk/test/Sema/warn-nullchar-nullptr.c
> Modified:
>     cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>     cfe/trunk/include/clang/Sema/Sema.h
>     cfe/trunk/lib/Sema/SemaExpr.cpp
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=367940&r1=367939&r2=367940&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Aug  5
> 15:15:40 2019
> @@ -3296,6 +3296,10 @@ def warn_impcast_bool_to_null_pointer :
>  def warn_non_literal_null_pointer : Warning<
>      "expression which evaluates to zero treated as a null pointer
> constant of "
>      "type %0">, InGroup<NonLiteralNullConversion>;
> +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<DiagGroup<"pointer-compare">>;
>  def warn_impcast_null_pointer_to_integer : Warning<
>      "implicit conversion of %select{NULL|nullptr}0 constant to %1">,
>      InGroup<NullConversion>;
>
> Modified: cfe/trunk/include/clang/Sema/Sema.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=367940&r1=367939&r2=367940&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Mon Aug  5 15:15:40 2019
> @@ -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);
>
> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=367940&r1=367939&r2=367940&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Mon Aug  5 15:15:40 2019
> @@ -10443,6 +10443,32 @@ static QualType checkArithmeticOrEnumera
>    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<CharacterLiteral>(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<CStyleCastExpr>(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(Expr
>    }
>
>    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() ||
>
> Added: cfe/trunk/test/Sema/warn-nullchar-nullptr.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-nullchar-nullptr.c?rev=367940&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/Sema/warn-nullchar-nullptr.c (added)
> +++ cfe/trunk/test/Sema/warn-nullchar-nullptr.c Mon Aug  5 15:15:40 2019
> @@ -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 at -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';
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190813/9ece165e/attachment-0001.html>


More information about the cfe-commits mailing list