Re: PRecord sets memory even when it says it did not

Michael Snyder
2009-09-14T19:08:50Z

Doug Evans wrote:
> On Mon, Sep 14, 2009 at 11:36 AM, Pedro Alves <pedro@codesourcery.com> wrote:
>> Hmmm. On a less quicker look, how about if we get rid of the
>> dcache_xfer_memory and dcache_update calls in memory_xfer_partial,
>>
>> (excuse the pseudo-patch-written-in-email)
>>
>> target.c:memory_xfer_partial
>>
>> - inf = find_inferior_pid (ptid_get_pid (inferior_ptid));
>> -
>> - if (inf != NULL
>> - && (region->attrib.cache
>> - || (stack_cache_enabled_p && object == TARGET_OBJECT_STACK_MEMORY)))
>> - {
>> - if (readbuf != NULL)
>> - res = dcache_xfer_memory (ops, target_dcache, memaddr, readbuf,
>> - reg_len, 0);
>> - else
>> - /* FIXME drow/2006-08-09: If we're going to preserve const
>> - correctness dcache_xfer_memory should take readbuf and
>> - writebuf. */
>> - res = dcache_xfer_memory (ops, target_dcache, memaddr,
>> - (void *) writebuf,
>> - reg_len, 1);
>> - if (res <= 0)
>> - return -1;
>> - else
>> - {
>> - if (readbuf && !show_memory_breakpoints)
>> - breakpoint_restore_shadows (readbuf, memaddr, reg_len);
>> - return res;
>> - }
>> - }
>> -
>> - /* 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
>> - to update the cache. */
>> -
>> - if (inf != NULL
>> - && readbuf == NULL
>> - && !region->attrib.cache
>> - && stack_cache_enabled_p
>> - && object != TARGET_OBJECT_STACK_MEMORY)
>> - {
>> - dcache_update (target_dcache, memaddr, (void *) writebuf, reg_len);
>> - }
>>
>>
>> and replaced this call below, something like so:
>>
>> do
>> {
>> - res = ops->to_xfer_partial (ops, TARGET_OBJECT_MEMORY, NULL,
>> - readbuf, writebuf, memaddr, reg_len);
>> + res = dcache_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);
>>
>> ... by a dcache_xfer_memory call, but tweak its interface to pass it
>> the object type? Things would be tidier and dcache_xfer_memory
>> would then handle all this dcache updating/invalidating itself.
>>
>> On the plus side, when the dcache is in effect, with that change,
>> we'd again walk the whole target stack, which isn't true currently
>> (and looks like a possible design flaw).
>
> OTOH, its nice having memory_xfer_partial do the region attribute processing.
>
> It seems like what's needed is to move
>
> /* 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
> to update the cache. */
>
> if (inf != NULL
> && readbuf == NULL
> && !region->attrib.cache
> && stack_cache_enabled_p
> && object != TARGET_OBJECT_STACK_MEMORY)
> {
> dcache_update (target_dcache, memaddr, (void *) writebuf, reg_len);
> }
>
> to after the call
>
> res = ops->to_xfer_partial (ops, TARGET_OBJECT_MEMORY, NULL,
> readbuf, writebuf, memaddr, reg_len);
>
> predicated on res > 0.
>
> [dcache.c does the write first, and then updates the cache if it
> succeeded, we just need to do that here too, methinks]

I like this better.

BTW, I've been checking other implementations of xxx_xfer_partial,
and I can't find any that call error(), but there are plenty that
return -1. Seems like this failure would apply to all of those too.