lkml.org 
[lkml]   [2021]   [Oct]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patches in this message
/
SubjectRe: [PATCH] compiler_types: mark __compiletime_assert failure as __noreturn
From
Date
On 14/10/2021 20.55, Nick Desaulniers wrote:
> On Thu, Oct 14, 2021 at 11:41 AM Miguel Ojeda
> <miguel.ojeda.sandonis@gmail.com> wrote:
>>
>> On Thu, Oct 14, 2021 at 8:33 PM Miguel Ojeda
>> <miguel.ojeda.sandonis@gmail.com> wrote:
>>>
>>> That would be a nice to do, but I am not sure about introducing one
>>> more macro about this... I think it would be simpler to submit patches
>>> for moves into `static_assert` even if we have to "flip" the meaning.
>
> $ grep -r BUILD_BUG_ON | wc -l
> 3405
>
>> Actually, what would be ideal is a compiler-backed lint that checks
>> whether it could be an `static_assert`, perhaps in clang-tidy?
>
> Oh, that is a good idea. There is one already for recommending the
> use of static_assert instead of assert. That's actually very nice.

So I did

diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index b6ff83a714ca..e212220216e8 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -295,12 +295,17 @@ struct ftrace_likely_data {
# define __compiletime_object_size(obj) -1
#endif

+#include <linux/const.h>
+
#ifdef __OPTIMIZE__
# define __compiletime_assert(condition, msg, prefix, suffix) \
do { \
extern void prefix ## suffix(void)
__compiletime_error(msg); \
+ extern void prefix ## suffix ## ice(void)
__compiletime_warning("Ice ice baby"); \
if (!(condition)) \
prefix ## suffix(); \
+ if (__is_constexpr(condition)) \
+ prefix ## suffix ## ice(); \
} while (0)
#else
# define __compiletime_assert(condition, msg, prefix, suffix) do { }
while (0)
and that throws a gazillion warnings. Picking one at random shows that
container_of() has a BUILD_BUG_ON. So we could do

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 2776423a587e..0a1969b11619 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -492,9 +492,9 @@ static inline void ftrace_dump(enum ftrace_dump_mode
oops_dump_mode) { }
*/
#define container_of(ptr, type, member) ({ \
void *__mptr = (void *)(ptr); \
- BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
- !__same_type(*(ptr), void), \
- "pointer type mismatch in container_of()"); \
+ static_assert(__same_type(*(ptr), ((type *)0)->member) || \
+ __same_type(*(ptr), void), \
+ "pointer type mismatch in container_of()"); \
((type *)(__mptr - offsetof(type, member))); })

/**
@@ -507,9 +507,9 @@ static inline void ftrace_dump(enum ftrace_dump_mode
oops_dump_mode) { }
*/
#define container_of_safe(ptr, type, member) ({
\
void *__mptr = (void *)(ptr); \
- BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
- !__same_type(*(ptr), void), \
- "pointer type mismatch in container_of()"); \
+ static_assert(__same_type(*(ptr), ((type *)0)->member) || \
+ __same_type(*(ptr), void), \
+ "pointer type mismatch in container_of_safe()"); \
IS_ERR_OR_NULL(__mptr) ? ERR_CAST(__mptr) : \
((type *)(__mptr - offsetof(type, member))); })
[fixing the copy-pasto in container_of_safe while at it].

Basically, all BUILD_BUG_ONs like that that do type checking are very
obviously candidates for using static_assert instead (they very
naturally go at the end of all the locally declared variables, so one
won't hit the declaration-after-statement problem that can otherwise
prevent using static_assert).

> I was playing with trying to adapt clang-tidy's C++11 `auto` fixit to
> work on GNU C code to automate the replacement of:
>
> __typeof(x) y = (x);
>
> with:
>
> __auto_type y = (x);
>
> in macros. That's perhaps interesting, too. Given the volume of code
> in the kernel, I wouldn't waste time with one off patches;

Well, for the kind of macros that are used _everywhere_ a few one-off
patches might be in order. It's also interesting if one could measure
any speedup from switching those core macros to static_assert.

Rasmus

\
 
 \ /
  Last update: 2021-10-15 09:56    [W:0.083 / U:0.104 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site