[FFmpeg-devel] [PATCH 001/244] Add a new channel layout API

Anton Khirnov anton at khirnov.net
Tue Jan 7 20:38:54 EET 2020


Quoting Nicolas George (2019-12-31 15:01:03)
> Anton Khirnov (12019-12-29):
> > Maybe I missed something, but I am not aware of the UB-ness of signed
> > overflow being a practical problem. Typically, your computation will
> > return a meaningless result. You would get a similarly meaningless
> > result from the analogous perfectly well-defined unsigned computation.
> > 
> > to be clear: I am not objecting against fixing UB, but clarifying my
> > 'theoretical gain' comment above.
> 
> It seems you have missed some of the drama that happened on this list
> recently. Michael and others have been intent on fixing the UB caused by
> integer overflows, including cases that I personally find futile.
> 
> But I do not consider this case futile: a channel number will typically
> be used as an array index, and an invalid value will cause an invalid
> memory access and a segmentation fault, or an exploitable memory
> corruption.
> 
> What makes signed overflows, which are UB, worse than unsigned
> overflows, which are completely specified, is that you cannot guard
> against it. For example, if your write:
> 
>         if (ch < 0 || ch >= nb_channels)
>             return AVERROR_BUG;
> 
> you think you are safe, but if the compiler detects that ch cannot be
> negative without overflow, it will silently discard the ch<0 test. Then,
> if the overflows does happen, there is no protection against invalid
> memory access.
> 
> And this is not theoretical: I have seen entire blogs and twitter
> accounts dedicated to posting examples of actual cases where an
> optimizing compiler produces very unintuitive code because there is a
> tiny UB in the middle.

How is it any better in the unsigned case? You do a well-defined
unsigned overflow and end up with an invalid channel count (which might
even look sane).

-- 
Anton Khirnov


More information about the ffmpeg-devel mailing list