Re: [PATCH RFA] c-family: attribute ((aligned, mode)) [PR100545]

[Marek Polacek <polacek@redhat.com>]

On Wed, Apr 27, 2022 at 11:19:57AM -0400, Jason Merrill via Gcc-patches wrote:
> The problem here was that handle_mode_attribute clobbered the changes of any
> previous attribute, only copying type qualifiers to the new type.  And
> common_handle_aligned_attribute had previously set up the typedef, so when
> we later called set_underlying_type it saw DECL_ORIGINAL_TYPE set and just
> returned, even though handle_mode_attribute had messed up the TREE_TYPE.
> So, let's fix handle_mode_attribute to copy attributes, alignment, and
> typedefness to the new type.
> 
> Tested x86_64-pc-linux-gnu, OK for trunk now or after 12.1?

I think I'd slightly prefer 12.1, it doesn't seem to be a regression.
 
> 	PR c/100545
> 
> gcc/c-family/ChangeLog:
> 
> 	* c-attribs.cc (handle_mode_attribute): Copy attributes, aligned,
> 	and typedef.
> 	* c-common.cc (set_underlying_type): Add assert.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* c-c++-common/attr-mode-1.c: New test.
> ---
>  gcc/c-family/c-attribs.cc                | 15 ++++++++++++++-
>  gcc/c-family/c-common.cc                 |  7 ++++---
>  gcc/testsuite/c-c++-common/attr-mode-1.c |  3 +++
>  3 files changed, 21 insertions(+), 4 deletions(-)
>  create mode 100644 gcc/testsuite/c-c++-common/attr-mode-1.c
> 
> diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
> index 111a33f405a..26876d0f309 100644
> --- a/gcc/c-family/c-attribs.cc
> +++ b/gcc/c-family/c-attribs.cc
> @@ -2199,7 +2199,20 @@ handle_mode_attribute (tree *node, tree name, tree args,
>  	  return NULL_TREE;
>  	}
>  
> -      *node = build_qualified_type (typefm, TYPE_QUALS (type));
> +      /* Copy any quals and attributes to the new type.  */
> +      *node = build_type_attribute_qual_variant (typefm, TYPE_ATTRIBUTES (type),
> +						 TYPE_QUALS (type));
> +      if (TYPE_USER_ALIGN (type))
> +	*node = build_aligned_type (*node, TYPE_ALIGN (type));
> +      if (typedef_variant_p (type))
> +	{
> +	  /* Set up the typedef all over again.  */
> +	  tree decl = TYPE_NAME (type);
> +	  DECL_ORIGINAL_TYPE (decl) = NULL_TREE;
> +	  TREE_TYPE (decl) = *node;
> +	  set_underlying_type (decl);
> +	  *node = TREE_TYPE (decl);
> +	}
>      }
>  
>    return NULL_TREE;
> diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
> index bb0544eeaea..730faa9e87f 100644
> --- a/gcc/c-family/c-common.cc
> +++ b/gcc/c-family/c-common.cc
> @@ -8153,15 +8153,16 @@ check_missing_format_attribute (tree ltype, tree rtype)
>  void
>  set_underlying_type (tree x)
>  {
> -  if (x == error_mark_node)
> +  if (x == error_mark_node || TREE_TYPE (x) == error_mark_node)

Maybe error_operand_p?

>      return;
>    if (DECL_IS_UNDECLARED_BUILTIN (x) && TREE_CODE (TREE_TYPE (x)) != ARRAY_TYPE)
>      {
>        if (TYPE_NAME (TREE_TYPE (x)) == 0)
>  	TYPE_NAME (TREE_TYPE (x)) = x;
>      }
> -  else if (TREE_TYPE (x) != error_mark_node
> -	   && DECL_ORIGINAL_TYPE (x) == NULL_TREE)
> +  else if (DECL_ORIGINAL_TYPE (x))
> +    gcc_checking_assert (TYPE_NAME (TREE_TYPE (x)) == x);
> +  else
>      {
>        tree tt = TREE_TYPE (x);
>        DECL_ORIGINAL_TYPE (x) = tt;
> diff --git a/gcc/testsuite/c-c++-common/attr-mode-1.c b/gcc/testsuite/c-c++-common/attr-mode-1.c
> new file mode 100644
> index 00000000000..59b20cd99e8
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/attr-mode-1.c
> @@ -0,0 +1,3 @@
> +// PR c/100545
> +
> +typedef int fatp_t __attribute__((aligned, mode(SI)));

I think you need to add "dg-options -g", otherwise the bug doesn't
manifest.

Marek