From 96f382d1132c42203b4307a3039edeebe918ef70 Mon Sep 17 00:00:00 2001 From: Rob Norris Date: Tue, 22 Oct 2024 05:38:30 +1100 Subject: [PATCH] spl/thread: explicitly define thread_func_t as noreturn All of our thread entry functions have this signature: void (*)(void*) __attribute__((noreturn)) The low-level `__thread_create()` function accepts a `thread_func_t` as the entry point, which is defined more simply as: void (*)(void *) And then the `thread_create()` and `thread_create_named()` macros cast the passed-in function point down to `thread_func_t`, that is, casting away the `noreturn` attribute. Clang considers casting between these two types to be invalid because both the caller and the callee may have elided parts of the stack frame save and restore, knowing that they won't be needed. Recent Linux appears to be setting `-Wcast-function-type-strict`, which causes this invalid cast to emit a warning, which with `-Werror` is converted to an error, breaking the build. This commit fixes this in the simplest possible way: adding `noreturn` to the `thread_func_t` attribute. Since all our thread entry functions already have this attribute, it's arguably a just a consistency fix anyway. I considered removing the casts in the macros, which silences the warnings, but it turns out that Clang has a bug that won't emit this error for implicit conversions, only explicit casts. So leaving them there seems like a reasonable belt-and-suspenders approach. Also, frankly, this whole mechanism seems a little undercooked inside LLVM, so I'm content go with my intuition about the smallest, least invaisve change. **NOTE**: `__thread_create` is exported by `spl.ko` and has a `thread_func_t` arg, so this is an ABI break. Whether that matters in practice, I have no idea. Further reading: - https://github.com/llvm/llvm-project/commit/1aad641c793090b4d036c03e737df2ebe2c32c57 - https://github.com/llvm/llvm-project/issues/7325 - https://github.com/llvm/llvm-project/issues/41465 Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf Signed-off-by: Rob Norris Closes #16672 Closes #16673 --- include/os/linux/spl/sys/thread.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/os/linux/spl/sys/thread.h b/include/os/linux/spl/sys/thread.h index 4f7f659e5..c7ef7efa0 100644 --- a/include/os/linux/spl/sys/thread.h +++ b/include/os/linux/spl/sys/thread.h @@ -42,7 +42,7 @@ #define TS_ZOMB EXIT_ZOMBIE #define TS_STOPPED TASK_STOPPED -typedef void (*thread_func_t)(void *); +typedef void (*thread_func_t)(void *) __attribute__((noreturn)); #define thread_create_named(name, stk, stksize, func, arg, len, \ pp, state, pri) \