On Fri, Apr 12, 2024 at 9:08 AM Bill Wendling <[email protected]> wrote: > > On Thu, Apr 11, 2024 at 11:45 AM Nathan Chancellor <[email protected]> wrote: > > > > After commit 6d029c25b71f ("selftests/timers/posix_timers: Reimplement > > check_timer_distribution()"), clang warns: > > > > tools/testing/selftests/timers/../kselftest.h:398:6: warning: variable > > 'major' is used uninitialized whenever '||' condition is true > > [-Wsometimes-uninitialized] > > 398 | if (uname(&info) || sscanf(info.release, "%u.%u.", > > &major, &minor) != 2) > > | ^~~~~~~~~~~~ > > tools/testing/selftests/timers/../kselftest.h:401:9: note: uninitialized > > use occurs here > > 401 | return major > min_major || (major == min_major && minor > > >= min_minor); > > | ^~~~~ > > tools/testing/selftests/timers/../kselftest.h:398:6: note: remove the > > '||' if its condition is always false > > 398 | if (uname(&info) || sscanf(info.release, "%u.%u.", > > &major, &minor) != 2) > > | ^~~~~~~~~~~~~~~ > > tools/testing/selftests/timers/../kselftest.h:395:20: note: initialize > > the variable 'major' to silence this warning > > 395 | unsigned int major, minor; > > | ^ > > | = 0 > > > > This is a false positive because if uname() fails, ksft_exit_fail_msg() > > will be called, which unconditionally calls exit(), a noreturn function. > > However, clang does not know that ksft_exit_fail_msg() will call exit() > > at the point in the pipeline that the warning is emitted because > > inlining has not occurred, so it assumes control flow will resume > > normally after ksft_exit_fail_msg() is called. > > > > Make it clear to clang that all of the functions that call exit() > > unconditionally in kselftest.h are noreturn transitively by marking them > > explicitly with '__attribute__((__noreturn__))', which clears up the > > warning above and any future warnings that may appear for the same > > reason. > > > > Fixes: 6d029c25b71f ("selftests/timers/posix_timers: Reimplement > > check_timer_distribution()") > > Reported-by: John Stultz <[email protected]> > > Closes: > > https://lore.kernel.org/all/[email protected]/ > > Signed-off-by: Nathan Chancellor <[email protected]> > > --- > > I have based this change on timers/urgent, as the commit that introduces > > this particular warning is there and it is marked for stable, even > > though this appears to be a generic kselftest issue. I think it makes > > the most sense for this change to go via timers/urgent with Shuah's ack. > > While __noreturn with a return type other than 'void' does not make much > > sense semantically, there are many places that these functions are used > > as the return value for other functions such as main(), so I did not > > change the return type of these functions from 'int' to 'void' to > > minimize the necessary changes for a backport (it is an existing issue > > anyways). > > > > I see there is another instance of this problem that will need to be > > addressed in -next, introduced by commit f07041728422 ("selftests: add > > ksft_exit_fail_perror()"). > > --- > > tools/testing/selftests/kselftest.h | 15 +++++++++------ > > 1 file changed, 9 insertions(+), 6 deletions(-) > > > > diff --git a/tools/testing/selftests/kselftest.h > > b/tools/testing/selftests/kselftest.h > > index 973b18e156b2..0591974b57e0 100644 > > --- a/tools/testing/selftests/kselftest.h > > +++ b/tools/testing/selftests/kselftest.h > > @@ -80,6 +80,9 @@ > > #define KSFT_XPASS 3 > > #define KSFT_SKIP 4 > > > > +#ifndef __noreturn > > +#define __noreturn __attribute__((__noreturn__)) > > +#endif > > #define __printf(a, b) __attribute__((format(printf, a, b))) > > > > /* counters */ > > @@ -300,13 +303,13 @@ void ksft_test_result_code(int exit_code, const char > > *test_name, > > va_end(args); > > } > > > > -static inline int ksft_exit_pass(void) > > +static inline __noreturn int ksft_exit_pass(void) > > Orthogonal to this fix, but why does a "no return" function have a > non-void return type? > Oops...That's been asked. Sorry for the extra message.
-bw
