[review v11] gdb/mi: Add -max-results parameter to some -symbol-info-* commands

Andrew Burgess (Code Review)
2019-12-03T00:00:25Z

One other thing to note is that the global_symbol_searcher::search by
default returns UINT_MAX results, there's no longer a mechanism to
return an unlimited number of results, though hopefully this will not
be a huge issue.

gdb/ChangeLog:

* mi/mi-symbol-cmds.c (mi_symbol_info): Take extra parameter, and
add it into the search spec.
(parse_max_results_option): New function.
(mi_info_functions_or_variables): Parse -max-results flag and pass
it to mi_symbol_info.
(mi_cmd_symbol_info_modules): Likewise.
(mi_cmd_symbol_info_types): Likewise.
* symtab.c (global_symbol_searcher::add_matching_symbols): Change
return type to bool, change result container into a set, and don't
add new results if we have enough already.
(global_symbol_searcher::add_matching_msymbols): Change return
type to bool, and don't add new results if we have enough already.
(sort_search_symbols_remove_dups): Delete.
(global_symbol_searcher::search): Early exit from search loop when
we have enough results. Use a std::set to collect the results
from calling add_matching_symbols.
* symtab.h (global_symbol_searcher) <set_max_seach_results>: New
member function.
(global_symbol_searcher) <m_max_search_results>: New member
variable.
(global_symbol_searcher) <add_matching_symbols>: Update header
comment and change return type to bool.
(global_symbol_searcher) <add_matching_msymbols>: Update header
comment and change return type to bool.

gdb/doc/ChangeLog:

* doc/gdb.texinfo (GDB/MI Symbol Query): Add documentation of
-max-results to some -symbol-info-* commands.

gdb/testsuite/ChangeLog:

* gdb.mi/mi-sym-info.exp: Add tests for -max-results parameter.

Change-Id: I90a28feb55b388fb46461a096c5db08b6b0bd427
---
M gdb/ChangeLog
M gdb/doc/ChangeLog
M gdb/doc/gdb.texinfo
M gdb/mi/mi-symbol-cmds.c
M gdb/symtab.c
M gdb/symtab.h
M gdb/testsuite/ChangeLog
M gdb/testsuite/gdb.mi/mi-sym-info.exp
8 files changed, 179 insertions(+), 35 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 53161f5..4a8cb96 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,32 @@
2019-12-02 Andrew Burgess <andrew.burgess@embecosm.com>

+ * mi/mi-symbol-cmds.c (mi_symbol_info): Take extra parameter, and
+ add it into the search spec.
+ (parse_max_results_option): New function.
+ (mi_info_functions_or_variables): Parse -max-results flag and pass
+ it to mi_symbol_info.
+ (mi_cmd_symbol_info_modules): Likewise.
+ (mi_cmd_symbol_info_types): Likewise.
+ * symtab.c (global_symbol_searcher::add_matching_symbols): Change
+ return type to bool, change result container into a set, and don't
+ add new results if we have enough already.
+ (global_symbol_searcher::add_matching_msymbols): Change return
+ type to bool, and don't add new results if we have enough already.
+ (sort_search_symbols_remove_dups): Delete.
+ (global_symbol_searcher::search): Early exit from search loop when
+ we have enough results. Use a std::set to collect the results
+ from calling add_matching_symbols.
+ * symtab.h (global_symbol_searcher) <set_max_seach_results>: New
+ member function.
+ (global_symbol_searcher) <m_max_search_results>: New member
+ variable.
+ (global_symbol_searcher) <add_matching_symbols>: Update header
+ comment and change return type to bool.
+ (global_symbol_searcher) <add_matching_msymbols>: Update header
+ comment and change return type to bool.
+
+2019-12-02 Andrew Burgess <andrew.burgess@embecosm.com>
+
* symtab.c (symbol_search::compare_search_syms): Update header
comment.
(global_symbol_searcher::is_suitable_msymbol): New function.
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 10e2207..df6026f 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,5 +1,10 @@
2019-12-02 Andrew Burgess <andrew.burgess@embecosm.com>

+ * doc/gdb.texinfo (GDB/MI Symbol Query): Add documentation of
+ -max-results to some -symbol-info-* commands.
+
+2019-12-02 Andrew Burgess <andrew.burgess@embecosm.com>
+
* doc/gdb.texinfo (GDB/MI Symbol Query): Document new MI command
-symbol-info-module-functions and -symbol-info-module-variables.

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 6fb7e8a..544e632 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -33962,6 +33962,7 @@
-symbol-info-functions [--include-nondebug]
[--type @var{type_regexp}]
[--name @var{name_regexp}]
+ [--max-results @var{limit}]
@end smallexample

@noindent
@@ -33977,6 +33978,11 @@
to be filtered based on either the name of the function, or the type
signature of the function.

+The option @code{--max-results} restricts the command to return no
+more than @var{limit} results. If exactly @var{limit} results are
+returned then there might be additional results available if a higher
+limit is used.
+
@subsubheading @value{GDBN} Command

The corresponding @value{GDBN} command is @samp{info functions}.
@@ -34208,6 +34214,8 @@

@smallexample
-symbol-info-modules [--name @var{name_regexp}]
+ [--max-results @var{limit}]
+
@end smallexample

@noindent
@@ -34218,6 +34226,11 @@
The option @code{--name} allows the modules returned to be filtered
based the name of the module.

+The option @code{--max-results} restricts the command to return no
+more than @var{limit} results. If exactly @var{limit} results are
+returned then there might be additional results available if a higher
+limit is used.
+
@subsubheading @value{GDBN} Command

The corresponding @value{GDBN} command is @samp{info modules}.
@@ -34262,6 +34275,8 @@

@smallexample
-symbol-info-types [--name @var{name_regexp}]
+ [--max-results @var{limit}]
+
@end smallexample

@noindent
@@ -34275,6 +34290,11 @@
The option @code{--name} allows the list of types returned to be
filtered by name.

+The option @code{--max-results} restricts the command to return no
+more than @var{limit} results. If exactly @var{limit} results are
+returned then there might be additional results available if a higher
+limit is used.
+
@subsubheading @value{GDBN} Command

The corresponding @value{GDBN} command is @samp{info types}.
@@ -34322,6 +34342,8 @@
-symbol-info-variables [--include-nondebug]
[--type @var{type_regexp}]
[--name @var{name_regexp}]
+ [--max-results @var{limit}]
+
@end smallexample

@noindent
@@ -34337,6 +34359,11 @@
to be filtered based on either the name of the variable, or the type
of the variable.

+The option @code{--max-results} restricts the command to return no
+more than @var{limit} results. If exactly @var{limit} results are
+returned then there might be additional results available if a higher
+limit is used.
+
@subsubheading @value{GDBN} Command

The corresponding @value{GDBN} command is @samp{info variables}.
diff --git a/gdb/mi/mi-symbol-cmds.c b/gdb/mi/mi-symbol-cmds.c
index 6334e39..59e1112 100644
--- a/gdb/mi/mi-symbol-cmds.c
+++ b/gdb/mi/mi-symbol-cmds.c
@@ -111,11 +111,13 @@

static void
mi_symbol_info (enum search_domain kind, const char *name_regexp,
- const char *type_regexp, bool exclude_minsyms)
+ const char *type_regexp, bool exclude_minsyms,
+ size_t max_results)
{
global_symbol_searcher sym_search (kind, name_regexp);
sym_search.set_symbol_type_regexp (type_regexp);
sym_search.set_exclude_minsyms (exclude_minsyms);
+ sym_search.set_max_search_results (max_results);
std::vector<symbol_search> symbols = sym_search.search ();
ui_out *uiout = current_uiout;
int i = 0;
@@ -166,25 +168,42 @@
}
}

+/* Helper to parse the option text from an -max-results argument and return
+ the parsed value. If the text can't be parsed then an error is thrown. */
+
+static size_t
+parse_max_results_option (const char *arg)
+{
+ char *ptr = arg;
+ long long val = strtoll (arg, &ptr, 10);
+ if (arg == ptr || val > SIZE_MAX || val < 0)
+ error (_("invalid value for --max-results argument"));
+ size_t max_results = (size_t) val;
+
+ return max_results;
+}
+
/* Helper for mi_cmd_symbol_info_{functions,variables} - depending on KIND.
Processes command line options from ARGV and ARGC. */

static void
mi_info_functions_or_variables (enum search_domain kind, char **argv, int argc)
{
+ size_t max_results = SIZE_MAX;
const char *regexp = nullptr;
const char *t_regexp = nullptr;
bool exclude_minsyms = true;

enum opt
{
- INCLUDE_NONDEBUG_OPT, TYPE_REGEXP_OPT, NAME_REGEXP_OPT
+ INCLUDE_NONDEBUG_OPT, TYPE_REGEXP_OPT, NAME_REGEXP_OPT, MAX_RESULTS_OPT
};
static const struct mi_opt opts[] =
{
{"-include-nondebug" , INCLUDE_NONDEBUG_OPT, 0},
{"-type", TYPE_REGEXP_OPT, 1},
{"-name", NAME_REGEXP_OPT, 1},
+ {"-max-results", MAX_RESULTS_OPT, 1},
{ 0, 0, 0 }
};

@@ -210,10 +229,13 @@
case NAME_REGEXP_OPT:
regexp = oarg;
break;
+ case MAX_RESULTS_OPT:
+ parse_max_results_option = parse_max_results_option (oarg);
+ break;
}
}

- mi_symbol_info (kind, regexp, t_regexp, exclude_minsyms);
+ mi_symbol_info (kind, regexp, t_regexp, exclude_minsyms, max_results);
}

/* Helper for mi_info_module_functions_or_variables. Display the results
@@ -382,15 +404,17 @@
void
mi_cmd_symbol_info_modules (const char *command, char **argv, int argc)
{
+ size_t max_results = SIZE_MAX;
const char *regexp = nullptr;

enum opt
{
- NAME_REGEXP_OPT
+ NAME_REGEXP_OPT, MAX_RESULTS_OPT
};
static const struct mi_opt opts[] =
{
{"-name", NAME_REGEXP_OPT, 1},
+ {"-max-results", MAX_RESULTS_OPT, 1},
{ 0, 0, 0 }
};

@@ -408,10 +432,13 @@
case NAME_REGEXP_OPT:
regexp = oarg;
break;
+ case MAX_RESULTS_OPT:
+ parse_max_results_option = parse_max_results_option (oarg);
+ break;
}
}

- mi_symbol_info (MODULES_DOMAIN, regexp, nullptr, true);
+ mi_symbol_info (MODULES_DOMAIN, regexp, nullptr, true, max_results);
}

/* Implement -symbol-info-types command. */
@@ -419,15 +446,17 @@
void
mi_cmd_symbol_info_types (const char *command, char **argv, int argc)
{
+ size_t max_results = SIZE_MAX;
const char *regexp = nullptr;

enum opt
{
- NAME_REGEXP_OPT
+ NAME_REGEXP_OPT, MAX_RESULTS_OPT
};
static const struct mi_opt opts[] =
{
{"-name", NAME_REGEXP_OPT, 1},
+ {"-max-results", MAX_RESULTS_OPT, 1},
{ 0, 0, 0 }
};

@@ -445,10 +474,13 @@
case NAME_REGEXP_OPT:
regexp = oarg;
break;
+ case MAX_RESULTS_OPT:
+ parse_max_results_option = parse_max_results_option (oarg);
+ break;
}
}

- mi_symbol_info (TYPES_DOMAIN, regexp, nullptr, true);
+ mi_symbol_info (TYPES_DOMAIN, regexp, nullptr, true, max_results);
}

/* Implement -symbol-info-variables command. */
diff --git a/gdb/symtab.c b/gdb/symtab.c
index f3f0bd5..88b5bd8 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4534,12 +4534,12 @@

/* See symtab.h. */

-void
+bool
global_symbol_searcher::add_matching_symbols
(objfile *objfile,
const gdb::optional<compiled_regex> &preg,
const gdb::optional<compiled_regex> &treg,
- std::vector<symbol_search> *results) const
+ std::set<symbol_search> *result_set) const
{
enum search_domain kind = m_kind;

@@ -4597,17 +4597,26 @@
&& SYMBOL_DOMAIN (sym) == MODULE_DOMAIN
&& SYMBOL_LINE (sym) != 0))))
{
- /* Match, insert if not already in the results. */
- results->emplace_back (block, sym);
+ if (result_set->size () < m_max_search_results)
+ {
+ /* Match, insert if not already in the results. */
+ symbol_search ss (block, sym);
+ if (result_set->find (ss) == result_set->end ())
+ result_set->insert (ss);
+ }
+ else
+ return false;
}
}
}
}
+
+ return true;
}

/* See symtab.h. */

-void
+bool
global_symbol_searcher::add_matching_msymbols
(objfile *objfile, const gdb::optional<compiled_regex> &preg,
std::vector<symbol_search> *results) const
@@ -4639,22 +4648,17 @@
VAR_DOMAIN).symbol == NULL)
{
/* Matching msymbol, add it to the results list. */
- results->emplace_back (GLOBAL_BLOCK, msymbol, objfile);
+ if (results->size () < m_max_search_results)
+ results->emplace_back (GLOBAL_BLOCK, msymbol, objfile);
+ else
+ return false;
}
}
}
}
}
-}

-/* Sort the symbols in RESULT and remove duplicates. */
-
-static void
-sort_search_symbols_remove_dups (std::vector<symbol_search> *result)
-{
- std::sort (result->begin (), result->end ());
- result->erase (std::unique (result->begin (), result->end ()),
- result->end ());
+ return true;
}

/* See symtab.h. */
@@ -4720,20 +4724,24 @@
}

bool found_msymbol = false;
- std::vector<symbol_search> result;
+ std::set<symbol_search> result_set;
for (objfile *objfile : current_program_space->objfiles ())
{
/* Expand symtabs within objfile that possibly contain matching
symbols. */
found_msymbol |= expand_symtabs (objfile, preg);

- /* Find matching symbols within OBJFILE and add them in to the RESULT
- vector. */
- add_matching_symbols (objfile, preg, treg, &result);
+ /* Find matching symbols within OBJFILE and add them in to the
+ RESULT_SET set. Use a set here so that we can easily detect
+ duplicates as we go, and can therefore track how many unique
+ matches we have found so far. */
+ if (!add_matching_symbols (objfile, preg, treg, &result_set))
+ break;
}

- if (!result.empty ())
- sort_search_symbols_remove_dups (&result);
+ /* Convert the result set into a sorted result list. */
+ std::vector<symbol_search> result (result_set.begin (), result_set.end ());
+ std::sort (result.begin (), result.end ());

/* 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
@@ -4745,7 +4753,8 @@
{
gdb_assert (m_kind == VARIABLES_DOMAIN || m_kind == FUNCTIONS_DOMAIN);
for (objfile *objfile : current_program_space->objfiles ())
- add_matching_msymbols (objfile, preg, &result);
+ if (!add_matching_msymbols (objfile, preg, &result))
+ break;
}

return result;
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 41abf1f..e8321d4 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -23,6 +23,7 @@
#include <array>
#include <vector>
#include <string>
+#include <set>
#include "gdbsupport/gdb_vecs.h"
#include "gdbtypes.h"
#include "gdb_obstack.h"
@@ -2092,6 +2093,12 @@
m_exclude_minsyms = exclude_minsyms;
}

+ /* Set the maximum number of search results to be returned. */
+ void set_max_search_results (size_t max_search_results)
+ {
+ m_max_search_results = max_search_results;
+ }
+
/* Search the symbols from all objfiles in the current program space
looking for matches as defined by the current state of this object.

@@ -2125,6 +2132,10 @@
be included in the results, otherwise they are excluded. */
bool m_exclude_minsyms = false;

+ /* Maximum number of search results. We currently impose a hard limit
+ of SIZE_MAX, there is no "unlimited". */
+ size_t m_max_search_results = SIZE_MAX;
+
/* Expand symtabs in OBJFILE that match PREG, are of type M_KIND. Return
true if any msymbols were seen that we should later consider adding to
the results list. */
@@ -2132,15 +2143,23 @@
const gdb::optional<compiled_regex> &preg) const;

/* Add symbols from symtabs in OBJFILE that match PREG, and TREG, and are
- of type M_KIND, to the results vector RESULTS. */
- void add_matching_symbols (objfile *objfile,
+ of type M_KIND, to the results set RESULTS_SET. Return false if we
+ stop adding results early due to having already found too many results
+ (based on M_MAX_SEARCH_RESULTS limit), otherwise return true.
+ Returning true does not indicate that any results were added, just
+ that we didn't _not_ add a result due to reaching MAX_SEARCH_RESULTS. */
+ bool add_matching_symbols (objfile *objfile,
const gdb::optional<compiled_regex> &preg,
const gdb::optional<compiled_regex> &treg,
- std::vector<symbol_search> *results) const;
+ std::set<symbol_search> *result_set) const;

- /* Add msymbols from OBJFILE that match PREG and M_KIND, to the
- results vector RESULTS. */
- void add_matching_msymbols (objfile *objfile,
+ /* Add msymbols from OBJFILE that match PREG and M_KIND, to the results
+ vector RESULTS. Return false if we stop adding results early due to
+ having already found too many results (based on max search results
+ limit M_MAX_SEARCH_RESULTS), otherwise return true. Returning true
+ does not indicate that any results were added, just that we didn't
+ _not_ add a result due to reaching MAX_SEARCH_RESULTS. */
+ bool add_matching_msymbols (objfile *objfile,
const gdb::optional<compiled_regex> &preg,
std::vector<symbol_search> *results) const;

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 7fa48e3..765fb84 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,9 @@
2019-12-02 Andrew Burgess <andrew.burgess@embecosm.com>

+ * gdb.mi/mi-sym-info.exp: Add tests for -max-results parameter.
+
+2019-12-02 Andrew Burgess <andrew.burgess@embecosm.com>
+
* gdb.mi/mi-fortran-modules.exp: Add additional tests for
-symbol-info-module-functions and -symbol-info-module-variables.

diff --git a/gdb/testsuite/gdb.mi/mi-sym-info.exp b/gdb/testsuite/gdb.mi/mi-sym-info.exp
index 33fe865..4a65bd6 100644
--- a/gdb/testsuite/gdb.mi/mi-sym-info.exp
+++ b/gdb/testsuite/gdb.mi/mi-sym-info.exp
@@ -127,3 +127,24 @@
mi_gdb_test "120-symbol-info-types --name _int_" \
"120\\^done,symbols=\{debug=\\\[\{filename=\"\[^\"\]+$srcfile\",fullname=\"\[^\"\]+$srcfile\",symbols=\\\[\{line=\"27\",name=\"my_int_t\"\}\\\]\},\{filename=\"\[^\"\]+$srcfile2\",fullname=\"\[^\"\]+$srcfile2\",symbols=\\\[\{line=\"23\",name=\"another_int_t\"\}\\\]\}\\\]\}" \
"List all types matching _int_"
+
+# Test the --max-results parameter.
+mi_gdb_test "121-symbol-info-functions --max-results 0" \
+ "121\\^done,symbols=\{\}" \
+ "-symbol-info-functions --max-results 0"
+
+mi_gdb_test "122-symbol-info-functions --max-results 1" \
+ "122\\^done,symbols=\{debug=\\\[\{filename=\"\[^\"\]+$srcfile2\",fullname=\"\[^\"\]+$srcfile2\",symbols=\\\[\{line=\"39\",name=\"f3\",type=\"int \\(another_int_t\\)\",description=\"int f3\\(another_int_t\\);\"\}\\\]\}\\\]\}" \
+ "-symbol-info-functions --max-results 1"
+
+mi_gdb_test "123-symbol-info-functions --max-results 2" \
+ "123\\^done,symbols=\{debug=\\\[\{filename=\"\[^\"\]+$srcfile2\",fullname=\"\[^\"\]+$srcfile2\",symbols=\\\[\{line=\"33\",name=\"f2\",type=\"float \\(another_float_t\\)\",description=\"float f2\\(another_float_t\\);\"\},\{line=\"39\",name=\"f3\",type=\"int \\(another_int_t\\)\",description=\"int f3\\(another_int_t\\);\"\}\\\]\}\\\]\}" \
+ "-symbol-info-functions --max-results 2"
+
+mi_gdb_test "124-symbol-info-variables --max-results 3" \
+ "124\\^done,symbols=\{debug=\\\[\{filename=\"\[^\"\]+$srcfile2\",fullname=\"\[^\"\]+$srcfile2\",symbols=\\\[\{line=\"21\",name=\"global_f2\",type=\"int\",description=\"int global_f2;\"\},\{line=\"20\",name=\"global_i2\",type=\"int\",description=\"int global_i2;\"\},\{line=\"19\",name=\"global_f1\",type=\"float\",description=\"static float global_f1;\"\}\\\]\}\\\]\}" \
+ "-symbol-info-variables --max-results 3"
+
+mi_gdb_test "125-symbol-info-types --max-results 4" \
+ "125\\^done,symbols=\{debug=\\\[\{filename=\"\[^\"\]+$srcfile2\",fullname=\"\[^\"\]+$srcfile2\",symbols=\\\[\{line=\"24\",name=\"another_float_t\"\},\{line=\"23\",name=\"another_int_t\"\},\{name=\"float\"\},\{name=\"int\"\}\\\]\}\\\]\}" \
+ "-symbol-info-types --max-results 4"

--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I90a28feb55b388fb46461a096c5db08b6b0bd427
Gerrit-Change-Number: 269
Gerrit-PatchSet: 11