[PATCH, PR70955] Tag {ms,sysv}_va_list_type_node with {ms,sysv}_abi attribute

[Tom de Vries]

On 25/08/16 13:48, Richard Biener wrote:
> On Wed, 24 Aug 2016, Tom de Vries wrote:
>
>> Hi,
>>
>> in PR70955, ix86_canonical_va_list_type fails to recognize a
>> __builtin_ms_va_list that was declared in a TU, because its type doesn't have
>> the same main variant as the ms_va_list_type_node initialized in lto1.
>>
>> This patch fixes the PR by tagging ms_va_list_type_node and
>> sysv_va_list_type_node with ms_abi/sysv_abi attributes.
>>
>> sysv_va_list_type_node is of type array of length one with elemtype record,
>> and I ran into trouble with both adding the attribute to the array type and
>> the record type, so I ended up adding it to the first field type.
>>
>> Bootstrapped and reg-tested on x86_64.
>>
>> OK for trunk, 6 branch?

> How did you build the sysv_abi tagged struct / array that ended up
> not working?

My first try to tag the struct was this:
...
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 2639c8c..f07d9f2 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -10548,6 +10548,9 @@ ix86_build_builtin_va_list_64 (void)

    layout_type (record);

+  tree attr = tree_cons (get_identifier ("sysv_abi"), NULL_TREE, 
NULL_TREE);
+  record = build_type_attribute_variant (record, attr);
+
    /* The correct type is an array type of one element.  */
    return build_array_type (record, build_index_type (size_zero_node));
  }
...

But we immediately run into:
...
<built-in>: warning: ignoring attributes applied to ‘__va_list_tag’ 
after definition [-Wattributes]
<built-in>: warning: ignoring attributes applied to ‘struct ’ after 
definition [-Wattributes]
<built-in>: warning: ignoring attributes applied to ‘struct ’ after 
definition [-Wattributes]
...


I tried to work around that by directly assigning to TYPE_ATTRIBUTES, as 
implemented in attached patch. But then I run into a libstdc++ build 
ICE, in mangle.c:write_type:
...
       tree t = TYPE_MAIN_VARIANT (type);
       if (TYPE_ATTRIBUTES (t) && !OVERLOAD_TYPE_P (t))
         {
           tree attrs = NULL_TREE;
           if (tx_safe_fn_type_p (type))
             attrs = tree_cons (get_identifier ("transaction_safe"),
                                NULL_TREE, attrs);
           t = cp_build_type_attribute_variant (t, attrs);
         }
       gcc_assert (t != type);
...

 > I suspect that in parameter passing the array somehow
 > decays to a pointer-to-element type

Yep, as mentioned in more detail at 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70955#c23 . That's why I 
didn't try very hard to get tagging the array with the attribute to work.

 > so it is important that the
 > TYPE_MAIN_VARIANT of the record type already contains the attribute.

AFAIU, the record type is its own TYPE_MAIN_VARIANT, so it contains the 
attributes.

Thanks,
- Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: tmp.patch
Type: text/x-patch
Size: 4393 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20160825/1ae9634d/attachment.bin>