[PATCH 04/31] stddef: Avoid warning with clang with offsetof()

Simon Glass sjg at chromium.org
Thu Jan 13 14:50:08 CET 2022


Hi Tom,

On Thu, 13 Jan 2022 at 06:41, Tom Rini <trini at konsulko.com> wrote:
>
> On Thu, Jan 13, 2022 at 06:37:27AM -0700, Simon Glass wrote:
> > Hi Tom, Rasmus,
> >
> > On Thu, 13 Jan 2022 at 06:07, Tom Rini <trini at konsulko.com> wrote:
> > >
> > > On Thu, Jan 13, 2022 at 09:08:13AM +0100, Rasmus Villemoes wrote:
> > > > On 01/11/2021 02.17, Simon Glass wrote:
> > > > > Some bright sparks have decided that a cast on a constant cannot be a
> > > > > constant, so offsetof() produces this warning on clang-10:
> > > > >
> > > > > include/intel_gnvs.h:113:1: error: static_assert expression is not an
> > > > >     integral constant expression
> > > > > check_member(acpi_global_nvs, unused2, GNVS_CHROMEOS_ACPI_OFFSET);
> > > > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > > include/linux/kernel.h:284:2: note: expanded from macro 'check_member'
> > > > >         offsetof(struct structure, member) == (offset), \
> > > > >         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > > include/linux/stddef.h:20:32: note: expanded from macro 'offsetof'
> > > > >                                 ^
> > > > > include/intel_gnvs.h:113:1: note: cast that performs the conversions of
> > > > >     a reinterpret_cast is ot allowed in a constant expression
> > > > > include/linux/stddef.h:20:33: note: expanded from macro 'offsetof'
> > > > >
> > > > > Fix it by using the compiler built-in version, if available.
> > > > >
> > > > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > > > ---
> > > > >
> > > > >  include/linux/stddef.h | 8 +++++++-
> > > > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/include/linux/stddef.h b/include/linux/stddef.h
> > > > > index c540f6100d4..a7f546fdfe5 100644
> > > > > --- a/include/linux/stddef.h
> > > > > +++ b/include/linux/stddef.h
> > > > > @@ -1,6 +1,8 @@
> > > > >  #ifndef _LINUX_STDDEF_H
> > > > >  #define _LINUX_STDDEF_H
> > > > >
> > > > > +#include <linux/compiler_types.h>
> > > > > +
> > > > >  #undef NULL
> > > > >  #if defined(__cplusplus)
> > > > >  #define NULL 0
> > > > > @@ -14,7 +16,11 @@
> > > > >
> > > > >  #ifndef __CHECKER__
> > > > >  #undef offsetof
> > > > > -#define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
> > > > > +#ifdef __compiler_offsetof
> > > > > +#define offsetof(TYPE, MEMBER)     __compiler_offsetof(TYPE, MEMBER)
> > > > > +#else
> > > > > +#define offsetof(TYPE, MEMBER)     ((size_t)&((TYPE *)0)->MEMBER)
> > > > > +#endif
> > > > >  #endif
> > > >
> > > >
> > > > Can we please just drop the useless indirections? Any compiler we care
> > > > about provides __builtin_offsetof(), so just make the whole thing
> > > >
> > > >   #undef offsetof
> > > >   #define offsetof(TYPE, MEMBER)  __builtin_offsetof(TYPE, MEMBER)
> > > >
> > > > And by "compilers we care about", that includes sparse - it has had
> > > > __builtin_offsetof ever since 2007.
> > > >
> > > > And we can nuke the __compiler_offsetof definition in compiler_types.h,
> > > > and if any users exist, just mechanically convert them to offsetof().
> > >
> > > We should re-sync this with upstream, but that will keep the indirection
> > > you're objecting to here, I think.
> >
> > Yes, with this patch, this part of the file is the same as Linux (e.g. v5.16).
> >
> > So I'd like to apply it as is and deal with improvements later.
> > Perhaps in lockstep with Linux?
>
> Please re-word to be clear this is a functional re-sync with v5.16,
> thanks!

OK, I sent it and cc'd Rasmus too.

I was planning to apply the bloblist stuff this morning, so let me know...

Regards,
Simon


More information about the U-Boot mailing list