[Bug middle-end/103839] New: __builtin_clear_padding doesn't zero alignment holes of unions with fields that overlap padding

[Bug middle-end/103839] New: __builtin_clear_padding doesn't zero alignment holes of unions with fields that overlap padding

[ibuclaw at gdcproject dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103839

            Bug ID: 103839
           Summary: __builtin_clear_padding doesn't zero alignment holes
                    of unions with fields that overlap padding
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: ibuclaw at gdcproject dot org
  Target Milestone: ---

Consider:

---
// { dg-additional-options "-ftrivial-auto-var-init=zero" }
union U
{
  struct { char a; int b; } c;
  int d;
};

int main()
{
  U u[2] = {{1, 2}, {3, 4}};
}
---

Because of `int d`, the three byte padding that would be required to fill the
gap between the two struct fields is ignored.

So after gimple lowering, we are left with:
---
  u[0].c.a = 1;
  u[0].c.b = 2;
  u[1].c.a = 3;
  u[1].c.b = 4;
  GIMPLE_NOP
---

Instead of the expected:
---
  u[0].c.a = 1;
  u[0].c.b = 2;
  u[1].c.a = 3;
  u[1].c.b = 4;
  MEM <char[3]> [(union U[2] *)&a + 1B] = {};
  MEM <char[3]> [(union U[2] *)&a + 9B] = {};
---

And inspecting the data in gdb, we visibly have garbage in the object.
---
(gdb) p u
$1 = {{c = {a = 1 '\001', b = 2}, d = 1025}, {c = {a = 3 '\003', b = 4}, d =
4355}}
---

I can work around this by setting DECL_PADDING_P on all fields in unions except
the first.  However I suspect this is a mistake, and the proper thing to do
would be to only consider the first field when clear padding a union.

[Bug middle-end/103839] __builtin_clear_padding doesn't zero alignment holes of unions with fields that overlap padding

[ibuclaw at gdcproject dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103839

--- Comment #1 from Iain Buclaw <ibuclaw at gdcproject dot org> ---
(In reply to Iain Buclaw from comment #0)
> I can work around this by setting DECL_PADDING_P on all fields in unions
> except the first.  However I suspect this is a mistake, and the proper thing
> to do would be to only consider the first field when clear padding a union.
The fix to do this being a one-liner in gimple-fold.c

diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index 64515aabc04..9426e9ca473 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -4537,6 +4537,7 @@ clear_padding_union (clear_padding_struct *buf, tree
type,
        clear_padding_type (union_buf, TREE_TYPE (field), fldsz,
for_auto_init);
        clear_padding_add_padding (union_buf, sz - fldsz);
        clear_padding_flush (union_buf, true);
+       break;
       }

   if (buf == union_buf)

[Bug middle-end/103839] __builtin_clear_padding doesn't zero alignment holes of unions with fields that overlap padding

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103839

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
For __builtin_clear_padding builtin, that behavior is completely intentional,
for unions the compiler most of the time doesn't know which union member is
currently active, so it acts conservatively, only clears bits that are padding
bits in all union members rather than some of them.
For the internal uses of the __builtin_clear_padding code we'd need some way
how to tell the code which union member is the currently active one so that it
could work differently.

[Bug middle-end/103839] __builtin_clear_padding doesn't zero alignment holes of unions with fields that overlap padding

[ibuclaw at gdcproject dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103839

--- Comment #3 from Iain Buclaw <ibuclaw at gdcproject dot org> ---
(In reply to Jakub Jelinek from comment #2)
> For __builtin_clear_padding builtin, that behavior is completely
> intentional, for unions the compiler most of the time doesn't know which
> union member is currently active, so it acts conservatively, only clears
> bits that are padding bits in all union members rather than some of them.
> For the internal uses of the __builtin_clear_padding code we'd need some way
> how to tell the code which union member is the currently active one so that
> it could work differently.
OK, poking around in another language, I certainly understand this considering
that C++ allows some more contrived cases.

union U
{
    struct { char a; int b; } c;
    struct { int d; char e; } f;
};
U u = { .f={1, 2} };

D certainly only considers the first field to be the definitive shape of the
object, so it's not possible to initialize it in a constructor like the above.

I'll just go ahead with DECL_PADDING_P for now, as I can't see any downsides
just yet on doing so.

[Bug middle-end/103839] __builtin_clear_padding doesn't zero alignment holes of unions with fields that overlap padding

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103839

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The right fix is IMHO to add
#define CONSTRUCTOR_ZERO_PADDING(NODE) \
  (CONSTRUCTOR_CHECK (NODE)->base.protected_flag)
and handle that during gimplification etc.
This is needed both for C++ where 
https://eel.is/c++draft/dcl.init#general-6
requires that zero initialization clears padding bits, so the FE should set
it for zero initialization ctors, and I bet for D as well.
gimplification should then ensure vars initialized by such CONSTRUCTORs have
the padding bits cleared in whatever way is most efficient.