| Message ID | 155137655081.44146.7139636183950580495.stgit@bahia.lan |
|---|---|
| State | New |
| Headers | show |
| Series | slirp: Fix build with gcc 9 | expand |
On Thu, 28 Feb 2019 at 17:55, Greg Kurz <groug@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
Hello, Peter Maydell, le jeu. 28 févr. 2019 18:14:40 +0000, a ecrit: > 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. This looks like cargo-culted indeed. Greg, please repost with the size constraint, we do not need it. Samuel
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); #define NDP_TABLE_SIZE 16
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. Signed-off-by: Greg Kurz <groug@kaod.org> --- slirp/slirp.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)