[PATCH] dlls/windows.globalization: don't allocate a partial structure

[Eric Pouech]

Le 23/03/2022 à 23:20, Rémi Bernon a écrit :
> 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)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20220324/89fb6d30/attachment.htm>