From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8429 invoked by alias); 23 Nov 2011 23:37:56 -0000 Received: (qmail 8170 invoked by uid 22791); 23 Nov 2011 23:37:54 -0000 X-SWARE-Spam-Status: No, hits=-7.2 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SPF_HELO_PASS,TW_OC X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 23 Nov 2011 23:37:35 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id pANNbUBp011545 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 23 Nov 2011 18:37:30 -0500 Received: from psique (ovpn-112-31.phx2.redhat.com [10.3.112.31]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id pANNbO9G006681; Wed, 23 Nov 2011 18:37:26 -0500 From: Sergio Durigan Junior To: "Ulrich Weigand" Cc: gdb-patches@sourceware.org, jan.kratochvil@redhat.com, pedro@codesourcery.com Subject: Re: [rfc] Options for "info mappings" etc. (Re: [PATCH] Implement new `info core mappings' command) References: <201111231632.pANGW8Ln008085@d06av02.portsmouth.uk.ibm.com> Date: Wed, 23 Nov 2011 23:37:00 -0000 In-Reply-To: <201111231632.pANGW8Ln008085@d06av02.portsmouth.uk.ibm.com> (Ulrich Weigand's message of "Wed, 23 Nov 2011 17:32:08 +0100 (CET)") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-IsSubscribed: yes 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 X-SW-Source: 2011-11/txt/msg00664.txt.bz2 Hi Ulrich, "Ulrich Weigand" writes: > Sergio Durigan Junior writes: >> > Ok, so I would like to ask for another round of review of this patch >> > then. I am aware of Ulrich's work towards a new `info mappings' >> > command, but since I've got no response from him yet, I decided to >> > continue my efforts here. >> >> Ping. > > First of all, sorry for the late reply. I ran into some issues with > implementing the proposed "info mappings" and then got side-tracked > ... No problem. > Anyway, here's my current thoughts on the issues and some options on > how to move forward. After reading the message at least three times, I like your approach in general. It seems a lot cleaner than what is current implemented. Only a few comments. > Today we have the following set of commands: > > - generate-core-file > > This uses the target_find_memory_regions callback, which is implemented > on Linux native by reading /proc/.../maps. My pending patch would have > implemented this callback on remote as well, via a new packet type > qXfer:process-map:read, implemented on Linux gdbserver by reading > /proc/.../maps > > - info proc ... (in particular, info proc mappings) > > This is implemented on Linux native target only, where it reads from > /proc directly. Note that: > * this reads a variety of other files beyond /proc/.../maps > * it can actually read from a completely different process > (info proc takes an optional PID argument) > > (There is also a separate implementation of info proc for procfs > targets, i.e. Solaris, Irix, Tru64). > > - info core ... (info core mappings and info core exe) > > This would be implemented by Sergio's patch, and available for > core file targets only (any platform). It would support only > "exe" and "mappings", and only the current core file. Maybe this is obvious, but I believe the patch can be (easily?) extended in order to support other core files than the current one. Anyway, this is probably not important now. > This state of affairs, even assuming both my and Sergio's pending > patch sets were applied, has a couple of drawbacks: > > - there is still no "info proc mappings" like command for the remote > target, even if the remote side runs gdbserver on Linux > > - there is still code duplication between the "info proc mappings" > and target_find_memory_regions implementations in linux-nat.c > > > To fix these, I had been thinking along the lines of implementing > the following set of features ("Option 1"): > > a) Extend target_find_memory_regions to provide mapped file names > This requires extending the implementation in linux-nat.c, and > updating other existing implementations and users. > > b) Implement target_find_memory_regions for remote targets > This would use xfer with a new object type TARGET_OBJECT_PROCESS_MAP, > implemented via a packet type qXfer:process-map:read providing a new > XML formatted . Implementation on gdbserver in my patch. > > c) Implement target_find_memory_regions in corefile.c > This would use techniques similar to Sergio's current patch. > > d) Implement new "info mappings" in core GDB > This would be independent of existing "info proc" commands. It would > be implemented across all targets, and simply call into the (newly > extended) target_find_memory_regions to get its data. > > > This would fix the first of the two problems mentioned above, in that > we now also have a working "info mappings" with remote gdbserver targets. > > However, we still have code duplication; in fact the duplication is now > even user-visible in the sense that we now have a generic "info mappings" > in addition to the Linux-specific "info proc mappings". > > Another drawback is that we do not have anything like Sergio's proposed > "info core exe" command; nor do have anything like "info proc ..." for > any other the *other* commands except "mappings" for remote targets. > > Finally, as a (very minor) drawback: in non-XML builds of GDB, the > "info mappings" command would not work with remote targets. > > > I have been thinking about ways to address these, and come up with one > that would basically export arbitrary /proc files via xfer ("Option 2"): > > a) Implement TARGET_OBJECT_PROC xfer object. > This would use the name of the /proc file as "Annex" (e.g. "maps" / > "exe" / "cmdline" ...). On Linux native targets, this can be directly > implemented via reads from /proc. For remote targets, this would be > implemented via a new qXfer:proc:read packet which simply returns raw > contents of the requested /proc file (no XML format required). For > core file targets, we could synthesize Linux /proc/.../maps and > /proc/.../exe contents via something like Sergio's patch. > > b) Implement gdbarch target_find_memory_regions fallback > This can be implemented on Linux targets (linux-tdep.c) via reading > TARGET_OBJECT_PROC annex "maps", which would then automatically work > for native, remote, and core file targets. The implementation in > linux-nat.c would then be superfluous. > > c) Implement generic "info proc ..." command > This would call out to a gdbarch architecture-specific implementation. > (We'd need to take care that on procfs targets, we still use the > original implementation in procfs.c instead of the generic one.) > A Linux (linux-tdep.c) implementation of that callback would then use > TARGET_OBJECT_PROC xfers (and thus work native, remote, and core). > This would make the linux-nat.c implementation superfluous. > > > This fixes all the drawbacks mentioned with Option 1 above: there are > no new commands, no more code duplication, we support "info proc exe" > for core files, and we don't even require XML. > > However, there are still problems with this option: > > - "info proc mappings" and "info proc exe" would work on core files, > but only *Linux* core files -- at least until other targets implement > equivalent support. Sergio's "info core ..." would work anywhere. > > - "info proc ..." would lose the possibility to query properties of > *other* processes except the current one (i.e. the "info proc PID" > variant could no longer be supported) This last drawback, as explained later by you, is not a big one IMO. > The first problem doesn't look really serious to me: if we wouldn't > support "info proc" on a native target, it doesn't seem important > to support it on a core file produced on that target (in particular > if the information we can synthesize is rather sparse anyway). > > The second problem also may not be really serious any more: with > current GDB, the user could always use multi-inferior support to > (temporarily) attach to the process they want to see info about, > instead of specifying the PID in the command. However, this would > indeed reflect a UI change ... > > > There is a variant of Option 2 that would actually solve that latter > problem as well: we might encode a target PID in the TARGET_OBJECT_PROC > request, e.g. by using "PID/maps" instead of plain "maps" as the Annex. > > (As minor variations, we could keep TARGET_OBJECT_PROC as-is and add > a new TARGET_OBJECT_REMOTE_PROC that takes the PID argument.) > > The only drawback of this method seems to be that it would introduce > a somewhat "alien" remote protocol packet type: all the xfer commands > usually refer to the current inferior, not some random other process > ... No, I believe we should not follow this path. Being able to display the memory mapping of one process while debugging another is not something used that often, I think. Also, there is always the possibility to use the multi-inferior feature as you mentioned above. This probably deserves a mention in the documentation, probably. Thanks, Sergio.