[Bug other/52944] New: [4.5/4.6 Regression] __builtin_object_size(..., 1) no longer returns (size_t)-1 for consecutive flexible/zero-length array members

[Bug other/52944] New: [4.5/4.6 Regression] __builtin_object_size(..., 1) no longer returns (size_t)-1 for consecutive flexible/zero-length array members

[vapier at gentoo dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52944

             Bug #: 52944
           Summary: [4.5/4.6 Regression] __builtin_object_size(..., 1) no
                    longer returns (size_t)-1 for consecutive
                    flexible/zero-length array members
    Classification: Unclassified
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: other
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: vapier@gentoo.org
            Target: x86_64-linux-gnu


consider the code:

struct stct {
    int i;
    union {
        short k;
        char buf[0];
    };
    char tail[];
};
char buf[100];
main()
{
    struct stct *foo = (void *)buf;
    printf("%i\n", __builtin_object_size(foo->buf, 1));
}

when compiled with gcc-4.4, we get -1.  but with gcc-4.5 and gcc-4.6, we get 0.
 granted, this code is a bit odd, but in some cases, it makes sense.  imo, the
trailing series of flexible/zero-length array members should get the same
treatment rather than just the last one.  gcc doesn't allow flexible array
members inside of unions which is unfortunate.

with tftp, the packet is described by:
struct tftphdr {
    short opcode;
    union {
        unsigned short tu_block;
        short tu_code;
        char tu_stuff[0];
    };
    char th_data[];
};

when opcode is 1, the rest of the packet is a C string.  i.e. the buffer:
    char x[] = { 1, 0, 'f', 'i', 'l', 'e', '\0', };
    opcode = 1, tu_stuff = "file"

when opcode is 3 though, the tu_block field will be a number, and the rest of
the data will be in th_data.  i.e. the buffer:
    char x[] = { 2, 0, 3, 0, <8KiB>, };
    opcode = 2, tu_block = 3, th_data = 8KiB

[Bug other/52944] [4.5/4.6 Regression] __builtin_object_size(..., 1) no longer returns (size_t)-1 for consecutive flexible/zero-length array members

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52944

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |WONTFIX

--- Comment #1 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-04-12 09:20:37 UTC ---
It works as expected.  buf[] is not made magically flexible by tail[].  Why
does the code not simply use

struct stct {
    int i;
    union {
        short k;
        char buf[0];
    };
};

?

Btw, With GCC 4.7 you get 96 ... which is similar conservative as -1.
But zero is certainly correct and expected.

[Bug other/52944] [4.5/4.6 Regression] __builtin_object_size(..., 1) no longer returns (size_t)-1 for consecutive flexible/zero-length array members

[vapier at gentoo dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52944

Mike Frysinger <vapier at gentoo dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.4.7
      Known to fail|                            |4.5.3, 4.6.3

--- Comment #2 from Mike Frysinger <vapier at gentoo dot org> 2012-04-12 15:13:25 UTC ---
if you look at the tftp example, you'd see that your proposed struct does not
work.  the protocol needs two flexible array members at different starting
points.

the 96 isn't conservative in this case, it's correct.  i declared a buffer of
100 bytes, so buf[0] can cover 96 bytes (as the leading integer is 32bits).  is
there a "simplish" patch that could be backported to the 4.5/4.6 branches for
that ?

thinking a bit more, the requirements can probably be pulled off with more
anonymous unions/structs like so:

struct tftphdr {
    short opcode;
    union {
        struct {
            union {
                unsigned short tu_block;
                short tu_code;
            };
            char tu_data[0];
        };
        char tu_stuff[0];
    } __attribute__ ((__packed__));
} __attribute__ ((__packed__));

[Bug other/52944] [4.5/4.6 Regression] __builtin_object_size(..., 1) no longer returns (size_t)-1 for consecutive flexible/zero-length array members

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52944

--- Comment #3 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-04-12 15:28:10 UTC ---
(In reply to comment #2)
> if you look at the tftp example, you'd see that your proposed struct does not
> work.  the protocol needs two flexible array members at different starting
> points.

Ah, indeed.  Now when inspecting your description closer.

---
with tftp, the packet is described by:
struct tftphdr {
    short opcode;
    union {
        unsigned short tu_block;
        short tu_code;
        char tu_stuff[0];
    };
    char th_data[];
};

when opcode is 1, the rest of the packet is a C string.  i.e. the buffer:
    char x[] = { 1, 0, 'f', 'i', 'l', 'e', '\0', };
    opcode = 1, tu_stuff = "file"

when opcode is 3 though, the tu_block field will be a number, and the rest of
the data will be in th_data.  i.e. the buffer:
    char x[] = { 2, 0, 3, 0, <8KiB>, };
    opcode = 2, tu_block = 3, th_data = 8KiB
---

So

struct tftphdr {
    short opcode;
    union {
        struct opcode_3 {
           unsigned short tu_block;
           char th_data[];
        } opcode3;
        short tu_code;
        char tu_stuff[0];
    };
};

would work.  At least

struct stct {
    int i;
    union {
        struct {
            short k;
            char tail[];
        } x;
        char buf[0];
    };
};
char buf[100];
main()
{
  struct stct *foo = (void *)buf;
  printf("%i\n", __builtin_object_size(foo->buf, 1));
  printf("%i\n", __builtin_object_size(foo->x.tail, 1));
}

produces the expected 96, 94 values?

> the 96 isn't conservative in this case, it's correct.  i declared a buffer of
> 100 bytes, so buf[0] can cover 96 bytes (as the leading integer is 32bits).  is
> there a "simplish" patch that could be backported to the 4.5/4.6 branches for
> that ?

No, and it's only because optimization triggers and we get to see buf[] here.
Otherwise you'd get still zero.

> thinking a bit more, the requirements can probably be pulled off with more
> anonymous unions/structs like so:
> 
> struct tftphdr {
>     short opcode;
>     union {
>         struct {
>             union {
>                 unsigned short tu_block;
>                 short tu_code;
>             };
>             char tu_data[0];
>         };
>         char tu_stuff[0];
>     } __attribute__ ((__packed__));
> } __attribute__ ((__packed__));

I don't see how this would avoid the issue for __builtin_object_size
(foo->...tu_data) though.

[Bug other/52944] [4.5/4.6 Regression] __builtin_object_size(..., 1) no longer returns (size_t)-1 for consecutive flexible/zero-length array members

[vapier at gentoo dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52944

--- Comment #4 from Mike Frysinger <vapier at gentoo dot org> 2012-04-12 15:42:49 UTC ---
(In reply to comment #3)

wouldn't it though ?  there's still a top level union there surrounding all the
members.  so flattening it, i'd get three choices:
 - th_block; th_data
 - th_code; th_data
 - th_stuff

the problem before was that it was a union followed by th_data

testing locally with gcc-4.6.2, i get the object sizes of:
 - -O0: th_data:-1 th_stuff:-1
 - -O[s123]: th_data:96 th_stuff:98

[Bug other/52944] [4.5/4.6 Regression] __builtin_object_size(..., 1) no longer returns (size_t)-1 for consecutive flexible/zero-length array members

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52944

--- Comment #5 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-04-13 09:30:48 UTC ---
(In reply to comment #4)
> (In reply to comment #3)
> 
> wouldn't it though ?  there's still a top level union there surrounding all the
> members.  so flattening it, i'd get three choices:
>  - th_block; th_data
>  - th_code; th_data
>  - th_stuff
> 
> the problem before was that it was a union followed by th_data
> 
> testing locally with gcc-4.6.2, i get the object sizes of:
>  - -O0: th_data:-1 th_stuff:-1
>  - -O[s123]: th_data:96 th_stuff:98

Ah, yeah.  That might work reliably indeed.  Though I find my solutions
nicer and more explicit ;)