[FFmpeg-devel] [PATCH] seek: Fix crashes in ff_seek_frame_binary if built with latest Clang 14

[Martin Storsjö]

On Sat, 6 Nov 2021, Andreas Rheinhardt wrote:

> Martin Storsjö:
>> On Sat, 6 Nov 2021, Andreas Rheinhardt wrote:
>> 
>>> For GCC and Clang av_uninit(x) is defined as x=x. And that is a problem:
>>> In case this macro is used to declare an automatic variable that is
>>> could be declared with the register storage class the
>>> pseudo-initialization is UB according to the above. So I think we will
>>> have to modify the macro to make it safe. Or just stop using it.
>>> (How could such a hack ever end up in a public header?)
>> 
>> FWIW, most of the issue here comes from the fact that the uninitialized
>> value is passed as a parameter - in that respect, av_uninit() isn't any
>> better or worse than just leaving the variable plain uninitialized. (Not
>> that one really should reason around where UB is ok...)
>> 
>
> It is UB even when it is not used as a function argument at all. While
> there seems to be no current compiler that uses this against us, there
> is no guarantee that it will stay this way. After all, this patch is
> about an unpleasant surprise and there might be more of that in the future.

Yes, that's true, as leaving a variable uninitialized then initializing it 
before actual use is fine, as long as it's not used uninitialized - while 
here we have the UB already in the initialization.

However - in the case of the Clang optimization breaking our code, there's 
no difference between leaving it entirely uninitialized or using the x=x 
trick though: In both cases, the compiler concluded that as the values 
passed to the function call must be initialized at that point, the 
codepaths that initialize the values must have been taken, and thus 
optimizing out the conditions.

// Martin