[gcc r9-9221] Fix regression with partial rep clause on variant record type
Eric Botcazou
ebotcazou@gcc.gnu.org
Wed Feb 3 10:40:45 GMT 2021
https://gcc.gnu.org/g:46de8a148b054a34c342b6e0050c78e80e2d7055
commit r9-9221-g46de8a148b054a34c342b6e0050c78e80e2d7055
Author: Eric Botcazou <ebotcazou@adacore.com>
Date: Wed Feb 3 11:38:04 2021 +0100
Fix regression with partial rep clause on variant record type
It can yield an incorrect layout when there is a partial representation
clause on a discriminated record type with a variant part.
gcc/ada/
* gcc-interface/decl.c (components_to_record): If the first component
with rep clause is the _Parent field with variable size, temporarily
set it aside when computing the internal layout of the REP part again.
* gcc-interface/utils.c (finish_record_type): Revert to taking the
maximum when merging sizes for all record types with rep clause.
(merge_sizes): Put SPECIAL parameter last and adjust recursive calls.
Diff:
---
gcc/ada/gcc-interface/decl.c | 77 +++++++++++++++++++++++++++++++++----------
gcc/ada/gcc-interface/utils.c | 39 +++++++++++-----------
2 files changed, 79 insertions(+), 37 deletions(-)
diff --git a/gcc/ada/gcc-interface/decl.c b/gcc/ada/gcc-interface/decl.c
index 37eb58d8b22..de43c66b77c 100644
--- a/gcc/ada/gcc-interface/decl.c
+++ b/gcc/ada/gcc-interface/decl.c
@@ -8148,12 +8148,12 @@ components_to_record (Node_Id gnat_component_list, Entity_Id gnat_record_type,
if (p_gnu_rep_list && gnu_rep_list)
*p_gnu_rep_list = chainon (*p_gnu_rep_list, gnu_rep_list);
- /* Deal with the annoying case of an extension of a record with variable size
- and partial rep clause, for which the _Parent field is forced at offset 0
- and has variable size, which we do not support below. Note that we cannot
- do it if the field has fixed size because we rely on the presence of the
- REP part built below to trigger the reordering of the fields in a derived
- record type when all the fields have a fixed position. */
+ /* Deal with the case of an extension of a record type with variable size and
+ partial rep clause, for which the _Parent field is forced at offset 0 and
+ has variable size. Note that we cannot do it if the field has fixed size
+ because we rely on the presence of the REP part built below to trigger the
+ reordering of the fields in a derived record type when all the fields have
+ a fixed position. */
else if (gnu_rep_list
&& !DECL_CHAIN (gnu_rep_list)
&& TREE_CODE (DECL_SIZE (gnu_rep_list)) != INTEGER_CST
@@ -8171,33 +8171,52 @@ components_to_record (Node_Id gnat_component_list, Entity_Id gnat_record_type,
record, before the others, if we also have fields without rep clause. */
else if (gnu_rep_list)
{
- tree gnu_rep_type, gnu_rep_part;
- int i, len = list_length (gnu_rep_list);
- tree *gnu_arr = XALLOCAVEC (tree, len);
+ tree gnu_parent, gnu_rep_type;
/* If all the fields have a rep clause, we can do a flat layout. */
layout_with_rep = !gnu_field_list
&& (!gnu_variant_part || variants_have_rep);
+
+ /* Same as above but the extension itself has a rep clause, in which case
+ we need to set aside the _Parent field to lay out the REP part. */
+ if (TREE_CODE (DECL_SIZE (gnu_rep_list)) != INTEGER_CST
+ && !layout_with_rep
+ && !variants_have_rep
+ && first_free_pos
+ && integer_zerop (first_free_pos)
+ && integer_zerop (bit_position (gnu_rep_list)))
+ {
+ gnu_parent = gnu_rep_list;
+ gnu_rep_list = DECL_CHAIN (gnu_rep_list);
+ }
+ else
+ gnu_parent = NULL_TREE;
+
gnu_rep_type
= layout_with_rep ? gnu_record_type : make_node (RECORD_TYPE);
- for (gnu_field = gnu_rep_list, i = 0;
- gnu_field;
- gnu_field = DECL_CHAIN (gnu_field), i++)
- gnu_arr[i] = gnu_field;
+ /* Sort the fields in order of increasing bit position. */
+ const int len = list_length (gnu_rep_list);
+ tree *gnu_arr = XALLOCAVEC (tree, len);
+
+ gnu_field = gnu_rep_list;
+ for (int i = 0; i < len; i++)
+ {
+ gnu_arr[i] = gnu_field;
+ gnu_field = DECL_CHAIN (gnu_field);
+ }
qsort (gnu_arr, len, sizeof (tree), compare_field_bitpos);
- /* Put the fields in the list in order of increasing position, which
- means we start from the end. */
gnu_rep_list = NULL_TREE;
- for (i = len - 1; i >= 0; i--)
+ for (int i = len - 1; i >= 0; i--)
{
DECL_CHAIN (gnu_arr[i]) = gnu_rep_list;
gnu_rep_list = gnu_arr[i];
DECL_CONTEXT (gnu_arr[i]) = gnu_rep_type;
}
+ /* Do the layout of the REP part, if any. */
if (layout_with_rep)
gnu_field_list = gnu_rep_list;
else
@@ -8206,14 +8225,36 @@ components_to_record (Node_Id gnat_component_list, Entity_Id gnat_record_type,
= create_concat_name (gnat_record_type, "REP");
TYPE_REVERSE_STORAGE_ORDER (gnu_rep_type)
= TYPE_REVERSE_STORAGE_ORDER (gnu_record_type);
- finish_record_type (gnu_rep_type, gnu_rep_list, 1, debug_info);
+ finish_record_type (gnu_rep_type, gnu_rep_list, 1, false);
/* If FIRST_FREE_POS is nonzero, we need to ensure that the fields
without rep clause are laid out starting from this position.
Therefore, we force it as a minimal size on the REP part. */
- gnu_rep_part
+ tree gnu_rep_part
= create_rep_part (gnu_rep_type, gnu_record_type, first_free_pos);
+ /* If this is an extension, put back the _Parent field as the first
+ field of the REP part at offset 0 and update its layout. */
+ if (gnu_parent)
+ {
+ const unsigned int align = DECL_ALIGN (gnu_parent);
+ DECL_CHAIN (gnu_parent) = TYPE_FIELDS (gnu_rep_type);
+ TYPE_FIELDS (gnu_rep_type) = gnu_parent;
+ DECL_CONTEXT (gnu_parent) = gnu_rep_type;
+ if (align > TYPE_ALIGN (gnu_rep_type))
+ {
+ SET_TYPE_ALIGN (gnu_rep_type, align);
+ TYPE_SIZE (gnu_rep_type)
+ = round_up (TYPE_SIZE (gnu_rep_type), align);
+ TYPE_SIZE_UNIT (gnu_rep_type)
+ = round_up (TYPE_SIZE_UNIT (gnu_rep_type), align);
+ SET_DECL_ALIGN (gnu_rep_part, align);
+ }
+ }
+
+ if (debug_info)
+ rest_of_record_type_compilation (gnu_rep_type);
+
/* Chain the REP part at the beginning of the field list. */
DECL_CHAIN (gnu_rep_part) = gnu_field_list;
gnu_field_list = gnu_rep_part;
diff --git a/gcc/ada/gcc-interface/utils.c b/gcc/ada/gcc-interface/utils.c
index 21037530d68..882802a19e4 100644
--- a/gcc/ada/gcc-interface/utils.c
+++ b/gcc/ada/gcc-interface/utils.c
@@ -1866,7 +1866,6 @@ finish_record_type (tree record_type, tree field_list, int rep_level,
this_ada_size = this_size;
const bool variant_part = (TREE_CODE (type) == QUAL_UNION_TYPE);
- const bool variant_part_at_zero = variant_part && integer_zerop (pos);
/* Clear DECL_BIT_FIELD for the cases layout_decl does not handle. */
if (DECL_BIT_FIELD (field)
@@ -1909,7 +1908,7 @@ finish_record_type (tree record_type, tree field_list, int rep_level,
/* Clear DECL_BIT_FIELD_TYPE for a variant part at offset 0, it's simply
not supported by the DECL_BIT_FIELD_REPRESENTATIVE machinery because
the variant part is always the last field in the list. */
- if (variant_part_at_zero)
+ if (variant_part && integer_zerop (pos))
DECL_BIT_FIELD_TYPE (field) = NULL_TREE;
/* If we still have DECL_BIT_FIELD set at this point, we know that the
@@ -1944,18 +1943,20 @@ finish_record_type (tree record_type, tree field_list, int rep_level,
case RECORD_TYPE:
/* Since we know here that all fields are sorted in order of
increasing bit position, the size of the record is one
- higher than the ending bit of the last field processed,
- unless we have a variant part at offset 0, since in this
- case we might have a field outside the variant part that
- has a higher ending position; so use a MAX in this case.
- Also, if this field is a QUAL_UNION_TYPE, we need to take
- into account the previous size in the case of empty variants. */
+ higher than the ending bit of the last field processed
+ unless we have a rep clause, because we might be processing
+ the REP part of a record with a variant part for which the
+ variant part has a rep clause but not the fixed part, in
+ which case this REP part may contain overlapping fields
+ and thus needs to be treated like a union tyoe above, so
+ use a MAX in that case. Also, if this field is a variant
+ part, we need to take into account the previous size in
+ the case of empty variants. */
ada_size
- = merge_sizes (ada_size, pos, this_ada_size, variant_part,
- variant_part_at_zero);
+ = merge_sizes (ada_size, pos, this_ada_size, rep_level > 0,
+ variant_part);
size
- = merge_sizes (size, pos, this_size, variant_part,
- variant_part_at_zero);
+ = merge_sizes (size, pos, this_size, rep_level > 0, variant_part);
break;
default:
@@ -2234,14 +2235,14 @@ rest_of_record_type_compilation (tree record_type)
}
/* Utility function of above to merge LAST_SIZE, the previous size of a record
- with FIRST_BIT and SIZE that describe a field. SPECIAL is true if this
- represents a QUAL_UNION_TYPE in which case we must look for COND_EXPRs and
- replace a value of zero with the old size. If MAX is true, we take the
+ with FIRST_BIT and SIZE that describe a field. If MAX is true, we take the
MAX of the end position of this field with LAST_SIZE. In all other cases,
- we use FIRST_BIT plus SIZE. Return an expression for the size. */
+ we use FIRST_BIT plus SIZE. SPECIAL is true if it's for a QUAL_UNION_TYPE,
+ in which case we must look for COND_EXPRs and replace a value of zero with
+ the old size. Return an expression for the size. */
static tree
-merge_sizes (tree last_size, tree first_bit, tree size, bool special, bool max)
+merge_sizes (tree last_size, tree first_bit, tree size, bool max, bool special)
{
tree type = TREE_TYPE (last_size);
tree new_size;
@@ -2258,11 +2259,11 @@ merge_sizes (tree last_size, tree first_bit, tree size, bool special, bool max)
integer_zerop (TREE_OPERAND (size, 1))
? last_size : merge_sizes (last_size, first_bit,
TREE_OPERAND (size, 1),
- 1, max),
+ max, special),
integer_zerop (TREE_OPERAND (size, 2))
? last_size : merge_sizes (last_size, first_bit,
TREE_OPERAND (size, 2),
- 1, max));
+ max, special));
/* We don't need any NON_VALUE_EXPRs and they can confuse us (especially
when fed through SUBSTITUTE_IN_EXPR) into thinking that a constant
More information about the Gcc-cvs
mailing list