Re: Seems like a bug in target_read_stack / dcache_xfer_memory?

[Hui Zhu <teawater@gmail.com>]

I just want talk to you about it.

+            if (target_read_memory (entry->u.mem.addr, mem, entry->u.mem.len))
+              {
+                entry->u.mem.mem_entry_not_accessible = 1;
+                if (record_debug)
+                  warning (_("Process record: error reading memory at "
+                             "addr = %s len = %d."),
+                           paddress (gdbarch, entry->u.mem.addr),
+                           entry->u.mem.len);
+              }
+            else
+              {
+                if (target_write_memory (entry->u.mem.addr,
+					 record_get_loc (entry),
+					 entry->u.mem.len))

Maybe we need change it to:
 if (target_read_memory (entry->u.mem.addr, mem, entry->u.mem.len) <= 0)
 if (target_write_memory (entry->u.mem.addr,
+					 record_get_loc (entry),
+					 entry->u.mem.len) <= 0)

Thanks,
Hui

On Mon, Oct 19, 2009 at 06:31, Michael Snyder <msnyder@vmware.com> wrote:
> OK, this is right at the hairy edge of my understanding, and I
> admit up front that I did not carefully follow the email thread, but...
>
> There was this email thread about reading beyond the stack, and
> using dcache, which I did not follow carefully.  And now I think
> I'm running into it.
>
> Short version:  code in memory_xfer_partial looks like this:
>
> 1280            /* FIXME drow/2006-08-09: If we're going to preserve const
> 1281               correctness dcache_xfer_memory should take readbuf and
> 1282               writebuf.  */
> 1283            res = dcache_xfer_memory (ops, target_dcache, memaddr,
> 1284                                      (void *) writebuf,
> 1285                                      reg_len, 1);
> 1286          if (res <= 0)
> 1287            return -1;
>
> I think that's wrong.  I think it needs to test for "res == 0".
>
> Comment at dcache_xfer_memory says:
>
>  The meaning of the result is the same as for target_write
>
> (Gripe: someone please fix that comment.  Why should I have to
> go find another function in another file to find out what this
> function returns?)
>
> So the comment at target_write says...  oh wait!  There is no
> comment at target_write!  I'm afraid I'm going to have to start
> getting grumpy now...
>
> Well, that returns target_write_with_progress.  The comment
> there makes no mention of the return value.  But it returns
> target_write_partial.  STILL no comment about the return value.
> But this returns target_xfer_partial.  GUESS WHAT?
>
> Well, this returns memory_xfer_partial, which is right back
> where I started.  And *its* comment says:
>
>  The arguments and return
>  value are just as for target_xfer_partial
>
> Ummm,  come on guys.  Its Sunday and I've had a long day.
> Joke's getting old.  Whoever took the comments, please put them back.
>
> Anyway, I don't even remember now how I figured this out, but
> I *THINK* what all these guys return is either 0 for success,
> or an errno value less than zoro.
>
> And if that's true, then line 1286 up there needs to bail out
> on zero and let the other target stack methods have a chance
> to read the memory.
>
> Eh?
>
> We now return you to the nice Michael.   ;-)
>
> Daniel, you're not the target, you're only Cc:ed because your
> name is in the one comment that I *did* find.
>
>
>