[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