Peter Maydell <[email protected]> writes: > If you compile QEMU with GCC with -fsanitize=address and > -Wstringop-overflow, this causes GCC to produce a false-positive > warning which it does not produce when the sanitizer is not enabled > (and which makes compilation fail if you're using -Werror, as we do > by default for builds from git): > > ../../hw/net/rtl8139.c: In function ‘rtl8139_io_writeb’: > ../../hw/net/rtl8139.c:2264:17: error: writing 8 bytes into a region of size > 0 [-Werror=stringop-overflow=] > 2264 | memcpy(data_to_checksum, saved_ip_header + 12, 8); > | ^ > In file included from ../../hw/net/rtl8139.c:62: > /home/pm215/qemu/include/net/eth.h:50:14: note: at offset [8, 48] into > destination object ‘ip_ver_len’ of size 1 > 50 | uint8_t ip_ver_len; /* version and header length */ > | ^~~~~~~~~~ > ../../hw/net/rtl8139.c:2192:21: error: writing 8 bytes into a region of size > 0 [-Werror=stringop-overflow=] > 2192 | memcpy(data_to_checksum, saved_ip_header + 12, 8); > | ^ > /home/pm215/qemu/include/net/eth.h:50:14: note: at offset [8, 48] into > destination object ‘ip_ver_len’ of size 1 > 50 | uint8_t ip_ver_len; /* version and header length */ > | ^~~~~~~~~~ > ../../hw/net/rtl8139.c:2192:21: error: writing 8 bytes into a region of size > 0 [-Werror=stringop-overflow=] > 2192 | memcpy(data_to_checksum, saved_ip_header + 12, 8); > | ^ > /home/pm215/qemu/include/net/eth.h:50:14: note: at offset [8, 48] into > destination object ‘ip_ver_len’ of size 1 > 50 | uint8_t ip_ver_len; /* version and header length */ > | ^~~~~~~~~~ > In file included from /home/pm215/qemu/include/system/memory.h:21, > from /home/pm215/qemu/include/hw/pci/pci.h:4, > from /home/pm215/qemu/include/hw/pci/pci_device.h:4, > from ../../hw/net/rtl8139.c:54: > In function ‘stl_he_p’, > inlined from ‘stl_be_p’ at /home/pm215/qemu/include/qemu/bswap.h:371:5, > inlined from ‘rtl8139_cplus_transmit_one’ at > ../../hw/net/rtl8139.c:2244:21, > inlined from ‘rtl8139_cplus_transmit’ at ../../hw/net/rtl8139.c:2345:28, > inlined from ‘rtl8139_io_writeb’ at ../../hw/net/rtl8139.c:2728:17: > /home/pm215/qemu/include/qemu/bswap.h:284:5: error: writing 4 bytes into a > region of size 0 [-Werror=stringop-overflow=] > 284 | __builtin_memcpy(ptr, &v, sizeof(v)); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > /home/pm215/qemu/include/net/eth.h: In function ‘rtl8139_io_writeb’: > /home/pm215/qemu/include/net/eth.h:50:14: note: at offset [24, 64] into > destination object ‘ip_ver_len’ of size 1 > 50 | uint8_t ip_ver_len; /* version and header length */ > | ^~~~~~~~~~ > > This has been triaged as a bug in GCC: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114494 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99673 > (the sanitizer pass rewrites the IR in a way that conflicts with its > use by the warning pass that runs afterwards). > > Since this is the only place in our code where we hit this, work > around it by disabling the -Wstringop-overflow in the part of > the function that hits it. We do this only when using the > address sanitizer on GCC, so that we still get the benefit > of the warning in most compilation scenarios. > > Cc: [email protected] > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3006 > Suggested-by: Daniel P. Berrangé <[email protected]> > Signed-off-by: Peter Maydell <[email protected]> > --- > v2: disable the warning rather than using the incorrect attempt > at a workaround that v1 had. > > On the fence about whether this is worth backporting to stable. > --- > hw/net/rtl8139.c | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c > index 2ad6338ebe..424af73a18 100644 > --- a/hw/net/rtl8139.c > +++ b/hw/net/rtl8139.c > @@ -2124,6 +2124,26 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s) > hlen, ip->ip_sum); > } > > + /* > + * The code in this function triggers a GCC bug where an > + * interaction between -fsanitize=address and -Wstringop-overflow > + * results in a false-positive stringop-overflow warning that is > + * only emitted when the address sanitizer is enabled: > + * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114494 > + * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99673 > + * GCC incorrectly thinks that the eth_payload_data buffer has > + * the type and size of the first field in 'struct ip_header', > i.e. > + * one byte, and then complains about all other attempts to > access > + * data in the buffer. > + * > + * Work around this by disabling the warning when building with > + * GCC and the address sanitizer is enabled. > + */ > +#pragma GCC diagnostic push > +#if !defined(__clang__) && defined(QEMU_SANITIZE_ADDRESS) > +#pragma GCC diagnostic ignored "-Wstringop-overflow" > +#endif > + > if ((txdw0 & CP_TX_LGSEN) && ip_protocol == IP_PROTO_TCP) > { > /* Large enough for the TCP header? */ > @@ -2307,6 +2327,9 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s) > /* restore IP header */ > memcpy(eth_payload_data, saved_ip_header, hlen); > } > + > +#pragma GCC diagnostic pop > + > } > > skip_offload:
Tested-by: Alex Bennée <[email protected]> Reviewed-by: Alex Bennée <[email protected]> -- Alex Bennée Virtualisation Tech Lead @ Linaro
