Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [commit] [gdbserver/linux] access memory of running processes
@ 2010-08-26 23:26 Pedro Alves
  2010-08-26 23:29 ` Michael Snyder
  0 siblings, 1 reply; 5+ messages in thread
From: Pedro Alves @ 2010-08-26 23:26 UTC (permalink / raw)
  To: gdb-patches

Of all the debug API's we've added non-stop support to (we've added
it to a few private targets), linux/ptrace is the only one so far that
does not allow accessing memory when you pass it the PID of a process
that is presently running to work with.  This affects the user experience
much, as any command that touches memory, even breakpoint command fail with a
hard error if you happen to have a running thread selected.  The workaround is
to stop one thread, switch to it, do what you need, and re-resume the thread.
You may leave all other threads running while doing that workaround.
Obviously, this is cumbersome.  Particularly more so since random commands
that weren't accessing target memory yesterday, may do so today.

I've been thinking of whether this should be addressed on the target
side, or if it should be addressed by GDB.  Fixing this on the GDB side
implies GDB interrupting a process, doing the access, and resuming it again.
Since not all targets need this, it would have to be dependent on a target
flag.  Fixing this on the target side means doing the temporarily-pause work,
but it ends up resolving faster, because there's no inherent RSP latency
involved.  The latter option is completely transparent to GDB, of course.

Thinking about it, I came up with the following points and conclusions:

 - Making sure that GDB has a consistent view of memory across a batch of
memory accesses is something that is desired in some scenarios but not
actually necessary in others, meaning, it's an independent issue from allowing
a single access while threads are running.  The former always needs to be
addressed by having GDB take the wheel.  The latter can be addressed in either
way.

 - This is really a debug API issue.  The fact that we have to access an
address space by passing to ptrace a stopped lwp is just a consequence of the
fact that you always have to pass a stopped process to all ptrace API calls.

 - The current RSP memory access and breakpoint commands have synchronous
semantics.  Doesn't sound like a strong argument, but, let me explain that I
considered having gdbserver immediately reply OK to Z0 breakpoints if the
target is running (and so GDB was happy), have gdbserver force an asynchronous
stop on the current inferior, and finally insert the breakpoint when the
inferior stops, and then transparently resume it.  The problem is, if the
breakpoint fails to insert, because say, its address wasn't actually mapped in
the address space, there's no way to inform GDB.  It's obviously worse with
memory read/write packets.  Sure, we could come up with new RSP packets, but,
as long as we have what we have, we might as well make them work (particularly
since they work for most targets without extra fuss anyway) (synchronously) on
linux too, and come up with new packets if we really find this way to be
limiting.

There are few ways to be smart here, but I've chosen to be as dumb as
possible, and then more dumb --- transparently stop all lwps at selected entry
points of memory accesses corresponding to requests coming in from GDB.  Since
the need for this is target specific, I've abstracted this out in new
prepare_to_access_memory/unprepare_to_access_memory target methods.

Tested on x86_64-unknown-linux-gnu and applied.

-- 
Pedro Alves

2010-08-26  Pedro Alves  <pedro@codesourcery.com>

	gdbserver/
	* linux-low.c (linux_prepare_to_access_memory): New.
	(linux_unprepare_to_access_memory): New.
	(linux_target_ops): Install them.
	* server.c (read_memory): Rename to ...
	(gdb_read_memory): ... this.  Use
	prepare_to_access_memory/prepare_to_access_memory.
	(write_memory): Rename to ...
	(gdb_write_memory): ... this.  Use
	prepare_to_access_memory/prepare_to_access_memory.
	(handle_search_memory_1): Adjust.
	(process_serial_event): Adjust.
	* target.h (struct target_ops): New fields
	prepare_to_access_memory and unprepare_to_access_memory.
	(prepare_to_access_memory, unprepare_to_access_memory): New.
	* linux-x86-low.c (x86_insert_point, x86_remove_point): Use
	prepare_to_access_memory/prepare_to_access_memory.
	* nto-low.c (nto_target_ops): Adjust.
	* spu-low.c (spu_target_ops): Adjust.
	* win32-low.c (win32_target_ops): Adjust.

---
 gdb/gdbserver/linux-low.c     |   21 +++++++++++++++++++++
 gdb/gdbserver/linux-x86-low.c |   24 +++++++++++++++++++++---
 gdb/gdbserver/nto-low.c       |    2 ++
 gdb/gdbserver/server.c        |   39 +++++++++++++++++++++++++++++----------
 gdb/gdbserver/spu-low.c       |    2 ++
 gdb/gdbserver/target.h        |   29 +++++++++++++++++++++++++++++
 gdb/gdbserver/win32-low.c     |    2 ++
 7 files changed, 106 insertions(+), 13 deletions(-)

Index: src/gdb/gdbserver/linux-low.c
===================================================================
--- src.orig/gdb/gdbserver/linux-low.c	2010-08-26 19:32:03.000000000 +0100
+++ src/gdb/gdbserver/linux-low.c	2010-08-26 21:14:56.000000000 +0100
@@ -5006,6 +5006,25 @@ linux_unpause_all (int unfreeze)
 }
 
 static int
+linux_prepare_to_access_memory (void)
+{
+  /* Neither ptrace nor /proc/PID/mem allow accessing memory through a
+     running LWP.  */
+  if (non_stop)
+    linux_pause_all (1);
+  return 0;
+}
+
+static void
+linux_unprepare_to_access_memory (void)
+{
+  /* Neither ptrace nor /proc/PID/mem allow accessing memory through a
+     running LWP.  */
+  if (non_stop)
+    linux_unpause_all (1);
+}
+
+static int
 linux_install_fast_tracepoint_jump_pad (CORE_ADDR tpoint, CORE_ADDR tpaddr,
 					CORE_ADDR collector,
 					CORE_ADDR lockaddr,
@@ -5043,6 +5062,8 @@ static struct target_ops linux_target_op
   linux_wait,
   linux_fetch_registers,
   linux_store_registers,
+  linux_prepare_to_access_memory,
+  linux_unprepare_to_access_memory,
   linux_read_memory,
   linux_write_memory,
   linux_look_up_symbols,
Index: src/gdb/gdbserver/server.c
===================================================================
--- src.orig/gdb/gdbserver/server.c	2010-08-26 16:37:50.000000000 +0100
+++ src/gdb/gdbserver/server.c	2010-08-26 19:38:25.000000000 +0100
@@ -539,8 +539,10 @@ monitor_show_help (void)
 /* Read trace frame or inferior memory.  */
 
 static int
-read_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len)
+gdb_read_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len)
 {
+  int ret;
+
   if (current_traceframe >= 0)
     {
       ULONGEST nbytes;
@@ -558,19 +560,36 @@ read_memory (CORE_ADDR memaddr, unsigned
       /* (assume no half-trace half-real blocks for now) */
     }
 
-  return read_inferior_memory (memaddr, myaddr, len);
+  ret = prepare_to_access_memory ();
+  if (ret == 0)
+    {
+      ret = read_inferior_memory (memaddr, myaddr, len);
+      unprepare_to_access_memory ();
+    }
+
+  return ret;
 }
 
 /* Write trace frame or inferior memory.  Actually, writing to trace
    frames is forbidden.  */
 
 static int
-write_memory (CORE_ADDR memaddr, const unsigned char *myaddr, int len)
+gdb_write_memory (CORE_ADDR memaddr, const unsigned char *myaddr, int len)
 {
   if (current_traceframe >= 0)
     return EIO;
   else
-    return write_inferior_memory (memaddr, myaddr, len);
+    {
+      int ret;
+
+      ret = prepare_to_access_memory ();
+      if (ret == 0)
+	{
+	  ret = write_inferior_memory (memaddr, myaddr, len);
+	  unprepare_to_access_memory ();
+	}
+      return ret;
+    }
 }
 
 /* Subroutine of handle_search_memory to simplify it.  */
@@ -584,7 +603,7 @@ handle_search_memory_1 (CORE_ADDR start_
 {
   /* Prime the search buffer.  */
 
-  if (read_memory (start_addr, search_buf, search_buf_size) != 0)
+  if (gdb_read_memory (start_addr, search_buf, search_buf_size) != 0)
     {
       warning ("Unable to access target memory at 0x%lx, halting search.",
 	       (long) start_addr);
@@ -635,8 +654,8 @@ handle_search_memory_1 (CORE_ADDR start_
 			? search_space_len - keep_len
 			: chunk_size);
 
-	  if (read_memory (read_addr, search_buf + keep_len,
-				    nr_to_read) != 0)
+	  if (gdb_read_memory (read_addr, search_buf + keep_len,
+			       nr_to_read) != 0)
 	    {
 	      warning ("Unable to access target memory at 0x%lx, halting search.",
 		       (long) read_addr);
@@ -2924,7 +2943,7 @@ process_serial_event (void)
     case 'm':
       require_running (own_buf);
       decode_m_packet (&own_buf[1], &mem_addr, &len);
-      if (read_memory (mem_addr, mem_buf, len) == 0)
+      if (gdb_read_memory (mem_addr, mem_buf, len) == 0)
 	convert_int_to_ascii (mem_buf, own_buf, len);
       else
 	write_enn (own_buf);
@@ -2932,7 +2951,7 @@ process_serial_event (void)
     case 'M':
       require_running (own_buf);
       decode_M_packet (&own_buf[1], &mem_addr, &len, &mem_buf);
-      if (write_memory (mem_addr, mem_buf, len) == 0)
+      if (gdb_write_memory (mem_addr, mem_buf, len) == 0)
 	write_ok (own_buf);
       else
 	write_enn (own_buf);
@@ -2941,7 +2960,7 @@ process_serial_event (void)
       require_running (own_buf);
       if (decode_X_packet (&own_buf[1], packet_len - 1,
 			   &mem_addr, &len, &mem_buf) < 0
-	  || write_memory (mem_addr, mem_buf, len) != 0)
+	  || gdb_write_memory (mem_addr, mem_buf, len) != 0)
 	write_enn (own_buf);
       else
 	write_ok (own_buf);
Index: src/gdb/gdbserver/target.h
===================================================================
--- src.orig/gdb/gdbserver/target.h	2010-08-26 16:37:50.000000000 +0100
+++ src/gdb/gdbserver/target.h	2010-08-26 21:28:50.000000000 +0100
@@ -181,6 +181,23 @@ struct target_ops
 
   void (*store_registers) (struct regcache *regcache, int regno);
 
+  /* Prepare to read or write memory from the inferior process.
+     Targets use this to do what is necessary to get the state of the
+     inferior such that it is possible to access memory.
+
+     This should generally only be called from client facing routines,
+     such as gdb_read_memory/gdb_write_memory, or the insert_point
+     callbacks.
+
+     Like `read_memory' and `write_memory' below, returns 0 on success
+     and errno on failure.  */
+
+  int (*prepare_to_access_memory) (void);
+
+  /* Undo the effects of prepare_to_access_memory.  */
+
+  void (*unprepare_to_access_memory) (void);
+
   /* Read memory from the inferior process.  This should generally be
      called through read_inferior_memory, which handles breakpoint shadowing.
 
@@ -469,6 +486,18 @@ int start_non_stop (int nonstop);
 ptid_t mywait (ptid_t ptid, struct target_waitstatus *ourstatus, int options,
 	       int connected_wait);
 
+#define prepare_to_access_memory()		\
+  (the_target->prepare_to_access_memory		\
+   ? (*the_target->prepare_to_access_memory) () \
+   : 0)
+
+#define unprepare_to_access_memory()			\
+  do							\
+    {							\
+      if (the_target->unprepare_to_access_memory)     	\
+	(*the_target->unprepare_to_access_memory) ();  	\
+    } while (0)
+
 int read_inferior_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len);
 
 int write_inferior_memory (CORE_ADDR memaddr, const unsigned char *myaddr,
Index: src/gdb/gdbserver/linux-x86-low.c
===================================================================
--- src.orig/gdb/gdbserver/linux-x86-low.c	2010-08-26 21:03:47.000000000 +0100
+++ src/gdb/gdbserver/linux-x86-low.c	2010-08-26 21:04:19.000000000 +0100
@@ -546,7 +546,7 @@ i386_dr_low_get_status (void)
   return x86_linux_dr_get (ptid, DR_STATUS);
 }
 \f
-/* Watchpoint support.  */
+/* Breakpoint/Watchpoint support.  */
 
 static int
 x86_insert_point (char type, CORE_ADDR addr, int len)
@@ -555,7 +555,16 @@ x86_insert_point (char type, CORE_ADDR a
   switch (type)
     {
     case '0':
-      return set_gdb_breakpoint_at (addr);
+      {
+	int ret;
+
+	ret = prepare_to_access_memory ();
+	if (ret)
+	  return -1;
+	ret = set_gdb_breakpoint_at (addr);
+	unprepare_to_access_memory ();
+	return ret;
+      }
     case '2':
     case '3':
     case '4':
@@ -574,7 +583,16 @@ x86_remove_point (char type, CORE_ADDR a
   switch (type)
     {
     case '0':
-      return delete_gdb_breakpoint_at (addr);
+      {
+	int ret;
+
+	ret = prepare_to_access_memory ();
+	if (ret)
+	  return -1;
+	ret = delete_gdb_breakpoint_at (addr);
+	unprepare_to_access_memory ();
+	return ret;
+      }
     case '2':
     case '3':
     case '4':
Index: src/gdb/gdbserver/nto-low.c
===================================================================
--- src.orig/gdb/gdbserver/nto-low.c	2010-06-16 10:55:22.000000000 +0100
+++ src/gdb/gdbserver/nto-low.c	2010-08-26 21:07:28.000000000 +0100
@@ -913,6 +913,8 @@ static struct target_ops nto_target_ops 
   nto_wait,
   nto_fetch_registers,
   nto_store_registers,
+  NULL, /* prepare_to_access_memory */
+  NULL, /* unprepare_to_access_memory */
   nto_read_memory,
   nto_write_memory,
   NULL, /* nto_look_up_symbols */
Index: src/gdb/gdbserver/spu-low.c
===================================================================
--- src.orig/gdb/gdbserver/spu-low.c	2010-06-20 23:17:44.000000000 +0100
+++ src/gdb/gdbserver/spu-low.c	2010-08-26 21:07:54.000000000 +0100
@@ -653,6 +653,8 @@ static struct target_ops spu_target_ops 
   spu_wait,
   spu_fetch_registers,
   spu_store_registers,
+  NULL, /* prepare_to_access_memory */
+  NULL, /* unprepare_to_access_memory */
   spu_read_memory,
   spu_write_memory,
   spu_look_up_symbols,
Index: src/gdb/gdbserver/win32-low.c
===================================================================
--- src.orig/gdb/gdbserver/win32-low.c	2010-06-16 10:58:30.000000000 +0100
+++ src/gdb/gdbserver/win32-low.c	2010-08-26 21:08:16.000000000 +0100
@@ -1781,6 +1781,8 @@ static struct target_ops win32_target_op
   win32_wait,
   win32_fetch_inferior_registers,
   win32_store_inferior_registers,
+  NULL, /* prepare_to_access_memory */
+  NULL, /* unprepare_to_access_memory */
   win32_read_inferior_memory,
   win32_write_inferior_memory,
   NULL, /* lookup_symbols */


^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-08-27  0:19 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-26 23:26 [commit] [gdbserver/linux] access memory of running processes Pedro Alves
2010-08-26 23:29 ` Michael Snyder
2010-08-27  0:02   ` Pedro Alves
2010-08-27  0:04     ` Michael Snyder
2010-08-27  0:19       ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox