Debug info for comdat functions

Debug info for comdat functions

[Jakub Jelinek]

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

Hi!

Something not addressed yet in dwz and unfortunately without
linker or compiler help not 100% addressable is debug info for
comdat functions.

Consider attached testcase with comdat foo function, seems the
current linker behavior (well, tested with 2.21.53.0.1 ld.bfd)
is that for DW_TAG_subprogram with DW_AT_low_pc/DW_AT_high_pc
having section relative relocs against comdat functions
if the comdat text section has the same size in both object
files, then DW_AT_low_pc (and DW_AT_high_pc) attributes
in both CUs will point to the same range.
E.g. when compiling g++ -gdwarf-4 -o t9 t91.C t92.C, both
.text._Z3fooi sections are indentical one byte.
I think if the section content is identical, then what the
linker does is fine and perhaps dwz could just do something
with it later on (currently it doesn't consider DIEs with
DW_AT_low_pc/DW_AT_high_pc/DW_AT_ranges attributes for dup
removal).  If both .text._Z3fooi sections have different
sizes, then the linker will clear DW_AT_low_pc/DW_AT_high_pc,
which is also fine (compile e.g. t91.C with -O2 and t92.C with -O0).
I guess most debug info consumers will ignore the 0..0 range
and dwz could be tought to do something about those DW_TAG_subprogram
nodes too (what exactly?  Drop DW_AT_{low_pc,high_pc,ranges} attribute
from them, drop all DW_TAG_inlined_subroutine/DW_TAG_lexical_block
children (perhaps all children?) of them, rewrite .debug_loc section
if some portion of it was only referenced by to be removed DIEs?).

The problematic case (I'd say a linker bug) is when the .text._Z3fooi
sections have the same size, but different content (compiled with
different options, but by lack of luck happened to have the same size).
Tested by hacking up t91.s and t92.s both built with -O2 to have different,
but same sized, instructions in .text._Z3fooi.  IMHO in that case
will debug info consumers see wrong debug info and dwz can't guess what
DIE describes the actual content and what DIE describes something
that has been removed.

For the libreoffice test files I have (and libstdc++.so) I've quickly
hacked up a guess how much could be saved by handling the comdats
in dwz - the numbers are the size of DW_TAG_subprogram DIE and all its
children if the same values of both DW_AT_low_pc/DW_AT_high_pc attributes
were already seen in another DIE.  Possible .debug_loc saving isn't
accounted for, on the other side cost of DW_TAG_imported_unit,
DW_TAG_partial_unit and/or keeping around a small portion of the
DW_TAG_subprogram die for 0..0 ranges isn't in either.

liblwpftlo.so.debug 1160625
libooxlo.so.debug 939155
libswlo.so.debug 819029
libooxmllo.so.debug 789318
libsclo.so.debug 740099
libchartmodello.so.debug 636127
libsdlo.so.debug 592827
libdbulo.so.debug 458561
libsvxcorelo.so.debug 455718
libchartcontrollerlo.so.debug 418735
libfrmlo.so.debug 410486
slideshow.uno.so.debug 392586
libdbalo.so.debug 374204
libfwklo.so.debug 359078
libxolo.so.debug 327187
libsfxlo.so.debug 294460
vbaobj.uno.so.debug 282619
libtklo.so.debug 239364
libacclo.so.debug 227900
libvcllo.so.debug 209380
libdrawinglayerlo.so.debug 202697
libbf_frmlo.so.debug 192942
libscfiltlo.so.debug 188769
libbf_xolo.so.debug 184482
libbf_svxlo.so.debug 178985
libdbtoolslo.so.debug 172211
vbaswobj.uno.so.debug 169971
libbf_swlo.so.debug 169310
libsvtlo.so.debug 165993
libmswordlo.so.debug 151021
libcharttoolslo.so.debug 148725
libdoctoklo.so.debug 148573
libcomphelpgcc3.so.debug 143429
cairocanvas.uno.so.debug 142835
libbf_sclo.so.debug 140327
libcuilo.so.debug 133518
libpcrlo.so.debug 132128
i18npool.uno.so.debug 125995
vclcanvas.uno.so.debug 117225
libdeployment.so.debug 109223
libchartviewlo.so.debug 101869
libsvxlo.so.debug 96798
librptuilo.so.debug 95625
postgresql-sdbc-impl.uno.so.debug 95311
libswuilo.so.debug 94986
msforms.uno.so.debug 88318
libutllo.so.debug 72778
libbf_svtlo.so.debug 67642
libunoxmllo.so.debug 65290
libfilterconfiglo.so.debug 64802
libsblo.so.debug 63344
librptlo.so.debug 60664
libvbahelperlo.so.debug 60292
libbf_schlo.so.debug 58526
configmgr.uno.so.debug 58439
libeditenglo.so.debug 57242
libfilelo.so.debug 56444
libfwllo.so.debug 53012
libpackage2.so.debug 50186
libxcrlo.so.debug 49733
libcppcanvaslo.so.debug 47957
libbasctllo.so.debug 40421
libbf_sdlo.so.debug 38870
libsvllo.so.debug 37970
libxsec_fw.so.debug 34438
libjdbclo.so.debug 33137
libdbaselo.so.debug 32789
libxmlsecurity.so.debug 32416
libhsqldb.so.debug 32403
libsmlo.so.debug 32339
libuuilo.so.debug 30973
liblnglo.so.debug 29532
libfwelo.so.debug 28311
libodbcbaselo.so.debug 27829
librptxmllo.so.debug 27530
libwpftlo.so.debug 26609
libscuilo.so.debug 26554
libucpchelp1.so.debug 25999
libdeploymentgui.so.debug 25982
libmysqllo.so.debug 25576
libfwilo.so.debug 25144
libembobj.so.debug 23555
libxstor.so.debug 23167
libsofficeapp.so.debug 23124
libmsfilterlo.so.debug 22551
libdbaxmllo.so.debug 22032
libucpfile1.so.debug 21467
libxsec_xmlsec.so.debug 19768
libevoablo.so.debug 19409
libspalo.so.debug 18660
libflatlo.so.debug 18345
libucbhelper4gcc3.so.debug 18066
libsdfiltlo.so.debug 17906
libcalclo.so.debug 17773
libucpdav1.so.debug 17769
libmsworkslo.so.debug 16706
libsduilo.so.debug 15592
libxoflo.so.debug 13298
librtftoklo.so.debug 12403
migrationoo2.uno.so.debug 12237
libpyuno.so.debug 11670
libxsltdlglo.so.debug 10628
libemboleobj.so.debug 10156
fps_office.uno.so.debug 10020
libstdc++.so 9718
libucb1.so.debug 9121
libcanvastoolslo.so.debug 9059
libwpgimportlo.so.debug 9035
libvisioimportlo.so.debug 9035
libpllo.so.debug 8327
libscriptframe.so.debug 8290
libbf_sblo.so.debug 7769
libbiblo.so.debug 7731
libvclplug_gtklo.so.debug 7691
libloglo.so.debug 7591
libucpftp1.so.debug 7388
libfwmlo.so.debug 7056
ucptdoc1.uno.so.debug 7025
libsvgfilterlo.so.debug 6935
libcached1.so.debug 6869
basprov.uno.so.debug 6577
libvclplug_genlo.so.debug 6439
libsotlo.so.debug 6156
libhelplinkerlo.so.debug 6143
libdbplo.so.debug 5872
libunordflo.so.debug 5840
libucphier1.so.debug 5810
libsdbtlo.so.debug 4974
libreslo.so.debug 4886
libbf_smlo.so.debug 4785
libwriterfilterlo.so.debug 4776
libvclplug_svplo.so.debug 4688
libdeploymentmisclo.so.debug 4503
libpdffilterlo.so.debug 4500
libunopkgapp.so.debug 4269
libbf_solo.so.debug 4264
libdbmmlo.so.debug 4246
fsstorage.uno.so.debug 4107
libhwplo.so.debug 3978
libbasegfxlo.so.debug 3864
libctllo.so.debug 3723
libabplo.so.debug 3607
libresourcemodello.so.debug 3123
libflashlo.so.debug 2889
ucpgio1.uno.so.debug 2612
expwrap.uno.so.debug 2504
dlgprov.uno.so.debug 2481
cmdmail.uno.so.debug 2388
liblnthlo.so.debug 2236
libicglo.so.debug 2145
libucppkg1.so.debug 2075
libdbpool2.so.debug 2048
libforuilo.so.debug 1858
ucpcmis1.uno.so.debug 1838
libspelllo.so.debug 1838
libhyphenlo.so.debug 1838
libxsltfilterlo.so.debug 1837
nsplugin.debug 1737
libtllo.so.debug 1682
libsrtrs1.so.debug 1599
libavmediagst.so.debug 1462
libavmedialo.so.debug 1409
hatchwindowfactory.uno.so.debug 1386
libtvhlp1.so.debug 1375
libanimcorelo.so.debug 1345
libanalysislo.so.debug 1278
libmozbootstrap.so.debug 1258
libbasebmplo.so.debug 1229
passwordcontainer.uno.so.debug 1151
libsaxlo.so.debug 1094
fastsax.uno.so.debug 1061
libtextconversiondlgslo.so.debug 941
ucpext.uno.so.debug 690
libplacewarelo.so.debug 654
libadabasuilo.so.debug 647
libguesslanglo.so.debug 617
libmcnttype.so.debug 516
libt602filterlo.so.debug 512
libscnlo.so.debug 429
libforlo.so.debug 427
libxmlfalo.so.debug 333
kde4be1.uno.so.debug 204
libbf_wrapperlo.so.debug 186
libspllo.so.debug 184
gconfbe1.uno.so.debug 130
libvclplug_kde4lo.so.debug 127
spadmin.bin.debug 124
libxmxlo.so.debug 124
fps_kde4.uno.so.debug 124
syssh.uno.so.debug 116
libxmlfdlo.so.debug 62
libswdlo.so.debug 62
libsmdlo.so.debug 62
libsddlo.so.debug 62
libsdbc2.so.debug 62
libscdlo.so.debug 62
libbf_migratefilterlo.so.debug 62


	Jakub

[-- Attachment #2: t9.h --]
[-- Type: text/plain, Size: 237 bytes --]

inline void
bar (int d)
{
  int e = d + 5;
  asm ("");
}

inline void
foo (int a)
{
  int b = a + 1;
  asm ("");
  {
    int c = 2 * a + b + 26;
    asm ("");
  }
}

struct S
{
  void baz (int f) { int g = f + 4; asm (""); }
  int s;
};

[-- Attachment #3: t91.C --]
[-- Type: text/plain, Size: 118 bytes --]

#include "t9.h"

void *x = (void *) foo;

void
fn1 ()
{
  S s;
  foo (4);
  s.baz (12);
}

int
main ()
{
  return 0;
}

[-- Attachment #4: t92.C --]
[-- Type: text/plain, Size: 90 bytes --]

#include "t9.h"

void *y = (void *) foo;

void
fn2 ()
{
  S s;
  foo (4);
  s.baz (12);
}

Re: Debug info for comdat functions

[Jakub Jelinek]

Hi!

Sorry for following up to self, but something I forgot to add
about this:

On Wed, Apr 18, 2012 at 01:16:40PM +0200, Jakub Jelinek wrote:
> Something not addressed yet in dwz and unfortunately without
> linker or compiler help not 100% addressable is debug info for
> comdat functions.

When discussed on IRC recently Jason preferred to move the DW_TAG_subprogram
describing a comdat function to a comdat .debug_info DW_TAG_partial_unit
and just reference all DIEs that need to be referenced from it
using DW_FORM_ref_addr back to the non-comdat .debug_info.  Perhaps put its
sole .debug_loc contributions into comdat part as well, .debug_ranges maybe
too.  I've thought about that approach a little bit, but I see issues with
that, at least with the current linker behavior.
In particular, even for identical .text.* section content different CUs
might have slightly different partial units.  The comdat .debug_info
sections couldn't be hashed in any way, it would use normal comdat
mechanism.  We would have DW_TAG_imported_unit with DW_AT_import
attribute pointing to the start DW_TAG_partial_unit in the section
(we would need to hardcode the +11 bytes offset, assuming nobody
ever emits 64-bit DWARF) and not refer to any other DIEs from the partial
unit.  If the comdat .debug_info section sizes are the same, it will
work fine (unless the IMHO ld bug mentioned in previous mail is fixed,
then it would work only if the section is bitwise identical).  But if they
are different, the linker will put there 0 for the relocation rather,
which doesn't refer to any DW_TAG_*_unit and is thus invalid DWARF.

	Jakub

Re: Debug info for comdat functions

[Jason Merrill]

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

On 04/18/2012 07:53 AM, Jakub Jelinek wrote:
> Consider attached testcase with comdat foo function, seems the
> current linker behavior (well, tested with 2.21.53.0.1 ld.bfd)
> is that for DW_TAG_subprogram with DW_AT_low_pc/DW_AT_high_pc
> having section relative relocs against comdat functions
> if the comdat text section has the same size in both object
> files, then DW_AT_low_pc (and DW_AT_high_pc) attributes
> in both CUs will point to the same range.

This seems clearly wrong to me.  A reference to a symbol in a discarded 
section should not resolve to an offset into a different section.  I 
thought the linker always resolved such references to 0, and I think 
that is what we want.

> When discussed on IRC recently Jason preferred to move the DW_TAG_subprogram
> describing a comdat function to a comdat .debug_info DW_TAG_partial_unit
> and just reference all DIEs that need to be referenced from it
> using DW_FORM_ref_addr back to the non-comdat .debug_info.

I played around with implementing this in the compiler yesterday; my 
initial patch is attached.  It seems that with normal DWARF 4 this can 
work well, but I ran into issues with various GNU extensions:

DW_TAG_GNU_call_site wants to refer to the called function's DIE, so the 
function die in the separate unit needs to have its own symbol.  Perhaps 
_call_site could refer to the function symbol instead?  That seems more 
correct anyway, since with COMDAT functions you might end up calling a 
different version of the function that has a different DIE.

The typed stack ops such as DW_OP_GNU_deref_type want to refer to a type 
in the same CU, so we would need to copy any referenced base types into 
the separate function CU.  Could we add variants of these ops that take 
an offset from .debug_info?

> Perhaps put its
> sole .debug_loc contributions into comdat part as well, .debug_ranges maybe
> too.

I haven't done anything with .debug_loc yet.

.debug_ranges mostly goes away with this change; the main CU becomes 
just .text and the separate CUs are just their own function.  I suppose 
.debug_ranges would still be needed with hot/cold optimizations.

> We would have DW_TAG_imported_unit with DW_AT_import
> attribute pointing to the start DW_TAG_partial_unit in the section
> (we would need to hardcode the +11 bytes offset, assuming nobody
> ever emits 64-bit DWARF) and not refer to any other DIEs from the partial
> unit.

I think it would be both better and more correct to have the 
DW_AT_imported_unit going the other way, so the function CU imports the 
main CU.  That's what DWARF4 appendix E suggests.  My patch doesn't 
implement this yet.

Jason

[-- Attachment #2: comdat-fn-debug.patch --]
[-- Type: text/x-patch, Size: 14692 bytes --]

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index abe3f1b..c113c63 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -8612,6 +8612,7 @@ ix86_code_end (void)
 				       NULL_TREE, void_type_node);
       TREE_PUBLIC (decl) = 1;
       TREE_STATIC (decl) = 1;
+      DECL_IGNORED_P (decl) = 1;
 
 #if TARGET_MACHO
       if (TARGET_MACHO)
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 7e2ce58..0c33af2 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -1007,6 +1007,7 @@ dwarf2out_begin_prologue (unsigned int line ATTRIBUTE_UNUSED,
   fde->dw_fde_current_label = dup_label;
   fde->in_std_section = (fnsec == text_section
 			 || (cold_text_section && fnsec == cold_text_section));
+  fde->comdat = DECL_ONE_ONLY (current_function_decl);
 
   /* We only want to output line number information for the genuine dwarf2
      prologue case, not the eh frame case.  */
@@ -3291,8 +3292,10 @@ static void compute_section_prefix (dw_die_ref);
 static int is_type_die (dw_die_ref);
 static int is_comdat_die (dw_die_ref);
 static int is_symbol_die (dw_die_ref);
+static int is_abstract_die (dw_die_ref);
 static void assign_symbol_names (dw_die_ref);
 static void break_out_includes (dw_die_ref);
+static void break_out_comdat_functions (dw_die_ref);
 static int is_declaration_die (dw_die_ref);
 static int should_move_die_to_comdat (dw_die_ref);
 static dw_die_ref clone_as_declaration (dw_die_ref);
@@ -4105,6 +4108,9 @@ dwarf_attr_name (unsigned int attr)
     case DW_AT_GNU_macros:
       return "DW_AT_GNU_macros";
 
+    case DW_AT_GNU_comdat:
+      return "DW_AT_GNU_comdat";
+
     case DW_AT_GNAT_descriptive_type:
       return "DW_AT_GNAT_descriptive_type";
 
@@ -6698,6 +6704,9 @@ is_symbol_die (dw_die_ref c)
 {
   return (is_type_die (c)
 	  || is_declaration_die (c)
+	  || is_abstract_die (c)
+	  /* DW_TAG_GNU_call_site can refer to subprograms.  */
+	  || c->die_tag == DW_TAG_subprogram
 	  || c->die_tag == DW_TAG_namespace
 	  || c->die_tag == DW_TAG_module);
 }
@@ -6728,6 +6737,8 @@ assign_symbol_names (dw_die_ref die)
 
   if (is_symbol_die (die))
     {
+      if (die->die_id.die_symbol)
+	return;
       if (comdat_symbol_id)
 	{
 	  char *p = XALLOCAVEC (char, strlen (comdat_symbol_id) + 64);
@@ -6900,6 +6911,65 @@ break_out_includes (dw_die_ref die)
   htab_delete (cu_hash_table);
 }
 
+static const char *
+function_die_label (const char *comdat_id)
+{
+  char *p = XALLOCAVEC (char, strlen (comdat_id) + 10);
+  sprintf (p, DIE_LABEL_PREFIX ".%s", comdat_id);
+  return xstrdup (p);
+}
+
+/* Traverse the DIE (which is always comp_unit_die), and set up additional
+   compilation units for each of the comdat functions we see.  */
+
+static void
+break_out_comdat_functions (dw_die_ref die)
+{
+  dw_die_ref c;
+  dw_die_ref unit = NULL;
+  limbo_die_node *node;
+  dw_die_ref prev;
+  bool found = false;
+
+  prev = die->die_child;
+  if (prev == NULL)
+    return;
+
+  do
+    {
+      c = prev->die_sib;
+      if (c->die_tag == DW_TAG_subprogram && get_AT (c, DW_AT_GNU_comdat))
+	{
+	  dw_die_ref unit = gen_compile_unit_die (NULL);
+
+	  /* This DIE is for a secondary CU; remove it from the main one.  */
+	  remove_child_with_prev (c, prev);
+	  add_child_die (unit, c);
+	  found = true;
+	}
+      else
+	prev = c;
+    }
+  while (c != die->die_child);
+
+  if (!found)
+    return;
+
+  assign_symbol_names (die);
+  for (node = limbo_die_list;
+       node;
+       node = node->next)
+    {
+      const char *comdat_id;
+      unit = node->die;
+      c = unit->die_child->die_sib;
+      comdat_id = get_AT_string (c, DW_AT_GNU_comdat);
+      remove_AT (c, DW_AT_GNU_comdat);
+      unit->die_id.die_symbol = comdat_id;
+      c->die_id.die_symbol = function_die_label (comdat_id);
+    }
+}
+
 /* Return non-zero if this DIE is a declaration.  */
 
 static int
@@ -6915,6 +6985,37 @@ is_declaration_die (dw_die_ref die)
   return 0;
 }
 
+/* Return non-zero if this DIE is part of an abstract inline.  That is, if
+   it's an inline function or a variable, label or parameter of an inline
+   function.  */
+
+static int
+is_abstract_die (dw_die_ref die)
+{
+  switch (die->die_tag)
+    {
+    case DW_TAG_subprogram:
+    case DW_TAG_variable:
+    case DW_TAG_label:
+    case DW_TAG_formal_parameter:
+      break;
+
+    default:
+      return 0;
+    }
+
+  for (; die->die_tag != DW_TAG_subprogram; die = die->die_parent)
+    {
+      if (die->die_tag == DW_TAG_compile_unit
+	  || die->die_tag == DW_TAG_namespace)
+	return 0;
+    }
+
+  if (get_AT (die, DW_AT_inline))
+    return 1;
+  return 0;
+}
+
 /* Return non-zero if this DIE is nested inside a subprogram.  */
 
 static int
@@ -8560,8 +8661,7 @@ output_compilation_unit_header (void)
 static void
 output_comp_unit (dw_die_ref die, int output_if_empty)
 {
-  const char *secname;
-  char *oldsym, *tmp;
+  char *oldsym;
 
   /* Unless we are outputting main CU, we may throw away empty ones.  */
   if (!output_if_empty && die->die_child == NULL)
@@ -8583,12 +8683,19 @@ output_comp_unit (dw_die_ref die, int output_if_empty)
   oldsym = die->die_id.die_symbol;
   if (oldsym)
     {
-      tmp = XALLOCAVEC (char, strlen (oldsym) + 24);
+#ifdef OBJECT_FORMAT_ELF
+      targetm.asm_out.named_section (".debug_info",
+				     SECTION_DEBUG | SECTION_LINKONCE,
+				     get_identifier (oldsym));
+#else
+      char *tmp = XALLOCAVEC (char, strlen (oldsym) + 24);
+      const char *secname;
 
       sprintf (tmp, ".gnu.linkonce.wi.%s", oldsym);
       secname = tmp;
-      die->die_id.die_symbol = NULL;
       switch_to_section (get_section (secname, SECTION_DEBUG, NULL));
+#endif
+      die->die_id.die_symbol = NULL;
     }
   else
     {
@@ -17298,13 +17405,13 @@ gen_subprogram_die (tree decl, dw_die_ref context_die)
   else if (!DECL_EXTERNAL (decl))
     {
       HOST_WIDE_INT cfa_fb_offset;
+      dw_fde_ref fde = cfun->fde;
 
       if (!old_die || !get_AT (old_die, DW_AT_inline))
 	equate_decl_number_to_die (decl, subr_die);
 
       if (!flag_reorder_blocks_and_partition)
 	{
-	  dw_fde_ref fde = cfun->fde;
 	  if (fde->dw_fde_begin)
 	    {
 	      /* We have already generated the labels.  */
@@ -17352,8 +17459,6 @@ gen_subprogram_die (tree decl, dw_die_ref context_die)
       else
 	{
 	  /* Generate pubnames entries for the split function code ranges.  */
-	  dw_fde_ref fde = cfun->fde;
-
 	  if (fde->dw_fde_second_begin)
 	    {
 	      if (dwarf_version >= 3 || !dwarf_strict)
@@ -17455,6 +17560,13 @@ gen_subprogram_die (tree decl, dw_die_ref context_die)
 	    add_AT_loc (subr_die, DW_AT_frame_base, list->expr);
 	}
 
+      if (fde->comdat)
+	{
+	  gcc_assert (context_die == comp_unit_die ());
+	  add_AT_string (subr_die, DW_AT_GNU_comdat,
+			 IDENTIFIER_POINTER (DECL_COMDAT_GROUP (decl)));
+	}
+
       /* Compute a displacement from the "steady-state frame pointer" to
 	 the CFA.  The former is what all stack slots and argument slots
 	 will reference in the rtl; the later is what we've told the
@@ -18152,7 +18264,6 @@ add_high_low_attributes (tree stmt, dw_die_ref die)
 	}
 
       add_AT_range_list (die, DW_AT_ranges, add_ranges (stmt));
-
       chain = BLOCK_FRAGMENT_CHAIN (stmt);
       do
 	{
@@ -22609,6 +22720,8 @@ dwarf2out_finish (const char *filename)
       prune_unused_types ();
     }
 
+  break_out_comdat_functions (comp_unit_die ());
+
   /* Traverse the DIE's and add add sibling attributes to those DIE's
      that have children.  */
   add_sibling_attributes (comp_unit_die ());
@@ -22626,67 +22739,115 @@ dwarf2out_finish (const char *filename)
       targetm.asm_out.internal_label (asm_out_file, COLD_END_LABEL, 0);
     }
 
-  /* We can only use the low/high_pc attributes if all of the code was
-     in .text.  */
-  if (!have_multiple_function_sections 
-      || (dwarf_version < 3 && dwarf_strict))
-    {
-      /* Don't add if the CU has no associated code.  */
-      if (text_section_used)
+  /* Add range information to the main CU.  We can only use the low/high_pc
+     attributes if all of the code was in .text.  */
+  {
+    unsigned fde_idx;
+    dw_fde_ref fde;
+    bool found = false;
+
+    /* Are there any extra function sections that belong to the main CU? */
+    FOR_EACH_VEC_ELT (dw_fde_ref, fde_vec, fde_idx, fde)
+      if (!fde->comdat && !DECL_IGNORED_P (fde->decl)
+	  && (!fde->in_std_section
+	      || (fde->dw_fde_second_begin && !fde->second_in_std_section)))
 	{
-	  add_AT_lbl_id (comp_unit_die (), DW_AT_low_pc, text_section_label);
-	  add_AT_lbl_id (comp_unit_die (), DW_AT_high_pc, text_end_label);
+	  found = true;
+	  break;
 	}
-    }
-  else
+
+    if (!(found || cold_text_section_used)
+	|| (dwarf_version < 3 && dwarf_strict))
+      {
+	/* Don't add if the CU has no associated code.  */
+	if (text_section_used)
+	  {
+	    add_AT_lbl_id (comp_unit_die (), DW_AT_low_pc, text_section_label);
+	    add_AT_lbl_id (comp_unit_die (), DW_AT_high_pc, text_end_label);
+	  }
+      }
+    else
+      {
+	bool range_list_added = false;
+
+	if (text_section_used)
+	  add_ranges_by_labels (comp_unit_die (), text_section_label,
+				text_end_label, &range_list_added);
+	if (cold_text_section_used)
+	  add_ranges_by_labels (comp_unit_die (), cold_text_section_label,
+				cold_end_label, &range_list_added);
+
+	FOR_EACH_VEC_ELT (dw_fde_ref, fde_vec, fde_idx, fde)
+	  {
+	    if (fde->comdat || DECL_IGNORED_P (fde->decl))
+	      continue;
+	    if (!fde->in_std_section)
+	      add_ranges_by_labels (comp_unit_die (), fde->dw_fde_begin,
+				    fde->dw_fde_end, &range_list_added);
+	    if (fde->dw_fde_second_begin && !fde->second_in_std_section)
+	      add_ranges_by_labels (comp_unit_die (), fde->dw_fde_second_begin,
+				    fde->dw_fde_second_end, &range_list_added);
+	  }
+
+	if (range_list_added)
+	  {
+	    /* We need to give .debug_loc and .debug_ranges an appropriate
+	       "base address".  Use zero so that these addresses become
+	       absolute.  Historically, we've emitted the unexpected
+	       DW_AT_entry_pc instead of DW_AT_low_pc for this purpose.
+	       Emit both to give time for other tools to adapt.  */
+	    add_AT_addr (comp_unit_die (), DW_AT_low_pc, const0_rtx);
+	    if (! dwarf_strict && dwarf_version < 4)
+	      add_AT_addr (comp_unit_die (), DW_AT_entry_pc, const0_rtx);
+
+	    add_ranges (NULL);
+	  }
+      }
+  }
+
+  /* Also add ranges to the CUs for comdat functions.  */
+  for (node = limbo_die_list; node; node = node->next)
     {
-      unsigned fde_idx;
-      dw_fde_ref fde;
-      bool range_list_added = false;
-
-      if (text_section_used)
-	add_ranges_by_labels (comp_unit_die (), text_section_label,
-			      text_end_label, &range_list_added);
-      if (cold_text_section_used)
-	add_ranges_by_labels (comp_unit_die (), cold_text_section_label,
-			      cold_end_label, &range_list_added);
-
-      FOR_EACH_VEC_ELT (dw_fde_ref, fde_vec, fde_idx, fde)
-	{
-	  if (!fde->in_std_section)
-	    add_ranges_by_labels (comp_unit_die (), fde->dw_fde_begin,
-				  fde->dw_fde_end, &range_list_added);
-	  if (fde->dw_fde_second_begin && !fde->second_in_std_section)
-	    add_ranges_by_labels (comp_unit_die (), fde->dw_fde_second_begin,
-				  fde->dw_fde_second_end, &range_list_added);
-	}
+      dw_die_ref c = node->die->die_child->die_sib;
+      dw_attr_ref a;
+
+      if (c->die_tag != DW_TAG_subprogram)
+	continue;
 
-      if (range_list_added)
+      if ((a = get_AT (c, DW_AT_ranges)))
+	add_AT_range_list (node->die, DW_AT_ranges,
+			   a->dw_attr_val.v.val_offset);
+      else
 	{
-	  /* We need to give .debug_loc and .debug_ranges an appropriate
-	     "base address".  Use zero so that these addresses become
-	     absolute.  Historically, we've emitted the unexpected
-	     DW_AT_entry_pc instead of DW_AT_low_pc for this purpose.
-	     Emit both to give time for other tools to adapt.  */
-	  add_AT_addr (comp_unit_die (), DW_AT_low_pc, const0_rtx);
-	  if (! dwarf_strict && dwarf_version < 4)
-	    add_AT_addr (comp_unit_die (), DW_AT_entry_pc, const0_rtx);
-
-	  add_ranges (NULL);
+	  add_AT_lbl_id (node->die, DW_AT_low_pc, get_AT_low_pc (c));
+	  add_AT_lbl_id (node->die, DW_AT_high_pc, get_AT_hi_pc (c));
 	}
     }
 
   if (debug_info_level >= DINFO_LEVEL_NORMAL)
-    add_AT_lineptr (comp_unit_die (), DW_AT_stmt_list,
-		    debug_line_section_label);
+    {
+      add_AT_lineptr (comp_unit_die (), DW_AT_stmt_list,
+		      debug_line_section_label);
+      /* ??? comdat line info for comdat functions?  */
+      for (node = limbo_die_list; node; node = node->next)
+	add_AT_lineptr (node->die, DW_AT_stmt_list,
+			debug_line_section_label);
+    }
 
   if (debug_info_level >= DINFO_LEVEL_VERBOSE)
-    add_AT_macptr (comp_unit_die (),
-		   dwarf_strict ? DW_AT_macro_info : DW_AT_GNU_macros,
-		   macinfo_section_label);
+    {
+      add_AT_macptr (comp_unit_die (),
+		     dwarf_strict ? DW_AT_macro_info : DW_AT_GNU_macros,
+		     macinfo_section_label);
+      for (node = limbo_die_list; node; node = node->next)
+	add_AT_macptr (node->die,
+		       dwarf_strict ? DW_AT_macro_info : DW_AT_GNU_macros,
+		       macinfo_section_label);
+    }
 
   if (have_location_lists)
-    optimize_location_lists (comp_unit_die ());
+    for (node = limbo_die_list; node; node = node->next)
+      optimize_location_lists (node->die);
 
   /* Output all of the compilation units.  We put the main one last so that
      the offsets are available to output_pubnames.  */
@@ -22735,6 +22896,8 @@ dwarf2out_finish (const char *filename)
 				   DEBUG_LOC_SECTION_LABEL, 0);
       ASM_OUTPUT_LABEL (asm_out_file, loc_section_label);
       output_location_lists (comp_unit_die ());
+      for (node = limbo_die_list; node; node = node->next)
+	output_location_lists (node->die);
     }
 
   /* Output public names table if necessary.  */
diff --git a/gcc/dwarf2out.h b/gcc/dwarf2out.h
index 711e8ab..bee757a 100644
--- a/gcc/dwarf2out.h
+++ b/gcc/dwarf2out.h
@@ -109,6 +109,8 @@ typedef struct GTY(()) dw_fde_struct {
   /* True iff dw_fde_second_begin label is in text_section or
      cold_text_section.  */
   unsigned second_in_std_section : 1;
+  /* True iff the function is part of a comdat group.  */
+  unsigned comdat : 1;
 }
 dw_fde_node;
 
diff --git a/include/dwarf2.h b/include/dwarf2.h
index 8c0c9ed..d3c8cb3 100644
--- a/include/dwarf2.h
+++ b/include/dwarf2.h
@@ -379,6 +379,8 @@ enum dwarf_attribute
     DW_AT_GNU_addr_base = 0x2133,
     DW_AT_GNU_pubnames = 0x2134,
     DW_AT_GNU_pubtypes = 0x2135,
+    /* Never actually emitted.  */
+    DW_AT_GNU_comdat = 0x2136,
     /* VMS extensions.  */
     DW_AT_VMS_rtnbeg_pd_address = 0x2201,
     /* GNAT extensions.  */

Re: Debug info for comdat functions

[Jakub Jelinek]

On Wed, Apr 18, 2012 at 08:43:37AM -0400, Jason Merrill wrote:
> On 04/18/2012 07:53 AM, Jakub Jelinek wrote:
> >Consider attached testcase with comdat foo function, seems the
> >current linker behavior (well, tested with 2.21.53.0.1 ld.bfd)
> >is that for DW_TAG_subprogram with DW_AT_low_pc/DW_AT_high_pc
> >having section relative relocs against comdat functions
> >if the comdat text section has the same size in both object
> >files, then DW_AT_low_pc (and DW_AT_high_pc) attributes
> >in both CUs will point to the same range.
> 
> This seems clearly wrong to me.  A reference to a symbol in a
> discarded section should not resolve to an offset into a different
> section.  I thought the linker always resolved such references to 0,
> and I think that is what we want.

If the .text (and all other allocated sections) in the comdat group
is bitwise identical, I think it isn't a problem to refer to that,
it really doesn't matter at that point which object file won owning it.
But if it is different, I really think it is a bug not to clear it.

> >When discussed on IRC recently Jason preferred to move the DW_TAG_subprogram
> >describing a comdat function to a comdat .debug_info DW_TAG_partial_unit
> >and just reference all DIEs that need to be referenced from it
> >using DW_FORM_ref_addr back to the non-comdat .debug_info.
> 
> I played around with implementing this in the compiler yesterday; my
> initial patch is attached.  It seems that with normal DWARF 4 this
> can work well, but I ran into issues with various GNU extensions:

Importing the non-comdat .debug_info from comdat .debug_info is an
interesting approach.  My slight problem with that is that the debug info
no longer describes the input source file 1:1, eventhough the
DW_TAG_imported_unit brings in stuff like local variables (so that when
settping through that comdat e.g. static variables in the same source file
will be visible), other comdat functions in that file won't.  But perhaps
that is not a big issue, guess it is up to the debug info consumer folks
to chime in about that.

> DW_TAG_GNU_call_site wants to refer to the called function's DIE, so
> the function die in the separate unit needs to have its own symbol.
> Perhaps _call_site could refer to the function symbol instead?  That
> seems more correct anyway, since with COMDAT functions you might end
> up calling a different version of the function that has a different
> DIE.

At this point it is too late to change the specification of the extension.
But you could just put in a DW_TAG_subprogram DW_AT_external declaration
in the main .debug_info and just refer to that from call_site as well
as from DW_AT_specification in the comdat .debug_info.  That
DW_AT_abstract_origin is meant there to be just one of the possible many
DIEs referring to the callee, the debug info consumer is supposed to find
out the actual DIE that contains the code from it using its usual
mechanisms.
Or, for call references to the comdat functions you can drop
DW_AT_abstract_origin attribute and instead provide
DW_AT_call_site_target with DW_OP_addr <address_of_the_comdat_function>.
The latter has the disadvantage that the linker will clear it from time to
time (if its .text.* section size is different).

> The typed stack ops such as DW_OP_GNU_deref_type want to refer to a
> type in the same CU, so we would need to copy any referenced base
> types into the separate function CU.  Could we add variants of these
> ops that take an offset from .debug_info?

The DW_TAG_base_type is small enough that we can duplicate it, in the
dup we actually could drop DW_AT_name (i.e. keep it as is in the main
.debug_info and in the comdat just provide DW_AT_encoding/DW_AT_byte_size).
That is 3 bytes for the base type, small enough that it offsets for the
smaller uleb128 sizes.

	Jakub

Re: Debug info for comdat functions

[Cary Coutant]

> This seems clearly wrong to me.  A reference to a symbol in a discarded
> section should not resolve to an offset into a different section.  I thought
> the linker always resolved such references to 0, and I think that is what we
> want.

Even resolving to 0 can cause problems. In the Gnu linker, all
references to a discarded symbol get relocated to 0, ignoring any
addend. This can result in spurious (0,0) pairs in range lists. In
Gold, we treat the discarded symbol as 0, but still apply the addend,
and count on GDB to recognize that the function starting at 0 must
have been discarded. Neither solution is ideal. That's why debug info
for COMDAT functions ought to be in the same COMDAT group as the
function...

>> When discussed on IRC recently Jason preferred to move the
>> DW_TAG_subprogram
>> describing a comdat function to a comdat .debug_info DW_TAG_partial_unit
>> and just reference all DIEs that need to be referenced from it
>> using DW_FORM_ref_addr back to the non-comdat .debug_info.
>
> I played around with implementing this in the compiler yesterday; my initial
> patch is attached.  It seems that with normal DWARF 4 this can work well,
> but I ran into issues with various GNU extensions:

Nice -- I've been wanting to do that for a while, but I always thought
it would be a lot harder. I see that you've based this on the
infrastructure created for -feliminate-dwarf2-dups. I don't think that
will play nice with -fdebug-types-section, though, since I basically
made those two options incompatible with each other by unioning
die_symbol with die_type_node.

In the HP-UX compilers, we basically put a complete set of .debug_*
sections in each COMDAT group, and treated the group as a compilation
unit of its own (not a partial unit). That worked well, and avoided
some of the problems you're running into (although clearly is more
wasteful in terms of object file size). Readelf and friends will need
to be taught how to find the right auxiliary debug sections, though --
they currently have a built-in assumption that there's only one of
each.

-cary

Re: Debug info for comdat functions

[Jason Merrill]

On 04/18/2012 07:40 PM, Cary Coutant wrote:
> Nice -- I've been wanting to do that for a while, but I always thought
> it would be a lot harder. I see that you've based this on the
> infrastructure created for -feliminate-dwarf2-dups. I don't think that
> will play nice with -fdebug-types-section, though, since I basically
> made those two options incompatible with each other by unioning
> die_symbol with die_type_node.

I think it should be OK because I wait until after the debug_types 
processing is done, at which point limbo_die_list is empty.  Or am I 
just not seeing the problem?

> In the HP-UX compilers, we basically put a complete set of .debug_*
> sections in each COMDAT group, and treated the group as a compilation
> unit of its own (not a partial unit).

So you copy anything that the function refers to into the CU for the 
function?

> wasteful in terms of object file size). Readelf and friends will need
> to be taught how to find the right auxiliary debug sections, though --
> they currently have a built-in assumption that there's only one of
> each.

Good to know.

Jason

Re: Debug info for comdat functions

[Jakub Jelinek]

On Wed, Apr 18, 2012 at 03:23:35PM +0200, Jakub Jelinek wrote:
> > DW_TAG_GNU_call_site wants to refer to the called function's DIE, so
> > the function die in the separate unit needs to have its own symbol.
> > Perhaps _call_site could refer to the function symbol instead?  That
> > seems more correct anyway, since with COMDAT functions you might end
> > up calling a different version of the function that has a different
> > DIE.
> 
> At this point it is too late to change the specification of the extension.
> But you could just put in a DW_TAG_subprogram DW_AT_external declaration
> in the main .debug_info and just refer to that from call_site as well
> as from DW_AT_specification in the comdat .debug_info.  That
> DW_AT_abstract_origin is meant there to be just one of the possible many
> DIEs referring to the callee, the debug info consumer is supposed to find
> out the actual DIE that contains the code from it using its usual
> mechanisms.

That could be easily done by keeping around the original die_node of the
DW_TAG_subprogram for comdat in the main CU, create a new die_node in the
comdat unit and move all (or all but formal_parameter?) children to it
and copy/move attributes.  Thus all references to the subprogram would go
to the main .debug_info.

	Jakub