Summary: | dwarf2read.c:9730: internal-error: void dw2_add_symbol_to_list(symbol*, pending**): Assertion `(*listhead) == NULL || (SYMBOL_LANGUAGE ((*listhead)->symbol[0]) == SYMBOL_LANGUAGE (symbol))' failed | ||
---|---|---|---|
Product: | gdb | Reporter: | Richard Biener <rguenth> |
Component: | gdb | Assignee: | Keith Seitz <keiths> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | brobecker, ilg, keiths, sergiodj, vries |
Priority: | P2 | ||
Version: | 8.2 | ||
Target Milestone: | 8.3 | ||
Host: | Target: | ||
Build: | Last reconfirmed: | ||
Attachments: | genchecksum binary (x86_64-linux) |
Description
Richard Biener
2018-09-25 13:35:23 UTC
So I was experimenting and doing sth like diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index 4a35e389e9..72d7962678 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -21328,7 +21328,11 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu, OBJSTAT (objfile, n_syms++); /* Cache this symbol's name and the name's demangled form (if any). */ - SYMBOL_SET_LANGUAGE (sym, cu->language, &objfile->objfile_obstack); + dwarf2_cu *lang_cu = cu; + attribute *spec = dwarf2_attr (die, DW_AT_abstract_origin, cu); + if (spec) + follow_die_ref (die, spec, &lang_cu); + SYMBOL_SET_LANGUAGE (sym, lang_cu->language, &objfile->objfile_obstack); linkagename = dwarf2_physname (name, die, cu); SYMBOL_SET_NAMES (sym, linkagename, strlen (linkagename), 0, objfile); gets us further, now running into #1 0x000000000055d2db in insert_symbol_hashed (dict=0x142a0e0, sym=0x142a020) at /space/rguenther/src/binutils-gdb/gdb/dictionary.c:690 690 gdb_assert (SYMBOL_LANGUAGE (sym) == DICT_LANGUAGE (dict)->la_language); (top-gdb) l 685 unsigned int hash; 686 struct symbol **buckets = DICT_HASHED_BUCKETS (dict); 687 688 /* We don't want to insert a symbol into a dictionary of a different 689 language. The two may not use the same hashing algorithm. */ 690 gdb_assert (SYMBOL_LANGUAGE (sym) == DICT_LANGUAGE (dict)->la_language); 691 for which there is a duplicate bugreport I think. The issue here seems to be that we are doing #3 0x00000000004b6eaa in buildsym_compunit::finish_block_internal ( this=0x13bc890, symbol=0x0, listhead=0x13bca10, old_blocks=0x0, static_link=0x0, start=0x400bf0, end=0x4012cc, is_global=0, expandable=0) at /space/rguenther/src/binutils-gdb/gdb/buildsym.c:245 245 m_language, *listhead); (top-gdb) l 240 } 241 else 242 { 243 BLOCK_DICT (block) = 244 dict_create_hashed (&m_objfile->objfile_obstack, 245 m_language, *listhead); 246 } with m_language == language_cplus but all symbols are language_c. diff --git a/gdb/buildsym.c b/gdb/buildsym.c index a9d2698584..ea0c9a537e 100644 --- a/gdb/buildsym.c +++ b/gdb/buildsym.c @@ -225,24 +225,27 @@ buildsym_compunit::finish_block_internal ? allocate_global_block (&m_objfile->objfile_obstack) : allocate_block (&m_objfile->objfile_obstack)); + language lang = m_language; + if (*listhead && (*listhead)->nsyms != 0) + lang = (*listhead)->symbol[0]->ginfo.language; if (symbol) { BLOCK_DICT (block) = dict_create_linear (&m_objfile->objfile_obstack, - m_language, *listhead); + lang, *listhead); } else { if (expandable) { - BLOCK_DICT (block) = dict_create_hashed_expandable (m_language); + BLOCK_DICT (block) = dict_create_hashed_expandable (lang); dict_add_pending (BLOCK_DICT (block), *listhead); } else { BLOCK_DICT (block) = dict_create_hashed (&m_objfile->objfile_obstack, - m_language, *listhead); + lang, *listhead); } } "cures" that, just getting us back to the first assert ;) This time with (top-gdb) p symbol->ginfo $1 = {name = 0x13cdfa0 "main(int, char**)", value = {ivalue = 0, block = 0x0, bytes = 0x0, address = 0x0, common_block = 0x0, chain = 0x0}, language_specific = {obstack = 0x0, demangled_name = 0x0}, language = language_cplus, ada_mangled = 0, section = -1} (top-gdb) p (*listhead)->symbol[0]->ginfo $2 = {name = 0x13cdc80 "md5_process_block(void const*, size_t, md5_ctx*)", value = {ivalue = 21143856, block = 0x142a130, bytes = 0x142a130 "\220\v@", address = 0x142a130, common_block = 0x142a130, chain = 0x142a130}, language_specific = {obstack = 0x0, demangled_name = 0x0}, language = language_c, ada_mangled = 0, section = -1} where the languages are correct. I suppose gdb isn't really up to handling uses of different "language" symbols in a single CU :/ What's the issue with that anyways? Why not simply _have_ different language symbols in the symbol table? For non-inline instances a solution on the GCC side might be to create separate CUs matching the abstract instances CU but that wouldn't work for inline instances nor for fixing 23713. Eventually the assert()s can be weakened to allow "compatible enough" (same hash, etc.) languages? I realize that allowing LTO-like mixed language environments looks like a very much larger change in gdb than I thought. Though it looks like C and C++ use different symbol hashing... in fact all but C++ seem to use default_search_name_hash ... And what's the issue here? Just store the hash alongside the entries and then the "only" issue is that gdb doesn't properly switch languages automatically (PR23713) so user name lookups might fail spuriously. (In reply to Richard Biener from comment #4) > Eventually the assert()s can be weakened to allow "compatible enough" (same > hash, etc.) languages? I realize that allowing LTO-like mixed language > environments looks like a very much larger change in gdb than I thought. > > Though it looks like C and C++ use different symbol hashing... in fact all > but C++ seem to use default_search_name_hash ... > > And what's the issue here? Just store the hash alongside the entries and > then > the "only" issue is that gdb doesn't properly switch languages automatically > (PR23713) so user name lookups might fail spuriously. I just wanted to let you know that I am looking at fixing this once and for all, but I am on vacation until next week. I have the (good) beginnings of a patch for this. Alas, there is a recent patch on HEAD that is causing problems for me and will require further investigation. (In reply to Keith Seitz from comment #5) > (In reply to Richard Biener from comment #4) > > Eventually the assert()s can be weakened to allow "compatible enough" (same > > hash, etc.) languages? I realize that allowing LTO-like mixed language > > environments looks like a very much larger change in gdb than I thought. > > > > Though it looks like C and C++ use different symbol hashing... in fact all > > but C++ seem to use default_search_name_hash ... > > > > And what's the issue here? Just store the hash alongside the entries and > > then > > the "only" issue is that gdb doesn't properly switch languages automatically > > (PR23713) so user name lookups might fail spuriously. > > I just wanted to let you know that I am looking at fixing this once and > for all, but I am on vacation until next week. I have the (good) beginnings > of a patch for this. That's great news! Mark this one as needed for 8.2.1 (see discussion on gdb-patches: https://www.sourceware.org/ml/gdb-patches/2018-11/msg00096.html). Btw, if GCC can do anything better here please speak up. Currently the CUs generated at link-time (possibly combining different source language CUs) get sth like Compilation Unit @ offset 0x138: Length: 0x8e (32-bit) Version: 4 Abbrev Offset: 0xd2 Pointer Size: 8 <0><143>: Abbrev Number: 1 (DW_TAG_compile_unit) <144> DW_AT_producer : (indirect string, offset: 0x22e): GNU GIMPLE 9.0.0 20181108 (experimental) -mtune=generic -march=x86-64 -mtune=generic -march=x86-64 -g -fno-openmp -fno-openacc -fno-pie -fltrans <148> DW_AT_language : 12 (ANSI C99) <149> DW_AT_name : (indirect string, offset: 0x221): <artificial> <14d> DW_AT_comp_dir : (indirect string, offset: 0x1ac): /abuild/rguenther/obj-slpcost-g/gcc <151> DW_AT_low_pc : 0x4004b2 <159> DW_AT_high_pc : 0x3c <161> DW_AT_stmt_list : 0x10c where DW_AT_language is either the single common language of the source CUs contributing to this, the "newest" C/C++ when mixing C and C++ (where C++ is "newer" than C), or as fallback DW_LANG_C (for example when combining C and fortran). The standard says a CU entry _may_ have a DW_AT_language attribute - would gdb be more happy when we do not emit any DW_AT_language attribute for link-time generated DIEs? You can see we set a producer of GNU GIMPLE which gdb may use to trigger special behavior (OTOH it could do that whenever seeing a unit import with different DW_AT_language...) Re-confirmed on master. Any progress here? It's quite a blocker for LTO adoption to us. Hi Richard. I'm not personally involved in this ticket, but I know there is movement, and it's not as trivial as we would have hoped. There is a 5-patches series being discussed at the moment. Here are some links: last month -> https://www.sourceware.org/ml/gdb-patches/2018-11/msg00138.html this month -> https://www.sourceware.org/ml/gdb-patches/2018-12/msg00187.html On Wed, 19 Dec 2018, brobecker at gnat dot com wrote: > https://sourceware.org/bugzilla/show_bug.cgi?id=23712 > > --- Comment #10 from Joel Brobecker <brobecker at gnat dot com> --- > Hi Richard. I'm not personally involved in this ticket, but I know there is > movement, and it's not as trivial as we would have hoped. I figured that when trying to poke myself... > There is a 5-patches > series being discussed at the moment. Here are some links: > > last month -> https://www.sourceware.org/ml/gdb-patches/2018-11/msg00138.html > this month -> https://www.sourceware.org/ml/gdb-patches/2018-12/msg00187.html That's good news - I hope it will make the next release! Too late now, for 8.2.1. Hoping we will be able to fix it for 8.3. The master branch has been updated by Keith Seitz <kseitz@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=c7748ee9ceb5a394658cd07aeb0445924599e442 commit c7748ee9ceb5a394658cd07aeb0445924599e442 Author: Keith Seitz <keiths@redhat.com> Date: Thu Jan 10 13:57:08 2019 -0800 gdb/23712: Introduce multidictionary's gdb/23712 is a new manifestation of the now-infamous (at least to me) symtab/23010 assertion failure (DICT_LANGUAGE == SYMBOL_LANGAUGE). An example of the problem (using test case from symtab/23010): Reading symbols from /home/rdiez/rdiez/arduino/JtagDue/BuildOutput/JtagDue-obj-release/firmware.elf...done. (gdb) p SysTick_Handler dwarf2read.c:9715: internal-error: void dw2_add_symbol_to_list(symbol*, pending**): Assertion `(*listhead) == NULL || (SYMBOL_LANGUAGE ((*listhead)->symbol[0]) == SYMBOL_LANGUAGE (symbol))' failed. A problem internal to GDB has been detected, further debugging may prove unreliable. Quit this debugging session? (y or n) This assertion was added specifically to catch this condition (of adding symbols of different languages to a single pending list). The problems we're now seeing on systems utilizing DWARF debugging seem to be caused by the use of LTO, which adds a CU with an artificial DIE of language C99 which references DIEs in other CUs of language C++. Thus, we create a dictionary containing symbols of C99 but end up stuffing C++ symbols into it, and the dw2_add_symbol_to_list triggers. The approach taken here to fix this is to introduce multi-language dictionaries to "replace" the standard, single-language dictionaries used today. Note to reviewers: This patch introduces some temporary functions to aide with review. This and other artifacts (such as "See dictionary.h" which appear incorrect) will all be valid at the end of the series. This first patch introduces the new multidictionary and its API (which is, by design, identical to the old dictionary interface). It also mutates dict_create_hashed and dict_create_linear so that they take a std::vector instead of the usual struct pending linked list. This will be needed later on. This patch does /not/ actually enable multidictionary's. That is left for a subsequent patch in the series. I've done exhaustive performance testing with this approach, and I've attempted to minimize the overhead for the (overwhelmingly) most common one-language scenario. On average, a -g3 -O0 GDB (the one we developers use) will see approximately a 4% slowdown when initially reading symbols. [I've tested only GDB and firefox with -readnow.] When using -O2, this difference shrinks to ~0.5%. Since a number of runs with these patches actually run /faster/ than unpatched GDB, I conclude that these tests have at least a 0.5% error margin. On our own gdb.perf test suite, again, results appear to be pretty negligible. Differences to unpatched GDB range from -7.8% (yes, patched version is again faster than unpatched) to 27%. All tests lying outside "negligible," such as the 27% slowdown, involve a total run time of 0.0007 (or less) with smaller numbers of CUs/DSOs (usually 10 or 100). In all cases, the follow-up tests with more CUs/DSOs is never more than 3% difference to the baseline, unpatched GDB. In my opinion, these results are satisfactory. gdb/ChangeLog: PR gdb/23712 PR symtab/23010 * dictionary.c: Include unordered_map. (pending_to_vector): New function. (dict_create_hashed_1, dict_create_linear_1, dict_add_pending_1): Rewrite the non-"_1" functions to take vector instead of linked list. (dict_create_hashed, dict_create_linear, dict_add_pending): Use the "new" _1 versions of the same name. (multidictionary): Define. (std::hash<enum language): New definition. (collate_pending_symbols_by_language, mdict_create_hashed) (mdict_create_hashed_expandable, mdict_create_linear) (mdict_create_linear_expandable, mdict_free) (find_language_dictionary, create_new_language_dictionary) (mdict_add_symbol, mdict_add_pending, mdict_iterator_first) (mdict_iterator_next, mdict_iter_match_first, mdict_iter_match_next) (mdict_size, mdict_empty): New functions. * dictionary.h (mdict_iterator): Define. The master branch has been updated by Keith Seitz <kseitz@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=b026f59345a336cabf74719fce9f96cab7c7ab4d commit b026f59345a336cabf74719fce9f96cab7c7ab4d Author: Keith Seitz <keiths@redhat.com> Date: Thu Jan 10 13:57:08 2019 -0800 gdb/23712: Use new multidictionary API This patch builds on the previous by enabling the `new' multidictionary API. A lot of the hunks are simply textual replacements of "dict_" with "mdict_" and similar transformations. A word of warning, even with the use of multidictionaries, the code still does not satisfactorily fix the reported problems with gdb/23712 (or gdb/23010). We still have additional changes to make before that happens. gdb/ChangeLog: PR gdb/23712 PR symtab/23010 * dictionary.h (struct dictionary): Replace declaration with multidictionary. (dict_create_hashed, dict_create_hashed_expandable) (dict_create_linear, dict_create_linear_expandable) (dict_free, dict_add_symbol, dict_add_pending, dict_empty) (dict_iterator_first, dict_iterator_next, dict_iter_match_first) (dict_iter_match_next, dict_size): Rename to "mdict_" versions taking multidictionary argument. [ALL_DICT_SYMBOLS]: Update for multidictionary. * block.h (struct block) <dict>: Change to multidictionary and rename `multidict'. * block.c, buildsym.c, jit.c, mdebugread.c, objfiles.c, symmisc.c: Update all dictionary references to multidictionary. The master branch has been updated by Keith Seitz <kseitz@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=63a20375b401e24c30987367a10b47b289612e1c commit 63a20375b401e24c30987367a10b47b289612e1c Author: Keith Seitz <keiths@redhat.com> Date: Thu Jan 10 13:57:08 2019 -0800 gdb/23712: Cleanup/Remove temporary dictionary functions Now that multidictionary's are being used, there is no longer any need to retain the four temporary functions introduced in the beginning of this series. This patch removes them. As an additional cleanup, since the single-language dictionaries are no longer used outside dictionary.c, make all of those functions static. gdb/ChangeLog: PR gdb/23712 PR symtab/23010 * dictionary.c (pending_to_vector): Remove. (dict_create_hashed_1, dict_create_linear_1, dict_add_pending_1): Remove _1 suffix, replacing functions of the same name. Update all callers. (dict_create_hashed, dict_create_hashed_expandable) (dict_create_linear, dict_create_linear_expandable, dict_free) (dict_add_symbol, dict_add_pending, dict_size, dict_empty): Make functions static. The master branch has been updated by Keith Seitz <kseitz@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d3cb68081112a4976979df3f8eae7ca926e76519 commit d3cb68081112a4976979df3f8eae7ca926e76519 Author: Keith Seitz <keiths@redhat.com> Date: Thu Jan 10 13:57:08 2019 -0800 gdb/23712: Remove dw2_add_symbol_to_list Finally, we can remove dw2_add_symbol_to_list since the wrapper function originally introduced to catch this multi-language scenario is no longer needed. With multi-language dictionaries, we can now support adding symbols of multiple languages, negating the need for the assertion entirely. This patch should now fix gdb/23712 (and symtab/23010). At least it will if the NULL buildsym_compunit problem doesn't strike first (see gdb/23773). gdb/ChangeLog: PR gdb/23712 PR symtab/23010 * dwarf2read.c (dw2_add_symbol_to_list): Remove. (fixup_go_packaging, new_symbol): Use add_symbol_to_list. The master branch has been updated by Keith Seitz <kseitz@sourceware.org>: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=b56f80d8b27dffd0f8c02b8b11068b71b9fec375 commit b56f80d8b27dffd0f8c02b8b11068b71b9fec375 Author: Keith Seitz <keiths@redhat.com> Date: Thu Jan 10 13:57:09 2019 -0800 gdb/23712: Test case for multidictionary This is a test derived from one of the reproducers in symtab/23010. The DIE tree used here is typical of compilations with LTO, where an artificial parent DIE of language C99 imports DIEs of other languages. gdb/testsuite/ChangeLog: PR gdb/23712 PR symtab/23010 * gdb.dwarf2/multidictionary.exp: New file. debugging cc1 now seems to work without crashing gdb. debugging experience is quite bad though - gdb doesn't seem to be able to do breakpoints on inlined instances. Btw, how can I debug (gdb) b choose_multiplier Breakpoint 2 at 0x143f8d0: file /space/rguenther/src/svn/trunk2/gcc/hwint.h, line 245. which shows bogus source line info - it's of course in libiberties hashtab.c, hwint.h at this place has static inline int ceil_log2 (unsigned HOST_WIDE_INT x) { return x == 0 ? 0 : floor_log2 (x - 1) + 1; full dwarf is of course big... But this bug can be closed as from my side unless you are aware of missing pieces of the fix. (In reply to Richard Biener from comment #18) > Btw, how can I debug > > (gdb) b choose_multiplier > Breakpoint 2 at 0x143f8d0: file /space/rguenther/src/svn/trunk2/gcc/hwint.h, > line 245. My Jan 16-dated build (from master) behaves differently: $ ./gdb -nx -q gcc/cc1 -ex "b choose_multiplier" -ex "i b" Reading symbols from gcc/cc1... Breakpoint 1 at 0xb2f94f: file ../../gcc/gcc/expmed.c, line 3649. Num Type Disp Enb Address What 1 breakpoint keep y 0x0000000000b2f94f in choose_multiplier(unsigned long, int, int, unsigned long*, int*, int*) at ../../gcc/gcc/expmed.c:3649 (gdb) list choose_multiplier 3639 3640 unsigned HOST_WIDE_INT 3641 choose_multiplier (unsigned HOST_WIDE_INT d, int n, int precision, 3642 unsigned HOST_WIDE_INT *multiplier_ptr, 3643 int *post_shift_ptr, int *lgup_ptr) 3644 { 3645 int lgup, post_shift; 3646 int pow, pow2; 3647 3648 /* lgup = ceil(log2(divisor)); */ (gdb) info func choose_multiplier All functions matching regular expression "choose_multiplier": File ../../gcc/gcc/expmed.c: 3641: unsigned long choose_multiplier(unsigned long, int, int, unsigned long*, int*, int*); (gdb) In my sources, choose_multiplier in hashtab.c is wrapped with #if 0...#endif. I'm closing this bug as resolved/fixed. If you continue to have usage problems debugging cc1, please open a new bug so that we don't lose track of it. *** Bug 24145 has been marked as a duplicate of this bug. *** |