[2/2] tree-object-size: More consistent behaviour with flex arrays

  The tree object size pass tries to fail when it detects a flex array in
the struct, but it ends up doing the right thing only when the flex
array is in the outermost struct.  For nested cases (such as arrays
nested in a union or an inner struct), it ends up taking whatever value
the flex array is declared with, using zero for the standard flex array,
i.e. [].

Rework subobject size computation to make it more consistent across the
board, honoring -fstrict-flex-arrays.  With this change, any array at
the end of the struct will end up causing __bos to use the allocated
value of the outer object, bailing out in the maximum case when it can't
find it.  In the minimum case, it will return the subscript value or the
allocated value of the outer object, whichever is larger.

gcc/ChangeLog:

	PR tree-optimization/107952
	* tree-object-size.cc (size_from_objects): New function.
	(addr_object_size): Call it.  Fully rely on
	array_ref_flexible_size_p call to determine flex array.

gcc/testsuite/ChangeLog:

	PR tree-optimization/107952
	* g++.dg/ext/builtin-object-size1.C (test1, test6, test7,
	test8): Adjust expected result for object size type 3 and 1.
	* g++.dg/ext/builtin-object-size2.C (test1, test6, test7,
	test8): Likewise.
	* gcc.dg/builtin-object-size-13.c (main): Likewise.
	* gcc.dg/builtin-object-size-6.c (test1, test6, test7, test8):
	Likewise.
	* gcc.dg/builtin-object-size-8.c (main): Likewise.
	* gcc.dg/builtin-object-size-flex-common.h: Common code for new
	tests.
	* gcc.dg/builtin-object-size-flex-nested-struct-nonzero.c: New
	test.
	* gcc.dg/builtin-object-size-flex-nested-struct-zero.c: New
	test.
	* gcc.dg/builtin-object-size-flex-nested-struct.c: New test.
	* gcc.dg/builtin-object-size-flex-nested-union-nonzero.c: New
	test.
	* gcc.dg/builtin-object-size-flex-nested-union-zero.c: New test.
	* gcc.dg/builtin-object-size-flex-nested-union.c: New test.
	* gcc.dg/builtin-object-size-flex-nonzero.c: New test.
	* gcc.dg/builtin-object-size-flex-zero.c: New test.
	* gcc.dg/builtin-object-size-flex.c: New test.

Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
---
 .../g++.dg/ext/builtin-object-size1.C         |  10 +-
 .../g++.dg/ext/builtin-object-size2.C         |  10 +-
 gcc/testsuite/gcc.dg/builtin-object-size-13.c |   4 +-
 gcc/testsuite/gcc.dg/builtin-object-size-6.c  |  10 +-
 gcc/testsuite/gcc.dg/builtin-object-size-8.c  |   4 +-
 .../gcc.dg/builtin-object-size-flex-common.h  |  90 +++++++++++
 ...n-object-size-flex-nested-struct-nonzero.c |   6 +
 ...ltin-object-size-flex-nested-struct-zero.c |   6 +
 .../builtin-object-size-flex-nested-struct.c  |  22 +++
 ...in-object-size-flex-nested-union-nonzero.c |   6 +
 ...iltin-object-size-flex-nested-union-zero.c |   6 +
 .../builtin-object-size-flex-nested-union.c   |  28 ++++
 .../gcc.dg/builtin-object-size-flex-nonzero.c |   6 +
 .../gcc.dg/builtin-object-size-flex-zero.c    |   6 +
 .../gcc.dg/builtin-object-size-flex.c         |  18 +++
 gcc/tree-object-size.cc                       | 150 ++++++------------
 16 files changed, 265 insertions(+), 117 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-flex-common.h
 create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-flex-nested-struct-nonzero.c
 create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-flex-nested-struct-zero.c
 create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-flex-nested-struct.c
 create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-flex-nested-union-nonzero.c
 create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-flex-nested-union-zero.c
 create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-flex-nested-union.c
 create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-flex-nonzero.c
 create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-flex-zero.c
 create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-flex.c
  

Hi, Siddhesh,

Thanks a lot for this patch, after -fstrict-flex-array functionality has been added into GCC,
 I think that making the tree-object-size to have consistent behavior with flex arrays is a 
valuable and natural work that need to be added.

I also like the comments you added into tree-object-size.cc, making the code much easier to be understood.

Minor comments below:

> On Dec 21, 2022, at 5:25 PM, Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
> 
> The tree object size pass tries to fail when it detects a flex array in
> the struct, but it ends up doing the right thing only when the flex
> array is in the outermost struct.  For nested cases (such as arrays
> nested in a union or an inner struct), it ends up taking whatever value
> the flex array is declared with, using zero for the standard flex array,
> i.e. [].
> 
> Rework subobject size computation to make it more consistent across the
> board, honoring -fstrict-flex-arrays.  With this change, any array at
> the end of the struct will end up causing __bos to use the allocated
> value of the outer object, bailing out in the maximum case when it can't
> find it.  In the minimum case, it will return the subscript value or the
> allocated value of the outer object, whichever is larger.

I see from the changes in the testing case, there are the following major changes for the existing behavior (can be show with the testing case)


****For non-nested structures:

struct A
{
  char a[10];
  int b;
  char c[10];
};

1.  The Minimum size of the reference to the subobject that is a trailing array of a structure is changed from “0” to “sizeof the subobject"
> -  if (__builtin_object_size (&p->c, 3) != 0)
+  if (__builtin_object_size (&p->c, 3) != 10)

****For nested structures:

struct D
{
  int i;
  struct D1
  {
    char b;
    char a[10];
  } j;
};

2.   The Maximum size of the reference to the subobject that is a trailing array of the inner structure is changed from “sizeof the subobject” to “-1"
> -  if (__builtin_object_size (&d->j.a[3], 1) != sizeof (d->j.a) - 3)
> +  if (__builtin_object_size (&d->j.a[3], 1) != (size_t) -1)

.
3.  The Minimum size of the reference to the subobject that is a trailing array of the inner structure is changed from “0” to “sizeof the subobject"
-  if (__builtin_object_size ((char *) &e->j[0], 3) != 0)
> +  if (__builtin_object_size ((char *) &e->j[0], 3) != sizeof (e->j))


I think that all the above changes are good. My only concern is, for the change of the Minimum size of the reference to the subobject that is a trailing array (the above case 1 and 3), will there be any negtive impact on the existing application that use it?

> 
> gcc/ChangeLog:
> 
> 	PR tree-optimization/107952
> 	* tree-object-size.cc (size_from_objects): New function.
> 	(addr_object_size): Call it.  Fully rely on
> 	array_ref_flexible_size_p call to determine flex array.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR tree-optimization/107952
> 	* g++.dg/ext/builtin-object-size1.C (test1, test6, test7,
> 	test8): Adjust expected result for object size type 3 and 1.
> 	* g++.dg/ext/builtin-object-size2.C (test1, test6, test7,
> 	test8): Likewise.
> 	* gcc.dg/builtin-object-size-13.c (main): Likewise.
> 	* gcc.dg/builtin-object-size-6.c (test1, test6, test7, test8):
> 	Likewise.
> 	* gcc.dg/builtin-object-size-8.c (main): Likewise.
> 	* gcc.dg/builtin-object-size-flex-common.h: Common code for new
> 	tests.
> 	* gcc.dg/builtin-object-size-flex-nested-struct-nonzero.c: New
> 	test.
> 	* gcc.dg/builtin-object-size-flex-nested-struct-zero.c: New
> 	test.
> 	* gcc.dg/builtin-object-size-flex-nested-struct.c: New test.
> 	* gcc.dg/builtin-object-size-flex-nested-union-nonzero.c: New
> 	test.
> 	* gcc.dg/builtin-object-size-flex-nested-union-zero.c: New test.
> 	* gcc.dg/builtin-object-size-flex-nested-union.c: New test.
> 	* gcc.dg/builtin-object-size-flex-nonzero.c: New test.
> 	* gcc.dg/builtin-object-size-flex-zero.c: New test.
> 	* gcc.dg/builtin-object-size-flex.c: New test.
> 
> Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
> ---
> .../g++.dg/ext/builtin-object-size1.C         |  10 +-
> .../g++.dg/ext/builtin-object-size2.C         |  10 +-
> gcc/testsuite/gcc.dg/builtin-object-size-13.c |   4 +-
> gcc/testsuite/gcc.dg/builtin-object-size-6.c  |  10 +-
> gcc/testsuite/gcc.dg/builtin-object-size-8.c  |   4 +-
> .../gcc.dg/builtin-object-size-flex-common.h  |  90 +++++++++++
> ...n-object-size-flex-nested-struct-nonzero.c |   6 +
> ...ltin-object-size-flex-nested-struct-zero.c |   6 +
> .../builtin-object-size-flex-nested-struct.c  |  22 +++
> ...in-object-size-flex-nested-union-nonzero.c |   6 +
> ...iltin-object-size-flex-nested-union-zero.c |   6 +
> .../builtin-object-size-flex-nested-union.c   |  28 ++++
> .../gcc.dg/builtin-object-size-flex-nonzero.c |   6 +
> .../gcc.dg/builtin-object-size-flex-zero.c    |   6 +
> .../gcc.dg/builtin-object-size-flex.c         |  18 +++
> gcc/tree-object-size.cc                       | 150 ++++++------------
> 16 files changed, 265 insertions(+), 117 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-flex-common.h
> create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-flex-nested-struct-nonzero.c
> create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-flex-nested-struct-zero.c
> create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-flex-nested-struct.c
> create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-flex-nested-union-nonzero.c
> create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-flex-nested-union-zero.c
> create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-flex-nested-union.c
> create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-flex-nonzero.c
> create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-flex-zero.c
> create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-flex.c
> 
> diff --git a/gcc/testsuite/g++.dg/ext/builtin-object-size1.C b/gcc/testsuite/g++.dg/ext/builtin-object-size1.C
> index 165b415683b..5b863637123 100644
> --- a/gcc/testsuite/g++.dg/ext/builtin-object-size1.C
> +++ b/gcc/testsuite/g++.dg/ext/builtin-object-size1.C
> @@ -103,7 +103,7 @@ test1 (A *p)
>     FAIL ();
>   if (__builtin_object_size (&p->b, 3) != sizeof (p->b))
>     FAIL ();
> -  if (__builtin_object_size (&p->c, 3) != 0)
> +  if (__builtin_object_size (&p->c, 3) != 10)
>     FAIL ();
>   c = p->a;
>   if (__builtin_object_size (c, 3) != sizeof (p->a))
> @@ -118,7 +118,7 @@ test1 (A *p)
>   if (__builtin_object_size (c, 3) != sizeof (p->b))
>     FAIL ();
>   c = (char *) &p->c;
> -  if (__builtin_object_size (c, 3) != 0)
> +  if (__builtin_object_size (c, 3) != 10)
>     FAIL ();
> }
> 
> @@ -344,7 +344,7 @@ test6 (struct D *d)
> {
>   if (__builtin_object_size (&d->j.a[3], 0) != (size_t) -1)
>     FAIL ();
> -  if (__builtin_object_size (&d->j.a[3], 1) != sizeof (d->j.a) - 3)
> +  if (__builtin_object_size (&d->j.a[3], 1) != (size_t) -1)
>     FAIL ();
>   if (__builtin_object_size (&d->j.a[3], 2) != 0)
>     FAIL ();
> @@ -380,7 +380,7 @@ test7 (struct E *e)
>     FAIL ();
>   if (__builtin_object_size ((char *) &e->j[0], 2) != 0)
>     FAIL ();
> -  if (__builtin_object_size ((char *) &e->j[0], 3) != 0)
> +  if (__builtin_object_size ((char *) &e->j[0], 3) != sizeof (e->j))
>     FAIL ();
> }
> 
> @@ -404,7 +404,7 @@ test8 (union F *f)
>     FAIL ();
>   if (__builtin_object_size (&f->d.c[3], 2) != 0)
>     FAIL ();
> -  if (__builtin_object_size (&f->d.c[3], 3) != 0)
> +  if (__builtin_object_size (&f->d.c[3], 3) != sizeof (f->d.c) - 3)
>     FAIL ();
> }
> 
> diff --git a/gcc/testsuite/g++.dg/ext/builtin-object-size2.C b/gcc/testsuite/g++.dg/ext/builtin-object-size2.C
> index c5dbd96193c..665dad886d4 100644
> --- a/gcc/testsuite/g++.dg/ext/builtin-object-size2.C
> +++ b/gcc/testsuite/g++.dg/ext/builtin-object-size2.C
> @@ -106,7 +106,7 @@ test1 (A *p)
>     FAIL ();
>   if (__builtin_object_size (&p->b, 3) != sizeof (p->b))
>     FAIL ();
> -  if (__builtin_object_size (&p->c, 3) != 0)
> +  if (__builtin_object_size (&p->c, 3) != 10)
>     FAIL ();
>   c = p->a;
>   if (__builtin_object_size (c, 3) != sizeof (p->a))
> @@ -121,7 +121,7 @@ test1 (A *p)
>   if (__builtin_object_size (c, 3) != sizeof (p->b))
>     FAIL ();
>   c = (char *) &p->c;
> -  if (__builtin_object_size (c, 3) != 0)
> +  if (__builtin_object_size (c, 3) != 10)
>     FAIL ();
> }
> 
> @@ -347,7 +347,7 @@ test6 (struct D *d)
> {
>   if (__builtin_object_size (&d->j.a[3], 0) != (size_t) -1)
>     FAIL ();
> -  if (__builtin_object_size (&d->j.a[3], 1) != sizeof (d->j.a) - 3)
> +  if (__builtin_object_size (&d->j.a[3], 1) != (size_t) -1)
>     FAIL ();
>   if (__builtin_object_size (&d->j.a[3], 2) != 0)
>     FAIL ();
> @@ -383,7 +383,7 @@ test7 (struct E *e)
>     FAIL ();
>   if (__builtin_object_size ((char *) &e->j[0], 2) != 0)
>     FAIL ();
> -  if (__builtin_object_size ((char *) &e->j[0], 3) != 0)
> +  if (__builtin_object_size ((char *) &e->j[0], 3) != sizeof (e->j))
>     FAIL ();
> }
> 
> @@ -407,7 +407,7 @@ test8 (union F *f)
>     FAIL ();
>   if (__builtin_object_size (&f->d.c[3], 2) != 0)
>     FAIL ();
> -  if (__builtin_object_size (&f->d.c[3], 3) != 0)
> +  if (__builtin_object_size (&f->d.c[3], 3) != sizeof (f->d.c) - 3)
>     FAIL ();
> }
> 
> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-13.c b/gcc/testsuite/gcc.dg/builtin-object-size-13.c
> index c7d58c941d4..6429dcaf426 100644
> --- a/gcc/testsuite/gcc.dg/builtin-object-size-13.c
> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-13.c
> @@ -332,9 +332,9 @@ main (void)
>   o2 = __builtin_offsetof (struct H, h2.e1.c2);
>   h1 = malloc (s);
>   h2 = malloc (o2 + 212);
> -  TA (h1->h2.e1.c2.a2, s - o2, sizeof (h1->h2.e1.c2.a2));
> +  TA (h1->h2.e1.c2.a2, s - o2, s - o2);
>   s = o2 + 212;
> -  TA (h2->h2.e1.c2.a2, s - o2, sizeof (h2->h2.e1.c2.a2));
> +  TA (h2->h2.e1.c2.a2, s - o2, s - o2);
>   free (h2);
>   free (h1);
>   s = sizeof (struct H);
> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-6.c b/gcc/testsuite/gcc.dg/builtin-object-size-6.c
> index 4ce05184143..1ae608d456b 100644
> --- a/gcc/testsuite/gcc.dg/builtin-object-size-6.c
> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-6.c
> @@ -103,7 +103,7 @@ test1 (struct A *p)
>     FAIL ();
>   if (__builtin_object_size (&p->b, 3) != sizeof (p->b))
>     FAIL ();
> -  if (__builtin_object_size (&p->c, 3) != 0)
> +  if (__builtin_object_size (&p->c, 3) != 10)
>     FAIL ();
>   c = p->a;
>   if (__builtin_object_size (c, 3) != sizeof (p->a))
> @@ -118,7 +118,7 @@ test1 (struct A *p)
>   if (__builtin_object_size (c, 3) != sizeof (p->b))
>     FAIL ();
>   c = (char *) &p->c;
> -  if (__builtin_object_size (c, 3) != 0)
> +  if (__builtin_object_size (c, 3) != 10)
>     FAIL ();
> }
> 
> @@ -344,7 +344,7 @@ test6 (struct D *d)
> {
>   if (__builtin_object_size (&d->j.a[3], 0) != (size_t) -1)
>     FAIL ();
> -  if (__builtin_object_size (&d->j.a[3], 1) != sizeof (d->j.a) - 3)
> +  if (__builtin_object_size (&d->j.a[3], 1) != -1)
>     FAIL ();
>   if (__builtin_object_size (&d->j.a[3], 2) != 0)
>     FAIL ();
> @@ -380,7 +380,7 @@ test7 (struct E *e)
>     FAIL ();
>   if (__builtin_object_size ((char *) &e->j[0], 2) != 0)
>     FAIL ();
> -  if (__builtin_object_size ((char *) &e->j[0], 3) != 0)
> +  if (__builtin_object_size ((char *) &e->j[0], 3) != sizeof (e->j))
>     FAIL ();
> }
> 
> @@ -404,7 +404,7 @@ test8 (union F *f)
>     FAIL ();
>   if (__builtin_object_size (&f->d.c[3], 2) != 0)
>     FAIL ();
> -  if (__builtin_object_size (&f->d.c[3], 3) != 0)
> +  if (__builtin_object_size (&f->d.c[3], 3) != 7)
>     FAIL ();
> }
> 
> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-8.c b/gcc/testsuite/gcc.dg/builtin-object-size-8.c
> index f67902e29f0..184769f1bd8 100644
> --- a/gcc/testsuite/gcc.dg/builtin-object-size-8.c
> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-8.c
> @@ -186,13 +186,13 @@ main (void)
>   TS (h1->h1, s);
>   TS (h1->h2.e1.c1, s - o);
>   TS (h1->h2.e1.c2.a1, s - o2);
> -  TA (h1->h2.e1.c2.a2, s - o2, sizeof (h1->h2.e1.c2.a2));
> +  TA (h1->h2.e1.c2.a2, s - o2, s - o2);
>   TF (h1->h2.e2, s - o);
>   s = o2 + 212;
>   TS (h2->h1, s);
>   TS (h2->h2.e1.c1, s - o);
>   TS (h2->h2.e1.c2.a1, s - o2);
> -  TA (h2->h2.e1.c2.a2, s - o2, sizeof (h2->h2.e1.c2.a2));
> +  TA (h2->h2.e1.c2.a2, s - o2, s - o2);
>   TF (h2->h2.e2, s - o);
>   free (h2);
>   free (h1);
> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-flex-common.h b/gcc/testsuite/gcc.dg/builtin-object-size-flex-common.h
> new file mode 100644
> index 00000000000..b8b68da762d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-flex-common.h
> @@ -0,0 +1,90 @@
> +#include "builtin-object-size-common.h"
> +
> +typedef __SIZE_TYPE__ size_t;
> +
> +void
> +__attribute__ ((noinline))
> +test_flexarray_allocate (const char *name)
> +{
> +  size_t n = sizeof (flex_t);
> +
> +  if (name != (void *) 0)
> +    n += __builtin_strlen (name) + 1;
> +
> +  flex_t *p = __builtin_malloc (n);
> +
> +  if (__builtin_dynamic_object_size (FLEX_ARRAY (p), 0)
> +      != n - sizeof (*p) + SIZEOF_FLEX_ARRAY)
> +    FAIL ();
> +
> +  if (__builtin_dynamic_object_size (FLEX_ARRAY (p), 1)
> +      != n - sizeof (*p) + SIZEOF_FLEX_ARRAY)
> +    FAIL ();
> +
> +  if (__builtin_dynamic_object_size (FLEX_ARRAY (p), 2)
> +      != n - sizeof (*p) + SIZEOF_FLEX_ARRAY)
> +    FAIL ();
> +
> +  if (__builtin_dynamic_object_size (FLEX_ARRAY (p), 3)
> +      != n - sizeof (*p) + SIZEOF_FLEX_ARRAY)
> +    FAIL ();
> +
> +  __builtin_free (p);
> +}
> +
> +void
> +__attribute__ ((noinline))
> +__attribute__ ((access (read_only, 1, 2)))
> +test_flexarray_access (flex_t *p, size_t sz)
> +{
> +  if (__builtin_dynamic_object_size (FLEX_ARRAY (p), 0)
> +      != (sz - 1) * sizeof (*p) + SIZEOF_FLEX_ARRAY)
> +    FAIL ();
> +
> +  if (__builtin_dynamic_object_size (FLEX_ARRAY (p), 1)
> +      != (sz - 1) * sizeof (*p) + SIZEOF_FLEX_ARRAY)
> +    FAIL ();
> +
> +  if (__builtin_dynamic_object_size (FLEX_ARRAY (p), 2)
> +      != (sz - 1) * sizeof (*p) + SIZEOF_FLEX_ARRAY)
> +    FAIL ();
> +
> +  if (__builtin_dynamic_object_size (FLEX_ARRAY (p), 3)
> +      != (sz - 1) * sizeof (*p) + SIZEOF_FLEX_ARRAY)
> +    FAIL ();
> +}
> +
> +void
> +__attribute__ ((noinline))
> +test_flexarray_none (flex_t *p)
> +{
> +  if (__builtin_dynamic_object_size (FLEX_ARRAY (p), 0) != -1)
> +    FAIL ();
> +
> +  if (__builtin_dynamic_object_size (FLEX_ARRAY (p), 1) != -1)
> +    FAIL ();
> +
> +  if (__builtin_dynamic_object_size (FLEX_ARRAY (p), 2) != 0)
> +    FAIL ();
> +
> +  if (__builtin_dynamic_object_size (FLEX_ARRAY (p), 3) != SIZEOF_FLEX_ARRAY)
> +    FAIL ();
> +}
> +
> +int
> +main (int argc, char **argv)
> +{
> +  const char *str = "qwertyuiopasdfgghjklzxcvbnm";
> +
> +  test_flexarray_allocate (str);
> +
> +  const size_t sz = 1024;
> +  flex_t *p = __builtin_malloc (sz * sizeof (*p));
> +
> +  test_flexarray_access (p, sz);
> +
> +  test_flexarray_none (p);
> +  __builtin_free (p);
> +
> +  DONE ();
> +}
> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-flex-nested-struct-nonzero.c b/gcc/testsuite/gcc.dg/builtin-object-size-flex-nested-struct-nonzero.c
> new file mode 100644
> index 00000000000..c50cd89a817
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-flex-nested-struct-nonzero.c
> @@ -0,0 +1,6 @@
> +/* { dg-do run } */
> +/* { dg-options "-O2" } */
> +
> +#define SIZEOF_FLEX_ARRAY 4
> +
> +#include "builtin-object-size-flex-nested-struct.c"
> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-flex-nested-struct-zero.c b/gcc/testsuite/gcc.dg/builtin-object-size-flex-nested-struct-zero.c
> new file mode 100644
> index 00000000000..554aece0b46
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-flex-nested-struct-zero.c
> @@ -0,0 +1,6 @@
> +/* { dg-do run } */
> +/* { dg-options "-O2" } */
> +
> +#define SIZEOF_FLEX_ARRAY 0
> +
> +#include "builtin-object-size-flex-nested-struct.c"
> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-flex-nested-struct.c b/gcc/testsuite/gcc.dg/builtin-object-size-flex-nested-struct.c
> new file mode 100644
> index 00000000000..1adeac16ac4
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-flex-nested-struct.c
> @@ -0,0 +1,22 @@
> +/* { dg-do run } */
> +/* { dg-options "-O2" } */
> +
> +#ifndef SIZEOF_FLEX_ARRAY
> +# define SIZEOF_FLEX_ARRAY_DECL
> +# define SIZEOF_FLEX_ARRAY 0
> +#else
> +# define SIZEOF_FLEX_ARRAY_DECL SIZEOF_FLEX_ARRAY
> +#endif
> +
> +typedef struct {
> +  unsigned pad;
> +  struct {
> +    unsigned pad;
> +    char data[SIZEOF_FLEX_ARRAY_DECL];
> +  } s;
> +} flex_t;
> +
> +#define FLEX_ARRAY(p) p->s.data
> +#define SIZE_OF_FLEX sizeof (unsigned) + sizeof (unsigned)
> +
> +#include "builtin-object-size-flex-common.h"
> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-flex-nested-union-nonzero.c b/gcc/testsuite/gcc.dg/builtin-object-size-flex-nested-union-nonzero.c
> new file mode 100644
> index 00000000000..58e387c8db6
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-flex-nested-union-nonzero.c
> @@ -0,0 +1,6 @@
> +/* { dg-do run } */
> +/* { dg-options "-O2" } */
> +
> +#define SIZEOF_FLEX_ARRAY 4
> +
> +#include "builtin-object-size-flex-nested-union.c"
> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-flex-nested-union-zero.c b/gcc/testsuite/gcc.dg/builtin-object-size-flex-nested-union-zero.c
> new file mode 100644
> index 00000000000..ec6429a5a9b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-flex-nested-union-zero.c
> @@ -0,0 +1,6 @@
> +/* { dg-do run } */
> +/* { dg-options "-O2" } */
> +
> +#define SIZEOF_FLEX_ARRAY 0
> +
> +#include "builtin-object-size-flex-nested-union.c"
> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-flex-nested-union.c b/gcc/testsuite/gcc.dg/builtin-object-size-flex-nested-union.c
> new file mode 100644
> index 00000000000..8326131c890
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-flex-nested-union.c
> @@ -0,0 +1,28 @@
> +/* { dg-do run } */
> +/* { dg-options "-O2" } */
> +
> +#ifndef SIZEOF_FLEX_ARRAY
> +# define SIZEOF_FLEX_ARRAY_DECL
> +# define SIZEOF_FLEX_ARRAY 0
> +#else
> +# define SIZEOF_FLEX_ARRAY_DECL SIZEOF_FLEX_ARRAY
> +#endif
> +
> +typedef struct {
> +  unsigned pad;
> +  union {
> +      struct {
> +	unsigned pad;
> +	char data[SIZEOF_FLEX_ARRAY_DECL];
> +      } s1;
> +      struct {
> +	unsigned pad;
> +	char data[SIZEOF_FLEX_ARRAY_DECL];
> +      } s2;
> +  } u;
> +} flex_t;
> +
> +#define FLEX_ARRAY(p) p->u.s1.data
> +#define SIZE_OF_FLEX sizeof (unsigned) + sizeof (unsigned)
> +
> +#include "builtin-object-size-flex-common.h"
> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-flex-nonzero.c b/gcc/testsuite/gcc.dg/builtin-object-size-flex-nonzero.c
> new file mode 100644
> index 00000000000..693d2e98758
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-flex-nonzero.c
> @@ -0,0 +1,6 @@
> +/* { dg-do run } */
> +/* { dg-options "-O2" } */
> +
> +#define SIZEOF_FLEX_ARRAY 4
> +
> +#include "builtin-object-size-flex.c"
> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-flex-zero.c b/gcc/testsuite/gcc.dg/builtin-object-size-flex-zero.c
> new file mode 100644
> index 00000000000..f38d0e01d4b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-flex-zero.c
> @@ -0,0 +1,6 @@
> +/* { dg-do run } */
> +/* { dg-options "-O2" } */
> +
> +#define SIZEOF_FLEX_ARRAY 0
> +
> +#include "builtin-object-size-flex.c"
> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-flex.c b/gcc/testsuite/gcc.dg/builtin-object-size-flex.c
> new file mode 100644
> index 00000000000..34aba8942d6
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-flex.c
> @@ -0,0 +1,18 @@
> +/* { dg-do run } */
> +/* { dg-options "-O2" } */
> +
> +#ifndef SIZEOF_FLEX_ARRAY
> +# define SIZEOF_FLEX_ARRAY_DECL
> +# define SIZEOF_FLEX_ARRAY 0
> +#else
> +# define SIZEOF_FLEX_ARRAY_DECL SIZEOF_FLEX_ARRAY
> +#endif
> +
> +typedef struct {
> +  unsigned pad;
> +  char data[SIZEOF_FLEX_ARRAY_DECL];
> +} flex_t;
> +
> +#define FLEX_ARRAY(p) p->data
> +
> +#include "builtin-object-size-flex-common.h"
> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
> index d9f25397c71..98ee3c2b527 100644
> --- a/gcc/tree-object-size.cc
> +++ b/gcc/tree-object-size.cc
> @@ -499,6 +499,19 @@ decl_init_size (tree decl, bool min)
>   return size;
> }
> 
> +/* Return the size of SUBOBJ that is within VAR where the latter has VAR_SIZE
> +   size.  */
> +
> +static tree
> +size_from_objects (const_tree subobj, const_tree var, tree var_size,
> +		   int object_size_type)
> +{
> +  tree bytes = compute_object_offset (subobj, var);
> +
> +  return (bytes != error_mark_node ? size_for_offset (var_size, bytes)
> +	  : size_unknown (object_size_type));
> +}
> +
> /* Compute __builtin_object_size for PTR, which is a ADDR_EXPR.
>    OBJECT_SIZE_TYPE is the second argument from __builtin_object_size.
>    If unknown, return size_unknown (object_size_type).  */
> @@ -589,105 +602,52 @@ addr_object_size (struct object_size_info *osi, const_tree ptr,
> 	{
> 	  var = TREE_OPERAND (ptr, 0);
> 
> +	  /* Get the immediate containing subobject.  Skip over conversions
> +	     because we don't need them.  */
> 	  while (var != pt_var
> -		 && TREE_CODE (var) != BIT_FIELD_REF
> -		 && TREE_CODE (var) != COMPONENT_REF
> -		 && TREE_CODE (var) != ARRAY_REF
> -		 && TREE_CODE (var) != ARRAY_RANGE_REF
> -		 && TREE_CODE (var) != REALPART_EXPR
> -		 && TREE_CODE (var) != IMAGPART_EXPR)
> +		 && (!handled_component_p (var)
> +		     || TREE_CODE (var) == VIEW_CONVERT_EXPR))
> 	    var = TREE_OPERAND (var, 0);
> 	  if (var != pt_var && TREE_CODE (var) == ARRAY_REF)
> 	    var = TREE_OPERAND (var, 0);
> +
> +	  /* If the subobject size cannot be easily inferred or is smaller than
> +	     the whole size, just use the whole size.  */

Should the above comment be:

+	  /* If the subobject size cannot be easily inferred or is larger than
+	     the whole size, just use the whole size.  */

> 	  if (! TYPE_SIZE_UNIT (TREE_TYPE (var))
> 	      || ! tree_fits_uhwi_p (TYPE_SIZE_UNIT (TREE_TYPE (var)))
> 	      || (pt_var_size && TREE_CODE (pt_var_size) == INTEGER_CST
> 		  && tree_int_cst_lt (pt_var_size,
> 				      TYPE_SIZE_UNIT (TREE_TYPE (var)))))
> 	    var = pt_var;


thanks.

Qing
> -	  else if (var != pt_var && TREE_CODE (pt_var) == MEM_REF)
> -	    {
> -	      tree v = var;
> -	      /* For &X->fld, compute object size if fld isn't a flexible array
> -		 member.  */
> -	      bool is_flexible_array_mem_ref = false;
> -	      while (v && v != pt_var)
> -		switch (TREE_CODE (v))
> -		  {
> -		  case ARRAY_REF:
> -		    if (TYPE_SIZE_UNIT (TREE_TYPE (TREE_OPERAND (v, 0))))
> -		      {
> -			tree domain
> -			  = TYPE_DOMAIN (TREE_TYPE (TREE_OPERAND (v, 0)));
> -			if (domain && TYPE_MAX_VALUE (domain))
> -			  {
> -			    v = NULL_TREE;
> -			    break;
> -			  }
> -		      }
> -		    v = TREE_OPERAND (v, 0);
> -		    break;
> -		  case REALPART_EXPR:
> -		  case IMAGPART_EXPR:
> -		    v = NULL_TREE;
> -		    break;
> -		  case COMPONENT_REF:
> -		    if (TREE_CODE (TREE_TYPE (v)) != ARRAY_TYPE)
> -		      {
> -			v = NULL_TREE;
> -			break;
> -		      }
> -		    is_flexible_array_mem_ref = array_ref_flexible_size_p (v);
> -		    while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
> -		      if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> -			  != UNION_TYPE
> -			  && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> -			  != QUAL_UNION_TYPE)
> -			break;
> -		      else
> -			v = TREE_OPERAND (v, 0);
> -		    if (TREE_CODE (v) == COMPONENT_REF
> -			&& TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> -			   == RECORD_TYPE)
> -		      {
> -			/* compute object size only if v is not a
> -			   flexible array member.  */
> -			if (!is_flexible_array_mem_ref)
> -			  {
> -			    v = NULL_TREE;
> -			    break;
> -			  }
> -			v = TREE_OPERAND (v, 0);
> -		      }
> -		    while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
> -		      if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> -			  != UNION_TYPE
> -			  && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
> -			  != QUAL_UNION_TYPE)
> -			break;
> -		      else
> -			v = TREE_OPERAND (v, 0);
> -		    if (v != pt_var)
> -		      v = NULL_TREE;
> -		    else
> -		      v = pt_var;
> -		    break;
> -		  default:
> -		    v = pt_var;
> -		    break;
> -		  }
> -	      if (v == pt_var)
> -		var = pt_var;
> -	    }
> 	}
>       else
> 	var = pt_var;
> 
> +      bool is_flexible_array_mem_ref = false;
> +      /* Find out if this is a flexible array.  This will change
> +	 according to -fstrict-flex-arrays.  */
> +      if (var != pt_var && TREE_CODE (pt_var) == MEM_REF)
> +	is_flexible_array_mem_ref = array_ref_flexible_size_p (var);
> +
> +      /* We cannot get a maximum estimate for a flex array without the
> +	 whole object size.  */
> +      if (is_flexible_array_mem_ref && !pt_var_size
> +	  && !(object_size_type & OST_MINIMUM))
> +	return false;
> +
>       if (var != pt_var)
> 	{
> -	  var_size = TYPE_SIZE_UNIT (TREE_TYPE (var));
> -	  if (!TREE_CONSTANT (var_size))
> -	    var_size = get_or_create_ssa_default_def (cfun, var_size);
> +	  /* For flexible arrays, we prefer the size based on the whole object
> +	     if it is available.  */
> +	  if (is_flexible_array_mem_ref && pt_var_size)
> +	    var_size = size_from_objects (var, pt_var, pt_var_size,
> +					  object_size_type);
> +	  else
> +	    {
> +	      var_size = TYPE_SIZE_UNIT (TREE_TYPE (var));
> +	      if (!TREE_CONSTANT (var_size))
> +		var_size = get_or_create_ssa_default_def (cfun, var_size);
> +	    }
> 	  if (!var_size)
> 	    return false;
> 	}
> @@ -695,23 +655,17 @@ addr_object_size (struct object_size_info *osi, const_tree ptr,
> 	return false;
>       else
> 	var_size = pt_var_size;
> -      bytes = compute_object_offset (TREE_OPERAND (ptr, 0), var);
> -      if (bytes != error_mark_node)
> +
> +      bytes = size_from_objects (TREE_OPERAND (ptr, 0), var, var_size,
> +				 object_size_type);
> +
> +      if (var != pt_var && pt_var_size && TREE_CODE (pt_var) == MEM_REF
> +	  && !is_flexible_array_mem_ref)
> 	{
> -	  bytes = size_for_offset (var_size, bytes);
> -	  if (var != pt_var && pt_var_size && TREE_CODE (pt_var) == MEM_REF)
> -	    {
> -	      tree bytes2 = compute_object_offset (TREE_OPERAND (ptr, 0),
> -						   pt_var);
> -	      if (bytes2 != error_mark_node)
> -		{
> -		  bytes2 = size_for_offset (pt_var_size, bytes2);
> -		  bytes = size_binop (MIN_EXPR, bytes, bytes2);
> -		}
> -	    }
> +	  tree bytes2 = size_from_objects (TREE_OPERAND (ptr, 0), pt_var,
> +					   pt_var_size, object_size_type);
> +	  bytes = size_binop (MIN_EXPR, bytes, bytes2);
> 	}
> -      else
> -	bytes = size_unknown (object_size_type);
> 
>       wholebytes
> 	= object_size_type & OST_SUBOBJECT ? var_size : pt_var_wholesize;
> -- 
> 2.38.1
>
  

On 2023-01-26 11:20, Qing Zhao wrote:
> Hi, Siddhesh,
> 
> Thanks a lot for this patch, after -fstrict-flex-array functionality has been added into GCC,
>   I think that making the tree-object-size to have consistent behavior with flex arrays is a
> valuable and natural work that need to be added.
> 
> I also like the comments you added into tree-object-size.cc, making the code much easier to be understood.
> 
> Minor comments below:
> 
>> On Dec 21, 2022, at 5:25 PM, Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
>>
>> The tree object size pass tries to fail when it detects a flex array in
>> the struct, but it ends up doing the right thing only when the flex
>> array is in the outermost struct.  For nested cases (such as arrays
>> nested in a union or an inner struct), it ends up taking whatever value
>> the flex array is declared with, using zero for the standard flex array,
>> i.e. [].
>>
>> Rework subobject size computation to make it more consistent across the
>> board, honoring -fstrict-flex-arrays.  With this change, any array at
>> the end of the struct will end up causing __bos to use the allocated
>> value of the outer object, bailing out in the maximum case when it can't
>> find it.  In the minimum case, it will return the subscript value or the
>> allocated value of the outer object, whichever is larger.
> 
> I see from the changes in the testing case, there are the following major changes for the existing behavior (can be show with the testing case)
> 
> 
> ****For non-nested structures:
> 
> struct A
> {
>    char a[10];
>    int b;
>    char c[10];
> };
> 
> 1.  The Minimum size of the reference to the subobject that is a trailing array of a structure is changed from “0” to “sizeof the subobject"
>> -  if (__builtin_object_size (&p->c, 3) != 0)
> +  if (__builtin_object_size (&p->c, 3) != 10)
> 
> ****For nested structures:
> 
> struct D
> {
>    int i;
>    struct D1
>    {
>      char b;
>      char a[10];
>    } j;
> };
> 
> 2.   The Maximum size of the reference to the subobject that is a trailing array of the inner structure is changed from “sizeof the subobject” to “-1"
>> -  if (__builtin_object_size (&d->j.a[3], 1) != sizeof (d->j.a) - 3)
>> +  if (__builtin_object_size (&d->j.a[3], 1) != (size_t) -1)
> 
> .
> 3.  The Minimum size of the reference to the subobject that is a trailing array of the inner structure is changed from “0” to “sizeof the subobject"
> -  if (__builtin_object_size ((char *) &e->j[0], 3) != 0)
>> +  if (__builtin_object_size ((char *) &e->j[0], 3) != sizeof (e->j))

All of the above is correct, thanks for the high level review!

> 
> I think that all the above changes are good. My only concern is, for the change of the Minimum size of the reference to the subobject that is a trailing array (the above case 1 and 3), will there be any negtive impact on the existing application that use it?

I doubt it, because the 0 return value for minimum object size is 
essentially a failure to determine minimum object size, i.e. a special 
value.  This change can be interpreted as succeeding to get the minimum 
object size in more cases.

Likewise for the maximum object size change, the change can be 
interpreted as failing to get the maximum object size in more cases. 
Does that sound reasonable?

>> +	  /* If the subobject size cannot be easily inferred or is smaller than
>> +	     the whole size, just use the whole size.  */
> 
> Should the above comment be:
> 
> +	  /* If the subobject size cannot be easily inferred or is larger than
> +	     the whole size, just use the whole size.  */
> 
>> 	  if (! TYPE_SIZE_UNIT (TREE_TYPE (var))
>> 	      || ! tree_fits_uhwi_p (TYPE_SIZE_UNIT (TREE_TYPE (var)))
>> 	      || (pt_var_size && TREE_CODE (pt_var_size) == INTEGER_CST
>> 		  && tree_int_cst_lt (pt_var_size,
>> 				      TYPE_SIZE_UNIT (TREE_TYPE (var)))))
>> 	    var = pt_var;
> 

Oops, yes indeed, fixed in my copy.

Thanks,
Sid
  

> On Jan 26, 2023, at 12:16 PM, Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
> 
> On 2023-01-26 11:20, Qing Zhao wrote:
>> Hi, Siddhesh,
>> Thanks a lot for this patch, after -fstrict-flex-array functionality has been added into GCC,
>>  I think that making the tree-object-size to have consistent behavior with flex arrays is a
>> valuable and natural work that need to be added.
>> I also like the comments you added into tree-object-size.cc, making the code much easier to be understood.
>> Minor comments below:
>>> On Dec 21, 2022, at 5:25 PM, Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
>>> 
>>> The tree object size pass tries to fail when it detects a flex array in
>>> the struct, but it ends up doing the right thing only when the flex
>>> array is in the outermost struct.  For nested cases (such as arrays
>>> nested in a union or an inner struct), it ends up taking whatever value
>>> the flex array is declared with, using zero for the standard flex array,
>>> i.e. [].
>>> 
>>> Rework subobject size computation to make it more consistent across the
>>> board, honoring -fstrict-flex-arrays.  With this change, any array at
>>> the end of the struct will end up causing __bos to use the allocated
>>> value of the outer object, bailing out in the maximum case when it can't
>>> find it.  In the minimum case, it will return the subscript value or the
>>> allocated value of the outer object, whichever is larger.
>> I see from the changes in the testing case, there are the following major changes for the existing behavior (can be show with the testing case)
>> ****For non-nested structures:
>> struct A
>> {
>>   char a[10];
>>   int b;
>>   char c[10];
>> };
>> 1.  The Minimum size of the reference to the subobject that is a trailing array of a structure is changed from “0” to “sizeof the subobject"
>>> -  if (__builtin_object_size (&p->c, 3) != 0)
>> +  if (__builtin_object_size (&p->c, 3) != 10)
>> ****For nested structures:
>> struct D
>> {
>>   int i;
>>   struct D1
>>   {
>>     char b;
>>     char a[10];
>>   } j;
>> };
>> 2.   The Maximum size of the reference to the subobject that is a trailing array of the inner structure is changed from “sizeof the subobject” to “-1"
>>> -  if (__builtin_object_size (&d->j.a[3], 1) != sizeof (d->j.a) - 3)
>>> +  if (__builtin_object_size (&d->j.a[3], 1) != (size_t) -1)
>> .
>> 3.  The Minimum size of the reference to the subobject that is a trailing array of the inner structure is changed from “0” to “sizeof the subobject"
>> -  if (__builtin_object_size ((char *) &e->j[0], 3) != 0)
>>> +  if (__builtin_object_size ((char *) &e->j[0], 3) != sizeof (e->j))
> 
> All of the above is correct, thanks for the high level review!
> 
>> I think that all the above changes are good. My only concern is, for the change of the Minimum size of the reference to the subobject that is a trailing array (the above case 1 and 3), will there be any negtive impact on the existing application that use it?
> 
> I doubt it, because the 0 return value for minimum object size is essentially a failure to determine minimum object size, i.e. a special value.  This change can be interpreted as succeeding to get the minimum object size in more cases.
> 
> Likewise for the maximum object size change, the change can be interpreted as failing to get the maximum object size in more cases. Does that sound reasonable?

Yes, make sense to me.

Thanks.

Qing
> 
>>> +	  /* If the subobject size cannot be easily inferred or is smaller than
>>> +	     the whole size, just use the whole size.  */
>> Should the above comment be:
>> +	  /* If the subobject size cannot be easily inferred or is larger than
>> +	     the whole size, just use the whole size.  */
>>> 	  if (! TYPE_SIZE_UNIT (TREE_TYPE (var))
>>> 	      || ! tree_fits_uhwi_p (TYPE_SIZE_UNIT (TREE_TYPE (var)))
>>> 	      || (pt_var_size && TREE_CODE (pt_var_size) == INTEGER_CST
>>> 		  && tree_int_cst_lt (pt_var_size,
>>> 				      TYPE_SIZE_UNIT (TREE_TYPE (var)))))
>>> 	    var = pt_var;
> 
> Oops, yes indeed, fixed in my copy.
> 
> Thanks,
> Sid
  

On Wed, Dec 21, 2022 at 05:25:54PM -0500, Siddhesh Poyarekar wrote:
> The tree object size pass tries to fail when it detects a flex array in
> the struct, but it ends up doing the right thing only when the flex
> array is in the outermost struct.  For nested cases (such as arrays
> nested in a union or an inner struct), it ends up taking whatever value
> the flex array is declared with, using zero for the standard flex array,
> i.e. [].
> 
> Rework subobject size computation to make it more consistent across the
> board, honoring -fstrict-flex-arrays.  With this change, any array at
> the end of the struct will end up causing __bos to use the allocated
> value of the outer object, bailing out in the maximum case when it can't
> find it.  In the minimum case, it will return the subscript value or the
> allocated value of the outer object, whichever is larger.

I think it is way too late in the GCC 13 cycle to change behavior here
except for when -fstrict-flex-arrays is used.
Plus, am not really convinced it is a good idea to change the behavior
here except for the new options, programs in the wild took 2+ years
to acommodate for what we GCC requiring and am not sure they'd be willing
to be adjusted again.

> gcc/ChangeLog:
> 
> 	PR tree-optimization/107952
> 	* tree-object-size.cc (size_from_objects): New function.
> 	(addr_object_size): Call it.  Fully rely on
> 	array_ref_flexible_size_p call to determine flex array.

	Jakub
  

On 2023-01-31 07:46, Jakub Jelinek wrote:
> On Wed, Dec 21, 2022 at 05:25:54PM -0500, Siddhesh Poyarekar wrote:
>> The tree object size pass tries to fail when it detects a flex array in
>> the struct, but it ends up doing the right thing only when the flex
>> array is in the outermost struct.  For nested cases (such as arrays
>> nested in a union or an inner struct), it ends up taking whatever value
>> the flex array is declared with, using zero for the standard flex array,
>> i.e. [].
>>
>> Rework subobject size computation to make it more consistent across the
>> board, honoring -fstrict-flex-arrays.  With this change, any array at
>> the end of the struct will end up causing __bos to use the allocated
>> value of the outer object, bailing out in the maximum case when it can't
>> find it.  In the minimum case, it will return the subscript value or the
>> allocated value of the outer object, whichever is larger.
> 
> I think it is way too late in the GCC 13 cycle to change behavior here
> except for when -fstrict-flex-arrays is used.

I agree.

> Plus, am not really convinced it is a good idea to change the behavior
> here except for the new options, programs in the wild took 2+ years
> to acommodate for what we GCC requiring and am not sure they'd be willing
> to be adjusted again.

The behaviour change basically does two things: better minimum object 
size estimates and more conservative object size estimates for trailing 
arrays.  ISTM that this should in fact reduce breakages due to flex 
arrays, although one could argue that protection gets reduced for 
trailing arrays without -fstrict-flex-arrays.  It wouldn't cause 
non-mitigation behaviour changes though, would it?

We don't need to rush this of course, we could consider this for gcc 14 
given that we're well into stage 4.

Thanks,
Sid