From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20617 invoked by alias); 14 Sep 2009 19:04:25 -0000 Received: (qmail 20590 invoked by uid 22791); 14 Sep 2009 19:04:23 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00,SARE_MSGID_LONG40,SPF_PASS X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (216.239.45.13) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 14 Sep 2009 19:04:16 +0000 Received: from zps18.corp.google.com (zps18.corp.google.com [172.25.146.18]) by smtp-out.google.com with ESMTP id n8EJ4FSd019975; Mon, 14 Sep 2009 12:04:15 -0700 Received: from ywh29 (ywh29.prod.google.com [10.192.8.29]) by zps18.corp.google.com with ESMTP id n8EJ43fA005146; Mon, 14 Sep 2009 12:04:13 -0700 Received: by ywh29 with SMTP id 29so5090418ywh.23 for ; Mon, 14 Sep 2009 12:04:12 -0700 (PDT) MIME-Version: 1.0 Received: by 10.150.16.28 with SMTP id 28mr10717411ybp.133.1252955052593; Mon, 14 Sep 2009 12:04:12 -0700 (PDT) In-Reply-To: <200909141936.10390.pedro@codesourcery.com> References: <200909141915.07943.pedro@codesourcery.com> <4AAE89C5.1030203@vmware.com> <200909141936.10390.pedro@codesourcery.com> Date: Mon, 14 Sep 2009 19:04:00 -0000 Message-ID: Subject: Re: PRecord sets memory even when it says it did not From: Doug Evans To: Pedro Alves Cc: Michael Snyder , Marc Khouzam , "gdb@sourceware.org" , Greg Law , Hui Zhu , gdb-patches ml Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-System-Of-Record: true 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/msg00437.txt.bz2 On Mon, Sep 14, 2009 at 11:36 AM, Pedro Alves wrot= e: > Hmmm. =A0On 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 > > - =A0inf =3D find_inferior_pid (ptid_get_pid (inferior_ptid)); > - > - =A0if (inf !=3D NULL > - =A0 =A0 =A0&& (region->attrib.cache > - =A0 =A0 =A0 =A0 || (stack_cache_enabled_p && object =3D=3D TARGET_OBJEC= T_STACK_MEMORY))) > - =A0 =A0{ > - =A0 =A0 =A0if (readbuf !=3D NULL) > - =A0 =A0 =A0 res =3D dcache_xfer_memory (ops, target_dcache, memaddr, re= adbuf, > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 reg_len= , 0); > - =A0 =A0 =A0else > - =A0 =A0 =A0 /* FIXME drow/2006-08-09: If we're going to preserve const > - =A0 =A0 =A0 =A0 =A0correctness dcache_xfer_memory should take readbuf a= nd > - =A0 =A0 =A0 =A0 =A0writebuf. =A0*/ > - =A0 =A0 =A0 res =3D dcache_xfer_memory (ops, target_dcache, memaddr, > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (void *= ) writebuf, > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 reg_len= , 1); > - =A0 =A0 =A0if (res <=3D 0) > - =A0 =A0 =A0 return -1; > - =A0 =A0 =A0else > - =A0 =A0 =A0 { > - =A0 =A0 =A0 =A0 if (readbuf && !show_memory_breakpoints) > - =A0 =A0 =A0 =A0 =A0 breakpoint_restore_shadows (readbuf, memaddr, reg_l= en); > - =A0 =A0 =A0 =A0 return res; > - =A0 =A0 =A0 } > - =A0 =A0} > - > - =A0/* Make sure the cache gets updated no matter what - if we are writi= ng > - =A0 =A0 to the stack, even if this write is not tagged as such, we stil= l need > - =A0 =A0 to update the cache. */ > - > - =A0if (inf !=3D NULL > - =A0 =A0 =A0&& readbuf =3D=3D NULL > - =A0 =A0 =A0&& !region->attrib.cache > - =A0 =A0 =A0&& stack_cache_enabled_p > - =A0 =A0 =A0&& object !=3D TARGET_OBJECT_STACK_MEMORY) > - =A0 =A0{ > - =A0 =A0 =A0dcache_update (target_dcache, memaddr, (void *) writebuf, re= g_len); > - =A0 =A0} > > > and replaced this call below, something like so: > > =A0do > =A0 =A0{ > - =A0 =A0 =A0res =3D ops->to_xfer_partial (ops, TARGET_OBJECT_MEMORY, NUL= L, > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 readbuf= , writebuf, memaddr, reg_len); > + =A0 =A0 =A0res =3D dcache_xfer_partial (ops, TARGET_OBJECT_MEMORY, NULL, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 readbuf= , writebuf, memaddr, reg_len); > > > =A0 =A0 =A0if (res > 0) > =A0 =A0 =A0 =A0break; > > =A0 =A0 =A0/* We want to continue past core files to executables, but not > =A0 =A0 =A0 =A0 past a running target's memory. =A0*/ > =A0 =A0 =A0if (ops->to_has_all_memory (ops)) > =A0 =A0 =A0 =A0break; > > =A0 =A0 =A0ops =3D ops->beneath; > =A0 =A0} > =A0while (ops !=3D NULL); > > ... by a dcache_xfer_memory call, but tweak its interface to pass it > the object type? =A0Things 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 processin= g. 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 !=3D NULL && readbuf =3D=3D NULL && !region->attrib.cache && stack_cache_enabled_p && object !=3D TARGET_OBJECT_STACK_MEMORY) { dcache_update (target_dcache, memaddr, (void *) writebuf, reg_len); } to after the call res =3D 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]