[Bug c/102967] New: -Waddress with nested structures: Incorrect "the comparison will always evaluate as 'true'"

[Bug c/102967] New: -Waddress with nested structures: Incorrect "the comparison will always evaluate as 'true'"

[andrew.cooper3 at citrix dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102967

            Bug ID: 102967
           Summary: -Waddress with nested structures: Incorrect "the
                    comparison will always evaluate as 'true'"
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: andrew.cooper3 at citrix dot com
  Target Milestone: ---

Hello,

I have been compiling Xen with gcc/master, and found what appears to be a
regression from GCC 11.

https://godbolt.org/z/qa61fs1hK is a simplified repro.

The address comparison seems to be incorrectly applied to only of the two
possible options.  The expression as a whole is most certainly not
unconditionally true.

[Bug c/102967] confusing location in -Waddress for a subexpression of a ternary expression

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102967

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
           Keywords|                            |diagnostic
   Last reconfirmed|                            |2021-10-27
     Ever confirmed|0                           |1
            Summary|-Waddress with nested       |confusing location in
                   |structures: Incorrect "the  |-Waddress for a
                   |comparison will always      |subexpression of a ternary
                   |evaluate as 'true'"         |expression
                 CC|                            |msebor at gcc dot gnu.org

--- Comment #1 from Martin Sebor <msebor at gcc dot gnu.org> ---
The warning is intended: it points out that the second operand of the
conditional expression is necessarily true:

  if ( !(pa ? &pa->c : NULL) )
              ^^^^^^

There's no point in testing the address of a member for equality to null
because  the member of no object can reside at that address.  The above can be
simplified to

  if (!pa)

Below are a couple of small examples to illustrate.

I confirm this as a report of the underlining being confusing.  In the message
on Godbolt:

   20 |     if ( !(pa ? &pa->c : NULL) ) // Macro expanded, just to check. 
Still wrong.
      |          ^

and in the first message below it could stand to be improved to point to the
second operand rather than the first (as I did above).

I also note that the C front end diagnoses both expressions as expected but the
C++ front end only the latter one.  That seems like an omission to me that
should be fixed.

$ cat a.c && gcc -S -Wall a.cstruct A { int i; };

int f (struct A *p)
{
  if (p ? &p->i : 0)   // -Waddress
    return 0;
  return 1;
}

int g (struct A *p)
{
  if (&p->i)           // -Waddress
    return 0;
  return 1;
}

a.c: In function ‘f’:
a.c:5:7: warning: the comparison will always evaluate as ‘true’ for the address
of ‘i’ will never be NULL [-Waddress]
    5 |   if (p ? &p->i : 0)   // -Waddress
      |       ^
a.c:1:16: note: ‘i’ declared here
    1 | struct A { int i; };
      |                ^
a.c: In function ‘g’:
a.c:12:7: warning: the comparison will always evaluate as ‘true’ for the
address of ‘i’ will never be NULL [-Waddress]
   12 |   if (&p->i)           // -Waddress
      |       ^
a.c:1:16: note: ‘i’ declared here
    1 | struct A { int i; };
      |                ^

When reporting bugs, please be sure to include the full test case and its
output as requested at https://gcc.gnu.org/bugs/#need.  Links to external sites
are not a substitute.  They might stop working, or the compiler used there
might become out of date, leaving us with insufficient detail to analyze the
report.

[Bug c/102967] confusing location in -Waddress for a subexpression of a ternary expression

[andrew.cooper3 at citrix dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102967

--- Comment #2 from Andrew Cooper <andrew.cooper3 at citrix dot com> ---
(In reply to Martin Sebor from comment #1)
> The warning is intended: it points out that the second operand of the
> conditional expression is necessarily true:
> 
>   if ( !(pa ? &pa->c : NULL) )
>               ^^^^^^
> 
> There's no point in testing the address of a member for equality to null
> because  the member of no object can reside at that address.  The above can
> be simplified to
> 
>   if (!pa)

Hmm, true.  I suppose I got hung up on the statement made by the diagnostic,
rather than the implication that a simplification could be made.

Moving the underlining would certainly help.

> When reporting bugs, please be sure to include the full test case and its
> output as requested at https://gcc.gnu.org/bugs/#need.

My apologies.  I'll try to be better next time.

[Bug c/102967] confusing location in -Waddress for a subexpression of a ternary expression

[jbeulich at suse dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102967

jbeulich at suse dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jbeulich at suse dot com

--- Comment #3 from jbeulich at suse dot com ---
(In reply to Martin Sebor from comment #1)
> The warning is intended: it points out that the second operand of the
> conditional expression is necessarily true:
> 
>   if ( !(pa ? &pa->c : NULL) )
>               ^^^^^^
> 
> There's no point in testing the address of a member for equality to null
> because  the member of no object can reside at that address.

Except that if pa is NULL and c is the first member of typeof(*pa), then &pa->c
is also NULL, isn't it?

>  The above can
> be simplified to
> 
>   if (!pa)

If there was no macro expansion involved in the former case, I'd agree with the
option of simplifying the expression. But the use of the macro expanding to
this construct may be intended, perhaps at least for the code to be
self-documenting. The macro itself, in turn, may be (and in our case is) also
used in contexts where the produced pointer actually gets used as such, not for
boolean NULL / non-NULL checks.

In any event I don't see how it matters here that, as you say, "the second
operand of the conditional expression is necessarily true". Especially
considering the macro aspect, changing to "pa ? 1 : NULL" or (to make the
construct type-correct) "pa ? 1 : 0" isn't an option, yet that's what you
appear to be suggesting (as intermediate step to arrive at simply "pa").

Imo the compiler wrongly connects the use of &pa->c in the conditional
expression to the boolean context of the enclosing if(), even irrespective of
the use of !() around the conditional operator. IOW

    if ( pa ? &pa->c : NULL )

should also go through without any diagnostic (at least as long as the compiler
doesn't also take into account whether the expression is the result of macro
expansion). Otherwise all you do is force people to write less legible code,
e.g. the macro becoming

#define a_to_c(pa) ({ \
    type_of_c *pc_ = (pa) ? &(pa)->c : NULL; \
    pc_; \
})

based on Andrew's observation that breaking out the expression doesn't result
in any warning (didn't check whether that in practice extends to the case
above).

[Bug c/102967] confusing location in -Waddress for a subexpression of a ternary expression

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102967

--- Comment #4 from Martin Sebor <msebor at gcc dot gnu.org> ---
The expression pa->c is only valid if pa points to a valid object.

[Bug c/102967] confusing location in -Waddress for a subexpression of a ternary expression

[jbeulich at suse dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102967

--- Comment #5 from jbeulich at suse dot com ---
(In reply to Martin Sebor from comment #4)
> The expression pa->c is only valid if pa points to a valid object.

Well, yes, you may not deref pa if it's NULL, i.e. I agree for pa->c. But is
&pa->c actually a deref?

[Bug c/102967] confusing location in -Waddress for a subexpression of a ternary expression

[schwab@linux-m68k.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102967

--- Comment #6 from Andreas Schwab <schwab@linux-m68k.org> ---
&*E is allowed for E == NULL, but I don't think this can be generalized to
&E->m.

[Bug c/102967] confusing location in -Waddress for a subexpression of a ternary expression

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102967

--- Comment #7 from Martin Sebor <msebor at gcc dot gnu.org> ---
Both for the purposes of the warning (which can be more restrictive than what
the language considers valid), and in the C language, the semantics of the ->
expression depend on the first operand designating an object.  In C they're
defined like so:

  A postfix expression followed by the -> operator and an identifier designates
a member of a structure or union object.  The value is that of the named member
of the object to which the first expression points, and is an lvalue. 106)

Footnote 106) If &E is a valid pointer expression (where & is the "address-of"
operator, which generates a pointer to its operand), the expression (&E)->MOS
is the same as E.MOS .

[Bug c/102967] confusing location in -Waddress for a subexpression of a ternary expression

[jbeulich at suse dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102967

--- Comment #8 from jbeulich at suse dot com ---
(In reply to Martin Sebor from comment #7)
> Both for the purposes of the warning (which can be more restrictive than
> what the language considers valid), and in the C language, the semantics of
> the -> expression depend on the first operand designating an object.  In C
> they're defined like so:
> 
>   A postfix expression followed by the -> operator and an identifier
> designates a member of a structure or union object.  The value is that of
> the named member of the object to which the first expression points, and is
> an lvalue. 106)

While gcc 12 has meanwhile been released, it's still not clear how we could at
least work around this (mis-)diagnostic, _without_ simplifying the conditional
expression (which, as said, actually is the result of macro expansion, and the
macro should not be open-coded [and then simplified] here).

It's also not just an issue with the underlining in the diagnostic - the
overall expression !() as well as the conditional expression on its own isn't
"always true" or "always false". It's only the 2nd operand of the ternary
expression which is, but that's not on its own used in boolean context.
Otherwise why would

        if(ps ? (void*)1 : (void*)0)

not result in a similar warning? And note that while

        if(ps ? &ps[10] : (void*)0)

does,

        if(ps ? &(&ps[10])[-10] : (void*)0)

again doesn't, while from &ps[10] being non-NULL (pointing at or one past a
valid object) it imo ought to follow that &(&ps[10])[-10] is also necessarily
non-NULL.

Note further that if the two array indexes don't sum up to 0, the offset
reported in the diagnostic also looks to be wrong - it appears to lack division
by sizeof(*ps).

[Bug c/102967] confusing location in -Waddress for a subexpression of a ternary expression

[amachronic at protonmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102967

Aidan MacDonald <amachronic at protonmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |amachronic at protonmail dot com

--- Comment #9 from Aidan MacDonald <amachronic at protonmail dot com> ---
I am also hitting this with GCC 12.2. One more detail to add is that the info
manual says the -Waddress warning should be suppressed if it's a result of a
macro expansion. This does indeed happen when the macro is the only thing in
the conditional, but not when the macro is part of a larger expression.

Example (https://godbolt.org/z/YEY4Yzdnf):

    #include <stddef.h>

    #define MACRO(buf, off) (off < 0 ? NULL : (void*)&buf[off])

    void func(char *buf, long off)
    {
        if (off < 0 ? NULL : (void*)&buf[off]) { } // warning (correct)
        if (!(off < 0 ? NULL : (void*)&buf[off])) { } // warning (correct)
        if (MACRO(buf, off)) { } // no warning (correct)
        if (!MACRO(buf, off)) { } // gives a false positive -Waddress
    }

[Bug c/102967] confusing location in -Waddress for a subexpression of a ternary expression

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102967

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jroemmler at altair dot com

--- Comment #10 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 105628 has been marked as a duplicate of this bug. ***

[Bug c/102967] confusing location in -Waddress for a subexpression of a ternary expression

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102967

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |arnaud.lb at gmail dot com

--- Comment #11 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 114609 has been marked as a duplicate of this bug. ***

[Bug c/102967] confusing location in -Waddress for a subexpression of a ternary expression

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102967

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |lsof at mailbox dot org

--- Comment #12 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 110402 has been marked as a duplicate of this bug. ***