[PATCH 6.1.y] KVM: arm64: silence -Wuninitialized-const-pointer warning

[PATCH 6.1.y] KVM: arm64: silence -Wuninitialized-const-pointer warning

[Justin Stitt]

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/00dacf8c22f065cb52efb14cd091d441f19b319e [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>

Re: [PATCH 6.1.y] KVM: arm64: silence -Wuninitialized-const-pointer warning

[Justin Stitt]

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/00dacf8c22f065cb52efb14cd091d441f19b319e [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>
> 

Re: [PATCH 6.1.y] KVM: arm64: silence -Wuninitialized-const-pointer warning

[Marc Zyngier]

[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/00dacf8c22f065cb52efb14cd091d441f19b319e [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.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH 6.1.y] KVM: arm64: silence -Wuninitialized-const-pointer warning

[Nathan Chancellor]

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

Re: [PATCH 6.1.y] KVM: arm64: silence -Wuninitialized-const-pointer warning

[Marc Zyngier]

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.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH 6.1.y] KVM: arm64: silence -Wuninitialized-const-pointer warning

[Greg KH]

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

Re: [PATCH 6.1.y] KVM: arm64: silence -Wuninitialized-const-pointer warning

[Nathan Chancellor]

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

Re: [PATCH 6.1.y] KVM: arm64: silence -Wuninitialized-const-pointer warning

[Greg KH]

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