[FFmpeg-devel] [PATCH 0/2] first steps to resolving float to int undefined behavior

[Ganesh Ajjanagadde]

Floating point to integer conversion is well defined when the float lies within
the representation bounds of the integer after discarding the fractional part.
However, in other cases, unfortunately the standard leaves it undefined.
In particular, assuming that it saturates in a sane way is a dangerous assumption.

In light of recent events, I would not have bothered if this was a merely theoretical
issue, and that common environments saturate correctly. Sadly, x86 (for example)
converts casts to a cvttsd2si instruction which saturates numbers > INT64_MAX to
INT64_MIN. This is mathematically completely bogus.
http://blog.frama-c.com/index.php?post/2013/10/09/Overflow-float-integer gives
a nice overview of the issue.

1/2 adds an av_clipd64 API for this purpose to clip a float to an integral range.
Obviously it will be slower than a cvttsd2si, but there is no option if one wants
safe and well-defined behavior. Of course, if one knows a priori that a float
lives in the integral type's range, then there is no issue. Safe speedups are
entirely possible, but API should be finalized first IMHO.
Most common anticipated usages are clipping to [INT64_MIN, INT64_MAX] or
[INT_MIN, INT_MAX].
I have given some thought as to whether a separate av_clipd32 API (for example)
is necessary. It seems to me to not be the case, since an IEEE-754 double is
guaranteed to represent exactly integers up to ~ 2^53.
Similarly, av_clipf64 and the like also seem unnecessary, since a double is guaranteed
to represent all the values a float does. Such an API may be useful for performance
though; I do not know how/what float to double conversions entail and at the
moment ignore such complications. 

1/2 also accordingly documents av_clipd64.

2/2 gives a simple illustration of it in action; something I observed while
working on av_strtod. There are many more such things, but I would like to get
the API finalized (i.e 1/2 in) before moving forward.

--------------------------------------------------------------------------------
I do not know the procedure or intricacies of version bumps, and request help
for it. It also remains to be seen how much of the codebase is affected by this issue.
The ffserver one here, although undefined behavior, does not seem to be an actual
security vulnerability at least on x86: the negative value will be rejected from
the config in subsequent code. This is also an issue that needs some thought:
some of the future work may need to be backported, but a version bump may also
be needed for the API addition.

Ganesh Ajjanagadde (2):
  avutil/common: add av_clipd64
  ffserver_config: replace strtod by av_strtod and correct undefined
    behavior

 ffserver_config.c  | 21 +++++----------------
 libavutil/common.h | 31 +++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 16 deletions(-)

-- 
2.6.2