diff mbox

[1/3] Move the traceframe_available_memory code from memory_xfer_partial_1 down to the targets

Message ID 1394541731-27486-1-git-send-email-yao@codesourcery.com
State Committed
Headers show

Commit Message

Yao Qi March 11, 2014, 12:42 p.m. UTC
As a follow-up to

  [PATCH 7/8] Adjust read_value_memory to use to_xfer_partial
  https://sourceware.org/ml/gdb-patches/2014-02/msg00384.html

this patch moves traceframe_available_memory down to the target side.
After this patch, the gdb core code is cleaner, and code on handling
unavailable memory is moved to remote/tfile/ctf targets.

In details, this patch moves traceframe_available_memory code from
memory_xfer_partial_1 to remote target only, so remote target still
uses traceframe_info mechanism to check unavailable memory, and use
remote_ops to read them from read-only sections.  We don't use
traceframe_info mechanism for tfile and ctf target, because it is
fast to iterate all traceframes from trace file, so the summary
information got from traceframe_info is not necessary.

This patch also moves two functions to remote.c from target.c,
because they are only used in remote.c.  I'll clean them up in another
patch.

This series is tested on x86_64-linux.

gdb:

2014-03-11  Yao Qi  <yao@codesourcery.com>

	* ctf.c (ctf_xfer_partial): Check the return value of
	exec_read_partial_read_only, if it is not TARGET_XFER_OK,
	return TARGET_XFER_UNAVAILABLE.
	* tracefile-tfile.c (tfile_xfer_partial): Likewise.
	* target.c (target_read_live_memory): Move it to remote.c.
	(memory_xfer_live_readonly_partial): Likewise.
	(memory_xfer_partial_1): Move some code to remote_read_bytes.
	* remote.c (target_read_live_memory): Moved from target.c.
	(memory_xfer_live_readonly_partial): Likewise.
	(remote_read_bytes): Moved from memory_xfer_partial_1.
---
 gdb/ctf.c             |   16 +++++-
 gdb/remote.c          |  138 +++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/target.c          |  138 -------------------------------------------------
 gdb/tracefile-tfile.c |   16 +++++-
 4 files changed, 168 insertions(+), 140 deletions(-)

Comments

Yao Qi March 21, 2014, 9:39 a.m. UTC | #1
On 03/11/2014 08:42 PM, Yao Qi wrote:
> As a follow-up to
> 
>   [PATCH 7/8] Adjust read_value_memory to use to_xfer_partial
>   https://sourceware.org/ml/gdb-patches/2014-02/msg00384.html
> 
> this patch moves traceframe_available_memory down to the target side.
> After this patch, the gdb core code is cleaner, and code on handling
> unavailable memory is moved to remote/tfile/ctf targets.
> 
> In details, this patch moves traceframe_available_memory code from
> memory_xfer_partial_1 to remote target only, so remote target still
> uses traceframe_info mechanism to check unavailable memory, and use
> remote_ops to read them from read-only sections.  We don't use
> traceframe_info mechanism for tfile and ctf target, because it is
> fast to iterate all traceframes from trace file, so the summary
> information got from traceframe_info is not necessary.
> 
> This patch also moves two functions to remote.c from target.c,
> because they are only used in remote.c.  I'll clean them up in another
> patch.
> 
> This series is tested on x86_64-linux.
> 
> gdb:
> 
> 2014-03-11  Yao Qi  <yao@codesourcery.com>
> 
> 	* ctf.c (ctf_xfer_partial): Check the return value of
> 	exec_read_partial_read_only, if it is not TARGET_XFER_OK,
> 	return TARGET_XFER_UNAVAILABLE.
> 	* tracefile-tfile.c (tfile_xfer_partial): Likewise.
> 	* target.c (target_read_live_memory): Move it to remote.c.
> 	(memory_xfer_live_readonly_partial): Likewise.
> 	(memory_xfer_partial_1): Move some code to remote_read_bytes.
> 	* remote.c (target_read_live_memory): Moved from target.c.
> 	(memory_xfer_live_readonly_partial): Likewise.
> 	(remote_read_bytes): Moved from memory_xfer_partial_1.

Ping.  https://sourceware.org/ml/gdb-patches/2014-03/msg00262.html
Pedro Alves March 21, 2014, 11:56 a.m. UTC | #2
On 03/11/2014 12:42 PM, Yao Qi wrote:

> 	(remote_read_bytes): Moved from memory_xfer_partial_1.

Say:

 	(remote_read_bytes): New, factored out from memory_xfer_partial_1.

> @@ -6847,6 +6928,63 @@ remote_read_bytes (CORE_ADDR memaddr, gdb_byte *myaddr, ULONGEST len,
>    if (len == 0)
>      return 0;
>  
> +  if (get_traceframe_number () != -1)
> +    {
> +      VEC(mem_range_s) *available;
> +
> +      /* If we fail to get the set of available memory, then the
> +	 target does not support querying traceframe info, and so we
> +	 attempt reading from the traceframe anyway (assuming the
> +	 target implements the old QTro packet then).  */
> +      if (traceframe_available_memory (&available, memaddr, len))
> +	{
> +	  struct cleanup *old_chain;
> +
> +	  old_chain = make_cleanup (VEC_cleanup(mem_range_s), &available);
> +
> +	  if (VEC_empty (mem_range_s, available)
> +	      || VEC_index (mem_range_s, available, 0)->start != memaddr)
> +	    {
> +	      enum target_xfer_status res;
> +
> +	      /* Don't read into the traceframe's available
> +		 memory.  */
> +	      if (!VEC_empty (mem_range_s, available))
> +		{
> +		  LONGEST oldlen = len;
> +
> +		  len = VEC_index (mem_range_s, available, 0)->start - memaddr;
> +		  gdb_assert (len <= oldlen);
> +		}
> +
> +	      do_cleanups (old_chain);
> +
> +	      /* This goes through the topmost target again.  */
> +	      res = memory_xfer_live_readonly_partial (&remote_ops,

This "&remote_ops" here will be wrong in case we're connected
with extended-remote.  Looks like remote_read_bytes needs to be
gain an ops parameter.

Otherwise looks good.

Thanks,
diff mbox

Patch

diff --git a/gdb/ctf.c b/gdb/ctf.c
index 95fd31f..ebd40d6 100644
--- a/gdb/ctf.c
+++ b/gdb/ctf.c
@@ -1377,6 +1377,7 @@  ctf_xfer_partial (struct target_ops *ops, enum target_object object,
     {
       struct bt_iter_pos *pos;
       int i = 0;
+      enum target_xfer_status res;
 
       gdb_assert (ctf_iter != NULL);
       /* Save the current position.  */
@@ -1466,7 +1467,20 @@  ctf_xfer_partial (struct target_ops *ops, enum target_object object,
       /* Restore the position.  */
       bt_iter_set_pos (bt_ctf_get_iter (ctf_iter), pos);
 
-      return exec_read_partial_read_only (readbuf, offset, len, xfered_len);
+      /* Requested memory is unavailable in the context of traceframes,
+	 and this address falls within a read-only section, fallback
+	 to reading from executable.  */
+      res = exec_read_partial_read_only (readbuf, offset, len, xfered_len);
+
+      if (res == TARGET_XFER_OK)
+	return TARGET_XFER_OK;
+      else
+	{
+	  /* No use trying further, we know some memory starting
+	     at MEMADDR isn't available.  */
+	  *xfered_len = len;
+	  return TARGET_XFER_UNAVAILABLE;
+	}
     }
   else
     {
diff --git a/gdb/remote.c b/gdb/remote.c
index e03d3bf..afe9d12 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -6824,6 +6824,87 @@  remote_write_bytes (CORE_ADDR memaddr, const gdb_byte *myaddr, ULONGEST len,
 				 packet_format[0], 1);
 }
 
+/* Read memory from the live target, even if currently inspecting a
+   traceframe.  The return is the same as that of target_read.  */
+
+static enum target_xfer_status
+target_read_live_memory (enum target_object object,
+			 ULONGEST memaddr, gdb_byte *myaddr, ULONGEST len,
+			 ULONGEST *xfered_len)
+{
+  enum target_xfer_status ret;
+  struct cleanup *cleanup;
+
+  /* Switch momentarily out of tfind mode so to access live memory.
+     Note that this must not clear global state, such as the frame
+     cache, which must still remain valid for the previous traceframe.
+     We may be _building_ the frame cache at this point.  */
+  cleanup = make_cleanup_restore_traceframe_number ();
+  set_traceframe_number (-1);
+
+  ret = target_xfer_partial (current_target.beneath, object, NULL,
+			     myaddr, NULL, memaddr, len, xfered_len);
+
+  do_cleanups (cleanup);
+  return ret;
+}
+
+/* Using the set of read-only target sections of OPS, read live
+   read-only memory.  Note that the actual reads start from the
+   top-most target again.
+
+   For interface/parameters/return description see target.h,
+   to_xfer_partial.  */
+
+static enum target_xfer_status
+memory_xfer_live_readonly_partial (struct target_ops *ops,
+				   enum target_object object,
+				   gdb_byte *readbuf, ULONGEST memaddr,
+				   ULONGEST len, ULONGEST *xfered_len)
+{
+  struct target_section *secp;
+  struct target_section_table *table;
+
+  secp = target_section_by_addr (ops, memaddr);
+  if (secp != NULL
+      && (bfd_get_section_flags (secp->the_bfd_section->owner,
+				 secp->the_bfd_section)
+	  & SEC_READONLY))
+    {
+      struct target_section *p;
+      ULONGEST memend = memaddr + len;
+
+      table = target_get_section_table (ops);
+
+      for (p = table->sections; p < table->sections_end; p++)
+	{
+	  if (memaddr >= p->addr)
+	    {
+	      if (memend <= p->endaddr)
+		{
+		  /* Entire transfer is within this section.  */
+		  return target_read_live_memory (object, memaddr,
+						  readbuf, len, xfered_len);
+		}
+	      else if (memaddr >= p->endaddr)
+		{
+		  /* This section ends before the transfer starts.  */
+		  continue;
+		}
+	      else
+		{
+		  /* This section overlaps the transfer.  Just do half.  */
+		  len = p->endaddr - memaddr;
+		  return target_read_live_memory (object, memaddr,
+						  readbuf, len, xfered_len);
+		}
+	    }
+	}
+    }
+
+  return TARGET_XFER_EOF;
+}
+
 /* Read memory data directly from the remote machine.
    This does not use the data cache; the data cache uses this.
    MEMADDR is the address in the remote memory space.
@@ -6847,6 +6928,63 @@  remote_read_bytes (CORE_ADDR memaddr, gdb_byte *myaddr, ULONGEST len,
   if (len == 0)
     return 0;
 
+  if (get_traceframe_number () != -1)
+    {
+      VEC(mem_range_s) *available;
+
+      /* If we fail to get the set of available memory, then the
+	 target does not support querying traceframe info, and so we
+	 attempt reading from the traceframe anyway (assuming the
+	 target implements the old QTro packet then).  */
+      if (traceframe_available_memory (&available, memaddr, len))
+	{
+	  struct cleanup *old_chain;
+
+	  old_chain = make_cleanup (VEC_cleanup(mem_range_s), &available);
+
+	  if (VEC_empty (mem_range_s, available)
+	      || VEC_index (mem_range_s, available, 0)->start != memaddr)
+	    {
+	      enum target_xfer_status res;
+
+	      /* Don't read into the traceframe's available
+		 memory.  */
+	      if (!VEC_empty (mem_range_s, available))
+		{
+		  LONGEST oldlen = len;
+
+		  len = VEC_index (mem_range_s, available, 0)->start - memaddr;
+		  gdb_assert (len <= oldlen);
+		}
+
+	      do_cleanups (old_chain);
+
+	      /* This goes through the topmost target again.  */
+	      res = memory_xfer_live_readonly_partial (&remote_ops,
+						       TARGET_OBJECT_MEMORY,
+						       myaddr, memaddr,
+						       len, xfered_len);
+	      if (res == TARGET_XFER_OK)
+		return TARGET_XFER_OK;
+	      else
+		{
+		  /* No use trying further, we know some memory starting
+		     at MEMADDR isn't available.  */
+		  *xfered_len = len;
+		  return TARGET_XFER_UNAVAILABLE;
+		}
+	    }
+
+	  /* Don't try to read more than how much is available, in
+	     case the target implements the deprecated QTro packet to
+	     cater for older GDBs (the target's knowledge of read-only
+	     sections may be outdated by now).  */
+	  len = VEC_index (mem_range_s, available, 0)->length;
+
+	  do_cleanups (old_chain);
+	}
+    }
+
   max_buf_size = get_memory_read_packet_size ();
   /* The packet buffer will be large enough for the payload;
      get_memory_packet_size ensures this.  */
diff --git a/gdb/target.c b/gdb/target.c
index f7868c0..c2669c6 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -961,87 +961,6 @@  target_section_by_addr (struct target_ops *target, CORE_ADDR addr)
   return NULL;
 }
 
-/* Read memory from the live target, even if currently inspecting a
-   traceframe.  The return is the same as that of target_read.  */
-
-static enum target_xfer_status
-target_read_live_memory (enum target_object object,
-			 ULONGEST memaddr, gdb_byte *myaddr, ULONGEST len,
-			 ULONGEST *xfered_len)
-{
-  enum target_xfer_status ret;
-  struct cleanup *cleanup;
-
-  /* Switch momentarily out of tfind mode so to access live memory.
-     Note that this must not clear global state, such as the frame
-     cache, which must still remain valid for the previous traceframe.
-     We may be _building_ the frame cache at this point.  */
-  cleanup = make_cleanup_restore_traceframe_number ();
-  set_traceframe_number (-1);
-
-  ret = target_xfer_partial (current_target.beneath, object, NULL,
-			     myaddr, NULL, memaddr, len, xfered_len);
-
-  do_cleanups (cleanup);
-  return ret;
-}
-
-/* Using the set of read-only target sections of OPS, read live
-   read-only memory.  Note that the actual reads start from the
-   top-most target again.
-
-   For interface/parameters/return description see target.h,
-   to_xfer_partial.  */
-
-static enum target_xfer_status
-memory_xfer_live_readonly_partial (struct target_ops *ops,
-				   enum target_object object,
-				   gdb_byte *readbuf, ULONGEST memaddr,
-				   ULONGEST len, ULONGEST *xfered_len)
-{
-  struct target_section *secp;
-  struct target_section_table *table;
-
-  secp = target_section_by_addr (ops, memaddr);
-  if (secp != NULL
-      && (bfd_get_section_flags (secp->the_bfd_section->owner,
-				 secp->the_bfd_section)
-	  & SEC_READONLY))
-    {
-      struct target_section *p;
-      ULONGEST memend = memaddr + len;
-
-      table = target_get_section_table (ops);
-
-      for (p = table->sections; p < table->sections_end; p++)
-	{
-	  if (memaddr >= p->addr)
-	    {
-	      if (memend <= p->endaddr)
-		{
-		  /* Entire transfer is within this section.  */
-		  return target_read_live_memory (object, memaddr,
-						  readbuf, len, xfered_len);
-		}
-	      else if (memaddr >= p->endaddr)
-		{
-		  /* This section ends before the transfer starts.  */
-		  continue;
-		}
-	      else
-		{
-		  /* This section overlaps the transfer.  Just do half.  */
-		  len = p->endaddr - memaddr;
-		  return target_read_live_memory (object, memaddr,
-						  readbuf, len, xfered_len);
-		}
-	    }
-	}
-    }
-
-  return TARGET_XFER_EOF;
-}
-
 /* Read memory from more than one valid target.  A core file, for
    instance, could have some of memory but delegate other bits to
    the target below it.  So, we must manually try all targets.  */
@@ -1149,63 +1068,6 @@  memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
 	}
     }
 
-  /* If reading unavailable memory in the context of traceframes, and
-     this address falls within a read-only section, fallback to
-     reading from live memory.  */
-  if (readbuf != NULL && get_traceframe_number () != -1)
-    {
-      VEC(mem_range_s) *available;
-
-      /* If we fail to get the set of available memory, then the
-	 target does not support querying traceframe info, and so we
-	 attempt reading from the traceframe anyway (assuming the
-	 target implements the old QTro packet then).  */
-      if (traceframe_available_memory (&available, memaddr, len))
-	{
-	  struct cleanup *old_chain;
-
-	  old_chain = make_cleanup (VEC_cleanup(mem_range_s), &available);
-
-	  if (VEC_empty (mem_range_s, available)
-	      || VEC_index (mem_range_s, available, 0)->start != memaddr)
-	    {
-	      /* Don't read into the traceframe's available
-		 memory.  */
-	      if (!VEC_empty (mem_range_s, available))
-		{
-		  LONGEST oldlen = len;
-
-		  len = VEC_index (mem_range_s, available, 0)->start - memaddr;
-		  gdb_assert (len <= oldlen);
-		}
-
-	      do_cleanups (old_chain);
-
-	      /* This goes through the topmost target again.  */
-	      res = memory_xfer_live_readonly_partial (ops, object,
-						       readbuf, memaddr,
-						       len, xfered_len);
-	      if (res == TARGET_XFER_OK)
-		return TARGET_XFER_OK;
-	      else
-		{
-		  /* No use trying further, we know some memory starting
-		     at MEMADDR isn't available.  */
-		  *xfered_len = len;
-		  return TARGET_XFER_UNAVAILABLE;
-		}
-	    }
-
-	  /* Don't try to read more than how much is available, in
-	     case the target implements the deprecated QTro packet to
-	     cater for older GDBs (the target's knowledge of read-only
-	     sections may be outdated by now).  */
-	  len = VEC_index (mem_range_s, available, 0)->length;
-
-	  do_cleanups (old_chain);
-	}
-    }
-
   /* Try GDB's internal data cache.  */
   region = lookup_mem_region (memaddr);
   /* region->hi == 0 means there's no upper bound.  */
diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
index cbf746d..a36596f 100644
--- a/gdb/tracefile-tfile.c
+++ b/gdb/tracefile-tfile.c
@@ -900,6 +900,7 @@  tfile_xfer_partial (struct target_ops *ops, enum target_object object,
   if (get_traceframe_number () != -1)
     {
       int pos = 0;
+      enum target_xfer_status res;
 
       /* Iterate through the traceframe's blocks, looking for
 	 memory.  */
@@ -937,7 +938,20 @@  tfile_xfer_partial (struct target_ops *ops, enum target_object object,
 	  pos += (8 + 2 + mlen);
 	}
 
-      return exec_read_partial_read_only (readbuf, offset, len, xfered_len);
+      /* Requested memory is unavailable in the context of traceframes,
+	 and this address falls within a read-only section, fallback
+	 to reading from executable.  */
+      res = exec_read_partial_read_only (readbuf, offset, len, xfered_len);
+
+      if (res == TARGET_XFER_OK)
+	return TARGET_XFER_OK;
+      else
+	{
+	  /* No use trying further, we know some memory starting
+	     at MEMADDR isn't available.  */
+	  *xfered_len = len;
+	  return TARGET_XFER_UNAVAILABLE;
+	}
     }
   else
     {