Bug 48229 - 4.5.x should produce unambiguous DW_AT_accessibility
Summary: 4.5.x should produce unambiguous DW_AT_accessibility
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: debug (show other bugs)
Version: 4.5.3
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-03-22 09:38 UTC by Jan Kratochvil
Modified: 2011-03-28 11:21 UTC (History)
4 users (show)

See Also:
Host:
Target: x86_64-unknown-linux-gnu
Build:
Known to work:
Known to fail:
Last reconfirmed: 2011-03-22 20:31:59


Attachments
GDB patch attempt with ptype regressions. (2.40 KB, patch)
2011-03-28 09:12 UTC, Jan Kratochvil
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Kratochvil 2011-03-22 09:38:27 UTC
FAIL g++ (GCC) 4.5.3 20110322 (prerelease)
FAIL g++ (GCC) 4.6.0 20110322 (prerelease)
FAIL g++ (GCC) 4.7.0 20110322 (experimental)

echo 'class C {} c;' | g++ -o 1.o -c -g -Wall -x c++ - -gdwarf-4

Contents of the .debug_info section:
 <0><b>: Abbrev Number: 3 (DW_TAG_compile_unit)
    < c>   DW_AT_producer    : GNU C++ 4.5.3 20110322 (prerelease)      
+
Contents of the .debug_types section:
 <0><17>: Abbrev Number: 1 (DW_TAG_type_unit)
    <18>   DW_AT_language    : 4        (C++)
    <19>   DW_AT_GNU_odr_signature: 0xef4f1478 0xafb836e1       
    <21>   DW_AT_stmt_list   : 0x0      
 - no DW_AT_producer here to check for workaround of PR debug/45124.
Comment 1 Dodji Seketeli 2011-03-23 09:31:08 UTC
I am not sure to understand why the DW_TAG_type_unit DIE should have a
DW_AT_producer attribute.  From the DWARF-4 specification, I can't see
DW_AT_producer listed as a possible attribute of the DW_TAG_type_unit
DIE. AIUI, the DW_AT_producer attribute is meant to be hung off of
DW_AT_compile_unit and DW_AT_partial_unit.  I guess I am missing
something ...
Comment 2 Dodji Seketeli 2011-03-23 11:28:28 UTC
This is a very lightly tested patch to add the DW_AT_producer
attribute to the DW_TAG_type_unit DIE.

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index f62bb48..ff1adc3 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -6498,6 +6498,7 @@ static inline int local_scope_p (dw_die_ref);
 static inline int class_scope_p (dw_die_ref);
 static inline int class_or_namespace_scope_p (dw_die_ref);
 static void add_type_attribute (dw_die_ref, tree, int, int, dw_die_ref);
+static void add_producer_attribute (dw_die_ref);
 static void add_calling_convention_attribute (dw_die_ref, tree);
 static const char *type_tag (const_tree);
 static tree member_declared_type (const_tree);
@@ -10298,6 +10299,13 @@ break_out_comdat_types (dw_die_ref die)
         unit = new_die (DW_TAG_type_unit, NULL, NULL);
         add_AT_unsigned (unit, DW_AT_language,
                          get_AT_unsigned (comp_unit_die (), DW_AT_language));
+
+	/* Add a DW_AT_producer attribute.  This is not mandated by
+	   the DWARF-4 specification.  But it appears that GDB needs
+	   it for cases where different DW_TAG_type_unit DIEs might
+	   be emitted by different compilers.  */
+	add_producer_attribute (unit);
+
         type_node = ggc_alloc_cleared_comdat_type_node ();
         type_node->root_die = unit;
         type_node->next = comdat_type_list;
@@ -18014,6 +18022,28 @@ add_type_attribute (dw_die_ref object_die, tree type, int decl_const,
     add_AT_die_ref (object_die, DW_AT_type, type_die);
 }
 
+/* Add an DW_AT_producer attribute to a given DIE.  */
+
+static void
+add_producer_attribute (dw_die_ref die)
+{
+  char producer[250];
+  const char *language_string = lang_hooks.name;
+  
+   sprintf (producer, "%s %s", language_string, version_string);
+#ifdef MIPS_DEBUGGING_INFO
+  /* The MIPS/SGI compilers place the 'cc' command line options in the producer
+     string.  The SGI debugger looks for -g, -g1, -g2, or -g3; if they do
+     not appear in the producer string, the debugger reaches the conclusion
+     that the object file is stripped and has no debugging information.
+     To get the MIPS/SGI debugger to believe that there is debugging
+     information in the object file, we add a -g to the producer string.  */
+  if (debug_info_level > DINFO_LEVEL_TERSE)
+    strcat (producer, " -g");
+#endif
+  add_AT_string (die, DW_AT_producer, producer);
+}
+
 /* Given an object die, add the calling convention attribute for the
    function call type.  */
 static void
@@ -20101,7 +20131,6 @@ static dw_die_ref
 gen_compile_unit_die (const char *filename)
 {
   dw_die_ref die;
-  char producer[250];
   const char *language_string = lang_hooks.name;
   int language;
 
@@ -20114,21 +20143,8 @@ gen_compile_unit_die (const char *filename)
       if (!IS_ABSOLUTE_PATH (filename) && filename[0] != '<')
 	add_comp_dir_attribute (die);
     }
-
-  sprintf (producer, "%s %s", language_string, version_string);
-
-#ifdef MIPS_DEBUGGING_INFO
-  /* The MIPS/SGI compilers place the 'cc' command line options in the producer
-     string.  The SGI debugger looks for -g, -g1, -g2, or -g3; if they do
-     not appear in the producer string, the debugger reaches the conclusion
-     that the object file is stripped and has no debugging information.
-     To get the MIPS/SGI debugger to believe that there is debugging
-     information in the object file, we add a -g to the producer string.  */
-  if (debug_info_level > DINFO_LEVEL_TERSE)
-    strcat (producer, " -g");
-#endif
-
-  add_AT_string (die, DW_AT_producer, producer);
+  
+  add_producer_attribute (die);
 
   /* If our producer is LTO try to figure out a common language to use
      from the global list of translation units.  */
Comment 3 Jakub Jelinek 2011-03-23 11:56:05 UTC
I think it is a bad idea to add DW_AT_producer to .debug_type units.  Making each .debug_types addition 4 bytes longer is bad for debug info size, we might have many thousands DW_TAG_type_units for each source.
I think you can safely use referring DW_TAG_compile_unit's DW_AT_producer for your purposes.  While it is true that .debug_types sections in the end comes from different *.o file and be produced by different compiler, it will be referenced by the current CU only if it hashes the same, thus has the same content (disregarding hash collisions, in which case you are in very bad trouble, but you are in that very bad trouble no matter whether you just
want to make sure it was produced by the same or compatible producer - when hash collision happens, it might refer to completely unrelated type).
Comment 4 Jan Kratochvil 2011-03-23 13:07:02 UTC
(In reply to comment #3)
> I think it is a bad idea to add DW_AT_producer to .debug_type units.  Making
> each .debug_types addition 4 bytes longer is bad for debug info size, we might
> have many thousands DW_TAG_type_units for each source.
> I think you can safely use referring DW_TAG_compile_unit's DW_AT_producer for
> your purposes.  While it is true that .debug_types sections in the end comes
> from different *.o file and be produced by different compiler, it will be
> referenced by the current CU only if it hashes the same, thus has the same
> content (disregarding hash collisions, in which case you are in very bad
> trouble, but you are in that very bad trouble no matter whether you just
> want to make sure it was produced by the same or compatible producer - when
> hash collision happens, it might refer to completely unrelated type).

I guess there cannot be two struct/class definitions with different fields accessibility accidentally matching each other due to ODR.

Still I believe DW_TAG_type_unit should have DW_AT_producer as it is a standalone entity - but currently it is not.  I guess I have to implement your described althorithm into GDB instead.  Dodji, thanks and sorry for the patch.
Comment 5 Jan Kratochvil 2011-03-28 09:12:20 UTC
Created attachment 23788 [details]
GDB patch attempt with ptype regressions.

(In reply to comment #4)
> Still I believe DW_TAG_type_unit should have DW_AT_producer as it is a
> standalone entity - but currently it is not.  I guess I have to implement your
> described althorithm into GDB instead.

It cannot work as GDB often looks up the type without any referrer from DW_TAG_compile_unit, such as during the `ptype' GDB command.  A draft patch thus has many regressions such as:
-PASS: gdb.base/nofield.exp: ptype struct not_empty
+FAIL: gdb.base/nofield.exp: ptype struct not_empty (GDB internal error)

This means GDB will have to start full read (like -readnow) of CUs till it finds some CU referencing the specific type to find its DW_AT_producer.  During scan of .debug_info for GDB partial symbols GDB currently skips over subtrees of DIEs which reference the type signature for performance reasons, it no longer can.

With .gdb_index there is no GDB partial symbols scan but .gdb_index also indexes types by their name and there is no referrer ever seen during the `ptype' GDB command.

So it means performance regression for non-.gdb_index case and a possible new extension/version of .gdb_index to handle it.
Comment 6 Jakub Jelinek 2011-03-28 09:22:57 UTC
.debug_types only appears with -gdwarf-4, I don't think any gcc defaulted to that yet, so if you are talking about DW_AT_accessibility default, IMHO just assume the DWARF3+ semantics in .debug_types.
Comment 7 Jan Kratochvil 2011-03-28 11:21:41 UTC
In such case only the gcc-4.5.x branch should start to produce DW_AT_accessibility unambiguously (explicitly) at least in the -gdwarf-4 mode.
This way it will not remain a long term regression testsuite problem.