public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug tree-optimization/107952] New: tree-object-size: inconsistent size for flexible arrays nested in structs
@ 2022-12-02 14:04 siddhesh at gcc dot gnu.org
  2022-12-05  8:05 ` [Bug tree-optimization/107952] " rguenth at gcc dot gnu.org
                   ` (19 more replies)
  0 siblings, 20 replies; 21+ messages in thread
From: siddhesh at gcc dot gnu.org @ 2022-12-02 14:04 UTC (permalink / raw)
  To: gcc-bugs

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

            Bug ID: 107952
           Summary: tree-object-size: inconsistent size for flexible
                    arrays nested in structs
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: siddhesh at gcc dot gnu.org
  Target Milestone: ---

With -fstrict-flex-arrays, there is a clearer definition of what constitutes
flexible array members, controlled by the frontend.  tree-object-size however
doesn't seem to handle this well enough when the flex array is nested.  e.g.:

typedef struct {
  char pad;
  char data[8];
} F2;

typedef struct {
  unsigned pad;
  F2 flex;
} S2;

#define NULL (void *) 0

__SIZE_TYPE__
nested_flexarray (__SIZE_TYPE__ n)
{
  S2 *p = (S2 *) __builtin_malloc (n);

  return __builtin_dynamic_object_size (p->flex.data, 1);
}

__SIZE_TYPE__
nested_flexarray2 (S2 *p)
{
  return __builtin_dynamic_object_size (p->flex.data, 1);
}

The current behaviour is to treat data as a flex array and nested_flexarray
returns the size as if it were a flex array.  nested_flexarray2 however returns
the subscripted size, thinking of it as a fixed array, which should not be the
expected behaviour with -fstrict-flex-arrays=0.  Instead it should return -1
for maximum size and perhaps 8 as minimum size.

If F2 is changed to:

typedef struct {
  char pad;
  char data[0];
} F2;

the current behaviour ends up returning 0 in both cases, which is incorrect. 
Again, it should return the size as if it were a flex array in nested_flexarray
and -1 for maximum size for nested_flexarray2.

I have a patch that does this, but I need to fix up tests that currently expect
older behaviour and wanted to get some consensus before I fixed them up.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Bug tree-optimization/107952] tree-object-size: inconsistent size for flexible arrays nested in structs
  2022-12-02 14:04 [Bug tree-optimization/107952] New: tree-object-size: inconsistent size for flexible arrays nested in structs siddhesh at gcc dot gnu.org
@ 2022-12-05  8:05 ` rguenth at gcc dot gnu.org
  2022-12-05 12:46 ` siddhesh at gcc dot gnu.org
                   ` (18 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-12-05  8:05 UTC (permalink / raw)
  To: gcc-bugs

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu.org
           Keywords|                            |wrong-code

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC considered this as a flex-array.  Does the C standard not allow this with
data[]?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Bug tree-optimization/107952] tree-object-size: inconsistent size for flexible arrays nested in structs
  2022-12-02 14:04 [Bug tree-optimization/107952] New: tree-object-size: inconsistent size for flexible arrays nested in structs siddhesh at gcc dot gnu.org
  2022-12-05  8:05 ` [Bug tree-optimization/107952] " rguenth at gcc dot gnu.org
@ 2022-12-05 12:46 ` siddhesh at gcc dot gnu.org
  2022-12-05 13:15 ` jakub at gcc dot gnu.org
                   ` (17 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: siddhesh at gcc dot gnu.org @ 2022-12-05 12:46 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #2 from Siddhesh Poyarekar <siddhesh at gcc dot gnu.org> ---
The standard does not allow the nesting, but gcc supports it as an extension.

The middle end does see the array as a flex array correctly, but
tree-object-size doesn't seem to do the right thing consistently enough.  Or
rather, the current behaviour seems a bit mixed up, which is why I want to try
and specify the behaviour for tree-object-size more clearly with all
combinations of -fstrict-flex-array and arrays, i.e. nested or otherwise.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Bug tree-optimization/107952] tree-object-size: inconsistent size for flexible arrays nested in structs
  2022-12-02 14:04 [Bug tree-optimization/107952] New: tree-object-size: inconsistent size for flexible arrays nested in structs siddhesh at gcc dot gnu.org
  2022-12-05  8:05 ` [Bug tree-optimization/107952] " rguenth at gcc dot gnu.org
  2022-12-05 12:46 ` siddhesh at gcc dot gnu.org
@ 2022-12-05 13:15 ` jakub at gcc dot gnu.org
  2022-12-05 15:11 ` rguenther at suse dot de
                   ` (16 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-12-05 13:15 UTC (permalink / raw)
  To: gcc-bugs

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
When writing tree-object-size.c, the intent was to find some balance between
catching up dangerous cases and allowing what a lot of programs in the wild use
and it took about a year to stabilize that.  Sure, with a new non-default
option that requests stricter behavior it is possible to change it.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Bug tree-optimization/107952] tree-object-size: inconsistent size for flexible arrays nested in structs
  2022-12-02 14:04 [Bug tree-optimization/107952] New: tree-object-size: inconsistent size for flexible arrays nested in structs siddhesh at gcc dot gnu.org
                   ` (2 preceding siblings ...)
  2022-12-05 13:15 ` jakub at gcc dot gnu.org
@ 2022-12-05 15:11 ` rguenther at suse dot de
  2022-12-05 15:28 ` siddhesh at gcc dot gnu.org
                   ` (15 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: rguenther at suse dot de @ 2022-12-05 15:11 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #4 from rguenther at suse dot de <rguenther at suse dot de> ---
On Mon, 5 Dec 2022, siddhesh at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107952
> 
> --- Comment #2 from Siddhesh Poyarekar <siddhesh at gcc dot gnu.org> ---
> The standard does not allow the nesting, but gcc supports it as an extension.

Does it allow the nesting when nested in a union?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Bug tree-optimization/107952] tree-object-size: inconsistent size for flexible arrays nested in structs
  2022-12-02 14:04 [Bug tree-optimization/107952] New: tree-object-size: inconsistent size for flexible arrays nested in structs siddhesh at gcc dot gnu.org
                   ` (3 preceding siblings ...)
  2022-12-05 15:11 ` rguenther at suse dot de
@ 2022-12-05 15:28 ` siddhesh at gcc dot gnu.org
  2023-01-23 19:39 ` qinzhao at gcc dot gnu.org
                   ` (14 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: siddhesh at gcc dot gnu.org @ 2022-12-05 15:28 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #5 from Siddhesh Poyarekar <siddhesh at gcc dot gnu.org> ---
(In reply to rguenther@suse.de from comment #4)
> Does it allow the nesting when nested in a union?

data[] cannot be nested directly in a union (i.e. union { char d; char data[];
} is invalid) but something like below is allowed, again as a gcc extension,
not by the standard.  The standard AFAICT doesn't allow anything other than the
flex array directly at the end of the top level struct.

typedef struct {
  unsigned pad;
  union {
    struct {char d; char data[];} f1;
    struct {char d; char data[];} f2;
  } flex;
} S2;

In fact this is the use case that led me into this rabbithole.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Bug tree-optimization/107952] tree-object-size: inconsistent size for flexible arrays nested in structs
  2022-12-02 14:04 [Bug tree-optimization/107952] New: tree-object-size: inconsistent size for flexible arrays nested in structs siddhesh at gcc dot gnu.org
                   ` (4 preceding siblings ...)
  2022-12-05 15:28 ` siddhesh at gcc dot gnu.org
@ 2023-01-23 19:39 ` qinzhao at gcc dot gnu.org
  2023-01-23 19:44 ` qinzhao at gcc dot gnu.org
                   ` (13 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: qinzhao at gcc dot gnu.org @ 2023-01-23 19:39 UTC (permalink / raw)
  To: gcc-bugs

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

qinzhao at gcc dot gnu.org changed:

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

--- Comment #6 from qinzhao at gcc dot gnu.org ---
(In reply to Siddhesh Poyarekar from comment #2)
> The standard does not allow the nesting, but gcc supports it as an extension.

when GCC supports it as an extension, I see two possible situations:

A. the structure with the flexible array member will be the last field of the
outer structure;

B. the structure with the flexible array member will be the middle field of the
outer structure;

I see GCC compile the above 2 cases without any complain (i.e, GCC extension
accepts both A and B) and Adding -Wpedantic issues warnings for both.

My questions:

1. Should GCC extension support the above case B? (it should NOT, right? what's
the point to support it)
2. If GCC extension support the above case A (looks like this is the case, and
some application use this case extensively, for example, Linux Kernel uses this
a lot, See bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101832), what's the
clear definition for it? Does it still treat the flexible array member in the
inner structure as a flexible array member in the outer structure? If so, we
might need to clearly document this in GCC's extension, and then user will have
consistent expectation on this.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Bug tree-optimization/107952] tree-object-size: inconsistent size for flexible arrays nested in structs
  2022-12-02 14:04 [Bug tree-optimization/107952] New: tree-object-size: inconsistent size for flexible arrays nested in structs siddhesh at gcc dot gnu.org
                   ` (5 preceding siblings ...)
  2023-01-23 19:39 ` qinzhao at gcc dot gnu.org
@ 2023-01-23 19:44 ` qinzhao at gcc dot gnu.org
  2023-01-23 21:30 ` siddhesh at gcc dot gnu.org
                   ` (12 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: qinzhao at gcc dot gnu.org @ 2023-01-23 19:44 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #7 from qinzhao at gcc dot gnu.org ---
(In reply to Richard Biener from comment #1)
> GCC considered this as a flex-array. 

do you mean for the following example:

typedef struct {
  char pad;
  char data[];
} F2;

typedef struct {
  unsigned pad;
  F2 flex;
} S2;

although C standard disallow the above, GCC extension treats S2.flex.data as a
flex-array? 

How about:

typedef struct {
  char pad;
  char data[];
} F2;

typedef struct {
  F2 flex;
  unsigned pad;
} S2;

do we have any documentation on this Gcc extension?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Bug tree-optimization/107952] tree-object-size: inconsistent size for flexible arrays nested in structs
  2022-12-02 14:04 [Bug tree-optimization/107952] New: tree-object-size: inconsistent size for flexible arrays nested in structs siddhesh at gcc dot gnu.org
                   ` (6 preceding siblings ...)
  2023-01-23 19:44 ` qinzhao at gcc dot gnu.org
@ 2023-01-23 21:30 ` siddhesh at gcc dot gnu.org
  2023-01-24 10:04 ` rguenth at gcc dot gnu.org
                   ` (11 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: siddhesh at gcc dot gnu.org @ 2023-01-23 21:30 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #8 from Siddhesh Poyarekar <siddhesh at gcc dot gnu.org> ---
(In reply to qinzhao from comment #7)
> (In reply to Richard Biener from comment #1)
> > GCC considered this as a flex-array. 
> 
> do you mean for the following example:
> 
> typedef struct {
>   char pad;
>   char data[];
> } F2;
> 
> typedef struct {
>   unsigned pad;
>   F2 flex;
> } S2;
> 
> although C standard disallow the above, GCC extension treats S2.flex.data as
> a flex-array? 
> 
> How about:
> 
> typedef struct {
>   char pad;
>   char data[];
> } F2;
> 
> typedef struct {
>   F2 flex;
>   unsigned pad;
> } S2;
> 
> do we have any documentation on this Gcc extension?

There's an open bug to document these semantics:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77650

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Bug tree-optimization/107952] tree-object-size: inconsistent size for flexible arrays nested in structs
  2022-12-02 14:04 [Bug tree-optimization/107952] New: tree-object-size: inconsistent size for flexible arrays nested in structs siddhesh at gcc dot gnu.org
                   ` (7 preceding siblings ...)
  2023-01-23 21:30 ` siddhesh at gcc dot gnu.org
@ 2023-01-24 10:04 ` rguenth at gcc dot gnu.org
  2023-01-24 15:24 ` qing.zhao at oracle dot com
                   ` (10 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: rguenth at gcc dot gnu.org @ 2023-01-24 10:04 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to qinzhao from comment #7)
> (In reply to Richard Biener from comment #1)
> > GCC considered this as a flex-array. 
> 
> do you mean for the following example:
> 
> typedef struct {
>   char pad;
>   char data[];
> } F2;
> 
> typedef struct {
>   unsigned pad;
>   F2 flex;
> } S2;
> 
> although C standard disallow the above, GCC extension treats S2.flex.data as
> a flex-array? 
> 
> How about:
> 
> typedef struct {
>   char pad;
>   char data[];
> } F2;
> 
> typedef struct {
>   F2 flex;
>   unsigned pad;
> } S2;
> 
> do we have any documentation on this Gcc extension?

GCC handles for example

struct A { char data[1]; };
struct B { int n; struct A a; };

as if the a.data[] array is a flex-array.  It also handles

struct C { int n; struct A a; int x; };

as if a.data[] can be up to 4 elements large (we allow an array to extend
flexibly to padding - but only if it is trailing).  I see that's not
consistently handled though.

I think the [] syntax should follow the C standard as what testcases are
accepted/rejected by the frontend and any extensions there should be
documented (those are separate from what the former array_at_struct_end_p
allowed semantically and where GCC is careful with optimization because
of code out in the wild).

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Bug tree-optimization/107952] tree-object-size: inconsistent size for flexible arrays nested in structs
  2022-12-02 14:04 [Bug tree-optimization/107952] New: tree-object-size: inconsistent size for flexible arrays nested in structs siddhesh at gcc dot gnu.org
                   ` (8 preceding siblings ...)
  2023-01-24 10:04 ` rguenth at gcc dot gnu.org
@ 2023-01-24 15:24 ` qing.zhao at oracle dot com
  2023-01-25  7:32 ` rguenther at suse dot de
                   ` (9 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: qing.zhao at oracle dot com @ 2023-01-24 15:24 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #10 from Qing Zhao <qing.zhao at oracle dot com> ---
> --- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
> 
> GCC handles for example
> 
> struct A { char data[1]; };
> struct B { int n; struct A a; };
> 
> as if the a.data[] array is a flex-array.

Okay. Then the maximum size of __builtin_object_size for it should be -1,
right?

>  It also handles
> 
> struct C { int n; struct A a; int x; };
> 
> as if a.data[] can be up to 4 elements large (we allow an array to extend
> flexibly to padding - but only if it is trailing).

A little confused here:  
        when the structure with a trailing flexible-array member is a middle
field of 
        an outer structure, GCC extension will treat it as a flexible-array
too? But the
        maximum size of this flexible-array will be bounded by the size of the
padding
        of that field? 
Is the above understanding correct?

>  I see that's not
> consistently handled though.
> 
> I think the [] syntax should follow the C standard as what testcases are
> accepted/rejected by the frontend and any extensions there should be
> documented

Agreed, usually where these extension should be documented?

> (those are separate from what the former array_at_struct_end_p
> allowed semantically

So, your mean to leave such extension out of “array_at_struct_end_p” (the
current “array_ref_flexible_size_p”)? 
Handle them separately instead?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Bug tree-optimization/107952] tree-object-size: inconsistent size for flexible arrays nested in structs
  2022-12-02 14:04 [Bug tree-optimization/107952] New: tree-object-size: inconsistent size for flexible arrays nested in structs siddhesh at gcc dot gnu.org
                   ` (9 preceding siblings ...)
  2023-01-24 15:24 ` qing.zhao at oracle dot com
@ 2023-01-25  7:32 ` rguenther at suse dot de
  2023-01-25 12:44 ` siddhesh at gcc dot gnu.org
                   ` (8 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: rguenther at suse dot de @ 2023-01-25  7:32 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #11 from rguenther at suse dot de <rguenther at suse dot de> ---
On Tue, 24 Jan 2023, qing.zhao at oracle dot com wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107952
> 
> --- Comment #10 from Qing Zhao <qing.zhao at oracle dot com> ---
> > --- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
> > 
> > GCC handles for example
> > 
> > struct A { char data[1]; };
> > struct B { int n; struct A a; };
> > 
> > as if the a.data[] array is a flex-array.
> 
> Okay. Then the maximum size of __builtin_object_size for it should be -1,
> right?

I think so.

> >  It also handles
> > 
> > struct C { int n; struct A a; int x; };
> > 
> > as if a.data[] can be up to 4 elements large (we allow an array to extend
> > flexibly to padding - but only if it is trailing).
> 
> A little confused here:  
>         when the structure with a trailing flexible-array member is a middle
> field of 
>         an outer structure, GCC extension will treat it as a flexible-array
> too? But the
>         maximum size of this flexible-array will be bounded by the size of the
> padding
>         of that field? 
> Is the above understanding correct?

That's correct - at least when using the get_ref_base_and_extent
API.  I see that when using array_ref_flexible_size_p it doesn't
return true (but it's technically not _flexible_, we just treat it with
a bound size that doesn't match the declared bound).

> >  I see that's not
> > consistently handled though.
> > 
> > I think the [] syntax should follow the C standard as what testcases are
> > accepted/rejected by the frontend and any extensions there should be
> > documented
> 
> Agreed, usually where these extension should be documented?

They are usually documented in doc/extend.texi

> > (those are separate from what the former array_at_struct_end_p
> > allowed semantically
> 
> So, your mean to leave such extension out of ?array_at_struct_end_p? (the
> current ?array_ref_flexible_size_p?)?

The first is handled by the function just fine, it's the second
with the bound size that's not and that isn't a good fit there,
though we do handle padding to the end of a declaration where
we return true.

> Handle them separately instead?

I'm not sure how important it is to hande the middle array
extending to padding, ISTR there was some real world code
"miscompiled" when treating the array domain as written.

Richard.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Bug tree-optimization/107952] tree-object-size: inconsistent size for flexible arrays nested in structs
  2022-12-02 14:04 [Bug tree-optimization/107952] New: tree-object-size: inconsistent size for flexible arrays nested in structs siddhesh at gcc dot gnu.org
                   ` (10 preceding siblings ...)
  2023-01-25  7:32 ` rguenther at suse dot de
@ 2023-01-25 12:44 ` siddhesh at gcc dot gnu.org
  2023-01-25 15:14 ` qing.zhao at oracle dot com
                   ` (7 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: siddhesh at gcc dot gnu.org @ 2023-01-25 12:44 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #12 from Siddhesh Poyarekar <siddhesh at gcc dot gnu.org> ---
(In reply to qinzhao from comment #7)
> (In reply to Richard Biener from comment #1)
> > GCC considered this as a flex-array. 
> 
> do you mean for the following example:
> 
> typedef struct {
>   char pad;
>   char data[];
> } F2;
> 
> typedef struct {
>   unsigned pad;
>   F2 flex;
> } S2;
> 
> although C standard disallow the above, GCC extension treats S2.flex.data as
> a flex-array? 
> 
> How about:
> 
> typedef struct {
>   char pad;
>   char data[];
> } F2;
> 
> typedef struct {
>   F2 flex;
>   unsigned pad;
> } S2;
> 
> do we have any documentation on this Gcc extension?

There's an open bug to document these semantics:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77650(In reply to
rguenther@suse.de from comment #11)
> On Tue, 24 Jan 2023, qing.zhao at oracle dot com wrote:
> 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107952
> > 
> > --- Comment #10 from Qing Zhao <qing.zhao at oracle dot com> ---
> > > --- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
> > > 
> > > GCC handles for example
> > > 
> > > struct A { char data[1]; };
> > > struct B { int n; struct A a; };
> > > 
> > > as if the a.data[] array is a flex-array.
> > 
> > Okay. Then the maximum size of __builtin_object_size for it should be -1,
> > right?
> 
> I think so.

Why?  If the a B object is allocated with a visible allocator call, we can
return the correct size here too.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Bug tree-optimization/107952] tree-object-size: inconsistent size for flexible arrays nested in structs
  2022-12-02 14:04 [Bug tree-optimization/107952] New: tree-object-size: inconsistent size for flexible arrays nested in structs siddhesh at gcc dot gnu.org
                   ` (11 preceding siblings ...)
  2023-01-25 12:44 ` siddhesh at gcc dot gnu.org
@ 2023-01-25 15:14 ` qing.zhao at oracle dot com
  2023-01-25 16:12 ` siddhesh at gcc dot gnu.org
                   ` (6 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: qing.zhao at oracle dot com @ 2023-01-25 15:14 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #13 from Qing Zhao <qing.zhao at oracle dot com> ---
> On Jan 25, 2023, at 2:32 AM, rguenther at suse dot de <gcc-bugzilla@gcc.gnu.org> wrote:
>> 
>> A little confused here:  
>>        when the structure with a trailing flexible-array member is a middle
>> field of 
>>        an outer structure, GCC extension will treat it as a flexible-array
>> too? But the
>>        maximum size of this flexible-array will be bounded by the size of the
>> padding
>>        of that field? 
>> Is the above understanding correct?
> 
> That's correct - at least when using the get_ref_base_and_extent
> API.  I see that when using array_ref_flexible_size_p it doesn't
> return true (but it's technically not _flexible_, we just treat it with
> a bound size that doesn't match the declared bound).
For the current array_ref_flexible_size_p, we include the following 3 cases:

   A. a ref to a flexible array member at the end of a structure;
   B. a ref to an array with a different type against the original decl;
      for example:

   short a[16] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 };
   (*((char(*)[16])&a[0]))[i+8]

   C. a ref to an array that was passed as a parameter;
      for example:

   int test (uint8_t *p, uint32_t t[1][1], int n) {
   for (int i = 0; i < 4; i++, p++)
     t[i][0] = …;

It basically mean: return true if REF is to an array whose actual size might be
larger than its upper bound implies. 

I feel that the case "when the structure with a trailing flexible-array member
is a middle field of an outer structure” still fit here, but not very sure,
need more thinking...

> 
> The first is handled by the function just fine,

No, even the first case is not recognized by the current
“array_ref_flexible_size_p”, it’s not been identified as a flexible array right
now.
Shall we include this case into “array_ref_flexible_size_p”?  (It’s a GCC
extension).

> it's the second with the bound size that's not and that isn't a good fit there,
> though we do handle padding to the end of a declaration where
> we return true.
> 
>> Handle them separately instead?
> 
> I'm not sure how important it is to hande the middle array
> extending to padding, ISTR there was some real world code
> "miscompiled" when treating the array domain as written.

We can leave the 2nd case in a later time to resolve -:)
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Bug tree-optimization/107952] tree-object-size: inconsistent size for flexible arrays nested in structs
  2022-12-02 14:04 [Bug tree-optimization/107952] New: tree-object-size: inconsistent size for flexible arrays nested in structs siddhesh at gcc dot gnu.org
                   ` (12 preceding siblings ...)
  2023-01-25 15:14 ` qing.zhao at oracle dot com
@ 2023-01-25 16:12 ` siddhesh at gcc dot gnu.org
  2023-01-25 16:40 ` qing.zhao at oracle dot com
                   ` (5 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: siddhesh at gcc dot gnu.org @ 2023-01-25 16:12 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #14 from Siddhesh Poyarekar <siddhesh at gcc dot gnu.org> ---
(In reply to Qing Zhao from comment #13)
> > 
> > The first is handled by the function just fine,
> 
> No, even the first case is not recognized by the current
> “array_ref_flexible_size_p”, it’s not been identified as a flexible array
> right now.
> Shall we include this case into “array_ref_flexible_size_p”?  (It’s a GCC
> extension).

In the first case, array_ref_flexible_size_p recognizes S2.flex.data as having
flexible size.  The tests in my patch[1] for this bug checks for this.

However, array_ref_flexible_size_p does not recognize S2.flex as having
flexible size.  It might make sense to support that, i.e. any struct or union
with the last element as a flex array should be recognized as having flexible
size.

[1] https://sourceware.org/pipermail/gcc-patches/2022-December/608912.html

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Bug tree-optimization/107952] tree-object-size: inconsistent size for flexible arrays nested in structs
  2022-12-02 14:04 [Bug tree-optimization/107952] New: tree-object-size: inconsistent size for flexible arrays nested in structs siddhesh at gcc dot gnu.org
                   ` (13 preceding siblings ...)
  2023-01-25 16:12 ` siddhesh at gcc dot gnu.org
@ 2023-01-25 16:40 ` qing.zhao at oracle dot com
  2023-01-25 21:16 ` siddhesh at gcc dot gnu.org
                   ` (4 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: qing.zhao at oracle dot com @ 2023-01-25 16:40 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #15 from Qing Zhao <qing.zhao at oracle dot com> ---
> On Jan 25, 2023, at 11:12 AM, siddhesh at gcc dot gnu.org <gcc-bugzilla@gcc.gnu.org> wrote:
>> 
>>> The first is handled by the function just fine,
>> 
>> No, even the first case is not recognized by the current
>> “array_ref_flexible_size_p”, it’s not been identified as a flexible array
>> right now.
>> Shall we include this case into “array_ref_flexible_size_p”?  (It’s a GCC
>> extension).
> 
> In the first case, array_ref_flexible_size_p recognizes S2.flex.data as having
> flexible size.  The tests in my patch[1] for this bug checks for this.
Oh, yes. That’s right.
> 
> However, array_ref_flexible_size_p does not recognize S2.flex as having
> flexible size.  It might make sense to support that, i.e. any struct or union
> with the last element as a flex array should be recognized as having flexible
> size.

Since S2.flex is not an “array_ref”, it’s correct for array_ref_fleixble_size_p
to return false for it, I think. 
We might add a new utility routine to determine whether a ref to a struct or
union have flexible array?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Bug tree-optimization/107952] tree-object-size: inconsistent size for flexible arrays nested in structs
  2022-12-02 14:04 [Bug tree-optimization/107952] New: tree-object-size: inconsistent size for flexible arrays nested in structs siddhesh at gcc dot gnu.org
                   ` (14 preceding siblings ...)
  2023-01-25 16:40 ` qing.zhao at oracle dot com
@ 2023-01-25 21:16 ` siddhesh at gcc dot gnu.org
  2023-01-25 21:43 ` qinzhao at gcc dot gnu.org
                   ` (3 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: siddhesh at gcc dot gnu.org @ 2023-01-25 21:16 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #16 from Siddhesh Poyarekar <siddhesh at gcc dot gnu.org> ---
(In reply to Qing Zhao from comment #15)
> Since S2.flex is not an “array_ref”, it’s correct for
> array_ref_fleixble_size_p to return false for it, I think. 
> We might add a new utility routine to determine whether a ref to a struct or
> union have flexible array?

It will be useful for __bos/__bdos for sure.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Bug tree-optimization/107952] tree-object-size: inconsistent size for flexible arrays nested in structs
  2022-12-02 14:04 [Bug tree-optimization/107952] New: tree-object-size: inconsistent size for flexible arrays nested in structs siddhesh at gcc dot gnu.org
                   ` (15 preceding siblings ...)
  2023-01-25 21:16 ` siddhesh at gcc dot gnu.org
@ 2023-01-25 21:43 ` qinzhao at gcc dot gnu.org
  2023-01-26  7:20 ` rguenth at gcc dot gnu.org
                   ` (2 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: qinzhao at gcc dot gnu.org @ 2023-01-25 21:43 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #17 from qinzhao at gcc dot gnu.org ---
(In reply to Siddhesh Poyarekar from comment #16)
> > We might add a new utility routine to determine whether a ref to a struct or
> > union have flexible array?
> 
> It will be useful for __bos/__bdos for sure.

Yes, this new utility routine can be added and used by tree-object-size, then
we can fix the PR 101832 separately.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101832

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Bug tree-optimization/107952] tree-object-size: inconsistent size for flexible arrays nested in structs
  2022-12-02 14:04 [Bug tree-optimization/107952] New: tree-object-size: inconsistent size for flexible arrays nested in structs siddhesh at gcc dot gnu.org
                   ` (16 preceding siblings ...)
  2023-01-25 21:43 ` qinzhao at gcc dot gnu.org
@ 2023-01-26  7:20 ` rguenth at gcc dot gnu.org
  2023-01-26 22:13 ` qinzhao at gcc dot gnu.org
  2023-01-27  7:46 ` rguenther at suse dot de
  19 siblings, 0 replies; 21+ messages in thread
From: rguenth at gcc dot gnu.org @ 2023-01-26  7:20 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #18 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Qing Zhao from comment #15)
> > On Jan 25, 2023, at 11:12 AM, siddhesh at gcc dot gnu.org <gcc-bugzilla@gcc.gnu.org> wrote:
> >> 
> >>> The first is handled by the function just fine,
> >> 
> >> No, even the first case is not recognized by the current
> >> “array_ref_flexible_size_p”, it’s not been identified as a flexible array
> >> right now.
> >> Shall we include this case into “array_ref_flexible_size_p”?  (It’s a GCC
> >> extension).
> > 
> > In the first case, array_ref_flexible_size_p recognizes S2.flex.data as having
> > flexible size.  The tests in my patch[1] for this bug checks for this.
> Oh, yes. That’s right.
> > 
> > However, array_ref_flexible_size_p does not recognize S2.flex as having
> > flexible size.  It might make sense to support that, i.e. any struct or union
> > with the last element as a flex array should be recognized as having flexible
> > size.
> 
> Since S2.flex is not an “array_ref”, it’s correct for
> array_ref_fleixble_size_p to return false for it, I think. 

Yes, that's correct.

The routine is supposed to answer if an actual _access_ is to a flexible
size part.  The whole aggregate is not part of a flexible size part here.

tree-object-size usually wants to know if a pointer points to an object
with flexible size which is really something different and might not
share much with the logic in array_ref_fleixble_size_p

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Bug tree-optimization/107952] tree-object-size: inconsistent size for flexible arrays nested in structs
  2022-12-02 14:04 [Bug tree-optimization/107952] New: tree-object-size: inconsistent size for flexible arrays nested in structs siddhesh at gcc dot gnu.org
                   ` (17 preceding siblings ...)
  2023-01-26  7:20 ` rguenth at gcc dot gnu.org
@ 2023-01-26 22:13 ` qinzhao at gcc dot gnu.org
  2023-01-27  7:46 ` rguenther at suse dot de
  19 siblings, 0 replies; 21+ messages in thread
From: qinzhao at gcc dot gnu.org @ 2023-01-26 22:13 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #19 from qinzhao at gcc dot gnu.org ---
(In reply to rguenther@suse.de from comment #11)

> > Agreed, usually where these extension should be documented?
> 
> They are usually documented in doc/extend.texi

there is one section on "Zero Length" (Arrays of Length Zero), which mentioned
this a little bit:

"A structure containing a flexible array member, or a union containing
such a structure (possibly recursively), may not be a member of a
structure or an element of an array.  (However, these uses are
permitted by GCC as extensions.)"

We might add one more sub-section inside this section to clarify how GCC
handles the situation when a structure containing a flexible array member
becomes a member of another structure. 

is that a good place to put the documentation?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Bug tree-optimization/107952] tree-object-size: inconsistent size for flexible arrays nested in structs
  2022-12-02 14:04 [Bug tree-optimization/107952] New: tree-object-size: inconsistent size for flexible arrays nested in structs siddhesh at gcc dot gnu.org
                   ` (18 preceding siblings ...)
  2023-01-26 22:13 ` qinzhao at gcc dot gnu.org
@ 2023-01-27  7:46 ` rguenther at suse dot de
  19 siblings, 0 replies; 21+ messages in thread
From: rguenther at suse dot de @ 2023-01-27  7:46 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #20 from rguenther at suse dot de <rguenther at suse dot de> ---
On Thu, 26 Jan 2023, qinzhao at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107952
> 
> --- Comment #19 from qinzhao at gcc dot gnu.org ---
> (In reply to rguenther@suse.de from comment #11)
> 
> > > Agreed, usually where these extension should be documented?
> > 
> > They are usually documented in doc/extend.texi
> 
> there is one section on "Zero Length" (Arrays of Length Zero), which mentioned
> this a little bit:
> 
> "A structure containing a flexible array member, or a union containing
> such a structure (possibly recursively), may not be a member of a
> structure or an element of an array.  (However, these uses are
> permitted by GCC as extensions.)"
> 
> We might add one more sub-section inside this section to clarify how GCC
> handles the situation when a structure containing a flexible array member
> becomes a member of another structure. 
> 
> is that a good place to put the documentation?

Yes, I think so.

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2023-01-27  7:46 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-02 14:04 [Bug tree-optimization/107952] New: tree-object-size: inconsistent size for flexible arrays nested in structs siddhesh at gcc dot gnu.org
2022-12-05  8:05 ` [Bug tree-optimization/107952] " rguenth at gcc dot gnu.org
2022-12-05 12:46 ` siddhesh at gcc dot gnu.org
2022-12-05 13:15 ` jakub at gcc dot gnu.org
2022-12-05 15:11 ` rguenther at suse dot de
2022-12-05 15:28 ` siddhesh at gcc dot gnu.org
2023-01-23 19:39 ` qinzhao at gcc dot gnu.org
2023-01-23 19:44 ` qinzhao at gcc dot gnu.org
2023-01-23 21:30 ` siddhesh at gcc dot gnu.org
2023-01-24 10:04 ` rguenth at gcc dot gnu.org
2023-01-24 15:24 ` qing.zhao at oracle dot com
2023-01-25  7:32 ` rguenther at suse dot de
2023-01-25 12:44 ` siddhesh at gcc dot gnu.org
2023-01-25 15:14 ` qing.zhao at oracle dot com
2023-01-25 16:12 ` siddhesh at gcc dot gnu.org
2023-01-25 16:40 ` qing.zhao at oracle dot com
2023-01-25 21:16 ` siddhesh at gcc dot gnu.org
2023-01-25 21:43 ` qinzhao at gcc dot gnu.org
2023-01-26  7:20 ` rguenth at gcc dot gnu.org
2023-01-26 22:13 ` qinzhao at gcc dot gnu.org
2023-01-27  7:46 ` rguenther at suse dot de

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).