|
From: | Michael Walle |
Subject: | Re: [Qemu-devel] [PATCH 2/2] milkymist-pfpu: fix GCC 5.0.0 aggressive-loop-optimizations warning |
Date: | Fri, 20 Feb 2015 15:52:27 +0100 |
User-agent: | Roundcube Webmail/0.9.5 |
Am 2015-02-20 15:36, schrieb Paolo Bonzini:
On 20/02/2015 15:18, Radim Krčmář wrote:man gcc: Warn if in a loop with constant number of iterations the compiler detects undefined behavior in some statement during one or more of the iterations.Refactored the code a bit to avoid the GCC warning, in an objectionableway, hw/misc/milkymist-pfpu.c: In function ‘pfpu_write’:hw/misc/milkymist-pfpu.c:365:20: error: loop exit may only be reached after undefined behavior [-Werror=aggressive-loop-optimizations]if (i++ >= MICROCODE_WORDS) { ^hw/misc/milkymist-pfpu.c:167:14: note: possible undefined statement is hereuint32_t insn = s->microcode[pc]; ^ Signed-off-by: Radim Krčmář <address@hidden> --- hw/misc/milkymist-pfpu.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/hw/misc/milkymist-pfpu.c b/hw/misc/milkymist-pfpu.c index 609f33f9cd14..133f5b8c5153 100644 --- a/hw/misc/milkymist-pfpu.c +++ b/hw/misc/milkymist-pfpu.c @@ -164,6 +164,13 @@ output_queue_advance(MilkymistPFPUState *s) static int pfpu_decode_insn(MilkymistPFPUState *s) { uint32_t pc = s->regs[R_PC]; + + if (pc > MICROCODE_WORDS) { + error_report("milkymist_pfpu: too many instructions " + "executed in microcode. No VECTOUT?"); + return 0; + } +
I don't like this syntax, eg declaration, statements, declaration. Can you just declare the variable first and then assign them? Also the error message is then misleading. I'd prefer something like "milkymist_pfpu: program counter out of bounds. No VECTOUT?"
uint32_t insn = s->microcode[pc]; uint32_t reg_a = (insn >> 18) & 0x7f; uint32_t reg_b = (insn >> 11) & 0x7f; @@ -348,7 +355,6 @@ static int pfpu_decode_insn(MilkymistPFPUState *s) static void pfpu_start(MilkymistPFPUState *s) { int x, y; - int i; for (y = 0; y <= s->regs[R_VMESHLAST]; y++) { for (x = 0; x <= s->regs[R_HMESHLAST]; x++) { @@ -359,15 +365,7 @@ static void pfpu_start(MilkymistPFPUState *s) s->gp_regs[GPR_Y] = y; /* run microcode on this position */ - i = 0; - while (pfpu_decode_insn(s)) { - /* decode at most MICROCODE_WORDS instructions */ - if (i++ >= MICROCODE_WORDS) {Isn't the fix just to say "++i" instead of "i++"?
In the first run, s->regs[R_PC] may have any value, therefore the "insn = s->microcode[pc]" from above may access out of bounds.
-michael
[Prev in Thread] | Current Thread | [Next in Thread] |