* [PATCH 0/3 v2] Limit tab-completion result when list is large
@ 2014-11-06 10:50 Gary Benson
2014-11-06 10:51 ` [PATCH 1/3 v2] Add expansion_notify callback to expand_symtabs_matching Gary Benson
` (2 more replies)
0 siblings, 3 replies; 29+ messages in thread
From: Gary Benson @ 2014-11-06 10:50 UTC (permalink / raw)
To: gdb-patches
Hi all,
When debugging large programs, pressing the Tab key at the wrong
time causes GDB to lock up while it builds a completion list. As
an example, with LibreOffice:
$ gdb /usr/lib64/libreoffice/program/soffice.bin
(gdb) start
(gdb) b <Tab>
On my (pretty fast!) machine the user is left hanging for nearly a
minute while GDB builds a list of the 212405 places the user could
break on. This is PR cli/11920.
PR cli/15548 proposes to abort building the completion list if it
grows too large, the theory being that nobody is going to scroll
through thousands of screens of completions to find the one they
want. These three patches implement this. The default limit is 200
completions, which the user can override with a new set/show option.
Tested on RHEL6.5 x86_64, no regressions.
Is this ok to commit?
Thanks,
Gary
--
http://gbenson.net/
^ permalink raw reply [flat|nested] 29+ messages in thread* [PATCH 1/3 v2] Add expansion_notify callback to expand_symtabs_matching 2014-11-06 10:50 [PATCH 0/3 v2] Limit tab-completion result when list is large Gary Benson @ 2014-11-06 10:51 ` Gary Benson 2014-11-06 10:51 ` [PATCH 3/3 v2] Implement completion limiting Gary Benson 2014-11-06 10:51 ` [PATCH 2/3 v2] Interleave completion list building with symbol table expansion Gary Benson 2 siblings, 0 replies; 29+ messages in thread From: Gary Benson @ 2014-11-06 10:51 UTC (permalink / raw) To: gdb-patches This commit adds a new callback parameter, "expansion_notify", to the top-level expand_symtabs_matching function and to all the vectorized functions it defers to. If expansion_notify is non-NULL, it will be called every time a symbol table is expanded. gdb/ChangeLog: * symfile.h (expand_symtabs_exp_notify_ftype): New typedef. (struct quick_symbol_functions) <expand_symtabs_matching>: New argument expansion_notify. All uses updated. (expand_symtabs_matching): New argument expansion_notify. All uses updated. * symfile-debug.c (debug_qf_expand_symtabs_matching): Also print expansion notify. * symtab.c (expand_symtabs_matching_via_partial): Call expansion_notify whenever a partial symbol table is expanded. * dwarf2read.c (dw2_expand_symtabs_matching): Call expansion_notify whenever a symbol table is instantiated. --- gdb/ChangeLog | 14 ++++++++++++++ gdb/ada-lang.c | 6 +++--- gdb/dwarf2read.c | 14 +++++++++++++- gdb/linespec.c | 2 +- gdb/psymtab.c | 8 +++++++- gdb/symfile-debug.c | 5 ++++- gdb/symfile.c | 4 +++- gdb/symfile.h | 8 ++++++++ gdb/symmisc.c | 2 +- gdb/symtab.c | 6 +++--- 10 files changed, 57 insertions(+), 12 deletions(-) diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c index 5793cd2..8f6e086 100644 --- a/gdb/ada-lang.c +++ b/gdb/ada-lang.c @@ -6178,8 +6178,8 @@ ada_make_symbol_completion_list (const char *text0, const char *word, data.word = word; data.wild_match = wild_match_p; data.encoded = encoded_p; - expand_symtabs_matching (NULL, ada_complete_symbol_matcher, ALL_DOMAIN, - &data); + expand_symtabs_matching (NULL, ada_complete_symbol_matcher, NULL, + ALL_DOMAIN, &data); } /* At this point scan through the misc symbol vectors and add each @@ -12839,7 +12839,7 @@ ada_add_global_exceptions (regex_t *preg, VEC(ada_exc_info) **exceptions) struct objfile *objfile; struct symtab *s; - expand_symtabs_matching (NULL, ada_exc_search_name_matches, + expand_symtabs_matching (NULL, ada_exc_search_name_matches, NULL, VARIABLES_DOMAIN, preg); ALL_PRIMARY_SYMTABS (objfile, s) diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index 3f2a127..402cf96 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -3782,6 +3782,7 @@ dw2_expand_symtabs_matching (struct objfile *objfile, expand_symtabs_file_matcher_ftype *file_matcher, expand_symtabs_symbol_matcher_ftype *symbol_matcher, + expand_symtabs_exp_notify_ftype *expansion_notify, enum search_domain kind, void *data) { @@ -3953,7 +3954,18 @@ dw2_expand_symtabs_matching per_cu = dw2_get_cutu (cu_index); if (file_matcher == NULL || per_cu->v.quick->mark) - dw2_instantiate_symtab (per_cu); + { + int symtab_was_null = (per_cu->v.quick->symtab == NULL); + + dw2_instantiate_symtab (per_cu); + + if (expansion_notify != NULL + && symtab_was_null + && per_cu->v.quick->symtab != NULL) + { + expansion_notify (per_cu->v.quick->symtab, data); + } + } } } } diff --git a/gdb/linespec.c b/gdb/linespec.c index 5325702..21d7011 100644 --- a/gdb/linespec.c +++ b/gdb/linespec.c @@ -1028,7 +1028,7 @@ iterate_over_all_matching_symtabs (struct linespec_state *state, if (objfile->sf) objfile->sf->qf->expand_symtabs_matching (objfile, NULL, iterate_name_matcher, - ALL_DOMAIN, + NULL, ALL_DOMAIN, &matcher_data); ALL_OBJFILE_PRIMARY_SYMTABS (objfile, symtab) diff --git a/gdb/psymtab.c b/gdb/psymtab.c index 6c0c880..e28b100 100644 --- a/gdb/psymtab.c +++ b/gdb/psymtab.c @@ -1364,6 +1364,7 @@ expand_symtabs_matching_via_partial (struct objfile *objfile, expand_symtabs_file_matcher_ftype *file_matcher, expand_symtabs_symbol_matcher_ftype *symbol_matcher, + expand_symtabs_exp_notify_ftype *expansion_notify, enum search_domain kind, void *data) { @@ -1406,7 +1407,12 @@ expand_symtabs_matching_via_partial } if (recursively_search_psymtabs (ps, objfile, kind, symbol_matcher, data)) - psymtab_to_symtab (objfile, ps); + { + struct symtab *symtab = psymtab_to_symtab (objfile, ps); + + if (expansion_notify != NULL) + expansion_notify (symtab, data); + } } } diff --git a/gdb/symfile-debug.c b/gdb/symfile-debug.c index 170ba3a..75ef025 100644 --- a/gdb/symfile-debug.c +++ b/gdb/symfile-debug.c @@ -287,22 +287,25 @@ debug_qf_expand_symtabs_matching (struct objfile *objfile, expand_symtabs_file_matcher_ftype *file_matcher, expand_symtabs_symbol_matcher_ftype *symbol_matcher, + expand_symtabs_exp_notify_ftype *expansion_notify, enum search_domain kind, void *data) { const struct debug_sym_fns_data *debug_data = objfile_data (objfile, symfile_debug_objfile_data_key); fprintf_filtered (gdb_stdlog, - "qf->expand_symtabs_matching (%s, %s, %s, %s, %s)\n", + "qf->expand_symtabs_matching (%s, %s, %s, %s, %s, %s)\n", debug_objfile_name (objfile), host_address_to_string (file_matcher), host_address_to_string (symbol_matcher), + host_address_to_string (expansion_notify), search_domain_name (kind), host_address_to_string (data)); debug_data->real_sf->qf->expand_symtabs_matching (objfile, file_matcher, symbol_matcher, + expansion_notify, kind, data); } diff --git a/gdb/symfile.c b/gdb/symfile.c index b16abe4..b35ddd9 100644 --- a/gdb/symfile.c +++ b/gdb/symfile.c @@ -3902,6 +3902,7 @@ symfile_free_objfile (struct objfile *objfile) void expand_symtabs_matching (expand_symtabs_file_matcher_ftype *file_matcher, expand_symtabs_symbol_matcher_ftype *symbol_matcher, + expand_symtabs_exp_notify_ftype *expansion_notify, enum search_domain kind, void *data) { @@ -3911,7 +3912,8 @@ expand_symtabs_matching (expand_symtabs_file_matcher_ftype *file_matcher, { if (objfile->sf) objfile->sf->qf->expand_symtabs_matching (objfile, file_matcher, - symbol_matcher, kind, + symbol_matcher, + expansion_notify, kind, data); } } diff --git a/gdb/symfile.h b/gdb/symfile.h index f56aff3..bcff871 100644 --- a/gdb/symfile.h +++ b/gdb/symfile.h @@ -137,6 +137,12 @@ typedef int (expand_symtabs_file_matcher_ftype) (const char *filename, typedef int (expand_symtabs_symbol_matcher_ftype) (const char *name, void *data); +/* Callback for quick_symbol_functions->expand_symtabs_matching + to be called after a symtab has been expanded. */ + +typedef void (expand_symtabs_exp_notify_ftype) (struct symtab *symtab, + void *data); + /* The "quick" symbol functions exist so that symbol readers can avoiding an initial read of all the symbols. For example, symbol readers might choose to use the "partial symbol table" utilities, @@ -282,6 +288,7 @@ struct quick_symbol_functions (struct objfile *objfile, expand_symtabs_file_matcher_ftype *file_matcher, expand_symtabs_symbol_matcher_ftype *symbol_matcher, + expand_symtabs_exp_notify_ftype *expansion_notify, enum search_domain kind, void *data); @@ -565,6 +572,7 @@ extern struct cleanup *increment_reading_symtab (void); void expand_symtabs_matching (expand_symtabs_file_matcher_ftype *, expand_symtabs_symbol_matcher_ftype *, + expand_symtabs_exp_notify_ftype *, enum search_domain kind, void *data); void map_symbol_filenames (symbol_filename_ftype *fun, void *data, diff --git a/gdb/symmisc.c b/gdb/symmisc.c index 223a7d1..e48fae7 100644 --- a/gdb/symmisc.c +++ b/gdb/symmisc.c @@ -897,7 +897,7 @@ maintenance_expand_symtabs (char *args, int from_tty) { objfile->sf->qf->expand_symtabs_matching (objfile, maintenance_expand_file_matcher, - maintenance_expand_name_matcher, ALL_DOMAIN, regexp); + maintenance_expand_name_matcher, NULL, ALL_DOMAIN, regexp); } } diff --git a/gdb/symtab.c b/gdb/symtab.c index ed164f7..836f3bc 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -3775,7 +3775,7 @@ search_symbols (const char *regexp, enum search_domain kind, ? NULL : search_symbols_file_matches), search_symbols_name_matches, - kind, &datum); + NULL, kind, &datum); /* Here, we search through the minimal symbol tables for functions and variables that match, and force their symbols to be read. @@ -4564,8 +4564,8 @@ default_make_symbol_completion_list_break_on (const char *text, /* Look through the partial symtabs for all symbols which begin by matching SYM_TEXT. Expand all CUs that you find to the list. The real names will get added by COMPLETION_LIST_ADD_SYMBOL below. */ - expand_symtabs_matching (NULL, symbol_completion_matcher, ALL_DOMAIN, - &datum); + expand_symtabs_matching (NULL, symbol_completion_matcher, NULL, + ALL_DOMAIN, &datum); /* At this point scan through the misc symbol vectors and add each symbol you find to the list. Eventually we want to ignore -- 1.7.1 ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 3/3 v2] Implement completion limiting 2014-11-06 10:50 [PATCH 0/3 v2] Limit tab-completion result when list is large Gary Benson 2014-11-06 10:51 ` [PATCH 1/3 v2] Add expansion_notify callback to expand_symtabs_matching Gary Benson @ 2014-11-06 10:51 ` Gary Benson 2014-11-06 16:28 ` Eli Zaretskii 2014-11-24 6:09 ` Doug Evans 2014-11-06 10:51 ` [PATCH 2/3 v2] Interleave completion list building with symbol table expansion Gary Benson 2 siblings, 2 replies; 29+ messages in thread From: Gary Benson @ 2014-11-06 10:51 UTC (permalink / raw) To: gdb-patches This commit adds a new exception, TOO_MANY_COMPLETIONS_ERROR, to be thrown whenever the completer has generated too many possibilities to be useful. A new user-settable variable, "max_completions", is added to control this behaviour. A top-level completion limit is added to complete_line_internal, as the final check to ensure the user never sees too many completions. An additional limit is added to default_make_symbol_completion_list_break_on, to halt time-consuming symbol table expansions. gdb/ChangeLog: PR cli/9007 PR cli/11920 PR cli/15548 * common/common-exceptions.h (enum errors) <TOO_MANY_COMPLETIONS_ERROR>: New value. * completer.h (completion_tracker_t): New typedef. (new_completion_tracker): New declaration. (make_cleanup_free_completion_tracker): Likewise. (maybe_limit_completions): Likewise. * completer.c [TUI]: Include tui/tui.h and tui/tui-io.h. (max_completions): New static variable. (new_completion_tracker): New function. (make_cleanup_free_completion_tracker): Likewise. (maybe_limit_completions): Likewise. (complete_line_internal): Do not generate any completions if max_completions = 0. Limit the number of completions if max_completions >= 0. (line_completion_function): Handle TOO_MANY_COMPLETIONS_ERROR. (_initialize_completer): New declaration and function. * symtab.c: Include completer.h. (completion_tracker): New static variable. (completion_list_add_name): Call maybe_limit_completions. (default_make_symbol_completion_list_break_on): Maintain completion_tracker across calls to completion_list_add_name. * NEWS (New Options): Mention set/show max-completions. gdb/doc/ChangeLog: * gdb.texinfo (Command Completion): Document new "set/show max-completions" option. gdb/testsuite/ChangeLog: * gdb.base/completion.exp: Disable completion limiting for existing tests. Add new tests to check completion limiting. * gdb.linespec/ls-errs.exp: Disable completion limiting. --- gdb/ChangeLog | 28 ++++++ gdb/NEWS | 11 ++- gdb/common/common-exceptions.h | 4 + gdb/completer.c | 146 ++++++++++++++++++++++++++++++-- gdb/completer.h | 23 +++++ gdb/doc/ChangeLog | 5 + gdb/doc/gdb.texinfo | 25 ++++++ gdb/symtab.c | 22 +++++- gdb/testsuite/ChangeLog | 6 ++ gdb/testsuite/gdb.base/completion.exp | 33 +++++++ gdb/testsuite/gdb.linespec/ls-errs.exp | 3 + 11 files changed, 298 insertions(+), 8 deletions(-) diff --git a/gdb/NEWS b/gdb/NEWS index 649c29e..b5d971e 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -22,11 +22,20 @@ ** $_any_caller_is(name [, number_of_frames]) ** $_any_caller_matches(regexp [, number_of_frames]) -* New commands +* New commands (for set/show, see "New options" below) queue-signal signal-name-or-number Queue a signal to be delivered to the thread when it is resumed. +* New options + +set max-completions +show max-completions + Set the maximum number of possibilities to be considered during + completion. The default value is 200. This limit allows GDB to + avoid generating large completion lists, the computation of which + can cause the debugger to become temporarily unresponsive. + * On resume, GDB now always passes the signal the program had stopped for to the thread the signal was sent to, even if the user changed threads before resuming. Previously GDB would often (but not diff --git a/gdb/common/common-exceptions.h b/gdb/common/common-exceptions.h index 5f750c3..73fd255 100644 --- a/gdb/common/common-exceptions.h +++ b/gdb/common/common-exceptions.h @@ -99,6 +99,10 @@ enum errors { /* Requested feature, method, mechanism, etc. is not supported. */ NOT_SUPPORTED_ERROR, + /* The number of candidates generated during line completion has + exceeded the user's specified limit. */ + TOO_MANY_COMPLETIONS_ERROR, + /* Add more errors here. */ NR_ERRORS }; diff --git a/gdb/completer.c b/gdb/completer.c index a0f3fa3..b26d403 100644 --- a/gdb/completer.c +++ b/gdb/completer.c @@ -39,6 +39,11 @@ #include "completer.h" +#ifdef TUI +#include "tui/tui.h" +#include "tui/tui-io.h" +#endif + /* Prototypes for local functions. */ static char *line_completion_function (const char *text, int matches, @@ -778,9 +783,67 @@ complete_line_internal (const char *text, return list; } -/* Generate completions all at once. Returns a vector of strings. - Each element is allocated with xmalloc. It can also return NULL if - there are no completions. + +/* Maximum number of possibilities to consider before the completer + bails by throwing TOO_MANY_COMPLETIONS_ERROR. Negative values + disable limiting. */ +static int max_completions = 200; + +/* See completer.h. */ + +completion_tracker_t +new_completion_tracker (void) +{ + if (max_completions < 1) + return NULL; + + return htab_create_alloc (max_completions, + htab_hash_string, (htab_eq) streq, + NULL, xcalloc, xfree); +} + +/* See completer.h. */ + +struct cleanup * +make_cleanup_free_completion_tracker (completion_tracker_t tracker) +{ + if (tracker == NULL) + return make_cleanup (null_cleanup, NULL); + + return make_cleanup_htab_delete (tracker); +} + +/* See completer.h. */ + +void +maybe_limit_completions (completion_tracker_t tracker, char *name) +{ + if (max_completions < 0) + return; + + if (tracker != NULL) + { + void **slot = htab_find_slot (tracker, name, INSERT); + + if (*slot != HTAB_EMPTY_ENTRY) + return; + + if (htab_elements (tracker) <= max_completions) + { + *slot = name; + return; + } + } + + throw_error (TOO_MANY_COMPLETIONS_ERROR, + _("Too many possibilities.")); +} + +/* Generate completions all at once. Returns a vector of strings + allocated with xmalloc. Returns NULL if there are no completions + or if max_completions is 0. Throws TOO_MANY_COMPLETIONS_ERROR if + max_completions is greater than zero and the number of completions + is greater than max_completions. TEXT is the caller's idea of the "word" we are looking at. @@ -793,8 +856,33 @@ complete_line_internal (const char *text, VEC (char_ptr) * complete_line (const char *text, const char *line_buffer, int point) { - return complete_line_internal (text, line_buffer, - point, handle_completions); + VEC (char_ptr) *list = NULL; + struct cleanup *old_chain; + + list = complete_line_internal (text, line_buffer, point, + handle_completions); + old_chain = make_cleanup_free_char_ptr_vec (list); + + /* Possibly throw TOO_MANY_COMPLETIONS_ERROR. Individual + completers may do this too, to avoid unnecessary work, + but this is the ultimate check that stops users seeing + more completions than they wanted. */ + if (max_completions >= 0) + { + completion_tracker_t tracker = new_completion_tracker (); + struct cleanup *limit_chain = + make_cleanup_free_completion_tracker (tracker); + char *candidate; + int ix; + + for (ix = 0; VEC_iterate (char_ptr, list, ix, candidate); ++ix) + maybe_limit_completions (tracker, candidate); + + do_cleanups (limit_chain); + } + + discard_cleanups (old_chain); + return list; } /* Complete on command names. Used by "help". */ @@ -881,6 +969,8 @@ line_completion_function (const char *text, int matches, if (matches == 0) { + volatile struct gdb_exception ex; + /* The caller is beginning to accumulate a new set of completions, so we need to find all of them now, and cache them for returning one at a time on future calls. */ @@ -894,7 +984,35 @@ line_completion_function (const char *text, int matches, VEC_free (char_ptr, list); } index = 0; - list = complete_line (text, line_buffer, point); + + TRY_CATCH (ex, RETURN_MASK_ALL) + list = complete_line (text, line_buffer, point); + + if (ex.reason < 0) + { + if (ex.error != TOO_MANY_COMPLETIONS_ERROR) + throw_exception (ex); + + if (rl_completion_type != TAB) + { +#if defined(TUI) + if (tui_active) + { + tui_puts ("\n"); + tui_puts (ex.message); + tui_puts ("\n"); + } + else +#endif + { + rl_crlf (); + fputs (ex.message, rl_outstream); + rl_crlf (); + } + + rl_on_new_line (); + } + } } /* If we found a list of potential completions during initialization @@ -978,3 +1096,19 @@ skip_quoted (const char *str) { return skip_quoted_chars (str, NULL, NULL); } + +extern initialize_file_ftype _initialize_completer; /* -Wmissing-prototypes */ + +void +_initialize_completer (void) +{ + add_setshow_zuinteger_unlimited_cmd ("max-completions", no_class, + &max_completions, _("\ +Set maximum number of line completion possibilities."), _("\ +Show maximum number of line completion possibilities."), _("\ +Use this to limit the number of possibilities considered\n\ +during line completion. Specifying \"unlimited\" or -1\n\ +disables limiting. Note that setting either no limit or\n\ +a very large limit can cause pauses during completion."), + NULL, NULL, &setlist, &showlist); +} diff --git a/gdb/completer.h b/gdb/completer.h index bc7ed96..5079343 100644 --- a/gdb/completer.h +++ b/gdb/completer.h @@ -63,4 +63,27 @@ extern const char *skip_quoted_chars (const char *, const char *, extern const char *skip_quoted (const char *); +/* Object to track how many unique completions have been generated. + Used to limit the size of generated completion lists. */ + +typedef htab_t completion_tracker_t; + +/* Create a new completion tracker. */ + +extern completion_tracker_t new_completion_tracker (void); + +/* Make a cleanup to free a completion tracker. */ + +extern struct cleanup *make_cleanup_free_completion_tracker + (completion_tracker_t tracker); + +/* Add the completion NAME to the list of generated completions if + it is not there already. Throw TOO_MANY_COMPLETIONS_ERROR if + max_completions >= 0 and the number of generated completions is + greater than max_completions. Do nothing if max_completions is + negative. */ + +extern void maybe_limit_completions (completion_tracker_t tracker, + char *name); + #endif /* defined (COMPLETER_H) */ diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 15c2908..2c63581 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -1600,6 +1600,31 @@ means @kbd{@key{META} ?}. You can type this either by holding down a key designated as the @key{META} shift on your keyboard (if there is one) while typing @kbd{?}, or as @key{ESC} followed by @kbd{?}. +If the number of possible completions is large, @value{GDBN} will +print a message rather than displaying the list: + +@smallexample +(@value{GDBP}) b @key{TAB}@key{TAB} +Too many possibilities. +(@value{GDBP}) b +@end smallexample + +@noindent +This behavior can be controlled with the following commands: + +@table @code +@kindex set max-completions +@item set max-completions @var{limit} +@itemx set max-completions unlimited +Set the maximum number of possibilities to be considered during +completion. The default value is 200. Note that setting either +no limit or a very large limit can cause pauses during completion. +@kindex show max-completions +@item show max-completions +Show the maximum number of possibilities to be considered during +completion. +@end table + @cindex quotes in commands @cindex completion of quoted strings Sometimes the string you need, while logically a ``word'', may contain diff --git a/gdb/symtab.c b/gdb/symtab.c index ebb7b8f..a2a1e09 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -60,6 +60,7 @@ #include "macroscope.h" #include "parser-defs.h" +#include "completer.h" /* Forward declarations for local functions. */ @@ -4256,6 +4257,15 @@ static VEC (char_ptr) *return_val; completion_list_add_name \ (MSYMBOL_NATURAL_NAME (symbol), (sym_text), (len), (text), (word)) +/* Tracker for how many unique completions have been generated. Used + to terminate completion list generation early if the list has grown + to a size so large as to be useless. This helps avoid GDB seeming + to lock up in the event the user requests to complete on something + vague that necessitates the time consuming expansion of many symbol + tables. */ + +completion_tracker_t completion_tracker; + /* Test to see if the symbol specified by SYMNAME (which is already demangled for C++ symbols) matches SYM_TEXT in the first SYM_TEXT_LEN characters. If so, add it to the current completion list. */ @@ -4296,6 +4306,12 @@ completion_list_add_name (const char *symname, } VEC_safe_push (char_ptr, return_val, new); + + /* Throw TOO_MANY_COMPLETIONS_ERROR if we've generated too many + completions. We check this after pushing NEW to RETURN_VAL so + it's freed by default_make_symbol_completion_list_break_on's + cleanup if the exception is thrown. */ + maybe_limit_completions (completion_tracker, new); } } @@ -4532,7 +4548,7 @@ default_make_symbol_completion_list_break_on (const char *text, /* Length of sym_text. */ int sym_text_len; struct add_name_data datum; - struct cleanup *back_to; + struct cleanup *back_to, *limit_chain; /* Now look for the symbol we are supposed to complete on. */ { @@ -4606,6 +4622,9 @@ default_make_symbol_completion_list_break_on (const char *text, return_val = NULL; back_to = make_cleanup (do_free_completion_list, &return_val); + completion_tracker = new_completion_tracker (); + limit_chain = make_cleanup_free_completion_tracker (completion_tracker); + datum.sym_text = sym_text; datum.sym_text_len = sym_text_len; datum.text = text; @@ -4717,6 +4736,7 @@ default_make_symbol_completion_list_break_on (const char *text, macro_for_each (macro_user_macros, add_macro_name, &datum); } + do_cleanups (limit_chain); discard_cleanups (back_to); return (return_val); } diff --git a/gdb/testsuite/gdb.base/completion.exp b/gdb/testsuite/gdb.base/completion.exp index c633a51..9b92a05 100644 --- a/gdb/testsuite/gdb.base/completion.exp +++ b/gdb/testsuite/gdb.base/completion.exp @@ -67,6 +67,7 @@ if ![runto_main] then { } set timeout 30 +gdb_test_no_output "set max-completions unlimited" gdb_test_no_output "complete print values\[0\].x." \ "field completion with invalid field" @@ -747,4 +748,36 @@ gdb_test_multiple "" "$test" { } } +# +# Completion limiting. +# + +gdb_test_no_output "set max-completions 5" + +set test "completion limiting using tab character" +send_gdb "p\t" +gdb_test_multiple "" "$test" { + -re "^p\\\x07$" { + send_gdb "\t" + gdb_test_multiple "" "$test" { + -re "Too many possibilities.\r\n\\\x07$gdb_prompt p$" { + send_gdb "\n" + gdb_test_multiple "" "$test" { + -re "$gdb_prompt $" { + pass "$test" + } + } + } + } + } +} + +set test "completion limiting using complete command" +send_gdb "complete p\n" +gdb_test_multiple "" "$test" { + -re "Too many possibilities.\r\n$gdb_prompt $" { + pass "$test" + } +} + return 0 diff --git a/gdb/testsuite/gdb.linespec/ls-errs.exp b/gdb/testsuite/gdb.linespec/ls-errs.exp index 86056c5..30b4716 100644 --- a/gdb/testsuite/gdb.linespec/ls-errs.exp +++ b/gdb/testsuite/gdb.linespec/ls-errs.exp @@ -26,6 +26,9 @@ if {[prepare_for_testing $testfile $exefile $srcfile \ # Turn off the pending breakpoint queries. gdb_test_no_output "set breakpoint pending off" +# Turn off completion limiting +gdb_test_no_output "set max-completions unlimited" + # We intentionally do not use gdb_breakpoint for these tests. # Break at 'linespec' and expect the message in ::error_messages indexed by -- 1.7.1 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3 v2] Implement completion limiting 2014-11-06 10:51 ` [PATCH 3/3 v2] Implement completion limiting Gary Benson @ 2014-11-06 16:28 ` Eli Zaretskii 2014-11-21 10:46 ` Gary Benson 2014-11-24 6:09 ` Doug Evans 1 sibling, 1 reply; 29+ messages in thread From: Eli Zaretskii @ 2014-11-06 16:28 UTC (permalink / raw) To: Gary Benson; +Cc: gdb-patches > From: Gary Benson <gbenson@redhat.com> > Date: Thu, 6 Nov 2014 10:50:46 +0000 > > --- a/gdb/NEWS > +++ b/gdb/NEWS OK for this part. > +void > +_initialize_completer (void) > +{ > + add_setshow_zuinteger_unlimited_cmd ("max-completions", no_class, > + &max_completions, _("\ > +Set maximum number of line completion possibilities."), _("\ > +Show maximum number of line completion possibilities."), _("\ I suggest to use "completion candidates" instead of "line completion possibilities". > +Use this to limit the number of possibilities considered\n\ > +during line completion. Specifying \"unlimited\" or -1\n\ ^^^^^^^^^^^^^^^ And here use just "completion". "Line completion" is IMO confusing, since we don't complete "lines". > +@table @code > +@kindex set max-completions > +@item set max-completions @var{limit} > +@itemx set max-completions unlimited > +Set the maximum number of possibilities to be considered during ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ "Candidates to be shown" is better. > +completion. The default value is 200. Note that setting either > +no limit or a very large limit can cause pauses during completion. Not "cause pauses", but rather "make completion slow", I think. > +@kindex show max-completions > +@item show max-completions > +Show the maximum number of possibilities to be considered during > +completion. Suggest the same change here as above. The documentation part of the patch is OK with those changes. Thanks. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3 v2] Implement completion limiting 2014-11-06 16:28 ` Eli Zaretskii @ 2014-11-21 10:46 ` Gary Benson 0 siblings, 0 replies; 29+ messages in thread From: Gary Benson @ 2014-11-21 10:46 UTC (permalink / raw) To: Eli Zaretskii; +Cc: gdb-patches Eli Zaretskii wrote: > > From: Gary Benson <gbenson@redhat.com> > > Date: Thu, 6 Nov 2014 10:50:46 +0000 > > --- a/gdb/NEWS > > +++ b/gdb/NEWS > > OK for this part. > > > +void > > +_initialize_completer (void) > > +{ > > + add_setshow_zuinteger_unlimited_cmd ("max-completions", no_class, > > + &max_completions, _("\ > > +Set maximum number of line completion possibilities."), _("\ > > +Show maximum number of line completion possibilities."), _("\ > > I suggest to use "completion candidates" instead of "line completion > possibilities". > > > +Use this to limit the number of possibilities considered\n\ > > +during line completion. Specifying \"unlimited\" or -1\n\ > ^^^^^^^^^^^^^^^ > And here use just "completion". "Line completion" is IMO confusing, > since we don't complete "lines". > > > +@table @code > > +@kindex set max-completions > > +@item set max-completions @var{limit} > > +@itemx set max-completions unlimited > > +Set the maximum number of possibilities to be considered during > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > "Candidates to be shown" is better. > > > +completion. The default value is 200. Note that setting either > > +no limit or a very large limit can cause pauses during completion. > > Not "cause pauses", but rather "make completion slow", I think. > > > +@kindex show max-completions > > +@item show max-completions > > +Show the maximum number of possibilities to be considered during > > +completion. > > Suggest the same change here as above. > > The documentation part of the patch is OK with those changes. > > Thanks. Thanks Eli. Cheers, Gary -- http://gbenson.net/ ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3 v2] Implement completion limiting 2014-11-06 10:51 ` [PATCH 3/3 v2] Implement completion limiting Gary Benson 2014-11-06 16:28 ` Eli Zaretskii @ 2014-11-24 6:09 ` Doug Evans 1 sibling, 0 replies; 29+ messages in thread From: Doug Evans @ 2014-11-24 6:09 UTC (permalink / raw) To: Gary Benson; +Cc: gdb-patches Gary Benson <gbenson@redhat.com> writes: > This commit adds a new exception, TOO_MANY_COMPLETIONS_ERROR, to be > thrown whenever the completer has generated too many possibilities to > be useful. A new user-settable variable, "max_completions", is added > to control this behaviour. A top-level completion limit is added to > complete_line_internal, as the final check to ensure the user never > sees too many completions. An additional limit is added to > default_make_symbol_completion_list_break_on, to halt time-consuming > symbol table expansions. > > gdb/ChangeLog: > > PR cli/9007 > PR cli/11920 > PR cli/15548 > * common/common-exceptions.h (enum errors) > <TOO_MANY_COMPLETIONS_ERROR>: New value. > * completer.h (completion_tracker_t): New typedef. > (new_completion_tracker): New declaration. > (make_cleanup_free_completion_tracker): Likewise. > (maybe_limit_completions): Likewise. > * completer.c [TUI]: Include tui/tui.h and tui/tui-io.h. > (max_completions): New static variable. > (new_completion_tracker): New function. > (make_cleanup_free_completion_tracker): Likewise. > (maybe_limit_completions): Likewise. > (complete_line_internal): Do not generate any completions if > max_completions = 0. Limit the number of completions if > max_completions >= 0. > (line_completion_function): Handle TOO_MANY_COMPLETIONS_ERROR. > (_initialize_completer): New declaration and function. > * symtab.c: Include completer.h. > (completion_tracker): New static variable. > (completion_list_add_name): Call maybe_limit_completions. > (default_make_symbol_completion_list_break_on): Maintain > completion_tracker across calls to completion_list_add_name. > * NEWS (New Options): Mention set/show max-completions. Hi. I only skimmed this patch set. I want to give it a more thorough look once the patch has been updated to apply to current HEAD. Thanks very much for working on this! > +void > +maybe_limit_completions (completion_tracker_t tracker, char *name) > +{ > + if (max_completions < 0) > + return; > + > + if (tracker != NULL) > + { > + void **slot = htab_find_slot (tracker, name, INSERT); > + > + if (*slot != HTAB_EMPTY_ENTRY) > + return; > + > + if (htab_elements (tracker) <= max_completions) > + { > + *slot = name; > + return; > + } > + } > + > + throw_error (TOO_MANY_COMPLETIONS_ERROR, > + _("Too many possibilities.")); > +} This will throw an error if tracker == NULL. Is that intended? ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 2/3 v2] Interleave completion list building with symbol table expansion 2014-11-06 10:50 [PATCH 0/3 v2] Limit tab-completion result when list is large Gary Benson 2014-11-06 10:51 ` [PATCH 1/3 v2] Add expansion_notify callback to expand_symtabs_matching Gary Benson 2014-11-06 10:51 ` [PATCH 3/3 v2] Implement completion limiting Gary Benson @ 2014-11-06 10:51 ` Gary Benson 2 siblings, 0 replies; 29+ messages in thread From: Gary Benson @ 2014-11-06 10:51 UTC (permalink / raw) To: gdb-patches This commit makes default_make_symbol_completion_list_break_on build the list of completions as it expands the necessary symbol tables, rather than expanding all necessary symbol tables first and then building the completion lists second. This allows for the early termination of symbol table expansion if required. gdb/ChangeLog: * symtab.c (struct add_name_data) <code>: New field. Updated comments. (add_symtab_completions): New function. (symtab_expansion_callback): Likewise. (default_make_symbol_completion_list_break_on): Set datum.code. Move minimal symbol scan before calling expand_symtabs_matching. Scan known primary symtabs for externs and statics before calling expand_symtabs_matching. Pass symtab_expansion_callback as expansion_notify argument to expand_symtabs_matching. Do not scan primary symtabs for externs and statics after calling expand_symtabs_matching. --- gdb/ChangeLog | 14 +++++++ gdb/symtab.c | 106 ++++++++++++++++++++++++++++++++++++--------------------- 2 files changed, 81 insertions(+), 39 deletions(-) diff --git a/gdb/symtab.c b/gdb/symtab.c index 836f3bc..ebb7b8f 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -4424,15 +4424,19 @@ completion_list_add_fields (struct symbol *sym, const char *sym_text, } } -/* Type of the user_data argument passed to add_macro_name or - symbol_completion_matcher. The contents are simply whatever is - needed by completion_list_add_name. */ +/* Type of the user_data argument passed to add_macro_name, + symbol_completion_matcher and symtab_expansion_callback. */ + struct add_name_data { + /* Arguments required by completion_list_add_name. */ const char *sym_text; int sym_text_len; const char *text; const char *word; + + /* Extra argument required for add_symtab_completions. */ + enum type_code code; }; /* A callback used with macro_for_each and macro_for_each_in_scope. @@ -4460,6 +4464,52 @@ symbol_completion_matcher (const char *name, void *user_data) return compare_symbol_name (name, datum->sym_text, datum->sym_text_len); } +/* Add matching symbols from SYMTAB to the current completion list. */ + +static void +add_symtab_completions (struct symtab *symtab, + const char *sym_text, int sym_text_len, + const char *text, const char *word, + enum type_code code) +{ + struct symbol *sym; + const struct block *b; + struct block_iterator iter; + int i; + + if (!symtab->primary) + return; + + for (i = GLOBAL_BLOCK; i <= STATIC_BLOCK; i++) + { + QUIT; + b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (symtab), i); + ALL_BLOCK_SYMBOLS (b, iter, sym) + { + if (code == TYPE_CODE_UNDEF + || (SYMBOL_DOMAIN (sym) == STRUCT_DOMAIN + && TYPE_CODE (SYMBOL_TYPE (sym)) == code)) + COMPLETION_LIST_ADD_SYMBOL (sym, + sym_text, sym_text_len, + text, word); + } + } +} + +/* Callback to add completions to the current list when symbol tables + are expanded during completion list generation. */ + +static void +symtab_expansion_callback (struct symtab *symtab, void *user_data) +{ + struct add_name_data *datum = (struct add_name_data *) user_data; + + add_symtab_completions (symtab, + datum->sym_text, datum->sym_text_len, + datum->text, datum->word, + datum->code); +} + VEC (char_ptr) * default_make_symbol_completion_list_break_on (const char *text, const char *word, @@ -4560,17 +4610,12 @@ default_make_symbol_completion_list_break_on (const char *text, datum.sym_text_len = sym_text_len; datum.text = text; datum.word = word; - - /* Look through the partial symtabs for all symbols which begin - by matching SYM_TEXT. Expand all CUs that you find to the list. - The real names will get added by COMPLETION_LIST_ADD_SYMBOL below. */ - expand_symtabs_matching (NULL, symbol_completion_matcher, NULL, - ALL_DOMAIN, &datum); + datum.code = code; /* At this point scan through the misc symbol vectors and add each symbol you find to the list. Eventually we want to ignore anything that isn't a text symbol (everything else will be - handled by the psymtab code above). */ + handled by the psymtab code below). */ if (code == TYPE_CODE_UNDEF) { @@ -4585,6 +4630,18 @@ default_make_symbol_completion_list_break_on (const char *text, } } + /* Add completions for all currently loaded symbol tables. */ + ALL_PRIMARY_SYMTABS (objfile, s) + add_symtab_completions (s, sym_text, sym_text_len, text, word, code); + + /* Look through the partial symtabs for all symbols which begin + by matching SYM_TEXT. Expand all CUs that you find to the list. + symtab_expansion_callback is called for each expanded symtab, + causing those symtab's completions to be added to the list too. */ + expand_symtabs_matching (NULL, symbol_completion_matcher, + symtab_expansion_callback, ALL_DOMAIN, + &datum); + /* Search upwards from currently selected frame (so that we can complete on local vars). Also catch fields of types defined in this places which match our text string. Only complete on types @@ -4634,35 +4691,6 @@ default_make_symbol_completion_list_break_on (const char *text, completion_list_add_fields (sym, sym_text, sym_text_len, text, word); } - /* Go through the symtabs and check the externs and statics for - symbols which match. */ - - ALL_PRIMARY_SYMTABS (objfile, s) - { - QUIT; - b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), GLOBAL_BLOCK); - ALL_BLOCK_SYMBOLS (b, iter, sym) - { - if (code == TYPE_CODE_UNDEF - || (SYMBOL_DOMAIN (sym) == STRUCT_DOMAIN - && TYPE_CODE (SYMBOL_TYPE (sym)) == code)) - COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word); - } - } - - ALL_PRIMARY_SYMTABS (objfile, s) - { - QUIT; - b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), STATIC_BLOCK); - ALL_BLOCK_SYMBOLS (b, iter, sym) - { - if (code == TYPE_CODE_UNDEF - || (SYMBOL_DOMAIN (sym) == STRUCT_DOMAIN - && TYPE_CODE (SYMBOL_TYPE (sym)) == code)) - COMPLETION_LIST_ADD_SYMBOL (sym, sym_text, sym_text_len, text, word); - } - } - /* Skip macros if we are completing a struct tag -- arguable but usually what is expected. */ if (current_language->la_macro_expansion == macro_expansion_c -- 1.7.1 ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 0/3 v2] Abort completion when list becomes large @ 2014-11-27 13:16 Gary Benson 2014-11-27 13:16 ` [PATCH 3/3 v2] Implement completion limiting Gary Benson 0 siblings, 1 reply; 29+ messages in thread From: Gary Benson @ 2014-11-27 13:16 UTC (permalink / raw) To: gdb-patches; +Cc: Doug Evans, Eli Zaretskii Hi all, When debugging large programs, pressing the Tab key at the wrong time causes GDB to lock up while it builds a completion list. As an example, with LibreOffice: $ gdb /usr/lib64/libreoffice/program/soffice.bin (gdb) start (gdb) b <Tab> On my (pretty fast!) machine the user is left hanging for nearly a minute while GDB builds a list of the 212405 places the user could break on. This is PR cli/11920. PR cli/15548 proposes to abort building the completion list if it grows too large, the theory being that nobody is going to scroll through thousands of screens of completions to find the one they want. These three patches implement this. The default limit is 200 completions, which the user can override with a new set/show option. This version 2 series has been updated for the symtab rework Doug recently pushed, and with Eli's suggested documentation changes. Tested on RHEL6.5 x86_64, no regressions. Is this ok to commit? Thanks, Gary ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 3/3 v2] Implement completion limiting 2014-11-27 13:16 [PATCH 0/3 v2] Abort completion when list becomes large Gary Benson @ 2014-11-27 13:16 ` Gary Benson 2014-11-27 16:25 ` Eli Zaretskii 2014-12-05 23:54 ` Doug Evans 0 siblings, 2 replies; 29+ messages in thread From: Gary Benson @ 2014-11-27 13:16 UTC (permalink / raw) To: gdb-patches; +Cc: Doug Evans, Eli Zaretskii This commit adds a new exception, TOO_MANY_COMPLETIONS_ERROR, to be thrown whenever the completer has generated too many candidates to be useful. A new user-settable variable, "max_completions", is added to control this behaviour. A top-level completion limit is added to complete_line_internal, as the final check to ensure the user never sees too many completions. An additional limit is added to default_make_symbol_completion_list_break_on, to halt time-consuming symbol table expansions. gdb/ChangeLog: PR cli/9007 PR cli/11920 PR cli/15548 * common/common-exceptions.h (enum errors) <TOO_MANY_COMPLETIONS_ERROR>: New value. * completer.h (completion_tracker_t): New typedef. (new_completion_tracker): New declaration. (make_cleanup_free_completion_tracker): Likewise. (maybe_limit_completions): Likewise. * completer.c [TUI]: Include tui/tui.h and tui/tui-io.h. (max_completions): New static variable. (new_completion_tracker): New function. (make_cleanup_free_completion_tracker): Likewise. (maybe_limit_completions): Likewise. (complete_line_internal): Do not generate any completions if max_completions = 0. Limit the number of completions if max_completions >= 0. (line_completion_function): Handle TOO_MANY_COMPLETIONS_ERROR. (_initialize_completer): New declaration and function. * symtab.c: Include completer.h. (completion_tracker): New static variable. (completion_list_add_name): Call maybe_limit_completions. (default_make_symbol_completion_list_break_on): Maintain completion_tracker across calls to completion_list_add_name. * NEWS (New Options): Mention set/show max-completions. gdb/doc/ChangeLog: * gdb.texinfo (Command Completion): Document new "set/show max-completions" option. gdb/testsuite/ChangeLog: * gdb.base/completion.exp: Disable completion limiting for existing tests. Add new tests to check completion limiting. * gdb.linespec/ls-errs.exp: Disable completion limiting. --- gdb/ChangeLog | 28 ++++++ gdb/NEWS | 11 ++- gdb/common/common-exceptions.h | 4 + gdb/completer.c | 146 ++++++++++++++++++++++++++++++-- gdb/completer.h | 23 +++++ gdb/doc/ChangeLog | 5 + gdb/doc/gdb.texinfo | 24 +++++ gdb/symtab.c | 22 +++++- gdb/testsuite/ChangeLog | 6 ++ gdb/testsuite/gdb.base/completion.exp | 33 +++++++ gdb/testsuite/gdb.linespec/ls-errs.exp | 3 + 11 files changed, 297 insertions(+), 8 deletions(-) diff --git a/gdb/NEWS b/gdb/NEWS index d6a8b61..af743d9 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -22,11 +22,20 @@ ** $_any_caller_is(name [, number_of_frames]) ** $_any_caller_matches(regexp [, number_of_frames]) -* New commands +* New commands (for set/show, see "New options" below) queue-signal signal-name-or-number Queue a signal to be delivered to the thread when it is resumed. +* New options + +set max-completions +show max-completions + Set the maximum number of candidates to be considered during + completion. The default value is 200. This limit allows GDB + to avoid generating large completion lists, the computation of + which can cause the debugger to become temporarily unresponsive. + * On resume, GDB now always passes the signal the program had stopped for to the thread the signal was sent to, even if the user changed threads before resuming. Previously GDB would often (but not diff --git a/gdb/common/common-exceptions.h b/gdb/common/common-exceptions.h index 5f750c3..73fd255 100644 --- a/gdb/common/common-exceptions.h +++ b/gdb/common/common-exceptions.h @@ -99,6 +99,10 @@ enum errors { /* Requested feature, method, mechanism, etc. is not supported. */ NOT_SUPPORTED_ERROR, + /* The number of candidates generated during line completion has + exceeded the user's specified limit. */ + TOO_MANY_COMPLETIONS_ERROR, + /* Add more errors here. */ NR_ERRORS }; diff --git a/gdb/completer.c b/gdb/completer.c index a0f3fa3..4a2302c 100644 --- a/gdb/completer.c +++ b/gdb/completer.c @@ -39,6 +39,11 @@ #include "completer.h" +#ifdef TUI +#include "tui/tui.h" +#include "tui/tui-io.h" +#endif + /* Prototypes for local functions. */ static char *line_completion_function (const char *text, int matches, @@ -778,9 +783,67 @@ complete_line_internal (const char *text, return list; } -/* Generate completions all at once. Returns a vector of strings. - Each element is allocated with xmalloc. It can also return NULL if - there are no completions. + +/* Maximum number of candidates to consider before the completer + bails by throwing TOO_MANY_COMPLETIONS_ERROR. Negative values + disable limiting. */ +static int max_completions = 200; + +/* See completer.h. */ + +completion_tracker_t +new_completion_tracker (void) +{ + if (max_completions < 1) + return NULL; + + return htab_create_alloc (max_completions, + htab_hash_string, (htab_eq) streq, + NULL, xcalloc, xfree); +} + +/* See completer.h. */ + +struct cleanup * +make_cleanup_free_completion_tracker (completion_tracker_t tracker) +{ + if (tracker == NULL) + return make_cleanup (null_cleanup, NULL); + + return make_cleanup_htab_delete (tracker); +} + +/* See completer.h. */ + +void +maybe_limit_completions (completion_tracker_t tracker, char *name) +{ + if (max_completions < 0) + return; + + if (tracker != NULL) + { + void **slot = htab_find_slot (tracker, name, INSERT); + + if (*slot != HTAB_EMPTY_ENTRY) + return; + + if (htab_elements (tracker) <= max_completions) + { + *slot = name; + return; + } + } + + throw_error (TOO_MANY_COMPLETIONS_ERROR, + _("Too many possibilities.")); +} + +/* Generate completions all at once. Returns a vector of strings + allocated with xmalloc. Returns NULL if there are no completions + or if max_completions is 0. Throws TOO_MANY_COMPLETIONS_ERROR if + max_completions is greater than zero and the number of completions + is greater than max_completions. TEXT is the caller's idea of the "word" we are looking at. @@ -793,8 +856,33 @@ complete_line_internal (const char *text, VEC (char_ptr) * complete_line (const char *text, const char *line_buffer, int point) { - return complete_line_internal (text, line_buffer, - point, handle_completions); + VEC (char_ptr) *list = NULL; + struct cleanup *old_chain; + + list = complete_line_internal (text, line_buffer, point, + handle_completions); + old_chain = make_cleanup_free_char_ptr_vec (list); + + /* Possibly throw TOO_MANY_COMPLETIONS_ERROR. Individual + completers may do this too, to avoid unnecessary work, + but this is the ultimate check that stops users seeing + more completions than they wanted. */ + if (max_completions >= 0) + { + completion_tracker_t tracker = new_completion_tracker (); + struct cleanup *limit_chain = + make_cleanup_free_completion_tracker (tracker); + char *candidate; + int ix; + + for (ix = 0; VEC_iterate (char_ptr, list, ix, candidate); ++ix) + maybe_limit_completions (tracker, candidate); + + do_cleanups (limit_chain); + } + + discard_cleanups (old_chain); + return list; } /* Complete on command names. Used by "help". */ @@ -881,6 +969,8 @@ line_completion_function (const char *text, int matches, if (matches == 0) { + volatile struct gdb_exception ex; + /* The caller is beginning to accumulate a new set of completions, so we need to find all of them now, and cache them for returning one at a time on future calls. */ @@ -894,7 +984,35 @@ line_completion_function (const char *text, int matches, VEC_free (char_ptr, list); } index = 0; - list = complete_line (text, line_buffer, point); + + TRY_CATCH (ex, RETURN_MASK_ALL) + list = complete_line (text, line_buffer, point); + + if (ex.reason < 0) + { + if (ex.error != TOO_MANY_COMPLETIONS_ERROR) + throw_exception (ex); + + if (rl_completion_type != TAB) + { +#if defined(TUI) + if (tui_active) + { + tui_puts ("\n"); + tui_puts (ex.message); + tui_puts ("\n"); + } + else +#endif + { + rl_crlf (); + fputs (ex.message, rl_outstream); + rl_crlf (); + } + + rl_on_new_line (); + } + } } /* If we found a list of potential completions during initialization @@ -978,3 +1096,19 @@ skip_quoted (const char *str) { return skip_quoted_chars (str, NULL, NULL); } + +extern initialize_file_ftype _initialize_completer; /* -Wmissing-prototypes */ + +void +_initialize_completer (void) +{ + add_setshow_zuinteger_unlimited_cmd ("max-completions", no_class, + &max_completions, _("\ +Set maximum number of completion candidates."), _("\ +Show maximum number of completion candidates."), _("\ +Use this to limit the number of candidates considered\n\ +during completion. Specifying \"unlimited\" or -1\n\ +disables limiting. Note that setting either no limit or\n\ +a very large limit can make completion slow."), + NULL, NULL, &setlist, &showlist); +} diff --git a/gdb/completer.h b/gdb/completer.h index bc7ed96..5079343 100644 --- a/gdb/completer.h +++ b/gdb/completer.h @@ -63,4 +63,27 @@ extern const char *skip_quoted_chars (const char *, const char *, extern const char *skip_quoted (const char *); +/* Object to track how many unique completions have been generated. + Used to limit the size of generated completion lists. */ + +typedef htab_t completion_tracker_t; + +/* Create a new completion tracker. */ + +extern completion_tracker_t new_completion_tracker (void); + +/* Make a cleanup to free a completion tracker. */ + +extern struct cleanup *make_cleanup_free_completion_tracker + (completion_tracker_t tracker); + +/* Add the completion NAME to the list of generated completions if + it is not there already. Throw TOO_MANY_COMPLETIONS_ERROR if + max_completions >= 0 and the number of generated completions is + greater than max_completions. Do nothing if max_completions is + negative. */ + +extern void maybe_limit_completions (completion_tracker_t tracker, + char *name); + #endif /* defined (COMPLETER_H) */ diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 15c2908..2cac470 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -1600,6 +1600,30 @@ means @kbd{@key{META} ?}. You can type this either by holding down a key designated as the @key{META} shift on your keyboard (if there is one) while typing @kbd{?}, or as @key{ESC} followed by @kbd{?}. +If the number of possible completions is large, @value{GDBN} will +print a message rather than displaying the list: + +@smallexample +(@value{GDBP}) b @key{TAB}@key{TAB} +Too many possibilities. +(@value{GDBP}) b +@end smallexample + +@noindent +This behavior can be controlled with the following commands: + +@table @code +@kindex set max-completions +@item set max-completions @var{limit} +@itemx set max-completions unlimited +Set the maximum number of candidates to be shown during completion. +The default value is 200. Note that setting either no limit or a +very large limit can make completion slow. +@kindex show max-completions +@item show max-completions +Show the maximum number of candidates to be shown during completion. +@end table + @cindex quotes in commands @cindex completion of quoted strings Sometimes the string you need, while logically a ``word'', may contain diff --git a/gdb/symtab.c b/gdb/symtab.c index a419e62..9bd4bd8 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -60,6 +60,7 @@ #include "macroscope.h" #include "parser-defs.h" +#include "completer.h" /* Forward declarations for local functions. */ @@ -4155,6 +4156,15 @@ static VEC (char_ptr) *return_val; completion_list_add_name \ (MSYMBOL_NATURAL_NAME (symbol), (sym_text), (len), (text), (word)) +/* Tracker for how many unique completions have been generated. Used + to terminate completion list generation early if the list has grown + to a size so large as to be useless. This helps avoid GDB seeming + to lock up in the event the user requests to complete on something + vague that necessitates the time consuming expansion of many symbol + tables. */ + +completion_tracker_t completion_tracker; + /* Test to see if the symbol specified by SYMNAME (which is already demangled for C++ symbols) matches SYM_TEXT in the first SYM_TEXT_LEN characters. If so, add it to the current completion list. */ @@ -4195,6 +4205,12 @@ completion_list_add_name (const char *symname, } VEC_safe_push (char_ptr, return_val, new); + + /* Throw TOO_MANY_COMPLETIONS_ERROR if we've generated too many + completions. We check this after pushing NEW to RETURN_VAL so + it's freed by default_make_symbol_completion_list_break_on's + cleanup if the exception is thrown. */ + maybe_limit_completions (completion_tracker, new); } } @@ -4429,7 +4445,7 @@ default_make_symbol_completion_list_break_on (const char *text, /* Length of sym_text. */ int sym_text_len; struct add_name_data datum; - struct cleanup *back_to; + struct cleanup *back_to, *limit_chain; /* Now look for the symbol we are supposed to complete on. */ { @@ -4503,6 +4519,9 @@ default_make_symbol_completion_list_break_on (const char *text, return_val = NULL; back_to = make_cleanup (do_free_completion_list, &return_val); + completion_tracker = new_completion_tracker (); + limit_chain = make_cleanup_free_completion_tracker (completion_tracker); + datum.sym_text = sym_text; datum.sym_text_len = sym_text_len; datum.text = text; @@ -4615,6 +4634,7 @@ default_make_symbol_completion_list_break_on (const char *text, macro_for_each (macro_user_macros, add_macro_name, &datum); } + do_cleanups (limit_chain); discard_cleanups (back_to); return (return_val); } diff --git a/gdb/testsuite/gdb.base/completion.exp b/gdb/testsuite/gdb.base/completion.exp index c633a51..9b92a05 100644 --- a/gdb/testsuite/gdb.base/completion.exp +++ b/gdb/testsuite/gdb.base/completion.exp @@ -67,6 +67,7 @@ if ![runto_main] then { } set timeout 30 +gdb_test_no_output "set max-completions unlimited" gdb_test_no_output "complete print values\[0\].x." \ "field completion with invalid field" @@ -747,4 +748,36 @@ gdb_test_multiple "" "$test" { } } +# +# Completion limiting. +# + +gdb_test_no_output "set max-completions 5" + +set test "completion limiting using tab character" +send_gdb "p\t" +gdb_test_multiple "" "$test" { + -re "^p\\\x07$" { + send_gdb "\t" + gdb_test_multiple "" "$test" { + -re "Too many possibilities.\r\n\\\x07$gdb_prompt p$" { + send_gdb "\n" + gdb_test_multiple "" "$test" { + -re "$gdb_prompt $" { + pass "$test" + } + } + } + } + } +} + +set test "completion limiting using complete command" +send_gdb "complete p\n" +gdb_test_multiple "" "$test" { + -re "Too many possibilities.\r\n$gdb_prompt $" { + pass "$test" + } +} + return 0 diff --git a/gdb/testsuite/gdb.linespec/ls-errs.exp b/gdb/testsuite/gdb.linespec/ls-errs.exp index 86056c5..30b4716 100644 --- a/gdb/testsuite/gdb.linespec/ls-errs.exp +++ b/gdb/testsuite/gdb.linespec/ls-errs.exp @@ -26,6 +26,9 @@ if {[prepare_for_testing $testfile $exefile $srcfile \ # Turn off the pending breakpoint queries. gdb_test_no_output "set breakpoint pending off" +# Turn off completion limiting +gdb_test_no_output "set max-completions unlimited" + # We intentionally do not use gdb_breakpoint for these tests. # Break at 'linespec' and expect the message in ::error_messages indexed by -- 1.7.1 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3 v2] Implement completion limiting 2014-11-27 13:16 ` [PATCH 3/3 v2] Implement completion limiting Gary Benson @ 2014-11-27 16:25 ` Eli Zaretskii 2014-12-05 23:54 ` Doug Evans 1 sibling, 0 replies; 29+ messages in thread From: Eli Zaretskii @ 2014-11-27 16:25 UTC (permalink / raw) To: Gary Benson; +Cc: gdb-patches, xdje42 > From: Gary Benson <gbenson@redhat.com> > Cc: Doug Evans <xdje42@gmail.com>, Eli Zaretskii <eliz@gnu.org> > Date: Thu, 27 Nov 2014 13:16:08 +0000 > > This commit adds a new exception, TOO_MANY_COMPLETIONS_ERROR, to be > thrown whenever the completer has generated too many candidates to > be useful. A new user-settable variable, "max_completions", is added > to control this behaviour. A top-level completion limit is added to > complete_line_internal, as the final check to ensure the user never > sees too many completions. An additional limit is added to > default_make_symbol_completion_list_break_on, to halt time-consuming > symbol table expansions. > > gdb/ChangeLog: > > PR cli/9007 > PR cli/11920 > PR cli/15548 > * common/common-exceptions.h (enum errors) > <TOO_MANY_COMPLETIONS_ERROR>: New value. > * completer.h (completion_tracker_t): New typedef. > (new_completion_tracker): New declaration. > (make_cleanup_free_completion_tracker): Likewise. > (maybe_limit_completions): Likewise. > * completer.c [TUI]: Include tui/tui.h and tui/tui-io.h. > (max_completions): New static variable. > (new_completion_tracker): New function. > (make_cleanup_free_completion_tracker): Likewise. > (maybe_limit_completions): Likewise. > (complete_line_internal): Do not generate any completions if > max_completions = 0. Limit the number of completions if > max_completions >= 0. > (line_completion_function): Handle TOO_MANY_COMPLETIONS_ERROR. > (_initialize_completer): New declaration and function. > * symtab.c: Include completer.h. > (completion_tracker): New static variable. > (completion_list_add_name): Call maybe_limit_completions. > (default_make_symbol_completion_list_break_on): Maintain > completion_tracker across calls to completion_list_add_name. > * NEWS (New Options): Mention set/show max-completions. > > gdb/doc/ChangeLog: > > * gdb.texinfo (Command Completion): Document new > "set/show max-completions" option. OK for the documentation parts. Thanks. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3 v2] Implement completion limiting 2014-11-27 13:16 ` [PATCH 3/3 v2] Implement completion limiting Gary Benson 2014-11-27 16:25 ` Eli Zaretskii @ 2014-12-05 23:54 ` Doug Evans 2014-12-10 12:22 ` Gary Benson 1 sibling, 1 reply; 29+ messages in thread From: Doug Evans @ 2014-12-05 23:54 UTC (permalink / raw) To: Gary Benson; +Cc: gdb-patches, Eli Zaretskii Gary Benson <gbenson@redhat.com> writes: > This commit adds a new exception, TOO_MANY_COMPLETIONS_ERROR, to be > thrown whenever the completer has generated too many candidates to > be useful. A new user-settable variable, "max_completions", is added > to control this behaviour. A top-level completion limit is added to > complete_line_internal, as the final check to ensure the user never > sees too many completions. An additional limit is added to > default_make_symbol_completion_list_break_on, to halt time-consuming > symbol table expansions. > > gdb/ChangeLog: > > PR cli/9007 > PR cli/11920 > PR cli/15548 > * common/common-exceptions.h (enum errors) > <TOO_MANY_COMPLETIONS_ERROR>: New value. > * completer.h (completion_tracker_t): New typedef. > (new_completion_tracker): New declaration. > (make_cleanup_free_completion_tracker): Likewise. > (maybe_limit_completions): Likewise. > * completer.c [TUI]: Include tui/tui.h and tui/tui-io.h. > (max_completions): New static variable. > (new_completion_tracker): New function. > (make_cleanup_free_completion_tracker): Likewise. > (maybe_limit_completions): Likewise. > (complete_line_internal): Do not generate any completions if > max_completions = 0. Limit the number of completions if > max_completions >= 0. > (line_completion_function): Handle TOO_MANY_COMPLETIONS_ERROR. > (_initialize_completer): New declaration and function. > * symtab.c: Include completer.h. > (completion_tracker): New static variable. > (completion_list_add_name): Call maybe_limit_completions. > (default_make_symbol_completion_list_break_on): Maintain > completion_tracker across calls to completion_list_add_name. > * NEWS (New Options): Mention set/show max-completions. > > gdb/doc/ChangeLog: > > * gdb.texinfo (Command Completion): Document new > "set/show max-completions" option. > > gdb/testsuite/ChangeLog: > > * gdb.base/completion.exp: Disable completion limiting for > existing tests. Add new tests to check completion limiting. > * gdb.linespec/ls-errs.exp: Disable completion limiting. Hi. I played with the patch a bit. I have a few questions on the u/i. 1) IWBN if, when "Too many possibilities" is hit, the user was still shown the completions thus far. I'd rather not have to abort the command I'm trying to do, increase max-completions, and then try again (or anything else to try to find what I'm looking for in order to complete the command). At least not if I don't have to: the completions thus far may provide a hint at what I'm looking for. Plus GDB has already computed them, might as well print them. Imagine if the total count is MAX+1, the user might find it annoying to not be shown anything just because the count is one beyond the max. So instead of "Too many possibilities", how about printing the completions thus far and then include a message saying the list is clipped due to max-completions being reached? [Maybe readline makes this difficult, but I think it'd be really nice have. Thoughts?] 2) Readline has a limiting facility already: rl_completion_query_items. But it's only applied after all completions have been computed so it doesn't help us. It would be good for the docs to explain the difference. E.g. with the default of 200 for max-completions, if I do (top-gdb) b dwarf2<tab><tab> I get Display all 159 possibilities? (y or n) As a user, I'm kinda wondering why I'm being asked this if, for example, I've explicitly set max-completions to some value. [I know normally users might not even be aware of max-completions, but suppose for the sake of discussion that they've set it to some value larger than rl_completion_query_items.] Note: rl_completion_query_items can be set in ~/.inputrc with the completion-query-items parameter. 3) rl_completion_query_items uses a value of zero to mean unlimited, whereas max_completions uses -1 (or "unlimited"). While it might be nice to provide a way to disable completions completely (by setting max-completions to zero), I'm trying to decide whether that benefit is sufficient to justify the inconsistency with rl_completion_query_items. Thoughts? 4) Is there a use-case that involves wanting both rl_completion_query_items and max_completions parameters? Certainly we need to limit the number while we're building them, but do we need both parameters? [IOW, could we fold them into one?] I can imagine wanting to set max-completions to some large value but still be given a prompt if the completion-query-items threshold is reached, so I think we want both. I'm just raising the possibility that maybe we don't want both in case someone wants to comment. At the least, it'd probably be good to mention how both interact in the docs. > diff --git a/gdb/completer.c b/gdb/completer.c > index a0f3fa3..4a2302c 100644 > --- a/gdb/completer.c > +++ b/gdb/completer.c > [...] > @@ -894,7 +984,35 @@ line_completion_function (const char *text, int matches, > VEC_free (char_ptr, list); > } > index = 0; > - list = complete_line (text, line_buffer, point); > + > + TRY_CATCH (ex, RETURN_MASK_ALL) > + list = complete_line (text, line_buffer, point); > + > + if (ex.reason < 0) > + { > + if (ex.error != TOO_MANY_COMPLETIONS_ERROR) > + throw_exception (ex); > + > + if (rl_completion_type != TAB) > + { > +#if defined(TUI) > + if (tui_active) > + { > + tui_puts ("\n"); > + tui_puts (ex.message); > + tui_puts ("\n"); > + } > + else > +#endif > + { > + rl_crlf (); > + fputs (ex.message, rl_outstream); > + rl_crlf (); > + } > + > + rl_on_new_line (); > + } > + } > } > > /* If we found a list of potential completions during initialization Bubbling up TUI implementation details into GDB core gives me pause. I'm left wondering if there are more problems, and this is just fixing one of them. I see that TUI has special code for readline, (grep for readline in tui-io.c) so at the least I'm wondering why this is necessary. And if it is, let's push it down into tui/ as much as possible (with a comment explaining why the code exists :-)). ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3 v2] Implement completion limiting 2014-12-05 23:54 ` Doug Evans @ 2014-12-10 12:22 ` Gary Benson 2014-12-10 16:25 ` Doug Evans 0 siblings, 1 reply; 29+ messages in thread From: Gary Benson @ 2014-12-10 12:22 UTC (permalink / raw) To: Doug Evans; +Cc: gdb-patches, Eli Zaretskii Doug Evans wrote: > Gary Benson <gbenson@redhat.com> writes: > > This commit adds a new exception, TOO_MANY_COMPLETIONS_ERROR, to > > be thrown whenever the completer has generated too many candidates > > to be useful. A new user-settable variable, "max_completions", is > > added to control this behaviour. A top-level completion limit is > > added to complete_line_internal, as the final check to ensure the > > user never sees too many completions. An additional limit is > > added to default_make_symbol_completion_list_break_on, to halt > > time-consuming symbol table expansions. > > 1) IWBN if, when "Too many possibilities" is hit, the user was still > shown the completions thus far. I'd rather not have to abort the > command I'm trying to do, increase max-completions, and then try > again (or anything else to try to find what I'm looking for in order > to complete the command). At least not if I don't have to: the > completions thus far may provide a hint at what I'm looking for. > Plus GDB has already computed them, might as well print them. > Imagine if the total count is MAX+1, the user might find it annoying > to not be shown anything just because the count is one beyond the > max. > So instead of "Too many possibilities", how about printing the > completions thus far and then include a message saying the list is > clipped due to max-completions being reached? [Maybe readline makes > this difficult, but I think it'd be really nice have. Thoughts?] It's a nice idea but I'm not volunteering to implement it :) I already spent too much time figuring out how to thread things through readline. > 2) Readline has a limiting facility already: rl_completion_query_items. > But it's only applied after all completions have been computed so it > doesn't help us. > It would be good for the docs to explain the difference. > E.g. with the default of 200 for max-completions, if I do > > (top-gdb) b dwarf2<tab><tab> > > I get > > Display all 159 possibilities? (y or n) > > As a user, I'm kinda wondering why I'm being asked this if, for > example, I've explicitly set max-completions to some value. > [I know normally users might not even be aware of max-completions, > but suppose for the sake of discussion that they've set it to some > value larger than rl_completion_query_items.] > Note: rl_completion_query_items can be set in ~/.inputrc with > the completion-query-items parameter. rl_completion_query_items is a different thing. From http://web.mit.edu/gnu/doc/html/rlman_2.html: Up to this many items will be displayed in response to a possible- completions call. After that, we ask the user if she is sure she wants to see them all. The default value is 100. Basically it's the threshold readline uses to decide whether to say "Display all 159 possibilities? (y or n)" or just print all the candidates without asking. > 3) rl_completion_query_items uses a value of zero to mean unlimited, > whereas max_completions uses -1 (or "unlimited"). While it might be > nice to provide a way to disable completions completely (by setting > max-completions to zero), I'm trying to decide whether that benefit > is sufficient to justify the inconsistency with > rl_completion_query_items. Thoughts? GDB's "remotetimeout" and "trace-buffer-size" options use -1 to denote unlimited, as do Scheme things defined with PARAM_ZUINTEGER_UNLIMITED. I prefer "max-completions" being consistent with other GDB options over "max-completions" being consistent with readline options. It's not important to me that users can disable completion, I put the functionality there because existing GDB code uses -1 so I had to handle 0 somehow. > 4) Is there a use-case that involves wanting both > rl_completion_query_items and max_completions parameters? > Certainly we need to limit the number while we're building them, > but do we need both parameters? [IOW, could we fold them into one?] > I can imagine wanting to set max-completions to some large value > but still be given a prompt if the completion-query-items > threshold is reached, so I think we want both. > I'm just raising the possibility that maybe we don't want both > in case someone wants to comment. I think we want both. > At the least, it'd probably be good to mention how both interact in > in the docs. I can do this. > > diff --git a/gdb/completer.c b/gdb/completer.c > > index a0f3fa3..4a2302c 100644 > > --- a/gdb/completer.c > > +++ b/gdb/completer.c > > [...] > > @@ -894,7 +984,35 @@ line_completion_function (const char *text, int matche> > [...] > > + if (rl_completion_type != TAB) > > + { > > +#if defined(TUI) > > + if (tui_active) > > + { > > + tui_puts ("\n"); > > + tui_puts (ex.message); > > + tui_puts ("\n"); > > + } > > + else > > +#endif > > + { > > + rl_crlf (); > > + fputs (ex.message, rl_outstream); > > + rl_crlf (); > > + } > > + > > + rl_on_new_line (); > > + } > > Bubbling up TUI implementation details into GDB core gives me pause. > I'm left wondering if there are more problems, and this is just > fixing one of them. I see that TUI has special code for readline, > (grep for readline in tui-io.c) so at the least I'm wondering why > this is necessary. > And if it is, let's push it down into tui/ as much as possible > (with a comment explaining why the code exists :-)). I'm no TUI expert, so I can't comment on whether it's necessary or not. Assuming it is necessary, I don't know how I could remove this block without vectorizing the CLI/TUI interface... or is this done already? There are other places where "#ifdef TUI" and variants are used, at least four: gdb/cli/cli-cmds.c gdb/main.c gdb/printcmd.c gdb/utils.c Cheers, Gary -- http://gbenson.net/ ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3 v2] Implement completion limiting 2014-12-10 12:22 ` Gary Benson @ 2014-12-10 16:25 ` Doug Evans 2015-01-03 2:09 ` Doug Evans 0 siblings, 1 reply; 29+ messages in thread From: Doug Evans @ 2014-12-10 16:25 UTC (permalink / raw) To: Gary Benson; +Cc: gdb-patches, Eli Zaretskii Gary Benson <gbenson@redhat.com> writes: > Doug Evans wrote: >> 1) IWBN if, when "Too many possibilities" is hit, the user was still >> shown the completions thus far. I'd rather not have to abort the >> command I'm trying to do, increase max-completions, and then try >> again (or anything else to try to find what I'm looking for in order >> to complete the command). At least not if I don't have to: the >> completions thus far may provide a hint at what I'm looking for. >> Plus GDB has already computed them, might as well print them. >> Imagine if the total count is MAX+1, the user might find it annoying >> to not be shown anything just because the count is one beyond the >> max. >> So instead of "Too many possibilities", how about printing the >> completions thus far and then include a message saying the list is >> clipped due to max-completions being reached? [Maybe readline makes >> this difficult, but I think it'd be really nice have. Thoughts?] > > It's a nice idea but I'm not volunteering to implement it :) > I already spent too much time figuring out how to thread things > through readline. One thought I had was one could add a final completion entry that was the message. Would that work? >> 2) Readline has a limiting facility already: rl_completion_query_items. >> But it's only applied after all completions have been computed so it >> doesn't help us. >> It would be good for the docs to explain the difference. >> E.g. with the default of 200 for max-completions, if I do >> >> (top-gdb) b dwarf2<tab><tab> >> >> I get >> >> Display all 159 possibilities? (y or n) >> >> As a user, I'm kinda wondering why I'm being asked this if, for >> example, I've explicitly set max-completions to some value. >> [I know normally users might not even be aware of max-completions, >> but suppose for the sake of discussion that they've set it to some >> value larger than rl_completion_query_items.] >> Note: rl_completion_query_items can be set in ~/.inputrc with >> the completion-query-items parameter. > > rl_completion_query_items is a different thing. > [...] I realize that. Still, the two interact, and we should think through the u/i. >> 3) rl_completion_query_items uses a value of zero to mean unlimited, >> whereas max_completions uses -1 (or "unlimited"). While it might be >> nice to provide a way to disable completions completely (by setting >> max-completions to zero), I'm trying to decide whether that benefit >> is sufficient to justify the inconsistency with >> rl_completion_query_items. Thoughts? > > GDB's "remotetimeout" and "trace-buffer-size" options use -1 to denote > unlimited, as do Scheme things defined with PARAM_ZUINTEGER_UNLIMITED. > I prefer "max-completions" being consistent with other GDB options > over "max-completions" being consistent with readline options. > It's not important to me that users can disable completion, I put the > functionality there because existing GDB code uses -1 so I had to > handle 0 somehow. OTOH, [dje@seba gdb]$ grep add_setshow_uinteger *.c | wc 11 50 797 [dje@seba gdb]$ grep add_setshow_zuinteger_unlimited *.c | wc 2 8 163 gdb uses a mix, so consistency with gdb is a bit of a toss up. From a u/i perspective does a value of zero for max-completions make sense? Maybe it does, but I dunno, it doesn't feel like it. >> 4) Is there a use-case that involves wanting both >> rl_completion_query_items and max_completions parameters? >> Certainly we need to limit the number while we're building them, >> but do we need both parameters? [IOW, could we fold them into one?] >> I can imagine wanting to set max-completions to some large value >> but still be given a prompt if the completion-query-items >> threshold is reached, so I think we want both. >> I'm just raising the possibility that maybe we don't want both >> in case someone wants to comment. > > I think we want both. "works for me" >> At the least, it'd probably be good to mention how both interact in >> in the docs. > > I can do this. Thanks. btw, I noticed this in completer.c: /* Generate completions all at once. Returns a vector of strings allocated with xmalloc. Returns NULL if there are no completions or if max_completions is 0. Throws TOO_MANY_COMPLETIONS_ERROR if max_completions is greater than zero and the number of completions is greater than max_completions. But that's not what the code does AFAICT: /* Possibly throw TOO_MANY_COMPLETIONS_ERROR. Individual completers may do this too, to avoid unnecessary work, but this is the ultimate check that stops users seeing more completions than they wanted. */ if (max_completions >= 0) >> > diff --git a/gdb/completer.c b/gdb/completer.c >> > index a0f3fa3..4a2302c 100644 >> > --- a/gdb/completer.c >> > +++ b/gdb/completer.c >> > [...] >> > @@ -894,7 +984,35 @@ line_completion_function (const char *text, int matche> > [...] >> > + if (rl_completion_type != TAB) >> > + { >> > +#if defined(TUI) >> > + if (tui_active) >> > + { >> > + tui_puts ("\n"); >> > + tui_puts (ex.message); >> > + tui_puts ("\n"); >> > + } >> > + else >> > +#endif >> > + { >> > + rl_crlf (); >> > + fputs (ex.message, rl_outstream); >> > + rl_crlf (); >> > + } >> > + >> > + rl_on_new_line (); >> > + } >> >> Bubbling up TUI implementation details into GDB core gives me pause. >> I'm left wondering if there are more problems, and this is just >> fixing one of them. I see that TUI has special code for readline, >> (grep for readline in tui-io.c) so at the least I'm wondering why >> this is necessary. >> And if it is, let's push it down into tui/ as much as possible >> (with a comment explaining why the code exists :-)). > > I'm no TUI expert, so I can't comment on whether it's necessary or > not. Assuming it is necessary, I don't know how I could remove this > block without vectorizing the CLI/TUI interface... or is this done > already? There are other places where "#ifdef TUI" and variants > are used, at least four: > > gdb/cli/cli-cmds.c > gdb/main.c > gdb/printcmd.c > gdb/utils.c If we don't know whether it's necessary then why are we adding it? ["it" being the test for tui_active and the following code] I don't understand. Was this derived from another place in gdb that needed to do a similar thing? I grepped all uses of tui_active outside of tui/*.c and didn't see anything. One hope I had was that this would be enough: >> > + rl_crlf (); >> > + fputs (ex.message, rl_outstream); >> > + rl_crlf (); and that the efforts tui/*.c goes to to support readline would make that work regardless of the value of tui_active. But I confess I haven't tried it. I wouldn't suggest vectorizing the tui interface. But I do, at the least, want to understand why this is necessary ("this" being the test for tui_active and the different code depending on whether it is true or not), and if it is then I would at a minimum put this code: >> > +#if defined(TUI) >> > + if (tui_active) >> > + { >> > + tui_puts ("\n"); >> > + tui_puts (ex.message); >> > + tui_puts ("\n"); >> > + } >> > + else >> > +#endif >> > + { >> > + rl_crlf (); >> > + fputs (ex.message, rl_outstream); >> > + rl_crlf (); >> > + } >> > + >> > + rl_on_new_line (); into a function and call it from line_completion_function. completer.c up until now has had zero references to tui_*, and adding one with no explanation of why makes the code hard to reason about. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3 v2] Implement completion limiting 2014-12-10 16:25 ` Doug Evans @ 2015-01-03 2:09 ` Doug Evans 2015-01-07 8:44 ` Gary Benson 0 siblings, 1 reply; 29+ messages in thread From: Doug Evans @ 2015-01-03 2:09 UTC (permalink / raw) To: Gary Benson; +Cc: gdb-patches, Eli Zaretskii Doug Evans writes: > Gary Benson <gbenson@redhat.com> writes: > > Doug Evans wrote: > >> 1) IWBN if, when "Too many possibilities" is hit, the user was still > >> shown the completions thus far. I'd rather not have to abort the > >> command I'm trying to do, increase max-completions, and then try > >> again (or anything else to try to find what I'm looking for in order > >> to complete the command). At least not if I don't have to: the > >> completions thus far may provide a hint at what I'm looking for. > >> Plus GDB has already computed them, might as well print them. > >> Imagine if the total count is MAX+1, the user might find it annoying > >> to not be shown anything just because the count is one beyond the > >> max. > >> So instead of "Too many possibilities", how about printing the > >> completions thus far and then include a message saying the list is > >> clipped due to max-completions being reached? [Maybe readline makes > >> this difficult, but I think it'd be really nice have. Thoughts?] > > > > It's a nice idea but I'm not volunteering to implement it :) > > I already spent too much time figuring out how to thread things > > through readline. > > One thought I had was one could add a final completion entry > that was the message. > Would that work? I looked into this a bit. readline provides a hook to print the completion list: rl_completion_display_matches_hook and a routine to display the matches: rl_display_match_list The code in readline/complete.c:display_matches is pretty straightforward (though they've apparently forgotten to export a way for the hook to set rl_display_fixed - we'll want to be as equivalent as possible), so I think(!) this will be rather easy to do. > One hope I had was that this would be enough: > > >> > + rl_crlf (); > >> > + fputs (ex.message, rl_outstream); > >> > + rl_crlf (); > > and that the efforts tui/*.c goes to to support readline would > make that work regardless of the value of tui_active. > But I confess I haven't tried it. > > I wouldn't suggest vectorizing the tui interface. > But I do, at the least, want to understand why this is necessary > ("this" being the test for tui_active and the different code > depending on whether it is true or not), > and if it is then I would at a minimum put this code: > > >> > +#if defined(TUI) > >> > + if (tui_active) > >> > + { > >> > + tui_puts ("\n"); > >> > + tui_puts (ex.message); > >> > + tui_puts ("\n"); > >> > + } > >> > + else > >> > +#endif > >> > + { > >> > + rl_crlf (); > >> > + fputs (ex.message, rl_outstream); > >> > + rl_crlf (); > >> > + } > >> > + > >> > + rl_on_new_line (); So that leaves this as just the remaining thing to resolve (AFAICT). I'll look into this more next week. I'd really like to get this into 7.9. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3 v2] Implement completion limiting 2015-01-03 2:09 ` Doug Evans @ 2015-01-07 8:44 ` Gary Benson 2015-01-09 1:29 ` Doug Evans 2015-01-10 2:32 ` Doug Evans 0 siblings, 2 replies; 29+ messages in thread From: Gary Benson @ 2015-01-07 8:44 UTC (permalink / raw) To: Doug Evans; +Cc: gdb-patches, Eli Zaretskii Doug Evans wrote: > Doug Evans writes: > > Gary Benson <gbenson@redhat.com> writes: > > > Doug Evans wrote: > > >> 1) IWBN if, when "Too many possibilities" is hit, the user was still > > >> shown the completions thus far. I'd rather not have to abort the > > >> command I'm trying to do, increase max-completions, and then try > > >> again (or anything else to try to find what I'm looking for in order > > >> to complete the command). At least not if I don't have to: the > > >> completions thus far may provide a hint at what I'm looking for. > > >> Plus GDB has already computed them, might as well print them. > > >> Imagine if the total count is MAX+1, the user might find it annoying > > >> to not be shown anything just because the count is one beyond the > > >> max. > > >> So instead of "Too many possibilities", how about printing the > > >> completions thus far and then include a message saying the list is > > >> clipped due to max-completions being reached? [Maybe readline makes > > >> this difficult, but I think it'd be really nice have. Thoughts?] > > > > > > It's a nice idea but I'm not volunteering to implement it :) > > > I already spent too much time figuring out how to thread things > > > through readline. > > > > One thought I had was one could add a final completion entry > > that was the message. > > Would that work? > > I looked into this a bit. > readline provides a hook to print the completion list: > rl_completion_display_matches_hook > and a routine to display the matches: > rl_display_match_list > > The code in readline/complete.c:display_matches is > pretty straightforward (though they've apparently > forgotten to export a way for the hook to set > rl_display_fixed - we'll want to be as equivalent > as possible), so I think(!) this will be rather easy to do. > > > One hope I had was that this would be enough: > > > > >> > + rl_crlf (); > > >> > + fputs (ex.message, rl_outstream); > > >> > + rl_crlf (); > > > > and that the efforts tui/*.c goes to to support readline would > > make that work regardless of the value of tui_active. > > But I confess I haven't tried it. > > > > I wouldn't suggest vectorizing the tui interface. > > But I do, at the least, want to understand why this is necessary > > ("this" being the test for tui_active and the different code > > depending on whether it is true or not), > > and if it is then I would at a minimum put this code: > > > > >> > +#if defined(TUI) > > >> > + if (tui_active) > > >> > + { > > >> > + tui_puts ("\n"); > > >> > + tui_puts (ex.message); > > >> > + tui_puts ("\n"); > > >> > + } > > >> > + else > > >> > +#endif > > >> > + { > > >> > + rl_crlf (); > > >> > + fputs (ex.message, rl_outstream); > > >> > + rl_crlf (); > > >> > + } > > >> > + > > >> > + rl_on_new_line (); > > So that leaves this as just the remaining thing to resolve (AFAICT). > I'll look into this more next week. > I'd really like to get this into 7.9. If you want it in 7.9 then how about I commit it as it is then submit a followup patch to remove the #ifdef, and you can make your own patch to add whatever functionality you want. The readline part of this series took a good week to get right and I can guarantee you this will drag past 7.9 if I touch it. Cheers, Gary -- http://gbenson.net/ ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3 v2] Implement completion limiting 2015-01-07 8:44 ` Gary Benson @ 2015-01-09 1:29 ` Doug Evans 2015-01-10 2:32 ` Doug Evans 1 sibling, 0 replies; 29+ messages in thread From: Doug Evans @ 2015-01-09 1:29 UTC (permalink / raw) To: Gary Benson; +Cc: gdb-patches, Eli Zaretskii On Wed, Jan 7, 2015 at 12:42 AM, Gary Benson <gbenson@redhat.com> wrote: > Doug Evans wrote: >> Doug Evans writes: >> > Gary Benson <gbenson@redhat.com> writes: >> > > Doug Evans wrote: >> > >> 1) IWBN if, when "Too many possibilities" is hit, the user was still >> > >> shown the completions thus far. I'd rather not have to abort the >> > >> command I'm trying to do, increase max-completions, and then try >> > >> again (or anything else to try to find what I'm looking for in order >> > >> to complete the command). At least not if I don't have to: the >> > >> completions thus far may provide a hint at what I'm looking for. >> > >> Plus GDB has already computed them, might as well print them. >> > >> Imagine if the total count is MAX+1, the user might find it annoying >> > >> to not be shown anything just because the count is one beyond the >> > >> max. >> > >> So instead of "Too many possibilities", how about printing the >> > >> completions thus far and then include a message saying the list is >> > >> clipped due to max-completions being reached? [Maybe readline makes >> > >> this difficult, but I think it'd be really nice have. Thoughts?] >> > > >> > > It's a nice idea but I'm not volunteering to implement it :) >> > > I already spent too much time figuring out how to thread things >> > > through readline. >> > >> > One thought I had was one could add a final completion entry >> > that was the message. >> > Would that work? >> >> I looked into this a bit. >> readline provides a hook to print the completion list: >> rl_completion_display_matches_hook >> and a routine to display the matches: >> rl_display_match_list >> >> The code in readline/complete.c:display_matches is >> pretty straightforward (though they've apparently >> forgotten to export a way for the hook to set >> rl_display_fixed - we'll want to be as equivalent >> as possible), so I think(!) this will be rather easy to do. >> >> > One hope I had was that this would be enough: >> > >> > >> > + rl_crlf (); >> > >> > + fputs (ex.message, rl_outstream); >> > >> > + rl_crlf (); >> > >> > and that the efforts tui/*.c goes to to support readline would >> > make that work regardless of the value of tui_active. >> > But I confess I haven't tried it. >> > >> > I wouldn't suggest vectorizing the tui interface. >> > But I do, at the least, want to understand why this is necessary >> > ("this" being the test for tui_active and the different code >> > depending on whether it is true or not), >> > and if it is then I would at a minimum put this code: >> > >> > >> > +#if defined(TUI) >> > >> > + if (tui_active) >> > >> > + { >> > >> > + tui_puts ("\n"); >> > >> > + tui_puts (ex.message); >> > >> > + tui_puts ("\n"); >> > >> > + } >> > >> > + else >> > >> > +#endif >> > >> > + { >> > >> > + rl_crlf (); >> > >> > + fputs (ex.message, rl_outstream); >> > >> > + rl_crlf (); >> > >> > + } >> > >> > + >> > >> > + rl_on_new_line (); >> >> So that leaves this as just the remaining thing to resolve (AFAICT). >> I'll look into this more next week. >> I'd really like to get this into 7.9. > > If you want it in 7.9 then how about I commit it as it is then submit > a followup patch to remove the #ifdef, and you can make your own patch > to add whatever functionality you want. The readline part of this > series took a good week to get right and I can guarantee you this will > drag past 7.9 if I touch it. No worries. I have a tentative combined patch which I'll submit tomorrow. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3 v2] Implement completion limiting 2015-01-07 8:44 ` Gary Benson 2015-01-09 1:29 ` Doug Evans @ 2015-01-10 2:32 ` Doug Evans 2015-01-10 9:23 ` Eli Zaretskii 1 sibling, 1 reply; 29+ messages in thread From: Doug Evans @ 2015-01-10 2:32 UTC (permalink / raw) To: Gary Benson, Eli Zaretskii; +Cc: gdb-patches Gary Benson writes: > Doug Evans wrote: > > Doug Evans writes: > > > Gary Benson <gbenson@redhat.com> writes: > > > > Doug Evans wrote: > > > >> 1) IWBN if, when "Too many possibilities" is hit, the user was still > > > >> shown the completions thus far. I'd rather not have to abort the > > > >> command I'm trying to do, increase max-completions, and then try > > > >> again (or anything else to try to find what I'm looking for in order > > > >> to complete the command). At least not if I don't have to: the > > > >> completions thus far may provide a hint at what I'm looking for. > > > >> Plus GDB has already computed them, might as well print them. > > > >> Imagine if the total count is MAX+1, the user might find it annoying > > > >> to not be shown anything just because the count is one beyond the > > > >> max. > > > >> So instead of "Too many possibilities", how about printing the > > > >> completions thus far and then include a message saying the list is > > > >> clipped due to max-completions being reached? [Maybe readline makes > > > >> this difficult, but I think it'd be really nice have. Thoughts?] > > > > > > > > It's a nice idea but I'm not volunteering to implement it :) > > > > I already spent too much time figuring out how to thread things > > > > through readline. > > > > > > One thought I had was one could add a final completion entry > > > that was the message. > > > Would that work? > > > > I looked into this a bit. > > readline provides a hook to print the completion list: > > rl_completion_display_matches_hook > > and a routine to display the matches: > > rl_display_match_list > > > > The code in readline/complete.c:display_matches is > > pretty straightforward (though they've apparently > > forgotten to export a way for the hook to set > > rl_display_fixed - we'll want to be as equivalent > > as possible), so I think(!) this will be rather easy to do. > > > > > One hope I had was that this would be enough: > > > > > > >> > + rl_crlf (); > > > >> > + fputs (ex.message, rl_outstream); > > > >> > + rl_crlf (); > > > > > > and that the efforts tui/*.c goes to to support readline would > > > make that work regardless of the value of tui_active. > > > But I confess I haven't tried it. > > > > > > I wouldn't suggest vectorizing the tui interface. > > > But I do, at the least, want to understand why this is necessary > > > ("this" being the test for tui_active and the different code > > > depending on whether it is true or not), > > > and if it is then I would at a minimum put this code: > > > > > > >> > +#if defined(TUI) > > > >> > + if (tui_active) > > > >> > + { > > > >> > + tui_puts ("\n"); > > > >> > + tui_puts (ex.message); > > > >> > + tui_puts ("\n"); > > > >> > + } > > > >> > + else > > > >> > +#endif > > > >> > + { > > > >> > + rl_crlf (); > > > >> > + fputs (ex.message, rl_outstream); > > > >> > + rl_crlf (); > > > >> > + } > > > >> > + > > > >> > + rl_on_new_line (); > > > > So that leaves this as just the remaining thing to resolve (AFAICT). > > I'll look into this more next week. > > I'd really like to get this into 7.9. > > If you want it in 7.9 then how about I commit it as it is then submit > a followup patch to remove the #ifdef, and you can make your own patch > to add whatever functionality you want. The readline part of this > series took a good week to get right and I can guarantee you this will > drag past 7.9 if I touch it. I've played with various approaches, I like some of them, other's not so much. This is the one I'm submitting. It might be possible in the future to consolidate TUI's and CLI's code for displaying matches, but there's no need to do that now. The main change in this patch is that gdb displays the matches it has, as well as notifying the user that the list may be truncated. I tested this with emacs and it works there too. To achieve that the "list may be truncated" message needs to be included with the text being completed. There was no test for symbol completion limiting so I added one. I've played with this in emacs, cli, and tui, and couldn't find any problems. Plus the displaying of the truncated list instead of nothing has been nice. This replaces part 3 of the patchset. https://sourceware.org/ml/gdb-patches/2014-11/msg00685.html 2015-01-09 Gary Benson <gbenson@redhat.com> Doug Evans <dje@google.com> PR cli/9007 PR cli/11920 PR cli/15548 * cli/cli-cmds.c (complete_command): Notify user if max-completions reached. * common/common-exceptions.h (enum errors) <MAX_COMPLETIONS_REACHED_ERROR>: New value. * completer.h (gdb_rl_display_match_list): New declaration. (max_completions): Likewise. (completion_tracker_t): New typedef. (new_completion_tracker): New declaration. (make_cleanup_free_completion_tracker): Likewise. (maybe_add_completion_enum): New enum. (maybe_add_completion): New declaration. (throw_max_completions_reached_error): Likewise. * completer.c (max_completions): New global variable. (new_completion_tracker): New function. (free_completion_tracker): Likewise. (make_cleanup_free_completion_tracker): Likewise. (maybe_add_completions): Likewise. (throw_max_completions_reached_error): Likewise. (complete_line): Remove duplicates and limit result to max_completions entries. (gdb_get_y_or_n): New function. (gdb_rl_display_match_list): Likewise. (_initialize_completer): New declaration and function. * symtab.c: Include completer.h. (completion_tracker): New static variable. (completion_list_add_name): Call maybe_add_completion. (default_make_symbol_completion_list_break_on_1): Renamed from default_make_symbol_completion_list_break_on. Maintain completion_tracker across calls to completion_list_add_name. (default_make_symbol_completion_list_break_on): New function. * top.c (init_main): Set rl_completion_display_matches_hook. * tui/tui-io.c: Include completer.h. (tui_old_rl_display_matches_hook): New static global. (tui_rl_display_match_list): Notify user if max-completions reached. (tui_setup_io): Save/restore rl_completion_display_matches_hook. * NEWS (New Options): Mention set/show max-completions. doc/ * gdb.texinfo (Command Completion): Document new "set/show max-completions" option. testsuite/ * gdb.base/completion.exp: Disable completion limiting for existing tests. Add new tests to check completion limiting. * gdb.linespec/ls-errs.exp: Disable completion limiting. diff --git a/gdb/NEWS b/gdb/NEWS index 9a668c4..9954229 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -45,7 +45,7 @@ compile code [-raw|-r] [--] [source code] compile file [-raw|-r] filename -* New commands +* New commands (for set/show, see "New options" below) queue-signal signal-name-or-number Queue a signal to be delivered to the thread when it is resumed. @@ -66,6 +66,15 @@ compile file [-r|-raw] filename produced by compiling the source code stored in the filename provided. +* New options + +set max-completions +show max-completions + Set the maximum number of candidates to be considered during + completion. The default value is 200. This limit allows GDB + to avoid generating large completion lists, the computation of + which can cause the debugger to become temporarily unresponsive. + * On resume, GDB now always passes the signal the program had stopped for to the thread the signal was sent to, even if the user changed threads before resuming. Previously GDB would often (but not diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c index e20d8dd..e769e62 100644 --- a/gdb/cli/cli-cmds.c +++ b/gdb/cli/cli-cmds.c @@ -236,7 +236,8 @@ help_command (char *command, int from_tty) help_cmd (command, gdb_stdout); } \f -/* The "complete" command is used by Emacs to implement completion. */ +/* Note: The "complete" command is used by Emacs to implement completion. + [Is that why this function writes output with *_unfiltered?] */ static void complete_command (char *arg, int from_tty) @@ -247,6 +248,18 @@ complete_command (char *arg, int from_tty) dont_repeat (); + if (max_completions == 0) + { + /* Only print this for non-mi frontends. An MI frontend may not + be able to handle this. */ + if (!ui_out_is_mi_like_p (current_uiout)) + { + printf_unfiltered (_("max-completions is zero," + " completion is disabled.\n")); + } + return; + } + if (arg == NULL) arg = ""; argpoint = strlen (arg); @@ -293,6 +306,13 @@ complete_command (char *arg, int from_tty) xfree (prev); VEC_free (char_ptr, completions); + + if (size == max_completions) + { + printf_unfiltered (_("%s%s *** List may be truncated," + " max-completions reached. ***\n"), + arg_prefix, point); + } } } diff --git a/gdb/common/common-exceptions.h b/gdb/common/common-exceptions.h index 4f60ad8..e349ed0 100644 --- a/gdb/common/common-exceptions.h +++ b/gdb/common/common-exceptions.h @@ -99,6 +99,12 @@ enum errors { /* Requested feature, method, mechanism, etc. is not supported. */ NOT_SUPPORTED_ERROR, + /* The number of candidates generated during line completion has + reached the user's specified limit. This isn't an error, this exception + is used to halt searching for more completions, but for consistency + "_ERROR" is appended to the name. */ + MAX_COMPLETIONS_REACHED_ERROR, + /* Add more errors here. */ NR_ERRORS }; diff --git a/gdb/completer.c b/gdb/completer.c index 2b6aa87..39cdcec 100644 --- a/gdb/completer.c +++ b/gdb/completer.c @@ -781,9 +781,93 @@ complete_line_internal (const char *text, return list; } -/* Generate completions all at once. Returns a vector of strings. - Each element is allocated with xmalloc. It can also return NULL if - there are no completions. + +/* See completer.h. */ + +int max_completions = 200; + +/* See completer.h. */ + +completion_tracker_t +new_completion_tracker (void) +{ + if (max_completions <= 0) + return NULL; + + return htab_create_alloc (max_completions, + htab_hash_string, (htab_eq) streq, + NULL, xcalloc, xfree); +} + +/* Cleanup routine to free a completion tracker and reset the pointer + to NULL. */ + +static void +free_completion_tracker (void *p) +{ + completion_tracker_t *tracker_ptr = p; + + htab_delete (*tracker_ptr); + *tracker_ptr = NULL; +} + +/* See completer.h. */ + +struct cleanup * +make_cleanup_free_completion_tracker (completion_tracker_t *tracker_ptr) +{ + if (*tracker_ptr == NULL) + return make_cleanup (null_cleanup, NULL); + + return make_cleanup (free_completion_tracker, tracker_ptr); +} + +/* See completer.h. */ + +enum maybe_add_completion_enum +maybe_add_completion (completion_tracker_t tracker, char *name) +{ + void **slot; + + if (max_completions < 0) + return MAYBE_ADD_COMPLETION_OK; + if (max_completions == 0) + return MAYBE_ADD_COMPLETION_MAX_REACHED; + + gdb_assert (tracker != NULL); + + if (htab_elements (tracker) >= max_completions) + return MAYBE_ADD_COMPLETION_MAX_REACHED; + + slot = htab_find_slot (tracker, name, INSERT); + + if (*slot != HTAB_EMPTY_ENTRY) + return MAYBE_ADD_COMPLETION_DUPLICATE; + + *slot = name; + + return (htab_elements (tracker) < max_completions + ? MAYBE_ADD_COMPLETION_OK + : MAYBE_ADD_COMPLETION_OK_MAX_REACHED); +} + +void +throw_max_completions_reached_error (void) +{ + throw_error (MAX_COMPLETIONS_REACHED_ERROR, _("Max completions reached.")); +} + +/* Generate completions all at once. Returns a vector of unique strings + allocated with xmalloc. Returns NULL if there are no completions + or if max_completions is 0. If max_completions is non-negative, this will + return at most max_completions + 1 strings. + + If max_completions strings are collected, an extra string is added which + is a text message to inform the user that the list may be truncated. + This extra string serves two purposes: + 1) Inform the user. + 2) Prevent readline from being able to find a common prefix to advance + point to, since it's working with an incomplete list. TEXT is the caller's idea of the "word" we are looking at. @@ -796,8 +880,66 @@ complete_line_internal (const char *text, VEC (char_ptr) * complete_line (const char *text, const char *line_buffer, int point) { - return complete_line_internal (text, line_buffer, - point, handle_completions); + VEC (char_ptr) *list; + VEC (char_ptr) *result = NULL; + struct cleanup *cleanups; + completion_tracker_t tracker; + char *candidate; + int ix, max_reached; + + if (max_completions == 0) + return NULL; + list = complete_line_internal (text, line_buffer, point, + handle_completions); + if (max_completions < 0) + return list; + + tracker = new_completion_tracker (); + cleanups = make_cleanup_free_completion_tracker (&tracker); + make_cleanup_free_char_ptr_vec (list); + + /* Do a final test for too many completions. Individual completers may + do some of this, but are not required to. Duplicates are also removed + here. Otherwise the user is left scratching his/her head: readline and + complete_command will remove duplicates, and if removal of duplicates + there brings the total under max_completions the user may think gdb quit + searching too early. */ + + for (ix = 0, max_reached = 0; + !max_reached && VEC_iterate (char_ptr, list, ix, candidate); + ++ix) + { + enum maybe_add_completion_enum add_status; + + add_status = maybe_add_completion (tracker, candidate); + + switch (add_status) + { + case MAYBE_ADD_COMPLETION_OK: + VEC_safe_push (char_ptr, result, xstrdup (candidate)); + break; + case MAYBE_ADD_COMPLETION_OK_MAX_REACHED: + VEC_safe_push (char_ptr, result, xstrdup (candidate)); + max_reached = 1; + break; + case MAYBE_ADD_COMPLETION_MAX_REACHED: + gdb_assert_not_reached ("more than max completions reached"); + case MAYBE_ADD_COMPLETION_DUPLICATE: + break; + } + } + + if (max_reached) + { + VEC_safe_push (char_ptr, result, + xstrprintf (_("%s *** List may be truncated," + " max-completions reached. ***"), + text)); + } + + do_cleanups (cleanups); + + return result; } /* Complete on command names. Used by "help". */ @@ -1020,3 +1162,90 @@ skip_quoted (const char *str) { return skip_quoted_chars (str, NULL, NULL); } + +/* gdb version of readline/complete.c:get_y_or_n. + The user must press "y" or "n". Non-zero return means "y" pressed. + Also supported: space == 'y', RUBOUT == 'n', ctrl-g == start over. */ + +static int +gdb_get_y_or_n (void) +{ + int c; + + for (;;) + { + RL_SETSTATE (RL_STATE_MOREINPUT); + c = rl_read_key (); + RL_UNSETSTATE (RL_STATE_MOREINPUT); + + if (c == 'y' || c == 'Y' || c == ' ') + return 1; + if (c == 'n' || c == 'N' || c == RUBOUT) + return 0; + if (c == ABORT_CHAR || c < 0) + { + /* Note: The arguments to rl_abort are ignored. */ + rl_abort (0, 0); + } + rl_ding (); + } +} + +/* See completer.h. + This follows complete.c:display_matches as close as possible. */ + +void +gdb_rl_display_match_list (char **matches, int len, int max) +{ + int actual_len = len; + + /* Readline will never call this if complete_line returned NULL. */ + gdb_assert (max_completions != 0); + + /* complete_line will never return more than this. */ + if (max_completions > 0) + gdb_assert (len <= max_completions + 1); + + /* If max_completions is reached, complete_line adds a last entry. */ + if (len == max_completions + 1) + actual_len = len - 1; + + /* If there are many items, then ask the user if she really wants to + see them all. */ + if (rl_completion_query_items > 0 && actual_len >= rl_completion_query_items) + { + /* We can't use *query here because they wait for <RET> which is + wrong here. This follows the readline version as closely as possible + for compatibility's sake. See readline/complete.c. */ + rl_crlf (); + fprintf (rl_outstream, _("Display all %d possibilities? (y or n)"), + actual_len); + fflush (rl_outstream); + if (gdb_get_y_or_n () == 0) + { + rl_crlf (); + rl_forced_update_display (); + return; + } + } + + rl_display_match_list (matches, len, max); + + rl_forced_update_display (); +} + +extern initialize_file_ftype _initialize_completer; /* -Wmissing-prototypes */ + +void +_initialize_completer (void) +{ + add_setshow_zuinteger_unlimited_cmd ("max-completions", no_class, + &max_completions, _("\ +Set maximum number of completion candidates."), _("\ +Show maximum number of completion candidates."), _("\ +Use this to limit the number of candidates considered\n\ +during completion. Specifying \"unlimited\" or -1\n\ +disables limiting. Note that setting either no limit or\n\ +a very large limit can make completion slow."), + NULL, NULL, &setlist, &showlist); +} diff --git a/gdb/completer.h b/gdb/completer.h index 8f925fe..dc94caa 100644 --- a/gdb/completer.h +++ b/gdb/completer.h @@ -59,6 +59,12 @@ extern char *gdb_completion_word_break_characters (void); extern void set_gdb_completion_word_break_characters (completer_ftype *fn); +/* rl_completion_display_matches_hook for gdb. + We wrap readline's default routine to be able to display an extra line + notifying the user that there may be more matches. */ + +extern void gdb_rl_display_match_list (char **matches, int len, int max); + /* Exported to linespec.c */ extern const char *skip_quoted_chars (const char *, const char *, @@ -66,4 +72,68 @@ extern const char *skip_quoted_chars (const char *, const char *, extern const char *skip_quoted (const char *); +/* Maximum number of candidates to consider before the completer + bails by throwing MAX_COMPLETIONS_REACHED_ERROR. Negative values + disable limiting. */ + +extern int max_completions; + +/* Object to track how many unique completions have been generated. + Used to limit the size of generated completion lists. */ + +typedef htab_t completion_tracker_t; + +/* Create a new completion tracker. + The result is a hash table to track added completions, or NULL + if max_completions <= 0. If max_completions < 0, tracking is disabled. + If max_completions == 0, the max is indeed zero. */ + +extern completion_tracker_t new_completion_tracker (void); + +/* Make a cleanup to free a completion tracker, and reset its pointer + to NULL. */ + +extern struct cleanup *make_cleanup_free_completion_tracker + (completion_tracker_t *tracker_ptr); + +/* Return values for maybe_add_completion. */ + +enum maybe_add_completion_enum +{ + /* NAME has been recorded and max_completions has not been reached, + or completion tracking is disabled (max_completions < 0). */ + MAYBE_ADD_COMPLETION_OK, + + /* NAME has been recorded and max_completions has been reached + (thus the caller can stop searching). */ + MAYBE_ADD_COMPLETION_OK_MAX_REACHED, + + /* max-completions entries has been reached. + Whether NAME is a duplicate or not is not determined. */ + MAYBE_ADD_COMPLETION_MAX_REACHED, + + /* NAME has already been recorded. + Note that this is never returned if completion tracking is disabled + (max_completions < 0). */ + MAYBE_ADD_COMPLETION_DUPLICATE +}; + +/* Add the completion NAME to the list of generated completions if + it is not there already. + If max_completions is negative, nothing is done, not even watching + for duplicates, and MAYBE_ADD_COMPLETION_OK is always returned. + + If MAYBE_ADD_COMPLETION_MAX_REACHED is returned, callers are required to + record at least one more completion. The final list will be pruned to + max_completions, but recording at least one more than max_completions is + the signal to the completion machinery that too many completions were + found. */ + +extern enum maybe_add_completion_enum + maybe_add_completion (completion_tracker_t tracker, char *name); + +/* Wrapper to throw MAX_COMPLETIONS_REACHED_ERROR. */ + +extern void throw_max_completions_reached_error (void); + #endif /* defined (COMPLETER_H) */ diff --git a/gdb/symtab.c b/gdb/symtab.c index 94b04fb..1d3d8ee 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -60,6 +60,7 @@ #include "macroscope.h" #include "parser-defs.h" +#include "completer.h" /* Forward declarations for local functions. */ @@ -4309,6 +4310,15 @@ static VEC (char_ptr) *return_val; completion_list_add_name \ (MSYMBOL_NATURAL_NAME (symbol), (sym_text), (len), (text), (word)) +/* Tracker for how many unique completions have been generated. Used + to terminate completion list generation early if the list has grown + to a size so large as to be useless. This helps avoid GDB seeming + to lock up in the event the user requests to complete on something + vague that necessitates the time consuming expansion of many symbol + tables. */ + +static completion_tracker_t completion_tracker; + /* Test to see if the symbol specified by SYMNAME (which is already demangled for C++ symbols) matches SYM_TEXT in the first SYM_TEXT_LEN characters. If so, add it to the current completion list. */ @@ -4327,6 +4337,7 @@ completion_list_add_name (const char *symname, { char *new; + enum maybe_add_completion_enum add_status; if (word == sym_text) { @@ -4348,7 +4359,22 @@ completion_list_add_name (const char *symname, strcat (new, symname); } - VEC_safe_push (char_ptr, return_val, new); + add_status = maybe_add_completion (completion_tracker, new); + + switch (add_status) + { + case MAYBE_ADD_COMPLETION_OK: + VEC_safe_push (char_ptr, return_val, new); + break; + case MAYBE_ADD_COMPLETION_OK_MAX_REACHED: + VEC_safe_push (char_ptr, return_val, new); + throw_max_completions_reached_error (); + case MAYBE_ADD_COMPLETION_MAX_REACHED: + throw_max_completions_reached_error (); + case MAYBE_ADD_COMPLETION_DUPLICATE: + xfree (new); + break; + } } } @@ -4561,11 +4587,11 @@ symtab_expansion_callback (struct compunit_symtab *symtab, datum->code); } -VEC (char_ptr) * -default_make_symbol_completion_list_break_on (const char *text, - const char *word, - const char *break_on, - enum type_code code) +static void +default_make_symbol_completion_list_break_on_1 (const char *text, + const char *word, + const char *break_on, + enum type_code code) { /* Problem: All of the symbols have to be copied because readline frees them. I'm not going to worry about this; hopefully there @@ -4583,7 +4609,7 @@ default_make_symbol_completion_list_break_on (const char *text, /* Length of sym_text. */ int sym_text_len; struct add_name_data datum; - struct cleanup *back_to; + struct cleanup *cleanups; /* Now look for the symbol we are supposed to complete on. */ { @@ -4618,7 +4644,7 @@ default_make_symbol_completion_list_break_on (const char *text, /* A double-quoted string is never a symbol, nor does it make sense to complete it any other way. */ { - return NULL; + return; } else { @@ -4654,8 +4680,8 @@ default_make_symbol_completion_list_break_on (const char *text, } gdb_assert (sym_text[sym_text_len] == '\0' || sym_text[sym_text_len] == '('); - return_val = NULL; - back_to = make_cleanup (do_free_completion_list, &return_val); + completion_tracker = new_completion_tracker (); + cleanups = make_cleanup_free_completion_tracker (&completion_tracker); datum.sym_text = sym_text; datum.sym_text_len = sym_text_len; @@ -4769,8 +4795,34 @@ default_make_symbol_completion_list_break_on (const char *text, macro_for_each (macro_user_macros, add_macro_name, &datum); } + do_cleanups (cleanups); +} + +VEC (char_ptr) * +default_make_symbol_completion_list_break_on (const char *text, + const char *word, + const char *break_on, + enum type_code code) +{ + struct cleanup *back_to; + volatile struct gdb_exception except; + + return_val = NULL; + back_to = make_cleanup (do_free_completion_list, &return_val); + + TRY_CATCH (except, RETURN_MASK_ERROR) + { + default_make_symbol_completion_list_break_on_1 (text, word, + break_on, code); + } + if (except.reason < 0) + { + if (except.error != MAX_COMPLETIONS_REACHED_ERROR) + throw_exception (except); + } + discard_cleanups (back_to); - return (return_val); + return return_val; } VEC (char_ptr) * diff --git a/gdb/top.c b/gdb/top.c index b85ea1a..0234ac8 100644 --- a/gdb/top.c +++ b/gdb/top.c @@ -1761,6 +1761,7 @@ init_main (void) rl_completion_entry_function = readline_line_completion_function; rl_completer_word_break_characters = default_word_break_characters (); rl_completer_quote_characters = get_gdb_completer_quote_characters (); + rl_completion_display_matches_hook = gdb_rl_display_match_list; rl_readline_name = "gdb"; rl_terminal_name = getenv ("TERM"); diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c index 7e8a3bc..3348a47 100644 --- a/gdb/tui/tui-io.c +++ b/gdb/tui/tui-io.c @@ -37,6 +37,7 @@ #include <fcntl.h> #include <signal.h> #include "filestuff.h" +#include "completer.h" #include "gdb_curses.h" @@ -132,6 +133,7 @@ static rl_getc_func_t *tui_old_rl_getc_function; static rl_voidfunc_t *tui_old_rl_redisplay_function; static rl_vintfunc_t *tui_old_rl_prep_terminal; static rl_voidfunc_t *tui_old_rl_deprep_terminal; +static rl_compdisp_func_t *tui_old_rl_display_matches_hook; static int tui_old_rl_echoing_p; /* Readline output stream. @@ -412,18 +414,30 @@ tui_rl_display_match_list (char **matches, int len, int max) int count, limit, printed_len; int i, j, k, l; const char *temp; + int actual_len = len; /* Screen dimension correspond to the TUI command window. */ int screenwidth = TUI_CMD_WIN->generic.width; + /* Readline will never call this if complete_line returned NULL. */ + gdb_assert (max_completions != 0); + + /* complete_line will never return more than this. */ + if (max_completions > 0) + gdb_assert (len <= max_completions + 1); + + /* If max_completions is reached, complete_line adds a last entry. */ + if (len == max_completions + 1) + actual_len = len - 1; + /* If there are many items, then ask the user if she really wants to see them all. */ - if (len >= rl_completion_query_items) + if (actual_len >= rl_completion_query_items) { char msg[256]; xsnprintf (msg, sizeof (msg), - "\nDisplay all %d possibilities? (y or n)", len); + "\nDisplay all %d possibilities? (y or n)", actual_len); tui_puts (msg); if (get_y_or_n () == 0) { @@ -521,6 +535,7 @@ tui_setup_io (int mode) tui_old_rl_deprep_terminal = rl_deprep_term_function; tui_old_rl_prep_terminal = rl_prep_term_function; tui_old_rl_getc_function = rl_getc_function; + tui_old_rl_display_matches_hook = rl_completion_display_matches_hook; tui_old_rl_outstream = rl_outstream; tui_old_rl_echoing_p = _rl_echoing_p; rl_redisplay_function = tui_redisplay_readline; @@ -564,8 +579,8 @@ tui_setup_io (int mode) rl_deprep_term_function = tui_old_rl_deprep_terminal; rl_prep_term_function = tui_old_rl_prep_terminal; rl_getc_function = tui_old_rl_getc_function; + rl_completion_display_matches_hook = tui_old_rl_display_matches_hook; rl_outstream = tui_old_rl_outstream; - rl_completion_display_matches_hook = 0; _rl_echoing_p = tui_old_rl_echoing_p; rl_already_prompted = 0; diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index f4d7132..fe4f2a5 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -1600,6 +1600,34 @@ means @kbd{@key{META} ?}. You can type this either by holding down a key designated as the @key{META} shift on your keyboard (if there is one) while typing @kbd{?}, or as @key{ESC} followed by @kbd{?}. +If the number of possible completions is large, @value{GDBN} will +print as much of the list as it has collected, as well as a message +indicating that the list may be truncated. + +@smallexample +(@value{GDBP}) b m@key{TAB}@key{TAB} +m *** List may be truncated, max-completions reached. *** +main +<... the rest of the possible completions ...> +(@value{GDBP}) b m +@end smallexample + +@noindent +This behavior can be controlled with the following commands: + +@table @code +@kindex set max-completions +@item set max-completions @var{limit} +@itemx set max-completions unlimited +Set the maximum number of candidates to be shown during completion. +The default value is 200. A value of zero disables tab-completion. +Note that setting either no limit or a very large limit can make +completion slow. +@kindex show max-completions +@item show max-completions +Show the maximum number of candidates to be shown during completion. +@end table + @cindex quotes in commands @cindex completion of quoted strings Sometimes the string you need, while logically a ``word'', may contain diff --git a/gdb/testsuite/gdb.base/completion.exp b/gdb/testsuite/gdb.base/completion.exp index 1123b99..f77bfe2 100644 --- a/gdb/testsuite/gdb.base/completion.exp +++ b/gdb/testsuite/gdb.base/completion.exp @@ -67,6 +67,7 @@ if ![runto_main] then { } set timeout 30 +gdb_test_no_output "set max-completions unlimited" gdb_test_no_output "complete print values\[0\].x." \ "field completion with invalid field" @@ -775,4 +776,86 @@ gdb_test_multiple "" "$test" { } } -return 0 +# +# Completion limiting. +# + +gdb_test_no_output "set max-completions 5" + +set test "command-name completion limiting using tab character" +send_gdb "p\t" +gdb_test_multiple "" "$test" { + -re "^p\\\x07$" { + send_gdb "\t" + gdb_test_multiple "" "$test" { + -re "List may be truncated, max-completions reached.*\r\n$gdb_prompt p$" { + # Complete the command and ignore the output to resync + # gdb for the next test. + send_gdb "\n" + gdb_test_multiple "" "$test" { + -re "$gdb_prompt $" { + pass "$test" + } + } + } + -re "$gdb_prompt p$" { + # Complete the command and ignore the output to resync + # gdb for the next test. + send_gdb "\n" + gdb_test_multiple "" "$test" { + -re "$gdb_prompt $" { + fail "$test" + } + } + } + } + } +} + +set test "command-name completion limiting using complete command" +send_gdb "complete p\n" +gdb_test_multiple "" "$test" { + -re "List may be truncated, max-completions reached.*\r\n$gdb_prompt $" { + pass "$test" + } +} + +gdb_test_no_output "set max-completions 3" + +set test "symbol-name completion limiting using tab character" +send_gdb "p marker\t" +gdb_test_multiple "" "$test" { + -re "^p marker\\\x07$" { + send_gdb "\t" + gdb_test_multiple "" "$test" { + -re "List may be truncated, max-completions reached.*\r\n$gdb_prompt p marker$" { + # Complete the command and ignore the output to resync + # gdb for the next test. + send_gdb "\n" + gdb_test_multiple "" "$test" { + -re "$gdb_prompt $" { + pass "$test" + } + } + } + -re "$gdb_prompt p marker$" { + # Complete the command and ignore the output to resync + # gdb for the next test. + send_gdb "\n" + gdb_test_multiple "" "$test" { + -re "$gdb_prompt $" { + fail "$test" + } + } + } + } + } +} + +set test "symbol-name completion limiting using complete command" +send_gdb "complete p mark\n" +gdb_test_multiple "" "$test" { + -re "List may be truncated, max-completions reached.*\r\n$gdb_prompt $" { + pass "$test" + } +} diff --git a/gdb/testsuite/gdb.linespec/ls-errs.exp b/gdb/testsuite/gdb.linespec/ls-errs.exp index 322598e..019312c 100644 --- a/gdb/testsuite/gdb.linespec/ls-errs.exp +++ b/gdb/testsuite/gdb.linespec/ls-errs.exp @@ -26,6 +26,9 @@ if {[prepare_for_testing $testfile $exefile $srcfile \ # Turn off the pending breakpoint queries. gdb_test_no_output "set breakpoint pending off" +# Turn off completion limiting +gdb_test_no_output "set max-completions unlimited" + # We intentionally do not use gdb_breakpoint for these tests. # Break at 'linespec' and expect the message in ::error_messages indexed by ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3 v2] Implement completion limiting 2015-01-10 2:32 ` Doug Evans @ 2015-01-10 9:23 ` Eli Zaretskii 2015-01-12 18:50 ` Doug Evans 0 siblings, 1 reply; 29+ messages in thread From: Eli Zaretskii @ 2015-01-10 9:23 UTC (permalink / raw) To: Doug Evans; +Cc: gbenson, gdb-patches > From: Doug Evans <dje@google.com> > Date: Fri, 9 Jan 2015 18:32:01 -0800 > Cc: gdb-patches@sourceware.org > > +If the number of possible completions is large, @value{GDBN} will > +print as much of the list as it has collected, as well as a message > +indicating that the list may be truncated. > + > +@smallexample > +(@value{GDBP}) b m@key{TAB}@key{TAB} > +m *** List may be truncated, max-completions reached. *** > +main > +<... the rest of the possible completions ...> > +(@value{GDBP}) b m Doesn't the "list may be truncated" message scrolls off the screen, if the list is long enough? If so, wouldn't it be better to display that message at the end instead, saying something like (More completions follow; max-completions exceeded.) The documentation parts are OK, thanks. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3 v2] Implement completion limiting 2015-01-10 9:23 ` Eli Zaretskii @ 2015-01-12 18:50 ` Doug Evans 2015-01-15 15:39 ` Gary Benson 0 siblings, 1 reply; 29+ messages in thread From: Doug Evans @ 2015-01-12 18:50 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Gary Benson, gdb-patches On Sat, Jan 10, 2015 at 1:23 AM, Eli Zaretskii <eliz@gnu.org> wrote: >> From: Doug Evans <dje@google.com> >> Date: Fri, 9 Jan 2015 18:32:01 -0800 >> Cc: gdb-patches@sourceware.org >> >> +If the number of possible completions is large, @value{GDBN} will >> +print as much of the list as it has collected, as well as a message >> +indicating that the list may be truncated. >> + >> +@smallexample >> +(@value{GDBP}) b m@key{TAB}@key{TAB} >> +m *** List may be truncated, max-completions reached. *** >> +main >> +<... the rest of the possible completions ...> >> +(@value{GDBP}) b m > > Doesn't the "list may be truncated" message scrolls off the screen, if > the list is long enough? If so, wouldn't it be better to display that > message at the end instead, saying something like > > (More completions follow; max-completions exceeded.) If the user sets a bad height, or if pagination is turned off, possibly. I can put it at the end. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3 v2] Implement completion limiting 2015-01-12 18:50 ` Doug Evans @ 2015-01-15 15:39 ` Gary Benson 2015-01-23 7:32 ` Doug Evans 0 siblings, 1 reply; 29+ messages in thread From: Gary Benson @ 2015-01-15 15:39 UTC (permalink / raw) To: Doug Evans; +Cc: Eli Zaretskii, gdb-patches Doug Evans wrote: > On Sat, Jan 10, 2015 at 1:23 AM, Eli Zaretskii <eliz@gnu.org> wrote: > > > From: Doug Evans <dje@google.com> > > > Date: Fri, 9 Jan 2015 18:32:01 -0800 > > > Cc: gdb-patches@sourceware.org > > > > > > +If the number of possible completions is large, @value{GDBN} will > > > +print as much of the list as it has collected, as well as a message > > > +indicating that the list may be truncated. > > > + > > > +@smallexample > > > +(@value{GDBP}) b m@key{TAB}@key{TAB} > > > +m *** List may be truncated, max-completions reached. *** > > > +main > > > +<... the rest of the possible completions ...> > > > +(@value{GDBP}) b m > > > > Doesn't the "list may be truncated" message scrolls off the > > screen, if the list is long enough? If so, wouldn't it be better > > to display that message at the end instead, saying something like > > > > (More completions follow; max-completions exceeded.) > > If the user sets a bad height, or if pagination is turned off, > possibly. > > I can put it at the end. I'd prefer it at the end. Cheers, Gary -- http://gbenson.net/ ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3 v2] Implement completion limiting 2015-01-15 15:39 ` Gary Benson @ 2015-01-23 7:32 ` Doug Evans 2015-01-23 10:59 ` Eli Zaretskii 0 siblings, 1 reply; 29+ messages in thread From: Doug Evans @ 2015-01-23 7:32 UTC (permalink / raw) To: Gary Benson, Eli Zaretskii, gdb-patches Gary Benson <gbenson@redhat.com> writes: > Doug Evans wrote: >> On Sat, Jan 10, 2015 at 1:23 AM, Eli Zaretskii <eliz@gnu.org> wrote: >> > > From: Doug Evans <dje@google.com> >> > > Date: Fri, 9 Jan 2015 18:32:01 -0800 >> > > Cc: gdb-patches@sourceware.org >> > > >> > > +If the number of possible completions is large, @value{GDBN} will >> > > +print as much of the list as it has collected, as well as a message >> > > +indicating that the list may be truncated. >> > > + >> > > +@smallexample >> > > +(@value{GDBP}) b m@key{TAB}@key{TAB} >> > > +m *** List may be truncated, max-completions reached. *** >> > > +main >> > > +<... the rest of the possible completions ...> >> > > +(@value{GDBP}) b m >> > >> > Doesn't the "list may be truncated" message scrolls off the >> > screen, if the list is long enough? If so, wouldn't it be better >> > to display that message at the end instead, saying something like >> > >> > (More completions follow; max-completions exceeded.) >> >> If the user sets a bad height, or if pagination is turned off, >> possibly. >> >> I can put it at the end. > > I'd prefer it at the end. This patch puts the warning at the end. It replaces 3/3 of this series (still requires 1/3 and 2/3 of course): https://sourceware.org/ml/gdb-patches/2014-11/msg00685.html and requires this patch: https://sourceware.org/ml/gdb-patches/2015-01/msg00573.html 2015-01-22 Gary Benson <gbenson@redhat.com> Doug Evans <dje@google.com> PR cli/9007 PR cli/11920 PR cli/15548 * cli/cli-cmds.c (complete_command): Notify user if max-completions reached. * common/common-exceptions.h (enum errors) <MAX_COMPLETIONS_REACHED_ERROR>: New value. * completer.h (get_max_completions_reached_message): New declaration. (max_completions): Likewise. (completion_tracker_t): New typedef. (new_completion_tracker): New declaration. (make_cleanup_free_completion_tracker): Likewise. (maybe_add_completion_enum): New enum. (maybe_add_completion): New declaration. (throw_max_completions_reached_error): Likewise. * completer.c (max_completions): New global variable. (new_completion_tracker): New function. (free_completion_tracker): Likewise. (make_cleanup_free_completion_tracker): Likewise. (maybe_add_completions): Likewise. (throw_max_completions_reached_error): Likewise. (complete_line): Remove duplicates and limit result to max_completions entries. (get_max_completions_reached_message): New function. (gdb_display_match_list): Handle max_completions. (_initialize_completer): New declaration and function. * symtab.c: Include completer.h. (completion_tracker): New static variable. (completion_list_add_name): Call maybe_add_completion. (default_make_symbol_completion_list_break_on_1): Renamed from default_make_symbol_completion_list_break_on. Maintain completion_tracker across calls to completion_list_add_name. (default_make_symbol_completion_list_break_on): New function. * top.c (init_main): Set rl_completion_display_matches_hook. * tui/tui-io.c: Include completer.h. (tui_old_rl_display_matches_hook): New static global. (tui_rl_display_match_list): Notify user if max-completions reached. (tui_setup_io): Save/restore rl_completion_display_matches_hook. * NEWS (New Options): Mention set/show max-completions. doc/ * gdb.texinfo (Command Completion): Document new "set/show max-completions" option. testsuite/ * gdb.base/completion.exp: Disable completion limiting for existing tests. Add new tests to check completion limiting. * gdb.linespec/ls-errs.exp: Disable completion limiting. diff --git a/gdb/NEWS b/gdb/NEWS index 2d2c941..09a5742 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -3,6 +3,15 @@ *** Changes since GDB 7.9 +* New options + +set max-completions +show max-completions + Set the maximum number of candidates to be considered during + completion. The default value is 200. This limit allows GDB + to avoid generating large completion lists, the computation of + which can cause the debugger to become temporarily unresponsive. + *** Changes in GDB 7.9 * GDB now supports hardware watchpoints on x86 GNU Hurd. diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c index e20d8dd..e46f036 100644 --- a/gdb/cli/cli-cmds.c +++ b/gdb/cli/cli-cmds.c @@ -236,7 +236,8 @@ help_command (char *command, int from_tty) help_cmd (command, gdb_stdout); } \f -/* The "complete" command is used by Emacs to implement completion. */ +/* Note: The "complete" command is used by Emacs to implement completion. + [Is that why this function writes output with *_unfiltered?] */ static void complete_command (char *arg, int from_tty) @@ -247,6 +248,18 @@ complete_command (char *arg, int from_tty) dont_repeat (); + if (max_completions == 0) + { + /* Only print this for non-mi frontends. An MI frontend may not + be able to handle this. */ + if (!ui_out_is_mi_like_p (current_uiout)) + { + printf_unfiltered (_("max-completions is zero," + " completion is disabled.\n")); + } + return; + } + if (arg == NULL) arg = ""; argpoint = strlen (arg); @@ -293,6 +306,15 @@ complete_command (char *arg, int from_tty) xfree (prev); VEC_free (char_ptr, completions); + + if (size == max_completions) + { + /* ARG_PREFIX and POINT are included in the output so that emacs + will include the message in the output. */ + printf_unfiltered (_("%s%s %s\n"), + arg_prefix, point, + get_max_completions_reached_message ()); + } } } diff --git a/gdb/common/common-exceptions.h b/gdb/common/common-exceptions.h index 4f60ad8..e349ed0 100644 --- a/gdb/common/common-exceptions.h +++ b/gdb/common/common-exceptions.h @@ -99,6 +99,12 @@ enum errors { /* Requested feature, method, mechanism, etc. is not supported. */ NOT_SUPPORTED_ERROR, + /* The number of candidates generated during line completion has + reached the user's specified limit. This isn't an error, this exception + is used to halt searching for more completions, but for consistency + "_ERROR" is appended to the name. */ + MAX_COMPLETIONS_REACHED_ERROR, + /* Add more errors here. */ NR_ERRORS }; diff --git a/gdb/completer.c b/gdb/completer.c index 88c8e16..8d27a8b 100644 --- a/gdb/completer.c +++ b/gdb/completer.c @@ -781,9 +781,93 @@ complete_line_internal (const char *text, return list; } -/* Generate completions all at once. Returns a vector of strings. - Each element is allocated with xmalloc. It can also return NULL if - there are no completions. + +/* See completer.h. */ + +int max_completions = 200; + +/* See completer.h. */ + +completion_tracker_t +new_completion_tracker (void) +{ + if (max_completions <= 0) + return NULL; + + return htab_create_alloc (max_completions, + htab_hash_string, (htab_eq) streq, + NULL, xcalloc, xfree); +} + +/* Cleanup routine to free a completion tracker and reset the pointer + to NULL. */ + +static void +free_completion_tracker (void *p) +{ + completion_tracker_t *tracker_ptr = p; + + htab_delete (*tracker_ptr); + *tracker_ptr = NULL; +} + +/* See completer.h. */ + +struct cleanup * +make_cleanup_free_completion_tracker (completion_tracker_t *tracker_ptr) +{ + if (*tracker_ptr == NULL) + return make_cleanup (null_cleanup, NULL); + + return make_cleanup (free_completion_tracker, tracker_ptr); +} + +/* See completer.h. */ + +enum maybe_add_completion_enum +maybe_add_completion (completion_tracker_t tracker, char *name) +{ + void **slot; + + if (max_completions < 0) + return MAYBE_ADD_COMPLETION_OK; + if (max_completions == 0) + return MAYBE_ADD_COMPLETION_MAX_REACHED; + + gdb_assert (tracker != NULL); + + if (htab_elements (tracker) >= max_completions) + return MAYBE_ADD_COMPLETION_MAX_REACHED; + + slot = htab_find_slot (tracker, name, INSERT); + + if (*slot != HTAB_EMPTY_ENTRY) + return MAYBE_ADD_COMPLETION_DUPLICATE; + + *slot = name; + + return (htab_elements (tracker) < max_completions + ? MAYBE_ADD_COMPLETION_OK + : MAYBE_ADD_COMPLETION_OK_MAX_REACHED); +} + +void +throw_max_completions_reached_error (void) +{ + throw_error (MAX_COMPLETIONS_REACHED_ERROR, _("Max completions reached.")); +} + +/* Generate completions all at once. Returns a vector of unique strings + allocated with xmalloc. Returns NULL if there are no completions + or if max_completions is 0. If max_completions is non-negative, this will + return at most max_completions + 1 strings. + + If max_completions strings are collected, an extra string is added which + is a text message to inform the user that the list may be truncated. + This extra string serves two purposes: + 1) Inform the user. + 2) Prevent readline from being able to find a common prefix to advance + point to, since it's working with an incomplete list. TEXT is the caller's idea of the "word" we are looking at. @@ -796,8 +880,68 @@ complete_line_internal (const char *text, VEC (char_ptr) * complete_line (const char *text, const char *line_buffer, int point) { - return complete_line_internal (text, line_buffer, - point, handle_completions); + VEC (char_ptr) *list; + VEC (char_ptr) *result = NULL; + struct cleanup *cleanups; + completion_tracker_t tracker; + char *candidate; + int ix, max_reached; + + if (max_completions == 0) + return NULL; + list = complete_line_internal (text, line_buffer, point, + handle_completions); + if (max_completions < 0) + return list; + + tracker = new_completion_tracker (); + cleanups = make_cleanup_free_completion_tracker (&tracker); + make_cleanup_free_char_ptr_vec (list); + + /* Do a final test for too many completions. Individual completers may + do some of this, but are not required to. Duplicates are also removed + here. Otherwise the user is left scratching his/her head: readline and + complete_command will remove duplicates, and if removal of duplicates + there brings the total under max_completions the user may think gdb quit + searching too early. */ + + for (ix = 0, max_reached = 0; + !max_reached && VEC_iterate (char_ptr, list, ix, candidate); + ++ix) + { + enum maybe_add_completion_enum add_status; + + add_status = maybe_add_completion (tracker, candidate); + + switch (add_status) + { + case MAYBE_ADD_COMPLETION_OK: + VEC_safe_push (char_ptr, result, xstrdup (candidate)); + break; + case MAYBE_ADD_COMPLETION_OK_MAX_REACHED: + VEC_safe_push (char_ptr, result, xstrdup (candidate)); + max_reached = 1; + break; + case MAYBE_ADD_COMPLETION_MAX_REACHED: + gdb_assert_not_reached ("more than max completions reached"); + case MAYBE_ADD_COMPLETION_DUPLICATE: + break; + } + } + +#if 0 //xyzdje + if (max_reached) + { + VEC_safe_push (char_ptr, result, + xstrprintf (_("%s *** List may be truncated," + " max-completions reached. ***"), + text)); + } +#endif + + do_cleanups (cleanups); + + return result; } /* Complete on command names. Used by "help". */ @@ -1020,6 +1164,15 @@ skip_quoted (const char *str) { return skip_quoted_chars (str, NULL, NULL); } + +/* Return a message indicating that the maximum number of completions + has been reached and that there may be more. */ + +const char * +get_max_completions_reached_message (void) +{ + return _("*** List may be truncated, max-completions reached. ***"); +} \f /* GDB replacement for rl_display_match_list. Readline doesn't provide a clean interface for TUI(curses). @@ -1413,9 +1566,10 @@ gdb_complete_get_screenwidth (const struct match_list_displayer *displayer) } /* GDB version of readline/complete.c:rl_display_match_list. - See gdb_display_match_list for a description of MATCHES, LEN, MAX. */ + See gdb_display_match_list for a description of MATCHES, LEN, MAX. + Returns non-zero if all matches are displayed. */ -static void +static int gdb_display_match_list_1 (char **matches, int len, int max, const struct match_list_displayer *displayer) { @@ -1501,7 +1655,7 @@ gdb_display_match_list_1 (char **matches, int len, int max, { lines = gdb_display_match_list_pager (lines, displayer); if (lines < 0) - return; + return 0; } } } @@ -1523,7 +1677,7 @@ gdb_display_match_list_1 (char **matches, int len, int max, { lines = gdb_display_match_list_pager (lines, displayer); if (lines < 0) - return; + return 0; } } else @@ -1533,6 +1687,8 @@ gdb_display_match_list_1 (char **matches, int len, int max, } displayer->crlf (displayer); } + + return 1; } /* Utility for displaying completion list matches, used by both CLI and TUI. @@ -1550,6 +1706,13 @@ void gdb_display_match_list (char **matches, int len, int max, const struct match_list_displayer *displayer) { + /* Readline will never call this if complete_line returned NULL. */ + gdb_assert (max_completions != 0); + + /* complete_line will never return more than this. */ + if (max_completions > 0) + gdb_assert (len <= max_completions); + if (rl_completion_query_items > 0 && len >= rl_completion_query_items) { char msg[100]; @@ -1572,5 +1735,33 @@ gdb_display_match_list (char **matches, int len, int max, } } - gdb_display_match_list_1 (matches, len, max, displayer); + if (gdb_display_match_list_1 (matches, len, max, displayer)) + { + /* Note: MAX_COMPLETIONS may be -1 or zero, but LEN is always > 0. */ + if (len == max_completions) + { + /* The maximum number of completions has been reached. Warn the user + that there may be more. */ + const char *message = get_max_completions_reached_message (); + + displayer->puts (displayer, message); + displayer->crlf (displayer); + } + } +} +\f +extern initialize_file_ftype _initialize_completer; /* -Wmissing-prototypes */ + +void +_initialize_completer (void) +{ + add_setshow_zuinteger_unlimited_cmd ("max-completions", no_class, + &max_completions, _("\ +Set maximum number of completion candidates."), _("\ +Show maximum number of completion candidates."), _("\ +Use this to limit the number of candidates considered\n\ +during completion. Specifying \"unlimited\" or -1\n\ +disables limiting. Note that setting either no limit or\n\ +a very large limit can make completion slow."), + NULL, NULL, &setlist, &showlist); } diff --git a/gdb/completer.h b/gdb/completer.h index dbb1cfb..56e1a2b 100644 --- a/gdb/completer.h +++ b/gdb/completer.h @@ -66,6 +66,8 @@ struct match_list_displayer extern void gdb_display_match_list (char **matches, int len, int max, const struct match_list_displayer *); +extern const char *get_max_completions_reached_message (void); + extern VEC (char_ptr) *complete_line (const char *text, const char *line_buffer, int point); @@ -112,4 +114,68 @@ extern const char *skip_quoted_chars (const char *, const char *, extern const char *skip_quoted (const char *); +/* Maximum number of candidates to consider before the completer + bails by throwing MAX_COMPLETIONS_REACHED_ERROR. Negative values + disable limiting. */ + +extern int max_completions; + +/* Object to track how many unique completions have been generated. + Used to limit the size of generated completion lists. */ + +typedef htab_t completion_tracker_t; + +/* Create a new completion tracker. + The result is a hash table to track added completions, or NULL + if max_completions <= 0. If max_completions < 0, tracking is disabled. + If max_completions == 0, the max is indeed zero. */ + +extern completion_tracker_t new_completion_tracker (void); + +/* Make a cleanup to free a completion tracker, and reset its pointer + to NULL. */ + +extern struct cleanup *make_cleanup_free_completion_tracker + (completion_tracker_t *tracker_ptr); + +/* Return values for maybe_add_completion. */ + +enum maybe_add_completion_enum +{ + /* NAME has been recorded and max_completions has not been reached, + or completion tracking is disabled (max_completions < 0). */ + MAYBE_ADD_COMPLETION_OK, + + /* NAME has been recorded and max_completions has been reached + (thus the caller can stop searching). */ + MAYBE_ADD_COMPLETION_OK_MAX_REACHED, + + /* max-completions entries has been reached. + Whether NAME is a duplicate or not is not determined. */ + MAYBE_ADD_COMPLETION_MAX_REACHED, + + /* NAME has already been recorded. + Note that this is never returned if completion tracking is disabled + (max_completions < 0). */ + MAYBE_ADD_COMPLETION_DUPLICATE +}; + +/* Add the completion NAME to the list of generated completions if + it is not there already. + If max_completions is negative, nothing is done, not even watching + for duplicates, and MAYBE_ADD_COMPLETION_OK is always returned. + + If MAYBE_ADD_COMPLETION_MAX_REACHED is returned, callers are required to + record at least one more completion. The final list will be pruned to + max_completions, but recording at least one more than max_completions is + the signal to the completion machinery that too many completions were + found. */ + +extern enum maybe_add_completion_enum + maybe_add_completion (completion_tracker_t tracker, char *name); + +/* Wrapper to throw MAX_COMPLETIONS_REACHED_ERROR. */ + +extern void throw_max_completions_reached_error (void); + #endif /* defined (COMPLETER_H) */ diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index f413e23..90e4a61 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -1600,6 +1600,34 @@ means @kbd{@key{META} ?}. You can type this either by holding down a key designated as the @key{META} shift on your keyboard (if there is one) while typing @kbd{?}, or as @key{ESC} followed by @kbd{?}. +If the number of possible completions is large, @value{GDBN} will +print as much of the list as it has collected, as well as a message +indicating that the list may be truncated. + +@smallexample +(@value{GDBP}) b m@key{TAB}@key{TAB} +main +<... the rest of the possible completions ...> +*** List may be truncated, max-completions reached. *** +(@value{GDBP}) b m +@end smallexample + +@noindent +This behavior can be controlled with the following commands: + +@table @code +@kindex set max-completions +@item set max-completions @var{limit} +@itemx set max-completions unlimited +Set the maximum number of candidates to be shown during completion. +The default value is 200. A value of zero disables tab-completion. +Note that setting either no limit or a very large limit can make +completion slow. +@kindex show max-completions +@item show max-completions +Show the maximum number of candidates to be shown during completion. +@end table + @cindex quotes in commands @cindex completion of quoted strings Sometimes the string you need, while logically a ``word'', may contain diff --git a/gdb/symtab.c b/gdb/symtab.c index 94b04fb..1d3d8ee 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -60,6 +60,7 @@ #include "macroscope.h" #include "parser-defs.h" +#include "completer.h" /* Forward declarations for local functions. */ @@ -4309,6 +4310,15 @@ static VEC (char_ptr) *return_val; completion_list_add_name \ (MSYMBOL_NATURAL_NAME (symbol), (sym_text), (len), (text), (word)) +/* Tracker for how many unique completions have been generated. Used + to terminate completion list generation early if the list has grown + to a size so large as to be useless. This helps avoid GDB seeming + to lock up in the event the user requests to complete on something + vague that necessitates the time consuming expansion of many symbol + tables. */ + +static completion_tracker_t completion_tracker; + /* Test to see if the symbol specified by SYMNAME (which is already demangled for C++ symbols) matches SYM_TEXT in the first SYM_TEXT_LEN characters. If so, add it to the current completion list. */ @@ -4327,6 +4337,7 @@ completion_list_add_name (const char *symname, { char *new; + enum maybe_add_completion_enum add_status; if (word == sym_text) { @@ -4348,7 +4359,22 @@ completion_list_add_name (const char *symname, strcat (new, symname); } - VEC_safe_push (char_ptr, return_val, new); + add_status = maybe_add_completion (completion_tracker, new); + + switch (add_status) + { + case MAYBE_ADD_COMPLETION_OK: + VEC_safe_push (char_ptr, return_val, new); + break; + case MAYBE_ADD_COMPLETION_OK_MAX_REACHED: + VEC_safe_push (char_ptr, return_val, new); + throw_max_completions_reached_error (); + case MAYBE_ADD_COMPLETION_MAX_REACHED: + throw_max_completions_reached_error (); + case MAYBE_ADD_COMPLETION_DUPLICATE: + xfree (new); + break; + } } } @@ -4561,11 +4587,11 @@ symtab_expansion_callback (struct compunit_symtab *symtab, datum->code); } -VEC (char_ptr) * -default_make_symbol_completion_list_break_on (const char *text, - const char *word, - const char *break_on, - enum type_code code) +static void +default_make_symbol_completion_list_break_on_1 (const char *text, + const char *word, + const char *break_on, + enum type_code code) { /* Problem: All of the symbols have to be copied because readline frees them. I'm not going to worry about this; hopefully there @@ -4583,7 +4609,7 @@ default_make_symbol_completion_list_break_on (const char *text, /* Length of sym_text. */ int sym_text_len; struct add_name_data datum; - struct cleanup *back_to; + struct cleanup *cleanups; /* Now look for the symbol we are supposed to complete on. */ { @@ -4618,7 +4644,7 @@ default_make_symbol_completion_list_break_on (const char *text, /* A double-quoted string is never a symbol, nor does it make sense to complete it any other way. */ { - return NULL; + return; } else { @@ -4654,8 +4680,8 @@ default_make_symbol_completion_list_break_on (const char *text, } gdb_assert (sym_text[sym_text_len] == '\0' || sym_text[sym_text_len] == '('); - return_val = NULL; - back_to = make_cleanup (do_free_completion_list, &return_val); + completion_tracker = new_completion_tracker (); + cleanups = make_cleanup_free_completion_tracker (&completion_tracker); datum.sym_text = sym_text; datum.sym_text_len = sym_text_len; @@ -4769,8 +4795,34 @@ default_make_symbol_completion_list_break_on (const char *text, macro_for_each (macro_user_macros, add_macro_name, &datum); } + do_cleanups (cleanups); +} + +VEC (char_ptr) * +default_make_symbol_completion_list_break_on (const char *text, + const char *word, + const char *break_on, + enum type_code code) +{ + struct cleanup *back_to; + volatile struct gdb_exception except; + + return_val = NULL; + back_to = make_cleanup (do_free_completion_list, &return_val); + + TRY_CATCH (except, RETURN_MASK_ERROR) + { + default_make_symbol_completion_list_break_on_1 (text, word, + break_on, code); + } + if (except.reason < 0) + { + if (except.error != MAX_COMPLETIONS_REACHED_ERROR) + throw_exception (except); + } + discard_cleanups (back_to); - return (return_val); + return return_val; } VEC (char_ptr) * diff --git a/gdb/testsuite/gdb.base/completion.exp b/gdb/testsuite/gdb.base/completion.exp index 1123b99..f77bfe2 100644 --- a/gdb/testsuite/gdb.base/completion.exp +++ b/gdb/testsuite/gdb.base/completion.exp @@ -67,6 +67,7 @@ if ![runto_main] then { } set timeout 30 +gdb_test_no_output "set max-completions unlimited" gdb_test_no_output "complete print values\[0\].x." \ "field completion with invalid field" @@ -775,4 +776,86 @@ gdb_test_multiple "" "$test" { } } -return 0 +# +# Completion limiting. +# + +gdb_test_no_output "set max-completions 5" + +set test "command-name completion limiting using tab character" +send_gdb "p\t" +gdb_test_multiple "" "$test" { + -re "^p\\\x07$" { + send_gdb "\t" + gdb_test_multiple "" "$test" { + -re "List may be truncated, max-completions reached.*\r\n$gdb_prompt p$" { + # Complete the command and ignore the output to resync + # gdb for the next test. + send_gdb "\n" + gdb_test_multiple "" "$test" { + -re "$gdb_prompt $" { + pass "$test" + } + } + } + -re "$gdb_prompt p$" { + # Complete the command and ignore the output to resync + # gdb for the next test. + send_gdb "\n" + gdb_test_multiple "" "$test" { + -re "$gdb_prompt $" { + fail "$test" + } + } + } + } + } +} + +set test "command-name completion limiting using complete command" +send_gdb "complete p\n" +gdb_test_multiple "" "$test" { + -re "List may be truncated, max-completions reached.*\r\n$gdb_prompt $" { + pass "$test" + } +} + +gdb_test_no_output "set max-completions 3" + +set test "symbol-name completion limiting using tab character" +send_gdb "p marker\t" +gdb_test_multiple "" "$test" { + -re "^p marker\\\x07$" { + send_gdb "\t" + gdb_test_multiple "" "$test" { + -re "List may be truncated, max-completions reached.*\r\n$gdb_prompt p marker$" { + # Complete the command and ignore the output to resync + # gdb for the next test. + send_gdb "\n" + gdb_test_multiple "" "$test" { + -re "$gdb_prompt $" { + pass "$test" + } + } + } + -re "$gdb_prompt p marker$" { + # Complete the command and ignore the output to resync + # gdb for the next test. + send_gdb "\n" + gdb_test_multiple "" "$test" { + -re "$gdb_prompt $" { + fail "$test" + } + } + } + } + } +} + +set test "symbol-name completion limiting using complete command" +send_gdb "complete p mark\n" +gdb_test_multiple "" "$test" { + -re "List may be truncated, max-completions reached.*\r\n$gdb_prompt $" { + pass "$test" + } +} diff --git a/gdb/testsuite/gdb.linespec/ls-errs.exp b/gdb/testsuite/gdb.linespec/ls-errs.exp index 322598e..019312c 100644 --- a/gdb/testsuite/gdb.linespec/ls-errs.exp +++ b/gdb/testsuite/gdb.linespec/ls-errs.exp @@ -26,6 +26,9 @@ if {[prepare_for_testing $testfile $exefile $srcfile \ # Turn off the pending breakpoint queries. gdb_test_no_output "set breakpoint pending off" +# Turn off completion limiting +gdb_test_no_output "set max-completions unlimited" + # We intentionally do not use gdb_breakpoint for these tests. # Break at 'linespec' and expect the message in ::error_messages indexed by diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c index bac76c3..e5cab45 100644 --- a/gdb/tui/tui-io.c +++ b/gdb/tui/tui-io.c @@ -132,6 +132,7 @@ static rl_getc_func_t *tui_old_rl_getc_function; static rl_voidfunc_t *tui_old_rl_redisplay_function; static rl_vintfunc_t *tui_old_rl_prep_terminal; static rl_voidfunc_t *tui_old_rl_deprep_terminal; +static rl_compdisp_func_t *tui_old_rl_display_matches_hook; static int tui_old_rl_echoing_p; /* Readline output stream. @@ -442,6 +443,7 @@ tui_setup_io (int mode) tui_old_rl_deprep_terminal = rl_deprep_term_function; tui_old_rl_prep_terminal = rl_prep_term_function; tui_old_rl_getc_function = rl_getc_function; + tui_old_rl_display_matches_hook = rl_completion_display_matches_hook; tui_old_rl_outstream = rl_outstream; tui_old_rl_echoing_p = _rl_echoing_p; rl_redisplay_function = tui_redisplay_readline; @@ -485,8 +487,8 @@ tui_setup_io (int mode) rl_deprep_term_function = tui_old_rl_deprep_terminal; rl_prep_term_function = tui_old_rl_prep_terminal; rl_getc_function = tui_old_rl_getc_function; + rl_completion_display_matches_hook = tui_old_rl_display_matches_hook; rl_outstream = tui_old_rl_outstream; - rl_completion_display_matches_hook = 0; _rl_echoing_p = tui_old_rl_echoing_p; rl_already_prompted = 0; ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3 v2] Implement completion limiting 2015-01-23 7:32 ` Doug Evans @ 2015-01-23 10:59 ` Eli Zaretskii 2015-01-23 16:38 ` Doug Evans 0 siblings, 1 reply; 29+ messages in thread From: Eli Zaretskii @ 2015-01-23 10:59 UTC (permalink / raw) To: Doug Evans; +Cc: gbenson, gdb-patches > From: Doug Evans <xdje42@gmail.com> > Date: Thu, 22 Jan 2015 21:19:24 -0800 > > This patch puts the warning at the end. Thanks. > +#if 0 //xyzdje > + if (max_reached) > + { > + VEC_safe_push (char_ptr, result, > + xstrprintf (_("%s *** List may be truncated," > + " max-completions reached. ***"), > + text)); > + } > +#endif This part should probably go away. > +/* Return a message indicating that the maximum number of completions > + has been reached and that there may be more. */ > + > +const char * > +get_max_completions_reached_message (void) > +{ > + return _("*** List may be truncated, max-completions reached. ***"); > +} I'd prefer a different wording: (More completions follow; omitted because max-completions exceeded.) My problem with your wording is two-fold: . "may be truncated" can be interpreted to the effect that GDB doesn't know whether truncation really happened; I think it does . "reached" is inaccurate; "exceeded" is more accurate The rest of the change in wording is just to follow the style that I frequently see in other applications in similar cases. The documentation parts are OK (but will need an update if you accept the above suggestion). ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3 v2] Implement completion limiting 2015-01-23 10:59 ` Eli Zaretskii @ 2015-01-23 16:38 ` Doug Evans 2015-01-23 16:49 ` Eli Zaretskii 0 siblings, 1 reply; 29+ messages in thread From: Doug Evans @ 2015-01-23 16:38 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Gary Benson, gdb-patches On Fri, Jan 23, 2015 at 2:15 AM, Eli Zaretskii <eliz@gnu.org> wrote: >> +/* Return a message indicating that the maximum number of completions >> + has been reached and that there may be more. */ >> + >> +const char * >> +get_max_completions_reached_message (void) >> +{ >> + return _("*** List may be truncated, max-completions reached. ***"); >> +} > > I'd prefer a different wording: > > (More completions follow; omitted because max-completions exceeded.) > > My problem with your wording is two-fold: > > . "may be truncated" can be interpreted to the effect that GDB > doesn't know whether truncation really happened; I think it does > > . "reached" is inaccurate; "exceeded" is more accurate > > The rest of the change in wording is just to follow the style that I > frequently see in other applications in similar cases. > > The documentation parts are OK (but will need an update if you accept > the above suggestion). Actually, the wording is correct for the current implementation. The patch stops searching when the limit is reached. It doesn't keep looking for at least one more to see if there are any more. Is this absolutely critical? Why? ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3 v2] Implement completion limiting 2015-01-23 16:38 ` Doug Evans @ 2015-01-23 16:49 ` Eli Zaretskii 2015-01-23 20:28 ` Doug Evans 0 siblings, 1 reply; 29+ messages in thread From: Eli Zaretskii @ 2015-01-23 16:49 UTC (permalink / raw) To: Doug Evans; +Cc: gbenson, gdb-patches > Date: Fri, 23 Jan 2015 08:14:15 -0800 > From: Doug Evans <xdje42@gmail.com> > Cc: Gary Benson <gbenson@redhat.com>, > "gdb-patches@sourceware.org" <gdb-patches@sourceware.org> > > The patch stops searching when the limit is reached. > It doesn't keep looking for at least one more to see if there are any more. > Is this absolutely critical? Why? Because if the user sets the maximum at N, she wants to see N elements, and if there are exactly N, she shouldn't get any warnings, IMO. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3 v2] Implement completion limiting 2015-01-23 16:49 ` Eli Zaretskii @ 2015-01-23 20:28 ` Doug Evans 2015-01-24 1:55 ` Eli Zaretskii 0 siblings, 1 reply; 29+ messages in thread From: Doug Evans @ 2015-01-23 20:28 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Gary Benson, gdb-patches On Fri, Jan 23, 2015 at 8:34 AM, Eli Zaretskii <eliz@gnu.org> wrote: >> Date: Fri, 23 Jan 2015 08:14:15 -0800 >> From: Doug Evans <xdje42@gmail.com> >> Cc: Gary Benson <gbenson@redhat.com>, >> "gdb-patches@sourceware.org" <gdb-patches@sourceware.org> >> >> The patch stops searching when the limit is reached. >> It doesn't keep looking for at least one more to see if there are any more. >> Is this absolutely critical? Why? > > Because if the user sets the maximum at N, she wants to see N > elements, and if there are exactly N, she shouldn't get any warnings, > IMO. How often will there be exactly N? And in that particular and massively rare case, once gdb has found N, how much extra work will be performed searching the entire executable and all its shared libraries just to verify there are in fact no more completions? [because that's what has to happen if we're to avoid printing *any* message] The user waits 5 minutes for the entire list and gets her 200 completions, and wonders why it took so long. Then she digs a bit deeper and finds out they were found in the first 5 seconds. Ugh. I don't see the benefit of going to the trouble of avoiding printing any message when there are exactly N completions. If there are exactly N completions and I see the message, no big deal. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3 v2] Implement completion limiting 2015-01-23 20:28 ` Doug Evans @ 2015-01-24 1:55 ` Eli Zaretskii 2015-01-24 8:50 ` Doug Evans 0 siblings, 1 reply; 29+ messages in thread From: Eli Zaretskii @ 2015-01-24 1:55 UTC (permalink / raw) To: Doug Evans; +Cc: gbenson, gdb-patches > Date: Fri, 23 Jan 2015 08:56:59 -0800 > From: Doug Evans <xdje42@gmail.com> > Cc: Gary Benson <gbenson@redhat.com>, > "gdb-patches@sourceware.org" <gdb-patches@sourceware.org> > > How often will there be exactly N? About the same frequency as there will be exactly N-1, I guess. > And in that particular and massively rare case, > once gdb has found N, how much extra work will > be performed searching the entire executable > and all its shared libraries just to verify there are > in fact no more completions? > [because that's what has to happen if > we're to avoid printing *any* message] > > The user waits 5 minutes for the entire list and > gets her 200 completions, and wonders > why it took so long. Then she digs a bit > deeper and finds out they were found > in the first 5 seconds. Ugh. 200 is just a random number. It's not magic in any way. So you could have 199 completions found in the first 5 sec, followed by 5 minutes of waiting for the 200th which is never found. Ugh. There's no way around this issue. > I don't see the benefit of going to the trouble > of avoiding printing any message when there are > exactly N completions. That's fine, but then the option's name and its documentation should be changed to reflect this. They currently support what I thought user will get, not what you describe above. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3 v2] Implement completion limiting 2015-01-24 1:55 ` Eli Zaretskii @ 2015-01-24 8:50 ` Doug Evans 2015-01-24 14:12 ` Eli Zaretskii 0 siblings, 1 reply; 29+ messages in thread From: Doug Evans @ 2015-01-24 8:50 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Gary Benson, gdb-patches On Fri, Jan 23, 2015 at 12:28 PM, Eli Zaretskii <eliz@gnu.org> wrote: >> Date: Fri, 23 Jan 2015 08:56:59 -0800 >> From: Doug Evans <xdje42@gmail.com> >> Cc: Gary Benson <gbenson@redhat.com>, >> "gdb-patches@sourceware.org" <gdb-patches@sourceware.org> >> >> How often will there be exactly N? > > About the same frequency as there will be exactly N-1, I guess. My point is, given any particular value I set for "set max-completions N", what's the probability that exactly that N completions will be found? Rather low I expect. >> And in that particular and massively rare case, >> once gdb has found N, how much extra work will >> be performed searching the entire executable >> and all its shared libraries just to verify there are >> in fact no more completions? >> [because that's what has to happen if >> we're to avoid printing *any* message] >> >> The user waits 5 minutes for the entire list and >> gets her 200 completions, and wonders >> why it took so long. Then she digs a bit >> deeper and finds out they were found >> in the first 5 seconds. Ugh. > > 200 is just a random number. It's not magic in any way. So you > could have 199 completions found in the first 5 sec, followed by 5 > minutes of waiting for the 200th which is never found. Ugh. > > There's no way around this issue. But if I, as a user, do "set max-completions 199" and then find out gdb is looking for 200 completions I would be disappointed. I might file a bug report even. If I do "set max-completions N" I expect gdb to stop looking when it gets to N. >> I don't see the benefit of going to the trouble >> of avoiding printing any message when there are >> exactly N completions. > > That's fine, but then the option's name and its documentation should > be changed to reflect this. They currently support what I thought > user will get, not what you describe above. The option name and documentation is fine. If I do "set max-completions 42" I expect gdb to stop looking when it finds 42 completions. It's a rather straightforward interpretation of the option's name IMO. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3 v2] Implement completion limiting 2015-01-24 8:50 ` Doug Evans @ 2015-01-24 14:12 ` Eli Zaretskii 2015-01-25 15:55 ` Doug Evans 0 siblings, 1 reply; 29+ messages in thread From: Eli Zaretskii @ 2015-01-24 14:12 UTC (permalink / raw) To: Doug Evans; +Cc: gbenson, gdb-patches > Date: Fri, 23 Jan 2015 16:32:29 -0800 > From: Doug Evans <xdje42@gmail.com> > Cc: Gary Benson <gbenson@redhat.com>, > "gdb-patches@sourceware.org" <gdb-patches@sourceware.org> > > On Fri, Jan 23, 2015 at 12:28 PM, Eli Zaretskii <eliz@gnu.org> wrote: > >> Date: Fri, 23 Jan 2015 08:56:59 -0800 > >> From: Doug Evans <xdje42@gmail.com> > >> Cc: Gary Benson <gbenson@redhat.com>, > >> "gdb-patches@sourceware.org" <gdb-patches@sourceware.org> > >> > >> How often will there be exactly N? > > > > About the same frequency as there will be exactly N-1, I guess. > > My point is, given any particular value I set for > "set max-completions N", what's the probability > that exactly that N completions will be found? > Rather low I expect. Rather low, I agree. But having N-1 is as low as having N. So the problem with wasting 99% of the time searching for a candidate that isn't there is not solved by not searching for the N+1st candidate. > > 200 is just a random number. It's not magic in any way. So you > > could have 199 completions found in the first 5 sec, followed by 5 > > minutes of waiting for the 200th which is never found. Ugh. > > > > There's no way around this issue. > > But if I, as a user, do "set max-completions 199" > and then find out gdb is looking for 200 completions > I would be disappointed. I might file a bug report even. When the user sets max-completions to 199, it expects GDB not to _show_ more than that. The documentation says: @item set max-completions @var{limit} @itemx set max-completions unlimited Set the maximum number of candidates to be shown during completion. [...] ^^^^^^^^^^^ @item show max-completions Show the maximum number of candidates to be shown during completion. ^^^^^^^^^^^ This usually means "find the completions, but don't show me more than the limit". If GDB is not going to try to see if there are more than the limit, then we need to tell that in the docs, otherwise some user might become disappointed and might file a bug report, claiming that GDB told it there might be more candidates, when there were none. > If I do "set max-completions N" I expect gdb to stop looking > when it gets to N. Well, at least one frequent user of GDB -- yours truly -- would expect GDB to actually try at least one more. So we should be explicit this is not what GDB does (assuming we are okay with that modus operandi). > >> I don't see the benefit of going to the trouble > >> of avoiding printing any message when there are > >> exactly N completions. > > > > That's fine, but then the option's name and its documentation should > > be changed to reflect this. They currently support what I thought > > user will get, not what you describe above. > > The option name and documentation is fine. See above. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3 v2] Implement completion limiting 2015-01-24 14:12 ` Eli Zaretskii @ 2015-01-25 15:55 ` Doug Evans 2015-01-25 19:14 ` Eli Zaretskii 0 siblings, 1 reply; 29+ messages in thread From: Doug Evans @ 2015-01-25 15:55 UTC (permalink / raw) To: Eli Zaretskii; +Cc: gbenson, gdb-patches Eli Zaretskii <eliz@gnu.org> writes: >> Date: Fri, 23 Jan 2015 16:32:29 -0800 >> From: Doug Evans <xdje42@gmail.com> >> Cc: Gary Benson <gbenson@redhat.com>, >> "gdb-patches@sourceware.org" <gdb-patches@sourceware.org> >> >> On Fri, Jan 23, 2015 at 12:28 PM, Eli Zaretskii <eliz@gnu.org> wrote: >> >> Date: Fri, 23 Jan 2015 08:56:59 -0800 >> >> From: Doug Evans <xdje42@gmail.com> >> >> Cc: Gary Benson <gbenson@redhat.com>, >> >> "gdb-patches@sourceware.org" <gdb-patches@sourceware.org> >> >> >> >> How often will there be exactly N? >> > >> > About the same frequency as there will be exactly N-1, I guess. >> >> My point is, given any particular value I set for >> "set max-completions N", what's the probability >> that exactly that N completions will be found? >> Rather low I expect. > > Rather low, I agree. But having N-1 is as low as having N. So the > problem with wasting 99% of the time searching for a candidate that > isn't there is not solved by not searching for the N+1st candidate. Eh? I think we're talking past each other so I'm going to set this aside, for the moment. >> > 200 is just a random number. It's not magic in any way. So you >> > could have 199 completions found in the first 5 sec, followed by 5 >> > minutes of waiting for the 200th which is never found. Ugh. >> > >> > There's no way around this issue. >> >> But if I, as a user, do "set max-completions 199" >> and then find out gdb is looking for 200 completions >> I would be disappointed. I might file a bug report even. > > When the user sets max-completions to 199, it expects GDB not to > _show_ more than that. The documentation says: > > @item set max-completions @var{limit} > @itemx set max-completions unlimited > Set the maximum number of candidates to be shown during completion. > [...] ^^^^^^^^^^^ > @item show max-completions > Show the maximum number of candidates to be shown during completion. > > This usually means "find the completions, but don't show me more than > the limit". If GDB is not going to try to see if there are more than > the limit, then we need to tell that in the docs, otherwise some user > might become disappointed and might file a bug report, claiming that > GDB told it there might be more candidates, when there were none. Hmmm ... If I told someone X might be true, *would* they claim I was wrong if it wasn't? What definition of the phrase "might be" (or "may be") are you using? It feels like you're splitting hairs here. >> If I do "set max-completions N" I expect gdb to stop looking >> when it gets to N. > > Well, at least one frequent user of GDB -- yours truly -- would expect > GDB to actually try at least one more. So we should be explicit this > is not what GDB does (assuming we are okay with that modus operandi). You have a different definition of "maximum" than I do, but whatever ... 2015-01-22 Gary Benson <gbenson@redhat.com> Doug Evans <dje@google.com> PR cli/9007 PR cli/11920 PR cli/15548 * cli/cli-cmds.c (complete_command): Notify user if max-completions reached. * common/common-exceptions.h (enum errors) <MAX_COMPLETIONS_REACHED_ERROR>: New value. * completer.h (get_max_completions_reached_message): New declaration. (max_completions): Likewise. (completion_tracker_t): New typedef. (new_completion_tracker): New declaration. (make_cleanup_free_completion_tracker): Likewise. (maybe_add_completion_enum): New enum. (maybe_add_completion): New declaration. (throw_max_completions_reached_error): Likewise. * completer.c (max_completions): New global variable. (new_completion_tracker): New function. (free_completion_tracker): Likewise. (make_cleanup_free_completion_tracker): Likewise. (maybe_add_completions): Likewise. (throw_max_completions_reached_error): Likewise. (complete_line): Remove duplicates and limit result to max_completions entries. (get_max_completions_reached_message): New function. (gdb_display_match_list): Handle max_completions. (_initialize_completer): New declaration and function. * symtab.c: Include completer.h. (completion_tracker): New static variable. (completion_list_add_name): Call maybe_add_completion. (default_make_symbol_completion_list_break_on_1): Renamed from default_make_symbol_completion_list_break_on. Maintain completion_tracker across calls to completion_list_add_name. (default_make_symbol_completion_list_break_on): New function. * top.c (init_main): Set rl_completion_display_matches_hook. * tui/tui-io.c: Include completer.h. (tui_old_rl_display_matches_hook): New static global. (tui_rl_display_match_list): Notify user if max-completions reached. (tui_setup_io): Save/restore rl_completion_display_matches_hook. * NEWS (New Options): Mention set/show max-completions. doc/ * gdb.texinfo (Command Completion): Document new "set/show max-completions" option. testsuite/ * gdb.base/completion.exp: Disable completion limiting for existing tests. Add new tests to check completion limiting. * gdb.linespec/ls-errs.exp: Disable completion limiting. diff --git a/gdb/NEWS b/gdb/NEWS index 2d2c941..09a5742 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -3,6 +3,15 @@ *** Changes since GDB 7.9 +* New options + +set max-completions +show max-completions + Set the maximum number of candidates to be considered during + completion. The default value is 200. This limit allows GDB + to avoid generating large completion lists, the computation of + which can cause the debugger to become temporarily unresponsive. + *** Changes in GDB 7.9 * GDB now supports hardware watchpoints on x86 GNU Hurd. diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c index e20d8dd..e46f036 100644 --- a/gdb/cli/cli-cmds.c +++ b/gdb/cli/cli-cmds.c @@ -236,7 +236,8 @@ help_command (char *command, int from_tty) help_cmd (command, gdb_stdout); } \f -/* The "complete" command is used by Emacs to implement completion. */ +/* Note: The "complete" command is used by Emacs to implement completion. + [Is that why this function writes output with *_unfiltered?] */ static void complete_command (char *arg, int from_tty) @@ -247,6 +248,18 @@ complete_command (char *arg, int from_tty) dont_repeat (); + if (max_completions == 0) + { + /* Only print this for non-mi frontends. An MI frontend may not + be able to handle this. */ + if (!ui_out_is_mi_like_p (current_uiout)) + { + printf_unfiltered (_("max-completions is zero," + " completion is disabled.\n")); + } + return; + } + if (arg == NULL) arg = ""; argpoint = strlen (arg); @@ -293,6 +306,15 @@ complete_command (char *arg, int from_tty) xfree (prev); VEC_free (char_ptr, completions); + + if (size == max_completions) + { + /* ARG_PREFIX and POINT are included in the output so that emacs + will include the message in the output. */ + printf_unfiltered (_("%s%s %s\n"), + arg_prefix, point, + get_max_completions_reached_message ()); + } } } diff --git a/gdb/common/common-exceptions.h b/gdb/common/common-exceptions.h index 4f60ad8..e349ed0 100644 --- a/gdb/common/common-exceptions.h +++ b/gdb/common/common-exceptions.h @@ -99,6 +99,12 @@ enum errors { /* Requested feature, method, mechanism, etc. is not supported. */ NOT_SUPPORTED_ERROR, + /* The number of candidates generated during line completion has + reached the user's specified limit. This isn't an error, this exception + is used to halt searching for more completions, but for consistency + "_ERROR" is appended to the name. */ + MAX_COMPLETIONS_REACHED_ERROR, + /* Add more errors here. */ NR_ERRORS }; diff --git a/gdb/completer.c b/gdb/completer.c index 88c8e16..287e9dc 100644 --- a/gdb/completer.c +++ b/gdb/completer.c @@ -781,9 +781,93 @@ complete_line_internal (const char *text, return list; } -/* Generate completions all at once. Returns a vector of strings. - Each element is allocated with xmalloc. It can also return NULL if - there are no completions. + +/* See completer.h. */ + +int max_completions = 200; + +/* See completer.h. */ + +completion_tracker_t +new_completion_tracker (void) +{ + if (max_completions <= 0) + return NULL; + + return htab_create_alloc (max_completions, + htab_hash_string, (htab_eq) streq, + NULL, xcalloc, xfree); +} + +/* Cleanup routine to free a completion tracker and reset the pointer + to NULL. */ + +static void +free_completion_tracker (void *p) +{ + completion_tracker_t *tracker_ptr = p; + + htab_delete (*tracker_ptr); + *tracker_ptr = NULL; +} + +/* See completer.h. */ + +struct cleanup * +make_cleanup_free_completion_tracker (completion_tracker_t *tracker_ptr) +{ + if (*tracker_ptr == NULL) + return make_cleanup (null_cleanup, NULL); + + return make_cleanup (free_completion_tracker, tracker_ptr); +} + +/* See completer.h. */ + +enum maybe_add_completion_enum +maybe_add_completion (completion_tracker_t tracker, char *name) +{ + void **slot; + + if (max_completions < 0) + return MAYBE_ADD_COMPLETION_OK; + if (max_completions == 0) + return MAYBE_ADD_COMPLETION_MAX_REACHED; + + gdb_assert (tracker != NULL); + + if (htab_elements (tracker) >= max_completions) + return MAYBE_ADD_COMPLETION_MAX_REACHED; + + slot = htab_find_slot (tracker, name, INSERT); + + if (*slot != HTAB_EMPTY_ENTRY) + return MAYBE_ADD_COMPLETION_DUPLICATE; + + *slot = name; + + return (htab_elements (tracker) < max_completions + ? MAYBE_ADD_COMPLETION_OK + : MAYBE_ADD_COMPLETION_OK_MAX_REACHED); +} + +void +throw_max_completions_reached_error (void) +{ + throw_error (MAX_COMPLETIONS_REACHED_ERROR, _("Max completions reached.")); +} + +/* Generate completions all at once. Returns a vector of unique strings + allocated with xmalloc. Returns NULL if there are no completions + or if max_completions is 0. If max_completions is non-negative, this will + return at most max_completions + 1 strings. + + If max_completions strings are collected, an extra string is added which + is a text message to inform the user that the list may be truncated. + This extra string serves two purposes: + 1) Inform the user. + 2) Prevent readline from being able to find a common prefix to advance + point to, since it's working with an incomplete list. TEXT is the caller's idea of the "word" we are looking at. @@ -796,8 +880,58 @@ complete_line_internal (const char *text, VEC (char_ptr) * complete_line (const char *text, const char *line_buffer, int point) { - return complete_line_internal (text, line_buffer, - point, handle_completions); + VEC (char_ptr) *list; + VEC (char_ptr) *result = NULL; + struct cleanup *cleanups; + completion_tracker_t tracker; + char *candidate; + int ix, max_reached; + + if (max_completions == 0) + return NULL; + list = complete_line_internal (text, line_buffer, point, + handle_completions); + if (max_completions < 0) + return list; + + tracker = new_completion_tracker (); + cleanups = make_cleanup_free_completion_tracker (&tracker); + make_cleanup_free_char_ptr_vec (list); + + /* Do a final test for too many completions. Individual completers may + do some of this, but are not required to. Duplicates are also removed + here. Otherwise the user is left scratching his/her head: readline and + complete_command will remove duplicates, and if removal of duplicates + there brings the total under max_completions the user may think gdb quit + searching too early. */ + + for (ix = 0, max_reached = 0; + !max_reached && VEC_iterate (char_ptr, list, ix, candidate); + ++ix) + { + enum maybe_add_completion_enum add_status; + + add_status = maybe_add_completion (tracker, candidate); + + switch (add_status) + { + case MAYBE_ADD_COMPLETION_OK: + VEC_safe_push (char_ptr, result, xstrdup (candidate)); + break; + case MAYBE_ADD_COMPLETION_OK_MAX_REACHED: + VEC_safe_push (char_ptr, result, xstrdup (candidate)); + max_reached = 1; + break; + case MAYBE_ADD_COMPLETION_MAX_REACHED: + gdb_assert_not_reached ("more than max completions reached"); + case MAYBE_ADD_COMPLETION_DUPLICATE: + break; + } + } + + do_cleanups (cleanups); + + return result; } /* Complete on command names. Used by "help". */ @@ -1020,6 +1154,15 @@ skip_quoted (const char *str) { return skip_quoted_chars (str, NULL, NULL); } + +/* Return a message indicating that the maximum number of completions + has been reached and that there may be more. */ + +const char * +get_max_completions_reached_message (void) +{ + return _("*** List may be truncated, max-completions reached. ***"); +} \f /* GDB replacement for rl_display_match_list. Readline doesn't provide a clean interface for TUI(curses). @@ -1413,9 +1556,10 @@ gdb_complete_get_screenwidth (const struct match_list_displayer *displayer) } /* GDB version of readline/complete.c:rl_display_match_list. - See gdb_display_match_list for a description of MATCHES, LEN, MAX. */ + See gdb_display_match_list for a description of MATCHES, LEN, MAX. + Returns non-zero if all matches are displayed. */ -static void +static int gdb_display_match_list_1 (char **matches, int len, int max, const struct match_list_displayer *displayer) { @@ -1501,7 +1645,7 @@ gdb_display_match_list_1 (char **matches, int len, int max, { lines = gdb_display_match_list_pager (lines, displayer); if (lines < 0) - return; + return 0; } } } @@ -1523,7 +1667,7 @@ gdb_display_match_list_1 (char **matches, int len, int max, { lines = gdb_display_match_list_pager (lines, displayer); if (lines < 0) - return; + return 0; } } else @@ -1533,6 +1677,8 @@ gdb_display_match_list_1 (char **matches, int len, int max, } displayer->crlf (displayer); } + + return 1; } /* Utility for displaying completion list matches, used by both CLI and TUI. @@ -1550,6 +1696,13 @@ void gdb_display_match_list (char **matches, int len, int max, const struct match_list_displayer *displayer) { + /* Readline will never call this if complete_line returned NULL. */ + gdb_assert (max_completions != 0); + + /* complete_line will never return more than this. */ + if (max_completions > 0) + gdb_assert (len <= max_completions); + if (rl_completion_query_items > 0 && len >= rl_completion_query_items) { char msg[100]; @@ -1572,5 +1725,33 @@ gdb_display_match_list (char **matches, int len, int max, } } - gdb_display_match_list_1 (matches, len, max, displayer); + if (gdb_display_match_list_1 (matches, len, max, displayer)) + { + /* Note: MAX_COMPLETIONS may be -1 or zero, but LEN is always > 0. */ + if (len == max_completions) + { + /* The maximum number of completions has been reached. Warn the user + that there may be more. */ + const char *message = get_max_completions_reached_message (); + + displayer->puts (displayer, message); + displayer->crlf (displayer); + } + } +} +\f +extern initialize_file_ftype _initialize_completer; /* -Wmissing-prototypes */ + +void +_initialize_completer (void) +{ + add_setshow_zuinteger_unlimited_cmd ("max-completions", no_class, + &max_completions, _("\ +Set maximum number of completion candidates."), _("\ +Show maximum number of completion candidates."), _("\ +Use this to limit the number of candidates considered\n\ +during completion. Specifying \"unlimited\" or -1\n\ +disables limiting. Note that setting either no limit or\n\ +a very large limit can make completion slow."), + NULL, NULL, &setlist, &showlist); } diff --git a/gdb/completer.h b/gdb/completer.h index dbb1cfb..56e1a2b 100644 --- a/gdb/completer.h +++ b/gdb/completer.h @@ -66,6 +66,8 @@ struct match_list_displayer extern void gdb_display_match_list (char **matches, int len, int max, const struct match_list_displayer *); +extern const char *get_max_completions_reached_message (void); + extern VEC (char_ptr) *complete_line (const char *text, const char *line_buffer, int point); @@ -112,4 +114,68 @@ extern const char *skip_quoted_chars (const char *, const char *, extern const char *skip_quoted (const char *); +/* Maximum number of candidates to consider before the completer + bails by throwing MAX_COMPLETIONS_REACHED_ERROR. Negative values + disable limiting. */ + +extern int max_completions; + +/* Object to track how many unique completions have been generated. + Used to limit the size of generated completion lists. */ + +typedef htab_t completion_tracker_t; + +/* Create a new completion tracker. + The result is a hash table to track added completions, or NULL + if max_completions <= 0. If max_completions < 0, tracking is disabled. + If max_completions == 0, the max is indeed zero. */ + +extern completion_tracker_t new_completion_tracker (void); + +/* Make a cleanup to free a completion tracker, and reset its pointer + to NULL. */ + +extern struct cleanup *make_cleanup_free_completion_tracker + (completion_tracker_t *tracker_ptr); + +/* Return values for maybe_add_completion. */ + +enum maybe_add_completion_enum +{ + /* NAME has been recorded and max_completions has not been reached, + or completion tracking is disabled (max_completions < 0). */ + MAYBE_ADD_COMPLETION_OK, + + /* NAME has been recorded and max_completions has been reached + (thus the caller can stop searching). */ + MAYBE_ADD_COMPLETION_OK_MAX_REACHED, + + /* max-completions entries has been reached. + Whether NAME is a duplicate or not is not determined. */ + MAYBE_ADD_COMPLETION_MAX_REACHED, + + /* NAME has already been recorded. + Note that this is never returned if completion tracking is disabled + (max_completions < 0). */ + MAYBE_ADD_COMPLETION_DUPLICATE +}; + +/* Add the completion NAME to the list of generated completions if + it is not there already. + If max_completions is negative, nothing is done, not even watching + for duplicates, and MAYBE_ADD_COMPLETION_OK is always returned. + + If MAYBE_ADD_COMPLETION_MAX_REACHED is returned, callers are required to + record at least one more completion. The final list will be pruned to + max_completions, but recording at least one more than max_completions is + the signal to the completion machinery that too many completions were + found. */ + +extern enum maybe_add_completion_enum + maybe_add_completion (completion_tracker_t tracker, char *name); + +/* Wrapper to throw MAX_COMPLETIONS_REACHED_ERROR. */ + +extern void throw_max_completions_reached_error (void); + #endif /* defined (COMPLETER_H) */ diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index f413e23..3a1239e 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -1600,6 +1600,38 @@ means @kbd{@key{META} ?}. You can type this either by holding down a key designated as the @key{META} shift on your keyboard (if there is one) while typing @kbd{?}, or as @key{ESC} followed by @kbd{?}. +If the number of possible completions is large, @value{GDBN} will +print as much of the list as it has collected, as well as a message +indicating that the list may be truncated. + +@smallexample +(@value{GDBP}) b m@key{TAB}@key{TAB} +main +<... the rest of the possible completions ...> +*** List may be truncated, max-completions reached. *** +(@value{GDBP}) b m +@end smallexample + +@noindent +This behavior can be controlled with the following commands: + +@table @code +@kindex set max-completions +@item set max-completions @var{limit} +@itemx set max-completions unlimited +Set the maximum number of candidates that @value{GDBN} will collect +and show during completion. This is useful when completing on things +like function names as collecting all the possible candidates can be +time consuming. +The default value is 200. A value of zero disables tab-completion. +Note that setting either no limit or a very large limit can make +completion slow. +@kindex show max-completions +@item show max-completions +Show the maximum number of candidates that @value{GDBN} will collect and show +during completion. +@end table + @cindex quotes in commands @cindex completion of quoted strings Sometimes the string you need, while logically a ``word'', may contain diff --git a/gdb/symtab.c b/gdb/symtab.c index 94b04fb..1d3d8ee 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -60,6 +60,7 @@ #include "macroscope.h" #include "parser-defs.h" +#include "completer.h" /* Forward declarations for local functions. */ @@ -4309,6 +4310,15 @@ static VEC (char_ptr) *return_val; completion_list_add_name \ (MSYMBOL_NATURAL_NAME (symbol), (sym_text), (len), (text), (word)) +/* Tracker for how many unique completions have been generated. Used + to terminate completion list generation early if the list has grown + to a size so large as to be useless. This helps avoid GDB seeming + to lock up in the event the user requests to complete on something + vague that necessitates the time consuming expansion of many symbol + tables. */ + +static completion_tracker_t completion_tracker; + /* Test to see if the symbol specified by SYMNAME (which is already demangled for C++ symbols) matches SYM_TEXT in the first SYM_TEXT_LEN characters. If so, add it to the current completion list. */ @@ -4327,6 +4337,7 @@ completion_list_add_name (const char *symname, { char *new; + enum maybe_add_completion_enum add_status; if (word == sym_text) { @@ -4348,7 +4359,22 @@ completion_list_add_name (const char *symname, strcat (new, symname); } - VEC_safe_push (char_ptr, return_val, new); + add_status = maybe_add_completion (completion_tracker, new); + + switch (add_status) + { + case MAYBE_ADD_COMPLETION_OK: + VEC_safe_push (char_ptr, return_val, new); + break; + case MAYBE_ADD_COMPLETION_OK_MAX_REACHED: + VEC_safe_push (char_ptr, return_val, new); + throw_max_completions_reached_error (); + case MAYBE_ADD_COMPLETION_MAX_REACHED: + throw_max_completions_reached_error (); + case MAYBE_ADD_COMPLETION_DUPLICATE: + xfree (new); + break; + } } } @@ -4561,11 +4587,11 @@ symtab_expansion_callback (struct compunit_symtab *symtab, datum->code); } -VEC (char_ptr) * -default_make_symbol_completion_list_break_on (const char *text, - const char *word, - const char *break_on, - enum type_code code) +static void +default_make_symbol_completion_list_break_on_1 (const char *text, + const char *word, + const char *break_on, + enum type_code code) { /* Problem: All of the symbols have to be copied because readline frees them. I'm not going to worry about this; hopefully there @@ -4583,7 +4609,7 @@ default_make_symbol_completion_list_break_on (const char *text, /* Length of sym_text. */ int sym_text_len; struct add_name_data datum; - struct cleanup *back_to; + struct cleanup *cleanups; /* Now look for the symbol we are supposed to complete on. */ { @@ -4618,7 +4644,7 @@ default_make_symbol_completion_list_break_on (const char *text, /* A double-quoted string is never a symbol, nor does it make sense to complete it any other way. */ { - return NULL; + return; } else { @@ -4654,8 +4680,8 @@ default_make_symbol_completion_list_break_on (const char *text, } gdb_assert (sym_text[sym_text_len] == '\0' || sym_text[sym_text_len] == '('); - return_val = NULL; - back_to = make_cleanup (do_free_completion_list, &return_val); + completion_tracker = new_completion_tracker (); + cleanups = make_cleanup_free_completion_tracker (&completion_tracker); datum.sym_text = sym_text; datum.sym_text_len = sym_text_len; @@ -4769,8 +4795,34 @@ default_make_symbol_completion_list_break_on (const char *text, macro_for_each (macro_user_macros, add_macro_name, &datum); } + do_cleanups (cleanups); +} + +VEC (char_ptr) * +default_make_symbol_completion_list_break_on (const char *text, + const char *word, + const char *break_on, + enum type_code code) +{ + struct cleanup *back_to; + volatile struct gdb_exception except; + + return_val = NULL; + back_to = make_cleanup (do_free_completion_list, &return_val); + + TRY_CATCH (except, RETURN_MASK_ERROR) + { + default_make_symbol_completion_list_break_on_1 (text, word, + break_on, code); + } + if (except.reason < 0) + { + if (except.error != MAX_COMPLETIONS_REACHED_ERROR) + throw_exception (except); + } + discard_cleanups (back_to); - return (return_val); + return return_val; } VEC (char_ptr) * diff --git a/gdb/testsuite/gdb.base/completion.exp b/gdb/testsuite/gdb.base/completion.exp index 1123b99..f77bfe2 100644 --- a/gdb/testsuite/gdb.base/completion.exp +++ b/gdb/testsuite/gdb.base/completion.exp @@ -67,6 +67,7 @@ if ![runto_main] then { } set timeout 30 +gdb_test_no_output "set max-completions unlimited" gdb_test_no_output "complete print values\[0\].x." \ "field completion with invalid field" @@ -775,4 +776,86 @@ gdb_test_multiple "" "$test" { } } -return 0 +# +# Completion limiting. +# + +gdb_test_no_output "set max-completions 5" + +set test "command-name completion limiting using tab character" +send_gdb "p\t" +gdb_test_multiple "" "$test" { + -re "^p\\\x07$" { + send_gdb "\t" + gdb_test_multiple "" "$test" { + -re "List may be truncated, max-completions reached.*\r\n$gdb_prompt p$" { + # Complete the command and ignore the output to resync + # gdb for the next test. + send_gdb "\n" + gdb_test_multiple "" "$test" { + -re "$gdb_prompt $" { + pass "$test" + } + } + } + -re "$gdb_prompt p$" { + # Complete the command and ignore the output to resync + # gdb for the next test. + send_gdb "\n" + gdb_test_multiple "" "$test" { + -re "$gdb_prompt $" { + fail "$test" + } + } + } + } + } +} + +set test "command-name completion limiting using complete command" +send_gdb "complete p\n" +gdb_test_multiple "" "$test" { + -re "List may be truncated, max-completions reached.*\r\n$gdb_prompt $" { + pass "$test" + } +} + +gdb_test_no_output "set max-completions 3" + +set test "symbol-name completion limiting using tab character" +send_gdb "p marker\t" +gdb_test_multiple "" "$test" { + -re "^p marker\\\x07$" { + send_gdb "\t" + gdb_test_multiple "" "$test" { + -re "List may be truncated, max-completions reached.*\r\n$gdb_prompt p marker$" { + # Complete the command and ignore the output to resync + # gdb for the next test. + send_gdb "\n" + gdb_test_multiple "" "$test" { + -re "$gdb_prompt $" { + pass "$test" + } + } + } + -re "$gdb_prompt p marker$" { + # Complete the command and ignore the output to resync + # gdb for the next test. + send_gdb "\n" + gdb_test_multiple "" "$test" { + -re "$gdb_prompt $" { + fail "$test" + } + } + } + } + } +} + +set test "symbol-name completion limiting using complete command" +send_gdb "complete p mark\n" +gdb_test_multiple "" "$test" { + -re "List may be truncated, max-completions reached.*\r\n$gdb_prompt $" { + pass "$test" + } +} diff --git a/gdb/testsuite/gdb.linespec/ls-errs.exp b/gdb/testsuite/gdb.linespec/ls-errs.exp index 322598e..019312c 100644 --- a/gdb/testsuite/gdb.linespec/ls-errs.exp +++ b/gdb/testsuite/gdb.linespec/ls-errs.exp @@ -26,6 +26,9 @@ if {[prepare_for_testing $testfile $exefile $srcfile \ # Turn off the pending breakpoint queries. gdb_test_no_output "set breakpoint pending off" +# Turn off completion limiting +gdb_test_no_output "set max-completions unlimited" + # We intentionally do not use gdb_breakpoint for these tests. # Break at 'linespec' and expect the message in ::error_messages indexed by diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c index bac76c3..e5cab45 100644 --- a/gdb/tui/tui-io.c +++ b/gdb/tui/tui-io.c @@ -132,6 +132,7 @@ static rl_getc_func_t *tui_old_rl_getc_function; static rl_voidfunc_t *tui_old_rl_redisplay_function; static rl_vintfunc_t *tui_old_rl_prep_terminal; static rl_voidfunc_t *tui_old_rl_deprep_terminal; +static rl_compdisp_func_t *tui_old_rl_display_matches_hook; static int tui_old_rl_echoing_p; /* Readline output stream. @@ -442,6 +443,7 @@ tui_setup_io (int mode) tui_old_rl_deprep_terminal = rl_deprep_term_function; tui_old_rl_prep_terminal = rl_prep_term_function; tui_old_rl_getc_function = rl_getc_function; + tui_old_rl_display_matches_hook = rl_completion_display_matches_hook; tui_old_rl_outstream = rl_outstream; tui_old_rl_echoing_p = _rl_echoing_p; rl_redisplay_function = tui_redisplay_readline; @@ -485,8 +487,8 @@ tui_setup_io (int mode) rl_deprep_term_function = tui_old_rl_deprep_terminal; rl_prep_term_function = tui_old_rl_prep_terminal; rl_getc_function = tui_old_rl_getc_function; + rl_completion_display_matches_hook = tui_old_rl_display_matches_hook; rl_outstream = tui_old_rl_outstream; - rl_completion_display_matches_hook = 0; _rl_echoing_p = tui_old_rl_echoing_p; rl_already_prompted = 0; ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3 v2] Implement completion limiting 2015-01-25 15:55 ` Doug Evans @ 2015-01-25 19:14 ` Eli Zaretskii 0 siblings, 0 replies; 29+ messages in thread From: Eli Zaretskii @ 2015-01-25 19:14 UTC (permalink / raw) To: Doug Evans; +Cc: gbenson, gdb-patches > From: Doug Evans <xdje42@gmail.com> > Cc: gbenson@redhat.com, gdb-patches@sourceware.org > Date: Sat, 24 Jan 2015 17:11:24 -0800 > > +@item set max-completions @var{limit} > +@itemx set max-completions unlimited > +Set the maximum number of candidates that @value{GDBN} will collect > +and show during completion. I'd prefer saying this more explicitly, for example: Set the maximum number of completion candidates. @value{GDBN} will stop looking for more completions once it collects this many candidates. WDYT? ^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2015-01-25 16:17 UTC | newest] Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-11-06 10:50 [PATCH 0/3 v2] Limit tab-completion result when list is large Gary Benson 2014-11-06 10:51 ` [PATCH 1/3 v2] Add expansion_notify callback to expand_symtabs_matching Gary Benson 2014-11-06 10:51 ` [PATCH 3/3 v2] Implement completion limiting Gary Benson 2014-11-06 16:28 ` Eli Zaretskii 2014-11-21 10:46 ` Gary Benson 2014-11-24 6:09 ` Doug Evans 2014-11-06 10:51 ` [PATCH 2/3 v2] Interleave completion list building with symbol table expansion Gary Benson 2014-11-27 13:16 [PATCH 0/3 v2] Abort completion when list becomes large Gary Benson 2014-11-27 13:16 ` [PATCH 3/3 v2] Implement completion limiting Gary Benson 2014-11-27 16:25 ` Eli Zaretskii 2014-12-05 23:54 ` Doug Evans 2014-12-10 12:22 ` Gary Benson 2014-12-10 16:25 ` Doug Evans 2015-01-03 2:09 ` Doug Evans 2015-01-07 8:44 ` Gary Benson 2015-01-09 1:29 ` Doug Evans 2015-01-10 2:32 ` Doug Evans 2015-01-10 9:23 ` Eli Zaretskii 2015-01-12 18:50 ` Doug Evans 2015-01-15 15:39 ` Gary Benson 2015-01-23 7:32 ` Doug Evans 2015-01-23 10:59 ` Eli Zaretskii 2015-01-23 16:38 ` Doug Evans 2015-01-23 16:49 ` Eli Zaretskii 2015-01-23 20:28 ` Doug Evans 2015-01-24 1:55 ` Eli Zaretskii 2015-01-24 8:50 ` Doug Evans 2015-01-24 14:12 ` Eli Zaretskii 2015-01-25 15:55 ` Doug Evans 2015-01-25 19:14 ` Eli Zaretskii
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).