This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[gdbserver/commit] handle partial traceframe memory reads


I've applied this gdbserver+testsuite patch.

Basically, if we collect addresses [0,4) and [4,8) with separate
collect actions, the memory ends up recorded in separate
traceframe blocks (of the same traceframe).  If GDB tries to
read all that with single [0,8) request, gdbserver errors out,
when it shouldn't.

I've added a test that fails without the fix.

A possible improvement I didn't bother doing, is for
traceframe_read_mem to handle reading from more than one block
to fill in the outgoing buffer as much as possible (as long
as we can serve contiguous data), to avoid splitting the
transfer into more than one RSP packet.

The tfile target already handles this case similarly.

-- 
Pedro Alves

2011-02-07  Pedro Alves  <pedro@codesourcery.com>

	gdb/server/
	* server.c (gdb_read_memory): Change return semantics to allow
	partial transfers.
	(handle_search_memory_1): Adjust.
	(process_serial_event) <'m' packet>: Handle partial transfers.
	* tracepoint.c (traceframe_read_mem): Handle partial transfers.

	gdb/testsuite/
	* gdb.trace/collection.c (global_pieces): New.
	* gdb.trace/collection.exp (gdb_collect_global_in_pieces_test):
	New procedure.
	(gdb_trace_collection_test): Call it.

---
 gdb/gdbserver/server.c                 |   43 ++++++++++++++++++++-----------
 gdb/gdbserver/tracepoint.c             |   15 +++++++----
 gdb/testsuite/gdb.trace/collection.c   |    8 +++++
 gdb/testsuite/gdb.trace/collection.exp |   45 +++++++++++++++++++++++++++++++++
 4 files changed, 91 insertions(+), 20 deletions(-)

Index: src/gdb/testsuite/gdb.trace/collection.c
===================================================================
--- src.orig/gdb/testsuite/gdb.trace/collection.c	2011-02-07 12:07:23.616706007 +0000
+++ src/gdb/testsuite/gdb.trace/collection.c	2011-02-07 12:07:39.106706007 +0000
@@ -27,6 +27,14 @@ test_struct  globalstruct;
 test_struct *globalp;
 int          globalarr[16];
 
+struct global_pieces {
+  unsigned int a;
+  unsigned int b;
+} global_pieces =
+  {
+    0x12345678, 0x87654321
+  };
+
 /*
  * Additional globals used in arithmetic tests
  */
Index: src/gdb/testsuite/gdb.trace/collection.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.trace/collection.exp	2011-02-07 12:07:23.616706007 +0000
+++ src/gdb/testsuite/gdb.trace/collection.exp	2011-02-07 12:07:39.106706007 +0000
@@ -513,6 +513,50 @@ proc gdb_collect_globals_test { } {
 	    "collect globals: cease trace debugging"
 }
 
+# Test that when we've collected all fields of a structure
+# individually, we can print the whole structure in one go.
+proc gdb_collect_global_in_pieces_test { } {
+    global gdb_prompt
+
+    prepare_for_trace_test
+
+    # Find the comment-identified line for setting this tracepoint.
+    set testline 0
+    set msg "collect global in pieces: find tracepoint line"
+    gdb_test_multiple "list globals_test_func, +30" "$msg" {
+	-re "\[\r\n\](\[0-9\]+)\[^\r\n\]+ Set_Tracepoint_Here .*$gdb_prompt" {
+	    set testline $expect_out(1,string)
+	    pass "$msg"
+	}
+    }
+
+    if {$testline == 0} {
+	return
+    }
+
+    gdb_test "trace $testline" \
+	"Tracepoint \[0-9\]+ at .*" \
+	"collect global in pieces: set tracepoint"
+    gdb_trace_setactions "collect global in pieces: define actions" \
+	    "" \
+	    "collect global_pieces.a, global_pieces.b" \
+	    "^$"
+
+    # Begin the test.
+    run_trace_experiment "global in pieces" globals_test_func
+
+    gdb_test "print /x global_pieces.a" " = 0x12345678" \
+	"collect global in pieces: print piece a"
+    gdb_test "print /x global_pieces.b" " = 0x87654321" \
+	"collect global in pieces: print piece b"
+
+    gdb_test "print /x global_pieces" " = \{a = 0x12345678, b = 0x87654321\}" \
+	"collect global in pieces: print whole object"
+
+    gdb_test "tfind none" "#0  end .*" \
+	"collect global in pieces: cease trace debugging"
+}
+
 proc gdb_trace_collection_test {} {
     global fpreg
     global spreg
@@ -548,6 +592,7 @@ proc gdb_trace_collection_test {} {
     gdb_collect_registers_test "\$regs"
     gdb_collect_registers_test "\$$fpreg, \$$spreg, \$$pcreg"
     gdb_collect_globals_test
+    gdb_collect_global_in_pieces_test
     
     #
     # Expression tests:
Index: src/gdb/gdbserver/server.c
===================================================================
--- src.orig/gdb/gdbserver/server.c	2011-02-07 12:07:23.896706002 +0000
+++ src/gdb/gdbserver/server.c	2011-02-07 12:07:39.106706007 +0000
@@ -556,12 +556,20 @@ monitor_show_help (void)
   monitor_output ("    Quit GDBserver\n");
 }
 
-/* Read trace frame or inferior memory.  */
+/* Read trace frame or inferior memory.  Returns the number of bytes
+   actually read, zero when no further transfer is possible, and -1 on
+   error.  Return of a positive value smaller than LEN does not
+   indicate there's no more to be read, only the end of the transfer.
+   E.g., when GDB reads memory from a traceframe, a first request may
+   be served from a memory block that does not cover the whole request
+   length.  A following request gets the rest served from either
+   another block (of the same traceframe) or from the read-only
+   regions.  */
 
 static int
 gdb_read_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len)
 {
-  int ret;
+  int res;
 
   if (current_traceframe >= 0)
     {
@@ -572,22 +580,24 @@ gdb_read_memory (CORE_ADDR memaddr, unsi
 			       memaddr, myaddr, len, &nbytes))
 	return EIO;
       /* Data read from trace buffer, we're done.  */
-      if (nbytes == length)
-	return 0;
+      if (nbytes > 0)
+	return nbytes;
       if (!in_readonly_region (memaddr, length))
-	return EIO;
+	return -1;
       /* Otherwise we have a valid readonly case, fall through.  */
       /* (assume no half-trace half-real blocks for now) */
     }
 
-  ret = prepare_to_access_memory ();
-  if (ret == 0)
+  res = prepare_to_access_memory ();
+  if (res == 0)
     {
-      ret = read_inferior_memory (memaddr, myaddr, len);
+      res = read_inferior_memory (memaddr, myaddr, len);
       done_accessing_memory ();
-    }
 
-  return ret;
+      return res == 0 ? len : -1;
+    }
+  else
+    return -1;
 }
 
 /* Write trace frame or inferior memory.  Actually, writing to trace
@@ -623,7 +633,8 @@ handle_search_memory_1 (CORE_ADDR start_
 {
   /* Prime the search buffer.  */
 
-  if (gdb_read_memory (start_addr, search_buf, search_buf_size) != 0)
+  if (gdb_read_memory (start_addr, search_buf, search_buf_size)
+      != search_buf_size)
     {
       warning ("Unable to access target memory at 0x%lx, halting search.",
 	       (long) start_addr);
@@ -675,7 +686,7 @@ handle_search_memory_1 (CORE_ADDR start_
 			: chunk_size);
 
 	  if (gdb_read_memory (read_addr, search_buf + keep_len,
-			       nr_to_read) != 0)
+			       nr_to_read) != search_buf_size)
 	    {
 	      warning ("Unable to access target memory "
 		       "at 0x%lx, halting search.",
@@ -2664,6 +2675,7 @@ process_serial_event (void)
   int i = 0;
   int signal;
   unsigned int len;
+  int res;
   CORE_ADDR mem_addr;
   int pid;
   unsigned char sig;
@@ -2902,10 +2914,11 @@ process_serial_event (void)
     case 'm':
       require_running (own_buf);
       decode_m_packet (&own_buf[1], &mem_addr, &len);
-      if (gdb_read_memory (mem_addr, mem_buf, len) == 0)
-	convert_int_to_ascii (mem_buf, own_buf, len);
-      else
+      res = gdb_read_memory (mem_addr, mem_buf, len);
+      if (res < 0)
 	write_enn (own_buf);
+      else
+	convert_int_to_ascii (mem_buf, own_buf, res);
       break;
     case 'M':
       require_running (own_buf);
Index: src/gdb/gdbserver/tracepoint.c
===================================================================
--- src.orig/gdb/gdbserver/tracepoint.c	2011-02-07 12:07:23.896706002 +0000
+++ src/gdb/gdbserver/tracepoint.c	2011-02-07 12:07:39.116706003 +0000
@@ -4909,12 +4909,17 @@ traceframe_read_mem (int tfnum, CORE_ADD
       trace_debug ("traceframe %d has %d bytes at %s",
 		   tfnum, mlen, paddress (maddr));
 
-      /* Check that requested data is in bounds.  */
-      if (maddr <= addr && (addr + length) <= (maddr + mlen))
+      /* If the block includes the first part of the desired range,
+	 return as much it has; GDB will re-request the remainder,
+	 which might be in a different block of this trace frame.  */
+      if (maddr <= addr && addr < (maddr + mlen))
 	{
-	  /* Block includes the requested range, copy it out.  */
-	  memcpy (buf, dataptr + (addr - maddr), length);
-	  *nbytes = length;
+	  ULONGEST amt = (maddr + mlen) - addr;
+	  if (amt > length)
+	    amt = length;
+
+	  memcpy (buf, dataptr + (addr - maddr), amt);
+	  *nbytes = amt;
 	  return 0;
 	}
 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]