[PATCH] Delegate to target_ops->beneath to read cache lines

[PATCH] Delegate to target_ops->beneath to read cache lines

[Yao Qi]

GDB on x86_64-linux is unable to disassemble on core-file target.

$ ./gdb ./testsuite/gdb.base/corefile
(gdb) core-file ./testsuite/gdb.base/corefile.core
(gdb) disassemble main
Dump of assembler code for function main:
   0x0000000000400976 <+0>:	Cannot access memory at address 0x400976

However, it works if we turn code-cache off.

(gdb) set code-cache off
(gdb) disassemble main,+4
Dump of assembler code from 0x400976 to 0x40097a:
   0x0000000000400976 <main+0>:	push   %rbp
   0x0000000000400977 <main+1>:	mov    %rsp,%rbp
End of assembler dump.

When code-cache is off, GDB will iterate target_ops from top and call
to_xfer_partial.  When current_target is "core", it will call
to_xfer_partial of target "exec", which reads the contents for
disassemble.  However, dcache doesn't have such mechanism, and that is
the cause for the error.

This patch adds something similar in dcache_read_line to go through
target_ops from top to bottom, and call to_xfer_partial.
The original code uses TARGET_OBJECT_RAW_MEMORY, which is replaced
by TARGET_OBJECT_MEMORY in target_xfer_partial,

      enum target_object raw_object = object;

      /* If this is a raw memory transfer, request the normal
	 memory object from other layers.  */
      if (raw_object == TARGET_OBJECT_RAW_MEMORY)
	raw_object = TARGET_OBJECT_MEMORY;

so we can use TARGET_OBJECT_MEMORY here.  Regression tested on
x86_64-linux.

gdb:

2013-11-27  Yao Qi  <yao@codesourcery.com>

	* dcache.c (dcache_read_line): Don't call target_read.  Use
	beneath->to_xfer_partial in a loop.
---
 gdb/dcache.c |   23 +++++++++++++++++++----
 1 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/gdb/dcache.c b/gdb/dcache.c
index ea2b732..f52fc17 100644
--- a/gdb/dcache.c
+++ b/gdb/dcache.c
@@ -313,6 +313,7 @@ dcache_read_line (DCACHE *dcache, struct dcache_block *db)
   int res;
   int reg_len;
   struct mem_region *region;
+  struct target_ops *ops = current_target.beneath;
 
   len = dcache->line_size;
   memaddr = db->addr;
@@ -336,10 +337,24 @@ dcache_read_line (DCACHE *dcache, struct dcache_block *db)
 	  len     -= reg_len;
 	  continue;
 	}
-      
-      res = target_read (&current_target, TARGET_OBJECT_RAW_MEMORY,
-			 NULL, myaddr, memaddr, reg_len);
-      if (res < reg_len)
+
+      do
+	{
+	  res = ops->to_xfer_partial (ops, TARGET_OBJECT_MEMORY, NULL,
+				      myaddr, NULL, memaddr, reg_len);
+	  if (res > 0)
+	    break;
+
+	  /* We want to continue past core files to executables, but not
+	     past a running target's memory.  */
+	  if (ops->to_has_all_memory (ops))
+	    break;
+
+	  ops = ops->beneath;
+	}
+      while (ops != NULL);
+
+      if (res <= 0)
 	return 0;
 
       memaddr += res;
-- 
1.7.7.6

Re: [PATCH] Delegate to target_ops->beneath to read cache lines

[Pedro Alves]

On 11/27/2013 12:20 PM, Yao Qi wrote:
> GDB on x86_64-linux is unable to disassemble on core-file target.
> 
> $ ./gdb ./testsuite/gdb.base/corefile
> (gdb) core-file ./testsuite/gdb.base/corefile.core
> (gdb) disassemble main
> Dump of assembler code for function main:
>    0x0000000000400976 <+0>:	Cannot access memory at address 0x400976
> 
> However, it works if we turn code-cache off.
> 
> (gdb) set code-cache off
> (gdb) disassemble main,+4
> Dump of assembler code from 0x400976 to 0x40097a:
>    0x0000000000400976 <main+0>:	push   %rbp
>    0x0000000000400977 <main+1>:	mov    %rsp,%rbp
> End of assembler dump.
> 
> When code-cache is off, GDB will iterate target_ops from top and call
> to_xfer_partial.  When current_target is "core", it will call
> to_xfer_partial of target "exec", which reads the contents for
> disassemble.  However, dcache doesn't have such mechanism, and that is
> the cause for the error.

This points out that we end up caching core and exec code, which
isn't really necessary.  We could limit it to has_all_memory targets,
I think.  Not sure if that'd complicate things.

> This patch adds something similar in dcache_read_line to go through
> target_ops from top to bottom, and call to_xfer_partial.
> The original code uses TARGET_OBJECT_RAW_MEMORY, which is replaced
> by TARGET_OBJECT_MEMORY in target_xfer_partial,
> 
>       enum target_object raw_object = object;
> 
>       /* If this is a raw memory transfer, request the normal
> 	 memory object from other layers.  */
>       if (raw_object == TARGET_OBJECT_RAW_MEMORY)
> 	raw_object = TARGET_OBJECT_MEMORY;
> 
> so we can use TARGET_OBJECT_MEMORY here.  Regression tested on
> x86_64-linux.

The dcache is the sole user of TARGET_OBJECT_RAW_MEMORY.  This shows
that if we want to code from lower targets too, then this sole user also
wants to do the top to bottom delegation that memory_xfer_partial_1 does.

So this can all be done within target.c.  Factor out the
memory_xfer_partial_1 top to bottom memory read code to a separate
raw_memory_xfer_partial function (despite the name, it'd request
TARGET_OBJECT_MEMORY from the targets), and make target_xfer_partial
call that for TARGET_OBJECT_RAW_MEMORY:

LONGEST
target_xfer_partial (struct target_ops *ops,
		     enum target_object object, const char *annex,
		     void *readbuf, const void *writebuf,
		     ULONGEST offset, LONGEST len)
{
  LONGEST retval;

  /* If this is a memory transfer, let the memory-specific code
     have a look at it instead.  Memory transfers are more
     complicated.  */
  if (object == TARGET_OBJECT_MEMORY || object == TARGET_OBJECT_STACK_MEMORY
      || object == TARGET_OBJECT_CODE_MEMORY)
    retval = memory_xfer_partial (ops, object, readbuf,
				  writebuf, offset, len);
  else if (object == TARGET_OBJECT_RAW_MEMORY)
    {
      /* Request the normal memory object from other layers.  */
      retval = raw_memory_xfer_partial (ops, readbuf, writebuf, offset, len);
    }
  else
    {
      retval = ops->to_xfer_partial (ops, raw_object, annex, readbuf,
				     writebuf, offset, len);
    }

Then dcache.c doesn't need to change, and doesn't need to inline that
top to bottom dance.

-- 
Pedro Alves

Re: [PATCH] Delegate to target_ops->beneath to read cache lines

[Yao Qi]

On 11/27/2013 09:49 PM, Pedro Alves wrote:
> The dcache is the sole user of TARGET_OBJECT_RAW_MEMORY.  This shows
> that if we want to code from lower targets too, then this sole user also
> wants to do the top to bottom delegation that memory_xfer_partial_1 does.
> 
> So this can all be done within target.c.  Factor out the
> memory_xfer_partial_1 top to bottom memory read code to a separate
> raw_memory_xfer_partial function (despite the name, it'd request
> TARGET_OBJECT_MEMORY from the targets), and make target_xfer_partial
> call that for TARGET_OBJECT_RAW_MEMORY:

Yeah, that looks cleaner than my patch.  How about this one?

-- 
Yao (齐尧)

Subject: [PATCH] Delegate to target_ops->beneath for TARGET_OBJECT_RAW_MEMORY

GDB on x86_64-linux is unable to disassemble on core-file target.

$ ./gdb ./testsuite/gdb.base/corefile
(gdb) core-file ./testsuite/gdb.base/corefile.core
(gdb) disassemble main
Dump of assembler code for function main:
   0x0000000000400976 <+0>:	Cannot access memory at address 0x400976

However, it works if we turn code-cache off.

(gdb) set code-cache off
(gdb) disassemble main,+4
Dump of assembler code from 0x400976 to 0x40097a:
   0x0000000000400976 <main+0>:	push   %rbp
   0x0000000000400977 <main+1>:	mov    %rsp,%rbp
End of assembler dump.

When code-cache is off, GDB will iterate target_ops from top to bottom
and call to_xfer_partial.  When current_target is "core", it will call
to_xfer_partial of target "exec", which reads the contents for
disassemble.  However, dcache uses TARGET_OBJECT_RAW_MEMORY to read,
but target_xfer_partial doesn't delegate requests to beneath for
TARGET_OBJECT_RAW_MEMORY.

This patch factors out the iteration from top to bottom to a new
function, raw_memory_xfer_partial, and use it for
TARGET_OBJECT_RAW_MEMORY.

Note that using &current_target in dcache_read_line will cause an
endless recursion, so I change it to current_target.beneath.  IMO,
other &current_target usages should be changed to
current_target.beneath too.

Regression tested on x86_64-linux.

gdb:

2013-11-29  Yao Qi  <yao@codesourcery.com>
	    Pedro Alves  <palves@redhat.com>

	* dcache.c (dcache_read_line): Use current_target.beneath
	instead of &current_target.
	* target.c (memory_xfer_partial_1): Factor code out to ...
	(raw_memory_xfer_partial): ... it.  New function.
	(target_xfer_partial): Call raw_memory_xfer_partial if OBJECT
	is TARGET_OBJECT_RAW_MEMORY.
---
 gdb/dcache.c |    4 +-
 gdb/target.c |   67 ++++++++++++++++++++++++++++++++--------------------------
 2 files changed, 39 insertions(+), 32 deletions(-)

diff --git a/gdb/dcache.c b/gdb/dcache.c
index ea2b732..12d1a4b 100644
--- a/gdb/dcache.c
+++ b/gdb/dcache.c
@@ -336,8 +336,8 @@ dcache_read_line (DCACHE *dcache, struct dcache_block *db)
 	  len     -= reg_len;
 	  continue;
 	}
-      
-      res = target_read (&current_target, TARGET_OBJECT_RAW_MEMORY,
+
+      res = target_read (current_target.beneath, TARGET_OBJECT_RAW_MEMORY,
 			 NULL, myaddr, memaddr, reg_len);
       if (res < reg_len)
 	return 0;
diff --git a/gdb/target.c b/gdb/target.c
index cc6194b..6c72e70 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1398,6 +1398,35 @@ memory_xfer_live_readonly_partial (struct target_ops *ops,
   return 0;
 }
 
+/* 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.  */
+
+static LONGEST
+raw_memory_xfer_partial (struct target_ops *ops, void *readbuf,
+			 const void *writebuf, ULONGEST memaddr, LONGEST len)
+{
+  LONGEST res;
+
+  do
+    {
+      res = ops->to_xfer_partial (ops, TARGET_OBJECT_MEMORY, NULL,
+				  readbuf, writebuf, memaddr, len);
+      if (res > 0)
+	break;
+
+      /* We want to continue past core files to executables, but not
+	 past a running target's memory.  */
+      if (ops->to_has_all_memory (ops))
+	break;
+
+      ops = ops->beneath;
+    }
+  while (ops != NULL);
+
+  return res;
+}
+
 /* Perform a partial memory transfer.
    For docs see target.h, to_xfer_partial.  */
 
@@ -1571,26 +1600,8 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
      to_xfer_partial is enough; if it doesn't recognize an object
      it will call the to_xfer_partial of the next target down.
      But for memory this won't do.  Memory is the only target
-     object which can be read 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.  */
-
-  do
-    {
-      res = ops->to_xfer_partial (ops, TARGET_OBJECT_MEMORY, NULL,
-				  readbuf, writebuf, memaddr, reg_len);
-      if (res > 0)
-	break;
-
-      /* We want to continue past core files to executables, but not
-	 past a running target's memory.  */
-      if (ops->to_has_all_memory (ops))
-	break;
-
-      ops = ops->beneath;
-    }
-  while (ops != NULL);
+     object which can be read from more than one valid target.  */
+  res = raw_memory_xfer_partial (ops, readbuf, writebuf, memaddr, reg_len);
 
   /* Make sure the cache gets updated no matter what - if we are writing
      to the stack.  Even if this write is not tagged as such, we still need
@@ -1702,18 +1713,14 @@ target_xfer_partial (struct target_ops *ops,
       || object == TARGET_OBJECT_CODE_MEMORY)
     retval = memory_xfer_partial (ops, object, readbuf,
 				  writebuf, offset, len);
-  else
+  else if (object == TARGET_OBJECT_RAW_MEMORY)
     {
-      enum target_object raw_object = object;
-
-      /* If this is a raw memory transfer, request the normal
-	 memory object from other layers.  */
-      if (raw_object == TARGET_OBJECT_RAW_MEMORY)
-	raw_object = TARGET_OBJECT_MEMORY;
-
-      retval = ops->to_xfer_partial (ops, raw_object, annex, readbuf,
-				     writebuf, offset, len);
+      /* Request the normal memory object from other layers.  */
+      retval = raw_memory_xfer_partial (ops, readbuf, writebuf, offset, len);
     }
+  else
+    retval = ops->to_xfer_partial (ops, object, annex, readbuf,
+				   writebuf, offset, len);
 
   if (targetdebug)
     {
-- 
1.7.7.6

Re: [PATCH] Delegate to target_ops->beneath to read cache lines

[Pedro Alves]

Patch is OK, thanks.

A comment on the commit log:

On 11/29/2013 02:52 AM, Yao Qi wrote:
> 
> Note that using &current_target in dcache_read_line will cause an
> endless recursion, so I change it to current_target.beneath.  

Hmm, that's not what I recall and documented in
target_read_memory, etc.

  /* Dispatch to the topmost target, not the flattened current_target.
     Memory accesses check target->to_has_(all_)memory, and the
     flattened target doesn't inherit those.  */

And indeed, if I tweak the patch to drop that hunk,
that's still what I see.  What recursion did you see?

-- 
Pedro Alves

Re: [PATCH] Delegate to target_ops->beneath to read cache lines

[Yao Qi]

On 11/29/2013 08:08 PM, Pedro Alves wrote:
> Hmm, that's not what I recall and documented in
> target_read_memory, etc.
> 
>    /* Dispatch to the topmost target, not the flattened current_target.
>       Memory accesses check target->to_has_(all_)memory, and the
>       flattened target doesn't inherit those.  */
> 
> And indeed, if I tweak the patch to drop that hunk,
> that's still what I see.  What recursion did you see?

I can see a segmentation fault instead of a endless recursion today :-/

(top-gdb) n
1420          if (ops->to_has_all_memory (ops))
(top-gdb) p ops->to_has_all_memory
$13 = (int (*)(struct target_ops *)) 0x0
(top-gdb) p current_target.to_has_all_memory
$14 = (int (*)(struct target_ops *)) 0x0

The quoted documentation correctly explainss the behaviour here.

The endless recursion I saw yesterday is that both current_target.to_xfer_partial
and current_target.beneath->to_xfer_partial are core_xfer_partial.  Looks
like it delegates to "itself", and causes an endless recursion, so I use
current_target.beneath instead of &current_target.  I may mess up
something then.

> Note that using &current_target in dcache_read_line will cause an
> endless recursion, so I change it to current_target.beneath.  IMO,
> other &current_target usages should be changed to
> current_target.beneath too.

I'll get rid of it from commit log.

-- 
Yao (齐尧)

Re: [PATCH] Delegate to target_ops->beneath to read cache lines

[Pedro Alves]

On 11/29/2013 01:28 PM, Yao Qi wrote:
> The endless recursion I saw yesterday is that both current_target.to_xfer_partial
> and current_target.beneath->to_xfer_partial are core_xfer_partial.  Looks
> like it delegates to "itself", and causes an endless recursion, so I use
> current_target.beneath instead of &current_target.  I may mess up
> something then.

Not really.  to_xfer_partial is not INHERITed in update_current_target.
current_target.to_xfer_partial is set to current_xfer_partial, which
always delegates to current_target.beneath.

Even it is was inherited, I don't think we'd recurse forever,
because although the callback would be the same, the target_ops
instances are not.  It's just that core_xfer_partial would end
up unnecessarily being called twice, before reaching the the exec target:

current_target.to_xfer_partial
  ==> core_xfer_partial  (squashed target, returns 0, try beneath)

current_target.beneath->to_xfer_partial
  ==> core_xfer_partial  (corelow target, returns 0, try beneath)

current_target.beneath->beneath->to_xfer_partial
  ==> exec_xfer_partial  (exec target, returns != 0)

(This is all less clear than it should be;  I think
Tromey's target delegation method patch should set us
in the direction of making these things a little clearer.)

-- 
Pedro Alves

Re: [PATCH] Delegate to target_ops->beneath to read cache lines

[Doug Evans]

On Thu, Nov 28, 2013 at 6:52 PM, Yao Qi <yao@codesourcery.com> wrote:
> [...]
> 2013-11-29  Yao Qi  <yao@codesourcery.com>
>             Pedro Alves  <palves@redhat.com>
>
>         * dcache.c (dcache_read_line): Use current_target.beneath
>         instead of &current_target.
>         * target.c (memory_xfer_partial_1): Factor code out to ...
>         (raw_memory_xfer_partial): ... it.  New function.
>         (target_xfer_partial): Call raw_memory_xfer_partial if OBJECT
>         is TARGET_OBJECT_RAW_MEMORY.
> ---
>  gdb/dcache.c |    4 +-
>  gdb/target.c |   67 ++++++++++++++++++++++++++++++++--------------------------
>  2 files changed, 39 insertions(+), 32 deletions(-)
>
> diff --git a/gdb/dcache.c b/gdb/dcache.c
> index ea2b732..12d1a4b 100644
> --- a/gdb/dcache.c
> +++ b/gdb/dcache.c
> @@ -336,8 +336,8 @@ dcache_read_line (DCACHE *dcache, struct dcache_block *db)
>           len     -= reg_len;
>           continue;
>         }
> -
> -      res = target_read (&current_target, TARGET_OBJECT_RAW_MEMORY,
> +
> +      res = target_read (current_target.beneath, TARGET_OBJECT_RAW_MEMORY,
>                          NULL, myaddr, memaddr, reg_len);
>        if (res < reg_len)
>         return 0;
> [...]

I think a comment is required here explaining why things are the way they are.
i.e., why we use current_target.beneath instead of &current_target.

Re: [PATCH] Delegate to target_ops->beneath to read cache lines

[Pedro Alves]

On 11/29/2013 07:42 PM, Doug Evans wrote:
> I think a comment is required here explaining why things are the way they are.
> i.e., why we use current_target.beneath instead of &current_target.

I suggest adding a target_read_raw_memory function, similar,
and next to target_read_memory, target_read_stack, etc., and
put the comment there.  Incidentally, I notice target_read_code
misses the comment.

-- 
Pedro Alves

Re: [PATCH] Delegate to target_ops->beneath to read cache lines

[Yao Qi]

On 11/30/2013 03:42 AM, Doug Evans wrote:
> I think a comment is required here explaining why things are the way they are.
> i.e., why we use current_target.beneath instead of &current_target.

I find the comments in target_read_memory can do some explanations,
so I copy that paragraph here.  Is it clear to you?

-- 
Yao (齐尧)

gdb:

2013-12-02  Yao Qi  <yao@codesourcery.com>

	* dcache.c (dcache_read_line): Add comments.
---
 gdb/dcache.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/gdb/dcache.c b/gdb/dcache.c
index 12d1a4b..0707cdf 100644
--- a/gdb/dcache.c
+++ b/gdb/dcache.c
@@ -337,6 +337,9 @@ dcache_read_line (DCACHE *dcache, struct dcache_block *db)
 	  continue;
 	}
 
+      /* Dispatch to the topmost target, not the flattened current_target.
+	 Memory accesses check target->to_has_(all_)memory, and the
+	 flattened target doesn't inherit those.  */
       res = target_read (current_target.beneath, TARGET_OBJECT_RAW_MEMORY,
 			 NULL, myaddr, memaddr, reg_len);
       if (res < reg_len)
-- 
1.7.7.6

Re: [PATCH] Delegate to target_ops->beneath to read cache lines

[Pedro Alves]

On 12/02/2013 06:05 AM, Yao Qi wrote:
> On 11/30/2013 03:42 AM, Doug Evans wrote:
>> I think a comment is required here explaining why things are the way they are.
>> i.e., why we use current_target.beneath instead of &current_target.
> 
> I find the comments in target_read_memory can do some explanations,
> so I copy that paragraph here.  Is it clear to you?
> 

I'd much prefer following my suggestion:

  https://sourceware.org/ml/gdb-patches/2013-11/msg00941.html

Note we already have a target_write_raw_memory function.

Do you see a problem with this?

---------
Add new target_read_raw_memory function, and consolidate comments.

Tested on x86_64 Fedora 17.

gdb/
2013-12-02  Pedro Alves  <palves@redhat.com>

	* dcache.c (dcache_read_line): Use target_read_raw_memory.
	* target.c (target_read_raw_memory): New function.
	(target_read_stack, target_write_memory, target_write_raw_memory):
	Update comment.
	(target_read_core): Add comment.
	* target.h (target_read_raw_memory): Declare.
---

 gdb/dcache.c |   11 +++++------
 gdb/target.c |   34 ++++++++++++++++++++++++----------
 gdb/target.h |    3 +++
 3 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/gdb/dcache.c b/gdb/dcache.c
index 12d1a4b..804d567 100644
--- a/gdb/dcache.c
+++ b/gdb/dcache.c
@@ -337,14 +337,13 @@ dcache_read_line (DCACHE *dcache, struct dcache_block *db)
 	  continue;
 	}
 
-      res = target_read (current_target.beneath, TARGET_OBJECT_RAW_MEMORY,
-			 NULL, myaddr, memaddr, reg_len);
-      if (res < reg_len)
+      res = target_read_raw_memory (memaddr, myaddr, reg_len);
+      if (res != 0)
 	return 0;
 
-      memaddr += res;
-      myaddr += res;
-      len -= res;
+      memaddr += reg_len;
+      myaddr += reg_len;
+      len -= reg_len;
     }
 
   return 1;
diff --git a/gdb/target.c b/gdb/target.c
index 6c72e70..85b5037 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1791,16 +1791,30 @@ target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
     return TARGET_XFER_E_IO;
 }
 
+/* Like target_read_memory, but specify explicitly that this is a read
+   from the target's raw memory.  That is, this read bypasses the
+   dcache, breakpoint shadowing, etc.  */
+
+int
+target_read_raw_memory (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
+{
+  /* See comment in target_read_memory about why the request starts at
+     current_target.beneath.  */
+  if (target_read (current_target.beneath, TARGET_OBJECT_RAW_MEMORY, NULL,
+		   myaddr, memaddr, len) == len)
+    return 0;
+  else
+    return TARGET_XFER_E_IO;
+}
+
 /* Like target_read_memory, but specify explicitly that this is a read from
    the target's stack.  This may trigger different cache behavior.  */
 
 int
 target_read_stack (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
 {
-  /* Dispatch to the topmost target, not the flattened current_target.
-     Memory accesses check target->to_has_(all_)memory, and the
-     flattened target doesn't inherit those.  */
-
+  /* See comment in target_read_memory about why the request starts at
+     current_target.beneath.  */
   if (target_read (current_target.beneath, TARGET_OBJECT_STACK_MEMORY, NULL,
 		   myaddr, memaddr, len) == len)
     return 0;
@@ -1814,6 +1828,8 @@ target_read_stack (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
 int
 target_read_code (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
 {
+  /* See comment in target_read_memory about why the request starts at
+     current_target.beneath.  */
   if (target_read (current_target.beneath, TARGET_OBJECT_CODE_MEMORY, NULL,
 		   myaddr, memaddr, len) == len)
     return 0;
@@ -1830,9 +1846,8 @@ target_read_code (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
 int
 target_write_memory (CORE_ADDR memaddr, const gdb_byte *myaddr, ssize_t len)
 {
-  /* Dispatch to the topmost target, not the flattened current_target.
-     Memory accesses check target->to_has_(all_)memory, and the
-     flattened target doesn't inherit those.  */
+  /* See comment in target_read_memory about why the request starts at
+     current_target.beneath.  */
   if (target_write (current_target.beneath, TARGET_OBJECT_MEMORY, NULL,
 		    myaddr, memaddr, len) == len)
     return 0;
@@ -1849,9 +1864,8 @@ target_write_memory (CORE_ADDR memaddr, const gdb_byte *myaddr, ssize_t len)
 int
 target_write_raw_memory (CORE_ADDR memaddr, const gdb_byte *myaddr, ssize_t len)
 {
-  /* Dispatch to the topmost target, not the flattened current_target.
-     Memory accesses check target->to_has_(all_)memory, and the
-     flattened target doesn't inherit those.  */
+  /* See comment in target_read_memory about why the request starts at
+     current_target.beneath.  */
   if (target_write (current_target.beneath, TARGET_OBJECT_RAW_MEMORY, NULL,
 		    myaddr, memaddr, len) == len)
     return 0;
diff --git a/gdb/target.h b/gdb/target.h
index 890171d..f22e5c6 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1051,6 +1051,9 @@ extern int target_read_string (CORE_ADDR, char **, int, int *);
 extern int target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr,
 			       ssize_t len);
 
+extern int target_read_raw_memory (CORE_ADDR memaddr, gdb_byte *myaddr,
+				   ssize_t len);
+
 extern int target_read_stack (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len);
 
 extern int target_read_code (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len);

Re: [PATCH] Delegate to target_ops->beneath to read cache lines

[Yao Qi]

On 12/02/2013 06:36 PM, Pedro Alves wrote:
> Do you see a problem with this?
>

No.

> gdb/
> 2013-12-02  Pedro Alves<palves@redhat.com>
>
> 	* dcache.c (dcache_read_line): Use target_read_raw_memory.
> 	* target.c (target_read_raw_memory): New function.
> 	(target_read_stack, target_write_memory, target_write_raw_memory):
> 	Update comment.
> 	(target_read_core): Add comment.
                     ^^^^^
target_read_code.

-- 
Yao (齐尧)

Re: [PATCH] Delegate to target_ops->beneath to read cache lines

[Pedro Alves]

On 12/02/2013 11:02 AM, Yao Qi wrote:
> On 12/02/2013 06:36 PM, Pedro Alves wrote:
>> Do you see a problem with this?
>>
> 
> No.

Thanks.  I pushed it in (...)

>> gdb/
>> 2013-12-02  Pedro Alves<palves@redhat.com>
>>
>> 	* dcache.c (dcache_read_line): Use target_read_raw_memory.
>> 	* target.c (target_read_raw_memory): New function.
>> 	(target_read_stack, target_write_memory, target_write_raw_memory):
>> 	Update comment.
>> 	(target_read_core): Add comment.
>                      ^^^^^
> target_read_code.

(...) with that fixed.

-- 
Pedro Alves