[Bug c/101832] New: __builtin_object_size(P->M, 1) where M ends with a flex-array behaves like sizeof()

[Bug c/101832] New: __builtin_object_size(P->M, 1) where M ends with a flex-array behaves like sizeof()

[kees at outflux dot net]

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

            Bug ID: 101832
           Summary: __builtin_object_size(P->M, 1) where M ends with a
                    flex-array behaves like sizeof()
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: kees at outflux dot net
  Target Milestone: ---

Created attachment 51279
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51279&action=edit
bos1 fails to recognize flex array under specific conditions

It is unclear to me if this is a duplicate of bug 64715.

bos1 in at least one situation fails to notice when a member contains a
flex-array, and returns sizeof() instead of -1. For example:

struct nlmsg {
        __u32           nlmsg_len;
        __u16           nlmsg_type;
        __u16           nlmsg_flags;
        __u32           nlmsg_seq;
        __u32           nlmsg_pid;
        __u8            nlmsg_content[];
};

struct wrapper {
    __u8 a;
    __u8 b;
    struct nlmsg msg;
};


ok:  sizeof(wrap->msg) == 16
ok:  __builtin_object_size(wrap->msg.nlmsg_content, 1) == -1
ok:  __builtin_object_size(&wrap->msg, 0) == -1
WAT: __builtin_object_size(&wrap->msg, 1) == 16 (expected -1)

https://godbolt.org/z/95n4ofT53

[Bug c/101832] __builtin_object_size(P->M, 1) where M ends with a flex-array behaves like sizeof()

[kees at outflux dot net]

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

--- Comment #1 from Kees Cook <kees at outflux dot net> ---
This is even more visible when the size IS known (via malloc hinting, for
example):

https://godbolt.org/z/4v5rKbhaf

[Bug c/101832] __builtin_object_size(P->M, 1) where M ends with a flex-array behaves like sizeof()

[kees at outflux dot net]

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

--- Comment #2 from Kees Cook <kees at outflux dot net> ---
Created attachment 51280
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51280&action=edit
Same PoC, but with malloc to provide non-unlimited bounds

[Bug c/101832] __builtin_object_size(P->M, 1) where M ends with a flex-array behaves like sizeof()

[jakub at gcc dot gnu.org]

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

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> ---
This is intentional, if you embed an aggregate with flex array into another
struct and ask not to cross the field boundaries (i.e. bos1), then the size of
that field is exactly what is the maximum size.

[Bug c/101832] __builtin_object_size(P->M, 1) where M ends with a flex-array behaves like sizeof()

[kees at outflux dot net]

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

--- Comment #4 from Kees Cook <kees at outflux dot net> ---
It seems like this isn't about crossing field boundaries -- it's asking "how
large is this particular member?" and bos can't know the answer because there
is a flex-array.

Why would 

    __builtin_object_size(wrap->msg.nlmsg_content, 1);

and

    __builtin_object_size(&wrap->msg, 1);

differ?


Or, if bos lacked "introspecition depth" to find the flex-array, why would

    __builtin_object_size(msg->nlmsg_content, 1);

and

    __builtin_object_size(msg, 1);

be the same?


It seems like the latter pair (same results) is correct, and the former pair
(differing result) is wrong.

[Bug c/101832] __builtin_object_size(P->M, 1) where M ends with a flex-array behaves like sizeof()

[kees at outflux dot net]

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

--- Comment #5 from Kees Cook <kees at outflux dot net> ---
Perhaps the best question to ask is "given an arbitrary argument, how can code
detect the remaining bytes of a member, including if the member contains a
flexible array?"

Because right now, this does not work:

#define __bytes_until_end_of_member(p) __builtin_object_size(p, 1)

since this gives different answers, depending on the level of dereference:

__bytes_until_end_of_member(wrap) == -1
__bytes_until_end_of_member(&wrap->msg) == 16
__bytes_until_end_of_member(&wrap->msg.nlmsg_content) == -1

How can "wrap->msg" be 16 if "wrap" and "warp->msg.nlmsg_content" are -1?

[Bug c/101832] __builtin_object_size(P->M, 1) where M ends with a flex-array behaves like sizeof()

[qinzhao at gcc dot gnu.org]

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

--- Comment #6 from qinzhao at gcc dot gnu.org ---
(In reply to Jakub Jelinek from comment #3)
> This is intentional, if you embed an aggregate with flex array into another
> struct and ask not to cross the field boundaries (i.e. bos1), then the size
> of that field is exactly what is the maximum size.

don't quite understand here:

why "embedding an aggregate with flex array member into another struct" is
asking not to cross the field boundaries? could you please explain a little bit
on this?

[Bug c/101832] __builtin_object_size(P->M, 1) where M ends with a flex-array behaves like sizeof()

[qinzhao at gcc dot gnu.org]

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

--- Comment #7 from qinzhao at gcc dot gnu.org ---
from the standard:

A structure or union shall not contain a member with incomplete or function
type (hence, a structure shall not contain an instance of itself, but may
contain a pointer to an instance of itself), except that the last member of a
structure with more than one named member may have incomplete array type; such
a structure (and any union containing, possibly recursively, a member that is
such a structure) shall not be a member of a structure or an element of an
array.

as a result, if you add -Wpedantic to the compilation line:

[opc@qinzhao-ol8u3-x86 101832]$ sh t
/home/opc/Install/latest-d/bin/gcc -O1 -Wpedantic t.c
t.c:30:18: warning: invalid use of structure with flexible array member
[-Wpedantic]
   30 |     struct nlmsg msg;
      |                  ^~~


However, looks like that GCC extension allow such usage, but I am not sure
whether there is any documentation on such extension?

[Bug c/101832] __builtin_object_size(P->M, 1) where M ends with a flex-array behaves like sizeof()

[qinzhao at gcc dot gnu.org]

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

--- Comment #8 from qinzhao at gcc dot gnu.org ---
(In reply to Jakub Jelinek from comment #3)
> This is intentional, if you embed an aggregate with flex array into another
> struct and ask not to cross the field boundaries (i.e. bos1), then the size
> of that field is exactly what is the maximum size.

As we discussed in PR 107952
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107952):

GCC extension accepts the following two cases:

**Case 1:

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

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

**Case 2: 

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

as if a.data[] can be up to 4 elements. 

So, what's you mean by "not to cross the field boundaries" is for the above
Case 2? 
For Case 1, we should treat A.data as flexible array, and then B.A as a
structure that has flexible array, therefore B.A's size is flexible too. 

Is my understanding correct?

[Bug c/101832] __builtin_object_size(P->M, 1) where M ends with a flex-array behaves like sizeof()

[qinzhao at gcc dot gnu.org]

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

--- Comment #9 from qinzhao at gcc dot gnu.org ---
I will add a routine in tree-object-size.cc to check whether a reference to a
structure or union field includes a flexible array member in the inner
structure, such structure or union should be considered as having flexible
size, too. 

then should resolve this bug.

[Bug c/101832] __builtin_object_size(P->M, 1) where M ends with a flex-array behaves like sizeof()

[qinzhao at gcc dot gnu.org]

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

--- Comment #10 from qinzhao at gcc dot gnu.org ---
shall we support the following multi-level nesting?

struct A { int len; char data[]; };
struct B { int n; struct A a; };
struct C { int m, struct B b; };

struct C *outer;

__builtin_object_size (&outer->b, 1);
__builtin_object_size (&outer->b.a, 1);


with the current GCC:

__builtin_object_size(&wrap->b, 1) == 8 
__builtin_object_size(&wrap->b.a, 1) == 4 

We expect both are -1 since the innermost structure A has a trailing flexible
array member.

[Bug tree-optimization/101832] __builtin_object_size(P->M, 1) where M ends with a flex-array behaves like sizeof()

[qinzhao at gcc dot gnu.org]

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

--- Comment #11 from qinzhao at gcc dot gnu.org ---
> (In reply to Jakub Jelinek from comment #3)
> > This is intentional, if you embed an aggregate with flex array into another
> > struct and ask not to cross the field boundaries (i.e. bos1), then the size
> > of that field is exactly what is the maximum size.
> 
> As we discussed in PR 107952
> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107952):
> 
> GCC extension accepts the following two cases:
> 
> **Case 1:
> 
> struct A { char data[1]; };
> struct B { int n; struct A a; };
> 
> as if the a.data[] array is a flex-array.  
> 
> **Case 2: 
> 
> struct C { int n; struct A a; int x; };
> 
> as if a.data[] can be up to 4 elements. 
> 
> So, what's you mean by "not to cross the field boundaries" is for the above
> Case 2? 
> For Case 1, we should treat A.data as flexible array, and then B.A as a
> structure that has flexible array, therefore B.A's size is flexible too. 
> 
> Is my understanding correct?


After discussion, the following patch to clarify the above two cases was
approved:
https://gcc.gnu.org/pipermail/gcc-patches/2023-May/620122.html

with this clarification, the above case 1 is accepted as a GCC extension, but
case 2 will be deprecated. a warning option -Wflex-array-member-not-at-end is
provided to detect case 2 in the user code. 

please see also PR77650.

[Bug tree-optimization/101832] __builtin_object_size(P->M, 1) where M ends with a flex-array behaves like sizeof()

[cvs-commit at gcc dot gnu.org]

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

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Qing Zhao <qinzhao@gcc.gnu.org>:

https://gcc.gnu.org/g:e050ce7c3adf71eedd5482c29cf54b827e026642

commit r14-2225-ge050ce7c3adf71eedd5482c29cf54b827e026642
Author: Qing Zhao <qing.zhao@oracle.com>
Date:   Fri Jun 30 18:24:34 2023 +0000

    Use TYPE_INCLUDES_FLEXARRAY in __builtin_object_size [PR
tree-optimization/101832]

    __builtin_object_size should treat struct with TYPE_INCLUDES_FLEXARRAY as
    flexible size.

    gcc/ChangeLog:

            PR tree-optimization/101832
            * tree-object-size.cc (addr_object_size): Handle structure/union
type
            when it has flexible size.

    gcc/testsuite/ChangeLog:

            PR tree-optimization/101832
            * gcc.dg/builtin-object-size-pr101832.c: New test.

[Bug tree-optimization/101832] __builtin_object_size(P->M, 1) where M ends with a flex-array behaves like sizeof()

[qinzhao at gcc dot gnu.org]

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

qinzhao at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2023-07-19
             Status|UNCONFIRMED                 |ASSIGNED

[Bug tree-optimization/101832] __builtin_object_size(P->M, 1) where M ends with a flex-array behaves like sizeof()

[qinzhao at gcc dot gnu.org]

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

--- Comment #13 from qinzhao at gcc dot gnu.org ---
the fix has been committed into GCC14.

[Bug tree-optimization/101832] __builtin_object_size(P->M, 1) where M ends with a flex-array behaves like sizeof()

[qinzhao at gcc dot gnu.org]

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

--- Comment #14 from qinzhao at gcc dot gnu.org ---
since it's opened again GCC12, the patch need to be backported to GCC12. then
will be closed at that time.