C/C++ PATCH to implement -Wpointer-compare warning (PR c++/64767) (version 2)

[Marek Polacek <polacek at redhat dot com>]

This is another version of the patch I posted a while ago.  To recap:

Spurred by the recent <http://www.viva64.com/en/b/0425/> findings, I decided to
implement a warning that warns when a pointer is compared with a zero character
literal (constant), because this isn't likely to be intended.  So e.g.

  ptr == L'\0'

is probably wrong and should've been written as

  ptr[0] == L'\0'

This patch adds the warning for the C FE as well as the C++ FE, but since this
code is actually invalid in C++11 (and we reject it), it's only active in C++03.

One issue: in the C FE wchar_type_node == integer_type_node, so we won't warn
for "p == (wchar_t) 0" in the C FE, but we will in the C++ FE.  I'm hoping this
isn't that important an issue.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2017-01-04  Marek Polacek  <polacek@redhat.com>

	PR c++/64767
	* c.opt (Wpointer-compare): New option.

	* c-parser.c (c_parser_postfix_expression): Mark zero character
	constants by setting original_type in c_expr.
	* c-typeck.c (parser_build_binary_op): Warn when a pointer is compared
	with a zero character constant.
	(char_type_p): New function.

	* typeck.c (cp_build_binary_op): Warn when a pointer is compared with
	a zero character literal.

	* doc/invoke.texi: Document -Wpointer-compare.

	* c-c++-common/Wpointer-compare-1.c: New test.

diff --git gcc/c-family/c.opt gcc/c-family/c.opt
index 3c06aec..eb70647 100644
--- gcc/c-family/c.opt
+++ gcc/c-family/c.opt
@@ -870,6 +870,10 @@ Wpointer-sign
 C ObjC Var(warn_pointer_sign) Warning LangEnabledBy(C ObjC,Wall || Wpedantic)
 Warn when a pointer differs in signedness in an assignment.
 
+Wpointer-compare
+C ObjC C++ ObjC++ Var(warn_pointer_compare) Init(1) Warning
+Warn when a pointer is compared with a zero character constant.
+
 Wpointer-to-int-cast
 C ObjC Var(warn_pointer_to_int_cast) Init(1) Warning
 Warn when a pointer is cast to an integer of a different size.
diff --git gcc/c/c-parser.c gcc/c/c-parser.c
index 6d443da..fb1fd56 100644
--- gcc/c/c-parser.c
+++ gcc/c/c-parser.c
@@ -7530,6 +7530,9 @@ c_parser_postfix_expression (c_parser *parser)
     case CPP_CHAR32:
     case CPP_WCHAR:
       expr.value = c_parser_peek_token (parser)->value;
+      /* For the purpose of warning when a pointer is compared with
+	 a zero character constant.  */
+      expr.original_type = char_type_node;
       set_c_expr_source_range (&expr, tok_range);
       c_parser_consume_token (parser);
       break;
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 54ae7d8..bd6b8eb 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -3595,6 +3595,18 @@ parser_build_unary_op (location_t loc, enum tree_code code, struct c_expr arg)
   return result;
 }
 
+/* Returns true if TYPE is a character type, *not* including wchar_t.  */
+
+static bool
+char_type_p (tree type)
+{
+  return (type == char_type_node
+	  || type == unsigned_char_type_node
+	  || type == signed_char_type_node
+	  || type == char16_type_node
+	  || type == char32_type_node);
+}
+
 /* This is the entry point used by the parser to build binary operators
    in the input.  CODE, a tree_code, specifies the binary operator, and
    ARG1 and ARG2 are the operands.  In addition to constructing the
@@ -3714,6 +3726,21 @@ parser_build_binary_op (location_t location, enum tree_code code,
 	      && !integer_zerop (tree_strip_nop_conversions (arg1.value))))
 	warning_at (location, OPT_Waddress,
 		    "comparison with string literal results in unspecified behavior");
+      /* Warn for ptr == '\0', it's likely that it should've been ptr[0].  */
+      if (POINTER_TYPE_P (type1)
+	   && null_pointer_constant_p (arg2.value)
+	   && char_type_p (type2)
+	   && warning_at (location, OPT_Wpointer_compare,
+			  "comparison between pointer and zero character "
+			  "constant"))
+	inform (arg1.get_start (), "did you mean to dereference the pointer?");
+      else if (POINTER_TYPE_P (type2)
+	       && null_pointer_constant_p (arg1.value)
+	       && char_type_p (type1)
+	       && warning_at (location, OPT_Wpointer_compare,
+			      "comparison between pointer and zero character "
+			      "constant"))
+	inform (arg2.get_start (), "did you mean to dereference the pointer?");
     }
   else if (TREE_CODE_CLASS (code) == tcc_comparison
 	   && (code1 == STRING_CST || code2 == STRING_CST))
diff --git gcc/cp/typeck.c gcc/cp/typeck.c
index 57a69ef..4eff223 100644
--- gcc/cp/typeck.c
+++ gcc/cp/typeck.c
@@ -4604,6 +4604,12 @@ cp_build_binary_op (location_t location,
 	  else
 	    result_type = type0;
 
+	  if (char_type_p (TREE_TYPE (orig_op1))
+	      && warning (OPT_Wpointer_compare,
+			  "comparison between pointer and zero character "
+			  "constant"))
+	    inform (input_location,
+		    "did you mean to dereference the pointer?");
 	  warn_for_null_address (location, op0, complain);
 	}
       else if (((code1 == POINTER_TYPE || TYPE_PTRDATAMEM_P (type1))
@@ -4618,6 +4624,12 @@ cp_build_binary_op (location_t location,
 	  else
 	    result_type = type1;
 
+	  if (char_type_p (TREE_TYPE (orig_op0))
+	      && warning (OPT_Wpointer_compare,
+			  "comparison between pointer and zero character "
+			  "constant"))
+	    inform (input_location,
+		    "did you mean to dereference the pointer?");
 	  warn_for_null_address (location, op1, complain);
 	}
       else if ((code0 == POINTER_TYPE && code1 == POINTER_TYPE)
diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
index 2bd105a..3028b69 100644
--- gcc/doc/invoke.texi
+++ gcc/doc/invoke.texi
@@ -295,7 +295,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wpacked  -Wpacked-bitfield-compat  -Wpadded @gol
 -Wparentheses -Wno-pedantic-ms-format @gol
 -Wplacement-new -Wplacement-new=@var{n} @gol
--Wpointer-arith  -Wno-pointer-to-int-cast @gol
+-Wpointer-arith  -Wpointer-compare  -Wno-pointer-to-int-cast @gol
 -Wno-pragmas -Wredundant-decls -Wrestrict  -Wno-return-local-addr @gol
 -Wreturn-type  -Wsequence-point  -Wshadow  -Wno-shadow-ivar @gol
 -Wshadow=global, -Wshadow=local, -Wshadow=compatible-local @gol
@@ -5637,6 +5637,22 @@ convenience in calculations with @code{void *} pointers and pointers
 to functions.  In C++, warn also when an arithmetic operation involves
 @code{NULL}.  This warning is also enabled by @option{-Wpedantic}.
 
+@item -Wpointer-compare
+@opindex Wpointer-compare
+@opindex Wno-pointer-compare
+Warn if a pointer is compared with a zero character constant.  This usually
+means that the pointer was meant to be dereferenced.  For example:
+
+@smallexample
+const char *p = foo ();
+if (p == '\0')
+  return 42;
+@end smallexample
+
+Note that the code above is illegal in C++11.
+
+This warning is enabled by default.
+
 @item -Wtype-limits
 @opindex Wtype-limits
 @opindex Wno-type-limits
diff --git gcc/testsuite/c-c++-common/Wpointer-compare-1.c gcc/testsuite/c-c++-common/Wpointer-compare-1.c
index e69de29..440341e 100644
--- gcc/testsuite/c-c++-common/Wpointer-compare-1.c
+++ gcc/testsuite/c-c++-common/Wpointer-compare-1.c
@@ -0,0 +1,65 @@
+/* PR c++/64767 */
+/* { dg-do compile } */
+/* { dg-options "-Wpointer-compare" } */
+/* { dg-additional-options "-std=c++03" { target c++ } } */
+
+int
+f1 (int *p, int **q)
+{
+  int r = 0;
+
+  r += p == '\0'; /* { dg-warning "comparison between pointer and zero character" } */
+  r += p == L'\0'; /* { dg-warning "comparison between pointer and zero character" } */
+  r += p != '\0'; /* { dg-warning "comparison between pointer and zero character" } */
+  r += p != L'\0'; /* { dg-warning "comparison between pointer and zero character" } */
+
+  r += '\0' == p; /* { dg-warning "comparison between pointer and zero character" } */
+  r += L'\0' == p; /* { dg-warning "comparison between pointer and zero character" } */
+  r += '\0' != p; /* { dg-warning "comparison between pointer and zero character" } */
+  r += L'\0' != p; /* { dg-warning "comparison between pointer and zero character" } */
+
+  r += q == '\0'; /* { dg-warning "comparison between pointer and zero character" } */
+  r += q == L'\0'; /* { dg-warning "comparison between pointer and zero character" } */
+  r += q != '\0'; /* { dg-warning "comparison between pointer and zero character" } */
+  r += q != L'\0'; /* { dg-warning "comparison between pointer and zero character" } */
+
+  r += '\0' == q; /* { dg-warning "comparison between pointer and zero character" } */
+  r += L'\0' == q; /* { dg-warning "comparison between pointer and zero character" } */
+  r += '\0' != q; /* { dg-warning "comparison between pointer and zero character" } */
+  r += L'\0' != q; /* { dg-warning "comparison between pointer and zero character" } */
+
+  return r;
+}
+
+int
+f2 (int *p)
+{
+  int r = 0;
+
+  /* Keep quiet.  */
+  r += p == (void *) 0;
+  r += p != (void *) 0;
+  r += (void *) 0 == p;
+  r += (void *) 0 != p;
+
+  r += p == 0;
+  r += p != 0;
+  r += 0 == p;
+  r += 0 != p;
+
+  return r;
+}
+
+int
+f3 (int *p)
+{
+  int r = 0;
+
+  r += p == (char) 0; /* { dg-warning "comparison between pointer and zero character" } */
+  r += p != (char) 0; /* { dg-warning "comparison between pointer and zero character" } */
+
+  r += (char) 0 == p; /* { dg-warning "comparison between pointer and zero character" } */
+  r += (char) 0 != p; /* { dg-warning "comparison between pointer and zero character" } */
+
+  return r;
+}

	Marek