#4149 _Optional analysis incorrectly accepts repeated volatile structure member reads

[Christopher Bazley]

1: Sample code that reproduces the problem.

struct S
{
  char padding;
  _Optional int *p;
};

int read_value(volatile struct S *s)
{
  if (s->p)
    return *s->p;
  return 0;
}

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

sdcc -mz80 --std-c99 -c volatile-member.c

3: SDCC version tested

SDCC trunk r16999.

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

There is no warning about *s->p whereas I expected warning 355. Access through a pointer to a volatile structure makes a pointer member volatile. Use of the pointer member's value in the if statement's controlling expression and the dereference of the same pointer in the return expression read the value of that member separately; the second read may yield null even if the first did not. The guarding if statement therefore does not establish that the value used for dereference is non-null.

The same omission occurs with z80, mcs51, r2k, stm8, s08 and pdk14.

Removing padding makes trunk warn on all six targets. This layout-dependent warning masks the underlying mishandling of volatile qualification. Fixing the separate first-member problem reported in [bugs:#4148] also removes that warning unless the volatile issue is fixed.

[Christopher Bazley]

Fixing the separate first-member problem reported in [bugs:#4148] also removes that warning unless the volatile issue is fixed.

In other words, we do want a diagnostic message to be produced for the sample code, but we do not want it to be produced for the wrong reason.

[Christopher Bazley]

Please consider the attached candidate patch.

[Philipp Klaus Krause]

While this doesn't apply for me against current trunk, I applied the src/SDCCicode.c changes manually to test, and it looks good to me. I did a few quick tests using hand-written testcases around struct with volatile members, or members that are pointers to volatile, and did not notice a regression wjen reading the resulting .asm and looking at iCode and AST dumps.

In general, I am worried about SDCC bugs wrt. volatile, since they tend to be hard to test for. So I welcome this testing opportunity _Optional gives us here.

If you can think of further tests relating to volatile that we could get with _Optional, please look into that (though that might be beyond the scope of this ticket).