This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
Hi Pedro, I make a new patch according to your mail. Please help me review it. Thanks, Hui On Tue, Mar 10, 2009 at 04:34, Pedro Alves <pedro@codesourcery.com> wrote: > On Monday 23 February 2009 09:20:13, teawater wrote: >> This is the new version patches that follow cvs-head. > > Thanks. > >> >> ?gdb/Makefile.in | ? ?4 >> ?gdb/record.c ? ?| 1283 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> ?gdb/record.h ? ?| ? 87 +++ >> ?3 files changed, 1372 insertions(+), 2 deletions(-) >> >> Index: src/gdb/Makefile.in >> =================================================================== >> --- src.orig/gdb/Makefile.in ?2009-02-21 15:53:27.000000000 +0000 >> +++ src/gdb/Makefile.in ? ? ? 2009-02-28 20:23:20.000000000 +0000 >> @@ -662,7 +662,7 @@ SFILES = ada-exp.y ada-lang.c ada-typepr >> ? ? ? valarith.c valops.c valprint.c value.c varobj.c vec.c \ >> ? ? ? wrapper.c \ >> ? ? ? xml-tdesc.c xml-support.c \ >> - ? ? inferior.c >> + ? ? inferior.c record.c >> >> ?LINTFILES = $(SFILES) $(YYFILES) $(CONFIG_SRCS) init.c >> >> @@ -813,7 +813,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $ >> ? ? ? solib.o solib-null.o \ >> ? ? ? prologue-value.o memory-map.o xml-support.o \ >> ? ? ? target-descriptions.o target-memory.o xml-tdesc.o xml-builtin.o \ >> - ? ? inferior.o osdata.o >> + ? ? inferior.o osdata.o record.o >> >> ?TSOBS = inflow.o >> >> Index: src/gdb/record.c >> =================================================================== >> --- /dev/null 1970-01-01 00:00:00.000000000 +0000 >> +++ src/gdb/record.c ?2009-02-28 20:23:20.000000000 +0000 >> @@ -0,0 +1,1283 @@ >> +/* Process record and replay target for GDB, the GNU debugger. >> + >> + ? Copyright (C) 2008 Free Software Foundation, Inc. > > Oops, you'll have to update this. > >> + >> + ? This file is part of GDB. >> + >> + ? This program is free software; you can redistribute it and/or modify >> + ? it under the terms of the GNU General Public License as published by >> + ? the Free Software Foundation; either version 3 of the License, or >> + ? (at your option) any later version. >> + >> + ? This program is distributed in the hope that it will be useful, >> + ? but WITHOUT ANY WARRANTY; without even the implied warranty of >> + ? MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. ?See the >> + ? GNU General Public License for more details. >> + >> + ? You should have received a copy of the GNU General Public License >> + ? along with this program. ?If not, see <http://www.gnu.org/licenses/>. ?*/ >> + >> +#include "defs.h" >> +#include "target.h" >> +#include "gdbcmd.h" >> +#include "regcache.h" >> +#include "inferior.h" >> +#include "gdbthread.h" >> +#include "event-top.h" >> +#include "annotate.h" >> +#include "observer.h" >> +#include "record.h" > > Why did you need annotate.h? ?Can you check which headers are > really needed here? > >> + >> +#include <signal.h> >> + >> +#define DEFAULT_RECORD_INSN_MAX_NUM ?200000 >> + >> +int record_debug = 0; >> + >> +record_t record_first; >> +record_t *record_list = &record_first; >> +record_t *record_arch_list_head = NULL; >> +record_t *record_arch_list_tail = NULL; >> +struct regcache *record_regcache = NULL; >> + >> +/* 1 ask user. 0 auto delete the last record_t. ?*/ > ? ? ? ? ? ? ? ? ^ double-space please. > >> +static int record_stop_at_limit = 1; >> +static int record_insn_max_num = DEFAULT_RECORD_INSN_MAX_NUM; >> +static int record_insn_num = 0; >> + >> +struct target_ops record_ops; >> +static int record_resume_step = 0; >> +static enum target_signal record_resume_siggnal; >> +static int record_get_sig = 0; >> +static sigset_t record_maskall; >> +static int record_gdb_operation_disable = 0; >> +int record_will_store_registers = 0; > > You've got a bunch of magic variables here. ?Could you > add some comments explaining what they're for? > >> + >> +extern struct bp_location *bp_location_chain; > > This is not right. ?You're accessing a breakpoints.c > internal implementation detail. ?We need to come up with interfaces > to replace this hack. > >> + >> +/* The beneath function pointers. ?*/ >> +static struct target_ops *record_beneath_to_resume_ops = NULL; >> +static void (*record_beneath_to_resume) (struct target_ops *, ptid_t, int, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? enum target_signal) = NULL; >> +static struct target_ops *record_beneath_to_wait_ops = NULL; >> +static ptid_t (*record_beneath_to_wait) (struct target_ops *, ptid_t, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct target_waitstatus *) = NULL; >> +static struct target_ops *record_beneath_to_store_registers_ops = NULL; >> +static void (*record_beneath_to_store_registers) (struct target_ops *, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct regcache *, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? int regno) = NULL; >> +static struct target_ops *record_beneath_to_xfer_partial_ops = NULL; >> +static LONGEST (*record_beneath_to_xfer_partial) (struct target_ops * ops, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? enum target_object object, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? const char *annex, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? gdb_byte * readbuf, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? const gdb_byte * writebuf, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ULONGEST offset, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? LONGEST len) = NULL; >> +static int (*record_beneath_to_insert_breakpoint) (struct bp_target_info *) = >> + ?NULL; >> +static int (*record_beneath_to_remove_breakpoint) (struct bp_target_info *) = >> + ?NULL; > > I can't say I'm happy with this mechanism yet, but it is much > better than the previous version of making target.c aware of record.c's callbacks. > > >> + >> +static void >> +record_list_release (record_t * rec) > > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?^ no space after *, please. > >> +{ >> + ?record_t *tmp; >> + >> + ?if (!rec) >> + ? ?return; >> + >> + ?while (rec->next) >> + ? ?{ >> + ? ? ?rec = rec->next; >> + ? ?} >> + >> + ?while (rec->prev) >> + ? ?{ >> + ? ? ?tmp = rec; >> + ? ? ?rec = rec->prev; >> + ? ? ?if (tmp->type == record_reg) >> + ? ? xfree (tmp->u.reg.val); >> + ? ? ?else if (tmp->type == record_mem) >> + ? ? xfree (tmp->u.mem.val); >> + ? ? ?xfree (tmp); >> + ? ?} >> + >> + ?if (rec != &record_first) >> + ? ?xfree (rec); >> +} >> + >> +static void >> +record_list_release_next (void) >> +{ >> + ?record_t *rec = record_list; >> + ?record_t *tmp = rec->next; >> + ?rec->next = NULL; >> + ?while (tmp) >> + ? ?{ >> + ? ? ?rec = tmp->next; >> + ? ? ?if (tmp->type == record_reg) >> + ? ? record_insn_num--; >> + ? ? ?else if (tmp->type == record_reg) >> + ? ? xfree (tmp->u.reg.val); >> + ? ? ?else if (tmp->type == record_mem) >> + ? ? xfree (tmp->u.mem.val); >> + ? ? ?xfree (tmp); >> + ? ? ?tmp = rec; >> + ? ?} >> +} >> + >> +static void >> +record_list_release_first (void) >> +{ >> + ?record_t *tmp = NULL; >> + ?enum record_type type; >> + >> + ?if (!record_first.next) >> + ? ?return; >> + >> + ?while (1) >> + ? ?{ >> + ? ? ?type = record_first.next->type; >> + >> + ? ? ?if (type == record_reg) >> + ? ? xfree (record_first.next->u.reg.val); >> + ? ? ?else if (type == record_mem) >> + ? ? xfree (record_first.next->u.mem.val); >> + ? ? ?tmp = record_first.next; >> + ? ? ?record_first.next = tmp->next; >> + ? ? ?xfree (tmp); >> + >> + ? ? ?if (!record_first.next) >> + ? ? { >> + ? ? ? gdb_assert (record_insn_num == 1); >> + ? ? ? break; >> + ? ? } >> + >> + ? ? ?record_first.next->prev = &record_first; >> + >> + ? ? ?if (type == record_end) >> + ? ? break; >> + ? ?} >> + >> + ?record_insn_num--; >> +} >> + >> +/* Add a record_t to record_arch_list. ?*/ >> +static void >> +record_arch_list_add (record_t * rec) >> +{ >> + ?if (record_debug > 1) >> + ? ?fprintf_unfiltered (gdb_stdlog, >> + ? ? ? ? ? ? ? ? ? ? "Process record: record_arch_list_add %s.\n", >> + ? ? ? ? ? ? ? ? ? ? host_address_to_string (rec)); >> + >> + ?if (record_arch_list_tail) >> + ? ?{ >> + ? ? ?record_arch_list_tail->next = rec; >> + ? ? ?rec->prev = record_arch_list_tail; >> + ? ? ?record_arch_list_tail = rec; >> + ? ?} >> + ?else >> + ? ?{ >> + ? ? ?record_arch_list_head = rec; >> + ? ? ?record_arch_list_tail = rec; >> + ? ?} >> +} >> + >> +/* Record the value of a register ("num") to record_arch_list. ?*/ > > When we want to mention the value of a parameter, we mentions it in > uppercase, like so: > > /* Record the value of register NUM to record_arch_list. ?*/ > > Also, there should be an empty line between the comment and > the function itself. ?Here and elsewhere. > >> +int >> +record_arch_list_add_reg (int num) >> +{ >> + ?record_t *rec; >> + >> + ?if (record_debug > 1) >> + ? ?fprintf_unfiltered (gdb_stdlog, >> + ? ? ? ? ? ? ? ? ? ? "Process record: add register num = %d to " >> + ? ? ? ? ? ? ? ? ? ? "record list.\n", >> + ? ? ? ? ? ? ? ? ? ? num); >> + >> + ?rec = (record_t *) xmalloc (sizeof (record_t)); >> + ?rec->u.reg.val = (gdb_byte *) xmalloc (MAX_REGISTER_SIZE); >> + ?rec->prev = NULL; >> + ?rec->next = NULL; >> + ?rec->type = record_reg; >> + ?rec->u.reg.num = num; >> + >> + ?regcache_raw_read (record_regcache, num, rec->u.reg.val); >> + >> + ?record_arch_list_add (rec); >> + >> + ?return 0; >> +} >> + >> +/* Record the value of a region of memory whose address is "addr" and >> + ? length is "len" to record_arch_list. ?*/ > > /* Record the value of a region of memory whose address is ADDR and > ? length is LEN to record_arch_list. ?*/ > >> + >> +int >> +record_arch_list_add_mem (CORE_ADDR addr, int len) >> +{ >> + ?record_t *rec; >> + >> + ?if (record_debug > 1) >> + ? ?fprintf_unfiltered (gdb_stdlog, >> + ? ? ? ? ? ? ? ? ? ? "Process record: add mem addr = 0x%s len = %d to " >> + ? ? ? ? ? ? ? ? ? ? "record list.\n", >> + ? ? ? ? ? ? ? ? ? ? paddr_nz (addr), len); >> + > >> + ?if (!addr) >> + ? ?return 0; > > Why is this check for addr == 0 necessary here? > >> + >> + ?rec = (record_t *) xmalloc (sizeof (record_t)); >> + ?rec->u.mem.val = (gdb_byte *) xmalloc (len); >> + ?rec->prev = NULL; >> + ?rec->next = NULL; >> + ?rec->type = record_mem; >> + ?rec->u.mem.addr = addr; >> + ?rec->u.mem.len = len; >> + >> + ?if (target_read_memory (addr, rec->u.mem.val, len)) >> + ? ?{ >> + ? ? ?if (record_debug) >> + ? ? fprintf_unfiltered (gdb_stdlog, >> + ? ? ? ? ? ? ? ? ? ? ? ? "Process record: error reading memory at " >> + ? ? ? ? ? ? ? ? ? ? ? ? "addr = 0x%s len = %d.\n", >> + ? ? ? ? ? ? ? ? ? ? ? ? paddr_nz (addr), len); >> + ? ? ?xfree (rec->u.mem.val); >> + ? ? ?xfree (rec); >> + ? ? ?return -1; >> + ? ?} >> + >> + ?record_arch_list_add (rec); >> + >> + ?return 0; >> +} >> + >> +/* Add a record_end type record_t to record_arch_list. ?*/ >> +int >> +record_arch_list_add_end (int need_dasm) >> +{ >> + ?record_t *rec; >> + >> + ?if (record_debug > 1) >> + ? ?fprintf_unfiltered (gdb_stdlog, >> + ? ? ? ? ? ? ? ? ? ? "Process record: add end need_dasm = %d to " >> + ? ? ? ? ? ? ? ? ? ? "arch list.\n", >> + ? ? ? ? ? ? ? ? ? ? need_dasm); >> + >> + ?rec = (record_t *) xmalloc (sizeof (record_t)); >> + ?rec->prev = NULL; >> + ?rec->next = NULL; >> + ?rec->type = record_end; >> + >> + ?rec->u.need_dasm = need_dasm; >> + >> + ?record_arch_list_add (rec); >> + >> + ?return 0; >> +} >> + >> +static void >> +record_check_insn_num (int set_terminal) >> +{ >> + ?if (record_insn_max_num) >> + ? ?{ >> + ? ? ?gdb_assert (record_insn_num <= record_insn_max_num); >> + ? ? ?if (record_insn_num == record_insn_max_num) >> + ? ? { >> + ? ? ? /* Ask user what to do. ?*/ >> + ? ? ? if (record_stop_at_limit) >> + ? ? ? ? { >> + ? ? ? ? ? int q; >> + ? ? ? ? ? if (set_terminal) >> + ? ? ? ? ? ? target_terminal_ours (); >> + ? ? ? ? ? q = yquery (_("Do you want to auto delete previous execution " >> + ? ? ? ? ? ? ? ? ? ? ? ? "log entries when record/replay buffer becomes " >> + ? ? ? ? ? ? ? ? ? ? ? ? "full (record-stop-at-limit)?")); >> + ? ? ? ? ? if (set_terminal) >> + ? ? ? ? ? ? target_terminal_inferior (); >> + ? ? ? ? ? if (q) >> + ? ? ? ? ? ? record_stop_at_limit = 0; >> + ? ? ? ? ? else >> + ? ? ? ? ? ? error (_("Process record: inferior program stopped.")); >> + ? ? ? ? } >> + ? ? } >> + ? ?} >> +} >> + >> +static void >> +record_normal_stop (void) >> +{ >> + ?finish_thread_state (minus_one_ptid); >> + >> + ?if (!breakpoints_always_inserted_mode () && target_has_execution) >> + ? ?remove_breakpoints (); >> + >> + ?target_terminal_ours (); >> + >> + ?if (target_has_stack && !stop_stack_dummy) >> + ? ?set_current_sal_from_frame (get_current_frame (), 1); >> + >> + ?select_frame (get_current_frame ()); >> + >> + ?annotate_stopped (); >> + ?if (!suppress_stop_observer) >> + ? ?{ >> + ? ? ?if (!ptid_equal (inferior_ptid, null_ptid)) >> + ? ? observer_notify_normal_stop (inferior_thread ()->stop_bpstat, 0); >> + ? ? ?else >> + ? ? observer_notify_normal_stop (NULL, 0); >> + ? ?} >> +} > > Nope sorry, this is worse than before. ?When I asked to not call > normal_stop, I didn't mean that you should inline chunks of > it here. ?This is papering over a different problem. ?Why > is it that record.c needs to do this, while other targets do not? > If we need to do something like this, it should be needed by all > targets that can throw from target_wait (which, is documented > as something you shouldn't do anyway). ?Actually, do you still need > any of this in current head? > >> + >> +/* Before inferior step (when GDB record the running message, inferior >> + ? only can step), GDB will call this function to record the values to >> + ? record_list. ?This function will call gdbarch_process_record to >> + ? record the running message of inferior and set them to >> + ? record_arch_list, and add it to record_list. ?*/ >> + >> +static void >> +record_message_cleanups (void *ignore) >> +{ >> + ?record_list_release (record_arch_list_tail); >> + ?set_executing (inferior_ptid, 0); >> + ?record_normal_stop (); >> +} > > Same as above. ?This isn't a record.c problem. > >> + >> +void >> +record_message (struct gdbarch *gdbarch) >> +{ >> + ?int ret; >> + ?struct cleanup *old_cleanups = make_cleanup (record_message_cleanups, 0); >> + >> + ?record_arch_list_head = NULL; >> + ?record_arch_list_tail = NULL; >> + >> + ?/* Check record_insn_num. ?*/ >> + ?record_check_insn_num (1); >> + >> + ?record_regcache = get_current_regcache (); >> + >> + ?ret = gdbarch_process_record (gdbarch, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? regcache_read_pc (record_regcache)); >> + ?if (ret > 0) >> + ? ?error (_("Process record: inferior program stopped.")); >> + ?if (ret < 0) >> + ? ?error (_("Process record: failed to record execution log.")); >> + >> + ?discard_cleanups (old_cleanups); >> + >> + ?record_list->next = record_arch_list_head; >> + ?record_arch_list_head->prev = record_list; >> + ?record_list = record_arch_list_tail; >> + >> + ?if (record_insn_num == record_insn_max_num && record_insn_max_num) >> + ? ?record_list_release_first (); >> + ?else >> + ? ?record_insn_num++; >> +} >> + > > >> +/* Things to clean up if we error or QUIT out of function that set >> + ? record_gdb_operation_disable (ie. command that caused target_wait to >> + ? be called). ?*/ >> +static void >> +record_gdb_operation_disable_cleanups (void *ignore) >> +{ >> + ?record_gdb_operation_disable = 0; >> +} >> + >> +struct cleanup * >> +record_gdb_operation_disable_set (void) >> +{ >> + ?struct cleanup *old_cleanups = >> + ? ?make_cleanup (record_gdb_operation_disable_cleanups, 0); >> + ?record_gdb_operation_disable = 1; >> + >> + ?return old_cleanups; >> +} > > This is make_cleanup_restore_integer. > >> + >> +static void >> +record_open (char *name, int from_tty) >> +{ >> + ?struct target_ops *t; >> + >> + ?if (record_debug) >> + ? ?fprintf_unfiltered (gdb_stdlog, "Process record: record_open\n"); >> + >> + ?/* check exec */ > > and core. > >> + ?if (!target_has_execution) >> + ? ?error (_("Process record: the program is not being run.")); >> + ?if (non_stop) >> + ? ?error (_("Process record target can't debug inferior in non-stop mode " >> + ? ? ? ? ?"(non-stop).")); >> + ?if (target_async_permitted) >> + ? ?error (_("Process record target can't debug inferior in asynchronous " >> + ? ? ? ? ?"mode (target-async).")); >> + >> + ?if (!gdbarch_process_record_p (current_gdbarch)) >> + ? ?error (_("Process record: the current architecture doesn't support " >> + ? ? ? ? ?"record function.")); >> + >> + ?/* Check if record target is already running. ?*/ >> + ?if (TARGET_IS_PROCESS_RECORD) >> + ? ?{ >> + ? ? ?if (!nquery >> + ? ? ? (_("Process record target already running, do you want to delete " >> + ? ? ? ? ?"the old record log?"))) >> + ? ? return; >> + ? ?} >> + >> + ?/* Set the beneath function pointers. ?*/ >> + ?for (t = current_target.beneath; t != NULL; t = t->beneath) >> + ? ?{ >> + ? ? ?if (!record_beneath_to_resume) >> + ? ? ? ?{ >> + ? ? ? record_beneath_to_resume = t->to_resume; >> + ? ? ? record_beneath_to_resume_ops = t; >> + ? ? ? ?} >> + ? ? ?if (!record_beneath_to_wait) >> + ? ? ? ?{ >> + ? ? ? record_beneath_to_wait = t->to_wait; >> + ? ? ? record_beneath_to_wait_ops = t; >> + ? ? ? ?} >> + ? ? ?if (!record_beneath_to_store_registers) >> + ? ? ? ?{ >> + ? ? ? record_beneath_to_store_registers = t->to_store_registers; >> + ? ? ? record_beneath_to_store_registers_ops = t; >> + ? ? ? ?} >> + ? ? ?if (!record_beneath_to_xfer_partial) >> + ? ? ? ?{ >> + ? ? ? record_beneath_to_xfer_partial = t->to_xfer_partial; >> + ? ? ? record_beneath_to_xfer_partial_ops = t; >> + ? ? ? ?} >> + ? ? ?if (!record_beneath_to_insert_breakpoint) >> + ? ? record_beneath_to_insert_breakpoint = t->to_insert_breakpoint; >> + ? ? ?if (!record_beneath_to_remove_breakpoint) >> + ? ? record_beneath_to_remove_breakpoint = t->to_remove_breakpoint; >> + ? ?} >> + ?if (!record_beneath_to_resume) >> + ? ?error (_("Process record can't get to_resume.")); >> + ?if (!record_beneath_to_wait) >> + ? ?error (_("Process record can't get to_wait.")); >> + ?if (!record_beneath_to_store_registers) >> + ? ?error (_("Process record can't get to_store_registers.")); >> + ?if (!record_beneath_to_xfer_partial) >> + ? ?error (_("Process record can't get to_xfer_partial.")); >> + ?if (!record_beneath_to_insert_breakpoint) >> + ? ?error (_("Process record can't get to_insert_breakpoint.")); >> + ?if (!record_beneath_to_remove_breakpoint) >> + ? ?error (_("Process record can't get to_remove_breakpoint.")); > > As I said above, this is better than making target.c aware of > these pointers, but, it still isn't ideal. ?For one thing, if > some other layer/target is pushed after the record target is opened, > these will be wrong. ?I would prefer that this beneath lookup > would be done at each callback implementation (record_resume, etc.), > but, I'm happy enough with this for now. ?It is now contained, so we can > clean this up afterwards... > >> + >> + ?push_target (&record_ops); >> + >> + ?/* Reset */ >> + ?record_insn_num = 0; >> + ?record_list = &record_first; >> + ?record_list->next = NULL; >> +} >> + >> +static void >> +record_close (int quitting) >> +{ >> + ?if (record_debug) >> + ? ?fprintf_unfiltered (gdb_stdlog, "Process record: record_close\n"); >> + >> + ?record_list_release (record_list); >> +} > > Shouldn't this clear the record_beneath_* pointers as well? > >> + >> +static void >> +record_resume (struct target_ops *ops, ptid_t ptid, int step, >> + ? ? ? ? ? ? ? enum target_signal siggnal) >> +{ >> + ?record_resume_step = step; >> + ?record_resume_siggnal = siggnal; >> + >> + ?if (!RECORD_IS_REPLAY) >> + ? ?{ >> + ? ? ?record_message (current_gdbarch); >> + ? ? ?record_beneath_to_resume (record_beneath_to_resume_ops, ptid, 1, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?siggnal); >> + ? ?} >> +} >> + >> +static void >> +record_sig_handler (int signo) >> +{ >> + ?if (record_debug) >> + ? ?fprintf_unfiltered (gdb_stdlog, "Process record: get a signal\n"); >> + >> + ?record_resume_step = 1; >> + ?record_get_sig = 1; >> +} > > This handler is magical. ?Why is it setting resume_step, for instance? > It would definitelly benefic from some comments. ?In fact, most of the > file is undercommented. > >> + >> +static void >> +record_wait_cleanups (void *ignore) >> +{ >> + ?if (execution_direction == EXEC_REVERSE) >> + ? ?{ >> + ? ? ?if (record_list->next) >> + ? ? record_list = record_list->next; >> + ? ?} >> + ?else >> + ? ?record_list = record_list->prev; >> + >> + ?set_executing (inferior_ptid, 0); >> + ?record_normal_stop (); >> +} > > See comments about record_normal_stop above. > >> + >> +/* record_wait > > Please remove the function name from the comment. ?It's redundant. > >> + ? In replay mode, this function examines the recorded log and >> + ? determines where to stop. ?*/ >> + >> +static ptid_t >> +record_wait (struct target_ops *ops, >> + ? ? ? ? ? ? ?ptid_t ptid, struct target_waitstatus *status) >> +{ >> + ?struct cleanup *set_cleanups = record_gdb_operation_disable_set (); >> + >> + ?if (record_debug) >> + ? ?fprintf_unfiltered (gdb_stdlog, >> + ? ? ? ? ? ? ? ? ? ? "Process record: record_wait " >> + ? ? ? ? ? ? ? ? ? ? "record_resume_step = %d\n", >> + ? ? ? ? ? ? ? ? ? ? record_resume_step); >> + >> + ?if (!RECORD_IS_REPLAY) >> + ? ?{ >> + ? ? ?if (record_resume_step) >> + ? ? { >> + ? ? ? /* This is a single step. ?*/ >> + ? ? ? return record_beneath_to_wait (record_beneath_to_wait_ops, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?ptid, status); >> + ? ? } >> + ? ? ?else >> + ? ? { >> + ? ? ? if (record_resume_step) >> + ? ? ? ? { >> + ? ? ? ? ? /* This is a single step. ?*/ >> + ? ? ? ? ? return record_beneath_to_wait (record_beneath_to_wait_ops, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?ptid, status); >> + ? ? ? ? } >> + ? ? ? else >> + ? ? ? ? { >> + ? ? ? ? ? /* This is not a single step. ?*/ >> + ? ? ? ? ? ptid_t ret; >> + ? ? ? ? ? int is_breakpoint = 1; >> + ? ? ? ? ? CORE_ADDR pc = 0; >> + ? ? ? ? ? int pc_is_read = 0; >> + ? ? ? ? ? struct bp_location *bl; >> + ? ? ? ? ? struct breakpoint *b; >> + >> + ? ? ? ? ? do >> + ? ? ? ? ? ? { >> + ? ? ? ? ? ? ? ret = record_beneath_to_wait (record_beneath_to_wait_ops, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?ptid, status); >> + >> + ? ? ? ? ? ? ? if (status->kind == TARGET_WAITKIND_STOPPED >> + ? ? ? ? ? ? ? ? ? && status->value.sig == TARGET_SIGNAL_TRAP) >> + ? ? ? ? ? ? ? ? { >> + ? ? ? ? ? ? ? ? ? /* Check if there is a breakpoint. ?*/ >> + ? ? ? ? ? ? ? ? ? pc_is_read = 0; >> + ? ? ? ? ? ? ? ? ? registers_changed (); >> + ? ? ? ? ? ? ? ? ? for (bl = bp_location_chain; bl; bl = bl->global_next) > > This will need to be fixed. ?Can you use the breakpoint_here-like functions > exported by breakpoint.h instead of referencing bp_location_chain directly? > >> + ? ? ? ? ? ? ? ? ? ? { >> + ? ? ? ? ? ? ? ? ? ? ? b = bl->owner; >> + ? ? ? ? ? ? ? ? ? ? ? gdb_assert (b); >> + ? ? ? ? ? ? ? ? ? ? ? if (b->enable_state != bp_enabled >> + ? ? ? ? ? ? ? ? ? ? ? ? ? && b->enable_state != bp_permanent) >> + ? ? ? ? ? ? ? ? ? ? ? ? continue; >> + ? ? ? ? ? ? ? ? ? ? ? if (!pc_is_read) >> + ? ? ? ? ? ? ? ? ? ? ? ? { >> + ? ? ? ? ? ? ? ? ? ? ? ? ? pc = >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? regcache_read_pc (get_thread_regcache (ret)); >> + ? ? ? ? ? ? ? ? ? ? ? ? ? pc_is_read = 1; >> + ? ? ? ? ? ? ? ? ? ? ? ? } >> + ? ? ? ? ? ? ? ? ? ? ? switch (b->type) >> + ? ? ? ? ? ? ? ? ? ? ? ? { >> + ? ? ? ? ? ? ? ? ? ? ? ? default: >> + ? ? ? ? ? ? ? ? ? ? ? ? ? if (bl->address == pc) >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? goto breakpoint; >> + ? ? ? ? ? ? ? ? ? ? ? ? ? break; >> + >> + ? ? ? ? ? ? ? ? ? ? ? ? case bp_watchpoint: >> + ? ? ? ? ? ? ? ? ? ? ? ? ? /* XXX teawater: I still not very clear how to >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?deal with it. ?*/ >> + ? ? ? ? ? ? ? ? ? ? ? ? ? goto breakpoint; >> + ? ? ? ? ? ? ? ? ? ? ? ? ? break; >> + >> + ? ? ? ? ? ? ? ? ? ? ? ? case bp_catchpoint: >> + ? ? ? ? ? ? ? ? ? ? ? ? ? gdb_assert (b->ops != NULL >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? && b->ops->breakpoint_hit != NULL); >> + ? ? ? ? ? ? ? ? ? ? ? ? ? if (b->ops->breakpoint_hit (b)) >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? goto breakpoint; >> + ? ? ? ? ? ? ? ? ? ? ? ? ? break; >> + >> + ? ? ? ? ? ? ? ? ? ? ? ? case bp_hardware_watchpoint: >> + ? ? ? ? ? ? ? ? ? ? ? ? case bp_read_watchpoint: >> + ? ? ? ? ? ? ? ? ? ? ? ? case bp_access_watchpoint: >> + ? ? ? ? ? ? ? ? ? ? ? ? ? if (STOPPED_BY_WATCHPOINT (0)) >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? goto breakpoint; >> + ? ? ? ? ? ? ? ? ? ? ? ? ? break; >> + ? ? ? ? ? ? ? ? ? ? ? ? } >> + ? ? ? ? ? ? ? ? ? ? } >> + >> + ? ? ? ? ? ? ? ? ? /* There is not a breakpoint. ?*/ >> + ? ? ? ? ? ? ? ? ? record_message (current_gdbarch); >> + ? ? ? ? ? ? ? ? ? record_beneath_to_resume (record_beneath_to_resume_ops, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?ptid, 1, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? record_resume_siggnal); >> + ? ? ? ? ? ? ? ? ? continue; >> + ? ? ? ? ? ? ? ? } >> + >> + ? ? ? ? ? ? ? is_breakpoint = 0; >> + >> + ? ? ? ? ? ? breakpoint: >> + ? ? ? ? ? ? ? /* Add gdbarch_decr_pc_after_break to pc because gdb will >> + ? ? ? ? ? ? ? ? ?expect the pc to be at address plus decr_pc_after_break >> + ? ? ? ? ? ? ? ? ?when the inferior stops at a breakpoint. ?*/ >> + ? ? ? ? ? ? ? if (is_breakpoint) >> + ? ? ? ? ? ? ? ? { >> + ? ? ? ? ? ? ? ? ? CORE_ADDR decr_pc_after_break = >> + ? ? ? ? ? ? ? ? ? ? gdbarch_decr_pc_after_break (current_gdbarch); >> + ? ? ? ? ? ? ? ? ? if (decr_pc_after_break) >> + ? ? ? ? ? ? ? ? ? ? { >> + ? ? ? ? ? ? ? ? ? ? ? if (!pc_is_read) >> + ? ? ? ? ? ? ? ? ? ? ? ? pc = >> + ? ? ? ? ? ? ? ? ? ? ? ? ? regcache_read_pc (get_thread_regcache (ret)); >> + ? ? ? ? ? ? ? ? ? ? ? regcache_write_pc (get_thread_regcache (ret), >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?pc + decr_pc_after_break); >> + ? ? ? ? ? ? ? ? ? ? } >> + ? ? ? ? ? ? ? ? } >> + >> + ? ? ? ? ? ? ? break; >> + ? ? ? ? ? ? } >> + ? ? ? ? ? while (1); >> + >> + ? ? ? ? ? return ret; >> + ? ? ? ? } >> + ? ? } >> + ? ?} >> + ?else >> + ? ?{ >> + ? ? ?int need_dasm = 0; >> + ? ? ?struct regcache *regcache = get_current_regcache (); >> + ? ? ?int continue_flag = 1; >> + ? ? ?int first_record_end = 1; >> + ? ? ?struct cleanup *old_cleanups = make_cleanup (record_wait_cleanups, 0); >> + ? ? ?CORE_ADDR tmp_pc; >> + >> + ? ? ?status->kind = TARGET_WAITKIND_STOPPED; >> + >> + ? ? ?/* Check breakpoint when forward execute. ?*/ >> + ? ? ?if (execution_direction == EXEC_FORWARD) >> + ? ? { >> + ? ? ? tmp_pc = regcache_read_pc (regcache); >> + ? ? ? if (breakpoint_inserted_here_p (tmp_pc)) >> + ? ? ? ? { >> + ? ? ? ? ? if (record_debug) >> + ? ? ? ? ? ? fprintf_unfiltered (gdb_stdlog, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Process record: break at 0x%s.\n", >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? paddr_nz (tmp_pc)); >> + ? ? ? ? ? if (gdbarch_decr_pc_after_break (get_regcache_arch (regcache)) >> + ? ? ? ? ? ? ? && !record_resume_step) >> + ? ? ? ? ? ? regcache_write_pc (regcache, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?tmp_pc + >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?gdbarch_decr_pc_after_break >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?(get_regcache_arch (regcache))); >> + ? ? ? ? ? goto replay_out; >> + ? ? ? ? } >> + ? ? } >> + >> + ? ? ?record_get_sig = 0; >> + ? ? ?signal (SIGINT, record_sig_handler); >> + ? ? ?/* If GDB is in terminal_inferior mode, it will not get the signal. >> + ? ? ? ? And in GDB replay mode, GDB doesn't need to be in terminal_inferior >> + ? ? ? ? mode, because inferior will not executed. >> + ? ? ? ? Then set it to terminal_ours to make GDB get the signal. ?*/ >> + ? ? ?target_terminal_ours (); >> + >> + ? ? ?/* In EXEC_FORWARD mode, record_list points to the tail of prev >> + ? ? ? ? instruction. ?*/ >> + ? ? ?if (execution_direction == EXEC_FORWARD && record_list->next) >> + ? ? record_list = record_list->next; >> + >> + ? ? ?/* Loop over the record_list, looking for the next place to >> + ? ? ?stop. ?*/ >> + ? ? ?do >> + ? ? { >> + ? ? ? /* Check for beginning and end of log. ?*/ >> + ? ? ? if (execution_direction == EXEC_REVERSE >> + ? ? ? ? ? && record_list == &record_first) >> + ? ? ? ? { >> + ? ? ? ? ? /* Hit beginning of record log in reverse. ?*/ >> + ? ? ? ? ? status->kind = TARGET_WAITKIND_NO_HISTORY; >> + ? ? ? ? ? break; >> + ? ? ? ? } >> + ? ? ? if (execution_direction != EXEC_REVERSE && !record_list->next) >> + ? ? ? ? { >> + ? ? ? ? ? /* Hit end of record log going forward. ?*/ >> + ? ? ? ? ? status->kind = TARGET_WAITKIND_NO_HISTORY; >> + ? ? ? ? ? break; >> + ? ? ? ? } >> + >> + ? ? ? /* Set ptid, register and memory according to record_list. ?*/ >> + ? ? ? if (record_list->type == record_reg) >> + ? ? ? ? { >> + ? ? ? ? ? /* reg */ >> + ? ? ? ? ? gdb_byte reg[MAX_REGISTER_SIZE]; >> + ? ? ? ? ? if (record_debug > 1) >> + ? ? ? ? ? ? fprintf_unfiltered (gdb_stdlog, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Process record: record_reg %s to " >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "inferior num = %d.\n", >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? host_address_to_string (record_list), >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? record_list->u.reg.num); >> + ? ? ? ? ? regcache_cooked_read (regcache, record_list->u.reg.num, reg); >> + ? ? ? ? ? regcache_cooked_write (regcache, record_list->u.reg.num, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?record_list->u.reg.val); >> + ? ? ? ? ? memcpy (record_list->u.reg.val, reg, MAX_REGISTER_SIZE); >> + ? ? ? ? } >> + ? ? ? else if (record_list->type == record_mem) >> + ? ? ? ? { >> + ? ? ? ? ? /* mem */ >> + ? ? ? ? ? gdb_byte *mem = alloca (record_list->u.mem.len); >> + ? ? ? ? ? if (record_debug > 1) >> + ? ? ? ? ? ? fprintf_unfiltered (gdb_stdlog, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Process record: record_mem %s to " >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "inferior addr = 0x%s len = %d.\n", >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? host_address_to_string (record_list), >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? paddr_nz (record_list->u.mem.addr), >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? record_list->u.mem.len); >> + >> + ? ? ? ? ? if (target_read_memory >> + ? ? ? ? ? ? ? (record_list->u.mem.addr, mem, record_list->u.mem.len)) >> + ? ? ? ? ? ? error (_("Process record: error reading memory at " >> + ? ? ? ? ? ? ? ? ? ? ?"addr = 0x%s len = %d."), >> + ? ? ? ? ? ? ? ? ? ?paddr_nz (record_list->u.mem.addr), >> + ? ? ? ? ? ? ? ? ? ?record_list->u.mem.len); >> + >> + ? ? ? ? ? if (target_write_memory >> + ? ? ? ? ? ? ? (record_list->u.mem.addr, record_list->u.mem.val, >> + ? ? ? ? ? ? ? ?record_list->u.mem.len)) >> + ? ? ? ? ? ? error (_ >> + ? ? ? ? ? ? ? ? ? ?("Process record: error writing memory at " >> + ? ? ? ? ? ? ? ? ? ? "addr = 0x%s len = %d."), >> + ? ? ? ? ? ? ? ? ? ?paddr_nz (record_list->u.mem.addr), >> + ? ? ? ? ? ? ? ? ? ?record_list->u.mem.len); >> + >> + ? ? ? ? ? memcpy (record_list->u.mem.val, mem, record_list->u.mem.len); >> + ? ? ? ? } >> + ? ? ? else >> + ? ? ? ? { >> + ? ? ? ? ? if (record_debug > 1) >> + ? ? ? ? ? ? fprintf_unfiltered (gdb_stdlog, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Process record: record_end %s to " >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "inferior need_dasm = %d.\n", >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? host_address_to_string (record_list), >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? record_list->u.need_dasm); >> + >> + ? ? ? ? ? if (execution_direction == EXEC_FORWARD) >> + ? ? ? ? ? ? need_dasm = record_list->u.need_dasm; >> + ? ? ? ? ? if (need_dasm) >> + ? ? ? ? ? ? gdbarch_process_record_dasm (current_gdbarch); >> + >> + ? ? ? ? ? if (first_record_end && execution_direction == EXEC_REVERSE) >> + ? ? ? ? ? ? { >> + ? ? ? ? ? ? ? /* When reverse excute, the first record_end is the part of >> + ? ? ? ? ? ? ? ? ?current instruction. ?*/ >> + ? ? ? ? ? ? ? first_record_end = 0; >> + ? ? ? ? ? ? } >> + ? ? ? ? ? else >> + ? ? ? ? ? ? { >> + ? ? ? ? ? ? ? /* In EXEC_REVERSE mode, this is the record_end of prev >> + ? ? ? ? ? ? ? ? ?instruction. >> + ? ? ? ? ? ? ? ? ?In EXEC_FORWARD mode, this is the record_end of current >> + ? ? ? ? ? ? ? ? ?instruction. ?*/ >> + ? ? ? ? ? ? ? /* step */ >> + ? ? ? ? ? ? ? if (record_resume_step) >> + ? ? ? ? ? ? ? ? { >> + ? ? ? ? ? ? ? ? ? if (record_debug > 1) >> + ? ? ? ? ? ? ? ? ? ? fprintf_unfiltered (gdb_stdlog, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Process record: step.\n"); >> + ? ? ? ? ? ? ? ? ? continue_flag = 0; >> + ? ? ? ? ? ? ? ? } >> + >> + ? ? ? ? ? ? ? /* check breakpoint */ >> + ? ? ? ? ? ? ? tmp_pc = regcache_read_pc (regcache); >> + ? ? ? ? ? ? ? if (breakpoint_inserted_here_p (tmp_pc)) >> + ? ? ? ? ? ? ? ? { >> + ? ? ? ? ? ? ? ? ? if (record_debug) >> + ? ? ? ? ? ? ? ? ? ? fprintf_unfiltered (gdb_stdlog, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Process record: break " >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "at 0x%s.\n", >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? paddr_nz (tmp_pc)); >> + ? ? ? ? ? ? ? ? ? if (gdbarch_decr_pc_after_break (get_regcache_arch (regcache)) >> + ? ? ? ? ? ? ? ? ? ? ? && execution_direction == EXEC_FORWARD >> + ? ? ? ? ? ? ? ? ? ? ? && !record_resume_step) >> + ? ? ? ? ? ? ? ? ? ? regcache_write_pc (regcache, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?tmp_pc + >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?gdbarch_decr_pc_after_break >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?(get_regcache_arch (regcache))); >> + ? ? ? ? ? ? ? ? ? continue_flag = 0; >> + ? ? ? ? ? ? ? ? } >> + ? ? ? ? ? ? } >> + ? ? ? ? ? if (execution_direction == EXEC_REVERSE) >> + ? ? ? ? ? ? need_dasm = record_list->u.need_dasm; >> + ? ? ? ? } >> + >> +next: >> + ? ? ? if (continue_flag) >> + ? ? ? ? { >> + ? ? ? ? ? if (execution_direction == EXEC_REVERSE) >> + ? ? ? ? ? ? { >> + ? ? ? ? ? ? ? if (record_list->prev) >> + ? ? ? ? ? ? ? ? record_list = record_list->prev; >> + ? ? ? ? ? ? } >> + ? ? ? ? ? else >> + ? ? ? ? ? ? { >> + ? ? ? ? ? ? ? if (record_list->next) >> + ? ? ? ? ? ? ? ? record_list = record_list->next; >> + ? ? ? ? ? ? } >> + ? ? ? ? } >> + ? ? } >> + ? ? ?while (continue_flag); >> + >> + ? ? ?signal (SIGINT, handle_sigint); >> + >> +replay_out: >> + ? ? ?if (record_get_sig) >> + ? ? status->value.sig = TARGET_SIGNAL_INT; >> + ? ? ?else >> + ? ? status->value.sig = TARGET_SIGNAL_TRAP; >> + >> + ? ? ?discard_cleanups (old_cleanups); >> + ? ?} >> + >> + ?do_cleanups (set_cleanups); >> + ?return inferior_ptid; >> +} > > I have to say that I find that function confusing, due to > the use of gotos, and excessive nesting. ?Personally, I much prefer > code that does: > > ?if (foo) > ? ?{ > ? ? ?/* something */ > ? ? ?return; > ? ?} > > ?if (bar) > ? ?{ > ? ? ?/* something */ > ? ? ?return; > ? ?} > > ?if (lala) > ? ?{ > ? ? ?/* something */ > ? ? ?return; > ? ?} > > Over: > > ?if (foo) > ? ?{ > ? ? ?/* something */ > ? ? ?return; > ? ?} > ?else > ? { > ? ? if (bar) > ? ? ? { > ? ? ? ? /* something */ > ? ? ? ? return; > ? ? ? } > ? ? else > ? ? ?{ > ? ? ? ? if (lala) > ? ? ? ? ? { > ? ? ? ? ? ? /* something */ > ? ? ? ? ? ? return; > ? ? ? ? ? } > ? ? ?} > ? } > > >> + >> +static void >> +record_disconnect (struct target_ops *target, char *args, int from_tty) >> +{ >> + ?if (record_debug) >> + ? ?fprintf_unfiltered (gdb_stdlog, "Process record: record_disconnect\n"); >> + >> + ?unpush_target (&record_ops); >> + ?target_disconnect (args, from_tty); >> +} >> + >> +static void >> +record_detach (struct target_ops *ops, char *args, int from_tty) >> +{ >> + ?if (record_debug) >> + ? ?fprintf_unfiltered (gdb_stdlog, "Process record: record_detach\n"); >> + >> + ?unpush_target (&record_ops); >> + ?target_detach (args, from_tty); >> +} > > This trick you're using happens to work, but, could you try > this instead? ?Here and elsewhere similarly. > > ?struct target_ops *beneath = find_target_beaneath (ops); > ?unpush_target (ops); > ?beneath->to_detach (args, from_tty); > >> + >> +static void >> +record_mourn_inferior (struct target_ops *ops) >> +{ >> + ?if (record_debug) >> + ? ?fprintf_unfiltered (gdb_stdlog, "Process record: " >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "record_mourn_inferior\n"); >> + >> + ?unpush_target (&record_ops); >> + ?target_mourn_inferior (); >> +} >> + >> +/* Close process record target before killing the inferior process. ?*/ >> +static void >> +record_kill (void) >> +{ >> + ?if (record_debug) >> + ? ?fprintf_unfiltered (gdb_stdlog, "Process record: record_kill\n"); >> + >> + ?unpush_target (&record_ops); >> + ?target_kill (); >> +} >> + >> +/* Record registers change (by user or by GDB) to list as an instruction. ?*/ >> +static void >> +record_registers_change (struct regcache *regcache, int regnum) >> +{ >> + ?/* Check record_insn_num. ?*/ >> + ?record_check_insn_num (0); >> + >> + ?record_arch_list_head = NULL; >> + ?record_arch_list_tail = NULL; >> + >> + ?record_regcache = get_current_regcache (); >> + >> + ?if (regnum < 0) >> + ? ?{ >> + ? ? ?int i; >> + ? ? ?for (i = 0; i < gdbarch_num_regs (get_regcache_arch (regcache)); i++) >> + ? ? { >> + ? ? ? if (record_arch_list_add_reg (i)) >> + ? ? ? ? { >> + ? ? ? ? ? record_list_release (record_arch_list_tail); >> + ? ? ? ? ? error (_("Process record: failed to record execution log.")); >> + ? ? ? ? } >> + ? ? } >> + ? ?} >> + ?else >> + ? ?{ >> + ? ? ?if (record_arch_list_add_reg (regnum)) >> + ? ? { >> + ? ? ? record_list_release (record_arch_list_tail); >> + ? ? ? error (_("Process record: failed to record execution log.")); >> + ? ? } >> + ? ?} >> + ?if (record_arch_list_add_end (0)) >> + ? ?{ >> + ? ? ?record_list_release (record_arch_list_tail); >> + ? ? ?error (_("Process record: failed to record execution log.")); >> + ? ?} >> + ?record_list->next = record_arch_list_head; >> + ?record_arch_list_head->prev = record_list; >> + ?record_list = record_arch_list_tail; >> + >> + ?if (record_insn_num == record_insn_max_num && record_insn_max_num) >> + ? ?record_list_release_first (); >> + ?else >> + ? ?record_insn_num++; >> +} >> + >> +static void >> +record_store_registers (struct target_ops *ops, struct regcache *regcache, >> + ? ? ? ? ? ? ? ? ? ? ? ?int regno) >> +{ >> + ?if (!record_gdb_operation_disable) >> + ? ?{ >> + ? ? ?if (RECORD_IS_REPLAY) >> + ? ? { >> + ? ? ? int n; >> + ? ? ? struct cleanup *old_cleanups; >> + >> + ? ? ? /* Let user choose if he wants to write register or not. ?*/ >> + ? ? ? if (regno < 0) >> + ? ? ? ? n = >> + ? ? ? ? ? nquery (_("Because GDB is in replay mode, changing the " >> + ? ? ? ? ? ? ? ? ? ? "value of a register will make the execution " >> + ? ? ? ? ? ? ? ? ? ? "log unusable from this point onward. ?" >> + ? ? ? ? ? ? ? ? ? ? "Change all registers?")); >> + ? ? ? else >> + ? ? ? ? n = >> + ? ? ? ? ? nquery (_("Because GDB is in replay mode, changing the value " >> + ? ? ? ? ? ? ? ? ? ? "of a register will make the execution log unusable " >> + ? ? ? ? ? ? ? ? ? ? "from this point onward. ?Change register %s?"), >> + ? ? ? ? ? ? ? ? ? gdbarch_register_name (get_regcache_arch (regcache), >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?regno)); >> + >> + ? ? ? if (!n) >> + ? ? ? ? { >> + ? ? ? ? ? /* Invalidate the value of regcache that was 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 (_("Process record canceled the operation.")); >> + ? ? ? ? } >> + >> + ? ? ? /* Destroy the record from here forward. ?*/ >> + ? ? ? record_list_release_next (); >> + ? ? } >> + >> + ? ? ?record_registers_change (regcache, regno); >> + ? ?} >> + ?record_beneath_to_store_registers (record_beneath_to_store_registers_ops, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? regcache, regno); >> +} >> + >> +/* record_xfer_partial -- behavior is conditional on RECORD_IS_REPLAY. >> + ? In replay mode, we cannot write memory unles we are willing to >> + ? invalidate the record/replay log from this point forward. ?*/ >> + >> +static LONGEST >> +record_xfer_partial (struct target_ops *ops, enum target_object object, >> + ? ? ? ? ? ? ? ? ?const char *annex, gdb_byte * readbuf, >> + ? ? ? ? ? ? ? ? ?const gdb_byte * writebuf, ULONGEST offset, LONGEST len) >> +{ >> + ?if (!record_gdb_operation_disable >> + ? ? ?&& (object == TARGET_OBJECT_MEMORY >> + ? ? ? || object == TARGET_OBJECT_RAW_MEMORY) && writebuf) >> + ? ?{ >> + ? ? ?if (RECORD_IS_REPLAY) >> + ? ? { >> + ? ? ? /* Let user choose if he wants to write memory or not. ?*/ >> + ? ? ? if (!nquery (_("Because GDB is in replay mode, writing to memory " >> + ? ? ? ? ? ? ? ? ? ? ?"will make the execution log unusable from this " >> + ? ? ? ? ? ? ? ? ? ? ?"point onward. ?Write memory at address 0x%s?"), >> + ? ? ? ? ? ? ? ? ? ?paddr_nz (offset))) >> + ? ? ? ? return -1; >> + >> + ? ? ? /* Destroy the record from here forward. ?*/ >> + ? ? ? record_list_release_next (); >> + ? ? } >> + >> + ? ? ?/* Check record_insn_num */ >> + ? ? ?record_check_insn_num (0); >> + >> + ? ? ?/* Record registers change to list as an instruction. ?*/ >> + ? ? ?record_arch_list_head = NULL; >> + ? ? ?record_arch_list_tail = NULL; >> + ? ? ?if (record_arch_list_add_mem (offset, len)) >> + ? ? { >> + ? ? ? record_list_release (record_arch_list_tail); >> + ? ? ? if (record_debug) >> + ? ? ? ? fprintf_unfiltered (gdb_stdlog, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? _("Process record: failed to record " >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "execution log.")); >> + ? ? ? return -1; >> + ? ? } >> + ? ? ?if (record_arch_list_add_end (0)) >> + ? ? { >> + ? ? ? record_list_release (record_arch_list_tail); >> + ? ? ? if (record_debug) >> + ? ? ? ? fprintf_unfiltered (gdb_stdlog, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? _("Process record: failed to record " >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "execution log.")); >> + ? ? ? return -1; >> + ? ? } >> + ? ? ?record_list->next = record_arch_list_head; >> + ? ? ?record_arch_list_head->prev = record_list; >> + ? ? ?record_list = record_arch_list_tail; >> + >> + ? ? ?if (record_insn_num == record_insn_max_num && record_insn_max_num) >> + ? ? record_list_release_first (); >> + ? ? ?else >> + ? ? record_insn_num++; >> + ? ?} >> + >> + ?return record_beneath_to_xfer_partial (record_beneath_to_xfer_partial_ops, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? object, annex, readbuf, writebuf, >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? offset, len); >> +} >> + >> +/* record_insert_breakpoint >> + ? record_remove_breakpoint >> + ? Behavior is conditional on RECORD_IS_REPLAY. >> + ? We will not actually insert or remove breakpoints when replaying, >> + ? nor when recording. ?*/ >> + >> +static int >> +record_insert_breakpoint (struct bp_target_info *bp_tgt) >> +{ >> + ?if (!RECORD_IS_REPLAY) >> + ? ?{ >> + ? ? ?struct cleanup *old_cleanups = record_gdb_operation_disable_set (); >> + ? ? ?int ret = record_beneath_to_insert_breakpoint (bp_tgt); >> + >> + ? ? ?do_cleanups (old_cleanups); >> + >> + ? ? ?return ret; >> + ? ?} >> + >> + ?return 0; >> +} >> + >> +static int >> +record_remove_breakpoint (struct bp_target_info *bp_tgt) >> +{ >> + ?if (!RECORD_IS_REPLAY) >> + ? ?{ >> + ? ? ?struct cleanup *old_cleanups = record_gdb_operation_disable_set (); >> + ? ? ?int ret = record_beneath_to_remove_breakpoint (bp_tgt); >> + >> + ? ? ?do_cleanups (old_cleanups); >> + >> + ? ? ?return ret; >> + ? ?} >> + >> + ?return 0; >> +} >> + >> +static int >> +record_can_execute_reverse (void) >> +{ >> + ?return 1; >> +} >> + >> +static void >> +init_record_ops (void) >> +{ >> + ?record_ops.to_shortname = "record"; >> + ?record_ops.to_longname = "Process record and replay target"; >> + ?record_ops.to_doc = >> + ? ?"Log program while executing and replay execution from log."; >> + ?record_ops.to_open = record_open; >> + ?record_ops.to_close = record_close; >> + ?record_ops.to_resume = record_resume; >> + ?record_ops.to_wait = record_wait; >> + ?record_ops.to_disconnect = record_disconnect; >> + ?record_ops.to_detach = record_detach; >> + ?record_ops.to_mourn_inferior = record_mourn_inferior; >> + ?record_ops.to_kill = record_kill; >> + ?record_ops.to_create_inferior = find_default_create_inferior; >> + ?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; >> + ?record_ops.to_can_execute_reverse = record_can_execute_reverse; >> + ?record_ops.to_stratum = record_stratum; >> + ?record_ops.to_magic = OPS_MAGIC; >> +} >> + >> +static void >> +show_record_debug (struct ui_file *file, int from_tty, >> + ? ? ? ? ? ? ? ?struct cmd_list_element *c, const char *value) >> +{ >> + ?fprintf_filtered (file, _("Debugging of process record target is %s.\n"), >> + ? ? ? ? ? ? ? ? value); >> +} >> + >> +/* cmd_record_start -- alias for "target record". ?*/ >> + >> +static void >> +cmd_record_start (char *args, int from_tty) >> +{ >> + ?execute_command ("target record", from_tty); >> +} >> + >> +/* cmd_record_delete -- truncate the record log from the present point >> + ? of replay until the end. ?*/ >> + >> +static void >> +cmd_record_delete (char *args, int from_tty) >> +{ >> + ?if (TARGET_IS_PROCESS_RECORD) >> + ? ?{ >> + ? ? ?if (RECORD_IS_REPLAY) >> + ? ? { >> + ? ? ? if (!from_tty || query (_("Delete the log from this point forward " >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "and begin to record the running message " >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "at current PC?"))) >> + ? ? ? ? record_list_release_next (); >> + ? ? } >> + ? ? ?else >> + ? ? ? printf_unfiltered (_("Already at end of record list.\n")); >> + >> + ? ?} >> + ?else >> + ? ?printf_unfiltered (_("Process record is not started.\n")); >> +} >> + >> +/* cmd_record_stop -- implement the "stoprecord" command. ?*/ >> + >> +static void >> +cmd_record_stop (char *args, int from_tty) >> +{ >> + ?if (TARGET_IS_PROCESS_RECORD) >> + ? ?{ >> + ? ? ?if (!record_list || !from_tty || query (_("Delete recorded log and " >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "stop recording?"))) >> + ? ? unpush_target (&record_ops); >> + ? ?} >> + ?else >> + ? ?printf_unfiltered (_("Process record is not started.\n")); >> +} >> + >> +/* set_record_insn_max_num -- set upper limit of record log size. ?*/ >> + >> +static void >> +set_record_insn_max_num (char *args, int from_tty, struct cmd_list_element *c) >> +{ >> + ?if (record_insn_num > record_insn_max_num && record_insn_max_num) >> + ? ?{ >> + ? ? ?printf_unfiltered (_("Record instructions number is bigger than " >> + ? ? ? ? ? ? ? ? ? ? ? ?"record instructions max number. ?Auto delete " >> + ? ? ? ? ? ? ? ? ? ? ? ?"the first ones?\n")); >> + >> + ? ? ?while (record_insn_num > record_insn_max_num) >> + ? ? record_list_release_first (); >> + ? ?} >> +} >> + >> +/* show_record_insn_number -- print the current index >> + ? into the record log (number of insns recorded so far). ?*/ >> + >> +static void >> +show_record_insn_number (char *ignore, int from_tty) >> +{ >> + ?printf_unfiltered (_("Record instruction number is %d.\n"), >> + ? ? ? ? ? ? ? ? ?record_insn_num); >> +} >> + >> +void >> +_initialize_record (void) >> +{ >> + ?/* Init record_maskall. ?*/ >> + ?if (sigfillset (&record_maskall) == -1) >> + ? ?perror_with_name (_("Process record: sigfillset failed")); > > This will not build on all hosts. ?Is it still needed? ?I can't > find any other reference to this variable in this patch. > >> + >> + ?/* Init record_first. ?*/ >> + ?record_first.prev = NULL; >> + ?record_first.next = NULL; >> + ?record_first.type = record_end; >> + ?record_first.u.need_dasm = 0; >> + >> + ?init_record_ops (); >> + ?add_target (&record_ops); >> + >> + ?add_setshow_zinteger_cmd ("record", no_class, &record_debug, >> + ? ? ? ? ? ? ? ? ? ? ? ? _("Set debugging of record/replay feature."), >> + ? ? ? ? ? ? ? ? ? ? ? ? _("Show debugging of record/replay feature."), >> + ? ? ? ? ? ? ? ? ? ? ? ? _("When enabled, debugging output for " >> + ? ? ? ? ? ? ? ? ? ? ? ? ? "record/replay feature is displayed."), >> + ? ? ? ? ? ? ? ? ? ? ? ? NULL, show_record_debug, &setdebuglist, >> + ? ? ? ? ? ? ? ? ? ? ? ? &showdebuglist); >> + >> + ?add_com ("record", class_obscure, cmd_record_start, >> + ? ? ? ?_("Abbreviated form of \"target record\" command.")); >> + >> + ?add_com_alias ("rec", "record", class_obscure, 1); >> + >> + ?/* XXX: I try to use some simple commands such as "disconnect" and >> + ? ? "detach" to support this functions. ?But these commands all have >> + ? ? other affect to GDB such as call function "no_shared_libraries". >> + ? ? So I add special commands to GDB. ?*/ >> + ?add_com ("delrecord", class_obscure, cmd_record_delete, >> + ? ? ? ?_("Delete the rest of execution log and start recording it anew.")); >> + ?add_com_alias ("dr", "delrecord", class_obscure, 1); >> + ?add_com ("stoprecord", class_obscure, cmd_record_stop, >> + ? ? ? ?_("Stop the record/replay target.")); >> + ?add_com_alias ("sr", "stoprecord", class_obscure, 1); >> + >> + ?/* Record instructions number limit command. ?*/ >> + ?add_setshow_boolean_cmd ("record-stop-at-limit", no_class, >> + ? ? ? ? ? ? ? ? ? ? ? ? &record_stop_at_limit, >> + ? ? ? ? ? ? ? ? ? ? ? ? _("Set whether record/replay stop when " >> + ? ? ? ? ? ? ? ? ? ? ? ? ? "record/replay buffer becomes full."), >> + ? ? ? ? ? ? ? ? ? ? ? ? _("Show whether record/replay stop when " >> + ? ? ? ? ? ? ? ? ? ? ? ? ? "record/replay buffer becomes full."), >> + ? ? ? ? ? ? ? ? ? ? ? ? _("Enable is default value.\n" >> + ? ? ? ? ? ? ? ? ? ? ? ? ? "When enabled, if the record/replay buffer " >> + ? ? ? ? ? ? ? ? ? ? ? ? ? "becomes full,\n" >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"ask user what to do.\n" >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"When disabled, if the record/replay buffer " >> + ? ? ? ? ? ? ? ? ? ? ? ? ? "becomes full,\n" >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"delete it and start new recording."), >> + ? ? ? ? ? ? ? ? ? ? ? ? NULL, NULL, &setlist, &showlist); >> + ?add_setshow_zinteger_cmd ("record-insn-number-max", no_class, >> + ? ? ? ? ? ? ? ? ? ? ? ? &record_insn_max_num, >> + ? ? ? ? ? ? ? ? ? ? ? ? _("Set record/replay buffer limit."), >> + ? ? ? ? ? ? ? ? ? ? ? ? _("Show record/replay buffer limit."), >> + ? ? ? ? ? ? ? ? ? ? ? ? _("Set the maximum number of instructions to be " >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"stored in the\n" >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"record/replay buffer. ?" >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"Zero means unlimited (default 200000)."), >> + ? ? ? ? ? ? ? ? ? ? ? ? set_record_insn_max_num, >> + ? ? ? ? ? ? ? ? ? ? ? ? NULL, &setlist, &showlist); >> + ?add_info ("record-insn-number", show_record_insn_number, >> + ? ? ? ? _("Show the current number of instructions in the " >> + ? ? ? ? ? "record/replay buffer.")); >> +} >> Index: src/gdb/record.h >> =================================================================== >> --- /dev/null 1970-01-01 00:00:00.000000000 +0000 >> +++ src/gdb/record.h ?2009-02-28 20:23:20.000000000 +0000 >> @@ -0,0 +1,87 @@ >> +/* Process record and replay target for GDB, the GNU debugger. >> + >> + ? Copyright (C) 2008 Free Software Foundation, Inc. >> + >> + ? This file is part of GDB. >> + >> + ? This program is free software; you can redistribute it and/or modify >> + ? it under the terms of the GNU General Public License as published by >> + ? the Free Software Foundation; either version 3 of the License, or >> + ? (at your option) any later version. >> + >> + ? This program is distributed in the hope that it will be useful, >> + ? but WITHOUT ANY WARRANTY; without even the implied warranty of >> + ? MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. ?See the >> + ? GNU General Public License for more details. >> + >> + ? You should have received a copy of the GNU General Public License >> + ? along with this program. ?If not, see <http://www.gnu.org/licenses/>. ?*/ >> + >> +#ifndef _RECORD_H_ >> +#define _RECORD_H_ >> + >> +#define TARGET_IS_PROCESS_RECORD ? \ >> + ? ? (current_target.beneath == &record_ops) > > Sorry, but I repeat the request I've made several times already. ?This is > not the right way to do this. ?You need to add a new target_ops method or > property that the core of GDB checks on. ?It is not correct that make > the core of GDB reference record_ops directly. ?To come up with > the target callback's name, at each call site of TARGET_IS_PROCESS_RECORD, > consider what is the property of the current target that GDB needs to > know about the current target. ?Is it something like: > > ?target_is_recording () ? > ?target_is_replaying () ? > ?target_is_read_only () ? > > etc. > >> +#define RECORD_IS_REPLAY \ >> + ? ? (record_list->next || execution_direction == EXEC_REVERSE) > > AFAICS, this macro is not used outside of record.c. ?It should move > there, along with anything that isn't used outside of record.c. > >> + >> +typedef struct record_reg_s >> +{ >> + ?int num; >> + ?gdb_byte *val; >> +} record_reg_t; >> + >> +typedef struct record_mem_s >> +{ >> + ?CORE_ADDR addr; >> + ?int len; >> + ?gdb_byte *val; >> +} record_mem_t; >> + >> +enum record_type >> +{ >> + ?record_end = 0, >> + ?record_reg, >> + ?record_mem >> +}; >> + >> +/* This is the core struct of record function. >> + >> + ? An entity of record_t is a record of the value change of a register >> + ? ("record_reg") or a part of memory ("record_mem"). ?And each >> + ? instruction must has a record_t ("record_end") that points out this >> + ? is the last record_t of this instruction. >> + >> + ? Each record_t is linked to "record_list" by "prev" and "next". >> + */ >> +typedef struct record_s >> +{ >> + ?struct record_s *prev; >> + ?struct record_s *next; >> + ?enum record_type type; >> + ?union >> + ?{ >> + ? ?/* reg */ >> + ? ?record_reg_t reg; >> + ? ?/* mem */ >> + ? ?record_mem_t mem; >> + ? ?/* end */ >> + ? ?int need_dasm; >> + ?} u; >> +} record_t; >> + >> +extern int record_debug; >> +extern record_t *record_list; >> +extern record_t *record_arch_list_head; >> +extern record_t *record_arch_list_tail; >> +extern struct regcache *record_regcache; > > Most of these things don't appear to be used anywhere else other > than in record.c. ?Can you remove these declarations from the > public header, and make them static in record.c? > >> + >> +extern struct target_ops record_ops; > > Once you get rid of TARGET_IS_PROCESS_RECORD this doesn't > need to be public anymore. > >> + >> +extern int record_arch_list_add_reg (int num); >> +extern int record_arch_list_add_mem (CORE_ADDR addr, int len); >> +extern int record_arch_list_add_end (int need_dasm); >> +extern void record_message (struct gdbarch *gdbarch); >> +extern struct cleanup * record_gdb_operation_disable_set (void); >> + >> +#endif /* _RECORD_H_ */ >> > > -- > Pedro Alves >
Attachment:
3-record_target.txt
Description: Text document
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |