From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23010 invoked by alias); 14 Sep 2009 19:08:50 -0000 Received: (qmail 22993 invoked by uid 22791); 14 Sep 2009 19:08:48 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from smtp-outbound-2.vmware.com (HELO smtp-outbound-2.vmware.com) (65.115.85.73) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 14 Sep 2009 19:08:44 +0000 Received: from mailhost4.vmware.com (mailhost4.vmware.com [10.16.67.124]) by smtp-outbound-2.vmware.com (Postfix) with ESMTP id D26095800C; Mon, 14 Sep 2009 12:08:40 -0700 (PDT) Received: from [10.20.94.141] (msnyder-server.eng.vmware.com [10.20.94.141]) by mailhost4.vmware.com (Postfix) with ESMTP id C58CDC9BFC; Mon, 14 Sep 2009 12:08:40 -0700 (PDT) Message-ID: <4AAE94CA.3010303@vmware.com> Date: Mon, 14 Sep 2009 19:08:00 -0000 From: Michael Snyder User-Agent: Thunderbird 1.5.0.12 (X11/20080411) MIME-Version: 1.0 To: Doug Evans CC: Pedro Alves , Marc Khouzam , "gdb@sourceware.org" , Greg Law , Hui Zhu , gdb-patches ml Subject: Re: PRecord sets memory even when it says it did not References: <200909141915.07943.pedro@codesourcery.com> <4AAE89C5.1030203@vmware.com> <200909141936.10390.pedro@codesourcery.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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/msg00438.txt.bz2 Doug Evans wrote: > On Mon, Sep 14, 2009 at 11:36 AM, Pedro Alves 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.