| Message ID | 20241126211736.122285-2-pierrick.bouvier@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Enable clang build on Windows | expand |
On 26/11/2024 22.17, Pierrick Bouvier wrote: > This attribute is not recognized by clang. > > An investigation has been performed to ensure this attribute has no > effect on layout of structures we use in QEMU [1], so it's safe to > remove now. > > In the future, we'll forbid introducing new bitfields in packed struct. > > [1] https://lore.kernel.org/qemu-devel/66c346de-7e20-4831-b3eb-1cda83240af9@linaro.org/ > > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > --- > meson.build | 5 ----- > include/qemu/compiler.h | 7 +------ > scripts/cocci-macro-file.h | 6 +----- > subprojects/libvhost-user/libvhost-user.h | 6 +----- > 4 files changed, 3 insertions(+), 21 deletions(-) Reviewed-by: Thomas Huth <thuth@redhat.com>
On Tue, Nov 26, 2024 at 01:17:34PM -0800, Pierrick Bouvier wrote: >This attribute is not recognized by clang. > >An investigation has been performed to ensure this attribute has no >effect on layout of structures we use in QEMU [1], so it's safe to >remove now. > >In the future, we'll forbid introducing new bitfields in packed struct. Why? I suggest reporting in this commit description what you wrote in the following patch in the documentation (or a summary), because at first glance I could not understand the correlation between removing gcc_struct and bitfields in packed struct. > >[1] https://lore.kernel.org/qemu-devel/66c346de-7e20-4831-b3eb-1cda83240af9@linaro.org/ > >Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >--- > meson.build | 5 ----- > include/qemu/compiler.h | 7 +------ > scripts/cocci-macro-file.h | 6 +----- > subprojects/libvhost-user/libvhost-user.h | 6 +----- > 4 files changed, 3 insertions(+), 21 deletions(-) The patch LGTM. Acked-by: Stefano Garzarella <sgarzare@redhat.com> > >diff --git a/meson.build b/meson.build >index e0b880e4e13..fa6d24e1233 100644 >--- a/meson.build >+++ b/meson.build >@@ -354,11 +354,6 @@ elif host_os == 'sunos' > qemu_common_flags += '-D__EXTENSIONS__' > elif host_os == 'haiku' > qemu_common_flags += ['-DB_USE_POSITIVE_POSIX_ERRORS', '-D_BSD_SOURCE', '-fPIC'] >-elif host_os == 'windows' >- if not compiler.compiles('struct x { int y; } __attribute__((gcc_struct));', >- args: '-Werror') >- error('Your compiler does not support __attribute__((gcc_struct)) - please use GCC instead of Clang') >- endif > endif > > # Choose instruction set (currently x86-only) >diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h >index c06954ccb41..d904408e5ed 100644 >--- a/include/qemu/compiler.h >+++ b/include/qemu/compiler.h >@@ -22,12 +22,7 @@ > #define QEMU_EXTERN_C extern > #endif > >-#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__)) >-# define QEMU_PACKED __attribute__((gcc_struct, packed)) >-#else >-# define QEMU_PACKED __attribute__((packed)) >-#endif >- >+#define QEMU_PACKED __attribute__((packed)) > #define QEMU_ALIGNED(X) __attribute__((aligned(X))) > > #ifndef glue >diff --git a/scripts/cocci-macro-file.h b/scripts/cocci-macro-file.h >index d247a5086e9..c64831d5408 100644 >--- a/scripts/cocci-macro-file.h >+++ b/scripts/cocci-macro-file.h >@@ -23,11 +23,7 @@ > #define G_GNUC_WARN_UNUSED_RESULT __attribute__((warn_unused_result)) > #define G_GNUC_NULL_TERMINATED __attribute__((sentinel)) > >-#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__)) >-# define QEMU_PACKED __attribute__((gcc_struct, packed)) >-#else >-# define QEMU_PACKED __attribute__((packed)) >-#endif >+#define QEMU_PACKED __attribute__((packed)) > > #define cat(x,y) x ## y > #define cat2(x,y) cat(x,y) >diff --git a/subprojects/libvhost-user/libvhost-user.h b/subprojects/libvhost-user/libvhost-user.h >index deb40e77b3f..2ffc58c11b1 100644 >--- a/subprojects/libvhost-user/libvhost-user.h >+++ b/subprojects/libvhost-user/libvhost-user.h >@@ -186,11 +186,7 @@ typedef struct VhostUserShared { > unsigned char uuid[UUID_LEN]; > } VhostUserShared; > >-#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__)) >-# define VU_PACKED __attribute__((gcc_struct, packed)) >-#else >-# define VU_PACKED __attribute__((packed)) >-#endif >+#define VU_PACKED __attribute__((packed)) > > typedef struct VhostUserMsg { > int request; >-- >2.39.5 >
On 11/27/24 01:20, Stefano Garzarella wrote: > On Tue, Nov 26, 2024 at 01:17:34PM -0800, Pierrick Bouvier wrote: >> This attribute is not recognized by clang. >> >> An investigation has been performed to ensure this attribute has no >> effect on layout of structures we use in QEMU [1], so it's safe to >> remove now. >> >> In the future, we'll forbid introducing new bitfields in packed struct. > > Why? > I suggest reporting in this commit description what you wrote in the > following patch in the documentation (or a summary), because at first > glance I could not understand the correlation between removing > gcc_struct and bitfields in packed struct. > Thanks, I'll mention the problem came explicitely from the bitfields. >> >> [1] https://lore.kernel.org/qemu-devel/66c346de-7e20-4831-b3eb-1cda83240af9@linaro.org/ >> >> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >> --- >> meson.build | 5 ----- >> include/qemu/compiler.h | 7 +------ >> scripts/cocci-macro-file.h | 6 +----- >> subprojects/libvhost-user/libvhost-user.h | 6 +----- >> 4 files changed, 3 insertions(+), 21 deletions(-) > > The patch LGTM. > > Acked-by: Stefano Garzarella <sgarzare@redhat.com> > >> >> diff --git a/meson.build b/meson.build >> index e0b880e4e13..fa6d24e1233 100644 >> --- a/meson.build >> +++ b/meson.build >> @@ -354,11 +354,6 @@ elif host_os == 'sunos' >> qemu_common_flags += '-D__EXTENSIONS__' >> elif host_os == 'haiku' >> qemu_common_flags += ['-DB_USE_POSITIVE_POSIX_ERRORS', '-D_BSD_SOURCE', '-fPIC'] >> -elif host_os == 'windows' >> - if not compiler.compiles('struct x { int y; } __attribute__((gcc_struct));', >> - args: '-Werror') >> - error('Your compiler does not support __attribute__((gcc_struct)) - please use GCC instead of Clang') >> - endif >> endif >> >> # Choose instruction set (currently x86-only) >> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h >> index c06954ccb41..d904408e5ed 100644 >> --- a/include/qemu/compiler.h >> +++ b/include/qemu/compiler.h >> @@ -22,12 +22,7 @@ >> #define QEMU_EXTERN_C extern >> #endif >> >> -#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__)) >> -# define QEMU_PACKED __attribute__((gcc_struct, packed)) >> -#else >> -# define QEMU_PACKED __attribute__((packed)) >> -#endif >> - >> +#define QEMU_PACKED __attribute__((packed)) >> #define QEMU_ALIGNED(X) __attribute__((aligned(X))) >> >> #ifndef glue >> diff --git a/scripts/cocci-macro-file.h b/scripts/cocci-macro-file.h >> index d247a5086e9..c64831d5408 100644 >> --- a/scripts/cocci-macro-file.h >> +++ b/scripts/cocci-macro-file.h >> @@ -23,11 +23,7 @@ >> #define G_GNUC_WARN_UNUSED_RESULT __attribute__((warn_unused_result)) >> #define G_GNUC_NULL_TERMINATED __attribute__((sentinel)) >> >> -#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__)) >> -# define QEMU_PACKED __attribute__((gcc_struct, packed)) >> -#else >> -# define QEMU_PACKED __attribute__((packed)) >> -#endif >> +#define QEMU_PACKED __attribute__((packed)) >> >> #define cat(x,y) x ## y >> #define cat2(x,y) cat(x,y) >> diff --git a/subprojects/libvhost-user/libvhost-user.h b/subprojects/libvhost-user/libvhost-user.h >> index deb40e77b3f..2ffc58c11b1 100644 >> --- a/subprojects/libvhost-user/libvhost-user.h >> +++ b/subprojects/libvhost-user/libvhost-user.h >> @@ -186,11 +186,7 @@ typedef struct VhostUserShared { >> unsigned char uuid[UUID_LEN]; >> } VhostUserShared; >> >> -#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__)) >> -# define VU_PACKED __attribute__((gcc_struct, packed)) >> -#else >> -# define VU_PACKED __attribute__((packed)) >> -#endif >> +#define VU_PACKED __attribute__((packed)) >> >> typedef struct VhostUserMsg { >> int request; >> -- >> 2.39.5 >> >
diff --git a/meson.build b/meson.build index e0b880e4e13..fa6d24e1233 100644 --- a/meson.build +++ b/meson.build @@ -354,11 +354,6 @@ elif host_os == 'sunos' qemu_common_flags += '-D__EXTENSIONS__' elif host_os == 'haiku' qemu_common_flags += ['-DB_USE_POSITIVE_POSIX_ERRORS', '-D_BSD_SOURCE', '-fPIC'] -elif host_os == 'windows' - if not compiler.compiles('struct x { int y; } __attribute__((gcc_struct));', - args: '-Werror') - error('Your compiler does not support __attribute__((gcc_struct)) - please use GCC instead of Clang') - endif endif # Choose instruction set (currently x86-only) diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h index c06954ccb41..d904408e5ed 100644 --- a/include/qemu/compiler.h +++ b/include/qemu/compiler.h @@ -22,12 +22,7 @@ #define QEMU_EXTERN_C extern #endif -#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__)) -# define QEMU_PACKED __attribute__((gcc_struct, packed)) -#else -# define QEMU_PACKED __attribute__((packed)) -#endif - +#define QEMU_PACKED __attribute__((packed)) #define QEMU_ALIGNED(X) __attribute__((aligned(X))) #ifndef glue diff --git a/scripts/cocci-macro-file.h b/scripts/cocci-macro-file.h index d247a5086e9..c64831d5408 100644 --- a/scripts/cocci-macro-file.h +++ b/scripts/cocci-macro-file.h @@ -23,11 +23,7 @@ #define G_GNUC_WARN_UNUSED_RESULT __attribute__((warn_unused_result)) #define G_GNUC_NULL_TERMINATED __attribute__((sentinel)) -#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__)) -# define QEMU_PACKED __attribute__((gcc_struct, packed)) -#else -# define QEMU_PACKED __attribute__((packed)) -#endif +#define QEMU_PACKED __attribute__((packed)) #define cat(x,y) x ## y #define cat2(x,y) cat(x,y) diff --git a/subprojects/libvhost-user/libvhost-user.h b/subprojects/libvhost-user/libvhost-user.h index deb40e77b3f..2ffc58c11b1 100644 --- a/subprojects/libvhost-user/libvhost-user.h +++ b/subprojects/libvhost-user/libvhost-user.h @@ -186,11 +186,7 @@ typedef struct VhostUserShared { unsigned char uuid[UUID_LEN]; } VhostUserShared; -#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__)) -# define VU_PACKED __attribute__((gcc_struct, packed)) -#else -# define VU_PACKED __attribute__((packed)) -#endif +#define VU_PACKED __attribute__((packed)) typedef struct VhostUserMsg { int request;
This attribute is not recognized by clang. An investigation has been performed to ensure this attribute has no effect on layout of structures we use in QEMU [1], so it's safe to remove now. In the future, we'll forbid introducing new bitfields in packed struct. [1] https://lore.kernel.org/qemu-devel/66c346de-7e20-4831-b3eb-1cda83240af9@linaro.org/ Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> --- meson.build | 5 ----- include/qemu/compiler.h | 7 +------ scripts/cocci-macro-file.h | 6 +----- subprojects/libvhost-user/libvhost-user.h | 6 +----- 4 files changed, 3 insertions(+), 21 deletions(-)