From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 44576 invoked by alias); 25 May 2018 14:14:45 -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 44566 invoked by uid 89); 25 May 2018 14:14:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=winwin, win-win 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; Fri, 25 May 2018 14:14:43 +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 w4PEEaiT014559 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Fri, 25 May 2018 10:14:41 -0400 Received: by simark.ca (Postfix, from userid 112) id 51E221F21B; Fri, 25 May 2018 10:14:36 -0400 (EDT) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id B61641E75F; Fri, 25 May 2018 10:14:35 -0400 (EDT) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Fri, 25 May 2018 14:58:00 -0000 From: Simon Marchi To: Petr Tesarik Cc: gdb-patches@sourceware.org, Jeff Mahoney Subject: Re: [PATCH] Add an optional offset option to the "symbol-file" command In-Reply-To: <20180525072312.3d50b6b3@ezekiel.suse.cz> References: <20180427112449.4e3e3f06@ezekiel.suse.cz> <20180523104956.35a01bcc@ezekiel.suse.cz> <092eb92c83693a6a444ff517c9b0168f@polymtl.ca> <20180525072312.3d50b6b3@ezekiel.suse.cz> Message-ID: <71f5bfcc0ee2efc88b5fda63c04f3957@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.6 X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Fri, 25 May 2018 14:14:36 +0000 X-IsSubscribed: yes X-SW-Source: 2018-05/txt/msg00665.txt.bz2 On 2018-05-25 01:23, Petr Tesarik wrote: > Yes, technically, add-symbol-file can be used for the same purpose, but > it is very inconvenient. Most notably, it requires listing all ELF > sections explicitly, and the Linux kernel has a lot of them (typically > a few dozen). > > So, my current solution is: > > 1. exec-file vmlinux > 2. info target > 3. # parse the output, adding an offset to each section's start > 4. add-symbol-file vmlinux $textaddr -s ... # a very long list > 5. exec-file # to make sure that only target memory is used > > Although I have already written a Python script to initialise the > session, it's ugly, especially when it comes to parsing the output of > "info target". Thanks for confirming. > Regarding consistency, add-symbol-file does not currently have any > conflicting "-o" option, so I can add one for the same purpose. > > Shall I do that? I think it would be a good idea. It would improve the usability of add-symbol-file and keep both commands more or less in sync, so it sounds like a win-win to me. I assume the second positional argument of add-symbol-file (which gives the start address of .text) would become optional? You just need to think carefully about what should happen when mixing the different arguments. The .text positional argument and -o would probably be mutually exclusive? -o applies to all sections, but you could use -s to override the address of a particular section? > Indeed. I tend to forget that gdb has switched to C++. Yep, you can get wild! >> >> @@ -1568,6 +1579,8 @@ symbol_file_command (const char *args, int >> >> from_tty) >> >> flags |= OBJF_READNOW; >> >> else if (strcmp (arg, "-readnever") == 0) >> >> flags |= OBJF_READNEVER; >> >> + else if (strcmp (arg, "-o") == 0) >> >> + expecting_offset = true; >> >> This doesn't handle correctly (IMO) "symbol-file foo -o", which >> should be rejected with an error message. I think it would be >> simpler to do something like this: >> >> else if (strcmp (arg, "-o") == 0) >> { >> arg = built_argv[++idx]; >> if (arg == NULL) >> error (_("Missing argument to -o")); >> >> offset = parse_and_eval_address (arg); >> } > > Ah, so that's how it's done. Honestly, I was quite surprised to find no > variant of getopt() here... I think GDB would benefit of having such a variant of getopt to standardize how arguments are parsed throughout the CLI. And to avoid having to do manual parsing like this, which often gives a fragile result. >> You can simplify these using gdb_assert: >> >> gdb_assert "${new_static_foo_addr} == ${static_foo_addr} + $offset" \ >> "static variable foo is moved by offset" > > Will do. I should probably simplify other similar stanzas in the test > suite in a separate patch. If you are up for it, it would be appreciated! > Thank you for your review! I will send an improved patch soon (but > maybe not today, as I'm at a conference). But but... it's the ideal time to work on side-quest patches! ;) Thanks, Simon