From 40fc416f4e22913ba2a2bafcc8da05f59c677b7d Mon Sep 17 00:00:00 2001 From: Sergio Durigan Junior Date: Wed, 29 Nov 2017 16:36:13 -0500 Subject: [PATCH] Make '{add-,}symbol-file' not care about the position of command line arguments This is a bug that's been detected while doing the readnever work. If you use 'symbol-file' or 'add-symbol-file', the position of each argument passed to the command matters. This means that if you do: (gdb) symbol-file -readnow /foo/bar The symbol file specified will (correctly) have all of its symbols read by GDB (because of the -readnow flag). However, if you do: (gdb) symbol-file /foo/bar -readnow GDB will silently ignore the -readnow flag, because it was specified after the filename. This is not a good thing to do and may confuse the user. To address that, I've modified the argument parsing mechanisms of symbol_file_command and add_symbol_file_command to be "position-independent". I have also added one error call at the end of add_symbol_file_command's argument parsing logic, which now clearly complains if no filename has been specified. Both commands now support the "--" option to stop argument processing. This patch provides a testcase for both commands, in order to make sure that the argument order does not matter. It has been regression-tested on BuildBot. gdb/ChangeLog: 2017-12-01 Sergio Durigan Junior * symfile.c (symbol_file_command): Call 'symbol_file_add_main_1' only after processing all command line options. (add_symbol_file_command): Modify logic to make arguments position-independent. gdb/testsuite/ChangeLog: 2017-12-01 Sergio Durigan Junior * gdb.base/relocate.exp: Add tests to guarantee that arguments to 'symbol-file' and 'add-symbol-file' can be position-independent. --- gdb/ChangeLog | 8 +++ gdb/symfile.c | 98 +++++++++++++++-------------- gdb/testsuite/ChangeLog | 6 ++ gdb/testsuite/gdb.base/relocate.exp | 86 +++++++++++++++++++++++++ 4 files changed, 151 insertions(+), 47 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 4eef5a9e736..b839e077165 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,11 @@ +2017-12-01 Sergio Durigan Junior + + * symfile.c (symbol_file_command): Call + 'symbol_file_add_main_1' only after processing all command + line options. + (add_symbol_file_command): Modify logic to make arguments + position-independent. + 2017-12-01 Joel Brobecker * ada-lang.c (symbol_list_obstack): Delete. diff --git a/gdb/symfile.c b/gdb/symfile.c index 4bbe0b5a622..1c9f836eb31 100644 --- a/gdb/symfile.c +++ b/gdb/symfile.c @@ -1622,26 +1622,35 @@ symbol_file_command (const char *args, int from_tty) objfile_flags flags = OBJF_USERLOADED; symfile_add_flags add_flags = 0; char *name = NULL; + bool stop_processing_options = false; + int idx; + char *arg; if (from_tty) add_flags |= SYMFILE_VERBOSE; gdb_argv built_argv (args); - for (char *arg : built_argv) + for (arg = built_argv[0], idx = 0; arg != NULL; arg = built_argv[++idx]) { - if (strcmp (arg, "-readnow") == 0) - flags |= OBJF_READNOW; - else if (*arg == '-') - error (_("unknown option `%s'"), arg); - else + if (stop_processing_options || *arg != '-') { - symbol_file_add_main_1 (arg, add_flags, flags); - name = arg; + if (name == NULL) + name = arg; + else + error (_("Unrecognized argument \"%s\""), arg); } + else if (strcmp (arg, "-readnow") == 0) + flags |= OBJF_READNOW; + else if (strcmp (arg, "--") == 0) + stop_processing_options = true; + else + error (_("Unrecognized argument \"%s\""), arg); } if (name == NULL) error (_("no symbol file name was specified")); + + symbol_file_add_main_1 (name, add_flags, flags); } } @@ -2180,8 +2189,6 @@ add_symbol_file_command (const char *args, int from_tty) char *arg; int argcnt = 0; int sec_num = 0; - int expecting_sec_name = 0; - int expecting_sec_addr = 0; struct objfile *objf; objfile_flags flags = OBJF_USERLOADED | OBJF_SHARED; symfile_add_flags add_flags = 0; @@ -2196,7 +2203,8 @@ add_symbol_file_command (const char *args, int from_tty) }; struct section_addr_info *section_addrs; - std::vector sect_opts; + std::vector sect_opts = { { ".text", NULL } }; + bool stop_processing_options = false; struct cleanup *my_cleanups = make_cleanup (null_cleanup, NULL); dont_repeat (); @@ -2204,63 +2212,59 @@ add_symbol_file_command (const char *args, int from_tty) if (args == NULL) error (_("add-symbol-file takes a file name and an address")); + bool seen_addr = false; gdb_argv argv (args); for (arg = argv[0], argcnt = 0; arg != NULL; arg = argv[++argcnt]) { - /* Process the argument. */ - if (argcnt == 0) - { - /* The first argument is the file name. */ - filename.reset (tilde_expand (arg)); - } - else if (argcnt == 1) - { - /* The second argument is always the text address at which - to load the program. */ - sect_opt sect = { ".text", arg }; - sect_opts.push_back (sect); - } - else + if (stop_processing_options || *arg != '-') { - /* It's an option (starting with '-') or it's an argument - to an option. */ - if (expecting_sec_name) + if (filename == NULL) { - sect_opt sect = { arg, NULL }; - sect_opts.push_back (sect); - expecting_sec_name = 0; + /* First non-option argument is always the filename. */ + filename.reset (tilde_expand (arg)); } - else if (expecting_sec_addr) + else if (!seen_addr) { - sect_opts.back ().value = arg; - expecting_sec_addr = 0; - } - else if (strcmp (arg, "-readnow") == 0) - flags |= OBJF_READNOW; - else if (strcmp (arg, "-s") == 0) - { - expecting_sec_name = 1; - expecting_sec_addr = 1; + /* The second non-option argument is always the text + address at which to load the program. */ + sect_opts[0].value = arg; + seen_addr = true; } else error (_("Unrecognized argument \"%s\""), arg); } + else if (strcmp (arg, "-readnow") == 0) + flags |= OBJF_READNOW; + else if (strcmp (arg, "-s") == 0) + { + if (argv[argcnt + 1] == NULL) + error (_("Missing section name after \"-s\"")); + else if (argv[argcnt + 2] == NULL) + error (_("Missing section address after \"-s\"")); + + sect_opt sect = { argv[argcnt + 1], argv[argcnt + 2] }; + + sect_opts.push_back (sect); + argcnt += 2; + } + else if (strcmp (arg, "--") == 0) + stop_processing_options = true; + else + error (_("Unrecognized argument \"%s\""), arg); } + if (filename == NULL) + error (_("You must provide a filename to be loaded.")); + /* This command takes at least two arguments. The first one is a filename, and the second is the address where this file has been loaded. Abort now if this address hasn't been provided by the user. */ - if (sect_opts.empty ()) + if (!seen_addr) error (_("The address where %s has been loaded is missing"), filename.get ()); - if (expecting_sec_name) - error (_("Missing section name after \"-s\"")); - else if (expecting_sec_addr) - error (_("Missing section address after \"-s\"")); - /* Print the prompt for the query below. And save the arguments into a sect_addr_info structure to be passed around to other functions. We have to split this up into separate print diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 69e1c3ad9e8..d0c91ef4e41 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2017-12-01 Sergio Durigan Junior + + * gdb.base/relocate.exp: Add tests to guarantee that arguments + to 'symbol-file' and 'add-symbol-file' can be + position-independent. + 2017-12-01 Yao Qi * gdb.arch/aarch64-atomic-inst.exp: Replace mail address with diff --git a/gdb/testsuite/gdb.base/relocate.exp b/gdb/testsuite/gdb.base/relocate.exp index 6eef15fb20e..d0a839ae59c 100644 --- a/gdb/testsuite/gdb.base/relocate.exp +++ b/gdb/testsuite/gdb.base/relocate.exp @@ -37,6 +37,92 @@ foreach x {"-raednow" "readnow" "foo" "-readnow s"} { "add-symbol-file: unknown option $word" } +# Check that we can pass parameters using any position in the command +# line. +set test "add-symbol-file positionless -readnow" +gdb_test_multiple "add-symbol-file -readnow $binfile 0x100 -s .bss 0x3" $test { + -re "add symbol table from file \"${binfile}\" at\r\n\t\.text_addr = 0x100\r\n\t\.bss_addr = 0x3\r\n\\(y or n\\) " { + gdb_test "n" "Not confirmed\." $test + } +} +# When we use -s as the first argument, the section will be printed +# first as well. +set test "add-symbol-file positionless -s" +gdb_test_multiple "add-symbol-file -s .bss 0x3 -readnow $binfile 0x100" $test { + -re "add symbol table from file \"${binfile}\" at\r\n\t\.text_addr = 0x100\r\n\t\.bss_addr = 0x3\r\n\\(y or n\\) " { + gdb_test "n" "Not confirmed\." $test + } +} +set test "add-symbol-file positionless -s, no -readnow" +gdb_test_multiple "add-symbol-file $binfile 0x100 -s .bss 0x3" $test { + -re "add symbol table from file \"${binfile}\" at\r\n\t\.text_addr = 0x100\r\n\t\.bss_addr = 0x3\r\n\\(y or n\\) " { + gdb_test "n" "Not confirmed\." $test + } +} +# Check that passing "-s .text", no matter the position, always has +# the same result. +set test "add-symbol-file different -s .text, after file" +gdb_test_multiple "add-symbol-file $binfile 0x100 -s .text 0x200" $test { + -re "add symbol table from file \"${binfile}\" at\r\n\t\.text_addr = 0x100\r\n\t\.text_addr = 0x200\r\n\\(y or n\\) " { + gdb_test "n" "Not confirmed\." $test + } +} +set test "add-symbol-file different -s .text, before file" +gdb_test_multiple "add-symbol-file -s .text 0x200 $binfile 0x100" $test { + -re "add symbol table from file \"${binfile}\" at\r\n\t\.text_addr = 0x100\r\n\t\.text_addr = 0x200\r\n\\(y or n\\) " { + gdb_test "n" "Not confirmed\." $test + } +} +# Test that passing "--" disables option processing. +gdb_test "add-symbol-file -- $binfile 0x100 -s .bss 0x3" \ + "Unrecognized argument \"-s\"" \ + "add-symbol-file with -- disables option processing" +set test "add-symbol-file with -- disables option processing, non-existent filename" +gdb_test_multiple "add-symbol-file -- -non-existent-file 0x100" $test { + -re "add symbol table from file \"-non-existent-file\" at\r\n\t\.text_addr = 0x100\r\n\\(y or n\\) " { + gdb_test "y" "-non-existent-file: No such file or directory\." $test + } +} +# Test that passing the wrong number of arguments to '-s' leads to an +# error. +gdb_test "add-symbol-file $binfile -s" \ + "Missing section name after \"-s\"" \ + "add-symbol-file with -s without section name" +gdb_test "add-symbol-file $binfile -s .bss" \ + "Missing section address after \"-s\"" \ + "add-symbol-file with -s without section address" +# Test that '-s' accepts section names with '-' +set test "add-symbol-file with -s using section name starting with dash" +gdb_test_multiple "add-symbol-file -s -section-name 0x200 $binfile 0x100" $test { + -re "add symbol table from file \"${binfile}\" at\r\n\t\.text_addr = 0x100\r\n\t\-section-name_addr = 0x200\r\n\\(y or n\\) " { + gdb_test "n" "Not confirmed\." $test + } +} +# Since we're here, might as well test the 'symbol-file' command and +# if its arguments can also be passed at any position. +gdb_test "symbol-file -readnow $binfile" \ + "Reading symbols from ${binfile}\.\.\.expanding to full symbols\.\.\.done\." \ + "symbol-file with -readnow first" +clean_restart +gdb_test "symbol-file $binfile -readnow" \ + "Reading symbols from ${binfile}\.\.\.expanding to full symbols\.\.\.done\." \ + "symbol-file with -readnow second" +gdb_test "symbol-file -readnow" \ + "no symbol file name was specified" \ + "symbol-file without filename" +gdb_test "symbol-file -- -non-existent-file" \ + "-non-existent-file: No such file or directory\." \ + "symbol-file with -- disables option processing" +clean_restart +gdb_test "symbol-file -readnow -- $binfile" \ + "Reading symbols from ${binfile}\.\.\.expanding to full symbols\.\.\.done\." \ + "symbol-file with -- and -readnow" +gdb_test "symbol-file -- $binfile -readnow" \ + "Unrecognized argument \"-readnow\"" \ + "symbol-file with -- and -readnow, invalid option" + +clean_restart + gdb_test "add-symbol-file ${binfile} 0 -s" \ "Missing section name after .-s." \ "add-symbol-file bare -s" -- 2.30.2