lkml.org 
[lkml]   [2022]   [Oct]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH RESEND v3] x86/fpu: use _Alignof to avoid UB in TYPE_ALIGN
On Sat, Oct 29, 2022 at 5:27 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

YingChi,
You may retain my reviewed by tag when resending a rebased patch that
hasn't changed significantly.

Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

That reminds me, I need to resend one of my own patches; the x86
maintainers must be very busy.

> ---
> v3:
> - commit message changes suggested by Nick and David
>
> v2: https://lore.kernel.org/all/20220927153338.4177854-1-me@inclyc.cn/
> Signed-off-by: YingChi Long <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 8946f89761cc..851eb13edc01 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.37.4
>


--
Thanks,
~Nick Desaulniers

\
 
 \ /
  Last update: 2022-10-31 19:30    [W:0.101 / U:0.228 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site