On 3/23/22 22:54, Chris Robinson wrote:

On Wednesday, March 23, 2022 12:32:52 PM PDT Alexandre Julliard wrote:

An empty size is probably OK to use at this point. It's not clear how
much benefit it brings though, because obviously 1-size arrays in public
structures can't be changed.

And I'm not sure it would change anything regarding this patch and ensuring
the object is properly allocated. A flexible array member isn't guaranteed to
be at the very end of the struct, it can overlap with some padding:

struct Foo {
     int a;
     char b;
     char c[];
};

On most systems, sizeof(struct Foo) will be 8 bytes, but 'c' would immediately
follow 'b' causing offsetof(struct Foo, c[0]) to be 5. So if you use
offsetof(struct Foo, c[count]) for the allocation size, anything less than
c[3] would be under-allocating and create potential implicit overruns just
like before. so you still need to do something like

max(sizeof(struct Foo), offsetof(struct Foo, c[count])

to ensure a proper minimum allocation size.

Sounds like an unfortunate and an aweful case, but GCC would probably emit the warning and we can make sure the flexible array is aligned with the struct.

Or we can (and probably should) also add a C_ASSERT to validate that sizeof(struct) == offsetof(struct, array[0]) for structs with flexible arrays. That would make the expectations much more explicit.

In any case, I think we should avoid max(sizeof, offsetof), it makes the code less readable imho. Unless required by the Win32 ABI compatibility and public header definitions, of course.

In this particular case here the array is of pointers, and there's no alignment issues.

so to summarize:

- -Warray-bounds from GCC >= 11 need to be taken care of

- when possible, use flexible array member (*)

- if not, allocate the at least the whole struct

I'll resubmit this patch with FAM instead

(*) note that msvc supports them even in C++. And they are some instances of FAM present in MS SDK headers; MSVC generate warnings, but those are silenced in those headers with pragmas warning(disable: 4200)). g++ supports them as well (with a warning in -pedantic mode)