Seems like a bug in target_read_stack / dcache_xfer_memory?

Michael Snyder
2009-10-18T22:37:59Z

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.