This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Emit -Waddress warnings for comparing address of reference against NULL
- From: Patrick Palka <patrick at parcs dot ath dot cx>
- To: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Cc: Jason Merrill <jason at redhat dot com>, Manuel LÃpez-IbÃÃez <lopezibanez at gmail dot com>, Patrick Palka <patrick at parcs dot ath dot cx>
- Date: Sun, 3 May 2015 17:29:35 -0400
- Subject: Re: [PATCH] Emit -Waddress warnings for comparing address of reference against NULL
- Authentication-results: sourceware.org; auth=none
- References: <CA+C-WL9eUpg0tSp+3VThi01-M8zZOJOjTqgEL9YbEcTUpVkd-A at mail dot gmail dot com> <1430096197-29836-1-git-send-email-patrick at parcs dot ath dot cx>
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.