Re: [PATCH 2/4] [nto] Fixes for nto procfs.

Aleksandar Ristovski
2015-10-20T12:44:23Z

[-- Attachment #1: Type: text/plain, Size: 1663 bytes --]

On 15-10-15 01:41 PM, Pedro Alves wrote:
> It would have been nicer to see this split into a fix/theme
> per patch, and add something to the commit log about each
> fix. E.g., the aux bits could easily be a separate patch.
>
> Anyway, this is pretty isolated to NTO bits.

I'm trying to catch up and submit local changes for previous ports.

While patches may not be minimalistic, I am trying to at least bring
certain rounded-up improvement (e.g. having a debug session).

But I will try to make more granulated patches.

>
> LGTM with the nits below addressed.
>
> On 10/13/2015 05:01 PM, Aleksandar Ristovski wrote:
>
>> }
>>
>> do_cleanups (inner_cleanup);
>> @@ -599,9 +612,40 @@ procfs_files_info (struct target_ops *ignore)
>>
>> printf_unfiltered ("\tUsing the running image of %s %s via %s.\n",
>> inf->attach_flag ? "attached" : "child",
>> - target_pid_to_str (inferior_ptid), nto_procfs_path);
>> + target_pid_to_str (inferior_ptid),
>> + nodestr ? nodestr : "local node");
>
> Write 'nodestr != NULL'.

Done. Here and other places where pointer is used as a logical expression.
...
>> + if (rd <= 0)
>> + {
>> + proc_path[0] = '\0';
>> + return NULL;
>> + }
>> + else
>> + proc_path[rd] = '\0';
>> +
>> + return proc_path;
>
> Either write:
>
> else
> {
> proc_path[rd] = '\0';
> return proc_path;
> }
>
> Or drop the "else".

Dropped 'else'.

...
>> +
>> + if (!tempbuf)
>> + return TARGET_XFER_E_IO;
>
> if (tempbuf == NULL)
>
> Can NTO's alloca really return NULL?

Yes.

Attached fixed version of the patch.

Thanks,

Aleksandar Ristovski

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-nto-Fixes-for-nto-procfs.patch --]
[-- Type: text/x-patch; name="0001-nto-Fixes-for-nto-procfs.patch", Size: 13407 bytes --]

From 62774c1deef7eb01108ac65636e2ccf974992d85 Mon Sep 17 00:00:00 2001
From: Aleksandar Ristovski <aristovski@qnx.com>
Date: Mon, 5 Oct 2015 10:32:16 -0400
Subject: [PATCH] [nto] Fixes for nto procfs.

gdb/ChangeLog:

* nto-procfs.c (sys/auxv.h): Include.
(nto_procfs_path): Rename to...
(nodestr): ... this, and change type.
(nto_node): Use new variable and logic accordingly.
(procfs_open_1): Use new variable name. Use local buffer to construct
procfs path.
(procfs_pidlist): Use NODESTR to construct procfs path.
(procfs_files_info): Use NODESTR to output meaningful text.
(procfs_pid_to_exec_file): New target function.
(do_attach): Construct procfs using NODESTR.
(procfs_xfer_partial): Logic for reading TARGET_OBJECT_AUXV.
(procfs_create_inferior): Compare pointer to NULL.
(init_procfs_targets): Wire procfs_pid_to_exec_file.
* nto-tdep.c (symfile.h): Include.
(nto_read_auxv_from_initial_stack): New function.
* nto-tdep.h (nto_read_auxv_from_initial_stack): New function
declaration.
---
gdb/nto-procfs.c | 128 ++++++++++++++++++++++++++++++++++++++++++++-----------
gdb/nto-tdep.c | 87 +++++++++++++++++++++++++++++++++++++
gdb/nto-tdep.h | 4 ++
3 files changed, 194 insertions(+), 25 deletions(-)

diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c
index d659f79..176ceea 100644
--- a/gdb/nto-procfs.c
+++ b/gdb/nto-procfs.c
@@ -30,6 +30,8 @@
#include <sys/syspage.h>
#include <dirent.h>
#include <sys/netmgr.h>
+#include <sys/auxv.h>
+
#include "gdbcore.h"
#include "inferior.h"
#include "target.h"
@@ -73,7 +75,7 @@ static int procfs_stopped_by_watchpoint (struct target_ops *ops);
referenced elsewhere. 'nto_procfs_node' is a flag used to say
whether we are local, or we should get the current node descriptor
for the remote QNX node. */
-static char nto_procfs_path[PATH_MAX] = { "/proc" };
+static char *nodestr;
static unsigned nto_procfs_node = ND_LOCAL_NODE;

/* Return the current QNX Node, or error out. This is a simple
@@ -85,10 +87,11 @@ nto_node (void)
{
unsigned node;

- if (ND_NODE_CMP (nto_procfs_node, ND_LOCAL_NODE) == 0)
+ if (ND_NODE_CMP (nto_procfs_node, ND_LOCAL_NODE) == 0
+ || nodestr == NULL)
return ND_LOCAL_NODE;

- node = netmgr_strtond (nto_procfs_path, 0);
+ node = netmgr_strtond (nodestr, 0);
if (node == -1)
error (_("Lost the QNX node. Debug session probably over."));

@@ -108,12 +111,12 @@ procfs_is_nto_target (bfd *abfd)
static void
procfs_open_1 (struct target_ops *ops, const char *arg, int from_tty)
{
- char *nodestr;
char *endstr;
char buffer[50];
int fd, total_size;
procfs_sysinfo *sysinfo;
struct cleanup *cleanups;
+ char nto_procfs_path[PATH_MAX];

/* Offer to kill previous inferiors before opening this target. */
target_preopen (from_tty);
@@ -123,8 +126,11 @@ procfs_open_1 (struct target_ops *ops, const char *arg, int from_tty)
/* Set the default node used for spawning to this one,
and only override it if there is a valid arg. */

+ xfree (nodestr);
+ nodestr = NULL;
+
nto_procfs_node = ND_LOCAL_NODE;
- nodestr = arg ? xstrdup (arg) : NULL;
+ nodestr = (arg != NULL) ? xstrdup (arg) : NULL;

init_thread_list ();

@@ -149,10 +155,8 @@ procfs_open_1 (struct target_ops *ops, const char *arg, int from_tty)
*endstr = 0;
}
}
- snprintf (nto_procfs_path, PATH_MAX - 1, "%s%s", nodestr ? nodestr : "",
- "/proc");
- if (nodestr)
- xfree (nodestr);
+ snprintf (nto_procfs_path, PATH_MAX - 1, "%s%s",
+ (nodestr != NULL) ? nodestr : "", "/proc");

fd = open (nto_procfs_path, O_RDONLY);
if (fd == -1)
@@ -174,7 +178,7 @@ procfs_open_1 (struct target_ops *ops, const char *arg, int from_tty)
{
total_size = sysinfo->total_size;
sysinfo = alloca (total_size);
- if (!sysinfo)
+ if (sysinfo == NULL)
{
printf_filtered ("Memory error: %d (%s)\n", errno,
safe_strerror (errno));
@@ -359,7 +363,7 @@ procfs_pidlist (char *args, int from_tty)
{
DIR *dp = NULL;
struct dirent *dirp = NULL;
- char buf[512];
+ char buf[PATH_MAX];
procfs_info *pidinfo = NULL;
procfs_debuginfo *info = NULL;
procfs_status *status = NULL;
@@ -367,12 +371,16 @@ procfs_pidlist (char *args, int from_tty)
pid_t pid;
char name[512];
struct cleanup *cleanups;
+ char procfs_dir[PATH_MAX];

- dp = opendir (nto_procfs_path);
+ snprintf (procfs_dir, sizeof (procfs_dir), "%s%s",
+ (nodestr != NULL) ? nodestr : "", "/proc");
+
+ dp = opendir (procfs_dir);
if (dp == NULL)
{
fprintf_unfiltered (gdb_stderr, "failed to opendir \"%s\" - %d (%s)",
- nto_procfs_path, errno, safe_strerror (errno));
+ procfs_dir, errno, safe_strerror (errno));
return;
}

@@ -395,7 +403,9 @@ procfs_pidlist (char *args, int from_tty)
do_cleanups (cleanups);
return;
}
- snprintf (buf, 511, "%s/%s/as", nto_procfs_path, dirp->d_name);
+ snprintf (buf, sizeof (buf), "%s%s/%s/as",
+ (nodestr != NULL) ? nodestr : "",
+ "/proc", dirp->d_name);
pid = atoi (dirp->d_name);
}
while (pid == 0);
@@ -406,8 +416,7 @@ procfs_pidlist (char *args, int from_tty)
{
fprintf_unfiltered (gdb_stderr, "failed to open %s - %d (%s)\n",
buf, errno, safe_strerror (errno));
- do_cleanups (cleanups);
- return;
+ continue;
}
inner_cleanup = make_cleanup_close (fd);

@@ -431,11 +440,16 @@ procfs_pidlist (char *args, int from_tty)
status = (procfs_status *) buf;
for (status->tid = 1; status->tid <= num_threads; status->tid++)
{
- if (devctl (fd, DCMD_PROC_TIDSTATUS, status, sizeof (buf), 0) != EOK
- && status->tid != 0)
- break;
- if (status->tid != 0)
- printf_filtered ("%s - %d/%d\n", name, pid, status->tid);
+ const int err
+ = devctl (fd, DCMD_PROC_TIDSTATUS, status, sizeof (buf), 0);
+ printf_filtered ("%s - %d", name, pid);
+ if (err == EOK && status->tid != 0)
+ printf_filtered ("/%d\n", status->tid);
+ else
+ {
+ printf_filtered ("\n");
+ break;
+ }
}

do_cleanups (inner_cleanup);
@@ -599,9 +613,38 @@ procfs_files_info (struct target_ops *ignore)

printf_unfiltered ("\tUsing the running image of %s %s via %s.\n",
inf->attach_flag ? "attached" : "child",
- target_pid_to_str (inferior_ptid), nto_procfs_path);
+ target_pid_to_str (inferior_ptid),
+ (nodestr != NULL) ? nodestr : "local node");
}

+/* Read executable file name for the given PID. */
+
+static char *
+procfs_pid_to_exec_file (struct target_ops *ops, const int pid)
+{
+ int proc_fd;
+ static char proc_path[PATH_MAX];
+ ssize_t rd;
+
+ /* Read exe file name. */
+ snprintf (proc_path, sizeof (proc_path), "%s/proc/%d/exefile",
+ (nodestr != NULL) ? nodestr : "", pid);
+ proc_fd = open (proc_path, O_RDONLY);
+ if (proc_fd == -1)
+ return NULL;
+
+ rd = read (proc_fd, proc_path, sizeof (proc_path) - 1);
+ close (proc_fd);
+ if (rd <= 0)
+ {
+ proc_path[0] = '\0';
+ return NULL;
+ }
+ proc_path[rd] = '\0';
+ return proc_path;
+}
+
+
/* Attach to process PID, then initialize for debugging it. */
static void
procfs_attach (struct target_ops *ops, const char *args, int from_tty)
@@ -653,8 +696,8 @@ do_attach (ptid_t ptid)
struct sigevent event;
char path[PATH_MAX];

- snprintf (path, PATH_MAX - 1, "%s/%d/as", nto_procfs_path,
- ptid_get_pid (ptid));
+ snprintf (path, PATH_MAX - 1, "%s%s/%d/as",
+ (nodestr != NULL) ? nodestr : "", "/proc", ptid_get_pid (ptid));
ctl_fd = open (path, O_RDWR);
if (ctl_fd == -1)
error (_("Couldn't open proc file %s, error %d (%s)"), path, errno,
@@ -872,6 +915,40 @@ procfs_xfer_partial (struct target_ops *ops, enum target_object object,
{
case TARGET_OBJECT_MEMORY:
return procfs_xfer_memory (readbuf, writebuf, offset, len, xfered_len);
+ case TARGET_OBJECT_AUXV:
+ if (readbuf != NULL)
+ {
+ int err;
+ CORE_ADDR initial_stack;
+ debug_process_t procinfo;
+ /* For 32-bit architecture, size of auxv_t is 8 bytes. */
+ const unsigned int sizeof_auxv_t = sizeof (auxv_t);
+ const unsigned int sizeof_tempbuf = 20 * sizeof_auxv_t;
+ int tempread;
+ gdb_byte *const tempbuf = alloca (sizeof_tempbuf);
+
+ if (tempbuf == NULL)
+ return TARGET_XFER_E_IO;
+
+ err = devctl (ctl_fd, DCMD_PROC_INFO, &procinfo,
+ sizeof procinfo, 0);
+ if (err != EOK)
+ return TARGET_XFER_E_IO;
+
+ /* Similar as in the case of a core file, we read auxv from
+ initial_stack. */
+ initial_stack = procinfo.initial_stack;
+
+ /* procfs is always 'self-hosted', no byte-order manipulation. */
+ tempread = nto_read_auxv_from_initial_stack (initial_stack, tempbuf,
+ sizeof_tempbuf,
+ sizeof (auxv_t));
+ tempread = min (tempread, len) - offset;
+ memcpy (readbuf, tempbuf + offset, tempread);
+ *xfered_len = tempread;
+ return tempread ? TARGET_XFER_OK : TARGET_XFER_EOF;
+ }
+ /* Fallthru */
default:
return ops->beneath->to_xfer_partial (ops->beneath, object, annex,
readbuf, writebuf, offset, len,
@@ -1121,7 +1198,7 @@ procfs_create_inferior (struct target_ops *ops, char *exec_file,
}

args = xstrdup (allargs);
- breakup_args (args, exec_file ? &argv[1] : &argv[0]);
+ breakup_args (args, (exec_file != NULL) ? &argv[1] : &argv[0]);

argv = nto_parse_redirection (argv, &in, &out, &err);

@@ -1444,6 +1521,7 @@ init_procfs_targets (void)
t->to_interrupt = procfs_interrupt;
t->to_have_continuable_watchpoint = 1;
t->to_extra_thread_info = nto_extra_thread_info;
+ t->to_pid_to_exec_file = procfs_pid_to_exec_file;

nto_native_ops = t;

diff --git a/gdb/nto-tdep.c b/gdb/nto-tdep.c
index 81ee7fb..63094e8 100644
--- a/gdb/nto-tdep.c
+++ b/gdb/nto-tdep.c
@@ -31,6 +31,7 @@
#include "solib-svr4.h"
#include "gdbcore.h"
#include "objfiles.h"
+#include "symfile.h"

#ifdef __CYGWIN__
#include <sys/cygwin.h>
@@ -394,3 +395,89 @@ nto_initialize_signals (void)
signal_pass_update (SIGPHOTON, 1);
#endif
}
+
+
+/* Read AUXV from initial_stack. */
+LONGEST
+nto_read_auxv_from_initial_stack (CORE_ADDR initial_stack, gdb_byte *readbuf,
+ LONGEST len, size_t sizeof_auxv_t)
+{
+ gdb_byte targ32[4]; /* For 32 bit target values. */
+ gdb_byte targ64[8]; /* For 64 bit target values. */
+ CORE_ADDR data_ofs = 0;
+ ULONGEST anint;
+ LONGEST len_read = 0;
+ gdb_byte *buff;
+ enum bfd_endian byte_order;
+ int ptr_size;
+
+ if (sizeof_auxv_t == 16)
+ ptr_size = 8;
+ else
+ ptr_size = 4;
+
+ /* Skip over argc, argv and envp... Comment from ldd.c:
+
+ The startup frame is set-up so that we have:
+ auxv
+ NULL
+ ...
+ envp2
+ envp1 <----- void *frame + (argc + 2) * sizeof(char *)
+ NULL
+ ...
+ argv2
+ argv1
+ argc <------ void * frame
+
+ On entry to ldd, frame gives the address of argc on the stack. */
+ /* Read argc. 4 bytes on both 64 and 32 bit arches and luckily little
+ * endian. So we just read first 4 bytes. */
+ if (target_read_memory (initial_stack + data_ofs, targ32, 4) != 0)
+ return 0;
+
+ byte_order = gdbarch_byte_order (target_gdbarch ());
+
+ anint = extract_unsigned_integer (targ32, sizeof (targ32), byte_order);
+
+ /* Size of pointer is assumed to be 4 bytes (32 bit arch.) */
+ data_ofs += (anint + 2) * ptr_size; /* + 2 comes from argc itself and
+ NULL terminating pointer in
+ argv. */
+
+ /* Now loop over env table: */
+ anint = 0;
+ while (target_read_memory (initial_stack + data_ofs, targ64, ptr_size)
+ == 0)
+ {
+ if (extract_unsigned_integer (targ64, ptr_size, byte_order) == 0)
+ anint = 1; /* Keep looping until non-null entry is found. */
+ else if (anint)
+ break;
+ data_ofs += ptr_size;
+ }
+ initial_stack += data_ofs;
+
+ memset (readbuf, 0, len);
+ buff = readbuf;
+ while (len_read <= len-sizeof_auxv_t)
+ {
+ if (target_read_memory (initial_stack + len_read, buff, sizeof_auxv_t)
+ == 0)
+ {
+ /* Both 32 and 64 bit structures have int as the first field. */
+ const ULONGEST a_type
+ = extract_unsigned_integer (buff, sizeof (targ32), byte_order);
+
+ if (a_type == AT_NULL)
+ break;
+ buff += sizeof_auxv_t;
+ len_read += sizeof_auxv_t;
+ }
+ else
+ break;
+ }
+ return len_read;
+}
+
+
diff --git a/gdb/nto-tdep.h b/gdb/nto-tdep.h
index bd85d2a..7089a12 100644
--- a/gdb/nto-tdep.h
+++ b/gdb/nto-tdep.h
@@ -168,4 +168,8 @@ int nto_in_dynsym_resolve_code (CORE_ADDR pc);

char *nto_extra_thread_info (struct target_ops *self, struct thread_info *);

+LONGEST nto_read_auxv_from_initial_stack (CORE_ADDR inital_stack,
+ gdb_byte *readbuf,
+ LONGEST len, size_t sizeof_auxv_t);
+
#endif
--
1.9.1