diff mbox

[2/2] milkymist-pfpu: fix GCC 5.0.0 aggressive-loop-optimizations warning

Message ID 1424448376-15599-3-git-send-email-rkrcmar@redhat.com
State New
Headers show

Commit Message

Radim Krčmář Feb. 20, 2015, 4:06 p.m. UTC
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.

Milkymist pfpu has no jump instructions, so checking for MICROCODE_WORDS
instructions should have kept us in bounds of s->microcode, but i++
allowed one loop too many,

  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 here
       uint32_t insn = s->microcode[pc];
                ^

The code can still access out of bounds, because it presumes that PC register
always begins at 0, and we allow writing to it.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 v2: used a simpler solution [Paolo, Peter]

 hw/misc/milkymist-pfpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Michael Walle Feb. 23, 2015, 5:32 p.m. UTC | #1
Am 2015-02-20 17:06, schrieb Radim Krčmář:
> 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.
> 
> Milkymist pfpu has no jump instructions, so checking for 
> MICROCODE_WORDS
> instructions should have kept us in bounds of s->microcode, but i++
> allowed one loop too many,
> 
>   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 here
>        uint32_t insn = s->microcode[pc];
>                 ^
> 
> The code can still access out of bounds, because it presumes that PC 
> register
> always begins at 0, and we allow writing to it.
> 
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
>  v2: used a simpler solution [Paolo, Peter]
> 
>  hw/misc/milkymist-pfpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/misc/milkymist-pfpu.c b/hw/misc/milkymist-pfpu.c
> index 609f33f9cd14..08b604f13f4b 100644
> --- a/hw/misc/milkymist-pfpu.c
> +++ b/hw/misc/milkymist-pfpu.c
> @@ -362,7 +362,7 @@ static void pfpu_start(MilkymistPFPUState *s)
>              i = 0;
>              while (pfpu_decode_insn(s)) {
>                  /* decode at most MICROCODE_WORDS instructions */
> -                if (i++ >= MICROCODE_WORDS) {
> +                if (++i >= MICROCODE_WORDS) {
>                      error_report("milkymist_pfpu: too many 
> instructions "
>                              "executed in microcode. No VECTOUT?");
>                      break;

Acked-by: Michael Walle <michael@walle.cc>
diff mbox

Patch

diff --git a/hw/misc/milkymist-pfpu.c b/hw/misc/milkymist-pfpu.c
index 609f33f9cd14..08b604f13f4b 100644
--- a/hw/misc/milkymist-pfpu.c
+++ b/hw/misc/milkymist-pfpu.c
@@ -362,7 +362,7 @@  static void pfpu_start(MilkymistPFPUState *s)
             i = 0;
             while (pfpu_decode_insn(s)) {
                 /* decode at most MICROCODE_WORDS instructions */
-                if (i++ >= MICROCODE_WORDS) {
+                if (++i >= MICROCODE_WORDS) {
                     error_report("milkymist_pfpu: too many instructions "
                             "executed in microcode. No VECTOUT?");
                     break;