Skip to content

Commit

Permalink
spl/thread: explicitly define thread_func_t as noreturn
Browse files Browse the repository at this point in the history
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:
- llvm/llvm-project@1aad641
- llvm/llvm-project#7325
- llvm/llvm-project#41465

Sponsored-by: https://despairlabs.com/sponsor/
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Closes #16672 
Closes #16673
  • Loading branch information
robn authored Oct 21, 2024
1 parent 21cba06 commit 96f382d
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion include/os/linux/spl/sys/thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) \
Expand Down

0 comments on commit 96f382d

Please sign in to comment.