From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5496 invoked by alias); 11 Jan 2012 16:04:48 -0000 Received: (qmail 5466 invoked by uid 22791); 11 Jan 2012 16:04:36 -0000 X-SWARE-Spam-Status: No, hits=-6.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_FRT_PROFILE2,T_RP_MATCHES_RCVD 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, 11 Jan 2012 16:04:11 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q0BG491G010092 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 11 Jan 2012 11:04:09 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q0BG46We020448; Wed, 11 Jan 2012 11:04:06 -0500 Message-ID: <4F0DB2F6.7050807@redhat.com> Date: Wed, 11 Jan 2012 16:38:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:9.0) Gecko/20111222 Thunderbird/9.0 MIME-Version: 1.0 To: Ulrich Weigand 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) References: <201201091543.q09FhnrG024010@d06av02.portsmouth.uk.ibm.com> In-Reply-To: <201201091543.q09FhnrG024010@d06av02.portsmouth.uk.ibm.com> Content-Type: multipart/mixed; boundary="------------030001050106010303090204" 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: 2012-01/txt/msg00354.txt.bz2 This is a multi-part message in MIME format. --------------030001050106010303090204 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-length: 8087 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 > --------------030001050106010303090204 Content-Type: text/x-patch; name="01-target_fileio.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="01-target_fileio.diff" Content-length: 5080 Expose the fileio methods through the target vector. From: Pedro Alves --- 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. */ --------------030001050106010303090204 Content-Type: text/x-patch; name="02-proc_access_target.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="02-proc_access_target.diff" Content-length: 7502 Adjust linux's "info proc" command to read the /proc files from the target. From: Pedro Alves 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 --------------030001050106010303090204 Content-Type: text/x-patch; name="03-process_id.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="03-process_id.diff" Content-length: 4864 Record in `struct inferior' whether the target process id we have has been faked by GDB or not. From: Pedro Alves --- 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); --------------030001050106010303090204 Content-Type: text/x-patch; name="04-allow_multi_on_remote.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="04-allow_multi_on_remote.diff" Content-length: 3066 Allow the remote protocol thread id extension on plain target remote. From: Pedro Alves --- 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 --------------030001050106010303090204 Content-Type: text/x-patch; name="05-abstract_if_necessary.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="05-abstract_if_necessary.diff" Content-length: 4483 Hide getting at the current process's PID behind a target method. From: Pedro Alves --- 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. */ --------------030001050106010303090204 Content-Type: text/plain; name="series" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="series" Content-length: 128 01-target_fileio.diff 02-proc_access_target.diff 03-process_id.diff 04-allow_multi_on_remote.diff 05-abstract_if_necessary.diff --------------030001050106010303090204--