[pushed] Don't remove C++ aliases from completions if symbol doesn't match (Re: [PATCHv2 3/3] gdb: Remove C++ symbol aliases from completion list)

[Pedro Alves]

On 5/24/20 12:35 PM, Pedro Alves via Gdb-patches wrote:

> This causes GDB to crash for me, when debugging GDB itself, and then
> doing "b main<TAB>".

...

> GDB seemingly crashes due to infinite recursion.  

> 
> The symbol on which we call cp_canonicalize_string_no_typedefs is:
> 

> (top-gdb) p sym->m_name 
> $6 = 0x980b2c0 "gdb::option::boolean_option_def<filename_partial_match_opts>::boolean_option_def(char const*, bool* (*)(filename_partial_match_opts*), void (*)(ui_file*, in t, cmd_list_element*, char const*), char const*, char const*, char const*)"

Note that this symbol doesn't match "main", so trying to remove
its aliases is pointless work.  I can't do performance testing,
because GDB just crashes for me, but I'd think this caused a
performance regression.

I'm applying this patch, which makes GDB not crash when completing "main",
though of course it still crashes with "complete b gdb::option::", 
so we need to fix that.

The new testcase gdb.linespec/cp-completion-aliases.exp passes with this
patch, though I had to force-disable the completion styling, which itself
broke the testcase...

>From e08bd6c5081f4957ddb60117ac94775dcd618db7 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Sun, 24 May 2020 13:32:25 +0100
Subject: [PATCH] Don't remove C++ aliases from completions if symbol doesn't
 match

completion_list_add_symbol currently tries to remove C++ function
aliases from the completions match list even if the symbol passed down
wasn't successfully added to the completion list because it didn't
match.  I.e., we call cp_canonicalize_string_no_typedefs for each and
every C++ function in the program, which is useful work.  This patch
skips that useless work.

gdb/ChangeLog:
2020-05-24  Pedro Alves  <palves@redhat.com>

	* symtab.c (completion_list_add_name): Return boolean indication
	of whether the symbol matched.
	(completion_list_add_symbol): Don't try to remove C++ aliases if
	the symbol didn't match in the first place.
	* symtab.h (completion_list_add_name): Return bool.
---
 gdb/ChangeLog |  8 ++++++++
 gdb/symtab.c  | 13 ++++++++-----
 gdb/symtab.h  |  5 +++--
 3 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 35fdc614ee4..9f65e2df970 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2020-05-24  Pedro Alves  <palves@redhat.com>
+
+	* symtab.c (completion_list_add_name): Return boolean indication
+	of whether the symbol matched.
+	(completion_list_add_symbol): Don't try to remove C++ aliases if
+	the symbol didn't match in the first place.
+	* symtab.h (completion_list_add_name): Return bool.
+
 2020-05-23  Simon Marchi  <simon.marchi@polymtl.ca>
 
 	* gdbtypes.h (TYPE_FIELD): Remove.  Replace all uses with
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 3f90ea99647..5c4e282c024 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -5260,7 +5260,7 @@ compare_symbol_name (const char *symbol_name, language symbol_language,
 
 /*  See symtab.h.  */
 
-void
+bool
 completion_list_add_name (completion_tracker &tracker,
 			  language symbol_language,
 			  const char *symname,
@@ -5272,7 +5272,7 @@ completion_list_add_name (completion_tracker &tracker,
 
   /* Clip symbols that cannot match.  */
   if (!compare_symbol_name (symname, symbol_language, lookup_name, match_res))
-    return;
+    return false;
 
   /* Refresh SYMNAME from the match string.  It's potentially
      different depending on language.  (E.g., on Ada, the match may be
@@ -5296,6 +5296,8 @@ completion_list_add_name (completion_tracker &tracker,
     tracker.add_completion (std::move (completion),
 			    &match_res.match_for_lcd, text, word);
   }
+
+  return true;
 }
 
 /* completion_list_add_name wrapper for struct symbol.  */
@@ -5306,9 +5308,10 @@ completion_list_add_symbol (completion_tracker &tracker,
 			    const lookup_name_info &lookup_name,
 			    const char *text, const char *word)
 {
-  completion_list_add_name (tracker, sym->language (),
-			    sym->natural_name (),
-			    lookup_name, text, word);
+  if (!completion_list_add_name (tracker, sym->language (),
+				 sym->natural_name (),
+				 lookup_name, text, word))
+    return;
 
   /* C++ function symbols include the parameters within both the msymbol
      name and the symbol name.  The problem is that the msymbol name will
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 05e6a311b87..9972e8125ba 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -2332,8 +2332,9 @@ const char *
 /* Test to see if the symbol of language SYMBOL_LANGUAGE 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.  */
-void completion_list_add_name (completion_tracker &tracker,
+   the current completion list and return true.  Otherwise, return
+   false.  */
+bool completion_list_add_name (completion_tracker &tracker,
 			       language symbol_language,
 			       const char *symname,
 			       const lookup_name_info &lookup_name,

base-commit: bb68f22c8e648032a0d1c1d17353eec599ff5e6a
-- 
2.14.5