Re: [Qemu-devel] [PATCH] linux-user: Suppress address-of-packed-member warnings in __get/put_user_e

[Liam Merwick]

On 28/09/18 15:25, Peter Maydell wrote:
> Our __get_user_e() and __put_user_e() macros cause newer versions
> of clang to generate false-positive -Waddress-of-packed-member
> warnings if they are passed the address of a member of a packed
> struct (see https://bugs.llvm.org/show_bug.cgi?id=39113).
> Suppress these using the _Pragma() operator.
>
> To put in the pragmas we need to convert the macros from
> expressions to statements, but all the callsites effectively
> treat them as statements already so this is OK.
>
> Signed-off-by: Peter Maydell <address@hidden>

Reviewed-by: Liam Merwick <address@hidden>

> ---
>   linux-user/qemu.h | 57 +++++++++++++++++++++++++++++++----------------
>   1 file changed, 38 insertions(+), 19 deletions(-)
>
> diff --git a/linux-user/qemu.h b/linux-user/qemu.h
> index b4959e41c6e..56c4f2d5374 100644
> --- a/linux-user/qemu.h
> +++ b/linux-user/qemu.h
> @@ -461,27 +461,46 @@ static inline int access_ok(int type, abi_ulong addr, 
> abi_ulong size)
>      These are usually used to access struct data members once the struct has
>      been locked - usually with lock_user_struct.  */
>   
> -/* Tricky points:
> -   - Use __builtin_choose_expr to avoid type promotion from ?:,
> -   - Invalid sizes result in a compile time error stemming from
> -     the fact that abort has no parameters.
> -   - It's easier to use the endian-specific unaligned load/store
> -     functions than host-endian unaligned load/store plus tswapN.  */
> +/*
> + * Tricky points:
> + * - Use __builtin_choose_expr to avoid type promotion from ?:,
> + * - Invalid sizes result in a compile time error stemming from
> + *   the fact that abort has no parameters.
> + * - It's easier to use the endian-specific unaligned load/store
> + *   functions than host-endian unaligned load/store plus tswapN.
> + * - The pragmas are necessary only to silence a clang false-positive
> + *   warning: see https://bugs.llvm.org/show_bug.cgi?id=39113 .
> + * - We have to disable -Wpragmas warnings to avoid a complaint about
> + *   an unknown warning type from older compilers that don't know about
> + *   -Waddress-of-packed-member.
> + */
> +#define __put_user_e(x, hptr, e)                                            \
> +    do {                                                                    \
> +        _Pragma("GCC diagnostic push");                                     \
> +        _Pragma("GCC diagnostic ignored \"-Wpragmas\"");                    \
> +        _Pragma("GCC diagnostic ignored \"-Waddress-of-packed-member\"");   \
> +        (__builtin_choose_expr(sizeof(*(hptr)) == 1, stb_p,                 \
> +        __builtin_choose_expr(sizeof(*(hptr)) == 2, stw_##e##_p,            \
> +        __builtin_choose_expr(sizeof(*(hptr)) == 4, stl_##e##_p,            \
> +        __builtin_choose_expr(sizeof(*(hptr)) == 8, stq_##e##_p, abort))))  \
> +            ((hptr), (x)), (void)0);                                        \
> +        _Pragma("GCC diagnostic pop");                                      \
> +    } while (0)
>   
> -#define __put_user_e(x, hptr, e)                                        \
> -  (__builtin_choose_expr(sizeof(*(hptr)) == 1, stb_p,                   \
> -   __builtin_choose_expr(sizeof(*(hptr)) == 2, stw_##e##_p,             \
> -   __builtin_choose_expr(sizeof(*(hptr)) == 4, stl_##e##_p,             \
> -   __builtin_choose_expr(sizeof(*(hptr)) == 8, stq_##e##_p, abort))))   \
> -     ((hptr), (x)), (void)0)
> +#define __get_user_e(x, hptr, e)                                            \
> +    do {                                                                    \
> +        _Pragma("GCC diagnostic push");                                     \
> +        _Pragma("GCC diagnostic ignored \"-Wpragmas\"");                    \
> +        _Pragma("GCC diagnostic ignored \"-Waddress-of-packed-member\"");   \
> +        ((x) = (typeof(*hptr))(                                             \
> +        __builtin_choose_expr(sizeof(*(hptr)) == 1, ldub_p,                 \
> +        __builtin_choose_expr(sizeof(*(hptr)) == 2, lduw_##e##_p,           \
> +        __builtin_choose_expr(sizeof(*(hptr)) == 4, ldl_##e##_p,            \
> +        __builtin_choose_expr(sizeof(*(hptr)) == 8, ldq_##e##_p, abort))))  \
> +            (hptr)), (void)0);                                              \
> +        _Pragma("GCC diagnostic pop");                                      \
> +    } while (0)
>   
> -#define __get_user_e(x, hptr, e)                                        \
> -  ((x) = (typeof(*hptr))(                                               \
> -   __builtin_choose_expr(sizeof(*(hptr)) == 1, ldub_p,                  \
> -   __builtin_choose_expr(sizeof(*(hptr)) == 2, lduw_##e##_p,            \
> -   __builtin_choose_expr(sizeof(*(hptr)) == 4, ldl_##e##_p,             \
> -   __builtin_choose_expr(sizeof(*(hptr)) == 8, ldq_##e##_p, abort))))   \
> -     (hptr)), (void)0)
>   
>   #ifdef TARGET_WORDS_BIGENDIAN
>   # define __put_user(x, hptr)  __put_user_e(x, hptr, be)