RE: [patch] only update dcache after write succeeds

[Marc Khouzam <marc.khouzam@ericsson.com>]

 

> -----Original Message-----
> From: Michael Snyder [mailto:msnyder@vmware.com] 
> Sent: Monday, September 14, 2009 3:27 PM
> To: Doug Evans
> Cc: gdb-patches@sourceware.org; Marc Khouzam; Greg Law; Pedro 
> Alves; Hui Zhu
> Subject: Re: [patch] only update dcache after write succeeds
> 
> Doug Evans wrote:
> > Hi.
> > 
> > Marc, Greg: Can you see if this patch fixes things for you?
> 
> Hey Doug,
> 
> I tested this change, and it does indeed seem to fix the problem
> with target record -- but.   ;-)
> 
> Part of the reason that it works is that record_xfer_partial
> calls error() instead of returning -1.  If I change it so that
> it returns -1, things get more complicated.

Just a note that PRecord used to return -1 which caused memory
to be changed even when the query was answered with 'n'.
That is why Hui changed it to error()

> 
> The do-while loop that used to follow and now preceeds this code
> calls target-beneath, which in our case results in several different
> target methods being called, one of which eventually returns > 0.
> 
> That just means that in the present case, calling error is correct.
> But I worry about some of the other cases where the target method
> returns -1, and whether badness might occur in some other cases.
> 
> Michael
> 
> 
> > 2009-09-14  Doug Evans  <dje@google.com>
> > 
> > 	* target.c (memory_xfer_partial): Only update dcache 
> after we know
> > 	the write succeeded.
> > 
> > Index: target.c
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/target.c,v
> > retrieving revision 1.223
> > diff -u -p -r1.223 target.c
> > --- target.c	8 Sep 2009 23:52:45 -0000	1.223
> > +++ target.c	14 Sep 2009 19:11:38 -0000
> > @@ -1289,19 +1289,6 @@ memory_xfer_partial (struct target_ops *
> >  	}
> >      }
> >  
> > -  /* 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);
> > -    }
> > -
> >    /* If none of those methods found the memory we wanted, fall back
> >       to a target partial transfer.  Normally a single call to
> >       to_xfer_partial is enough; if it doesn't recognize an object
> > @@ -1331,6 +1318,20 @@ memory_xfer_partial (struct target_ops *
> >    if (readbuf && !show_memory_breakpoints)
> >      breakpoint_restore_shadows (readbuf, memaddr, reg_len);
> >  
> > +  /* 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 (res > 0
> > +      && inf != NULL
> > +      && writebuf != NULL
> > +      && !region->attrib.cache
> > +      && stack_cache_enabled_p
> > +      && object != TARGET_OBJECT_STACK_MEMORY)
> > +    {
> > +      dcache_update (target_dcache, memaddr, (void *) 
> writebuf, reg_len);
> > +    }
> > +
> >    /* If we still haven't got anything, return the last error.  We
> >       give up.  */
> >    return res;
> 
>