Fails to build with GCC9

[superm1]

This was reported in Debian:

[...]
x86_64-linux-gnu-gcc -nostdlib -Wl,--warn-common -Wl,--no-undefined -Wl,--fatal-warnings -Wl,-shared -Wl,-Bsymbolic -L/usr/lib/x86_64-linux-gnu -L/usr/lib -Wl,--build-id=sha1 -Wl,--hash-style=sysv /usr/lib/crt0-efi-x86_64.o -o fakeesrt2.so fakeesrt2.o -lefi -lgnuefi \
    /usr/lib/gcc/x86_64-linux-gnu/9/libgcc.a \
    -T elf_x86_64_efi.lds
x86_64-linux-gnu-objcopy -j .text -j .sdata -j .data -j .dynamic -j .dynsym \
    -j .rel* -j .rela* -j .reloc -j .eh_frame \
    --target efi-app-x86_64 fakeesrt2.so fakeesrt2.efi
x86_64-linux-gnu-gcc -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -Wformat -Werror=format-security -fpic -Werror -Wall -Wextra -fshort-wchar -Wno-error=missing-field-initializers -Wno-missing-field-initializers -fno-merge-constants -ffreestanding -fno-stack-protector -fno-stack-check --std=gnu11 -DCONFIG_x86_64 -I/usr/include/efi/ -I/usr/include/efi/x86_64/ -iquote/<<PKGBUILDDIR>>/include "-DDEBUGDIR=L\"/\"" -mno-mmx -mno-sse -mno-red-zone -nostdinc -maccumulate-outgoing-args -DEFI_FUNCTION_WRAPPER -DGNU_EFI_USE_MS_ABI -I/usr/lib/gcc/x86_64-linux-gnu/9/include -c -o fakeesrt.o fakeesrt.c
x86_64-linux-gnu-gcc -nostdlib -Wl,--warn-common -Wl,--no-undefined -Wl,--fatal-warnings -Wl,-shared -Wl,-Bsymbolic -L/usr/lib/x86_64-linux-gnu -L/usr/lib -Wl,--build-id=sha1 -Wl,--hash-style=sysv /usr/lib/crt0-efi-x86_64.o -o fakeesrt.so fakeesrt.o -lefi -lgnuefi \
    /usr/lib/gcc/x86_64-linux-gnu/9/libgcc.a \
    -T elf_x86_64_efi.lds
x86_64-linux-gnu-objcopy -j .text -j .sdata -j .data -j .dynamic -j .dynsym \
    -j .rel* -j .rela* -j .reloc -j .eh_frame \
    --target efi-app-x86_64 fakeesrt.so fakeesrt.efi
x86_64-linux-gnu-gcc -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -Wformat -Werror=format-security -fpic -Werror -Wall -Wextra -fshort-wchar -Wno-error=missing-field-initializers -Wno-missing-field-initializers -fno-merge-constants -ffreestanding -fno-stack-protector -fno-stack-check --std=gnu11 -DCONFIG_x86_64 -I/usr/include/efi/ -I/usr/include/efi/x86_64/ -iquote/<<PKGBUILDDIR>>/include "-DDEBUGDIR=L\"/\"" -mno-mmx -mno-sse -mno-red-zone -nostdinc -maccumulate-outgoing-args -DEFI_FUNCTION_WRAPPER -DGNU_EFI_USE_MS_ABI -I/usr/lib/gcc/x86_64-linux-gnu/9/include -c -o dumpesrt.o dumpesrt.c
x86_64-linux-gnu-gcc -nostdlib -Wl,--warn-common -Wl,--no-undefined -Wl,--fatal-warnings -Wl,-shared -Wl,-Bsymbolic -L/usr/lib/x86_64-linux-gnu -L/usr/lib -Wl,--build-id=sha1 -Wl,--hash-style=sysv /usr/lib/crt0-efi-x86_64.o -o dumpesrt.so dumpesrt.o -lefi -lgnuefi \
    /usr/lib/gcc/x86_64-linux-gnu/9/libgcc.a \
    -T elf_x86_64_efi.lds
x86_64-linux-gnu-objcopy -j .text -j .sdata -j .data -j .dynamic -j .dynsym \
    -j .rel* -j .rela* -j .reloc -j .eh_frame \
    --target efi-app-x86_64 dumpesrt.so dumpesrt.efi
x86_64-linux-gnu-gcc -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -Wformat -Werror=format-security -fpic -Werror -Wall -Wextra -fshort-wchar -Wno-error=missing-field-initializers -Wno-missing-field-initializers -fno-merge-constants -ffreestanding -fno-stack-protector -fno-stack-check --std=gnu11 -DCONFIG_x86_64 -I/usr/include/efi/ -I/usr/include/efi/x86_64/ -iquote/<<PKGBUILDDIR>>/include "-DDEBUGDIR=L\"/\"" -mno-mmx -mno-sse -mno-red-zone -nostdinc -maccumulate-outgoing-args -DEFI_FUNCTION_WRAPPER -DGNU_EFI_USE_MS_ABI -I/usr/lib/gcc/x86_64-linux-gnu/9/include -c -o fwupdate.o fwupdate.c
In file included from /usr/include/efi/efi.h:41,
                 from fwupdate.c:11:
fwupdate.c: In function 'find_updates':
fwupdate.c:529:8: error: taking address of packed member of 'struct update_info_s' may result in an unaligned pointer value [-Werror=address-of-packed-member]
  529 |        &update->info->time_attempted,
      |        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/efi/x86_64/efibind.h:296:51: note: in definition of macro 'uefi_call_wrapper'
  296 | #define uefi_call_wrapper(func, va_num, ...) func(__VA_ARGS__)
      |                                                   ^~~~~~~~~~~
fwupdate.c: In function 'add_capsule':
fwupdate.c:1060:16: error: taking address of packed member of 'struct update_info_s' may result in an unaligned pointer value [-Werror=address-of-packed-member]
 1060 |  if ((guid_cmp(&update->info->guid, (efi_guid_t *)fbuf) == 0 ||
      |                ^~~~~~~~~~~~~~~~~~~
fwupdate.c:1076:41: error: taking address of packed member of 'struct update_info_s' may result in an unaligned pointer value [-Werror=address-of-packed-member]
 1076 |   if (!cap_out->Flags && !is_ux_capsule(&update->info->guid)) {
      |                                         ^~~~~~~~~~~~~~~~~~~
fwupdate.c:1023:39: note: in definition of macro 'is_ux_capsule'
 1023 | #define is_ux_capsule(guid) (guid_cmp(guid, &ux_capsule_guid) == 0)
      |                                       ^~~~
fwupdate.c:1099:22: error: taking address of packed member of 'struct update_info_s' may result in an unaligned pointer value [-Werror=address-of-packed-member]
 1099 |   if (!is_ux_capsule(&update->info->guid)) {
      |                      ^~~~~~~~~~~~~~~~~~~
fwupdate.c:1023:39: note: in definition of macro 'is_ux_capsule'
 1023 | #define is_ux_capsule(guid) (guid_cmp(guid, &ux_capsule_guid) == 0)
      |                                       ^~~~
fwupdate.c:1118:20: error: taking address of packed member of 'struct update_info_s' may result in an unaligned pointer value [-Werror=address-of-packed-member]
 1118 |  if (is_ux_capsule(&update->info->guid)) {
      |                    ^~~~~~~~~~~~~~~~~~~
fwupdate.c:1023:39: note: in definition of macro 'is_ux_capsule'
 1023 | #define is_ux_capsule(guid) (guid_cmp(guid, &ux_capsule_guid) == 0)
      |                                       ^~~~
fwupdate.c: In function 'efi_main':
fwupdate.c:1295:22: error: taking address of packed member of 'struct update_info_s' may result in an unaligned pointer value [-Werror=address-of-packed-member]
 1295 |        is_ux_capsule(&updates[i]->info->guid))
      |                      ^~~~~~~~~~~~~~~~~~~~~~~
fwupdate.c:1023:39: note: in definition of macro 'is_ux_capsule'
 1023 | #define is_ux_capsule(guid) (guid_cmp(guid, &ux_capsule_guid) == 0)
      |                                       ^~~~
cc1: all warnings being treated as errors
make[3]: *** [/<<PKGBUILDDIR>>/efi/Makefile:119: fwupdate.o] Error 1
rm fakeesrt.so dumpesrt.so fakeesrt2.o fakeesrt2.so fakeesrt.o dumpesrt.o
make[3]: Leaving directory '/<<PKGBUILDDIR>>/efi'
make[2]: *** [Makefile:16: all] Error 2
make[2]: Leaving directory '/<<PKGBUILDDIR>>'
dh_auto_build: make -j1 libdir=/usr/lib/x86_64-linux-gnu libexecdir=\${prefix}/lib/ datadir=/var/cache/ localedir=/usr/share/locale/ GNUEFIDIR=\${prefix}/lib CROSS_COMPILE=x86_64-linux-gnu- EFIDIR=debian returned exit code 2
make[1]: *** [debian/rules:23: override_dh_auto_build] Error 2
make[1]: Leaving directory '/<<PKGBUILDDIR>>'
make: *** [debian/rules:20: build-arch] Error 2
dpkg-buildpackage: error: debian/rules build-arch subprocess returned exit status 2

[hughsie]

FWIW, I don't think Werror=address-of-packed-member makes any sense.

[superm1]

At least reading the thought process behind it in clang:

Taking the address of a packed member is dangerous since the reduced
alignment of the pointee is lost. This can lead to memory alignment
faults in some architectures if the pointer value is dereferenced.

I don't know which architectures behave like that but in general it makes sense to me in that context.

I guess since it's a gcc9 default though, any project which thinks it's fine just needs to suppress those warnings.

[dankamongmen]

Please see my pull request #132 , which resolves this. The warning is appropriate and valid.

[ikrabbe]

I can't believe such a comment I don't think Werror=address-of-packed-member makes any sense If it makes a sense somewhere, then in a hardware and cross platform environment, such as some unified, run on any hardware and in any situation bootloader. I found a fedora bug report about grub "sometimes" not booting, because of memory algnment failures. I currently experience the same. I guess that's because of your great code. But you have been warned. Thank you! I will fix that bad stuff now and check if that works better.

[dankamongmen]

I can't believe such a comment I don't think Werror=address-of-packed-member makes any sense If it makes a sense somewhere, then in a hardware and cross platform environment, such as some unified, run on any hardware and in any situation bootloader. I found a fedora bug report about grub "sometimes" not booting, because of memory algnment failures. I currently experience the same. I guess that's because of your great code. But you have been warned. Thank you! I will fix that bad stuff now and check if that works better.

You can pull #132 and it'll fix it.

[rffontenelle]

FWIW, also happening on Arch Linux, when building the fwupdate package.

[superm1]

https://github.com/rhboot/fwupdate/pull/136