[PATCHv4 12/14] gdb: add remove-symbol-file command completion

Andrew Burgess
2024-07-04T14:24:31Z

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.
---
gdb/NEWS | 10 +
gdb/doc/gdb.texinfo | 1 +
gdb/symfile.c | 114 ++++++--
.../gdb.base/filename-completion.exp | 3 +-
gdb/testsuite/gdb.base/sym-file.exp | 258 ++++++++++--------
5 files changed, 238 insertions(+), 148 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 47677cb773a..8808fb3e1f6 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -13,6 +13,16 @@
This may cause breakage when using an incompatible libc, like uclibc or
newlib, or an older glibc.

+* Changed commands
+
+remove-symbol-file
+ This command now supports command 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.
+
*** Changes in GDB 15

* The MPX commands "show/set mpx bound" have been deprecated, as Intel
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 86cd420832a..7261a5559c4 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -21885,6 +21885,7 @@
(@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 1bf3f47a6f5..8789b216446 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -2355,39 +2355,90 @@ add_symbol_file_command (const char *args, int from_tty)
}
\f

+/* 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;
@@ -2395,21 +2446,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;
@@ -3855,14 +3903,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 cf00d007ca7..62fb49570a6 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 3e36e65d1c5..17650cbce3a 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
+ }
}
--
2.25.4