[PULL 0/1] Libslirp update

[PULL 0/1] Libslirp update

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

The following changes since commit a2376507f615495b1d16685449ce0ea78c2caf9d:

  Merge remote-tracking branch 'remotes/bonzini-gitlab/tags/for-upstream' into staging (2021-07-24 11:04:57 +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 565301284e83fd5f12024a4e7959895380491668:

  Update libslirp to v4.6.1 (2021-07-28 13:11:11 +0400)

----------------------------------------------------------------
Update libslirp

Hi,

Let's update libslirp to 4.6.1, with its various fixes.

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

Marc-André Lureau (1):
  Update libslirp to v4.6.1

 slirp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.32.0.264.g75ae10bc75

[PULL 1/1] Update libslirp to v4.6.1

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Switch from stable-4.2 branch to upstream v4.6.1 release + fixes.

## [Unreleased]

### Fixed

 - Haiku fixes. !98 !99
 - Fix a minor DHCP regression introduced in 4.6.0. !97

## [4.6.1] - 2021-06-18

### Fixed

 - Fix DHCP regression introduced in 4.6.0. !95

## [4.6.0] - 2021-06-14

### Added

 - mbuf: Add debugging helpers for allocation. !90

### Changed

 -  Revert "Set macOS deployment target to macOS 10.4". !93

### Fixed

 - mtod()-related buffer overflows (CVE-2021-3592 #44, CVE-2021-3593 #45,
   CVE-2021-3594 #47, CVE-2021-3595 #46).
 - poll_fd: add missing fd registration for UDP and ICMP
 - ncsi: make ncsi_calculate_checksum work with unaligned data. !89
 - Various typos and doc fixes. !88

## [4.5.0] - 2021-05-18

### Added

 - IPv6 forwarding. !62 !75 !77
 - slirp_neighbor_info() to dump the ARP/NDP tables. !71

### Changed

 - Lazy guest address resolution for IPv6. !81
 - Improve signal handling when spawning a child. !61
 - Set macOS deployment target to macOS 10.4. !72
 - slirp_add_hostfwd: Ensure all error paths set errno. !80
 - More API documentation.

### Fixed

 - Assertion failure on unspecified IPv6 address. !86
 - Disable polling for PRI on MacOS, fixing some closing streams issues. !73
 - Various memory leak fixes on fastq/batchq. !68
 - Memory leak on IPv6 fast-send. !67
 - Slow socket response on Windows. !64
 - Misc build and code cleanups. !60 !63 !76 !79 !84

## [4.4.0] - 2020-12-02

### Added

 - udp, udp6, icmp: handle TTL value. !48
 - Enable forwarding ICMP errors. !49
 - Add DNS resolving for iOS. !54

### Changed

 - Improve meson subproject() support. !53
 - Removed Makefile-based build system. !56

### Fixed

 - socket: consume empty packets. !55
 - check pkt_len before reading protocol header (CVE-2020-29129). !57
 - ip_stripoptions use memmove (fixes undefined behaviour). !47
 - various Coverity-related changes/fixes.

## [4.3.1] - 2020-07-08

### Changed

 - A silent truncation could occur in `slirp_fmt()`, which will now print a
   critical message. See also #22.

### Fixed

 - CVE-2020-10756 - Drop bogus IPv6 messages that could lead to data leakage.
   See !44 and !42.
 - Fix win32 builds by using the SLIRP_PACKED definition.
 - Various coverity scan errors fixed. !41
 - Fix new GCC warnings. !43

## [4.3.0] - 2020-04-22

### Added

 - `SLIRP_VERSION_STRING` macro, with the git sha suffix when building from git
 - `SlirpConfig.disable_dns`, to disable DNS redirection #16

### Changed

 - `slirp_version_string()` now has the git sha suffix when building form git
 - Limit DNS redirection to port 53 #16

### Fixed

 - Fix build regression with mingw & NetBSD
 - Fix use-afte-free in `ip_reass()` (CVE-2020-1983)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Doug Evans <dje@google.com>
---
 slirp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/slirp b/slirp
index 8f43a99191..a88d9ace23 160000
--- a/slirp
+++ b/slirp
@@ -1 +1 @@
-Subproject commit 8f43a99191afb47ca3f3c6972f6306209f367ece
+Subproject commit a88d9ace234a24ce1c17189642ef9104799425e0
-- 
2.32.0.264.g75ae10bc75

Re: [PULL 0/1] Libslirp update

[Peter Maydell]

On Wed, 28 Jul 2021 at 11:51, <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> The following changes since commit a2376507f615495b1d16685449ce0ea78c2caf9d:
>
>   Merge remote-tracking branch 'remotes/bonzini-gitlab/tags/for-upstream' into staging (2021-07-24 11:04:57 +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 565301284e83fd5f12024a4e7959895380491668:
>
>   Update libslirp to v4.6.1 (2021-07-28 13:11:11 +0400)
>
> ----------------------------------------------------------------
> Update libslirp
>
> Hi,
>
> Let's update libslirp to 4.6.1, with its various fixes.
>

Fails to build, OSX, when linking the qemu-system-* executables:

Undefined symbols for architecture x86_64:
  "_res_9_getservers", referenced from:
      _get_dns_addr_libresolv in libslirp.a(slirp_src_slirp.c.o)
  "_res_9_nclose", referenced from:
      _get_dns_addr_libresolv in libslirp.a(slirp_src_slirp.c.o)
  "_res_9_ninit", referenced from:
      _get_dns_addr_libresolv in libslirp.a(slirp_src_slirp.c.o)
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

-- PMM

Re: [PULL 0/1] Libslirp update

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 1864 bytes --]

Hi

On Wed, Jul 28, 2021 at 7:23 PM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Wed, 28 Jul 2021 at 11:51, <marcandre.lureau@redhat.com> wrote:
> >
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > The following changes since commit
> a2376507f615495b1d16685449ce0ea78c2caf9d:
> >
> >   Merge remote-tracking branch
> 'remotes/bonzini-gitlab/tags/for-upstream' into staging (2021-07-24
> 11:04:57 +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 565301284e83fd5f12024a4e7959895380491668:
> >
> >   Update libslirp to v4.6.1 (2021-07-28 13:11:11 +0400)
> >
> > ----------------------------------------------------------------
> > Update libslirp
> >
> > Hi,
> >
> > Let's update libslirp to 4.6.1, with its various fixes.
> >
>
> Fails to build, OSX, when linking the qemu-system-* executables:
>
> Undefined symbols for architecture x86_64:
>   "_res_9_getservers", referenced from:
>       _get_dns_addr_libresolv in libslirp.a(slirp_src_slirp.c.o)
>   "_res_9_nclose", referenced from:
>       _get_dns_addr_libresolv in libslirp.a(slirp_src_slirp.c.o)
>   "_res_9_ninit", referenced from:
>       _get_dns_addr_libresolv in libslirp.a(slirp_src_slirp.c.o)
> ld: symbol(s) not found for architecture x86_64
> clang: error: linker command failed with exit code 1 (use -v to see
> invocation)
>
> -- PMM
>
>
Argh.. that's because we don't use it properly as a submodule, and libslirp
must link with 'resolv' on macos now.

I wish my previous pull request with the submodule change would receive
more help or attention, as I either couldn't reproduce the failure (neither
CI) or it was just some one-time warnings due to the transition...



-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 2645 bytes --]

Re: [PULL 0/1] Libslirp update

[Peter Maydell]

On Wed, 28 Jul 2021 at 16:47, Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
> I wish my previous pull request with the submodule change would
> receive more help or attention, as I either couldn't reproduce the
> failure (neither CI) or it was just some one-time warnings due to the
> transition...

Well, I reported the failures back to you. I can't do a lot more,
because libslirp development is now much more opaque to me because
it doesn't happen in-tree. So instead of "some small change happens and
we pick up issues with it early", you have to deal with all of
the accumulated problems at once when you update the submodule :-(

rc2 is on Tuesday, so we're starting to run short on time to
get an updated slirp in for 6.1.

-- PMM

Re: [PULL 0/1] Libslirp update

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 1233 bytes --]

Hi Peter

On Sun, Aug 1, 2021 at 4:10 PM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Wed, 28 Jul 2021 at 16:47, Marc-André Lureau
> <marcandre.lureau@gmail.com> wrote:
> > I wish my previous pull request with the submodule change would
> > receive more help or attention, as I either couldn't reproduce the
> > failure (neither CI) or it was just some one-time warnings due to the
> > transition...
>
> Well, I reported the failures back to you. I can't do a lot more,
> because libslirp development is now much more opaque to me because
> it doesn't happen in-tree. So instead of "some small change happens and
> we pick up issues with it early", you have to deal with all of
> the accumulated problems at once when you update the submodule :-(
>
> rc2 is on Tuesday, so we're starting to run short on time to
> get an updated slirp in for 6.1.
>
>
Do you mind checking the https://github.com/elmarco/qemu/tree/libslirp
branch?

From https://mail.gnu.org/archive/html/qemu-devel/2021-06/msg00031.html,
there would still be the one-time warnings from git, but osx and dist error
should be gone.

Only one left as a mystery is the Ubuntu-ASAN link issue.

thanks!

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 1967 bytes --]

Re: [PULL 0/1] Libslirp update

[Peter Maydell]

On Mon, 2 Aug 2021 at 19:58, Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> Hi Peter
>
> On Sun, Aug 1, 2021 at 4:10 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>>
>> On Wed, 28 Jul 2021 at 16:47, Marc-André Lureau
>> <marcandre.lureau@gmail.com> wrote:
>> > I wish my previous pull request with the submodule change would
>> > receive more help or attention, as I either couldn't reproduce the
>> > failure (neither CI) or it was just some one-time warnings due to the
>> > transition...
>>
>> Well, I reported the failures back to you. I can't do a lot more,
>> because libslirp development is now much more opaque to me because
>> it doesn't happen in-tree. So instead of "some small change happens and
>> we pick up issues with it early", you have to deal with all of
>> the accumulated problems at once when you update the submodule :-(
>>
>> rc2 is on Tuesday, so we're starting to run short on time to
>> get an updated slirp in for 6.1.
>>
>
> Do you mind checking the https://github.com/elmarco/qemu/tree/libslirp branch?

OK, I'm running that through a test build. (If it works, I think you'll
need to squash the two commits there together, to avoid breaking bisection
on OSX hosts.)

-- PMM

Re: [PULL 0/1] Libslirp update

[Peter Maydell]

On Mon, 2 Aug 2021 at 19:58, Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> Hi Peter
>
> On Sun, Aug 1, 2021 at 4:10 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>>
>> On Wed, 28 Jul 2021 at 16:47, Marc-André Lureau
>> <marcandre.lureau@gmail.com> wrote:
>> > I wish my previous pull request with the submodule change would
>> > receive more help or attention, as I either couldn't reproduce the
>> > failure (neither CI) or it was just some one-time warnings due to the
>> > transition...
>>
>> Well, I reported the failures back to you. I can't do a lot more,
>> because libslirp development is now much more opaque to me because
>> it doesn't happen in-tree. So instead of "some small change happens and
>> we pick up issues with it early", you have to deal with all of
>> the accumulated problems at once when you update the submodule :-(
>>
>> rc2 is on Tuesday, so we're starting to run short on time to
>> get an updated slirp in for 6.1.
>>
>
> Do you mind checking the https://github.com/elmarco/qemu/tree/libslirp branch?
>
> From https://mail.gnu.org/archive/html/qemu-devel/2021-06/msg00031.html, there would still be the one-time warnings from git, but osx and dist error should be gone.

Yep, I still see the git "warning: unable to rmdir 'slirp': Directory
not empty", but I think we can ignore that.

I also see
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

but I think that is also a one-off?

> Only one left as a mystery is the Ubuntu-ASAN link issue.

This one is still here:

subprojects/libslirp/libslirp.so.0.3.1.p/src_arp_table.c.o: In
function `arp_table_add':
/mnt/nvmedisk/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:51:
undefined reference to `__ubsan_handle_type_mismatch_v1'
/mnt/nvmedisk/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:51:
undefined reference to `__ubsan_handle_type_mismatch_v1'

when building the subprojects/libslirp/libslirp.so.0.3.1

configure options:
'../../configure' '--cc=clang' '--cxx=clang++' '--enable-gtk'
'--extra-cflags=-fsanitize=undefined  -fno-sanitize=shift-base
-Werror'

This happens because (as noted in the clang documentation for the
sanitizer: https://clang.llvm.org/docs/AddressSanitizer.html)
when linking a shared library with the sanitizers, clang does not
link in the sanitizer runtime library. That library is linked in
with the executable, and the shared library's references to the
sanitizer runtime functions are satisfied that way. However
you/meson are building libslirp.so with -Wl,--no-undefined
so the link of the .so fails.
(This does not happen with gcc, because gcc chose to make the
default for sanitizers to be to link against a shared libasan,
not a static one, the reverse of clang's default.)

What I don't understand is why we're building the .so at all.
I just tried a fresh build with
'../../configure' '--cc=clang' '--cxx=clang++' '--enable-gtk'
'--enable-sanitizers'
to check that telling configure (and possibly thus meson) about
the sanitizers more directly still demonstrated the problem:
but that sidesteps it because it never builds the .so.
My other build directories (the ones that do plain old gcc
builds with no sanitizer) seem to have built the .so file
as well, though, so this isn't related to either clang or to
the sanitizers -- meson just doesn't seem to be consistent
about what we build.

A related meson bug:
https://github.com/mesonbuild/meson/issues/764
(which was closed by just making meson warn if you tell it
to both use --no-undefined (which is the default) and to use
the sanitizer.)

The ideal fix seems to me to be to figure out why we're
building the libslirp .so and not do that.

A simple fix/workaround would be to set "b_lundef = false" in
default_options in your meson.build (which will suppress the
-Wl,--no-undefined option). That does mean you won't get
any warnings if you accidentally make libslirp use a function
that is provided by the QEMU executable, I suppose.

thanks
-- PMM

Re: [PULL 0/1] Libslirp update

[Peter Maydell]

On Mon, 2 Aug 2021 at 21:54, Peter Maydell <peter.maydell@linaro.org> wrote:
> A simple fix/workaround would be to set "b_lundef = false" in
> default_options in your meson.build (which will suppress the
> -Wl,--no-undefined option).

...though I just tried that as a local change in my tree
and it didn't seem to cause meson to actually change the
command line it was using to build the .so file...

-- PMM

Re: [PULL 0/1] Libslirp update

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 5875 bytes --]

Hi

On Tue, Aug 3, 2021 at 12:55 AM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Mon, 2 Aug 2021 at 19:58, Marc-André Lureau
> <marcandre.lureau@gmail.com> wrote:
> >
> > Hi Peter
> >
> > On Sun, Aug 1, 2021 at 4:10 PM Peter Maydell <peter.maydell@linaro.org>
> wrote:
> >>
> >> On Wed, 28 Jul 2021 at 16:47, Marc-André Lureau
> >> <marcandre.lureau@gmail.com> wrote:
> >> > I wish my previous pull request with the submodule change would
> >> > receive more help or attention, as I either couldn't reproduce the
> >> > failure (neither CI) or it was just some one-time warnings due to the
> >> > transition...
> >>
> >> Well, I reported the failures back to you. I can't do a lot more,
> >> because libslirp development is now much more opaque to me because
> >> it doesn't happen in-tree. So instead of "some small change happens and
> >> we pick up issues with it early", you have to deal with all of
> >> the accumulated problems at once when you update the submodule :-(
> >>
> >> rc2 is on Tuesday, so we're starting to run short on time to
> >> get an updated slirp in for 6.1.
> >>
> >
> > Do you mind checking the https://github.com/elmarco/qemu/tree/libslirp
> branch?
> >
> > From https://mail.gnu.org/archive/html/qemu-devel/2021-06/msg00031.html,
> there would still be the one-time warnings from git, but osx and dist error
> should be gone.
>
> Yep, I still see the git "warning: unable to rmdir 'slirp': Directory
> not empty", but I think we can ignore that.
>
> I also see
> 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
>
> but I think that is also a one-off?
>

yes


> > Only one left as a mystery is the Ubuntu-ASAN link issue.
>
> This one is still here:
>
> subprojects/libslirp/libslirp.so.0.3.1.p/src_arp_table.c.o: In
> function `arp_table_add':
>
> /mnt/nvmedisk/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:51:
> undefined reference to `__ubsan_handle_type_mismatch_v1'
>
> /mnt/nvmedisk/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:51:
> undefined reference to `__ubsan_handle_type_mismatch_v1'
>
> when building the subprojects/libslirp/libslirp.so.0.3.1
>
> configure options:
> '../../configure' '--cc=clang' '--cxx=clang++' '--enable-gtk'
> '--extra-cflags=-fsanitize=undefined  -fno-sanitize=shift-base
> -Werror'
>

I am not able to reproduce. Could you check the value of default_library
for libslirp when you run "meson configure". It should be "static".

I tested with "make vm-build-ubuntu.amd64", with tests/vm/ubuntu.amd64:
import sys
import basevm
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 clang",
}

class UbuntuX64VM(ubuntuvm.UbuntuVM):
    name = "ubuntu.amd64"
    arch = "x86_64"
    image_link="https://cloud-images.ubuntu.com/releases/bionic/"\
               "release-20191114/ubuntu-18.04-server-cloudimg-amd64.img"

image_sha256="0c55fded766f3e4efb082f604ed71dd58c8e81f04bd1a66b4ced80ad62617547"
    BUILD_SCRIPT = """
        set -e;
        cd $(mktemp -d);
        sudo chmod a+r /dev/vdb;
        tar -xf /dev/vdb;
        ./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};
    """

if __name__ == "__main__":
    sys.exit(basevm.main(UbuntuX64VM, DEFAULT_CONFIG))



> This happens because (as noted in the clang documentation for the
> sanitizer: https://clang.llvm.org/docs/AddressSanitizer.html)
> when linking a shared library with the sanitizers, clang does not
> link in the sanitizer runtime library. That library is linked in
> with the executable, and the shared library's references to the
> sanitizer runtime functions are satisfied that way. However
> you/meson are building libslirp.so with -Wl,--no-undefined
> so the link of the .so fails.
> (This does not happen with gcc, because gcc chose to make the
> default for sanitizers to be to link against a shared libasan,
> not a static one, the reverse of clang's default.)
>
> What I don't understand is why we're building the .so at all.
> I just tried a fresh build with
> '../../configure' '--cc=clang' '--cxx=clang++' '--enable-gtk'
> '--enable-sanitizers'
> to check that telling configure (and possibly thus meson) about
> the sanitizers more directly still demonstrated the problem:
> but that sidesteps it because it never builds the .so.
> My other build directories (the ones that do plain old gcc
> builds with no sanitizer) seem to have built the .so file
> as well, though, so this isn't related to either clang or to
> the sanitizers -- meson just doesn't seem to be consistent
> about what we build.
>
> A related meson bug:
> https://github.com/mesonbuild/meson/issues/764
> (which was closed by just making meson warn if you tell it
> to both use --no-undefined (which is the default) and to use
> the sanitizer.)
>
> The ideal fix seems to me to be to figure out why we're
> building the libslirp .so and not do that.
>
> A simple fix/workaround would be to set "b_lundef = false" in
> default_options in your meson.build (which will suppress the
> -Wl,--no-undefined option). That does mean you won't get
> any warnings if you accidentally make libslirp use a function
> that is provided by the QEMU executable, I suppose.
>
>
What if you pass --extra-ldflags='-fsanitize=undefined' then?

thanks

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 8208 bytes --]

Re: [PULL 0/1] Libslirp update

[Peter Maydell]

On Tue, 3 Aug 2021 at 09:30, Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
> On Tue, Aug 3, 2021 at 12:55 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>> This one is still here:
>>
>> subprojects/libslirp/libslirp.so.0.3.1.p/src_arp_table.c.o: In
>> function `arp_table_add':
>> /mnt/nvmedisk/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:51:
>> undefined reference to `__ubsan_handle_type_mismatch_v1'
>> /mnt/nvmedisk/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:51:
>> undefined reference to `__ubsan_handle_type_mismatch_v1'
>>
>> when building the subprojects/libslirp/libslirp.so.0.3.1
>>
>> configure options:
>> '../../configure' '--cc=clang' '--cxx=clang++' '--enable-gtk'
>> '--extra-cflags=-fsanitize=undefined  -fno-sanitize=shift-base
>> -Werror'
>
>
> I am not able to reproduce. Could you check the value of default_library for libslirp when you run "meson configure". It should be "static".

I never run "meson configure". I just run the QEMU "make".

Are you testing by starting with a before-the-libslirp-merge
change QEMU, doing a build, then updating to the libslirp
changes, and then doing a 'make' without reconfigure or
'make clean' ? I suspect this is perhaps to do with it being
an incremental build.

>> This happens because (as noted in the clang documentation for the
>> sanitizer: https://clang.llvm.org/docs/AddressSanitizer.html)
>> when linking a shared library with the sanitizers, clang does not
>> link in the sanitizer runtime library. That library is linked in
>> with the executable, and the shared library's references to the
>> sanitizer runtime functions are satisfied that way. However
>> you/meson are building libslirp.so with -Wl,--no-undefined
>> so the link of the .so fails.
>> (This does not happen with gcc, because gcc chose to make the
>> default for sanitizers to be to link against a shared libasan,
>> not a static one, the reverse of clang's default.)
>>
>> What I don't understand is why we're building the .so at all.
>> I just tried a fresh build with
>> '../../configure' '--cc=clang' '--cxx=clang++' '--enable-gtk'
>> '--enable-sanitizers'
>> to check that telling configure (and possibly thus meson) about
>> the sanitizers more directly still demonstrated the problem:
>> but that sidesteps it because it never builds the .so.
>> My other build directories (the ones that do plain old gcc
>> builds with no sanitizer) seem to have built the .so file
>> as well, though, so this isn't related to either clang or to
>> the sanitizers -- meson just doesn't seem to be consistent
>> about what we build.
>>
>> A related meson bug:
>> https://github.com/mesonbuild/meson/issues/764
>> (which was closed by just making meson warn if you tell it
>> to both use --no-undefined (which is the default) and to use
>> the sanitizer.)
>>
>> The ideal fix seems to me to be to figure out why we're
>> building the libslirp .so and not do that.
>>
>> A simple fix/workaround would be to set "b_lundef = false" in
>> default_options in your meson.build (which will suppress the
>> -Wl,--no-undefined option). That does mean you won't get
>> any warnings if you accidentally make libslirp use a function
>> that is provided by the QEMU executable, I suppose.
>>
>
> What if you pass --extra-ldflags='-fsanitize=undefined' then?

-fsanitize=undefined is already in the ldflags. That doesn't
help because clang is still going to decide not to statically
link libasan and give an error because of -Wl,--noundefined.

-- PMM

Re: [PULL 0/1] Libslirp update

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 4185 bytes --]

Hi

On Tue, Aug 3, 2021 at 1:09 PM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Tue, 3 Aug 2021 at 09:30, Marc-André Lureau
> <marcandre.lureau@gmail.com> wrote:
> > On Tue, Aug 3, 2021 at 12:55 AM Peter Maydell <peter.maydell@linaro.org>
> wrote:
> >> This one is still here:
> >>
> >> subprojects/libslirp/libslirp.so.0.3.1.p/src_arp_table.c.o: In
> >> function `arp_table_add':
> >>
> /mnt/nvmedisk/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:51:
> >> undefined reference to `__ubsan_handle_type_mismatch_v1'
> >>
> /mnt/nvmedisk/linaro/qemu-for-merges/build/clang/../../subprojects/libslirp/src/arp_table.c:51:
> >> undefined reference to `__ubsan_handle_type_mismatch_v1'
> >>
> >> when building the subprojects/libslirp/libslirp.so.0.3.1
> >>
> >> configure options:
> >> '../../configure' '--cc=clang' '--cxx=clang++' '--enable-gtk'
> >> '--extra-cflags=-fsanitize=undefined  -fno-sanitize=shift-base
> >> -Werror'
> >
> >
> > I am not able to reproduce. Could you check the value of default_library
> for libslirp when you run "meson configure". It should be "static".
>
> I never run "meson configure". I just run the QEMU "make".
>
>
Yes, here running "meson configure" after configure/make allows me to check
what meson has actually recorded.


> Are you testing by starting with a before-the-libslirp-merge
> change QEMU, doing a build, then updating to the libslirp
> changes, and then doing a 'make' without reconfigure or
> 'make clean' ? I suspect this is perhaps to do with it being
> an incremental build.
>
>
I tested with the "make vm-build-ubuntu.amd64" setup I gave before, so it
is a fresh build. Doing incremental build test is tedious, but I can give
it a try.

>> This happens because (as noted in the clang documentation for the
> >> sanitizer: https://clang.llvm.org/docs/AddressSanitizer.html)
> >> when linking a shared library with the sanitizers, clang does not
> >> link in the sanitizer runtime library. That library is linked in
> >> with the executable, and the shared library's references to the
> >> sanitizer runtime functions are satisfied that way. However
> >> you/meson are building libslirp.so with -Wl,--no-undefined
> >> so the link of the .so fails.
> >> (This does not happen with gcc, because gcc chose to make the
> >> default for sanitizers to be to link against a shared libasan,
> >> not a static one, the reverse of clang's default.)
> >>
> >> What I don't understand is why we're building the .so at all.
> >> I just tried a fresh build with
> >> '../../configure' '--cc=clang' '--cxx=clang++' '--enable-gtk'
> >> '--enable-sanitizers'
> >> to check that telling configure (and possibly thus meson) about
> >> the sanitizers more directly still demonstrated the problem:
> >> but that sidesteps it because it never builds the .so.
> >> My other build directories (the ones that do plain old gcc
> >> builds with no sanitizer) seem to have built the .so file
> >> as well, though, so this isn't related to either clang or to
> >> the sanitizers -- meson just doesn't seem to be consistent
> >> about what we build.
> >>
> >> A related meson bug:
> >> https://github.com/mesonbuild/meson/issues/764
> >> (which was closed by just making meson warn if you tell it
> >> to both use --no-undefined (which is the default) and to use
> >> the sanitizer.)
> >>
> >> The ideal fix seems to me to be to figure out why we're
> >> building the libslirp .so and not do that.
> >>
> >> A simple fix/workaround would be to set "b_lundef = false" in
> >> default_options in your meson.build (which will suppress the
> >> -Wl,--no-undefined option). That does mean you won't get
> >> any warnings if you accidentally make libslirp use a function
> >> that is provided by the QEMU executable, I suppose.
> >>
> >
> > What if you pass --extra-ldflags='-fsanitize=undefined' then?
>
> -fsanitize=undefined is already in the ldflags. That doesn't
> help because clang is still going to decide not to statically
> link libasan and give an error because of -Wl,--noundefined.
>

Ok


-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 5954 bytes --]

Re: [PULL 0/1] Libslirp update

[Peter Maydell]

On Tue, 3 Aug 2021 at 10:08, Peter Maydell <peter.maydell@linaro.org> wrote:
> Are you testing by starting with a before-the-libslirp-merge
> change QEMU, doing a build, then updating to the libslirp
> changes, and then doing a 'make' without reconfigure or
> 'make clean' ? I suspect this is perhaps to do with it being
> an incremental build.

More specifically:

$ git checkout master
$ mkdir build/slirptest
$ (cd build/slirptest && ../../configure --target-list=arm-softmmu
--enable-debug --disable-docs --disable-tools)
$ make -C build/slirptest -j8
$ git checkout remotes/elmarco/libslirp
$ make -C build/slirptest
$ mkdir build/slirp2
$ (cd build/slirptest && ../../configure --target-list=arm-softmmu
--enable-debug --disable-docs --disable-tools)
$ make -C build/slirp2

The build/slirptest directory has built a .so, and the
build/slirp2 directory has built a static .a.

(Both builds succeed because they're not hitting the clang
static analyzer thing, but they shouldn't be building different
types of library.)

Running '../../meson/meson.py configure' in slirp2 gives
(ignoring the non libslirp parts of the output):

Subproject libslirp:

  Core options                  Current Value
Possible Values
Description
  ------------                  -------------
---------------
-----------
  libslirp:default_library      static
[shared, static, both]                                        Default
library type
  libslirp:werror               true                           [true,
false]                                                 Treat warnings
as errors

  Project options               Current Value
Possible Values
Description
  ---------------               -------------
---------------
-----------
  libslirp:version_suffix
                                                      Suffix to append
to SLIRP_VERSION_STRING

In slirptest I get only:

Subproject libslirp:

  Project options               Current Value
Possible Values
Description
  ---------------               -------------
---------------
-----------
  libslirp:version_suffix
                                                      Suffix to append
to SLIRP_VERSION_STRING


So somehow meson has failed to apply the default_library and werror options in
the incremental build case.

-- PMM

Re: [PULL 0/1] Libslirp update

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 2977 bytes --]

Hi

On Tue, Aug 3, 2021 at 1:50 PM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Tue, 3 Aug 2021 at 10:08, Peter Maydell <peter.maydell@linaro.org>
> wrote:
> > Are you testing by starting with a before-the-libslirp-merge
> > change QEMU, doing a build, then updating to the libslirp
> > changes, and then doing a 'make' without reconfigure or
> > 'make clean' ? I suspect this is perhaps to do with it being
> > an incremental build.
>
> More specifically:
>
> $ git checkout master
> $ mkdir build/slirptest
> $ (cd build/slirptest && ../../configure --target-list=arm-softmmu
> --enable-debug --disable-docs --disable-tools)
> $ make -C build/slirptest -j8
> $ git checkout remotes/elmarco/libslirp
> $ make -C build/slirptest
> $ mkdir build/slirp2
> $ (cd build/slirptest && ../../configure --target-list=arm-softmmu
> --enable-debug --disable-docs --disable-tools)
> $ make -C build/slirp2
>
> The build/slirptest directory has built a .so, and the
> build/slirp2 directory has built a static .a.
>
> (Both builds succeed because they're not hitting the clang
> static analyzer thing, but they shouldn't be building different
> types of library.)
>
> Running '../../meson/meson.py configure' in slirp2 gives
> (ignoring the non libslirp parts of the output):
>
> Subproject libslirp:
>
>   Core options                  Current Value
> Possible Values
> Description
>   ------------                  -------------
> ---------------
> -----------
>   libslirp:default_library      static
> [shared, static, both]                                        Default
> library type
>   libslirp:werror               true                           [true,
> false]                                                 Treat warnings
> as errors
>
>   Project options               Current Value
> Possible Values
> Description
>   ---------------               -------------
> ---------------
> -----------
>   libslirp:version_suffix
>                                                       Suffix to append
> to SLIRP_VERSION_STRING
>
> In slirptest I get only:
>
> Subproject libslirp:
>
>   Project options               Current Value
> Possible Values
> Description
>   ---------------               -------------
> ---------------
> -----------
>   libslirp:version_suffix
>                                                       Suffix to append
> to SLIRP_VERSION_STRING
>
>
> So somehow meson has failed to apply the default_library and werror
> options in
> the incremental build case.
>
>
Thanks for the detailed steps. It turns out that incremental build with
subprojects is broken with meson < 0.57.2 (
https://github.com/mesonbuild/meson/pull/8424).

Either we acknowledge that, or we fix the qemu meson.build for now with the
missing -lresolv for osx/bsd.

I am going to work on a patch for the second option, but leave the decision
open.





-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 3991 bytes --]

Re: [PULL 0/1] Libslirp update

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 3351 bytes --]

Considering that I am planning the update to 0.58.2 in a couple weeks after
release, either -lresolv is fixed for 6.1, or the update should be delayed
to 6.2.

Paolo

Il mar 3 ago 2021, 13:52 Marc-André Lureau <marcandre.lureau@gmail.com> ha
scritto:

> Hi
>
> On Tue, Aug 3, 2021 at 1:50 PM Peter Maydell <peter.maydell@linaro.org>
> wrote:
>
>> On Tue, 3 Aug 2021 at 10:08, Peter Maydell <peter.maydell@linaro.org>
>> wrote:
>> > Are you testing by starting with a before-the-libslirp-merge
>> > change QEMU, doing a build, then updating to the libslirp
>> > changes, and then doing a 'make' without reconfigure or
>> > 'make clean' ? I suspect this is perhaps to do with it being
>> > an incremental build.
>>
>> More specifically:
>>
>> $ git checkout master
>> $ mkdir build/slirptest
>> $ (cd build/slirptest && ../../configure --target-list=arm-softmmu
>> --enable-debug --disable-docs --disable-tools)
>> $ make -C build/slirptest -j8
>> $ git checkout remotes/elmarco/libslirp
>> $ make -C build/slirptest
>> $ mkdir build/slirp2
>> $ (cd build/slirptest && ../../configure --target-list=arm-softmmu
>> --enable-debug --disable-docs --disable-tools)
>> $ make -C build/slirp2
>>
>> The build/slirptest directory has built a .so, and the
>> build/slirp2 directory has built a static .a.
>>
>> (Both builds succeed because they're not hitting the clang
>> static analyzer thing, but they shouldn't be building different
>> types of library.)
>>
>> Running '../../meson/meson.py configure' in slirp2 gives
>> (ignoring the non libslirp parts of the output):
>>
>> Subproject libslirp:
>>
>>   Core options                  Current Value
>> Possible Values
>> Description
>>   ------------                  -------------
>> ---------------
>> -----------
>>   libslirp:default_library      static
>> [shared, static, both]                                        Default
>> library type
>>   libslirp:werror               true                           [true,
>> false]                                                 Treat warnings
>> as errors
>>
>>   Project options               Current Value
>> Possible Values
>> Description
>>   ---------------               -------------
>> ---------------
>> -----------
>>   libslirp:version_suffix
>>                                                       Suffix to append
>> to SLIRP_VERSION_STRING
>>
>> In slirptest I get only:
>>
>> Subproject libslirp:
>>
>>   Project options               Current Value
>> Possible Values
>> Description
>>   ---------------               -------------
>> ---------------
>> -----------
>>   libslirp:version_suffix
>>                                                       Suffix to append
>> to SLIRP_VERSION_STRING
>>
>>
>> So somehow meson has failed to apply the default_library and werror
>> options in
>> the incremental build case.
>>
>>
> Thanks for the detailed steps. It turns out that incremental build with
> subprojects is broken with meson < 0.57.2 (
> https://github.com/mesonbuild/meson/pull/8424).
>
> Either we acknowledge that, or we fix the qemu meson.build for now with
> the missing -lresolv for osx/bsd.
>
> I am going to work on a patch for the second option, but leave the
> decision open.
>
>
>
>
>
> --
> Marc-André Lureau
>

[-- Attachment #2: Type: text/html, Size: 4617 bytes --]