From: Patrick Palka <patrick@parcs.ath.cx>
To: GCC Patches <gcc-patches@gcc.gnu.org>
Cc: "Jason Merrill" <jason@redhat.com>,
"Manuel López-Ibáñez" <lopezibanez@gmail.com>,
"Patrick Palka" <patrick@parcs.ath.cx>
Subject: Re: [PATCH] Emit -Waddress warnings for comparing address of reference against NULL
Date: Wed, 10 Jun 2015 02:46:00 -0000 [thread overview]
Message-ID: <CA+C-WL_jDHcvC-EcPXuW9J2UFB4F10etyv0RcnaO-Q-ChFu5SQ@mail.gmail.com> (raw)
In-Reply-To: <CA+C-WL9AocJccd0JBrCEBm1zv54m58ybjQaKtrdKKyV4eVHHYQ@mail.gmail.com>
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.
next prev parent reply other threads:[~2015-06-10 2:39 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-21 2:37 Patrick Palka
2015-04-23 15:12 ` Jason Merrill
2015-04-23 15:34 ` Manuel López-Ibáñez
2015-04-23 18:17 ` Jason Merrill
2015-04-24 1:16 ` Patrick Palka
2015-04-27 0:56 ` Patrick Palka
2015-05-03 21:29 ` Patrick Palka
2015-06-10 2:46 ` Patrick Palka [this message]
2015-06-11 21:47 ` Jason Merrill
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CA+C-WL_jDHcvC-EcPXuW9J2UFB4F10etyv0RcnaO-Q-ChFu5SQ@mail.gmail.com \
--to=patrick@parcs.ath.cx \
--cc=gcc-patches@gcc.gnu.org \
--cc=jason@redhat.com \
--cc=lopezibanez@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).