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

[Valentine]

On Sun, Jun 09, 2019 at 03:55:13PM +0200, Geert Stappers wrote:
> 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.
> 
> However is  refcnt also an structure,
> but that structure is not packed.

Sorry, not sure what difference it makes.

> 
> 
> > This disables the warning to workaround the compilation issue.
>  
> [ ... patch saying "Compiler, don't warn me about this , I know better" ... ]
> 

Is there a problem? It's been done before with -Wno-address and
-Wno-stringop-truncation.

> 
> If I had GCC 9.1  I would test this:

You can download and install one for free :)

> 
> --- a/src/include/ipxe/refcnt.h
> +++ b/src/include/ipxe/refcnt.h
> @@ -41,7 +41,7 @@ struct refcnt {
>          * first element of your reference-counted struct.
>          */
>         void ( * free ) ( struct refcnt *refcnt );
> -};
> +} __attribute__ (( packed ));
>  
>  /**
>   * Initialise a reference counter
> 

This adds another warning:
  In file included from include/ipxe/list.h:14,
                   from include/ipxe/iobuf.h:14,
                   from core/downloader.c:29:
  core/downloader.c: In function ‘downloader_free’:
  core/downloader.c:68:33: error: converting a packed ‘const volatile struct refcnt’ pointer (alignment 1)
  to a ‘struct downloader’ pointer
  (alignment 4) may result in an unaligned pointer value
>
> And also test with
>
> --- a/src/include/ipxe/uri.h
> +++ b/src/include/ipxe/uri.h
> @@ -70,7 +70,7 @@ struct uri {
>         const char *fragment;
>         /** Form parameters */
>         struct parameters *params;
> -} __attribute__ (( packed ));
> +};
>  
>  /**
>   * Access URI field
> 

With this change the 'refcnt' warning is not shown.
Next warning becomes the fist one shown:

  core/gdbudp.c:235:47: error: taking address of packed member of ‘struct sockaddr_in’
  may result in an unaligned pointer value [-Werror=address-of-packed-member]

As I've said before it generates about 1200 warnings like that when built with NO_WERROR=1.

However, I think that removing the "packed" attribute is a lot more dangerous
than disabling the warning. IMHO, If there was a problem with alignment
it would have shown on earlier gcc versions.

> 
> I leave this up to more knowledgable people.
> 
> It is "Patch has been seen" is what I wanted to tell    :-)

Thank you! That's more than I could have hoped for :)

> 
> 
> 
> Groeten
> Geert Stappers
> -- 
> Leven en laten leven
> _______________________________________________
> ipxe-devel mailing list
> ipxe-devel at lists.ipxe.org
> https://lists.ipxe.org/mailman/listinfo.cgi/ipxe-devel

-- 
Thanks,
Val.