Re: [PATCH v13 06/30] powerpc/ptrace: Adapt gpr32_get, gpr32_set functions for transaction

From: Daniel Axtens <dja@axtens.net>
To: wei.guo.simon@gmail.com, linuxppc-dev@lists.ozlabs.org,
	linux-kernel@vger.kernel.org
Cc: Michael Ellerman <mpe@ellerman.id.au>,
	Shuah Khan <shuahkh@osg.samsung.com>,
	Anton Blanchard <anton@samba.org>, Cyril Bur <cyrilbur@gmail.com>,
	Anshuman Khandual <khandual@linux.vnet.ibm.com>,
	Simon Guo <wei.guo.simon@gmail.com>,
	Ulrich Weigand <ulrich.weigand@de.ibm.com>,
	Michael Neuling <mikey@neuling.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Kees Cook <keescook@chromium.org>,
	Rashmica Gupta <rashmicy@gmail.com>,
	Khem Raj <raj.khem@gmail.com>, Jessica Yu <jeyu@redhat.com>,
	Jiri Kosina <jkosina@suse.cz>, Miroslav Benes <mbenes@suse.cz>,
	Suraj Jitindar Singh <sjitindarsingh@gmail.com>,
	Chris Smart <chris@distroguy.com>,
	linux-kselftest@vger.kernel.org
Subject: Re: [PATCH v13 06/30] powerpc/ptrace: Adapt gpr32_get, gpr32_set functions for transaction
Date: Thu, 04 Aug 2016 16:36:22 +1000	[thread overview]
Message-ID: <877fbxqbvd.fsf@possimpible.ozlabs.ibm.com> (raw)
In-Reply-To: <1469674679-8580-7-git-send-email-wei.guo.simon@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2187 bytes --]

Hi all,

This is causing cppcheck warnings (having just landed in next):

[arch/powerpc/kernel/ptrace.c:2062]: (error) Uninitialized variable: ckpt_regs
[arch/powerpc/kernel/ptrace.c:2130]: (error) Uninitialized variable: ckpt_regs

This is from...
> -static int gpr32_get(struct task_struct *target,
> +static int gpr32_get_common(struct task_struct *target,
>  		     const struct user_regset *regset,
>  		     unsigned int pos, unsigned int count,
> -		     void *kbuf, void __user *ubuf)
> +			    void *kbuf, void __user *ubuf, bool tm_active)
>  {
>  	const unsigned long *regs = &target->thread.regs->gpr[0];
> +	const unsigned long *ckpt_regs;
>  	compat_ulong_t *k = kbuf;
>  	compat_ulong_t __user *u = ubuf;
>  	compat_ulong_t reg;
>  	int i;
>  
> -	if (target->thread.regs == NULL)
> -		return -EIO;
> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> +	ckpt_regs = &target->thread.ckpt_regs.gpr[0];
> +#endif
> +	if (tm_active) {
> +		regs = ckpt_regs;
... this bit here. If the ifdef doesn't trigger, cppcheck can't find an
initialisation for ckpt_regs, so it complains.

Techinically it's a false positive as (I assume!) tm_active cannot ever
be true in the absense of CONFIG_PPC_TRANSACTIONAL_MEM.

Is there a nice simple fix we could deploy to squash this warning, or
will we just live with it?

> -static int gpr32_set(struct task_struct *target,
> +static int gpr32_set_common(struct task_struct *target,
>  		     const struct user_regset *regset,
>  		     unsigned int pos, unsigned int count,
> -		     const void *kbuf, const void __user *ubuf)
> +		     const void *kbuf, const void __user *ubuf, bool tm_active)
>  {
>  	unsigned long *regs = &target->thread.regs->gpr[0];
> +	unsigned long *ckpt_regs;
>  	const compat_ulong_t *k = kbuf;
>  	const compat_ulong_t __user *u = ubuf;
>  	compat_ulong_t reg;
>  
> -	if (target->thread.regs == NULL)
> -		return -EIO;
> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> +	ckpt_regs = &target->thread.ckpt_regs.gpr[0];
> +#endif
>  
> -	CHECK_FULL_REGS(target->thread.regs);
> +	if (tm_active) {
> +		regs = ckpt_regs;
FWIW it happens again here.

Regards,
Daniel Axtens

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 859 bytes --]