From: Pedro Alves <palves@redhat.com>
To: Andrew Burgess <andrew.burgess@embecosm.com>, gdb-patches@sourceware.org
Subject: [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)
Date: Sun, 24 May 2020 13:34:17 +0100 [thread overview]
Message-ID: <85f1cfd3-6d5e-e14b-c1bf-02c7aea6d6dd@redhat.com> (raw)
In-Reply-To: <ce2a4b99-fff1-7ff5-ef32-a7f568ecc5f9@redhat.com>
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
next prev parent reply other threads:[~2020-05-24 12:34 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-28 0:37 [PATCHv2 0/3] Remove C++ Symbol Aliases From Completion List Andrew Burgess
2019-12-27 21:32 ` [PATCH 0/2] " Andrew Burgess
2019-12-27 21:32 ` [PATCH 1/2] gdb: Convert completion tracker to use std types Andrew Burgess
2020-01-24 19:18 ` Tom Tromey
2020-01-24 19:47 ` Christian Biesinger via gdb-patches
2020-01-26 16:01 ` Tom Tromey
2019-12-27 21:32 ` [PATCH 2/2] gdb: Remove C++ symbol aliases from completion list Andrew Burgess
2020-01-24 19:08 ` Tom Tromey
2020-01-28 0:37 ` [PATCHv2 3/3] " Andrew Burgess
2020-05-24 11:35 ` Pedro Alves
2020-05-24 12:34 ` Pedro Alves [this message]
2020-05-25 14:34 ` GDB crash due to infinite recursion in typedef substitution (Re: [PATCHv2 3/3] gdb: Remove C++ symbol aliases from completion list) Pedro Alves
2020-05-26 17:02 ` Andrew Burgess
2020-05-26 18:09 ` Pedro Alves
2020-05-26 21:17 ` Keith Seitz
2020-05-27 19:36 ` Pedro Alves
2020-05-28 13:40 ` Pedro Alves
2020-01-28 0:37 ` [PATCHv2 2/3] gdb: Restructure the completion_tracker class Andrew Burgess
2020-04-03 23:00 ` Luis Machado
2020-04-04 15:37 ` [PATCH] gdb: Don't corrupt completions hash when expanding the hash table Andrew Burgess
2020-04-06 20:27 ` Tom Tromey
2020-04-15 15:46 ` Andrew Burgess
2020-01-28 0:50 ` [PATCHv2 1/3] libiberty/hashtab: More const parameters Andrew Burgess
2020-02-25 17:35 ` Andrew Burgess
2020-03-12 10:33 ` [PATCHv2 0/3] Remove C++ Symbol Aliases From Completion List Andrew Burgess
2020-03-12 19:17 ` Tom Tromey
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=85f1cfd3-6d5e-e14b-c1bf-02c7aea6d6dd@redhat.com \
--to=palves@redhat.com \
--cc=andrew.burgess@embecosm.com \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).