#4161 Second volatile read of a pointer array element is omitted

[Christopher Bazley]

1: Sample code that reproduces the problem.

struct S
{
  char padding;
  _Optional int *a[2];
};

int read_value(volatile struct S *s)
{
  if (s->a[1])
    return *s->a[1];
  return 0;
}

Compiler Explorer, showing 65C02 output from both versions below.

2: Exact command used to run SDCC on this sample code.

sdcc -mmos65c02 --std-c99 --stack-auto --max-allocs-per-node 1 -S volatile-array-member.c

3: SDCC versions tested.

SDCC 4.6.0 #16555 and SDCC trunk 4.6.4 #17013, both on Compiler Explorer.
The problem also remains in the local compiler sources updated through r17035, including the fix for [bugs:#4149].

4: Copy of the error message or incorrect output, or a clear description of the observed versus expected behaviour.

The non-null path should read the volatile pointer stored in s->a[1] twice: once in the controlling expression of the if statement and again in the return expression. Instead, the generated code reads that pointer once, saves it on the stack, and dereferences the saved value. The second volatile read is omitted.

This is the read of s->a[1] that happens only once:

        ldy     #0x03
        lda     [DPTR],y
        tsx
        sta     0x101,x
        iny
        lda     [DPTR],y
        sta     0x102,x

A related bug is that there is also no warning 355. I expected that warning because the second read may yield null even if the first did not.

Removing _Optional produces the same incorrect instructions. Changing the member declaration to _Optional int *volatile a[2]; restores the second read and warning 355.

This is distinct from [bugs:#4148], which produces an unwanted warning for a checked non-volatile member. Applying its candidate fix to the local r17035 compiler sources removes that warning but leaves this example's assembly unchanged, on both Z80 and 65C02.

[Christopher Bazley]

Attaching the Codex-generated tests that exposed this bug (suggested by @spth).

[Christopher Bazley]

Please consider the attached candidate fix for this bug.

regression:
Summary for 'ucz80' : 0 failures, 37128 tests, 6458 test cases, 7479654 bytes, 1614308126 ticks

valdiag:
Summary for 'pdk15' : 0 failures, 1212 tests, 809 test cases, 0 bytes, 0 ticks
Summary for 'ds390' : 0 failures, 1214 tests, 809 test cases, 0 bytes, 0 ticks
Summary for 'f8' : 0 failures, 1212 tests, 809 test cases, 0 bytes, 0 ticks
Summary for 'hc08' : 0 failures, 1214 tests, 809 test cases, 0 bytes, 0 ticks
Summary for 'huc6280' : 0 failures, 1213 tests, 809 test cases, 0 bytes, 0 ticks
Summary for 'mcs51-large' : 0 failures, 1226 tests, 809 test cases, 0 bytes, 0 ticks
Summary for 'mcs51-stack-auto' : 0 failures, 1227 tests, 809 test cases, 0 bytes, 0 ticks
Summary for 'mcs51' : 0 failures, 1228 tests, 809 test cases, 0 bytes, 0 ticks
Summary for 'mos6502' : 0 failures, 1214 tests, 809 test cases, 0 bytes, 0 ticks
Summary for 'pdk13' : 0 failures, 1216 tests, 809 test cases, 0 bytes, 0 ticks
Summary for 'pdk14' : 0 failures, 1214 tests, 809 test cases, 0 bytes, 0 ticks
Summary for 'pdk15' : 0 failures, 1212 tests, 809 test cases, 0 bytes, 0 ticks
Summary for 'r2k' : 0 failures, 1214 tests, 809 test cases, 0 bytes, 0 ticks
Summary for 'r4k' : 0 failures, 1214 tests, 809 test cases, 0 bytes, 0 ticks
Summary for 's08' : 0 failures, 1214 tests, 809 test cases, 0 bytes, 0 ticks
Summary for 'sm83' : 0 failures, 1214 tests, 809 test cases, 0 bytes, 0 ticks
Summary for 'stm8' : 0 failures, 1216 tests, 809 test cases, 0 bytes, 0 ticks
Summary for 'tlcs90' : 0 failures, 1214 tests, 809 test cases, 0 bytes, 0 ticks
Summary for 'z180' : 0 failures, 1214 tests, 809 test cases, 0 bytes, 0 ticks
Summary for 'z80' : 0 failures, 1215 tests, 809 test cases, 0 bytes, 0 ticks

[Philipp Klaus Krause]

Looks good to me.

[Christopher Bazley]

Fixed by [r17041].