Fix an UBSan error (signed overflow) in saturating addition and subtraction

webrtc/common_audio/signal_processing/include/spl_inl.h

 /*
  *  Copyright (c) 2011 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 17 matching lines @@
   int16_t out16 = (int16_t) value32;
 
   if (value32 > 32767)
     out16 = 32767;
   else if (value32 < -32768)
     out16 = -32768;
 
   return out16;
 }
 
-static __inline int32_t WebRtcSpl_AddSatW32(int32_t l_var1, int32_t l_var2) {
-  int32_t l_sum;
+static __inline int32_t WebRtcSpl_AddSatW32(int32_t a, int32_t b) {
+  // Do the addition in unsigned numbers, since signed overflow is UB.

[tlegrand-webrtc, 2016/05/20 11:07:14]

UB? Please write undefined behavior instead.

[kwiberg-webrtc, 2016/05/23 08:30:04]

Done.

+  const int32_t sum = (int32_t)((uint32_t)a + (uint32_t)b);
 
-  // Perform long addition
-  l_sum = l_var1 + l_var2;
-
-  if (l_var1 < 0) {  // Check for underflow.
-    if ((l_var2 < 0) && (l_sum >= 0)) {
-        l_sum = (int32_t)0x80000000;
-    }
-  } else {  // Check for overflow.
-    if ((l_var2 > 0) && (l_sum < 0)) {
-        l_sum = (int32_t)0x7FFFFFFF;
-    }
+  // a + b can't overflow if a and b have different signs. If they have the
+  // same sign, a + b also has the same sign iff it didn't overflow.
+  if ((a < 0) == (b < 0) && (a < 0) != (sum < 0)) {
+    // The direction of the overflow is obvious from the sign of a + b.
+    return sum < 0 ? INT32_MAX : INT32_MIN;

[tlegrand-webrtc, 2016/05/20 11:07:14]

Clever!

[kwiberg-webrtc, 2016/05/23 08:30:04]

Acknowledged.

   }
-
-  return l_sum;
+  return sum;
 }
 
-static __inline int32_t WebRtcSpl_SubSatW32(int32_t l_var1, int32_t l_var2) {
-  int32_t l_diff;
+static __inline int32_t WebRtcSpl_SubSatW32(int32_t a, int32_t b) {
+  // Do the subtraction in unsigned numbers, since signed overflow is UB.

[tlegrand-webrtc, 2016/05/20 11:07:14]

Here as well.

[kwiberg-webrtc, 2016/05/23 08:30:04]

Done.

+  const int32_t diff = (int32_t)((uint32_t)a - (uint32_t)b);
 
-  // Perform subtraction.
-  l_diff = l_var1 - l_var2;
-
-  if (l_var1 < 0) {  // Check for underflow.
-    if ((l_var2 > 0) && (l_diff > 0)) {
-      l_diff = (int32_t)0x80000000;
-    }
-  } else {  // Check for overflow.
-    if ((l_var2 < 0) && (l_diff < 0)) {
-      l_diff = (int32_t)0x7FFFFFFF;
-    }
+  // a - b can't overflow if a and b have the same sign. If they have different
+  // signs, a - b has the same sign as a iff it didn't overflow.
+  if ((a < 0) != (b < 0) && (a < 0) != (diff < 0)) {
+    // The direction of the overflow is obvious from the sign of a - b.
+    return diff < 0 ? INT32_MAX : INT32_MIN;
   }
-
-  return l_diff;
+  return diff;
 }
 
 static __inline int16_t WebRtcSpl_AddSatW16(int16_t a, int16_t b) {
   return WebRtcSpl_SatW32ToW16((int32_t) a + (int32_t) b);
 }
 
 static __inline int16_t WebRtcSpl_SubSatW16(int16_t var1, int16_t var2) {
   return WebRtcSpl_SatW32ToW16((int32_t) var1 - (int32_t) var2);
 }
 #endif  // #if !defined(MIPS_DSP_R1_LE)
@@ skipping 80 matching lines @@
 }
 
 static __inline int32_t WebRtc_MulAccumW16(int16_t a, int16_t b, int32_t c) {
   return (a * b + c);
 }
 #endif  // #if !defined(MIPS32_LE)
 
 #endif  // WEBRTC_ARCH_ARM_V7
 
 #endif  // WEBRTC_SPL_SPL_INL_H_