From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10821 invoked by alias); 6 Oct 2008 17:39:30 -0000 Received: (qmail 10812 invoked by uid 22791); 6 Oct 2008 17:39:28 -0000 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.31) with ESMTP; Mon, 06 Oct 2008 17:38:52 +0000 Received: from mailhost5.vmware.com (mailhost5.vmware.com [10.16.68.131]) by smtp-outbound-2.vmware.com (Postfix) with ESMTP id D8B90F007; Mon, 6 Oct 2008 10:38:47 -0700 (PDT) Received: from [10.20.92.59] (promb-2s-dhcp59.eng.vmware.com [10.20.92.59]) by mailhost5.vmware.com (Postfix) with ESMTP id CF945DC054; Mon, 6 Oct 2008 10:38:47 -0700 (PDT) Message-ID: <48EA4CBC.8070400@vmware.com> Date: Mon, 06 Oct 2008 17:39:00 -0000 From: Michael Snyder User-Agent: Thunderbird 1.5.0.12 (X11/20080411) MIME-Version: 1.0 To: teawater CC: "gdb-patches@sourceware.org" Subject: Re: [reverse RFA] Change from "to_prepare_to_store" to "to_store_registers" References: <48E96A51.1090506@vmware.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: 2008-10/txt/msg00150.txt.bz2 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) >>> { >>