#2632 z80: array index incorrectly computed by truncating 16-bit computed value to 8-bit

[alvin]

sdcc -v
9949 (MINGW64)

test.c

extern int printf(char *fmt, ...);

unsigned char testArr255[255];  // Fails
unsigned char testArr256[256];  // Succeeds

void main(void)
{
    int i, j;

    i = 5;
    for (j = 7; j < 10; j++)
    {
        printf("&testArr255[128] = %u\n", &testArr255[i * 24 + j]);
        printf("&testArr256[128] = %u\n", &testArr256[i * 24 + j]);
    }
}

sdcc -mz80 -S test.c

The generated asm for the two print statements above is:

;yyy.c:13: printf("&testArr255[128] = %u\n", &testArr255[i * 24 + j]);
    ld  a, c
    add a, #0x78
    ld  e, a
    rla
    sbc a, a
    ld  d, a
    ld  hl, #_testArr255
    add hl, de
    push    bc
    push    hl
    ld  hl, #___str_0
    push    hl
    call    _printf
    pop af
    pop af
    pop bc
;yyy.c:14: printf("&testArr256[128] = %u\n", &testArr256[i * 24 + j]);
    ld  hl, #0x0078
    add hl, bc
    ld  de, #_testArr256
    add hl, de
    push    bc
    push    hl
    ld  hl, #___str_1
    push    hl
    call    _printf
    pop af
    pop af
    pop bc

For the first array where the array size is less than 256 bytes, "&testArr255[i * 24 + j]" is sign extending the bottom 8-bits of the integer result of the expression "i * 24 + j". When the computed index value should be 128, the compiler is instead using an index value of -1, leading to incorrect array access.

For the second array where the array size is 256 bytes, "&testArr256[i * 24 + j]" is computed properly with the full 16-bit result of the expression "i * 24 + j" used to index the array.

It looks like the compiler has decided that "testArr255" can be indexed by 8-bit values but it shouldn't be truncating the index value it has to 8-bits and then sign extending it. I'm not sure if it's valid to truncate the index to 8-bits and then treat that as an unsigned value. That would resolve this issue but you'd still have to take care to allow the generation of a pointer to a byte just past the end of the array. I think sdcc may be taking care of that because the "testArr256" case will do this.

[Philipp Klaus Krause]

I can reproduce the problem both in sdcc [r9939] and [r9958].

Philipp

[Maarten Brock]

Increasing priority as it silently produces wrong code.

[Philipp Klaus Krause]

I can reproduce the issue in SDCC 3.6.9 #10099. It also happens for targets other than z80. The iCode at dumpraw0 is already wrong, so this looks like a frontend issue.

Philipp

P.S.: Actually, the bug is already visible in the AST.

[Philipp Klaus Krause]

Fixed in r[10101].

Philipp