Help Request - Fix Out of Bounds Subscript Warning in Raspberry Pi

[Pavel Pisa]

Hello Joel,

On Wednesday 26 of April 2017 01:03:30 Joel Sherrill wrote:
> Pavel,
>
> Feel free to commit the patch.

the patch should be tested and (unfortunately) I do not get
to that in next weeks. I have not even check that it compiles.
If you apply it and it compiles for you and you are happy with
it, push it please.

As for the GCC analysis, it does not warn for preceding
sequence

      *bank_number = bank;

      ACQUIRE_LOCK(gpio_bank_state[bank].lock);

but the first iteration has to go through this branch of the if.
So the range check has to be valid for values stored 
into *bank_number for all following iterations.

I think that GCC is puzzled because it does not
find that there is no other way into else branch then
after if branch.

At least it is my expectation.

Best wishes,

Pavel


> Why do you think the warning is a false positive?
>
> --joel
>
> On Tue, Apr 25, 2017 at 4:53 AM, Pavel Pisa <ppisa4lists at pikron.com> wrote:
> > From 3a2957faeb744af08196f1fd3baac71f15d76c85 Mon Sep 17 00:00:00 2001
> > Message-Id: <3a2957faeb744af08196f1fd3baac71f15d76c85.1493113587.git.
> > ppisa at pikron.com>
> > From: Pavel Pisa <ppisa at pikron.com>
> > Date: Tue, 25 Apr 2017 11:45:57 +0200
> > Subject: [PATCH] bsp/raspberrypi: GPIO silence warning and ensure that
> > lock is
> >  always released.
> > To: rtems-devel at rtems.org
> >
> > Signed-off-by: Pavel Pisa <ppisa at pikron.com>
> > ---
> >  c/src/lib/libbsp/shared/gpio.c | 18 ++++++++++++------
> >  1 file changed, 12 insertions(+), 6 deletions(-)
> >
> > Not tested or even compiled quick fix of warning and more serious real
> > problem.
> > The waning seems to be GCC false positive. The first iteration
> > (i == 0) sets *bank_number for sure. But return with RTEMS_INVALID_ID can
> > happen without lock release.
> >
> > diff --git a/c/src/lib/libbsp/shared/gpio.c b/c/src/lib/libbsp/shared/
> > gpio.c
> > index 9ceeb407..bf0e415 100644
> > --- a/c/src/lib/libbsp/shared/gpio.c
> > +++ b/c/src/lib/libbsp/shared/gpio.c
> > @@ -303,6 +303,7 @@ static rtems_status_code get_pin_bitmask(
> >    uint32_t pin_number;
> >    uint32_t bank;
> >    uint8_t i;
> > +  int locked_bank = -1;
> >
> >    if ( pin_count < 1 ) {
> >      return RTEMS_UNSATISFIED;
> > @@ -314,18 +315,23 @@ static rtems_status_code get_pin_bitmask(
> >      pin_number = pins[i];
> >
> >      if ( pin_number < 0 || pin_number >= BSP_GPIO_PIN_COUNT ) {
> > +      if (locked_bank >= 0)
> > +        RELEASE_LOCK(gpio_bank_state[locked_bank].lock);
> > +
> >        return RTEMS_INVALID_ID;
> >      }
> >
> >      bank = BANK_NUMBER(pin_number);
> >
> > -    if ( i == 0 ) {
> > +    if ( locked_bank < 0 ) {
> >        *bank_number = bank;
> > +      locked_bank = bank;
> >
> > -      ACQUIRE_LOCK(gpio_bank_state[bank].lock);
> > +      ACQUIRE_LOCK(gpio_bank_state[locked_bank].lock);
> >      }
> > -    else if ( bank != *bank_number ) {
> > -      RELEASE_LOCK(gpio_bank_state[*bank_number].lock);
> > +    else if ( bank != locked_bank ) {
> > +      if (locked_bank >= 0)
> > +        RELEASE_LOCK(gpio_bank_state[locked_bank].lock);
> >
> >        return RTEMS_UNSATISFIED;
> >      }
> > @@ -334,7 +340,7 @@ static rtems_status_code get_pin_bitmask(
> >          gpio_pin_state[pin_number].pin_function != function ||
> >          gpio_pin_state[pin_number].on_group
> >      ) {
> > -      RELEASE_LOCK(gpio_bank_state[bank].lock);
> > +      RELEASE_LOCK(gpio_bank_state[locked_bank].lock);
> >
> >        return RTEMS_NOT_CONFIGURED;
> >      }
> > @@ -342,7 +348,7 @@ static rtems_status_code get_pin_bitmask(
> >      *bitmask |= (1 << PIN_NUMBER(pin_number));
> >    }
> >
> > -  RELEASE_LOCK(gpio_bank_state[bank].lock);
> > +  RELEASE_LOCK(gpio_bank_state[locked_bank].lock);
> >
> >    return RTEMS_SUCCESSFUL;
> >  }
> > --
> > 1.9.1