From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 61391 invoked by alias); 26 Jun 2018 02:14:38 -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 60568 invoked by uid 89); 26 Jun 2018 02:14:30 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-24.9 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=gdb.texinfo, gdbtexinfo, UD:gdb.texinfo X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 26 Jun 2018 02:14:28 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id w5Q2EMCQ031721 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Mon, 25 Jun 2018 22:14:27 -0400 Received: by simark.ca (Postfix, from userid 112) id 5CD411EF28; Mon, 25 Jun 2018 22:14:22 -0400 (EDT) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id AD0B01E003; Mon, 25 Jun 2018 22:14:21 -0400 (EDT) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Tue, 26 Jun 2018 02:14:00 -0000 From: Simon Marchi To: Petr Tesarik Cc: gdb-patches@sourceware.org, Jeff Mahoney Subject: Re: [PATCH v2 2/4] Make add-symbol-file's address argument optional In-Reply-To: <20180611120835.27343-3-ptesarik@suse.cz> References: <20180611120835.27343-1-ptesarik@suse.cz> <20180611120835.27343-3-ptesarik@suse.cz> Message-ID: <5c9840aa77974522c769c43679c7a895@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.6 X-IsSubscribed: yes X-SW-Source: 2018-06/txt/msg00593.txt.bz2 Hi Petr, The patch LGTM, with some nits to address before pushing. On 2018-06-11 08:08, Petr Tesarik wrote: > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index 973365574f..84600bfe5f 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -18917,18 +18917,21 @@ the program is running. To do this, use the > @code{kill} command > > @kindex add-symbol-file > @cindex dynamic linking > -@item add-symbol-file @var{filename} @var{address} > -@itemx add-symbol-file @var{filename} @var{address} @r{[} -readnow > @r{|} -readnever @r{]} > -@itemx add-symbol-file @var{filename} @var{address} -s @var{section} > @var{address} @dots{} > +@item add-symbol-file @var{filename} @r{[} @var{address} @r{]} > +@itemx add-symbol-file @var{filename} @r{[} @var{address} @r{]} @r{[} > -readnow @r{|} -readnever @r{]} > +@itemx add-symbol-file @var{filename} @r{[} @var{address} @r{]} -s > @var{section} @var{address} @dots{} > The @code{add-symbol-file} command reads additional symbol table > information from the file @var{filename}. You would use this command > when @var{filename} has been dynamically loaded (by some other means) > into the program that is running. The @var{address} should give the > memory > -address at which the file has been loaded; @value{GDBN} cannot figure > -this out for itself. You can additionally specify an arbitrary number > +address at which the file has been loaded. > +You can additionally specify an arbitrary number > of @samp{-s @var{section} @var{address}} pairs, to give an explicit > section name and base address for that section. You can specify any > @var{address} as an expression. > +If @var{address} is omitted, @value{GDBN} will use the section > +addresses found in @var{filename}. You can use @samp{-s} to > +override this default and load a section at a different address. I really think that this section could use some improvements: - There are two arguments named "address", so it's not clear what the text refers to. - I don't think it's useful to have the synopsis on three different lines, since the options are not mutually exclusive. - It should be made clear that the positional "address" argument specifies the start of the .text section. Since it is now optional, I also think that this positional argument should be deprecated in favor of using "-s .text ..."... But none of this is a direct consequence of your patch, so your patch looks ok to me. > > The symbol table of the file @var{filename} is added to the symbol > table > originally read with the @code{symbol-file} command. You can use the > diff --git a/gdb/symfile.c b/gdb/symfile.c > index 461f60d074..3e3ab20412 100644 > --- a/gdb/symfile.c > +++ b/gdb/symfile.c > @@ -2161,29 +2161,26 @@ add_symbol_file_command (const char *args, int > from_tty) There might be the error message: error (_("add-symbol-file takes a file name and an address")); that would need to be updated, now that only the file name is mandatory. > diff --git a/gdb/testsuite/gdb.base/relocate.exp > b/gdb/testsuite/gdb.base/relocate.exp > index 77f6a88159..a3af8cea61 100644 > --- a/gdb/testsuite/gdb.base/relocate.exp > +++ b/gdb/testsuite/gdb.base/relocate.exp > @@ -73,6 +73,21 @@ gdb_test_multiple "add-symbol-file -s .text 0x200 > $binfile 0x100" $test { > gdb_test "n" "Not confirmed\." $test > } > } > +# Check that passing a single "-s .text" is equivallent to passing equivallent -> equivalent. Thanks, Simon