avutil/mathematics: make av_gcd more robust
authorGanesh Ajjanagadde <gajjanagadde@gmail.com>
Thu, 29 Oct 2015 02:02:25 +0000 (22:02 -0400)
committerGanesh Ajjanagadde <gajjanagadde@gmail.com>
Thu, 29 Oct 2015 23:13:55 +0000 (19:13 -0400)
This ensures that no undefined behavior is invoked, while retaining
identical return values in all cases and at no loss of performance
(identical asm on clang and gcc).
Essentially, this patch exchanges undefined behavior with implementation
defined behavior, a strict improvement.

Rationale:
1. The ideal solution is to have the return type a uint64_t. This
unfortunately requires an API change.
2. The only pathological behavior happens if both arguments are
INT64_MIN, to the best of my knowledge. In such a case, the
implementation defined behavior is invoked in the sense that UINT64_MAX
is interpreted as INT64_MIN, which any reasonable implementation will
do. In any case, any usage where both arguments are INT64_MIN is a
fuzzer anyway.
3. Alternatives of checking, etc require branching and lose performance
for no concrete gain - no client cares about av_gcd's actual value when
both args are INT64_MIN. Even if it did, on sane platforms (e.g all the
ones FFmpeg cares about), it produces a correct gcd, namely INT64_MIN.

Reviewed-by: Michael Niedermayer <michael@niedermayer.cc>
Signed-off-by: Ganesh Ajjanagadde <gajjanagadde@gmail.com>
libavutil/mathematics.c

index 16e4eba..fde460c 100644 (file)
@@ -52,7 +52,7 @@ int64_t av_gcd(int64_t a, int64_t b) {
         v -= u;
         v >>= ff_ctzll(v);
     }
-    return u << k;
+    return (uint64_t)u << k;
 }
 
 int64_t av_rescale_rnd(int64_t a, int64_t b, int64_t c, enum AVRounding rnd)