* [Qemu-devel] [PATCH v2] linux-user: Suppress address-of-packed-member warnings in __get/put_user_e
@ 2018-10-09 16:18 Peter Maydell
2018-10-09 16:22 ` Laurent Vivier
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Peter Maydell @ 2018-10-09 16:18 UTC (permalink / raw)
To: qemu-devel; +Cc: patches, Laurent Vivier, Riku Voipio
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\""); \
+ _Pragma("GCC diagnostic ignored \"-Waddress-of-packed-member\"")
-#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 PRAGMA_REENABLE_PACKED_WARNING \
+ _Pragma("GCC diagnostic pop")
+
+#else
+#define PRAGMA_DISABLE_PACKED_WARNING
+#define PRAGMA_REENABLE_PACKED_WARNING
+#endif
+
+#define __put_user_e(x, hptr, e) \
+ do { \
+ PRAGMA_DISABLE_PACKED_WARNING; \
+ (__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_REENABLE_PACKED_WARNING; \
+ } while (0)
+
+#define __get_user_e(x, hptr, e) \
+ do { \
+ PRAGMA_DISABLE_PACKED_WARNING; \
+ ((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_REENABLE_PACKED_WARNING; \
+ } 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)
--
2.19.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH v2] linux-user: Suppress address-of-packed-member warnings in __get/put_user_e
2018-10-09 16:18 [Qemu-devel] [PATCH v2] linux-user: Suppress address-of-packed-member warnings in __get/put_user_e Peter Maydell
@ 2018-10-09 16:22 ` Laurent Vivier
2018-10-09 16:24 ` Peter Maydell
2018-10-09 17:01 ` Laurent Vivier
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Laurent Vivier @ 2018-10-09 16:22 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: patches, Riku Voipio
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH v2] linux-user: Suppress address-of-packed-member warnings in __get/put_user_e
2018-10-09 16:22 ` Laurent Vivier
@ 2018-10-09 16:24 ` Peter Maydell
0 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2018-10-09 16:24 UTC (permalink / raw)
To: Laurent Vivier; +Cc: QEMU Developers, patches@linaro.org, Riku Voipio
On 9 October 2018 at 17:22, Laurent Vivier <laurent@vivier.eu> wrote:
> Le 09/10/2018 à 18:18, Peter Maydell a écrit :
>> +#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.
Yes, it's necessary. Older versions of clang don't recognize
-Waddress-of-packed-member.
thanks
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] linux-user: Suppress address-of-packed-member warnings in __get/put_user_e
2018-10-09 16:18 [Qemu-devel] [PATCH v2] linux-user: Suppress address-of-packed-member warnings in __get/put_user_e Peter Maydell
2018-10-09 16:22 ` Laurent Vivier
@ 2018-10-09 17:01 ` Laurent Vivier
2018-10-09 17:41 ` Richard Henderson
2018-10-12 18:33 ` Laurent Vivier
3 siblings, 0 replies; 6+ messages in thread
From: Laurent Vivier @ 2018-10-09 17:01 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: patches, Riku Voipio
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\""); \
> + _Pragma("GCC diagnostic ignored \"-Waddress-of-packed-member\"")
>
> -#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 PRAGMA_REENABLE_PACKED_WARNING \
> + _Pragma("GCC diagnostic pop")
> +
> +#else
> +#define PRAGMA_DISABLE_PACKED_WARNING
> +#define PRAGMA_REENABLE_PACKED_WARNING
> +#endif
> +
> +#define __put_user_e(x, hptr, e) \
> + do { \
> + PRAGMA_DISABLE_PACKED_WARNING; \
> + (__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_REENABLE_PACKED_WARNING; \
> + } while (0)
> +
> +#define __get_user_e(x, hptr, e) \
> + do { \
> + PRAGMA_DISABLE_PACKED_WARNING; \
> + ((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_REENABLE_PACKED_WARNING; \
> + } 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)
>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH v2] linux-user: Suppress address-of-packed-member warnings in __get/put_user_e
2018-10-09 16:18 [Qemu-devel] [PATCH v2] linux-user: Suppress address-of-packed-member warnings in __get/put_user_e Peter Maydell
2018-10-09 16:22 ` Laurent Vivier
2018-10-09 17:01 ` Laurent Vivier
@ 2018-10-09 17:41 ` Richard Henderson
2018-10-12 18:33 ` Laurent Vivier
3 siblings, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2018-10-09 17:41 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Riku Voipio, Laurent Vivier, patches
On 10/9/18 9:18 AM, 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. 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(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] linux-user: Suppress address-of-packed-member warnings in __get/put_user_e
2018-10-09 16:18 [Qemu-devel] [PATCH v2] linux-user: Suppress address-of-packed-member warnings in __get/put_user_e Peter Maydell
` (2 preceding siblings ...)
2018-10-09 17:41 ` Richard Henderson
@ 2018-10-12 18:33 ` Laurent Vivier
3 siblings, 0 replies; 6+ messages in thread
From: Laurent Vivier @ 2018-10-12 18:33 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: patches, Riku Voipio
On 09/10/2018 18:18, 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. 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(-)
>
Applied to my branch linux-user-for-3.1
Thanks,
Laurent
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-10-12 18:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-10-09 16:18 [Qemu-devel] [PATCH v2] linux-user: Suppress address-of-packed-member warnings in __get/put_user_e Peter Maydell
2018-10-09 16:22 ` Laurent Vivier
2018-10-09 16:24 ` Peter Maydell
2018-10-09 17:01 ` Laurent Vivier
2018-10-09 17:41 ` Richard Henderson
2018-10-12 18:33 ` Laurent Vivier
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).