On Sun, Jun 09, 2019 at 01:30:12PM +0300, Valentine Barshak wrote: > Compiling with gcc 9.1 generates lots of "taking address of packed > member of ... may result in an unaligned pointer value" warnings: > > include/ipxe/uri.h: In function ‘uri_get’: > include/ipxe/uri.h:178:12: error: taking address of packed member of > ‘struct uri’ > may result in an unaligned pointer value [-Werror=address-of-packed-member] > 178 | ref_get ( &uri->refcnt ); > | ^~~~~~~~~~~~ > > The problem is that gcc does not check the alignment, but shows > lots of false positive warnings. For example, 'refcnt' is the first > member of the 'uri' structure, so its alignment is not affected > by the 'packed' attribute.
I took a closer look at it. It's a lot more messed up to say the least than it initially seemed. According to gcc documentation, the "packed" attribute doesn't just remove padding from the structure. It also tells the compiler to make no assumptions about structure alignment. This indeed indicates that the code might not work on some architectures. We may want to use "packed, aligned(X)" attributes in some cases to fix that. However, there's another issue. Even if we access a char or void pointer, gcc still shows the warning if the structure contains a VLA. For example: net/tls.c: In function ‘tls_send_client_hello’: net/tls.c:1091:47: error: taking address of packed member of ‘struct <anonymous>’ may result in an unaligned pointer value [-Werror=address-of-packed-member] 1091 | memcpy ( hello.extensions.server_name.list[0].name, session->name, | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~ The "name" is a byte array, so there should be no alignment issues whatsoever. But because the structure contains "uint8_t session_id[tls->session_id_len];", which is a Variable-Length Array, it shows the warning. If the session_id length was predefined, it would have worked regardless of the actual session_id length. I'm not sure how to nicely cope with this problem. The only thing I could come up with is converting "hello" structure to byte buffer and using offsetof, as follows: struct hello hello; char *buffer (char *)hello; ... extensions.server_name.list[0].name) memcpy ( buffer + offsetof(hello, extensions.server_name.list[0].name), session->name, ... It's not pretty, but it makes gcc happy. I'd really like to hear any suggestions from the maintainers because the problem has been around for more than a month now, and nobody seems to really care. The easiest would be to disable the warning. It doesn't break anything. You cannot break what's already broken. However, if we really want to support other architectures which are more sensitive to pointer alignment, the code has to be rewritten. > > This disables the warning to workaround the compilation issue. > > Signed-off-by: Valentine Barshak <gva...@gmail.com> > --- > src/Makefile.housekeeping | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/src/Makefile.housekeeping b/src/Makefile.housekeeping > index f8334921..3634c4b2 100644 > --- a/src/Makefile.housekeeping > +++ b/src/Makefile.housekeeping > @@ -185,6 +185,13 @@ WNST_TEST = $(CC) -Wstringop-truncation -x c -c > /dev/null -o /dev/null \ > >/dev/null 2>&1 > WNST_FLAGS := $(shell $(WNST_TEST) && $(ECHO) '-Wno-stringop-truncation') > WORKAROUND_CFLAGS += $(WNST_FLAGS) > + > +# gcc 9.1 generates false positive warnings for taking address of a packed > +# structure member which may result in an unaligned pointer. Turn them off. > +WNST_TEST = $(CC) -Wno-address-of-packed-member -x c -c /dev/null -o > /dev/null \ > + >/dev/null 2>&1 > +WNST_FLAGS := $(shell $(WNST_TEST) && $(ECHO) > '-Wno-address-of-packed-member') > +WORKAROUND_CFLAGS += $(WNST_FLAGS) > endif > > # Some versions of gas choke on division operators, treating them as > -- > 2.21.0 > -- Thanks, Val. _______________________________________________ ipxe-devel mailing list ipxe-devel@lists.ipxe.org https://lists.ipxe.org/mailman/listinfo.cgi/ipxe-devel