Unlikely to happen.

hw/timer/pxa2xx_timer.c:145:19: warning: Out of bound memory access (accessed memory precedes memory block)
        counter = counters[n];
                  ^~~~~~~~~~~

Reported-by: Clang Static Analyzer
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/timer/pxa2xx_timer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/timer/pxa2xx_timer.c b/hw/timer/pxa2xx_timer.c
index 68ba5a70b3..d47f463636 100644
--- a/hw/timer/pxa2xx_timer.c
+++ b/hw/timer/pxa2xx_timer.c
@@ -139,7 +139,7 @@ static void pxa2xx_timer_update4(void *opaque, uint64_t now_qemu, int n)
     if (s->tm4[n].control & (1 << 7))
         counter = n;
     else
-        counter = counters[n];
+        counter = counters[n & 7];
 
     if (!s->tm4[counter].freq) {
         timer_del(s->tm4[n].tm.qtimer);
-- 
2.13.3

On 24 July 2017 at 19:27, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Unlikely to happen.
>
> hw/timer/pxa2xx_timer.c:145:19: warning: Out of bound memory access (accessed memory precedes memory block)
>         counter = counters[n];
>                   ^~~~~~~~~~~
>
> Reported-by: Clang Static Analyzer
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/timer/pxa2xx_timer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/timer/pxa2xx_timer.c b/hw/timer/pxa2xx_timer.c
> index 68ba5a70b3..d47f463636 100644
> --- a/hw/timer/pxa2xx_timer.c
> +++ b/hw/timer/pxa2xx_timer.c
> @@ -139,7 +139,7 @@ static void pxa2xx_timer_update4(void *opaque, uint64_t now_qemu, int n)
>      if (s->tm4[n].control & (1 << 7))
>          counter = n;
>      else
> -        counter = counters[n];
> +        counter = counters[n & 7];
>
>      if (!s->tm4[counter].freq) {
>          timer_del(s->tm4[n].tm.qtimer);
> --

This looks rather odd, because we use a mask to guard
the counters[] array index, but we do an access into
another 8-element array with n both immediately
above and immediately below that.

It's not actually possible to call this function
with n not between 0 and 7 -- if the static
analyser can't figure that out does adding an
assert at the top of the function help it out?

thanks
-- PMM

On 07/24/2017 06:01 PM, Peter Maydell wrote:
> On 24 July 2017 at 19:27, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> Unlikely to happen.
>>
>> hw/timer/pxa2xx_timer.c:145:19: warning: Out of bound memory access (accessed memory precedes memory block)
>>          counter = counters[n];
>>                    ^~~~~~~~~~~
>>
>> Reported-by: Clang Static Analyzer
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>   hw/timer/pxa2xx_timer.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/timer/pxa2xx_timer.c b/hw/timer/pxa2xx_timer.c
>> index 68ba5a70b3..d47f463636 100644
>> --- a/hw/timer/pxa2xx_timer.c
>> +++ b/hw/timer/pxa2xx_timer.c
>> @@ -139,7 +139,7 @@ static void pxa2xx_timer_update4(void *opaque, uint64_t now_qemu, int n)
>>       if (s->tm4[n].control & (1 << 7))
>>           counter = n;
>>       else
>> -        counter = counters[n];
>> +        counter = counters[n & 7];
>>
>>       if (!s->tm4[counter].freq) {
>>           timer_del(s->tm4[n].tm.qtimer);
>> --
> 
> This looks rather odd, because we use a mask to guard
> the counters[] array index, but we do an access into
> another 8-element array with n both immediately
> above and immediately below that.
> 
> It's not actually possible to call this function
> with n not between 0 and 7 -- if the static
> analyser can't figure that out does adding an
> assert at the top of the function help it out?

Yep, I'm keeping patches with "static analyzer hints" for 2.11 unless 
there is interest in having them in 2.10 (this patch now included in 
that 2.11 series).

Thanks,

Phil.