* Debug info for comdat functions @ 2012-04-18 11:17 Jakub Jelinek 2012-04-18 11:53 ` Jakub Jelinek 0 siblings, 1 reply; 7+ messages in thread From: Jakub Jelinek @ 2012-04-18 11:17 UTC (permalink / raw) To: gcc, binutils Cc: Roland McGrath, Cary Coutant, Tom Tromey, Mark Wielaard, Jason Merrill, Nick Clifton, Alan Modra [-- 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); } ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Debug info for comdat functions 2012-04-18 11:17 Debug info for comdat functions Jakub Jelinek @ 2012-04-18 11:53 ` Jakub Jelinek 2012-04-18 12:44 ` Jason Merrill 0 siblings, 1 reply; 7+ messages in thread From: Jakub Jelinek @ 2012-04-18 11:53 UTC (permalink / raw) To: gcc, binutils Cc: Roland McGrath, Cary Coutant, Tom Tromey, Mark Wielaard, Jason Merrill, Nick Clifton, Alan Modra 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Debug info for comdat functions 2012-04-18 11:53 ` Jakub Jelinek @ 2012-04-18 12:44 ` Jason Merrill 2012-04-18 13:24 ` Jakub Jelinek 2012-04-18 23:40 ` Cary Coutant 0 siblings, 2 replies; 7+ messages in thread From: Jason Merrill @ 2012-04-18 12:44 UTC (permalink / raw) To: Jakub Jelinek Cc: gcc, binutils, Roland McGrath, Cary Coutant, Tom Tromey, Mark Wielaard, Nick Clifton, Alan Modra [-- 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. */ ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Debug info for comdat functions 2012-04-18 12:44 ` Jason Merrill @ 2012-04-18 13:24 ` Jakub Jelinek 2012-04-19 5:29 ` Jakub Jelinek 2012-04-18 23:40 ` Cary Coutant 1 sibling, 1 reply; 7+ messages in thread From: Jakub Jelinek @ 2012-04-18 13:24 UTC (permalink / raw) To: Jason Merrill; +Cc: gcc, binutils 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Debug info for comdat functions 2012-04-18 13:24 ` Jakub Jelinek @ 2012-04-19 5:29 ` Jakub Jelinek 0 siblings, 0 replies; 7+ messages in thread From: Jakub Jelinek @ 2012-04-19 5:29 UTC (permalink / raw) To: Jason Merrill; +Cc: gcc 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Debug info for comdat functions 2012-04-18 12:44 ` Jason Merrill 2012-04-18 13:24 ` Jakub Jelinek @ 2012-04-18 23:40 ` Cary Coutant 2012-04-19 4:44 ` Jason Merrill 1 sibling, 1 reply; 7+ messages in thread From: Cary Coutant @ 2012-04-18 23:40 UTC (permalink / raw) To: Jason Merrill Cc: Jakub Jelinek, gcc, binutils, Roland McGrath, Tom Tromey, Mark Wielaard, Nick Clifton, Alan Modra > 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Debug info for comdat functions 2012-04-18 23:40 ` Cary Coutant @ 2012-04-19 4:44 ` Jason Merrill 0 siblings, 0 replies; 7+ messages in thread From: Jason Merrill @ 2012-04-19 4:44 UTC (permalink / raw) To: Cary Coutant Cc: Jakub Jelinek, gcc, binutils, Roland McGrath, Tom Tromey, Mark Wielaard, Nick Clifton, Alan Modra 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-04-19 5:29 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-04-18 11:17 Debug info for comdat functions Jakub Jelinek 2012-04-18 11:53 ` Jakub Jelinek 2012-04-18 12:44 ` Jason Merrill 2012-04-18 13:24 ` Jakub Jelinek 2012-04-19 5:29 ` Jakub Jelinek 2012-04-18 23:40 ` Cary Coutant 2012-04-19 4:44 ` Jason Merrill
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).