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

Pedro Alves
2009-09-14T18:36:13Z

A Monday 14 September 2009 19:21:57, Michael Snyder escreveu:
> Pedro Alves wrote:
> > A Monday 14 September 2009 18:59:46, Michael Snyder escreveu:
> >> Pedro Alves wrote:
> >>> On Monday 14 September 2009 18:53:51, Marc Khouzam wrote:
> >>>>> -----Original Message-----
> >>>>> From: Pedro Alves [mailto:pedro@codesourcery.com]
> >>>>> Sent: Monday, September 14, 2009 1:50 PM
> >>>>> To: gdb@sourceware.org
> >>>>> Cc: Greg Law; Hui Zhu; Marc Khouzam; Michael Snyder; gdb-patches ml
> >>>>> Subject: Re: PRecord sets memory even when it says it did not
> >>>>>
> >>>>>> Could this be related to the caching changes that have happened
> >>>>>> recently? i.e. does the cache get updated even though the
> >>>>> underlying
> >>>>>> poke operation failed? If so, this issue would seem to be
> >>>>> wider than
> >>>>>> just prec (and wider than reverse, too).
> >>>>> If so, then there's an easy way to find out: try again with
> >>>>> "set stack-cache off".
> >>>>>
> >>>> Yes, that seems to fix everything.
> >>> Then I'd suspect either a bug dcache_xfer_memory, or a missing
> >>> target_dcache_invalidate/dcache_invalidate call somewhere.
> >> I'm not too familiar with this dcache.
> >> Is it up to the target to_xfer_memory method
> >> to invalidate it before erroring out?
> >
> > No. dcache_xfer_memory tries to handle it itself already, with
> > dcache_invalidate_line, but there's probably a bug over there, which
> > should be easy to catch stepping through dcache_xfer_memory in the
> > offending case. Note that it is dcache_xfer_memory
> > that calls the target to_xfer_memory routine, not
> > memory_xfer_partial.
> >
>
> Umm, that last sentence does not seem to be true.
> 1) I can see the call to ops->to_xfer_partial in memory_xfer_partial src
> 2) I've got a backtrace right here showing record_xfer_partial
> called by memory_xfer_partial.

Ah, oops. I missed the dcache_update call. Sorry, wasn't paying
that much attention.

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

??? Doug?

--
Pedro Alves