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

2017-04-25 Thread 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  wrote:
> > From 3a2957faeb744af08196f1fd3baac71f15d76c85 Mon Sep 17 00:00:00 2001
> > Message-Id: <3a2957faeb744af08196f1fd3baac71f15d76c85.1493113587.git.
> > pp...@pikron.com>
> > From: Pavel Pisa 
> > 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-de...@rtems.org
> >
> > Signed-off-by: Pavel Pisa 
> > ---
> >  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

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

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

2017-04-25 Thread Joel Sherrill 
Pavel,

Feel free to commit the patch.

Why do you think the warning is a false positive?

--joel

On Tue, Apr 25, 2017 at 4:53 AM, Pavel Pisa  wrote:

> From 3a2957faeb744af08196f1fd3baac71f15d76c85 Mon Sep 17 00:00:00 2001
> Message-Id: <3a2957faeb744af08196f1fd3baac71f15d76c85.1493113587.git.
> pp...@pikron.com>
> From: Pavel Pisa 
> 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-de...@rtems.org
>
> Signed-off-by: Pavel Pisa 
> ---
>  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
>
>
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

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

2017-04-25 Thread Pavel Pisa 
>From 3a2957faeb744af08196f1fd3baac71f15d76c85 Mon Sep 17 00:00:00 2001
Message-Id: 
<3a2957faeb744af08196f1fd3baac71f15d76c85.1493113587.git.pp...@pikron.com>
From: Pavel Pisa 
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-de...@rtems.org

Signed-off-by: Pavel Pisa 
---
 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

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

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

2017-04-24 Thread Joel Sherrill 
Hi

It would be appreciated if someone could look into this and fix it.

log/arm-raspberrypi2.log:../../../../../../../../rtems/c/src/lib/libbsp/arm/raspberrypi/../../shared/gpio.c:328:35:
warning: array subscript is outside array bounds [-Warray-bounds]
log/arm-raspberrypi.log:../../../../../../../../rtems/c/src/lib/libbsp/arm/raspberrypi/../../shared/gpio.c:328:35:
warning: array subscript is outside array bounds [-Warray-bounds]


Thanks.

--joel
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel