From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19707 invoked by alias); 14 Sep 2009 18:36:15 -0000 Received: (qmail 19371 invoked by uid 22791); 14 Sep 2009 18:36:14 -0000 X-SWARE-Spam-Status: No, hits=-2.3 required=5.0 tests=AWL,BAYES_00,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 14 Sep 2009 18:36:08 +0000 Received: (qmail 7316 invoked from network); 14 Sep 2009 18:36:06 -0000 Received: from unknown (HELO orlando) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 14 Sep 2009 18:36:06 -0000 From: Pedro Alves To: Michael Snyder Subject: Re: PRecord sets memory even when it says it did not Date: Mon, 14 Sep 2009 18:36:00 -0000 User-Agent: KMail/1.9.10 Cc: Marc Khouzam , "'gdb@sourceware.org'" , "'Greg Law'" , "'Hui Zhu'" , "'gdb-patches ml'" References: <200909141915.07943.pedro@codesourcery.com> <4AAE89C5.1030203@vmware.com> In-Reply-To: <4AAE89C5.1030203@vmware.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200909141936.10390.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-09/txt/msg00435.txt.bz2 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