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

[Eric Pouech]

Le 15/02/2022 à 18:54, Zebediah Figura a écrit :
>
> [1] claims the following:
>
> "It's not valid to access an object of one type using an lvalue of 
> another.  In simple cases GCC diagnoses violations of this requirement 
> by -Wstrict-aliasing.  -Warray-bounds doesn't detect aliasing 
> violations but it does detect a subset of the problem that's apparent 
> when the size of the lvalue's type is greater than the size of the 
> object.  The object must be big enough for the whole lvalue, even if 
> the accessed member is within the bounds of the smaller object."
>
> I can't easily find this in the C specification, but it's also not the 
> easiest thing to search for, and it wouldn't surprise me if it's true.

I think you mean this (from C89/90 ; 6.3 expressions)

Anobjectshallhaveitsstoredvalueaccessedonlybyanlvaluethathasoneofthefollowing
types?”
-thedeclaredtypeoftheobject,
-a qualifiedversionofthedeclaredtypeoftheobject.
-a typethatis 
thesignedorunsignedtypecorrespondingtothedeclaredtypeoftheobject,
-a typethatisthesignedorunsignedtypecorrespondingtoa 
qualifiedversionofthedeclared
typeoftheobject.
-anaggregateoruniontypethatincludesoneoftheaforementionedtypesamongitsmembers
(including.recursively.a memberofa hubaggregateorcontainedunion).or
-a charactertype

> Still, I'm not convinced we really want to change our usage 
> accordingly. I'm guessing this is one of those things that may be 
> undefined behaviour but always works in practice, and from the looks 
> of things we have more false positives from -Warray-bounds than true 
> positives.

I'm not proposing to find and fix all UB in Wine code [1]; I'm just 
trying to reduce the warnigs generated by our default settings : roughly 
200 warnings for a full Wine compilation for gcc (mingw 11.2 cross 
compiler) [2]

very few true positive indeed (IMO the years of coverity and other tools 
did clean up a lot of them) (only one found [3])

>
> But if we really do want -Warray-bounds, I'd rather just allocate the 
> whole VIDEOINFO structure here.

I think it's still useful for detecting bad allocations in future code

I'll resubmit with the whole allocation then

thanks for looking into it

>
>
> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103292
>
> P.S. can you please remember to keep wine-devel in CC?
sorry, my bad

[1] even though having ubsan and asan running on Wine's PE code would be 
nice (mingw clang may provide it)

[2] it maybe I get nauseous about those warnings because of all the full 
recompilations for the 'long types' thingie

[3] the funny story is that the first code committed was correct but it 
has been (badly) changed to fix a Coverity report <g>

https://source.winehq.org/git/wine.git/?a=commit;h=14f8089dc1793a6c7d91d3dfe822d2f51526bbe8
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20220215/caf4afc1/attachment.htm>