[ipxe-devel] [PATCH 2/2] [build] Workaround compilation error with gcc 9.1

[Philippe Mathieu-Daudé]

Hi Valentine,

On 5/30/19 7:38 PM, 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.
> 
> For example:
> 
>   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 );
>         |            ^~~~~~~~~~~~
>   include/ipxe/refcnt.h:95:18: note: in definition of macro ‘ref_get’
>      95 |  ref_increment ( refcnt );     \
>         |                  ^~~~~~
>   include/ipxe/uri.h: In function ‘uri_put’:
>   include/ipxe/uri.h:189:12: error: taking address of packed member of ‘struct uri’
>   may result in an unaligned pointer value [-Werror=address-of-packed-member]
>     189 |  ref_put ( &uri->refcnt );
>         |            ^~~~~~~~~~~~
>   include/ipxe/refcnt.h:109:18: note: in definition of macro ‘ref_put’
>     109 |  ref_decrement ( refcnt );     \
>         |                  ^~~~~~
> 
> This disables the warning to workaround the compilation issue.

We should ask yourself why do we get this warning, before ignoring it.

If 'struct uri *' is not aligned, the ref_put/get functions will indeed
access unaligned pointer, and might trigger exceptions on some archs.

Following parse_uri() -> zalloc() -> malloc() -> realloc()

/**
 * Reallocate memory
 *
 * @v old_ptr		Memory previously allocated by malloc(), or NULL
 * @v new_size		Requested size
 * @ret new_ptr		Allocated memory, or NULL
 *
 * Allocates memory with no particular alignment requirement.  @c
 * new_ptr will be aligned to at least a multiple of sizeof(void*).
 ...

Is that enough to confirm 'struct uri *' is always aligned?

If so, this should be clearly documented in the commit message and in
the Makefile around WORKAROUND_CFLAGS += -Wno-address-of-packed-member.

Regards,

Phil.

> Signed-off-by: Valentine Barshak <gvaxon at gmail.com>
> ---
>  src/Makefile.housekeeping | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/src/Makefile.housekeeping b/src/Makefile.housekeeping
> index f8334921..ad9644c2 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 warnings for taking address of packed member which
> +# may result in an unaligned pointer value. Inhibit the warnings.
> +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
>