On Tue, 3 Mar 2026 at 09:18, Daniel P. Berrangé <[email protected]> wrote:
>
> On Mon, Mar 02, 2026 at 10:32:38AM +0000, Peter Maydell wrote:
> > On Sun, 1 Mar 2026 at 15:15, Yodel Eldar <[email protected]> wrote:
> > >
> > > Hi, Lukas
> > >
> > > On 01/03/2026 05:13, Lukas Straub wrote:
> >
> > > > ../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/lukas/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 */
> > > >        |              ^~~~~~~~~~
> >
> > On the face of it, this looks like a compiler bug (warning false
> > positive), because we set:
> >
> >     uint8_t *data_to_checksum     = eth_payload_data + hlen - 12;
> >
> > and earlier
> >
> >             eth_payload_data = saved_buffer + ETH_HLEN;
> >
> > where
> >         uint8_t *saved_buffer  = s->cplus_txbuffer;
> >
> > and s->cplus_txbuffer is a uint8_t* which we set up via g_malloc().
> > None of that is an ip_ver_len byte, or even an ip_header struct.
> > So it looks like GCC has incorrectly decided that this uint8_t
> > buffer has a type which it does not.
>
> Yes, the problem is earlier code that also consumed eth_payload_data,
> by assigning it to a 'struct ip_header' pointer:
>
>             struct ip_header *ip = NULL;
>             ...
>             ip = (struct ip_header*)eth_payload_data;
>
> As a result GCC now believes that "eth_payload_data" has the
> 'struct ip_header *' type for the remainder of its life.

Andrew Pinski kindly found this bug in their system where it had
been previously reported:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114494
It's been marked as a dup of a different bug that's still open:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99673
(the sanitizer pass can rewrite the IL so that an address
expression is changed to one that has the same value but is
pointing into what the compiler thinks is a smaller object; the
warning-generation pass runs after that and assumes it can rely
on the sizes of things being pointed to in the IL).

So this is definitely a gcc bug, and it comes down to how much
we care about working around it.

> Eliminating the intermediate 'eth_payload_data' squashes the bogus
> type inference
>
>             ip = (struct ip_header*)saved_buffer + ETH_HLEN;

Mmm. This is a bit ugly though because it's expressing the
intent less well. Conceptually we have the packet payload
data in the buffer; then the eth level payload data is at
offset ETH_HLEN from that; then the IP header is at the
start of the eth level payload data.

-- PMM

Reply via email to