[PATCH/RFC 02/02 v2] Refactor PRPSINFO handling on GDB

Sergio Durigan Junior
2012-12-17T04:09:30Z

Hi,

This is a follow-up on:
<http://sourceware.org/ml/binutils/2012-11/msg00240.html>

Here is the second attempt to refactor the PRPSINFO handling on GDB and
Binutils. This patch specifically touches the GDB area, but it is
dependent on the first patch which touches BFD.

The basic idea of the patch is to fill the `struct
elf_internal_prpsinfo' with proper information about the process being
debugged. The new function `linux_nat_fill_prpsinfo' will be called
whenever the user requests GDB to generate a corefile.

I did my best to make the function easy to understand, but there are
some "tricks" there (specially the `sscanf' thing) which I'd like to ask
for opinions. It's working OK and I don't think it's bad code, but it's
better to be safe and consult the maintainers.

I have tested this code on x86_64 (with -m32 too), PPC64 (with -m32 too)
and ARM. I read the corefile generated using eu-readelf and it
correctly displayed the PRPSINFO section. I would like some help with
the procfs code, because I could not find a way to test it.

Comments? Ok to check-in?

--
Sergio

2012-12-17 Sergio Durigan Junior <sergiodj@redhat.com>

* linux-nat.c: Include `cli/cli-utils.h' and `elf-psinfo.h'.
(linux_nat_fill_prpsinfo): New function.
* linux-nat.h (elf_internal_prpsinfo): Forward declaration for
structure.
(linux_nat_fill_prpsinfo): New declaration.
* linux-tdep.c: Include `elf-psinfo.h' and `linux-nat.h'.
(linux_make_corefile_notes): Refactor code to gather information for
PRPSINFO, using linux_nat_fill_prpsinfo.
* procfs.c: Include `elf-psinfo.h'.
(procfs_make_note_section): Refactor code to gather basic information
for PRPSINFO.

---
gdb/linux-nat.c | 164 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
gdb/linux-nat.h | 14 +++++
gdb/linux-tdep.c | 23 ++------
gdb/procfs.c | 37 +++++++------
4 files changed, 204 insertions(+), 34 deletions(-)

diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index f5ca977..15f69b1 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -67,6 +67,8 @@
#include "linux-ptrace.h"
#include "buffer.h"
#include "target-descriptions.h"
+#include "cli/cli-utils.h"
+#include "elf-psinfo.h" /* for `struct elf_internal_prpsinfo' */

#ifndef SPUFS_MAGIC
#define SPUFS_MAGIC 0x23c9b64e
@@ -4368,6 +4370,168 @@ linux_nat_collect_thread_registers (const struct regcache *regcache,
return note_data;
}

+/* See definition at linux-nat.h. */
+
+int
+linux_nat_fill_prpsinfo (struct elf_internal_prpsinfo *p)
+{
+ /* The filename which we will use to obtain some info about the process.
+ We will basically use this to store the `/proc/PID/FILENAME' file. */
+ char filename[100];
+ /* The full name of the program which generated the corefile. */
+ char *fname;
+ /* The basename of the executable. */
+ const char *basename;
+ /* The arguments of the program. */
+ char *psargs;
+ char *infargs;
+ /* The contents of `/proc/PID/stat' file. */
+ char *proc_stat;
+ /* The valid states of a process, according to the Linux kernel. */
+ const char valid_states[] = "RSDTZW";
+ /* The state of the process. */
+ char pr_sname;
+ /* The PID of the program which generated the corefile. */
+ pid_t pid;
+ /* Parent PID, process group ID and session ID. */
+ int pr_ppid, pr_pgrp, pr_sid;
+ /* Process flags. */
+ unsigned int pr_flag;
+ /* Process nice value. */
+ long pr_nice;
+ /* The stat of the `/proc/PID/stat' file. */
+ struct stat proc_st;
+ /* The number of fields read by `sscanf'. */
+ int n_fields = 0;
+ /* Cleanups. */
+ struct cleanup *c;
+ int i;
+
+ gdb_assert (p != NULL);
+
+ /* Initializing the cleanup chain. */
+ c = make_cleanup (null_cleanup, NULL);
+
+ /* Obtaining PID and filename. */
+ pid = ptid_get_pid (inferior_ptid);
+ xsnprintf (filename, sizeof (filename), "/proc/%d/cmdline", pid);
+ fname = target_fileio_read_stralloc (filename);
+
+ if (fname == NULL || strcmp (fname, "") == 0)
+ {
+ /* No program name was read, so we won't be able to retrieve more
+ information about the process. */
+ return 0;
+ }
+
+ make_cleanup (xfree, fname);
+ memset (p, 0, sizeof (*p));
+
+ /* Obtaining the file stat as well. */
+ stat (filename, &proc_st);
+
+ /* Defining the PID. */
+ p->pr_pid = pid;
+
+ /* Copying the program name. Only the basename matters. */
+ basename = lbasename (fname);
+ strncpy (p->pr_fname, basename, sizeof (p->pr_fname));
+ p->pr_fname[sizeof (p->pr_fname) - 1] = '\0';
+
+ /* Generating and copying the program's arguments. */
+ psargs = xstrdup (fname);
+ infargs = get_inferior_args ();
+
+ if (infargs != NULL)
+ psargs = reconcat (psargs, psargs, " ", infargs, NULL);
+
+ make_cleanup (xfree, psargs);
+
+ strncpy (p->pr_psargs, psargs, sizeof (p->pr_psargs));
+ p->pr_psargs[sizeof (p->pr_psargs) - 1] = '\0';
+
+ xsnprintf (filename, sizeof (filename), "/proc/%d/stat", pid);
+ proc_stat = target_fileio_read_stralloc (filename);
+
+ if (proc_stat == NULL || strcmp (proc_stat, "") == 0)
+ {
+ /* Despite being unable to read more information about the process, we
+ return 1 here because at least we have its command line, PID and
+ arguments. */
+ do_cleanups (c);
+ return 1;
+ }
+
+ /* Ok, we have the stats. It's time to do a little parsing of the contents
+ of the buffer, so that we end up reading what we want.
+
+ The following parsing mechanism is strongly based on the information
+ generated by the `fs/proc/array.c' file, present in the Linux kernel
+ tree. More details about how the information is displayed can be
+ obtained by seeing the manpage of proc(5), specifically under the entry
+ of `/proc/[pid]/stat'. */
+
+ /* Getting rid of the PID, since we already have it. */
+ while (isdigit (*proc_stat))
+ ++proc_stat;
+
+ proc_stat = skip_spaces (proc_stat);
+
+ /* Getting rid of the executable name, since we already have it. We know
+ that this name will be in parentheses, so we can safely look for the
+ close-paren. */
+ while (*proc_stat != ')')
+ ++proc_stat;
+ ++proc_stat;
+
+ proc_stat = skip_spaces (proc_stat);
+
+ n_fields = sscanf (proc_stat,
+ "%c " /* Process state. */
+ "%d %d %d " /* Parent PID, group ID, session ID. */
+ "%*d %*d " /* tty_nr, tpgid (not used). */
+ "%u " /* Flags. */
+ "%*s %*s %*s %*s " /* minflt, cminflt, majflt,
+ cmajflt (not used). */
+ "%*s %*s %*s %*s " /* utime, stime, cutime,
+ cstime (not used). */
+ "%*s " /* Priority (not used). */
+ "%ld ", /* Nice. */
+ &pr_sname,
+ &pr_ppid, &pr_pgrp, &pr_sid,
+ &pr_flag,
+ &pr_nice);
+
+ if (n_fields != 6)
+ {
+ /* Again, we couldn't read the complementary information about the
+ process state. However, we already have minimal information, so we
+ just return 1 here. */
+ do_cleanups (c);
+ return 1;
+ }
+
+ /* Filling the structure fields. */
+ for (i = 0; i < sizeof (valid_states); ++i)
+ if (pr_sname == valid_states[i])
+ break;
+
+ p->pr_state = i;
+ p->pr_sname = pr_sname;
+ p->pr_zomb = pr_sname == 'Z';
+ p->pr_nice = pr_nice;
+ p->pr_flag = pr_flag;
+ p->pr_uid = proc_st.st_uid;
+ p->pr_gid = proc_st.st_gid;
+ p->pr_ppid = pr_ppid;
+ p->pr_pgrp = pr_pgrp;
+ p->pr_sid = pr_sid;
+
+ do_cleanups (c);
+
+ return 1;
+}
+
/* Fills the "to_make_corefile_note" target vector. Builds the note
section for a corefile, and returns it in a malloc buffer. */

diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h
index 50998b8..8615e4f 100644
--- a/gdb/linux-nat.h
+++ b/gdb/linux-nat.h
@@ -23,6 +23,11 @@

struct arch_lwp_info;

+/* Forward declaration of the PRPSINFO structure. For more details, look at
+ `bfd/elf-psinfo.h'. */
+
+struct elf_internal_prpsinfo;
+
/* Ways to "resume" a thread. */

enum resume_kind
@@ -201,3 +206,12 @@ int linux_nat_get_siginfo (ptid_t ptid, siginfo_t *siginfo);
/* Set alternative SIGTRAP-like events recognizer. */
void linux_nat_set_status_is_event (struct target_ops *t,
int (*status_is_event) (int status));
+
+/* Fill the PRPSINFO structure with information about the process being
+ debugged. Returns 1 in case of success, 0 for failures. Please note that
+ even if the structure cannot be entirely filled (e.g., GDB was unable to
+ gather information about the process UID/GID), this function will still
+ return 1 since some information was already recorded. It will only return
+ 0 iff nothing can be gathered. */
+
+int linux_nat_fill_prpsinfo (struct elf_internal_prpsinfo *p);
diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index d5ad6e3..e1c76eb 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -28,10 +28,12 @@
#include "regset.h"
#include "elf/common.h"
#include "elf-bfd.h" /* for elfcore_write_* */
+#include "elf-psinfo.h" /* for `struct elf_internal_prpsinfo' */
#include "inferior.h"
#include "cli/cli-utils.h"
#include "arch-utils.h"
#include "gdb_obstack.h"
+#include "linux-nat.h"

#include <ctype.h>

@@ -1161,27 +1163,14 @@ linux_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size,
linux_collect_thread_registers_ftype collect)
{
struct linux_corefile_thread_data thread_args;
+ struct elf_internal_prpsinfo prpsinfo;
char *note_data = NULL;
gdb_byte *auxv;
int auxv_len;

- /* Process information. */
- if (get_exec_file (0))
- {
- const char *fname = lbasename (get_exec_file (0));
- char *psargs = xstrdup (fname);
-
- if (get_inferior_args ())
- psargs = reconcat (psargs, psargs, " ", get_inferior_args (),
- (char *) NULL);
-
- note_data = elfcore_write_prpsinfo (obfd, note_data, note_size,
- fname, psargs);
- xfree (psargs);
-
- if (!note_data)
- return NULL;
- }
+ if (linux_nat_fill_prpsinfo (&prpsinfo))
+ note_data = elfcore_write_prpsinfo (obfd, note_data, note_size,
+ &prpsinfo);

/* Thread register information. */
thread_args.gdbarch = gdbarch;
diff --git a/gdb/procfs.c b/gdb/procfs.c
index 1c5cc13..3125c02 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -25,6 +25,7 @@
#include "target.h"
#include "gdbcore.h"
#include "elf-bfd.h" /* for elfcore_write_* */
+#include "elf-psinfo.h" /* for struct elf_internal_prpsinfo */
#include "gdbcmd.h"
#include "gdbthread.h"
#include "regcache.h"
@@ -5471,39 +5472,41 @@ procfs_make_note_section (bfd *obfd, int *note_size)
struct cleanup *old_chain;
gdb_gregset_t gregs;
gdb_fpregset_t fpregs;
- char fname[16] = {'\0'};
- char psargs[80] = {'\0'};
+ struct elf_internal_prpsinfo prpsinfo;
procinfo *pi = find_procinfo_or_die (PIDGET (inferior_ptid), 0);
char *note_data = NULL;
- char *inf_args;
struct procfs_corefile_thread_data thread_args;
gdb_byte *auxv;
int auxv_len;
enum gdb_signal stop_signal;

+ memset (&prpsinfo, 0, sizeof (prpsinfo));
+
if (get_exec_file (0))
{
- strncpy (fname, lbasename (get_exec_file (0)), sizeof (fname));
- fname[sizeof (fname) - 1] = 0;
- strncpy (psargs, get_exec_file (0), sizeof (psargs));
- psargs[sizeof (psargs) - 1] = 0;
+ char *inf_args;
+ char *psargs;
+
+ strncpy (prpsinfo.pr_fname, lbasename (get_exec_file (0)),
+ sizeof (prpsinfo.pr_fname));
+ prpsinfo.pr_fname[sizeof (prpsinfo.pr_fname) - 1] = '\0';
+
+ psargs = xstrdup (prpsinfo.pr_fname);

inf_args = get_inferior_args ();
- if (inf_args && *inf_args &&
- strlen (inf_args) < ((int) sizeof (psargs) - (int) strlen (psargs)))
- {
- strncat (psargs, " ",
- sizeof (psargs) - strlen (psargs));
- strncat (psargs, inf_args,
- sizeof (psargs) - strlen (psargs));
- }
+ if (inf_args != NULL)
+ psargs = reconcat (psargs, psargs, " ", inf_args, NULL);
+
+ strncpy (prpsinfo.pr_psargs, psargs, sizeof (prpsinfo.pr_psargs));
+ prpsinfo.pr_psargs[sizeof (prpsinfo.pr_psargs) - 1] = '\0';
+
+ xfree (psargs);
}

note_data = (char *) elfcore_write_prpsinfo (obfd,
note_data,
note_size,
- fname,
- psargs);
+ &prpsinfo);

stop_signal = find_stop_signal ();

--
1.7.7.6