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

[Laurent Vivier <laurent@vivier.eu>]

Le 09/10/2018 à 18:18, Peter Maydell a écrit :
> 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. Unfortunately
> _Pragma() support in gcc is broken in some gcc versions and
> in some usage contexts, so we limit the pragma usage here to clang.
> 
> 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 <peter.maydell@linaro.org>
> ---
> Changes v1->v2: _Pragma() in gcc appears to be a disaster area;
> limit the use of it to clang only, since it's just clang that
> emits the bogus warning in this case. Tested on clang-3.8.0,
> clang-7, gcc 5.4.0 and gcc 8.0.1.
> 
>  linux-user/qemu.h | 70 ++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 51 insertions(+), 19 deletions(-)
> 
> diff --git a/linux-user/qemu.h b/linux-user/qemu.h
> index b4959e41c6e..1beb6a2cfc4 100644
> --- a/linux-user/qemu.h
> +++ b/linux-user/qemu.h
> @@ -461,27 +461,59 @@ 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.
> + * - gcc has bugs in its _Pragma() support in some versions, eg
> + *   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83256 -- so we only
> + *   include the warning-suppression pragmas for clang
> + */
> +#ifdef __clang__
> +#define PRAGMA_DISABLE_PACKED_WARNING                                   \
> +    _Pragma("GCC diagnostic push");                                     \
> +    _Pragma("GCC diagnostic ignored \"-Wpragmas\"");                    \

Do we really need this pragma now we don't build this for gcc?
I understood it was for older gcc, not older llvm.

Thanks,
Laurent