Re: [PATCH] tree-optimization/108522 Use COMPONENT_REF offset when available

[Richard Biener <richard.guenther@gmail.com>]

On Tue, Jan 24, 2023 at 10:54 PM Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
>
> Use the offset in TREE_OPERAND(component_ref, 2) when available instead
> of DECL_FIELD_OFFSET when trying to compute offset for a COMPONENT_REF.
>
> OK for gcc 13 and gcc 12?
>
> Co-authored-by: Jakub Jelinek <jakub@redhat.com>
>
> gcc/ChangeLog:
>
>         PR tree-optimization/108522
>         * tree-object-size.cc (compute_object_offset): Use
>         TREE_OPERAND(ref, 2) for COMPONENT_REF when available.
>
> gcc/testsuite/ChangeLog:
>
>         PR tree-optimization/108522
>         * builtin-dynamic-object-size-0.c (test_dynarray_struct_member):
>         new test.
>         (main): Call it.
>
> Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
> ---
> Testing:
>
> - Bootstrapped on x86_64, I'm checking to confirm if a couple of
>   seemingly unrelated failures are in fact unrelated.
> - ubsan config bootstrap and i686 tests in progress
>
>  .../gcc.dg/builtin-dynamic-object-size-0.c       | 16 ++++++++++++++++
>  gcc/tree-object-size.cc                          |  4 +++-
>  2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
> index f9047a037d9..569c0a87722 100644
> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
> @@ -314,6 +314,20 @@ test_dynarray_struct_subobj2 (size_t sz, size_t off, size_t *objsz)
>    return __builtin_dynamic_object_size (&bin.c[off], 1);
>  }
>
> +/* See pr #108522.  */
> +size_t
> +__attribute__ ((noinline))
> +test_dynarray_struct_member (size_t sz)
> +{
> +  struct
> +    {
> +      char a[sz];
> +      char b;
> +    } s;
> +
> +  return __builtin_dynamic_object_size (&s.b, 0);
> +}
> +
>  size_t
>  __attribute__ ((noinline))
>  test_substring (size_t sz, size_t off)
> @@ -619,6 +633,8 @@ main (int argc, char **argv)
>    if (test_dynarray_struct_subobj2 (42, 4, &objsz)
>      != objsz - 4 - sizeof (long) - sizeof (int))
>      FAIL ();
> +  if (test_dynarray_struct_member (42) != sizeof (char))
> +    FAIL ();
>    if (test_substring_ptrplus (128, 4) != (128 - 4) * sizeof (int))
>      FAIL ();
>    if (test_substring_ptrplus (128, 142) != 0)
> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
> index 356591c22cc..de93ffad9c9 100644
> --- a/gcc/tree-object-size.cc
> +++ b/gcc/tree-object-size.cc
> @@ -412,7 +412,9 @@ compute_object_offset (const_tree expr, const_tree var)
>         return base;
>
>        t = TREE_OPERAND (expr, 1);
> -      off = size_binop (PLUS_EXPR, DECL_FIELD_OFFSET (t),
> +      off = size_binop (PLUS_EXPR,
> +                       (TREE_OPERAND (expr, 2) ? TREE_OPERAND (expr, 2)
> +                        : DECL_FIELD_OFFSET (t)),

That isn't correct - operand 2 is the field offset in units of
DECL_OFFSET_ALIGN (t) / BITS_PER_UNIT.
See component_ref_filed_offset (), maybe you should be using that
function instead?

>                         size_int (tree_to_uhwi (DECL_FIELD_BIT_OFFSET (t))
>                                   / BITS_PER_UNIT));
>        break;
> --
> 2.38.1
>