Re: [Qemu-devel] [PULL 00/13] vhost, virtio, pci, pxe

[Peter Maydell]

On 19 February 2016 at 08:00, Michael S. Tsirkin <address@hidden> wrote:
> The following changes since commit a5af12871fd4601c44f08d9e49131e9ca13ef102:
>
>   Merge remote-tracking branch 'remotes/sstabellini/tags/xen-2016-02-12' into 
> staging (2016-02-12 17:36:12 +0000)
>
> are available in the git repository at:
>
>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>
> for you to fetch changes up to a28c393cc261afeb4863b05d7c33c2a5fc55ef38:
>
>   tests/vhost-user-bridge: add scattering of incoming packets (2016-02-18 
> 17:42:05 +0200)
>
> ----------------------------------------------------------------
> vhost, virtio, pci, pxe
>
> Fixes all over the place.
> New tests for pxe.
>
> Signed-off-by: Michael S. Tsirkin <address@hidden>

Hi. With this pullreq I now see new errors in the clang runtime
sanitizer config:

GTESTER check-qtest-i386
/home/petmay01/linaro/qemu-for-merges/slirp/slirp.c:751:13: runtime
error: index 1562 out of bounds for type 'char [1]'
/home/petmay01/linaro/qemu-for-merges/slirp/slirp.c:751:13: runtime
error: index 1562 out of bounds for type 'char [1]'

I think these are "we started testing something new and so the
sanitizer now warns about an old problem" issues. Still, it would
be nice to avoid introducing new sanitizer complaints into the
'make check' run if we can.

We can do that, I think, by updating slirp to use C99 flexible
arrays:

diff --git a/slirp/mbuf.h b/slirp/mbuf.h
index 38fedf4..ef5a4f7 100644
--- a/slirp/mbuf.h
+++ b/slirp/mbuf.h
@@ -81,11 +81,9 @@ struct mbuf {
        Slirp *slirp;
        bool    resolution_requested;
        uint64_t expiration_date;
+       char    *m_ext;
        /* start of dynamic buffer area, must be last element */
-       union {
-               char    m_dat[1]; /* ANSI don't like 0 sized arrays */
-               char    *m_ext;
-       };
+       char    m_dat[];
 };

 #define ifq_prev m_prev

[compile-tested, but need to audit the slirp code to check that
it doesn't care that m_dat and m_ext no longer have the same
offset within the struct... I'm assuming we don't care about
the fact we've used an extra 4 bytes of struct.]

However that then needs us to add a new disable-warning flag
-Wno-gnu-variable-sized-type-not-at-end, because slirp.h uses
these 'struct mbuf's embedded inside another structure, and that
is a GNU extension, which clang will default to complaining about.

So I guess I'll apply this pullreq for now and we can look
at improving the SLIRP code to be sanitizer-clean later.

thanks
-- PMM