public inbox for gdb-testers@sourceware.org
help / color / mirror / Atom feed
* [binutils-gdb] gdb: Remove C++ symbol aliases from completion list
@ 2020-04-03 17:03 gdb-buildbot
  2020-04-03 17:03 ` Failures on Fedora-x86_64-cc-with-index, branch master gdb-buildbot
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: gdb-buildbot @ 2020-04-03 17:03 UTC (permalink / raw)
  To: gdb-testers

*** TEST RESULTS FOR COMMIT 19a2740f7f2ea0f65745a3c00cf8a64647378aa3 ***

commit 19a2740f7f2ea0f65745a3c00cf8a64647378aa3
Author:     Andrew Burgess <andrew.burgess@embecosm.com>
AuthorDate: Fri Dec 27 18:31:09 2019 +0000
Commit:     Andrew Burgess <andrew.burgess@embecosm.com>
CommitDate: Thu Mar 19 08:23:30 2020 +0000

    gdb: Remove C++ symbol aliases from completion list
    
    Consider debugging the following C++ program:
    
      struct object
      { int a; };
    
      typedef object *object_p;
    
      static int
      get_value (object_p obj)
      {
        return obj->a;
      }
    
      int
      main ()
      {
        object obj;
        obj.a = 0;
    
        return get_value (&obj);
      }
    
    Now in a GDB session:
    
      (gdb) complete break get_value
      break get_value(object*)
      break get_value(object_p)
    
    Or:
    
      (gdb) break get_va<TAB>
      (gdb) break get_value(object<RETURN>
      Function "get_value(object" not defined.
      Make breakpoint pending on future shared library load? (y or [n]) n
    
    The reason this happens is that we add completions based on the
    msymbol names and on the symbol names.  For C++ both of these names
    include the parameter list, however, the msymbol names have some
    differences from the symbol names, for example:
    
      + typedefs are resolved,
      + whitespace rules are different around pointers,
      + the 'const' keyword is placed differently.
    
    What this means is that the msymbol names and symbol names appear to
    be completely different to GDB's completion tracker, and therefore to
    readline when it offers the completions.
    
    This commit builds on the previous commit which reworked the
    completion_tracker class.  It is now trivial to add a
    remove_completion member function, this is then used along with
    cp_canonicalize_string_no_typedefs to remove the msymbol aliases from
    the completion tracker as we add the symbol names.
    
    Now, for the above program GDB only presents a single completion for
    'get_value', which is 'get_value(object_p)'.
    
    It is still possible to reference the symbol using the msymbol name,
    so a user can manually type out 'break get_value (object *)' if they
    wish and will get the expected behaviour.
    
    I did consider adding an option to make this alias exclusion optional,
    in the end I didn't bother as I didn't think it would be very useful,
    but I can easily add such an option if people think it would be
    useful.
    
    gdb/ChangeLog:
    
            * completer.c (completion_tracker::remove_completion): Define new
            function.
            * completer.h (completion_tracker::remove_completion): Declare new
            function.
            * symtab.c (completion_list_add_symbol): Remove aliasing msymbols
            when adding a C++ function symbol.
    
    gdb/testsuite/ChangeLog:
    
            * gdb.linespec/cp-completion-aliases.cc: New file.
            * gdb.linespec/cp-completion-aliases.exp: New file.
    
    Change-Id: Ie5c7c9fc8ecf973072cfb4a9650867104bf7f50c

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index bc927d4be4..1fe2e588ad 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+2020-03-19  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* completer.c (completion_tracker::remove_completion): Define new
+	function.
+	* completer.h (completion_tracker::remove_completion): Declare new
+	function.
+	* symtab.c (completion_list_add_symbol): Remove aliasing msymbols
+	when adding a C++ function symbol.
+
 2020-03-19  Andrew Burgess  <andrew.burgess@embecosm.com>
 
 	* completer.c (completion_tracker::completion_hash_entry): Define
diff --git a/gdb/completer.c b/gdb/completer.c
index 14c7a57997..67dce30fbe 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -1678,6 +1678,20 @@ completion_tracker::add_completions (completion_list &&list)
     add_completion (std::move (candidate));
 }
 
+/* See completer.h.  */
+
+void
+completion_tracker::remove_completion (const char *name)
+{
+  hashval_t hash = htab_hash_string (name);
+  if (htab_find_slot_with_hash (m_entries_hash, name, hash, NO_INSERT)
+      != NULL)
+    {
+      htab_remove_elt_with_hash (m_entries_hash, name, hash);
+      m_lowest_common_denominator_valid = false;
+    }
+}
+
 /* Helper for the make_completion_match_str overloads.  Returns NULL
    as an indication that we want MATCH_NAME exactly.  It is up to the
    caller to xstrdup that string if desired.  */
diff --git a/gdb/completer.h b/gdb/completer.h
index 7bfe0d5814..fd0d47b206 100644
--- a/gdb/completer.h
+++ b/gdb/completer.h
@@ -326,6 +326,10 @@ public:
      LIST.  */
   void add_completions (completion_list &&list);
 
+  /* Remove completion matching NAME from the completion list, does nothing
+     if NAME is not already in the completion list.  */
+  void remove_completion (const char *name);
+
   /* Set the quote char to be appended after a unique completion is
      added to the input line.  Set to '\0' to clear.  See
      m_quote_char's description.  */
diff --git a/gdb/symtab.c b/gdb/symtab.c
index aa415a9399..f300d759e0 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -5296,6 +5296,27 @@ completion_list_add_symbol (completion_tracker &tracker,
   completion_list_add_name (tracker, sym->language (),
 			    sym->natural_name (),
 			    lookup_name, text, word);
+
+  /* C++ function symbols include the parameters within both the msymbol
+     name and the symbol name.  The problem is that the msymbol name will
+     describe the parameters in the most basic way, with typedefs stripped
+     out, while the symbol name will represent the types as they appear in
+     the program.  This means we will see duplicate entries in the
+     completion tracker.  The following converts the symbol name back to
+     the msymbol name and removes the msymbol name from the completion
+     tracker.  */
+  if (sym->language () == language_cplus
+      && SYMBOL_DOMAIN (sym) == VAR_DOMAIN
+      && SYMBOL_CLASS (sym) == LOC_BLOCK)
+    {
+      /* The call to canonicalize returns the empty string if the input
+	 string is already in canonical form, thanks to this we don't
+	 remove the symbol we just added above.  */
+      std::string str
+	= cp_canonicalize_string_no_typedefs (sym->natural_name ());
+      if (!str.empty ())
+	tracker.remove_completion (str.c_str ());
+    }
 }
 
 /* completion_list_add_name wrapper for struct minimal_symbol.  */
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 0588fe6ac1..6574270e4d 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2020-03-19  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* gdb.linespec/cp-completion-aliases.cc: New file.
+	* gdb.linespec/cp-completion-aliases.exp: New file.
+
 2020-03-19  Tom de Vries  <tdevries@suse.de>
 
 	* gdb.opt/inline-locals.exp: Add kfail PR number.  Make kfail matching
diff --git a/gdb/testsuite/gdb.linespec/cp-completion-aliases.cc b/gdb/testsuite/gdb.linespec/cp-completion-aliases.cc
new file mode 100644
index 0000000000..5f2fb5c663
--- /dev/null
+++ b/gdb/testsuite/gdb.linespec/cp-completion-aliases.cc
@@ -0,0 +1,73 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019-2020 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <cstring>
+
+template<typename T>
+struct magic
+{
+  T x;
+};
+
+struct object
+{
+  int a;
+};
+
+typedef magic<int> int_magic_t;
+
+typedef object *object_p;
+
+typedef const char *my_string_t;
+
+static int
+get_value (object_p obj)
+{
+  return obj->a;
+}
+
+static int
+get_something (object_p obj)
+{
+  return obj->a;
+}
+
+static int
+get_something (my_string_t msg)
+{
+  return strlen (msg);
+}
+
+static int
+grab_it (int_magic_t *var)
+{
+  return var->x;
+}
+
+int
+main ()
+{
+  magic<int> m;
+  m.x = 4;
+
+  object obj;
+  obj.a = 0;
+
+  int val = (get_value (&obj) + get_something (&obj)
+	     + get_something ("abc") + grab_it (&m));
+  return val;
+}
diff --git a/gdb/testsuite/gdb.linespec/cp-completion-aliases.exp b/gdb/testsuite/gdb.linespec/cp-completion-aliases.exp
new file mode 100644
index 0000000000..313ff844b3
--- /dev/null
+++ b/gdb/testsuite/gdb.linespec/cp-completion-aliases.exp
@@ -0,0 +1,54 @@
+# Copyright 2019-2020 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file tests GDB's ability to remove symbol aliases from the
+# completion list in C++.
+
+load_lib completion-support.exp
+
+standard_testfile .cc
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug}]} {
+    return -1
+}
+
+# Tests below are about tab-completion, which doesn't work if readline
+# library isn't used.  Check it first.
+
+if { ![readline_is_used] } {
+    untested "no tab completion support without readline"
+    return -1
+}
+
+# Disable the completion limit for the whole testcase.
+gdb_test_no_output "set max-completions unlimited"
+
+test_gdb_complete_tab_unique "break get_v" \
+    "break get_value\\(object_p\\)" " "
+
+test_gdb_complete_cmd_unique "break get_v" \
+    "break get_value\\(object_p\\)"
+
+test_gdb_complete_tab_unique "break gr" \
+    "break grab_it\\(int_magic_t\\*\\)" " "
+
+test_gdb_complete_cmd_unique "break gr" \
+    "break grab_it\\(int_magic_t\\*\\)"
+
+test_gdb_complete_tab_multiple "break get_som" "ething(" \
+    { "get_something(my_string_t)" "get_something(object_p)" }
+
+test_gdb_complete_cmd_multiple "break " "get_som" \
+    { "get_something(my_string_t)" "get_something(object_p)" }


^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-04-08 12:01 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-03 17:03 [binutils-gdb] gdb: Remove C++ symbol aliases from completion list gdb-buildbot
2020-04-03 17:03 ` Failures on Fedora-x86_64-cc-with-index, branch master gdb-buildbot
2020-04-03 17:08 ` Failures on Fedora-i686, " gdb-buildbot
2020-04-03 17:33 ` Failures on Fedora-x86_64-m32, " gdb-buildbot
2020-04-03 17:55 ` Failures on Fedora-x86_64-m64, " gdb-buildbot
2020-04-03 18:15 ` Failures on Fedora-x86_64-native-extended-gdbserver-m32, " gdb-buildbot
2020-04-03 18:51 ` Failures on Fedora-x86_64-native-extended-gdbserver-m64, " gdb-buildbot
2020-04-07  1:55 ` Failures on Fedora-x86_64-native-gdbserver-m32, " gdb-buildbot
2020-04-07 14:23 ` Failures on Fedora-x86_64-native-gdbserver-m64, " gdb-buildbot
2020-04-08 12:01 ` Failures on Ubuntu-Aarch64-native-extended-gdbserver-m64, " gdb-buildbot

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).