[Bug ada/98228] [11 Regression] ICE: Assert_Failure atree.adb:931: Error detected at s-gearop.adb:382:34 [a-ngrear.adb:313:7 [a-nllrar.ads:18:1]] on s390x-linux-gnu

[mhillen at linux dot ibm.com]

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

--- Comment #15 from Marius Hillenbrand <mhillen at linux dot ibm.com> ---
tl;dr: I found the root cause and a way to repro on x86. When the
gnat/gcc interface converts gnat entities into tree decls,
maybe_pad_type() pads some record types.  maybe_pad_type() calls
make_packable_type() to potentially pad a record into an integral type.
On s390x, we hit that case for sem_type__interp, which is padded from 12
bytes in BLKmode to a TImode. That results in the wrapped record to be
flagged as nonaddressable, which causes mayhem in tbaa (one way this
blows up below).

On x86-64 in contrast, maybe_pad_type() rejects the padding to TI in
that specific case, because TI requires 128-bit or larger alignment on
x86-64, while a 64-bit alignment is enough to get TI chosen on s390x).
When removing that condition, the issue reproduces on x86-64. When I
force that rejection on s390x, bootstrapping is successful (tested
with bootstrap-lto-lean, which extensively reproduces; not yet with a
full profiledbootstrap).

       tree packable_type = make_packable_type (type, true, align);
       if (TYPE_MODE (packable_type) != BLKmode)
           && align >= TYPE_ALIGN (packable_type)) // <- false on x86
         type = packable_type;


(I also have tweaked the calling convention for
sem_type__get_next_interp to mimic that on s390x, with a pointer for
the output parameter It)
  procedure Get_Next_Interp (I : in out Interp_Index; It : out Interp);
  pragma Export (C, Get_Next_Interp);
  pragma Export_Procedure (Get_Next_Interp,
               External => "sem_type__get_next_interp",
               Mechanism => (I => Value, It => Reference));
  pragma No_Inline (Get_Next_Interp); -- causes repro in more places




# High-level flow of the resulting crashes:

- the procedure sem_type__get_next_interp has an output parameter of
  record type sem_type__interp and overwrites that completely.
- in locations that call sem_type__get_next_interp, local variables of
  type sem_type__interp get wrapped by maybe_pad_type in an outer
  padding record for proper alignment. the padding record has a single
  field "F" for the inner record.
- on s390x, that field gets falsely flagged as nonaddressable (see
  zoom-in below).
- as a consequence of that flag, type based alias analysis does not
  relate the padded record to the alias set of the inner record.
- modref_may_conflict disambiguates references to the local variables
  (padded record) from sem_type__get_next_interp actually overwriting
  the (inner) record -- "correct" decision based on the data, but
  clearly the wrong result.
- as a result, loops that iterate via sem_type__get_next_interp are
  "optimized" into endless loops, because their abort condition is
  never checked against the updated data.
- these loops overrun All_Interp.Table and trigger assertions or
  segfault (i've seen both).



# How does the field get marked nonaddressable?

maybe_pad_type calls make_packable_type, which attempts to find an
integral type that fits the record to be padded; in our case it
chooses TI.
<record_type # sem_type__interp packed TI
on x86-64, maybe_pad_type rejects that choice: the newly crafted type
requests 128-bit alignment which is above the requested/granted(?)
alignment of 64 bits. in contrast, on s390x TImode is ok with 64-bit
alignment and we proceed with the new type, TImode and 128 bits / 16 B
size.

however, maybe_pad_type creates the outer record first, before
deciding on inflating to TImode. so, the outer record gets size 96 /
12 B.

when creating the field "F", create_field_decl decides to mark it as a
bit-field, because the field's size (12) is different from the size of
its type (TI: 16).

then, maybe_pad_type calls finish_record_type to attach the field F to
the padding record. that finds that it cannot resolve the bit-field
property of the field and consequently has to mark it as
nonaddressable.


# Potential fix? Not enough:

simply passing adressable=-1 to create_field_decl (as in make_alignment_type)
is not enough. on a first glance, the padded records are not marked
nonaddressable any more, but there are still missing relations between alias
sets and resulting incorrect optimizations.

diff --git a/gcc/ada/gcc-interface/utils.c b/gcc/ada/gcc-interface/utils.c
index 494f60e0879..aad343d3028 100644
--- a/gcc/ada/gcc-interface/utils.c
+++ b/gcc/ada/gcc-interface/utils.c
@@ -1577,12 +1577,17 @@ maybe_pad_type (tree type, tree size, unsigned int
align,

   /* Now create the field with the original size.  */
   field = create_field_decl (get_identifier ("F"), type, record, orig_size,
-                            bitsize_zero_node, 0, 1);
+                            bitsize_zero_node, 0, -1);
   DECL_INTERNAL_P (field) = 1;

   /* We will output additional debug info manually below.  */
   finish_record_type (record, field, 1, false);

+  /* Check that we did not accidentally create a bit-field or the field turned
+     nonaddressable. (PR98228)  */
+  gcc_assert (!DECL_BIT_FIELD(TYPE_FIELDS(record)));
+  gcc_assert (!DECL_NONADDRESSABLE_P(TYPE_FIELDS(record)));
+
   /* Set the RM size if requested.  */
   if (set_rm_size)
     {