From: Qing Zhao <qing.zhao@oracle.com>
To: Siddhesh Poyarekar <siddhesh@gotplt.org>
Cc: gcc Patches <gcc-patches@gcc.gnu.org>,
Jakub Jelinek <jakub@redhat.com>,
kees Cook <keescook@chromium.org>
Subject: Re: [PATCH 2/2] tree-object-size: More consistent behaviour with flex arrays
Date: Thu, 26 Jan 2023 16:20:30 +0000 [thread overview]
Message-ID: <50FEF91F-65B2-4BDC-92C2-BEB8A4A5E3C3@oracle.com> (raw)
In-Reply-To: <20221221222554.4141678-3-siddhesh@gotplt.org>
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
>
next prev parent reply other threads:[~2023-01-26 16:20 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-21 22:25 [PATCH 0/2] __bos and " Siddhesh Poyarekar
2022-12-21 22:25 ` [PATCH 1/2] testsuite: Run __bos tests to completion Siddhesh Poyarekar
2023-01-31 12:33 ` Jakub Jelinek
2023-02-01 16:46 ` [committed v2] " Siddhesh Poyarekar
2022-12-21 22:25 ` [PATCH 2/2] tree-object-size: More consistent behaviour with flex arrays Siddhesh Poyarekar
2023-01-26 16:20 ` Qing Zhao [this message]
2023-01-26 17:16 ` Siddhesh Poyarekar
2023-01-26 17:42 ` Qing Zhao
2023-01-31 12:46 ` Jakub Jelinek
2023-01-31 13:01 ` Siddhesh Poyarekar
2023-01-03 14:30 ` [ping][PATCH 0/2] __bos and " Siddhesh Poyarekar
2023-01-11 13:14 ` [ping2][PATCH " Siddhesh Poyarekar
2023-01-20 19:38 ` [ping3][PATCH " Siddhesh Poyarekar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=50FEF91F-65B2-4BDC-92C2-BEB8A4A5E3C3@oracle.com \
--to=qing.zhao@oracle.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jakub@redhat.com \
--cc=keescook@chromium.org \
--cc=siddhesh@gotplt.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).