On 4/11/24 12:45, Nathan Chancellor 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()").
Thank you. Assuming this is going through tip/timers/urgent Acked-by: Shuah Khan <[email protected]> Usama, please send patch fixing this problem in next on top of commit f07041728422 ("selftests: add
ksft_exit_fail_perror()").
thanks, -- Shuah
