1. Patchew
  2. QEMU
  3. View series

[PULL 0/1] Quick fix patches

Warner Losh posted 1 patch 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230830022205.57878-1-imp@bsdimp.com
Maintainers: Laurent Vivier <laurent@vivier.eu>
There is a newer version of this series
include/qemu/compiler.h |  6 +-----
linux-user/qemu.h       | 26 --------------------------
2 files changed, 1 insertion(+), 31 deletions(-)
[PULL 0/1] Quick fix patches
Posted by Warner Losh 8 months ago 
The following changes since commit 813bac3d8d70d85cb7835f7945eb9eed84c2d8d0:

  Merge tag '2023q3-bsd-user-pull-request' of https://gitlab.com/bsdimp/qemu into staging (2023-08-29 08:58:00 -0400)

are available in the Git repository at:

  https://gitlab.com/bsdimp/qemu.git tags/quick-fix-pull-request

for you to fetch changes up to de287fb4e8987b32e133f7f37b990e09f3aa6325:

  linux-user: Move PRAGMA_DISABLE_PACKED_WARNING to compiler.h (2023-08-29 20:12:25 -0600)

----------------------------------------------------------------
Pull request: Quick fix for clang user-mode job

Move the linux-user version of PRAGMA_DISABLE_PACKED_WARNING
to qemu/compiler.h and remove it from linux-user/qemu.h.
-----BEGIN PGP SIGNATURE-----
Comment: GPGTools - https://gpgtools.org

iQIzBAABCgAdFiEEIDX4lLAKo898zeG3bBzRKH2wEQAFAmTup4sACgkQbBzRKH2w
EQChXw/9H/hKENL0vLz9LE1iq05+bJ6/uY/Kl4avXX3/ZBq39/ZvNHrgT6h26PMb
wU2vFYFL8UTZQsfC8t35B1khmoK3ZUtbTYYUjzpmVQQA1+MGNpflgciSQhsITGkG
zOraHo9kSkM/ByHE246zSxqJlgHTziE/mQ1Hg8AFNvI5KChgedMblFz4gu99ADMA
sVQwBUTAeOJv3uvY9DhXCxtvg5Lj+ZcJd7Uu4pYl86jHp0RSE7Jk6jrJXo+Xp3GF
MnDxK9IrShEmIK1ci+tG8YBiY91GW/GEPVJJxL03JsvWxuRhj8GQsIopD1Mo4xbp
mniDs6AbDTpxnE3DpqrN8UFh+3Ko0qZw+/OjCxckYbQadYrWVeL6n+uHxcs15Z+R
SmIURzBrcLqzvFmvpUD4KHBQxSdIGdZrCQA+PC54Ghx0tqBBPapd/4LPL5kJsVqX
6DOYwegbbnDNcGIXv/5RXoL+sIF00mWpWslV+xCrTP5Dz9KQmjSC/fgPnNucr2H5
MBbe0BAxZvn1KHbgUhxCVNd1WFyaq1Gu5XZRNsXy0BBs5/NzkrJm614JOLbS+jtO
DgvEHvbo57LGB145IBZQOUfAUQUnizyUhb27cK+L8hzg3MHsMG+coQyi5I4zaBGn
5JddIPvbpUFk9mS+i9oeZRr/gPmDgPbylaOQhNGQvYpu8xB8/lU=
=QC7n
-----END PGP SIGNATURE-----

----------------------------------------------------------------

Warner Losh (1):
  linux-user: Move PRAGMA_DISABLE_PACKED_WARNING to compiler.h

 include/qemu/compiler.h |  6 +-----
 linux-user/qemu.h       | 26 --------------------------
 2 files changed, 1 insertion(+), 31 deletions(-)

-- 
2.41.0
Re: [PULL 0/1] Quick fix patches
Posted by Stefan Hajnoczi 8 months ago 
Hi,
The patch introduces the following build failure:

cc -m64 -mcx16 -Isubprojects/libvhost-user/libvhost-user.a.p
-Isubprojects/libvhost-user -I../subprojects/libvhost-user
-fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu99 -O2 -g
-Wsign-compare -Wdeclaration-after-statement -Wstrict-aliasing
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing
-fno-common -fwrapv -fPIE -pthread -D_GNU_SOURCE -MD -MQ
subprojects/libvhost-user/libvhost-user.a.p/libvhost-user.c.o -MF
subprojects/libvhost-user/libvhost-user.a.p/libvhost-user.c.o.d -o
subprojects/libvhost-user/libvhost-user.a.p/libvhost-user.c.o -c
../subprojects/libvhost-user/libvhost-user.c
In file included from ../subprojects/libvhost-user/include/atomic.h:18,
from ../subprojects/libvhost-user/libvhost-user.c:53:
../subprojects/libvhost-user/include/compiler.h:38:40: error: missing
binary operator before token "("
38 | #if defined(__clang__) && __has_warning("-Waddress-of-packed-member")
| ^

https://gitlab.com/qemu-project/qemu/-/jobs/4981576093

Stefan
Re: [PULL 0/1] Quick fix patches
Posted by Warner Losh 8 months ago 
On Wed, Aug 30, 2023, 7:16 AM Stefan Hajnoczi <stefanha@gmail.com> wrote:

> Hi,
> The patch introduces the following build failure:
>
> cc -m64 -mcx16 -Isubprojects/libvhost-user/libvhost-user.a.p
> -Isubprojects/libvhost-user -I../subprojects/libvhost-user
> -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu99 -O2 -g
> -Wsign-compare -Wdeclaration-after-statement -Wstrict-aliasing
> -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing
> -fno-common -fwrapv -fPIE -pthread -D_GNU_SOURCE -MD -MQ
> subprojects/libvhost-user/libvhost-user.a.p/libvhost-user.c.o -MF
> subprojects/libvhost-user/libvhost-user.a.p/libvhost-user.c.o.d -o
> subprojects/libvhost-user/libvhost-user.a.p/libvhost-user.c.o -c
> ../subprojects/libvhost-user/libvhost-user.c
> In file included from ../subprojects/libvhost-user/include/atomic.h:18,
> from ../subprojects/libvhost-user/libvhost-user.c:53:
> ../subprojects/libvhost-user/include/compiler.h:38:40: error: missing
> binary operator before token "("
> 38 | #if defined(__clang__) && __has_warning("-Waddress-of-packed-member")
> | ^
>
> https://gitlab.com/qemu-project/qemu/-/jobs/4981576093


Looks like the macros should be removed there too...  but I don't know
about the subproject tree. Can I submit to it the same way?

And for a quick fix... maybe I just move it back to bsd-user/qemu.h until I
have more minutes and can test things better or fund the time to setup a
Linux build box for docker...

Warner


> Stefan
>
Re: [PULL 0/1] Quick fix patches
Posted by Thomas Huth 8 months ago 
On 30/08/2023 15.16, Stefan Hajnoczi wrote:
> Hi,
> The patch introduces the following build failure:
> 
> cc -m64 -mcx16 -Isubprojects/libvhost-user/libvhost-user.a.p
> -Isubprojects/libvhost-user -I../subprojects/libvhost-user
> -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu99 -O2 -g
> -Wsign-compare -Wdeclaration-after-statement -Wstrict-aliasing
> -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing
> -fno-common -fwrapv -fPIE -pthread -D_GNU_SOURCE -MD -MQ
> subprojects/libvhost-user/libvhost-user.a.p/libvhost-user.c.o -MF
> subprojects/libvhost-user/libvhost-user.a.p/libvhost-user.c.o.d -o
> subprojects/libvhost-user/libvhost-user.a.p/libvhost-user.c.o -c
> ../subprojects/libvhost-user/libvhost-user.c
> In file included from ../subprojects/libvhost-user/include/atomic.h:18,
> from ../subprojects/libvhost-user/libvhost-user.c:53:
> ../subprojects/libvhost-user/include/compiler.h:38:40: error: missing
> binary operator before token "("
> 38 | #if defined(__clang__) && __has_warning("-Waddress-of-packed-member")
> | ^
> 
> https://gitlab.com/qemu-project/qemu/-/jobs/4981576093

IIRC older versions of GCC do not have __has_warning() yet, so if you want 
to use this in compiler.h, you have to do it below the line in compiler.h 
that adds this:

#ifndef __has_warning
#define __has_warning(x) 0 /* compatibility with non-clang compilers */
#endif

  HTH,
   Thomas
Re: [PULL 0/1] Quick fix patches
Posted by Warner Losh 8 months ago 
On Wed, Aug 30, 2023, 7:26 AM Thomas Huth <thuth@redhat.com> wrote:

> On 30/08/2023 15.16, Stefan Hajnoczi wrote:
> > Hi,
> > The patch introduces the following build failure:
> >
> > cc -m64 -mcx16 -Isubprojects/libvhost-user/libvhost-user.a.p
> > -Isubprojects/libvhost-user -I../subprojects/libvhost-user
> > -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu99 -O2 -g
> > -Wsign-compare -Wdeclaration-after-statement -Wstrict-aliasing
> > -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing
> > -fno-common -fwrapv -fPIE -pthread -D_GNU_SOURCE -MD -MQ
> > subprojects/libvhost-user/libvhost-user.a.p/libvhost-user.c.o -MF
> > subprojects/libvhost-user/libvhost-user.a.p/libvhost-user.c.o.d -o
> > subprojects/libvhost-user/libvhost-user.a.p/libvhost-user.c.o -c
> > ../subprojects/libvhost-user/libvhost-user.c
> > In file included from ../subprojects/libvhost-user/include/atomic.h:18,
> > from ../subprojects/libvhost-user/libvhost-user.c:53:
> > ../subprojects/libvhost-user/include/compiler.h:38:40: error: missing
> > binary operator before token "("
> > 38 | #if defined(__clang__) &&
> __has_warning("-Waddress-of-packed-member")
> > | ^
> >
> > https://gitlab.com/qemu-project/qemu/-/jobs/4981576093
>
> IIRC older versions of GCC do not have __has_warning() yet, so if you want
> to use this in compiler.h, you have to do it below the line in compiler.h
> that adds this:
>
> #ifndef __has_warning
> #define __has_warning(x) 0 /* compatibility with non-clang compilers */
> #endif
>

This already works for linux-user. If there are gcc versions that break,
our current CI jobs don't show it. Why add complexity for unsupported gcc
versions? And how do I know I got it right?

I'm really starting to think the feedback 'move it to compilers.h' should
have just been ignored... it's turning into a lot of my time to correct
that I don't have when I'm also out of CI minutes to test with.

Warner





>   HTH,
>    Thomas
>
>
Re: [PULL 0/1] Quick fix patches
Posted by Daniel P. Berrangé 8 months ago 
On Wed, Aug 30, 2023 at 08:31:15AM -0600, Warner Losh wrote:
> On Wed, Aug 30, 2023, 7:26 AM Thomas Huth <thuth@redhat.com> wrote:
> 
> > On 30/08/2023 15.16, Stefan Hajnoczi wrote:
> > > Hi,
> > > The patch introduces the following build failure:
> > >
> > > cc -m64 -mcx16 -Isubprojects/libvhost-user/libvhost-user.a.p
> > > -Isubprojects/libvhost-user -I../subprojects/libvhost-user
> > > -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu99 -O2 -g
> > > -Wsign-compare -Wdeclaration-after-statement -Wstrict-aliasing
> > > -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing
> > > -fno-common -fwrapv -fPIE -pthread -D_GNU_SOURCE -MD -MQ
> > > subprojects/libvhost-user/libvhost-user.a.p/libvhost-user.c.o -MF
> > > subprojects/libvhost-user/libvhost-user.a.p/libvhost-user.c.o.d -o
> > > subprojects/libvhost-user/libvhost-user.a.p/libvhost-user.c.o -c
> > > ../subprojects/libvhost-user/libvhost-user.c
> > > In file included from ../subprojects/libvhost-user/include/atomic.h:18,
> > > from ../subprojects/libvhost-user/libvhost-user.c:53:
> > > ../subprojects/libvhost-user/include/compiler.h:38:40: error: missing
> > > binary operator before token "("
> > > 38 | #if defined(__clang__) &&
> > __has_warning("-Waddress-of-packed-member")
> > > | ^
> > >
> > > https://gitlab.com/qemu-project/qemu/-/jobs/4981576093
> >
> > IIRC older versions of GCC do not have __has_warning() yet, so if you want
> > to use this in compiler.h, you have to do it below the line in compiler.h
> > that adds this:
> >
> > #ifndef __has_warning
> > #define __has_warning(x) 0 /* compatibility with non-clang compilers */
> > #endif
> >
> 
> This already works for linux-user. If there are gcc versions that break,
> our current CI jobs don't show it. Why add complexity for unsupported gcc
> versions? And how do I know I got it right?

IIUC, /no/ GCC version has __has_warning. The no-op stub we have works
because we merely need the preprocessor to be able to parse the
expression

  #if defined(__clang__) && __has_warning("....")

it'll never actually evaluate the __has_warning clause under GCC
because the defined(__clang__) will be false.

> I'm really starting to think the feedback 'move it to compilers.h' should
> have just been ignored... it's turning into a lot of my time to correct
> that I don't have when I'm also out of CI minutes to test with.

FWIW, if you have a Linux VM with docker/podman present it also possible
to run the CI environments locally with 'make docker-help' has more info

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PULL 0/1] Quick fix patches
Posted by Daniel P. Berrangé 8 months ago 
On Wed, Aug 30, 2023 at 03:26:25PM +0200, Thomas Huth wrote:
> On 30/08/2023 15.16, Stefan Hajnoczi wrote:
> > Hi,
> > The patch introduces the following build failure:
> > 
> > cc -m64 -mcx16 -Isubprojects/libvhost-user/libvhost-user.a.p
> > -Isubprojects/libvhost-user -I../subprojects/libvhost-user
> > -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu99 -O2 -g
> > -Wsign-compare -Wdeclaration-after-statement -Wstrict-aliasing
> > -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing
> > -fno-common -fwrapv -fPIE -pthread -D_GNU_SOURCE -MD -MQ
> > subprojects/libvhost-user/libvhost-user.a.p/libvhost-user.c.o -MF
> > subprojects/libvhost-user/libvhost-user.a.p/libvhost-user.c.o.d -o
> > subprojects/libvhost-user/libvhost-user.a.p/libvhost-user.c.o -c
> > ../subprojects/libvhost-user/libvhost-user.c
> > In file included from ../subprojects/libvhost-user/include/atomic.h:18,
> > from ../subprojects/libvhost-user/libvhost-user.c:53:
> > ../subprojects/libvhost-user/include/compiler.h:38:40: error: missing
> > binary operator before token "("
> > 38 | #if defined(__clang__) && __has_warning("-Waddress-of-packed-member")
> > | ^
> > 
> > https://gitlab.com/qemu-project/qemu/-/jobs/4981576093
> 
> IIRC older versions of GCC do not have __has_warning() yet, so if you want
> to use this in compiler.h, you have to do it below the line in compiler.h
> that adds this:
> 
> #ifndef __has_warning
> #define __has_warning(x) 0 /* compatibility with non-clang compilers */
> #endif

We should probably move those basic __has_XXX compat defs right to
the top of compiler.h 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.