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

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

[remi]

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));
     }
 }
 
-- 
2.37.2

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

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

[Andreas Rheinhardt]

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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

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

[Rémi Denis-Courmont]

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 uses av_float2int() / av_int2float() instead.
---
 libavcodec/audiodsp.c | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/libavcodec/audiodsp.c b/libavcodec/audiodsp.c
index eba6e809fd..c5427d3535 100644
--- a/libavcodec/audiodsp.c
+++ b/libavcodec/audiodsp.c
@@ -22,36 +22,36 @@
 #include "libavutil/common.h"
 #include "audiodsp.h"
 
-static inline uint32_t clipf_c_one(uint32_t a, uint32_t mini,
-                                   uint32_t maxi, uint32_t maxisign)
+static inline float clipf_c_one(float a, uint32_t mini,
+                                uint32_t maxi, uint32_t maxisign)
 {
-    if (a > mini)
-        return mini;
-    else if ((a ^ (1U << 31)) > maxisign)
-        return maxi;
+    uint32_t ai = av_float2int(a);
+
+    if (ai > mini)
+        return av_int2float(mini);
+    else if ((ai ^ (1U << 31)) > maxisign)
+        return av_int2float(maxi);
     else
         return a;
 }
 
 static void vector_clipf_c_opposite_sign(float *dst, const float *src,
-                                         float *min, float *max, int len)
+                                         float min, float max, int len)
 {
     int i;
-    uint32_t mini        = *(uint32_t *) min;
-    uint32_t maxi        = *(uint32_t *) max;
+    uint32_t mini        = av_float2int(min);
+    uint32_t maxi        = av_float2int(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] = clipf_c_one(src[i + 0], mini, maxi, maxisign);
+        dst[i + 1] = clipf_c_one(src[i + 1], mini, maxi, maxisign);
+        dst[i + 2] = clipf_c_one(src[i + 2], mini, maxi, maxisign);
+        dst[i + 3] = clipf_c_one(src[i + 3], mini, maxi, maxisign);
+        dst[i + 4] = clipf_c_one(src[i + 4], mini, maxi, maxisign);
+        dst[i + 5] = clipf_c_one(src[i + 5], mini, maxi, maxisign);
+        dst[i + 6] = clipf_c_one(src[i + 6], mini, maxi, maxisign);
+        dst[i + 7] = clipf_c_one(src[i + 7], mini, maxi, maxisign);
     }
 }
 
@@ -61,7 +61,7 @@ static void vector_clipf_c(float *dst, const float *src, int len,
     int i;
 
     if (min < 0 && max > 0) {
-        vector_clipf_c_opposite_sign(dst, src, &min, &max, len);
+        vector_clipf_c_opposite_sign(dst, src, min, max, len);
     } else {
         for (i = 0; i < len; i += 8) {
             dst[i]     = av_clipf(src[i], min, max);
-- 
2.40.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".