This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [rfc] Options for "info mappings" etc. (Re: [PATCH] Implement new `info core mappings' command)


On 01/09/2012 03:43 PM, Ulrich Weigand wrote:
> 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.

Note that I'm not talking about the notion of a "process id" escape
escape to common/core code, but only to the linux gdbarch/osabi
specific bits; or other code that can deal with it, guarded by
"is there a notion of a process id on this target.

> 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 ...

As mentioned before, by broadcasting support for multi-process extensions
even with "target remote" (but don't allow debugging multiple processes),
and defaulting to assume a 1:1 mapping between target process id
and RSP process id, until some target needs to do something else, at
which point we define a new packet.

I've spent a bit today prototyping /proc access this way the way
I was imagining it.  See the attached patch series.  This is far from
complete.  It's just enough to get something working.

> 
>>  > 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?

- Decoupling of the objects in question from a "/proc" idea, so they
can be more generally used in other scenarios, like e.g., a remote
protocol implementation of target_pid_to_str (TARGET_OBJECT_PROC/exe).
- Let GDB have a finer grained idea of what subset of /proc-ish objects
are supported upfront (through qSupported), without any new mechanism.

> 
> Bye,
> Ulrich
> 
Expose the fileio methods through the target vector.

From: Pedro Alves <palves@redhat.com>


---
 gdb/remote.c |    6 ++++++
 gdb/target.c |   25 +++++++++++++++++++++++++
 gdb/target.h |   47 ++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 77 insertions(+), 1 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 60d7ecd..7542882 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -10684,6 +10684,12 @@ Specify the serial device it is connected to\n\
   remote_ops.to_static_tracepoint_markers_by_strid
     = remote_static_tracepoint_markers_by_strid;
   remote_ops.to_traceframe_info = remote_traceframe_info;
+
+  remote_ops.to_file_open = remote_hostio_open;
+  remote_ops.to_file_pwrite = remote_hostio_pwrite;
+  remote_ops.to_file_pread = remote_hostio_pread;
+  remote_ops.to_file_close = remote_hostio_close;
+  remote_ops.to_file_unlink = remote_hostio_unlink;
 }
 
 /* Set up the extended remote vector by making a copy of the standard
diff --git a/gdb/target.c b/gdb/target.c
index 9aaa0ea..74238a5 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -697,6 +697,13 @@ update_current_target (void)
       INHERIT (to_static_tracepoint_marker_at, t);
       INHERIT (to_static_tracepoint_markers_by_strid, t);
       INHERIT (to_traceframe_info, t);
+
+      INHERIT (to_file_open, t);
+      INHERIT (to_file_pwrite, t);
+      INHERIT (to_file_pread, t);
+      INHERIT (to_file_close, t);
+      INHERIT (to_file_unlink, t);
+
       INHERIT (to_magic, t);
       /* Do not inherit to_memory_map.  */
       /* Do not inherit to_flash_erase.  */
@@ -926,6 +933,24 @@ update_current_target (void)
 	    tcomplain);
   de_fault (to_execution_direction, default_execution_direction);
 
+  de_fault (to_file_open,
+	    (int (*) (const char *, int, int, int *))
+	    tcomplain);
+  de_fault (to_file_pwrite,
+	    (int (*) (int, const gdb_byte *, int,
+		      ULONGEST, int *))
+	    tcomplain);
+  de_fault (to_file_pread,
+	    (int (*) (int, gdb_byte *, int,
+		      ULONGEST, int *))
+	    tcomplain);
+  de_fault (to_file_close,
+	    (int (*) (int, int *))
+	    tcomplain);
+  de_fault (to_file_unlink,
+	    (int (*) (const char *, int *))
+	    tcomplain);
+
 #undef de_fault
 
   /* Finally, position the target-stack beneath the squashed
diff --git a/gdb/target.h b/gdb/target.h
index 7d0bed1..96176a5 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -795,6 +795,32 @@ struct target_ops
        re-fetching when necessary.  */
     struct traceframe_info *(*to_traceframe_info) (void);
 
+    /* Open FILENAME on the target, using FLAGS and MODE.  Return a
+       target file descriptor, or -1 if an error occurs (and set
+       *TARGET_ERRNO).  */
+    int (*to_file_open) (const char *filename, int flags, int mode,
+		      int *target_errno);
+
+    /* Write up to LEN bytes from WRITE_BUF to FD on the target.
+       Return the number of bytes written, or -1 if an error occurs
+       (and set *TARGET_ERRNO).  */
+    int (*to_file_pwrite) (int fd, const gdb_byte *write_buf, int len,
+			ULONGEST offset, int *target_errno);
+
+    /* Read up to LEN bytes FD on the target into READ_BUF.  Return
+       the number of bytes read, or -1 if an error occurs (and set
+       *TARGET_ERRNO).  */
+    int (*to_file_pread) (int fd, gdb_byte *read_buf, int len,
+		       ULONGEST offset, int *target_errno);
+
+    /* Close FD on the target.  Return 0, or -1 if an error occurs
+       (and set *TARGET_ERRNO).  */
+    int (*to_file_close) (int fd, int *target_errno);
+
+    /* Unlink FILENAME on the target.  Return 0, or -1 if an error
+       occurs (and set *TARGET_ERRNO).  */
+    int (*to_file_unlink) (const char *filename, int *target_errno);
+
     int to_magic;
     /* Need sub-structure for target machine related rather than comm related?
      */
@@ -1493,7 +1519,6 @@ extern int target_search_memory (CORE_ADDR start_addr,
 
 #define target_trace_init() \
   (*current_target.to_trace_init) ()
-
 #define target_download_tracepoint(t) \
   (*current_target.to_download_tracepoint) (t)
 
@@ -1744,6 +1769,26 @@ extern int may_stop;
 
 extern void update_target_permissions (void);
 
+/* See to_file_open for description.  */
+#define target_file_open(filename, flags, mode, target_errno)		\
+  (*current_target.to_file_open) (filename, flags, mode, target_errno)
+
+/* See to_file_pwrite for description.  */
+#define target_file_pwrite(fd, write_buf, len, offset, target_errno)	\
+  (*current_target.to_file_pwrite) (fd, write_buf, len, offset, target_errno)
+
+/* See to_file_pread for description.  */
+#define target_file_pread(fd, read_buf, len, offset, target_errno)	\
+  (*current_target.to_file_pread) (fd, read_buf, len, offset, target_errno)
+
+/* See to_file_close for description.  */
+#define target_file_close(fd, target_errno)		\
+  (*current_target.to_file_close) (fd, target_errno)
+
+/* See to_file_unlink for description.  */
+#define target_file_unlink(filename, target_errno)		\
+  (*current_target.to_file_close) (filename, target_errno)
+
 
 /* Imported from machine dependent code.  */
 
Adjust linux's "info proc" command to read the /proc files from the target.

From: Pedro Alves <palves@redhat.com>

Obviously not a complete patch.  The command' implementation would
need to move behind a gdbarch callback, and only one /proc file access
has been adjusted.
---
 gdb/linux-nat.c |  203 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 195 insertions(+), 8 deletions(-)

diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index f6b36a2..d333c44 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4830,13 +4830,190 @@ enum info_proc_what
     IP_ALL
   };
 
+#include "gdb/fileio.h"
+
+static int
+fileio_open_flags_to_host (int fileio_open_flags, int *open_flags_p)
+{
+  int open_flags = 0;
+
+  if (fileio_open_flags & ~FILEIO_O_SUPPORTED)
+    return -1;
+
+  if (fileio_open_flags & FILEIO_O_CREAT)
+    open_flags |= O_CREAT;
+  if (fileio_open_flags & FILEIO_O_EXCL)
+    open_flags |= O_EXCL;
+  if (fileio_open_flags & FILEIO_O_TRUNC)
+    open_flags |= O_TRUNC;
+  if (fileio_open_flags & FILEIO_O_APPEND)
+    open_flags |= O_APPEND;
+  if (fileio_open_flags & FILEIO_O_RDONLY)
+    open_flags |= O_RDONLY;
+  if (fileio_open_flags & FILEIO_O_WRONLY)
+    open_flags |= O_WRONLY;
+  if (fileio_open_flags & FILEIO_O_RDWR)
+    open_flags |= O_RDWR;
+/* On systems supporting binary and text mode, always open files in
+   binary mode. */
+#ifdef O_BINARY
+  open_flags |= O_BINARY;
+#endif
+
+  *open_flags_p = open_flags;
+  return 0;
+}
+
+static int
+errno_to_fileio_error (int error)
+{
+  switch (error)
+    {
+    case EPERM:
+      return FILEIO_EPERM;
+    case ENOENT:
+      return FILEIO_ENOENT;
+    case EINTR:
+      return FILEIO_EINTR;
+    case EIO:
+      return FILEIO_EIO;
+    case EBADF:
+      return FILEIO_EBADF;
+    case EACCES:
+      return FILEIO_EACCES;
+    case EFAULT:
+      return FILEIO_EFAULT;
+    case EBUSY:
+      return FILEIO_EBUSY;
+    case EEXIST:
+      return FILEIO_EEXIST;
+    case ENODEV:
+      return FILEIO_ENODEV;
+    case ENOTDIR:
+      return FILEIO_ENOTDIR;
+    case EISDIR:
+      return FILEIO_EISDIR;
+    case EINVAL:
+      return FILEIO_EINVAL;
+    case ENFILE:
+      return FILEIO_ENFILE;
+    case EMFILE:
+      return FILEIO_EMFILE;
+    case EFBIG:
+      return FILEIO_EFBIG;
+    case ENOSPC:
+      return FILEIO_ENOSPC;
+    case ESPIPE:
+      return FILEIO_ESPIPE;
+    case EROFS:
+      return FILEIO_EROFS;
+    case ENOSYS:
+      return FILEIO_ENOSYS;
+    case ENAMETOOLONG:
+      return FILEIO_ENAMETOOLONG;
+    }
+
+  return FILEIO_EUNKNOWN;
+}
+
+static int
+linux_nat_file_open (const char *filename, int flags, int mode,
+		    int *target_errno)
+{
+  int nat_flags;
+  int fd;
+
+  if (fileio_open_flags_to_host (flags, &nat_flags) == -1)
+    {
+      *target_errno = FILEIO_EINVAL;
+      return -1;
+    }
+
+  /* We do not need to convert MODE, since the fileio protocol uses
+     the standard values.  */
+  fd = open (filename, nat_flags, mode);
+  if (fd == -1)
+    *target_errno = errno_to_fileio_error (errno);
+  return fd;
+}
+
+static int
+linux_nat_file_pwrite (int fd, const gdb_byte *write_buf, int len,
+		      ULONGEST offset, int *target_errno)
+{
+  int ret;
+
+#ifdef HAVE_PWRITE
+  ret = pwrite (fd, write_buf, len, offset);
+#else
+  ret = lseek (fd, offset, SEEK_SET);
+  if (ret != -1)
+    ret = write (fd, write_buf, len);
+#endif
+
+  if (ret == -1)
+    *target_errno = errno_to_fileio_error (errno);
+  return ret;
+}
+
+static int
+linux_nat_file_pread (int fd, gdb_byte *read_buf, int len,
+		     ULONGEST offset, int *target_errno)
+{
+  int ret;
+
+#ifdef HAVE_PREAD
+  ret = pread (fd, read_buf, len, offset);
+#else
+  ret = lseek (fd, offset, SEEK_SET);
+  if (ret != -1)
+    ret = read (fd, read_buf, len);
+#endif
+
+  if (ret == -1)
+    *target_errno = errno_to_fileio_error (errno);
+
+  return ret;
+}
+
+static int
+linux_nat_file_close (int fd, int *target_errno)
+{
+  int ret;
+
+  ret = close (fd);
+  if (ret == -1)
+    *target_errno = errno_to_fileio_error (errno);
+
+  return ret;
+}
+
+static void
+do_target_file_close (void *arg)
+{
+  int fd = *(int *) arg;
+  int target_errno;
+
+  target_file_close (fd, &target_errno);
+  xfree (arg);
+}
+
+static struct cleanup *
+make_cleanup_target_file_close (int fd)
+{
+  int *fdp = XNEW (int);
+
+  *fdp = fd;
+  return make_cleanup (do_target_file_close, fdp);
+}
+
 static void
 linux_nat_info_proc_cmd_1 (char *args, enum info_proc_what what, int from_tty)
 {
   /* A long is used for pid instead of an int to avoid a loss of precision
      compiler warning from the output of strtoul.  */
   long pid = PIDGET (inferior_ptid);
-  FILE *procfile;
+  int procfile;
   char buffer[MAXPATHLEN];
   char fname1[MAXPATHLEN], fname2[MAXPATHLEN];
   int cmdline_f = (what == IP_MINIMAL || what == IP_CMDLINE || what == IP_ALL);
@@ -4846,6 +5023,7 @@ linux_nat_info_proc_cmd_1 (char *args, enum info_proc_what what, int from_tty)
   int status_f = (what == IP_STATUS || what == IP_ALL);
   int stat_f = (what == IP_STAT || what == IP_ALL);
   struct stat dummy;
+  int target_errno;
 
   if (args && isdigit (args[0]))
     pid = strtoul (args, &args, 10);
@@ -4857,19 +5035,17 @@ linux_nat_info_proc_cmd_1 (char *args, enum info_proc_what what, int from_tty)
   if (pid == 0)
     error (_("No current process: you must name one."));
 
-  sprintf (fname1, "/proc/%ld", pid);
-  if (stat (fname1, &dummy) != 0)
-    error (_("No /proc directory: '%s'"), fname1);
-
   printf_filtered (_("process %ld\n"), pid);
   if (cmdline_f)
     {
       sprintf (fname1, "/proc/%ld/cmdline", pid);
-      if ((procfile = fopen (fname1, "r")) != NULL)
+      if ((procfile = target_file_open (fname1, FILEIO_O_RDONLY,
+					0, &target_errno)) != -1)
 	{
-	  struct cleanup *cleanup = make_cleanup_fclose (procfile);
+	  struct cleanup *cleanup = make_cleanup_target_file_close (procfile);
 
-          if (fgets (buffer, sizeof (buffer), procfile))
+          if (target_file_pread (procfile, buffer, sizeof (buffer),
+				 0, &target_errno) != -1)
             printf_filtered ("cmdline = '%s'\n", buffer);
           else
             warning (_("unable to read '%s'"), fname1);
@@ -4898,6 +5074,8 @@ linux_nat_info_proc_cmd_1 (char *args, enum info_proc_what what, int from_tty)
     }
   if (mappings_f)
     {
+      FILE *procfile;
+
       sprintf (fname1, "/proc/%ld/maps", pid);
       if ((procfile = fopen (fname1, "r")) != NULL)
 	{
@@ -4960,6 +5138,8 @@ linux_nat_info_proc_cmd_1 (char *args, enum info_proc_what what, int from_tty)
     }
   if (status_f)
     {
+      FILE *procfile;
+
       sprintf (fname1, "/proc/%ld/status", pid);
       if ((procfile = fopen (fname1, "r")) != NULL)
 	{
@@ -4974,6 +5154,8 @@ linux_nat_info_proc_cmd_1 (char *args, enum info_proc_what what, int from_tty)
     }
   if (stat_f)
     {
+      FILE *procfile;
+
       sprintf (fname1, "/proc/%ld/stat", pid);
       if ((procfile = fopen (fname1, "r")) != NULL)
 	{
@@ -5880,6 +6062,11 @@ linux_nat_add_target (struct target_ops *t)
 
   t->to_core_of_thread = linux_nat_core_of_thread;
 
+  t->to_file_open = linux_nat_file_open;
+  t->to_file_pwrite = linux_nat_file_pwrite;
+  t->to_file_pread = linux_nat_file_pread;
+  t->to_file_close = linux_nat_file_close;
+
   /* We don't change the stratum; this target will sit at
      process_stratum and thread_db will set at thread_stratum.  This
      is a little strange, since this is a multi-threaded-capable
Record in `struct inferior' whether the target process id we have has been faked by GDB or not.

From: Pedro Alves <palves@redhat.com>


---
 gdb/inferior.c  |    1 +
 gdb/inferior.h  |    2 ++
 gdb/linux-nat.c |   18 ++++++++++++++----
 gdb/remote.c    |   42 ++++++++++++++++++++++++++++++++----------
 4 files changed, 49 insertions(+), 14 deletions(-)

diff --git a/gdb/inferior.c b/gdb/inferior.c
index 65948c4..4df8c77 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -276,6 +276,7 @@ exit_inferior_1 (struct inferior *inftoex, int silent)
   observer_notify_inferior_exit (inf);
 
   inf->pid = 0;
+  inf->fake_pid_p = 0;
   if (inf->vfork_parent != NULL)
     {
       inf->vfork_parent->vfork_child = NULL;
diff --git a/gdb/inferior.h b/gdb/inferior.h
index f05789f..7857cbf 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -421,6 +421,8 @@ struct inferior
   /* Actual target inferior id, usually, a process id.  This matches
      the ptid_t.pid member of threads of this inferior.  */
   int pid;
+  /* True if the PID was actually faked by GDB.  */
+  int fake_pid_p;
 
   /* State of GDB control of inferior process execution.
      See `struct inferior_control_state'.  */
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index d333c44..17236ec 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -5012,7 +5012,8 @@ linux_nat_info_proc_cmd_1 (char *args, enum info_proc_what what, int from_tty)
 {
   /* A long is used for pid instead of an int to avoid a loss of precision
      compiler warning from the output of strtoul.  */
-  long pid = PIDGET (inferior_ptid);
+  long pid = 0;
+  int explicit_pid = 0;
   int procfile;
   char buffer[MAXPATHLEN];
   char fname1[MAXPATHLEN], fname2[MAXPATHLEN];
@@ -5026,14 +5027,23 @@ linux_nat_info_proc_cmd_1 (char *args, enum info_proc_what what, int from_tty)
   int target_errno;
 
   if (args && isdigit (args[0]))
-    pid = strtoul (args, &args, 10);
+    {
+      pid = strtoul (args, &args, 10);
+      explicit_pid = 1;
+    }
 
   args = skip_spaces (args);
   if (args && args[0])
     error (_("Too many parameters: %s"), args);
 
-  if (pid == 0)
-    error (_("No current process: you must name one."));
+  if (!explicit_pid)
+    {
+      if (!target_has_execution)
+	error (_("No current process: you must name one."));
+      if (current_inferior ()->fake_pid_p)
+	error (_("Can't determine the current process's PID: you must name one."));
+      pid = ptid_get_pid (inferior_ptid);
+    }
 
   printf_filtered (_("process %ld\n"), pid);
   if (cmdline_f)
diff --git a/gdb/remote.c b/gdb/remote.c
index 7542882..c5c9481 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -3253,6 +3253,10 @@ remote_start_remote (int from_tty, struct target_ops *target, int extended_p)
 
   if (!non_stop)
     {
+      ptid_t ptid;
+      int fake_pid_p = 0;
+      struct inferior *inf;
+
       if (rs->buf[0] == 'W' || rs->buf[0] == 'X')
 	{
 	  if (!extended_p)
@@ -3272,19 +3276,37 @@ remote_start_remote (int from_tty, struct target_ops *target, int extended_p)
       /* Let the stub know that we want it to return the thread.  */
       set_continue_thread (minus_one_ptid);
 
-      /* Without this, some commands which require an active target
-	 (such as kill) won't work.  This variable serves (at least)
-	 double duty as both the pid of the target process (if it has
-	 such), and as a flag indicating that a target is active.
-	 These functions should be split out into seperate variables,
-	 especially since GDB will someday have a notion of debugging
-	 several processes.  */
-      inferior_ptid = magic_null_ptid;
+      inferior_ptid = minus_one_ptid;
 
       /* Now, if we have thread information, update inferior_ptid.  */
-      inferior_ptid = remote_current_thread (inferior_ptid);
+      ptid = remote_current_thread (inferior_ptid);
+      if (!ptid_equal (ptid, minus_one_ptid))
+	{
+	  if (ptid_get_pid (ptid) == -1)
+	    {
+	      ptid = ptid_build (ptid_get_pid (magic_null_ptid),
+				 ptid_get_lwp (ptid),
+				 ptid_get_tid (ptid));
+	      fake_pid_p = 1;
+	    }
+
+	  inferior_ptid = ptid;
+	}
+      else
+	{
+	  /* Without this, some commands which require an active
+	     target (such as kill) won't work.  This variable serves
+	     (at least) double duty as both the pid of the target
+	     process (if it has such), and as a flag indicating that a
+	     target is active.  These functions should be split out
+	     into seperate variables, especially since GDB will
+	     someday have a notion of debugging several processes.  */
+	  inferior_ptid = magic_null_ptid;
+	  fake_pid_p = 1;
+	}
 
-      remote_add_inferior (ptid_get_pid (inferior_ptid), -1);
+      inf = remote_add_inferior (ptid_get_pid (inferior_ptid), -1);
+      inf->fake_pid_p = fake_pid_p;
 
       /* Always add the main thread.  */
       add_thread_silent (inferior_ptid);
Allow the remote protocol thread id extension on plain target remote.

From: Pedro Alves <palves@redhat.com>


---
 gdb/gdbserver/server.c |    4 ++--
 gdb/remote.c           |   15 +++++++++------
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index a3bc6c9..5ae6795 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -2140,7 +2140,7 @@ handle_v_requests (char *own_buf, int packet_len, int *new_packet_len)
 
   if (strncmp (own_buf, "vAttach;", 8) == 0)
     {
-      if (!multi_process && target_running ())
+      if ((!extended_protocol || !multi_process) && target_running ())
 	{
 	  fprintf (stderr, "Already debugging a process\n");
 	  write_enn (own_buf);
@@ -2152,7 +2152,7 @@ handle_v_requests (char *own_buf, int packet_len, int *new_packet_len)
 
   if (strncmp (own_buf, "vRun;", 5) == 0)
     {
-      if (!multi_process && target_running ())
+      if ((!extended_protocol || !multi_process) && target_running ())
 	{
 	  fprintf (stderr, "Already debugging a process\n");
 	  write_enn (own_buf);
diff --git a/gdb/remote.c b/gdb/remote.c
index c5c9481..e78853c 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -359,7 +359,7 @@ free_private_thread_info (struct private_thread_info *info)
 static int
 remote_multi_process_p (struct remote_state *rs)
 {
-  return rs->extended && rs->multi_process_aware;
+  return rs->multi_process_aware;
 }
 
 /* This data could be associated with a target, but we do not always
@@ -1713,7 +1713,7 @@ set_general_process (void)
   struct remote_state *rs = get_remote_state ();
 
   /* If the remote can't handle multiple processes, don't bother.  */
-  if (!remote_multi_process_p (rs))
+  if (!rs->extended || !remote_multi_process_p (rs))
     return;
 
   /* We only need to change the remote current thread if it's pointing
@@ -3885,8 +3885,7 @@ remote_query_supported (void)
       char *q = NULL;
       struct cleanup *old_chain = make_cleanup (free_current_contents, &q);
 
-      if (rs->extended)
-	q = remote_query_supported_append (q, "multiprocess+");
+      q = remote_query_supported_append (q, "multiprocess+");
 
       if (remote_support_xml)
 	q = remote_query_supported_append (q, remote_support_xml);
@@ -7440,7 +7439,7 @@ extended_remote_kill (struct target_ops *ops)
   struct remote_state *rs = get_remote_state ();
 
   res = remote_vkill (pid, rs);
-  if (res == -1 && !remote_multi_process_p (rs))
+  if (res == -1 && !(rs->extended && remote_multi_process_p (rs)))
     {
       /* Don't try 'k' on a multi-process aware stub -- it has no way
 	 to specify the pid.  */
@@ -9773,7 +9772,11 @@ remote_supports_multi_process (void)
 {
   struct remote_state *rs = get_remote_state ();
 
-  return remote_multi_process_p (rs);
+  /* Only extended-remote handles being attached to multiple
+     processes, even though plain remote can use the multi-process
+     thread id extensions, so that GDB knows the target process's
+     PID.  */
+  return rs->extended && remote_multi_process_p (rs);
 }
 
 int
Hide getting at the current process's PID behind a target method.

From: Pedro Alves <palves@redhat.com>


---
 gdb/linux-nat.c |   18 ++++++++++++++++--
 gdb/remote.c    |   26 ++++++++++++++++++++++++++
 gdb/target.c    |    5 +++++
 gdb/target.h    |    4 ++++
 4 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 17236ec..1d06352 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -5007,6 +5007,16 @@ make_cleanup_target_file_close (int fd)
   return make_cleanup (do_target_file_close, fdp);
 }
 
+static int
+linux_nat_current_process_id (LONGEST *pid)
+{
+  if (!target_has_execution)
+    noprocess ();
+
+  *pid = ptid_get_pid (inferior_ptid);
+  return 0;
+}
+
 static void
 linux_nat_info_proc_cmd_1 (char *args, enum info_proc_what what, int from_tty)
 {
@@ -5038,11 +5048,13 @@ linux_nat_info_proc_cmd_1 (char *args, enum info_proc_what what, int from_tty)
 
   if (!explicit_pid)
     {
+      LONGEST tpid;
+
       if (!target_has_execution)
 	error (_("No current process: you must name one."));
-      if (current_inferior ()->fake_pid_p)
+      if (target_current_process_id (&tpid) == -1)
 	error (_("Can't determine the current process's PID: you must name one."));
-      pid = ptid_get_pid (inferior_ptid);
+      pid = tpid;
     }
 
   printf_filtered (_("process %ld\n"), pid);
@@ -6077,6 +6089,8 @@ linux_nat_add_target (struct target_ops *t)
   t->to_file_pread = linux_nat_file_pread;
   t->to_file_close = linux_nat_file_close;
 
+  t->to_current_process_id = linux_nat_current_process_id;
+
   /* We don't change the stratum; this target will sit at
      process_stratum and thread_db will set at thread_stratum.  This
      is a little strange, since this is a multi-threaded-capable
diff --git a/gdb/remote.c b/gdb/remote.c
index e78853c..2bd7d13 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -9779,6 +9779,30 @@ remote_supports_multi_process (void)
   return rs->extended && remote_multi_process_p (rs);
 }
 
+/* Return in *PID the current inferior's target process ID.  Returns 0
+   on success, and -1 if not possible to determine the target PID.
+   Throws an error if there is no current inferior.  */
+
+static int
+remote_current_process_id (LONGEST *pid)
+{
+  if (!target_has_execution)
+    noprocess ();
+
+  /* Add query to remote target side here if/when necessary.  */
+
+  /* Default to assuming there's a 1:1 mapping between RSP process ID
+     and target process ID.  */
+
+  /* With no multi-process extensions, we have nothing to fall back
+     to.  */
+  if (current_inferior ()->fake_pid_p)
+    return -1;
+
+  *pid = ptid_get_pid (inferior_ptid);
+  return 0;
+}
+
 int
 remote_supports_cond_tracepoints (void)
 {
@@ -10715,6 +10739,8 @@ Specify the serial device it is connected to\n\
   remote_ops.to_file_pread = remote_hostio_pread;
   remote_ops.to_file_close = remote_hostio_close;
   remote_ops.to_file_unlink = remote_hostio_unlink;
+
+  remote_ops.to_current_process_id = remote_current_process_id;
 }
 
 /* Set up the extended remote vector by making a copy of the standard
diff --git a/gdb/target.c b/gdb/target.c
index 74238a5..1422117 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -704,6 +704,8 @@ update_current_target (void)
       INHERIT (to_file_close, t);
       INHERIT (to_file_unlink, t);
 
+      INHERIT (to_current_process_id, t);
+
       INHERIT (to_magic, t);
       /* Do not inherit to_memory_map.  */
       /* Do not inherit to_flash_erase.  */
@@ -950,6 +952,9 @@ update_current_target (void)
   de_fault (to_file_unlink,
 	    (int (*) (const char *, int *))
 	    tcomplain);
+  de_fault (to_current_process_id,
+	    (int (*) (LONGEST *))
+	    tcomplain);
 
 #undef de_fault
 
diff --git a/gdb/target.h b/gdb/target.h
index 96176a5..449be04 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -821,6 +821,7 @@ struct target_ops
        occurs (and set *TARGET_ERRNO).  */
     int (*to_file_unlink) (const char *filename, int *target_errno);
 
+    int (*to_current_process_id) (LONGEST *pid);
     int to_magic;
     /* Need sub-structure for target machine related rather than comm related?
      */
@@ -1789,6 +1790,9 @@ extern void update_target_permissions (void);
 #define target_file_unlink(filename, target_errno)		\
   (*current_target.to_file_close) (filename, target_errno)
 
+#define target_current_process_id(PID)		\
+  (*current_target.to_current_process_id) (PID)
+
 
 /* Imported from machine dependent code.  */
 

Attachment: series
Description: Text document


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]