[PATCH] sparc: fix array bounds error setting up PCIC NMI trap

[PATCH] sparc: fix array bounds error setting up PCIC NMI trap

[Ian Campbell]

From a sparc64 defconfig:
  CC      arch/sparc/kernel/pcic.o
arch/sparc/kernel/pcic.c: In function 'pcic_probe':
arch/sparc/kernel/pcic.c:359:33: error: array subscript is above array bounds [-Werror=array-bounds]
arch/sparc/kernel/pcic.c:359:8: error: array subscript is above array bounds [-Werror=array-bounds]
arch/sparc/kernel/pcic.c:360:33: error: array subscript is above array bounds [-Werror=array-bounds]
arch/sparc/kernel/pcic.c:360:8: error: array subscript is above array bounds [-Werror=array-bounds]
arch/sparc/kernel/pcic.c:361:33: error: array subscript is above array bounds [-Werror=array-bounds]
arch/sparc/kernel/pcic.c:361:8: error: array subscript is above array bounds [-Werror=array-bounds]
cc1: all warnings being treated as errors

I'm not particularly familiar with sparc but t_nmi (defined in head_32.S via
the TRAP_ENTRY macro) and pcic_nmi_trap_patch (defined in entry.S) both appear
to be 4 instructions long and I presume from the usage that instructions are
int sized.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: sparclinux@vger.kernel.org
---
 arch/sparc/kernel/pcic.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/sparc/kernel/pcic.c b/arch/sparc/kernel/pcic.c
index a19f041..1aaf8c1 100644
--- a/arch/sparc/kernel/pcic.c
+++ b/arch/sparc/kernel/pcic.c
@@ -352,8 +352,8 @@ int __init pcic_probe(void)
 	strcpy(pbm->prom_name, namebuf);
 
 	{
-		extern volatile int t_nmi[1];
-		extern int pcic_nmi_trap_patch[1];
+		extern volatile int t_nmi[4];
+		extern int pcic_nmi_trap_patch[4];
 
 		t_nmi[0] = pcic_nmi_trap_patch[0];
 		t_nmi[1] = pcic_nmi_trap_patch[1];
-- 
1.7.2.5

Re: [PATCH] sparc: fix array bounds error setting up PCIC NMI trap

[David Miller]

From: Ian Campbell <ian.campbell@citrix.com>
Date: Thu, 18 Aug 2011 09:14:57 +0100

>>From a sparc64 defconfig:
>   CC      arch/sparc/kernel/pcic.o

A sparc64 defconfig?  This is code only built for 32-bit sparc :-)

Re: [PATCH] sparc: fix array bounds error setting up PCIC NMI trap

[Ian Campbell]

On Thu, 2011-08-18 at 09:59 +0100, David Miller wrote:
> From: Ian Campbell <ian.campbell@citrix.com>
> Date: Thu, 18 Aug 2011 09:14:57 +0100
> 
> >>From a sparc64 defconfig:
> >   CC      arch/sparc/kernel/pcic.o
> 
> A sparc64 defconfig?  This is code only built for 32-bit sparc :-)

My local script confused me, it translated sparc64 into ARCH=sparc for
me, where it turns out the defconfig = sparc32_defconfig, so I was
actually building 32 bit. Sorry about that.

Turns out I also saw this on 32-bit sparc defconfig, which is hardly
surprising given the two .configs are exactly the same ;-)

Ian.

Re: [PATCH] sparc: fix array bounds error setting up PCIC NMI trap

[Sam Ravnborg]

On Thu, Aug 18, 2011 at 10:25:15AM +0100, Ian Campbell wrote:
> On Thu, 2011-08-18 at 09:59 +0100, David Miller wrote:
> > From: Ian Campbell <ian.campbell@citrix.com>
> > Date: Thu, 18 Aug 2011 09:14:57 +0100
> > 
> > >>From a sparc64 defconfig:
> > >   CC      arch/sparc/kernel/pcic.o
> > 
> > A sparc64 defconfig?  This is code only built for 32-bit sparc :-)
> 
> My local script confused me, it translated sparc64 into ARCH=sparc for
> me, where it turns out the defconfig = sparc32_defconfig, so I was
> actually building 32 bit. Sorry about that.
> 
> Turns out I also saw this on 32-bit sparc defconfig, which is hardly
> surprising given the two .configs are exactly the same ;-)
sparse complains too - I just never got around to fix it up.

If you do nto beat me I will look at it soon.

	Sam

Re: [PATCH] sparc: fix array bounds error setting up PCIC NMI trap

[Ian Campbell]

On Thu, 2011-08-18 at 10:49 +0100, Sam Ravnborg wrote:
> On Thu, Aug 18, 2011 at 10:25:15AM +0100, Ian Campbell wrote:
> > On Thu, 2011-08-18 at 09:59 +0100, David Miller wrote:
> > > From: Ian Campbell <ian.campbell@citrix.com>
> > > Date: Thu, 18 Aug 2011 09:14:57 +0100
> > > 
> > > >>From a sparc64 defconfig:
> > > >   CC      arch/sparc/kernel/pcic.o
> > > 
> > > A sparc64 defconfig?  This is code only built for 32-bit sparc :-)
> > 
> > My local script confused me, it translated sparc64 into ARCH=sparc for
> > me, where it turns out the defconfig = sparc32_defconfig, so I was
> > actually building 32 bit. Sorry about that.
> > 
> > Turns out I also saw this on 32-bit sparc defconfig, which is hardly
> > surprising given the two .configs are exactly the same ;-)
> sparse complains too - I just never got around to fix it up.
> 
> If you do nto beat me I will look at it soon.

With my patch applied I don't see any sparse warnings for these lines of
pcic.c, although there are others:
  CHECK   /local/scratch/ianc/devel/kernels/linux-2.6/arch/sparc/kernel/pcic.c
arch/sparc/kernel/pcic.c:163:14: warning: symbol 'pcic_regs' was not declared. Should it be static?
arch/sparc/kernel/pcic.c:164:14: warning: symbol 'pcic_speculative' was not declared. Should it be static?
arch/sparc/kernel/pcic.c:165:14: warning: symbol 'pcic_trapped' was not declared. Should it be static?
arch/sparc/kernel/pcic.c:331:66: warning: Using plain integer as NULL pointer
arch/sparc/kernel/pcic.c:343:66: warning: Using plain integer as NULL pointer
arch/sparc/kernel/pcic.c:539:38: warning: Using plain integer as NULL pointer
arch/sparc/kernel/pcic.c:677:1: warning: symbol 'pcic_pin_to_irq' was not declared. Should it be static?
arch/sparc/kernel/pcic.c:718:5: warning: symbol 'pci_gettimeoffset' was not declared. Should it be static?
arch/sparc/kernel/pcic.c:787:6: warning: symbol 'pcic_nmi' was not declared. Should it be static?
arch/sparc/kernel/pcic.c:883:5: warning: symbol 'pcibios_assign_resource' was not declared. Should it be static?
  CC      arch/sparc/kernel/pcic.o

Ian.

Re: [PATCH] sparc: fix array bounds error setting up PCIC NMI trap

[Sam Ravnborg]

Hi Ian.

On Thu, Aug 18, 2011 at 11:05:48AM +0100, Ian Campbell wrote:
> On Thu, 2011-08-18 at 10:49 +0100, Sam Ravnborg wrote:
> > On Thu, Aug 18, 2011 at 10:25:15AM +0100, Ian Campbell wrote:
> > > On Thu, 2011-08-18 at 09:59 +0100, David Miller wrote:
> > > > From: Ian Campbell <ian.campbell@citrix.com>
> > > > Date: Thu, 18 Aug 2011 09:14:57 +0100
> > > > 
> > > > >>From a sparc64 defconfig:
> > > > >   CC      arch/sparc/kernel/pcic.o
> > > > 
> > > > A sparc64 defconfig?  This is code only built for 32-bit sparc :-)
> > > 
> > > My local script confused me, it translated sparc64 into ARCH=sparc for
> > > me, where it turns out the defconfig = sparc32_defconfig, so I was
> > > actually building 32 bit. Sorry about that.
> > > 
> > > Turns out I also saw this on 32-bit sparc defconfig, which is hardly
> > > surprising given the two .configs are exactly the same ;-)
> > sparse complains too - I just never got around to fix it up.
> > 
> > If you do nto beat me I will look at it soon.

Somehow I missed that you already had provided a patch in the first mail.
this patch has my:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

> 
> With my patch applied I don't see any sparse warnings for these lines of
> pcic.c, although there are others:
>   CHECK   /local/scratch/ianc/devel/kernels/linux-2.6/arch/sparc/kernel/pcic.c
> arch/sparc/kernel/pcic.c:163:14: warning: symbol 'pcic_regs' was not declared. Should it be static?
> arch/sparc/kernel/pcic.c:164:14: warning: symbol 'pcic_speculative' was not declared. Should it be static?
> arch/sparc/kernel/pcic.c:165:14: warning: symbol 'pcic_trapped' was not declared. Should it be static?
> arch/sparc/kernel/pcic.c:331:66: warning: Using plain integer as NULL pointer
> arch/sparc/kernel/pcic.c:343:66: warning: Using plain integer as NULL pointer
> arch/sparc/kernel/pcic.c:539:38: warning: Using plain integer as NULL pointer
> arch/sparc/kernel/pcic.c:677:1: warning: symbol 'pcic_pin_to_irq' was not declared. Should it be static?
> arch/sparc/kernel/pcic.c:718:5: warning: symbol 'pci_gettimeoffset' was not declared. Should it be static?
> arch/sparc/kernel/pcic.c:787:6: warning: symbol 'pcic_nmi' was not declared. Should it be static?
> arch/sparc/kernel/pcic.c:883:5: warning: symbol 'pcibios_assign_resource' was not declared. Should it be static?
>   CC      arch/sparc/kernel/pcic.oA

Trivial stuff - mostly missing extern stuff.
But we should kill these warnings soon so they do not hide more serious warnings.

	Sam

Re: [PATCH] sparc: fix array bounds error setting up PCIC NMI trap

[David Miller]

From: Sam Ravnborg <sam@ravnborg.org>
Date: Thu, 18 Aug 2011 21:06:26 +0200

> Hi Ian.
> 
> On Thu, Aug 18, 2011 at 11:05:48AM +0100, Ian Campbell wrote:
>> On Thu, 2011-08-18 at 10:49 +0100, Sam Ravnborg wrote:
>> > On Thu, Aug 18, 2011 at 10:25:15AM +0100, Ian Campbell wrote:
>> > > On Thu, 2011-08-18 at 09:59 +0100, David Miller wrote:
>> > > > From: Ian Campbell <ian.campbell@citrix.com>
>> > > > Date: Thu, 18 Aug 2011 09:14:57 +0100
>> > > > 
>> > > > >>From a sparc64 defconfig:
>> > > > >   CC      arch/sparc/kernel/pcic.o
>> > > > 
>> > > > A sparc64 defconfig?  This is code only built for 32-bit sparc :-)
>> > > 
>> > > My local script confused me, it translated sparc64 into ARCH=sparc for
>> > > me, where it turns out the defconfig = sparc32_defconfig, so I was
>> > > actually building 32 bit. Sorry about that.
>> > > 
>> > > Turns out I also saw this on 32-bit sparc defconfig, which is hardly
>> > > surprising given the two .configs are exactly the same ;-)
>> > sparse complains too - I just never got around to fix it up.
>> > 
>> > If you do nto beat me I will look at it soon.
> 
> Somehow I missed that you already had provided a patch in the first mail.
> this patch has my:
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

Applied, thanks everyone.