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

[Rémi Bernon]

On 3/23/22 12:11, Eric Pouech wrote:
> Le 23/03/2022 à 11:15, Rémi Bernon a écrit :
>> On 3/23/22 08:19, Eric Pouech wrote:
>>> GCC 11 complains about accessing struct hstring_vector (-Warray-bounds)
>>> when the allocation is made for a 0-sized vector
>>>
>>> so ensure that we always allocate a memory block to fit a whole
>>> structure
>>>
>>> Signed-off-by: Eric Pouech <eric.pouech at gmail.com>
>>>
>>> ---
>>>   dlls/windows.globalization/main.c |    6 ++++--
>>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/dlls/windows.globalization/main.c 
>>> b/dlls/windows.globalization/main.c
>>> index 3e5a59bde14..363e0150af1 100644
>>> --- a/dlls/windows.globalization/main.c
>>> +++ b/dlls/windows.globalization/main.c
>>> @@ -213,8 +213,10 @@ static const struct IVectorView_HSTRINGVtbl 
>>> hstring_vector_vtbl =
>>>   static HRESULT hstring_vector_create(HSTRING *values, SIZE_T count, 
>>> IVectorView_HSTRING **out)
>>>   {
>>>       struct hstring_vector *impl;
>>> -
>>> -    if (!(impl = malloc(offsetof(struct hstring_vector, 
>>> values[count])))) return E_OUTOFMEMORY;
>>> +    /* always allocate at least the full structure to avoid GCC11 
>>> warnings */
>>> +    if (!(impl = malloc(max(offsetof(struct hstring_vector, 
>>> values[count]),
>>> +                            sizeof(struct hstring_vector)))))
>>> +        return E_OUTOFMEMORY;
>>>       impl->ref = 1;
>>>         impl->IVectorView_HSTRING_iface.lpVtbl = &hstring_vector_vtbl;
>>>
>>>
>>
>> IMHO GCC should fix its warning instead, we do that in many places and 
>> I think it's completely valid.
> 
> Hi Rémi
> 
> see 
> https://www.winehq.org/pipermail/wine-devel/2022-February/thread.html#207795 
> for a previous discussion on a similar issue (and the final decision of 
> over-allocating)
> 
> to my understanding, accessing through a pointer of type mytype* a 
> memory block which storage is strictly smaller to sizeo(mytype) is 
> clearly undefined behavior (I'm not stating that it does in fact 
> generate wrong results)
> 
> (even if the accessed field is inside the allocated memory size)
> 
> in this precise case, defining the structure with 0 length array would 
> be another option, yet non portable
> 
> and for the record, gcc12 (even if non yet released) generates a few 
> more warnings about this subject on wine code (and I'm not even talking 
> of mingw port of gcc12)
> 
> A+
> 
> 


Well I think this makes the code very bloated, and if it is truly UB 
then we should instead find a way to use 0-sized arrays in a portable way.

I believe 0 sized array for the last struct member is standard C99, so 
is there any compiler not supporting it yet (or some variation of it 
like with empty size)?

Of course this doesn't apply to structs which need a specific size with 
1-sized array for Win32 ABI compat, but it's probably not that many and 
not the case here.
-- 
Rémi Bernon <rbernon at codeweavers.com>