[Bug c/105684] New: Bogus `-Warray-bounds` in partially allocated struct

[Bug c/105684] New: Bogus `-Warray-bounds` in partially allocated struct

[sagebar at web dot de]

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

            Bug ID: 105684
           Summary: Bogus `-Warray-bounds` in partially allocated struct
           Product: gcc
           Version: 12.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: sagebar at web dot de
  Target Milestone: ---

Accessing the fields of a partially allocated struct results in
`-Warray-bounds`, even when all accessed fields lie within the allocated
portion of the struct (a fact that can be proven to be known to gcc by asking
it for `__builtin_object_size()`)

Example (compile with `gcc -c infile.c -O2 -Warray-bounds -S -o -`):

```
struct obj {
        unsigned long long kind;
        union {
                struct {
                        unsigned long long a;
                } data0;
                struct {
                        unsigned long long a;
                        unsigned long long b;
                } data1;
        };
};

extern __attribute__((alloc_size(1))) void *my_malloc(unsigned long);
#define offsetafter(s, m) (__builtin_offsetof(s, m) + sizeof(((s *)0)->m))

struct obj *create_object_kind_0(void) {
        struct obj *result;
        result = (struct obj *)my_malloc(offsetafter(struct obj, data0));
        __asm__ __volatile__("# Object size is: %p0" : : "X"
(__builtin_object_size(result, 0)));
        result->kind    = 0;
        result->data0.a = 1;
        return result;
}
```

Real-world use-case & rationale: the idea is that access to `data1` vs. `data0`
is governed by the value of `kind` (`obj` is a "typed variant"), and by
allocating only the relevant portion of the object, we can save memory, and
create an environment where buggy code that wrongfully accesses (e.g.) certain
fields of `data1` of a `kind=0` object might cause the program to crash (rather
than remain unnoticed as would be the case when simply using `sizeof(struct
obj)` as allocation size).

Expected behavior: no warnings should be generated

====== Actual behavior (Truncated) =======
>[stderr]infile.c: In function 'create_object_kind_0':
>[stderr]infile.c:22:15: warning: array subscript 'struct obj[0]' is partly outside array bounds of 'unsigned char[16]' [-Warray-bounds]
>[stderr]   21 |         result->kind    = 0;
>[stderr]      |               ^~
>[stderr]infile.c:20:32: note: object of size 16 allocated by 'my_malloc'
>[stderr]   19 |         result = (struct obj *)my_malloc(offsetafter(struct obj, data0));
>[stderr]      |                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>[stderr]infile.c:23:15: warning: array subscript 'struct obj[0]' is partly outside array bounds of 'unsigned char[16]' [-Warray-bounds]
>[stderr]   22 |         result->data0.a = 1;
>[stderr]      |               ^~
>[stderr]infile.c:20:32: note: object of size 16 allocated by 'my_malloc'
>[stderr]   19 |         result = (struct obj *)my_malloc(offsetafter(struct obj, data0));
>[stderr]      |                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>[stdout][...]
>[stdout]/  22 "infile.c" 1
>[stdout]        # Object size is: 16
>[stdout]/  0 "" 2
>[stdout]/NO_APP
>[stdout]        movl    $0, (%eax)
>[stdout]        movl    $0, 4(%eax)
>[stdout]        movl    $1, 8(%eax)
>[stdout]        movl    $0, 12(%eax)
>[stdout]        leave
>[stdout]        ret
>[stdout][...]

As can be seen by `# Object size is: 16` (and repeated in warning messages),
gcc knows that the allocated size of the object is 16 bytes, and as can be seen
by the offsets used by the `movl` instructions, this limit is never exceeded.

Judging by what the warnings state (and confirmed by replacing
`offsetafter(struct obj, data0)` with `sizeof(struct obj)`), the
`-Warray-bounds` warning will always be generated for any accessed field
(irregardless of that field's offset) so-long as `__builtin_object_size(result,
0) < sizeof(struct obj)`.

I am uncertain about the intended semantics of `-Warray-bounds`, and
technically speaking, the warning is stating the truth: "'struct obj[0]' is
partly outside array bounds" (emphasis on the "partly"). However, as it stands
right now, warning about this situation is less than useful (a warning might
arguably be useful when passing `result` -- or a pointer to one of its members
-- to some other function, but even in that situation a warning would probably
be a false positive). So in my opinion, at `-Warray-bounds[=1]`, a warning
should only be generated exactly in those cases where an out-of-bounds access
*actually* happens (*possible* out-of-bounds warnings may appear at
`-Warray-bounds=2`, but definitely shouldn't at `-Warray-bounds=1`).

==================================================


Additionally, after making the following changes to the above code (causing
`sizeof(struct obj) == offsetafter(struct obj, data0)`):

```
@@ -6,7 +6,7 @@
                } data0;
                struct {
                        unsigned long long a;
-                       unsigned long long b;
+                       unsigned long long b[];
                } data1;
        };
 };
@@ -20,6 +20,7 @@
        __asm__ __volatile__("# Object size is: %p0" : : "X"
(__builtin_object_size(result, 0)));
        result->kind    = 0;
        result->data0.a = 1;
+       result->data1.b[0] = 1;
        return result;
 }
```

Expected behavior: A warnings should be generated for `result->data1.b[0] = 1`

====== Actual behavior =======

No warnings are generated, even though **now** an out-of-bounds access actually
**does** take place (`result->data1.b[0] = 1`), and gcc should easily be able
to spot this (see suggestion below).



====== Conclusion =======

It would seem that an expression like `obj-><FIELDEXPR> = v` only causes gcc to
make the following check:
> if (__builtin_object_size(obj, 0) < sizeof(typeof(*obj))) WARN("-Warray-bounds");
Rather than checking (suggestion):
> if (__builtin_object_size(obj, 0) < offsetafter(typeof(*obj), <FIELDEXPR>)) WARN("-Warray-bounds");

The former obviously breaks for partially allocated structs while not producing
any warning for out-of-bounds access to a flexible array member (since sizeof()
on such an object only returns the offset of the flexible array member).
However, if instead the suggested check were to be made, the false warnings
wouldn't appear, and the currently missing warning would be generated.

[Bug tree-optimization/105684] Bogus `-Warray-bounds` in partially allocated struct

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|c                           |tree-optimization
           Keywords|                            |diagnostic

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I don't think it is defined behavior if it is partially allocated. Unions it
would be questionable but not structs.

[Bug tree-optimization/105684] Bogus `-Warray-bounds` in partially allocated struct

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |56456

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
The issue is that 'result->data0.a' is (*result).data0.a, and so to GCC you are
accessing an object of type 'obj' for which there isn't enough allocated space.

Now - I think the array bound diagnostic could be improved to better highlight
this.

A workaround might be to declare 'obj' as

struct obj {
        unsigned long long kind;
        unsigned long long data[];
};

if in your case all data is really uniform 'unsigned long long'.  There's
no way to union several different typed flexarrays though.


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56456
[Bug 56456] [meta-bug] bogus/missing -Warray-bounds

[Bug tree-optimization/105684] Bogus `-Warray-bounds` in partially allocated struct

[sagebar at web dot de]

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

--- Comment #3 from sagebar at web dot de ---
>The issue is that 'result->data0.a' is (*result).data0.a, and so to GCC you are
>accessing an object of type 'obj' for which there isn't enough allocated space.

Technically true, and yes: not dereferencing `result` while it's still a
`struct obj` makes the warning go away:
```
-       result->kind    = 0;
-       result->data0.a = 1;
+       *(unsigned long long *)((char *)result + __builtin_offsetof(struct obj,
kind)) = 0;
+       *(unsigned long long *)((char *)result + __builtin_offsetof(struct obj,
data0.a)) = 1;
```

But that's like saying `int x = obj->field;` is the same as `int x = ({
typeof(*obj) c = *obj; c.field; })` (i.e. accessing 1 field is like accessing
_all_ fields) -- They're just not the same: accessing a single field simply
doesn't cause the rest of the containing struct to also be loaded (if there's
some weird arch where it does ???, fine; but then this should be an
arch-specific warning).



>A workaround might be to declare 'obj' as
>
>struct obj {
>	unsigned long long kind;
>	unsigned long long data[];
>};
>
>if in your case all data is really uniform 'unsigned long long'.  There's
>no way to union several different typed flexarrays though.

It would for the demo-code, but that's not the context where I encountered this
problem in the real world. For reference, the following is the "typed
variant"-style struct which lead me to create this bug report:
https://github.com/GrieferAtWork/KOSmk4/blob/master/kos/src/kernel/moddbx/include/ctype.h#L193

I simply crafted the minimal viable code that still causes the warning for the
sake of making this bug report.