<div dir="ltr">This seems a bit narrow - could it be generalized to all cases of '\0' as a null pointer?</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Aug 5, 2019 at 3:14 PM George Burgess IV via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Author: gbiv<br>
Date: Mon Aug  5 15:15:40 2019<br>
New Revision: 367940<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=367940&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=367940&view=rev</a><br>
Log:<br>
[Sema] Add -Wpointer-compare<br>
<br>
This patch adds a warning that diagnoses comparisons of pointers to<br>
'\0'. This is often indicative of a bug (e.g. the user might've<br>
forgotten to dereference the pointer).<br>
<br>
Patch by Elaina Guan!<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D65595" rel="noreferrer" target="_blank">https://reviews.llvm.org/D65595</a><br>
<br>
Added:<br>
    cfe/trunk/test/Sema/warn-nullchar-nullptr.c<br>
Modified:<br>
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
    cfe/trunk/include/clang/Sema/Sema.h<br>
    cfe/trunk/lib/Sema/SemaExpr.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=367940&r1=367939&r2=367940&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=367940&r1=367939&r2=367940&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)<br>
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Aug  5 15:15:40 2019<br>
@@ -3296,6 +3296,10 @@ def warn_impcast_bool_to_null_pointer :<br>
 def warn_non_literal_null_pointer : Warning<<br>
     "expression which evaluates to zero treated as a null pointer constant of "<br>
     "type %0">, InGroup<NonLiteralNullConversion>;<br>
+def warn_pointer_compare : Warning<<br>
+    "comparing a pointer to a null character constant; did you mean "<br>
+    "to compare to %select{NULL|(void *)0}0?">,<br>
+    InGroup<DiagGroup<"pointer-compare">>;<br>
 def warn_impcast_null_pointer_to_integer : Warning<<br>
     "implicit conversion of %select{NULL|nullptr}0 constant to %1">,<br>
     InGroup<NullConversion>;<br>
<br>
Modified: cfe/trunk/include/clang/Sema/Sema.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=367940&r1=367939&r2=367940&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=367940&r1=367939&r2=367940&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Sema/Sema.h (original)<br>
+++ cfe/trunk/include/clang/Sema/Sema.h Mon Aug  5 15:15:40 2019<br>
@@ -10022,6 +10022,7 @@ public:<br>
   QualType CheckShiftOperands( // C99 6.5.7<br>
     ExprResult &LHS, ExprResult &RHS, SourceLocation Loc,<br>
     BinaryOperatorKind Opc, bool IsCompAssign = false);<br>
+  void CheckPtrComparisonWithNullChar(ExprResult &E, ExprResult &NullE);<br>
   QualType CheckCompareOperands( // C99 6.5.8/9<br>
       ExprResult &LHS, ExprResult &RHS, SourceLocation Loc,<br>
       BinaryOperatorKind Opc);<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=367940&r1=367939&r2=367940&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=367940&r1=367939&r2=367940&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Mon Aug  5 15:15:40 2019<br>
@@ -10443,6 +10443,32 @@ static QualType checkArithmeticOrEnumera<br>
   return S.Context.getLogicalOperationType();<br>
 }<br>
<br>
+void Sema::CheckPtrComparisonWithNullChar(ExprResult &E, ExprResult &NullE) {<br>
+  if (!NullE.get()->getType()->isAnyPointerType())<br>
+    return;<br>
+  int NullValue = PP.isMacroDefined("NULL") ? 0 : 1;<br>
+  if (!E.get()->getType()->isAnyPointerType() &&<br>
+      E.get()->isNullPointerConstant(Context,<br>
+                                     Expr::NPC_ValueDependentIsNotNull) ==<br>
+        Expr::NPCK_ZeroExpression) {<br>
+    if (const auto *CL = dyn_cast<CharacterLiteral>(E.get())) {<br>
+      if (CL->getValue() == 0)<br>
+        Diag(E.get()->getExprLoc(), diag::warn_pointer_compare)<br>
+            << NullValue<br>
+            << FixItHint::CreateReplacement(E.get()->getExprLoc(),<br>
+                                            NullValue ? "NULL" : "(void *)0");<br>
+    } else if (const auto *CE = dyn_cast<CStyleCastExpr>(E.get())) {<br>
+        TypeSourceInfo *TI = CE->getTypeInfoAsWritten();<br>
+        QualType T = Context.getCanonicalType(TI->getType()).getUnqualifiedType();<br>
+        if (T == Context.CharTy)<br>
+          Diag(E.get()->getExprLoc(), diag::warn_pointer_compare)<br>
+              << NullValue<br>
+              << FixItHint::CreateReplacement(E.get()->getExprLoc(),<br>
+                                              NullValue ? "NULL" : "(void *)0");<br>
+      }<br>
+  }<br>
+}<br>
+<br>
 // C99 6.5.8, C++ [expr.rel]<br>
 QualType Sema::CheckCompareOperands(ExprResult &LHS, ExprResult &RHS,<br>
                                     SourceLocation Loc,<br>
@@ -10476,6 +10502,10 @@ QualType Sema::CheckCompareOperands(Expr<br>
   }<br>
<br>
   checkArithmeticNull(*this, LHS, RHS, Loc, /*IsCompare=*/true);<br>
+  if (!getLangOpts().CPlusPlus && BinaryOperator::isEqualityOp(Opc)) {<br>
+    CheckPtrComparisonWithNullChar(LHS, RHS);<br>
+    CheckPtrComparisonWithNullChar(RHS, LHS);<br>
+  }<br>
<br>
   // Handle vector comparisons separately.<br>
   if (LHS.get()->getType()->isVectorType() ||<br>
<br>
Added: cfe/trunk/test/Sema/warn-nullchar-nullptr.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-nullchar-nullptr.c?rev=367940&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-nullchar-nullptr.c?rev=367940&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/Sema/warn-nullchar-nullptr.c (added)<br>
+++ cfe/trunk/test/Sema/warn-nullchar-nullptr.c Mon Aug  5 15:15:40 2019<br>
@@ -0,0 +1,49 @@<br>
+// RUN: %clang_cc1 -fsyntax-only -verify -Wall %s<br>
+<br>
+int test1(int *a) {<br>
+  return a == '\0'; // expected-warning {{comparing a pointer to a null character constant; did you mean to compare to (void *)0?}}<br>
+}<br>
+<br>
+int test2(int *a) {<br>
+  return '\0' == a; // expected-warning {{comparing a pointer to a null character constant; did you mean to compare to (void *)0?}}<br>
+}<br>
+<br>
+int test3(int *a) {<br>
+  return a == L'\0'; // expected-warning {{comparing a pointer to a null character constant; did you mean to compare to (void *)0?}}<br>
+}<br>
+<br>
+int test4(int *a) {<br>
+  return a == u'\0'; // expected-warning {{comparing a pointer to a null character constant; did you mean to compare to (void *)0?}}<br>
+}<br>
+<br>
+int test5(int *a) {<br>
+  return a == U'\0'; // expected-warning {{comparing a pointer to a null character constant; did you mean to compare to (void *)0?}}<br>
+}<br>
+<br>
+int test6(int *a) {<br>
+  return a == (char)0; // expected-warning {{comparing a pointer to a null character constant; did you mean to compare to (void *)0?}}<br>
+}<br>
+<br>
+typedef char my_char;<br>
+int test7(int *a) {<br>
+  return a == (my_char)0;<br>
+  // expected-warning@-1 {{comparing a pointer to a null character constant; did you mean to compare to (void *)0?}}<br>
+}<br>
+<br>
+int test8(int *a) {<br>
+  return a != '\0'; // expected-warning {{comparing a pointer to a null character constant; did you mean to compare to (void *)0?}}<br>
+}<br>
+<br>
+#define NULL (void *)0<br>
+int test9(int *a) {<br>
+  return a == '\0'; // expected-warning {{comparing a pointer to a null character constant; did you mean to compare to NULL?}}<br>
+}<br>
+<br>
+#define MYCHAR char<br>
+int test10(int *a) {<br>
+  return a == (MYCHAR)0; // expected-warning {{comparing a pointer to a null character constant; did you mean to compare to NULL?}}<br>
+}<br>
+<br>
+int test11(int *a) {<br>
+  return a > '\0';<br>
+}<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>