[Bug lto/112716] New: LTO optimization with struct of variable ssize

[Bug lto/112716] New: LTO optimization with struct of variable ssize

[muecker at gwdg dot de]

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

            Bug ID: 112716
           Summary: LTO optimization with struct of variable ssize
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: lto
          Assignee: unassigned at gcc dot gnu.org
          Reporter: muecker at gwdg dot de
                CC: marxin at gcc dot gnu.org
  Target Milestone: ---

The following code using a GNU extension gets miscompiled due to incorrect
aliasing assumptions with -flto and -O2: 

gcc -flto -O2 y.c y2.c  -DT1="int[n]" -DT2="int[n]"
// y.c
void bar(void*);

[[gnu::noinline,gnu::noipa]]
int foo(void *p, void *q)
{
        int n = 5;
        struct foo { int x; typeof(T1) y; } *p2 = p;
        p2->x = 1;
        bar(q);
        return p2->x;
}

int main()
{
        int n = 5;
        void *p = __builtin_malloc(sizeof(struct foo { int x; typeof(T1) y;
}));

        if (!p)
                return 0;

        if (2 != foo(p, p))
                __builtin_abort();

        return 0;
}
// y2
void bar(void* q)
{       
        int n = 5;
        struct foo { int x; typeof(T2) y; } *q2 = q;
        q2->x = 2;
}

[Bug lto/112716] LTO optimization with struct of variable ssize

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |alias

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I am 99% sure they are different types and have different aliasing  sets which
case this is undefined.


That is struct foo inside foo and struct foo inside bar are 2 totally different
types and therefor a store to one won't change a store to another.

[Bug lto/112716] LTO optimization with struct of variable ssize

[uecker at gcc dot gnu.org]

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

uecker at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |uecker at gcc dot gnu.org

--- Comment #2 from uecker at gcc dot gnu.org ---
.
Inside the same TU there are different types.  But note that this is across TUs
where the structs with same tag and and compatible members are supposed to be
compatible (they could come from a shared header).  I would assume these rules
hold also for the GNU extension.  In C23 the structs will be compatible also
inside a TU (without GNU extensions, but I I think the same rules should then
also apply to structs with the GNU extension).

[Bug lto/112716] LTO optimization with struct of variable ssize

[sjames at gcc dot gnu.org]

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

--- Comment #3 from Sam James <sjames at gcc dot gnu.org> ---
No warning from -Wlto-type-mismatch.

[Bug lto/112716] LTO optimization with struct with variable size

[rguenth at gcc dot gnu.org]

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

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
Not that local types are never "merged" for cross-TU aliasing, they keep being
distinct types.  In particular this applies to VLA types since the reference to
function-local vars ties them to the function IL sections and thus they do
not become subject to global var/type unification.

If it's undefined in C then the behavior is OK and expected.

I don't think the C standard requirement can extend to VLA types this way.

[Bug lto/112716] LTO optimization with struct with variable size

[muecker at gwdg dot de]

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

--- Comment #5 from Martin Uecker <muecker at gwdg dot de> ---
It works (and is required to work) for other types, e.g.

[[gnu::noinline,gnu::noipa]]
int foo(void *p, void *q)
{
        int n = 5;
        int (*p2)[n] = p;
        (*p2)[0] = 1;
        bar(q);
        return (*p2)[0];
}

void bar(void* q)
{       
        int n = 5;
        int (*q2)[n] = q;
        (*q2)[0] = 2;
}

One could argue that there is a weaker requirement for having an object of type
int[n] present than for struct { int x[n]; } because we do not access the array
directly but it decays to a pointer. (but then, other languages have array
assignment, so why does the middle-end care about this C peculiarity?) 

There is also no indication in documentation that structs with variable size
follow different rules than conventional structs.   So my preference would be
to fix this somehow.  Otherwise we should document this as a limitation.

[Bug lto/112716] LTO optimization with struct with variable size

[rguenther at suse dot de]

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

--- Comment #6 from rguenther at suse dot de <rguenther at suse dot de> ---
On Mon, 27 Nov 2023, muecker at gwdg dot de wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112716
> 
> --- Comment #5 from Martin Uecker <muecker at gwdg dot de> ---
> It works (and is required to work) for other types, e.g.
> 
> [[gnu::noinline,gnu::noipa]]
> int foo(void *p, void *q)
> {
>         int n = 5;
>         int (*p2)[n] = p;
>         (*p2)[0] = 1;
>         bar(q);
>         return (*p2)[0];
> }
> 
> void bar(void* q)
> {       
>         int n = 5;
>         int (*q2)[n] = q;
>         (*q2)[0] = 2;
> }
> 
> One could argue that there is a weaker requirement for having an object of type
> int[n] present than for struct { int x[n]; } because we do not access the array
> directly but it decays to a pointer. (but then, other languages have array
> assignment, so why does the middle-end care about this C peculiarity?) 

So in theory we could disregard the VLA-sized components for TBAA
which would make the access behaved as if it were a int * indirect access.
I think if you write it as array as above that's already what happens.

Note that even without LTO when you enable inlining you'd expose two
different structures with two different alias-sets, possibly leading
to wrong-code (look at the RTL expansion dump and check alias-sets).

As said, for arrays it probably works as-is since these gets the alias
set of the component.

> There is also no indication in documentation that structs with variable size
> follow different rules than conventional structs.   So my preference would be
> to fix this somehow.  Otherwise we should document this as a limitation.

Local declared structs in principle follow the same logic (but they
get promoted "global" due to implementation details I think).

[Bug lto/112716] LTO optimization with struct with variable size

[uecker at gcc dot gnu.org]

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

--- Comment #7 from uecker at gcc dot gnu.org ---
(In reply to rguenther@suse.de from comment #6)
> On Mon, 27 Nov 2023, muecker at gwdg dot de wrote:
> 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112716
> > 
> > --- Comment #5 from Martin Uecker <muecker at gwdg dot de> ---
> > It works (and is required to work) for other types, e.g.
> > 
> > [[gnu::noinline,gnu::noipa]]
> > int foo(void *p, void *q)
> > {
> >         int n = 5;
> >         int (*p2)[n] = p;
> >         (*p2)[0] = 1;
> >         bar(q);
> >         return (*p2)[0];
> > }
> > 
> > void bar(void* q)
> > {       
> >         int n = 5;
> >         int (*q2)[n] = q;
> >         (*q2)[0] = 2;
> > }
> > 
> > One could argue that there is a weaker requirement for having an object of type
> > int[n] present than for struct { int x[n]; } because we do not access the array
> > directly but it decays to a pointer. (but then, other languages have array
> > assignment, so why does the middle-end care about this C peculiarity?) 
> 
> So in theory we could disregard the VLA-sized components for TBAA
> which would make the access behaved as if it were a int * indirect access.
> I think if you write it as array as above that's already what happens.

What does "disregard the VLA-sized component" mean?

For full interoperability I think one either has to assign 
equivalence classes for structs by ignoring the sizes of all
fields of array type (not just VLA) and also the offsets 
for the following struct members, or, alternately, one has
to give alias set 0 to  structs with VLA fields.  The later
seems preferable and is what I have included in the current
version of my patch for C23 for structs with VLA fields 
(but we could also decide to not support full ISO C rules for
such structs, of course)

> 
> Note that even without LTO when you enable inlining you'd expose two
> different structures with two different alias-sets, possibly leading
> to wrong-code (look at the RTL expansion dump and check alias-sets).

Yes, for pre C23 this is true for all structs even without VLA.
But for C23 this changes.

The main case where the GNU extension is interesting and useful is
when the VLA field is at the end. So at least for this case it would
be nice to have a solution.

> 
> As said, for arrays it probably works as-is since these gets the alias
> set of the component.
> 
> > There is also no indication in documentation that structs with variable size
> > follow different rules than conventional structs.   So my preference would be
> > to fix this somehow.  Otherwise we should document this as a limitation.
> 
> Local declared structs in principle follow the same logic (but they
> get promoted "global" due to implementation details I think).

[Bug lto/112716] LTO optimization with struct with variable size

[rguenth at gcc dot gnu.org]

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

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to uecker from comment #7)
> (In reply to rguenther@suse.de from comment #6)
> > On Mon, 27 Nov 2023, muecker at gwdg dot de wrote:
> > 
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112716
> > > 
> > > --- Comment #5 from Martin Uecker <muecker at gwdg dot de> ---
> > > It works (and is required to work) for other types, e.g.
> > > 
> > > [[gnu::noinline,gnu::noipa]]
> > > int foo(void *p, void *q)
> > > {
> > >         int n = 5;
> > >         int (*p2)[n] = p;
> > >         (*p2)[0] = 1;
> > >         bar(q);
> > >         return (*p2)[0];
> > > }
> > > 
> > > void bar(void* q)
> > > {       
> > >         int n = 5;
> > >         int (*q2)[n] = q;
> > >         (*q2)[0] = 2;
> > > }
> > > 
> > > One could argue that there is a weaker requirement for having an object of type
> > > int[n] present than for struct { int x[n]; } because we do not access the array
> > > directly but it decays to a pointer. (but then, other languages have array
> > > assignment, so why does the middle-end care about this C peculiarity?) 
> > 
> > So in theory we could disregard the VLA-sized components for TBAA
> > which would make the access behaved as if it were a int * indirect access.
> > I think if you write it as array as above that's already what happens.
> 
> What does "disregard the VLA-sized component" mean?

Hmm, it wouldn't help I guess.  The problem in the end will be
disambiguation of aggregate copies, not so much the accesses to
the array elements of a VLA component.

> For full interoperability I think one either has to assign 
> equivalence classes for structs by ignoring the sizes of all
> fields of array type (not just VLA) and also the offsets 
> for the following struct members, or, alternately, one has
> to give alias set 0 to  structs with VLA fields.  The later
> seems preferable and is what I have included in the current
> version of my patch for C23 for structs with VLA fields 
> (but we could also decide to not support full ISO C rules for
> such structs, of course)

Using alias set 0 of course works (also with LTO).

> > 
> > Note that even without LTO when you enable inlining you'd expose two
> > different structures with two different alias-sets, possibly leading
> > to wrong-code (look at the RTL expansion dump and check alias-sets).
> 
> Yes, for pre C23 this is true for all structs even without VLA.
> But for C23 this changes.
> 
> The main case where the GNU extension is interesting and useful is
> when the VLA field is at the end. So at least for this case it would
> be nice to have a solution.

So what's the GNU extension here?  VLA inside structs are not a C thing?
I suppose if they were then C23 would make only the "abstract" types
compatible but the concrete types (actual 'n') would only be compatible
when 'n' is the same?

I think the GNU extension is incomplete, IIRC you can't have

foo (int n, struct bar { int x; int a[n]; } b) -> struct bar
{
  return bar;
}

and there's no way to 'declare' bar in a way that it's usable across
functions.

So I'm not sure assigning C23 "semantics" to this extension is very
useful.  Your examples are awkward workarounds for an incomplete
language extension.

[Bug lto/112716] LTO optimization with struct with variable size

[muecker at gwdg dot de]

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

--- Comment #9 from Martin Uecker <muecker at gwdg dot de> ---
(In reply to Richard Biener from comment #8)
> (In reply to uecker from comment #7)
>

> > > 
> > > Note that even without LTO when you enable inlining you'd expose two
> > > different structures with two different alias-sets, possibly leading
> > > to wrong-code (look at the RTL expansion dump and check alias-sets).
> > 
> > Yes, for pre C23 this is true for all structs even without VLA.
> > But for C23 this changes.
> > 
> > The main case where the GNU extension is interesting and useful is
> > when the VLA field is at the end. So at least for this case it would
> > be nice to have a solution.
> 
> So what's the GNU extension here?  VLA inside structs are not a C thing?

ISO C does not currently allow VLAs or variably-modified types
inside structs. So all these are GNU extensions.

WG14 is thinking about allowing pointers to VLAs  inside structs.

struct foo {
  int n;
  char (*buf)[.n];
};

But this is a bit different because it would not depend on an
external value.

> I suppose if they were then C23 would make only the "abstract" types
> compatible but the concrete types (actual 'n') would only be compatible
> when 'n' is the same?

Yes, this is how it works for other variably-modified types
in C (since C99) where it is then run-time undefined behavior
if 'n' turns out not to be the same.

> 
> I think the GNU extension is incomplete, IIRC you can't have
> 
> foo (int n, struct bar { int x; int a[n]; } b) -> struct bar
> {
>   return bar;
> }
> 
> and there's no way to 'declare' bar in a way that it's usable across
> functions.

You could declare it again in another function

void xyz(int n)
{
  struct bar { int x; int a[n]; } y;
  foo(n, y);
}

and with C23 compatibility rules this would work. 

> 
> So I'm not sure assigning C23 "semantics" to this extension is very
> useful.  Your examples are awkward workarounds for an incomplete
> language extension.

In any case, we already have the extension and we should
either we make it more useful, or document its limitations, 
or deprecate it completely.