Avoids undefined behaviour in ARM-optimized code
authorJean-Marc Valin <jmvalin@jmvalin.ca>
Thu, 26 Jan 2017 02:56:48 +0000 (21:56 -0500)
committerJean-Marc Valin <jmvalin@jmvalin.ca>
Thu, 26 Jan 2017 03:20:25 +0000 (22:20 -0500)
Casting to unsigned to avoid shifting negative values left.

celt/arm/fixed_armv4.h
celt/arm/fixed_armv5e.h
silk/arm/NSQ_neon.h
silk/arm/macros_armv4.h
silk/arm/macros_armv5e.h

index efb3b18..d84888a 100644 (file)
@@ -37,7 +37,7 @@ static OPUS_INLINE opus_val32 MULT16_32_Q16_armv4(opus_val16 a, opus_val32 b)
       "#MULT16_32_Q16\n\t"
       "smull %0, %1, %2, %3\n\t"
       : "=&r"(rd_lo), "=&r"(rd_hi)
-      : "%r"(b),"r"(a<<16)
+      : "%r"(b),"r"(SHL32(a,16))
   );
   return rd_hi;
 }
@@ -54,10 +54,10 @@ static OPUS_INLINE opus_val32 MULT16_32_Q15_armv4(opus_val16 a, opus_val32 b)
       "#MULT16_32_Q15\n\t"
       "smull %0, %1, %2, %3\n\t"
       : "=&r"(rd_lo), "=&r"(rd_hi)
-      : "%r"(b), "r"(a<<16)
+      : "%r"(b), "r"(SHL32(a,16))
   );
   /*We intentionally don't OR in the high bit of rd_lo for speed.*/
-  return rd_hi<<1;
+  return SHL32(rd_hi,1);
 }
 #define MULT16_32_Q15(a, b) (MULT16_32_Q15_armv4(a, b))
 
index 36a6321..6bf73cb 100644 (file)
@@ -59,7 +59,7 @@ static OPUS_INLINE opus_val32 MULT16_32_Q15_armv5e(opus_val16 a, opus_val32 b)
       : "=r"(res)
       : "r"(b), "r"(a)
   );
-  return res<<1;
+  return SHL32(res,1);
 }
 #define MULT16_32_Q15(a, b) (MULT16_32_Q15_armv5e(a, b))
 
@@ -76,7 +76,7 @@ static OPUS_INLINE opus_val32 MAC16_32_Q15_armv5e(opus_val32 c, opus_val16 a,
       "#MAC16_32_Q15\n\t"
       "smlawb %0, %1, %2, %3;\n"
       : "=r"(res)
-      : "r"(b<<1), "r"(a), "r"(c)
+      : "r"(SHL32(b,1)), "r"(a), "r"(c)
   );
   return res;
 }
index 77c946a..b31d944 100644 (file)
@@ -28,30 +28,31 @@ POSSIBILITY OF SUCH DAMAGE.
 #define SILK_NSQ_NEON_H
 
 #include "cpu_support.h"
+#include "SigProc_FIX.h"
 
 #undef silk_short_prediction_create_arch_coef
 /* For vectorized calc, reverse a_Q12 coefs, convert to 32-bit, and shift for vqdmulhq_s32. */
 static OPUS_INLINE void silk_short_prediction_create_arch_coef_neon(opus_int32 *out, const opus_int16 *in, opus_int order)
 {
-    out[15] = in[0] << 15;
-    out[14] = in[1] << 15;
-    out[13] = in[2] << 15;
-    out[12] = in[3] << 15;
-    out[11] = in[4] << 15;
-    out[10] = in[5] << 15;
-    out[9]  = in[6] << 15;
-    out[8]  = in[7] << 15;
-    out[7]  = in[8] << 15;
-    out[6]  = in[9] << 15;
+    out[15] = silk_LSHIFT32(in[0], 15);
+    out[14] = silk_LSHIFT32(in[1], 15);
+    out[13] = silk_LSHIFT32(in[2], 15);
+    out[12] = silk_LSHIFT32(in[3], 15);
+    out[11] = silk_LSHIFT32(in[4], 15);
+    out[10] = silk_LSHIFT32(in[5], 15);
+    out[9]  = silk_LSHIFT32(in[6], 15);
+    out[8]  = silk_LSHIFT32(in[7], 15);
+    out[7]  = silk_LSHIFT32(in[8], 15);
+    out[6]  = silk_LSHIFT32(in[9], 15);
 
     if (order == 16)
     {
-        out[5] = in[10] << 15;
-        out[4] = in[11] << 15;
-        out[3] = in[12] << 15;
-        out[2] = in[13] << 15;
-        out[1] = in[14] << 15;
-        out[0] = in[15] << 15;
+        out[5] = silk_LSHIFT32(in[10], 15);
+        out[4] = silk_LSHIFT32(in[11], 15);
+        out[3] = silk_LSHIFT32(in[12], 15);
+        out[2] = silk_LSHIFT32(in[13], 15);
+        out[1] = silk_LSHIFT32(in[14], 15);
+        out[0] = silk_LSHIFT32(in[15], 15);
     }
     else
     {
index 3f30e97..877eb18 100644 (file)
@@ -28,6 +28,11 @@ POSSIBILITY OF SUCH DAMAGE.
 #ifndef SILK_MACROS_ARMv4_H
 #define SILK_MACROS_ARMv4_H
 
+/* This macro only avoids the undefined behaviour from a left shift of
+   a negative value. It should only be used in macros that can't include
+   SigProc_FIX.h. In other cases, use silk_LSHIFT32(). */
+#define SAFE_SHL(a,b) ((opus_int32)((opus_uint32)(a) << (b)))
+
 /* (a32 * (opus_int32)((opus_int16)(b32))) >> 16 output have to be 32bit int */
 #undef silk_SMULWB
 static OPUS_INLINE opus_int32 silk_SMULWB_armv4(opus_int32 a, opus_int16 b)
@@ -38,7 +43,7 @@ static OPUS_INLINE opus_int32 silk_SMULWB_armv4(opus_int32 a, opus_int16 b)
       "#silk_SMULWB\n\t"
       "smull %0, %1, %2, %3\n\t"
       : "=&r"(rd_lo), "=&r"(rd_hi)
-      : "%r"(a), "r"(b<<16)
+      : "%r"(a), "r"(SAFE_SHL(b,16))
   );
   return rd_hi;
 }
@@ -80,7 +85,7 @@ static OPUS_INLINE opus_int32 silk_SMULWW_armv4(opus_int32 a, opus_int32 b)
     : "=&r"(rd_lo), "=&r"(rd_hi)
     : "%r"(a), "r"(b)
   );
-  return (rd_hi<<16)+(rd_lo>>16);
+  return SAFE_SHL(rd_hi,16)+(rd_lo>>16);
 }
 #define silk_SMULWW(a, b) (silk_SMULWW_armv4(a, b))
 
@@ -96,8 +101,10 @@ static OPUS_INLINE opus_int32 silk_SMLAWW_armv4(opus_int32 a, opus_int32 b,
     : "=&r"(rd_lo), "=&r"(rd_hi)
     : "%r"(b), "r"(c)
   );
-  return a+(rd_hi<<16)+(rd_lo>>16);
+  return a+SAFE_SHL(rd_hi,16)+(rd_lo>>16);
 }
 #define silk_SMLAWW(a, b, c) (silk_SMLAWW_armv4(a, b, c))
 
+#undef SAFE_SHL
+
 #endif /* SILK_MACROS_ARMv4_H */
index aad4117..b14ec65 100644 (file)
@@ -29,6 +29,11 @@ POSSIBILITY OF SUCH DAMAGE.
 #ifndef SILK_MACROS_ARMv5E_H
 #define SILK_MACROS_ARMv5E_H
 
+/* This macro only avoids the undefined behaviour from a left shift of
+   a negative value. It should only be used in macros that can't include
+   SigProc_FIX.h. In other cases, use silk_LSHIFT32(). */
+#define SAFE_SHL(a,b) ((opus_int32)((opus_uint32)(a) << (b)))
+
 /* (a32 * (opus_int32)((opus_int16)(b32))) >> 16 output have to be 32bit int */
 #undef silk_SMULWB
 static OPUS_INLINE opus_int32 silk_SMULWB_armv5e(opus_int32 a, opus_int16 b)
@@ -190,7 +195,7 @@ static OPUS_INLINE opus_int32 silk_CLZ16_armv5(opus_int16 in16)
       "#silk_CLZ16\n\t"
       "clz %0, %1;\n"
       : "=r"(res)
-      : "r"(in16<<16|0x8000)
+      : "r"(SAFE_SHL(in16,16)|0x8000)
   );
   return res;
 }
@@ -210,4 +215,6 @@ static OPUS_INLINE opus_int32 silk_CLZ32_armv5(opus_int32 in32)
 }
 #define silk_CLZ32(in32) (silk_CLZ32_armv5(in32))
 
+#undef SAFE_SHL
+
 #endif /* SILK_MACROS_ARMv5E_H */