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

[Richard Biener]

On Thu, 25 Aug 2016, Tom de Vries wrote:

> 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]
> ...

Yeah, and this also builds a variant type.

> 
> I tried to work around that by directly assigning to TYPE_ATTRIBUTES, as
> implemented in attached patch.

I expected that this would work ...

> 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);
> ...

Hmm, you shouldn't run into write_CV_qualifiers_for_type as the
type shouldn't be CV qualified (and any CV qualified record type
should be indeed a variant type of the unqualified record).

Ah, so write_CV_qualifiers_for_type looks at ABI changing attributes
and it seems yours counts as such (because it is a documented
attribute even only documented as function attribute).  I suppose
you could get away with using sth entirely private to the backend,
like "sysv abi valist", also not ever user-creatable.  Does this
side-step the FE issue?

Richard.