[PULL 0/2] Libslirp patches

marcandre.lureau@redhat.com posted 2 patches 4 months, 3 weeks ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210529185522.78816-1-marcandre.lureau@redhat.com
configure            |  2 +-
meson.build          | 63 +++-----------------------------------------
.gitmodules          |  4 +--
slirp                |  1 -
subprojects/libslirp |  1 +
5 files changed, 8 insertions(+), 63 deletions(-)
delete mode 160000 slirp
create mode 160000 subprojects/libslirp

[PULL 0/2] Libslirp patches

Posted by marcandre.lureau@redhat.com 4 months, 3 weeks ago
From: Marc-André Lureau <marcandre.lureau@redhat.com>

The following changes since commit 62c0ac5041e9130b041adfa13a41583d3c3ddd24:

  Merge remote-tracking branch 'remotes/rth-gitlab/tags/pull-tcg-20210526' into staging (2021-05-28 16:25:21 +0100)

are available in the Git repository at:

  git@github.com:elmarco/qemu.git tags/libslirp-pull-request

for you to fetch changes up to b060428091c758781acc4d42849accc036d3c816:

  build-sys: make libslirp a meson subproject (2021-05-29 22:52:37 +0400)

----------------------------------------------------------------
Update libslirp & make it a subproject

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

Marc-André Lureau (2):
  Update libslirp to v4.5.0
  build-sys: make libslirp a meson subproject

 configure            |  2 +-
 meson.build          | 63 +++-----------------------------------------
 .gitmodules          |  4 +--
 slirp                |  1 -
 subprojects/libslirp |  1 +
 5 files changed, 8 insertions(+), 63 deletions(-)
 delete mode 160000 slirp
 create mode 160000 subprojects/libslirp

-- 
2.29.0



Re: [PULL 0/2] Libslirp patches

Posted by Peter Maydell 4 months, 3 weeks ago
On Sat, 29 May 2021 at 19:55, <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> The following changes since commit 62c0ac5041e9130b041adfa13a41583d3c3ddd24:
>
>   Merge remote-tracking branch 'remotes/rth-gitlab/tags/pull-tcg-20210526' into staging (2021-05-28 16:25:21 +0100)
>
> are available in the Git repository at:
>
>   git@github.com:elmarco/qemu.git tags/libslirp-pull-request
>
> for you to fetch changes up to b060428091c758781acc4d42849accc036d3c816:
>
>   build-sys: make libslirp a meson subproject (2021-05-29 22:52:37 +0400)
>
> ----------------------------------------------------------------
> Update libslirp & make it a subproject
>
> ----------------------------------------------------------------

All hosts, odd warnings on checkout and running configure:

warning: unable to rmdir 'slirp': Directory not empty
make: Entering directory '/home/pm/qemu/build/all'
config-host.mak is out-of-date, running configure
  GIT     ui/keycodemapdb meson tests/fp/berkeley-testfloat-3
tests/fp/berkeley-softfloat-3 dtc capstone slirp
warn: ignoring non-existent submodule slirp

BSD VMs: error message just before launching the VM (though the VM did
seem to then launch OK):

Found ninja-1.8.2 at /usr/bin/ninja
ninja: no work to do.
(GIT="git" "/home/peter.maydell/qemu-netbsd/scripts/git-submodule.sh"
update ui/keycodemapdb meson tests/fp/berkeley-testfloat-3 tests/fp/be
rkeley-softfloat-3 dtc capstone slirp)
warn: ignoring non-existent submodule slirp
/usr/bin/python3 -B /home/peter.maydell/qemu-netbsd/tests/vm/netbsd
--debug  --jobs 8 --verbose    --image
"/home/peter.maydell/.cache/qemu
-vm/images/netbsd.img"  --snapshot --build-qemu
/home/peter.maydell/qemu-netbsd --
DEBUG:root:Creating archive
/home/peter.maydell/qemu-netbsd/build/vm-test-6kefrq76.tmp/data-f706c.tar
for src_dir dir: /home/peter.maydell/q
emu-netbsd
error: pathspec 'slirp' did not match any file(s) known to git.

clang sanitizer build: link failure:
subprojects/libslirp/libslirp.so.0.3.0.p/src_arp_table.c.o: In
function `arp_table_add':
/home/petmay01/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:51:
undefined reference to `__ubsan_handle_type_mismatch_v1'
/home/petmay01/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:51:
undefined reference to `__ubsan_handle_type_mismatch_v1'
/home/petmay01/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:51:
undefined reference to `__ubsan_handle_type_mismatch_v1'
/home/petmay01/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:34:
undefined reference to `__ubsan_handle_type_mismatch_v1'
/home/petmay01/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:34:
undefined reference to `__ubsan_handle_type_mismatch_v1'
(and lots more similar)

OSX: linker warnings linking libslirp.0.dylib:


[34/1977] Linking target subprojects/libslirp/libslirp.0.dylib
ld: warning: dylib
(/usr/local/Cellar/glib/2.68.0/lib/libgthread-2.0.dylib) was built for
newer macOS version (10.15) than being linked (10.4)
ld: warning: dylib
(/usr/local/Cellar/glib/2.68.0/lib/libglib-2.0.dylib) was built for
newer macOS version (10.15) than being linked (10.4)
ld: warning: dylib (/usr/local/opt/gettext/lib/libintl.dylib) was
built for newer macOS version (10.14) than being linked (10.4)

-- PMM

Re: [PULL 0/2] Libslirp patches

Posted by Marc-André Lureau 4 months, 3 weeks ago
Hi Peter

On Tue, Jun 1, 2021 at 1:17 PM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Sat, 29 May 2021 at 19:55, <marcandre.lureau@redhat.com> wrote:
> >
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > The following changes since commit
> 62c0ac5041e9130b041adfa13a41583d3c3ddd24:
> >
> >   Merge remote-tracking branch
> 'remotes/rth-gitlab/tags/pull-tcg-20210526' into staging (2021-05-28
> 16:25:21 +0100)
> >
> > are available in the Git repository at:
> >
> >   git@github.com:elmarco/qemu.git tags/libslirp-pull-request
> >
> > for you to fetch changes up to b060428091c758781acc4d42849accc036d3c816:
> >
> >   build-sys: make libslirp a meson subproject (2021-05-29 22:52:37 +0400)
> >
> > ----------------------------------------------------------------
> > Update libslirp & make it a subproject
> >
> > ----------------------------------------------------------------
>
> All hosts, odd warnings on checkout and running configure:
>
> warning: unable to rmdir 'slirp': Directory not empty
>

This one is from git itself. It doesn't clean up old submodule locations,
even though they are actually "clean". git submodule "(re)move" has its
limits I guess.

make: Entering directory '/home/pm/qemu/build/all'
> config-host.mak is out-of-date, running configure
>   GIT     ui/keycodemapdb meson tests/fp/berkeley-testfloat-3
> tests/fp/berkeley-softfloat-3 dtc capstone slirp
> warn: ignoring non-existent submodule slirp
>

 However, I don't get this when simply running make. Maybe you run make in
parallel, and config-host.mak didn't have the time to regenerate with a new
GIT_SUBMODULES.

I wonder if we miss a dependency like "git-submodule-update:
config-host.mak" ?

Running configure before make should also prevent this from happening.


> BSD VMs: error message just before launching the VM (though the VM did
> seem to then launch OK):
>
> Found ninja-1.8.2 at /usr/bin/ninja
> ninja: no work to do.
> (GIT="git" "/home/peter.maydell/qemu-netbsd/scripts/git-submodule.sh"
> update ui/keycodemapdb meson tests/fp/berkeley-testfloat-3 tests/fp/be
> rkeley-softfloat-3 dtc capstone slirp)
> warn: ignoring non-existent submodule slirp
> /usr/bin/python3 -B /home/peter.maydell/qemu-netbsd/tests/vm/netbsd
> --debug  --jobs 8 --verbose    --image
> "/home/peter.maydell/.cache/qemu
> -vm/images/netbsd.img"  --snapshot --build-qemu
> /home/peter.maydell/qemu-netbsd --
> DEBUG:root:Creating archive
> /home/peter.maydell/qemu-netbsd/build/vm-test-6kefrq76.tmp/data-f706c.tar
> for src_dir dir: /home/peter.maydell/q
> emu-netbsd
> error: pathspec 'slirp' did not match any file(s) known to git.
>

> clang sanitizer build: link failure:
> subprojects/libslirp/libslirp.so.0.3.0.p/src_arp_table.c.o: In
> function `arp_table_add':
>
> /home/petmay01/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:51:
> undefined reference to `__ubsan_handle_type_mismatch_v1'
>
> /home/petmay01/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:51:
> undefined reference to `__ubsan_handle_type_mismatch_v1'
>
> /home/petmay01/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:51:
> undefined reference to `__ubsan_handle_type_mismatch_v1'
>
> /home/petmay01/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:34:
> undefined reference to `__ubsan_handle_type_mismatch_v1'
>
> /home/petmay01/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:34:
> undefined reference to `__ubsan_handle_type_mismatch_v1'
> (and lots more similar)
>
>
I don't get this  when running make vm-build-netbsd. What else am I missing?

>

> OSX: linker warnings linking libslirp.0.dylib:
>
>
> [34/1977] Linking target subprojects/libslirp/libslirp.0.dylib
> ld: warning: dylib
> (/usr/local/Cellar/glib/2.68.0/lib/libgthread-2.0.dylib) was built for
> newer macOS version (10.15) than being linked (10.4)
> ld: warning: dylib
> (/usr/local/Cellar/glib/2.68.0/lib/libglib-2.0.dylib) was built for
> newer macOS version (10.15) than being linked (10.4)
> ld: warning: dylib (/usr/local/opt/gettext/lib/libintl.dylib) was
> built for newer macOS version (10.14) than being linked (10.4)
>
>
This looks related to:
https://gitlab.freedesktop.org/slirp/libslirp/-/commit/410e296a52fb274648f8ecf53561eaab4b33c52c

It could be that we need to use the version information from glib (or from
any libraries used).

It looks safe to ignore although I re-opened:
 https://gitlab.freedesktop.org/slirp/libslirp/-/issues/36#note_940695

-- 
Marc-André Lureau

Re: [PULL 0/2] Libslirp patches

Posted by Peter Maydell 4 months, 2 weeks ago
On Tue, 1 Jun 2021 at 12:01, Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> Hi Peter
>
> On Tue, Jun 1, 2021 at 1:17 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>>
>> On Sat, 29 May 2021 at 19:55, <marcandre.lureau@redhat.com> wrote:
>> >
>> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
>> >
>> > The following changes since commit 62c0ac5041e9130b041adfa13a41583d3c3ddd24:
>> >
>> >   Merge remote-tracking branch 'remotes/rth-gitlab/tags/pull-tcg-20210526' into staging (2021-05-28 16:25:21 +0100)
>> >
>> > are available in the Git repository at:
>> >
>> >   git@github.com:elmarco/qemu.git tags/libslirp-pull-request
>> >
>> > for you to fetch changes up to b060428091c758781acc4d42849accc036d3c816:
>> >
>> >   build-sys: make libslirp a meson subproject (2021-05-29 22:52:37 +0400)
>> >
>> > ----------------------------------------------------------------
>> > Update libslirp & make it a subproject
>> >
>> > ----------------------------------------------------------------
>>
>> All hosts, odd warnings on checkout and running configure:
>>
>> warning: unable to rmdir 'slirp': Directory not empty
>
>
> This one is from git itself. It doesn't clean up old submodule locations, even though they are actually "clean". git submodule "(re)move" has its limits I guess.

Yeah, I guess we have to live with this one.

>> make: Entering directory '/home/pm/qemu/build/all'
>> config-host.mak is out-of-date, running configure
>>   GIT     ui/keycodemapdb meson tests/fp/berkeley-testfloat-3
>> tests/fp/berkeley-softfloat-3 dtc capstone slirp
>> warn: ignoring non-existent submodule slirp
>
>
>  However, I don't get this when simply running make. Maybe you run make in parallel, and config-host.mak didn't have the time to regenerate with a new GIT_SUBMODULES.
>
> I wonder if we miss a dependency like "git-submodule-update: config-host.mak" ?

Something looks like it's still using an old list of submodules.

> Running configure before make should also prevent this from happening.

Incremental build needs to keep working.

>>
>> BSD VMs: error message just before launching the VM (though the VM did
>> seem to then launch OK):
>>
>> Found ninja-1.8.2 at /usr/bin/ninja
>> ninja: no work to do.
>> (GIT="git" "/home/peter.maydell/qemu-netbsd/scripts/git-submodule.sh"
>> update ui/keycodemapdb meson tests/fp/berkeley-testfloat-3 tests/fp/be
>> rkeley-softfloat-3 dtc capstone slirp)
>> warn: ignoring non-existent submodule slirp
>> /usr/bin/python3 -B /home/peter.maydell/qemu-netbsd/tests/vm/netbsd
>> --debug  --jobs 8 --verbose    --image
>> "/home/peter.maydell/.cache/qemu
>> -vm/images/netbsd.img"  --snapshot --build-qemu
>> /home/peter.maydell/qemu-netbsd --
>> DEBUG:root:Creating archive
>> /home/peter.maydell/qemu-netbsd/build/vm-test-6kefrq76.tmp/data-f706c.tar
>> for src_dir dir: /home/peter.maydell/q
>> emu-netbsd
>> error: pathspec 'slirp' did not match any file(s) known to git.


Maybe this is something needing updating in the "create the archive"
script?

>>
>> clang sanitizer build: link failure:
>> subprojects/libslirp/libslirp.so.0.3.0.p/src_arp_table.c.o: In
>> function `arp_table_add':
>> /home/petmay01/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:51:
>> undefined reference to `__ubsan_handle_type_mismatch_v1'
>> /home/petmay01/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:51:
>> undefined reference to `__ubsan_handle_type_mismatch_v1'
>> /home/petmay01/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:51:
>> undefined reference to `__ubsan_handle_type_mismatch_v1'
>> /home/petmay01/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:34:
>> undefined reference to `__ubsan_handle_type_mismatch_v1'
>> /home/petmay01/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:34:
>> undefined reference to `__ubsan_handle_type_mismatch_v1'
>> (and lots more similar)

> I don't get this  when running make vm-build-netbsd. What else am I missing?

This isn't NetBSD related, it's just a clang sanitizer build on Linux.

>> OSX: linker warnings linking libslirp.0.dylib:
>>
>>
>> [34/1977] Linking target subprojects/libslirp/libslirp.0.dylib
>> ld: warning: dylib
>> (/usr/local/Cellar/glib/2.68.0/lib/libgthread-2.0.dylib) was built for
>> newer macOS version (10.15) than being linked (10.4)
>> ld: warning: dylib
>> (/usr/local/Cellar/glib/2.68.0/lib/libglib-2.0.dylib) was built for
>> newer macOS version (10.15) than being linked (10.4)
>> ld: warning: dylib (/usr/local/opt/gettext/lib/libintl.dylib) was
>> built for newer macOS version (10.14) than being linked (10.4)
>>
>
> This looks related to:
> https://gitlab.freedesktop.org/slirp/libslirp/-/commit/410e296a52fb274648f8ecf53561eaab4b33c52c
>
> It could be that we need to use the version information from glib (or from any libraries used).
>
> It looks safe to ignore although I re-opened:
>  https://gitlab.freedesktop.org/slirp/libslirp/-/issues/36#note_940695

I'm not generally a fan of ignoring warnings. I would prefer it if
we understood why it was happening and how shared libraries are
supposed to be being built.

thanks
-- PMM

Re: [PULL 0/2] Libslirp patches

Posted by Marc-André Lureau 4 months, 2 weeks ago
Hi

On Mon, Jun 7, 2021 at 4:17 PM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Tue, 1 Jun 2021 at 12:01, Marc-André Lureau
> <marcandre.lureau@gmail.com> wrote:
> >
> > Hi Peter
> >
> > On Tue, Jun 1, 2021 at 1:17 PM Peter Maydell <peter.maydell@linaro.org>
> wrote:
> >>
> >> On Sat, 29 May 2021 at 19:55, <marcandre.lureau@redhat.com> wrote:
> >> >
> >> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >> >
> >> > The following changes since commit
> 62c0ac5041e9130b041adfa13a41583d3c3ddd24:
> >> >
> >> >   Merge remote-tracking branch
> 'remotes/rth-gitlab/tags/pull-tcg-20210526' into staging (2021-05-28
> 16:25:21 +0100)
> >> >
> >> > are available in the Git repository at:
> >> >
> >> >   git@github.com:elmarco/qemu.git tags/libslirp-pull-request
> >> >
> >> > for you to fetch changes up to
> b060428091c758781acc4d42849accc036d3c816:
> >> >
> >> >   build-sys: make libslirp a meson subproject (2021-05-29 22:52:37
> +0400)
> >> >
> >> > ----------------------------------------------------------------
> >> > Update libslirp & make it a subproject
> >> >
> >> > ----------------------------------------------------------------
> >>
> >> All hosts, odd warnings on checkout and running configure:
> >>
> >> warning: unable to rmdir 'slirp': Directory not empty
> >
> >
> > This one is from git itself. It doesn't clean up old submodule
> locations, even though they are actually "clean". git submodule "(re)move"
> has its limits I guess.
>
> Yeah, I guess we have to live with this one.
>
> >> make: Entering directory '/home/pm/qemu/build/all'
> >> config-host.mak is out-of-date, running configure
> >>   GIT     ui/keycodemapdb meson tests/fp/berkeley-testfloat-3
> >> tests/fp/berkeley-softfloat-3 dtc capstone slirp
> >> warn: ignoring non-existent submodule slirp
> >
> >
> >  However, I don't get this when simply running make. Maybe you run make
> in parallel, and config-host.mak didn't have the time to regenerate with a
> new GIT_SUBMODULES.
> >
> > I wonder if we miss a dependency like "git-submodule-update:
> config-host.mak" ?
>
> Something looks like it's still using an old list of submodules.
>

Yes, but I don't see how I could tell git-submodule-update until after
config-host.mak is regenerated and read again.

Paolo, any idea?

It's a transient issue, similar to the git warning.


> > Running configure before make should also prevent this from happening.
>
> Incremental build needs to keep working.
>
>
Sure, but one-step warnings during incremental build are blockers?


> >>
> >> BSD VMs: error message just before launching the VM (though the VM did
> >> seem to then launch OK):
> >>
> >> Found ninja-1.8.2 at /usr/bin/ninja
> >> ninja: no work to do.
> >> (GIT="git" "/home/peter.maydell/qemu-netbsd/scripts/git-submodule.sh"
> >> update ui/keycodemapdb meson tests/fp/berkeley-testfloat-3 tests/fp/be
> >> rkeley-softfloat-3 dtc capstone slirp)
> >> warn: ignoring non-existent submodule slirp
> >> /usr/bin/python3 -B /home/peter.maydell/qemu-netbsd/tests/vm/netbsd
> >> --debug  --jobs 8 --verbose    --image
> >> "/home/peter.maydell/.cache/qemu
> >> -vm/images/netbsd.img"  --snapshot --build-qemu
> >> /home/peter.maydell/qemu-netbsd --
> >> DEBUG:root:Creating archive
> >>
> /home/peter.maydell/qemu-netbsd/build/vm-test-6kefrq76.tmp/data-f706c.tar
> >> for src_dir dir: /home/peter.maydell/q
> >> emu-netbsd
> >> error: pathspec 'slirp' did not match any file(s) known to git.
>
>
> Maybe this is something needing updating in the "create the archive"
> script?
>

Correct, my bad. No idea why I couldn't reproduce this before..

I guess we should run scripts/archive-source.sh in CI.


> >>
> >> clang sanitizer build: link failure:
> >> subprojects/libslirp/libslirp.so.0.3.0.p/src_arp_table.c.o: In
> >> function `arp_table_add':
> >>
> /home/petmay01/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:51:
> >> undefined reference to `__ubsan_handle_type_mismatch_v1'
> >>
> /home/petmay01/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:51:
> >> undefined reference to `__ubsan_handle_type_mismatch_v1'
> >>
> /home/petmay01/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:51:
> >> undefined reference to `__ubsan_handle_type_mismatch_v1'
> >>
> /home/petmay01/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:34:
> >> undefined reference to `__ubsan_handle_type_mismatch_v1'
> >>
> /home/petmay01/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:34:
> >> undefined reference to `__ubsan_handle_type_mismatch_v1'
> >> (and lots more similar)
>
> > I don't get this  when running make vm-build-netbsd. What else am I
> missing?
>
>
This isn't NetBSD related, it's just a clang sanitizer build on Linux.
>


I am running configure with '--enable-sanitizers' --cc=clang --cxx=clang++
--host-cc=clang, I can't reproduce.

What's your distro? (or meson + clang versions)


> >> OSX: linker warnings linking libslirp.0.dylib:
> >>
> >>
> >> [34/1977] Linking target subprojects/libslirp/libslirp.0.dylib
> >> ld: warning: dylib
> >> (/usr/local/Cellar/glib/2.68.0/lib/libgthread-2.0.dylib) was built for
> >> newer macOS version (10.15) than being linked (10.4)
> >> ld: warning: dylib
> >> (/usr/local/Cellar/glib/2.68.0/lib/libglib-2.0.dylib) was built for
> >> newer macOS version (10.15) than being linked (10.4)
> >> ld: warning: dylib (/usr/local/opt/gettext/lib/libintl.dylib) was
> >> built for newer macOS version (10.14) than being linked (10.4)
> >>
> >
> > This looks related to:
> >
> https://gitlab.freedesktop.org/slirp/libslirp/-/commit/410e296a52fb274648f8ecf53561eaab4b33c52c
> >
> > It could be that we need to use the version information from glib (or
> from any libraries used).
> >
> > It looks safe to ignore although I re-opened:
> >  https://gitlab.freedesktop.org/slirp/libslirp/-/issues/36#note_940695
>
> I'm not generally a fan of ignoring warnings. I would prefer it if
> we understood why it was happening and how shared libraries are
> supposed to be being built.
>


I reverted the change. MacOS build can override the macosx-version-min with
CFLAGS.
See also https://gitlab.freedesktop.org/slirp/libslirp/-/issues/36 why this
was introduced.

thanks

-- 
Marc-André Lureau

Re: [PULL 0/2] Libslirp patches

Posted by Paolo Bonzini 3 months, 2 weeks ago
On 08/06/21 17:55, Marc-André Lureau wrote:
>      > I wonder if we miss a dependency like "git-submodule-update:
>     config-host.mak" ?

Adding the dependency should work (it doesn't seem to me that it would 
add any dependency loop).

Paolo


Re: [PULL 0/2] Libslirp patches

Posted by Peter Maydell 4 months, 2 weeks ago
On Tue, 8 Jun 2021 at 16:55, Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> Hi
>
> On Mon, Jun 7, 2021 at 4:17 PM Peter Maydell <peter.maydell@linaro.org> wrote:

>> >> clang sanitizer build: link failure:
>> >> subprojects/libslirp/libslirp.so.0.3.0.p/src_arp_table.c.o: In
>> >> function `arp_table_add':
>> >> /home/petmay01/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:51:
>> >> undefined reference to `__ubsan_handle_type_mismatch_v1'
>> >> /home/petmay01/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:51:
>> >> undefined reference to `__ubsan_handle_type_mismatch_v1'
>> >> /home/petmay01/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:51:
>> >> undefined reference to `__ubsan_handle_type_mismatch_v1'
>> >> /home/petmay01/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:34:
>> >> undefined reference to `__ubsan_handle_type_mismatch_v1'
>> >> /home/petmay01/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:34:
>> >> undefined reference to `__ubsan_handle_type_mismatch_v1'
>> >> (and lots more similar)
>>
>> > I don't get this  when running make vm-build-netbsd. What else am I missing?
>>
>>
>> This isn't NetBSD related, it's just a clang sanitizer build on Linux.
>
>
>
> I am running configure with '--enable-sanitizers' --cc=clang --cxx=clang++ --host-cc=clang, I can't reproduce.
>
> What's your distro? (or meson + clang versions)

Ubuntu 18.04.5 LTS (bionic); configure arguments
'--cc=clang' '--cxx=clang++' '--enable-gtk'
'--extra-cflags=-fsanitize=undefined  -fno-sanitize=shift-base
-Werror'
clang version 6.0.0-1ubuntu2

-- PMM

Re: [PULL 0/2] Libslirp patches

Posted by Marc-André Lureau 4 months, 2 weeks ago
Hi

On Tue, Jun 8, 2021 at 8:55 PM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Tue, 8 Jun 2021 at 16:55, Marc-André Lureau
> <marcandre.lureau@gmail.com> wrote:
> >
> > Hi
> >
> > On Mon, Jun 7, 2021 at 4:17 PM Peter Maydell <peter.maydell@linaro.org>
> wrote:
>
> >> >> clang sanitizer build: link failure:
> >> >> subprojects/libslirp/libslirp.so.0.3.0.p/src_arp_table.c.o: In
> >> >> function `arp_table_add':
> >> >>
> /home/petmay01/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:51:
> >> >> undefined reference to `__ubsan_handle_type_mismatch_v1'
> >> >>
> /home/petmay01/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:51:
> >> >> undefined reference to `__ubsan_handle_type_mismatch_v1'
> >> >>
> /home/petmay01/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:51:
> >> >> undefined reference to `__ubsan_handle_type_mismatch_v1'
> >> >>
> /home/petmay01/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:34:
> >> >> undefined reference to `__ubsan_handle_type_mismatch_v1'
> >> >>
> /home/petmay01/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:34:
> >> >> undefined reference to `__ubsan_handle_type_mismatch_v1'
> >> >> (and lots more similar)
> >>
> >> > I don't get this  when running make vm-build-netbsd. What else am I
> missing?
> >>
> >>
> >> This isn't NetBSD related, it's just a clang sanitizer build on Linux.
> >
> >
> >
> > I am running configure with '--enable-sanitizers' --cc=clang
> --cxx=clang++ --host-cc=clang, I can't reproduce.
> >
> > What's your distro? (or meson + clang versions)
>
> Ubuntu 18.04.5 LTS (bionic); configure arguments
> '--cc=clang' '--cxx=clang++' '--enable-gtk'
> '--extra-cflags=-fsanitize=undefined  -fno-sanitize=shift-base
> -Werror'
> clang version 6.0.0-1ubuntu2


Per subproject `default_library` was added in 0.54, and we require 0.55.3.
Why is it trying to build libslirp.so?

I tried to make vm-build-ubuntu.i386 with the following changes:

 diff --git a/tests/vm/ubuntu.i386 b/tests/vm/ubuntu.i386
index 47681b6f87..21d0b64eb1 100755
--- a/tests/vm/ubuntu.i386
+++ b/tests/vm/ubuntu.i386
@@ -18,7 +18,7 @@ import ubuntuvm
 DEFAULT_CONFIG = {
     'install_cmds' : "apt-get update,"\
                      "apt-get build-dep -y qemu,"\
-                     "apt-get install -y libfdt-dev language-pack-en
ninja-build",
+                     "apt-get install -y libfdt-dev language-pack-en
ninja-build clang",
 }

 class UbuntuX86VM(ubuntuvm.UbuntuVM):
@@ -32,7 +32,7 @@ class UbuntuX86VM(ubuntuvm.UbuntuVM):
         cd $(mktemp -d);
         sudo chmod a+r /dev/vdb;
         tar -xf /dev/vdb;
-        ./configure {configure_opts};
+        ./configure {configure_opts} --cc=clang --cxx=clang++
--host-cc=clang --extra-cflags='-fsanitize=undefined
 -fno-sanitize=shift-base -Werror';
         make --output-sync {target} -j{jobs} {verbose};
     """

(or with EXTRA_CONFIGURE_OPTS)

And it failed with:

[2363/9207] Linking target qemu-system-aarch64
FAILED: qemu-system-aarch64
clang++ @qemu-system-aarch64.rsp
libqemu-aarch64-softmmu.fa.p/accel_tcg_cputlb.c.o: In function
`helper_atomic_cmpxchgq_le_mmu':
/tmp/tmp.VkWONZ62bA/build/../accel/tcg/atomic_template.h:86: undefined
reference to `__atomic_compare_exchange_8'
libqemu-aarch64-softmmu.fa.p/accel_tcg_cputlb.c.o: In function
`helper_atomic_xchgq_le_mmu':
/tmp/tmp.VkWONZ62bA/build/../accel/tcg/atomic_template.h:134: undefined
reference to `__atomic_exchange_8'
libqemu-aarch64-softmmu.fa.p/accel_tcg_cputlb.c.o: In function
`helper_atomic_fetch_addq_le_mmu':

Any idea what I am missing?

thanks

-- 
Marc-André Lureau

Re: [PULL 0/2] Libslirp patches

Posted by Philippe Mathieu-Daudé 3 months, 2 weeks ago
+Richard/Stefan for "atomic" error.

On 6/8/21 10:35 PM, Marc-André Lureau wrote:

> Per subproject `default_library` was added in 0.54, and we require
> 0.55.3. Why is it trying to build libslirp.so?
> 
> I tried to make vm-build-ubuntu.i386 with the following changes:
> 
>  diff --git a/tests/vm/ubuntu.i386 b/tests/vm/ubuntu.i386
> index 47681b6f87..21d0b64eb1 100755
> --- a/tests/vm/ubuntu.i386
> +++ b/tests/vm/ubuntu.i386
> @@ -18,7 +18,7 @@ import ubuntuvm
>  DEFAULT_CONFIG = {
>      'install_cmds' : "apt-get update,"\
>                       "apt-get build-dep -y qemu,"\
> -                     "apt-get install -y libfdt-dev language-pack-en
> ninja-build",
> +                     "apt-get install -y libfdt-dev language-pack-en
> ninja-build clang",
>  }
>  
>  class UbuntuX86VM(ubuntuvm.UbuntuVM):
> @@ -32,7 +32,7 @@ class UbuntuX86VM(ubuntuvm.UbuntuVM):
>          cd $(mktemp -d);
>          sudo chmod a+r /dev/vdb;
>          tar -xf /dev/vdb;
> -        ./configure {configure_opts};
> +        ./configure {configure_opts} --cc=clang --cxx=clang++
> --host-cc=clang --extra-cflags='-fsanitize=undefined
>  -fno-sanitize=shift-base -Werror';
>          make --output-sync {target} -j{jobs} {verbose};
>      """
> 
> (or with EXTRA_CONFIGURE_OPTS)
> 
> And it failed with:
> 
> [2363/9207] Linking target qemu-system-aarch64
> FAILED: qemu-system-aarch64
> clang++ @qemu-system-aarch64.rsp
> libqemu-aarch64-softmmu.fa.p/accel_tcg_cputlb.c.o: In function
> `helper_atomic_cmpxchgq_le_mmu':
> /tmp/tmp.VkWONZ62bA/build/../accel/tcg/atomic_template.h:86: undefined
> reference to `__atomic_compare_exchange_8'
> libqemu-aarch64-softmmu.fa.p/accel_tcg_cputlb.c.o: In function
> `helper_atomic_xchgq_le_mmu':
> /tmp/tmp.VkWONZ62bA/build/../accel/tcg/atomic_template.h:134: undefined
> reference to `__atomic_exchange_8'
> libqemu-aarch64-softmmu.fa.p/accel_tcg_cputlb.c.o: In function
> `helper_atomic_fetch_addq_le_mmu':
> 
> Any idea what I am missing?
> 
> thanks
> 
> -- 
> Marc-André Lureau


Re: [PULL 0/2] Libslirp patches

Posted by Stefan Hajnoczi 3 months, 2 weeks ago
On Mon, Jul 05, 2021 at 12:31:01PM +0200, Philippe Mathieu-Daudé wrote:
> +Richard/Stefan for "atomic" error.
...
> > [2363/9207] Linking target qemu-system-aarch64
> > FAILED: qemu-system-aarch64
> > clang++ @qemu-system-aarch64.rsp
> > libqemu-aarch64-softmmu.fa.p/accel_tcg_cputlb.c.o: In function
> > `helper_atomic_cmpxchgq_le_mmu':
> > /tmp/tmp.VkWONZ62bA/build/../accel/tcg/atomic_template.h:86: undefined
> > reference to `__atomic_compare_exchange_8'
> > libqemu-aarch64-softmmu.fa.p/accel_tcg_cputlb.c.o: In function
> > `helper_atomic_xchgq_le_mmu':
> > /tmp/tmp.VkWONZ62bA/build/../accel/tcg/atomic_template.h:134: undefined
> > reference to `__atomic_exchange_8'
> > libqemu-aarch64-softmmu.fa.p/accel_tcg_cputlb.c.o: In function
> > `helper_atomic_fetch_addq_le_mmu':

According to docs/devel/atomics.rst:

  These operations are polymorphic; they operate on any type that is as
  wide as a pointer or smaller.

It looks like the compiler doesn't support 8-bit atomics here?

Stefan

Re: [PULL 0/2] Libslirp patches

Posted by Peter Maydell 3 months, 2 weeks ago
On Mon, 5 Jul 2021 at 17:25, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> On Mon, Jul 05, 2021 at 12:31:01PM +0200, Philippe Mathieu-Daudé wrote:
> > +Richard/Stefan for "atomic" error.
> ...
> > > [2363/9207] Linking target qemu-system-aarch64
> > > FAILED: qemu-system-aarch64
> > > clang++ @qemu-system-aarch64.rsp
> > > libqemu-aarch64-softmmu.fa.p/accel_tcg_cputlb.c.o: In function
> > > `helper_atomic_cmpxchgq_le_mmu':
> > > /tmp/tmp.VkWONZ62bA/build/../accel/tcg/atomic_template.h:86: undefined
> > > reference to `__atomic_compare_exchange_8'
> > > libqemu-aarch64-softmmu.fa.p/accel_tcg_cputlb.c.o: In function
> > > `helper_atomic_xchgq_le_mmu':
> > > /tmp/tmp.VkWONZ62bA/build/../accel/tcg/atomic_template.h:134: undefined
> > > reference to `__atomic_exchange_8'
> > > libqemu-aarch64-softmmu.fa.p/accel_tcg_cputlb.c.o: In function
> > > `helper_atomic_fetch_addq_le_mmu':
>
> According to docs/devel/atomics.rst:
>
>   These operations are polymorphic; they operate on any type that is as
>   wide as a pointer or smaller.
>
> It looks like the compiler doesn't support 8-bit atomics here?

8 here means "8 bytes", not "8 bits". And indeed on i386 you can't
do 8-byte atomics with simple insns. The compiler's answer to this
is "emit a call to a helper in libatomic, which will emulate an
atomic access by taking some kind of lock". We don't ever want to
fall back to "take a lock" because sometimes our accesses to the
atomic variables are from TCG generated code -- this is why we
don't link against libatomic. The problem is that we have not
correctly detected that this compiler can't do inline atomics
for 64-bit values and avoided using them. But at least we have
made this a compile failure rather than a silently-wrong-code bug :-)

thanks
-- PMM

Re: [PULL 0/2] Libslirp patches

Posted by Paolo Bonzini 3 months, 2 weeks ago
On 05/07/21 18:31, Peter Maydell wrote:
> 8 here means "8 bytes", not "8 bits". And indeed on i386 you can't
> do 8-byte atomics with simple insns.

You can, there's a cmpxchg8b instruction.  The problem is that somehow 
configure's view of this disagrees with what happens during compilation.

If anybody can send a config.log and make V=1 log, I can look at it.

Paolo

> The compiler's answer to this
> is "emit a call to a helper in libatomic, which will emulate an
> atomic access by taking some kind of lock". We don't ever want to
> fall back to "take a lock" because sometimes our accesses to the
> atomic variables are from TCG generated code -- this is why we
> don't link against libatomic. The problem is that we have not
> correctly detected that this compiler can't do inline atomics
> for 64-bit values and avoided using them. But at least we have
> made this a compile failure rather than a silently-wrong-code bug:-)