[Bug tree-optimization/102586] New: [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[Bug tree-optimization/102586] New: [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[marxin at gcc dot gnu.org]

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

            Bug ID: 102586
           Summary: [12 Regression] ICE in clear_padding_type, at
                    gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Keywords: ice-on-valid-code
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: marxin at gcc dot gnu.org
                CC: qing.zhao at oracle dot com
  Target Milestone: ---

The following fails:

$ cat vbase.cc
struct C0 {};
struct C1 {};
struct C2 : C1, virtual C0 {};
struct C4 : virtual C2, C1 {};
struct C5 : virtual C4, C1 {};
struct C9 : C5 {};

int main() { C9 c9; }

$ g++ vbase.cc -ftrivial-auto-var-init=pattern -c
vbase.cc:4:8: warning: direct base ‘C1’ inaccessible in ‘C4’ due to ambiguity
[-Winaccessible-base]
    4 | struct C4 : virtual C2, C1 {};
      |        ^~
vbase.cc:5:8: warning: direct base ‘C1’ inaccessible in ‘C5’ due to ambiguity
[-Winaccessible-base]
    5 | struct C5 : virtual C4, C1 {};
      |        ^~
during GIMPLE pass: lower
vbase.cc: In function ‘int main()’:
vbase.cc:8:5: internal compiler error: in clear_padding_type, at
gimple-fold.c:4803
    8 | int main() { C9 c9; }
      |     ^~~~
0x7e794a clear_padding_type
        /home/marxin/Programming/gcc/gcc/gimple-fold.c:4803
0xee52e4 clear_padding_type
        /home/marxin/Programming/gcc/gcc/gimple-fold.c:4798
0xee6063 gimple_fold_builtin_clear_padding
        /home/marxin/Programming/gcc/gcc/gimple-fold.c:5005
0xef6178 gimple_fold_builtin
        /home/marxin/Programming/gcc/gcc/gimple-fold.c:5179
0xef6178 gimple_fold_call
        /home/marxin/Programming/gcc/gcc/gimple-fold.c:5587
0xef752b fold_stmt_1
        /home/marxin/Programming/gcc/gcc/gimple-fold.c:6289
0x1e69a8f lower_stmt
        /home/marxin/Programming/gcc/gcc/gimple-low.c:390
0x1e69bb2 lower_sequence
        /home/marxin/Programming/gcc/gcc/gimple-low.c:217
0x1e69bb2 lower_stmt
        /home/marxin/Programming/gcc/gcc/gimple-low.c:286
0x1e693ca lower_sequence
        /home/marxin/Programming/gcc/gcc/gimple-low.c:217
0x1e693ca lower_gimple_bind
        /home/marxin/Programming/gcc/gcc/gimple-low.c:475
0x1e693ca lower_sequence
        /home/marxin/Programming/gcc/gcc/gimple-low.c:217
0x1e693ca lower_gimple_bind
        /home/marxin/Programming/gcc/gcc/gimple-low.c:475
0x1e6a6cb lower_function_body
        /home/marxin/Programming/gcc/gcc/gimple-low.c:110
0x1e6a6cb execute
        /home/marxin/Programming/gcc/gcc/gimple-low.c:195
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |12.0
   Last reconfirmed|                            |2021-10-04
                 CC|                            |rguenth at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
struct C0 {};
struct C1 {};
struct C2 : C1, virtual C0 {};
struct C4 : virtual C2, C1 {};
struct C5 : virtual C4, C1 {};
struct C9 : C5 {};

void
foo (C9 *p)
{
  __builtin_clear_padding (p);
}

ICEs too, so it isn't -ftrivial-auto-var-init= bug but __builtin_clear_padding
bug that started likely with r11-5196-g1bea0d0aa5936cb36b6f86f721ca03c1a1bb601d

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[marxin at gcc dot gnu.org]

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

--- Comment #2 from Martin Liška <marxin at gcc dot gnu.org> ---
> ICEs too, so it isn't -ftrivial-auto-var-init= bug but
> __builtin_clear_padding bug that started likely with
> r11-5196-g1bea0d0aa5936cb36b6f86f721ca03c1a1bb601d

Oh, interesting..

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jason at gcc dot gnu.org,
                   |                            |ppalka at gcc dot gnu.org

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
So, seems the C9 type is 32-byte, with 8-byte virtual table pointers at offset
0 and 16 bytes (and the rest is padding), C5 is 24-byte with virtual table
pointers at offsets 0 and 8 bytes (and the rest is padding), C4 is 16-byte with
virtual table pointers at offset 0 (and the rest is padding).
But what the middle-end sees is that there is a FIELD_DECL with C5 type at
offset 0 with size 9 bytes and then a FIELD_DECL with C4 type at offset 16 with
size 9 bytes.
The __builtin_clear_padding handling code when called on a FIELD_DECL with
smaller DECL_SIZE_UNIT than TYPE_SIZE_UNIT is able to ignore fields that are
wholy beyond the size and also the default behavior is that everything is
padding until proven otherwise, so for padding bytes nothing changes.
The reason for the ICE is that the first FIELD_DECL with C5 type says 9 bytes
and the type has 8 byte, 8 byte and 1 byte fields (2 virtual table pointers and
one padding byte).  So, in that case it is unclear what the code should do.
Shall it treat the fields that partially fall into the range and partially
don't as padding (ignore them), or is there no way to do this using the
TREE_TYPE of FIELD_DECLs that the middle-end sees and one needs to use some
other types (I vaguely remember the C++ FE has some variant RECORD_TYPEs for
the virtual bases that actually reflect what is present and where, but I have
no idea if that is visible to the middle-end somewhere)?

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[jakub at gcc dot gnu.org]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Seems in the C++ FE it is CLASSTYPE_AS_BASE, so shall clear_padding_type for
FIELD_DECLs where DECL_SIZE_UNIT (fld) is smaller than TYPE_SIZE_UNIT
(TREE_TYPE (fld)) call some new langhook which for C++ would return
CLASSTYPE_AS_BASE?
All gimple_fold_builtin calls for BUILT_IN_CLEAR_PADDING should fold the call
away, so I'd hope that all the folding happens before IPA.

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[jakub at gcc dot gnu.org]

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
*** Bug 103249 has been marked as a duplicate of this bug. ***

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[jakub at gcc dot gnu.org]

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Jason, any thoughts on this?

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[jason at gcc dot gnu.org]

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

--- Comment #7 from Jason Merrill <jason at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #3)
> Shall it treat the fields that partially fall into the range and partially
> don't as padding (ignore them)

In short yes, but...

What a wacky class hierarchy.

So, when laying out C5, we first choose C2 as its primary base class; C4 is not
eligible because its direct C1 base is at offset 8 (in C4) to avoid conflicting
with C2's C1.

So then C5's C1 direct base is also at offset 8 to avoid C2's C1.

Then we place the C4 virtual base, which can also go at offset 8, because its
C1 will end up at offset 16 in C5.

But the C5 base of C9 doesn't include the C4 vbase, so when we look at C4 vbase
we see that it overlaps the end and yes, we can safely ignore it.

It would be possible to make a hierarchy where a one-byte vbase shares the
padding byte with C1:

struct smol { char c; };
struct C5 : virtual smol, virtual C4, C1 {};

Now smol and C1 are both one byte at offset 8, so we only know to ignore smol
if we know whether we're supposed to look at vbases or not.  So we might
wrongly treat that byte as data in C9 when it's actually padding.

I'd think it should be possible to handle this without a langhook by looking at
the BINFOs at the same time, i.e. within a base field, ignore BINFO_VIRTUAL_P
with non-0 offset.

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[jakub at gcc dot gnu.org]

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

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Which binfo has non-0 offset though?
When processing that C5 type with sz of 9, the FIELD_DECLs have C2 type and
 debug_tree (field)
 <field_decl 0x7fffea3bdda8 D.2410
    type <record_type 0x7fffea3a8348 C2 addressable needs-constructing
cxx-odr-p type_5 BLK
        size <integer_cst 0x7fffea23ef48 constant 64>
        unit-size <integer_cst 0x7fffea23ef60 constant 8>
...
    private ignored decl_6 BLK pr102586.C:5:8 size <integer_cst 0x7fffea23ef48
64> unit-size <integer_cst 0x7fffea23ef60 8>
...
and
p debug_tree (field->typed.type->type_non_common.maxval)
 <tree_binfo 0x7fffea3bc000
    type <record_type 0x7fffea3a8348 C2 addressable needs-constructing
cxx-odr-p type_5 BLK
...
    tree_2 bases:2 offset <integer_cst 0x7fffea23ef78 0>>
then there is a FIELD_DECL with C1 type which is ignored because it is an empty
type and then
p debug_tree (field)
 <field_decl 0x7fffea3c4130 D.2415
    type <record_type 0x7fffea3bf348 C4 addressable needs-constructing
cxx-odr-p type_5 BLK
        size <integer_cst 0x7fffea23ef90 constant 128>
        unit-size <integer_cst 0x7fffea23efa8 constant 16>
...
    size <integer_cst 0x7fffea3b5ca8 type <integer_type 0x7fffea25f0a8
bitsizetype> constant 72>
    unit-size <integer_cst 0x7fffea3b5c78 type <integer_type 0x7fffea25f000
sizetype> constant 9>
...
p debug_tree (field->typed.type->type_non_common.maxval)
 <tree_binfo 0x7fffea3bc070
    type <record_type 0x7fffea3bf348 C4 addressable needs-constructing
cxx-odr-p type_5 BLK
...
    tree_2 bases:2 offset <integer_cst 0x7fffea23ef78 0>>
so TYPE_BINFO (TREE_TYPE (field)) is non-NULL and
BINFO_OFFSET (TYPE_BINFO (TREE_TYPE (field))) is integer_zerop in both cases.

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[jakub at gcc dot gnu.org]

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

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 52404
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52404&action=edit
gcc12-pr102586.patch

With this WIP patch which just ignores fields that partially overlap there is
no ICE and the (in the new testcase renamed) C5 case looks correct to me,
__builtin_clear_padding there clears on x86_64 8 bytes at offsets 8 and 24 into
the 32-byte object, i.e. everything except for the 2 virtual table pointers.
And the testcase verifies those virtual table pointers aren't cleared and so
can be still dereferenced.
But the C8 case which is derived from your smol case above looks weird.
I'd expect (but need to look at the layout details yet) that the still 32-byte
c8 object contains 2 virtual pointers and 1 byte c member and everything else
is padding.  But __builtin_clear_padding clears there 6 and 8 bytes instead of
15 bytes together.

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[jakub at gcc dot gnu.org]

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

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
So, on
struct C0 {};
struct C1 {};
struct C2 : C1, virtual C0 {};
struct C3 : virtual C2, C1 { virtual int foo () { return 1; } };
struct C6 { char c; };
struct C7 : virtual C6, virtual C3, C1 { virtual int foo () { return 4; } };
struct C8 : C7 { virtual int foo () { return 5; } };

__attribute__((noipa)) void
foo (C8 *q)
{
  __builtin_clear_padding (q);
}

C8 c8;
C7 c7;

void
bar ()
{
  c8.c = 42;
  c7.c = 43;
}

distilled from the testcase in the patch, c8.c is at 9 bytes into c8, while
c7.c is at 8 bytes into c7, so that is likely the reason why the code considers
both byte at offset 8 and 9 as non-padding - the first FIELD_DECL in C8 is
one with C7 type (32 byte long like C8) but the field size is 9 bytes, so that
the __builtin_clear_padding code thinks the first 9 bytes are non-padding,
virtual table pointer inside of C2, then empty type C1 at off 8, then C6 at off
8 with the c 1-byte field, then C3 at offset 16 with 9 byte size but all this
is already outside of the size of FIELD_DECL with C7 type.
But C8 has a FIELD_DECL with C6 type at off 9...

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[jakub at gcc dot gnu.org]

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

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Trying clang++ it agrees with g++ that both c8 and c7 are 32-bytes, contain 2
vtable pointers at offsets 0 and 16 into the objects and that c8.c is at offset
9 and c7.c is at offset 8.  The big question is what is the byte at offset 8
into c8, is that padding, or something else?

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[jason at gcc dot gnu.org]

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

--- Comment #12 from Jason Merrill <jason at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #11)
> Trying clang++ it agrees with g++ that both c8 and c7 are 32-bytes, contain
> 2 vtable pointers at offsets 0 and 16 into the objects and that c8.c is at
> offset 9 and c7.c is at offset 8.  The big question is what is the byte at
> offset 8 into c8, is that padding, or something else?

It's padding: It's the byte allocated for the empty C1 base of C7, which can no
longer share its address with the virtual C6 base because the C6 base is laid
out  in C8 and we don't consider the C1 byte to be tail padding in that
context.

BTW, are you aware of -fdump-lang-class?

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[jakub at gcc dot gnu.org]

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

--- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
So, perhaps if RECORD_TYPE type has TYPE_BINFO in clear_padding_type, we should
be ignoring all the DECL_ARTIFICIAL unnamed fields in the structure and instead
walk the BINFO_BASE_BINFOS ?
Though, in the C8 case, the C7 type of the first FIELD_DECL is the same as
TREE_TYPE of BINFO_BASE_BINFOS[0], so I'm lost where to find the right types.

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[jason at gcc dot gnu.org]

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

--- Comment #14 from Jason Merrill <jason at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #13)
> So, perhaps if RECORD_TYPE type has TYPE_BINFO in clear_padding_type, we
> should be ignoring all the DECL_ARTIFICIAL unnamed fields in the structure
> and instead walk the BINFO_BASE_BINFOS ?

That would make sense.

> Though, in the C8 case, the C7 type of the first FIELD_DECL is the same as
> TREE_TYPE of BINFO_BASE_BINFOS[0], so I'm lost where to find the right types.

I'd think that should be fine; iIf you ignore the base fields, the other fields
are the same between the complete class type and the as-base type.

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[jakub at gcc dot gnu.org]

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

--- Comment #15 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 52408
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52408&action=edit
gcc12-pr102586.patch

I can make it work with a langhook like this.  But I can't figure out where in
BINFO to find the as_base type or something similar to that.

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[jason at gcc dot gnu.org]

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

--- Comment #16 from Jason Merrill <jason at gcc dot gnu.org> ---
Created attachment 52410
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52410&action=edit
sketch of vbase handling

This is roughly what I had in mind, though it's algorithmically poor because it
walks all the bases each time.

But then it occurred to me that unless we know the complete object type, we
don't know whether we're dealing with a base subobject, and risk clearing too
much:

struct C1 { virtual void f() {} char d; }; // vfn to make non-layout-POD
struct C2: C1 { char c; };

__attribute__((noipa)) void
foo (C1 *q)
{
  __builtin_clear_padding (q);
}

int main()
{
  C2 c2;
  c2.c = 42;
  foo (&c2);
  if (c2.c != 42)
    __builtin_abort();
}

Because of this, I think we probably want __builtin_clear_padding to reject
arguments that point to non-trivially-copyable type.  And then we don't need to
worry about vbases.

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |redi at gcc dot gnu.org,
                   |                            |rodgertq at gcc dot gnu.org

--- Comment #17 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I guess for the <atomic> purposes restricting __builtin_clear_padding to
trivially copyable types is ok,
http://eel.is/c++draft/atomics.types.generic#general-1.1 requires that.
But we use __builtin_clear_padding or the infrastructure also for:
1) __builtin_bit_cast constant evaluation
2) OpenMP atomics
3) -fauto-var-init=
I don't remember the std::bit_cast case right now, OpenMP atomics are about
scalar floats only, 3) will always call it with address of a variable and
therefore know the complete object.

So perhaps go with your patch and diagnose if the builtin is called on
non-trivially copyable type unless it is called with address of a decl?

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[jakub at gcc dot gnu.org]

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

--- Comment #18 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
But whether a type is trivially copyable is something only the FE knows, so
that checking should be done somewhere in the FE.

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[jakub at gcc dot gnu.org]

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

--- Comment #19 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
With the C7/C8 case, it is actually not just about clearing too much, but
clearing different bits:
struct C0 {};
struct C1 {};
struct C2 : C1, virtual C0 {};
struct C3 : virtual C2, C1 {};
struct C6 { char c; };
struct C7 : virtual C6, virtual C3, C1 {};
struct C8 : C7 {};

__attribute__((noipa)) void
foo (C8 *q)
{
  __builtin_clear_padding (q);
}

__attribute__((noipa)) void
bar (C7 *q)
{
  __builtin_clear_padding (q);
}

void
baz ()
{
  C8 c8;
  foo (&c8);
  bar (&c8);
}
will clear in one case the padding byte at offset 8, in the other case will
keep that unchanged and will clear the byte after it.

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[rodgertq at gcc dot gnu.org]

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

--- Comment #20 from Thomas Rodgers <rodgertq at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #17)
...

> I don't remember the std::bit_cast case right now, OpenMP atomics are about

Not sure if this is what you are talking about (frankly most of this is well
above my grokability) but, for std::bit_cast<To, From>, the standard says -

"Constraints:
—(1.1) sizeof(To) == sizeof(From) is true;
—(1.2) is_trivially_copyable_v<To> is true; and
—(1.3) is_trivially_copyable_v<From> is true"

So same as for <atomic>

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[jakub at gcc dot gnu.org]

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

--- Comment #21 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 52413
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52413&action=edit
gcc12-pr102586.patch

Indeed, and when creating BIT_CAST_EXPR we actually verify those
trivially_copyable_p types.

This patch implements the rejection of pointers to non-trivially-copyable types
with the exception of &var or &parm, because in those cases we know it is
complete object type and so don't need to worry about it being just a
subobject.
I think the exception for that case is useful, so that users can still clear
the padding bits on them, but the restriction means that they need to use the
builtin directly or in a macro rather than through inline function.
But the risks of clobbering real data otherwise is real.

The other option would be just document that it shouldn't be called on
non-trivially-copyable base classes and make it user's responsibility.
For the <atomic> and std::bit_cast uses it doesn't really matter.

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[jason at gcc dot gnu.org]

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

--- Comment #22 from Jason Merrill <jason at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #21)
> This patch implements the rejection of pointers to non-trivially-copyable
> types with the exception of &var or &parm, because in those cases we know it
> is complete object type and so don't need to worry about it being just a
> subobject.

OK.

I wonder why -fauto-var-init uses builtin_clear_padding instead of just
zero-initializing the whole object before normal initialization, as with
value-initialization?  With a new object we don't need to get clever.

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[qing.zhao at oracle dot com]

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

--- Comment #23 from Qing Zhao <qing.zhao at oracle dot com> ---
> On Feb 11, 2022, at 9:29 AM, jason at gcc dot gnu.org <gcc-bugzilla@gcc.gnu.org> wrote:
> 
> I wonder why -fauto-var-init uses builtin_clear_padding instead of just
> zero-initializing the whole object before normal initialization, as with
> value-initialization?  With a new object we don't need to get clever.

In the initial several versions of the implementation, I didn’t use
builtin_clear_padding, other that
that, I just zero-initialized the whole object before normal initialization.
However, multiple people
suggested to use builtin_clear_padding instead. Then in the later
implementations, I used
builtin_clear_padding for the padding initialization.

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[jakub at gcc dot gnu.org]

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

--- Comment #24 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I bet with the default -flifetime-dse=2 we would then have a
  var = {};
  var ={v} {CLOBBER}; // Start of ctor clobber
  var.whatever = ...; // Ctor content
and the zero initialization would be optimized away then.
I guess ideal would be to emit such zero initialization in the complete object
ctors right after the -flifetime-dse=2 beginning clobber and ideally one that
clearly says to the gimplifier and rest of middle-end that it must initialize
even padding bits.
Because right now gimplification of var = {} will often happily lower that to
  var.a = 0; var.b = 0; var.c = 0;
even when there are padding bits, which is ok for most of ctors, but not ok for
D and not ok for C++ zero initialization.
So, I think we need some flag on CONSTRUCTORS requesting the initialize also
padding bits behavior, set it for D ctors and for C++ zero initialization,
properly propagate it through the C++ FE and honor it during gimplification
(and wherever else it is needed).

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[qinzhao at gcc dot gnu.org]

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

qinzhao at gcc dot gnu.org changed:

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

--- Comment #25 from qinzhao at gcc dot gnu.org ---
(In reply to Jakub Jelinek from comment #24)
> I bet with the default -flifetime-dse=2 we would then have a
>   var = {};
>   var ={v} {CLOBBER}; // Start of ctor clobber
>   var.whatever = ...; // Ctor content
> and the zero initialization would be optimized away then.
> I guess ideal would be to emit such zero initialization in the complete
> object ctors right after the -flifetime-dse=2 beginning clobber and ideally
> one that clearly says to the gimplifier and rest of middle-end that it must
> initialize even padding bits.
> Because right now gimplification of var = {} will often happily lower that to
>   var.a = 0; var.b = 0; var.c = 0;
> even when there are padding bits,

oh right, I recall that this was the reason why adding "var = {}" to initialize
the whole structure didn't work well for initialize all paddings. therefore we
have to explicitly call __builtin_clear_padding for clear all paddings.

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[jakub at gcc dot gnu.org]

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

--- Comment #26 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Though, I must say I don't understand how that works currently at all.
For say
struct C0 {};
struct C1 {};
struct C2 : C1, virtual C0 {};
struct C3 : virtual C2, C1 {};
struct C6 { char c; };
struct C7 : virtual C6, virtual C3, C1 {};
struct C8 : C7 {};
void bar (C8 *);

void
foo ()
{
  C8 c;
  bar (&c);
}
I see in gimple dump:
      c = .DEFERRED_INIT (32, 1, &"c"[0]);
      __builtin_clear_padding (&c, 0B, 1);
      C8::C8 (&c);
      bar (&c);
and it does something only because C8::C8 doesn't get the -flifetime-dse=2
CLOBBER at the start.

But if I do:
struct C { char a; int b; char c; long d; C () : a (42), b (42), c (42), d (42)
{} };
void bar (C *);
void
foo ()
{
  C c;
  bar (&c);
}
then *.gimple is:
      c = .DEFERRED_INIT (24, 1, &"c"[0]);
      __builtin_clear_padding (&c, 0B, 1);
      C::C (&c);
      bar (&c);
...
void C::C (struct C * const this)
{
  *this = {CLOBBER};
  {
    this->a = 42;
    this->b = 42;
    this->c = 42;
    this->d = 42;
  }
}
After einline this is:
  c = .DEFERRED_INIT (24, 1, &"c"[0]);
  MEM <char[3]> [(struct C *)&c + 1B] = {};
  MEM <char[7]> [(struct C *)&c + 9B] = {};
  c ={v} {CLOBBER};
  c.a = 42;
  c.b = 42;
  c.c = 42;
  c.d = 42;
  bar (&c);
and that keeps until dse1 which optimizes that out:
  c ={v} {CLOBBER};
  c.a = 42;
  c.b = 42;
  c.c = 42;
  c.d = 42;
  bar (&c);
so there is no zero padding initialization at all.

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[qing.zhao at oracle dot com]

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

--- Comment #27 from Qing Zhao <qing.zhao at oracle dot com> ---
> On Feb 11, 2022, at 11:47 AM, jakub at gcc dot gnu.org <gcc-bugzilla@gcc.gnu.org> wrote:
> But if I do:
> struct C { char a; int b; char c; long d; C () : a (42), b (42), c (42), d (42)
> {} };
> void bar (C *);
> void
> foo ()
> {
>  C c;
>  bar (&c);
> }
> then *.gimple is:
>      c = .DEFERRED_INIT (24, 1, &"c"[0]);
>      __builtin_clear_padding (&c, 0B, 1);
>      C::C (&c);
>      bar (&c);
> ...
> void C::C (struct C * const this)
> {
>  *this = {CLOBBER};
>  {
>    this->a = 42;
>    this->b = 42;
>    this->c = 42;
>    this->d = 42;
>  }
> }
> After einline this is:
>  c = .DEFERRED_INIT (24, 1, &"c"[0]);
>  MEM <char[3]> [(struct C *)&c + 1B] = {};
>  MEM <char[7]> [(struct C *)&c + 9B] = {};
>  c ={v} {CLOBBER};
>  c.a = 42;
>  c.b = 42;
>  c.c = 42;
>  c.d = 42;
>  bar (&c);
> and that keeps until dse1 which optimizes that out:
>  c ={v} {CLOBBER};
>  c.a = 42;
>  c.b = 42;
>  c.c = 42;
>  c.d = 42;
>  bar (&c);
> so there is no zero padding initialization at all.

Does this issue only exist with -flifetime-dse=2?
When -flifetime-dse=2, the call to __builtin_clear_padding should be inserted
AFTER the
start point of the constructor of the object, otherwise it’s dead and will be
eliminated by DSE. 
And with -flifetime-dse=2, the padding initialization should be done by C++ FE
instead of middle end.
Is this understanding correct?

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[jason at gcc dot gnu.org]

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

--- Comment #28 from Jason Merrill <jason at gcc dot gnu.org> ---
(In reply to Qing Zhao from comment #27)
> Does this issue only exist with -flifetime-dse=2?
> When -flifetime-dse=2, the call to __builtin_clear_padding should be
> inserted AFTER the start point of the constructor of the object, otherwise
> it’s dead and will be eliminated by DSE. 
> And with -flifetime-dse=2, the padding initialization should be done by C++
> FE instead of middle end.
> Is this understanding correct?

Yes, -flifetime-dse=2 conflicts with -ftrivial-auto-var-init in that both are
trying to define the initial state of the variable.

Perhaps -ftrivial-auto-var-init should lower to -flifetime-dse=1.

Or change build_clobber_this to do the specified initialization instead of a
clobber, though that would mean doing the initialization for any object of that
type, not just automatics.

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[cvs-commit at gcc dot gnu.org]

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

--- Comment #29 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:c879b92c30d088ff01eb0c3e912eb8a01f9fc6e3

commit r12-7641-gc879b92c30d088ff01eb0c3e912eb8a01f9fc6e3
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Mon Mar 14 10:47:38 2022 +0100

    c++: Reject __builtin_clear_padding on non-trivially-copyable types with
one exception [PR102586]

    As mentioned by Jason in the PR, non-trivially-copyable types (or non-POD
    for purposes of layout?) types can be base classes of derived classes in
    which the padding in those non-trivially-copyable types can be reused for
    some real data members or even the layout can change and data members can
    be moved to other positions.
    __builtin_clear_padding is right now used for multiple purposes,
    in <atomic> where it isn't used yet but was planned as the main spot
    it can be used for trivially copyable types only, ditto for std::bit_cast
    where we also use it.  It is used for OpenMP long double atomics too but
    long double is trivially copyable, and lastly for -ftrivial-auto-var-init=.

    The following patch restricts the builtin to pointers to trivially-copyable
    types, with the exception when it is called directly on an address of a
    variable, in that case already the FE can verify it is the complete object
    type and so it is safe to clear all the paddings in it.

    2022-03-14  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/102586
    gcc/
            * doc/extend.texi (__builtin_clear_padding): Clearify that for C++
            argument type should be pointer to trivially-copyable type unless
it
            is address of a variable or parameter.
    gcc/cp/
            * call.cc (build_cxx_call): Diagnose __builtin_clear_padding where
            first argument's type is pointer to non-trivially-copyable type
unless
            it is address of a variable or parameter.
    gcc/testsuite/
            * g++.dg/cpp2a/builtin-clear-padding1.C: New test.

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[cvs-commit at gcc dot gnu.org]

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

--- Comment #30 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:46c739d40c6c1028295931c53cf414d581519072

commit r12-8042-g46c739d40c6c1028295931c53cf414d581519072
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Apr 7 09:14:07 2022 +0200

    c++: Handle __builtin_clear_padding on non-trivially-copyable types
[PR102586]

    On Fri, Feb 11, 2022 at 07:55:50PM +0100, Jakub Jelinek via Gcc-patches
wrote:
    > Something like the
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102586#c16
    > will still be needed with adjusted testcase from
    > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102586#c15 such that
    > __builtin_clear_padding is called directly on var addresses rather than
    > in separate functions.

    Here is an updated version of the
    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102586#c15
    patch which uses FIELD_DECL in the langhook instead of its TREE_TYPE,
    and the testcases have been adjusted for the builtin accepting
    pointers to non-trivially-copyable types only if it is address of a
    declaration.

    2022-04-07  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/102586
    gcc/
            * langhooks.h (struct lang_hooks_for_types): Add classtype_as_base
            langhook.
            * langhooks-def.h (LANG_HOOKS_CLASSTYPE_AS_BASE): Define.
            (LANG_HOOKS_FOR_TYPES_INITIALIZER): Add it.
            * gimple-fold.cc (clear_padding_type): Use ftype instead of
            TREE_TYPE (field) some more.  For artificial FIELD_DECLs without
            name try the lang_hooks.types.classtype_as_base langhook and
            if it returns non-NULL, use that instead of ftype for recursive
call.
    gcc/cp/
            * cp-objcp-common.h (cp_classtype_as_base): Declare.
            (LANG_HOOKS_CLASSTYPE_AS_BASE): Redefine.
            * cp-objcp-common.cc (cp_classtype_as_base): New function.
    gcc/testsuite/
            * g++.dg/torture/builtin-clear-padding-5.C: New test.
            * g++.dg/cpp2a/builtin-clear-padding1.C (bar): Uncomment one
            call that is now accepted.

[Bug tree-optimization/102586] [12 Regression] ICE in clear_padding_type, at gimple-fold.c:4798 since r12-3433-ga25e0b5e6ac8a77a

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #31 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Now fixed.