Re: Seems like a bug in target_read_stack / dcache_xfer_memory?

[Hui Zhu <teawater at gmail dot 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.
>
>
>