[Qemu-devel] [PATCH] hw/slavio_intctl.c: fix gcc warning about array bounds overrun

[Qemu-devel] [PATCH] hw/slavio_intctl.c: fix gcc warning about array bounds overrun

[Peter Maydell]

The Ubuntu 10.10 gcc for ARM complains that we might be overrunning
the cpu_irqs[][] array: silence this by correcting the bounds on the
loop. (In fact we would not have overrun the array because bit
MAX_PILS in pil_pending and irl_out will always be 0.)

Also add a comment about why the loop's lower bound is OK.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
I've tested that with this change we still boot the sparc
Debian image from http://people.debian.org/~aurel32/qemu/sparc/
and the change makes sense according to my understanding of
http://www.ibiblio.org/pub/historic-linux/early-ports/Sparc/NCR/NCR89C105.txt

 hw/slavio_intctl.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/hw/slavio_intctl.c b/hw/slavio_intctl.c
index fd69354..a83e5b8 100644
--- a/hw/slavio_intctl.c
+++ b/hw/slavio_intctl.c
@@ -289,7 +289,12 @@ static void slavio_check_interrupts(SLAVIO_INTCTLState *s, int set_irqs)
         pil_pending |= (s->slaves[i].intreg_pending & CPU_SOFTIRQ_MASK) >> 16;
 
         if (set_irqs) {
-            for (j = MAX_PILS; j > 0; j--) {
+            /* Since there is not really an interrupt 0 (and pil_pending
+             * and irl_out bit zero are thus always zero) there is no need
+             * to do anything with cpu_irqs[i][0] and it is OK not to do
+             * the j=0 iteration of this loop.
+             */
+            for (j = MAX_PILS-1; j > 0; j--) {
                 if (pil_pending & (1 << j)) {
                     if (!(s->slaves[i].irl_out & (1 << j))) {
                         qemu_irq_raise(s->cpu_irqs[i][j]);
-- 
1.7.1

[Qemu-devel] Re: [PATCH] hw/slavio_intctl.c: fix gcc warning about array bounds overrun

[Blue Swirl]

Thanks, applied.

On Mon, Jan 31, 2011 at 10:42 AM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> The Ubuntu 10.10 gcc for ARM complains that we might be overrunning
> the cpu_irqs[][] array: silence this by correcting the bounds on the
> loop. (In fact we would not have overrun the array because bit
> MAX_PILS in pil_pending and irl_out will always be 0.)
>
> Also add a comment about why the loop's lower bound is OK.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I've tested that with this change we still boot the sparc
> Debian image from http://people.debian.org/~aurel32/qemu/sparc/
> and the change makes sense according to my understanding of
> http://www.ibiblio.org/pub/historic-linux/early-ports/Sparc/NCR/NCR89C105.txt
>
>  hw/slavio_intctl.c |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/hw/slavio_intctl.c b/hw/slavio_intctl.c
> index fd69354..a83e5b8 100644
> --- a/hw/slavio_intctl.c
> +++ b/hw/slavio_intctl.c
> @@ -289,7 +289,12 @@ static void slavio_check_interrupts(SLAVIO_INTCTLState *s, int set_irqs)
>         pil_pending |= (s->slaves[i].intreg_pending & CPU_SOFTIRQ_MASK) >> 16;
>
>         if (set_irqs) {
> -            for (j = MAX_PILS; j > 0; j--) {
> +            /* Since there is not really an interrupt 0 (and pil_pending
> +             * and irl_out bit zero are thus always zero) there is no need
> +             * to do anything with cpu_irqs[i][0] and it is OK not to do
> +             * the j=0 iteration of this loop.
> +             */
> +            for (j = MAX_PILS-1; j > 0; j--) {
>                 if (pil_pending & (1 << j)) {
>                     if (!(s->slaves[i].irl_out & (1 << j))) {
>                         qemu_irq_raise(s->cpu_irqs[i][j]);
> --
> 1.7.1
>
>