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

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

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".