Re: [PULL 0/1] Libslirp update

[Marc-André Lureau]

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