public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: "Ulrich Weigand" <uweigand@de.ibm.com>
To: alves.ped@gmail.com (Pedro Alves)
Cc: gdb-patches@sourceware.org, jan.kratochvil@redhat.com,
	sergiodj@redhat.com
Subject: Re: [rfc] Options for "info mappings" etc. (Re: [PATCH] Implement new `info core mappings' command)
Date: Mon, 09 Jan 2012 15:44:00 -0000	[thread overview]
Message-ID: <201201091543.q09FhnrG024010@d06av02.portsmouth.uk.ibm.com> (raw)
In-Reply-To: <4F06E8A2.4090109@gmail.com> from "Pedro Alves" at Jan 06, 2012 12:27:14 PM

Pedro Alves wrote:
> On 01/05/2012 07:53 PM, Ulrich Weigand wrote:
> > Well, I guess one could implement some heuristics along those lines
> > that do indeed work with Linux native and gdbserver targets.
> > But this would still make a number of assumptions about how those fields
> > are used -- which may be true at the moment, but not guaranteed.
> >
> > In particular, it seems to me that the remote protocol specification
> > considers TID values to be defined by the remote side; the host side
> > should only use them as identifiers without interpreting them.
> 
> Yes, that is true in general.
> 
> > While gdbserver does use the LWPID here, I don't think it is guaranteed that
> > other implementations of the remote protocol do the same, even on
> > Linux targets (i.e. where linux-tdep files get used).
> 
> ... however, let's revisit the problem with a fresh start.
> 
> We have a command "info proc FOO PID", that accepts any target process ID,
> whose description is:
> 
>   (gdb) help info proc
>   Show /proc process information about any running process.
>   Specify any process id, or use the program being debugged by default.
> 
> Let's ignore the "or use the program being debugged by default" part
> for now; it is a (big) convenience, and a separate problem, one of mapping
> the program being debugged to a process id.  (The dropping of the
> support for specifying any process id in the proposed implementations
> seems more like accommodating the implementation, so let's step back on
> that too.)

It seems to me we're getting to the core of the problem here.  For the
existing "info proc" command, it indeed makes sense to talk about
"process IDs", *because* the command is specific to a (native) target
that supports processes and uses process IDs.

Except for this, common GDB user interface commands do not currently
refer to "process IDs"; while GDB obviously used "ptid_t" internally,
it is *interpreted* only by target code, common code at most uses it
for comparions (is this the same process as another one).

[ The one obvious exception is the "attach" command which also takes
a process ID.  However, this is arguably also a target-specific command,
in that the generic implementation quickly calls through to target code
that implements the operation, starting with *parsing the argument*. ]

Instead of refering to process IDs, GDB commands usually operate on
the "current inferior", or else take GDB inferior (or thread) IDs as
argument, which have no direct connection to the underlying OS
process IDs.

It seems to me this was a deliberate decision to try and isolate the
common GDB code and user interfaces from OS implementation details:
GDB is supposed to work just the same on targets where there are no
processes, or where they are identified by different means that just
an integer ID.


Now, given the "anomaly" of the "info proc" command as is, I guess
there's two ways to look at how to move forward:

- We could say the anomaly was a mistake that we want to get rid of.
  In this view, it naturally follows that we remove the PID argument
  option and have the command operate on the "current inferior" as
  all other commands.  This also makes an underlying interface that
  would retrieve proc file content of the current inferior natural.
  (This is what my patch set has implemented.)

- We say instead that, yes, we *want* OS PIDs to be used as first-
  class user interface elements.  But this means to me that we
  need to more generally make OS PIDs present in GDB common code
  so that common code is at least able to associate its internal
  notions of "inferior" with those IDs.  At a minimum, this would
  imply we no longer consider "ptid_t" values to be opaque to
  GDB common code, but rather enforce that they agree with user-
  visible OS PID values.

Now, I'm not necessarily opposed to the second approach, I'm just
not sure I see all the consequences ...   At a minimum, the use
of the "magic 42000" becomes problematic if there is any chance
the ptid_t gets exposed to the user at any point.

> So this sub-problem can be simply specified as:
> 
>    Given a gdb inferior or thread, return it's target process id.
> 
> Now, we can come up with a new method to retrieve that info from
> the server.  However, all the Linux targets I know about that have
> /proc contents access (gdbserver), have a 1:1 mapping in the RSP
> between RSP process, and target process, so we might as well start
> out with defaulting to assume that, and add such a mechanism when
> we find a need for it.  IMO.

I don't quite understand what you mean here.  Could you elaborate
how you propose to implement this routine "return the target process
ID of a given GDB inferior/thread" without remote interface changes?
This was exactly the "magic 42000" problem I was running into ...

>  > So all in all, this still looks a violation of abstraction layers
>  > to me,
> 
> So I see this as two distinct, but related problems.  And when I look
> at the /proc retrieval part alone, I favor the remote filesystem
> access, as the most natural way to transparently make the command
> work with remote targets.  And for cores too, as you yourself
> mentioned, the kernel can put /proc contents in cores, and, I could
> imagine taking a snapshot of /proc for the whole process tree (either
> above a given process (children, siblings, etc.), or starting at
> PID 1, and storing that in, or along the core, to be something useful.

I could imaging a core file of a process containing snapshots of /proc
files *for that process*.  It would seem rather odd to me to have /proc
files of *other* processes as part of a core file.  But that's certainly
a side discussion ...

>  > which is the main reason why I advocated going back to the
>  > TARGET_OBJECT_PROC approach ...
> 
> If you're still not convinced, and others don't support my view,
> than I will step back and won't block your going forward with that.
> However, what did you think of my earlier comments regarding
> splitting that into separate objects?

I must admit I don't see what the benefit of this is supposed to be.
This seems to me to be the exact use case that "annex" is there to
cover: a bunch of information with related content semantics, which
are all accessed the same way, and the exact set is somewhat dynamic.

Not using the annex would mean defining a whole bunch of new packet
types, duplicated boilerplate code in GDB and gdbserver to hook them
up, and then still the drawback that every new /proc file that may
come up in the future will require extra code (including new gdbserver-side
code!) to support.  And for all those drawbacks, I don't see any single
benefit ...  Maybe you can elaborate?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

  reply	other threads:[~2012-01-09 15:44 UTC|newest]

Thread overview: 83+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-26 21:08 [PATCH] Implement new `info core mappings' command Sergio Durigan Junior
2011-10-26 21:25 ` Sergio Durigan Junior
2011-10-27  7:30   ` Eli Zaretskii
2011-10-27 18:09     ` Sergio Durigan Junior
2011-10-29 19:48       ` Eli Zaretskii
2011-10-31  0:34 ` Jan Kratochvil
2011-10-31  7:00   ` Sergio Durigan Junior
2011-10-31  8:13     ` Jan Kratochvil
2011-10-31 12:57       ` Pedro Alves
2011-11-01 11:54         ` [patch] `info proc ' completion [Re: [PATCH] Implement new `info core mappings' command] Jan Kratochvil
2011-11-01 16:23           ` Eli Zaretskii
2011-11-03 14:12             ` [patch] `info proc *' help fix [Re: [patch] `info proc ' completion] Jan Kratochvil
2011-11-03 16:57               ` Eli Zaretskii
2011-11-03 17:07                 ` Jan Kratochvil
2011-11-03 18:08                   ` Eli Zaretskii
2011-11-03 18:25                     ` Jan Kratochvil
2011-11-02 18:30           ` [patch] `info proc ' completion [Re: [PATCH] Implement new `info core mappings' command] Pedro Alves
2011-11-02 18:48             ` [commit] " Jan Kratochvil
2011-11-03 20:01       ` [PATCH] Implement new `info core mappings' command Sergio Durigan Junior
2011-11-04 10:38         ` Eli Zaretskii
2011-11-04 16:27         ` Jan Kratochvil
2011-11-08  1:49           ` Sergio Durigan Junior
2011-11-08 21:47             ` Jan Kratochvil
2011-11-09 20:32             ` Jan Kratochvil
2011-11-16  4:10               ` Sergio Durigan Junior
2011-11-21 16:15                 ` Sergio Durigan Junior
2011-11-23 16:32                   ` [rfc] Options for "info mappings" etc. (Re: [PATCH] Implement new `info core mappings' command) Ulrich Weigand
2011-11-23 23:37                     ` Sergio Durigan Junior
2011-12-01 19:51                       ` Ulrich Weigand
2011-12-05 12:59                     ` Pedro Alves
2011-12-05 15:02                       ` Ulrich Weigand
2011-12-06 16:01                         ` Pedro Alves
2011-12-06 17:19                           ` Ulrich Weigand
2011-12-07 16:29                             ` Pedro Alves
2011-12-07 17:24                               ` Pedro Alves
2011-12-07 20:14                               ` Ulrich Weigand
2011-12-09 13:28                                 ` Pedro Alves
2011-12-09 14:10                                   ` Pedro Alves
2011-12-20 23:08                                 ` Ulrich Weigand
2011-12-21 22:36                                   ` Jan Kratochvil
2011-12-22 16:15                                     ` Ulrich Weigand
2012-01-05 16:02                                   ` Pedro Alves
2012-01-05 18:03                                     ` Ulrich Weigand
2012-01-05 18:20                                       ` Pedro Alves
2012-01-05 19:54                                         ` Ulrich Weigand
2012-01-06  6:41                                           ` Joel Brobecker
2012-01-06 12:29                                             ` Pedro Alves
2012-01-06 12:27                                           ` Pedro Alves
2012-01-09 15:44                                             ` Ulrich Weigand [this message]
2012-01-11 16:38                                               ` Pedro Alves
2012-01-11 18:32                                                 ` Ulrich Weigand
2012-01-05 18:37                                       ` Mark Kettenis
2012-01-05 19:35                                         ` Ulrich Weigand
  -- strict thread matches above, loose matches on Subject: below --
2012-04-06  3:28 [PATCH 0/4 v2] Implement support for SystemTap probes on userspace Sergio Durigan Junior
2012-04-06  3:32 ` [PATCH 1/4 v2] Refactor internal variable mechanism Sergio Durigan Junior
2012-04-06  3:36 ` [PATCH 2/4 v2] Implement new features needed for handling SystemTap probes Sergio Durigan Junior
2012-04-11 19:06   ` Jan Kratochvil
2012-04-11 22:14     ` Sergio Durigan Junior
2012-04-11 23:33       ` Jan Kratochvil
2012-04-06  3:37 ` [PATCH 4/4 v2] Documentation and testsuite changes Sergio Durigan Junior
2012-04-06  9:27   ` Eli Zaretskii
2012-04-09 21:37     ` Sergio Durigan Junior
2012-04-06  4:11 ` [PATCH 3/4 v2] Use longjmp and exception probes when available Sergio Durigan Junior
2011-04-04  3:09 [PATCH 4/6] Implement support for SystemTap probes Sergio Durigan Junior
2011-04-04 19:06 ` Eli Zaretskii
2011-04-06 20:20 ` Tom Tromey
2011-04-06 20:52   ` Sergio Durigan Junior
2011-04-07  2:41 ` Yao Qi
2011-04-07  3:32   ` Sergio Durigan Junior
2011-04-07 17:04   ` Tom Tromey
2011-04-11  3:21     ` Yao Qi
2011-04-08 12:38   ` Sergio Durigan Junior
2011-04-11  3:52     ` Yao Qi
2011-08-12 15:45     ` Jan Kratochvil
2011-08-12 17:22       ` Frank Ch. Eigler
2011-08-12 21:33         ` Sergio Durigan Junior
2011-04-19 16:42 ` Jan Kratochvil
2012-05-07 19:36   ` Jan Kratochvil
2012-05-07 19:54     ` Sergio Durigan Junior
2012-05-07 19:58       ` Jan Kratochvil
2012-05-07 20:26         ` Sergio Durigan Junior
2012-05-07 20:38           ` Jan Kratochvil
2012-05-08  1:36             ` Sergio Durigan Junior

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201201091543.q09FhnrG024010@d06av02.portsmouth.uk.ibm.com \
    --to=uweigand@de.ibm.com \
    --cc=alves.ped@gmail.com \
    --cc=gdb-patches@sourceware.org \
    --cc=jan.kratochvil@redhat.com \
    --cc=sergiodj@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).