Re: [PATCH] PR63300 'const volatile' sometimes stripped in debug info.

[Andreas Arnez <arnez at linux dot vnet dot ibm dot com>]

On Mon, Sep 22 2014, Andreas Arnez wrote:

> 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.
>>
> [...]
>
> Also note that the logic wouldn't scale too well for yet more
> qualifiers...

Considering this, I've tried a different approach below.  What do you
think?

-- >8 --
Subject: [PATCH] PR63300 'const volatile' sometimes stripped in debug info.

When adding DW_TAG_restrict_type the handling of multiple modifiers
was adjusted incorrectly.  This patch fixes it with the help of a new
tree function get_nearest_type_subqualifiers.

gcc/ChangeLog

	* tree.c (check_base_type): New.
	(check_qualified_type): Exploit new helper function above.
	(get_nearest_type_subqualifiers): New.
	* tree.h (get_nearest_type_subqualifiers): New prototype.
	* dwarf2out.c (modified_type_die): Fix handling for qualifiers.
	Next qualifier to "peel off" is now determined with the help of
	get_nearest_type_subqualifiers.
---
 gcc/dwarf2out.c | 61 +++++++++++++++++++++++++++++----------------------------
 gcc/tree.c      | 51 ++++++++++++++++++++++++++++++++++++++++++-----
 gcc/tree.h      |  7 +++++++
 3 files changed, 84 insertions(+), 35 deletions(-)

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index e87ade2..ec881d1 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -10474,12 +10474,14 @@ modified_type_die (tree type, int cv_quals, dw_die_ref context_die)
   tree qualified_type;
   tree name, low, high;
   dw_die_ref mod_scope;
+  /* Only these cv-qualifiers are currently handled.  */
+  const int cv_qual_mask = (TYPE_QUAL_CONST | TYPE_QUAL_VOLATILE
+			    | TYPE_QUAL_RESTRICT);
 
   if (code == ERROR_MARK)
     return NULL;
 
-  /* Only these cv-qualifiers are currently handled.  */
-  cv_quals &= (TYPE_QUAL_CONST | TYPE_QUAL_VOLATILE | TYPE_QUAL_RESTRICT);
+  cv_quals &= cv_qual_mask;
 
   /* Don't emit DW_TAG_restrict_type for DWARFv2, since it is a type
      tag modifier (and not an attribute) old consumers won't be able
@@ -10530,7 +10532,7 @@ modified_type_die (tree type, int cv_quals, dw_die_ref context_die)
       else
 	{
 	  int dquals = TYPE_QUALS_NO_ADDR_SPACE (dtype);
-	  dquals &= (TYPE_QUAL_CONST | TYPE_QUAL_VOLATILE | TYPE_QUAL_RESTRICT);
+	  dquals &= cv_qual_mask;
 	  if ((dquals & ~cv_quals) != TYPE_UNQUALIFIED
 	      || (cv_quals == dquals && DECL_ORIGINAL_TYPE (name) != type))
 	    /* cv-unqualified version of named type.  Just use
@@ -10543,33 +10545,32 @@ 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)))
-    {
-      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)))
-    {
-      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)
-    {
-      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);
+  if (cv_quals)
+    {
+      int q;
+      enum dwarf_tag t;
+
+      q = get_nearest_type_subqualifiers (type, cv_quals, cv_qual_mask);
+      q = cv_quals & ~q;
+
+      if (q & TYPE_QUAL_CONST)
+	{
+	  q = TYPE_QUAL_CONST;
+	  t = DW_TAG_const_type;
+	}
+      else if (q & TYPE_QUAL_VOLATILE)
+	{
+	  q = TYPE_QUAL_VOLATILE;
+	  t = DW_TAG_volatile_type;
+	}
+      else
+	{
+	  q = TYPE_QUAL_RESTRICT;
+	  t = DW_TAG_restrict_type;
+	}
+
+      mod_type_die = new_die (t, mod_scope, type);
+      sub_die = modified_type_die (type, cv_quals & ~q, context_die);
     }
   else if (code == POINTER_TYPE)
     {
diff --git a/gcc/tree.c b/gcc/tree.c
index 83df030..83f74dd 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -6195,13 +6195,12 @@ set_type_quals (tree type, int type_quals)
   TYPE_ADDR_SPACE (type) = DECODE_QUAL_ADDR_SPACE (type_quals);
 }
 
-/* Returns true iff CAND is equivalent to BASE with TYPE_QUALS.  */
+/* Returns true iff unqualified CAND and BASE are equivalent.  */
 
-bool
-check_qualified_type (const_tree cand, const_tree base, int type_quals)
+static bool
+check_base_type (const_tree cand, const_tree base)
 {
-  return (TYPE_QUALS (cand) == type_quals
-	  && TYPE_NAME (cand) == TYPE_NAME (base)
+  return (TYPE_NAME (cand) == TYPE_NAME (base)
 	  /* Apparently this is needed for Objective-C.  */
 	  && TYPE_CONTEXT (cand) == TYPE_CONTEXT (base)
 	  /* Check alignment.  */
@@ -6210,6 +6209,15 @@ check_qualified_type (const_tree cand, const_tree base, int type_quals)
 				   TYPE_ATTRIBUTES (base)));
 }
 
+/* Returns true iff CAND is equivalent to BASE with TYPE_QUALS.  */
+
+bool
+check_qualified_type (const_tree cand, const_tree base, int type_quals)
+{
+  return (TYPE_QUALS (cand) == type_quals
+	  && check_base_type (cand, base));
+}
+
 /* Returns true iff CAND is equivalent to BASE with ALIGN.  */
 
 static bool
@@ -6289,6 +6297,39 @@ get_qualified_type (tree type, int type_quals)
   return NULL_TREE;
 }
 
+/* Determine the TYPE whose qualifiers match the largest strict subset
+   of the given TYPE_QUALS, and return its qualifiers.  Ignore all
+   qualifiers outside QUAL_MASK.  */
+
+int
+get_nearest_type_subqualifiers (tree type, int type_quals, int qual_mask)
+{
+  tree t;
+  int best_rank = -1, best_qual = 0;
+
+  type_quals &= qual_mask;
+
+  for (t = TYPE_MAIN_VARIANT (type); t; t = TYPE_NEXT_VARIANT (t))
+    {
+      int q = TYPE_QUALS (t) & qual_mask;
+      int rank;
+
+      if (q == type_quals
+	  || (q & ~type_quals) != TYPE_UNQUALIFIED
+	  || !check_base_type (t, type))
+	continue;
+
+      rank = __builtin_popcount (q);
+      if (rank > best_rank)
+	{
+	  best_rank = rank;
+	  best_qual = q;
+	}
+    }
+
+  return best_qual;
+}
+
 /* Like get_qualified_type, but creates the type if it does not
    exist.  This function never returns NULL_TREE.  */
 
diff --git a/gcc/tree.h b/gcc/tree.h
index 93a12d4..b38dbc7 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -3840,6 +3840,13 @@ extern bool check_qualified_type (const_tree, const_tree, int);
 
 extern tree get_qualified_type (tree, int);
 
+/* Determine the TYPE whose qualifiers match the largest strict subset
+   of the given TYPE_QUALS, and return its qualifiers.  Ignore all
+   qualifiers outside QUAL_MASK.  */
+
+extern int get_nearest_type_subqualifiers (tree type, int type_quals,
+					   int qual_mask);
+
 /* Like get_qualified_type, but creates the type if it does not
    exist.  This function never returns NULL_TREE.  */
 
-- 
1.8.4.2