From patchwork Fri Nov 22 16:41:52 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Simon Marchi (Code Review)" X-Patchwork-Id: 36117 Received: (qmail 80981 invoked by alias); 22 Nov 2019 16:42:06 -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 80899 invoked by uid 89); 22 Nov 2019 16:42:06 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.4 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_STOCKGEN autolearn=ham version=3.3.1 spammy=interrupted, 1000s, search_domain X-HELO: mx1.osci.io Received: from polly.osci.io (HELO mx1.osci.io) (8.43.85.229) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 22 Nov 2019 16:42:03 +0000 Received: by mx1.osci.io (Postfix, from userid 994) id 8482E2041E; Fri, 22 Nov 2019 11:42:01 -0500 (EST) Received: from gnutoolchain-gerrit.osci.io (gnutoolchain-gerrit.osci.io [IPv6:2620:52:3:1:5054:ff:fe06:16ca]) by mx1.osci.io (Postfix) with ESMTP id 240A320249; Fri, 22 Nov 2019 11:41:54 -0500 (EST) Received: from localhost (localhost [127.0.0.1]) by gnutoolchain-gerrit.osci.io (Postfix) with ESMTP id EB2DA2816F; Fri, 22 Nov 2019 11:41:53 -0500 (EST) X-Gerrit-PatchSet: 4 Date: Fri, 22 Nov 2019 11:41:52 -0500 From: "Andrew Burgess (Code Review)" To: Tom Tromey , Simon Marchi , gdb-patches@sourceware.org Cc: Christian Biesinger , Joel Brobecker Auto-Submitted: auto-generated X-Gerrit-MessageType: newpatchset Subject: [review v4] gdb: Introduce global_symbol_searcher X-Gerrit-Change-Id: I488ab292a892d9e9e84775c632c5f198b6ad3710 X-Gerrit-Change-Number: 264 X-Gerrit-ChangeURL: X-Gerrit-Commit: 430d88fa4ca7aea66fce1c570d6f8e81d8a6b499 In-Reply-To: References: Reply-To: andrew.burgess@embecosm.com, simon.marchi@polymtl.ca, tromey@sourceware.org, cbiesinger@google.com, brobecker@adacore.com, gdb-patches@sourceware.org MIME-Version: 1.0 Content-Disposition: inline User-Agent: Gerrit/3.0.3-79-g83ff7f88f1 Message-Id: <20191122164153.EB2DA2816F@gnutoolchain-gerrit.osci.io> Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/264 ...................................................................... gdb: Introduce global_symbol_searcher Introduce a new class to wrap up the parameters needed for the function search_symbols, which has now become a member function of this new class. The motivation is that search_symbols already takes a lot of parameters, and a future commit is going to add even more. This commit hopefully makes collecting the state required for a search easier. As part of this conversion the list of filenames in which to search has been converted to a std::vector. There should be no user visible changes after this commit. gdb/ChangeLog: * python/python.c (gdbpy_rbreak): Convert to using global_symbol_searcher. * symtab.c (file_matches): Convert return type to bool, change file list to std::vector, update header comment. (search_symbols): Rename to... (global_symbol_searcher::search): ...this and update now its a member function of global_symbol_searcher. Take account of the changes to file_matches. (symtab_symbol_info): Convert to using global_symbol_searcher. (rbreak_command): Likewise. (search_module_symbols): Likewise. * symtab.h (enum symbol_search): Update comment. (search_symbols): Remove declaration. (class global_symbol_searcher): New class. Change-Id: I488ab292a892d9e9e84775c632c5f198b6ad3710 --- M gdb/ChangeLog M gdb/python/python.c M gdb/symtab.c M gdb/symtab.h 4 files changed, 160 insertions(+), 123 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 93258c3..4a8bc0f 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,20 @@ +2019-11-22 Andrew Burgess + + * python/python.c (gdbpy_rbreak): Convert to using + global_symbol_searcher. + * symtab.c (file_matches): Convert return type to bool, change + file list to std::vector, update header comment. + (search_symbols): Rename to... + (global_symbol_searcher::search): ...this and update now its + a member function of global_symbol_searcher. Take account of the + changes to file_matches. + (symtab_symbol_info): Convert to using global_symbol_searcher. + (rbreak_command): Likewise. + (search_module_symbols): Likewise. + * symtab.h (enum symbol_search): Update comment. + (search_symbols): Remove declaration. + (class global_symbol_searcher): New class. + 2019-11-22 Tom de Vries * contrib/words.sh: Improve words extraction. diff --git a/gdb/python/python.c b/gdb/python/python.c index f94214e..da68874 100644 --- a/gdb/python/python.c +++ b/gdb/python/python.c @@ -645,19 +645,6 @@ static PyObject * gdbpy_rbreak (PyObject *self, PyObject *args, PyObject *kw) { - /* A simple type to ensure clean up of a vector of allocated strings - when a C interface demands a const char *array[] type - interface. */ - struct symtab_list_type - { - ~symtab_list_type () - { - for (const char *elem: vec) - xfree ((void *) elem); - } - std::vector vec; - }; - char *regex = NULL; std::vector symbols; unsigned long count = 0; @@ -667,7 +654,6 @@ unsigned int throttle = 0; static const char *keywords[] = {"regex","minsyms", "throttle", "symtabs", NULL}; - symtab_list_type symtab_paths; if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|O!IO", keywords, ®ex, &PyBool_Type, @@ -684,6 +670,12 @@ minsyms_p = cmp; } + global_symbol_searcher spec (FUNCTIONS_DOMAIN, regex); + SCOPE_EXIT { + for (const char *elem : spec.filenames) + xfree ((void *) elem); + }; + /* The "symtabs" keyword is any Python iterable object that returns a gdb.Symtab on each iteration. If specified, iterate through the provided gdb.Symtabs and extract their full path. As @@ -729,20 +721,13 @@ /* Make sure there is a definite place to store the value of filename before it is released. */ - symtab_paths.vec.push_back (nullptr); - symtab_paths.vec.back () = filename.release (); + spec.filenames.push_back (nullptr); + spec.filenames.back () = filename.release (); } } - if (symtab_list) - { - const char **files = symtab_paths.vec.data (); - - symbols = search_symbols (regex, FUNCTIONS_DOMAIN, NULL, - symtab_paths.vec.size (), files, false); - } - else - symbols = search_symbols (regex, FUNCTIONS_DOMAIN, NULL, 0, NULL, false); + /* The search spec. */ + symbols = spec.search (); /* Count the number of symbols (both symbols and optionally minimal symbols) so we can correctly check the throttle limit. */ diff --git a/gdb/symtab.c b/gdb/symtab.c index 0064800..f9eddb0 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -4339,27 +4339,24 @@ printf_filtered ("\n"); } -/* Compare FILE against all the NFILES entries of FILES. If BASENAMES is - non-zero compare only lbasename of FILES. */ +/* Compare FILE against all the entries of FILENAMES. If BASENAMES is + true compare only lbasename of FILENAMES. */ -static int -file_matches (const char *file, const char *files[], int nfiles, int basenames) +static bool +file_matches (const char *file, const std::vector &filenames, + bool basenames) { - int i; + if (filenames.empty ()) + return true; - if (file != NULL && nfiles != 0) + for (const char *name : filenames) { - for (i = 0; i < nfiles; i++) - { - if (compare_filenames_for_search (file, (basenames - ? lbasename (files[i]) - : files[i]))) - return 1; - } + name = (basenames ? lbasename (name) : name); + if (compare_filenames_for_search (file, name)) + return true; } - else if (nfiles == 0) - return 1; - return 0; + + return false; } /* Helper function for sort_search_symbols_remove_dups and qsort. Can only @@ -4436,30 +4433,10 @@ result->end ()); } -/* Search the symbol table for matches to the regular expression REGEXP, - returning the results. - - Only symbols of KIND are searched: - VARIABLES_DOMAIN - search all symbols, excluding functions, type names, - and constants (enums). - if T_REGEXP is not NULL, only returns var that have - a type matching regular expression T_REGEXP. - FUNCTIONS_DOMAIN - search all functions - TYPES_DOMAIN - search all type names - ALL_DOMAIN - an internal error for this function - - Within each file the results are sorted locally; each symtab's global and - static blocks are separately alphabetized. - Duplicate entries are removed. - - When EXCLUDE_MINSYMS is false then matching minsyms are also returned, - otherwise they are excluded. */ +/* See symtab.h. */ std::vector -search_symbols (const char *regexp, enum search_domain kind, - const char *t_regexp, - int nfiles, const char *files[], - bool exclude_minsyms) +global_symbol_searcher::search () const { const struct blockvector *bv; const struct block *b; @@ -4483,21 +4460,23 @@ gdb::optional preg; gdb::optional treg; - gdb_assert (kind != ALL_DOMAIN); + gdb_assert (m_kind != ALL_DOMAIN); - ourtype = types[kind]; - ourtype2 = types2[kind]; - ourtype3 = types3[kind]; - ourtype4 = types4[kind]; + ourtype = types[m_kind]; + ourtype2 = types2[m_kind]; + ourtype3 = types3[m_kind]; + ourtype4 = types4[m_kind]; - if (regexp != NULL) + if (m_symbol_name_regexp != NULL) { + const char *symbol_name_regexp = m_symbol_name_regexp; + /* Make sure spacing is right for C++ operators. This is just a courtesy to make the matching less sensitive to how many spaces the user leaves between 'operator' and or . */ const char *opend; - const char *opname = operator_chars (regexp, &opend); + const char *opname = operator_chars (symbol_name_regexp, &opend); if (*opname) { @@ -4522,29 +4501,34 @@ char *tmp = (char *) alloca (8 + fix + strlen (opname) + 1); sprintf (tmp, "operator%.*s%s", fix, " ", opname); - regexp = tmp; + symbol_name_regexp = tmp; } } int cflags = REG_NOSUB | (case_sensitivity == case_sensitive_off ? REG_ICASE : 0); - preg.emplace (regexp, cflags, _("Invalid regexp")); + preg.emplace (symbol_name_regexp, cflags, + _("Invalid m_symbol_name_regexp")); } - if (t_regexp != NULL) + if (m_symbol_type_regexp != NULL) { int cflags = REG_NOSUB | (case_sensitivity == case_sensitive_off ? REG_ICASE : 0); - treg.emplace (t_regexp, cflags, _("Invalid regexp")); + treg.emplace (m_symbol_type_regexp, cflags, + _("Invalid m_symbol_namregexp")); } /* Search through the partial symtabs *first* for all symbols - matching the regexp. That way we don't have to reproduce all of + matching the m_symbol_namregexp. That way we don't have to reproduce all of the machinery below. */ expand_symtabs_matching ([&] (const char *filename, bool basenames) { - return file_matches (filename, files, nfiles, - basenames); + /* EXPAND_SYMTABS_MATCHING expects a callback + that returns an integer, not a boolean as + FILE_MATCHES does. */ + return file_matches (filename, filenames, + basenames) ? 1 : 0; }, lookup_name_info::match_any (), [&] (const char *symname) @@ -4554,7 +4538,7 @@ 0, NULL, 0) == 0); }, NULL, - kind); + m_kind); /* Here, we search through the minimal symbol tables for functions and variables that match, and force their symbols to be read. @@ -4572,7 +4556,8 @@ all objfiles. In large programs (1000s of shared libs) searching all objfiles is not worth the pain. */ - if (nfiles == 0 && (kind == VARIABLES_DOMAIN || kind == FUNCTIONS_DOMAIN)) + if (filenames.empty () && (m_kind == VARIABLES_DOMAIN + || m_kind == FUNCTIONS_DOMAIN)) { for (objfile *objfile : current_program_space->objfiles ()) { @@ -4596,7 +4581,7 @@ lookup functions is to expand the symbol table if msymbol is found, for the benefit of the next loop on compunits. */ - if (kind == FUNCTIONS_DOMAIN + if (m_kind == FUNCTIONS_DOMAIN ? (find_pc_compunit_symtab (MSYMBOL_VALUE_ADDRESS (objfile, msymbol)) == NULL) @@ -4628,16 +4613,16 @@ /* Check first sole REAL_SYMTAB->FILENAME. It does not need to be a substring of symtab_to_fullname as it may contain "./" etc. */ - if ((file_matches (real_symtab->filename, files, nfiles, 0) + if ((file_matches (real_symtab->filename, filenames, false) || ((basenames_may_differ || file_matches (lbasename (real_symtab->filename), - files, nfiles, 1)) + filenames, true)) && file_matches (symtab_to_fullname (real_symtab), - files, nfiles, 0))) + filenames, false))) && ((!preg.has_value () || preg->exec (SYMBOL_NATURAL_NAME (sym), 0, NULL, 0) == 0) - && ((kind == VARIABLES_DOMAIN + && ((m_kind == VARIABLES_DOMAIN && SYMBOL_CLASS (sym) != LOC_TYPEDEF && SYMBOL_CLASS (sym) != LOC_UNRESOLVED && SYMBOL_CLASS (sym) != LOC_BLOCK @@ -4650,15 +4635,15 @@ == TYPE_CODE_ENUM)) && (!treg.has_value () || treg_matches_sym_type_name (*treg, sym))) - || (kind == FUNCTIONS_DOMAIN + || (m_kind == FUNCTIONS_DOMAIN && SYMBOL_CLASS (sym) == LOC_BLOCK && (!treg.has_value () || treg_matches_sym_type_name (*treg, sym))) - || (kind == TYPES_DOMAIN + || (m_kind == TYPES_DOMAIN && SYMBOL_CLASS (sym) == LOC_TYPEDEF && SYMBOL_DOMAIN (sym) != MODULE_DOMAIN) - || (kind == MODULES_DOMAIN + || (m_kind == MODULES_DOMAIN && SYMBOL_DOMAIN (sym) == MODULE_DOMAIN && SYMBOL_LINE (sym) != 0)))) { @@ -4675,11 +4660,11 @@ /* If there are no eyes, avoid all contact. I mean, if there are no debug symbols, then add matching minsyms. But if the user wants - to see symbols matching a type regexp, then never give a minimal symbol, + to see symbols matching a type m_symbol_namregexp, then never give a minimal symbol, as we assume that a minimal symbol does not have a type. */ - if ((found_misc || (nfiles == 0 && kind != FUNCTIONS_DOMAIN)) - && !exclude_minsyms + if ((found_misc || (filenames.empty () && m_kind != FUNCTIONS_DOMAIN)) + && !m_exclude_minsyms && !treg.has_value ()) { for (objfile *objfile : current_program_space->objfiles ()) @@ -4702,7 +4687,7 @@ { /* For functions we can do a quick check of whether the symbol might be found via find_pc_symtab. */ - if (kind != FUNCTIONS_DOMAIN + if (m_kind != FUNCTIONS_DOMAIN || (find_pc_compunit_symtab (MSYMBOL_VALUE_ADDRESS (objfile, msymbol)) == NULL)) @@ -4844,9 +4829,10 @@ regexp = nullptr; /* Must make sure that if we're interrupted, symbols gets freed. */ - std::vector symbols = search_symbols (regexp, kind, - t_regexp, 0, NULL, - exclude_minsyms); + global_symbol_searcher spec (kind, regexp); + spec.set_symbol_type_regexp (t_regexp); + spec.set_exclude_minsyms (exclude_minsyms); + std::vector symbols = spec.search (); if (!quiet) { @@ -5085,11 +5071,9 @@ rbreak_command (const char *regexp, int from_tty) { std::string string; - const char **files = NULL; - const char *file_name; - int nfiles = 0; + const char *file_name = nullptr; - if (regexp) + if (regexp != nullptr) { const char *colon = strchr (regexp, ':'); @@ -5105,17 +5089,14 @@ while (isspace (local_name[colon_index])) local_name[colon_index--] = 0; file_name = local_name; - files = &file_name; - nfiles = 1; regexp = skip_spaces (colon + 1); } } - std::vector symbols = search_symbols (regexp, - FUNCTIONS_DOMAIN, - NULL, - nfiles, files, - false); + global_symbol_searcher spec (FUNCTIONS_DOMAIN, regexp); + if (file_name != nullptr) + spec.filenames.push_back (file_name); + std::vector symbols = spec.search (); scoped_rbreak_breakpoints finalize; for (const symbol_search &p : symbols) @@ -6349,17 +6330,17 @@ std::vector results; /* Search for all modules matching MODULE_REGEXP. */ - std::vector modules = search_symbols (module_regexp, - MODULES_DOMAIN, - NULL, 0, NULL, - true); + global_symbol_searcher spec1 (MODULES_DOMAIN, module_regexp); + spec1.set_exclude_minsyms (true); + std::vector modules = spec1.search (); /* Now search for all symbols of the required KIND matching the required regular expressions. We figure out which ones are in which modules below. */ - std::vector symbols = search_symbols (regexp, kind, - type_regexp, 0, - NULL, true); + global_symbol_searcher spec2 (kind, regexp); + spec2.set_symbol_type_regexp (type_regexp); + spec2.set_exclude_minsyms (true); + std::vector symbols = spec2.search (); /* Now iterate over all MODULES, checking to see which items from SYMBOLS are in each module. */ diff --git a/gdb/symtab.h b/gdb/symtab.h index 8f95a3a..43aa9a9 100644 --- a/gdb/symtab.h +++ b/gdb/symtab.h @@ -811,7 +811,7 @@ extern const char *domain_name (domain_enum); -/* Searching domains, used for `search_symbols'. Element numbers are +/* Searching domains, used when searching for symbols. Element numbers are hardcoded in GDB, check all enum uses before changing it. */ enum search_domain @@ -2026,11 +2026,9 @@ extern symbol *find_function_alias_target (bound_minimal_symbol msymbol); /* Symbol searching */ -/* Note: struct symbol_search, search_symbols, et.al. are declared here, - instead of making them local to symtab.c, for gdbtk's sake. */ -/* When using search_symbols, a vector of the following structs is - returned. */ +/* When using the symbol_searcher struct to search for symbols, a vector of + the following structs is returned. */ struct symbol_search { symbol_search (int block_, struct symbol *symbol_) @@ -2079,12 +2077,68 @@ const symbol_search &sym_b); }; -extern std::vector search_symbols (const char *, - enum search_domain, - const char *, - int, - const char **, - bool); +/* In order to search for global symbols of a particular kind matching + particular regular expressions, create an instance of this structure and + call the SEARCH member function. */ +class global_symbol_searcher +{ +public: + + /* Constructor. */ + global_symbol_searcher (enum search_domain kind, + const char *symbol_name_regexp) + : m_kind (kind), + m_symbol_name_regexp (symbol_name_regexp) + { + /* The symbol searching is designed to only find one kind of thing. */ + gdb_assert (m_kind != ALL_DOMAIN); + } + + /* Set the optional regexp that matches against the symbol type. */ + void set_symbol_type_regexp (const char *regexp) + { + m_symbol_type_regexp = regexp; + } + + /* Set the flag to exclude minsyms from the search results. */ + void set_exclude_minsyms (bool exclude_minsyms) + { + m_exclude_minsyms = exclude_minsyms; + } + + /* Search the symbols from all objfiles in the current program space + looking for matches as defined by the current state of this object. + + Within each file the results are sorted locally; each symtab's global + and static blocks are separately alphabetized. Duplicate entries are + removed. */ + std::vector search () const; + + /* The set of source files to search in for matching symbols. This is + currently public so that it can be populated after this object has + been constructed. */ + std::vector filenames; + +private: + /* The kind of symbols are we searching for. + VARIABLES_DOMAIN - Search all symbols, excluding functions, type + names, and constants (enums). + FUNCTIONS_DOMAIN - Search all functions.. + TYPES_DOMAIN - Search all type names. + MODULES_DOMAIN - Search all Fortran modules. + ALL_DOMAIN - Not valid for this function. */ + enum search_domain m_kind; + + /* Regular expression to match against the symbol name. */ + const char *m_symbol_name_regexp = nullptr; + + /* Regular expression to match against the symbol type. */ + const char *m_symbol_type_regexp = nullptr; + + /* When this flag is false then minsyms that match M_SYMBOL_REGEXP will + be included in the results, otherwise they are excluded. */ + bool m_exclude_minsyms = false; +}; /* When searching for Fortran symbols within modules (functions/variables) we return a vector of this type. The first item in the pair is the