public inbox for gdb-testers@sourceware.org help / color / mirror / Atom feed
From: sergiodj+buildbot@sergiodj.net To: gdb-testers@sourceware.org Subject: [binutils-gdb/gdb-8.1-branch] Fix regresssion(internal-error) printing subprogram argument (PR gdb/22670) Date: Fri, 05 Jan 2018 18:31:00 -0000 [thread overview] Message-ID: <a76cf1f43d27f022562d9e70eb91e212dbd0da4a@gdb-build> (raw) *** TEST RESULTS FOR COMMIT a76cf1f43d27f022562d9e70eb91e212dbd0da4a *** Author: Pedro Alves <palves@redhat.com> Branch: gdb-8.1-branch Commit: a76cf1f43d27f022562d9e70eb91e212dbd0da4a 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.
next reply other threads:[~2018-01-05 18:31 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-01-05 18:31 sergiodj+buildbot [this message] 2018-01-05 18:31 ` *** COMPILATION FAILED *** Failures on Fedora-x86_64-m64, branch gdb-8.1-branch *** BREAKAGE *** sergiodj+buildbot 2018-01-05 18:32 ` *** COMPILATION FAILED *** Failures on Fedora-x86_64-native-gdbserver-m32, " sergiodj+buildbot 2018-01-05 18:33 ` *** COMPILATION FAILED *** Failures on Fedora-i686, " sergiodj+buildbot 2018-01-05 18:35 ` *** COMPILATION FAILED *** Failures on Fedora-x86_64-m32, " sergiodj+buildbot 2018-01-05 18:36 ` *** COMPILATION FAILED *** Failures on Fedora-x86_64-native-extended-gdbserver-m32, " sergiodj+buildbot 2018-01-05 18:37 ` *** COMPILATION FAILED *** Failures on Fedora-x86_64-cc-with-index, " sergiodj+buildbot 2018-01-05 18:39 ` *** COMPILATION FAILED *** Failures on Fedora-x86_64-native-gdbserver-m64, " sergiodj+buildbot 2018-01-05 18:39 ` *** COMPILATION FAILED *** Failures on Fedora-x86_64-native-extended-gdbserver-m64, " sergiodj+buildbot 2018-01-05 20:36 ` *** COMPILATION FAILED *** Failures on Ubuntu-AArch64-native-gdbserver-m64, " sergiodj+buildbot 2018-01-05 20:45 ` *** COMPILATION FAILED *** Failures on Ubuntu-AArch64-m64, " sergiodj+buildbot 2018-01-06 0:16 ` *** COMPILATION FAILED *** Failures on Ubuntu-AArch32-native-extended-gdbserver-m32, " sergiodj+buildbot 2018-01-06 0:26 ` *** COMPILATION FAILED *** Failures on Ubuntu-AArch32-native-gdbserver-m32, " sergiodj+buildbot 2018-01-06 0:36 ` *** COMPILATION FAILED *** Failures on Ubuntu-AArch32-m32, " sergiodj+buildbot
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=a76cf1f43d27f022562d9e70eb91e212dbd0da4a@gdb-build \ --to=sergiodj+buildbot@sergiodj.net \ --cc=gdb-testers@sourceware.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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).