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 <pola...@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
