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

[Nicolas George]

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.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20191231/477ec630/attachment.sig>