Messages in this thread | | | From | Nick Desaulniers <> | Date | Thu, 6 Oct 2022 10:34:31 -0700 | Subject | Re: [PATCH v3] x86/fpu: use _Alignof to avoid UB in TYPE_ALIGN |
| |
On Thu, Oct 6, 2022 at 7:15 AM YingChi Long <me@inclyc.cn> wrote: > > WG14 N2350 made very clear that it is an UB having type definitions with > in "offsetof". This patch change the implementation of macro > "TYPE_ALIGN" to builtin "_Alignof" to avoid undefined behavior. > > I've grepped all source files to find any type definitions within > "offsetof". > > offsetof\(struct .*\{ .*, > > This implementation of macro "TYPE_ALIGN" seemes to be the only case of > type definitions within offsetof in the kernel codebase. > > I've made a clang patch that rejects any definitions within > __builtin_offsetof (usually #defined with "offsetof"), and tested > compiling with this patch, there are no error if this patch applied. > > ISO C11 _Alignof is subtly different from the GNU C extension > __alignof__. __alignof__ is the preferred alignment and _Alignof the > minimal alignment. For 'long long' on x86 these are 8 and 4 > respectively. > > The macro TYPE_ALIGN we're replacing has behavior that matches > _Alignof rather than __alignof__. > > Signed-off-by: YingChi Long <me@inclyc.cn> > Link: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2350.htm > Link: https://godbolt.org/z/sPs1GEhbT > Link: https://gcc.gnu.org/onlinedocs/gcc/Alignment.html > Link: https://reviews.llvm.org/D133574
Thanks for the patch! Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> --- > v3: > - commit message changes suggested by Nick and David > > v2: https://lore.kernel.org/all/20220927153338.4177854-1-me@inclyc.cn/ > --- > arch/x86/kernel/fpu/init.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/kernel/fpu/init.c b/arch/x86/kernel/fpu/init.c > index 621f4b6cac4a..de96c11e1fe9 100644 > --- a/arch/x86/kernel/fpu/init.c > +++ b/arch/x86/kernel/fpu/init.c > @@ -133,9 +133,6 @@ static void __init fpu__init_system_generic(void) > fpu__init_system_mxcsr(); > } > > -/* Get alignment of the TYPE. */ > -#define TYPE_ALIGN(TYPE) offsetof(struct { char x; TYPE test; }, test) > - > /* > * Enforce that 'MEMBER' is the last field of 'TYPE'. > * > @@ -143,8 +140,8 @@ static void __init fpu__init_system_generic(void) > * because that's how C aligns structs. > */ > #define CHECK_MEMBER_AT_END_OF(TYPE, MEMBER) \ > - BUILD_BUG_ON(sizeof(TYPE) != ALIGN(offsetofend(TYPE, MEMBER), \ > - TYPE_ALIGN(TYPE))) > + BUILD_BUG_ON(sizeof(TYPE) != \ > + ALIGN(offsetofend(TYPE, MEMBER), _Alignof(TYPE))) > > /* > * We append the 'struct fpu' to the task_struct: > -- > 2.35.1 >
-- Thanks, ~Nick Desaulniers
| |