From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14095 invoked by alias); 6 Oct 2008 23:00:07 -0000 Received: (qmail 13945 invoked by uid 22791); 6 Oct 2008 23:00:03 -0000 X-Spam-Check-By: sourceware.org Received: from mail-gx0-f10.google.com (HELO mail-gx0-f10.google.com) (209.85.217.10) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 06 Oct 2008 22:59:28 +0000 Received: by gxk3 with SMTP id 3so8136769gxk.0 for ; Mon, 06 Oct 2008 15:59:25 -0700 (PDT) Received: by 10.110.40.8 with SMTP id n8mr7068361tin.50.1223333963662; Mon, 06 Oct 2008 15:59:23 -0700 (PDT) Received: by 10.110.42.9 with HTTP; Mon, 6 Oct 2008 15:59:23 -0700 (PDT) Message-ID: Date: Mon, 06 Oct 2008 23:00:00 -0000 From: teawater To: "Michael Snyder" Subject: Re: [reverse RFA] Change from "to_prepare_to_store" to "to_store_registers" Cc: "gdb-patches@sourceware.org" In-Reply-To: <48EA4CBC.8070400@vmware.com> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <48E96A51.1090506@vmware.com> <48EA4CBC.8070400@vmware.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: 2008-10/txt/msg00189.txt.bz2 Thanks. And I understand the branch. On Tue, Oct 7, 2008 at 01:37, Michael Snyder wrote: > teawater wrote: >> >> This patch is for reverse-20080930-branch. >> Do you think I can check-in code for reverse-20080717-branch directly? >> >> In actually, 2 branches make me puzzled. > > reverse-20080930 branch is not ready for public consumption yet. > I'm using it for syncing changes and marshalling them for submission. > Let's just consider that to be my private branch until I say it's ready. > > Your patch applies cleanly to reverse-20080717-branch, and > I've built and tested it there (with the corresponding change > to regcache). This eliminates the use of a global variable > to pass information between regcache and record.c. > > Sorry about the branches confusion -- allow me to check this > in for you, to get us back in sync. > > >> >> On Mon, Oct 6, 2008 at 09:30, Michael Snyder wrote: >>> >>> Good, please commit >>> (would you add the corresponding change to regcache.c too?) >>> >>> teawater wrote: >>>> >>>> Hi, >>>> >>>> To_prepare_to_store can make process record control the register >>>> change operation before set the value of regcache in function >>>> "regcache_raw_write". But it can't get the regnum. >>>> So I change it to "to_store_registers". It can get the regnum. And if >>>> record want cancel the operation the operation. It can invalidate the >>>> value with itself. >>>> >>>> 2008-10-04 Hui Zhu >>>> >>>> Change from "to_prepare_to_store" to "to_store_registers". >>>> >>>> * record.c (record_beneath_to_prepare_to_store): Removed. >>>> (record_beneath_to_store_registers): New function pointer. >>>> Instead "record_beneath_to_prepare_to_store". Will point >>>> to the low strata target "to_store_registers" function. >>>> (record_prepare_to_store): Removed. >>>> (record_store_registers): New function. >>>> Instead "record_prepare_to_store". Record the change of >>>> registers from GDB. >>>> (init_record_ops): Change record_prepare_to_store to >>>> record_store_registers. >>>> * record.h (record_beneath_to_prepare_to_store): Removed. >>>> (record_beneath_to_store_registers): New extern. >>>> * target.c (update_current_target): Change >>>> record_beneath_to_prepare_to_store to >>>> record_beneath_to_store_registers. >>>> >>>> Thanks, >>>> Hui >>>> >>>> >>>> ------------------------------------------------------------------------ >>>> >>>> --- a/ChangeLog >>>> +++ b/ChangeLog >>>> @@ -1,3 +1,23 @@ >>>> +2008-10-04 Hui Zhu >>>> + >>>> + Change from "to_prepare_to_store" to "to_store_registers". >>>> + >>>> + * record.c (record_beneath_to_prepare_to_store): Removed. >>>> + (record_beneath_to_store_registers): New function pointer. >>>> + Instead "record_beneath_to_prepare_to_store". Will point >>>> + to the low strata target "to_store_registers" function. >>>> + (record_prepare_to_store): Removed. >>>> + (record_store_registers): New function. >>>> + Instead "record_prepare_to_store". Record the change of >>>> + registers from GDB. >>>> + (init_record_ops): Change record_prepare_to_store to >>>> + record_store_registers. >>>> + * record.h (record_beneath_to_prepare_to_store): Removed. >>>> + (record_beneath_to_store_registers): New extern. >>>> + * target.c (update_current_target): Change >>>> + record_beneath_to_prepare_to_store to >>>> + record_beneath_to_store_registers. >>>> + >>>> 2008-10-02 Michael Snyder >>>> * reverse.c (reverse-continue): Remove a comma from docs string, >>>> --- a/record.c >>>> +++ b/record.c >>>> @@ -56,7 +56,7 @@ extern struct bp_location *bp_location_c >>>> /* The real beneath function pointers. */ >>>> void (*record_beneath_to_resume) (ptid_t, int, enum target_signal); >>>> ptid_t (*record_beneath_to_wait) (ptid_t, struct target_waitstatus *); >>>> -void (*record_beneath_to_prepare_to_store) (struct regcache *); >>>> +void (*record_beneath_to_store_registers) (struct regcache *, int >>>> regno); >>>> LONGEST (*record_beneath_to_xfer_partial) (struct target_ops * ops, >>>> enum target_object object, >>>> const char *annex, >>>> @@ -805,23 +805,51 @@ record_registers_change (struct regcache >>>> } >>>> } >>>> -/* XXX: I don't know how to do if GDB call function >>>> target_store_registers >>>> - without call function target_prepare_to_store. */ >>>> - >>>> static void >>>> -record_prepare_to_store (struct regcache *regcache) >>>> +record_store_registers (struct regcache *regcache, int regno) >>>> { >>>> if (!record_not_record) >>>> { >>>> if (RECORD_IS_REPLAY) >>>> { >>>> + int n; >>>> struct cleanup *old_cleanups; >>>> + >>>> /* Let user choice if he want to write register or not. */ >>>> - if (!nquery (_("Becuse GDB is in replay mode, changing the >>>> value >>>> of a register will destroy the record from this point forward. Change >>>> register %s?"), >>>> - gdbarch_register_name (get_regcache_arch >>>> - (regcache), >>>> - >>>> record_regcache_raw_write_regnum))) >>>> + if (regno < 0) >>>> + { >>>> + n = >>>> + nquery (_ >>>> + ("Becuse GDB is in replay mode, changing the >>>> value >>>> of a register will destroy the record from this point forward. Change >>>> all >>>> register?")); >>>> + } >>>> + else >>>> + { >>>> + n = >>>> + nquery (_ >>>> + ("Becuse GDB is in replay mode, changing the >>>> value >>>> of a register will destroy the record from this point forward. Change >>>> register %s?"), >>>> + gdbarch_register_name (get_regcache_arch >>>> (regcache), >>>> + regno)); >>>> + } >>>> + >>>> + if (!n) >>>> { >>>> + /* Invalidate the value of regcache that set in function >>>> + "regcache_raw_write". */ >>>> + if (regno < 0) >>>> + { >>>> + int i; >>>> + for (i = 0; >>>> + i < gdbarch_num_regs (get_regcache_arch >>>> (regcache)); >>>> + i++) >>>> + { >>>> + regcache_invalidate (regcache, i); >>>> + } >>>> + } >>>> + else >>>> + { >>>> + regcache_invalidate (regcache, regno); >>>> + } >>>> + >>>> error (_("Record: record cancel the operation.")); >>>> } >>>> @@ -829,9 +857,9 @@ record_prepare_to_store (struct regcache >>>> record_list_release_next (); >>>> } >>>> - record_registers_change (regcache, >>>> record_regcache_raw_write_regnum); >>>> + record_registers_change (regcache, regno); >>>> } >>>> - record_beneath_to_prepare_to_store (regcache); >>>> + record_beneath_to_store_registers (regcache, regno); >>>> } >>>> /* record_xfer_partial -- behavior is conditional on RECORD_IS_REPLAY. >>>> @@ -964,7 +992,7 @@ init_record_ops (void) >>>> record_ops.to_mourn_inferior = record_mourn_inferior; >>>> record_ops.to_kill = record_kill; >>>> record_ops.to_create_inferior = find_default_create_inferior; /* >>>> Make record suppport command "run". */ >>>> - record_ops.to_prepare_to_store = record_prepare_to_store; >>>> + record_ops.to_store_registers = record_store_registers; >>>> record_ops.to_xfer_partial = record_xfer_partial; >>>> record_ops.to_insert_breakpoint = record_insert_breakpoint; >>>> record_ops.to_remove_breakpoint = record_remove_breakpoint; >>>> --- a/record.h >>>> +++ b/record.h >>>> @@ -75,7 +75,6 @@ extern struct regcache *record_regcache; >>>> extern struct target_ops record_ops; >>>> extern int record_resume_step; >>>> -extern int record_regcache_raw_write_regnum; >>>> extern enum exec_direction_kind record_execdir; >>>> extern int record_arch_list_add_reg (int num); >>>> @@ -86,7 +85,7 @@ extern void record_not_record_set (void) >>>> extern void (*record_beneath_to_resume) (ptid_t, int, enum >>>> target_signal); >>>> extern ptid_t (*record_beneath_to_wait) (ptid_t, struct >>>> target_waitstatus >>>> *); >>>> -extern void (*record_beneath_to_prepare_to_store) (struct regcache *); >>>> +extern void (*record_beneath_to_store_registers) (struct regcache *, >>>> int >>>> regno); >>>> extern LONGEST (*record_beneath_to_xfer_partial) (struct target_ops * >>>> ops, >>>> enum target_object >>>> object, >>>> const char *annex, >>>> --- a/target.c >>>> +++ b/target.c >>>> @@ -380,7 +380,7 @@ update_current_target (void) >>>> current_target.FIELD = (TARGET)->FIELD >>>> record_beneath_to_resume = NULL; >>>> - record_beneath_to_prepare_to_store = NULL; >>>> + record_beneath_to_store_registers = NULL; >>>> record_beneath_to_xfer_partial = NULL; >>>> record_beneath_to_insert_breakpoint = NULL; >>>> record_beneath_to_remove_breakpoint = NULL; >>>> @@ -485,9 +485,9 @@ update_current_target (void) >>>> { >>>> record_beneath_to_wait = t->to_wait; >>>> } >>>> - if (!record_beneath_to_prepare_to_store) >>>> + if (!record_beneath_to_store_registers) >>>> { >>>> - record_beneath_to_prepare_to_store = >>>> t->to_prepare_to_store; >>>> + record_beneath_to_store_registers = >>>> t->to_store_registers; >>>> } >>>> if (!record_beneath_to_xfer_partial) >>>> { >>> > >