From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12270 invoked by alias); 2 Aug 2009 04:11:06 -0000 Received: (qmail 12258 invoked by uid 22791); 2 Aug 2009 04:11:04 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL,BAYES_00,SARE_MSGID_LONG40,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail-pz0-f203.google.com (HELO mail-pz0-f203.google.com) (209.85.222.203) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 02 Aug 2009 04:10:59 +0000 Received: by pzk41 with SMTP id 41so2072884pzk.12 for ; Sat, 01 Aug 2009 21:10:57 -0700 (PDT) MIME-Version: 1.0 Received: by 10.142.245.17 with SMTP id s17mr151049wfh.225.1249186257094; Sat, 01 Aug 2009 21:10:57 -0700 (PDT) In-Reply-To: <4A74C90D.4040004@vmware.com> References: <4A69182D.9090004@vmware.com> <4A6B6E02.5080702@vmware.com> <4A74C90D.4040004@vmware.com> From: Hui Zhu Date: Sun, 02 Aug 2009 04:11:00 -0000 Message-ID: Subject: Re: [PREC/RFA] Add not_replay to make precord support release memory better To: Michael Snyder Cc: gdb-patches ml Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable 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-08/txt/msg00008.txt.bz2 I think this patch is very good. I a new changelog for it: 2009-08-02 Michael Snyder Hui Zhu * record.c (record_mem_entry): New field 'mem_entry_not_accessible'. (record_arch_list_add_mem): Initialize 'mem_entry_not_accessible' to 0. (record_wait): Set 'mem_entry_not_accessible' flag if target memory not readable. Don't try to change target memory if 'mem_entry_not_accessible' is set. Do you think it's ok? Thanks, Hui On Sun, Aug 2, 2009 at 07:00, Michael Snyder wrote: > Hui Zhu wrote: >> >> On Sun, Jul 26, 2009 at 04:41, Michael Snyder wrote: >>> >>> Hui Zhu wrote: >>>> >>>> On Fri, Jul 24, 2009 at 10:10, Michael Snyder wrot= e: >>>>> >>>>> 1) During the recording "pass", there's no change. >>>>> 2) During the reverse-replay pass, if the memory is >>>>> not readable or writable, we will set this flag to TRUE. >>>>> 3) Finally, during the forward-replay pass, if the flag >>>>> has previously been set to TRUE, we will skip this entry >>>>> (and set the flag to FALSE.) >>>>> >>>>> So my question is -- is there any reason to set it to FALSE here? >>>>> Is there any way that the memory can ever become readable again? >>>>> >>>>> Seems to me, once it is set TRUE, we might as well just leave it TRUE. >>>>> Am I right? >>>> >>>> I thought about it too. =A0I think if we don't need this entry. =A0We = can >>>> delete it directly. >>>> But there is a special status, after release memory, inferior alloc >>>> some memory and its address is same with the memory that released >>>> memory. =A0Then the memory entry will can be use in this status. =A0Us= er >>>> can get the right value of memory before this entry. =A0So I think may= be >>>> we can keep it. >>>> >>>> What do you think about it? >>> >>> Let's say a program does something like this: >>> >>> buf =3D mmap (...); >>> munmap (...); >>> buf =3D mmap (...); >>> munmap (...); >>> buf =3D mmap (...); >>> >>> and so on. =A0And suppose that, for whatever reason, mmap always >>> returns the same address. >>> >>> Then it seems to me (and please correct me if I am wrong), that >>> it all depends on where we stop the recording. >>> >>> If we stop the recording after mmap, but before munmap, then >>> the memory will be readable throughout the ENTIRE recording. >>> >>> But if we stop the recording after munmap, but before mmap, then >>> the memory will NOT be readable (again for the entire recording). >>> >>> So as you replay backward and forward through the recording, the >>> readability state of the memory location will never change -- it >>> will remain either readable, or unreadable, depending only on >>> the mapped-state when the recording ended. >>> >>> The only way for it to change, I think, would be if we could >>> resume the process and add some more execution to the end of >>> the existing recording. >>> >> >> Agree with you. =A0We can do more thing around release memory. >> >> But this patch make prec work OK even if inferior release some memory. >> =A0Of course, the released memory we still can not access. =A0But other >> memory is OK. >> >> This is a low cost idea for release memory. =A0And we still can add >> other thing about =A0release memory. > > Yes, I like the patch, and I want to see it go in, but > you haven't really answered my question: > > Once we have set this flag to TRUE in a recording entry, > why would we ever need to set it to FALSE? > > The patch is simpler and easier to understand if we simply > leave the flag TRUE. =A0It worries me to see it toggling back > and forth between TRUE and FALSE every time we play through > the recording, if there is no benefit to doing that. =A0Plus > it means that we keep trying to read the target memory even > though we know it will fail. > > I'm attaching a diff to show what I mean, in case it isn't clear. > This diff gives me the same behavior with your munmap test case > as your diff does. > > > > > Index: record.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > RCS file: /cvs/src/src/gdb/record.c,v > retrieving revision 1.9 > diff -u -p -r1.9 record.c > --- record.c =A0 =A022 Jul 2009 05:31:26 -0000 =A0 =A0 =A01.9 > +++ record.c =A0 =A01 Aug 2009 22:59:55 -0000 > @@ -51,6 +51,7 @@ struct record_mem_entry > =A0{ > =A0 CORE_ADDR addr; > =A0 int len; > + =A0int mem_entry_not_accessible; > =A0 gdb_byte *val; > =A0}; > > @@ -275,6 +276,7 @@ record_arch_list_add_mem (CORE_ADDR addr > =A0 rec->type =3D record_mem; > =A0 rec->u.mem.addr =3D addr; > =A0 rec->u.mem.len =3D len; > + =A0rec->u.mem.mem_entry_not_accessible =3D 0; > > =A0 if (target_read_memory (addr, rec->u.mem.val, len)) > =A0 =A0 { > @@ -727,32 +729,52 @@ record_wait (struct target_ops *ops, > =A0 =A0 =A0 =A0 =A0else if (record_list->type =3D=3D record_mem) > =A0 =A0 =A0 =A0 =A0 =A0{ > =A0 =A0 =A0 =A0 =A0 =A0 =A0/* mem */ > - =A0 =A0 =A0 =A0 =A0 =A0 gdb_byte *mem =3D alloca (record_list->u.mem.le= n); > - =A0 =A0 =A0 =A0 =A0 =A0 if (record_debug > 1) > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 fprintf_unfiltered (gdb_stdlog, > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "Pr= ocess record: record_mem %s to " > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "in= ferior addr =3D %s len =3D %d.\n", > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 hos= t_address_to_string (record_list), > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pad= dress (gdbarch, > record_list->u.mem.addr), > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 rec= ord_list->u.mem.len); > - > - =A0 =A0 =A0 =A0 =A0 =A0 if (target_read_memory > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (record_list->u.mem.addr, mem, record_l= ist->u.mem.len)) > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 error (_("Process record: error reading mem= ory at " > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"addr =3D %s len =3D %d.= "), > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0paddress (gdbarch, record_li= st->u.mem.addr), > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0record_list->u.mem.len); > - > - =A0 =A0 =A0 =A0 =A0 =A0 if (target_write_memory > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (record_list->u.mem.addr, record_list->= u.mem.val, > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0record_list->u.mem.len)) > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 error (_ > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0("Process record: error writ= ing memory at " > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "addr =3D %s len =3D %d."), > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0paddress (gdbarch, record_li= st->u.mem.addr), > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0record_list->u.mem.len); > - > - =A0 =A0 =A0 =A0 =A0 =A0 memcpy (record_list->u.mem.val, mem, record_lis= t->u.mem.len); > + =A0 =A0 =A0 =A0 =A0 =A0 if (record_list->u.mem.mem_entry_not_accessible= =3D=3D 0) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 gdb_byte *mem =3D alloca (record_list->= u.mem.len); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (record_debug > 1) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 fprintf_unfiltered (gdb_stdlog, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 "Process record: record_mem %s to " > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 "inferior addr =3D %s len =3D %d.\n", > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 host_address_to_string > (record_list), > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 paddress (gdbarch, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 record_list->u.mem.addr), > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 record_list->u.mem.len); > + > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (target_read_memory (record_list->u.= mem.addr, mem, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 record_list->u.mem.len)) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (execution_direction !=3D EX= EC_REVERSE) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 error (_("Process record: e= rror reading memory at " > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"addr = =3D %s len =3D %d."), > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0paddress (gd= barch, record_list->u.mem.addr), > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0record_list-= >u.mem.len); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 else > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 record_list->u.mem.mem_entr= y_not_accessible =3D 1; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 else > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (target_write_memory (record= _list->u.mem.addr, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0record_list->u.mem.val, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0record_list->u.mem.len)) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (execution_direction= !=3D EXEC_REVERSE) > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 error (_("Process r= ecord: error writing memory > at " > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= "addr =3D %s len =3D %d."), > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0padd= ress (gdbarch, > record_list->u.mem.addr), > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0reco= rd_list->u.mem.len); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 else > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 record_list->u.= mem.mem_entry_not_accessible =3D > 1; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 else > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 memcpy (record_list->u.= mem.val, mem, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 record_= list->u.mem.len); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 } > =A0 =A0 =A0 =A0 =A0 =A0} > =A0 =A0 =A0 =A0 =A0else > =A0 =A0 =A0 =A0 =A0 =A0{ > >