[FFmpeg-cvslog] avcodec/fate-idct8x8: Defined behavior: eliminate negative left-shifts.

Katerina Barone-Adesi git at videolan.org
Tue May 20 04:34:39 CEST 2014


ffmpeg | branch: master | Katerina Barone-Adesi <katerinab at gmail.com> | Mon May 19 17:27:52 2014 +0200| [e50ae60d46181814245f70b61b70394311e10373] | committer: Michael Niedermayer

avcodec/fate-idct8x8: Defined behavior: eliminate negative left-shifts.

The original code left-shifts negative values, which is undefined
in the C99 specification (the one used during normal FFmpeg compilation).
This change multiplies by (1 << shift), which is functionally equivalent,
but has defined behavior.
With this change, fate-idct8x8 compiled with --fsanitize=undefined works.

Bug-Id: 686

Signed-off-by: Michael Niedermayer <michaelni at gmx.at>

The assembly generated by gcc is unchanged by this commit

> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=e50ae60d46181814245f70b61b70394311e10373
---

 libavcodec/jfdctint_template.c    |    4 ++--
 libavcodec/jrevdct.c              |   36 ++++++++++++++++++------------------
 libavcodec/simple_idct_template.c |   10 +++++-----
 3 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/libavcodec/jfdctint_template.c b/libavcodec/jfdctint_template.c
index c6a1638..3ea2f5d 100644
--- a/libavcodec/jfdctint_template.c
+++ b/libavcodec/jfdctint_template.c
@@ -216,8 +216,8 @@ static av_always_inline void FUNC(row_fdct)(int16_t *data)
     tmp11 = tmp1 + tmp2;
     tmp12 = tmp1 - tmp2;
 
-    dataptr[0] = (int16_t) ((tmp10 + tmp11) << PASS1_BITS);
-    dataptr[4] = (int16_t) ((tmp10 - tmp11) << PASS1_BITS);
+    dataptr[0] = (int16_t) ((tmp10 + tmp11) * (1 << PASS1_BITS));
+    dataptr[4] = (int16_t) ((tmp10 - tmp11) * (1 << PASS1_BITS));
 
     z1 = MULTIPLY(tmp12 + tmp13, FIX_0_541196100);
     dataptr[2] = (int16_t) DESCALE(z1 + MULTIPLY(tmp13, FIX_0_765366865),
diff --git a/libavcodec/jrevdct.c b/libavcodec/jrevdct.c
index 91780b2..d4e6571 100644
--- a/libavcodec/jrevdct.c
+++ b/libavcodec/jrevdct.c
@@ -249,8 +249,8 @@ void ff_j_rev_dct(DCTBLOCK data)
       /* AC terms all zero */
       if (d0) {
           /* Compute a 32 bit value to assign. */
-          int16_t dcval = (int16_t) (d0 << PASS1_BITS);
-          register int v = (dcval & 0xffff) | ((dcval << 16) & 0xffff0000);
+          int16_t dcval = (int16_t) (d0 * (1 << PASS1_BITS));
+          register int v = (dcval & 0xffff) | ((dcval * (1 << 16)) & 0xffff0000);
 
           idataptr[0] = v;
           idataptr[1] = v;
@@ -272,8 +272,8 @@ void ff_j_rev_dct(DCTBLOCK data)
                     tmp2 = z1 + MULTIPLY(-d6, FIX_1_847759065);
                     tmp3 = z1 + MULTIPLY(d2, FIX_0_765366865);
 
-                    tmp0 = (d0 + d4) << CONST_BITS;
-                    tmp1 = (d0 - d4) << CONST_BITS;
+                    tmp0 = (d0 + d4) * CONST_SCALE;
+                    tmp1 = (d0 - d4) * CONST_SCALE;
 
                     tmp10 = tmp0 + tmp3;
                     tmp13 = tmp0 - tmp3;
@@ -284,8 +284,8 @@ void ff_j_rev_dct(DCTBLOCK data)
                     tmp2 = MULTIPLY(-d6, FIX_1_306562965);
                     tmp3 = MULTIPLY(d6, FIX_0_541196100);
 
-                    tmp0 = (d0 + d4) << CONST_BITS;
-                    tmp1 = (d0 - d4) << CONST_BITS;
+                    tmp0 = (d0 + d4) * CONST_SCALE;
+                    tmp1 = (d0 - d4) * CONST_SCALE;
 
                     tmp10 = tmp0 + tmp3;
                     tmp13 = tmp0 - tmp3;
@@ -298,8 +298,8 @@ void ff_j_rev_dct(DCTBLOCK data)
                     tmp2 = MULTIPLY(d2, FIX_0_541196100);
                     tmp3 = MULTIPLY(d2, FIX_1_306562965);
 
-                    tmp0 = (d0 + d4) << CONST_BITS;
-                    tmp1 = (d0 - d4) << CONST_BITS;
+                    tmp0 = (d0 + d4) * CONST_SCALE;
+                    tmp1 = (d0 - d4) * CONST_SCALE;
 
                     tmp10 = tmp0 + tmp3;
                     tmp13 = tmp0 - tmp3;
@@ -307,8 +307,8 @@ void ff_j_rev_dct(DCTBLOCK data)
                     tmp12 = tmp1 - tmp2;
             } else {
                     /* d0 != 0, d2 == 0, d4 != 0, d6 == 0 */
-                    tmp10 = tmp13 = (d0 + d4) << CONST_BITS;
-                    tmp11 = tmp12 = (d0 - d4) << CONST_BITS;
+                    tmp10 = tmp13 = (d0 + d4) * CONST_SCALE;
+                    tmp11 = tmp12 = (d0 - d4) * CONST_SCALE;
             }
       }
 
@@ -618,8 +618,8 @@ void ff_j_rev_dct(DCTBLOCK data)
                     tmp2 = z1 + MULTIPLY(-d6, FIX_1_847759065);
                     tmp3 = z1 + MULTIPLY(d2, FIX_0_765366865);
 
-                    tmp0 = (d0 + d4) << CONST_BITS;
-                    tmp1 = (d0 - d4) << CONST_BITS;
+                    tmp0 = (d0 + d4) * CONST_SCALE;
+                    tmp1 = (d0 - d4) * CONST_SCALE;
 
                     tmp10 = tmp0 + tmp3;
                     tmp13 = tmp0 - tmp3;
@@ -630,8 +630,8 @@ void ff_j_rev_dct(DCTBLOCK data)
                     tmp2 = MULTIPLY(-d6, FIX_1_306562965);
                     tmp3 = MULTIPLY(d6, FIX_0_541196100);
 
-                    tmp0 = (d0 + d4) << CONST_BITS;
-                    tmp1 = (d0 - d4) << CONST_BITS;
+                    tmp0 = (d0 + d4) * CONST_SCALE;
+                    tmp1 = (d0 - d4) * CONST_SCALE;
 
                     tmp10 = tmp0 + tmp3;
                     tmp13 = tmp0 - tmp3;
@@ -644,8 +644,8 @@ void ff_j_rev_dct(DCTBLOCK data)
                     tmp2 = MULTIPLY(d2, FIX_0_541196100);
                     tmp3 = MULTIPLY(d2, FIX_1_306562965);
 
-                    tmp0 = (d0 + d4) << CONST_BITS;
-                    tmp1 = (d0 - d4) << CONST_BITS;
+                    tmp0 = (d0 + d4) * CONST_SCALE;
+                    tmp1 = (d0 - d4) * CONST_SCALE;
 
                     tmp10 = tmp0 + tmp3;
                     tmp13 = tmp0 - tmp3;
@@ -653,8 +653,8 @@ void ff_j_rev_dct(DCTBLOCK data)
                     tmp12 = tmp1 - tmp2;
             } else {
                     /* d0 != 0, d2 == 0, d4 != 0, d6 == 0 */
-                    tmp10 = tmp13 = (d0 + d4) << CONST_BITS;
-                    tmp11 = tmp12 = (d0 - d4) << CONST_BITS;
+                    tmp10 = tmp13 = (d0 + d4) * CONST_SCALE;
+                    tmp11 = tmp12 = (d0 - d4) * CONST_SCALE;
             }
     }
 
diff --git a/libavcodec/simple_idct_template.c b/libavcodec/simple_idct_template.c
index 95844a2..789db8d 100644
--- a/libavcodec/simple_idct_template.c
+++ b/libavcodec/simple_idct_template.c
@@ -110,12 +110,12 @@ static inline void FUNC(idctRowCondDC)(int16_t *row, int extra_shift)
     if (((((uint64_t *)row)[0] & ~ROW0_MASK) | ((uint64_t *)row)[1]) == 0) {
         uint64_t temp;
         if (DC_SHIFT - extra_shift >= 0) {
-            temp = (row[0] << (DC_SHIFT - extra_shift)) & 0xffff;
+            temp = (row[0] * (1 << (DC_SHIFT - extra_shift))) & 0xffff;
         } else {
             temp = ((row[0] + (1<<(extra_shift - DC_SHIFT-1))) >> (extra_shift - DC_SHIFT)) & 0xffff;
         }
-        temp += temp << 16;
-        temp += temp << 32;
+        temp += temp * (1 << 16);
+        temp += temp * ((uint64_t) 1 << 32);
         ((uint64_t *)row)[0] = temp;
         ((uint64_t *)row)[1] = temp;
         return;
@@ -127,11 +127,11 @@ static inline void FUNC(idctRowCondDC)(int16_t *row, int extra_shift)
           row[1])) {
         uint32_t temp;
         if (DC_SHIFT - extra_shift >= 0) {
-            temp = (row[0] << (DC_SHIFT - extra_shift)) & 0xffff;
+            temp = (row[0] * (1 << (DC_SHIFT - extra_shift))) & 0xffff;
         } else {
             temp = ((row[0] + (1<<(extra_shift - DC_SHIFT-1))) >> (extra_shift - DC_SHIFT)) & 0xffff;
         }
-        temp += temp << 16;
+        temp += temp * (1 << 16);
         ((uint32_t*)row)[0]=((uint32_t*)row)[1] =
             ((uint32_t*)row)[2]=((uint32_t*)row)[3] = temp;
         return;



More information about the ffmpeg-cvslog mailing list