RE: [PATCH v14 1/2] eal: add __rte_may_alias to unaligned typedefs

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Scott Mitchell <scott.k.mitch1@gmail.com>
> 
> Add __rte_may_alias attribute to unaligned_uint{16,32,64}_t typedefs
> to prevent GCC strict-aliasing optimization bugs. GCC has a bug where
> it incorrectly elides struct initialization when strict aliasing is
> enabled, causing reads from uninitialized memory.
> 
> The __rte_may_alias attribute signals to the compiler that these types
> can alias other types, preventing the incorrect optimization.

I'm wondering if this is the right place to add __rte_may_alias, i.e. if the scope of the workaround is correct.

Are the unaligned_uintNN_t types only used in a way where they are affected by the GCC bug?
If not, adding __rte_may_alias to the types themselves may be too broad.

Does the GCC bug only affect the unaligned_uintNN_t types?
Or does it occur elsewhere or for other types too? Then this workaround only solves the problem for parts of the code.

Minor detail:
If the bug only occurs on GCC, not Clang, please make the workaround GCC-only, using the preprocessor.

> 
> Signed-off-by: Scott Mitchell <scott.k.mitch1@gmail.com>
> ---
>  lib/eal/include/rte_common.h | 34 +++++++++++++++++++---------------
>  1 file changed, 19 insertions(+), 15 deletions(-)
> 
> diff --git a/lib/eal/include/rte_common.h
> b/lib/eal/include/rte_common.h
> index 9e7d84f929..ac70270cfb 100644
> --- a/lib/eal/include/rte_common.h
> +++ b/lib/eal/include/rte_common.h
> @@ -121,14 +121,27 @@ extern "C" {
>  #define __rte_aligned(a) __attribute__((__aligned__(a)))
>  #endif
> 
> +/**
> + * Macro to mark a type that is not subject to type-based aliasing
> rules
> + */
> +#ifdef RTE_TOOLCHAIN_MSVC
> +#define __rte_may_alias
> +#else
> +#define __rte_may_alias __attribute__((__may_alias__))
> +#endif
> +
> +/**
> + * __rte_may_alias avoids compiler bugs (GCC) that elide
> initialization
> + * of memory when strict-aliasing is enabled.
> + */
>  #ifdef RTE_ARCH_STRICT_ALIGN
> -typedef uint64_t unaligned_uint64_t __rte_aligned(1);
> -typedef uint32_t unaligned_uint32_t __rte_aligned(1);
> -typedef uint16_t unaligned_uint16_t __rte_aligned(1);
> +typedef uint64_t unaligned_uint64_t __rte_may_alias __rte_aligned(1);
> +typedef uint32_t unaligned_uint32_t __rte_may_alias __rte_aligned(1);
> +typedef uint16_t unaligned_uint16_t __rte_may_alias __rte_aligned(1);
>  #else
> -typedef uint64_t unaligned_uint64_t;
> -typedef uint32_t unaligned_uint32_t;
> -typedef uint16_t unaligned_uint16_t;
> +typedef uint64_t unaligned_uint64_t __rte_may_alias;
> +typedef uint32_t unaligned_uint32_t __rte_may_alias;
> +typedef uint16_t unaligned_uint16_t __rte_may_alias;
>  #endif
> 
>  /**
> @@ -159,15 +172,6 @@ typedef uint16_t unaligned_uint16_t;
>  #define __rte_packed_end __attribute__((__packed__))
>  #endif
> 
> -/**
> - * Macro to mark a type that is not subject to type-based aliasing
> rules
> - */
> -#ifdef RTE_TOOLCHAIN_MSVC
> -#define __rte_may_alias
> -#else
> -#define __rte_may_alias __attribute__((__may_alias__))
> -#endif
> -
>  /******* Macro to mark functions and fields scheduled for removal
> *****/
>  #ifdef RTE_TOOLCHAIN_MSVC
>  #define __rte_deprecated
> --
> 2.39.5 (Apple Git-154)