On 03/17/2016 10:48 AM, Patrick Palka wrote:
On Thu, Mar 17, 2016 at 12:27 PM, Jeff Law <law@redhat.com> wrote:
On 03/16/2016 06:43 PM, Martin Sebor wrote:
@@ -3974,6 +3974,38 @@ build_vec_cmp (tree_code code, tree type,
return build3 (VEC_COND_EXPR, type, cmp, minus_one_vec,
zero_vec);
}
+/* Possibly warn about an address never being NULL. */
+
+static void
+warn_for_null_address (location_t location, tree op, tsubst_flags_t
complain)
+{
...
+ if (TREE_CODE (cop) == ADDR_EXPR
+ && decl_with_nonnull_addr_p (TREE_OPERAND (cop, 0))
+ && !TREE_NO_WARNING (cop))
+ warning_at (location, OPT_Waddress, "the address of %qD will
never "
+ "be NULL", TREE_OPERAND (cop, 0));
+
+ if (CONVERT_EXPR_P (op)
+ && TREE_CODE (TREE_TYPE (TREE_OPERAND (op, 0))) ==
REFERENCE_TYPE)
+ {
+ tree inner_op = op;
+ STRIP_NOPS (inner_op);
+
+ if (DECL_P (inner_op))
+ warning_at (location, OPT_Waddress,
+ "the compiler can assume that the address of "
+ "%qD will never be NULL", inner_op);
Since I noted the subtle differences between the phrasing of
the various -Waddress warnings in the bug, I have to ask: what is
the significance of the difference between the two warnings here?
Would it not be appropriate to issue the first warning in the latter
case? Or perhaps even use the same text as is already used elsewhere:
"the address of %qD will always evaluate as âtrueâ" (since it may not
be the macro NULL that's mentioned in the expression).
They were added at different times AFAICT. The former is fairly old
(Douglas Gregor, 2008) at this point. The latter was added by
Patrick Palka
for 65168 about a year ago.
You could directly ask Patrick about motivations for a different
message.
There is no plausible way for the address of a non-reference variable
to be NULL even in code with UB (aside from __attribute__ ((weak)) in
which case the warning is suppressed). But the address of a reference
can easily seem to be NULL if one performs UB and assigns to it *(int
*)NULL or something like that. I think that was my motivation, anyway
:)
Thanks (everyone) for the explanation.
I actually think the warning Patrick added is the most accurate
and would be appropriate in all cases.
I suppose what bothers me besides the mention of NULL even when
there is no NULL in the code, is that a) the text of the warnings
is misleading (contradictory) in some interesting cases, and b)
I can't think of a way in which the difference between the three
phrasings of the diagnostic could be useful to a user. All three
imply the same thing: compilers can and GCC is some cases does
assume that the address of an ordinary (non weak) function, object,
or reference is not null.
To see (a), consider the invalid test program below, in which
GCC makes this assumption about the address of i even though
the warning doesn't mention it (but it makes a claim that's
contrary to the actual address), yet doesn't make the same
assumption about the address of the reference even though
the diagnostic says it can.
While I would find the warning less misleading if it simply said
in all three cases: "the address of 'x' will always evaluate as
âtrueâ" I think it would be even more accurate if it said
"the address of 'x' may be assumed to evaluate to âtrueâ" That
avoids making claims about whether or not it actually is null,
doesn't talk about the NULL macro when one isn't used in the
code, and by saying "may assume" it allows for both making
the assumption as well as not making one.
I'm happy to submit a patch to make this change in stage 1 if
no one objects to it.
Martin
$ cat x.c && /home/msebor/build/gcc-trunk-svn/gcc/xgcc
-B/home/msebor/build/gcc-trunk-svn/gcc -c -xc++ x.c &&
/home/msebor/build/gcc-trunk-svn/gcc/xgcc
-B/home/msebor/build/gcc-trunk-svn/gcc -DMAIN -Wall -Wextra -Wpedantic
x.o -xc++ x.c && ./a.out
#if MAIN
extern int i;
extern int &r;
extern void f ();
int main ()
{
f ();
#define TEST(x) __builtin_printf ("%s is %s\n", #x, (x) ? "true" : "false")
TEST (&i != 0);
TEST (&r != 0);
TEST (&i);
}
#else
extern __attribute__ ((weak)) int i;
int &r = i;
void f ()
{
__builtin_printf ("&i = %p\n&r = %p\n", &i, &r);
}
#endif
x.c: In function âint main()â:
x.c:14:17: warning: the address of âiâ will never be NULL [-Waddress]
TEST (&i != 0);
^
x.c:12:54: note: in definition of macro âTESTâ
#define TEST(x) __builtin_printf ("%s is %s\n", #x, (x) ? "true" :
"false")
^
x.c:15:14: warning: the compiler can assume that the address of ârâ will
never be NULL [-Waddress]
TEST (&r != 0);
~~~^~~~
x.c:12:54: note: in definition of macro âTESTâ
#define TEST(x) __builtin_printf ("%s is %s\n", #x, (x) ? "true" :
"false")
^
x.c:12:68: warning: the address of âiâ will always evaluate as âtrueâ
[-Waddres]
#define TEST(x) __builtin_printf ("%s is %s\n", #x, (x) ? "true" :
"false")
^
x.c:16:5: note: in expansion of macro âTESTâ
TEST (&i);
^~~~
&i = (nil)
&r = (nil)
&i != 0 is true
&r != 0 is false
&i is true