PR63300 'const volatile' sometimes stripped in debug info.

When adding DW_TAG_restrict_type I made a mistake when updating the
code that handled types with multiple modifiers. This patch fixes it
by putting the logic for finding the "sub-qualified" type in a separate
function and fall back to adding the modifiers separately if there is
no such existing type. The old tests didn't catch this case because
there always was an existing sub-qualified type already. The new testcase
fails before and succeeds after this patch.

gcc/ChangeLog

	* dwarf2out.c (existing_sub_qualified_type): New function.
	(modified_type_die): Use existing_sub_qualified_type. Fall
	back to adding modifiers one by one of there is no existing
	sub-qualified type.

gcc/testsuite/ChangeLog

	* gcc.dg/guality/pr63300-const-volatile.c: New testcase.
---
 gcc/dwarf2out.c                                    | 85 ++++++++++++++++++----
 .../gcc.dg/guality/pr63300-const-volatile.c        | 12 +++
 2 files changed, 84 insertions(+), 13 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/guality/pr63300-const-volatile.c

On Sat, Sep 20 2014, Mark Wielaard wrote:

> When adding DW_TAG_restrict_type I made a mistake when updating the
> code that handled types with multiple modifiers. This patch fixes it
> by putting the logic for finding the "sub-qualified" type in a separate
> function and fall back to adding the modifiers separately if there is
> no such existing type. The old tests didn't catch this case because
> there always was an existing sub-qualified type already. The new testcase
> fails before and succeeds after this patch.
>
> gcc/ChangeLog
>
> 	* dwarf2out.c (existing_sub_qualified_type): New function.
> 	(modified_type_die): Use existing_sub_qualified_type. Fall
> 	back to adding modifiers one by one of there is no existing
> 	sub-qualified type.
>
> gcc/testsuite/ChangeLog
>
> 	* gcc.dg/guality/pr63300-const-volatile.c: New testcase.
> ---
>  gcc/dwarf2out.c                                    | 85 ++++++++++++++++++----
>  .../gcc.dg/guality/pr63300-const-volatile.c        | 12 +++
>  2 files changed, 84 insertions(+), 13 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/guality/pr63300-const-volatile.c
>
> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> index e87ade2..0cbc316 100644
> --- a/gcc/dwarf2out.c
> +++ b/gcc/dwarf2out.c
> @@ -10461,6 +10461,51 @@ decl_quals (const_tree decl)
>  	     ? TYPE_QUAL_VOLATILE : TYPE_UNQUALIFIED));
>  }
>  
> +/* Returns true if CV_QUALS contains QUAL and we have a qualified
> +   variant of TYPE that has at least one other qualifier found in
> +   CV_QUALS.  Returns false if CV_QUALS doesn't contain QUAL, if
> +   CV_QUALS is empty after subtracting QUAL, or if we don't have a
> +   TYPE that has at least one qualifier from CV_QUALS minus QUAL.  */
> +static bool
> +existing_sub_qualified_type (tree type, int cv_quals, int qual)
> +{
> +  int sub_qual, sub_quals = cv_quals & ~qual;
> +  if ((cv_quals & qual) == TYPE_UNQUALIFIED || sub_quals == TYPE_UNQUALIFIED)
> +    return false;
> +
> +  sub_qual = TYPE_QUAL_CONST;
> +  if ((sub_quals & ~sub_qual) != TYPE_UNQUALIFIED
> +      && get_qualified_type (type, sub_quals & ~sub_qual) != NULL_TREE)
> +    return true;
> +
> +  sub_qual = TYPE_QUAL_VOLATILE;
> +  if ((sub_quals & ~sub_qual) != TYPE_UNQUALIFIED
> +      && get_qualified_type (type, sub_quals & ~sub_qual) != NULL_TREE)
> +    return true;
> +
> +  sub_qual = TYPE_QUAL_RESTRICT;
> +  if ((sub_quals & ~sub_qual) != TYPE_UNQUALIFIED
> +      && get_qualified_type (type, sub_quals & ~sub_qual) != NULL_TREE)
> +    return true;
> +
> +  sub_qual = TYPE_QUAL_CONST & TYPE_QUAL_VOLATILE;

You probably mean '|' instead of '&' here.

> +  if ((sub_quals & ~sub_qual) != TYPE_UNQUALIFIED
> +      && get_qualified_type (type, sub_quals & ~sub_qual) != NULL_TREE)
> +    return true;
> +
> +  sub_qual = TYPE_QUAL_CONST & TYPE_QUAL_RESTRICT;

See above.

> +  if ((sub_quals & ~sub_qual) != TYPE_UNQUALIFIED
> +      && get_qualified_type (type, sub_quals & ~sub_qual) != NULL_TREE)
> +    return true;
> +
> +  sub_qual = TYPE_QUAL_VOLATILE & TYPE_QUAL_RESTRICT;

See above.

> +  if ((sub_quals & ~sub_qual) != TYPE_UNQUALIFIED
> +      && get_qualified_type (type, sub_quals & ~sub_qual) != NULL_TREE)
> +    return true;
> +
> +  return false;
> +}

IIUC, 'sub_qual' above represents the qualifiers to *omit* from the ones
we're interested in, right?  Maybe it would be more straightforward to
reverse the logic, i.e., start with

        sub_qual = TYPE_QUAL_VOLATILE | TYPE_QUAL_RESTRICT;

and then always use sub_qual instead of ~sub_qual.

Also note that the logic wouldn't scale too well for yet more
qualifiers...

> +
>  /* Given a pointer to an arbitrary ..._TYPE tree node, return a debugging
>     entry that chains various modifiers in front of the given type.  */
>  
> @@ -10543,34 +10588,48 @@ modified_type_die (tree type, int cv_quals, dw_die_ref context_die)
>  
>    mod_scope = scope_die_for (type, context_die);
>  
> -  if ((cv_quals & TYPE_QUAL_CONST)
> -      /* If there are multiple type modifiers, prefer a path which
> -	 leads to a qualified type.  */
> -      && (((cv_quals & ~TYPE_QUAL_CONST) == TYPE_UNQUALIFIED)
> -	  || get_qualified_type (type, cv_quals) == NULL_TREE
> -	  || (get_qualified_type (type, cv_quals & ~TYPE_QUAL_CONST)
> -	      != NULL_TREE)))
> +  /* If there are multiple type modifiers, prefer a path which
> +     leads to a qualified type.  */
> +  if (existing_sub_qualified_type (type, cv_quals, TYPE_QUAL_CONST))
>      {
>        mod_type_die = new_die (DW_TAG_const_type, mod_scope, type);
>        sub_die = modified_type_die (type, cv_quals & ~TYPE_QUAL_CONST,
>  				   context_die);
>      }
> -  else if ((cv_quals & TYPE_QUAL_VOLATILE)
> -	   && (((cv_quals & ~TYPE_QUAL_VOLATILE) == TYPE_UNQUALIFIED)
> -	       || get_qualified_type (type, cv_quals) == NULL_TREE
> -	       || (get_qualified_type (type, cv_quals & ~TYPE_QUAL_VOLATILE)
> -		   != NULL_TREE)))
> +  else if (existing_sub_qualified_type (type, cv_quals, TYPE_QUAL_VOLATILE))
>      {
>        mod_type_die = new_die (DW_TAG_volatile_type, mod_scope, type);
>        sub_die = modified_type_die (type, cv_quals & ~TYPE_QUAL_VOLATILE,
>  				   context_die);
>      }
> -  else if (cv_quals & TYPE_QUAL_RESTRICT)
> +  else if (existing_sub_qualified_type (type, cv_quals, TYPE_QUAL_RESTRICT))
>      {
>        mod_type_die = new_die (DW_TAG_restrict_type, mod_scope, type);
>        sub_die = modified_type_die (type, cv_quals & ~TYPE_QUAL_RESTRICT,
>  				   context_die);
>      }
> +  else if (cv_quals)
> +    {
> +      /* No existing path, just add qualifiers one by one.  */
> +      if (cv_quals & TYPE_QUAL_CONST)
> +	{
> +	  mod_type_die = new_die (DW_TAG_const_type, mod_scope, type);
> +	  sub_die = modified_type_die (type, cv_quals & ~TYPE_QUAL_CONST,
> +				       context_die);
> +	}
> +      else if (cv_quals & TYPE_QUAL_VOLATILE)
> +	{
> +	  mod_type_die = new_die (DW_TAG_volatile_type, mod_scope, type);
> +	  sub_die = modified_type_die (type, cv_quals & ~TYPE_QUAL_VOLATILE,
> +				       context_die);
> +	}
> +      else
> +	{
> +	  mod_type_die = new_die (DW_TAG_restrict_type, mod_scope, type);
> +	  sub_die = modified_type_die (type, cv_quals & ~TYPE_QUAL_RESTRICT,
> +				       context_die);
> +	}
> +    }
>    else if (code == POINTER_TYPE)
>      {
>        mod_type_die = new_die (DW_TAG_pointer_type, mod_scope, type);
> diff --git a/gcc/testsuite/gcc.dg/guality/pr63300-const-volatile.c b/gcc/testsuite/gcc.dg/guality/pr63300-const-volatile.c
> new file mode 100644
> index 0000000..b8d75ed
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/guality/pr63300-const-volatile.c
> @@ -0,0 +1,12 @@
> +/* PR63300 'const volatile' sometimes stripped in debug info */
> +/* { dg-do run } */
> +/* { dg-options "-g" } */
> +
> +int
> +main (int argc, char **argv)
> +{
> +  const volatile int v = argc;
> +  return v - argc;
> +}
> +
> +/* { dg-final { gdb-test 9 "type:v" "const volatile int" } } */