A new warning in Clang 22 [1] complains that @clidr passed to get_clidr_el1() is an uninitialized const pointer. get_clidr_el1() doesn't really care since it casts away the const-ness anyways.
Silence the warning by initializing the struct.
This patch won't apply to anything past v6.1 as this code section was reworked in Commit 7af0c2534f4c ("KVM: arm64: Normalize cache configuration").
Cc: stable@vger.kernel.org Fixes: 7c8c5e6a9101e ("arm64: KVM: system register handling") Link: https://github.com/llvm/llvm-project/commit/00dacf8c22f065cb52efb14cd091d441... [1] Signed-off-by: Justin Stitt justinstitt@google.com --- arch/arm64/kvm/sys_regs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index f4a7c5abcbca..d7ebd7387221 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -2948,7 +2948,7 @@ int kvm_sys_reg_table_init(void) { bool valid = true; unsigned int i; - struct sys_reg_desc clidr; + struct sys_reg_desc clidr = {0};
/* Make sure tables are unique and in order. */ valid &= check_sysreg_table(sys_reg_descs, ARRAY_SIZE(sys_reg_descs), false);
--- base-commit: 830b3c68c1fb1e9176028d02ef86f3cf76aa2476 change-id: 20250724-b4-clidr-unint-const-ptr-7edb960bc3bd
Best regards, -- Justin Stitt justinstitt@google.com
On Thu, Jul 24, 2025 at 06:15:28PM -0700, Justin Stitt wrote:
A new warning in Clang 22 [1] complains that @clidr passed to get_clidr_el1() is an uninitialized const pointer. get_clidr_el1() doesn't really care since it casts away the const-ness anyways.
Silence the warning by initializing the struct.
For posterity's sake, here's the warning message which I meant to include in the patch:
../arch/arm64/kvm/sys_regs.c:2978:23: warning: variable 'clidr' is uninitialized when passed as a const pointer argument here [-Wuninitialized-const-pointer] 2978 | get_clidr_el1(NULL, &clidr); /* Ugly... */
This patch won't apply to anything past v6.1 as this code section was reworked in Commit 7af0c2534f4c ("KVM: arm64: Normalize cache configuration").
Cc: stable@vger.kernel.org Fixes: 7c8c5e6a9101e ("arm64: KVM: system register handling") Link: https://github.com/llvm/llvm-project/commit/00dacf8c22f065cb52efb14cd091d441... [1] Signed-off-by: Justin Stitt justinstitt@google.com
arch/arm64/kvm/sys_regs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index f4a7c5abcbca..d7ebd7387221 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -2948,7 +2948,7 @@ int kvm_sys_reg_table_init(void) { bool valid = true; unsigned int i;
- struct sys_reg_desc clidr;
- struct sys_reg_desc clidr = {0};
/* Make sure tables are unique and in order. */ valid &= check_sysreg_table(sys_reg_descs, ARRAY_SIZE(sys_reg_descs), false);
base-commit: 830b3c68c1fb1e9176028d02ef86f3cf76aa2476 change-id: 20250724-b4-clidr-unint-const-ptr-7edb960bc3bd
Best regards,
Justin Stitt justinstitt@google.com
[Dropping a few emails from the list, as they are very likely to simply bounce]
On Fri, 25 Jul 2025 02:15:28 +0100, Justin Stitt justinstitt@google.com wrote:
A new warning in Clang 22 [1] complains that @clidr passed to get_clidr_el1() is an uninitialized const pointer. get_clidr_el1() doesn't really care since it casts away the const-ness anyways.
Silence the warning by initializing the struct.
This patch won't apply to anything past v6.1 as this code section was reworked in Commit 7af0c2534f4c ("KVM: arm64: Normalize cache configuration").
Cc: stable@vger.kernel.org Fixes: 7c8c5e6a9101e ("arm64: KVM: system register handling")
No, this really doesn't fix anything other than paper over an overzealous warning.
Link: https://github.com/llvm/llvm-project/commit/00dacf8c22f065cb52efb14cd091d441... [1] Signed-off-by: Justin Stitt justinstitt@google.com
arch/arm64/kvm/sys_regs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index f4a7c5abcbca..d7ebd7387221 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -2948,7 +2948,7 @@ int kvm_sys_reg_table_init(void) { bool valid = true; unsigned int i;
- struct sys_reg_desc clidr;
- struct sys_reg_desc clidr = {0};
/* Make sure tables are unique and in order. */ valid &= check_sysreg_table(sys_reg_descs, ARRAY_SIZE(sys_reg_descs), false);
Frankly, this sort of things is the worse you can do, as
- it perpetuates a bad design
- it is completely pointless, as you pointed out
- it is only going to make it harder to backport other patches
The correct fix would be to backport the series described in e8789ab7047a8, which should be easy enough to apply. it would also make 6.1 less of a terrible kernel.
M.
On Fri, Jul 25, 2025 at 08:30:21AM +0100, Marc Zyngier wrote:
The correct fix would be to backport the series described in e8789ab7047a8, which should be easy enough to apply. it would also make 6.1 less of a terrible kernel.
If doing that is reasonable to clear this up, I think that would be fine to do. This is the only stable-only instance of that warning that I have seen in the build logs, I have sent patches to deal with all the other instances upstream. We would need this in 5.15 to avoid failures from -Werror as well but if it is too hard to backport that series there, we could just disable this warning for this file since we know it is a false positive.
The whole reason the warning occurs is due to the constness of the sys_reg_desc parameter in the function created by FUNCTION_INVARIANT(), which I am guessing cannot be removed because it is present in ->access() and it proliferates out from there?
Cheers, Nathan
On Fri, 25 Jul 2025 17:26:54 +0100, Nathan Chancellor nathan@kernel.org wrote:
On Fri, Jul 25, 2025 at 08:30:21AM +0100, Marc Zyngier wrote:
The correct fix would be to backport the series described in e8789ab7047a8, which should be easy enough to apply. it would also make 6.1 less of a terrible kernel.
If doing that is reasonable to clear this up, I think that would be fine to do. This is the only stable-only instance of that warning that I have seen in the build logs, I have sent patches to deal with all the other instances upstream. We would need this in 5.15 to avoid failures from -Werror as well but if it is too hard to backport that series there, we could just disable this warning for this file since we know it is a false positive.
5.15 would be rather challenging, I'm afraid, and I wouldn't want to review such a thing.
The whole reason the warning occurs is due to the constness of the sys_reg_desc parameter in the function created by FUNCTION_INVARIANT(), which I am guessing cannot be removed because it is present in ->access() and it proliferates out from there?
Exactly. Which was a rather bad move when it was introduced over a decade ago (in v3.11), and we only got 'round to killing it entirely in v6.15.
Thanks,
M.
On Thu, Jul 24, 2025 at 06:15:28PM -0700, Justin Stitt wrote:
A new warning in Clang 22 [1] complains that @clidr passed to get_clidr_el1() is an uninitialized const pointer. get_clidr_el1() doesn't really care since it casts away the const-ness anyways.
Is clang-22 somehow now a supported kernel for the 6.1.y tree? Last I looked, Linus's tree doesn't even build properly for it, so why worry about this one just yet?
Silence the warning by initializing the struct.
Why not fix the compiler not to do this instead? We hate doing foolish work-arounds for broken compilers.
thanks,
greg k-h
On Fri, Jul 25, 2025 at 10:58:05AM +0200, Greg KH wrote:
On Thu, Jul 24, 2025 at 06:15:28PM -0700, Justin Stitt wrote:
A new warning in Clang 22 [1] complains that @clidr passed to get_clidr_el1() is an uninitialized const pointer. get_clidr_el1() doesn't really care since it casts away the const-ness anyways.
Is clang-22 somehow now a supported kernel for the 6.1.y tree? Last I looked, Linus's tree doesn't even build properly for it, so why worry about this one just yet?
Our goal is to have tip of tree LLVM / clang be able to build any supported branch of the kernel so that whenever it branches and releases, the fixes for it are already present in released kernel versions so users can just pick them up and go. We are going to have to worry about this at some point since it is a stable-only issue so why not tackle it now?
Silence the warning by initializing the struct.
Why not fix the compiler not to do this instead? We hate doing foolish work-arounds for broken compilers.
While casting away the const from the pointer in this case is "fine" because the object it pointed to was not const, I am fairly certain it is undefined behavior to cast away the const from a pointer to a const object, see commit 12051b318bc3 ("mips: avoid explicit UB in assignment of mips_io_port_base") for an exampile, so I am not sure the warning is entirely unreasonable.
Cheers, Nathan
On Fri, Jul 25, 2025 at 09:38:51AM -0700, Nathan Chancellor wrote:
On Fri, Jul 25, 2025 at 10:58:05AM +0200, Greg KH wrote:
On Thu, Jul 24, 2025 at 06:15:28PM -0700, Justin Stitt wrote:
A new warning in Clang 22 [1] complains that @clidr passed to get_clidr_el1() is an uninitialized const pointer. get_clidr_el1() doesn't really care since it casts away the const-ness anyways.
Is clang-22 somehow now a supported kernel for the 6.1.y tree? Last I looked, Linus's tree doesn't even build properly for it, so why worry about this one just yet?
Our goal is to have tip of tree LLVM / clang be able to build any supported branch of the kernel so that whenever it branches and releases, the fixes for it are already present in released kernel versions so users can just pick them up and go. We are going to have to worry about this at some point since it is a stable-only issue so why not tackle it now?
Silence the warning by initializing the struct.
Why not fix the compiler not to do this instead? We hate doing foolish work-arounds for broken compilers.
While casting away the const from the pointer in this case is "fine" because the object it pointed to was not const, I am fairly certain it is undefined behavior to cast away the const from a pointer to a const object, see commit 12051b318bc3 ("mips: avoid explicit UB in assignment of mips_io_port_base") for an exampile, so I am not sure the warning is entirely unreasonable.
Hah, we've been doing that for _decades_ with container_of(), so if that is UB, and the compiler can't handle it, I'd declare that a broken compiler :)
Look at e78f70bad29c ("time/timecounter: Fix the lie that struct cyclecounter is const") in linux-next as one example of me trying to fix that mess up. It's going to take a bunch of work to get there, but eventually we will. We will not be backporting all of those patches though, that would be way too much work.
Anyway, as the maintainer doesn't seem to want this, I guess I'll just ignore it for now?
thanks,
greg k-h
[ Sasha's backport helper bot ]
Hi,
Summary of potential issues: ⚠️ Could not find matching upstream commit
No upstream commit was identified. Using temporary commit for testing.
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | origin/linux-6.1.y | Success | Success |
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Justin Stitt -
Marc Zyngier -
Nathan Chancellor -
Sasha Levin