> -----Original Message----- > From: Nick Desaulniers <ndesaulniers@xxxxxxxxxx> > Sent: Thursday, June 11, 2020 10:06 AM > To: Kaneda, Erik <erik.kaneda@xxxxxxxxx> > Cc: Jung-uk Kim <jkim@xxxxxxxxxxx>; Wysocki, Rafael J > <rafael.j.wysocki@xxxxxxxxx>; Ard Biesheuvel <ardb@xxxxxxxxxx>; > dvyukov@xxxxxxxxxx; glider@xxxxxxxxxx; linux-arm- > kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > lorenzo.pieralisi@xxxxxxx; mark.rutland@xxxxxxx; pcc@xxxxxxxxxx; > rjw@xxxxxxxxxxxxx; will@xxxxxxxxxx; stable@xxxxxxxxxxxxxxx; linux- > acpi@xxxxxxxxxxxxxxx; devel@xxxxxxxxxx > Subject: Re: [Devel] Re: [PATCH] ACPICA: fix UBSAN warning using > __builtin_offsetof > > On Thu, Jun 11, 2020 at 9:45 AM Kaneda, Erik <erik.kaneda@xxxxxxxxx> > wrote: > > > > > From: Jung-uk Kim <jkim@xxxxxxxxxxx> > > > > > > Actually, I think we should let platform-specific acfoo.h decide what to > > > do here, i.e., > > > > That's a better solution. For Linux, it would look something like this: > > > > --- a/include/acpi/actypes.h > > +++ b/include/acpi/actypes.h > > @@ -508,10 +508,15 @@ typedef u64 acpi_integer; > > > > #define ACPI_TO_POINTER(i) ACPI_CAST_PTR (void, (acpi_size) (i)) > > #define ACPI_TO_INTEGER(p) ACPI_PTR_DIFF (p, (void *) 0) > > -#define ACPI_OFFSET(d, f) ACPI_PTR_DIFF (&(((d *) 0)->f), (void *) > 0) > > #define ACPI_PHYSADDR_TO_PTR(i) ACPI_TO_POINTER(i) > > #define ACPI_PTR_TO_PHYSADDR(i) ACPI_TO_INTEGER(i) > > > > +/* Platforms may want to define their own ACPI_OFFSET */ > > + > > +#ifndef ACPI_OFFSET > > +#define ACPI_OFFSET(d, f) ACPI_PTR_DIFF (&(((d *) 0)->f), (void > *) 0) > > +#endif > > + > > /* Optimizations for 4-character (32-bit) acpi_name manipulation */ > > > > #ifndef ACPI_MISALIGNMENT_NOT_SUPPORTED > > diff --git a/include/acpi/platform/aclinux.h > b/include/acpi/platform/aclinux.h > > index 987e2af7c335..5d1ca6015fce 100644 > > --- a/include/acpi/platform/aclinux.h > > +++ b/include/acpi/platform/aclinux.h > > @@ -71,6 +71,11 @@ > > #undef ACPI_DEBUG_DEFAULT > > #define ACPI_DEBUG_DEFAULT (ACPI_LV_INFO | ACPI_LV_REPAIR) > > > > +/* Use gcc's builtin offset instead of the default */ > > + > > +#undef ACPI_OFFSET > > +#define ACPI_OFFSET(a,b) __builtin_offsetof(a,b) > > + > > #ifndef CONFIG_ACPI > > > Hi, Sorry about the delayed response. > Looks good at first glance. Wouldn't actypes.h need to include > platform/acenv.h first though? Otherwise you put some header > inclusion order dependency on folks who include actypes.h to first > include acenv.h otherwise we're not getting the definition in terms of > __builtin_offsetof. Actypes.h is intended for ACPICA use. For files using ACPI_OFFSET, they include headers like acpi.h that ends up including aclinux.h before including actypes.h. For those using ACPI_OFFSET, precedence will be given to the definition in aclinux.h before falling back to the definition in actypes. Erik > > -- > Thanks, > ~Nick Desaulniers
