[PATCH] target_read_aux_vector

[PATCH] target_read_aux_vector

[Roland McGrath]

This patch adds target_read_aux_vector to read the auxv information as a
whole block (there is no code here that interprets its contents).  It also
makes ELF core file writing (gcore) use this to produce an NT_AUXV note.
I have only tested the linux-proc.c code in actuality, but the procfs.c
(Solaris) and corelow.c code is so simple it would be hard for it to be
wrong.

The function signature used here is a little goofy if you ask me.  But I
modelled it on target_make_corefile_notes, which is another recent addition
and so I assume it is in the style that gdb folk prefer.  I would be happy
to change the signature if there is a different preference.  I made the
malloc'd-ness of the returned pointer part of the interface rather than
having it call make_cleanup and magically know that's the right scope as
the to_make_corefile_notes implementations do.

The one way I've tested this is that "gcore" used on Linux 2.6 du jour
produces an NT_AUXV note in the core file matching what a kernel-written
dump has.  The Solaris additions should make it do the same there as well.

The core_ops implementation of target_read_aux_vector is not presently
used at all, because you can't do gcore when examining a core file.
(Incidentally, I think gdb should support that.  Doing that and comparing
what gdb wrote to the original core file is a good test of both core file
reading and core file writing.)  It would be used by the tdep code looking
for AT_SYSINFO_EHDR, as we have been discussing here.

I am not happy that the identical function is duplicated in procfs.c and
linux-proc.c; but I did not see any place for common code that is usable
for both flavors of /proc filesystem.  The to_make_corefile_notes hooks in
those two files are close to identical as well.


Comments?


Thanks,
Roland


2003-10-06  Roland McGrath  <roland@redhat.com>

	* target.h (struct target_ops): New field `to_read_aux_vector'.
	(target_read_aux_vector): New macro.
	* target.c (update_current_target): Do INHERIT for to_read_aux_vector.
	(dummy_read_aux_vector): New function.
	(init_dummy_target): Initialize dummy_target.to_read_aux_vector.
	* inftarg.c (inftarg_set_read_aux_vector): New function.
	* linux-proc.c (procfs_read_aux_vector): New function.
	(_initialize_linux_proc): Call inftarg_set_read_aux_vector.
	(linux_make_note_section): Add an NT_AUXV note.
	* procfs.c (procfs_read_aux_vector): New function.
	(init_procfs_ops): Initialize procfs_ops.to_read_aux_vector.
	(procfs_make_note_section): Add an NT_AUXV note.
	* sol-thread.c (sol_read_aux_vector): New function.
	(init_sol_thread_ops): Initialize sol_thread_ops.to_read_aux_vector.
	* corelow.c (core_read_aux_vector): New function.
	(init_core_ops): Initialize core_ops.to_read_aux_vector.

Index: target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.55
diff -b -p -u -r1.55 target.c
--- target.c	2 Oct 2003 20:28:30 -0000	1.55
+++ target.c	7 Oct 2003 01:20:30 -0000
@@ -624,6 +624,7 @@ update_current_target (void)
       INHERIT (to_async, t);
       INHERIT (to_async_mask_value, t);
       INHERIT (to_find_memory_regions, t);
+      INHERIT (to_read_aux_vector, t);
       INHERIT (to_make_corefile_notes, t);
       INHERIT (to_get_thread_local_address, t);
       INHERIT (to_magic, t);
@@ -1500,6 +1501,13 @@ static int dummy_find_memory_regions (in
   return 0;
 }
 
+/* Error-catcher for target_read_aux_vector */
+static char *dummy_read_aux_vector (int *ignore1)
+{
+  error ("No target.");
+  return NULL;
+}
+
 /* Error-catcher for target_make_corefile_notes */
 static char * dummy_make_corefile_notes (bfd *ignore1, int *ignore2)
 {
@@ -1521,6 +1529,7 @@ init_dummy_target (void)
   dummy_target.to_pid_to_str = normal_pid_to_str;
   dummy_target.to_stratum = dummy_stratum;
   dummy_target.to_find_memory_regions = dummy_find_memory_regions;
+  dummy_target.to_read_aux_vector = dummy_read_aux_vector;
   dummy_target.to_make_corefile_notes = dummy_make_corefile_notes;
   dummy_target.to_magic = OPS_MAGIC;
 }
Index: target.h
===================================================================
RCS file: /cvs/src/src/gdb/target.h,v
retrieving revision 1.41
diff -b -p -u -r1.41 target.h
--- target.h	17 Jun 2003 20:28:13 -0000	1.41
+++ target.h	7 Oct 2003 01:20:30 -0000
@@ -322,6 +322,7 @@ struct target_ops
 					    int, int, int, 
 					    void *), 
 				   void *);
+    char *(*to_read_aux_vector) (int *);
     char * (*to_make_corefile_notes) (bfd *, int *);
 
     /* Return the thread-local address at OFFSET in the
@@ -939,6 +940,14 @@ extern void (*target_new_objfile_hook) (
      (current_target.to_find_memory_regions) (FUNC, DATA)
 
 /*
+ * Function to read target startup auxilliary vector (ELF-specific).
+ * Returns a malloc'd buffer of ELF auxv data, and sets *LENP to its size.
+ */
+
+#define target_read_aux_vector(LENP) \
+     (current_target.to_read_aux_vector) (LENP)
+
+/*
  * Compose corefile .note section.
  */
 
Index: inftarg.c
===================================================================
RCS file: /cvs/src/src/gdb/inftarg.c,v
retrieving revision 1.18
diff -b -p -u -r1.18 inftarg.c
--- inftarg.c	21 Sep 2003 01:26:45 -0000	1.18
+++ inftarg.c	7 Oct 2003 01:20:30 -0000
@@ -628,6 +628,13 @@ inftarg_set_find_memory_regions (int (*f
   child_ops.to_find_memory_regions = func;
 }
 
+/* Take over the 'read_aux_vector' vector from inftarg.c. */
+void
+inftarg_set_read_aux_vector (char *(*func) (int *))
+{
+  child_ops.to_read_aux_vector = func;
+}
+
 /* Take over the 'make_corefile_notes' vector from inftarg.c. */
 extern void 
 inftarg_set_make_corefile_notes (char * (*func) (bfd *, int *))
Index: linux-proc.c
===================================================================
RCS file: /cvs/src/src/gdb/linux-proc.c,v
retrieving revision 1.20
diff -b -p -u -r1.20 linux-proc.c
--- linux-proc.c	1 Oct 2003 20:36:56 -0000	1.20
+++ linux-proc.c	7 Oct 2003 01:20:31 -0000
@@ -161,6 +161,46 @@ linux_find_memory_regions (int (*func) (
   return 0;
 }
 
+static char *
+procfs_read_aux_vector (int *lenp)
+{
+  char pathname[MAXPATHLEN];
+  int fd;
+  char *buf, *p;
+  int bufsz, n;
+
+  sprintf (pathname, "/proc/%d/auxv", PIDGET (inferior_ptid));
+  fd = open (pathname, O_RDONLY);
+  if (fd < 0)
+    return NULL;		/* XXX Call error here? */
+
+  bufsz = 256;
+  buf = (char *) xmalloc (bufsz);
+  p = buf;
+  while (1)
+    {
+      n = read (fd, p, buf + bufsz - p);
+      if (n == 0)		/* Hit EOF.  */
+	{
+	  *lenp = p - buf;
+	  break;
+	}
+      if (n < 0)		/* Read error.  */
+	{
+	  xfree (buf);
+	  buf = NULL;
+	  break;
+	}
+      bufsz *= 2;
+      n = p + n - buf;
+      buf = (char *) xrealloc (buf, bufsz);
+      p = buf + n;
+    }
+
+  close (fd);
+  return buf;
+}
+
 /* Function: linux_do_thread_registers
  *
  * Records the thread's register state for the corefile note section.
@@ -271,6 +311,8 @@ linux_make_note_section (bfd *obfd, int 
   char psargs[80] = { '\0' };
   char *note_data = NULL;
   ptid_t current_ptid = inferior_ptid;
+  char *auxv;
+  int auxv_len;
 
   if (get_exec_file (0))
     {
@@ -305,6 +347,14 @@ linux_make_note_section (bfd *obfd, int 
       note_data = thread_args.note_data;
     }
 
+  auxv = target_read_aux_vector (&auxv_len);
+  if (auxv != NULL)
+    {
+      note_data = elfcore_write_note (obfd, note_data, note_size,
+				      "CORE", NT_AUXV, auxv, auxv_len);
+      xfree (auxv);
+    }
+
   make_cleanup (xfree, note_data);
   return note_data;
 }
@@ -593,9 +643,11 @@ _initialize_linux_proc (void)
 {
   extern void inftarg_set_find_memory_regions ();
   extern void inftarg_set_make_corefile_notes ();
+  extern void inftarg_set_read_aux_vector ();
 
   inftarg_set_find_memory_regions (linux_find_memory_regions);
   inftarg_set_make_corefile_notes (linux_make_note_section);
+  inftarg_set_read_aux_vector (procfs_read_aux_vector);
 
   add_info ("proc", linux_info_proc_cmd,
 	    "Show /proc process information about any running process.\n\
Index: procfs.c
===================================================================
RCS file: /cvs/src/src/gdb/procfs.c,v
retrieving revision 1.48
diff -b -p -u -r1.48 procfs.c
--- procfs.c	21 Sep 2003 01:26:45 -0000	1.48
+++ procfs.c	7 Oct 2003 01:20:31 -0000
@@ -139,6 +139,7 @@ static int proc_find_memory_regions (int
 					      void *), 
 				     void *);
 
+static char *procfs_read_aux_vector (int *);
 static char * procfs_make_note_section (bfd *, int *);
 
 static int procfs_can_use_hw_breakpoint (int, int, int);
@@ -190,6 +191,7 @@ init_procfs_ops (void)
   procfs_ops.to_stratum             = process_stratum;
   procfs_ops.to_has_thread_control  = tc_schedlock;
   procfs_ops.to_find_memory_regions = proc_find_memory_regions;
+  procfs_ops.to_read_aux_vector     = procfs_read_aux_vector;
   procfs_ops.to_make_corefile_notes = procfs_make_note_section;
   procfs_ops.to_can_use_hw_breakpoint = procfs_can_use_hw_breakpoint;
   procfs_ops.to_magic               = OPS_MAGIC;
@@ -5737,6 +5739,47 @@ proc_untrace_sysexit_cmd (char *args, in
   proc_trace_syscalls (args, from_tty, PR_SYSEXIT, FLAG_RESET);
 }
 
+char *
+procfs_read_aux_vector (int *lenp)
+{
+  char pathname[MAX_PROC_NAME_SIZE];
+  int fd;
+  char *buf, *p;
+  int bufsz, n;
+
+  sprintf (pathname, "/proc/%d/auxv", PIDGET (inferior_ptid));
+  fd = open_with_retry (pathname, O_RDONLY);
+  if (fd < 0)
+    return NULL;		/* XXX Call error here? */
+
+  bufsz = 256;
+  buf = (char *) xmalloc (bufsz);
+  p = buf;
+  while (1)
+    {
+      n = read (fd, p, buf + bufsz - p);
+      if (n == 0)		/* Hit EOF.  */
+	{
+	  *lenp = p - bufsz;
+	  break;
+	}
+      if (n < 0)		/* Read error.  */
+	{
+	  xfree (buf);
+	  buf = NULL;
+	  break;
+	}
+      bufsz *= 2;
+      n = p + n - buf;
+      buf = (char *) xrealloc (buf, bufsz);
+      p = buf + n;
+    }
+
+  close (fd);
+  return buf;
+}
+
+
 
 void
 _initialize_procfs (void)
@@ -5851,6 +5894,8 @@ procfs_make_note_section (bfd *obfd, int
   char *note_data = NULL;
   char *inf_args;
   struct procfs_corefile_thread_data thread_args;
+  char *auxv;
+  int auxv_len;
 
   if (get_exec_file (0))
     {
@@ -5897,6 +5942,14 @@ procfs_make_note_section (bfd *obfd, int
   else
     {
       note_data = thread_args.note_data;
+    }
+
+  auxv = target_read_aux_vector (&auxv_len);
+  if (auxv != NULL)
+    {
+      note_data = elfcore_write_note (obfd, note_data, note_size,
+				      "CORE", NT_AUXV, auxv, auxv_len);
+      xfree (auxv);
     }
 
   make_cleanup (xfree, note_data);
Index: corelow.c
===================================================================
RCS file: /cvs/src/src/gdb/corelow.c,v
retrieving revision 1.30
diff -b -p -u -r1.30 corelow.c
--- corelow.c	21 Sep 2003 01:26:44 -0000	1.30
+++ corelow.c	7 Oct 2003 01:20:31 -0000
@@ -474,6 +548,30 @@ core_files_info (struct target_ops *t)
   print_section_info (t, core_bfd);
 }
 \f
+static char *
+core_read_aux_vector (int *lenp)
+{
+  sec_ptr section;
+  bfd_size_type size;
+  char *contents;
+
+  section = bfd_get_section_by_name (core_bfd, ".auxv");
+  if (section == NULL)
+    return NULL;
+
+  size = bfd_section_size (core_bfd, section);
+  contents = (char *) xmalloc (size);
+  if (! bfd_get_section_contents (core_bfd, section, contents,
+				  (file_ptr) 0, size))
+    {
+      xfree (contents);
+      warning ("Couldn't read NT_AUXV note in core file.");
+      return NULL;
+    }
+
+  return contents;
+}
+\f
 /* If mourn is being called in all the right places, this could be say
    `gdb internal error' (since generic_mourn calls breakpoint_init_inferior).  */
 
@@ -520,6 +618,7 @@ init_core_ops (void)
   core_ops.to_has_memory = 1;
   core_ops.to_has_stack = 1;
   core_ops.to_has_registers = 1;
+  core_ops.to_read_aux_vector = core_read_aux_vector;
   core_ops.to_magic = OPS_MAGIC;
 }
 
Index: sol-thread.c
===================================================================
RCS file: /cvs/src/src/gdb/sol-thread.c,v
retrieving revision 1.38
diff -b -p -u -r1.38 sol-thread.c
--- sol-thread.c	21 Sep 2003 01:26:45 -0000	1.38
+++ sol-thread.c	7 Oct 2003 01:28:38 -0000
@@ -1522,6 +1522,12 @@ sol_find_memory_regions (int (*func) (CO
 }
 
 static char *
+sol_read_aux_vector (int *auxv_size)
+{
+  return procfs_ops.to_read_aux_vector (auxv_size);
+}
+
+static char *
 sol_make_note_section (bfd *obfd, int *note_size)
 {
   return procfs_ops.to_make_corefile_notes (obfd, note_size);
@@ -1575,6 +1581,7 @@ init_sol_thread_ops (void)
   sol_thread_ops.to_has_execution = 1;
   sol_thread_ops.to_has_thread_control = tc_none;
   sol_thread_ops.to_find_memory_regions = sol_find_memory_regions;
+  sol_thread_ops.to_read_aux_vector = sol_read_aux_vector;
   sol_thread_ops.to_make_corefile_notes = sol_make_note_section;
   sol_thread_ops.to_magic = OPS_MAGIC;
 }

Re: [PATCH] target_read_aux_vector

[Daniel Jacobowitz]

On Mon, Oct 06, 2003 at 06:48:08PM -0700, Roland McGrath wrote:
> This patch adds target_read_aux_vector to read the auxv information as a
> whole block (there is no code here that interprets its contents).  It also
> makes ELF core file writing (gcore) use this to produce an NT_AUXV note.
> I have only tested the linux-proc.c code in actuality, but the procfs.c
> (Solaris) and corelow.c code is so simple it would be hard for it to be
> wrong.
> 
> The function signature used here is a little goofy if you ask me.  But I
> modelled it on target_make_corefile_notes, which is another recent addition
> and so I assume it is in the style that gdb folk prefer.  I would be happy
> to change the signature if there is a different preference.  I made the
> malloc'd-ness of the returned pointer part of the interface rather than
> having it call make_cleanup and magically know that's the right scope as
> the to_make_corefile_notes implementations do.
> 
> The one way I've tested this is that "gcore" used on Linux 2.6 du jour
> produces an NT_AUXV note in the core file matching what a kernel-written
> dump has.  The Solaris additions should make it do the same there as well.
> 
> The core_ops implementation of target_read_aux_vector is not presently
> used at all, because you can't do gcore when examining a core file.
> (Incidentally, I think gdb should support that.  Doing that and comparing
> what gdb wrote to the original core file is a good test of both core file
> reading and core file writing.)  It would be used by the tdep code looking
> for AT_SYSINFO_EHDR, as we have been discussing here.
> 
> I am not happy that the identical function is duplicated in procfs.c and
> linux-proc.c; but I did not see any place for common code that is usable
> for both flavors of /proc filesystem.  The to_make_corefile_notes hooks in
> those two files are close to identical as well.
> 
> 
> Comments?

Yes, comments would be nice :)  Just trivially: summary comments before
new functions.  And two spaces after full stops.

I have no substantive comments on this code.  Let's see if anyone
else does though.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [PATCH] target_read_aux_vector

[Andrew Cagney]

Roland, not sure why you posted this.  The relevant discussion has still 
not been resolved.

Andrew

Re: [PATCH] target_read_aux_vector

[Daniel Jacobowitz]

On Mon, Oct 06, 2003 at 09:58:51PM -0400, Andrew Cagney wrote:
> Roland, not sure why you posted this.  The relevant discussion has still 
> not been resolved.
> 
> Andrew

As a step in constructive discussion, perhaps?

I would like to add that, as a target vector entry, I prefer this
interface.  It's more reasonable for the remote protocol.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [PATCH] target_read_aux_vector

[Roland McGrath]

> Roland, not sure why you posted this.  The relevant discussion has still 
> not been resolved.

I like to write code more than I like to speculate about how I will write
it.  My favorite sort of discussion about code includes exchanges of "like
this?  [and some code that might work]", "no, different because of foo and
bar", "ok, so like this? [and some different code that might work]", and so
on.  Let's have a discussion like that!

I developed one of the alternatives being discussed to flesh out the issues
with doing it that way.  In the process, I noticed that part of it was
independently useful (fixing a missing part of gcore functionality).  I
posted this patch to demonstrate the utility of this alternative for the
purpose we are discussing, and the bonus feature that my patch as posted
fixes an existing, separate shortcoming of gdb.  Now discuss!

Re: [PATCH] target_read_aux_vector

[Andrew Cagney]

> Roland, not sure why you posted this.  The relevant discussion has still 
>> not been resolved.
> 
> 
> I like to write code more than I like to speculate about how I will write
> it.  My favorite sort of discussion about code includes exchanges of "like
> this?  [and some code that might work]", "no, different because of foo and
> bar", "ok, so like this? [and some different code that might work]", and so
> on.  Let's have a discussion like that!
> 
> I developed one of the alternatives being discussed to flesh out the issues
> with doing it that way.  In the process, I noticed that part of it was
> independently useful (fixing a missing part of gcore functionality).  I
> posted this patch to demonstrate the utility of this alternative for the
> purpose we are discussing, and the bonus feature that my patch as posted
> fixes an existing, separate shortcoming of gdb.  Now discuss!

In that case, can I suggest posting such things as [wip] 
(work-in-progress).  That way it's clear that the change is intended as 
a discussion point, and not a final waiting-on-approval patch.  It 
unfortunatly comes across as very strange when someone posts what looks 
like the final [rfa] for for a specific variant of a change when the 
related technical discussion has not been resolved.

enjoy,
Andrew

Re: [PATCH] target_read_aux_vector

[Jim Blandy]

Andrew Cagney <ac131313@redhat.com> writes:
> In that case, can I suggest posting such things as [wip]
> (work-in-progress).  That way it's clear that the change is intended
> as a discussion point, and not a final waiting-on-approval patch.  It
> unfortunatly comes across as very strange when someone posts what
> looks like the final [rfa] for for a specific variant of a change when
> the related technical discussion has not been resolved.

A WIP marker is nice, but I don't agree it's somehow an affront to
post a patch in the middle of a discussion.  Even if Roland had
intended it as a final RFA, everyone should be willing to consider
changes to their patches --- even after they've been committed.

Being inflexible, making unreasonable demands, and things like that
are what's offensive.

Re: [PATCH] target_read_aux_vector

[Andrew Cagney]

> Andrew Cagney <ac131313@redhat.com> writes:
> 
>> In that case, can I suggest posting such things as [wip]
>> (work-in-progress).  That way it's clear that the change is intended
>> as a discussion point, and not a final waiting-on-approval patch.  It
>> unfortunatly comes across as very strange when someone posts what
>> looks like the final [rfa] for for a specific variant of a change when
>> the related technical discussion has not been resolved.
> 
> 
> A WIP marker is nice, but I don't agree it's somehow an affront to
> post a patch in the middle of a discussion.  Even if Roland had
> intended it as a final RFA, everyone should be willing to consider
> changes to their patches --- even after they've been committed.
> 
> Being inflexible, making unreasonable demands, and things like that
> are what's offensive.

Jim,

Even if Roland doesn't use [wip], I'll now know what his intent was (and 
due credit to roland for taking the time to explain it) and understand 
the purpose.  However, come the future, others may not realise this and 
may also be confused.

As you know there is unfortunatly something of a history between Roland 
and various GDB developers, and that can be attributed at least in part 
to poor communication.  If we can find better and clearer ways to 
communicate than we will all benefit.

Andrew

Re: [PATCH] target_read_aux_vector

[Michael Snyder]

Andrew Cagney wrote:
>> Roland, not sure why you posted this.  The relevant discussion has still
>>
>>> not been resolved.
>>
>>
>>
>> I like to write code more than I like to speculate about how I will write
>> it.  My favorite sort of discussion about code includes exchanges of 
>> "like
>> this?  [and some code that might work]", "no, different because of foo 
>> and
>> bar", "ok, so like this? [and some different code that might work]", 
>> and so
>> on.  Let's have a discussion like that!
>>
>> I developed one of the alternatives being discussed to flesh out the 
>> issues
>> with doing it that way.  In the process, I noticed that part of it was
>> independently useful (fixing a missing part of gcore functionality).  I
>> posted this patch to demonstrate the utility of this alternative for the
>> purpose we are discussing, and the bonus feature that my patch as posted
>> fixes an existing, separate shortcoming of gdb.  Now discuss!
> 
> 
> In that case, can I suggest posting such things as [wip] 
> (work-in-progress).  That way it's clear that the change is intended as 
> a discussion point, and not a final waiting-on-approval patch.  It 
> unfortunatly comes across as very strange when someone posts what looks 
> like the final [rfa] for for a specific variant of a change when the 
> related technical discussion has not been resolved.

In fact (now that you mention it), I though I understood that
[PATCH] means "I have applied this patch", as opposed to
"please review my patch".  But what the heck, are these conventions
even written down anywhere?

Re: [PATCH] target_read_aux_vector

[Roland McGrath]

> In fact (now that you mention it), I though I understood that
> [PATCH] means "I have applied this patch", as opposed to
> "please review my patch".  But what the heck, are these conventions
> even written down anywhere?

If there are special conventions for this mailing list, I am not aware of
them and would appreciate a pointer.  In the rest of the world, the
"[PATCH]" prefix means, "This message contains a patch that actually
compiles and kind of works for me, not just me talking out of my ass and
maybe pasting in some random code."