gdb: add remove-symbol-file command completion

The 'remove-symbol-file' command doesn't currently offer command
completion.  This commit addresses this.

The 'remove-symbol-file' uses gdb_argv to split its command arguments,
this means that the filename the command expects can be quoted.

However, the 'remove-symbol-file' command is a little weird in that it
also has a '-a' option, if this option is passed then the command
expects not a filename, but an address.

Currently the remove_symbol_file_command function splits the command
args using gdb_argv, checks for a '-a' flag by looking at the first
argument value, and then expects the filename or address to occupy a
single entry in the gdb_argv array.

The first thing I do is handle the '-a' flag using GDB's option
system.  I model this option as a flag_option_def (a boolean option).

I've dropped the use of gdb_argv and instead use the new(ish) function
extract_single_filename_arg, which was added a couple of commits back,
to parse the filename argument (when '-a' is not given).

If '-a' is given the the remove-symbol-file command expects an address
rather than a filename.  As we previously split the arguments using
gdb_argv this meant the address needed to appear as a single
argument.  So a user could write:

  (gdb) remove-symbol-file 0x1234

Or they could write:

  (gdb) remove-symbol-file some_function

Both of these would work fine.  But a user could not write:

  (gdb) remove-symbol-file some_function + 0x1000

As only the 'some_function' part would be processed.  Now the user
could do this:

  (gdb) remove-symbol-file "some_function + 0x1000"

By enclosing the address expression in quotes this would be handled as
a single argument.  However, this is a little weird, that's not how
commands like 'print' or 'x' work.  Also this functionality was
neither documented, or tested.

And so, in this commit, by removing the use of gdb_argv I bring the
'remove-symbol-file' command inline with GDB's other commands that
take an expression, the quotes are no longer needed.

Usually in a completer we call 'complete_options', but don't actually
capture the option values.  But for remove-symbol-file I do.  This
allows me to spot when the '-a' option has been given, I can then
complete the rest of the command line as either a filename or an
expression.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>

5 files changed, 238 insertions, 148 deletions

diff --git a/gdb/NEWS b/gdb/NEWS
index 2f5d5eb..8b3b4f1 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -53,6 +53,16 @@ maintenance info blocks [ADDRESS]
   are listed starting at the inner global block out to the most inner
   block.
 
+* Changed commands
+
+remove-symbol-file
+  This command now supports file-name completion.
+
+remove-symbol-file -a ADDRESS
+  The ADDRESS expression can now be a full expression consisting of
+  multiple terms, e.g. 'function + 0x1000' (without quotes),
+  previously only a single term could be given.
+
 * New remote packets
 
 vFile:stat
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 8cde2f2..67e36fb 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -21893,6 +21893,7 @@ Remove symbol table from file "/home/user/gdb/mylib.so"? (y or n) y
 (@value{GDBP})
 @end smallexample
 
+The @var{address} can be any expression which evaluates to an address.
 
 @code{remove-symbol-file} does not repeat if you press @key{RET} after using it.
 
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 5054d10..2292eca 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -2330,39 +2330,90 @@ add_symbol_file_command (const char *args, int from_tty)
 }
 
 
+/* Option support for 'remove-symbol-file' command.  */
+
+struct remove_symbol_file_options
+{
+  /* True when the '-a' flag was passed.  */
+  bool address_flag = false;
+};
+
+using remove_symbol_file_options_opt_def
+  = gdb::option::flag_option_def<remove_symbol_file_options>;
+
+static const gdb::option::option_def remove_symbol_file_opt_defs[] = {
+  remove_symbol_file_options_opt_def {
+    "a",
+    [] (remove_symbol_file_options *opt) { return &opt->address_flag; },
+    N_("Select a symbol file containing ADDRESS.")
+  },
+};
+
+static inline gdb::option::option_def_group
+make_remove_symbol_file_def_group (remove_symbol_file_options *opts)
+{
+  return {{remove_symbol_file_opt_defs}, opts};
+}
+
+/* Completion function for 'remove-symbol-file' command.  */
+
+static void
+remove_symbol_file_command_completer (struct cmd_list_element *ignore,
+				      completion_tracker &tracker,
+				      const char *text, const char * /* word */)
+{
+  /* Unlike many command completion functions we do gather the option
+     values here.  How we complete the rest of the command depends on
+     whether the '-a' flag has been given or not.  */
+  remove_symbol_file_options opts;
+  auto grp = make_remove_symbol_file_def_group (&opts);
+  if (gdb::option::complete_options
+      (tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_ERROR, grp))
+    return;
+
+  /* Complete the rest of the command line as either a filename or an
+     expression (which will evaluate to an address) if the '-a' flag was
+     given.  */
+  if (!opts.address_flag)
+    {
+      const char *word
+	= advance_to_filename_maybe_quoted_complete_word_point (tracker, text);
+      filename_maybe_quoted_completer (ignore, tracker, text, word);
+    }
+  else
+    {
+      const char *word
+	= advance_to_expression_complete_word_point (tracker, text);
+      symbol_completer (ignore, tracker, text, word);
+    }
+}
+
 /* This function removes a symbol file that was added via add-symbol-file.  */
 
 static void
 remove_symbol_file_command (const char *args, int from_tty)
 {
-  struct objfile *objf = NULL;
-  struct program_space *pspace = current_program_space;
-
   dont_repeat ();
 
-  if (args == NULL)
-    error (_("remove-symbol-file: no symbol file provided"));
+  remove_symbol_file_options opts;
+  auto grp = make_remove_symbol_file_def_group (&opts);
+  gdb::option::process_options
+    (&args, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_ERROR, grp);
 
-  gdb_argv argv (args);
+  struct objfile *objf = nullptr;
 
-  if (strcmp (argv[0], "-a") == 0)
+  if (opts.address_flag)
     {
-      /* Interpret the next argument as an address.  */
-      CORE_ADDR addr;
+      if (args == nullptr || *args == '\0')
+	error (_("remove-symbol-file: no address provided"));
 
-      if (argv[1] == NULL)
-	error (_("Missing address argument"));
-
-      if (argv[2] != NULL)
-	error (_("Junk after %s"), argv[1]);
-
-      addr = parse_and_eval_address (argv[1]);
+      CORE_ADDR addr = parse_and_eval_address (args);
 
       for (objfile *objfile : current_program_space->objfiles ())
 	{
 	  if ((objfile->flags & OBJF_USERLOADED) != 0
 	      && (objfile->flags & OBJF_SHARED) != 0
-	      && objfile->pspace () == pspace
+	      && objfile->pspace () == current_program_space
 	      && is_addr_in_objfile (addr, objfile))
 	    {
 	      objf = objfile;
@@ -2370,21 +2421,18 @@ remove_symbol_file_command (const char *args, int from_tty)
 	    }
 	}
     }
-  else if (argv[0] != NULL)
+  else
     {
-      /* Interpret the current argument as a file name.  */
-
-      if (argv[1] != NULL)
-	error (_("Junk after %s"), argv[0]);
-
-      gdb::unique_xmalloc_ptr<char> filename (tilde_expand (argv[0]));
+      std::string filename = extract_single_filename_arg (args);
+      if (filename.empty ())
+	error (_("remove-symbol-file: no symbol file provided"));
 
       for (objfile *objfile : current_program_space->objfiles ())
 	{
 	  if ((objfile->flags & OBJF_USERLOADED) != 0
 	      && (objfile->flags & OBJF_SHARED) != 0
-	      && objfile->pspace () == pspace
-	      && filename_cmp (filename.get (), objfile_name (objfile)) == 0)
+	      && objfile->pspace () == current_program_space
+	      && filename_cmp (filename.c_str (), objfile_name (objfile)) == 0)
 	    {
 	      objf = objfile;
 	      break;
@@ -3830,14 +3878,22 @@ READNOW_READNEVER_HELP),
 	       &cmdlist);
   set_cmd_completer (c, filename_maybe_quoted_completer);
 
-  c = add_cmd ("remove-symbol-file", class_files,
-	       remove_symbol_file_command, _("\
+  const auto remove_symbol_file_opts
+    = make_remove_symbol_file_def_group (nullptr);
+  static std::string remove_symbol_file_cmd_help
+    = gdb::option::build_help (_("\
 Remove a symbol file added via the add-symbol-file command.\n\
 Usage: remove-symbol-file FILENAME\n\
        remove-symbol-file -a ADDRESS\n\
 The file to remove can be identified by its filename or by an address\n\
-that lies within the boundaries of this symbol file in memory."),
+that lies within the boundaries of this symbol file in memory.\n\
+Options:\n\
+%OPTIONS%"), remove_symbol_file_opts);
+  c = add_cmd ("remove-symbol-file", class_files,
+	       remove_symbol_file_command,
+	       remove_symbol_file_cmd_help.c_str (),
 	       &cmdlist);
+  set_cmd_completer_handle_brkchars (c, remove_symbol_file_command_completer);
 
   c = add_cmd ("load", class_files, load_command, _("\
 Dynamically load FILE into the running program.\n\
diff --git a/gdb/testsuite/gdb.base/filename-completion.exp b/gdb/testsuite/gdb.base/filename-completion.exp
index cf00d00..62fb495 100644
--- a/gdb/testsuite/gdb.base/filename-completion.exp
+++ b/gdb/testsuite/gdb.base/filename-completion.exp
@@ -122,7 +122,8 @@ proc test_gdb_complete_filename_multiple {
 proc run_quoting_and_escaping_tests { root } {
     # Test all the commands which allow quoting of filenames, and
     # which require whitespace to be escaped in unquoted filenames.
-    foreach_with_prefix cmd { file exec-file symbol-file add-symbol-file } {
+    foreach_with_prefix cmd { file exec-file symbol-file add-symbol-file \
+			      remove-symbol-file } {
 	gdb_start
 
 	# Completing 'thread apply all ...' commands uses a custom word
diff --git a/gdb/testsuite/gdb.base/sym-file.exp b/gdb/testsuite/gdb.base/sym-file.exp
index 3e36e65..17650cb 100644
--- a/gdb/testsuite/gdb.base/sym-file.exp
+++ b/gdb/testsuite/gdb.base/sym-file.exp
@@ -67,134 +67,156 @@ if {[prepare_for_testing "failed to prepare"  $binfile "$srcfile $srcfile2" $exe
 
 gdb_load_shlib ${lib_so}
 
-if {![runto_main]} {
-    return
-}
-
-# 1) Run to gdb_add_symbol_file in $srcfile for adding the library's
-#    symbols.
-gdb_breakpoint gdb_add_symbol_file
-gdb_continue_to_breakpoint gdb_add_symbol_file
-
-# 2) Set a pending breakpoint at bar in $srcfile3.
-set result [gdb_breakpoint bar allow-pending]
-if {!$result} {
-   return
-}
-
-# 3) Add the library's symbols using 'add-symbol-file'.
-set result [gdb_test "add-symbol-file ${lib_syms} addr" \
-		     "Reading symbols from .*${lib_syms}\\.\\.\\." \
-		     "add-symbol-file ${lib_basename}.so addr" \
-		     "add symbol table from file \".*${lib_basename}\\.so\"\
- at.*\\(y or n\\) " \
-		     "y"]
-if {$result != 0} {
-   return
-}
-
-# 4) 'info files' must display $srcfile3.
-gdb_test "info files" \
-	 "^(?=(.*${lib_basename})).*" \
-	 "info files must display ${lib_basename}"
-
-# 5) Continue to bar in $srcfile3 to ensure that the breakpoint
-#    was bound correctly after adding $shilb_name.
-set lnum_bar [gdb_get_line_number "break at bar" $srcfile3]
-gdb_continue_to_breakpoint bar ".*${lib_basename}\\.c:$lnum_bar.*"
-
-# 6) Set a breakpoint at foo in $srcfile3.
-set result [gdb_breakpoint foo]
-if {!$result} {
-    return
-}
-
-# 7) Continue to foo in $srcfile3 to ensure that the breakpoint
-#    was bound correctly.
-set lnum_foo [gdb_get_line_number "break at foo" $srcfile3]
-gdb_continue_to_breakpoint foo ".*${lib_basename}\\.c:$lnum_foo.*"
-
-# 8) Set a breakpoint at gdb_remove_symbol_file in $srcfile for
-#    removing the library's symbols.
-set result [gdb_breakpoint gdb_remove_symbol_file]
-if {!$result} {
-    return
-}
+proc do_test { remove_expr } {
+    global lib_basename lib_syms srcfile srcfile3
 
-# 9) Continue to gdb_remove_symbol_file in $srcfile.
-gdb_continue_to_breakpoint gdb_remove_symbol_file
-
-# 10) Remove the library's symbols using 'remove-symbol-file'.
-set result [gdb_test "remove-symbol-file -a addr" \
-		     ""\
-		     "remove-symbol-file -a addr" \
-		     "Remove symbol table from file \".*${lib_basename}\\.so\"\\?\
-.*\\(y or n\\) " \
-		     "y"]
-if {$result != 0} {
-    return
-}
+    clean_restart $::binfile
 
-# 11) 'info files' must not display ${lib_basename}, anymore.
-gdb_test "info files" \
-	 "^(?!(.*${lib_basename})).*" \
-	 "info files must not display ${lib_basename}"
+    if {![runto_main]} {
+	return
+    }
 
-# 12) Check that the breakpoints at foo and bar are pending after
-#     removing the library's symbols.
-gdb_test "info breakpoints 3" \
-	 ".*PENDING.*" \
-	 "breakpoint at foo is pending"
-
-gdb_test "info breakpoints 4" \
-	 ".*PENDING.*" \
-	 "breakpoint at bar is pending"
-
-# 13) Check that the execution can continue without error.
-set lnum_reload [gdb_get_line_number "reload lib here"]
-gdb_breakpoint $lnum_reload
-gdb_continue_to_breakpoint reload ".*${srcfile}:$lnum_reload.*"
-
-# 14) Regression test for a stale breakpoints bug.  Check whether
-# unloading symbols manually without the program actually unloading
-# the library, when breakpoints are inserted doesn't leave stale
-# breakpoints behind.
-with_test_prefix "stale bkpts" {
-    # Force breakpoints always inserted.
-    gdb_test_no_output "set breakpoint always-inserted on"
-
-    # Get past the library reload.
+    # 1) Run to gdb_add_symbol_file in $srcfile for adding the library's
+    #    symbols.
+    gdb_breakpoint gdb_add_symbol_file
     gdb_continue_to_breakpoint gdb_add_symbol_file
 
-    # Load the library's symbols.
-    gdb_test "add-symbol-file ${lib_syms} addr" \
-	"Reading symbols from .*${lib_syms}\\.\\.\\." \
-	"add-symbol-file ${lib_basename}.so addr" \
-	"add symbol table from file \".*${lib_syms}\"\
+    # 2) Set a pending breakpoint at bar in $srcfile3.
+    set result [gdb_breakpoint bar allow-pending]
+    if {!$result} {
+	return
+    }
+
+    # 3) Add the library's symbols using 'add-symbol-file'.
+    set result [gdb_test "add-symbol-file ${lib_syms} addr" \
+		    "Reading symbols from .*${lib_syms}\\.\\.\\." \
+		    "add-symbol-file ${lib_basename}.so addr" \
+		    "add symbol table from file \".*${lib_basename}\\.so\"\
+ at.*\\(y or n\\) " \
+		    "y"]
+    if {$result != 0} {
+	return
+    }
+
+    # 4) 'info files' must display $srcfile3.
+    gdb_test "info files" \
+	"^(?=(.*${lib_basename})).*" \
+	"info files must display ${lib_basename}"
+
+    # 5) Continue to bar in $srcfile3 to ensure that the breakpoint
+    #    was bound correctly after adding $shilb_name.
+    set lnum_bar [gdb_get_line_number "break at bar" $srcfile3]
+    gdb_continue_to_breakpoint bar ".*${lib_basename}\\.c:$lnum_bar.*"
+
+    # 6) Set a breakpoint at foo in $srcfile3.
+    set result [gdb_breakpoint foo]
+    if {!$result} {
+	return
+    }
+
+    # 7) Continue to foo in $srcfile3 to ensure that the breakpoint
+    #    was bound correctly.
+    set lnum_foo [gdb_get_line_number "break at foo" $srcfile3]
+    gdb_continue_to_breakpoint foo ".*${lib_basename}\\.c:$lnum_foo.*"
+
+    # 8) Set a breakpoint at gdb_remove_symbol_file in $srcfile for
+    #    removing the library's symbols.
+    set result [gdb_breakpoint gdb_remove_symbol_file]
+    if {!$result} {
+	return
+    }
+
+    # 9) Continue to gdb_remove_symbol_file in $srcfile.
+    gdb_continue_to_breakpoint gdb_remove_symbol_file
+
+    # 10) Remove the library's symbols using 'remove-symbol-file'.
+    set result [gdb_test "remove-symbol-file ${remove_expr}" \
+		    ""\
+		    "remove-symbol-file" \
+		    "Remove symbol table from file \".*${lib_basename}\\.so\"\\?\
+.*\\(y or n\\) " \
+		    "y"]
+    if {$result != 0} {
+	return
+    }
+
+    # 11) 'info files' must not display ${lib_basename}, anymore.
+    gdb_test "info files" \
+	"^(?!(.*${lib_basename})).*" \
+	"info files must not display ${lib_basename}"
+
+    # 12) Check that the breakpoints at foo and bar are pending after
+    #     removing the library's symbols.
+    gdb_test "info breakpoints 3" \
+	".*PENDING.*" \
+	"breakpoint at foo is pending"
+
+    gdb_test "info breakpoints 4" \
+	".*PENDING.*" \
+	"breakpoint at bar is pending"
+
+    # 13) Check that the execution can continue without error.
+    set lnum_reload [gdb_get_line_number "reload lib here"]
+    gdb_breakpoint $lnum_reload
+    gdb_continue_to_breakpoint reload ".*${srcfile}:$lnum_reload.*"
+
+    # 14) Regression test for a stale breakpoints bug.  Check whether
+    # unloading symbols manually without the program actually unloading
+    # the library, when breakpoints are inserted doesn't leave stale
+    # breakpoints behind.
+    with_test_prefix "stale bkpts" {
+	# Force breakpoints always inserted.
+	gdb_test_no_output "set breakpoint always-inserted on"
+
+	# Get past the library reload.
+	gdb_continue_to_breakpoint gdb_add_symbol_file
+
+	# Load the library's symbols.
+	gdb_test "add-symbol-file ${lib_syms} addr" \
+	    "Reading symbols from .*${lib_syms}\\.\\.\\." \
+	    "add-symbol-file ${lib_basename}.so addr" \
+	    "add symbol table from file \".*${lib_syms}\"\
 at.*\\(y or n\\) " \
-	"y"
+	    "y"
 
-    # Set a breakpoint at baz, in the library.
-    gdb_breakpoint baz
+	# Set a breakpoint at baz, in the library.
+	gdb_breakpoint baz
 
-    gdb_test "info breakpoints 7" ".*y.*0x.*in baz.*" \
-	"breakpoint at baz is resolved"
+	gdb_test "info breakpoints 7" ".*y.*0x.*in baz.*" \
+	    "breakpoint at baz is resolved"
 
-    # Unload symbols manually without the program actually unloading
-    # the library.
-    gdb_test "remove-symbol-file -a addr" \
-	"" \
-	"remove-symbol-file -a addr" \
-	"Remove symbol table from file \".*${lib_basename}\\.so\"\\?\
+	# Unload symbols manually without the program actually unloading
+	# the library.
+	gdb_test "remove-symbol-file ${remove_expr}" \
+	    "" \
+	    "remove-symbol-file" \
+	    "Remove symbol table from file \".*${lib_basename}\\.so\"\\?\
 .*\\(y or n\\) " \
-	"y"
+	    "y"
+
+	gdb_test "info breakpoints 7" ".*PENDING.*" \
+	    "breakpoint at baz is pending"
 
-    gdb_test "info breakpoints 7" ".*PENDING.*" \
-	"breakpoint at baz is pending"
+	# Check that execution can continue without error.  If GDB leaves
+	# breakpoints behind, we'll get back a spurious SIGTRAP.
+	set lnum_end [gdb_get_line_number "end here"]
+	gdb_breakpoint $lnum_end
+	gdb_continue_to_breakpoint "end here" ".*end here.*"
+    }
+}
 
-    # Check that execution can continue without error.  If GDB leaves
-    # breakpoints behind, we'll get back a spurious SIGTRAP.
-    set lnum_end [gdb_get_line_number "end here"]
-    gdb_breakpoint $lnum_end
-    gdb_continue_to_breakpoint "end here" ".*end here.*"
+foreach remove_expr [list addr bar "bar + 0x10" "${lib_syms}" ] {
+    # Don't use full filenames in the test prefix.  Also, add '-a' to
+    # all the REMOVE_EXPR values which are addresses rather than
+    # filenames.
+    set prefix $remove_expr
+    if { $prefix == $lib_syms } {
+	set prefix [file tail $prefix]
+    } else {
+	set remove_expr "-a $remove_expr"
+    }
+
+    with_test_prefix "remove_expr=$prefix" {
+	do_test $remove_expr
+    }
 }