From patchwork Thu Aug 6 19:15:15 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Keith Seitz X-Patchwork-Id: 8059 Received: (qmail 24183 invoked by alias); 6 Aug 2015 19:58:23 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 24112 invoked by uid 89); 6 Aug 2015 19:58:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.9 required=5.0 tests=AWL, BAYES_50, KAM_LAZY_DOMAIN_SECURITY, KAM_STOCKGEN, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 06 Aug 2015 19:58:11 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id D4458A145B for ; Thu, 6 Aug 2015 19:58:09 +0000 (UTC) Received: from valrhona.uglyboxes.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t76JFFcP012983 for ; Thu, 6 Aug 2015 15:15:15 -0400 Subject: [PATCH v3 02/19] Remove completion_tracker_t from the public completion API. From: Keith Seitz To: gdb-patches@sourceware.org Date: Thu, 06 Aug 2015 12:15:15 -0700 Message-ID: <20150806191455.32159.34003.stgit@valrhona.uglyboxes.com> In-Reply-To: <20150806191404.32159.50755.stgit@valrhona.uglyboxes.com> References: <20150806191404.32159.50755.stgit@valrhona.uglyboxes.com> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 X-IsSubscribed: yes Differences from last version: 1. Removed comments about recording an additional completion from add_completion. 2. Removed requirement to xstrdup arg to add_completion; add_completeion uses new function completer_strdup. 3. Consolidated (oft-repeated) partial copy code into completer_strdup. Removed this code from all completion functions. --- This patch removes the recently introduced "completion tracker" concept from the public completion API. In the previous patch, the completion tracker was added to the (new) completer_data structure that is now used by all completion functions. Since completion_tracker_t is now used entirely inside completer.c, there is no need to make its type opaque, so the typedef has been removed. This patch also fixes gdb/17960, which is easily demonstrated: $ gdb -nx -q gdb (gdb) b gdb.c:ma ./../src/gdb/completer.c:837: internal-error: maybe_add_completion: Assertion `tracker != NULL' failed. A problem internal to GDB has been detected, further debugging may prove unreliable. Quit this debugging session? (y or n) This occurs because the code path in this case is: complete_line - complete_line_internal -- location_completer gdb/ChangeLog PR gdb/17960 * completer.c (struct completer_data) : Change type to htab_t. (enum maybe_add_completion_enum): Make private; moved from completer.h. [DEFAULT_MAX_COMPLETIONS]: Define. (new_completion_tracker): Remove function. (new_completer_data): New function. (free_completion_tracker): Remove function. (free_completer_data): New function. (make_cleanup_free_completion_tracker): Remove function. (maybe_add_completion): Make static. Update comments. Use completer_data instead of completion_tracker_t. (completer_strdup): New function. (add_completion): New function. (complete_line, gdb_completion_word_break_characters): Use completer_data instead of completion_tracker. * completer.h (completion_tracker_t, new_completion_tracker) (make_cleanup_free_completion_tracker, maybe_add_completion) (enum maybe_add_completion): Remove declarations. (enum add_completion_status): Define. (add_completion): Declare. * symtab.c (completion_tracker): Remove variable. (completion_list_add_name): Use add_completion instead of maybe_add_completion and partial copy. (default_make_symbol_completion_list_break_on_1): Do not use completion_tracker. gdb/testsuite/ChangeLog PR gdb/17960 * gdb.base/completion.exp: Add some basic tests for the location completer, including a regression test for the gdb/17960 assertion failure. --- gdb/completer.c | 231 +++++++++++++++++++++++---------- gdb/completer.h | 66 +++------ gdb/symtab.c | 62 +-------- gdb/testsuite/gdb.base/completion.exp | 78 +++++++++++ 4 files changed, 260 insertions(+), 177 deletions(-) diff --git a/gdb/completer.c b/gdb/completer.c index 476f9f2..bf4137e 100644 --- a/gdb/completer.c +++ b/gdb/completer.c @@ -92,10 +92,31 @@ static char *gdb_completer_quote_characters = "'"; struct completer_data { - /* The completion tracker being used by the completer. */ - completion_tracker_t tracker; + /* A hashtable used to track completions. */ + htab_t tracker; }; +/* 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 +}; /* Accessor for some completer data that may interest other files. */ @@ -803,48 +824,41 @@ complete_line_internal (struct completer_data *cdata, /* See completer.h. */ -int max_completions = 200; +#define DEFAULT_MAX_COMPLETIONS 200 +int max_completions = DEFAULT_MAX_COMPLETIONS; -/* See completer.h. */ +/* Allocate a new completer data structure. */ -completion_tracker_t -new_completion_tracker (void) +static struct completer_data * +new_completer_data (int size) { - if (max_completions <= 0) - return NULL; + struct completer_data *cdata = XCNEW (struct completer_data); - return htab_create_alloc (max_completions, - htab_hash_string, (htab_eq) streq, - NULL, xcalloc, xfree); + cdata->tracker + = htab_create_alloc ((size < 0 ? DEFAULT_MAX_COMPLETIONS : size), + htab_hash_string, (htab_eq) streq, NULL, + xcalloc, xfree); + return cdata; } -/* Cleanup routine to free a completion tracker and reset the pointer - to NULL. */ +/* Free the completion data represented by P. */ static void -free_completion_tracker (void *p) +free_completer_data (void *p) { - completion_tracker_t *tracker_ptr = p; + struct completer_data *cdata = p; - htab_delete (*tracker_ptr); - *tracker_ptr = NULL; + htab_delete (cdata->tracker); + xfree (cdata); } -/* 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. */ +/* 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. */ -enum maybe_add_completion_enum -maybe_add_completion (completion_tracker_t tracker, char *name) +static enum maybe_add_completion_enum +maybe_add_completion (struct completer_data *cdata, char *name) { void **slot; @@ -853,23 +867,82 @@ maybe_add_completion (completion_tracker_t tracker, char *name) if (max_completions == 0) return MAYBE_ADD_COMPLETION_MAX_REACHED; - gdb_assert (tracker != NULL); - - if (htab_elements (tracker) >= max_completions) + if (htab_elements (cdata->tracker) >= max_completions) return MAYBE_ADD_COMPLETION_MAX_REACHED; - slot = htab_find_slot (tracker, name, INSERT); + slot = htab_find_slot (cdata->tracker, name, INSERT); if (*slot != HTAB_EMPTY_ENTRY) return MAYBE_ADD_COMPLETION_DUPLICATE; *slot = name; - return (htab_elements (tracker) < max_completions + return (htab_elements (cdata->tracker) < max_completions ? MAYBE_ADD_COMPLETION_OK : MAYBE_ADD_COMPLETION_OK_MAX_REACHED); } +/* A strdup-like function used by the completer to copy bits of completion + strings. */ + +static char * +completer_strdup (const char *match, const char *text, const char *word) +{ + char *alloc; + + if (text == NULL || word == NULL || text == word) + return xstrdup (match); + + if (word > text) + { + gdb_assert (strlen (match) > (word - text)); + + /* Return some portion of match. */ + match += (word - text); + alloc = xmalloc (strlen (match) + 1); + strcpy (alloc, match); + } + else + { + /* Return some of text plus match. */ + alloc = xmalloc (strlen (match) + (text - word) + 1); + strncpy (alloc, word, text - word); + alloc[text - word] = '\0'; + strcat (alloc, match); + } + + return alloc; +} + +/* See completer.h. */ + +enum add_completion_status +add_completion (struct completer_data *cdata, VEC (char_ptr) **result, + const char *match, const char *text, const char *word) +{ + enum maybe_add_completion_enum add_status; + char *alloc = completer_strdup (match, text, word); + + add_status = maybe_add_completion (cdata, alloc); + switch (add_status) + { + case MAYBE_ADD_COMPLETION_OK: + VEC_safe_push (char_ptr, *result, alloc); + break; + case MAYBE_ADD_COMPLETION_OK_MAX_REACHED: + VEC_safe_push (char_ptr, *result, alloc); + return ADD_COMPLETION_MAX_REACHED; + case MAYBE_ADD_COMPLETION_MAX_REACHED: + xfree (alloc); + return ADD_COMPLETION_MAX_REACHED; + case MAYBE_ADD_COMPLETION_DUPLICATE: + xfree (alloc); + break; + } + + return ADD_COMPLETION_OK; +} + void throw_max_completions_reached_error (void) { @@ -894,54 +967,63 @@ complete_line (const char *text, const char *line_buffer, int point) { VEC (char_ptr) *list; VEC (char_ptr) *result = NULL; - struct cleanup *cleanups; - completion_tracker_t tracker; + struct cleanup *cdata_cleanup, *list_cleanup; char *candidate; - int ix, max_reached; + int ix; + struct completer_data *cdata; if (max_completions == 0) return NULL; - list = complete_line_internal (NULL, text, line_buffer, point, + + cdata = new_completer_data (max_completions); + cdata_cleanup = make_cleanup (free_completer_data, cdata); + list = complete_line_internal (cdata, 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; + do_cleanups (cdata_cleanup); + return list; + } + + list_cleanup = make_cleanup_free_char_ptr_vec (list); - add_status = maybe_add_completion (tracker, candidate); + /* If complete_line_internal returned more completions than were + recorded by the completion tracker, then the completer function that + was run does not support completion tracking. In this case, + do a final test for too many completions. - switch (add_status) + 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. */ + + if (VEC_length (char_ptr, list) > htab_elements (cdata->tracker)) + { + enum add_completion_status max_reached = ADD_COMPLETION_OK; + + /* Clear the tracker so that we can re-use it to count the number + of returned completions. */ + htab_empty (cdata->tracker); + + for (ix = 0; (max_reached == ADD_COMPLETION_OK + && VEC_iterate (char_ptr, list, ix, candidate)); ++ix) { - 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; + max_reached = add_completion (cdata, &result, candidate, NULL, NULL); } + + /* The return result has been assembled and the original list from + complete_line_internal is no longer needed. Free it. */ + do_cleanups (list_cleanup); + } + else + { + /* There is a valid tracker for the completion -- simply return + the completed list. */ + discard_cleanups (list_cleanup); + result = list; } - do_cleanups (cleanups); + do_cleanups (cdata_cleanup); return result; } @@ -1074,9 +1156,14 @@ char * gdb_completion_word_break_characters (void) { VEC (char_ptr) *list; + struct completer_data *cdata; + struct cleanup *cleanup; - list = complete_line_internal (NULL, rl_line_buffer, rl_line_buffer, + cdata = new_completer_data (max_completions); + cleanup = make_cleanup (free_completer_data, cdata); + list = complete_line_internal (cdata, rl_line_buffer, rl_line_buffer, rl_point, handle_brkchars); + do_cleanups (cleanup); gdb_assert (list == NULL); return rl_completer_word_break_characters; } diff --git a/gdb/completer.h b/gdb/completer.h index 7139c0a..43d1321 100644 --- a/gdb/completer.h +++ b/gdb/completer.h @@ -132,59 +132,31 @@ extern const char *skip_quoted (const char *); extern int max_completions; -/* Object to track how many unique completions have been generated. - Used to limit the size of generated completion lists. */ +/* Return values for add_completion. */ -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 +enum add_completion_status { - /* 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 + /* Completion was added -- keep looking for more. */ + ADD_COMPLETION_OK, + + /* Maximum number of completions has been reached or the maximum + has already been reached by a previous call to add_completion. + Callers can make no assumptions about whether the completion was + added or not. */ + ADD_COMPLETION_MAX_REACHED }; -/* 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. +/* Add the given MATCH to the completer result in CDATA. + Returns one of the above enum add_completion_status codes to indicate + whether the caller should continue to look for/compute more completions. - 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. */ + If TEXT and WORD are non-NULL, then portions of the strings will be + saved into the completion list. See completer_strdup for more. */ -extern enum maybe_add_completion_enum - maybe_add_completion (completion_tracker_t tracker, char *name); +extern enum add_completion_status + add_completion (struct completer_data *cdata, + VEC (char_ptr) **result, const char *match, + const char *text, const char *word); /* Wrapper to throw MAX_COMPLETIONS_REACHED_ERROR. */ diff --git a/gdb/symtab.c b/gdb/symtab.c index f4407a0..34eb6d7 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -5046,15 +5046,6 @@ static VEC (char_ptr) *return_val; completion_list_add_name \ (cdata, 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. */ @@ -5070,50 +5061,11 @@ completion_list_add_name (struct completer_data *cdata, return; /* We have a match for a completion, so add SYMNAME to the current list - of matches. Note that the name is moved to freshly malloc'd space. */ - - { - char *newobj; - enum maybe_add_completion_enum add_status; - - if (word == sym_text) - { - newobj = xmalloc (strlen (symname) + 5); - strcpy (newobj, symname); - } - else if (word > sym_text) - { - /* Return some portion of symname. */ - newobj = xmalloc (strlen (symname) + 5); - strcpy (newobj, symname + (word - sym_text)); - } - else - { - /* Return some of SYM_TEXT plus symname. */ - newobj = xmalloc (strlen (symname) + (sym_text - word) + 5); - strncpy (newobj, word, sym_text - word); - newobj[sym_text - word] = '\0'; - strcat (newobj, symname); - } - - add_status = maybe_add_completion (completion_tracker, newobj); + of matches. */ - switch (add_status) - { - case MAYBE_ADD_COMPLETION_OK: - VEC_safe_push (char_ptr, return_val, newobj); - break; - case MAYBE_ADD_COMPLETION_OK_MAX_REACHED: - VEC_safe_push (char_ptr, return_val, newobj); - throw_max_completions_reached_error (); - case MAYBE_ADD_COMPLETION_MAX_REACHED: - xfree (newobj); - throw_max_completions_reached_error (); - case MAYBE_ADD_COMPLETION_DUPLICATE: - xfree (newobj); - break; - } - } + if (add_completion (cdata, &return_val, symname, sym_text, word) + == ADD_COMPLETION_MAX_REACHED) + throw_max_completions_reached_error (); } /* ObjC: In case we are completing on a selector, look as the msymbol @@ -5358,7 +5310,6 @@ default_make_symbol_completion_list_break_on_1 (struct completer_data *cdata, /* Length of sym_text. */ int sym_text_len; struct add_name_data datum; - struct cleanup *cleanups; /* Now look for the symbol we are supposed to complete on. */ { @@ -5429,9 +5380,6 @@ default_make_symbol_completion_list_break_on_1 (struct completer_data *cdata, } gdb_assert (sym_text[sym_text_len] == '\0' || sym_text[sym_text_len] == '('); - 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; datum.text = text; @@ -5549,8 +5497,6 @@ default_make_symbol_completion_list_break_on_1 (struct completer_data *cdata, /* User-defined macros are always visible. */ macro_for_each (macro_user_macros, add_macro_name, &datum); } - - do_cleanups (cleanups); } VEC (char_ptr) * diff --git a/gdb/testsuite/gdb.base/completion.exp b/gdb/testsuite/gdb.base/completion.exp index 1eb0fd8..710aac0 100644 --- a/gdb/testsuite/gdb.base/completion.exp +++ b/gdb/testsuite/gdb.base/completion.exp @@ -777,6 +777,84 @@ gdb_test_multiple "" "$test" { } # +# Tests for the location completer +# + +# Turn off pending breakpoint support so that we don't get queried +# all the time. +gdb_test_no_output "set breakpoint pending off" + +set subsrc [string range $srcfile 0 [expr {[string length $srcfile] - 3}]] +set test "tab complete break $subsrc" +send_gdb "break $subsrc\t\t" +gdb_test_multiple "" $test { + -re "break\.c.*break1\.c.*$gdb_prompt " { + send_gdb "1\t\n" + gdb_test_multiple "" $test { + -re ".*Function \"$srcfile2\" not defined\..*$gdb_prompt " { + pass $test + } + -re "$gdb_prompt p$" { + fail $test + } + } + } + + -re "$gdb_prompt p$" { + fail $test + } +} + +gdb_test "complete break $subsrc" "break\.c.*break1\.c" + +# gdb/17960 +set test "tab complete break $srcfile:ma" +send_gdb "break $srcfile:ma\t" +gdb_test_multiple "" $test { + -re "break $srcfile:main " { + send_gdb "\n" + gdb_test_multiple "" $test { + -re ".*Breakpoint.*at .*/$srcfile, line .*$gdb_prompt " { + pass $test + gdb_test_no_output "delete breakpoint \$bpnum" \ + "delete breakpoint for $test" + } + -re "$gdb_prompt p$" { + fail $test + } + } + } + -re "$gdb_prompt p$" { + fail $test + } +} + +gdb_test "complete break $srcfile:ma" "break\.c:main" + +set test "tab complete break need" +send_gdb "break need\t" +gdb_test_multiple "" $test { + -re "break need_malloc " { + send_gdb "\n" + gdb_test_multiple "" $test { + -re ".*Breakpoint.*at .*/$srcfile, line .*$gdb_prompt " { + pass $test + gdb_test_no_output "delete breakpoint \$bpnum" \ + "delete breakpoint for $test" + } + -re "$gdb_prompt p$" { + fail $test + } + } + } + -re "$gdb_prompt p$" { + fail $test + } +} + +gdb_test "complete break need" "need_malloc" + +# # Completion limiting. #