#4006 Loss of qualifiers from pointer target is not reported in cases of array-to-pointer decay

[Christopher Bazley]

1: Sample code that reproduces the problem.

struct SAC {
  char m[64];
};

char *str_from_struct(const struct SAC *pocs)
{
  // constraint violation: array to pointer decay does not remove const
  return pocs->m; // expected diagnostic is produced
}

char *str_from_array(const char (*paocc)[64])
{
  // constraint violation: array to pointer decay does not remove const
  return *paocc; // FAIL: missing diagnostic
}

char *str_from_static(void)
{
  // constraint violation: array to pointer decay does not remove const
  static const char s[10];
  return s;
}

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

sdcc --stack-auto --std=c23 -c decay2.c

3: SDCC version tested (type "sdcc -v" to find it)

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.5.24 #16456 (Mac OS X ppc)
published under GNU General Public License (GPL)

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

A constraint violation should be reported in all three functions, but it is only reported in the first:

decay2.c:8: warning 196: pointer target lost const qualifier

(I also noticed that replacing the const qualifiers with volatile prevents a constraint violation from being reported in any of the functions.)

[Christopher Bazley]

I am attaching a proposed bugfix for this issue, created with assistance from Codex in ChatGPT Work. I hope that it will prove acceptable when reviewed by SDCC's maintainers.

During development of the regression tests for this issue, I found what appears to be an unrelated bug, which I have reported as [#4072]

The new isUnqualifiedUnionField member of struct operand is a workaround for bug 4072. Changing the representation of unions as part of the fix for 4006 seems unnecessarily risky. I suggest that 4072 should be fixed separately from and after 4006, because the scope and risk of the required changes is likely to be bigger.

I did the following testing of the proposed patch:

• complete diagnostic-validation results (support/valdiag) for the default targets.
• complete runtime regression (support/regression) results for ucz80.

i.e.
make -j2
make -C support/regression clean-results
make -C support/regression -j2 test-ucz80
make -C support/valdiag clean
make -C support/valdiag -j2

Summary for 'ucz80': 0 failures, 36740 tests, 6407 test cases, 7348738 bytes, 1613643237 ticks
Summary for 'mcs51': 0 failures, 652 tests, 367 test cases, 0 bytes, 0 ticks
Summary for 'mcs51-large': 0 failures, 650 tests, 367 test cases, 0 bytes, 0 ticks
Summary for 'mcs51-stack-auto': 0 failures, 651 tests, 367 test cases, 0 bytes, 0 ticks
Summary for 'ds390': 0 failures, 638 tests, 367 test cases, 0 bytes, 0 ticks
Summary for 'z80': 0 failures, 641 tests, 367 test cases, 0 bytes, 0 ticks
Summary for 'z180': 0 failures, 640 tests, 367 test cases, 0 bytes, 0 ticks
Summary for 'r2k': 0 failures, 640 tests, 367 test cases, 0 bytes, 0 ticks
Summary for 'r4k': 0 failures, 640 tests, 367 test cases, 0 bytes, 0 ticks
Summary for 'tlcs90': 0 failures, 640 tests, 367 test cases, 0 bytes, 0 ticks
Summary for 'hc08': 0 failures, 639 tests, 367 test cases, 0 bytes, 0 ticks
Summary for 'mos6502': 0 failures, 639 tests, 367 test cases, 0 bytes, 0 ticks
Summary for 'stm8': 0 failures, 642 tests, 367 test cases, 0 bytes, 0 ticks
Summary for 'f8': 0 failures, 638 tests, 367 test cases, 0 bytes, 0 ticks
Summary for 'pdk13': 0 failures, 641 tests, 367 test cases, 0 bytes, 0 ticks
Summary for 'pdk14': 0 failures, 639 tests, 367 test cases, 0 bytes, 0 ticks
Summary for 'pdk15': 0 failures, 637 tests, 367 test cases, 0 bytes, 0 ticks

[Christopher Bazley]

I have lost confidence that this patch passes all tests. I think it needs rework. Volatile seems particularly tricky in SDCC.

[Christopher Bazley]

I think that [#4072] (duplicated by [#4074]) should be fixed before attempting to fix this again.

[Christopher Bazley]

I am attaching a candidate bugfix, created with assistance from Codex. It depends on a previously proposed fix attached to [#4072], which fixes how pseudo-volatility of unions is handled. This follow-up patch then replaces a historical hack in checkPtrQualifiers:

-#if 0

-      // disabled because SDCC will make all union fields volatile
-      // but your ptr to it need not be
-      if (!IS_VOLATILE (ltype->next) && IS_VOLATILE (rtype->next))
+      if (!op->isVolatileEliminated && !isVolatile (ltype->next) && isVolatile (rtype->next))
         werror (W_TARGET_LOST_QUALIFIER, "volatile");
-#endif

A few existing tests violated constraints on assignment by implicitly dropping qualifiers. In such cases, the destination types have been qualified rather than adding casts. This prevents a flood of diagnostic messages when running the regression test suite.

I did the following testing of the proposed patch:

• complete diagnostic-validation results (support/valdiag) for the default targets.
• complete runtime regression (support/regression) results for ucz80.

i.e.
make -j2
make -C support/regression clean-results
make -C support/regression -j2 test-ucz80
make -C support/valdiag clean
make -C support/valdiag -j2

Summary for 'ucz80': 0 failures, 36740 tests, 6407 test cases, 7348137 bytes, 1609579721 ticks
Summary for 'mcs51': 0 failures, 657 tests, 369 test cases, 0 bytes, 0 ticks
Summary for 'mcs51-large': 0 failures, 655 tests, 369 test cases, 0 bytes, 0 ticks
Summary for 'mcs51-stack-auto': 0 failures, 656 tests, 369 test cases, 0 bytes, 0 ticks
Summary for 'ds390': 0 failures, 643 tests, 369 test cases, 0 bytes, 0 ticks
Summary for 'z80': 0 failures, 646 tests, 369 test cases, 0 bytes, 0 ticks
Summary for 'z180': 0 failures, 645 tests, 369 test cases, 0 bytes, 0 ticks
Summary for 'r2k': 0 failures, 645 tests, 369 test cases, 0 bytes, 0 ticks
Summary for 'r4k': 0 failures, 645 tests, 369 test cases, 0 bytes, 0 ticks
Summary for 'sm83': 0 failures, 645 tests, 369 test cases, 0 bytes, 0 ticks
Summary for 'tlcs90': 0 failures, 645 tests, 369 test cases, 0 bytes, 0 ticks
Summary for 'hc08': 0 failures, 644 tests, 369 test cases, 0 bytes, 0 ticks
Summary for 's08': 0 failures, 644 tests, 369 test cases, 0 bytes, 0 ticks
Summary for 'mos6502': 0 failures, 644 tests, 369 test cases, 0 bytes, 0 ticks
Summary for 'stm8': 0 failures, 647 tests, 369 test cases, 0 bytes, 0 ticks
Summary for 'f8': 0 failures, 643 tests, 369 test cases, 0 bytes, 0 ticks
Summary for 'pdk13': 0 failures, 646 tests, 369 test cases, 0 bytes, 0 ticks
Summary for 'pdk14': 0 failures, 644 tests, 369 test cases, 0 bytes, 0 ticks
Summary for 'pdk15': 0 failures, 642 tests, 369 test cases, 0 bytes, 0 ticks

[Christopher Bazley]

I am attaching an updated version of the candidate patch.

Changes compared to the previous candidate patch for this bug:

• Rebased onto the revised patches for bug #4004 and bug #4005.
• In checkPtrCast(), it uses convertArrayToPointerType() (from the fix for bug #4005) for array conversion and checkPtrTargetQualifiers() (from the fix for bug #4004) for diagnostics. This replaces duplicated checks for discarded const, volatile, and restrict.
• Seven fixes for constraint violations in existing regression tests are now already supplied by the fix for bug #4004.
• Consolidated the tests in one file by moving the union-member diagnostic cases from union-volatile.c into bug-4006.c.
• Changes in bug-2188.c now preserve the file’s CRLF line endings.
• Added a proper commit message.

Test methodology:

make -C device/lib clean
make -C device/lib -j2
make -j2
make -C support/regression clean-results
make -C support/regression -j2 test-ucz80
make -C support/valdiag clean
make -C support/valdiag -j2

Regression test results:

Summary for 'ucz80': 0 failures, 36746 tests, 6409 test cases, 7351115 bytes, 1609704941 ticks

Valdiag test results:

Summary for 'mcs51': 0 failures, 674 tests, 374 test cases, 0 bytes, 0 ticks
Summary for 'mcs51-large': 0 failures, 672 tests, 374 test cases, 0 bytes, 0 ticks
Summary for 'mcs51-stack-auto': 0 failures, 673 tests, 374 test cases, 0 bytes, 0 ticks
Summary for 'ds390': 0 failures, 660 tests, 374 test cases, 0 bytes, 0 ticks
Summary for 'z80': 0 failures, 663 tests, 374 test cases, 0 bytes, 0 ticks
Summary for 'z180': 0 failures, 662 tests, 374 test cases, 0 bytes, 0 ticks
Summary for 'r2k': 0 failures, 662 tests, 374 test cases, 0 bytes, 0 ticks
Summary for 'r4k': 0 failures, 662 tests, 374 test cases, 0 bytes, 0 ticks
Summary for 'sm83': 0 failures, 662 tests, 374 test cases, 0 bytes, 0 ticks
Summary for 'tlcs90': 0 failures, 662 tests, 374 test cases, 0 bytes, 0 ticks
Summary for 'hc08': 0 failures, 661 tests, 374 test cases, 0 bytes, 0 ticks
Summary for 's08': 0 failures, 661 tests, 374 test cases, 0 bytes, 0 ticks
Summary for 'mos6502': 0 failures, 661 tests, 374 test cases, 0 bytes, 0 ticks
Summary for 'stm8': 0 failures, 664 tests, 374 test cases, 0 bytes, 0 ticks
Summary for 'f8': 0 failures, 660 tests, 374 test cases, 0 bytes, 0 ticks
Summary for 'pdk13': 0 failures, 663 tests, 374 test cases, 0 bytes, 0 ticks
Summary for 'pdk14': 0 failures, 661 tests, 374 test cases, 0 bytes, 0 ticks
Summary for 'pdk15': 0 failures, 659 tests, 374 test cases, 0 bytes, 0 ticks