* [RFC] regresssion(internal-error) printing subprogram argument @ 2017-12-13 10:37 Joel Brobecker 2017-12-14 19:02 ` Pedro Alves 0 siblings, 1 reply; 27+ messages in thread From: Joel Brobecker @ 2017-12-13 10:37 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches [-- Attachment #1: Type: text/plain, Size: 4531 bytes --] Hi Pedro, I don't know if you remember about the patch series that introduces wild matching for C++ as well? I havent' verified this in details, because there is a series of patches, and they are fairly long, but I have a feeling that they may have introduced a regression. I can bisect tomorrow if needed. In any case, consider the following code which first declares a tagged type (the equivalent of a class in Ada), and then a procedure which takes a pointer (access) to this type's 'Class. package Pck is type Top_T is tagged record N : Integer := 1; end record; procedure Inspect (Obj: access Top_T'Class); end Pck; Putting a breakpoint in that procedure and then running to it triggers an internal error: (gdb) break inspect (gdb) continue Breakpoint 1, pck.inspect (obj=0x63e010 /[...]/gdb/stack.c:621: internal-error: void print_frame_args(symbol*, frame_info*, int, ui_file*): Assertion `nsym != NULL' failed. What's special about this subprogram is that it takes an access to what we call a 'Class type, and for implementation reasons, the compiler adds an extra argument named "objL". If you are curious why, it allows the compiler for perform dynamic accessibility checks that are mandated by the language. If we look at the location where we get the internal error (in stack.c), we find that we are looping over the symbol of each parameter, and for each parameter, we do: /* We have to look up the symbol because arguments can have two entries (one a parameter, one a local) and the one we want is the local, which lookup_symbol will find for us. [...] nsym = lookup_symbol (SYMBOL_LINKAGE_NAME (sym), b, VAR_DOMAIN, NULL).symbol; gdb_assert (nsym != NULL); The lookup_symbol goes through the lookup structure, which means the symbol's linkage name ("objL") gets transformed into a lookup_name_info object (in block_lookup_symbol), before it gets fed to the block symbol dictionary iterators. This, in turn, triggers the symbol matching by comparing the "lookup" name which, for Ada, means among other things, lowercasing the given name to "objl". It is this transformation that causes the lookup find no matches, and therefore trip this assertion. Going back to the "offending" call to lookup_symbol in stack.c, what we are trying to do, here, is do a lookup by linkage name. So, I think what we mean to be doing is a completely litteral symbol lookup, so maybe not even strcmp_iw, but actually just plain strcmp??? There doesn't seem to be a way to do just that anymore, unfortunately. While this wasn't necessarily part of the spec, in the past, in practice, you could get that effect by doing a lookup using the C language. But that doesn't work, because we still end up somehow using Ada's lookup_name routine which transforms "objL". So, ideally, as I hinted before, I think what we need is a way to perform a litteral lookup so that searches by linkage names like the above can be performed. But this might be a fairly involved change (maybe adding a new kind of lookup, and then making adjustments so we know which kind of name to use for name matching). Failing that, the attached prototype takes the easy approach of just making Ada special, and adjusting the name used for the lookup to encode in the name itself the fact that the lookup should be litteral. I tested it with both the official testsuite as well as AdaCore's testsuite, and it fixed the issue without regression that I could see. It's not ideal (not clean, IMO), but gets us back with minimal fuss. We could have that while we work on extending the current framework to allow litteral lookups -- I would just clean it up and post an official RFA for it. Can you tell me what you think? I'm sorry I didn't notice this when I did my review and round of testing. I did the best I could, but the current version of GDB showing 300+ failures in AdaCore's testsuite at that time, I used manual report comparison, and I must have missed these differences (7 failures). Note also that I consider this issue blocking for 8.1, as these are not just limited to access to 'Class parameters. These kinds of internal parameters can also be generated in other situations, and in particular when using tasking (the equivalent of threads). Tasking is fairly prevalent in Ada programs. We might even want to defer branching if we elect to take the more general fix of adding litteral lookup support... Thanks! -- Joel [-- Attachment #2: 0001-WIP-nsym-NULL-assert-failure-in-stack.c-print_frame_.patch --] [-- Type: text/x-diff, Size: 6325 bytes --] From c48fd2d065cddafba771c6bead7ee16806505089 Mon Sep 17 00:00:00 2001 From: Joel Brobecker <brobecker@adacore.com> Date: Wed, 13 Dec 2017 03:53:13 -0500 Subject: [PATCH] WIP: `nsym != NULL' assert failure in stack.c::print_frame_args --- gdb/stack.c | 6 +++- gdb/testsuite/gdb.ada/access_tagged_param.exp | 37 +++++++++++++++++++++++ gdb/testsuite/gdb.ada/access_tagged_param/foo.adb | 20 ++++++++++++ gdb/testsuite/gdb.ada/access_tagged_param/pck.adb | 21 +++++++++++++ gdb/testsuite/gdb.ada/access_tagged_param/pck.ads | 21 +++++++++++++ 5 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 gdb/testsuite/gdb.ada/access_tagged_param.exp create mode 100644 gdb/testsuite/gdb.ada/access_tagged_param/foo.adb create mode 100644 gdb/testsuite/gdb.ada/access_tagged_param/pck.adb create mode 100644 gdb/testsuite/gdb.ada/access_tagged_param/pck.ads diff --git a/gdb/stack.c b/gdb/stack.c index 6bd0d45..521cef1 100644 --- a/gdb/stack.c +++ b/gdb/stack.c @@ -615,8 +615,12 @@ print_frame_args (struct symbol *func, struct frame_info *frame, if (*SYMBOL_LINKAGE_NAME (sym)) { struct symbol *nsym; + std::string sym_linkage_name (SYMBOL_LINKAGE_NAME (sym)); - nsym = lookup_symbol (SYMBOL_LINKAGE_NAME (sym), + if (SYMBOL_LANGUAGE (sym) == language_ada) + sym_linkage_name = std::string("<") + sym_linkage_name + '>'; + + nsym = lookup_symbol (sym_linkage_name.c_str (), b, VAR_DOMAIN, NULL).symbol; gdb_assert (nsym != NULL); if (SYMBOL_CLASS (nsym) == LOC_REGISTER diff --git a/gdb/testsuite/gdb.ada/access_tagged_param.exp b/gdb/testsuite/gdb.ada/access_tagged_param.exp new file mode 100644 index 0000000..a5e399f --- /dev/null +++ b/gdb/testsuite/gdb.ada/access_tagged_param.exp @@ -0,0 +1,37 @@ +# Copyright 2017 Free Software Foundation, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +load_lib "ada.exp" + +standard_ada_testfile foo + +if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug]] != "" } { + return -1 +} + +clean_restart ${testfile} + +if ![runto "foo"] then { + perror "Couldn't run ${testfile}" + return +} + +gdb_breakpoint "pck.inspect" + +# Continue until we reach the breakpoint, and verify that we can print +# the value of all the parameters. + +gdb_test "continue" \ + ".*Breakpoint $decimal, pck\\.inspect \\(obj=$hex, <objL>=2\\) at .*" diff --git a/gdb/testsuite/gdb.ada/access_tagged_param/foo.adb b/gdb/testsuite/gdb.ada/access_tagged_param/foo.adb new file mode 100644 index 0000000..bd7e641 --- /dev/null +++ b/gdb/testsuite/gdb.ada/access_tagged_param/foo.adb @@ -0,0 +1,20 @@ +-- Copyright 2017 Free Software Foundation, Inc. +-- +-- This program is free software; you can redistribute it and/or modify +-- it under the terms of the GNU General Public License as published by +-- the Free Software Foundation; either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU General Public License for more details. +-- +-- You should have received a copy of the GNU General Public License +-- along with this program. If not, see <http://www.gnu.org/licenses/>. + +with Pck; use Pck; +procedure Foo is +begin + Inspect (new Top_T'(N => 2)); +end Foo; diff --git a/gdb/testsuite/gdb.ada/access_tagged_param/pck.adb b/gdb/testsuite/gdb.ada/access_tagged_param/pck.adb new file mode 100644 index 0000000..88fa970 --- /dev/null +++ b/gdb/testsuite/gdb.ada/access_tagged_param/pck.adb @@ -0,0 +1,21 @@ +-- Copyright 2017 Free Software Foundation, Inc. +-- +-- This program is free software; you can redistribute it and/or modify +-- it under the terms of the GNU General Public License as published by +-- the Free Software Foundation; either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU General Public License for more details. +-- +-- You should have received a copy of the GNU General Public License +-- along with this program. If not, see <http://www.gnu.org/licenses/>. + +package body Pck is + procedure Inspect (Obj: access Top_T'Class) is + begin + null; + end Inspect; +end Pck; diff --git a/gdb/testsuite/gdb.ada/access_tagged_param/pck.ads b/gdb/testsuite/gdb.ada/access_tagged_param/pck.ads new file mode 100644 index 0000000..bbb5c8d --- /dev/null +++ b/gdb/testsuite/gdb.ada/access_tagged_param/pck.ads @@ -0,0 +1,21 @@ +-- Copyright 2017 Free Software Foundation, Inc. +-- +-- This program is free software; you can redistribute it and/or modify +-- it under the terms of the GNU General Public License as published by +-- the Free Software Foundation; either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU General Public License for more details. +-- +-- You should have received a copy of the GNU General Public License +-- along with this program. If not, see <http://www.gnu.org/licenses/>. + +package Pck is + type Top_T is tagged record + N : Integer := 1; + end record; + procedure Inspect (Obj: access Top_T'Class); +end Pck; -- 2.1.4 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument 2017-12-13 10:37 [RFC] regresssion(internal-error) printing subprogram argument Joel Brobecker @ 2017-12-14 19:02 ` Pedro Alves 2017-12-14 19:41 ` Pedro Alves 2017-12-15 7:54 ` Joel Brobecker 0 siblings, 2 replies; 27+ messages in thread From: Pedro Alves @ 2017-12-14 19:02 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches [-- Attachment #1: Type: text/plain, Size: 5677 bytes --] On 12/13/2017 10:36 AM, Joel Brobecker wrote: > Hi Pedro, > > I don't know if you remember about the patch series that introduces > wild matching for C++ as well? I havent' verified this in details, > because there is a series of patches, and they are fairly long, but > I have a feeling that they may have introduced a regression. I can > bisect tomorrow if needed. > > In any case, consider the following code which first declares > a tagged type (the equivalent of a class in Ada), and then > a procedure which takes a pointer (access) to this type's 'Class. > > package Pck is > type Top_T is tagged record > N : Integer := 1; > end record; > procedure Inspect (Obj: access Top_T'Class); > end Pck; > > Putting a breakpoint in that procedure and then running to it triggers > an internal error: > > (gdb) break inspect > (gdb) continue > Breakpoint 1, pck.inspect (obj=0x63e010 > /[...]/gdb/stack.c:621: internal-error: void print_frame_args(symbol*, frame_info*, int, ui_file*): Assertion `nsym != NULL' failed. > > What's special about this subprogram is that it takes an access > to what we call a 'Class type, and for implementation reasons, > the compiler adds an extra argument named "objL". If you are > curious why, it allows the compiler for perform dynamic accessibility > checks that are mandated by the language. > > If we look at the location where we get the internal error > (in stack.c), we find that we are looping over the symbol of > each parameter, and for each parameter, we do: > > /* We have to look up the symbol because arguments can have > two entries (one a parameter, one a local) and the one we > want is the local, which lookup_symbol will find for us. > [...] > nsym = lookup_symbol (SYMBOL_LINKAGE_NAME (sym), > b, VAR_DOMAIN, NULL).symbol; > gdb_assert (nsym != NULL); > > The lookup_symbol goes through the lookup structure, which means > the symbol's linkage name ("objL") gets transformed into > a lookup_name_info object (in block_lookup_symbol), before > it gets fed to the block symbol dictionary iterators. This, > in turn, triggers the symbol matching by comparing the > "lookup" name which, for Ada, means among other things, lowercasing > the given name to "objl". It is this transformation that causes > the lookup find no matches, and therefore trip this assertion. > > Going back to the "offending" call to lookup_symbol in stack.c, > what we are trying to do, here, is do a lookup by linkage name. > So, I think what we mean to be doing is a completely litteral > symbol lookup, so maybe not even strcmp_iw, but actually just > plain strcmp??? > > There doesn't seem to be a way to do just that anymore, > unfortunately. While this wasn't necessarily part of the spec, > in the past, in practice, you could get that effect by doing > a lookup using the C language. But that doesn't work, because > we still end up somehow using Ada's lookup_name routine which > transforms "objL". > > So, ideally, as I hinted before, I think what we need is a way > to perform a litteral lookup so that searches by linkage names > like the above can be performed. But this might be a fairly > involved change (maybe adding a new kind of lookup, and then > making adjustments so we know which kind of name to use for > name matching). Indeed. I gave the "literal" lookup type a try today, and it turns out not being so bad, I think. See patch below. Thinking through this and experimenting a bit, I think I convinced myself that with the current symbol tables infrastructure, it's not literal _linkage_ names that we want to pass down, but instead literal _search_ names. I.e., notice that I've switched the lookup_symbol call in question to pass down SYMBOL_SEARCH_NAME instead of SYMBOL_LINKAGE_NAME. See comments in symtab.h in the patch. I was thinking about trying to add a symbol_name_match_type::LINKAGE and support searching by linkage name for any language, but the thing is that the dictionaries only work with SYMBOL_SEARCH_NAME, AFAICT, because that's what is used for hashing. We'd need separate dictionaries for hashed linkage names. The patch is not complete in the sense that there are symbol-lookup entry points that could be tweaked to pass down a symbol_name_match_type instead of assuming FULL. And also, I know there are places in ada-lang.c that are doing what you were doing here too (the "<...>" verbatim trick) which could be adjusted. But at least this fixes your testcase too, and causes no regressions. So maybe we could do this incrementally and pass adjust functions to pass around a symbol_name_match_type as we find a need? Functions that we miss passing down simply end up behaving like symbols_name_match_type::FULL, as today. > Can you tell me what you think? Your turn. :-) > > I'm sorry I didn't notice this when I did my review and round > of testing. I did the best I could, but the current version of > GDB showing 300+ failures in AdaCore's testsuite at that time, > I used manual report comparison, and I must have missed > these differences (7 failures). Well, no worries for me. :-) > Note also that I consider this issue blocking for 8.1, as these > are not just limited to access to 'Class parameters. These kinds > of internal parameters can also be generated in other situations, > and in particular when using tasking (the equivalent of threads). > Tasking is fairly prevalent in Ada programs. We might even want > to defer branching if we elect to take the more general fix of > adding litteral lookup support... Thanks, Pedro Alves [-- Attachment #2: 0001-symbol_name_match_type-LITERAL.patch --] [-- Type: text/x-patch, Size: 15487 bytes --] From f5a7a45a8e4979d7c47a48bf9a10d6cda3be5a73 Mon Sep 17 00:00:00 2001 From: Pedro Alves <palves@redhat.com> Date: Thu, 14 Dec 2017 18:22:42 +0000 Subject: [PATCH] symbol_name_match_type::LITERAL --- gdb/block.c | 3 ++- gdb/block.h | 1 + gdb/c-valprint.c | 9 ++++++--- gdb/compile/compile-object-load.c | 6 +++++- gdb/cp-namespace.c | 4 +++- gdb/infrun.c | 2 +- gdb/language.c | 27 +++++++++++++++++++++++++ gdb/p-valprint.c | 10 ++++++---- gdb/psymtab.c | 6 ++++-- gdb/stack.c | 8 ++++---- gdb/symtab.c | 42 +++++++++++++++++++++++++++++---------- gdb/symtab.h | 22 ++++++++++++++++++++ 12 files changed, 113 insertions(+), 27 deletions(-) diff --git a/gdb/block.c b/gdb/block.c index a8075a1..41a0c0c 100644 --- a/gdb/block.c +++ b/gdb/block.c @@ -673,12 +673,13 @@ block_iter_match_next (const lookup_name_info &name, struct symbol * block_lookup_symbol (const struct block *block, const char *name, + symbol_name_match_type match_type, const domain_enum domain) { struct block_iterator iter; struct symbol *sym; - lookup_name_info lookup_name (name, symbol_name_match_type::FULL); + lookup_name_info lookup_name (name, match_type); if (!BLOCK_FUNCTION (block)) { diff --git a/gdb/block.h b/gdb/block.h index 0326c18..66729dd 100644 --- a/gdb/block.h +++ b/gdb/block.h @@ -258,6 +258,7 @@ extern struct symbol *block_iter_match_next extern struct symbol *block_lookup_symbol (const struct block *block, const char *name, + symbol_name_match_type match_type, const domain_enum domain); /* Search BLOCK for symbol NAME in DOMAIN but only in primary symbol table of diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c index 653fed6..9b2911a 100644 --- a/gdb/c-valprint.c +++ b/gdb/c-valprint.c @@ -198,14 +198,17 @@ print_unpacked_pointer (struct type *type, struct type *elttype, struct symbol *wsym = NULL; struct type *wtype; struct block *block = NULL; - struct field_of_this_result is_this_fld; if (want_space) fputs_filtered (" ", stream); if (msymbol.minsym != NULL) - wsym = lookup_symbol (MSYMBOL_LINKAGE_NAME(msymbol.minsym), block, - VAR_DOMAIN, &is_this_fld).symbol; + { + const char *search_name + = MSYMBOL_SEARCH_NAME (msymbol.minsym); + wsym = lookup_symbol_literal (search_name, block, + VAR_DOMAIN).symbol; + } if (wsym) { diff --git a/gdb/compile/compile-object-load.c b/gdb/compile/compile-object-load.c index f1c8ccd..5f6b03f 100644 --- a/gdb/compile/compile-object-load.c +++ b/gdb/compile/compile-object-load.c @@ -439,7 +439,10 @@ get_out_value_type (struct symbol *func_sym, struct objfile *objfile, block = BLOCKVECTOR_BLOCK (bv, block_loop); if (BLOCK_FUNCTION (block) != NULL) continue; - gdb_val_sym = block_lookup_symbol (block, COMPILE_I_EXPR_VAL, VAR_DOMAIN); + gdb_val_sym = block_lookup_symbol (block, + COMPILE_I_EXPR_VAL, + symbol_name_match_type::LITERAL, + VAR_DOMAIN); if (gdb_val_sym == NULL) continue; @@ -466,6 +469,7 @@ get_out_value_type (struct symbol *func_sym, struct objfile *objfile, gdb_type = check_typedef (gdb_type); gdb_ptr_type_sym = block_lookup_symbol (block, COMPILE_I_EXPR_PTR_TYPE, + symbol_name_match_type::LITERAL, VAR_DOMAIN); if (gdb_ptr_type_sym == NULL) error (_("No \"%s\" symbol found"), COMPILE_I_EXPR_PTR_TYPE); diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c index 2a3ffef..b82481b 100644 --- a/gdb/cp-namespace.c +++ b/gdb/cp-namespace.c @@ -141,7 +141,9 @@ cp_basic_lookup_symbol (const char *name, const struct block *block, if (global_block != NULL) { - sym.symbol = lookup_symbol_in_block (name, global_block, domain); + sym.symbol = lookup_symbol_in_block (name, + symbol_name_match_type::FULL, + global_block, domain); sym.block = global_block; } } diff --git a/gdb/infrun.c b/gdb/infrun.c index d7df3c7..7dd6667 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -7489,7 +7489,7 @@ insert_exception_resume_breakpoint (struct thread_info *tp, CORE_ADDR handler; struct breakpoint *bp; - vsym = lookup_symbol (SYMBOL_LINKAGE_NAME (sym), b, VAR_DOMAIN, NULL); + vsym = lookup_symbol_literal (SYMBOL_SEARCH_NAME (sym), b, VAR_DOMAIN); value = read_var_value (vsym.symbol, vsym.block, frame); /* If the value was optimized out, revert to the old behavior. */ if (! value_optimized_out (value)) diff --git a/gdb/language.c b/gdb/language.c index c3872fc..5bcdfb9 100644 --- a/gdb/language.c +++ b/gdb/language.c @@ -724,14 +724,41 @@ default_symbol_name_matcher (const char *symbol_search_name, return false; } +/* A name matcher that matches the symbol name exactly, with + strcmp. */ + +static bool +literal_symbol_name_matcher (const char *symbol_search_name, + const lookup_name_info &lookup_name, + completion_match_result *comp_match_res) +{ + const std::string &name = lookup_name.name (); + + int cmp = (lookup_name.completion_mode () + ? strncmp (symbol_search_name, name.c_str (), name.size ()) + : strcmp (symbol_search_name, name.c_str ())); + if (cmp == 0) + { + if (comp_match_res != NULL) + comp_match_res->set_match (symbol_search_name); + return true; + } + else + return false; +} + /* See language.h. */ symbol_name_matcher_ftype * language_get_symbol_name_matcher (const language_defn *lang, const lookup_name_info &lookup_name) { + if (lookup_name.match_type () == symbol_name_match_type::LITERAL) + return literal_symbol_name_matcher; + if (lang->la_get_symbol_name_matcher != nullptr) return lang->la_get_symbol_name_matcher (lookup_name); + return default_symbol_name_matcher; } diff --git a/gdb/p-valprint.c b/gdb/p-valprint.c index d12b636..8810d67 100644 --- a/gdb/p-valprint.c +++ b/gdb/p-valprint.c @@ -246,15 +246,17 @@ pascal_val_print (struct type *type, struct symbol *wsym = NULL; struct type *wtype; struct block *block = NULL; - struct field_of_this_result is_this_fld; if (want_space) fputs_filtered (" ", stream); if (msymbol.minsym != NULL) - wsym = lookup_symbol (MSYMBOL_LINKAGE_NAME (msymbol.minsym), - block, - VAR_DOMAIN, &is_this_fld).symbol; + { + const char *search_name + = MSYMBOL_SEARCH_NAME (msymbol.minsym); + wsym = lookup_symbol_literal (search_name, block, + VAR_DOMAIN).symbol; + } if (wsym) { diff --git a/gdb/psymtab.c b/gdb/psymtab.c index c87ef25..862360e 100644 --- a/gdb/psymtab.c +++ b/gdb/psymtab.c @@ -2254,7 +2254,8 @@ maintenance_check_psymtabs (const char *ignore, int from_tty) length = ps->n_static_syms; while (length--) { - sym = block_lookup_symbol (b, SYMBOL_LINKAGE_NAME (*psym), + sym = block_lookup_symbol (b, SYMBOL_SEARCH_NAME (*psym), + symbol_name_match_type::LITERAL, SYMBOL_DOMAIN (*psym)); if (!sym) { @@ -2271,7 +2272,8 @@ maintenance_check_psymtabs (const char *ignore, int from_tty) length = ps->n_global_syms; while (length--) { - sym = block_lookup_symbol (b, SYMBOL_LINKAGE_NAME (*psym), + sym = block_lookup_symbol (b, SYMBOL_SEARCH_NAME (*psym), + symbol_name_match_type::LITERAL, SYMBOL_DOMAIN (*psym)); if (!sym) { diff --git a/gdb/stack.c b/gdb/stack.c index 6bd0d45..a51cf4f 100644 --- a/gdb/stack.c +++ b/gdb/stack.c @@ -616,8 +616,8 @@ print_frame_args (struct symbol *func, struct frame_info *frame, { struct symbol *nsym; - nsym = lookup_symbol (SYMBOL_LINKAGE_NAME (sym), - b, VAR_DOMAIN, NULL).symbol; + nsym = lookup_symbol_literal (SYMBOL_SEARCH_NAME (sym), + b, VAR_DOMAIN).symbol; gdb_assert (nsym != NULL); if (SYMBOL_CLASS (nsym) == LOC_REGISTER && !SYMBOL_IS_ARGUMENT (nsym)) @@ -2141,8 +2141,8 @@ iterate_over_block_arg_vars (const struct block *b, float). There are also LOC_ARG/LOC_REGISTER pairs which are not combined in symbol-reading. */ - sym2 = lookup_symbol (SYMBOL_LINKAGE_NAME (sym), - b, VAR_DOMAIN, NULL).symbol; + sym2 = lookup_symbol_literal (SYMBOL_SEARCH_NAME (sym), + b, VAR_DOMAIN).symbol; (*cb) (SYMBOL_PRINT_NAME (sym), sym2, cb_data); } } diff --git a/gdb/symtab.c b/gdb/symtab.c index 220ae09..78018a1 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -75,6 +75,7 @@ static int find_line_common (struct linetable *, int, int *, int); static struct block_symbol lookup_symbol_aux (const char *name, + symbol_name_match_type match_type, const struct block *block, const domain_enum domain, enum language language, @@ -82,6 +83,7 @@ static struct block_symbol static struct block_symbol lookup_local_symbol (const char *name, + symbol_name_match_type match_type, const struct block *block, const domain_enum domain, enum language language); @@ -1876,7 +1878,9 @@ lookup_symbol_in_language (const char *name, const struct block *block, demangle_result_storage storage; const char *modified_name = demangle_for_lookup (name, lang, storage); - return lookup_symbol_aux (modified_name, block, domain, lang, + return lookup_symbol_aux (modified_name, + symbol_name_match_type::FULL, + block, domain, lang, is_a_field_of_this); } @@ -1895,6 +1899,16 @@ lookup_symbol (const char *name, const struct block *block, /* See symtab.h. */ struct block_symbol +lookup_symbol_literal (const char *name, const struct block *block, + domain_enum domain) +{ + return lookup_symbol_aux (name, symbol_name_match_type::LITERAL, + block, domain, language_asm, NULL); +} + +/* See symtab.h. */ + +struct block_symbol lookup_language_this (const struct language_defn *lang, const struct block *block) { @@ -1915,7 +1929,8 @@ lookup_language_this (const struct language_defn *lang, { struct symbol *sym; - sym = block_lookup_symbol (block, lang->la_name_of_this, VAR_DOMAIN); + sym = block_lookup_symbol (block, lang->la_name_of_this, + symbol_name_match_type::LITERAL, VAR_DOMAIN); if (sym != NULL) { if (symbol_lookup_debug > 1) @@ -1986,7 +2001,8 @@ check_field (struct type *type, const char *name, (e.g., demangled name) of the symbol that we're looking for. */ static struct block_symbol -lookup_symbol_aux (const char *name, const struct block *block, +lookup_symbol_aux (const char *name, symbol_name_match_type match_type, + const struct block *block, const domain_enum domain, enum language language, struct field_of_this_result *is_a_field_of_this) { @@ -2015,7 +2031,7 @@ lookup_symbol_aux (const char *name, const struct block *block, /* Search specified block and its superiors. Don't search STATIC_BLOCK or GLOBAL_BLOCK. */ - result = lookup_local_symbol (name, block, domain, language); + result = lookup_local_symbol (name, match_type, block, domain, language); if (result.symbol != NULL) { if (symbol_lookup_debug) @@ -2097,7 +2113,9 @@ lookup_symbol_aux (const char *name, const struct block *block, Don't search STATIC_BLOCK or GLOBAL_BLOCK. */ static struct block_symbol -lookup_local_symbol (const char *name, const struct block *block, +lookup_local_symbol (const char *name, + symbol_name_match_type match_type, + const struct block *block, const domain_enum domain, enum language language) { @@ -2112,7 +2130,7 @@ lookup_local_symbol (const char *name, const struct block *block, while (block != static_block) { - sym = lookup_symbol_in_block (name, block, domain); + sym = lookup_symbol_in_block (name, match_type, block, domain); if (sym != NULL) return (struct block_symbol) {sym, block}; @@ -2165,7 +2183,8 @@ lookup_objfile_from_block (const struct block *block) /* See symtab.h. */ struct symbol * -lookup_symbol_in_block (const char *name, const struct block *block, +lookup_symbol_in_block (const char *name, symbol_name_match_type match_type, + const struct block *block, const domain_enum domain) { struct symbol *sym; @@ -2181,7 +2200,7 @@ lookup_symbol_in_block (const char *name, const struct block *block, domain_name (domain)); } - sym = block_lookup_symbol (block, name, domain); + sym = block_lookup_symbol (block, name, match_type, domain); if (sym) { if (symbol_lookup_debug > 1) @@ -2370,7 +2389,8 @@ lookup_symbol_via_quick_fns (struct objfile *objfile, int block_index, bv = COMPUNIT_BLOCKVECTOR (cust); block = BLOCKVECTOR_BLOCK (bv, block_index); - result.symbol = block_lookup_symbol (block, name, domain); + result.symbol = block_lookup_symbol (block, name, + symbol_name_match_type::FULL, domain); if (result.symbol == NULL) error_in_psymtab_expansion (block_index, name, cust); @@ -2483,7 +2503,9 @@ lookup_symbol_in_static_block (const char *name, domain_name (domain)); } - sym = lookup_symbol_in_block (name, static_block, domain); + sym = lookup_symbol_in_block (name, + symbol_name_match_type::FULL, + static_block, domain); if (symbol_lookup_debug) { fprintf_unfiltered (gdb_stdlog, diff --git a/gdb/symtab.h b/gdb/symtab.h index 239a479..2a1fac6 100644 --- a/gdb/symtab.h +++ b/gdb/symtab.h @@ -60,6 +60,15 @@ enum class symbol_name_match_type namespace/module/package. */ FULL, + /* Literal symbol matching. This is like FULL, but the search name + did not come from the user; instead it is already a search name + retrieved from a SYMBOL_SEARCH_NAME/MSYMBOL_SEARCH_NAME call. + Matches the symbol name exactly, with strcmp. The language + vector's get_symbol_name_matcher routines never see this -- it is + handled by the common language_get_symbol_name_matcher routine + instead. */ + LITERAL, + /* Expression matching. The same as FULL matching in most languages. The same as WILD matching in Ada. */ EXPRESSION, @@ -1554,6 +1563,18 @@ extern struct block_symbol lookup_symbol (const char *, const domain_enum, struct field_of_this_result *); +/* Find the definition for a specified symbol search name SEARCH_NAME + in domain DOMAIN, visible from lexical block BLOCK if non-NULL or + from global/static blocks if BLOCK is NULL. The symbol name is + matched literally, i.e., with a straight strcmp. See definition of + symbol_name_match_type::LITERAL. Returns the struct symbol + pointer, or NULL if no symbol is found. The symbol's section is + fixed up if necessary. */ + +extern struct block_symbol lookup_symbol_literal (const char *name, + const struct block *block, + domain_enum domain); + /* A default version of lookup_symbol_nonlocal for use by languages that can't think of anything better to do. This implements the C lookup rules. */ @@ -1603,6 +1624,7 @@ extern struct block_symbol extern struct symbol * lookup_symbol_in_block (const char *name, + symbol_name_match_type match_type, const struct block *block, const domain_enum domain); -- 2.5.5 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument 2017-12-14 19:02 ` Pedro Alves @ 2017-12-14 19:41 ` Pedro Alves 2017-12-15 9:48 ` Joel Brobecker 2017-12-15 7:54 ` Joel Brobecker 1 sibling, 1 reply; 27+ messages in thread From: Pedro Alves @ 2017-12-14 19:41 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches On 12/14/2017 07:02 PM, Pedro Alves wrote: > @@ -2271,7 +2272,8 @@ maintenance_check_psymtabs (const char *ignore, int from_tty) > length = ps->n_global_syms; > while (length--) > { > - sym = block_lookup_symbol (b, SYMBOL_LINKAGE_NAME (*psym), > + sym = block_lookup_symbol (b, SYMBOL_SEARCH_NAME (*psym), > + symbol_name_match_type::LITERAL, > SYMBOL_DOMAIN (*psym)); > if (!sym) > { Reading back the patch on the list, I realized that this must be fixing "maint check-psymtabs" for Ada. And indeed, without my patch, I get here: $ gdb ./testsuite/outputs/gdb.ada/var_arr_typedef/var_arr_typedef (gdb) start ... (gdb) maint check-psymtabs Global symbol `adaS' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab Global symbol `interfacesS' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab Global symbol `packB' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab Global symbol `packS' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab Global symbol `systemS' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab Global symbol `var_arr_typedefB' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab (gdb) After: (gdb) start ... (gdb) maint check-psymtabs (gdb Looks like we only test that command for C, currently... Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument 2017-12-14 19:41 ` Pedro Alves @ 2017-12-15 9:48 ` Joel Brobecker 2017-12-15 11:02 ` Pedro Alves 2017-12-15 18:31 ` Pedro Alves 0 siblings, 2 replies; 27+ messages in thread From: Joel Brobecker @ 2017-12-15 9:48 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches [-- Attachment #1: Type: text/plain, Size: 2624 bytes --] > Reading back the patch on the list, I realized that this must be > fixing "maint check-psymtabs" for Ada. And indeed, without my > patch, I get here: > > $ gdb ./testsuite/outputs/gdb.ada/var_arr_typedef/var_arr_typedef > (gdb) start > ... > (gdb) maint check-psymtabs > Global symbol `adaS' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab > Global symbol `interfacesS' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab > Global symbol `packB' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab > Global symbol `packS' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab > Global symbol `systemS' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab > Global symbol `var_arr_typedefB' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab > (gdb) > > After: > > (gdb) start > ... > (gdb) maint check-psymtabs > (gdb > > Looks like we only test that command for C, currently... Good point! Here is a commit which adds a testcase. Sadly, unlike you, I still get an error: (gdb) maintenance check-psymtabs Global symbol `interfaces__cS' only found in /[...]/maint_with_ada/b~var_arr_typedef.adb psymtab I am not sure why this is happening just yet; the symbol, at first, looked like it had an interesting feature, which is both a DW_AT_name and a DW_AT_linkage name: <1><ad2>: Abbrev Number: 35 (DW_TAG_variable) <ad3> DW_AT_name : (indirect string, offset: 0x476): ada_main__u00047 <ad7> DW_AT_decl_file : 5 <ad8> DW_AT_decl_line : 132 <ad9> DW_AT_linkage_name: (indirect string, offset: 0x1b7e): interfaces__cS <add> DW_AT_type : <0x79> <ae1> DW_AT_external : 1 <ae1> DW_AT_location : 9 byte block: 3 20 1 0 0 0 0 0 0 (DW_OP_addr: 120) However, there are plenty of other similar symbols, for instance: <1><b04>: Abbrev Number: 35 (DW_TAG_variable) <b05> DW_AT_name : (indirect string, offset: 0x4b9): ada_main__u00049 <b09> DW_AT_decl_file : 5 <b0a> DW_AT_decl_line : 136 <b0b> DW_AT_linkage_name: (indirect string, offset: 0x17cc): system__bounded_stringsS <b0f> DW_AT_type : <0x79> <b13> DW_AT_external : 1 <b13> DW_AT_location : 9 byte block: 3 28 1 0 0 0 0 0 0 (DW_OP_addr: 128) So I'm still not sure what makes interfaces__cS special. I will look into it when I have a chance... -- Joel [-- Attachment #2: 0001-gdb.ada-maint_with_ada.exp-New-testcase.patch --] [-- Type: text/x-diff, Size: 6181 bytes --] From e0a28e2429b23fd03723be5ab2833ea4aeece19a Mon Sep 17 00:00:00 2001 From: Joel Brobecker <brobecker@adacore.com> Date: Fri, 15 Dec 2017 04:38:39 -0500 Subject: [PATCH] gdb.ada/maint_with_ada.exp: New testcase --- gdb/testsuite/gdb.ada/maint_with_ada.exp | 37 ++++++++++++++++++++++ gdb/testsuite/gdb.ada/maint_with_ada/pack.adb | 25 +++++++++++++++ gdb/testsuite/gdb.ada/maint_with_ada/pack.ads | 29 +++++++++++++++++ .../gdb.ada/maint_with_ada/var_arr_typedef.adb | 28 ++++++++++++++++ 4 files changed, 119 insertions(+) create mode 100644 gdb/testsuite/gdb.ada/maint_with_ada.exp create mode 100644 gdb/testsuite/gdb.ada/maint_with_ada/pack.adb create mode 100644 gdb/testsuite/gdb.ada/maint_with_ada/pack.ads create mode 100644 gdb/testsuite/gdb.ada/maint_with_ada/var_arr_typedef.adb diff --git a/gdb/testsuite/gdb.ada/maint_with_ada.exp b/gdb/testsuite/gdb.ada/maint_with_ada.exp new file mode 100644 index 0000000..9ede035 --- /dev/null +++ b/gdb/testsuite/gdb.ada/maint_with_ada.exp @@ -0,0 +1,37 @@ +# Copyright 2015-2017 Free Software Foundation, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +load_lib "ada.exp" + +standard_ada_testfile var_arr_typedef + +if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug ]] != "" } { + return -1 +} + +clean_restart ${testfile} + +# Insert a breakpoint in each compilation unit, to force their psymtab's +# expansion to a full symtab. This will allow the check-psymtabs command +# to perform a more extensive check regarding those units which are in +# Ada. + +gdb_breakpoint "adainit" +gdb_breakpoint "Var_Arr_Typedef" +gdb_breakpoint "Do_Nothing" + +gdb_test_no_output "maintenance check-psymtabs" + +gdb_test_no_output "maintenance check-symtabs" diff --git a/gdb/testsuite/gdb.ada/maint_with_ada/pack.adb b/gdb/testsuite/gdb.ada/maint_with_ada/pack.adb new file mode 100644 index 0000000..dc6c732 --- /dev/null +++ b/gdb/testsuite/gdb.ada/maint_with_ada/pack.adb @@ -0,0 +1,25 @@ +-- Copyright 2015-2017 Free Software Foundation, Inc. +-- +-- This program is free software; you can redistribute it and/or modify +-- it under the terms of the GNU General Public License as published by +-- the Free Software Foundation; either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU General Public License for more details. +-- +-- You should have received a copy of the GNU General Public License +-- along with this program. If not, see <http://www.gnu.org/licenses/>. + +package body Pack is + + function Identity (I : Integer) return Integer is + begin + return I; + end Identity; + + procedure Do_Nothing (A : Array_Type) is null; + +end Pack; diff --git a/gdb/testsuite/gdb.ada/maint_with_ada/pack.ads b/gdb/testsuite/gdb.ada/maint_with_ada/pack.ads new file mode 100644 index 0000000..efd73a4 --- /dev/null +++ b/gdb/testsuite/gdb.ada/maint_with_ada/pack.ads @@ -0,0 +1,29 @@ +-- Copyright 2015-2017 Free Software Foundation, Inc. +-- +-- This program is free software; you can redistribute it and/or modify +-- it under the terms of the GNU General Public License as published by +-- the Free Software Foundation; either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU General Public License for more details. +-- +-- You should have received a copy of the GNU General Public License +-- along with this program. If not, see <http://www.gnu.org/licenses/>. + +package Pack is + type Rec_Type is record + I : Integer; + B : Boolean; + end record; + + type Vec_Type is array (1 .. 4) of Rec_Type; + + type Array_Type is array (Positive range <>) of Vec_Type; + + procedure Do_Nothing (A : Array_Type); + function Identity (I : Integer) return Integer; + +end Pack; diff --git a/gdb/testsuite/gdb.ada/maint_with_ada/var_arr_typedef.adb b/gdb/testsuite/gdb.ada/maint_with_ada/var_arr_typedef.adb new file mode 100644 index 0000000..224e78f --- /dev/null +++ b/gdb/testsuite/gdb.ada/maint_with_ada/var_arr_typedef.adb @@ -0,0 +1,28 @@ +-- Copyright 2015-2017 Free Software Foundation, Inc. +-- +-- This program is free software; you can redistribute it and/or modify +-- it under the terms of the GNU General Public License as published by +-- the Free Software Foundation; either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU General Public License for more details. +-- +-- You should have received a copy of the GNU General Public License +-- along with this program. If not, see <http://www.gnu.org/licenses/>. + +with Pack; use Pack; + +procedure Var_Arr_Typedef is + RA : constant Rec_Type := (3, False); + RB : constant Rec_Type := (2, True); + + VA : constant Vec_Type := (RA, RA, RB, RB); + VB : constant Vec_Type := (RB, RB, RA, RA); + + A : constant Array_Type (1 .. Identity (4)) := (VA, VA, VB, VB); +begin + Do_Nothing (A); -- BREAK +end Var_Arr_Typedef; -- 2.1.4 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument 2017-12-15 9:48 ` Joel Brobecker @ 2017-12-15 11:02 ` Pedro Alves 2017-12-15 18:57 ` Pedro Alves 2017-12-15 18:31 ` Pedro Alves 1 sibling, 1 reply; 27+ messages in thread From: Pedro Alves @ 2017-12-15 11:02 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches Hi Joel, I woke up realizing that I completely forgot that psymbols have no overload/function parameter info in C++, so a straight strcmp probably doesn't work properly for C++. Indeed, I remembered now to do "maint check-psymtabs" when debugging gdb, and I get back a few problems: (top-gdb) maint check-psymtabs Global symbol `__gnu_cxx::operator!=<symtab_and_line*, std::vector<symtab_and_line> >' only found in src/gdb/cli/cli-cmds.c psymtab Global symbol `__gnu_cxx::operator-<const symtab_and_line*, std::vector<symtab_and_line> >' only found in src/gdb/cli/cli-cmds.c psymtab [...] Maybe what we need is to be a little less aggressive then and add a new symbol_name_match_type::SEARCH_SYMBOL instead that takes as input a non-user-input search symbol like symbol_name_match_type::LITERAL was, and then we skip any decoding/demangling steps (like LITERAL) and make: - Ada treat that as a verbatim match, - other languages treat it as symbol_name_match_type::FULL. I can't look at this right now, though I'll try to play with it a bit more later today. I'm a bit worried on timing since I'm going to be away next week (today's my last official day before xmas holiday). :-/ No idea offhand on the issue below. Thanks, Pedro Alves On 12/15/2017 09:47 AM, Joel Brobecker wrote: >> Reading back the patch on the list, I realized that this must be >> fixing "maint check-psymtabs" for Ada. And indeed, without my >> patch, I get here: >> >> $ gdb ./testsuite/outputs/gdb.ada/var_arr_typedef/var_arr_typedef >> (gdb) start >> ... >> (gdb) maint check-psymtabs >> Global symbol `adaS' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab >> Global symbol `interfacesS' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab >> Global symbol `packB' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab >> Global symbol `packS' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab >> Global symbol `systemS' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab >> Global symbol `var_arr_typedefB' only found in gdb/testsuite/outputs/gdb.ada/var_arr_typedef/b~var_arr_typedef.adb psymtab >> (gdb) >> >> After: >> >> (gdb) start >> ... >> (gdb) maint check-psymtabs >> (gdb >> >> Looks like we only test that command for C, currently... > > Good point! > > Here is a commit which adds a testcase. > > Sadly, unlike you, I still get an error: > > (gdb) maintenance check-psymtabs > Global symbol `interfaces__cS' only found in /[...]/maint_with_ada/b~var_arr_typedef.adb psymtab > > I am not sure why this is happening just yet; the symbol, at first, > looked like it had an interesting feature, which is both a DW_AT_name > and a DW_AT_linkage name: > > <1><ad2>: Abbrev Number: 35 (DW_TAG_variable) > <ad3> DW_AT_name : (indirect string, offset: 0x476): ada_main__u00047 > <ad7> DW_AT_decl_file : 5 > <ad8> DW_AT_decl_line : 132 > <ad9> DW_AT_linkage_name: (indirect string, offset: 0x1b7e): interfaces__cS > <add> DW_AT_type : <0x79> > <ae1> DW_AT_external : 1 > <ae1> DW_AT_location : 9 byte block: 3 20 1 0 0 0 0 0 0 (DW_OP_addr: 120) > > However, there are plenty of other similar symbols, for instance: > > <1><b04>: Abbrev Number: 35 (DW_TAG_variable) > <b05> DW_AT_name : (indirect string, offset: 0x4b9): ada_main__u00049 > <b09> DW_AT_decl_file : 5 > <b0a> DW_AT_decl_line : 136 > <b0b> DW_AT_linkage_name: (indirect string, offset: 0x17cc): system__bounded_stringsS > <b0f> DW_AT_type : <0x79> > <b13> DW_AT_external : 1 > <b13> DW_AT_location : 9 byte block: 3 28 1 0 0 0 0 0 0 (DW_OP_addr: 128) > > So I'm still not sure what makes interfaces__cS special. I will look > into it when I have a chance... > ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument 2017-12-15 11:02 ` Pedro Alves @ 2017-12-15 18:57 ` Pedro Alves 2018-01-03 4:33 ` Joel Brobecker 0 siblings, 1 reply; 27+ messages in thread From: Pedro Alves @ 2017-12-15 18:57 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches On 12/15/2017 11:02 AM, Pedro Alves wrote: > Hi Joel, > > I woke up realizing that I completely forgot that psymbols have no > overload/function parameter info in C++, so a straight strcmp probably > doesn't work properly for C++. Indeed, I remembered now to do > "maint check-psymtabs" when debugging gdb, and I get back a few > problems: > > (top-gdb) maint check-psymtabs > Global symbol `__gnu_cxx::operator!=<symtab_and_line*, std::vector<symtab_and_line> >' only found in src/gdb/cli/cli-cmds.c psymtab > Global symbol `__gnu_cxx::operator-<const symtab_and_line*, std::vector<symtab_and_line> >' only found in src/gdb/cli/cli-cmds.c psymtab > [...] > > Maybe what we need is to be a little less aggressive then and > add a new symbol_name_match_type::SEARCH_SYMBOL instead that takes as > input a non-user-input search symbol like symbol_name_match_type::LITERAL > was, and then we skip any decoding/demangling steps (like LITERAL) and make: > - Ada treat that as a verbatim match, > - other languages treat it as symbol_name_match_type::FULL. > > I can't look at this right now, though I'll try to play with it a bit > more later today. I'm a bit worried on timing since I'm going to be > away next week (today's my last official day before xmas holiday). :-/ > Errr, it turns out that I had done the above on the wrong branch... I get the above output even if I use the system gdb (7.10) to debug the new gdb, and then do "maint check-psymtabs". So those are not a regression. However, (as suspected) with the LITERAL patch things indeed got worse for C++: Global symbol `error' only found in src/gdb/common/errors.c psymtab Global symbol `internal_error' only found in src/gdb/common/errors.c psymtab Global symbol `internal_warning' only found in src/gdb/common/errors.c psymtab Global symbol `warning' only found in src/gdb/common/errors.c psymtab Static symbol `info_command' only found in src/gdb/cli/cli-cmds.c psymtab Static symbol `show_command' only found in src/gdb/cli/cli-cmds.c psymtab Static symbol `help_command' only found in src/gdb/cli/cli-cmds.c psymtab Static symbol `complete_command' only found in src/gdb/cli/cli-cmds.c psymtab Static symbol `show_version' only found in src/gdb/cli/cli-cmds.c psymtab Static symbol `show_configuration' only found in src/gdb/cli/cli-cmds.c psymtab Static symbol `pwd_command' only found in src/gdb/cli/cli-cmds.c psymtab Static symbol `show_script_ext_mode' only found in src/gdb/cli/cli-cmds.c psymtab Static symbol `source_script_from_stream' only found in src/gdb/cli/cli-cmds.c psymtab [...many more...] With the patch below applied on top, which is a minimal version of the design change I suggested (LITERAL -> SEARCH_SYMBOL), I now get again the same "maint check-psymtabs" output that I get with system gdb. So seems like this is on the good track. Your internal-error test still passes as well as the new "maint check-psymtabs" for gdb.ada/ testcase. I've pushed the current state to users/palves/literal-matching. I probably won't be able to look at this more today. From 6b4025034b0b0e8b2511e4c355365c9ff69be822 Mon Sep 17 00:00:00 2001 From: Pedro Alves <palves@redhat.com> Date: Fri, 15 Dec 2017 17:46:56 +0000 Subject: [PATCH] Move literal matching to Ada --- gdb/ada-lang.c | 8 ++++++++ gdb/cp-support.c | 1 + gdb/language.c | 9 +++------ gdb/symtab.c | 12 ++++++++---- 4 files changed, 20 insertions(+), 10 deletions(-) diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c index 9e637eb..5f4e255 100644 --- a/gdb/ada-lang.c +++ b/gdb/ada-lang.c @@ -14065,12 +14065,20 @@ ada_symbol_name_matches (const char *symbol_search_name, comp_match_res); } +bool +literal_symbol_name_matcher (const char *symbol_search_name, + const lookup_name_info &lookup_name, + completion_match_result *comp_match_res); + /* Implement the "la_get_symbol_name_matcher" language_defn method for Ada. */ static symbol_name_matcher_ftype * ada_get_symbol_name_matcher (const lookup_name_info &lookup_name) { + if (lookup_name.match_type () == symbol_name_match_type::LITERAL) + return literal_symbol_name_matcher; + if (lookup_name.completion_mode ()) return ada_symbol_name_matches; else diff --git a/gdb/cp-support.c b/gdb/cp-support.c index 34a8439..7c14423 100644 --- a/gdb/cp-support.c +++ b/gdb/cp-support.c @@ -1793,6 +1793,7 @@ cp_get_symbol_name_matcher (const lookup_name_info &lookup_name) { case symbol_name_match_type::FULL: case symbol_name_match_type::EXPRESSION: + case symbol_name_match_type::LITERAL: return cp_fq_symbol_name_matches; case symbol_name_match_type::WILD: return cp_symbol_name_matches; diff --git a/gdb/language.c b/gdb/language.c index 5bcdfb9..44afdef 100644 --- a/gdb/language.c +++ b/gdb/language.c @@ -727,7 +727,7 @@ default_symbol_name_matcher (const char *symbol_search_name, /* A name matcher that matches the symbol name exactly, with strcmp. */ -static bool +bool literal_symbol_name_matcher (const char *symbol_search_name, const lookup_name_info &lookup_name, completion_match_result *comp_match_res) @@ -753,13 +753,10 @@ symbol_name_matcher_ftype * language_get_symbol_name_matcher (const language_defn *lang, const lookup_name_info &lookup_name) { - if (lookup_name.match_type () == symbol_name_match_type::LITERAL) - return literal_symbol_name_matcher; - if (lang->la_get_symbol_name_matcher != nullptr) return lang->la_get_symbol_name_matcher (lookup_name); - - return default_symbol_name_matcher; + else + return default_symbol_name_matcher; } /* Define the language that is no language. */ diff --git a/gdb/symtab.c b/gdb/symtab.c index 78018a1..6957757 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -1775,14 +1775,18 @@ demangle_for_lookup_info::demangle_for_lookup_info if (without_params != NULL) { - m_demangled_name = demangle_for_lookup (without_params.get (), - lang, storage); + if (lookup_name.match_type () != symbol_name_match_type::LITERAL) + m_demangled_name = demangle_for_lookup (without_params.get (), + lang, storage); return; } } - m_demangled_name = demangle_for_lookup (lookup_name.name ().c_str (), - lang, storage); + if (lookup_name.match_type () == symbol_name_match_type::LITERAL) + m_demangled_name = lookup_name.name (); + else + m_demangled_name = demangle_for_lookup (lookup_name.name ().c_str (), + lang, storage); } /* See symtab.h. */ -- 2.5.5 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument 2017-12-15 18:57 ` Pedro Alves @ 2018-01-03 4:33 ` Joel Brobecker 2018-01-04 9:48 ` Joel Brobecker 2018-01-05 16:28 ` Pedro Alves 0 siblings, 2 replies; 27+ messages in thread From: Joel Brobecker @ 2018-01-03 4:33 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches Hi Pedro, > > Maybe what we need is to be a little less aggressive then and > > add a new symbol_name_match_type::SEARCH_SYMBOL instead that takes as > > input a non-user-input search symbol like symbol_name_match_type::LITERAL > > was, and then we skip any decoding/demangling steps (like LITERAL) and make: > > - Ada treat that as a verbatim match, > > - other languages treat it as symbol_name_match_type::FULL. [...] > With the patch below applied on top, which is a minimal version of > the design change I suggested (LITERAL -> SEARCH_SYMBOL), I now get > again the same "maint check-psymtabs" output that I get with system gdb. > So seems like this is on the good track. > > Your internal-error test still passes as well as the new > "maint check-psymtabs" for gdb.ada/ testcase. Sounds like we are indeed on the right track! I ran the changes through both the official testsuite and AdaCore's testsuite, and I can confirm no regression :). Is there something else I can do to help? -- Joel ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument 2018-01-03 4:33 ` Joel Brobecker @ 2018-01-04 9:48 ` Joel Brobecker 2018-01-04 12:39 ` Pedro Alves 2018-01-05 16:28 ` Pedro Alves 1 sibling, 1 reply; 27+ messages in thread From: Joel Brobecker @ 2018-01-04 9:48 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches Hi Pedro, FYI, I just pushed 3 commits that add more testing in gdb.ada demonstrating issues introduced by the wild-matching patch series I hadn't noticed before. See: https://www.sourceware.org/ml/gdb-patches/2018-01/msg00052.html As hinted by the ChangeLog entries, I have also created PR gdb/22670 so as to be able to KFAIL the expected failures. And now that we have this PR, we can use it to track the fixes as well ;-). I also pushed the patch which adds "maint check psymtabs" testing; also with a setup_kfail. Here is my plan: I will continue going through the results I get when running AdaCore's GDB testsuite (almost there). If there are other tests related to this patch series, I will try to contribute those as well. Once I'm doing analyzing the current results, I will start investigating those failures one by one, and let you know what I find. For now, all I know is that litteral matching is not enough to fix those, so I am not sure what is happening just yet. I will let you know! -- Joel ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument 2018-01-04 9:48 ` Joel Brobecker @ 2018-01-04 12:39 ` Pedro Alves 0 siblings, 0 replies; 27+ messages in thread From: Pedro Alves @ 2018-01-04 12:39 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches Hi Joel, On 01/04/2018 09:48 AM, Joel Brobecker wrote: > Hi Pedro, > > FYI, I just pushed 3 commits that add more testing in gdb.ada > demonstrating issues introduced by the wild-matching patch series > I hadn't noticed before. See: > > https://www.sourceware.org/ml/gdb-patches/2018-01/msg00052.html > > As hinted by the ChangeLog entries, I have also created PR gdb/22670 > so as to be able to KFAIL the expected failures. And now that we have > this PR, we can use it to track the fixes as well ;-). > > I also pushed the patch which adds "maint check psymtabs" testing; > also with a setup_kfail. > > Here is my plan: I will continue going through the results I get > when running AdaCore's GDB testsuite (almost there). If there are > other tests related to this patch series, I will try to contribute > those as well. Once I'm doing analyzing the current results, I will > start investigating those failures one by one, and let you know > what I find. For now, all I know is that litteral matching is not > enough to fix those, so I am not sure what is happening just yet. > I will let you know! Thanks! I'm catching up to this and will take a look at the new tests. Pedro Alves ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument 2018-01-03 4:33 ` Joel Brobecker 2018-01-04 9:48 ` Joel Brobecker @ 2018-01-05 16:28 ` Pedro Alves 2018-01-16 11:54 ` Pedro Alves 1 sibling, 1 reply; 27+ messages in thread From: Pedro Alves @ 2018-01-05 16:28 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches On 01/03/2018 04:33 AM, Joel Brobecker wrote: > Hi Pedro, > >>> Maybe what we need is to be a little less aggressive then and >>> add a new symbol_name_match_type::SEARCH_SYMBOL instead that takes as >>> input a non-user-input search symbol like symbol_name_match_type::LITERAL >>> was, and then we skip any decoding/demangling steps (like LITERAL) and make: >>> - Ada treat that as a verbatim match, >>> - other languages treat it as symbol_name_match_type::FULL. > [...] >> With the patch below applied on top, which is a minimal version of >> the design change I suggested (LITERAL -> SEARCH_SYMBOL), I now get >> again the same "maint check-psymtabs" output that I get with system gdb. >> So seems like this is on the good track. >> >> Your internal-error test still passes as well as the new >> "maint check-psymtabs" for gdb.ada/ testcase. > > Sounds like we are indeed on the right track! > > I ran the changes through both the official testsuite and AdaCore's > testsuite, and I can confirm no regression :). Is there something > else I can do to help? I just needed to fold both patches in, do the renaming thing, update comments and compose git log + ChangeLog. After looking at the other regressions and convincing myself that this is good enough, that is. :-) Here's the resulting patch, which I merged to both master and branch now. I remembered to update the copyright year in the new testcase, and to include a short description of what the testcase is about in the .exp file. From de63c46b549d1cf4f7851e47872cb759a12983f4 Mon Sep 17 00:00:00 2001 From: Pedro Alves <palves@redhat.com> Date: Fri, 5 Jan 2018 14:04:09 +0000 Subject: [PATCH] Fix regresssion(internal-error) printing subprogram argument (PR gdb/22670) At <https://sourceware.org/ml/gdb-patches/2017-12/msg00298.html>, Joel wrote: ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Consider the following code which first declares a tagged type (the equivalent of a class in Ada), and then a procedure which takes a pointer (access) to this type's 'Class. package Pck is type Top_T is tagged record N : Integer := 1; end record; procedure Inspect (Obj: access Top_T'Class); end Pck; Putting a breakpoint in that procedure and then running to it triggers an internal error: (gdb) break inspect (gdb) continue Breakpoint 1, pck.inspect (obj=0x63e010 /[...]/gdb/stack.c:621: internal-error: void print_frame_args(symbol*, frame_info*, int, ui_file*): Assertion `nsym != NULL' failed. What's special about this subprogram is that it takes an access to what we call a 'Class type, and for implementation reasons, the compiler adds an extra argument named "objL". If you are curious why, it allows the compiler for perform dynamic accessibility checks that are mandated by the language. If we look at the location where we get the internal error (in stack.c), we find that we are looping over the symbol of each parameter, and for each parameter, we do: /* We have to look up the symbol because arguments can have two entries (one a parameter, one a local) and the one we want is the local, which lookup_symbol will find for us. [...] nsym = lookup_symbol (SYMBOL_LINKAGE_NAME (sym), b, VAR_DOMAIN, NULL).symbol; gdb_assert (nsym != NULL); The lookup_symbol goes through the lookup structure, which means the symbol's linkage name ("objL") gets transformed into a lookup_name_info object (in block_lookup_symbol), before it gets fed to the block symbol dictionary iterators. This, in turn, triggers the symbol matching by comparing the "lookup" name which, for Ada, means among other things, lowercasing the given name to "objl". It is this transformation that causes the lookup find no matches, and therefore trip this assertion. Going back to the "offending" call to lookup_symbol in stack.c, what we are trying to do, here, is do a lookup by linkage name. So, I think what we mean to be doing is a completely literal symbol lookup, so maybe not even strcmp_iw, but actually just plain strcmp??? In the past, in practice, you could get that effect by doing a lookup using the C language. But that doesn't work, because we still end up somehow using Ada's lookup_name routine which transforms "objL". So, ideally, as I hinted before, I think what we need is a way to perform a literal lookup so that searches by linkage names like the above can be performed. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ This commit fixes the problem by implementing something similar to Joel's literal idea, but with some important differences. I considered adding a symbol_name_match_type::LINKAGE and supporting searching by linkage name for any language, but the problem with that is that the dictionaries only work with SYMBOL_SEARCH_NAME, because that's what is used for hashing. We'd need separate dictionaries for hashed linkage names. So with the current symbol tables infrastructure, it's not literal linkage names that we want to pass down, but instead literal _search_ names (SYMBOL_SEARCH_NAME, etc.). However, psymbols have no overload/function parameter info in C++, so a straight strcmp doesn't work properly for C++ name matching. So what we do is be a little less aggressive then and add a new symbol_name_match_type::SEARCH_SYMBOL instead that takes as input a non-user-input search symbol, and then we skip any decoding/demangling steps and make: - Ada treat that as a verbatim match, - other languages treat it as symbol_name_match_type::FULL. This also fixes the new '"maint check-psymtabs" for Ada' testcase for me (gdb.ada/maint_with_ada.exp). I've not removed the kfail yet because Joel still sees that testcase failing with this patch. That'll be fixed in follow up patches. gdb/ChangeLog: 2018-01-05 Pedro Alves <palves@redhat.com> PR gdb/22670 * ada-lang.c (literal_symbol_name_matcher): New function. (ada_get_symbol_name_matcher): Use it for symbol_name_match_type::SEARCH_NAME. * block.c (block_lookup_symbol): New parameter 'match_type'. Pass it down instead of assuming symbol_name_match_type::FULL. * block.h (block_lookup_symbol): New parameter 'match_type'. * c-valprint.c (print_unpacked_pointer): Use lookup_symbol_search_name instead of lookup_symbol. * compile/compile-object-load.c (get_out_value_type): Pass down symbol_name_match_type::SEARCH_NAME. * cp-namespace.c (cp_basic_lookup_symbol): Pass down symbol_name_match_type::FULL. * cp-support.c (cp_get_symbol_name_matcher): Handle symbol_name_match_type::SEARCH_NAME. * infrun.c (insert_exception_resume_breakpoint): Use lookup_symbol_search_name. * p-valprint.c (pascal_val_print): Use lookup_symbol_search_name. * psymtab.c (maintenance_check_psymtabs): Use symbol_name_match_type::SEARCH_NAME and SYMBOL_SEARCH_NAME. * stack.c (print_frame_args): Use lookup_symbol_search_name and SYMBOL_SEARCH_NAME. * symtab.c (lookup_local_symbol): Don't demangle the lookup name if symbol_name_match_type::SEARCH_NAME. (lookup_symbol_in_language): Pass down symbol_name_match_type::FULL. (lookup_symbol_search_name): New. (lookup_language_this): Pass down symbol_name_match_type::SEARCH_NAME. (lookup_symbol_aux, lookup_local_symbol): New parameter 'match_type'. Pass it down. * symtab.h (symbol_name_match_type::SEARCH_NAME): New enumerator. (lookup_symbol_search_name): New declaration. (lookup_symbol_in_block): New 'match_type' parameter. gdb/testsuite/ChangeLog: 2018-01-05 Joel Brobecker <brobecker@adacore.com> PR gdb/22670 * gdb.ada/access_tagged_param.exp: New file. * gdb.ada/access_tagged_param/foo.adb: New file. --- gdb/ChangeLog | 37 +++++++++++++++ gdb/testsuite/ChangeLog | 6 +++ gdb/ada-lang.c | 26 +++++++++++ gdb/block.c | 3 +- gdb/block.h | 1 + gdb/c-valprint.c | 9 ++-- gdb/compile/compile-object-load.c | 6 ++- gdb/cp-namespace.c | 4 +- gdb/cp-support.c | 1 + gdb/infrun.c | 3 +- gdb/p-valprint.c | 10 +++-- gdb/psymtab.c | 6 ++- gdb/stack.c | 8 ++-- gdb/symtab.c | 55 +++++++++++++++++------ gdb/symtab.h | 25 +++++++++++ gdb/testsuite/gdb.ada/access_tagged_param.exp | 40 +++++++++++++++++ gdb/testsuite/gdb.ada/access_tagged_param/foo.adb | 20 +++++++++ gdb/testsuite/gdb.ada/access_tagged_param/pck.adb | 21 +++++++++ gdb/testsuite/gdb.ada/access_tagged_param/pck.ads | 21 +++++++++ 19 files changed, 271 insertions(+), 31 deletions(-) create mode 100644 gdb/testsuite/gdb.ada/access_tagged_param.exp create mode 100644 gdb/testsuite/gdb.ada/access_tagged_param/foo.adb create mode 100644 gdb/testsuite/gdb.ada/access_tagged_param/pck.adb create mode 100644 gdb/testsuite/gdb.ada/access_tagged_param/pck.ads diff --git a/gdb/ChangeLog b/gdb/ChangeLog index badced8f061..10aabcde3bb 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,40 @@ +2018-01-05 Pedro Alves <palves@redhat.com> + + PR gdb/22670 + * ada-lang.c (literal_symbol_name_matcher): New function. + (ada_get_symbol_name_matcher): Use it for + symbol_name_match_type::SEARCH_NAME. + * block.c (block_lookup_symbol): New parameter 'match_type'. Pass + it down instead of assuming symbol_name_match_type::FULL. + * block.h (block_lookup_symbol): New parameter 'match_type'. + * c-valprint.c (print_unpacked_pointer): Use + lookup_symbol_search_name instead of lookup_symbol. + * compile/compile-object-load.c (get_out_value_type): Pass down + symbol_name_match_type::SEARCH_NAME. + * cp-namespace.c (cp_basic_lookup_symbol): Pass down + symbol_name_match_type::FULL. + * cp-support.c (cp_get_symbol_name_matcher): Handle + symbol_name_match_type::SEARCH_NAME. + * infrun.c (insert_exception_resume_breakpoint): Use + lookup_symbol_search_name. + * p-valprint.c (pascal_val_print): Use lookup_symbol_search_name. + * psymtab.c (maintenance_check_psymtabs): Use + symbol_name_match_type::SEARCH_NAME and SYMBOL_SEARCH_NAME. + * stack.c (print_frame_args): Use lookup_symbol_search_name and + SYMBOL_SEARCH_NAME. + * symtab.c (lookup_local_symbol): Don't demangle the lookup name + if symbol_name_match_type::SEARCH_NAME. + (lookup_symbol_in_language): Pass down + symbol_name_match_type::FULL. + (lookup_symbol_search_name): New. + (lookup_language_this): Pass down + symbol_name_match_type::SEARCH_NAME. + (lookup_symbol_aux, lookup_local_symbol): New parameter + 'match_type'. Pass it down. + * symtab.h (symbol_name_match_type::SEARCH_NAME): New enumerator. + (lookup_symbol_search_name): New declaration. + (lookup_symbol_in_block): New 'match_type' parameter. + 2018-01-05 Pedro Alves <palves@redhat.com> PR gdb/22670 diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 94486316d24..18ad7403b49 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2018-01-05 Joel Brobecker <brobecker@adacore.com> + + PR gdb/22670 + * gdb.ada/access_tagged_param.exp: New file. + * gdb.ada/access_tagged_param/foo.adb: New file. + 2018-01-05 Pedro Alves <palves@redhat.com> PR gdb/22670 diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c index e7c2197ca7d..622cfd0a81e 100644 --- a/gdb/ada-lang.c +++ b/gdb/ada-lang.c @@ -14412,12 +14412,38 @@ ada_symbol_name_matches (const char *symbol_search_name, comp_match_res); } +/* A name matcher that matches the symbol name exactly, with + strcmp. */ + +static bool +literal_symbol_name_matcher (const char *symbol_search_name, + const lookup_name_info &lookup_name, + completion_match_result *comp_match_res) +{ + const std::string &name = lookup_name.name (); + + int cmp = (lookup_name.completion_mode () + ? strncmp (symbol_search_name, name.c_str (), name.size ()) + : strcmp (symbol_search_name, name.c_str ())); + if (cmp == 0) + { + if (comp_match_res != NULL) + comp_match_res->set_match (symbol_search_name); + return true; + } + else + return false; +} + /* Implement the "la_get_symbol_name_matcher" language_defn method for Ada. */ static symbol_name_matcher_ftype * ada_get_symbol_name_matcher (const lookup_name_info &lookup_name) { + if (lookup_name.match_type () == symbol_name_match_type::SEARCH_NAME) + return literal_symbol_name_matcher; + if (lookup_name.completion_mode ()) return ada_symbol_name_matches; else diff --git a/gdb/block.c b/gdb/block.c index fb91eb9fee0..57da7ba7393 100644 --- a/gdb/block.c +++ b/gdb/block.c @@ -673,12 +673,13 @@ block_iter_match_next (const lookup_name_info &name, struct symbol * block_lookup_symbol (const struct block *block, const char *name, + symbol_name_match_type match_type, const domain_enum domain) { struct block_iterator iter; struct symbol *sym; - lookup_name_info lookup_name (name, symbol_name_match_type::FULL); + lookup_name_info lookup_name (name, match_type); if (!BLOCK_FUNCTION (block)) { diff --git a/gdb/block.h b/gdb/block.h index 8f400cf8c0f..ff127256ad7 100644 --- a/gdb/block.h +++ b/gdb/block.h @@ -258,6 +258,7 @@ extern struct symbol *block_iter_match_next extern struct symbol *block_lookup_symbol (const struct block *block, const char *name, + symbol_name_match_type match_type, const domain_enum domain); /* Search BLOCK for symbol NAME in DOMAIN but only in primary symbol table of diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c index 7f8154e6601..c4c0918e26a 100644 --- a/gdb/c-valprint.c +++ b/gdb/c-valprint.c @@ -198,14 +198,17 @@ print_unpacked_pointer (struct type *type, struct type *elttype, struct symbol *wsym = NULL; struct type *wtype; struct block *block = NULL; - struct field_of_this_result is_this_fld; if (want_space) fputs_filtered (" ", stream); if (msymbol.minsym != NULL) - wsym = lookup_symbol (MSYMBOL_LINKAGE_NAME(msymbol.minsym), block, - VAR_DOMAIN, &is_this_fld).symbol; + { + const char *search_name + = MSYMBOL_SEARCH_NAME (msymbol.minsym); + wsym = lookup_symbol_search_name (search_name, block, + VAR_DOMAIN).symbol; + } if (wsym) { diff --git a/gdb/compile/compile-object-load.c b/gdb/compile/compile-object-load.c index 0b7215b3236..fac16d70dde 100644 --- a/gdb/compile/compile-object-load.c +++ b/gdb/compile/compile-object-load.c @@ -439,7 +439,10 @@ get_out_value_type (struct symbol *func_sym, struct objfile *objfile, block = BLOCKVECTOR_BLOCK (bv, block_loop); if (BLOCK_FUNCTION (block) != NULL) continue; - gdb_val_sym = block_lookup_symbol (block, COMPILE_I_EXPR_VAL, VAR_DOMAIN); + gdb_val_sym = block_lookup_symbol (block, + COMPILE_I_EXPR_VAL, + symbol_name_match_type::SEARCH_NAME, + VAR_DOMAIN); if (gdb_val_sym == NULL) continue; @@ -466,6 +469,7 @@ get_out_value_type (struct symbol *func_sym, struct objfile *objfile, gdb_type = check_typedef (gdb_type); gdb_ptr_type_sym = block_lookup_symbol (block, COMPILE_I_EXPR_PTR_TYPE, + symbol_name_match_type::SEARCH_NAME, VAR_DOMAIN); if (gdb_ptr_type_sym == NULL) error (_("No \"%s\" symbol found"), COMPILE_I_EXPR_PTR_TYPE); diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c index 0751fda7e63..185607e4eb8 100644 --- a/gdb/cp-namespace.c +++ b/gdb/cp-namespace.c @@ -141,7 +141,9 @@ cp_basic_lookup_symbol (const char *name, const struct block *block, if (global_block != NULL) { - sym.symbol = lookup_symbol_in_block (name, global_block, domain); + sym.symbol = lookup_symbol_in_block (name, + symbol_name_match_type::FULL, + global_block, domain); sym.block = global_block; } } diff --git a/gdb/cp-support.c b/gdb/cp-support.c index 2efc38557ac..dde417be80e 100644 --- a/gdb/cp-support.c +++ b/gdb/cp-support.c @@ -1793,6 +1793,7 @@ cp_get_symbol_name_matcher (const lookup_name_info &lookup_name) { case symbol_name_match_type::FULL: case symbol_name_match_type::EXPRESSION: + case symbol_name_match_type::SEARCH_NAME: return cp_fq_symbol_name_matches; case symbol_name_match_type::WILD: return cp_symbol_name_matches; diff --git a/gdb/infrun.c b/gdb/infrun.c index 3f7d185240b..7e8d8da5884 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -7489,7 +7489,8 @@ insert_exception_resume_breakpoint (struct thread_info *tp, CORE_ADDR handler; struct breakpoint *bp; - vsym = lookup_symbol (SYMBOL_LINKAGE_NAME (sym), b, VAR_DOMAIN, NULL); + vsym = lookup_symbol_search_name (SYMBOL_SEARCH_NAME (sym), + b, VAR_DOMAIN); value = read_var_value (vsym.symbol, vsym.block, frame); /* If the value was optimized out, revert to the old behavior. */ if (! value_optimized_out (value)) diff --git a/gdb/p-valprint.c b/gdb/p-valprint.c index 6d5358630e1..933dbfb6c4d 100644 --- a/gdb/p-valprint.c +++ b/gdb/p-valprint.c @@ -246,15 +246,17 @@ pascal_val_print (struct type *type, struct symbol *wsym = NULL; struct type *wtype; struct block *block = NULL; - struct field_of_this_result is_this_fld; if (want_space) fputs_filtered (" ", stream); if (msymbol.minsym != NULL) - wsym = lookup_symbol (MSYMBOL_LINKAGE_NAME (msymbol.minsym), - block, - VAR_DOMAIN, &is_this_fld).symbol; + { + const char *search_name + = MSYMBOL_SEARCH_NAME (msymbol.minsym); + wsym = lookup_symbol_search_name (search_name, block, + VAR_DOMAIN).symbol; + } if (wsym) { diff --git a/gdb/psymtab.c b/gdb/psymtab.c index 3075be29b77..88d234a7da2 100644 --- a/gdb/psymtab.c +++ b/gdb/psymtab.c @@ -2254,7 +2254,8 @@ maintenance_check_psymtabs (const char *ignore, int from_tty) length = ps->n_static_syms; while (length--) { - sym = block_lookup_symbol (b, SYMBOL_LINKAGE_NAME (*psym), + sym = block_lookup_symbol (b, SYMBOL_SEARCH_NAME (*psym), + symbol_name_match_type::SEARCH_NAME, SYMBOL_DOMAIN (*psym)); if (!sym) { @@ -2271,7 +2272,8 @@ maintenance_check_psymtabs (const char *ignore, int from_tty) length = ps->n_global_syms; while (length--) { - sym = block_lookup_symbol (b, SYMBOL_LINKAGE_NAME (*psym), + sym = block_lookup_symbol (b, SYMBOL_SEARCH_NAME (*psym), + symbol_name_match_type::SEARCH_NAME, SYMBOL_DOMAIN (*psym)); if (!sym) { diff --git a/gdb/stack.c b/gdb/stack.c index 263e7bb0fc6..9993ae654ad 100644 --- a/gdb/stack.c +++ b/gdb/stack.c @@ -616,8 +616,8 @@ print_frame_args (struct symbol *func, struct frame_info *frame, { struct symbol *nsym; - nsym = lookup_symbol (SYMBOL_LINKAGE_NAME (sym), - b, VAR_DOMAIN, NULL).symbol; + nsym = lookup_symbol_search_name (SYMBOL_SEARCH_NAME (sym), + b, VAR_DOMAIN).symbol; gdb_assert (nsym != NULL); if (SYMBOL_CLASS (nsym) == LOC_REGISTER && !SYMBOL_IS_ARGUMENT (nsym)) @@ -2141,8 +2141,8 @@ iterate_over_block_arg_vars (const struct block *b, float). There are also LOC_ARG/LOC_REGISTER pairs which are not combined in symbol-reading. */ - sym2 = lookup_symbol (SYMBOL_LINKAGE_NAME (sym), - b, VAR_DOMAIN, NULL).symbol; + sym2 = lookup_symbol_search_name (SYMBOL_SEARCH_NAME (sym), + b, VAR_DOMAIN).symbol; (*cb) (SYMBOL_PRINT_NAME (sym), sym2, cb_data); } } diff --git a/gdb/symtab.c b/gdb/symtab.c index 3865420cd1e..146dc2e4213 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -75,6 +75,7 @@ static int find_line_common (struct linetable *, int, int *, int); static struct block_symbol lookup_symbol_aux (const char *name, + symbol_name_match_type match_type, const struct block *block, const domain_enum domain, enum language language, @@ -82,6 +83,7 @@ static struct block_symbol static struct block_symbol lookup_local_symbol (const char *name, + symbol_name_match_type match_type, const struct block *block, const domain_enum domain, enum language language); @@ -1773,14 +1775,18 @@ demangle_for_lookup_info::demangle_for_lookup_info if (without_params != NULL) { - m_demangled_name = demangle_for_lookup (without_params.get (), - lang, storage); + if (lookup_name.match_type () != symbol_name_match_type::SEARCH_NAME) + m_demangled_name = demangle_for_lookup (without_params.get (), + lang, storage); return; } } - m_demangled_name = demangle_for_lookup (lookup_name.name ().c_str (), - lang, storage); + if (lookup_name.match_type () == symbol_name_match_type::SEARCH_NAME) + m_demangled_name = lookup_name.name (); + else + m_demangled_name = demangle_for_lookup (lookup_name.name ().c_str (), + lang, storage); } /* See symtab.h. */ @@ -1876,7 +1882,9 @@ lookup_symbol_in_language (const char *name, const struct block *block, demangle_result_storage storage; const char *modified_name = demangle_for_lookup (name, lang, storage); - return lookup_symbol_aux (modified_name, block, domain, lang, + return lookup_symbol_aux (modified_name, + symbol_name_match_type::FULL, + block, domain, lang, is_a_field_of_this); } @@ -1894,6 +1902,16 @@ lookup_symbol (const char *name, const struct block *block, /* See symtab.h. */ +struct block_symbol +lookup_symbol_search_name (const char *search_name, const struct block *block, + domain_enum domain) +{ + return lookup_symbol_aux (search_name, symbol_name_match_type::SEARCH_NAME, + block, domain, language_asm, NULL); +} + +/* See symtab.h. */ + struct block_symbol lookup_language_this (const struct language_defn *lang, const struct block *block) @@ -1915,7 +1933,9 @@ lookup_language_this (const struct language_defn *lang, { struct symbol *sym; - sym = block_lookup_symbol (block, lang->la_name_of_this, VAR_DOMAIN); + sym = block_lookup_symbol (block, lang->la_name_of_this, + symbol_name_match_type::SEARCH_NAME, + VAR_DOMAIN); if (sym != NULL) { if (symbol_lookup_debug > 1) @@ -1986,7 +2006,8 @@ check_field (struct type *type, const char *name, (e.g., demangled name) of the symbol that we're looking for. */ static struct block_symbol -lookup_symbol_aux (const char *name, const struct block *block, +lookup_symbol_aux (const char *name, symbol_name_match_type match_type, + const struct block *block, const domain_enum domain, enum language language, struct field_of_this_result *is_a_field_of_this) { @@ -2015,7 +2036,7 @@ lookup_symbol_aux (const char *name, const struct block *block, /* Search specified block and its superiors. Don't search STATIC_BLOCK or GLOBAL_BLOCK. */ - result = lookup_local_symbol (name, block, domain, language); + result = lookup_local_symbol (name, match_type, block, domain, language); if (result.symbol != NULL) { if (symbol_lookup_debug) @@ -2097,7 +2118,9 @@ lookup_symbol_aux (const char *name, const struct block *block, Don't search STATIC_BLOCK or GLOBAL_BLOCK. */ static struct block_symbol -lookup_local_symbol (const char *name, const struct block *block, +lookup_local_symbol (const char *name, + symbol_name_match_type match_type, + const struct block *block, const domain_enum domain, enum language language) { @@ -2112,7 +2135,7 @@ lookup_local_symbol (const char *name, const struct block *block, while (block != static_block) { - sym = lookup_symbol_in_block (name, block, domain); + sym = lookup_symbol_in_block (name, match_type, block, domain); if (sym != NULL) return (struct block_symbol) {sym, block}; @@ -2165,7 +2188,8 @@ lookup_objfile_from_block (const struct block *block) /* See symtab.h. */ struct symbol * -lookup_symbol_in_block (const char *name, const struct block *block, +lookup_symbol_in_block (const char *name, symbol_name_match_type match_type, + const struct block *block, const domain_enum domain) { struct symbol *sym; @@ -2181,7 +2205,7 @@ lookup_symbol_in_block (const char *name, const struct block *block, domain_name (domain)); } - sym = block_lookup_symbol (block, name, domain); + sym = block_lookup_symbol (block, name, match_type, domain); if (sym) { if (symbol_lookup_debug > 1) @@ -2370,7 +2394,8 @@ lookup_symbol_via_quick_fns (struct objfile *objfile, int block_index, bv = COMPUNIT_BLOCKVECTOR (cust); block = BLOCKVECTOR_BLOCK (bv, block_index); - result.symbol = block_lookup_symbol (block, name, domain); + result.symbol = block_lookup_symbol (block, name, + symbol_name_match_type::FULL, domain); if (result.symbol == NULL) error_in_psymtab_expansion (block_index, name, cust); @@ -2483,7 +2508,9 @@ lookup_symbol_in_static_block (const char *name, domain_name (domain)); } - sym = lookup_symbol_in_block (name, static_block, domain); + sym = lookup_symbol_in_block (name, + symbol_name_match_type::FULL, + static_block, domain); if (symbol_lookup_debug) { fprintf_unfiltered (gdb_stdlog, diff --git a/gdb/symtab.h b/gdb/symtab.h index b49d56ba2a8..a7b1ed2131c 100644 --- a/gdb/symtab.h +++ b/gdb/symtab.h @@ -60,6 +60,16 @@ enum class symbol_name_match_type namespace/module/package. */ FULL, + /* Search name matching. This is like FULL, but the search name did + not come from the user; instead it is already a search name + retrieved from a SYMBOL_SEARCH_NAME/MSYMBOL_SEARCH_NAME call. + For Ada, this avoids re-encoding an already-encoded search name + (which would potentially incorrectly lowercase letters in the + linkage/search name that should remain uppercase). For C++, it + avoids trying to demangle a name we already know is + demangled. */ + SEARCH_NAME, + /* Expression matching. The same as FULL matching in most languages. The same as WILD matching in Ada. */ EXPRESSION, @@ -1554,6 +1564,20 @@ extern struct block_symbol lookup_symbol (const char *, const domain_enum, struct field_of_this_result *); +/* Find the definition for a specified symbol search name in domain + DOMAIN, visible from lexical block BLOCK if non-NULL or from + global/static blocks if BLOCK is NULL. The passed-in search name + should not come from the user; instead it should already be a + search name as retrieved from a + SYMBOL_SEARCH_NAME/MSYMBOL_SEARCH_NAME call. See definition of + symbol_name_match_type::SEARCH_NAME. Returns the struct symbol + pointer, or NULL if no symbol is found. The symbol's section is + fixed up if necessary. */ + +extern struct block_symbol lookup_symbol_search_name (const char *search_name, + const struct block *block, + domain_enum domain); + /* A default version of lookup_symbol_nonlocal for use by languages that can't think of anything better to do. This implements the C lookup rules. */ @@ -1603,6 +1627,7 @@ extern struct block_symbol extern struct symbol * lookup_symbol_in_block (const char *name, + symbol_name_match_type match_type, const struct block *block, const domain_enum domain); diff --git a/gdb/testsuite/gdb.ada/access_tagged_param.exp b/gdb/testsuite/gdb.ada/access_tagged_param.exp new file mode 100644 index 00000000000..598cf89b4ec --- /dev/null +++ b/gdb/testsuite/gdb.ada/access_tagged_param.exp @@ -0,0 +1,40 @@ +# Copyright 2017-2018 Free Software Foundation, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +# Check that we can print values of parameters of type 'pointer +# (access) to tagged type'. See PR gdb/22670. + +load_lib "ada.exp" + +standard_ada_testfile foo + +if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug]] != "" } { + return -1 +} + +clean_restart ${testfile} + +if ![runto "foo"] then { + perror "Couldn't run ${testfile}" + return +} + +gdb_breakpoint "pck.inspect" + +# Continue until we reach the breakpoint, and verify that we can print +# the value of all the parameters. + +gdb_test "continue" \ + ".*Breakpoint $decimal, pck\\.inspect \\(obj=$hex, <objL>=2\\) at .*" diff --git a/gdb/testsuite/gdb.ada/access_tagged_param/foo.adb b/gdb/testsuite/gdb.ada/access_tagged_param/foo.adb new file mode 100644 index 00000000000..76d738c373c --- /dev/null +++ b/gdb/testsuite/gdb.ada/access_tagged_param/foo.adb @@ -0,0 +1,20 @@ +-- Copyright 2017-2018 Free Software Foundation, Inc. +-- +-- This program is free software; you can redistribute it and/or modify +-- it under the terms of the GNU General Public License as published by +-- the Free Software Foundation; either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU General Public License for more details. +-- +-- You should have received a copy of the GNU General Public License +-- along with this program. If not, see <http://www.gnu.org/licenses/>. + +with Pck; use Pck; +procedure Foo is +begin + Inspect (new Top_T'(N => 2)); +end Foo; diff --git a/gdb/testsuite/gdb.ada/access_tagged_param/pck.adb b/gdb/testsuite/gdb.ada/access_tagged_param/pck.adb new file mode 100644 index 00000000000..d156cee34b7 --- /dev/null +++ b/gdb/testsuite/gdb.ada/access_tagged_param/pck.adb @@ -0,0 +1,21 @@ +-- Copyright 2017-2018 Free Software Foundation, Inc. +-- +-- This program is free software; you can redistribute it and/or modify +-- it under the terms of the GNU General Public License as published by +-- the Free Software Foundation; either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU General Public License for more details. +-- +-- You should have received a copy of the GNU General Public License +-- along with this program. If not, see <http://www.gnu.org/licenses/>. + +package body Pck is + procedure Inspect (Obj: access Top_T'Class) is + begin + null; + end Inspect; +end Pck; diff --git a/gdb/testsuite/gdb.ada/access_tagged_param/pck.ads b/gdb/testsuite/gdb.ada/access_tagged_param/pck.ads new file mode 100644 index 00000000000..f1973d201aa --- /dev/null +++ b/gdb/testsuite/gdb.ada/access_tagged_param/pck.ads @@ -0,0 +1,21 @@ +-- Copyright 2017-2018 Free Software Foundation, Inc. +-- +-- This program is free software; you can redistribute it and/or modify +-- it under the terms of the GNU General Public License as published by +-- the Free Software Foundation; either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU General Public License for more details. +-- +-- You should have received a copy of the GNU General Public License +-- along with this program. If not, see <http://www.gnu.org/licenses/>. + +package Pck is + type Top_T is tagged record + N : Integer := 1; + end record; + procedure Inspect (Obj: access Top_T'Class); +end Pck; -- 2.14.3 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument 2018-01-05 16:28 ` Pedro Alves @ 2018-01-16 11:54 ` Pedro Alves 2018-01-17 9:13 ` Joel Brobecker 0 siblings, 1 reply; 27+ messages in thread From: Pedro Alves @ 2018-01-16 11:54 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches Hi Joel, On 01/05/2018 04:28 PM, Pedro Alves wrote: > > This also fixes the new '"maint check-psymtabs" for Ada' testcase for > me (gdb.ada/maint_with_ada.exp). I've not removed the kfail yet > because Joel still sees that testcase failing with this patch. > That'll be fixed in follow up patches. I'm curious to know if after all the fixing you still see the KFAIL on your end. I get: KPASS: gdb.ada/maint_with_ada.exp: maintenance check-psymtabs (PRMS gdb/22670) Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument 2018-01-16 11:54 ` Pedro Alves @ 2018-01-17 9:13 ` Joel Brobecker 2018-01-26 3:51 ` Joel Brobecker 2018-01-26 4:50 ` [RFC] regresssion(internal-error) printing subprogram argument Joel Brobecker 0 siblings, 2 replies; 27+ messages in thread From: Joel Brobecker @ 2018-01-17 9:13 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches Hi Pedro, > I'm curious to know if after all the fixing you still see > the KFAIL on your end. I get: > > KPASS: gdb.ada/maint_with_ada.exp: maintenance check-psymtabs (PRMS gdb/22670) Strange, I still get the error: (gdb) maintenance check-psymtabs Global symbol `interfaces__cS' only found in /[...]/gdb.ada/maint_with_ada/b~var_arr_typedef.adb psymtab (gdb) KFAIL: gdb.ada/maint_with_ada.exp: maintenance check-psymtabs (PRMS: gdb/22670) I'll set some time aside next week to look into this, and let you know what I find. -- Joel ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument 2018-01-17 9:13 ` Joel Brobecker @ 2018-01-26 3:51 ` Joel Brobecker 2018-01-26 11:28 ` Pedro Alves 2018-01-26 4:50 ` [RFC] regresssion(internal-error) printing subprogram argument Joel Brobecker 1 sibling, 1 reply; 27+ messages in thread From: Joel Brobecker @ 2018-01-26 3:51 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches Hi Pedro, > > I'm curious to know if after all the fixing you still see > > the KFAIL on your end. I get: > > > > KPASS: gdb.ada/maint_with_ada.exp: maintenance check-psymtabs (PRMS gdb/22670) I just started looking at this, and I am wondering whether your binder- generated file looks like, and in particular whether it has the following kind of declaration in b~var_arr_typedef.ads: u00045 : constant Version_32 := 16#f60287af#; pragma Export (C, u00045, "interfaces__cS"); IIRC, these are version consistency checks that are used by the distributed system annex. If you don't have those in this file, for some reason, then it would explain why you don't see the issue. Continuing the investigation... -- Joel ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument 2018-01-26 3:51 ` Joel Brobecker @ 2018-01-26 11:28 ` Pedro Alves 2018-01-29 10:38 ` Joel Brobecker 0 siblings, 1 reply; 27+ messages in thread From: Pedro Alves @ 2018-01-26 11:28 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches [-- Attachment #1: Type: text/plain, Size: 1347 bytes --] On 01/26/2018 03:50 AM, Joel Brobecker wrote: > Hi Pedro, > >>> I'm curious to know if after all the fixing you still see >>> the KFAIL on your end. I get: >>> >>> KPASS: gdb.ada/maint_with_ada.exp: maintenance check-psymtabs (PRMS gdb/22670) > > I just started looking at this, and I am wondering whether your binder- > generated file looks like, and in particular whether it has the > following kind of declaration in b~var_arr_typedef.ads: > > u00045 : constant Version_32 := 16#f60287af#; > pragma Export (C, u00045, "interfaces__cS"); > > IIRC, these are version consistency checks that are used by > the distributed system annex. If you don't have those in this file, > for some reason, then it would explain why you don't see the issue. Indeed, looks like I don't have that. I have "interfacesS". I've attached the whole file. Running: $ grep Export ./testsuite/outputs/gdb.ada/maint_with_ada/b~var_arr_typedef.ads | sed 's/.*, \"//' | sed 's/").*//g' | c++filt I notice that none of the Export symbols I have demangles as a C++ symbol, while "interfaces__cS" (the one you have) does: $ echo "interfaces__cS" | c++filt interfaces(char, signed) So it may be that we still need to add another special case for Ada somewhere. Would old GDB from before the C++ wildmatching pass this test for you? Thanks, Pedro Alves [-- Attachment #2: b~var_arr_typedef.ads --] [-- Type: text/x-adasrc, Size: 7862 bytes --] pragma Ada_95; pragma Warnings (Off); with System; package ada_main is gnat_argc : Integer; gnat_argv : System.Address; gnat_envp : System.Address; pragma Import (C, gnat_argc); pragma Import (C, gnat_argv); pragma Import (C, gnat_envp); gnat_exit_status : Integer; pragma Import (C, gnat_exit_status); GNAT_Version : constant String := "GNAT Version: 7.2.1 20170915 (Red Hat 7.2.1-2)" & ASCII.NUL; pragma Export (C, GNAT_Version, "__gnat_version"); Ada_Main_Program_Name : constant String := "_ada_var_arr_typedef" & ASCII.NUL; pragma Export (C, Ada_Main_Program_Name, "__gnat_ada_main_program_name"); procedure adainit; pragma Export (C, adainit, "adainit"); procedure adafinal; pragma Export (C, adafinal, "adafinal"); function main (argc : Integer; argv : System.Address; envp : System.Address) return Integer; pragma Export (C, main, "main"); type Version_32 is mod 2 ** 32; u00001 : constant Version_32 := 16#73e42c92#; pragma Export (C, u00001, "var_arr_typedefB"); u00002 : constant Version_32 := 16#b6df930e#; pragma Export (C, u00002, "system__standard_libraryB"); u00003 : constant Version_32 := 16#7ec093d3#; pragma Export (C, u00003, "system__standard_libraryS"); u00004 : constant Version_32 := 16#59841384#; pragma Export (C, u00004, "packB"); u00005 : constant Version_32 := 16#be226d3a#; pragma Export (C, u00005, "packS"); u00006 : constant Version_32 := 16#4635ec04#; pragma Export (C, u00006, "systemS"); u00007 : constant Version_32 := 16#a6359005#; pragma Export (C, u00007, "system__memoryB"); u00008 : constant Version_32 := 16#1f488a30#; pragma Export (C, u00008, "system__memoryS"); u00009 : constant Version_32 := 16#c2326fda#; pragma Export (C, u00009, "ada__exceptionsB"); u00010 : constant Version_32 := 16#6e98a13f#; pragma Export (C, u00010, "ada__exceptionsS"); u00011 : constant Version_32 := 16#76789da1#; pragma Export (C, u00011, "adaS"); u00012 : constant Version_32 := 16#e947e6a9#; pragma Export (C, u00012, "ada__exceptions__last_chance_handlerB"); u00013 : constant Version_32 := 16#41e5552e#; pragma Export (C, u00013, "ada__exceptions__last_chance_handlerS"); u00014 : constant Version_32 := 16#4e7785b8#; pragma Export (C, u00014, "system__soft_linksB"); u00015 : constant Version_32 := 16#d8b13451#; pragma Export (C, u00015, "system__soft_linksS"); u00016 : constant Version_32 := 16#b01dad17#; pragma Export (C, u00016, "system__parametersB"); u00017 : constant Version_32 := 16#381fe17b#; pragma Export (C, u00017, "system__parametersS"); u00018 : constant Version_32 := 16#30ad09e5#; pragma Export (C, u00018, "system__secondary_stackB"); u00019 : constant Version_32 := 16#fca7137e#; pragma Export (C, u00019, "system__secondary_stackS"); u00020 : constant Version_32 := 16#f103f468#; pragma Export (C, u00020, "system__storage_elementsB"); u00021 : constant Version_32 := 16#6bf6a600#; pragma Export (C, u00021, "system__storage_elementsS"); u00022 : constant Version_32 := 16#41837d1e#; pragma Export (C, u00022, "system__stack_checkingB"); u00023 : constant Version_32 := 16#c88a87ec#; pragma Export (C, u00023, "system__stack_checkingS"); u00024 : constant Version_32 := 16#87a448ff#; pragma Export (C, u00024, "system__exception_tableB"); u00025 : constant Version_32 := 16#1b9b8546#; pragma Export (C, u00025, "system__exception_tableS"); u00026 : constant Version_32 := 16#ce4af020#; pragma Export (C, u00026, "system__exceptionsB"); u00027 : constant Version_32 := 16#2e5681f2#; pragma Export (C, u00027, "system__exceptionsS"); u00028 : constant Version_32 := 16#843d48dc#; pragma Export (C, u00028, "system__exceptions__machineS"); u00029 : constant Version_32 := 16#aa0563fc#; pragma Export (C, u00029, "system__exceptions_debugB"); u00030 : constant Version_32 := 16#38bf15c0#; pragma Export (C, u00030, "system__exceptions_debugS"); u00031 : constant Version_32 := 16#6c2f8802#; pragma Export (C, u00031, "system__img_intB"); u00032 : constant Version_32 := 16#44ee0cc6#; pragma Export (C, u00032, "system__img_intS"); u00033 : constant Version_32 := 16#39df8c17#; pragma Export (C, u00033, "system__tracebackB"); u00034 : constant Version_32 := 16#181732c0#; pragma Export (C, u00034, "system__tracebackS"); u00035 : constant Version_32 := 16#9ed49525#; pragma Export (C, u00035, "system__traceback_entriesB"); u00036 : constant Version_32 := 16#466e1a74#; pragma Export (C, u00036, "system__traceback_entriesS"); u00037 : constant Version_32 := 16#6fd210f2#; pragma Export (C, u00037, "system__traceback__symbolicB"); u00038 : constant Version_32 := 16#dd19f67a#; pragma Export (C, u00038, "system__traceback__symbolicS"); u00039 : constant Version_32 := 16#701f9d88#; pragma Export (C, u00039, "ada__exceptions__tracebackB"); u00040 : constant Version_32 := 16#20245e75#; pragma Export (C, u00040, "ada__exceptions__tracebackS"); u00041 : constant Version_32 := 16#9f00b3d3#; pragma Export (C, u00041, "system__address_imageB"); u00042 : constant Version_32 := 16#e7d9713e#; pragma Export (C, u00042, "system__address_imageS"); u00043 : constant Version_32 := 16#8c33a517#; pragma Export (C, u00043, "system__wch_conB"); u00044 : constant Version_32 := 16#5d48ced6#; pragma Export (C, u00044, "system__wch_conS"); u00045 : constant Version_32 := 16#9721e840#; pragma Export (C, u00045, "system__wch_stwB"); u00046 : constant Version_32 := 16#7059e2d7#; pragma Export (C, u00046, "system__wch_stwS"); u00047 : constant Version_32 := 16#a831679c#; pragma Export (C, u00047, "system__wch_cnvB"); u00048 : constant Version_32 := 16#52ff7425#; pragma Export (C, u00048, "system__wch_cnvS"); u00049 : constant Version_32 := 16#5ab55268#; pragma Export (C, u00049, "interfacesS"); u00050 : constant Version_32 := 16#ece6fdb6#; pragma Export (C, u00050, "system__wch_jisB"); u00051 : constant Version_32 := 16#d28f6d04#; pragma Export (C, u00051, "system__wch_jisS"); u00052 : constant Version_32 := 16#36a43a0a#; pragma Export (C, u00052, "system__crtlS"); -- BEGIN ELABORATION ORDER -- ada%s -- interfaces%s -- system%s -- system.img_int%s -- system.img_int%b -- system.parameters%s -- system.parameters%b -- system.crtl%s -- system.storage_elements%s -- system.storage_elements%b -- system.stack_checking%s -- system.stack_checking%b -- system.traceback_entries%s -- system.traceback_entries%b -- system.wch_con%s -- system.wch_con%b -- system.wch_jis%s -- system.wch_jis%b -- system.wch_cnv%s -- system.wch_cnv%b -- system.traceback%s -- system.traceback%b -- system.wch_stw%s -- system.standard_library%s -- system.exceptions_debug%s -- system.exceptions_debug%b -- ada.exceptions%s -- system.wch_stw%b -- ada.exceptions.traceback%s -- system.soft_links%s -- system.exception_table%s -- system.exception_table%b -- system.exceptions%s -- system.exceptions%b -- system.secondary_stack%s -- system.address_image%s -- system.soft_links%b -- ada.exceptions.last_chance_handler%s -- system.memory%s -- system.memory%b -- system.standard_library%b -- ada.exceptions.traceback%b -- system.traceback.symbolic%s -- system.traceback.symbolic%b -- system.exceptions.machine%s -- system.secondary_stack%b -- system.address_image%b -- ada.exceptions.last_chance_handler%b -- ada.exceptions%b -- pack%s -- pack%b -- var_arr_typedef%b -- END ELABORATION ORDER end ada_main; ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument 2018-01-26 11:28 ` Pedro Alves @ 2018-01-29 10:38 ` Joel Brobecker 2018-01-29 15:33 ` Pedro Alves 0 siblings, 1 reply; 27+ messages in thread From: Joel Brobecker @ 2018-01-29 10:38 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches [-- Attachment #1: Type: text/plain, Size: 1369 bytes --] > Indeed, looks like I don't have that. I have "interfacesS". > I've attached the whole file. That explains it :). > So it may be that we still need to add another special case > for Ada somewhere. Would old GDB from before the C++ > wildmatching pass this test for you? I went binary searching for the source of the regression and when I found that it was "caused" by the change requiring variables without debugging information to be cast before they can be printed, I gently head-slapped myself, adjusted the testcase to use something other than an integer variable, and voila - even GDB 7.10 suffers from the problem. We just didn't see it for integer variables simply because we were lucky! I ran out of time again today, but at least the WIP patch got augmented with a testcase that currently fails before the patch is applied. I think the patch itself is probably correct, although I'd like to do some archeology to understand the comment attached to that location. I'm pretty confused by it, when we could simply say that symbols from languages that do not follow the C++ mangling should not be demangled by gdb_demangle -- at least for as long as gdb_demangle is equivalent to cplusplus_demangle in practice... And just as a reminder for myself, I said in my other email last week that I wanted also to review all the calls to gdb_demangle. -- Joel [-- Attachment #2: 0001-WIP-dwarf2read.c-dwarf2_physname-do-not-call-gdb_dem.patch --] [-- Type: text/x-diff, Size: 7678 bytes --] From 6f24c49e7c3ba2c6246aa3de92ef48c1315778f8 Mon Sep 17 00:00:00 2001 From: Joel Brobecker <brobecker@adacore.com> Date: Thu, 25 Jan 2018 23:23:54 -0500 Subject: [PATCH] WIP: dwarf2read.c::dwarf2_physname: do not call gdb_demangle with Ada symbols --- gdb/dwarf2read.c | 2 +- gdb/testsuite/gdb.ada/notcplusplus.exp | 45 ++++++++++++++++++++++++++++++ gdb/testsuite/gdb.ada/notcplusplus/foo.adb | 21 ++++++++++++++ gdb/testsuite/gdb.ada/notcplusplus/pck.adb | 21 ++++++++++++++ gdb/testsuite/gdb.ada/notcplusplus/pck.ads | 19 +++++++++++++ gdb/testsuite/gdb.ada/notcplusplus/ver.ads | 22 +++++++++++++++ 6 files changed, 129 insertions(+), 1 deletion(-) create mode 100644 gdb/testsuite/gdb.ada/notcplusplus.exp create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/foo.adb create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/pck.adb create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/pck.ads create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/ver.ads diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index 51d0f39..7febade 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -11150,7 +11150,7 @@ dwarf2_physname (const char *name, struct die_info *die, struct dwarf2_cu *cu) variant `long name(params)' does not have the proper inferior type. */ - if (cu->language == language_go) + if (cu->language == language_go || cu->language == language_ada) { /* This is a lie, but we already lie to the caller new_symbol. new_symbol assumes we return the mangled name. diff --git a/gdb/testsuite/gdb.ada/notcplusplus.exp b/gdb/testsuite/gdb.ada/notcplusplus.exp new file mode 100644 index 0000000..b2a24e8 --- /dev/null +++ b/gdb/testsuite/gdb.ada/notcplusplus.exp @@ -0,0 +1,45 @@ +# Copyright 2018 Free Software Foundation, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +load_lib "ada.exp" + +if { [skip_ada_tests] } { return -1 } + +standard_ada_testfile foo + +if {[gdb_compile_ada "${srcfile}" "${binfile}" executable {debug}] != ""} { + return -1 +} + +clean_restart ${testfile} + +gdb_test "print /x <symada__cS>" \ + "= \\(a => 0x60287af\\)" \ + "print <symada__cS> before loading symbols from ver.ads" + +# Force the partial symbosl from ver.ads to be expanded into full symbols. + +gdb_test \ + "list ver.ads:16" \ + [multi_line ".*" \ + "16\\s+package Ver is" \ + "17\\s+type Wrapper is record" \ + "18\\s+A : Integer;" \ + "19\\s+end record;" \ + "20\\s+u00045 : constant Wrapper := \\(A => 16#060287af#\\);"] + +gdb_test "print /x <symada__cS>" \ + "= \\(a => 0x60287af\\)" \ + "print <symada__cS> after loading symbols from ver.ads" diff --git a/gdb/testsuite/gdb.ada/notcplusplus/foo.adb b/gdb/testsuite/gdb.ada/notcplusplus/foo.adb new file mode 100644 index 0000000..89e42f9 --- /dev/null +++ b/gdb/testsuite/gdb.ada/notcplusplus/foo.adb @@ -0,0 +1,21 @@ +-- Copyright 2018 Free Software Foundation, Inc. +-- +-- This program is free software; you can redistribute it and/or modify +-- it under the terms of the GNU General Public License as published by +-- the Free Software Foundation; either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU General Public License for more details. +-- +-- You should have received a copy of the GNU General Public License +-- along with this program. If not, see <http://www.gnu.org/licenses/>. + +with Pck; use Pck; +with Ver; use Ver; +procedure Foo is +begin + Do_Nothing (u00045'Address); +end Foo; diff --git a/gdb/testsuite/gdb.ada/notcplusplus/pck.adb b/gdb/testsuite/gdb.ada/notcplusplus/pck.adb new file mode 100644 index 0000000..dcfb306 --- /dev/null +++ b/gdb/testsuite/gdb.ada/notcplusplus/pck.adb @@ -0,0 +1,21 @@ +-- Copyright 2018 Free Software Foundation, Inc. +-- +-- This program is free software; you can redistribute it and/or modify +-- it under the terms of the GNU General Public License as published by +-- the Free Software Foundation; either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU General Public License for more details. +-- +-- You should have received a copy of the GNU General Public License +-- along with this program. If not, see <http://www.gnu.org/licenses/>. + +package body Pck is + procedure Do_Nothing (A : System.Address) is + begin + null; + end Do_Nothing; +end Pck; diff --git a/gdb/testsuite/gdb.ada/notcplusplus/pck.ads b/gdb/testsuite/gdb.ada/notcplusplus/pck.ads new file mode 100644 index 0000000..33e369e --- /dev/null +++ b/gdb/testsuite/gdb.ada/notcplusplus/pck.ads @@ -0,0 +1,19 @@ +-- Copyright 2018 Free Software Foundation, Inc. +-- +-- This program is free software; you can redistribute it and/or modify +-- it under the terms of the GNU General Public License as published by +-- the Free Software Foundation; either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU General Public License for more details. +-- +-- You should have received a copy of the GNU General Public License +-- along with this program. If not, see <http://www.gnu.org/licenses/>. + +with System; +package Pck is + procedure Do_Nothing (A : System.Address); +end Pck; diff --git a/gdb/testsuite/gdb.ada/notcplusplus/ver.ads b/gdb/testsuite/gdb.ada/notcplusplus/ver.ads new file mode 100644 index 0000000..8f264d0 --- /dev/null +++ b/gdb/testsuite/gdb.ada/notcplusplus/ver.ads @@ -0,0 +1,22 @@ +-- Copyright 2018 Free Software Foundation, Inc. +-- +-- This program is free software; you can redistribute it and/or modify +-- it under the terms of the GNU General Public License as published by +-- the Free Software Foundation; either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU General Public License for more details. +-- +-- You should have received a copy of the GNU General Public License +-- along with this program. If not, see <http://www.gnu.org/licenses/>. + +package Ver is + type Wrapper is record + A : Integer; + end record; + u00045 : constant Wrapper := (A => 16#060287af#); + pragma Export (C, u00045, "symada__cS"); +end Ver; -- 2.1.4 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument 2018-01-29 10:38 ` Joel Brobecker @ 2018-01-29 15:33 ` Pedro Alves 2018-01-29 15:52 ` Joel Brobecker 2018-01-30 3:56 ` Joel Brobecker 0 siblings, 2 replies; 27+ messages in thread From: Pedro Alves @ 2018-01-29 15:33 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches On 01/29/2018 10:38 AM, Joel Brobecker wrote: >> Indeed, looks like I don't have that. I have "interfacesS". >> I've attached the whole file. > > That explains it :). > >> So it may be that we still need to add another special case >> for Ada somewhere. Would old GDB from before the C++ >> wildmatching pass this test for you? > > I went binary searching for the source of the regression and > when I found that it was "caused" by the change requiring variables > without debugging information to be cast before they can be printed, > I gently head-slapped myself, adjusted the testcase to use something > other than an integer variable, and voila - even GDB 7.10 suffers > from the problem. We just didn't see it for integer variables simply > because we were lucky! > > I ran out of time again today, but at least the WIP patch got > augmented with a testcase that currently fails before the patch > is applied. > > I think the patch itself is probably correct, although I'd like > to do some archeology to understand the comment attached to > that location. I'm pretty confused by it, when we could simply > say that symbols from languages that do not follow the C++ mangling > should not be demangled by gdb_demangle -- at least for as long > as gdb_demangle is equivalent to cplusplus_demangle in practice... Yeah, the commentary around that code isn't exactly clear. Why doesn't that code use language->la_demangle instead, for example. The other day I noticed that gdb_demangle -> bfd_demangle -> cplus_demangle does have support for demangling other languages. For Ada, see the GNAT_DEMANGLING handling in libiberty.c:cplus-dem.c:cplus_demangle, which takes you to: /* Demangle ada names. The encoding is documented in gcc/ada/exp_dbug.ads. */ char * ada_demangle (const char *mangled, int option ATTRIBUTE_UNUSED) { When I saw that, I wondered why is it that GDB has a separate implementation of Ada decoding in gdb/ada-lang.c. The only plausible explanation I came up with is that gdb's version decodes into a buffer that is shared between invocations while libiberty's version always heap-allocates the result. Maybe it was an efficiency thing? Do you know? I ran into that around this discussion <https://sourceware.org/ml/gdb/2017-11/msg00029.html> where I was wondering whether we could speed up demangling by letting bfd_demangle / cplus_demangle try all languages and return back which one worked: ~~~ An idea I had would be to try to combine most the language_sniff_from_mangled_name implementations in one call, by deferring to cplus_demangle which seems like could take care of gnu v3, rust, java, gnat, dlang and old C++ mangling schemes all at once. This would avoid the overhead of gdb_demangle and bfd_demangle for each language-specific demangling call, at least. Not sure. ~~~ Because ada_decode comes up high in profiles today... > > And just as a reminder for myself, I said in my other email > last week that I wanted also to review all the calls to gdb_demangle. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument 2018-01-29 15:33 ` Pedro Alves @ 2018-01-29 15:52 ` Joel Brobecker 2018-01-30 3:56 ` Joel Brobecker 1 sibling, 0 replies; 27+ messages in thread From: Joel Brobecker @ 2018-01-29 15:52 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches [-- Attachment #1: Type: text/plain, Size: 269 bytes --] Hi Pedro, It's the end of my day, now, and I haven't had a chance to look at your comments just yet. I just wanted to post version 3 of the WIP patch, because it also includes a testcase in C. A bit on the artificial side, but I thought could be useful too. -- Joel [-- Attachment #2: 0001-WIPv3-dwarf2read.c-dwarf2_physname-do-not-call-gdb_d.patch --] [-- Type: text/x-diff, Size: 11530 bytes --] From 98f782214fcd6916f9163936b1fcf308edb8c522 Mon Sep 17 00:00:00 2001 From: Joel Brobecker <brobecker@adacore.com> Date: Thu, 25 Jan 2018 23:23:54 -0500 Subject: [PATCH] WIPv3: dwarf2read.c::dwarf2_physname: do not call gdb_demangle with Ada symbols --- gdb/dwarf2read.c | 2 +- gdb/testsuite/gdb.ada/notcplusplus.exp | 45 ++++++++++++++++++++++++++++ gdb/testsuite/gdb.ada/notcplusplus/foo.adb | 21 +++++++++++++ gdb/testsuite/gdb.ada/notcplusplus/pck.adb | 21 +++++++++++++ gdb/testsuite/gdb.ada/notcplusplus/pck.ads | 19 ++++++++++++ gdb/testsuite/gdb.ada/notcplusplus/ver.ads | 22 ++++++++++++++ gdb/testsuite/gdb.base/c-linkage-name.c | 44 ++++++++++++++++++++++++++++ gdb/testsuite/gdb.base/c-linkage-name.exp | 47 ++++++++++++++++++++++++++++++ 8 files changed, 220 insertions(+), 1 deletion(-) create mode 100644 gdb/testsuite/gdb.ada/notcplusplus.exp create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/foo.adb create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/pck.adb create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/pck.ads create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/ver.ads create mode 100644 gdb/testsuite/gdb.base/c-linkage-name.c create mode 100644 gdb/testsuite/gdb.base/c-linkage-name.exp diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index 51d0f39..7febade 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -11150,7 +11150,7 @@ dwarf2_physname (const char *name, struct die_info *die, struct dwarf2_cu *cu) variant `long name(params)' does not have the proper inferior type. */ - if (cu->language == language_go) + if (cu->language == language_go || cu->language == language_ada) { /* This is a lie, but we already lie to the caller new_symbol. new_symbol assumes we return the mangled name. diff --git a/gdb/testsuite/gdb.ada/notcplusplus.exp b/gdb/testsuite/gdb.ada/notcplusplus.exp new file mode 100644 index 0000000..b2a24e8 --- /dev/null +++ b/gdb/testsuite/gdb.ada/notcplusplus.exp @@ -0,0 +1,45 @@ +# Copyright 2018 Free Software Foundation, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +load_lib "ada.exp" + +if { [skip_ada_tests] } { return -1 } + +standard_ada_testfile foo + +if {[gdb_compile_ada "${srcfile}" "${binfile}" executable {debug}] != ""} { + return -1 +} + +clean_restart ${testfile} + +gdb_test "print /x <symada__cS>" \ + "= \\(a => 0x60287af\\)" \ + "print <symada__cS> before loading symbols from ver.ads" + +# Force the partial symbosl from ver.ads to be expanded into full symbols. + +gdb_test \ + "list ver.ads:16" \ + [multi_line ".*" \ + "16\\s+package Ver is" \ + "17\\s+type Wrapper is record" \ + "18\\s+A : Integer;" \ + "19\\s+end record;" \ + "20\\s+u00045 : constant Wrapper := \\(A => 16#060287af#\\);"] + +gdb_test "print /x <symada__cS>" \ + "= \\(a => 0x60287af\\)" \ + "print <symada__cS> after loading symbols from ver.ads" diff --git a/gdb/testsuite/gdb.ada/notcplusplus/foo.adb b/gdb/testsuite/gdb.ada/notcplusplus/foo.adb new file mode 100644 index 0000000..89e42f9 --- /dev/null +++ b/gdb/testsuite/gdb.ada/notcplusplus/foo.adb @@ -0,0 +1,21 @@ +-- Copyright 2018 Free Software Foundation, Inc. +-- +-- This program is free software; you can redistribute it and/or modify +-- it under the terms of the GNU General Public License as published by +-- the Free Software Foundation; either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU General Public License for more details. +-- +-- You should have received a copy of the GNU General Public License +-- along with this program. If not, see <http://www.gnu.org/licenses/>. + +with Pck; use Pck; +with Ver; use Ver; +procedure Foo is +begin + Do_Nothing (u00045'Address); +end Foo; diff --git a/gdb/testsuite/gdb.ada/notcplusplus/pck.adb b/gdb/testsuite/gdb.ada/notcplusplus/pck.adb new file mode 100644 index 0000000..dcfb306 --- /dev/null +++ b/gdb/testsuite/gdb.ada/notcplusplus/pck.adb @@ -0,0 +1,21 @@ +-- Copyright 2018 Free Software Foundation, Inc. +-- +-- This program is free software; you can redistribute it and/or modify +-- it under the terms of the GNU General Public License as published by +-- the Free Software Foundation; either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU General Public License for more details. +-- +-- You should have received a copy of the GNU General Public License +-- along with this program. If not, see <http://www.gnu.org/licenses/>. + +package body Pck is + procedure Do_Nothing (A : System.Address) is + begin + null; + end Do_Nothing; +end Pck; diff --git a/gdb/testsuite/gdb.ada/notcplusplus/pck.ads b/gdb/testsuite/gdb.ada/notcplusplus/pck.ads new file mode 100644 index 0000000..33e369e --- /dev/null +++ b/gdb/testsuite/gdb.ada/notcplusplus/pck.ads @@ -0,0 +1,19 @@ +-- Copyright 2018 Free Software Foundation, Inc. +-- +-- This program is free software; you can redistribute it and/or modify +-- it under the terms of the GNU General Public License as published by +-- the Free Software Foundation; either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU General Public License for more details. +-- +-- You should have received a copy of the GNU General Public License +-- along with this program. If not, see <http://www.gnu.org/licenses/>. + +with System; +package Pck is + procedure Do_Nothing (A : System.Address); +end Pck; diff --git a/gdb/testsuite/gdb.ada/notcplusplus/ver.ads b/gdb/testsuite/gdb.ada/notcplusplus/ver.ads new file mode 100644 index 0000000..8f264d0 --- /dev/null +++ b/gdb/testsuite/gdb.ada/notcplusplus/ver.ads @@ -0,0 +1,22 @@ +-- Copyright 2018 Free Software Foundation, Inc. +-- +-- This program is free software; you can redistribute it and/or modify +-- it under the terms of the GNU General Public License as published by +-- the Free Software Foundation; either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU General Public License for more details. +-- +-- You should have received a copy of the GNU General Public License +-- along with this program. If not, see <http://www.gnu.org/licenses/>. + +package Ver is + type Wrapper is record + A : Integer; + end record; + u00045 : constant Wrapper := (A => 16#060287af#); + pragma Export (C, u00045, "symada__cS"); +end Ver; diff --git a/gdb/testsuite/gdb.base/c-linkage-name.c b/gdb/testsuite/gdb.base/c-linkage-name.c new file mode 100644 index 0000000..925004c --- /dev/null +++ b/gdb/testsuite/gdb.base/c-linkage-name.c @@ -0,0 +1,44 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2018 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +struct wrapper +{ + int a; +}; + +/* Create a global variable whose name in the assembly code + (aka the "linkage name") is different from the name in + the source code. The goal is to create a symbol described + in DWARF using a DW_AT_linkage_name attribute, with a name + which follows the C++ mangling. + + In this particular case, we chose "symada__cS" which, if it were + demangled, would translate to "symada (char, signed)". */ +struct wrapper mundane asm ("symada__cS") = {0x060287af}; + +void +do_something (struct wrapper *w) +{ + w->a++; +} + +int +main (void) +{ + do_something (&mundane); + return 0; +} diff --git a/gdb/testsuite/gdb.base/c-linkage-name.exp b/gdb/testsuite/gdb.base/c-linkage-name.exp new file mode 100644 index 0000000..c80a530 --- /dev/null +++ b/gdb/testsuite/gdb.base/c-linkage-name.exp @@ -0,0 +1,47 @@ +# Copyright 2018 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +# This file is part of the gdb testsuite. It is intended to test that +# gdb can correctly print arrays with indexes for each element of the +# array. + +standard_testfile .c + +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } { + untested "failed to compile" + return -1 +} + +clean_restart ${binfile} + +# Try to print MUNDANE, but using its linkage name. + +gdb_test "print symada__cS" \ + " = {a = 100829103}" \ + "print symada__cS before partial symtab expansion" + +# Force the symbols to be expanded for the unit that contains +# our symada__cS symbol by, e.g. inserting a breakpoint on one +# of the founction also provided by the same using. + +gdb_test "break main" \ + "Breakpoint $decimal at $hex: file .*$srcfile, line $decimal\\." + +# Try to print MUNDANE using its linkage name again, after partial +# symtab expansion. + +gdb_test "print symada__cS" \ + " = {a = 100829103}" \ + "print symada__cS after partial symtab expansion" -- 2.1.4 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument 2018-01-29 15:33 ` Pedro Alves 2018-01-29 15:52 ` Joel Brobecker @ 2018-01-30 3:56 ` Joel Brobecker 2018-02-05 9:57 ` Joel Brobecker 1 sibling, 1 reply; 27+ messages in thread From: Joel Brobecker @ 2018-01-30 3:56 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches > The other day I noticed that gdb_demangle -> bfd_demangle -> > cplus_demangle does have support for demangling other languages. > For Ada, see the GNAT_DEMANGLING handling in > libiberty.c:cplus-dem.c:cplus_demangle, which takes you to: > > /* Demangle ada names. The encoding is documented in gcc/ada/exp_dbug.ads. */ > > char * > ada_demangle (const char *mangled, int option ATTRIBUTE_UNUSED) > { Ha! Thanks for digging; who would have thought... > When I saw that, I wondered why is it that GDB has a separate > implementation of Ada decoding in gdb/ada-lang.c. The only plausible > explanation I came up with is that gdb's version decodes into a > buffer that is shared between invocations while libiberty's version > always heap-allocates the result. Maybe it was an efficiency thing? > Do you know? I don't unfortunately. I'm pretty sure GDB's ada_decode was already present before I joined AdaCore... It is true that start up performance is critical, particularly in Ada, because one of the strengths of Ada is to help design very large apps. I think that, eventually, we're going to have to accept that computing power is gained by adding parallelism, and try to take advantage of that to speed things up (with the dangers in terms of stability that it entails), so that aspect might become less relevant once we make that transition. > I ran into that around this discussion > <https://sourceware.org/ml/gdb/2017-11/msg00029.html> where I > was wondering whether we could speed up demangling by letting > bfd_demangle / cplus_demangle try all languages and return back > which one worked: > > ~~~ > An idea I had would be to try to combine most the language_sniff_from_mangled_name > implementations in one call, by deferring to cplus_demangle which seems > like could take care of gnu v3, rust, java, gnat, dlang and old C++ mangling > schemes all at once. This would avoid the overhead of gdb_demangle and > bfd_demangle for each language-specific demangling call, at least. Not sure. > ~~~ > > Because ada_decode comes up high in profiles today... Can we avoid the calls to most demanglers entirely by passing the language to symbol_set_names? I didn't do an extensive search of all the callers. We could have a fallback where language_unknown means try all the demanglers, but for the cases we care about, which is mostly DWARF debugging info, that could save a lot of unnecessary attempts, like you say. My feeling on the global situation with those demanglers is that we take a risk each time we use the demanglers themselves to try to determine which language the symbol belongs to. Both the C++ and the Ada mangling/encoding algorithms are not exclusive enough, and we're seeing in this thread how easy it is to get it wrong. For minimal symbols, we don't have the information, so that's one valid situation where we have no choice but to guess. But when reading DWARF debugging information, for instance, we do have the language information, so we should take advantage of it. Thinking out loud, this leads me to wonder whether it's a good idea to store the demangled name instead of the linkage name. You'd do the search on the mangled name rather than on the demangled one, so you would not have to guess when loading the symbols. Maybe it's a good idea if it helps performance, and perhaps we can plug that weakness differently... -- Joel ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument 2018-01-30 3:56 ` Joel Brobecker @ 2018-02-05 9:57 ` Joel Brobecker 2018-02-09 9:09 ` [RFA/RFC] fix PR gdb/22670 (pb looking up some symbols when they have a linkage name) (was: "Re: [RFC] regresssion(internal-error) printing subprogram argument") Joel Brobecker 0 siblings, 1 reply; 27+ messages in thread From: Joel Brobecker @ 2018-02-05 9:57 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches [-- Attachment #1: Type: text/plain, Size: 5228 bytes --] Here is the latest update for this thread. I'm pretty much ready to work on an official patch, but I'd like some guidance. A quick summary: We noticed that, with Ada programs, "maintainance check-psymtabs" reports consistency check error for symbols such as "interfaces__cS". This happens for symbols whose linkage name resembles a C++ mangled name. In that situation, dwarf2physname finds the linkage name, and then processes it through gdb_demangle, regardless of the unit's language. As a result, we end up storing the symbol linkage name using the demangled name, which makes no sense for Ada, and thus triggers the consistency check failure. The fix for the various testcases (one in C, one in the existing Ada testcase) is to patch dwarf2_physname to avoid calling gdb_demangle for languages where it doesn't make sense. The big question is: which languages should we exclude? For now, I've decided to only touch languages where I am sure: Ada, of course, but also C, Asm, and "minimal". There was the question of Go, but I'm not sure what go does in terms of mangling. If you remember, symbols from Go units do not process symbols' linkage name through gdb_demangle, but for reasons that are unclear. Here is the comment: if (cu->language == language_go) { /* This is a lie, but we already lie to the caller new_symbol. new_symbol assumes we return the mangled name. This just undoes that lie until things are cleaned up. */ I went looking for the origin of this comment, and unfortunately, it was introduced as part of the large patch that introduced GO support (circa Apr 2012). Not much information there. To play it safe, I decided to fix dwarf2_physname independently of go, so that go's comment doesn't bleed into this fix. I also spent some time investigating all the calls to gdb_demangle. The vast majority are obviously in a situation where we're dealing with C++ types, or at least language-specific types where the language is known. So the assumption is that we know what we're doing and the call is OK. As a result, there were only the 3 instances in dwarf2read.c. One of them is the dwarf2_physname function, which is fixed here. The other two are: (a) fixup_partial_die: | /* GCC might emit a nameless struct or union that has a linkage | name. See http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47510. */ | if (part_die->name == NULL | && (part_die->tag == DW_TAG_class_type | || part_die->tag == DW_TAG_interface_type | || part_die->tag == DW_TAG_structure_type | || part_die->tag == DW_TAG_union_type) | && part_die->linkage_name != NULL) (b) dwarf2_name: | case DW_TAG_class_type: | case DW_TAG_interface_type: | case DW_TAG_structure_type: | case DW_TAG_union_type: | [...] | /* GCC might emit a nameless typedef that has a linkage name. See | http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47510. */ | if (!attr || DW_STRING (attr) == NULL) Theoretically, we might have an issue where we might be calling gdb_demangle improperly for those types, and we could try to fix that. However, the fix in those two situations is not clearly obvious. And to top it all off, the equivalent situation might not exist outside of certain versions of C++. So I would like to suggest we leave those areas alone for now. In terms of the fix, we have several alternatives. I discarded the idea of hard-coding the list of languages we want to exclude in dwarf2_physname as we might need that list elsewhere. We have the option of a function (like in the attached patch), probably to be moved to symtab.h/symtab.c. I chose a name that spoke to me, but I'm completely familiar with the new lookup system entirely yet, so better name suggestions are welcome. For instance, instead of talking of symbol name storage format, we could have something like... symbol_lookup_uses_demanged_name_p (enum language lang); All in all, I think a better solution would be to put that information directly in the language_defn itself, via a new "attribute". Something like a... /* True if symbol search names should be stored in demangled format. False otherwise. */ const bool symbol_stored_in_demangle_form_p; Then, I'd set it to "true" for all languages, except the languages we selected (Ada, C, Asm, minimal). I didn't implement that just yet, as this requires a larger number of files being modified, so I thought I'd seek opinions before going ahead. My vote: a new language_defn attribute. Thoughts? gdb/ChangeLog: PR gdb/22670: * dwarf2read.c (symbols_stored_in_demangled_form_p): New function. (dwarf2_physname): Do not call gdb_demangle if symbols_stored_in_demangled_form_p returns false for the cu's language. gdb/testsuite/ChangeLog: * gdb.ada/notcplusplus: New testcase. * gdb.ada/maint_with_ada.exp: Remove setup_kfail. * gdb.base/c-linkage-name.c: New file. * gdb.base/c-linkage-name.exp: New testcase. Tested on x86_64-linux, no regression. Thank you! -- Joel [-- Attachment #2: 0001-WIPv4-dwarf2read.c-dwarf2_physname-conditionalize-ca.patch --] [-- Type: text/x-diff, Size: 13139 bytes --] From 4b3af7739001893595ec2b81b5cfb504b0b58859 Mon Sep 17 00:00:00 2001 From: Joel Brobecker <brobecker@adacore.com> Date: Thu, 25 Jan 2018 23:23:54 -0500 Subject: [PATCH] WIPv4: dwarf2read.c::dwarf2_physname: conditionalize call to gdb_demangle This patch avoids the call to gdb_demangle of the DW_AT_linkage_name for languages where either: demangling does not make sense, or where the language implementation chose to store symbol names in mangled form. --- gdb/dwarf2read.c | 26 ++++++++++++++++- gdb/testsuite/gdb.ada/maint_with_ada.exp | 1 - gdb/testsuite/gdb.ada/notcplusplus.exp | 45 ++++++++++++++++++++++++++++ gdb/testsuite/gdb.ada/notcplusplus/foo.adb | 21 +++++++++++++ gdb/testsuite/gdb.ada/notcplusplus/pck.adb | 21 +++++++++++++ gdb/testsuite/gdb.ada/notcplusplus/pck.ads | 19 ++++++++++++ gdb/testsuite/gdb.ada/notcplusplus/ver.ads | 22 ++++++++++++++ gdb/testsuite/gdb.base/c-linkage-name.c | 44 ++++++++++++++++++++++++++++ gdb/testsuite/gdb.base/c-linkage-name.exp | 47 ++++++++++++++++++++++++++++++ 9 files changed, 244 insertions(+), 2 deletions(-) create mode 100644 gdb/testsuite/gdb.ada/notcplusplus.exp create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/foo.adb create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/pck.adb create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/pck.ads create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/ver.ads create mode 100644 gdb/testsuite/gdb.base/c-linkage-name.c create mode 100644 gdb/testsuite/gdb.base/c-linkage-name.exp diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index d651725..91e7862 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -11109,6 +11109,26 @@ dwarf2_full_name (const char *name, struct die_info *die, struct dwarf2_cu *cu) return dwarf2_compute_name (name, die, cu, 0); } +static bool +symbols_stored_in_demangled_form_p (enum language lang) +{ + if (lang == language_asm + || lang == language_minimal + || lang == language_c) + { + /* No mangling for those languages. */ + return false; + } + if (lang == language_ada) + { + /* For Ada, we store the symbol names in encoded form + and then perform the lookups using that form. */ + return false; + } + + return true; +} + /* Construct a physname for the given DIE in CU. NAME may either be from a previous call to dwarf2_name or NULL. The result will be allocated on the objfile_objstack or NULL if the DIE does not have a @@ -11142,7 +11162,11 @@ dwarf2_physname (const char *name, struct die_info *die, struct dwarf2_cu *cu) if (mangled != NULL) { - if (cu->language == language_go) + if (! symbols_stored_in_demangled_form_p (cu->language)) + { + /* Do nothing (do not demangle the symbol name). */ + } + else if (cu->language == language_go) { /* This is a lie, but we already lie to the caller new_symbol. new_symbol assumes we return the mangled name. diff --git a/gdb/testsuite/gdb.ada/maint_with_ada.exp b/gdb/testsuite/gdb.ada/maint_with_ada.exp index 73da613..9ede035 100644 --- a/gdb/testsuite/gdb.ada/maint_with_ada.exp +++ b/gdb/testsuite/gdb.ada/maint_with_ada.exp @@ -32,7 +32,6 @@ gdb_breakpoint "adainit" gdb_breakpoint "Var_Arr_Typedef" gdb_breakpoint "Do_Nothing" -setup_kfail gdb/22670 "*-*-*" gdb_test_no_output "maintenance check-psymtabs" gdb_test_no_output "maintenance check-symtabs" diff --git a/gdb/testsuite/gdb.ada/notcplusplus.exp b/gdb/testsuite/gdb.ada/notcplusplus.exp new file mode 100644 index 0000000..b2a24e8 --- /dev/null +++ b/gdb/testsuite/gdb.ada/notcplusplus.exp @@ -0,0 +1,45 @@ +# Copyright 2018 Free Software Foundation, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +load_lib "ada.exp" + +if { [skip_ada_tests] } { return -1 } + +standard_ada_testfile foo + +if {[gdb_compile_ada "${srcfile}" "${binfile}" executable {debug}] != ""} { + return -1 +} + +clean_restart ${testfile} + +gdb_test "print /x <symada__cS>" \ + "= \\(a => 0x60287af\\)" \ + "print <symada__cS> before loading symbols from ver.ads" + +# Force the partial symbosl from ver.ads to be expanded into full symbols. + +gdb_test \ + "list ver.ads:16" \ + [multi_line ".*" \ + "16\\s+package Ver is" \ + "17\\s+type Wrapper is record" \ + "18\\s+A : Integer;" \ + "19\\s+end record;" \ + "20\\s+u00045 : constant Wrapper := \\(A => 16#060287af#\\);"] + +gdb_test "print /x <symada__cS>" \ + "= \\(a => 0x60287af\\)" \ + "print <symada__cS> after loading symbols from ver.ads" diff --git a/gdb/testsuite/gdb.ada/notcplusplus/foo.adb b/gdb/testsuite/gdb.ada/notcplusplus/foo.adb new file mode 100644 index 0000000..89e42f9 --- /dev/null +++ b/gdb/testsuite/gdb.ada/notcplusplus/foo.adb @@ -0,0 +1,21 @@ +-- Copyright 2018 Free Software Foundation, Inc. +-- +-- This program is free software; you can redistribute it and/or modify +-- it under the terms of the GNU General Public License as published by +-- the Free Software Foundation; either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU General Public License for more details. +-- +-- You should have received a copy of the GNU General Public License +-- along with this program. If not, see <http://www.gnu.org/licenses/>. + +with Pck; use Pck; +with Ver; use Ver; +procedure Foo is +begin + Do_Nothing (u00045'Address); +end Foo; diff --git a/gdb/testsuite/gdb.ada/notcplusplus/pck.adb b/gdb/testsuite/gdb.ada/notcplusplus/pck.adb new file mode 100644 index 0000000..dcfb306 --- /dev/null +++ b/gdb/testsuite/gdb.ada/notcplusplus/pck.adb @@ -0,0 +1,21 @@ +-- Copyright 2018 Free Software Foundation, Inc. +-- +-- This program is free software; you can redistribute it and/or modify +-- it under the terms of the GNU General Public License as published by +-- the Free Software Foundation; either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU General Public License for more details. +-- +-- You should have received a copy of the GNU General Public License +-- along with this program. If not, see <http://www.gnu.org/licenses/>. + +package body Pck is + procedure Do_Nothing (A : System.Address) is + begin + null; + end Do_Nothing; +end Pck; diff --git a/gdb/testsuite/gdb.ada/notcplusplus/pck.ads b/gdb/testsuite/gdb.ada/notcplusplus/pck.ads new file mode 100644 index 0000000..33e369e --- /dev/null +++ b/gdb/testsuite/gdb.ada/notcplusplus/pck.ads @@ -0,0 +1,19 @@ +-- Copyright 2018 Free Software Foundation, Inc. +-- +-- This program is free software; you can redistribute it and/or modify +-- it under the terms of the GNU General Public License as published by +-- the Free Software Foundation; either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU General Public License for more details. +-- +-- You should have received a copy of the GNU General Public License +-- along with this program. If not, see <http://www.gnu.org/licenses/>. + +with System; +package Pck is + procedure Do_Nothing (A : System.Address); +end Pck; diff --git a/gdb/testsuite/gdb.ada/notcplusplus/ver.ads b/gdb/testsuite/gdb.ada/notcplusplus/ver.ads new file mode 100644 index 0000000..8f264d0 --- /dev/null +++ b/gdb/testsuite/gdb.ada/notcplusplus/ver.ads @@ -0,0 +1,22 @@ +-- Copyright 2018 Free Software Foundation, Inc. +-- +-- This program is free software; you can redistribute it and/or modify +-- it under the terms of the GNU General Public License as published by +-- the Free Software Foundation; either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU General Public License for more details. +-- +-- You should have received a copy of the GNU General Public License +-- along with this program. If not, see <http://www.gnu.org/licenses/>. + +package Ver is + type Wrapper is record + A : Integer; + end record; + u00045 : constant Wrapper := (A => 16#060287af#); + pragma Export (C, u00045, "symada__cS"); +end Ver; diff --git a/gdb/testsuite/gdb.base/c-linkage-name.c b/gdb/testsuite/gdb.base/c-linkage-name.c new file mode 100644 index 0000000..925004c --- /dev/null +++ b/gdb/testsuite/gdb.base/c-linkage-name.c @@ -0,0 +1,44 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2018 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +struct wrapper +{ + int a; +}; + +/* Create a global variable whose name in the assembly code + (aka the "linkage name") is different from the name in + the source code. The goal is to create a symbol described + in DWARF using a DW_AT_linkage_name attribute, with a name + which follows the C++ mangling. + + In this particular case, we chose "symada__cS" which, if it were + demangled, would translate to "symada (char, signed)". */ +struct wrapper mundane asm ("symada__cS") = {0x060287af}; + +void +do_something (struct wrapper *w) +{ + w->a++; +} + +int +main (void) +{ + do_something (&mundane); + return 0; +} diff --git a/gdb/testsuite/gdb.base/c-linkage-name.exp b/gdb/testsuite/gdb.base/c-linkage-name.exp new file mode 100644 index 0000000..c80a530 --- /dev/null +++ b/gdb/testsuite/gdb.base/c-linkage-name.exp @@ -0,0 +1,47 @@ +# Copyright 2018 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +# This file is part of the gdb testsuite. It is intended to test that +# gdb can correctly print arrays with indexes for each element of the +# array. + +standard_testfile .c + +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } { + untested "failed to compile" + return -1 +} + +clean_restart ${binfile} + +# Try to print MUNDANE, but using its linkage name. + +gdb_test "print symada__cS" \ + " = {a = 100829103}" \ + "print symada__cS before partial symtab expansion" + +# Force the symbols to be expanded for the unit that contains +# our symada__cS symbol by, e.g. inserting a breakpoint on one +# of the founction also provided by the same using. + +gdb_test "break main" \ + "Breakpoint $decimal at $hex: file .*$srcfile, line $decimal\\." + +# Try to print MUNDANE using its linkage name again, after partial +# symtab expansion. + +gdb_test "print symada__cS" \ + " = {a = 100829103}" \ + "print symada__cS after partial symtab expansion" -- 2.1.4 ^ permalink raw reply [flat|nested] 27+ messages in thread
* [RFA/RFC] fix PR gdb/22670 (pb looking up some symbols when they have a linkage name) (was: "Re: [RFC] regresssion(internal-error) printing subprogram argument") 2018-02-05 9:57 ` Joel Brobecker @ 2018-02-09 9:09 ` Joel Brobecker 2018-02-21 3:02 ` PING: " Joel Brobecker 0 siblings, 1 reply; 27+ messages in thread From: Joel Brobecker @ 2018-02-09 9:09 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches [-- Attachment #1: Type: text/plain, Size: 1079 bytes --] > All in all, I think a better solution would be to put that information > directly in the language_defn itself, via a new "attribute". > Something like a... I ended up choosing this approach. lookup names is still a bit foggy for me, so the comments I wrote and/or the name of the new language_defn attribute might be a bit off. As always, I am grateful for suggestions :). gdb/ChangeLog: PR gdb/22670 * dwarf2read.c (dwarf2_physname): Do not return the demangled symbol name is the CU's language stores symbol names in linkage format. * language.h (struct language_defn) <la_store_sym_names_in_linkage_form_p>: New field. Adjust all instances of this struct. gdb/testsuite/ChangeLog: * gdb.ada/maint_with_ada.exp: Remove PR gdb/22670 setup_kfail. * gdb.ada/notcplusplus: New testcase. * gdb.base/c-linkage-name.c: New file. * gdb.base/c-linkage-name.exp: New testcase. Tested on x86_64-linux. This also passes AdaCore's internal GDB testsuite. OK to commit? Thank you! -- Joel [-- Attachment #2: 0001-problem-looking-up-some-symbols-when-they-have-a-lin.patch --] [-- Type: text/x-diff, Size: 24886 bytes --] From c22aef84cc86097d6d8b654b0586a3fbc1ff6af3 Mon Sep 17 00:00:00 2001 From: Joel Brobecker <brobecker@adacore.com> Date: Fri, 9 Feb 2018 02:13:34 -0500 Subject: [PATCH] problem looking up some symbols when they have a linkage name This patch fixes a known failure in gdb.ada/maint_with_ada.exp (maintenance check-psymtabs). Another way to witness the same issue is by considering the following Ada declarations... type Wrapper is record A : Integer; end record; u00045 : constant Wrapper := (A => 16#060287af#); pragma Export (C, u00045, "symada__cS"); ... which declares a variable name "u00045" but with a linkage name which is "symada__cS". This variable is a record with one component, the Ada equivalent of a struct with one field in C. Trying to print that variable's value currently yields: (gdb) p /x <symada__cS> 'symada(char, signed)' has unknown type; cast it to its declared type This indicates that GDB was only able to find the minimal symbol, but not the full symbol. The expected output is: (gdb) print /x <symada__cS> $1 = (a => 0x60287af) The error message gives a hint about what's happening: We processed the symbol through gdb_demangle, which in the case of this particular symbol name, ends up matching the C++ naming scheme. As a result, the demangler transforms our symbol name into 'symada(char, signed)', thus breaking Ada lookups. This patch fixes the issue by first introducing a new language_defn attribute called la_store_sym_names_in_linkage_form_p, which is a boolean to be set to true for the few languages that do not want their symbols to have their names stored in demangled form, and false otherwise. We then use this language attribute to skip the call to gdb_demangle for all languages whose la_store_sym_names_in_linkage_form_p is true. In terms of the selection of languages for which the new attribute is set to true, the selection errs on the side of preserving the existing behavior, and only changes the behavior for the languages where we are certain storing symbol names in demangling form is not needed. It is conceivable that other languages might be in the same situation, but I not knowing in detail the symbol name enconding strategy, I decided to play it safe and let other language maintainers potentially adjust their language if it makes sense to do so. gdb/ChangeLog: PR gdb/22670 * dwarf2read.c (dwarf2_physname): Do not return the demangled symbol name is the CU's language stores symbol names in linkage format. * language.h (struct language_defn) <la_store_sym_names_in_linkage_form_p>: New field. Adjust all instances of this struct. gdb/testsuite/ChangeLog: * gdb.ada/maint_with_ada.exp: Remove PR gdb/22670 setup_kfail. * gdb.ada/notcplusplus: New testcase. * gdb.base/c-linkage-name.c: New file. * gdb.base/c-linkage-name.exp: New testcase. Tested on x86_64-linux. This also passes AdaCore's internal GDB testsuite. --- gdb/ada-lang.c | 1 + gdb/c-lang.c | 4 +++ gdb/d-lang.c | 1 + gdb/dwarf2read.c | 6 +++- gdb/f-lang.c | 1 + gdb/go-lang.c | 1 + gdb/language.c | 2 ++ gdb/language.h | 20 +++++++++++++ gdb/m2-lang.c | 1 + gdb/objc-lang.c | 1 + gdb/opencl-lang.c | 1 + gdb/p-lang.c | 1 + gdb/rust-lang.c | 1 + gdb/testsuite/gdb.ada/maint_with_ada.exp | 1 - gdb/testsuite/gdb.ada/notcplusplus.exp | 45 ++++++++++++++++++++++++++++ gdb/testsuite/gdb.ada/notcplusplus/foo.adb | 21 +++++++++++++ gdb/testsuite/gdb.ada/notcplusplus/pck.adb | 21 +++++++++++++ gdb/testsuite/gdb.ada/notcplusplus/pck.ads | 19 ++++++++++++ gdb/testsuite/gdb.ada/notcplusplus/ver.ads | 22 ++++++++++++++ gdb/testsuite/gdb.base/c-linkage-name.c | 44 ++++++++++++++++++++++++++++ gdb/testsuite/gdb.base/c-linkage-name.exp | 47 ++++++++++++++++++++++++++++++ 21 files changed, 259 insertions(+), 2 deletions(-) create mode 100644 gdb/testsuite/gdb.ada/notcplusplus.exp create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/foo.adb create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/pck.adb create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/pck.ads create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/ver.ads create mode 100644 gdb/testsuite/gdb.base/c-linkage-name.c create mode 100644 gdb/testsuite/gdb.base/c-linkage-name.exp diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c index 0da58d9..7f99a2e 100644 --- a/gdb/ada-lang.c +++ b/gdb/ada-lang.c @@ -14521,6 +14521,7 @@ extern const struct language_defn ada_language_defn = { ada_read_var_value, /* la_read_var_value */ NULL, /* Language specific skip_trampoline */ NULL, /* name_of_this */ + true, /* la_store_sym_names_in_linkage_form_p */ ada_lookup_symbol_nonlocal, /* Looking up non-local symbols. */ basic_lookup_transparent_type, /* lookup_transparent_type */ ada_la_decode, /* Language specific symbol demangler */ diff --git a/gdb/c-lang.c b/gdb/c-lang.c index a0b553e..658c7f7 100644 --- a/gdb/c-lang.c +++ b/gdb/c-lang.c @@ -853,6 +853,7 @@ extern const struct language_defn c_language_defn = default_read_var_value, /* la_read_var_value */ NULL, /* Language specific skip_trampoline */ NULL, /* name_of_this */ + true, /* la_store_sym_names_in_linkage_form_p */ basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ basic_lookup_transparent_type,/* lookup_transparent_type */ NULL, /* Language specific symbol demangler */ @@ -998,6 +999,7 @@ extern const struct language_defn cplus_language_defn = default_read_var_value, /* la_read_var_value */ cplus_skip_trampoline, /* Language specific skip_trampoline */ "this", /* name_of_this */ + false, /* la_store_sym_names_in_linkage_form_p */ cp_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ cp_lookup_transparent_type, /* lookup_transparent_type */ gdb_demangle, /* Language specific symbol demangler */ @@ -1052,6 +1054,7 @@ extern const struct language_defn asm_language_defn = default_read_var_value, /* la_read_var_value */ NULL, /* Language specific skip_trampoline */ NULL, /* name_of_this */ + true, /* la_store_sym_names_in_linkage_form_p */ basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ basic_lookup_transparent_type,/* lookup_transparent_type */ NULL, /* Language specific symbol demangler */ @@ -1106,6 +1109,7 @@ extern const struct language_defn minimal_language_defn = default_read_var_value, /* la_read_var_value */ NULL, /* Language specific skip_trampoline */ NULL, /* name_of_this */ + true, /* la_store_sym_names_in_linkage_form_p */ basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ basic_lookup_transparent_type,/* lookup_transparent_type */ NULL, /* Language specific symbol demangler */ diff --git a/gdb/d-lang.c b/gdb/d-lang.c index e0afe48..688ae98 100644 --- a/gdb/d-lang.c +++ b/gdb/d-lang.c @@ -229,6 +229,7 @@ extern const struct language_defn d_language_defn = default_read_var_value, /* la_read_var_value */ NULL, /* Language specific skip_trampoline. */ "this", + false, /* la_store_sym_names_in_linkage_form_p */ d_lookup_symbol_nonlocal, basic_lookup_transparent_type, d_demangle, /* Language specific symbol demangler. */ diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index d651725..b4c087f 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -11142,7 +11142,11 @@ dwarf2_physname (const char *name, struct die_info *die, struct dwarf2_cu *cu) if (mangled != NULL) { - if (cu->language == language_go) + if (language_def (cu->language)->la_store_sym_names_in_linkage_form_p) + { + /* Do nothing (do not demangle the symbol name). */ + } + else if (cu->language == language_go) { /* This is a lie, but we already lie to the caller new_symbol. new_symbol assumes we return the mangled name. diff --git a/gdb/f-lang.c b/gdb/f-lang.c index 74f5622..81922f7 100644 --- a/gdb/f-lang.c +++ b/gdb/f-lang.c @@ -269,6 +269,7 @@ extern const struct language_defn f_language_defn = default_read_var_value, /* la_read_var_value */ NULL, /* Language specific skip_trampoline */ NULL, /* name_of_this */ + false, /* la_store_sym_names_in_linkage_form_p */ cp_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ basic_lookup_transparent_type,/* lookup_transparent_type */ diff --git a/gdb/go-lang.c b/gdb/go-lang.c index 022b8de..e9cba77 100644 --- a/gdb/go-lang.c +++ b/gdb/go-lang.c @@ -590,6 +590,7 @@ extern const struct language_defn go_language_defn = default_read_var_value, /* la_read_var_value */ NULL, /* Language specific skip_trampoline. */ NULL, /* name_of_this */ + false, /* la_store_sym_names_in_linkage_form_p */ basic_lookup_symbol_nonlocal, basic_lookup_transparent_type, go_demangle, /* Language specific symbol demangler. */ diff --git a/gdb/language.c b/gdb/language.c index 0d8604b..22199e0 100644 --- a/gdb/language.c +++ b/gdb/language.c @@ -864,6 +864,7 @@ const struct language_defn unknown_language_defn = default_read_var_value, /* la_read_var_value */ unk_lang_trampoline, /* Language specific skip_trampoline */ "this", /* name_of_this */ + true, /* store_sym_names_in_linkage_form_p */ basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ basic_lookup_transparent_type,/* lookup_transparent_type */ unk_lang_demangle, /* Language specific symbol demangler */ @@ -915,6 +916,7 @@ const struct language_defn auto_language_defn = default_read_var_value, /* la_read_var_value */ unk_lang_trampoline, /* Language specific skip_trampoline */ "this", /* name_of_this */ + false, /* store_sym_names_in_linkage_form_p */ basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ basic_lookup_transparent_type,/* lookup_transparent_type */ unk_lang_demangle, /* Language specific symbol demangler */ diff --git a/gdb/language.h b/gdb/language.h index 06b42ae..029de4a 100644 --- a/gdb/language.h +++ b/gdb/language.h @@ -264,6 +264,26 @@ struct language_defn const char *la_name_of_this; + /* True if the symbols names should be stored in GDB's data structures + for minimal/partial/full symbols using their linkage (aka mangled) + form; false if the symbol names should be demangled first. + + Most languages implement symbol lookup by comparing the demangled + names, in which case it is advantageous to store that information + already demangled, and so would set this field to false. + + On the other hand, some languages have opted for doing symbol + lookups by comparing mangled names instead, for reasons usually + specific to the language. Those languages should set this field + to true. + + And finally, other languages such as C or Asm do not have + the concept of mangled vs demangled name, so those languages + should set this field to true as well, to prevent any accidental + demangling through an unrelated language's demangler. */ + + const bool la_store_sym_names_in_linkage_form_p; + /* This is a function that lookup_symbol will call when it gets to the part of symbol lookup where C looks up static and global variables. */ diff --git a/gdb/m2-lang.c b/gdb/m2-lang.c index 11ccab3..6e6434b 100644 --- a/gdb/m2-lang.c +++ b/gdb/m2-lang.c @@ -377,6 +377,7 @@ extern const struct language_defn m2_language_defn = default_read_var_value, /* la_read_var_value */ NULL, /* Language specific skip_trampoline */ NULL, /* name_of_this */ + false, /* la_store_sym_names_in_linkage_form_p */ basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ basic_lookup_transparent_type,/* lookup_transparent_type */ NULL, /* Language specific symbol demangler */ diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c index c714ec3..4f7dc36 100644 --- a/gdb/objc-lang.c +++ b/gdb/objc-lang.c @@ -389,6 +389,7 @@ extern const struct language_defn objc_language_defn = { default_read_var_value, /* la_read_var_value */ objc_skip_trampoline, /* Language specific skip_trampoline */ "self", /* name_of_this */ + false, /* la_store_sym_names_in_linkage_form_p */ basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ basic_lookup_transparent_type,/* lookup_transparent_type */ objc_demangle, /* Language specific symbol demangler */ diff --git a/gdb/opencl-lang.c b/gdb/opencl-lang.c index 268c3c5..9c5b90c 100644 --- a/gdb/opencl-lang.c +++ b/gdb/opencl-lang.c @@ -1065,6 +1065,7 @@ extern const struct language_defn opencl_language_defn = default_read_var_value, /* la_read_var_value */ NULL, /* Language specific skip_trampoline */ NULL, /* name_of_this */ + false, /* la_store_sym_names_in_linkage_form_p */ basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ basic_lookup_transparent_type,/* lookup_transparent_type */ NULL, /* Language specific symbol demangler */ diff --git a/gdb/p-lang.c b/gdb/p-lang.c index 03db2df..3ff7f56 100644 --- a/gdb/p-lang.c +++ b/gdb/p-lang.c @@ -439,6 +439,7 @@ extern const struct language_defn pascal_language_defn = default_read_var_value, /* la_read_var_value */ NULL, /* Language specific skip_trampoline */ "this", /* name_of_this */ + false, /* la_store_sym_names_in_linkage_form_p */ basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ basic_lookup_transparent_type,/* lookup_transparent_type */ NULL, /* Language specific symbol demangler */ diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c index 5ff80b2..2d7c1f7 100644 --- a/gdb/rust-lang.c +++ b/gdb/rust-lang.c @@ -2290,6 +2290,7 @@ extern const struct language_defn rust_language_defn = default_read_var_value, /* la_read_var_value */ NULL, /* Language specific skip_trampoline */ NULL, /* name_of_this */ + false, /* la_store_sym_names_in_linkage_form_p */ rust_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ basic_lookup_transparent_type,/* lookup_transparent_type */ gdb_demangle, /* Language specific symbol demangler */ diff --git a/gdb/testsuite/gdb.ada/maint_with_ada.exp b/gdb/testsuite/gdb.ada/maint_with_ada.exp index 73da613..9ede035 100644 --- a/gdb/testsuite/gdb.ada/maint_with_ada.exp +++ b/gdb/testsuite/gdb.ada/maint_with_ada.exp @@ -32,7 +32,6 @@ gdb_breakpoint "adainit" gdb_breakpoint "Var_Arr_Typedef" gdb_breakpoint "Do_Nothing" -setup_kfail gdb/22670 "*-*-*" gdb_test_no_output "maintenance check-psymtabs" gdb_test_no_output "maintenance check-symtabs" diff --git a/gdb/testsuite/gdb.ada/notcplusplus.exp b/gdb/testsuite/gdb.ada/notcplusplus.exp new file mode 100644 index 0000000..b2a24e8 --- /dev/null +++ b/gdb/testsuite/gdb.ada/notcplusplus.exp @@ -0,0 +1,45 @@ +# Copyright 2018 Free Software Foundation, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +load_lib "ada.exp" + +if { [skip_ada_tests] } { return -1 } + +standard_ada_testfile foo + +if {[gdb_compile_ada "${srcfile}" "${binfile}" executable {debug}] != ""} { + return -1 +} + +clean_restart ${testfile} + +gdb_test "print /x <symada__cS>" \ + "= \\(a => 0x60287af\\)" \ + "print <symada__cS> before loading symbols from ver.ads" + +# Force the partial symbosl from ver.ads to be expanded into full symbols. + +gdb_test \ + "list ver.ads:16" \ + [multi_line ".*" \ + "16\\s+package Ver is" \ + "17\\s+type Wrapper is record" \ + "18\\s+A : Integer;" \ + "19\\s+end record;" \ + "20\\s+u00045 : constant Wrapper := \\(A => 16#060287af#\\);"] + +gdb_test "print /x <symada__cS>" \ + "= \\(a => 0x60287af\\)" \ + "print <symada__cS> after loading symbols from ver.ads" diff --git a/gdb/testsuite/gdb.ada/notcplusplus/foo.adb b/gdb/testsuite/gdb.ada/notcplusplus/foo.adb new file mode 100644 index 0000000..89e42f9 --- /dev/null +++ b/gdb/testsuite/gdb.ada/notcplusplus/foo.adb @@ -0,0 +1,21 @@ +-- Copyright 2018 Free Software Foundation, Inc. +-- +-- This program is free software; you can redistribute it and/or modify +-- it under the terms of the GNU General Public License as published by +-- the Free Software Foundation; either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU General Public License for more details. +-- +-- You should have received a copy of the GNU General Public License +-- along with this program. If not, see <http://www.gnu.org/licenses/>. + +with Pck; use Pck; +with Ver; use Ver; +procedure Foo is +begin + Do_Nothing (u00045'Address); +end Foo; diff --git a/gdb/testsuite/gdb.ada/notcplusplus/pck.adb b/gdb/testsuite/gdb.ada/notcplusplus/pck.adb new file mode 100644 index 0000000..dcfb306 --- /dev/null +++ b/gdb/testsuite/gdb.ada/notcplusplus/pck.adb @@ -0,0 +1,21 @@ +-- Copyright 2018 Free Software Foundation, Inc. +-- +-- This program is free software; you can redistribute it and/or modify +-- it under the terms of the GNU General Public License as published by +-- the Free Software Foundation; either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU General Public License for more details. +-- +-- You should have received a copy of the GNU General Public License +-- along with this program. If not, see <http://www.gnu.org/licenses/>. + +package body Pck is + procedure Do_Nothing (A : System.Address) is + begin + null; + end Do_Nothing; +end Pck; diff --git a/gdb/testsuite/gdb.ada/notcplusplus/pck.ads b/gdb/testsuite/gdb.ada/notcplusplus/pck.ads new file mode 100644 index 0000000..33e369e --- /dev/null +++ b/gdb/testsuite/gdb.ada/notcplusplus/pck.ads @@ -0,0 +1,19 @@ +-- Copyright 2018 Free Software Foundation, Inc. +-- +-- This program is free software; you can redistribute it and/or modify +-- it under the terms of the GNU General Public License as published by +-- the Free Software Foundation; either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU General Public License for more details. +-- +-- You should have received a copy of the GNU General Public License +-- along with this program. If not, see <http://www.gnu.org/licenses/>. + +with System; +package Pck is + procedure Do_Nothing (A : System.Address); +end Pck; diff --git a/gdb/testsuite/gdb.ada/notcplusplus/ver.ads b/gdb/testsuite/gdb.ada/notcplusplus/ver.ads new file mode 100644 index 0000000..8f264d0 --- /dev/null +++ b/gdb/testsuite/gdb.ada/notcplusplus/ver.ads @@ -0,0 +1,22 @@ +-- Copyright 2018 Free Software Foundation, Inc. +-- +-- This program is free software; you can redistribute it and/or modify +-- it under the terms of the GNU General Public License as published by +-- the Free Software Foundation; either version 3 of the License, or +-- (at your option) any later version. +-- +-- This program is distributed in the hope that it will be useful, +-- but WITHOUT ANY WARRANTY; without even the implied warranty of +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +-- GNU General Public License for more details. +-- +-- You should have received a copy of the GNU General Public License +-- along with this program. If not, see <http://www.gnu.org/licenses/>. + +package Ver is + type Wrapper is record + A : Integer; + end record; + u00045 : constant Wrapper := (A => 16#060287af#); + pragma Export (C, u00045, "symada__cS"); +end Ver; diff --git a/gdb/testsuite/gdb.base/c-linkage-name.c b/gdb/testsuite/gdb.base/c-linkage-name.c new file mode 100644 index 0000000..925004c --- /dev/null +++ b/gdb/testsuite/gdb.base/c-linkage-name.c @@ -0,0 +1,44 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2018 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +struct wrapper +{ + int a; +}; + +/* Create a global variable whose name in the assembly code + (aka the "linkage name") is different from the name in + the source code. The goal is to create a symbol described + in DWARF using a DW_AT_linkage_name attribute, with a name + which follows the C++ mangling. + + In this particular case, we chose "symada__cS" which, if it were + demangled, would translate to "symada (char, signed)". */ +struct wrapper mundane asm ("symada__cS") = {0x060287af}; + +void +do_something (struct wrapper *w) +{ + w->a++; +} + +int +main (void) +{ + do_something (&mundane); + return 0; +} diff --git a/gdb/testsuite/gdb.base/c-linkage-name.exp b/gdb/testsuite/gdb.base/c-linkage-name.exp new file mode 100644 index 0000000..c80a530 --- /dev/null +++ b/gdb/testsuite/gdb.base/c-linkage-name.exp @@ -0,0 +1,47 @@ +# Copyright 2018 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +# This file is part of the gdb testsuite. It is intended to test that +# gdb can correctly print arrays with indexes for each element of the +# array. + +standard_testfile .c + +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } { + untested "failed to compile" + return -1 +} + +clean_restart ${binfile} + +# Try to print MUNDANE, but using its linkage name. + +gdb_test "print symada__cS" \ + " = {a = 100829103}" \ + "print symada__cS before partial symtab expansion" + +# Force the symbols to be expanded for the unit that contains +# our symada__cS symbol by, e.g. inserting a breakpoint on one +# of the founction also provided by the same using. + +gdb_test "break main" \ + "Breakpoint $decimal at $hex: file .*$srcfile, line $decimal\\." + +# Try to print MUNDANE using its linkage name again, after partial +# symtab expansion. + +gdb_test "print symada__cS" \ + " = {a = 100829103}" \ + "print symada__cS after partial symtab expansion" -- 2.1.4 ^ permalink raw reply [flat|nested] 27+ messages in thread
* PING: [RFA/RFC] fix PR gdb/22670 (pb looking up some symbols when they have a linkage name) (was: "Re: [RFC] regresssion(internal-error) printing subprogram argument") 2018-02-09 9:09 ` [RFA/RFC] fix PR gdb/22670 (pb looking up some symbols when they have a linkage name) (was: "Re: [RFC] regresssion(internal-error) printing subprogram argument") Joel Brobecker @ 2018-02-21 3:02 ` Joel Brobecker 2018-03-19 21:22 ` PING^2: " Joel Brobecker 0 siblings, 1 reply; 27+ messages in thread From: Joel Brobecker @ 2018-02-21 3:02 UTC (permalink / raw) To: gdb-patches > > All in all, I think a better solution would be to put that information > > directly in the language_defn itself, via a new "attribute". > > Something like a... > > I ended up choosing this approach. lookup names is still a bit > foggy for me, so the comments I wrote and/or the name of the > new language_defn attribute might be a bit off. As always, I am > grateful for suggestions :). > > gdb/ChangeLog: > > PR gdb/22670 > * dwarf2read.c (dwarf2_physname): Do not return the demangled > symbol name is the CU's language stores symbol names in linkage > format. > * language.h (struct language_defn) > <la_store_sym_names_in_linkage_form_p>: New field. Adjust > all instances of this struct. > > gdb/testsuite/ChangeLog: > > * gdb.ada/maint_with_ada.exp: Remove PR gdb/22670 setup_kfail. > > * gdb.ada/notcplusplus: New testcase. > > * gdb.base/c-linkage-name.c: New file. > * gdb.base/c-linkage-name.exp: New testcase. > > Tested on x86_64-linux. > This also passes AdaCore's internal GDB testsuite. Any comment on this patch? Thanks! > >From c22aef84cc86097d6d8b654b0586a3fbc1ff6af3 Mon Sep 17 00:00:00 2001 > From: Joel Brobecker <brobecker@adacore.com> > Date: Fri, 9 Feb 2018 02:13:34 -0500 > Subject: [PATCH] problem looking up some symbols when they have a linkage name > > This patch fixes a known failure in gdb.ada/maint_with_ada.exp > (maintenance check-psymtabs). Another way to witness the same > issue is by considering the following Ada declarations... > > type Wrapper is record > A : Integer; > end record; > u00045 : constant Wrapper := (A => 16#060287af#); > pragma Export (C, u00045, "symada__cS"); > > ... which declares a variable name "u00045" but with a linkage > name which is "symada__cS". This variable is a record with one > component, the Ada equivalent of a struct with one field in C. > Trying to print that variable's value currently yields: > > (gdb) p /x <symada__cS> > 'symada(char, signed)' has unknown type; cast it to its declared type > > This indicates that GDB was only able to find the minimal symbol, > but not the full symbol. The expected output is: > > (gdb) print /x <symada__cS> > $1 = (a => 0x60287af) > > The error message gives a hint about what's happening: We processed > the symbol through gdb_demangle, which in the case of this particular > symbol name, ends up matching the C++ naming scheme. As a result, > the demangler transforms our symbol name into 'symada(char, signed)', > thus breaking Ada lookups. > > This patch fixes the issue by first introducing a new language_defn > attribute called la_store_sym_names_in_linkage_form_p, which is a boolean > to be set to true for the few languages that do not want their symbols > to have their names stored in demangled form, and false otherwise. > We then use this language attribute to skip the call to gdb_demangle > for all languages whose la_store_sym_names_in_linkage_form_p is true. > > In terms of the selection of languages for which the new attribute > is set to true, the selection errs on the side of preserving the > existing behavior, and only changes the behavior for the languages > where we are certain storing symbol names in demangling form is not > needed. It is conceivable that other languages might be in the same > situation, but I not knowing in detail the symbol name enconding > strategy, I decided to play it safe and let other language maintainers > potentially adjust their language if it makes sense to do so. > > gdb/ChangeLog: > > PR gdb/22670 > * dwarf2read.c (dwarf2_physname): Do not return the demangled > symbol name is the CU's language stores symbol names in linkage > format. > * language.h (struct language_defn) > <la_store_sym_names_in_linkage_form_p>: New field. Adjust > all instances of this struct. > > gdb/testsuite/ChangeLog: > > * gdb.ada/maint_with_ada.exp: Remove PR gdb/22670 setup_kfail. > > * gdb.ada/notcplusplus: New testcase. > > * gdb.base/c-linkage-name.c: New file. > * gdb.base/c-linkage-name.exp: New testcase. > > Tested on x86_64-linux. > This also passes AdaCore's internal GDB testsuite. > --- > gdb/ada-lang.c | 1 + > gdb/c-lang.c | 4 +++ > gdb/d-lang.c | 1 + > gdb/dwarf2read.c | 6 +++- > gdb/f-lang.c | 1 + > gdb/go-lang.c | 1 + > gdb/language.c | 2 ++ > gdb/language.h | 20 +++++++++++++ > gdb/m2-lang.c | 1 + > gdb/objc-lang.c | 1 + > gdb/opencl-lang.c | 1 + > gdb/p-lang.c | 1 + > gdb/rust-lang.c | 1 + > gdb/testsuite/gdb.ada/maint_with_ada.exp | 1 - > gdb/testsuite/gdb.ada/notcplusplus.exp | 45 ++++++++++++++++++++++++++++ > gdb/testsuite/gdb.ada/notcplusplus/foo.adb | 21 +++++++++++++ > gdb/testsuite/gdb.ada/notcplusplus/pck.adb | 21 +++++++++++++ > gdb/testsuite/gdb.ada/notcplusplus/pck.ads | 19 ++++++++++++ > gdb/testsuite/gdb.ada/notcplusplus/ver.ads | 22 ++++++++++++++ > gdb/testsuite/gdb.base/c-linkage-name.c | 44 ++++++++++++++++++++++++++++ > gdb/testsuite/gdb.base/c-linkage-name.exp | 47 ++++++++++++++++++++++++++++++ > 21 files changed, 259 insertions(+), 2 deletions(-) > create mode 100644 gdb/testsuite/gdb.ada/notcplusplus.exp > create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/foo.adb > create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/pck.adb > create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/pck.ads > create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/ver.ads > create mode 100644 gdb/testsuite/gdb.base/c-linkage-name.c > create mode 100644 gdb/testsuite/gdb.base/c-linkage-name.exp > > diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c > index 0da58d9..7f99a2e 100644 > --- a/gdb/ada-lang.c > +++ b/gdb/ada-lang.c > @@ -14521,6 +14521,7 @@ extern const struct language_defn ada_language_defn = { > ada_read_var_value, /* la_read_var_value */ > NULL, /* Language specific skip_trampoline */ > NULL, /* name_of_this */ > + true, /* la_store_sym_names_in_linkage_form_p */ > ada_lookup_symbol_nonlocal, /* Looking up non-local symbols. */ > basic_lookup_transparent_type, /* lookup_transparent_type */ > ada_la_decode, /* Language specific symbol demangler */ > diff --git a/gdb/c-lang.c b/gdb/c-lang.c > index a0b553e..658c7f7 100644 > --- a/gdb/c-lang.c > +++ b/gdb/c-lang.c > @@ -853,6 +853,7 @@ extern const struct language_defn c_language_defn = > default_read_var_value, /* la_read_var_value */ > NULL, /* Language specific skip_trampoline */ > NULL, /* name_of_this */ > + true, /* la_store_sym_names_in_linkage_form_p */ > basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ > basic_lookup_transparent_type,/* lookup_transparent_type */ > NULL, /* Language specific symbol demangler */ > @@ -998,6 +999,7 @@ extern const struct language_defn cplus_language_defn = > default_read_var_value, /* la_read_var_value */ > cplus_skip_trampoline, /* Language specific skip_trampoline */ > "this", /* name_of_this */ > + false, /* la_store_sym_names_in_linkage_form_p */ > cp_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ > cp_lookup_transparent_type, /* lookup_transparent_type */ > gdb_demangle, /* Language specific symbol demangler */ > @@ -1052,6 +1054,7 @@ extern const struct language_defn asm_language_defn = > default_read_var_value, /* la_read_var_value */ > NULL, /* Language specific skip_trampoline */ > NULL, /* name_of_this */ > + true, /* la_store_sym_names_in_linkage_form_p */ > basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ > basic_lookup_transparent_type,/* lookup_transparent_type */ > NULL, /* Language specific symbol demangler */ > @@ -1106,6 +1109,7 @@ extern const struct language_defn minimal_language_defn = > default_read_var_value, /* la_read_var_value */ > NULL, /* Language specific skip_trampoline */ > NULL, /* name_of_this */ > + true, /* la_store_sym_names_in_linkage_form_p */ > basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ > basic_lookup_transparent_type,/* lookup_transparent_type */ > NULL, /* Language specific symbol demangler */ > diff --git a/gdb/d-lang.c b/gdb/d-lang.c > index e0afe48..688ae98 100644 > --- a/gdb/d-lang.c > +++ b/gdb/d-lang.c > @@ -229,6 +229,7 @@ extern const struct language_defn d_language_defn = > default_read_var_value, /* la_read_var_value */ > NULL, /* Language specific skip_trampoline. */ > "this", > + false, /* la_store_sym_names_in_linkage_form_p */ > d_lookup_symbol_nonlocal, > basic_lookup_transparent_type, > d_demangle, /* Language specific symbol demangler. */ > diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c > index d651725..b4c087f 100644 > --- a/gdb/dwarf2read.c > +++ b/gdb/dwarf2read.c > @@ -11142,7 +11142,11 @@ dwarf2_physname (const char *name, struct die_info *die, struct dwarf2_cu *cu) > if (mangled != NULL) > { > > - if (cu->language == language_go) > + if (language_def (cu->language)->la_store_sym_names_in_linkage_form_p) > + { > + /* Do nothing (do not demangle the symbol name). */ > + } > + else if (cu->language == language_go) > { > /* This is a lie, but we already lie to the caller new_symbol. > new_symbol assumes we return the mangled name. > diff --git a/gdb/f-lang.c b/gdb/f-lang.c > index 74f5622..81922f7 100644 > --- a/gdb/f-lang.c > +++ b/gdb/f-lang.c > @@ -269,6 +269,7 @@ extern const struct language_defn f_language_defn = > default_read_var_value, /* la_read_var_value */ > NULL, /* Language specific skip_trampoline */ > NULL, /* name_of_this */ > + false, /* la_store_sym_names_in_linkage_form_p */ > cp_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ > basic_lookup_transparent_type,/* lookup_transparent_type */ > > diff --git a/gdb/go-lang.c b/gdb/go-lang.c > index 022b8de..e9cba77 100644 > --- a/gdb/go-lang.c > +++ b/gdb/go-lang.c > @@ -590,6 +590,7 @@ extern const struct language_defn go_language_defn = > default_read_var_value, /* la_read_var_value */ > NULL, /* Language specific skip_trampoline. */ > NULL, /* name_of_this */ > + false, /* la_store_sym_names_in_linkage_form_p */ > basic_lookup_symbol_nonlocal, > basic_lookup_transparent_type, > go_demangle, /* Language specific symbol demangler. */ > diff --git a/gdb/language.c b/gdb/language.c > index 0d8604b..22199e0 100644 > --- a/gdb/language.c > +++ b/gdb/language.c > @@ -864,6 +864,7 @@ const struct language_defn unknown_language_defn = > default_read_var_value, /* la_read_var_value */ > unk_lang_trampoline, /* Language specific skip_trampoline */ > "this", /* name_of_this */ > + true, /* store_sym_names_in_linkage_form_p */ > basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ > basic_lookup_transparent_type,/* lookup_transparent_type */ > unk_lang_demangle, /* Language specific symbol demangler */ > @@ -915,6 +916,7 @@ const struct language_defn auto_language_defn = > default_read_var_value, /* la_read_var_value */ > unk_lang_trampoline, /* Language specific skip_trampoline */ > "this", /* name_of_this */ > + false, /* store_sym_names_in_linkage_form_p */ > basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ > basic_lookup_transparent_type,/* lookup_transparent_type */ > unk_lang_demangle, /* Language specific symbol demangler */ > diff --git a/gdb/language.h b/gdb/language.h > index 06b42ae..029de4a 100644 > --- a/gdb/language.h > +++ b/gdb/language.h > @@ -264,6 +264,26 @@ struct language_defn > > const char *la_name_of_this; > > + /* True if the symbols names should be stored in GDB's data structures > + for minimal/partial/full symbols using their linkage (aka mangled) > + form; false if the symbol names should be demangled first. > + > + Most languages implement symbol lookup by comparing the demangled > + names, in which case it is advantageous to store that information > + already demangled, and so would set this field to false. > + > + On the other hand, some languages have opted for doing symbol > + lookups by comparing mangled names instead, for reasons usually > + specific to the language. Those languages should set this field > + to true. > + > + And finally, other languages such as C or Asm do not have > + the concept of mangled vs demangled name, so those languages > + should set this field to true as well, to prevent any accidental > + demangling through an unrelated language's demangler. */ > + > + const bool la_store_sym_names_in_linkage_form_p; > + > /* This is a function that lookup_symbol will call when it gets to > the part of symbol lookup where C looks up static and global > variables. */ > diff --git a/gdb/m2-lang.c b/gdb/m2-lang.c > index 11ccab3..6e6434b 100644 > --- a/gdb/m2-lang.c > +++ b/gdb/m2-lang.c > @@ -377,6 +377,7 @@ extern const struct language_defn m2_language_defn = > default_read_var_value, /* la_read_var_value */ > NULL, /* Language specific skip_trampoline */ > NULL, /* name_of_this */ > + false, /* la_store_sym_names_in_linkage_form_p */ > basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ > basic_lookup_transparent_type,/* lookup_transparent_type */ > NULL, /* Language specific symbol demangler */ > diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c > index c714ec3..4f7dc36 100644 > --- a/gdb/objc-lang.c > +++ b/gdb/objc-lang.c > @@ -389,6 +389,7 @@ extern const struct language_defn objc_language_defn = { > default_read_var_value, /* la_read_var_value */ > objc_skip_trampoline, /* Language specific skip_trampoline */ > "self", /* name_of_this */ > + false, /* la_store_sym_names_in_linkage_form_p */ > basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ > basic_lookup_transparent_type,/* lookup_transparent_type */ > objc_demangle, /* Language specific symbol demangler */ > diff --git a/gdb/opencl-lang.c b/gdb/opencl-lang.c > index 268c3c5..9c5b90c 100644 > --- a/gdb/opencl-lang.c > +++ b/gdb/opencl-lang.c > @@ -1065,6 +1065,7 @@ extern const struct language_defn opencl_language_defn = > default_read_var_value, /* la_read_var_value */ > NULL, /* Language specific skip_trampoline */ > NULL, /* name_of_this */ > + false, /* la_store_sym_names_in_linkage_form_p */ > basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ > basic_lookup_transparent_type,/* lookup_transparent_type */ > NULL, /* Language specific symbol demangler */ > diff --git a/gdb/p-lang.c b/gdb/p-lang.c > index 03db2df..3ff7f56 100644 > --- a/gdb/p-lang.c > +++ b/gdb/p-lang.c > @@ -439,6 +439,7 @@ extern const struct language_defn pascal_language_defn = > default_read_var_value, /* la_read_var_value */ > NULL, /* Language specific skip_trampoline */ > "this", /* name_of_this */ > + false, /* la_store_sym_names_in_linkage_form_p */ > basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ > basic_lookup_transparent_type,/* lookup_transparent_type */ > NULL, /* Language specific symbol demangler */ > diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c > index 5ff80b2..2d7c1f7 100644 > --- a/gdb/rust-lang.c > +++ b/gdb/rust-lang.c > @@ -2290,6 +2290,7 @@ extern const struct language_defn rust_language_defn = > default_read_var_value, /* la_read_var_value */ > NULL, /* Language specific skip_trampoline */ > NULL, /* name_of_this */ > + false, /* la_store_sym_names_in_linkage_form_p */ > rust_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ > basic_lookup_transparent_type,/* lookup_transparent_type */ > gdb_demangle, /* Language specific symbol demangler */ > diff --git a/gdb/testsuite/gdb.ada/maint_with_ada.exp b/gdb/testsuite/gdb.ada/maint_with_ada.exp > index 73da613..9ede035 100644 > --- a/gdb/testsuite/gdb.ada/maint_with_ada.exp > +++ b/gdb/testsuite/gdb.ada/maint_with_ada.exp > @@ -32,7 +32,6 @@ gdb_breakpoint "adainit" > gdb_breakpoint "Var_Arr_Typedef" > gdb_breakpoint "Do_Nothing" > > -setup_kfail gdb/22670 "*-*-*" > gdb_test_no_output "maintenance check-psymtabs" > > gdb_test_no_output "maintenance check-symtabs" > diff --git a/gdb/testsuite/gdb.ada/notcplusplus.exp b/gdb/testsuite/gdb.ada/notcplusplus.exp > new file mode 100644 > index 0000000..b2a24e8 > --- /dev/null > +++ b/gdb/testsuite/gdb.ada/notcplusplus.exp > @@ -0,0 +1,45 @@ > +# Copyright 2018 Free Software Foundation, Inc. > +# > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see <http://www.gnu.org/licenses/>. > + > +load_lib "ada.exp" > + > +if { [skip_ada_tests] } { return -1 } > + > +standard_ada_testfile foo > + > +if {[gdb_compile_ada "${srcfile}" "${binfile}" executable {debug}] != ""} { > + return -1 > +} > + > +clean_restart ${testfile} > + > +gdb_test "print /x <symada__cS>" \ > + "= \\(a => 0x60287af\\)" \ > + "print <symada__cS> before loading symbols from ver.ads" > + > +# Force the partial symbosl from ver.ads to be expanded into full symbols. > + > +gdb_test \ > + "list ver.ads:16" \ > + [multi_line ".*" \ > + "16\\s+package Ver is" \ > + "17\\s+type Wrapper is record" \ > + "18\\s+A : Integer;" \ > + "19\\s+end record;" \ > + "20\\s+u00045 : constant Wrapper := \\(A => 16#060287af#\\);"] > + > +gdb_test "print /x <symada__cS>" \ > + "= \\(a => 0x60287af\\)" \ > + "print <symada__cS> after loading symbols from ver.ads" > diff --git a/gdb/testsuite/gdb.ada/notcplusplus/foo.adb b/gdb/testsuite/gdb.ada/notcplusplus/foo.adb > new file mode 100644 > index 0000000..89e42f9 > --- /dev/null > +++ b/gdb/testsuite/gdb.ada/notcplusplus/foo.adb > @@ -0,0 +1,21 @@ > +-- Copyright 2018 Free Software Foundation, Inc. > +-- > +-- This program is free software; you can redistribute it and/or modify > +-- it under the terms of the GNU General Public License as published by > +-- the Free Software Foundation; either version 3 of the License, or > +-- (at your option) any later version. > +-- > +-- This program is distributed in the hope that it will be useful, > +-- but WITHOUT ANY WARRANTY; without even the implied warranty of > +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +-- GNU General Public License for more details. > +-- > +-- You should have received a copy of the GNU General Public License > +-- along with this program. If not, see <http://www.gnu.org/licenses/>. > + > +with Pck; use Pck; > +with Ver; use Ver; > +procedure Foo is > +begin > + Do_Nothing (u00045'Address); > +end Foo; > diff --git a/gdb/testsuite/gdb.ada/notcplusplus/pck.adb b/gdb/testsuite/gdb.ada/notcplusplus/pck.adb > new file mode 100644 > index 0000000..dcfb306 > --- /dev/null > +++ b/gdb/testsuite/gdb.ada/notcplusplus/pck.adb > @@ -0,0 +1,21 @@ > +-- Copyright 2018 Free Software Foundation, Inc. > +-- > +-- This program is free software; you can redistribute it and/or modify > +-- it under the terms of the GNU General Public License as published by > +-- the Free Software Foundation; either version 3 of the License, or > +-- (at your option) any later version. > +-- > +-- This program is distributed in the hope that it will be useful, > +-- but WITHOUT ANY WARRANTY; without even the implied warranty of > +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +-- GNU General Public License for more details. > +-- > +-- You should have received a copy of the GNU General Public License > +-- along with this program. If not, see <http://www.gnu.org/licenses/>. > + > +package body Pck is > + procedure Do_Nothing (A : System.Address) is > + begin > + null; > + end Do_Nothing; > +end Pck; > diff --git a/gdb/testsuite/gdb.ada/notcplusplus/pck.ads b/gdb/testsuite/gdb.ada/notcplusplus/pck.ads > new file mode 100644 > index 0000000..33e369e > --- /dev/null > +++ b/gdb/testsuite/gdb.ada/notcplusplus/pck.ads > @@ -0,0 +1,19 @@ > +-- Copyright 2018 Free Software Foundation, Inc. > +-- > +-- This program is free software; you can redistribute it and/or modify > +-- it under the terms of the GNU General Public License as published by > +-- the Free Software Foundation; either version 3 of the License, or > +-- (at your option) any later version. > +-- > +-- This program is distributed in the hope that it will be useful, > +-- but WITHOUT ANY WARRANTY; without even the implied warranty of > +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +-- GNU General Public License for more details. > +-- > +-- You should have received a copy of the GNU General Public License > +-- along with this program. If not, see <http://www.gnu.org/licenses/>. > + > +with System; > +package Pck is > + procedure Do_Nothing (A : System.Address); > +end Pck; > diff --git a/gdb/testsuite/gdb.ada/notcplusplus/ver.ads b/gdb/testsuite/gdb.ada/notcplusplus/ver.ads > new file mode 100644 > index 0000000..8f264d0 > --- /dev/null > +++ b/gdb/testsuite/gdb.ada/notcplusplus/ver.ads > @@ -0,0 +1,22 @@ > +-- Copyright 2018 Free Software Foundation, Inc. > +-- > +-- This program is free software; you can redistribute it and/or modify > +-- it under the terms of the GNU General Public License as published by > +-- the Free Software Foundation; either version 3 of the License, or > +-- (at your option) any later version. > +-- > +-- This program is distributed in the hope that it will be useful, > +-- but WITHOUT ANY WARRANTY; without even the implied warranty of > +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +-- GNU General Public License for more details. > +-- > +-- You should have received a copy of the GNU General Public License > +-- along with this program. If not, see <http://www.gnu.org/licenses/>. > + > +package Ver is > + type Wrapper is record > + A : Integer; > + end record; > + u00045 : constant Wrapper := (A => 16#060287af#); > + pragma Export (C, u00045, "symada__cS"); > +end Ver; > diff --git a/gdb/testsuite/gdb.base/c-linkage-name.c b/gdb/testsuite/gdb.base/c-linkage-name.c > new file mode 100644 > index 0000000..925004c > --- /dev/null > +++ b/gdb/testsuite/gdb.base/c-linkage-name.c > @@ -0,0 +1,44 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 2018 Free Software Foundation, Inc. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program. If not, see <http://www.gnu.org/licenses/>. */ > + > +struct wrapper > +{ > + int a; > +}; > + > +/* Create a global variable whose name in the assembly code > + (aka the "linkage name") is different from the name in > + the source code. The goal is to create a symbol described > + in DWARF using a DW_AT_linkage_name attribute, with a name > + which follows the C++ mangling. > + > + In this particular case, we chose "symada__cS" which, if it were > + demangled, would translate to "symada (char, signed)". */ > +struct wrapper mundane asm ("symada__cS") = {0x060287af}; > + > +void > +do_something (struct wrapper *w) > +{ > + w->a++; > +} > + > +int > +main (void) > +{ > + do_something (&mundane); > + return 0; > +} > diff --git a/gdb/testsuite/gdb.base/c-linkage-name.exp b/gdb/testsuite/gdb.base/c-linkage-name.exp > new file mode 100644 > index 0000000..c80a530 > --- /dev/null > +++ b/gdb/testsuite/gdb.base/c-linkage-name.exp > @@ -0,0 +1,47 @@ > +# Copyright 2018 Free Software Foundation, Inc. > + > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see <http://www.gnu.org/licenses/>. > + > +# This file is part of the gdb testsuite. It is intended to test that > +# gdb can correctly print arrays with indexes for each element of the > +# array. > + > +standard_testfile .c > + > +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } { > + untested "failed to compile" > + return -1 > +} > + > +clean_restart ${binfile} > + > +# Try to print MUNDANE, but using its linkage name. > + > +gdb_test "print symada__cS" \ > + " = {a = 100829103}" \ > + "print symada__cS before partial symtab expansion" > + > +# Force the symbols to be expanded for the unit that contains > +# our symada__cS symbol by, e.g. inserting a breakpoint on one > +# of the founction also provided by the same using. > + > +gdb_test "break main" \ > + "Breakpoint $decimal at $hex: file .*$srcfile, line $decimal\\." > + > +# Try to print MUNDANE using its linkage name again, after partial > +# symtab expansion. > + > +gdb_test "print symada__cS" \ > + " = {a = 100829103}" \ > + "print symada__cS after partial symtab expansion" > -- > 2.1.4 > -- Joel ^ permalink raw reply [flat|nested] 27+ messages in thread
* PING^2: [RFA/RFC] fix PR gdb/22670 (pb looking up some symbols when they have a linkage name) (was: "Re: [RFC] regresssion(internal-error) printing subprogram argument") 2018-02-21 3:02 ` PING: " Joel Brobecker @ 2018-03-19 21:22 ` Joel Brobecker 2018-03-26 14:26 ` PING^2: [RFA/RFC] fix PR gdb/22670 (pb looking up some symbols when they have a linkage name) Pedro Alves 0 siblings, 1 reply; 27+ messages in thread From: Joel Brobecker @ 2018-03-19 21:22 UTC (permalink / raw) To: gdb-patches On Wed, Feb 21, 2018 at 07:02:11AM +0400, Joel Brobecker wrote: > > > All in all, I think a better solution would be to put that information > > > directly in the language_defn itself, via a new "attribute". > > > Something like a... > > > > I ended up choosing this approach. lookup names is still a bit > > foggy for me, so the comments I wrote and/or the name of the > > new language_defn attribute might be a bit off. As always, I am > > grateful for suggestions :). > > > > gdb/ChangeLog: > > > > PR gdb/22670 > > * dwarf2read.c (dwarf2_physname): Do not return the demangled > > symbol name is the CU's language stores symbol names in linkage > > format. > > * language.h (struct language_defn) > > <la_store_sym_names_in_linkage_form_p>: New field. Adjust > > all instances of this struct. > > > > gdb/testsuite/ChangeLog: > > > > * gdb.ada/maint_with_ada.exp: Remove PR gdb/22670 setup_kfail. > > > > * gdb.ada/notcplusplus: New testcase. > > > > * gdb.base/c-linkage-name.c: New file. > > * gdb.base/c-linkage-name.exp: New testcase. > > > > Tested on x86_64-linux. > > This also passes AdaCore's internal GDB testsuite. I'm thinking of pushing this patch at some point, since it has been waiting for a review for a quite a while, now. If it's lack of time and you'd like a little more time before I push, please let me know! > Any comment on this patch? > > Thanks! > > > >From c22aef84cc86097d6d8b654b0586a3fbc1ff6af3 Mon Sep 17 00:00:00 2001 > > From: Joel Brobecker <brobecker@adacore.com> > > Date: Fri, 9 Feb 2018 02:13:34 -0500 > > Subject: [PATCH] problem looking up some symbols when they have a linkage name > > > > This patch fixes a known failure in gdb.ada/maint_with_ada.exp > > (maintenance check-psymtabs). Another way to witness the same > > issue is by considering the following Ada declarations... > > > > type Wrapper is record > > A : Integer; > > end record; > > u00045 : constant Wrapper := (A => 16#060287af#); > > pragma Export (C, u00045, "symada__cS"); > > > > ... which declares a variable name "u00045" but with a linkage > > name which is "symada__cS". This variable is a record with one > > component, the Ada equivalent of a struct with one field in C. > > Trying to print that variable's value currently yields: > > > > (gdb) p /x <symada__cS> > > 'symada(char, signed)' has unknown type; cast it to its declared type > > > > This indicates that GDB was only able to find the minimal symbol, > > but not the full symbol. The expected output is: > > > > (gdb) print /x <symada__cS> > > $1 = (a => 0x60287af) > > > > The error message gives a hint about what's happening: We processed > > the symbol through gdb_demangle, which in the case of this particular > > symbol name, ends up matching the C++ naming scheme. As a result, > > the demangler transforms our symbol name into 'symada(char, signed)', > > thus breaking Ada lookups. > > > > This patch fixes the issue by first introducing a new language_defn > > attribute called la_store_sym_names_in_linkage_form_p, which is a boolean > > to be set to true for the few languages that do not want their symbols > > to have their names stored in demangled form, and false otherwise. > > We then use this language attribute to skip the call to gdb_demangle > > for all languages whose la_store_sym_names_in_linkage_form_p is true. > > > > In terms of the selection of languages for which the new attribute > > is set to true, the selection errs on the side of preserving the > > existing behavior, and only changes the behavior for the languages > > where we are certain storing symbol names in demangling form is not > > needed. It is conceivable that other languages might be in the same > > situation, but I not knowing in detail the symbol name enconding > > strategy, I decided to play it safe and let other language maintainers > > potentially adjust their language if it makes sense to do so. > > > > gdb/ChangeLog: > > > > PR gdb/22670 > > * dwarf2read.c (dwarf2_physname): Do not return the demangled > > symbol name is the CU's language stores symbol names in linkage > > format. > > * language.h (struct language_defn) > > <la_store_sym_names_in_linkage_form_p>: New field. Adjust > > all instances of this struct. > > > > gdb/testsuite/ChangeLog: > > > > * gdb.ada/maint_with_ada.exp: Remove PR gdb/22670 setup_kfail. > > > > * gdb.ada/notcplusplus: New testcase. > > > > * gdb.base/c-linkage-name.c: New file. > > * gdb.base/c-linkage-name.exp: New testcase. > > > > Tested on x86_64-linux. > > This also passes AdaCore's internal GDB testsuite. > > --- > > gdb/ada-lang.c | 1 + > > gdb/c-lang.c | 4 +++ > > gdb/d-lang.c | 1 + > > gdb/dwarf2read.c | 6 +++- > > gdb/f-lang.c | 1 + > > gdb/go-lang.c | 1 + > > gdb/language.c | 2 ++ > > gdb/language.h | 20 +++++++++++++ > > gdb/m2-lang.c | 1 + > > gdb/objc-lang.c | 1 + > > gdb/opencl-lang.c | 1 + > > gdb/p-lang.c | 1 + > > gdb/rust-lang.c | 1 + > > gdb/testsuite/gdb.ada/maint_with_ada.exp | 1 - > > gdb/testsuite/gdb.ada/notcplusplus.exp | 45 ++++++++++++++++++++++++++++ > > gdb/testsuite/gdb.ada/notcplusplus/foo.adb | 21 +++++++++++++ > > gdb/testsuite/gdb.ada/notcplusplus/pck.adb | 21 +++++++++++++ > > gdb/testsuite/gdb.ada/notcplusplus/pck.ads | 19 ++++++++++++ > > gdb/testsuite/gdb.ada/notcplusplus/ver.ads | 22 ++++++++++++++ > > gdb/testsuite/gdb.base/c-linkage-name.c | 44 ++++++++++++++++++++++++++++ > > gdb/testsuite/gdb.base/c-linkage-name.exp | 47 ++++++++++++++++++++++++++++++ > > 21 files changed, 259 insertions(+), 2 deletions(-) > > create mode 100644 gdb/testsuite/gdb.ada/notcplusplus.exp > > create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/foo.adb > > create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/pck.adb > > create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/pck.ads > > create mode 100644 gdb/testsuite/gdb.ada/notcplusplus/ver.ads > > create mode 100644 gdb/testsuite/gdb.base/c-linkage-name.c > > create mode 100644 gdb/testsuite/gdb.base/c-linkage-name.exp > > > > diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c > > index 0da58d9..7f99a2e 100644 > > --- a/gdb/ada-lang.c > > +++ b/gdb/ada-lang.c > > @@ -14521,6 +14521,7 @@ extern const struct language_defn ada_language_defn = { > > ada_read_var_value, /* la_read_var_value */ > > NULL, /* Language specific skip_trampoline */ > > NULL, /* name_of_this */ > > + true, /* la_store_sym_names_in_linkage_form_p */ > > ada_lookup_symbol_nonlocal, /* Looking up non-local symbols. */ > > basic_lookup_transparent_type, /* lookup_transparent_type */ > > ada_la_decode, /* Language specific symbol demangler */ > > diff --git a/gdb/c-lang.c b/gdb/c-lang.c > > index a0b553e..658c7f7 100644 > > --- a/gdb/c-lang.c > > +++ b/gdb/c-lang.c > > @@ -853,6 +853,7 @@ extern const struct language_defn c_language_defn = > > default_read_var_value, /* la_read_var_value */ > > NULL, /* Language specific skip_trampoline */ > > NULL, /* name_of_this */ > > + true, /* la_store_sym_names_in_linkage_form_p */ > > basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ > > basic_lookup_transparent_type,/* lookup_transparent_type */ > > NULL, /* Language specific symbol demangler */ > > @@ -998,6 +999,7 @@ extern const struct language_defn cplus_language_defn = > > default_read_var_value, /* la_read_var_value */ > > cplus_skip_trampoline, /* Language specific skip_trampoline */ > > "this", /* name_of_this */ > > + false, /* la_store_sym_names_in_linkage_form_p */ > > cp_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ > > cp_lookup_transparent_type, /* lookup_transparent_type */ > > gdb_demangle, /* Language specific symbol demangler */ > > @@ -1052,6 +1054,7 @@ extern const struct language_defn asm_language_defn = > > default_read_var_value, /* la_read_var_value */ > > NULL, /* Language specific skip_trampoline */ > > NULL, /* name_of_this */ > > + true, /* la_store_sym_names_in_linkage_form_p */ > > basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ > > basic_lookup_transparent_type,/* lookup_transparent_type */ > > NULL, /* Language specific symbol demangler */ > > @@ -1106,6 +1109,7 @@ extern const struct language_defn minimal_language_defn = > > default_read_var_value, /* la_read_var_value */ > > NULL, /* Language specific skip_trampoline */ > > NULL, /* name_of_this */ > > + true, /* la_store_sym_names_in_linkage_form_p */ > > basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ > > basic_lookup_transparent_type,/* lookup_transparent_type */ > > NULL, /* Language specific symbol demangler */ > > diff --git a/gdb/d-lang.c b/gdb/d-lang.c > > index e0afe48..688ae98 100644 > > --- a/gdb/d-lang.c > > +++ b/gdb/d-lang.c > > @@ -229,6 +229,7 @@ extern const struct language_defn d_language_defn = > > default_read_var_value, /* la_read_var_value */ > > NULL, /* Language specific skip_trampoline. */ > > "this", > > + false, /* la_store_sym_names_in_linkage_form_p */ > > d_lookup_symbol_nonlocal, > > basic_lookup_transparent_type, > > d_demangle, /* Language specific symbol demangler. */ > > diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c > > index d651725..b4c087f 100644 > > --- a/gdb/dwarf2read.c > > +++ b/gdb/dwarf2read.c > > @@ -11142,7 +11142,11 @@ dwarf2_physname (const char *name, struct die_info *die, struct dwarf2_cu *cu) > > if (mangled != NULL) > > { > > > > - if (cu->language == language_go) > > + if (language_def (cu->language)->la_store_sym_names_in_linkage_form_p) > > + { > > + /* Do nothing (do not demangle the symbol name). */ > > + } > > + else if (cu->language == language_go) > > { > > /* This is a lie, but we already lie to the caller new_symbol. > > new_symbol assumes we return the mangled name. > > diff --git a/gdb/f-lang.c b/gdb/f-lang.c > > index 74f5622..81922f7 100644 > > --- a/gdb/f-lang.c > > +++ b/gdb/f-lang.c > > @@ -269,6 +269,7 @@ extern const struct language_defn f_language_defn = > > default_read_var_value, /* la_read_var_value */ > > NULL, /* Language specific skip_trampoline */ > > NULL, /* name_of_this */ > > + false, /* la_store_sym_names_in_linkage_form_p */ > > cp_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ > > basic_lookup_transparent_type,/* lookup_transparent_type */ > > > > diff --git a/gdb/go-lang.c b/gdb/go-lang.c > > index 022b8de..e9cba77 100644 > > --- a/gdb/go-lang.c > > +++ b/gdb/go-lang.c > > @@ -590,6 +590,7 @@ extern const struct language_defn go_language_defn = > > default_read_var_value, /* la_read_var_value */ > > NULL, /* Language specific skip_trampoline. */ > > NULL, /* name_of_this */ > > + false, /* la_store_sym_names_in_linkage_form_p */ > > basic_lookup_symbol_nonlocal, > > basic_lookup_transparent_type, > > go_demangle, /* Language specific symbol demangler. */ > > diff --git a/gdb/language.c b/gdb/language.c > > index 0d8604b..22199e0 100644 > > --- a/gdb/language.c > > +++ b/gdb/language.c > > @@ -864,6 +864,7 @@ const struct language_defn unknown_language_defn = > > default_read_var_value, /* la_read_var_value */ > > unk_lang_trampoline, /* Language specific skip_trampoline */ > > "this", /* name_of_this */ > > + true, /* store_sym_names_in_linkage_form_p */ > > basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ > > basic_lookup_transparent_type,/* lookup_transparent_type */ > > unk_lang_demangle, /* Language specific symbol demangler */ > > @@ -915,6 +916,7 @@ const struct language_defn auto_language_defn = > > default_read_var_value, /* la_read_var_value */ > > unk_lang_trampoline, /* Language specific skip_trampoline */ > > "this", /* name_of_this */ > > + false, /* store_sym_names_in_linkage_form_p */ > > basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ > > basic_lookup_transparent_type,/* lookup_transparent_type */ > > unk_lang_demangle, /* Language specific symbol demangler */ > > diff --git a/gdb/language.h b/gdb/language.h > > index 06b42ae..029de4a 100644 > > --- a/gdb/language.h > > +++ b/gdb/language.h > > @@ -264,6 +264,26 @@ struct language_defn > > > > const char *la_name_of_this; > > > > + /* True if the symbols names should be stored in GDB's data structures > > + for minimal/partial/full symbols using their linkage (aka mangled) > > + form; false if the symbol names should be demangled first. > > + > > + Most languages implement symbol lookup by comparing the demangled > > + names, in which case it is advantageous to store that information > > + already demangled, and so would set this field to false. > > + > > + On the other hand, some languages have opted for doing symbol > > + lookups by comparing mangled names instead, for reasons usually > > + specific to the language. Those languages should set this field > > + to true. > > + > > + And finally, other languages such as C or Asm do not have > > + the concept of mangled vs demangled name, so those languages > > + should set this field to true as well, to prevent any accidental > > + demangling through an unrelated language's demangler. */ > > + > > + const bool la_store_sym_names_in_linkage_form_p; > > + > > /* This is a function that lookup_symbol will call when it gets to > > the part of symbol lookup where C looks up static and global > > variables. */ > > diff --git a/gdb/m2-lang.c b/gdb/m2-lang.c > > index 11ccab3..6e6434b 100644 > > --- a/gdb/m2-lang.c > > +++ b/gdb/m2-lang.c > > @@ -377,6 +377,7 @@ extern const struct language_defn m2_language_defn = > > default_read_var_value, /* la_read_var_value */ > > NULL, /* Language specific skip_trampoline */ > > NULL, /* name_of_this */ > > + false, /* la_store_sym_names_in_linkage_form_p */ > > basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ > > basic_lookup_transparent_type,/* lookup_transparent_type */ > > NULL, /* Language specific symbol demangler */ > > diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c > > index c714ec3..4f7dc36 100644 > > --- a/gdb/objc-lang.c > > +++ b/gdb/objc-lang.c > > @@ -389,6 +389,7 @@ extern const struct language_defn objc_language_defn = { > > default_read_var_value, /* la_read_var_value */ > > objc_skip_trampoline, /* Language specific skip_trampoline */ > > "self", /* name_of_this */ > > + false, /* la_store_sym_names_in_linkage_form_p */ > > basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ > > basic_lookup_transparent_type,/* lookup_transparent_type */ > > objc_demangle, /* Language specific symbol demangler */ > > diff --git a/gdb/opencl-lang.c b/gdb/opencl-lang.c > > index 268c3c5..9c5b90c 100644 > > --- a/gdb/opencl-lang.c > > +++ b/gdb/opencl-lang.c > > @@ -1065,6 +1065,7 @@ extern const struct language_defn opencl_language_defn = > > default_read_var_value, /* la_read_var_value */ > > NULL, /* Language specific skip_trampoline */ > > NULL, /* name_of_this */ > > + false, /* la_store_sym_names_in_linkage_form_p */ > > basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ > > basic_lookup_transparent_type,/* lookup_transparent_type */ > > NULL, /* Language specific symbol demangler */ > > diff --git a/gdb/p-lang.c b/gdb/p-lang.c > > index 03db2df..3ff7f56 100644 > > --- a/gdb/p-lang.c > > +++ b/gdb/p-lang.c > > @@ -439,6 +439,7 @@ extern const struct language_defn pascal_language_defn = > > default_read_var_value, /* la_read_var_value */ > > NULL, /* Language specific skip_trampoline */ > > "this", /* name_of_this */ > > + false, /* la_store_sym_names_in_linkage_form_p */ > > basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ > > basic_lookup_transparent_type,/* lookup_transparent_type */ > > NULL, /* Language specific symbol demangler */ > > diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c > > index 5ff80b2..2d7c1f7 100644 > > --- a/gdb/rust-lang.c > > +++ b/gdb/rust-lang.c > > @@ -2290,6 +2290,7 @@ extern const struct language_defn rust_language_defn = > > default_read_var_value, /* la_read_var_value */ > > NULL, /* Language specific skip_trampoline */ > > NULL, /* name_of_this */ > > + false, /* la_store_sym_names_in_linkage_form_p */ > > rust_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */ > > basic_lookup_transparent_type,/* lookup_transparent_type */ > > gdb_demangle, /* Language specific symbol demangler */ > > diff --git a/gdb/testsuite/gdb.ada/maint_with_ada.exp b/gdb/testsuite/gdb.ada/maint_with_ada.exp > > index 73da613..9ede035 100644 > > --- a/gdb/testsuite/gdb.ada/maint_with_ada.exp > > +++ b/gdb/testsuite/gdb.ada/maint_with_ada.exp > > @@ -32,7 +32,6 @@ gdb_breakpoint "adainit" > > gdb_breakpoint "Var_Arr_Typedef" > > gdb_breakpoint "Do_Nothing" > > > > -setup_kfail gdb/22670 "*-*-*" > > gdb_test_no_output "maintenance check-psymtabs" > > > > gdb_test_no_output "maintenance check-symtabs" > > diff --git a/gdb/testsuite/gdb.ada/notcplusplus.exp b/gdb/testsuite/gdb.ada/notcplusplus.exp > > new file mode 100644 > > index 0000000..b2a24e8 > > --- /dev/null > > +++ b/gdb/testsuite/gdb.ada/notcplusplus.exp > > @@ -0,0 +1,45 @@ > > +# Copyright 2018 Free Software Foundation, Inc. > > +# > > +# This program is free software; you can redistribute it and/or modify > > +# it under the terms of the GNU General Public License as published by > > +# the Free Software Foundation; either version 3 of the License, or > > +# (at your option) any later version. > > +# > > +# This program is distributed in the hope that it will be useful, > > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > +# GNU General Public License for more details. > > +# > > +# You should have received a copy of the GNU General Public License > > +# along with this program. If not, see <http://www.gnu.org/licenses/>. > > + > > +load_lib "ada.exp" > > + > > +if { [skip_ada_tests] } { return -1 } > > + > > +standard_ada_testfile foo > > + > > +if {[gdb_compile_ada "${srcfile}" "${binfile}" executable {debug}] != ""} { > > + return -1 > > +} > > + > > +clean_restart ${testfile} > > + > > +gdb_test "print /x <symada__cS>" \ > > + "= \\(a => 0x60287af\\)" \ > > + "print <symada__cS> before loading symbols from ver.ads" > > + > > +# Force the partial symbosl from ver.ads to be expanded into full symbols. > > + > > +gdb_test \ > > + "list ver.ads:16" \ > > + [multi_line ".*" \ > > + "16\\s+package Ver is" \ > > + "17\\s+type Wrapper is record" \ > > + "18\\s+A : Integer;" \ > > + "19\\s+end record;" \ > > + "20\\s+u00045 : constant Wrapper := \\(A => 16#060287af#\\);"] > > + > > +gdb_test "print /x <symada__cS>" \ > > + "= \\(a => 0x60287af\\)" \ > > + "print <symada__cS> after loading symbols from ver.ads" > > diff --git a/gdb/testsuite/gdb.ada/notcplusplus/foo.adb b/gdb/testsuite/gdb.ada/notcplusplus/foo.adb > > new file mode 100644 > > index 0000000..89e42f9 > > --- /dev/null > > +++ b/gdb/testsuite/gdb.ada/notcplusplus/foo.adb > > @@ -0,0 +1,21 @@ > > +-- Copyright 2018 Free Software Foundation, Inc. > > +-- > > +-- This program is free software; you can redistribute it and/or modify > > +-- it under the terms of the GNU General Public License as published by > > +-- the Free Software Foundation; either version 3 of the License, or > > +-- (at your option) any later version. > > +-- > > +-- This program is distributed in the hope that it will be useful, > > +-- but WITHOUT ANY WARRANTY; without even the implied warranty of > > +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > +-- GNU General Public License for more details. > > +-- > > +-- You should have received a copy of the GNU General Public License > > +-- along with this program. If not, see <http://www.gnu.org/licenses/>. > > + > > +with Pck; use Pck; > > +with Ver; use Ver; > > +procedure Foo is > > +begin > > + Do_Nothing (u00045'Address); > > +end Foo; > > diff --git a/gdb/testsuite/gdb.ada/notcplusplus/pck.adb b/gdb/testsuite/gdb.ada/notcplusplus/pck.adb > > new file mode 100644 > > index 0000000..dcfb306 > > --- /dev/null > > +++ b/gdb/testsuite/gdb.ada/notcplusplus/pck.adb > > @@ -0,0 +1,21 @@ > > +-- Copyright 2018 Free Software Foundation, Inc. > > +-- > > +-- This program is free software; you can redistribute it and/or modify > > +-- it under the terms of the GNU General Public License as published by > > +-- the Free Software Foundation; either version 3 of the License, or > > +-- (at your option) any later version. > > +-- > > +-- This program is distributed in the hope that it will be useful, > > +-- but WITHOUT ANY WARRANTY; without even the implied warranty of > > +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > +-- GNU General Public License for more details. > > +-- > > +-- You should have received a copy of the GNU General Public License > > +-- along with this program. If not, see <http://www.gnu.org/licenses/>. > > + > > +package body Pck is > > + procedure Do_Nothing (A : System.Address) is > > + begin > > + null; > > + end Do_Nothing; > > +end Pck; > > diff --git a/gdb/testsuite/gdb.ada/notcplusplus/pck.ads b/gdb/testsuite/gdb.ada/notcplusplus/pck.ads > > new file mode 100644 > > index 0000000..33e369e > > --- /dev/null > > +++ b/gdb/testsuite/gdb.ada/notcplusplus/pck.ads > > @@ -0,0 +1,19 @@ > > +-- Copyright 2018 Free Software Foundation, Inc. > > +-- > > +-- This program is free software; you can redistribute it and/or modify > > +-- it under the terms of the GNU General Public License as published by > > +-- the Free Software Foundation; either version 3 of the License, or > > +-- (at your option) any later version. > > +-- > > +-- This program is distributed in the hope that it will be useful, > > +-- but WITHOUT ANY WARRANTY; without even the implied warranty of > > +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > +-- GNU General Public License for more details. > > +-- > > +-- You should have received a copy of the GNU General Public License > > +-- along with this program. If not, see <http://www.gnu.org/licenses/>. > > + > > +with System; > > +package Pck is > > + procedure Do_Nothing (A : System.Address); > > +end Pck; > > diff --git a/gdb/testsuite/gdb.ada/notcplusplus/ver.ads b/gdb/testsuite/gdb.ada/notcplusplus/ver.ads > > new file mode 100644 > > index 0000000..8f264d0 > > --- /dev/null > > +++ b/gdb/testsuite/gdb.ada/notcplusplus/ver.ads > > @@ -0,0 +1,22 @@ > > +-- Copyright 2018 Free Software Foundation, Inc. > > +-- > > +-- This program is free software; you can redistribute it and/or modify > > +-- it under the terms of the GNU General Public License as published by > > +-- the Free Software Foundation; either version 3 of the License, or > > +-- (at your option) any later version. > > +-- > > +-- This program is distributed in the hope that it will be useful, > > +-- but WITHOUT ANY WARRANTY; without even the implied warranty of > > +-- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > +-- GNU General Public License for more details. > > +-- > > +-- You should have received a copy of the GNU General Public License > > +-- along with this program. If not, see <http://www.gnu.org/licenses/>. > > + > > +package Ver is > > + type Wrapper is record > > + A : Integer; > > + end record; > > + u00045 : constant Wrapper := (A => 16#060287af#); > > + pragma Export (C, u00045, "symada__cS"); > > +end Ver; > > diff --git a/gdb/testsuite/gdb.base/c-linkage-name.c b/gdb/testsuite/gdb.base/c-linkage-name.c > > new file mode 100644 > > index 0000000..925004c > > --- /dev/null > > +++ b/gdb/testsuite/gdb.base/c-linkage-name.c > > @@ -0,0 +1,44 @@ > > +/* This testcase is part of GDB, the GNU debugger. > > + > > + Copyright 2018 Free Software Foundation, Inc. > > + > > + This program is free software; you can redistribute it and/or modify > > + it under the terms of the GNU General Public License as published by > > + the Free Software Foundation; either version 3 of the License, or > > + (at your option) any later version. > > + > > + This program is distributed in the hope that it will be useful, > > + but WITHOUT ANY WARRANTY; without even the implied warranty of > > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + GNU General Public License for more details. > > + > > + You should have received a copy of the GNU General Public License > > + along with this program. If not, see <http://www.gnu.org/licenses/>. */ > > + > > +struct wrapper > > +{ > > + int a; > > +}; > > + > > +/* Create a global variable whose name in the assembly code > > + (aka the "linkage name") is different from the name in > > + the source code. The goal is to create a symbol described > > + in DWARF using a DW_AT_linkage_name attribute, with a name > > + which follows the C++ mangling. > > + > > + In this particular case, we chose "symada__cS" which, if it were > > + demangled, would translate to "symada (char, signed)". */ > > +struct wrapper mundane asm ("symada__cS") = {0x060287af}; > > + > > +void > > +do_something (struct wrapper *w) > > +{ > > + w->a++; > > +} > > + > > +int > > +main (void) > > +{ > > + do_something (&mundane); > > + return 0; > > +} > > diff --git a/gdb/testsuite/gdb.base/c-linkage-name.exp b/gdb/testsuite/gdb.base/c-linkage-name.exp > > new file mode 100644 > > index 0000000..c80a530 > > --- /dev/null > > +++ b/gdb/testsuite/gdb.base/c-linkage-name.exp > > @@ -0,0 +1,47 @@ > > +# Copyright 2018 Free Software Foundation, Inc. > > + > > +# This program is free software; you can redistribute it and/or modify > > +# it under the terms of the GNU General Public License as published by > > +# the Free Software Foundation; either version 3 of the License, or > > +# (at your option) any later version. > > +# > > +# This program is distributed in the hope that it will be useful, > > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > +# GNU General Public License for more details. > > +# > > +# You should have received a copy of the GNU General Public License > > +# along with this program. If not, see <http://www.gnu.org/licenses/>. > > + > > +# This file is part of the gdb testsuite. It is intended to test that > > +# gdb can correctly print arrays with indexes for each element of the > > +# array. > > + > > +standard_testfile .c > > + > > +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } { > > + untested "failed to compile" > > + return -1 > > +} > > + > > +clean_restart ${binfile} > > + > > +# Try to print MUNDANE, but using its linkage name. > > + > > +gdb_test "print symada__cS" \ > > + " = {a = 100829103}" \ > > + "print symada__cS before partial symtab expansion" > > + > > +# Force the symbols to be expanded for the unit that contains > > +# our symada__cS symbol by, e.g. inserting a breakpoint on one > > +# of the founction also provided by the same using. > > + > > +gdb_test "break main" \ > > + "Breakpoint $decimal at $hex: file .*$srcfile, line $decimal\\." > > + > > +# Try to print MUNDANE using its linkage name again, after partial > > +# symtab expansion. > > + > > +gdb_test "print symada__cS" \ > > + " = {a = 100829103}" \ > > + "print symada__cS after partial symtab expansion" > > -- > > 2.1.4 > > > > > -- > Joel -- Joel ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: PING^2: [RFA/RFC] fix PR gdb/22670 (pb looking up some symbols when they have a linkage name) 2018-03-19 21:22 ` PING^2: " Joel Brobecker @ 2018-03-26 14:26 ` Pedro Alves 2018-03-27 14:02 ` Joel Brobecker 0 siblings, 1 reply; 27+ messages in thread From: Pedro Alves @ 2018-03-26 14:26 UTC (permalink / raw) To: Joel Brobecker, gdb-patches Hi Joel, On 03/19/2018 09:21 PM, Joel Brobecker wrote: > On Wed, Feb 21, 2018 at 07:02:11AM +0400, Joel Brobecker wrote: >>>> All in all, I think a better solution would be to put that information >>>> directly in the language_defn itself, via a new "attribute". >>>> Something like a... >>> >>> I ended up choosing this approach. lookup names is still a bit >>> foggy for me, so the comments I wrote and/or the name of the >>> new language_defn attribute might be a bit off. As always, I am >>> grateful for suggestions :). I looked again at this today, and I couldn't think of a better option right now. So I think you should go ahead and push this in. Note that this C++ mangled form like in "symada__cS", is not actually how C++ symbols are mangled nowadays. The modern Itanium mangling scheme has linkage names that always start with "_Z". In this case, "void symada(char, signed)" mangles as "_Z6symadaci". It's possible that we no longer need to support that older (and ambiguous with Ada and other languages) mangling scheme for C++, I don't know. I wish Ada linkage names also had some kind of leading prefix, but that can't happen without an ABI break, of course... Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: PING^2: [RFA/RFC] fix PR gdb/22670 (pb looking up some symbols when they have a linkage name) 2018-03-26 14:26 ` PING^2: [RFA/RFC] fix PR gdb/22670 (pb looking up some symbols when they have a linkage name) Pedro Alves @ 2018-03-27 14:02 ` Joel Brobecker 0 siblings, 0 replies; 27+ messages in thread From: Joel Brobecker @ 2018-03-27 14:02 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches > >>> I ended up choosing this approach. lookup names is still a bit > >>> foggy for me, so the comments I wrote and/or the name of the > >>> new language_defn attribute might be a bit off. As always, I am > >>> grateful for suggestions :). > > I looked again at this today, and I couldn't think of a better > option right now. So I think you should go ahead and push this in. Thanks, Pedro. I re-tested the patch on x86_64-linux, and then pushed it to master. > Note that this C++ mangled form like in "symada__cS", is not > actually how C++ symbols are mangled nowadays. The modern Itanium > mangling scheme has linkage names that always start with "_Z". In this > case, "void symada(char, signed)" mangles as "_Z6symadaci". Interesting. > It's possible that we no longer need to support that older (and > ambiguous with Ada and other languages) mangling scheme for C++, I > don't know. I wish Ada linkage names also had some kind of leading > prefix, but that can't happen without an ABI break, of course... Yeah, it's a good idea, but making that kind of change happen is going to be tough, especially since we know there are consumers of our debugging information that we have no control of. I'll mention it to the compiler team, though. Thanks Pedro. -- Joel ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument 2018-01-17 9:13 ` Joel Brobecker 2018-01-26 3:51 ` Joel Brobecker @ 2018-01-26 4:50 ` Joel Brobecker 1 sibling, 0 replies; 27+ messages in thread From: Joel Brobecker @ 2018-01-26 4:50 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches [-- Attachment #1: Type: text/plain, Size: 2831 bytes --] > > I'm curious to know if after all the fixing you still see > > the KFAIL on your end. I get: > > > > KPASS: gdb.ada/maint_with_ada.exp: maintenance check-psymtabs (PRMS gdb/22670) > > Strange, I still get the error: > > (gdb) maintenance check-psymtabs > Global symbol `interfaces__cS' only found in /[...]/gdb.ada/maint_with_ada/b~var_arr_typedef.adb psymtab > (gdb) KFAIL: gdb.ada/maint_with_ada.exp: maintenance check-psymtabs (PRMS: gdb/22670) For the sake of this investigation, I used a simpler program: | $ cat a.adb | procedure A is | begin | null; | end A; The symbol in question is declared in b~a.ads as follow: | u00045 : constant Version_32 := 16#f60287af#; | pragma Export (C, u00045, "interfaces__cS"); The debugging information for this variable looks like this: | <1><a9f>: Abbrev Number: 35 (DW_TAG_variable) | <aa0> DW_AT_name : (indirect string, offset: 0x3fd): ada_main__u00045 | <aa4> DW_AT_decl_file : 5 | <aa5> DW_AT_decl_line : 128 | <aa6> DW_AT_linkage_name: (indirect string, offset: 0x198c): interfaces__cS | <aaa> DW_AT_type : <0x79> | <aae> DW_AT_external : 1 | <aae> DW_AT_location : 9 byte block: 3 80 e5 41 0 0 0 0 0 (DW_OP_addr: 41e580) The important bit is that it has a DW_AT_linkage_name attribute. When we look at dwarf2read.c::new_symbol, we can see: | /* Cache this symbol's name and the name's demangled form (if any). */ | SYMBOL_SET_LANGUAGE (sym, cu->language, &objfile->objfile_obstack); | linkagename = dwarf2_physname (name, die, cu); | SYMBOL_SET_NAMES (sym, linkagename, strlen (linkagename), 0, objfile); In our case, LINKAGENAMES is "interfaces(char, signed)"; as you intuitively guessed, C++ demangling was applied on the symbol. Following that thread and looking at dwarf2_physname, we see that it basically calls gdb_demangle, and that works, then returns its value as the physname. Continuing on, gdb_demangle is just a wrapper for bfd_demangle, and neither take any language information as a parameter. And looking at bfd_demangle I found that it is just a wrapper around cplus_demangle. Since Ada does not follow the same encoding technique, this should not be done for Ada symbols. Attached is a prototype patch which hacks that idea into the current code. It was tested on x86_64-linux, and both fixes the KFAIL and shows no regression. I don't understand the command regarding go; it was just a convenient location for adding the condition. The part that worries me is that we have several other other locations where gdb_demangle is being called. Half of them are from language- specific areas, so probably not an issue for Ada. But I think I will review the other ones... To be continued. Thoughts? -- Joel [-- Attachment #2: 0001-WIP-dwarf2read.c-dwarf2_physname-do-not-call-gdb_dem.patch --] [-- Type: text/x-diff, Size: 883 bytes --] From d98976141c88b05f414bc8975ee2d77e3e655708 Mon Sep 17 00:00:00 2001 From: Joel Brobecker <brobecker@adacore.com> Date: Thu, 25 Jan 2018 23:23:54 -0500 Subject: [PATCH] WIP: dwarf2read.c::dwarf2_physname: do not call gdb_demangle with Ada symbols --- gdb/dwarf2read.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index 96026a8..dfed170 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -11127,7 +11127,7 @@ dwarf2_physname (const char *name, struct die_info *die, struct dwarf2_cu *cu) variant `long name(params)' does not have the proper inferior type. */ - if (cu->language == language_go) + if (cu->language == language_go || cu->language == language_ada) { /* This is a lie, but we already lie to the caller new_symbol. new_symbol assumes we return the mangled name. -- 2.1.4 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument 2017-12-15 9:48 ` Joel Brobecker 2017-12-15 11:02 ` Pedro Alves @ 2017-12-15 18:31 ` Pedro Alves 1 sibling, 0 replies; 27+ messages in thread From: Pedro Alves @ 2017-12-15 18:31 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches On 12/15/2017 09:47 AM, Joel Brobecker wrote: > However, there are plenty of other similar symbols, for instance: > > <1><b04>: Abbrev Number: 35 (DW_TAG_variable) > <b05> DW_AT_name : (indirect string, offset: 0x4b9): ada_main__u00049 > <b09> DW_AT_decl_file : 5 > <b0a> DW_AT_decl_line : 136 > <b0b> DW_AT_linkage_name: (indirect string, offset: 0x17cc): system__bounded_stringsS > <b0f> DW_AT_type : <0x79> > <b13> DW_AT_external : 1 > <b13> DW_AT_location : 9 byte block: 3 28 1 0 0 0 0 0 0 (DW_OP_addr: 128) > > So I'm still not sure what makes interfaces__cS special. I will look > into it when I have a chance... I wonder whether it's because it can demangle as a C++ symbol (using some older mangling scheme): $ echo interfaces__cS | c++filt interfaces(char, signed) Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC] regresssion(internal-error) printing subprogram argument 2017-12-14 19:02 ` Pedro Alves 2017-12-14 19:41 ` Pedro Alves @ 2017-12-15 7:54 ` Joel Brobecker 1 sibling, 0 replies; 27+ messages in thread From: Joel Brobecker @ 2017-12-15 7:54 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches > I gave the "literal" lookup type a try today, and it turns out > not being so bad, I think. See patch below. Thanks! > Thinking through this and experimenting a bit, I think I convinced > myself that with the current symbol tables infrastructure, > it's not literal _linkage_ names that we want to pass down, but > instead literal _search_ names. I.e., notice that I've switched > the lookup_symbol call in question to pass down > SYMBOL_SEARCH_NAME instead of SYMBOL_LINKAGE_NAME. See > comments in symtab.h in the patch. I forgot about how the hashes were calculated, so what you are saying makes sense. I did a round of testing, both with the official testsuite as well as ours, and I confirm what you see: issue fixed with no regression. I reviewed the patch as well, and it looks good. In particular, the choices you made in terms of what type of lookup to do, and what name to pass made sense to me. > The patch is not complete in the sense that there are > symbol-lookup entry points that could be tweaked to pass > down a symbol_name_match_type instead of assuming FULL. > > And also, I know there are places in ada-lang.c that are doing > what you were doing here too (the "<...>" verbatim trick) which > could be adjusted. But at least this fixes your testcase too, > and causes no regressions. So maybe we could do this incrementally > and pass adjust functions to pass around a symbol_name_match_type > as we find a need? Functions that we miss passing down simply > end up behaving like symbols_name_match_type::FULL, as today. I like this approach. I'll try going through ada-lang.c after this change gets in to see if I can spot some of the "<..."> tricks, and simplify them by using the new symbols_name_match_type instead. -- Joel ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2018-03-27 14:02 UTC | newest] Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-12-13 10:37 [RFC] regresssion(internal-error) printing subprogram argument Joel Brobecker 2017-12-14 19:02 ` Pedro Alves 2017-12-14 19:41 ` Pedro Alves 2017-12-15 9:48 ` Joel Brobecker 2017-12-15 11:02 ` Pedro Alves 2017-12-15 18:57 ` Pedro Alves 2018-01-03 4:33 ` Joel Brobecker 2018-01-04 9:48 ` Joel Brobecker 2018-01-04 12:39 ` Pedro Alves 2018-01-05 16:28 ` Pedro Alves 2018-01-16 11:54 ` Pedro Alves 2018-01-17 9:13 ` Joel Brobecker 2018-01-26 3:51 ` Joel Brobecker 2018-01-26 11:28 ` Pedro Alves 2018-01-29 10:38 ` Joel Brobecker 2018-01-29 15:33 ` Pedro Alves 2018-01-29 15:52 ` Joel Brobecker 2018-01-30 3:56 ` Joel Brobecker 2018-02-05 9:57 ` Joel Brobecker 2018-02-09 9:09 ` [RFA/RFC] fix PR gdb/22670 (pb looking up some symbols when they have a linkage name) (was: "Re: [RFC] regresssion(internal-error) printing subprogram argument") Joel Brobecker 2018-02-21 3:02 ` PING: " Joel Brobecker 2018-03-19 21:22 ` PING^2: " Joel Brobecker 2018-03-26 14:26 ` PING^2: [RFA/RFC] fix PR gdb/22670 (pb looking up some symbols when they have a linkage name) Pedro Alves 2018-03-27 14:02 ` Joel Brobecker 2018-01-26 4:50 ` [RFC] regresssion(internal-error) printing subprogram argument Joel Brobecker 2017-12-15 18:31 ` Pedro Alves 2017-12-15 7:54 ` Joel Brobecker
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).