Re: Seems like a bug in target_read_stack / dcache_xfer_memory?

[Michael Snyder <msnyder@vmware.com>]

Hui Zhu wrote:
> 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)

Yeah, I think you're right.
That warning used to be an error, so we wouldn't fall through,
but now we do.

However, this is not what causes my current problem.
I'll leave it up to you to find a fix for this one.    ;-)



> 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.
>>
>>
>>