From patchwork Thu Nov 30 19:59:50 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sergio Durigan Junior X-Patchwork-Id: 24643 Received: (qmail 96959 invoked by alias); 30 Nov 2017 20:00:39 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 95684 invoked by uid 89); 30 Nov 2017 20:00:26 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.7 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KB_WAM_FROM_NAME_SINGLEWORD, SPF_HELO_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=non-existent X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 30 Nov 2017 20:00:05 +0000 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2AA0B8008D for ; Thu, 30 Nov 2017 20:00:03 +0000 (UTC) Received: from psique.yyz.redhat.com (unused-10-15-17-193.yyz.redhat.com [10.15.17.193]) by smtp.corp.redhat.com (Postfix) with ESMTP id A3C1160DCC; Thu, 30 Nov 2017 19:59:59 +0000 (UTC) From: Sergio Durigan Junior To: GDB Patches Cc: Pedro Alves , Sergio Durigan Junior Subject: [PATCH v4] Make '{add-, }symbol-file' not care about the position of command line arguments Date: Thu, 30 Nov 2017 14:59:50 -0500 Message-Id: <20171130195950.872-1-sergiodj@redhat.com> In-Reply-To: <20171129214451.14257-1-sergiodj@redhat.com> References: <20171129214451.14257-1-sergiodj@redhat.com> X-IsSubscribed: yes Changes from v3: - 'add-symbol-file -s' now accepts sections whose names start with a dash. - Use a better way to test must-ask questions on the testsuite. 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-11-30 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-11-30 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/symfile.c | 98 +++++++++++++++++++------------------ gdb/testsuite/gdb.base/relocate.exp | 86 ++++++++++++++++++++++++++++++++ 2 files changed, 137 insertions(+), 47 deletions(-) diff --git a/gdb/symfile.c b/gdb/symfile.c index 4bbe0b5a62..1c9f836eb3 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/gdb.base/relocate.exp b/gdb/testsuite/gdb.base/relocate.exp index 6eef15fb20..d0a839ae59 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"