simple_idct_template: Fix strict aliasing violation

Message ID 1462733840-70544-1-git-send-email-martin@martin.st
State Committed
Headers show

Commit Message

Martin Storsjö May 8, 2016, 6:57 p.m.
From: Michael Niedermayer <michael@niedermayer.cc>

This fixes fate-wmv8-intrax8.
---
 libavcodec/simple_idct_template.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

Comments

Vittorio Giovara May 9, 2016, 4:13 p.m. | #1
On Sun, May 8, 2016 at 2:57 PM, Martin Storsjö <martin@martin.st> wrote:
> From: Michael Niedermayer <michael@niedermayer.cc>
>
> This fixes fate-wmv8-intrax8.

*on gcc 4.4.

> ---
>  libavcodec/simple_idct_template.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/libavcodec/simple_idct_template.c b/libavcodec/simple_idct_template.c
> index b287c4f..28afb77 100644
> --- a/libavcodec/simple_idct_template.c
> +++ b/libavcodec/simple_idct_template.c
> @@ -93,7 +93,7 @@ static inline void FUNC(idctRowCondDC)(int16_t *row, int extra_shift)
>
>  #if HAVE_FAST_64BIT
>  #define ROW0_MASK (0xffffLL << 48 * HAVE_BIGENDIAN)
> -    if (((((uint64_t *)row)[0] & ~ROW0_MASK) | ((uint64_t *)row)[1]) == 0) {
> +    if (((AV_RN64A(row) & ~ROW0_MASK) | AV_RN64A(row+4)) == 0) {
>          uint64_t temp;
>          if (DC_SHIFT - extra_shift > 0) {
>              temp = (row[0] << (DC_SHIFT - extra_shift)) & 0xffff;
> @@ -102,14 +102,14 @@ static inline void FUNC(idctRowCondDC)(int16_t *row, int extra_shift)
>          }
>          temp += temp << 16;
>          temp += temp << 32;
> -        ((uint64_t *)row)[0] = temp;
> -        ((uint64_t *)row)[1] = temp;
> +        AV_WN64A(row, temp);
> +        AV_WN64A(row + 4, temp);
>          return;
>      }
>  #else
> -    if (!(((uint32_t*)row)[1] |
> -          ((uint32_t*)row)[2] |
> -          ((uint32_t*)row)[3] |
> +    if (!(AV_RN32A(row+2) |
> +          AV_RN32A(row+4) |
> +          AV_RN32A(row+6) |
>            row[1])) {
>          uint32_t temp;
>          if (DC_SHIFT - extra_shift > 0) {
> @@ -118,8 +118,10 @@ static inline void FUNC(idctRowCondDC)(int16_t *row, int extra_shift)
>              temp = (row[0] >> (extra_shift - DC_SHIFT)) & 0xffff;
>          }
>          temp += temp << 16;
> -        ((uint32_t*)row)[0]=((uint32_t*)row)[1] =
> -            ((uint32_t*)row)[2]=((uint32_t*)row)[3] = temp;
> +        AV_WN32A(row, temp);
> +        AV_WN32A(row+2, temp);
> +        AV_WN32A(row+4, temp);
> +        AV_WN32A(row+6, temp);
>          return;
>      }
>  #endif
> --

Seems fine, thanks for looking into this.
If you want, I can adjust the spaces around + myself.
Derek Buitenhuis May 9, 2016, 4:15 p.m. | #2
On 5/9/2016 5:13 PM, Vittorio Giovara wrote:
> *on gcc 4.4.

On all compilers. 4.4 just happened to hit it.

- Derek
Vittorio Giovara May 9, 2016, 4:24 p.m. | #3
On Mon, May 9, 2016 at 12:15 PM, Derek Buitenhuis
<derek.buitenhuis@gmail.com> wrote:
> On 5/9/2016 5:13 PM, Vittorio Giovara wrote:
>> *on gcc 4.4.
>
> On all compilers. 4.4 just happened to hit it.

still relevant to mention it imho
Diego Biurrun May 9, 2016, 4:41 p.m. | #4
On Sun, May 08, 2016 at 09:57:20PM +0300, Martin Storsjö wrote:
> From: Michael Niedermayer <michael@niedermayer.cc>
> 
> This fixes fate-wmv8-intrax8.
> ---
>  libavcodec/simple_idct_template.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)

Nice, I'll fix up the whitespace.

Diego

Patch

diff --git a/libavcodec/simple_idct_template.c b/libavcodec/simple_idct_template.c
index b287c4f..28afb77 100644
--- a/libavcodec/simple_idct_template.c
+++ b/libavcodec/simple_idct_template.c
@@ -93,7 +93,7 @@  static inline void FUNC(idctRowCondDC)(int16_t *row, int extra_shift)
 
 #if HAVE_FAST_64BIT
 #define ROW0_MASK (0xffffLL << 48 * HAVE_BIGENDIAN)
-    if (((((uint64_t *)row)[0] & ~ROW0_MASK) | ((uint64_t *)row)[1]) == 0) {
+    if (((AV_RN64A(row) & ~ROW0_MASK) | AV_RN64A(row+4)) == 0) {
         uint64_t temp;
         if (DC_SHIFT - extra_shift > 0) {
             temp = (row[0] << (DC_SHIFT - extra_shift)) & 0xffff;
@@ -102,14 +102,14 @@  static inline void FUNC(idctRowCondDC)(int16_t *row, int extra_shift)
         }
         temp += temp << 16;
         temp += temp << 32;
-        ((uint64_t *)row)[0] = temp;
-        ((uint64_t *)row)[1] = temp;
+        AV_WN64A(row, temp);
+        AV_WN64A(row + 4, temp);
         return;
     }
 #else
-    if (!(((uint32_t*)row)[1] |
-          ((uint32_t*)row)[2] |
-          ((uint32_t*)row)[3] |
+    if (!(AV_RN32A(row+2) |
+          AV_RN32A(row+4) |
+          AV_RN32A(row+6) |
           row[1])) {
         uint32_t temp;
         if (DC_SHIFT - extra_shift > 0) {
@@ -118,8 +118,10 @@  static inline void FUNC(idctRowCondDC)(int16_t *row, int extra_shift)
             temp = (row[0] >> (extra_shift - DC_SHIFT)) & 0xffff;
         }
         temp += temp << 16;
-        ((uint32_t*)row)[0]=((uint32_t*)row)[1] =
-            ((uint32_t*)row)[2]=((uint32_t*)row)[3] = temp;
+        AV_WN32A(row, temp);
+        AV_WN32A(row+2, temp);
+        AV_WN32A(row+4, temp);
+        AV_WN32A(row+6, temp);
         return;
     }
 #endif