From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 36263 invoked by alias); 26 Jun 2018 02:52:10 -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 36102 invoked by uid 89); 26 Jun 2018 02:51:57 -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=adapts 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:51:55 +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 w5Q2pnk1008341 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Mon, 25 Jun 2018 22:51:53 -0400 Received: by simark.ca (Postfix, from userid 112) id 1CE651EF28; Mon, 25 Jun 2018 22:51:49 -0400 (EDT) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id 53C511E003; Mon, 25 Jun 2018 22:51:48 -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:52:00 -0000 From: Simon Marchi To: Petr Tesarik Cc: gdb-patches@sourceware.org, Jeff Mahoney Subject: Re: [PATCH v2 4/4] Add an optional offset option to the "add-symbol-file" command In-Reply-To: <20180611120835.27343-5-ptesarik@suse.cz> References: <20180611120835.27343-1-ptesarik@suse.cz> <20180611120835.27343-5-ptesarik@suse.cz> Message-ID: <0061dc594fcb938994c15b6deb389618@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.6 X-IsSubscribed: yes X-SW-Source: 2018-06/txt/msg00595.txt.bz2 On 2018-06-11 08:08, Petr Tesarik wrote: > @@ -2195,7 +2206,13 @@ add_symbol_file_command (const char *args, int > from_tty) > At this point, we don't know what file type this is, > so we can't determine what section names are valid. */ > } > - if (section_addrs.size () == 0) > + if (seen_offset) > + printf_unfiltered (_("%s offset by %s\n"), > + (section_addrs.size () == 0 > + ? _(" with all sections") > + : _("with other sections")), > + paddress (gdbarch, offset)); > + else if (section_addrs.size () == 0) Nice little detail, the message that adapts to how the command was used. You can use ".empty ()" instead of .size () == 0" (in the previous patches too). > printf_unfiltered ("\n"); > > if (from_tty && (!query ("%s", ""))) > @@ -2204,6 +2221,42 @@ add_symbol_file_command (const char *args, int > from_tty) > objf = symbol_file_add (filename.get (), add_flags, §ion_addrs, > flags); > > + if (seen_offset) > + { > + std::vector new_offsets > (objf->num_sections, > + { offset }); > + > + std::vector sect_addrs_sorted > + = addrs_section_sort (section_addrs); > + > + section_addr_info objf_addrs > + = build_section_addr_info_from_objfile (objf); > + std::vector objf_addrs_sorted > + = addrs_section_sort (objf_addrs); > + > + std::vector::iterator > sect_sorted_iter > + = sect_addrs_sorted.begin (); > + for (const struct other_sections *objf_sect : objf_addrs_sorted) > + { > + const char *objf_name = addr_section_name (objf_sect->name.c_str > ()); > + int cmp = -1; > + > + while (cmp < 0 && sect_sorted_iter != sect_addrs_sorted.end ()) > + { > + const struct other_sections *sect = *sect_sorted_iter; > + const char *sect_name = addr_section_name (sect->name.c_str > ()); > + cmp = strcmp (sect_name, objf_name); > + if (cmp <= 0) > + ++sect_sorted_iter; > + } > + > + if (cmp == 0) > + new_offsets[objf_sect->sectindex].offsets[0] = 0; > + } > + > + objfile_relocate (objf, new_offsets.data ()); > + } > + Could this new code be in a separate function? This one is starting to get long. Also, could you add a bit of comments to explain the intent of the code? There is a lot of magic going on, it's a bit hard to read. > diff --git a/gdb/testsuite/gdb.base/relocate.exp > b/gdb/testsuite/gdb.base/relocate.exp > index a3af8cea61..119495dd94 100644 > --- a/gdb/testsuite/gdb.base/relocate.exp > +++ b/gdb/testsuite/gdb.base/relocate.exp > @@ -235,6 +235,62 @@ set new_function_foo_addr [get_var_address > function_foo] > gdb_assert {${new_function_foo_addr} == ${function_foo_addr} + > $offset} \ > "function foo is moved by offset" > > +# Load the object using add-symbol-file with an offset and check that > +# all addresses are moved by that offset. > + > +set offset 0x10000 > +clean_restart > +gdb_test "add-symbol-file -o $offset $binfile" \ > + "Reading symbols from ${binfile}\.\.\.done\." \ > + "add-symbol-file with offset" \ > + "add symbol table from file \".*${testfile}\\.o\" with all > sections offset by $offset\[\r\n\]+\\(y or n\\) " \ > + "y" > + > +# Make sure the address of a static variable is moved by offset. > +set new_static_foo_addr [get_var_address static_foo] > +gdb_assert { ${new_static_foo_addr} == ${static_foo_addr} + $offset } > \ > + "static variable foo is moved by offset" > + > +# Make sure the address of a global variable is moved by offset. > +set new_global_foo_addr [get_var_address global_foo] > +gdb_assert { ${new_global_foo_addr} == ${global_foo_addr} + $offset } > \ > + "global variable foo is moved by offset" > + > +# Make sure the address of a function is moved by offset. > +set new_function_foo_addr [get_var_address function_foo] > +gdb_assert { ${new_function_foo_addr} == ${function_foo_addr} + > $offset } \ > + "function foo is moved by offset" > + > +# Re-load the object giving an explicit address for .text > + > +set text [ format "0x%x" [expr ${function_foo_addr} + 0x20000] ] > +clean_restart > +gdb_test "add-symbol-file $binfile -o $offset $text" \ > + "Reading symbols from ${binfile}\.\.\.done\." \ > + "add-symbol-file with offset, text address given" \ > + "add symbol table from file \".*${testfile}\\.o\" at\[ > \t\r\n\]+\.text_addr = ${text}\[\r\n\]+with other sections offset by > ${offset}\[\r\n\]+\\(y or n\\) " \ > + "y" > + > +# Make sure function has a different addresses now. "has a different addresses" -> "has a different address", repeated a few times. Simon