BUG: use _Alignof rather than offsetof() on most compilers#23016
BUG: use _Alignof rather than offsetof() on most compilers#23016seberg merged 2 commits intonumpy:mainfrom
_Alignof rather than offsetof() on most compilers#23016Conversation
WG14 N2350 made very clear that it is an UB having type definitions within "offsetof" [1]. This patch enhances the implementation of macro _ALIGN to use builtin "_Alignof" to avoid undefined behavior on when using std=c11 or newer clang 16+ has started to flag this [2] Fixes build when using -std >= gnu11 and using clang16+ Older compilers gcc < 4.9 or clang < 8 has buggy _Alignof even though it may support C11, exclude those compilers too [1] https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2350.htm [2] https://reviews.llvm.org/D133574 Signed-off-by: Khem Raj <raj.khem@gmail.com>
|
It seems this is a duplicate of #23008 or am I missing something? |
|
This one has a bug mitigation, @hawkinsp does this seem right to you to include the mitigation? (Also maybe it is interesting to you.) |
|
@seberg This looks like a better fix than mine. Let's merge this one (and please add it to the backport list, if you wouldn't mind). |
|
By the way, I'm not sure if NumPy specifies a minimum set of compilers, but it might make sense to do so because then you have a way to determine if you need this kind of workaround. I'm thinking of something along the lines of the LLVM minimum compiler versions: https://llvm.org/docs/GettingStarted.html#host-c-toolchain-both-compiler-and-standard-library |
seberg
left a comment
There was a problem hiding this comment.
Thanks, two nits (could also prettify the rest, but don't know how and its fine). May just apply and merge later.
_Alignof rather than offsetof() on most compilers
…#23016) WG14 N2350 made very clear that it is an UB having type definitions within "offsetof" [1]. This patch enhances the implementation of macro _ALIGN to use builtin "_Alignof" to avoid undefined behavior on when using std=c11 or newer clang 16+ has started to flag this [2] Fixes build when using -std >= gnu11 and using clang16+ Older compilers gcc < 4.9 or clang < 8 has buggy _Alignof even though it may support C11, exclude those compilers too [1] https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2350.htm [2] https://reviews.llvm.org/D133574 Signed-off-by: Khem Raj <raj.khem@gmail.com> * Apply suggestions from code review Signed-off-by: Khem Raj <raj.khem@gmail.com> Co-authored-by: Sebastian Berg <sebastian@sipsolutions.net>
_Alignof rather than offsetof() on most compilers_Alignof rather than offsetof() on most compilers
WG14 N2350 made very clear that it is an UB having type definitions within "offsetof" [1]. This patch enhances the implementation of macro _ALIGN to use builtin "_Alignof" to avoid undefined behavior on when using std=c11 or newer
clang 16+ has started to flag this [2]
Fixes build when using -std >= gnu11 and using clang16+
Older compilers gcc < 4.9 or clang < 8 has buggy _Alignof even though it may support C11, exclude those compilers too
[1] https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2350.htm [2] https://reviews.llvm.org/D133574
Signed-off-by: Khem Raj raj.khem@gmail.com