Menu ▾ ▴

#4240 Loop reversal reorders iterations when the loop variable is used through a cast

open
nobody
None
Front-end
5
2 days ago
2 days ago
No

1. Sample code

Attached: bug.c (self-contained, standard headers only). Both loops should leave bad == 0 and k == 15:

uint8_t bad, k, seq;
uint8_t get (void) { return seq++; }            /* returns 0, 1, 2, ... */

void withcall (void)
{
  static uint8_t i;
  for (i = 0; i < 15; i++)
    if (get () != (uint8_t)i)                   /* equal on every iteration in source order */
      bad++;
}

void nocall (void)
{
  static uint8_t i;
  for (i = 0; i < 15; i++)
    if ((uint8_t)i == k)                        /* k steps 0..15 in lock-step with i */
      k++;
}

main() prints both values through the ucsim simulator interface.

2. Commands

sdcc -mmcs51 bug.c
s51 -t 8051 -G -I 'if=xram[0xffff],out=out.txt' bug.ihx
cat out.txt

3. Version

SDCC : mcs51/z80/z180/r2k/r2ka/r3ka/r4k/r5k/r6k/sm83/tlcs90/ez80/z80n/r800/ds390/pic16/pic14/TININative/ds400/hc08/s08/stm8/pdk13/pdk14/pdk15/mos6502/mos65c02/f8/f8l TD- 4.6.0 #16555 (Mac OS X ppc)

macOS 26 on arm64 (Homebrew build). Also reproduced with a 4.5.2 trunk snapshot from 2025-06-18. In current trunk, isConformingBody() has the same unpatched checks (source read, not built).

4. Observed vs expected

Output
Expected bad=00 k=0f
Observed bad=0e k=01
With --noloopreverse bad=00 k=0f

Both loops are reversed. In withcall, get() is compared with i-1 = 14, 13, ..., 0, so 14 of 15 iterations mismatch:

    mov _withcall_i_10000_8,#0x0f       ; i = 15, counting down
00106$:
    lcall   _get
    mov r7, dpl
    mov a,_withcall_i_10000_8
    dec a                               ; compares get() with i-1
    mov r6,a
    mov a,r7
    cjne    a,ar6,00130$
    sjmp    00102$
00130$:
    inc _bad
00102$:
    djnz    _withcall_i_10000_8,00106$

The result is correct without the cast, or when i is at file scope.

Cause

reverseLoop() substitutes i-1 for i in the body and counts down. That's only valid when the body can't observe the iteration order. isConformingBody() (src/SDCCast.c) is meant to ensure that, but it refuses only when an operand is the loop symbol. (uint8_t)i is a CAST node, which falls through to default: and is accepted. The bug class is the same as #43 (a[i], 2001). Separately, case CALL: accepts calls with no arguments when i has block scope, although the manual says "No function calls within the loop". That is how withcall gets reversed.

Patch

Attached: fix-isConformingBody-cast.patch. It makes isConformingBody() refuse a cast of the loop variable (6 lines) and adds support/regression/tests/loopreverse-cast.c, which can be renamed bug-NNNN.c.

On mcs51-small, the new test fails without the fix and passes with it. Across the full suite (23753 tests), the only difference in failures is the new test (2 failures without the fix, 0 with it). Total code size changes by 1 byte. Only mcs51-small was run, on the 2025-06-18 snapshot.

2 Attachments

Discussion


Log in to post a comment.