[patch] fix darwin bootstrap by avoiding duplicate DIE attributes

[patch] fix darwin bootstrap by avoiding duplicate DIE attributes

[Aldy Hernandez]

[-- Attachment #1: Type: text/plain, Size: 495 bytes --]

Hi.

As explained in the PR, this patch fixes all the duplicate DIE attribute 
issues seen in Darwin.

This patch fixes real problems on Linux as well, but they're silently 
ignored by Linux's more permissive linker.  I have enhanced check_die() 
to check for these duplicate attributes in the future.

Comment #8 explains the add_location_or_const_value_attribute change and 
its testcase.

Everything else is pretty obvious.

Tested on x86_64 Linux and x86_64-apple-darwin*.

OK for mainline?

[-- Attachment #2: curr --]
[-- Type: text/plain, Size: 6251 bytes --]

commit 8645dcda5e98879d05c4eb7eb8fcc6699093494b
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Tue Jun 9 18:07:44 2015 -0400

    	PR bootstrap/66448
    	* dwarf2out.c (get_AT): Add "follow" argument.
    	(check_die): Check for common duplicate attributes.
    	(add_location_or_const_value_attribute): Do not add duplicate
    	attributes.
    	(gen_formal_parameter_die): Do not add DW_AT_artificial the second
    	time around.
    	(gen_struct_or_union_type_die): Bail early if TREE_ASM_WRITTEN.
    	(gen_type_die_with_usage): Call check_die.

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index ee2bcb1..9a95ba0 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -3089,7 +3089,7 @@ static void add_AT_offset (dw_die_ref, enum dwarf_attribute,
 static void add_AT_range_list (dw_die_ref, enum dwarf_attribute,
                                unsigned long, bool);
 static inline const char *AT_lbl (dw_attr_ref);
-static dw_attr_ref get_AT (dw_die_ref, enum dwarf_attribute);
+static dw_attr_ref get_AT (dw_die_ref, enum dwarf_attribute, bool follow = true);
 static const char *get_AT_low_pc (dw_die_ref);
 static const char *get_AT_hi_pc (dw_die_ref);
 static const char *get_AT_string (dw_die_ref, enum dwarf_attribute);
@@ -4579,10 +4579,14 @@ AT_lbl (dw_attr_ref a)
   return a->dw_attr_val.v.val_lbl_id;
 }
 
-/* Get the attribute of type attr_kind.  */
+/* Get the attribute of type attr_kind.
+
+   FOLLOW is true if we should drill down through DW_AT_specification
+   and DW_AT_abstract_origin attributes.  */
 
 static dw_attr_ref
-get_AT (dw_die_ref die, enum dwarf_attribute attr_kind)
+get_AT (dw_die_ref die, enum dwarf_attribute attr_kind,
+	bool follow /*= true*/)
 {
   dw_attr_ref a;
   unsigned ix;
@@ -4594,8 +4598,8 @@ get_AT (dw_die_ref die, enum dwarf_attribute attr_kind)
   FOR_EACH_VEC_SAFE_ELT (die->die_attr, ix, a)
     if (a->dw_attr == attr_kind)
       return a;
-    else if (a->dw_attr == DW_AT_specification
-	     || a->dw_attr == DW_AT_abstract_origin)
+    else if (follow && (a->dw_attr == DW_AT_specification
+			|| a->dw_attr == DW_AT_abstract_origin))
       spec = AT_ref (a);
 
   if (spec)
@@ -5690,20 +5694,55 @@ debug_dwarf (void)
 static void
 check_die (dw_die_ref die)
 {
-  /* A debugging information entry that is a member of an abstract
-     instance tree [that has DW_AT_inline] should not contain any
-     attributes which describe aspects of the subroutine which vary
-     between distinct inlined expansions or distinct out-of-line
-     expansions.  */
   unsigned ix;
   dw_attr_ref a;
   bool inline_found = false;
+  int n_location = 0, n_low_pc = 0, n_high_pc = 0, n_artificial = 0;
+  int n_decl_line = 0, n_decl_file = 0;
   FOR_EACH_VEC_SAFE_ELT (die->die_attr, ix, a)
-    if (a->dw_attr == DW_AT_inline && a->dw_attr_val.v.val_unsigned)
-      inline_found = true;
+    {
+      switch (a->dw_attr)
+	{
+	case DW_AT_inline:
+	  if (a->dw_attr_val.v.val_unsigned)
+	    inline_found = true;
+	  break;
+	case DW_AT_location:
+	  ++n_location;
+	  break;
+	case DW_AT_low_pc:
+	  ++n_low_pc;
+	  break;
+	case DW_AT_high_pc:
+	  ++n_high_pc;
+	  break;
+	case DW_AT_artificial:
+	  ++n_artificial;
+	  break;
+	case DW_AT_decl_line:
+	  ++n_decl_line;
+	  break;
+	case DW_AT_decl_file:
+	  ++n_decl_file;
+	  break;
+	default:
+	  break;
+	}
+    }
+  if (n_location > 1 || n_low_pc > 1 || n_high_pc > 1 || n_artificial > 1
+      || n_decl_line > 1 || n_decl_file > 1)
+    {
+      fprintf (stderr, "Duplicate attributes in DIE:\n");
+      debug_dwarf_die (die);
+      gcc_unreachable ();
+    }
   if (inline_found)
     {
-      /* Catch the most common mistakes.  */
+      /* A debugging information entry that is a member of an abstract
+	 instance tree [that has DW_AT_inline] should not contain any
+	 attributes which describe aspects of the subroutine which vary
+	 between distinct inlined expansions or distinct out-of-line
+	 expansions.  */
       FOR_EACH_VEC_SAFE_ELT (die->die_attr, ix, a)
 	gcc_assert (a->dw_attr != DW_AT_low_pc
 		    && a->dw_attr != DW_AT_high_pc
@@ -16095,6 +16134,9 @@ add_location_or_const_value_attribute (dw_die_ref die, tree decl, bool cache_p,
   if (TREE_CODE (decl) == ERROR_MARK)
     return false;
 
+  if (get_AT (die, attr, /*follow=*/false))
+    return true;
+
   gcc_assert (TREE_CODE (decl) == VAR_DECL || TREE_CODE (decl) == PARM_DECL
 	      || TREE_CODE (decl) == RESULT_DECL);
 
@@ -18055,10 +18097,9 @@ gen_formal_parameter_die (tree node, tree origin, bool emit_name_p,
 				decl_quals (node_or_origin),
 				context_die);
 	}
-    add_location:
       if (origin == NULL && DECL_ARTIFICIAL (node))
 	add_AT_flag (parm_die, DW_AT_artificial, 1);
-
+    add_location:
       if (node && node != origin)
         equate_decl_number_to_die (node, parm_die);
       if (! DECL_ABSTRACT_P (node_or_origin))
@@ -20354,15 +20395,19 @@ static void
 gen_struct_or_union_type_die (tree type, dw_die_ref context_die,
 				enum debug_info_usage usage)
 {
-  /* Fill in the bound of variable-length fields in late dwarf if
-     still incomplete.  */
-  if (TREE_ASM_WRITTEN (type)
-      && variably_modified_type_p (type, NULL)
-      && !early_dwarf)
+  if (TREE_ASM_WRITTEN (type))
     {
-      tree member;
-      for (member = TYPE_FIELDS (type); member; member = DECL_CHAIN (member))
-	fill_variable_array_bounds (TREE_TYPE (member));
+      /* Fill in the bound of variable-length fields in late dwarf if
+	 still incomplete.  */
+      if (variably_modified_type_p (type, NULL)
+	  && !early_dwarf)
+	{
+	  tree member;
+	  for (member = TYPE_FIELDS (type);
+	       member;
+	       member = DECL_CHAIN (member))
+	    fill_variable_array_bounds (TREE_TYPE (member));
+	}
       return;
     }
 
@@ -20842,7 +20887,13 @@ gen_type_die_with_usage (tree type, dw_die_ref context_die,
 static void
 gen_type_die (tree type, dw_die_ref context_die)
 {
-  gen_type_die_with_usage (type, context_die, DINFO_USAGE_DIR_USE);
+  if (type != error_mark_node)
+    {
+      gen_type_die_with_usage (type, context_die, DINFO_USAGE_DIR_USE);
+      dw_die_ref die = lookup_type_die (type);
+      if (die)
+	check_die (die);
+    }
 }
 
 /* Generate a DW_TAG_lexical_block DIE followed by DIEs to represent all of the

Re: [patch] fix darwin bootstrap by avoiding duplicate DIE attributes

[Aldy Hernandez]

[-- Attachment #1: Type: text/plain, Size: 97 bytes --]

Hi Jason.

Attached is a new patch with the off-list corrections.

Tested on x86_64 Linux.

Aldy

[-- Attachment #2: curr --]
[-- Type: text/plain, Size: 5046 bytes --]

commit 1775f74c2b78b1601c89befa724114d89fc2e8be
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Tue Jun 9 18:07:44 2015 -0400

    	PR bootstrap/66448
    	* dwarf2out.c (check_die): Check for common duplicate attributes.
    	(add_location_or_const_value_attribute): Do not add duplicate
    	attributes.
    	(gen_formal_parameter_die): Do not add DW_AT_artificial the second
    	time around.
    	(gen_struct_or_union_type_die): Bail early if TREE_ASM_WRITTEN.
    	(gen_type_die_with_usage): Call check_die.
    	(dwarf2out_decl): Only call check_die() when ENABLE_CHECKING.

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index ee2bcb1..910e9bf 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -5690,20 +5690,55 @@ debug_dwarf (void)
 static void
 check_die (dw_die_ref die)
 {
-  /* A debugging information entry that is a member of an abstract
-     instance tree [that has DW_AT_inline] should not contain any
-     attributes which describe aspects of the subroutine which vary
-     between distinct inlined expansions or distinct out-of-line
-     expansions.  */
   unsigned ix;
   dw_attr_ref a;
   bool inline_found = false;
+  int n_location = 0, n_low_pc = 0, n_high_pc = 0, n_artificial = 0;
+  int n_decl_line = 0, n_decl_file = 0;
   FOR_EACH_VEC_SAFE_ELT (die->die_attr, ix, a)
-    if (a->dw_attr == DW_AT_inline && a->dw_attr_val.v.val_unsigned)
-      inline_found = true;
+    {
+      switch (a->dw_attr)
+	{
+	case DW_AT_inline:
+	  if (a->dw_attr_val.v.val_unsigned)
+	    inline_found = true;
+	  break;
+	case DW_AT_location:
+	  ++n_location;
+	  break;
+	case DW_AT_low_pc:
+	  ++n_low_pc;
+	  break;
+	case DW_AT_high_pc:
+	  ++n_high_pc;
+	  break;
+	case DW_AT_artificial:
+	  ++n_artificial;
+	  break;
+	case DW_AT_decl_line:
+	  ++n_decl_line;
+	  break;
+	case DW_AT_decl_file:
+	  ++n_decl_file;
+	  break;
+	default:
+	  break;
+	}
+    }
+  if (n_location > 1 || n_low_pc > 1 || n_high_pc > 1 || n_artificial > 1
+      || n_decl_line > 1 || n_decl_file > 1)
+    {
+      fprintf (stderr, "Duplicate attributes in DIE:\n");
+      debug_dwarf_die (die);
+      gcc_unreachable ();
+    }
   if (inline_found)
     {
-      /* Catch the most common mistakes.  */
+      /* A debugging information entry that is a member of an abstract
+	 instance tree [that has DW_AT_inline] should not contain any
+	 attributes which describe aspects of the subroutine which vary
+	 between distinct inlined expansions or distinct out-of-line
+	 expansions.  */
       FOR_EACH_VEC_SAFE_ELT (die->die_attr, ix, a)
 	gcc_assert (a->dw_attr != DW_AT_low_pc
 		    && a->dw_attr != DW_AT_high_pc
@@ -16095,6 +16130,9 @@ add_location_or_const_value_attribute (dw_die_ref die, tree decl, bool cache_p,
   if (TREE_CODE (decl) == ERROR_MARK)
     return false;
 
+  if (get_AT (die, attr))
+    return true;
+
   gcc_assert (TREE_CODE (decl) == VAR_DECL || TREE_CODE (decl) == PARM_DECL
 	      || TREE_CODE (decl) == RESULT_DECL);
 
@@ -18055,10 +18093,9 @@ gen_formal_parameter_die (tree node, tree origin, bool emit_name_p,
 				decl_quals (node_or_origin),
 				context_die);
 	}
-    add_location:
       if (origin == NULL && DECL_ARTIFICIAL (node))
 	add_AT_flag (parm_die, DW_AT_artificial, 1);
-
+    add_location:
       if (node && node != origin)
         equate_decl_number_to_die (node, parm_die);
       if (! DECL_ABSTRACT_P (node_or_origin))
@@ -20354,15 +20391,15 @@ static void
 gen_struct_or_union_type_die (tree type, dw_die_ref context_die,
 				enum debug_info_usage usage)
 {
-  /* Fill in the bound of variable-length fields in late dwarf if
-     still incomplete.  */
-  if (TREE_ASM_WRITTEN (type)
-      && variably_modified_type_p (type, NULL)
-      && !early_dwarf)
+  if (TREE_ASM_WRITTEN (type))
     {
-      tree member;
-      for (member = TYPE_FIELDS (type); member; member = DECL_CHAIN (member))
-	fill_variable_array_bounds (TREE_TYPE (member));
+      /* Fill in the bound of variable-length fields in late dwarf if
+	 still incomplete.  */
+      if (!early_dwarf && variably_modified_type_p (type, NULL))
+	for (tree member = TYPE_FIELDS (type);
+	     member;
+	     member = DECL_CHAIN (member))
+	  fill_variable_array_bounds (TREE_TYPE (member));
       return;
     }
 
@@ -20842,7 +20879,15 @@ gen_type_die_with_usage (tree type, dw_die_ref context_die,
 static void
 gen_type_die (tree type, dw_die_ref context_die)
 {
-  gen_type_die_with_usage (type, context_die, DINFO_USAGE_DIR_USE);
+  if (type != error_mark_node)
+    {
+      gen_type_die_with_usage (type, context_die, DINFO_USAGE_DIR_USE);
+#ifdef ENABLE_CHECKING
+      dw_die_ref die = lookup_type_die (type);
+      if (die)
+	check_die (die);
+#endif
+    }
 }
 
 /* Generate a DW_TAG_lexical_block DIE followed by DIEs to represent all of the
@@ -21874,9 +21919,11 @@ dwarf2out_decl (tree decl)
 
   gen_decl_die (decl, NULL, context_die);
 
+#ifdef ENABLE_CHECKING
   dw_die_ref die = lookup_decl_die (decl);
   if (die)
     check_die (die);
+#endif
 }
 
 /* Write the debugging output for DECL.  */

Re: [patch] fix darwin bootstrap by avoiding duplicate DIE attributes

[Jason Merrill]

OK.

Jason

Re: [patch] fix darwin bootstrap by avoiding duplicate DIE attributes

[Andreas Schwab]

../../gcc/dwarf2out.c:5693:1: error: 'void check_die(dw_die_ref)' defined but not used [-Werror=unused-function]

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [patch] fix darwin bootstrap by avoiding duplicate DIE attributes

[Aldy Hernandez]

[-- Attachment #1: Type: text/plain, Size: 199 bytes --]

On 06/12/2015 12:22 AM, Andreas Schwab wrote:
> ../../gcc/dwarf2out.c:5693:1: error: 'void check_die(dw_die_ref)' defined but not used [-Werror=unused-function]
>
> Andreas.
>

Committed as obvious.

[-- Attachment #2: curr --]
[-- Type: text/plain, Size: 749 bytes --]

commit 76da2378b15ad786e8c2c64ddd8b39c132947738
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Fri Jun 12 06:42:08 2015 -0700

    	* dwarf2out.c (check_die): Protect with ENABLE_CHECKING.

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 92fa340..d2c516a 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -5687,6 +5687,7 @@ debug_dwarf (void)
   print_die (comp_unit_die (), stderr);
 }
 
+#ifdef ENABLE_CHECKING
 /* Sanity checks on DIEs.  */
 
 static void
@@ -5749,6 +5750,7 @@ check_die (dw_die_ref die)
 		    && a->dw_attr != DW_AT_GNU_all_call_sites);
     }
 }
+#endif
 \f
 /* Start a new compilation unit DIE for an include file.  OLD_UNIT is the CU
    for the enclosing include file, if any.  BINCL_DIE is the DW_TAG_GNU_BINCL