re PR c++/64767 (Could GCC warn when a pointer is compared against '\0'?)

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. From-SVN: r244076
2017-01-04 21:47:04 +00:00 · 2017-01-04 21:47:04 +00:00 · a9342885b1
parent abec4284a6
commit a9342885b1
11 changed files with 158 additions and 1 deletions
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@ -1,3 +1,8 @@
+2017-01-04  Marek Polacek  <polacek@redhat.com>
+
+	PR c++/64767
+	* doc/invoke.texi: Document -Wpointer-compare.
+
 2017-01-04  Jakub Jelinek  <jakub@redhat.com>

 	* optc-gen.awk: Emit #error for -W*/-f*/-m* Enum without
--- a/gcc/c-family/ChangeLog
+++ b/gcc/c-family/ChangeLog
@ -1,3 +1,8 @@
+2017-01-04  Marek Polacek  <polacek@redhat.com>
+
+	PR c++/64767
+	* c.opt (Wpointer-compare): New option.
+
 2017-01-04  Jakub Jelinek  <jakub@redhat.com>

 	PR driver/78957
--- a/gcc/c-family/c.opt
+++ b/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.
--- a/gcc/c/ChangeLog
+++ b/gcc/c/ChangeLog
@ -1,3 +1,12 @@
+2017-01-04  Marek Polacek  <polacek@redhat.com>
+
+	PR c++/64767
+	* 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.
+
 2017-01-04  David Malcolm  <dmalcolm@redhat.com>

 	* c-parser.c (c_parser_declaration_or_fndef): Create a
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@ -7540,6 +7540,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;
--- a/gcc/c/c-typeck.c
+++ b/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))
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@ -1,3 +1,9 @@
+2017-01-04  Marek Polacek  <polacek@redhat.com>
+
+	PR c++/64767
+	* typeck.c (cp_build_binary_op): Warn when a pointer is compared with
+	a zero character literal.
+
 2017-01-04  Jakub Jelinek  <jakub@redhat.com>

 	PR c++/78949
--- a/gcc/cp/typeck.c
+++ b/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)
--- a/gcc/doc/invoke.texi
+++ b/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 invalid in C++11.
+
+This warning is enabled by default.
+
@item -Wtype-limits
@opindex Wtype-limits
@opindex Wno-type-limits
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@ -1,3 +1,8 @@
+2017-01-04  Marek Polacek  <polacek@redhat.com>
+
+	PR c++/64767
+	* c-c++-common/Wpointer-compare-1.c: New test.
+
 2017-01-04  Jakub Jelinek  <jakub@redhat.com>

 	PR c++/78949
--- a/gcc/testsuite/c-c++-common/Wpointer-compare-1.c
+++ b/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;
+}