This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v2 2/3] Make "break foo" find "A::foo", A::B::foo", etc. [C++ and wild matching]


On 11/27/2017 05:13 PM, Pedro Alves wrote:
> In v2:
> 
>   - Implements Keith's suggestion of making "-qualified" a flag
>     option, instead of being a replacement for "-function".  This
>     means that "break -q filename.cc:function" interprets
>     "filename.cc:function" as a linespec with two components instead
>     of a (bogus) function name.  Basically, this folds in these
>     changes (with some additional cleanup here and there):
>      https://sourceware.org/ml/gdb-patches/2017-11/msg00621.html
>      https://sourceware.org/ml/gdb-patches/2017-11/msg00618.html

Rereading this, I realized that the "-qualified" options wasn't being saved
by "save breakpoints".  There were a couple problems.  First,
linespec.c:canonicalize_linespec was losing the symbol_name_match_type.
While addressing this, I realized that we don't really need to add
a new field to ls_parser to record the desired symbol_name_match_type,
since we can just use the one in PARSER_EXPLICIT.
The second is that I missed updating the "-qualified" handling in
explicit_to_string_internal to take into account the fact that
"-qualified" now appears with traditional linespecs as well.

Below's what I'm folding in to the patch, to address this.  New
testcase included.

>From 35d4e973c62fd51ad8554286aa2d984d63761b6c Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Mon, 27 Nov 2017 23:48:33 +0000
Subject: [PATCH] save-breakpoints

---
 gdb/linespec.c                             | 22 +++++----
 gdb/location.c                             |  9 ++--
 gdb/testsuite/gdb.cp/save-bp-qualified.cc  | 40 ++++++++++++++++
 gdb/testsuite/gdb.cp/save-bp-qualified.exp | 74 ++++++++++++++++++++++++++++++
 gdb/testsuite/lib/gdb.exp                  |  6 ++-
 5 files changed, 134 insertions(+), 17 deletions(-)
 create mode 100644 gdb/testsuite/gdb.cp/save-bp-qualified.cc
 create mode 100644 gdb/testsuite/gdb.cp/save-bp-qualified.exp

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 83600bb..c3e616f 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -302,9 +302,6 @@ struct ls_parser
   struct linespec result;
 #define PARSER_RESULT(PPTR) (&(PPTR)->result)
 
-  /* Whether to do full matching or wild matching.  */
-  symbol_name_match_type match_type;
-
   /* What the parser believes the current word point should complete
      to.  */
   linespec_complete_what complete_what;
@@ -1874,10 +1871,12 @@ linespec_parse_basic (linespec_parser *parser)
 	  completion_tracker tmp_tracker;
 	  const char *source_filename
 	    = PARSER_EXPLICIT (parser)->source_filename;
+	  symbol_name_match_type match_type
+	    = PARSER_EXPLICIT (parser)->func_name_match_type;
 
 	  linespec_complete_function (tmp_tracker,
 				      parser->completion_word,
-				      parser->match_type,
+				      match_type,
 				      source_filename);
 
 	  if (tmp_tracker.have_completions ())
@@ -1902,7 +1901,7 @@ linespec_parse_basic (linespec_parser *parser)
   /* Try looking it up as a function/method.  */
   find_linespec_symbols (PARSER_STATE (parser),
 			 PARSER_RESULT (parser)->file_symtabs, name,
-			 parser->match_type,
+			 PARSER_EXPLICIT (parser)->func_name_match_type,
 			 &symbols, &minimal_symbols);
 
   if (symbols != NULL || minimal_symbols != NULL)
@@ -2554,7 +2553,7 @@ parse_linespec (linespec_parser *parser, const char *arg,
   parser->lexer.stream = arg;
   parser->completion_word = arg;
   parser->complete_what = linespec_complete_what::FUNCTION;
-  parser->match_type = match_type;
+  PARSER_EXPLICIT (parser)->func_name_match_type = match_type;
 
   /* Initialize the default symtab and line offset.  */
   initialize_defaults (&PARSER_STATE (parser)->default_symtab,
@@ -2763,6 +2762,8 @@ linespec_parser_new (linespec_parser *parser,
   memset (parser, 0, sizeof (linespec_parser));
   parser->lexer.current.type = LSTOKEN_CONSUMED;
   memset (PARSER_RESULT (parser), 0, sizeof (struct linespec));
+  PARSER_EXPLICIT (parser)->func_name_match_type
+    = symbol_name_match_type::WILD;
   PARSER_EXPLICIT (parser)->line_offset.sign = LINE_OFFSET_UNKNOWN;
   linespec_state_constructor (PARSER_STATE (parser), flags, language,
 			      search_pspace,
@@ -2886,8 +2887,9 @@ complete_linespec_component (linespec_parser *parser,
     {
       completion_list fn_list;
 
-      linespec_complete_function (tracker, text, parser->match_type,
-				  source_filename);
+      symbol_name_match_type match_type
+	= PARSER_EXPLICIT (parser)->func_name_match_type;
+      linespec_complete_function (tracker, text, match_type, source_filename);
       if (source_filename == NULL)
 	{
 	  /* Haven't seen a source component, like in "b
@@ -3002,7 +3004,7 @@ linespec_complete (completion_tracker &tracker, const char *text,
   linespec_parser_new (&parser, 0, current_language, NULL, NULL, 0, NULL);
   cleanup = make_cleanup (linespec_parser_delete, &parser);
   parser.lexer.saved_arg = text;
-  parser.match_type = match_type;
+  PARSER_EXPLICIT (&parser)->func_name_match_type = match_type;
   PARSER_STREAM (&parser) = text;
 
   parser.completion_tracker = &tracker;
@@ -3060,7 +3062,7 @@ linespec_complete (completion_tracker &tracker, const char *text,
       VEC (bound_minimal_symbol_d) *minimal_symbols;
       find_linespec_symbols (PARSER_STATE (&parser),
 			     PARSER_RESULT (&parser)->file_symtabs,
-			     func_name, parser.match_type,
+			     func_name, match_type,
 			     &function_symbols, &minimal_symbols);
 
       PARSER_RESULT (&parser)->function_symbols = function_symbols;
diff --git a/gdb/location.c b/gdb/location.c
index d764f4c..6752462 100644
--- a/gdb/location.c
+++ b/gdb/location.c
@@ -251,13 +251,10 @@ explicit_to_string_internal (int as_linespec,
     {
       if (need_space)
 	buf.putc (space);
+      if (explicit_loc->func_name_match_type == symbol_name_match_type::FULL)
+	buf.puts ("-qualified ");
       if (!as_linespec)
-	{
-	  if (explicit_loc->func_name_match_type
-	      == symbol_name_match_type::FULL)
-	    buf.puts ("-qualified ");
-	  buf.puts ("-function ");
-	}
+	buf.puts ("-function ");
       buf.puts (explicit_loc->function_name);
       need_space = 1;
     }
diff --git a/gdb/testsuite/gdb.cp/save-bp-qualified.cc b/gdb/testsuite/gdb.cp/save-bp-qualified.cc
new file mode 100644
index 0000000..8dc2682
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/save-bp-qualified.cc
@@ -0,0 +1,40 @@
+/* Copyright 2017 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+struct S
+{
+  static void function ();
+};
+
+void
+S::function ()
+{
+}
+
+void
+function ()
+{
+}
+
+int
+main ()
+{
+  S::function ();
+  function ();
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.cp/save-bp-qualified.exp b/gdb/testsuite/gdb.cp/save-bp-qualified.exp
new file mode 100644
index 0000000..8498f24
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/save-bp-qualified.exp
@@ -0,0 +1,74 @@
+# Copyright (C) 2011-2017 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/>.
+
+# Test "save breakpoints" + "break -qualified".
+
+standard_testfile .cc
+
+if { [build_executable "failed to prepare" ${testfile} $srcfile {debug c++}] } {
+    return -1
+}
+
+proc restart {} {
+    global testfile
+
+    clean_restart $testfile
+
+    if ![runto_main] {
+	untested "could not run to main"
+	return 0
+    }
+    # Delete all breakpoints so that the "runto_main" breakpoint above
+    # does not interfere with our testing.
+    delete_breakpoints
+
+    return 1
+}
+
+with_test_prefix "save" {
+    if ![restart] {
+	return -1
+    }
+
+    gdb_breakpoint "function" qualified
+    gdb_breakpoint "function"
+
+    # Save the breakpoints into a file.
+    if {[is_remote host]} {
+	set bps bps
+    } else {
+	set bps [standard_output_file bps]
+    }
+    remote_file host delete "$bps"
+    gdb_test "save breakpoint $bps" "" "save breakpoint bps"
+}
+
+with_test_prefix "restore" {
+    if ![restart] {
+	return -1
+    }
+
+    # Restore the breakpoints.
+    gdb_test "source $bps" "" "source bps"
+
+    # Verify that all breakpoints have been created correctly.
+    gdb_test "info break" [multi_line \
+      "Num +Type +Disp +Enb +Address +What" \
+      "$decimal +breakpoint +keep +y +$hex +in function\\(\\) at \[^\r\n\]*$srcfile:$decimal" \
+      "$decimal +breakpoint +keep +y +<MULTIPLE> +" \
+      "$decimal.$decimal +y +$hex +in S::function\\(\\) at \[^\r\n\]*$srcfile:$decimal" \
+      "$decimal.$decimal +y +$hex +in function\\(\\) at \[^\r\n\]*$srcfile:$decimal" \
+    ]
+}
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 8d6972a..fc0278b 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -422,7 +422,7 @@ proc gdb_starti_cmd {args} {
 
 # Set a breakpoint at FUNCTION.  If there is an additional argument it is
 # a list of options; the supported options are allow-pending, temporary,
-# message, no-message, and passfail.
+# message, no-message, passfail and qualified.
 # The result is 1 for success, 0 for failure.
 #
 # Note: The handling of message vs no-message is messed up, but it's based
@@ -447,6 +447,10 @@ proc gdb_breakpoint { function args } {
 	set break_message "Temporary breakpoint"
     }
 
+    if {[lsearch -exact $args qualified] != -1} {
+	append break_command " -qualified"
+    }
+
     set print_pass 0
     set print_fail 1
     set no_message_loc [lsearch -exact $args no-message]
-- 
2.5.5



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]