| Message ID | 20170724182751.18261-33-f4bug@amsat.org |
|---|---|
| State | New |
| Headers | show |
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.
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);
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(-)