[PATCH 4/6] dlls/quartz: use VIDEOINFOHEADER instead of partially allocated VIDEOINFO structure

[Zebediah Figura]

On 2/14/22 12:27, Eric Pouech wrote:
> Le 14/02/2022 à 19:07, Zebediah Figura a écrit :
>> On 2/14/22 03:28, Eric Pouech wrote:
>>> this fixes a couple of GCC11 warnings (-Wbounds)
>>>
>>> Signed-off-by: Eric Pouech <eric.pouech at gmail.com>
>>>
>>> ---
>>>    dlls/quartz/avidec.c |    9 +++++----
>>>    1 file changed, 5 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/dlls/quartz/avidec.c b/dlls/quartz/avidec.c
>>> index f8660890273..1295e8704df 100644
>>> --- a/dlls/quartz/avidec.c
>>> +++ b/dlls/quartz/avidec.c
>>> @@ -331,7 +331,7 @@ static HRESULT
>>> avi_decompressor_source_get_media_type(struct strmbase_pin *iface
>>>          struct avi_decompressor *filter =
>>> impl_from_strmbase_filter(iface->filter);
>>>        const VIDEOINFOHEADER *sink_format;
>>> -    VIDEOINFO *format;
>>> +    VIDEOINFOHEADER *format;
>>>          if (!filter->sink.pin.peer)
>>>            return VFW_S_NO_MORE_ITEMS;
>>> @@ -363,9 +363,10 @@ static HRESULT
>>> avi_decompressor_source_get_media_type(struct strmbase_pin *iface
>>>              if (IsEqualGUID(formats[index].subtype,
>>> &MEDIASUBTYPE_RGB565))
>>>            {
>>> -            format->dwBitMasks[iRED] = 0xf800;
>>> -            format->dwBitMasks[iGREEN] = 0x07e0;
>>> -            format->dwBitMasks[iBLUE] = 0x001f;
>>> +            DWORD *dwBitMasks = (DWORD *)(format + 1);
>>> +            dwBitMasks[iRED] = 0xf800;
>>> +            dwBitMasks[iGREEN] = 0x07e0;
>>> +            dwBitMasks[iBLUE] = 0x001f;
>>>                mt->cbFormat = offsetof(VIDEOINFO, dwBitMasks[3]);
>>>            }
>>>            else
>>>
>>>
>>
>> I don't think this is an improvement.
>>
>> I also don't get the warning with gcc 11.2.0. How exactly are you
>> compiling and with which version?
>>
> Hi Zebdiah
> 
> compiling with stock gcc version from fedora and regular wine's
> configure (no parameter added)
> 
> this is what I get with compiling a 64bit tree, configured with
> 
> ./configure --enable-wine64
> 
> make[1]: Entering directory '/home/eric/work/wine-format/build64'
> x86_64-w64-mingw32-gcc -c -o dlls/quartz/avidec.cross.o
> /home/eric/work/wine-format/dlls/quartz/avidec.c -Idlls/quartz \
>     -I/home/eric/work/wine-format/dlls/quartz -Iinclude
> -I/home/eric/work/wine-format/include \
>     -I/home/eric/work/wine-format/include/msvcrt -D__WINESRC__ -D_UCRT
> -D__WINE_PE_BUILD -Wall \
>     -fno-strict-aliasing -Wdeclaration-after-statement -Wempty-body
> -Wignored-qualifiers -Winit-self \
>     -Wno-packed-not-aligned -Wshift-overflow=2 -Wstrict-prototypes
> -Wtype-limits \
>     -Wunused-but-set-parameter -Wvla -Wwrite-strings -Wpointer-arith
> -Wlogical-op -Wabsolute-value \
>     -Wno-format -Wformat-overflow -Wnonnull -mcx16 -gdwarf-4 -Wformat -g -O2
> /home/eric/work/wine-format/dlls/quartz/avidec.c: In function
> 'avi_decompressor_source_get_media_type':
> /home/eric/work/wine-format/dlls/quartz/avidec.c:349:15: warning: array
> subscript 'VIDEOINFO {aka struct tagVIDEOINFO}[0]' is partly outside
> array bounds of 'unsigned char[100]' [-Warray-bounds]
>     349 |         format->rcSource = sink_format->rcSource;
>         |               ^~
> /home/eric/work/wine-format/dlls/quartz/avidec.c:345:24: note:
> referencing an object of size 100 allocated by 'CoTaskMemAlloc'
>     345 |         if (!(format = CoTaskMemAlloc(offsetof(VIDEOINFO,
> dwBitMasks[3]))))
>         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Okay, I was wrong about my gcc version; for some reason Debian ships gcc 
and mingw-w64 gcc versions that are different.

I think this is a gcc bug. It now warns about *any* access to a 
structure which was allocated with anything but sizeof(). E.g. the 
following will generate warnings with -O2 -Warray-bounds=1:

--

#include <stddef.h>
#include <stdlib.h>

struct apple
{
     unsigned int u;
     char v[10];
};

struct apple *func(void)
{
     struct apple *a = malloc(offsetof(struct apple, v[10]));

     a->u = 1;
     a->v[9] = 3;
     return a;
}

--

Same with malloc(14). Only malloc(sizeof(struct apple)) works, even 
though the three expressions are obviously identical.

If we must work around this in Wine, I'd rather either disable 
-Warray-bounds or else just use sizeof() and over-allocate.