[PATCH v3] bsd-user: Move PRAGMA_DISABLE_PACKED_WARNING etc to qemu.h

Warner Losh posted 1 patch 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230830144743.53770-1-imp@bsdimp.com
Maintainers: Warner Losh <imp@bsdimp.com>, Kyle Evans <kevans@freebsd.org>
bsd-user/qemu.h         | 27 +++++++++++++++++++++++++++
include/qemu/compiler.h | 30 ------------------------------
2 files changed, 27 insertions(+), 30 deletions(-)
[PATCH v3] bsd-user: Move PRAGMA_DISABLE_PACKED_WARNING etc to qemu.h
Posted by Warner Losh 8 months ago
For the moment, move PRAGMA_DISABLE_PACKED_WARNING and
PRAGMA_ENABLE_PACKED_WARNING back to bsd-user/qemu.h.

Of course, these should be in compiler.h, but that interferes with too
many things at the moment, so take one step back to unbreak clang
linux-user builds first. Use the exact same version that's in
linux-user/qemu.h since that's what should be in compiler.h.
---
 bsd-user/qemu.h         | 27 +++++++++++++++++++++++++++
 include/qemu/compiler.h | 30 ------------------------------
 2 files changed, 27 insertions(+), 30 deletions(-)

diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
index 4cfd5c63371..d3158bc2edd 100644
--- a/bsd-user/qemu.h
+++ b/bsd-user/qemu.h
@@ -273,6 +273,33 @@ static inline bool 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.
+ * - The pragmas are necessary only to silence a clang false-positive
+ *   warning: see https://bugs.llvm.org/show_bug.cgi?id=39113 .
+ * - 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
+ */
+#if defined(__clang__) && __has_warning("-Waddress-of-packed-member")
+#define PRAGMA_DISABLE_PACKED_WARNING                                   \
+    _Pragma("GCC diagnostic push");                                     \
+    _Pragma("GCC diagnostic ignored \"-Waddress-of-packed-member\"")
+
+#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;                                      \
diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index b0374425180..a309f90c768 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -22,36 +22,6 @@
 #define QEMU_EXTERN_C extern
 #endif
 
-/*
- * 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 PRAGMA_REENABLE_PACKED_WARNING          \
-    _Pragma("GCC diagnostic pop")
-
-#else
-#define PRAGMA_DISABLE_PACKED_WARNING
-#define PRAGMA_REENABLE_PACKED_WARNING
-#endif
-
 #if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
 # define QEMU_PACKED __attribute__((gcc_struct, packed))
 #else
-- 
2.41.0
Re: [PATCH v3] bsd-user: Move PRAGMA_DISABLE_PACKED_WARNING etc to qemu.h
Posted by Peter Maydell 8 months ago
On Wed, 30 Aug 2023 at 15:51, Warner Losh <imp@bsdimp.com> wrote:
>
> For the moment, move PRAGMA_DISABLE_PACKED_WARNING and
> PRAGMA_ENABLE_PACKED_WARNING back to bsd-user/qemu.h.
>
> Of course, these should be in compiler.h, but that interferes with too
> many things at the moment, so take one step back to unbreak clang
> linux-user builds first. Use the exact same version that's in
> linux-user/qemu.h since that's what should be in compiler.h.
> ---
>  bsd-user/qemu.h         | 27 +++++++++++++++++++++++++++
>  include/qemu/compiler.h | 30 ------------------------------
>  2 files changed, 27 insertions(+), 30 deletions(-)

Yeah, let's un-break CI first and then think about a neater
thing afterwards.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM
Re: [PATCH v3] bsd-user: Move PRAGMA_DISABLE_PACKED_WARNING etc to qemu.h
Posted by Warner Losh 8 months ago
On Wed, Aug 30, 2023 at 8:55 AM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Wed, 30 Aug 2023 at 15:51, Warner Losh <imp@bsdimp.com> wrote:
> >
> > For the moment, move PRAGMA_DISABLE_PACKED_WARNING and
> > PRAGMA_ENABLE_PACKED_WARNING back to bsd-user/qemu.h.
> >
> > Of course, these should be in compiler.h, but that interferes with too
> > many things at the moment, so take one step back to unbreak clang
> > linux-user builds first. Use the exact same version that's in
> > linux-user/qemu.h since that's what should be in compiler.h.
> > ---
> >  bsd-user/qemu.h         | 27 +++++++++++++++++++++++++++
> >  include/qemu/compiler.h | 30 ------------------------------
> >  2 files changed, 27 insertions(+), 30 deletions(-)
>
> Yeah, let's un-break CI first and then think about a neater
> thing afterwards.
>

Yea. And in 2 days I'll have minutes again and in 7 I'll be past a deadline
at $WORK and have time to setup local gitlab CI... but trying to play
whack-a-mole
with this w/o either is going to take days...


> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
> thanks
> -- PMM
>