Hi
On Thu, 6 Apr 2023 at 09:16, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 4/6/23 01:48, Tom Rini wrote:
When building with clang we see this warning: field guid within 'struct efi_hii_keyboard_layout' is less aligned than 'efi_guid_t' and is usually due to 'struct efi_hii_keyboard_layout' being packed, which can lead to unaligned accesses [-Wunaligned-access]
Which is correct and true as EFI payloads are by specification allowed to do unaligned access. And so to silence the warning here and only here, we make use of #pragma to push/pop turning off the warning.
Signed-off-by: Tom Rini trini@konsulko.com
Cc: Heinrich Schuchardt xypron.glpk@gmx.de Cc: Ilias Apalodimas ilias.apalodimas@linaro.org
include/efi_api.h | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/include/efi_api.h b/include/efi_api.h index dc6e5ce236c9..f3bcbf593bea 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -1168,9 +1168,21 @@ struct efi_key_descriptor { u16 affected_attribute; } __packed;
+/* The warniing:
- field guid within 'struct efi_hii_keyboard_layout' is less aligned than 'efi_guid_t' and is usually due to 'struct efi_hii_keyboard_layout' being packed, which can lead to unaligned accesses [-Wunaligned-access]
- is intentional due to EFI requiring unaligned access to be supported.
- */ struct efi_hii_keyboard_layout { u16 layout_length;
+#ifdef CONFIG_CC_IS_CLANG +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wunaligned-access" +#endif efi_guid_t guid; +#ifdef CONFIG_CC_IS_CLANG +#pragma clang diagnostic pop +#endif
I don't think that the clang warning should be suppressed. We should replace efi_guid_t by u8[16] instead. This will force us to take the missing alignment into account when accessing the component in future.
efi_guid_t guid;
u8 guid[16];
I think what Heinrich suggests is fine, the EFI spec itself says that efi_guid must be 8-byte aligned unless otherwise specified. But we have another issue here. We aren't supposed to include structs that contain flexible length arrays into another array or struct. Quoting the C spec [0] Such a structure (and any union containing, possibly recursively, a member that is such a structure) shall not be a member of a structure or an element of an array. IOW efi_hii_keyboard_layout should not include struct efi_key_descriptor descriptors[]; since we include it at efi_hii_keyboard_package
Regards /Ilias
[0] https://www.dii.uchile.cl/~daespino/files/Iso_C_1999_definition.pdf chapter 6.7.2.1
Best regards
Heinrich
u32 layout_descriptor_string_offset; u8 descriptor_count; struct efi_key_descriptor descriptors[];