diff mbox series

[FFmpeg-devel] lavc/audiodsp: fix aliasing violation

Message ID 20220913160116.15158-1-remi@remlab.net
State New
Headers show
Series [FFmpeg-devel] lavc/audiodsp: fix aliasing violation | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Rémi Denis-Courmont Sept. 13, 2022, 4:01 p.m. UTC
From: Rémi Denis-Courmont <remi@remlab.net>

Even though they have the same size, and typically the same alignment,
uint32_t and float are under no circumstances compatible types in C.

The casts from float * to uint32_t * are invalid here. Insofar as the
resulting pointers are dereferenced, this is undefined behaviour.

This patch replaces the invalid cast with explicit type aliasing via
union. Fortunately, compilers are able to optimise the "conversion"
away and skip floating pointer registers here.
---
 libavcodec/audiodsp.c | 34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

Comments

Andreas Rheinhardt Sept. 13, 2022, 4:03 p.m. UTC | #1
remi@remlab.net:
> From: Rémi Denis-Courmont <remi@remlab.net>
> 
> Even though they have the same size, and typically the same alignment,
> uint32_t and float are under no circumstances compatible types in C.
> 
> The casts from float * to uint32_t * are invalid here. Insofar as the
> resulting pointers are dereferenced, this is undefined behaviour.
> 
> This patch replaces the invalid cast with explicit type aliasing via
> union. Fortunately, compilers are able to optimise the "conversion"
> away and skip floating pointer registers here.
> ---
>  libavcodec/audiodsp.c | 34 ++++++++++++++++++++++------------
>  1 file changed, 22 insertions(+), 12 deletions(-)
> 
> diff --git a/libavcodec/audiodsp.c b/libavcodec/audiodsp.c
> index ff43e87dce..1781ad8bc4 100644
> --- a/libavcodec/audiodsp.c
> +++ b/libavcodec/audiodsp.c
> @@ -22,6 +22,18 @@
>  #include "libavutil/common.h"
>  #include "audiodsp.h"
>  
> +static inline uint32_t pf2u(const float *f)
> +{
> +    union { uint32_t u; float f; } u = { .f = *f };
> +    return u.u;
> +}
> +
> +static inline float u2f(uint_fast32_t x)
> +{
> +    union { uint32_t u; float f; } u = { .u = x };
> +    return u.f;
> +}
> +
>  static inline uint32_t clipf_c_one(uint32_t a, uint32_t mini,
>                                     uint32_t maxi, uint32_t maxisign)
>  {
> @@ -37,21 +49,19 @@ static void vector_clipf_c_opposite_sign(float *dst, const float *src,
>                                           float *min, float *max, int len)
>  {
>      int i;
> -    uint32_t mini        = *(uint32_t *) min;
> -    uint32_t maxi        = *(uint32_t *) max;
> +    uint32_t mini        = pf2u(min);
> +    uint32_t maxi        = pf2u(max);
>      uint32_t maxisign    = maxi ^ (1U << 31);
> -    uint32_t *dsti       = (uint32_t *) dst;
> -    const uint32_t *srci = (const uint32_t *) src;
>  
>      for (i = 0; i < len; i += 8) {
> -        dsti[i + 0] = clipf_c_one(srci[i + 0], mini, maxi, maxisign);
> -        dsti[i + 1] = clipf_c_one(srci[i + 1], mini, maxi, maxisign);
> -        dsti[i + 2] = clipf_c_one(srci[i + 2], mini, maxi, maxisign);
> -        dsti[i + 3] = clipf_c_one(srci[i + 3], mini, maxi, maxisign);
> -        dsti[i + 4] = clipf_c_one(srci[i + 4], mini, maxi, maxisign);
> -        dsti[i + 5] = clipf_c_one(srci[i + 5], mini, maxi, maxisign);
> -        dsti[i + 6] = clipf_c_one(srci[i + 6], mini, maxi, maxisign);
> -        dsti[i + 7] = clipf_c_one(srci[i + 7], mini, maxi, maxisign);
> +        dst[i + 0] = u2f(clipf_c_one(pf2u(src + i + 0), mini, maxi, maxisign));
> +        dst[i + 1] = u2f(clipf_c_one(pf2u(src + i + 1), mini, maxi, maxisign));
> +        dst[i + 2] = u2f(clipf_c_one(pf2u(src + i + 2), mini, maxi, maxisign));
> +        dst[i + 3] = u2f(clipf_c_one(pf2u(src + i + 3), mini, maxi, maxisign));
> +        dst[i + 4] = u2f(clipf_c_one(pf2u(src + i + 4), mini, maxi, maxisign));
> +        dst[i + 5] = u2f(clipf_c_one(pf2u(src + i + 5), mini, maxi, maxisign));
> +        dst[i + 6] = u2f(clipf_c_one(pf2u(src + i + 6), mini, maxi, maxisign));
> +        dst[i + 7] = u2f(clipf_c_one(pf2u(src + i + 7), mini, maxi, maxisign));
>      }
>  }
>  

We already have av_float2int() in libavutil/intfloat.h.

- Andreas
diff mbox series

Patch

diff --git a/libavcodec/audiodsp.c b/libavcodec/audiodsp.c
index ff43e87dce..1781ad8bc4 100644
--- a/libavcodec/audiodsp.c
+++ b/libavcodec/audiodsp.c
@@ -22,6 +22,18 @@ 
 #include "libavutil/common.h"
 #include "audiodsp.h"
 
+static inline uint32_t pf2u(const float *f)
+{
+    union { uint32_t u; float f; } u = { .f = *f };
+    return u.u;
+}
+
+static inline float u2f(uint_fast32_t x)
+{
+    union { uint32_t u; float f; } u = { .u = x };
+    return u.f;
+}
+
 static inline uint32_t clipf_c_one(uint32_t a, uint32_t mini,
                                    uint32_t maxi, uint32_t maxisign)
 {
@@ -37,21 +49,19 @@  static void vector_clipf_c_opposite_sign(float *dst, const float *src,
                                          float *min, float *max, int len)
 {
     int i;
-    uint32_t mini        = *(uint32_t *) min;
-    uint32_t maxi        = *(uint32_t *) max;
+    uint32_t mini        = pf2u(min);
+    uint32_t maxi        = pf2u(max);
     uint32_t maxisign    = maxi ^ (1U << 31);
-    uint32_t *dsti       = (uint32_t *) dst;
-    const uint32_t *srci = (const uint32_t *) src;
 
     for (i = 0; i < len; i += 8) {
-        dsti[i + 0] = clipf_c_one(srci[i + 0], mini, maxi, maxisign);
-        dsti[i + 1] = clipf_c_one(srci[i + 1], mini, maxi, maxisign);
-        dsti[i + 2] = clipf_c_one(srci[i + 2], mini, maxi, maxisign);
-        dsti[i + 3] = clipf_c_one(srci[i + 3], mini, maxi, maxisign);
-        dsti[i + 4] = clipf_c_one(srci[i + 4], mini, maxi, maxisign);
-        dsti[i + 5] = clipf_c_one(srci[i + 5], mini, maxi, maxisign);
-        dsti[i + 6] = clipf_c_one(srci[i + 6], mini, maxi, maxisign);
-        dsti[i + 7] = clipf_c_one(srci[i + 7], mini, maxi, maxisign);
+        dst[i + 0] = u2f(clipf_c_one(pf2u(src + i + 0), mini, maxi, maxisign));
+        dst[i + 1] = u2f(clipf_c_one(pf2u(src + i + 1), mini, maxi, maxisign));
+        dst[i + 2] = u2f(clipf_c_one(pf2u(src + i + 2), mini, maxi, maxisign));
+        dst[i + 3] = u2f(clipf_c_one(pf2u(src + i + 3), mini, maxi, maxisign));
+        dst[i + 4] = u2f(clipf_c_one(pf2u(src + i + 4), mini, maxi, maxisign));
+        dst[i + 5] = u2f(clipf_c_one(pf2u(src + i + 5), mini, maxi, maxisign));
+        dst[i + 6] = u2f(clipf_c_one(pf2u(src + i + 6), mini, maxi, maxisign));
+        dst[i + 7] = u2f(clipf_c_one(pf2u(src + i + 7), mini, maxi, maxisign));
     }
 }