From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 57655 invoked by alias); 30 Nov 2017 12:38:12 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 57643 invoked by uid 89); 30 Nov 2017 12:38:12 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=BAYES_00,KB_WAM_FROM_NAME_SINGLEWORD,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 spammy= 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 12:38:10 +0000 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 772E2820F2 for ; Thu, 30 Nov 2017 12:38:09 +0000 (UTC) Received: from localhost (unused-10-15-17-193.yyz.redhat.com [10.15.17.193]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 4CD2560BE1; Thu, 30 Nov 2017 12:38:06 +0000 (UTC) From: Sergio Durigan Junior To: Pedro Alves Cc: GDB Patches Subject: Re: [PATCH v2] Make '{add-,}symbol-file' not care about the position of command line arguments References: <20171129214451.14257-1-sergiodj@redhat.com> <20171130042448.30882-1-sergiodj@redhat.com> <2fba79f1-e16e-38f2-bc0f-1c27da05e5c1@redhat.com> Date: Thu, 30 Nov 2017 12:38:00 -0000 In-Reply-To: <2fba79f1-e16e-38f2-bc0f-1c27da05e5c1@redhat.com> (Pedro Alves's message of "Thu, 30 Nov 2017 10:57:50 +0000") Message-ID: <87po807xhu.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2017-11/txt/msg00829.txt.bz2 On Thursday, November 30 2017, Pedro Alves wrote: > On 11/30/2017 04:24 AM, Sergio Durigan Junior wrote: >> Changes from v1: >> >> - Commit message has been rewritten. >> >> - Implemented position-independent argument parsing for >> 'add-symbol-file'. >> >> - Added testcases. > > Looks like you missed the comment about "--". Take a look at > maintenance_print_symbols for an example of a command > that supports ending options with "--". Can you add that > while you're at it, please? For a test, I'd suggest > e.g., "symbol-file -- -non-existent-file" and confirming > gdb errors out. That's simpler than actually creating a file. Oh, I didn't understand that you were asking me to implement it. I thought it was a comment for "something to be done later". I'll do it, then. >> + if (*arg != '-') >> { >> - /* It's an option (starting with '-') or it's an argument >> - to an option. */ >> if (expecting_sec_name) >> { >> sect_opt sect = { arg, NULL }; >> sect_opts.push_back (sect); >> - expecting_sec_name = 0; >> + expecting_sec_name = false; >> } >> else if (expecting_sec_addr) >> { >> sect_opts.back ().value = arg; >> - expecting_sec_addr = 0; >> + expecting_sec_addr = false; >> } >> - else if (strcmp (arg, "-readnow") == 0) >> - flags |= OBJF_READNOW; >> - else if (strcmp (arg, "-s") == 0) >> + else if (filename == NULL) >> + { >> + /* First non-option argument is always the filename. */ >> + filename.reset (tilde_expand (arg)); >> + } >> + else if (!seen_addr) >> { >> - 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_opt sect = { ".text", arg }; >> + sect_opts.push_back (sect); >> + seen_addr = true; > > Does this push_back directly here mean that these > two commands end up with different semantics? > > (gdb) add-symbol-file FILE 0 -s .text 0x1000 > (gdb) add-symbol-file -s .text 0x1000 FILE 0 The arguments are printed in a different order, yes: (gdb) add-symbol-file -s .text 0x100 ./gdb/testsuite/outputs/gdb.base/relocate/relocate.o 0 add symbol table from file "./gdb/testsuite/outputs/gdb.base/relocate/relocate.o" at .text_addr = 0x100 .text_addr = 0x0 (y or n) n Not confirmed. (gdb) add-symbol-file ./gdb/testsuite/outputs/gdb.base/relocate/relocate.o 0 -s .text 0x100 add symbol table from file "./gdb/testsuite/outputs/gdb.base/relocate/relocate.o" at .text_addr = 0x0 .text_addr = 0x100 (y or n) n Not confirmed. > Not sure that's a good idea. I'll work on it and make sure they're always printed in the same way. > Please add a test with "-s .text"... Will do. >> +# Check that we can pass parameters using any position in the command >> +# line. >> +gdb_test "add-symbol-file -readnow $binfile 0x0 -s .bss 0x3" \ >> + "Not confirmed\." \ >> + "add-symbol-file positionless -readnow" \ >> + "add symbol table from file \"${binfile}\" at\r\n\t\.text_addr = 0x0\r\n\t\.bss_addr = 0x3\r\n\\(y or n\\) " \ >> + "n" >> +# When we use -s as the first argument, the section will be printed >> +# first as well. >> +gdb_test "add-symbol-file -s .bss 0x3 -readnow $binfile 0x0" \ >> + "Not confirmed\." \ >> + "add-symbol-file positionless -s" \ >> + "add symbol table from file \"${binfile}\" at\r\n\t\.bss_addr = 0x3\r\n\t\.text_addr = 0x0\r\n\\(y or n\\) " \ >> + "n" >> +gdb_test "add-symbol-file $binfile 0x0 -s .bss 0x3" \ >> + "Not confirmed\." \ >> + "add-symbol-file positionless -s, no -readnow" \ >> + "add symbol table from file \"${binfile}\" at\r\n\t\.text_addr = 0x0\r\n\t\.bss_addr = 0x3\r\n\\(y or n\\) " \ >> + "n" > > Using a number != 0x0 is a little better, since its easy for > a variable to end up always zero-initialized / zero-propagated > by mistake, and the test wouldn't notice. OK. >> +# 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" >> +gdb_exit >> +gdb_start >> +gdb_reinitialize_dir $srcdir/$subdir > > Just use clean_restart with no argument. Done. >> +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_exit >> +gdb_start >> +gdb_reinitialize_dir $srcdir/$subdir > > Ditto. Done. >> + >> gdb_test "add-symbol-file ${binfile} 0 -s" \ >> "Missing section name after .-s." \ >> "add-symbol-file bare -s" >> > > Thanks, > Pedro Alves Thanks, -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/