* [PATCH] lib/base64: Silence clang-24 -Wconstant-conversion with diag pragmas
@ 2026-10-08 11:08 Nathan Chancellor
2026-10-09 16:59 ` Kuan-Wei Chiu
0 siblings, 1 reply; 3+ messages in thread
From: Nathan Chancellor @ 2026-10-08 11:08 UTC (permalink / raw)
To: Andrew Morton
Cc: Nick Desaulniers, Bill Wendling, Justin Stitt, Kuan-Wei Chiu,
Guan-Chun Wu, linux-kernel, llvm, stable, Nathan Chancellor
After a recent change in clang to warn on signed char conversions within
array initializers [1], there are several instances of this warning from
base64_rev_maps in lib/base64.c, which can break the build with W=e or
CONFIG_WERROR=y:
lib/base64.c:58:18: error: implicit conversion from 'int' to 's8' (aka 'signed char') changes value from 131 to -125 [-Werror,-Wconstant-conversion]
58 | [BASE64_IMAP] = BASE64_REV_INIT('+', ',')
| ^~~~~~~~~~~~~~~~~~~~~~~~~
lib/base64.c:52:2: note: expanded from macro 'BASE64_REV_INIT'
48 | #define BASE64_REV_INIT(ch_62, ch_63) { \
| ~
49 | [0 ... 0x1f] = -1, \
50 | INIT_32(0x20, ch_62, ch_63), \
51 | INIT_32(0x40, ch_62, ch_63), \
52 | INIT_32(0x60, ch_62, ch_63), \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
...
lib/base64.c:34:42: note: expanded from macro 'INIT_1'
34 | : (v) >= '0' && (v) <= '9' ? (v) - '0' + 52 \
| ~~~~~~~~~~^~~~
lib/base64.c:58:18: error: implicit conversion from 'int' to 's8' (aka 'signed char') changes value from 130 to -126 [-Werror,-Wconstant-conversion]
58 | [BASE64_IMAP] = BASE64_REV_INIT('+', ',')
| ^~~~~~~~~~~~~~~~~~~~~~~~~
lib/base64.c:52:2: note: expanded from macro 'BASE64_REV_INIT'
48 | #define BASE64_REV_INIT(ch_62, ch_63) { \
| ~
49 | [0 ... 0x1f] = -1, \
50 | INIT_32(0x20, ch_62, ch_63), \
51 | INIT_32(0x40, ch_62, ch_63), \
52 | INIT_32(0x60, ch_62, ch_63), \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
...
lib/base64.c:34:42: note: expanded from macro 'INIT_1'
34 | : (v) >= '0' && (v) <= '9' ? (v) - '0' + 52 \
| ~~~~~~~~~~^~~~
...
These are false positives, as the branch where the wraparound could
happen is unreachable with the values that clang reports. This is not
considered a bug by some clang folks [2][3], so silence the warnings
using the __diag macros the kernel has to workaround compiler warnings
when necessary.
Cc: stable@vger.kernel.org
Fixes: c4eb7ad32eab ("lib/base64: optimize base64_decode() with reverse lookup tables")
Closes: https://github.com/ClangBuiltLinux/linux/issues/2181
Link: https://github.com/llvm/llvm-project/commit/a5ef934a8d295dc03be3960f2b3744ec2e53238e [1]
Link: https://github.com/llvm/llvm-project/issues/223923#issuecomment-6056856245 [2]
Link: https://github.com/llvm/llvm-project/pull/226775#pullrequestreview-5454407389 [3]
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
lib/base64.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/lib/base64.c b/lib/base64.c
index 325c7332b049..e46be3a55585 100644
--- a/lib/base64.c
+++ b/lib/base64.c
@@ -52,11 +52,14 @@ static const char base64_tables[][65] = {
INIT_32(0x60, ch_62, ch_63), \
[0x80 ... 0xff] = -1 }
+__diag_push();
+__diag_ignore(clang, all, "-Wconstant-conversion", "https://github.com/llvm/llvm-project/issues/223923");
static const s8 base64_rev_maps[][256] = {
[BASE64_STD] = BASE64_REV_INIT('+', '/'),
[BASE64_URLSAFE] = BASE64_REV_INIT('-', '_'),
[BASE64_IMAP] = BASE64_REV_INIT('+', ',')
};
+__diag_pop();
#undef BASE64_REV_INIT
#undef INIT_32
---
base-commit: 0c2669a9f4a1d607e7591ae50ccf3c432a0aff08
change-id: 20261008-base64-silence-clang-24-constant-conversion-b682c6bbf563
Best regards,
--
Cheers,
Nathan
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] lib/base64: Silence clang-24 -Wconstant-conversion with diag pragmas
2026-10-08 11:08 [PATCH] lib/base64: Silence clang-24 -Wconstant-conversion with diag pragmas Nathan Chancellor
@ 2026-10-09 16:59 ` Kuan-Wei Chiu
2026-10-09 20:38 ` Nathan Chancellor
0 siblings, 1 reply; 3+ messages in thread
From: Kuan-Wei Chiu @ 2026-10-09 16:59 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Andrew Morton, Nick Desaulniers, Bill Wendling, Justin Stitt,
Guan-Chun Wu, linux-kernel, llvm, stable
On Thu, Oct 08, 2026 at 01:08:49PM +0200, Nathan Chancellor wrote:
> After a recent change in clang to warn on signed char conversions within
> array initializers [1], there are several instances of this warning from
> base64_rev_maps in lib/base64.c, which can break the build with W=e or
> CONFIG_WERROR=y:
>
> lib/base64.c:58:18: error: implicit conversion from 'int' to 's8' (aka 'signed char') changes value from 131 to -125 [-Werror,-Wconstant-conversion]
> 58 | [BASE64_IMAP] = BASE64_REV_INIT('+', ',')
> | ^~~~~~~~~~~~~~~~~~~~~~~~~
> lib/base64.c:52:2: note: expanded from macro 'BASE64_REV_INIT'
> 48 | #define BASE64_REV_INIT(ch_62, ch_63) { \
> | ~
> 49 | [0 ... 0x1f] = -1, \
> 50 | INIT_32(0x20, ch_62, ch_63), \
> 51 | INIT_32(0x40, ch_62, ch_63), \
> 52 | INIT_32(0x60, ch_62, ch_63), \
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> ...
> lib/base64.c:34:42: note: expanded from macro 'INIT_1'
> 34 | : (v) >= '0' && (v) <= '9' ? (v) - '0' + 52 \
> | ~~~~~~~~~~^~~~
> lib/base64.c:58:18: error: implicit conversion from 'int' to 's8' (aka 'signed char') changes value from 130 to -126 [-Werror,-Wconstant-conversion]
> 58 | [BASE64_IMAP] = BASE64_REV_INIT('+', ',')
> | ^~~~~~~~~~~~~~~~~~~~~~~~~
> lib/base64.c:52:2: note: expanded from macro 'BASE64_REV_INIT'
> 48 | #define BASE64_REV_INIT(ch_62, ch_63) { \
> | ~
> 49 | [0 ... 0x1f] = -1, \
> 50 | INIT_32(0x20, ch_62, ch_63), \
> 51 | INIT_32(0x40, ch_62, ch_63), \
> 52 | INIT_32(0x60, ch_62, ch_63), \
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> ...
> lib/base64.c:34:42: note: expanded from macro 'INIT_1'
> 34 | : (v) >= '0' && (v) <= '9' ? (v) - '0' + 52 \
> | ~~~~~~~~~~^~~~
> ...
>
> These are false positives, as the branch where the wraparound could
> happen is unreachable with the values that clang reports. This is not
> considered a bug by some clang folks [2][3], so silence the warnings
> using the __diag macros the kernel has to workaround compiler warnings
> when necessary.
>
> Cc: stable@vger.kernel.org
> Fixes: c4eb7ad32eab ("lib/base64: optimize base64_decode() with reverse lookup tables")
> Closes: https://github.com/ClangBuiltLinux/linux/issues/2181
> Link: https://github.com/llvm/llvm-project/commit/a5ef934a8d295dc03be3960f2b3744ec2e53238e [1]
> Link: https://github.com/llvm/llvm-project/issues/223923#issuecomment-6056856245 [2]
> Link: https://github.com/llvm/llvm-project/pull/226775#pullrequestreview-5454407389 [3]
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Acked-by: Kuan-Wei Chiu <visitorckw@gmail.com>
Reading the issue comment in the link, it seems somewhat subjective and
controversial whether the compiler should emit a warning in this
scenario. I'm not a compiler expert, but from a user's perspective,
it's a bit disappointing when we have to deal with this. When we are
confident that the code is correct and have provided the compiler with
enough context to determine that there is no runtime issue, suddenly
getting a new warning after an upgrade is a bit frustrating. But
anyway, let's go with this workaround to keep the compiler happy.
Regards,
Kuan-Wei
> ---
> lib/base64.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/lib/base64.c b/lib/base64.c
> index 325c7332b049..e46be3a55585 100644
> --- a/lib/base64.c
> +++ b/lib/base64.c
> @@ -52,11 +52,14 @@ static const char base64_tables[][65] = {
> INIT_32(0x60, ch_62, ch_63), \
> [0x80 ... 0xff] = -1 }
>
> +__diag_push();
> +__diag_ignore(clang, all, "-Wconstant-conversion", "https://github.com/llvm/llvm-project/issues/223923");
> static const s8 base64_rev_maps[][256] = {
> [BASE64_STD] = BASE64_REV_INIT('+', '/'),
> [BASE64_URLSAFE] = BASE64_REV_INIT('-', '_'),
> [BASE64_IMAP] = BASE64_REV_INIT('+', ',')
> };
> +__diag_pop();
>
> #undef BASE64_REV_INIT
> #undef INIT_32
>
> ---
> base-commit: 0c2669a9f4a1d607e7591ae50ccf3c432a0aff08
> change-id: 20261008-base64-silence-clang-24-constant-conversion-b682c6bbf563
>
> Best regards,
> --
> Cheers,
> Nathan
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] lib/base64: Silence clang-24 -Wconstant-conversion with diag pragmas
2026-10-09 16:59 ` Kuan-Wei Chiu
@ 2026-10-09 20:38 ` Nathan Chancellor
0 siblings, 0 replies; 3+ messages in thread
From: Nathan Chancellor @ 2026-10-09 20:38 UTC (permalink / raw)
To: Kuan-Wei Chiu
Cc: Andrew Morton, Nick Desaulniers, Bill Wendling, Justin Stitt,
Guan-Chun Wu, linux-kernel, llvm, stable
On Sat, Oct 10, 2026 at 12:59:01AM +0800, Kuan-Wei Chiu wrote:
> Reading the issue comment in the link, it seems somewhat subjective and
> controversial whether the compiler should emit a warning in this
> scenario. I'm not a compiler expert, but from a user's perspective,
> it's a bit disappointing when we have to deal with this. When we are
> confident that the code is correct and have provided the compiler with
> enough context to determine that there is no runtime issue, suddenly
> getting a new warning after an upgrade is a bit frustrating. But
> anyway, let's go with this workaround to keep the compiler happy.
Yes, I 100% agree. I have tried to make that a little clearer to the
clang folks, I guess we will see where that goes. Thanks for being
pragmatic here. We can always revisit this depending on how the upstream
discussions go but I would rather get this rolling on our side now, as
our CI has been broken for over three weeks and has missed other
regressions in the meantime.
--
Cheers,
Nathan
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-09 20:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-08 11:08 [PATCH] lib/base64: Silence clang-24 -Wconstant-conversion with diag pragmas Nathan Chancellor
2026-10-09 16:59 ` Kuan-Wei Chiu
2026-10-09 20:38 ` Nathan Chancellor
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®