[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 cf036b57608fcc7a5be901cfb3686082b1bdd7e6..a60d548334d4cf5d9137ac0d4bf6e6627ff35f84 100644 (file)
--- 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<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>;

diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index a709ebb249f68f0870d354eaa4c2c5b40de88b12..a617e71999f9d61435466b56efc2ea24b4876c77 100644 (file)
--- 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 52fa932eee81c8aa8df92d9e491bfbf05e92557b..b407fb3c1377111d253f4fc98d4cb957f25bb39e 100644 (file)
--- 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<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(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 (file)
index 0000000..36a126a
--- /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';
+}