On Fri, Apr 12, 2024 at 9:08 AM Bill Wendling <morbo@xxxxxxxxxx> wrote:
>
> On Thu, Apr 11, 2024 at 11:45 AM Nathan Chancellor <nathan@xxxxxxxxxx> 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 <jstultz@xxxxxxxxxx>
> > Closes: https://lore.kernel.org/all/20240410232637.4135564-2-jstultz@xxxxxxxxxx/
> > Signed-off-by: Nathan Chancellor <nathan@xxxxxxxxxx>
> > ---
> > 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
