#506 Fix for #3393 union-in-struct initalization issue

[Howard M. Harte]

[bugs:#3393] union-in-struct initalization issue

Root cause and a patch for this, still reproducible on the released 4.6.0 #16555 (and current trunk).

Root cause
promoteAnonStructs() (src/SDCCsymt.c) flattens the members of an anonymous union into the enclosing struct, giving them all the same offset and leaving no marker that they alias one another. createIvalStruct() (src/SDCCast.c) then walks that flattened field list and hands each member its own initializer, so the aliases consume the initializers belonging to the fields that follow the union. C11 6.7.9 makes an anonymous union a single member of the enclosing struct, so brace elision initializes it once, through its first member.

This is the counterpart to [bugs:#2643]: that one fixed the same confusion in printIvalStruct() on the copy path; createIvalStruct() on the iCode path was never fixed, which is why the failure shows up exactly on the targets [bugs:#2643] listed as unaffected.

Why it shows up on PDK and MCS51 specifically. Those targets (plus any other that emits initialized globals as generated code rather than through the __xinit_ copy) reach createIvalStruct() for a global. Targets using the copy path — ds390, hc08, s08, mos6502/65c02, z80 family, stm8, f8 — emit their data via printIval() and are masked for globals only. They are still wrong for local aggregates, which always go through createIvalStruct().

Reproducer 1 — the test in the ticket (support/regression/qct/0051-inits.c, no port guards):

sdcc -mmcs51 qct/0051-inits.c      # main() returns 3, should return 0
sdcc -mpdk14 qct/0051-inits.c      # same

Reading v out of simulator RAM after main() on pdk14, 4.6.0:

a=1  b=2  c=4  s.a=0  s.b=0        (expected 1, 2, 3, 4, 5)

The 4 meant for s.a landed on c, because d aliases it and consumed an initializer; s.a and s.b then had none left. Result byte 3 = the test's third check.

Reproducer 2 — local aggregate, no target or simulator needed. Returns 2 under SDCC, 0 under any hosted C compiler:

struct S { int a; union { int c; int d; }; int z; };

int main (void)
{
      struct S v = {1, 2, 3};
      if (v.a != 1) return 1;
      if (v.c != 2) return 2;   /* holds 3: d aliased c */
      if (v.z != 3) return 3;   /* holds 0: none left   */
      return 0;
}

Emitted code, 4.6.0 #16555. Two stores to one offset, nothing to the fields after the union:

mcs51:

mov ((_v + 0x0004) + 0),#0x03      ; c = 3
mov ((_v + 0x0004) + 0),#0x04      ; d = 4  <-- same offset
; _v+0x0006 and _v+0x0008 are never written

pdk14:

mov _v+4, a                        ; c = 3
 mov _v+4, a                        ; d = 4  <-- same offset

Runtime confirmation on 4.6.0 with the result byte written to a known address and read back in uCsim:

mcs51 res = 3, pdk14 res = 3 (0 = pass).

Fix (patch attached)
In createIvalStruct()'s brace-elision walk, skip the promoted siblings that alias storage already initialized in that walk, tracking the end offset of the last field initialized there. Bitfields are exempt, since several of them legitimately share one offset — so this does not address [bugs:#2840].

With the patch, qct/0051-inits passes on mcs51 and pdk14, reproducer 2 returns 0, and the stores land at +0, +2 and +4.

Regression Test

 pdk14  0 failures / 14,794 tests / 6,292 cases
 mcs51 small / medium / small-stack-auto / large-stack-auto: 0 failures
 mcs51 large / huge: 3 failures — identical without the patch

[Howard M. Harte]

Looks like the three failures in mcs51 large / huge may be [#4045]: https://sourceforge.net/p/sdcc/bugs/4045/

[Philipp Klaus Krause]

In [r16721], I added a test based on your "Reproducer 2". Before applying the fix, I'd like to see a second SDCC dev have a look, maybe @roybaer?

[Philipp Klaus Krause]

How does this behave if we mix anonymous unions with bit-fields, e.g.

struct S { int a; union { struct {int i:8;}; struct {int j:8;}; }; int z; };

But even if that would still be broken, applying the patch would be an improvement.

[Maarten Brock]

With the patch applied it still fails for bitfields:

struct S {
        int a;
        union {
                struct {
                        int i:4;
                        int j:4;
                };
                struct {
                        int c:8;
                };
                int d;
        };
        int z;
};

int f (void)
{
      struct S v = {1, 2, 3, 4};
      if (v.a != 1) return 1;            /* pass */
      if (v.i != 2) return 2;            /* fail, holds 0 */
      if (v.j != 3) return 3;            /* fail, holds 0 */
      if (v.c != 0x32) return 4;         /* fail, holds 0 */
      if (v.d != 0x32) return 5;         /* fail, holds 0 */
      if (v.z != 4) return 6;            /* fail, holds 0 */
      return 0;
}

1 is used for initializing a, correct.
2 is used for initializing i, correct.
3 is used for initializing j, correct.
4 is used for initializing c, incorrect !
0 is used for initializing d, incorrect !
0 is used for initializing z, incorrect !

[Maarten Brock]

Modified patch is applied in [r16733].
Setting prevEnd is done unconditionally.
The fix still fails for a second struct with bitfields (c:8 above).
The hack really is a hack. Anonymous structs and unions still need their hierarchy and should not be flattened.