Luiz Capitulino <lcapitul...@redhat.com> writes: > On Fri, 17 Aug 2012 16:10:12 +0200 > Markus Armbruster <arm...@redhat.com> wrote: > >> Stefan Weil <s...@weilnetz.de> writes: >> >> > ccc-analyzer reports these warnings: >> > >> > monitor.c:3532:21: warning: Division by zero >> > val %= val2; >> > ^ >> > monitor.c:3530:21: warning: Division by zero >> > val /= val2; >> > ^ >> > >> > Rewriting the code fixes this (and also a style issue). >> >> I'm afraid this doesn't actually fix anything, because... >> >> > Signed-off-by: Stefan Weil <s...@weilnetz.de> >> > --- >> > monitor.c | 7 ++++--- >> > 1 file changed, 4 insertions(+), 3 deletions(-) >> > >> > diff --git a/monitor.c b/monitor.c >> > index 0c34934..0ea2c14 100644 >> > --- a/monitor.c >> > +++ b/monitor.c >> > @@ -3524,12 +3524,13 @@ static int64_t expr_prod(Monitor *mon) >> > break; >> > case '/': >> > case '%': >> > - if (val2 == 0) >> > + if (val2 == 0) { >> > expr_error(mon, "division by zero"); >> > - if (op == '/') >> > + } else if (op == '/') { >> > val /= val2; >> > - else >> > + } else { >> > val %= val2; >> > + } >> > break; >> > } >> > } >> >> ... expr_error() longjmp()s out. The expression evaluator commonly >> exploits that. > > And that's correct. As far far I understood it's fixing clang, not qemu. > >> If expr_error() returned, the code would be just as wrong after your >> patch as before. > > Hmm, how? It checks for val2 == 0 first.
It would evaluate A % 0 into A, which is wrong. >> Perhaps the checker can be shut up by making expr_error() QEMU_NORETURN. > > That's indeed a better solution. Stefan, could you try that for us?