From patchwork Wed Nov 22 16:48:16 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 24447 Received: (qmail 112849 invoked by alias); 22 Nov 2017 16:48:39 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 112812 invoked by uid 89); 22 Nov 2017 16:48:35 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.2 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_STOCKGEN, KB_WAM_FROM_NAME_SINGLEWORD, SPF_HELO_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=keith, comprehensive, AAA, Break X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 22 Nov 2017 16:48:25 +0000 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8608661D0D for ; Wed, 22 Nov 2017 16:48:18 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1DF0D4DA68; Wed, 22 Nov 2017 16:48:16 +0000 (UTC) Subject: Re: [PATCH 32/40] Make "break foo" find "A::foo", A::B::foo", etc. [C++ and wild matching] To: Keith Seitz , gdb-patches@sourceware.org References: <1496406158-12663-1-git-send-email-palves@redhat.com> <1496406158-12663-33-git-send-email-palves@redhat.com> <024edecc-7bfa-08a4-de46-3536297f0654@redhat.com> From: Pedro Alves Message-ID: <943402c5-0bbb-8ff7-66e3-5522256fbc1e@redhat.com> Date: Wed, 22 Nov 2017 16:48:16 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <024edecc-7bfa-08a4-de46-3536297f0654@redhat.com> Hi Keith, Thanks again for the review! And sorry for the belated replies... I'm finally coming around to this, seeing about getting it into GDB 8.1, now that the lookup_name_info prereq is merged. (Joel wants to branch next week.) Please see question below. On 08/09/2017 12:48 AM, Keith Seitz wrote: > On 06/02/2017 05:22 AM, Pedro Alves wrote: > >> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c >> index 70c0e02..328b6db 100644 >> --- a/gdb/breakpoint.c >> +++ b/gdb/breakpoint.c >> @@ -15694,7 +15694,10 @@ Explicit locations are similar to linespecs but use an option/argument\n\ >> syntax to specify location parameters.\n\ >> Example: To specify the start of the label named \"the_top\" in the\n\ >> function \"fact\" in the file \"factorial.c\", use \"-source factorial.c\n\ >> --function fact -label the_top\".\n" >> +-function fact -label the_top\".\n\ >> +For C++, \"-function\" matches all functions with the given name ignoring\n\ > ^ > comma > Added. >> +missing leading specifiers (namespaces and classes). You can override\n\ >> +that by instead specifying a fully qualified name using \"-qualified\".\n" > > I think this would read better if it read: "This behavior may be overridden > by using the \"-qualified\" flag and specifying a fully qualified name." > [I am not a fan of using informal writing in documentation.] How about the even simpler: @@ -15295,7 +15295,10 @@ Explicit locations are similar to linespecs but use an option/argument\n\ syntax to specify location parameters.\n\ Example: To specify the start of the label named \"the_top\" in the\n\ function \"fact\" in the file \"factorial.c\", use \"-source factorial.c\n\ --function fact -label the_top\".\n" +-function fact -label the_top\".\n\ +For C++, \"-function\" matches functions and methods by name, ignoring\n\ +missing leading specifiers (namespaces and classes).\n\ +\"-qualified\" matches functions and methods by fully qualified name.\n" > >> /* This help string is used for the break, hbreak, tbreak and thbreak >> commands. It is defined as a macro to prevent duplication. >> diff --git a/gdb/completer.c b/gdb/completer.c >> index eabbce7..99e40a3 100644 >> --- a/gdb/completer.c >> +++ b/gdb/completer.c >> @@ -609,6 +612,7 @@ static const char *const explicit_options[] = >> { >> "-source", >> "-function", >> + "-qualified", >> "-line", >> "-label", >> NULL > > The "-qualified" option can be used with linespecs, too, right? Not really, no. > > (gdb) b -qualified A::b (a linespec location) > (gdb) b -qualified -function A::b (an explicit location) > > Actually, I see that it does not work (yet?). Consider: > > 1 struct A > 2 { > 3 int doit () > 4 { > 5 int i; > 6 > 7 for (i = 0; i < 10; ++i) > 8 { > 9 switch (i) > 10 { > 11 top: > 12 case 5: > 13 ++i; > 14 goto top; > 15 default: break; > 16 } > 17 } > 18 return i; > 19 } > 20 }; > > (gdb) b A::doit:top > Breakpoint 1 at 0x400633: file simple-label.cc, line 11. > (gdb) b -function A::doit -label top > Note: breakpoint 1 also set at pc 0x400633. > Breakpoint 2 at 0x400633: file simple-label.cc, line 11. > (gdb) b -qualified A::doit:top > Function "A::doit:top" not defined. > Make breakpoint pending on future shared library load? (y or [n]) n > (gdb) b -qualified A::doit -label top > Note: breakpoints 1 and 2 also set at pc 0x400633. > Breakpoint 3 at 0x400633: file simple-label.cc, line 11. > > Is there a reason to exclude linespecs? Perhaps naively, I would have thought to > make -qualified a parsing option instead of a replacement for -function. I went this route because it seemed to me that a separate option would end up being less convenient, because it forces you to type/combine two options: (gdb) b -q -f A::doit:top and I thought that since you're requiring an option, and explicit linespecs are just superior to regular linespecs, that it'd be OK to move users to explicit linespecs if they need a qualified name. Also, I thought that a separate option may cause confusion like this: (gdb) b -f func[TAB] # completes ignoring missing qualifiers *hmm, not what I wanted. *press up *add "-q" *move left 3 chars, to complete the function again. *press TAB again (gdb) b -f func[TAB] -q # still completes ignoring missing qualifiers *oh right, I need to put -q before -f ... (gdb) b -q -f func[TAB] # _now_ completes a fully qualified name. Dunno. Do you see "-qualified" being an alternative to "-function" instead of a flag as a blocker? Please let me know. > >> diff --git a/gdb/cp-support.c b/gdb/cp-support.c >> index 397c738..84d8a6b 100644 >> --- a/gdb/cp-support.c >> +++ b/gdb/cp-support.c >> @@ -1671,19 +1760,151 @@ cp_fq_symbol_name_matches (const char *symbol_search_name, >> return false; >> } >> >> +/* C++ symbol_name_matcher_ftype implementation for wild matches. >> + Defers work to cp_symbol_name_ncmp. */ > ^^^^^^^^^^^^^^^^^^^ > > I think that's supposed to be cp_symbol_name_matches_1. Indeed. >> --- a/gdb/cp-support.h >> +++ b/gdb/cp-support.h >> @@ -110,6 +110,8 @@ extern struct symbol **make_symbol_overload_list_adl (struct type **arg_types, >> extern struct type *cp_lookup_rtti_type (const char *name, >> struct block *block); >> >> +extern unsigned int cp_search_name_hash (const char *search_name); >> + > > Shouldn't the comment from cp-support.c be here? Fixed. > >> extern symbol_name_matcher_ftype *cp_get_symbol_name_matcher >> (const lookup_name_info &lookup_name); >> > [snip] > > WDYT? Here's the current/updated patch. From 311e7f279219840722871c57244ecbf405a46f7e Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Wed, 22 Nov 2017 16:33:22 +0000 Subject: [PATCH] Make "break foo" find "A::foo", A::B::foo", etc. [C++ and wild matching] Finally, this patch teaches GDB about setting breakpoints ignoring namespaces, etc. by default. Here's a contrived example: (gdb) b func (anonymous namespace)::A::function() Bn::(anonymous namespace)::B::function() function(int, int) (anonymous namespace)::B::function() Bn::(anonymous namespace)::function() gdb::(anonymous namespace)::A::function() (anonymous namespace)::B::function() const Bn::(anonymous namespace)::function(int, int) gdb::(anonymous namespace)::function() (anonymous namespace)::function() Bn::B::func() gdb::(anonymous namespace)::function(int, int) (anonymous namespace)::function(int, int) Bn::B::function() gdb::A::func() A::func() Bn::func() gdb::A::function() A::function() Bn::function() gdb::func() B::func() Bn::function(int, int) gdb::function() B::function() Bn::function(long) gdb::function(int, int) B::function() const func() gdb::function(long) B::function_const() const function() (gdb) b function Breakpoint 1 at 0x4005ce: function. (26 locations) (gdb) b B::function (anonymous namespace)::B::function() B::function() const Bn::B::function() (anonymous namespace)::B::function() const B::function_const() const B::function() Bn::(anonymous namespace)::B::function() (gdb) b B::function Breakpoint 1 at 0x40072c: B::function. (6 locations) Notice that the symbols that the completer finds matches the number of symbols that settting a breakpoint finds. The testsuite patch will add extensive and comprehensive tests to make sure of that in many cases. To get back the original behavior of interpreting the function name as a fully-qualified name, you use the new "-qualified" explicit linespec option. For example: (gdb) b B::function (anonymous namespace)::B::function() B::function() const Bn::B::function() (anonymous namespace)::B::function() const B::function_const() const B::function() Bn::(anonymous namespace)::B::function() vs: (gdb) b -qualified B::function B::function() B::function() const B::function_const() const I've chosen "qualified" because "f" is already taken, for "-function". To better understand how this is the better default, consider what happens when we get to the point when _all_ of GDB is wrapped under "namespace gdb {}". I have a patch series that does that, and when I started debugging that GDB, I immediately became frustrated. You now have to write "b gdb::internal_error", "b gdb::foo", "b gdb::bar", etc. etc., which gets annoying pretty quickly. OTOH, consider how this makes it very easy to set breakpoints in classes wrapped in anonymous namespaces. You just don't think of them, GDB finds the symbols for you automatically. Implementation-wise, what the patch does is: - make C++ symbol name hashing only consider the last component of a symbol name. - add a symbol name matcher for symbol_name_match_type::WILD. - add unit tests. - adjust a few tests to use "-qualified" when they mean it. gdb/ChangeLog: yyyy-mm-dd Pedro Alves * breakpoint.c (LOCATION_HELP_STRING): Mention "-qualified". * c-lang.c (cplus_language_defn): Install cp_search_name_hash. * completer.c (explicit_location_match_type::MATCH_QUALIFIED): New enumerator. (explicit_options): Add "-qualified". (collect_explicit_location_matches): Handle MATCH_QUALIFIED. * cp-support.c (cp_search_name_hash, cp_symbol_name_matches_1) (cp_symbol_name_matches): New functions. (cp_get_symbol_name_matcher): Return different matchers depending on the lookup name's match type. (selftests::test_cp_symbol_name_matches): New function. (_initialize_cp_support): Register it. * cp-support.h (cp_search_name_hash): New declaration. * dwarf2read.c (selftests::dw2_expand_symtabs_matching::test_symbols): Add symbols. (selftests::dw2_expand_symtabs_matching::run_test): Add wild matching tests. * linespec.c (linespec_parse_basic): Lookup function symbols using symbol_name_match_type::WILD. (convert_explicit_location_to_linespec): New symbol_name_match_type parameter. Pass it down to find_linespec_symbols. (convert_explicit_location_to_sals): Pass the location's name match type to convert_explicit_location_to_linespec. (linespec_complete_function): New symbol_name_match_type parameter. Use it. (complete_linespec_component): Complete symbols using symbol_name_match_type::WILD. (linespec_complete_label): New symbol_name_match_type parameter. Use it. (linespec_complete): Complete symbols using symbol_name_match_type::WILD. (find_function_symbols, find_linespec_symbols): New symbol_name_match_type parameter. Pass it down. * linespec.h (linespec_complete_function) (linespec_complete_label): New symbol_name_match_type parameter. * location.c (explicit_to_string_internal) (string_to_explicit_location): Handle "-qualified". * location.h (explicit_location::func_name_match_type): New field. gdb/testsuite/ChangeLog: yyyy-mm-dd Pedro Alves * gdb.base/langs.exp: Use -qualified. * gdb.cp/meth-typedefs.exp: Use -qualified, and add tests without it. * gdb.cp/namespace.exp: Use -qualified. --- gdb/breakpoint.c | 5 +- gdb/c-lang.c | 2 +- gdb/completer.c | 7 ++ gdb/cp-support.c | 223 ++++++++++++++++++++++++++++++++- gdb/cp-support.h | 7 ++ gdb/dwarf2read.c | 48 +++++++ gdb/linespec.c | 24 ++-- gdb/linespec.h | 9 +- gdb/location.c | 18 ++- gdb/location.h | 3 + gdb/testsuite/gdb.base/langs.exp | 2 +- gdb/testsuite/gdb.cp/meth-typedefs.exp | 39 +++++- gdb/testsuite/gdb.cp/namespace.exp | 2 +- 13 files changed, 366 insertions(+), 23 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index d8d0ed0..2d794ff 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -15295,7 +15295,10 @@ Explicit locations are similar to linespecs but use an option/argument\n\ syntax to specify location parameters.\n\ Example: To specify the start of the label named \"the_top\" in the\n\ function \"fact\" in the file \"factorial.c\", use \"-source factorial.c\n\ --function fact -label the_top\".\n" +-function fact -label the_top\".\n\ +For C++, \"-function\" matches functions and methods by name, ignoring\n\ +missing leading specifiers (namespaces and classes).\n\ +\"-qualified\" matches functions and methods by fully qualified name.\n" /* This help string is used for the break, hbreak, tbreak and thbreak commands. It is defined as a macro to prevent duplication. diff --git a/gdb/c-lang.c b/gdb/c-lang.c index 49077c7..8d96f94 100644 --- a/gdb/c-lang.c +++ b/gdb/c-lang.c @@ -1016,7 +1016,7 @@ extern const struct language_defn cplus_language_defn = c_watch_location_expression, cp_get_symbol_name_matcher, iterate_over_symbols, - default_search_name_hash, + cp_search_name_hash, &cplus_varobj_ops, NULL, NULL, diff --git a/gdb/completer.c b/gdb/completer.c index 0c50459..aa0f105 100644 --- a/gdb/completer.c +++ b/gdb/completer.c @@ -76,6 +76,9 @@ enum explicit_location_match_type /* The name of a function or method. */ MATCH_FUNCTION, + /* The fully-qualified name of a function or method. */ + MATCH_QUALIFIED, + /* A line number. */ MATCH_LINE, @@ -602,6 +605,7 @@ static const char *const explicit_options[] = { "-source", "-function", + "-qualified", "-line", "-label", NULL @@ -653,9 +657,11 @@ collect_explicit_location_matches (completion_tracker &tracker, break; case MATCH_FUNCTION: + case MATCH_QUALIFIED: { const char *function = string_or_empty (explicit_loc->function_name); linespec_complete_function (tracker, function, + explicit_loc->func_name_match_type, explicit_loc->source_filename); } break; @@ -670,6 +676,7 @@ collect_explicit_location_matches (completion_tracker &tracker, linespec_complete_label (tracker, language, explicit_loc->source_filename, explicit_loc->function_name, + explicit_loc->func_name_match_type, label); } break; diff --git a/gdb/cp-support.c b/gdb/cp-support.c index a917ada..5f077ac 100644 --- a/gdb/cp-support.c +++ b/gdb/cp-support.c @@ -1615,7 +1615,92 @@ gdb_sniff_from_mangled_name (const char *mangled, char **demangled) return *demangled != NULL; } -/* C++ symbol_name_matcher_ftype implementation. */ +/* See cp-support.h. */ + +unsigned int +cp_search_name_hash (const char *search_name) +{ + /* cp_entire_prefix_len assumes a fully-qualified name with no + leading "::". */ + if (startswith (search_name, "::")) + search_name += 2; + + unsigned int prefix_len = cp_entire_prefix_len (search_name); + if (prefix_len != 0) + search_name += prefix_len + 2; + + return default_search_name_hash (search_name); +} + +/* Helper for cp_symbol_name_matches (i.e., symbol_name_matcher_ftype + implementation for symbol_name_match_type::WILD matching). Split + to a separate function for unit-testing convenience. + + If SYMBOL_SEARCH_NAME has more scopes than LOOKUP_NAME, we try to + match ignoring the extra leading scopes of SYMBOL_SEARCH_NAME. + This allows conveniently setting breakpoints on functions/methods + inside any namespace/class without specifying the fully-qualified + name. + + E.g., these match: + + [symbol search name] [lookup name] + foo::bar::func foo::bar::func + foo::bar::func bar::func + foo::bar::func func + + While these don't: + + [symbol search name] [lookup name] + foo::zbar::func bar::func + foo::bar::func foo::func + + See more examples in the test_cp_symbol_name_matches selftest + function below. + + See symbol_name_matcher_ftype for description of SYMBOL_SEARCH_NAME + and COMP_MATCH_RES. . + + LOOKUP_NAME/LOOKUP_NAME_LEN is the name we're looking up. + + See strncmp_iw_with_mode for description of MODE. +*/ + +static bool +cp_symbol_name_matches_1 (const char *symbol_search_name, + const char *lookup_name, + size_t lookup_name_len, + strncmp_iw_mode mode, + completion_match_result *comp_match_res) +{ + const char *sname = symbol_search_name; + + while (true) + { + if (strncmp_iw_with_mode (sname, lookup_name, lookup_name_len, + mode) == 0) + { + if (comp_match_res != NULL) + { + comp_match_res->match.set_match (symbol_search_name); + comp_match_res->match_for_lcd.set_match (sname); + } + return true; + } + + unsigned int len = cp_find_first_component (sname); + + if (sname[len] == '\0') + return false; + + gdb_assert (sname[len] == ':'); + /* Skip the '::'. */ + sname += len + 2; + } +} + +/* C++ symbol_name_matcher_ftype implementation for fully qualified + matches. */ static bool cp_fq_symbol_name_matches (const char *symbol_search_name, @@ -1644,18 +1729,150 @@ cp_fq_symbol_name_matches (const char *symbol_search_name, return false; } +/* C++ symbol_name_matcher_ftype implementation for wild matches. + Defers work to cp_symbol_name_matches_1. */ + +static bool +cp_symbol_name_matches (const char *symbol_search_name, + const lookup_name_info &lookup_name, + completion_match_result *comp_match_res) +{ + const std::string &name = lookup_name.cplus ().lookup_name (); + + strncmp_iw_mode mode = (lookup_name.completion_mode () + ? strncmp_iw_mode::NORMAL + : strncmp_iw_mode::MATCH_PARAMS); + + return cp_symbol_name_matches_1 (symbol_search_name, + name.c_str (), name.size (), + mode, comp_match_res); +} + /* See cp-support.h. */ symbol_name_matcher_ftype * cp_get_symbol_name_matcher (const lookup_name_info &lookup_name) { - return cp_fq_symbol_name_matches; + switch (lookup_name.match_type ()) + { + case symbol_name_match_type::FULL: + case symbol_name_match_type::EXPRESSION: + return cp_fq_symbol_name_matches; + case symbol_name_match_type::WILD: + return cp_symbol_name_matches; + } + + gdb_assert_not_reached (""); } #if GDB_SELF_TEST namespace selftests { +void +test_cp_symbol_name_matches () +{ +#define CHECK_MATCH(SYMBOL, INPUT) \ + SELF_CHECK (cp_symbol_name_matches_1 (SYMBOL, \ + INPUT, sizeof (INPUT) - 1, \ + strncmp_iw_mode::MATCH_PARAMS, \ + NULL)) + +#define CHECK_NOT_MATCH(SYMBOL, INPUT) \ + SELF_CHECK (!cp_symbol_name_matches_1 (SYMBOL, \ + INPUT, sizeof (INPUT) - 1, \ + strncmp_iw_mode::MATCH_PARAMS, \ + NULL)) + + /* Like CHECK_MATCH, and also check that INPUT (and all substrings + that start at index 0) completes to SYMBOL. */ +#define CHECK_MATCH_C(SYMBOL, INPUT) \ + CHECK_MATCH (SYMBOL, INPUT); \ + for (size_t i = 0; i < sizeof (INPUT) - 1; i++) \ + SELF_CHECK (cp_symbol_name_matches_1 (SYMBOL, INPUT, i, \ + strncmp_iw_mode::NORMAL, \ + NULL)) + + /* Like CHECK_NOT_MATCH, and also check that INPUT does NOT complete + to SYMBOL. */ +#define CHECK_NOT_MATCH_C(SYMBOL, INPUT) \ + CHECK_NOT_MATCH (SYMBOL, INPUT); \ + SELF_CHECK (!cp_symbol_name_matches_1 (SYMBOL, INPUT, \ + sizeof (INPUT) - 1, \ + strncmp_iw_mode::NORMAL, \ + NULL)) + + /* Lookup name without parens matches all overloads. */ + CHECK_MATCH_C ("function()", "function"); + CHECK_MATCH_C ("function(int)", "function"); + + /* Check whitespace around parameters is ignored. */ + CHECK_MATCH_C ("function()", "function ()"); + CHECK_MATCH_C ("function ( )", "function()"); + CHECK_MATCH_C ("function ()", "function( )"); + CHECK_MATCH_C ("func(int)", "func( int )"); + CHECK_MATCH_C ("func(int)", "func ( int ) "); + CHECK_MATCH_C ("func ( int )", "func( int )"); + CHECK_MATCH_C ("func ( int )", "func ( int ) "); + + /* Check symbol name prefixes aren't incorrectly matched. */ + CHECK_NOT_MATCH ("func", "function"); + CHECK_NOT_MATCH ("function", "func"); + CHECK_NOT_MATCH ("function()", "func"); + + /* Check that if the lookup name includes parameters, only the right + overload matches. */ + CHECK_MATCH_C ("function(int)", "function(int)"); + CHECK_NOT_MATCH_C ("function(int)", "function()"); + + /* Tests matching symbols in some scope. */ + CHECK_MATCH_C ("foo::function()", "function"); + CHECK_MATCH_C ("foo::function(int)", "function"); + CHECK_MATCH_C ("foo::bar::function()", "function"); + CHECK_MATCH_C ("bar::function()", "bar::function"); + CHECK_MATCH_C ("foo::bar::function()", "bar::function"); + CHECK_MATCH_C ("foo::bar::function(int)", "bar::function"); + + /* Same, with parameters in the lookup name. */ + CHECK_MATCH_C ("foo::function()", "function()"); + CHECK_MATCH_C ("foo::bar::function()", "function()"); + CHECK_MATCH_C ("foo::function(int)", "function(int)"); + CHECK_MATCH_C ("foo::function()", "foo::function()"); + CHECK_MATCH_C ("foo::bar::function()", "bar::function()"); + CHECK_MATCH_C ("foo::bar::function(int)", "bar::function(int)"); + CHECK_NOT_MATCH_C ("foo::bar::function(int)", "bar::function()"); + + CHECK_MATCH_C ("(anonymous namespace)::bar::function(int)", + "bar::function(int)"); + CHECK_MATCH_C ("foo::(anonymous namespace)::bar::function(int)", + "function(int)"); + + /* Lookup scope wider than symbol scope, should not match. */ + CHECK_NOT_MATCH_C ("function()", "bar::function"); + CHECK_NOT_MATCH_C ("function()", "bar::function()"); + + /* An explicit global scope forces a fully qualified match. */ + CHECK_NOT_MATCH_C ("foo::function()", "::function"); + CHECK_NOT_MATCH_C ("foo::function()", "::function()"); + CHECK_NOT_MATCH_C ("foo::function(int)", "::function()"); + CHECK_NOT_MATCH_C ("foo::function(int)", "::function(int)"); + + CHECK_MATCH_C ("abc::def::ghi()", "abc::def::ghi()"); + CHECK_MATCH_C ("abc::def::ghi ( )", "abc::def::ghi()"); + CHECK_MATCH_C ("abc::def::ghi()", "abc::def::ghi ( )"); + CHECK_MATCH_C ("function()", "function()"); + CHECK_MATCH_C ("foo::function()", "function()"); + CHECK_MATCH_C ("foo::bar::function()", "function()"); + CHECK_MATCH_C ("bar::function()", "bar::function()"); + CHECK_MATCH_C ("foo::bar::function()", "bar::function"); + CHECK_MATCH_C ("(anonymous namespace)::bar::function(int)", + "function(int)"); + CHECK_MATCH_C ("foo::(anonymous namespace)::bar::function(int)", + "function(int)"); + CHECK_NOT_MATCH_C ("function()", "bar::function"); + CHECK_NOT_MATCH_C ("foo::function()", "::function"); +} + /* If non-NULL, return STR wrapped in quotes. Otherwise, return a "" string (with no quotes). */ @@ -1859,6 +2076,8 @@ display the offending symbol."), #endif #if GDB_SELF_TEST + selftests::register_test ("cp_symbol_name_matches", + selftests::test_cp_symbol_name_matches); selftests::register_test ("cp_remove_params", selftests::test_cp_remove_params); #endif diff --git a/gdb/cp-support.h b/gdb/cp-support.h index 44d8269..010fc9b 100644 --- a/gdb/cp-support.h +++ b/gdb/cp-support.h @@ -116,6 +116,13 @@ extern struct symbol **make_symbol_overload_list_adl (struct type **arg_types, extern struct type *cp_lookup_rtti_type (const char *name, struct block *block); +/* Produce an unsigned hash value from SEARCH_NAME that is compatible + with cp_symbol_name_matches. Only the last component in + "foo::bar::function()" is considered for hashing purposes (i.e., + the entire prefix is skipped), so that later on looking up for + "function" or "bar::function" in all namespaces is possible. */ +extern unsigned int cp_search_name_hash (const char *search_name); + /* Implement the "la_get_symbol_name_matcher" language_defn method for C++. */ extern symbol_name_matcher_ftype *cp_get_symbol_name_matcher diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index 686fa10..65f7019 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -4608,6 +4608,8 @@ static const char *test_symbols[] = { "ns::foo", "ns::foo", "ns::foo", + "ns2::tmpl::foo2", + "(anonymous namespace)::A::B::C", /* These are used to check that the increment-last-char in the matching algorithm for completion doesn't match "t1_fund" when @@ -4794,6 +4796,8 @@ test_dw2_expand_symtabs_matching_symbol () { CHECK_MATCH ("w", symbol_name_match_type::FULL, true, EXPECT ("w1::w2")); + CHECK_MATCH ("w", symbol_name_match_type::WILD, true, + EXPECT ("w1::w2")); } /* Same, with a "complicated" symbol. */ @@ -4821,6 +4825,10 @@ test_dw2_expand_symtabs_matching_symbol () { CHECK_MATCH ("std::zfunction(int)", symbol_name_match_type::FULL, true, EXPECT ("std::zfunction", "std::zfunction2")); + CHECK_MATCH ("zfunction(int)", symbol_name_match_type::WILD, true, + EXPECT ("std::zfunction", "std::zfunction2")); + CHECK_MATCH ("zfunc", symbol_name_match_type::WILD, true, + EXPECT ("std::zfunction", "std::zfunction2")); } /* Check that whitespace is ignored appropriately. A symbol with a @@ -4829,6 +4837,8 @@ test_dw2_expand_symtabs_matching_symbol () static const char expected[] = "ns::foo"; CHECK_MATCH ("ns :: foo < int > ", symbol_name_match_type::FULL, false, EXPECT (expected)); + CHECK_MATCH ("foo < int > ", symbol_name_match_type::WILD, false, + EXPECT (expected)); } /* Check that whitespace is ignored appropriately. A symbol with a @@ -4841,9 +4851,13 @@ test_dw2_expand_symtabs_matching_symbol () { CHECK_MATCH ("ns :: foo < char * >", symbol_name_match_type::FULL, completion_mode[i], EXPECT (expected)); + CHECK_MATCH ("foo < char * >", symbol_name_match_type::WILD, + completion_mode[i], EXPECT (expected)); CHECK_MATCH ("ns :: foo < char * > (int)", symbol_name_match_type::FULL, completion_mode[i], EXPECT (expected)); + CHECK_MATCH ("foo < char * > (int)", symbol_name_match_type::WILD, + completion_mode[i], EXPECT (expected)); } } @@ -4854,14 +4868,48 @@ test_dw2_expand_symtabs_matching_symbol () symbol_name_match_type::FULL, true, EXPECT (expected)); CHECK_MATCH ("ns :: foo < char * > ( int ) &&", symbol_name_match_type::FULL, true, EXPECT (expected)); + CHECK_MATCH ("foo < char * > ( int ) const", + symbol_name_match_type::WILD, true, EXPECT (expected)); + CHECK_MATCH ("foo < char * > ( int ) &&", + symbol_name_match_type::WILD, true, EXPECT (expected)); } /* Test lookup names that don't match anything. */ { + CHECK_MATCH ("bar2", symbol_name_match_type::WILD, false, + {}); + CHECK_MATCH ("doesntexist", symbol_name_match_type::FULL, false, {}); } + /* Some wild matching tests, exercising "(anonymous namespace)", + which should not be confused with a parameter list. */ + { + static const char *syms[] = { + "A::B::C", + "B::C", + "C", + "A :: B :: C ( int )", + "B :: C ( int )", + "C ( int )", + }; + + for (const char *s : syms) + { + CHECK_MATCH (s, symbol_name_match_type::WILD, false, + EXPECT ("(anonymous namespace)::A::B::C")); + } + } + + { + static const char expected[] = "ns2::tmpl::foo2"; + CHECK_MATCH ("tmp", symbol_name_match_type::WILD, true, + EXPECT (expected)); + CHECK_MATCH ("tmpl<", symbol_name_match_type::WILD, true, + EXPECT (expected)); + } + SELF_CHECK (!any_mismatch); #undef EXPECT diff --git a/gdb/linespec.c b/gdb/linespec.c index 3f7f171..c11ca1d 100644 --- a/gdb/linespec.c +++ b/gdb/linespec.c @@ -359,6 +359,7 @@ static VEC (symbolp) *find_label_symbols (struct linespec_state *self, static void find_linespec_symbols (struct linespec_state *self, VEC (symtab_ptr) *file_symtabs, const char *name, + symbol_name_match_type name_match_type, VEC (symbolp) **symbols, VEC (bound_minimal_symbol_d) **minsyms); @@ -1868,6 +1869,7 @@ linespec_parse_basic (linespec_parser *parser) linespec_complete_function (tmp_tracker, parser->completion_word, + symbol_name_match_type::WILD, source_filename); if (tmp_tracker.have_completions ()) @@ -1892,6 +1894,7 @@ linespec_parse_basic (linespec_parser *parser) /* Try looking it up as a function/method. */ find_linespec_symbols (PARSER_STATE (parser), PARSER_RESULT (parser)->file_symtabs, name, + symbol_name_match_type::WILD, &symbols, &minimal_symbols); if (symbols != NULL || minimal_symbols != NULL) @@ -2383,6 +2386,7 @@ convert_explicit_location_to_linespec (struct linespec_state *self, linespec_p result, const char *source_filename, const char *function_name, + symbol_name_match_type fname_match_type, const char *label_name, struct line_offset line_offset) { @@ -2412,8 +2416,8 @@ convert_explicit_location_to_linespec (struct linespec_state *self, if (function_name != NULL) { find_linespec_symbols (self, result->file_symtabs, - function_name, &symbols, - &minimal_symbols); + function_name, fname_match_type, + &symbols, &minimal_symbols); if (symbols == NULL && minimal_symbols == NULL) symbol_not_found_error (function_name, @@ -2453,6 +2457,7 @@ convert_explicit_location_to_sals (struct linespec_state *self, convert_explicit_location_to_linespec (self, result, explicit_loc->source_filename, explicit_loc->function_name, + explicit_loc->func_name_match_type, explicit_loc->label_name, explicit_loc->line_offset); return convert_linespec_to_sals (self, result); @@ -2828,10 +2833,10 @@ linespec_lex_to_end (const char **stringp) void linespec_complete_function (completion_tracker &tracker, const char *function, + symbol_name_match_type func_match_type, const char *source_filename) { complete_symbol_mode mode = complete_symbol_mode::LINESPEC; - symbol_name_match_type func_match_type = symbol_name_match_type::WILD; if (source_filename != NULL) { @@ -2870,7 +2875,8 @@ complete_linespec_component (linespec_parser *parser, { completion_list fn_list; - linespec_complete_function (tracker, text, source_filename); + linespec_complete_function (tracker, text, symbol_name_match_type::WILD, + source_filename); if (source_filename == NULL) { /* Haven't seen a source component, like in "b @@ -2940,6 +2946,7 @@ linespec_complete_label (completion_tracker &tracker, const struct language_defn *language, const char *source_filename, const char *function_name, + symbol_name_match_type func_name_match_type, const char *label_name) { linespec_parser parser; @@ -2956,6 +2963,7 @@ linespec_complete_label (completion_tracker &tracker, PARSER_RESULT (&parser), source_filename, function_name, + func_name_match_type, NULL, unknown_offset); } CATCH (ex, RETURN_MASK_ERROR) @@ -3039,7 +3047,7 @@ linespec_complete (completion_tracker &tracker, const char *text) VEC (bound_minimal_symbol_d) *minimal_symbols; find_linespec_symbols (PARSER_STATE (&parser), PARSER_RESULT (&parser)->file_symtabs, - func_name, + func_name, symbol_name_match_type::WILD, &function_symbols, &minimal_symbols); PARSER_RESULT (&parser)->function_symbols = function_symbols; @@ -3936,6 +3944,7 @@ symtabs_from_filename (const char *filename, static void find_function_symbols (struct linespec_state *state, VEC (symtab_ptr) *file_symtabs, const char *name, + symbol_name_match_type name_match_type, VEC (symbolp) **symbols, VEC (bound_minimal_symbol_d) **minsyms) { @@ -3955,8 +3964,7 @@ find_function_symbols (struct linespec_state *state, add_all_symbol_names_from_pspace (&info, state->search_pspace, symbol_names, FUNCTIONS_DOMAIN); else - add_matching_symbols_to_info (name, symbol_name_match_type::WILD, - FUNCTIONS_DOMAIN, + add_matching_symbols_to_info (name, name_match_type, FUNCTIONS_DOMAIN, &info, state->search_pspace); do_cleanups (cleanup); @@ -3985,6 +3993,7 @@ static void find_linespec_symbols (struct linespec_state *state, VEC (symtab_ptr) *file_symtabs, const char *lookup_name, + symbol_name_match_type name_match_type, VEC (symbolp) **symbols, VEC (bound_minimal_symbol_d) **minsyms) { @@ -4002,6 +4011,7 @@ find_linespec_symbols (struct linespec_state *state, 2) break class::method where method is in class (and not a baseclass) */ find_function_symbols (state, file_symtabs, lookup_name, + name_match_type, symbols, minsyms); /* If we were unable to locate a symbol of the same name, try dividing diff --git a/gdb/linespec.h b/gdb/linespec.h index b955728..7add83e 100644 --- a/gdb/linespec.h +++ b/gdb/linespec.h @@ -182,12 +182,14 @@ extern const char * const linespec_keywords[]; extern void linespec_complete (completion_tracker &tracker, const char *text); -/* Complete a function symbol, in linespec mode. If SOURCE_FILENAME - is non-NULL, limits completion to the list of functions defined in - source files that match SOURCE_FILENAME. */ +/* Complete a function symbol, in linespec mode, according to + FUNC_MATCH_TYPE. If SOURCE_FILENAME is non-NULL, limits completion + to the list of functions defined in source files that match + SOURCE_FILENAME. */ extern void linespec_complete_function (completion_tracker &tracker, const char *function, + symbol_name_match_type func_match_type, const char *source_filename); /* Complete a label symbol, in linespec mode. Only labels of @@ -199,6 +201,7 @@ extern void linespec_complete_label (completion_tracker &tracker, const struct language_defn *language, const char *source_filename, const char *function_name, + symbol_name_match_type name_match_type, const char *label_name); /* Evaluate the expression pointed to by EXP_PTR into a CORE_ADDR, diff --git a/gdb/location.c b/gdb/location.c index c78778e..e353bf7 100644 --- a/gdb/location.c +++ b/gdb/location.c @@ -245,7 +245,13 @@ explicit_to_string_internal (int as_linespec, if (need_space) buf.putc (space); if (!as_linespec) - buf.puts ("-function "); + { + if (explicit_loc->func_name_match_type + == symbol_name_match_type::FULL) + buf.puts ("-qualified "); + else + buf.puts ("-function "); + } buf.puts (explicit_loc->function_name); need_space = 1; } @@ -775,6 +781,16 @@ string_to_explicit_location (const char **argp, set_oarg (explicit_location_lex_one_function (argp, language, completion_info)); EL_EXPLICIT (location)->function_name = oarg.release (); + EL_EXPLICIT (location)->func_name_match_type + = symbol_name_match_type::WILD; + } + else if (strncmp (opt.get (), "-qualified", len) == 0) + { + set_oarg (explicit_location_lex_one_function (argp, language, + completion_info)); + EL_EXPLICIT (location)->function_name = oarg.release (); + EL_EXPLICIT (location)->func_name_match_type + = symbol_name_match_type::FULL; } else if (strncmp (opt.get (), "-line", len) == 0) { diff --git a/gdb/location.h b/gdb/location.h index d954eac..74fd868 100644 --- a/gdb/location.h +++ b/gdb/location.h @@ -79,6 +79,9 @@ struct explicit_location /* The function name. Malloc'd. */ char *function_name; + /* Whether the function name is fully-qualified or not. */ + symbol_name_match_type func_name_match_type; + /* The name of a label. Malloc'd. */ char *label_name; diff --git a/gdb/testsuite/gdb.base/langs.exp b/gdb/testsuite/gdb.base/langs.exp index 8dcd5ee..03c690c 100644 --- a/gdb/testsuite/gdb.base/langs.exp +++ b/gdb/testsuite/gdb.base/langs.exp @@ -38,7 +38,7 @@ if [get_compiler_info] { return -1 } -gdb_test_multiple "b langs0" "break on nonexistent function in langs.exp" { +gdb_test_multiple "b -qualified langs0" "break on nonexistent function in langs.exp" { -re "Function \"langs0\" not defined\..*Make breakpoint pending on future shared library load.*y or .n.. $" { gdb_test "n" "" "break on nonexistent function in langs.exp" diff --git a/gdb/testsuite/gdb.cp/meth-typedefs.exp b/gdb/testsuite/gdb.cp/meth-typedefs.exp index 08f1464..50690ab 100644 --- a/gdb/testsuite/gdb.cp/meth-typedefs.exp +++ b/gdb/testsuite/gdb.cp/meth-typedefs.exp @@ -145,15 +145,42 @@ foreach test $methods { set func [lindex $test 0] set result [lindex $test 1] - gdb_test "list $func" $result - gdb_test "list '$func'" $result - if {[gdb_breakpoint $func]} { - pass "break $func" + gdb_test "list -qualified $func" $result + gdb_test "list -qualified '$func'" $result + if {[gdb_breakpoint "-qualified $func"]} { + pass "break -qualified $func" } - if {[gdb_breakpoint '$func']} { - pass "break '$func'" + if {[gdb_breakpoint "-qualified '$func'"]} { + pass "break -qualified '$func'" } } +# The tests above use -qualified to explicitly pick the one "test" +# symbol each test cares about. Now check that both "break test(..)" +# and "list test(..)" without -qualified find "test(..)" in all the 3 +# scopes that have the this particular overload. +set func "test(aenum, astruct const&, aunion const***)" +set func_re "test\\(anon_enum, anon_struct const&, anon_union const\\*\\*\\*\\)" +set line1 [gdb_get_line_number " A::FOO::$func"] +set line2 [gdb_get_line_number " B::$func"] +set line3 [gdb_get_line_number " $func"] + +foreach f [list "$func" "'$func'"] { + set any "\[^\r\n\]*" + gdb_test \ + "list $f" \ + [multi_line \ + "file: \".*$srcfile\", line number: $line1, symbol: \"A::foo::$func_re\"" \ + "$line1${any}A::FOO::test${any}" \ + "file: \".*$srcfile\", line number: $line2, symbol: \"B::$func_re\"" \ + "$line2${any}B::test${any}" \ + "file: \".*$srcfile\", line number: $line3, symbol: \"$func_re\"" \ + "$line3${any}// test${any}"] \ + "list $f" + + delete_breakpoints + gdb_test "break $f" "\\(3 locations\\)" +} + gdb_exit return 0 diff --git a/gdb/testsuite/gdb.cp/namespace.exp b/gdb/testsuite/gdb.cp/namespace.exp index 640ee4f..4a6b863 100644 --- a/gdb/testsuite/gdb.cp/namespace.exp +++ b/gdb/testsuite/gdb.cp/namespace.exp @@ -120,7 +120,7 @@ gdb_test "break AAA::xyzq" \ # Break on a function in the global namespace. -gdb_test "break ::ensureOtherRefs" \ +gdb_test "break -qualified ::ensureOtherRefs" \ "Breakpoint.*at $hex: file.*$srcfile2, line $decimal\\." # Call a function in a nested namespace