In order to build with clang at all, in order to see these symptoms, one must first apply Valentin Obst's build fix for LLVM [1]. Once that is done, then when building with clang, via:
make LLVM=1 -C tools/testing/selftests
...clang emits a "format string is empty" warning. (gcc also emits a similar warning.)
Fix by passing NULL, instead of "", for the msg argument to ksft_test_result_code(). This removes dozens of warnings and a few errors (some tests have -Werror set).
[1] https://lore.kernel.org/all/20240329-selftests-libmk-llvm-rfc-v1-1-2f9ed7d1c...
Cc: Valentin Obst kernel@valentinobst.de Cc: Kees Cook keescook@chromium.org Cc: Nick Desaulniers ndesaulniers@google.com Cc: Nathan Chancellor nathan@kernel.org Cc: Shuah Khan shuah@kernel.org Cc: Justin Stitt justinstitt@google.com Cc: Bill Wendling morbo@google.com Signed-off-by: John Hubbard jhubbard@nvidia.com --- tools/testing/selftests/kselftest_harness.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h index d98702b6955d..456b8694e678 100644 --- a/tools/testing/selftests/kselftest_harness.h +++ b/tools/testing/selftests/kselftest_harness.h @@ -1207,8 +1207,10 @@ void __run_test(struct __fixture_metadata *f, else diagnostic = "unknown";
- ksft_test_result_code(t->exit_code, test_name, - diagnostic ? "%s" : NULL, diagnostic); + if (diagnostic) + ksft_test_result_code(t->exit_code, test_name, "%s", diagnostic); + else + ksft_test_result_code(t->exit_code, test_name, NULL); free(test_name); }
base-commit: 18daea77cca626f590fb140fc11e3a43c5d41354 prerequisite-patch-id: b901ece2a5b78503e2fb5480f20e304d36a0ea27
Hi John,
On Tue, Apr 30, 2024 at 07:08:13PM -0700, John Hubbard wrote:
In order to build with clang at all, in order to see these symptoms, one must first apply Valentin Obst's build fix for LLVM [1]. Once that is done, then when building with clang, via:
make LLVM=1 -C tools/testing/selftests...clang emits a "format string is empty" warning. (gcc also emits a similar warning.)
The warning you are describing here sounds like the same exact one that commit caed8eba2215 ("selftests: kselftest_harness: fix Clang warning about zero-length format") should have addressed in 6.9-rc5. Is this patch actually necessary?
Fix by passing NULL, instead of "", for the msg argument to
Because this text is describing what was done in caed8eba2215...
ksft_test_result_code(). This removes dozens of warnings and a few errors (some tests have -Werror set).
[1] https://lore.kernel.org/all/20240329-selftests-libmk-llvm-rfc-v1-1-2f9ed7d1c...
Cc: Valentin Obst kernel@valentinobst.de Cc: Kees Cook keescook@chromium.org Cc: Nick Desaulniers ndesaulniers@google.com Cc: Nathan Chancellor nathan@kernel.org Cc: Shuah Khan shuah@kernel.org Cc: Justin Stitt justinstitt@google.com Cc: Bill Wendling morbo@google.com Signed-off-by: John Hubbard jhubbard@nvidia.com
tools/testing/selftests/kselftest_harness.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h index d98702b6955d..456b8694e678 100644 --- a/tools/testing/selftests/kselftest_harness.h +++ b/tools/testing/selftests/kselftest_harness.h @@ -1207,8 +1207,10 @@ void __run_test(struct __fixture_metadata *f, else diagnostic = "unknown";
- ksft_test_result_code(t->exit_code, test_name,
diagnostic ? "%s" : NULL, diagnostic);
- if (diagnostic)
ksft_test_result_code(t->exit_code, test_name, "%s", diagnostic);- else
ksft_test_result_code(t->exit_code, test_name, NULL);
but this diff is not doing that because it is based on a tree that has caed8eba2215; instead, it appears to be a completely identical transformation?
Cheers, Nathan
free(test_name); }
base-commit: 18daea77cca626f590fb140fc11e3a43c5d41354 prerequisite-patch-id: b901ece2a5b78503e2fb5480f20e304d36a0ea27 -- 2.45.0
On 5/1/24 8:13 AM, Nathan Chancellor wrote:
Hi John,
On Tue, Apr 30, 2024 at 07:08:13PM -0700, John Hubbard wrote:
In order to build with clang at all, in order to see these symptoms, one must first apply Valentin Obst's build fix for LLVM [1]. Once that is done, then when building with clang, via:
make LLVM=1 -C tools/testing/selftests...clang emits a "format string is empty" warning. (gcc also emits a similar warning.)
The warning you are describing here sounds like the same exact one that commit caed8eba2215 ("selftests: kselftest_harness: fix Clang warning about zero-length format") should have addressed in 6.9-rc5. Is this patch actually necessary?
Apparently not!
Fix by passing NULL, instead of "", for the msg argument to
Because this text is describing what was done in caed8eba2215...
Yes. I took a while between discovering the issue, and posting a fix, I think that's why.
ksft_test_result_code(). This removes dozens of warnings and a few errors (some tests have -Werror set).
[1] https://lore.kernel.org/all/20240329-selftests-libmk-llvm-rfc-v1-1-2f9ed7d1c...
Cc: Valentin Obst kernel@valentinobst.de Cc: Kees Cook keescook@chromium.org Cc: Nick Desaulniers ndesaulniers@google.com Cc: Nathan Chancellor nathan@kernel.org Cc: Shuah Khan shuah@kernel.org Cc: Justin Stitt justinstitt@google.com Cc: Bill Wendling morbo@google.com Signed-off-by: John Hubbard jhubbard@nvidia.com
tools/testing/selftests/kselftest_harness.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h index d98702b6955d..456b8694e678 100644 --- a/tools/testing/selftests/kselftest_harness.h +++ b/tools/testing/selftests/kselftest_harness.h @@ -1207,8 +1207,10 @@ void __run_test(struct __fixture_metadata *f, else diagnostic = "unknown";
- ksft_test_result_code(t->exit_code, test_name,
diagnostic ? "%s" : NULL, diagnostic);
- if (diagnostic)
ksft_test_result_code(t->exit_code, test_name, "%s", diagnostic);- else
ksft_test_result_code(t->exit_code, test_name, NULL);but this diff is not doing that because it is based on a tree that has caed8eba2215; instead, it appears to be a completely identical transformation?
It's nice to see that someone else had the same fix, anyway. Sorry about the noise, and yes let's please just drop this one.
thanks,
linux-kselftest-mirror@lists.linaro.org
-
John Hubbard -
Nathan Chancellor