Re: linux-4.20-rc1/arch/arm/vfp/vfpmodule.c:576: possible cut'n'paste error

Re: linux-4.20-rc1/arch/arm/vfp/vfpmodule.c:576: possible cut'n'paste error

[Russell King - ARM Linux]

On Mon, Nov 05, 2018 at 01:53:13PM +0000, David Binderman wrote:
> Hello there,
> 
> 2nd try. Plain text might help.

Yep, Linux kernel development generally doesn't like wasteful html
emails, sorry.

> linux-4.20-rc1/arch/arm/vfp/vfpmodule.c:576]: (warning) Redundant assignment of 'ufp_exc->fpinst2' to itself.
> 
> Source code is
> 
>        ufp_exc->fpexc = hwstate->fpexc;
>        ufp_exc->fpinst = hwstate->fpinst;
>        ufp_exc->fpinst2 = ufp_exc->fpinst2;

Thanks for the report - it most certainly is a bug introduced by
Julien's patches, but I don't get your warning here.  Which compiler
produces that warning?

Julien - unfortunately, I've just asked Linus to take another fix
for Spectre, so we're going to have to wait a bit before I can
submit something for this.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: linux-4.20-rc1/arch/arm/vfp/vfpmodule.c:576: possible cut'n'paste error

[Julien Thierry]

Hi Russel, David,

On 06/11/18 16:20, Russell King - ARM Linux wrote:
> On Mon, Nov 05, 2018 at 01:53:13PM +0000, David Binderman wrote:
>> Hello there,
>>
>> 2nd try. Plain text might help.
> 
> Yep, Linux kernel development generally doesn't like wasteful html
> emails, sorry.
> 
>> linux-4.20-rc1/arch/arm/vfp/vfpmodule.c:576]: (warning) Redundant assignment of 'ufp_exc->fpinst2' to itself.
>>
>> Source code is
>>
>>         ufp_exc->fpexc = hwstate->fpexc;
>>         ufp_exc->fpinst = hwstate->fpinst;
>>         ufp_exc->fpinst2 = ufp_exc->fpinst2;
> 
> Thanks for the report - it most certainly is a bug introduced by
> Julien's patches, but I don't get your warning here.  Which compiler
> produces that warning?
> 

Hmmm, silly typo/copy-paste from my end. Last line should be:

	ufp_exc->fpinst2 = hwstate->fpinst2;

Sorry about that.

> Julien - unfortunately, I've just asked Linus to take another fix
> for Spectre, so we're going to have to wait a bit before I can
> submit something for this.
> 

It is just a one line fix, should I submit a patch to the LAKML or patch 
system?
Otherwise let me know when is it best for you to receive the fix then to 
send to Linus.

Thanks,

-- 
Julien Thierry

Re: linux-4.20-rc1/arch/arm/vfp/vfpmodule.c:576: possible cut'n'paste error

[Russell King - ARM Linux]

On Tue, Nov 06, 2018 at 04:29:54PM +0000, Julien Thierry wrote:
> Hi Russel, David,
> 
> On 06/11/18 16:20, Russell King - ARM Linux wrote:
> >On Mon, Nov 05, 2018 at 01:53:13PM +0000, David Binderman wrote:
> >>Hello there,
> >>
> >>2nd try. Plain text might help.
> >
> >Yep, Linux kernel development generally doesn't like wasteful html
> >emails, sorry.
> >
> >>linux-4.20-rc1/arch/arm/vfp/vfpmodule.c:576]: (warning) Redundant assignment of 'ufp_exc->fpinst2' to itself.
> >>
> >>Source code is
> >>
> >>        ufp_exc->fpexc = hwstate->fpexc;
> >>        ufp_exc->fpinst = hwstate->fpinst;
> >>        ufp_exc->fpinst2 = ufp_exc->fpinst2;
> >
> >Thanks for the report - it most certainly is a bug introduced by
> >Julien's patches, but I don't get your warning here.  Which compiler
> >produces that warning?
> >
> 
> Hmmm, silly typo/copy-paste from my end. Last line should be:
> 
> 	ufp_exc->fpinst2 = hwstate->fpinst2;
> 
> Sorry about that.
> 
> >Julien - unfortunately, I've just asked Linus to take another fix
> >for Spectre, so we're going to have to wait a bit before I can
> >submit something for this.
> >
> 
> It is just a one line fix, should I submit a patch to the LAKML or patch
> system?

Both I guess.

> Otherwise let me know when is it best for you to receive the fix then to
> send to Linus.

As mentioned, I'll have to wait a while, it's not fair to send a pull
request for one patch followed immediately by another pull request for
another patch on top of the same pull request.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: linux-4.20-rc1/arch/arm/vfp/vfpmodule.c:576: possible cut'n'paste error

[Julien Thierry]

On 06/11/18 16:39, Russell King - ARM Linux wrote:
> On Tue, Nov 06, 2018 at 04:29:54PM +0000, Julien Thierry wrote:
>> Hi Russel, David,
>>
>> On 06/11/18 16:20, Russell King - ARM Linux wrote:
>>> On Mon, Nov 05, 2018 at 01:53:13PM +0000, David Binderman wrote:
>>>> Hello there,
>>>>
>>>> 2nd try. Plain text might help.
>>>
>>> Yep, Linux kernel development generally doesn't like wasteful html
>>> emails, sorry.
>>>
>>>> linux-4.20-rc1/arch/arm/vfp/vfpmodule.c:576]: (warning) Redundant assignment of 'ufp_exc->fpinst2' to itself.
>>>>
>>>> Source code is
>>>>
>>>>         ufp_exc->fpexc = hwstate->fpexc;
>>>>         ufp_exc->fpinst = hwstate->fpinst;
>>>>         ufp_exc->fpinst2 = ufp_exc->fpinst2;
>>>
>>> Thanks for the report - it most certainly is a bug introduced by
>>> Julien's patches, but I don't get your warning here.  Which compiler
>>> produces that warning?
>>>
>>
>> Hmmm, silly typo/copy-paste from my end. Last line should be:
>>
>> 	ufp_exc->fpinst2 = hwstate->fpinst2;
>>
>> Sorry about that.
>>
>>> Julien - unfortunately, I've just asked Linus to take another fix
>>> for Spectre, so we're going to have to wait a bit before I can
>>> submit something for this.
>>>
>>
>> It is just a one line fix, should I submit a patch to the LAKML or patch
>> system?
> 
> Both I guess.
> 
>> Otherwise let me know when is it best for you to receive the fix then to
>> send to Linus.
> 
> As mentioned, I'll have to wait a while, it's not fair to send a pull
> request for one patch followed immediately by another pull request for
> another patch on top of the same pull request.
> 

Makes sense. I'll send the fix in a short time.

Thanks,

-- 
Julien Thierry

Re: linux-4.20-rc1/arch/arm/vfp/vfpmodule.c:576: possible cut'n'paste error

[Russell King - ARM Linux]

On Tue, Nov 06, 2018 at 04:33:26PM +0000, David Binderman wrote:
> hello there Russell,
> 
> > linux-4.20-rc1/arch/arm/vfp/vfpmodule.c:576]: (warning) Redundant assignment of >'ufp_exc->fpinst2' to itself.
> 
> >Thanks for the report - it most certainly is a bug introduced by
> >Julien's patches, but I don't get your warning here.  Which compiler
> >produces that warning?
> 
> Not a compiler, cppcheck, a static analyser for C and C++ code.
> 
> Interestingly, more of the same in file linux-4.20-rc1/arch/arm/mach-pxa/pxa3xx.c
> 
> [linux-4.20-rc1/arch/arm/mach-pxa/pxa3xx.c:84]: (warning) Redundant assignment of 'ASCR' to itself.
> [linux-4.20-rc1/arch/arm/mach-pxa/pxa3xx.c:85]: (warning) Redundant assignment of 'ARSR' to itself.
> [linux-4.20-rc1/arch/arm/mach-pxa/pxa3xx.c:120]: (warning) Redundant assignment of 'ASCR' to itself.
> [linux-4.20-rc1/arch/arm/mach-pxa/pxa3xx.c:121]: (warning) Redundant assignment of 'ARSR' to itself.
> 
> I don't know if these four are also worth fixing.

There are cases where this can be false positives.  In the case of a
status register with write-1-to-clear bits for example.  These ones
look very much like that.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: linux-4.20-rc1/arch/arm/vfp/vfpmodule.c:576: possible cut'n'paste error

[Robert Jarzmik]

Russell King - ARM Linux <linux@armlinux.org.uk> writes:

>> Interestingly, more of the same in file linux-4.20-rc1/arch/arm/mach-pxa/pxa3xx.c
>> 
>> [linux-4.20-rc1/arch/arm/mach-pxa/pxa3xx.c:84]: (warning) Redundant assignment of 'ASCR' to itself.
>> [linux-4.20-rc1/arch/arm/mach-pxa/pxa3xx.c:85]: (warning) Redundant assignment of 'ARSR' to itself.
>> [linux-4.20-rc1/arch/arm/mach-pxa/pxa3xx.c:120]: (warning) Redundant assignment of 'ASCR' to itself.
>> [linux-4.20-rc1/arch/arm/mach-pxa/pxa3xx.c:121]: (warning) Redundant assignment of 'ARSR' to itself.
>> 
>> I don't know if these four are also worth fixing.
>
> There are cases where this can be false positives.  In the case of a
> status register with write-1-to-clear bits for example.  These ones
> look very much like that.
And indeed they are, for bits 2,1,0 for ASCR and 3,2,1,0 for ARSR, and you
guessed correctly they are partly status registers.

If only your checker could filter out __iomem tagged pointers, you could have
far less of these false positives ...

Cheers.

-- 
Robert