linux-kernel - Re: arch/powerpc/kvm/powerpc.c:1141:7: warning: Redundant assignment of 'gpr' to itself. [selfAssignment]

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <87mtrczzr3.fsf@mpe.ellerman.id.au>
Date:   Sat, 26 Jun 2021 20:28:16 +1000
From:   Michael Ellerman <mpe@...erman.id.au>
To:     kernel test robot <lkp@...el.com>,
        Cédric Le Goater <clg@...d.org>
Cc:     kbuild-all@...ts.01.org, linux-kernel@...r.kernel.org
Subject: Re: arch/powerpc/kvm/powerpc.c:1141:7: warning: Redundant
 assignment of 'gpr' to itself. [selfAssignment]

kernel test robot <lkp@...el.com> writes:
> Hi Cédric,
>
> First bad commit (maybe != root cause):
>
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head:   44db63d1ad8d71c6932cbe007eb41f31c434d140
> commit: 9236f57a9e51c72ce426ccd2e53e123de7196a0f KVM: PPC: Make the VMX instruction emulation routines static
> date:   5 months ago
> compiler: powerpc64-linux-gcc (GCC) 9.3.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@...el.com>
>
>
> cppcheck warnings: (new ones prefixed by >>)
>>> arch/powerpc/kvm/powerpc.c:1141:7: warning: Redundant assignment of 'gpr' to itself. [selfAssignment]
>      gpr = sp_to_dp(gpr);
>          ^
>>> arch/powerpc/kvm/powerpc.c:1341:7: warning: Redundant assignment of 'val' to itself. [selfAssignment]
>      val = dp_to_sp(val);

Because of:

#ifdef CONFIG_PPC_FPU
static inline u64 sp_to_dp(u32 fprs)
{
	u64 fprd;

	preempt_disable();
	enable_kernel_fp();
	asm ("lfs%U1%X1 0,%1; stfd%U0%X0 0,%0" : "=m"UPD_CONSTR (fprd) : "m"UPD_CONSTR (fprs)
	     : "fr0");
	preempt_enable();
	return fprd;
}

static inline u32 dp_to_sp(u64 fprd)
{
	u32 fprs;

	preempt_disable();
	enable_kernel_fp();
	asm ("lfd%U1%X1 0,%1; stfs%U0%X0 0,%0" : "=m"UPD_CONSTR (fprs) : "m"UPD_CONSTR (fprd)
	     : "fr0");
	preempt_enable();
	return fprs;
}

#else
#define sp_to_dp(x)	(x)
#define dp_to_sp(x)	(x)
#endif /* CONFIG_PPC_FPU */


And you must be building the PPC_FPU=n case.

Surely the compiler is smart enough to generate no code for this. So
what's the harm? ie. why is this something we should be fixing?

cheers