On Thu, 28 Feb 2019 at 17:55, Greg Kurz <gr...@kaod.org> wrote: > > Build fails with gcc 9: > > CC slirp/ndp_table.o > slirp/ndp_table.c: In function ‘ndp_table_add’: > slirp/ndp_table.c:31:23: error: taking address of packed member of ‘struct > ndpentry’ may result in an unaligned pointer value > [-Werror=address-of-packed-member] > 31 | if (in6_equal(&ndp_table->table[i].ip_addr, &ip_addr)) { > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ > slirp/ndp_table.c: In function ‘ndp_table_search’: > slirp/ndp_table.c:75:23: error: taking address of packed member of ‘struct > ndpentry’ may result in an unaligned pointer value > [-Werror=address-of-packed-member] > 75 | if (in6_equal(&ndp_table->table[i].ip_addr, &ip_addr)) { > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ > cc1: all warnings being treated as errors > > All members of struct ndpentry are naturally aligned. Drop the QEMU_PACKED > attribute and check there isn't unwanted padding at build time.
> diff --git a/slirp/slirp.h b/slirp/slirp.h > index 752a4cd8c81c..97552962697a 100644 > --- a/slirp/slirp.h > +++ b/slirp/slirp.h > @@ -106,7 +106,9 @@ bool arp_table_search(Slirp *slirp, uint32_t ip_addr, > struct ndpentry { > unsigned char eth_addr[ETH_ALEN]; /* sender hardware address */ > struct in6_addr ip_addr; /* sender IP address */ > -} SLIRP_PACKED; > +}; > + > +G_STATIC_ASSERT(sizeof(struct ndpentry) == 22); This relies on "struct in6_addr" being only byte-aligned, since ETH_ALEN is 6. Otherwise the ip_addr field will not be naturally aligned. in6_addr seems to just be a byte array in Linux at least in some configurations, but on NetBSD https://ftp.netbsd.org/pub/NetBSD/NetBSD-current/src/sys/netinet6/in6.h it's defined as a union some of whose members are going to want more alignment than that, so I suspect that this will fail to compile there (though I haven't checked). My question is: why is this struct marked packed at all? As far as I can see, the only use of the ntpentry type is in the NdpTable and the code in ndp_table.c that uses it. But that code does not use the struct to model on-the-wire data or anything else that would care about the struct layout: it only ever works with the struct by simple operations on its member fields. So I think the correct answer here is that we can drop SLIRP_PACKED entirely and do not need to try to assert that the struct has a particular size. My guess is that this got a packed attribute mistakenly by analogy with the struct slirp_arphdr which is used in the ArpTable struct -- but that struct really is used to match on-the-wire data, and this one is not. thanks -- PMM