| Message ID | 1430096197-29836-1-git-send-email-patrick@parcs.ath.cx |
|---|---|
| State | New |
| Headers | show |
On Sun, Apr 26, 2015 at 8:56 PM, Patrick Palka <patrick@parcs.ath.cx> wrote: > Here is an updated version of the patch with, hopefully, all your > suggestions made. I decided to add calls to STRIP_NOPS before emitting > the warning so that we properly warn for cases where there's a cast in > between the whole thing, e.g. > > if (!&(int &)a) > > I also added guards to emit the warnings only when the stripped operand > is actually a decl so that we don't pass a non-decl argument to > warning_at() which can happen in a case like > > if (!&(int &)*(int *)0) > > Does this look OK now after testing? > > gcc/c-family/ChangeLog: > > PR c++/65168 > * c-common.c (c_common_truthvalue_conversion): Warn when > converting an address of a reference to a truth value. > > gcc/cp/ChangeLog: > > PR c++/65168 > * typeck.c (cp_build_binary_op): Warn when comparing an address > of a reference against NULL. > > gcc/testsuite/ChangeLog: > > PR c++/65168 > g++.dg/warn/Walways-true-3.C: New test. > --- > gcc/c-family/c-common.c | 14 ++++++++++ > gcc/cp/typeck.c | 34 ++++++++++++++++++++++ > gcc/testsuite/g++.dg/warn/Walways-true-3.C | 45 ++++++++++++++++++++++++++++++ > 3 files changed, 93 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/warn/Walways-true-3.C > > diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c > index 9797e17..c353c72 100644 > --- a/gcc/c-family/c-common.c > +++ b/gcc/c-family/c-common.c > @@ -4806,6 +4806,20 @@ c_common_truthvalue_conversion (location_t location, tree expr) > tree totype = TREE_TYPE (expr); > tree fromtype = TREE_TYPE (TREE_OPERAND (expr, 0)); > > + if (POINTER_TYPE_P (totype) > + && TREE_CODE (fromtype) == REFERENCE_TYPE) > + { > + tree inner = expr; > + STRIP_NOPS (inner); > + > + if (DECL_P (inner)) > + warning_at (location, > + OPT_Waddress, > + "the compiler can assume that the address of " > + "%qD will always evaluate to %<true%>", > + inner); > + } > + > /* Don't cancel the effect of a CONVERT_EXPR from a REFERENCE_TYPE, > since that affects how `default_conversion' will behave. */ > if (TREE_CODE (totype) == REFERENCE_TYPE > diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c > index 91db32a..13fb401 100644 > --- a/gcc/cp/typeck.c > +++ b/gcc/cp/typeck.c > @@ -4423,6 +4423,23 @@ cp_build_binary_op (location_t location, > warning (OPT_Waddress, "the address of %qD will never be NULL", > TREE_OPERAND (op0, 0)); > } > + > + if (CONVERT_EXPR_P (op0) > + && TREE_CODE (TREE_TYPE (TREE_OPERAND (op0, 0))) > + == REFERENCE_TYPE) > + { > + tree inner_op0 = op0; > + STRIP_NOPS (inner_op0); > + > + if ((complain & tf_warning) > + && c_inhibit_evaluation_warnings == 0 > + && !TREE_NO_WARNING (op0) > + && DECL_P (inner_op0)) > + warning (OPT_Waddress, > + "the compiler can assume that the address of " > + "%qD will never be NULL", > + inner_op0); > + } > } > else if (((code1 == POINTER_TYPE || TYPE_PTRDATAMEM_P (type1)) > && null_ptr_cst_p (op0)) > @@ -4445,6 +4462,23 @@ cp_build_binary_op (location_t location, > warning (OPT_Waddress, "the address of %qD will never be NULL", > TREE_OPERAND (op1, 0)); > } > + > + if (CONVERT_EXPR_P (op1) > + && TREE_CODE (TREE_TYPE (TREE_OPERAND (op1, 0))) > + == REFERENCE_TYPE) > + { > + tree inner_op1 = op1; > + STRIP_NOPS (inner_op1); > + > + if ((complain & tf_warning) > + && c_inhibit_evaluation_warnings == 0 > + && !TREE_NO_WARNING (op1) > + && DECL_P (inner_op1)) > + warning (OPT_Waddress, > + "the compiler can assume that the address of " > + "%qD will never be NULL", > + inner_op1); > + } > } > else if ((code0 == POINTER_TYPE && code1 == POINTER_TYPE) > || (TYPE_PTRDATAMEM_P (type0) && TYPE_PTRDATAMEM_P (type1))) > diff --git a/gcc/testsuite/g++.dg/warn/Walways-true-3.C b/gcc/testsuite/g++.dg/warn/Walways-true-3.C > new file mode 100644 > index 0000000..0d13d3f > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/Walways-true-3.C > @@ -0,0 +1,45 @@ > +// { dg-options "-Waddress" } > + > +void foo (void); > + > +int d; > +int &c = d; > + > +void > +bar (int &a) > +{ > + int &b = a; > + > + if ((int *)&a) // { dg-warning "address of" } > + foo (); > + > + if (&b) // { dg-warning "address of" } > + foo (); > + > + if (!&c) // { dg-warning "address of" } > + foo (); > + > + if (!&(int &)(int &)a) // { dg-warning "address of" } > + foo (); > + > + if (&a == 0) // { dg-warning "never be NULL" } > + foo (); > + > + if (&b != 0) // { dg-warning "never be NULL" } > + foo (); > + > + if (0 == &(int &)(int &)c) // { dg-warning "never be NULL" } > + foo (); > + > + if (&a != (int *)0) // { dg-warning "never be NULL" } > + foo (); > +} > + > +bool > +bar_1 (int &a) > +{ > + if (d == 5) > + return &a; // { dg-warning "address of" } > + else > + return !&(int &)(int &)a; // { dg-warning "address of" } > +} > -- > 2.4.0.rc2.31.g7c597ef > Ping.
On Sun, May 3, 2015 at 5:29 PM, Patrick Palka <patrick@parcs.ath.cx> wrote: > On Sun, Apr 26, 2015 at 8:56 PM, Patrick Palka <patrick@parcs.ath.cx> wrote: >> Here is an updated version of the patch with, hopefully, all your >> suggestions made. I decided to add calls to STRIP_NOPS before emitting >> the warning so that we properly warn for cases where there's a cast in >> between the whole thing, e.g. >> >> if (!&(int &)a) >> >> I also added guards to emit the warnings only when the stripped operand >> is actually a decl so that we don't pass a non-decl argument to >> warning_at() which can happen in a case like >> >> if (!&(int &)*(int *)0) >> >> Does this look OK now after testing? >> >> gcc/c-family/ChangeLog: >> >> PR c++/65168 >> * c-common.c (c_common_truthvalue_conversion): Warn when >> converting an address of a reference to a truth value. >> >> gcc/cp/ChangeLog: >> >> PR c++/65168 >> * typeck.c (cp_build_binary_op): Warn when comparing an address >> of a reference against NULL. >> >> gcc/testsuite/ChangeLog: >> >> PR c++/65168 >> g++.dg/warn/Walways-true-3.C: New test. >> --- >> gcc/c-family/c-common.c | 14 ++++++++++ >> gcc/cp/typeck.c | 34 ++++++++++++++++++++++ >> gcc/testsuite/g++.dg/warn/Walways-true-3.C | 45 ++++++++++++++++++++++++++++++ >> 3 files changed, 93 insertions(+) >> create mode 100644 gcc/testsuite/g++.dg/warn/Walways-true-3.C >> >> diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c >> index 9797e17..c353c72 100644 >> --- a/gcc/c-family/c-common.c >> +++ b/gcc/c-family/c-common.c >> @@ -4806,6 +4806,20 @@ c_common_truthvalue_conversion (location_t location, tree expr) >> tree totype = TREE_TYPE (expr); >> tree fromtype = TREE_TYPE (TREE_OPERAND (expr, 0)); >> >> + if (POINTER_TYPE_P (totype) >> + && TREE_CODE (fromtype) == REFERENCE_TYPE) >> + { >> + tree inner = expr; >> + STRIP_NOPS (inner); >> + >> + if (DECL_P (inner)) >> + warning_at (location, >> + OPT_Waddress, >> + "the compiler can assume that the address of " >> + "%qD will always evaluate to %<true%>", >> + inner); >> + } >> + >> /* Don't cancel the effect of a CONVERT_EXPR from a REFERENCE_TYPE, >> since that affects how `default_conversion' will behave. */ >> if (TREE_CODE (totype) == REFERENCE_TYPE >> diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c >> index 91db32a..13fb401 100644 >> --- a/gcc/cp/typeck.c >> +++ b/gcc/cp/typeck.c >> @@ -4423,6 +4423,23 @@ cp_build_binary_op (location_t location, >> warning (OPT_Waddress, "the address of %qD will never be NULL", >> TREE_OPERAND (op0, 0)); >> } >> + >> + if (CONVERT_EXPR_P (op0) >> + && TREE_CODE (TREE_TYPE (TREE_OPERAND (op0, 0))) >> + == REFERENCE_TYPE) >> + { >> + tree inner_op0 = op0; >> + STRIP_NOPS (inner_op0); >> + >> + if ((complain & tf_warning) >> + && c_inhibit_evaluation_warnings == 0 >> + && !TREE_NO_WARNING (op0) >> + && DECL_P (inner_op0)) >> + warning (OPT_Waddress, >> + "the compiler can assume that the address of " >> + "%qD will never be NULL", >> + inner_op0); >> + } >> } >> else if (((code1 == POINTER_TYPE || TYPE_PTRDATAMEM_P (type1)) >> && null_ptr_cst_p (op0)) >> @@ -4445,6 +4462,23 @@ cp_build_binary_op (location_t location, >> warning (OPT_Waddress, "the address of %qD will never be NULL", >> TREE_OPERAND (op1, 0)); >> } >> + >> + if (CONVERT_EXPR_P (op1) >> + && TREE_CODE (TREE_TYPE (TREE_OPERAND (op1, 0))) >> + == REFERENCE_TYPE) >> + { >> + tree inner_op1 = op1; >> + STRIP_NOPS (inner_op1); >> + >> + if ((complain & tf_warning) >> + && c_inhibit_evaluation_warnings == 0 >> + && !TREE_NO_WARNING (op1) >> + && DECL_P (inner_op1)) >> + warning (OPT_Waddress, >> + "the compiler can assume that the address of " >> + "%qD will never be NULL", >> + inner_op1); >> + } >> } >> else if ((code0 == POINTER_TYPE && code1 == POINTER_TYPE) >> || (TYPE_PTRDATAMEM_P (type0) && TYPE_PTRDATAMEM_P (type1))) >> diff --git a/gcc/testsuite/g++.dg/warn/Walways-true-3.C b/gcc/testsuite/g++.dg/warn/Walways-true-3.C >> new file mode 100644 >> index 0000000..0d13d3f >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/warn/Walways-true-3.C >> @@ -0,0 +1,45 @@ >> +// { dg-options "-Waddress" } >> + >> +void foo (void); >> + >> +int d; >> +int &c = d; >> + >> +void >> +bar (int &a) >> +{ >> + int &b = a; >> + >> + if ((int *)&a) // { dg-warning "address of" } >> + foo (); >> + >> + if (&b) // { dg-warning "address of" } >> + foo (); >> + >> + if (!&c) // { dg-warning "address of" } >> + foo (); >> + >> + if (!&(int &)(int &)a) // { dg-warning "address of" } >> + foo (); >> + >> + if (&a == 0) // { dg-warning "never be NULL" } >> + foo (); >> + >> + if (&b != 0) // { dg-warning "never be NULL" } >> + foo (); >> + >> + if (0 == &(int &)(int &)c) // { dg-warning "never be NULL" } >> + foo (); >> + >> + if (&a != (int *)0) // { dg-warning "never be NULL" } >> + foo (); >> +} >> + >> +bool >> +bar_1 (int &a) >> +{ >> + if (d == 5) >> + return &a; // { dg-warning "address of" } >> + else >> + return !&(int &)(int &)a; // { dg-warning "address of" } >> +} >> -- >> 2.4.0.rc2.31.g7c597ef >> > > Ping. Ping.
Let's use explicit location for the C++ front end warnings, too. OK with that change. Jason
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index 9797e17..c353c72 100644 --- a/gcc/c-family/c-common.c +++ b/gcc/c-family/c-common.c @@ -4806,6 +4806,20 @@ c_common_truthvalue_conversion (location_t location, tree expr) tree totype = TREE_TYPE (expr); tree fromtype = TREE_TYPE (TREE_OPERAND (expr, 0)); + if (POINTER_TYPE_P (totype) + && TREE_CODE (fromtype) == REFERENCE_TYPE) + { + tree inner = expr; + STRIP_NOPS (inner); + + if (DECL_P (inner)) + warning_at (location, + OPT_Waddress, + "the compiler can assume that the address of " + "%qD will always evaluate to %<true%>", + inner); + } + /* Don't cancel the effect of a CONVERT_EXPR from a REFERENCE_TYPE, since that affects how `default_conversion' will behave. */ if (TREE_CODE (totype) == REFERENCE_TYPE diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index 91db32a..13fb401 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -4423,6 +4423,23 @@ cp_build_binary_op (location_t location, warning (OPT_Waddress, "the address of %qD will never be NULL", TREE_OPERAND (op0, 0)); } + + if (CONVERT_EXPR_P (op0) + && TREE_CODE (TREE_TYPE (TREE_OPERAND (op0, 0))) + == REFERENCE_TYPE) + { + tree inner_op0 = op0; + STRIP_NOPS (inner_op0); + + if ((complain & tf_warning) + && c_inhibit_evaluation_warnings == 0 + && !TREE_NO_WARNING (op0) + && DECL_P (inner_op0)) + warning (OPT_Waddress, + "the compiler can assume that the address of " + "%qD will never be NULL", + inner_op0); + } } else if (((code1 == POINTER_TYPE || TYPE_PTRDATAMEM_P (type1)) && null_ptr_cst_p (op0)) @@ -4445,6 +4462,23 @@ cp_build_binary_op (location_t location, warning (OPT_Waddress, "the address of %qD will never be NULL", TREE_OPERAND (op1, 0)); } + + if (CONVERT_EXPR_P (op1) + && TREE_CODE (TREE_TYPE (TREE_OPERAND (op1, 0))) + == REFERENCE_TYPE) + { + tree inner_op1 = op1; + STRIP_NOPS (inner_op1); + + if ((complain & tf_warning) + && c_inhibit_evaluation_warnings == 0 + && !TREE_NO_WARNING (op1) + && DECL_P (inner_op1)) + warning (OPT_Waddress, + "the compiler can assume that the address of " + "%qD will never be NULL", + inner_op1); + } } else if ((code0 == POINTER_TYPE && code1 == POINTER_TYPE) || (TYPE_PTRDATAMEM_P (type0) && TYPE_PTRDATAMEM_P (type1))) diff --git a/gcc/testsuite/g++.dg/warn/Walways-true-3.C b/gcc/testsuite/g++.dg/warn/Walways-true-3.C new file mode 100644 index 0000000..0d13d3f --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Walways-true-3.C @@ -0,0 +1,45 @@ +// { dg-options "-Waddress" } + +void foo (void); + +int d; +int &c = d; + +void +bar (int &a) +{ + int &b = a; + + if ((int *)&a) // { dg-warning "address of" } + foo (); + + if (&b) // { dg-warning "address of" } + foo (); + + if (!&c) // { dg-warning "address of" } + foo (); + + if (!&(int &)(int &)a) // { dg-warning "address of" } + foo (); + + if (&a == 0) // { dg-warning "never be NULL" } + foo (); + + if (&b != 0) // { dg-warning "never be NULL" } + foo (); + + if (0 == &(int &)(int &)c) // { dg-warning "never be NULL" } + foo (); + + if (&a != (int *)0) // { dg-warning "never be NULL" } + foo (); +} + +bool +bar_1 (int &a) +{ + if (d == 5) + return &a; // { dg-warning "address of" } + else + return !&(int &)(int &)a; // { dg-warning "address of" } +}