[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <2fc36fd9-6c7a-492a-b5f8-65cfd52aadf2@paulmck-laptop>
Date: Wed, 23 Oct 2024 16:06:32 -0700
From: "Paul E. McKenney" <paulmck@...nel.org>
To: Ard Biesheuvel <ardb@...nel.org>
Cc: Dan Carpenter <dan.carpenter@...aro.org>,
clang-built-linux <llvm@...ts.linux.dev>,
Nathan Chancellor <nathan@...nel.org>, kobak@...dia.com,
mochs@...dia.com, rui.zhang@...el.com, rafael.j.wysocki@...el.com,
sfr@...b.auug.org.au, linux-kernel@...r.kernel.org,
linux-next@...r.kernel.org, linux-toolchains@...r.kernel.org
Subject: Re: [BUG] Argument-alignment build error with clang
On Thu, Oct 24, 2024 at 12:37:50AM +0200, Ard Biesheuvel wrote:
> (cc Dan, Nathan)
>
> On Thu, 24 Oct 2024 at 00:26, Paul E. McKenney <paulmck@...nel.org> wrote:
> >
> > Hello!
> >
> > Running rcutorture on next-20241023 got me lots of these:
> >
> > drivers/acpi/prmt.c:156:29: error: passing 1-byte aligned argument to 4-byte aligned parameter 1 of 'efi_pa_va_lookup' may result in an unaligned pointer access [-Werror,-Walign-mismatch]
> > 156 | (void *)efi_pa_va_lookup(&th->guid, handler_info->handler_address);
> >
> > This is built with CC=clang. I don't see this diagnostic with GCC.
> > But we are supposed to be able to build with clang, so...
> >
> > The first argument is the address of one of these:
> >
> > typedef struct {
> > __u8 b[UUID_SIZE];
> > } guid_t;
> >
> > Where UUID_SIZE is as follows:
> >
> > #define UUID_SIZE 16
> >
> > But this guid_t is a member of one of these:
> >
> > struct prm_handler_info {
> > guid_t guid;
> > efi_status_t (__efiapi *handler_addr)(u64, void *);
> > u64 static_data_buffer_addr;
> > u64 acpi_param_buffer_addr;
> >
> > struct list_head handler_list;
> > };
> >
> > One can argue that this structure must be 16-bit aligned on a
> > 64-bit build. So maybe this is a bug in clang's diagnostics, hence
> > linux-toolchains on CC.
> >
> > Thoughts?
> >
>
> Also discussed here:
> https://lore.kernel.org/all/CAMj1kXFXimHaGdeDBH3fOzuBiVcATA+JNpGqDs+m5h=8M_g+yA@mail.gmail.com/T/#u
>
> I agree that this looks like a spurious warning. Even if the alignment
> of the type is only 1 byte, the fact that it appears at the start of a
> 8-byte aligned non-packed struct guarantees sufficient alignment for
> this particular use of the type.
Thank you! I tried your s/guid_t/efi_guid_t/ change, and it works
fine here (see below in case I messed it up and got lucky). So:
Tested-by: Paul E. McKenney <paulmck@...nel.org>
Thanx, Paul
------------------------------------------------------------------------
diff --git a/drivers/acpi/prmt.c b/drivers/acpi/prmt.c
index d59307a76ca31..747f83f7114d2 100644
--- a/drivers/acpi/prmt.c
+++ b/drivers/acpi/prmt.c
@@ -52,7 +52,7 @@ struct prm_context_buffer {
static LIST_HEAD(prm_module_list);
struct prm_handler_info {
- guid_t guid;
+ efi_guid_t guid;
efi_status_t (__efiapi *handler_addr)(u64, void *);
u64 static_data_buffer_addr;
u64 acpi_param_buffer_addr;
Powered by blists - more mailing lists