From a9342885b149d3dd47037c937c012ba76580acd5 Mon Sep 17 00:00:00 2001 From: Marek Polacek Date: Wed, 4 Jan 2017 21:47:04 +0000 Subject: [PATCH] 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 --- gcc/ChangeLog | 5 ++ gcc/c-family/ChangeLog | 5 ++ gcc/c-family/c.opt | 4 ++ gcc/c/ChangeLog | 9 +++ gcc/c/c-parser.c | 3 + gcc/c/c-typeck.c | 27 ++++++++ gcc/cp/ChangeLog | 6 ++ gcc/cp/typeck.c | 12 ++++ gcc/doc/invoke.texi | 18 ++++- gcc/testsuite/ChangeLog | 5 ++ .../c-c++-common/Wpointer-compare-1.c | 65 +++++++++++++++++++ 11 files changed, 158 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/c-c++-common/Wpointer-compare-1.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 5181fe80e06..45efbb47c86 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,8 @@ +2017-01-04 Marek Polacek + + PR c++/64767 + * doc/invoke.texi: Document -Wpointer-compare. + 2017-01-04 Jakub Jelinek * optc-gen.awk: Emit #error for -W*/-f*/-m* Enum without diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog index 468b7dde8f5..b9bb5fe59a3 100644 --- a/gcc/c-family/ChangeLog +++ b/gcc/c-family/ChangeLog @@ -1,3 +1,8 @@ +2017-01-04 Marek Polacek + + PR c++/64767 + * c.opt (Wpointer-compare): New option. + 2017-01-04 Jakub Jelinek PR driver/78957 diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 3f6c67fbc7e..714ce3a34f0 100644 --- 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. diff --git a/gcc/c/ChangeLog b/gcc/c/ChangeLog index 6f8cf00a1da..6e1c9857016 100644 --- a/gcc/c/ChangeLog +++ b/gcc/c/ChangeLog @@ -1,3 +1,12 @@ +2017-01-04 Marek Polacek + + 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 * c-parser.c (c_parser_declaration_or_fndef): Create a diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c index c7679c2644a..a3504d3eec9 100644 --- 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; diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c index 63e0dc6b298..96e7351bf45 100644 --- 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)) diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index 8dc6588ec31..8e723be3eba 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,3 +1,9 @@ +2017-01-04 Marek Polacek + + 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 PR c++/78949 diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index 6da98f65bdd..b84f8beef53 100644 --- 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) diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 2bd105a867f..a6ea4259f52 100644 --- 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 diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index f56c11b957f..9b44a4a5843 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2017-01-04 Marek Polacek + + PR c++/64767 + * c-c++-common/Wpointer-compare-1.c: New test. + 2017-01-04 Jakub Jelinek PR c++/78949 diff --git a/gcc/testsuite/c-c++-common/Wpointer-compare-1.c b/gcc/testsuite/c-c++-common/Wpointer-compare-1.c new file mode 100644 index 00000000000..440341ed9ed --- /dev/null +++ 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; +} -- 2.30.2