[PR,gdb/17903] Implement to_pid_to_exec_file for solaris
Commit Message
This patch is to address an issue in Solaris,
bug 17903.
When attaching to a process by PID the current
implementation in gdb/procfs.c leaves
"to_pid_to_exec_file" at the default -- a stub
function that returns NULL.
This causes symbols not to load when attaching,
forcing the user to either attach by specifying
the path on the command line or inside gdb.
After specifying the file inside gdb it's also
necessary to manually load symbolic information.
The proposed change resembles the implementation
in gdb/linux-nat.c, but is generic enough that it
could work on most platforms supporting procfs
and could potentially be used as a replacement
for the current default.
If you only care about fixing this in Solaris 10+,
the loop could be done away with and readlink
could just read from "/proc/self/path/a.out",
allowing you to get rid of the xsnprintf call
as well.
I don't have DejaGNU installed on my development
machines at work and I don't have one [that functions]
at home, so I was unable to run the test suite (otherwise
I would have).
Lastly: I made these changes on a machine that
doesn't have network access. I had to hand-type
the following diff. My apologies if there are
typos; I did my best. Obviously I won't be able to
push this myself.
gdb/Changelog:
2015-12-16 Brian Vandenberg <phantall@gmail.com>
PR gdb/17903
* gdb/procfs.c (inclusions): Included ansidecl.h for UNUSED_ARG
(procfs_target): added procfs_child_pid_to_exec_file to target_ops struct
(procfs_child_pid_to_exec_file): to_pid_to_exec_file implementation for solaris
Comments
Hi Brian,
Thanks for the patch.
On 12/16/2015 09:47 PM, Brian Vandenberg wrote:
> This patch is to address an issue in Solaris,
> bug 17903.
>
> When attaching to a process by PID the current
> implementation in gdb/procfs.c leaves
> "to_pid_to_exec_file" at the default -- a stub
> function that returns NULL.
>
> This causes symbols not to load when attaching,
> forcing the user to either attach by specifying
> the path on the command line or inside gdb.
> After specifying the file inside gdb it's also
> necessary to manually load symbolic information.
>
> The proposed change resembles the implementation
> in gdb/linux-nat.c, but is generic enough that it
> could work on most platforms supporting procfs
> and could potentially be used as a replacement
> for the current default.
>
> If you only care about fixing this in Solaris 10+,
> the loop could be done away with and readlink
> could just read from "/proc/self/path/a.out",
> allowing you to get rid of the xsnprintf call
> as well.
Yes, please simplify the patch. There's no point in
trying to open files we know don't exist on the OS.
Why are you handling negative PIDs? How do we get here with such a PID?
BTW, the patch should follow the GNU coding standards formatting.
>
> I don't have DejaGNU installed on my development
> machines at work and I don't have one [that functions]
> at home, so I was unable to run the test suite (otherwise
> I would have).
Note that DejaGNU is very easy to build -- just configure/make.
I don't think is has any usually-unavailable dependency, other
than expect, and that one you may already have.
But did you try setting up a Solaris VM? Oracle provides pre-built
Solaris VMs for VirtualBox available, IIRC, so shouldn't be hard.
>
> Lastly: I made these changes on a machine that
> doesn't have network access. I had to hand-type
> the following diff. My apologies if there are
> typos; I did my best. Obviously I won't be able to
> push this myself.
>
Ouch. :-/
Thanks,
Pedro Alves
@@ -51,6 +51,7 @@
#include "auxv.h"
#include "procfs.h"
#include "observer.h"
+#include "ansidecl.h"
/* This module provides the interface between GDB and the
/proc file system, which is used on many versions of Unix
@@ -118,6 +119,7 @@ static void procfs_fetch_registers (struct target_ops *,
struct regcache *, int);
static void procfs_store_registers (struct target_ops *,
struct regcache *, int);
+static char* procfs_child_pid_to_exec_file( struct target_ops*, int );
static void procfs_pass_signals (struct target_ops *self,
int, unsigned char *);
static void procfs_kill_inferior (struct target_ops *ops);
@@ -192,6 +194,7 @@ procfs_target (void)
t->to_resume = procfs_resume;
t->to_fetch_registers = procfs_fetch_registers;
t->to_store_registers = procfs_store_registers;
+ t->to_pid_to_exec_file = procfs_child_pid_to_exec_file;
t->to_xfer_partial = procfs_xfer_partial;
t->to_pass_signals = procfs_pass_signals;
t->to_files_info = procfs_files_info;
@@ -3301,6 +3304,41 @@ procfs_store_registers (struct target_ops *ops,
}
}
+/* "self" may not exist on some platforms,
+ * otherwise that would be preferable to
+ * a format string. */
+static const char *proc_path_lookup[] = {
+ "/proc/%u/path/a.out" // Most UNIXes with procfs (Solaris)
+, "/proc/%u/file" // BSD
+, "/proc/%u/exe" // Linux, Windows, NetBSD
+, "/proc/%u/a.out" // No idea, I've only seen references to it
+};
+
+/* Get a fully qualified path to the debugged process */
+static char* procfs_child_pid_to_exec_file( struct target_ops *ARG_UNUSED(self), int pid ) {
+ static char buf[PATH_MAX] = {0};
+ char name[PATH_MAX] = {0};
+ ssize_t sz = -1;
+ size_t ii = 0;
+
+ pid = pid >= 0 ? pid : -pid;
+
+ for( ii = 0; ii < sizeof(proc_path_lookup)/sizeof(proc_path_lookup[0]); ++ii ) {
+ xsnprintf( name, sizeof(name), proc_path_lookup[ii], (unsigned int)pid );
+ sz = readlink( name, buf, sizeof( name ) );
+
+ if( 0 < sz ) {
+ buf[sz] = '\0';
+ return buf;
+ }
+ }
+
+ /* This is the default behavior as defined in target.h
+ * and implemented in inf_child_pid_to_exec_file() */
+ return NULL;
+}
+