Re: [rfc 3/5] record: make it build again

Jan Kratochvil
2013-02-10T22:11:23Z

On Fri, 08 Feb 2013 16:30:21 +0100, markus.t.metzger@intel.com wrote:
[...]
> --- a/gdb/record-full.c
> +++ b/gdb/record-full.c
> @@ -28,6 +28,7 @@
> #include "gdbcore.h"
> #include "exec.h"
> #include "record.h"
> +#include "record-full.h"
> #include "elf-bfd.h"
> #include "gcore.h"
> #include "event-loop.h"
> @@ -37,7 +38,7 @@
>
> #include <signal.h>
>
> -/* This module implements "target record", also known as "process
> +/* This module implements "target record-full", also known as "process
> record and replay". This target sits on top of a "normal" target
> (a target that "has execution"), and provides a record and replay
> functionality, including reverse debugging.
> @@ -205,6 +206,13 @@ static ULONGEST record_insn_count;
> static struct target_ops record_ops;
> static struct target_ops record_core_ops;
>
> +/* Command lists for "set/show record full". */
> +static struct cmd_list_element *set_record_full_cmdlist;
> +static struct cmd_list_element *show_record_full_cmdlist;
> +
> +/* Command list for "record full". */
> +static struct cmd_list_element *record_full_cmdlist;
> +
> /* The beneath function pointers. */
> static struct target_ops *record_beneath_to_resume_ops;
> static void (*record_beneath_to_resume) (struct target_ops *, ptid_t, int,
> @@ -234,6 +242,10 @@ static int (*record_beneath_to_stopped_data_address) (struct target_ops *,
> CORE_ADDR *);
> static void (*record_beneath_to_async) (void (*) (enum inferior_event_type, void *), void *);
>
> +static void record_goto_insn (struct record_entry *entry,
> + enum exec_direction_kind dir);
> +static void record_save (char *recfilename);
> +
> /* Alloc and free functions for record_reg, record_mem, and record_end
> entries. */
>
> @@ -564,7 +576,7 @@ record_check_insn_num (int 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)?"));
> + "full (record full stop-at-limit)?"));
> if (set_terminal)
> target_terminal_inferior ();
> if (q)
> @@ -1948,9 +1960,140 @@ record_execution_direction (void)
> }
>
> static void
> +record_info (void)

Such functions should be called/renamed to record_full_info, they are specific
for record-full.c and moreover other backends will have the same function.

You can rename everything, also record_ops, record_core_ops -> record_full_*.

GDB prevents using static names duplicated across files. (Maybe it comes from
the time before "ambiguous linespec" start to put breakpoints on all of them.)

> +{
> + struct record_entry *p;
> +
> + if (RECORD_IS_REPLAY)
> + printf_filtered (_("Replay mode:\n"));
> + else
> + printf_filtered (_("Record mode:\n"));
> +
> + /* Find entry for first actual instruction in the log. */
> + for (p = record_first.next;
> + p != NULL && p->type != record_end;
> + p = p->next)
> + ;
> +
> + /* Do we have a log at all? */
> + if (p != NULL && p->type == record_end)
> + {
> + /* Display instruction number for first instruction in the log. */
> + printf_filtered (_("Lowest recorded instruction number is %s.\n"),
> + pulongest (p->u.end.insn_num));
> +
> + /* If in replay mode, display where we are in the log. */
> + if (RECORD_IS_REPLAY)
> + printf_filtered (_("Current instruction number is %s.\n"),
> + pulongest (record_list->u.end.insn_num));
> +
> + /* Display instruction number for last instruction in the log. */
> + printf_filtered (_("Highest recorded instruction number is %s.\n"),
> + pulongest (record_insn_count));
> +
> + /* Display log count. */
> + printf_filtered (_("Log contains %d instructions.\n"),
> + record_insn_num);
> + }
> + else
> + printf_filtered (_("No instructions have been logged.\n"));
> +
> + /* Display max log size. */
> + printf_filtered (_("Max logged instructions is %d.\n"),
> + record_insn_max_num);
> +}
> +
> +/* The "to_record_delete" target method. */
> +
> +static void
> +record_delete (void)
> +{
> + record_list_release_following (record_list);
> +}
> +
> +/* The "to_record_is_replaying" target method. */
> +
> +static int
> +record_is_replaying (void)
> +{
> + return RECORD_IS_REPLAY;
> +}
> +
> +/* Go to a specific entry. */
> +
> +static void
> +record_goto_entry (struct record_entry *p)
> +{
> + if (p == NULL)
> + error (_("Target insn not found."));
> + else if (p == record_list)
> + error (_("Already at target insn."));
> + else if (p->u.end.insn_num > record_list->u.end.insn_num)
> + {
> + printf_filtered (_("Go forward to insn number %s\n"),
> + pulongest (p->u.end.insn_num));
> + record_goto_insn (p, EXEC_FORWARD);
> + }
> + else
> + {
> + printf_filtered (_("Go backward to insn number %s\n"),
> + pulongest (p->u.end.insn_num));
> + record_goto_insn (p, EXEC_REVERSE);
> + }
> +
> + registers_changed ();
> + reinit_frame_cache ();
> + print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC);
> +}
> +
> +/* The "to_goto_record_begin" target method. */
> +
> +static void
> +record_goto_begin (void)
> +{
> + struct record_entry *p = NULL;
> +
> + for (p = &record_first; p != NULL; p = p->next)
> + if (p->type == record_end)
> + break;
> +
> + record_goto_entry (p);
> +}
> +
> +/* The "to_goto_record_end" target method. */
> +
> +static void
> +record_goto_end (void)
> +{
> + struct record_entry *p = NULL;
> +
> + for (p = record_list; p->next != NULL; p = p->next)
> + ;
> + for (; p!= NULL; p = p->prev)
> + if (p->type == record_end)
> + break;
> +
> + record_goto_entry (p);
> +}
> +
> +/* The "to_goto_record" target method. */
> +
> +static void
> +record_goto (ULONGEST target_insn)
> +{
> + struct record_entry *p = NULL;
> +
> + for (p = &record_first; p != NULL; p = p->next)
> + if (p->type == record_end && p->u.end.insn_num == target_insn)
> + break;
> +
> + record_goto_entry (p);
> +}
> +
> +static void
> init_record_ops (void)
> {
> - record_ops.to_shortname = "record";
> + record_ops.to_shortname = "record-full";
> record_ops.to_longname = "Process record and replay target";
> record_ops.to_doc =
> "Log program while executing and replay execution from log.";
> @@ -1978,6 +2121,13 @@ init_record_ops (void)
> record_ops.to_can_async_p = record_can_async_p;
> record_ops.to_is_async_p = record_is_async_p;
> record_ops.to_execution_direction = record_execution_direction;
> + record_ops.to_info_record = record_info;
> + record_ops.to_save_record = record_save;
> + record_ops.to_delete_record = record_delete;
> + record_ops.to_record_is_replaying = record_is_replaying;
> + record_ops.to_goto_record_begin = record_goto_begin;
> + record_ops.to_goto_record_end = record_goto_end;
> + record_ops.to_goto_record = record_goto;
> record_ops.to_magic = OPS_MAGIC;
> }
>
> @@ -2203,6 +2353,12 @@ init_record_core_ops (void)
> record_core_ops.to_can_async_p = record_can_async_p;
> record_core_ops.to_is_async_p = record_is_async_p;
> record_core_ops.to_execution_direction = record_execution_direction;
> + record_core_ops.to_info_record = record_info;
> + record_core_ops.to_delete_record = record_delete;
> + record_core_ops.to_record_is_replaying = record_is_replaying;
> + record_core_ops.to_goto_record_begin = record_goto_begin;
> + record_core_ops.to_goto_record_end = record_goto_end;
> + record_core_ops.to_goto_record = record_goto;
> record_core_ops.to_magic = OPS_MAGIC;
> }
>
> @@ -2507,9 +2663,8 @@ record_save_cleanups (void *data)
> format, with an extra section for our data. */
>
> static void
> -cmd_record_save (char *args, int from_tty)
> +record_save (char *recfilename)

record_full_save etc.

> {
> - char *recfilename, recfilename_buffer[40];
> struct record_entry *cur_record_list;
> uint32_t magic;
> struct regcache *regcache;
> @@ -2521,20 +2676,6 @@ cmd_record_save (char *args, int from_tty)
> asection *osec = NULL;
> int bfd_offset = 0;
>
> - if (strcmp (current_target.to_shortname, "record") != 0)
> - error (_("This command can only be used with target 'record'.\n"
> - "Use 'target record' first.\n"));
> -
> - if (args && *args)
> - recfilename = args;
> - else
> - {
> - /* Default recfile name is "gdb_record.PID". */
> - snprintf (recfilename_buffer, sizeof (recfilename_buffer),
> - "gdb_record.%d", PIDGET (inferior_ptid));
> - recfilename = recfilename_buffer;
> - }
> -
> /* Open the save file. */
> if (record_debug)
> fprintf_unfiltered (gdb_stdlog, "Saving execution log to core file '%s'\n",
> @@ -2750,3 +2891,143 @@ record_goto_insn (struct record_entry *entry,
> } while (record_list != entry);
> do_cleanups (set_cleanups);
> }
> +
> +/* Alias for "target record-full". */
> +
> +static void
> +cmd_record_start (char *args, int from_tty)

cmd_record_full_start

> +{
> + execute_command ("target record-full", from_tty);
> +}
> +
> +static void
> +set_record_insn_max_num (char *args, int from_tty, struct cmd_list_element *c)

set_record_full_insn_max_num

> +{
> + if (record_insn_num > record_insn_max_num && record_insn_max_num)

record_full_insn_num

> + {
> + /* Count down record_insn_num while releasing records from list. */
> + while (record_insn_num > record_insn_max_num)
> + {
> + record_list_release_first ();

record_full_list_release_first etc.

> + record_insn_num--;
> + }
> + }
> +}
> +
> +/* The "set record full" command. */
> +
> +static void
> +set_record_full_command (char *args, int from_tty)
> +{
> + printf_unfiltered (_("\"set record full\" must be followed "
> + "by an apporpriate subcommand.\n"));
> + help_list (set_record_full_cmdlist, "set record full ", all_commands,
> + gdb_stdout);
> +}
> +
> +/* The "show record full" command. */
> +
> +static void
> +show_record_full_command (char *args, int from_tty)
> +{
> + cmd_show_list (show_record_full_cmdlist, from_tty, "");
> +}
> +
> +/* Provide a prototype to silence -Wmissing-prototypes. */
> +extern initialize_file_ftype _initialize_record_full;
> +
> +void
> +_initialize_record_full (void)
> +{
> + struct cmd_list_element *c;
> +
> + /* Init record_first. */
> + record_first.prev = NULL;
> + record_first.next = NULL;
> + record_first.type = record_end;
> +
> + init_record_ops ();
> + add_target (&record_ops);
> + add_deprecated_target_alias (&record_ops, "record");
> + init_record_core_ops ();
> + add_target (&record_core_ops);
> +
> + add_prefix_cmd ("full", class_obscure, cmd_record_start,
> + _("Start full execution recording."), &record_full_cmdlist,
> + "record full ", 0, &record_cmdlist);
> +
> + c = add_cmd ("restore", class_obscure, cmd_record_restore,
> + _("Restore the execution log from a file.\n\
> +Argument is filename. File must be created with 'record save'."),
> + &record_full_cmdlist);
> + set_cmd_completer (c, filename_completer);
> +
> + /* Deprecate the old version without "full" prefix. */
> + c = add_alias_cmd ("restore", "full restore", class_obscure, 1,
> + &record_cmdlist);
> + set_cmd_completer (c, filename_completer);
> + deprecate_cmd (c, "record full restore");

This (and all add_alias_cmd below) don't display the warning as discussed
before.

I guess we can keep it as is as the missing warning is tracked
deprecated_cmd_warning does not work for prefixed commands
http://sourceware.org/bugzilla/show_bug.cgi?id=15104
and the only command where it is most visible is "target record" which you
have successfully workarounded in the patchset.

a bit offtopic: 'git am' somehow broken on applicating the patchset (but later
it went OK by hand), it would be easier to have it in a public GIT branch,
possibly in
http://sourceware.org/gdb/wiki/ArcherBranchManagement
needing an account http://sourceware.org/cgi-bin/pdw/ps_form.cgi which you are
going to get for the later check-in anyway; or you could use github or some
such site.

> +
> + add_prefix_cmd ("full", class_support, set_record_full_command,
> + _("Set record options"), &set_record_full_cmdlist,
> + "set record full ", 0, &set_record_cmdlist);
> +
> + add_prefix_cmd ("full", class_support, show_record_full_command,
> + _("Show record options"), &show_record_full_cmdlist,
> + "show record full ", 0, &show_record_cmdlist);
> +
> + /* Record instructions number limit command. */
> + add_setshow_boolean_cmd ("stop-at-limit", no_class,
> + &record_stop_at_limit, _("\
> +Set whether record/replay stops when record/replay buffer becomes full."), _("\
> +Show whether record/replay stops when record/replay buffer becomes full."),
> + _("Default is ON.\n\
> +When ON, if the record/replay buffer becomes full, ask user what to do.\n\
> +When OFF, if the record/replay buffer becomes full,\n\
> +delete the oldest recorded instruction to make room for each new one."),
> + NULL, NULL,
> + &set_record_full_cmdlist, &show_record_full_cmdlist);
> +
> + c = add_alias_cmd ("stop-at-limit", "full stop-at-limit", no_class, 1,
> + &set_record_cmdlist);
> + deprecate_cmd (c, "set record full stop-at-limit");
> +
> + c = add_alias_cmd ("stop-at-limit", "full stop-at-limit", no_class, 1,
> + &show_record_cmdlist);
> + deprecate_cmd (c, "show record full stop-at-limit");
> +
> + add_setshow_uinteger_cmd ("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 is 200000."),
> + set_record_insn_max_num,
> + NULL, &set_record_full_cmdlist,
> + &show_record_full_cmdlist);
> +
> + c = add_alias_cmd ("insn-number-max", "full insn-number-max", no_class, 1,
> + &set_record_cmdlist);
> + deprecate_cmd (c, "set record full insn-number-max");
> +
> + c = add_alias_cmd ("insn-number-max", "full insn-number-max", no_class, 1,
> + &show_record_cmdlist);
> + deprecate_cmd (c, "show record full insn-number-max");
> +
> + add_setshow_boolean_cmd ("memory-query", no_class, &record_memory_query, _("\
> +Set whether query if PREC cannot record memory change of next instruction."),
> + _("\
> +Show whether query if PREC cannot record memory change of next instruction."),
> + _("\
> +Default is OFF.\n\
> +When ON, query if PREC cannot record memory change of next instruction."),
> + NULL, NULL,
> + &set_record_full_cmdlist, &show_record_full_cmdlist);
> +
> + c = add_alias_cmd ("memory-query", "full memory-query", no_class, 1,
> + &set_record_cmdlist);
> + deprecate_cmd (c, "set record full memory-query");
> +
> + c = add_alias_cmd ("memory-query", "full memory-query", no_class, 1,
> + &show_record_cmdlist);
> + deprecate_cmd (c, "show record full memory-query");
> +}
> diff --git a/gdb/record.c b/gdb/record.c
> index 2e970ae..5293417 100644
> --- a/gdb/record.c
> +++ b/gdb/record.c
> @@ -19,29 +19,46 @@
>
> #include "defs.h"
> #include "gdbcmd.h"
> -#include "regcache.h"
> -#include "gdbthread.h"
> -#include "event-top.h"
> -#include "exceptions.h"
> #include "completer.h"
> -#include "arch-utils.h"
> -#include "gdbcore.h"
> -#include "exec.h"
> #include "record.h"
> -#include "elf-bfd.h"
> -#include "gcore.h"
> -#include "event-loop.h"
> -#include "inf-loop.h"
> -#include "gdb_bfd.h"
> #include "observer.h"
> -
> -#include <signal.h>
> +#include "inferior.h"
>
> /* This is the debug switch for process record. */
> unsigned int record_debug = 0;
>
> -/* The implementation of the command "record goto". */
> -static void cmd_record_goto (char *, int);
> +struct cmd_list_element *record_cmdlist = NULL;
> +struct cmd_list_element *set_record_cmdlist = NULL;
> +struct cmd_list_element *show_record_cmdlist = NULL;
> +struct cmd_list_element *info_record_cmdlist = NULL;
> +
> +/* Find the record target in the target stack. */
> +
> +static struct target_ops *
> +find_record_target (void)
> +{
> + struct target_ops *t;
> +
> + for (t = current_target.beneath; t != NULL; t = t->beneath)
> + if (t->to_stratum == record_stratum)
> + return t;
> +
> + return NULL;
> +}
> +
> +/* Check that recording is active. Throw an error, if it isn't. */
> +
> +static struct target_ops *
> +require_record_target (void)
> +{
> + struct target_ops *t;
> +
> + t = find_record_target ();
> + if (t == NULL)
> + error (_("No record target is currently active."));

cmd_record_save formerly had more suggestive:
error (_("This command can only be used with target 'record'.\n"
"Use 'target record' first.\n"));

So require_record_target could have now:
error (_("No record target is currently active.\n"
"Use one of the 'target record-<tab><tab>' commands first.\n"));

> +
> + return t;
> +}
>
> /* Implement "show record debug" command. */
>
> @@ -58,7 +75,7 @@ show_record_debug (struct ui_file *file, int from_tty,
> static void
> cmd_record_start (char *args, int from_tty)
> {
> - execute_command ("target record", from_tty);
> + execute_command ("target record-full", from_tty);
> }
>
> /* Truncate the record log from the present point
> @@ -67,21 +84,25 @@ cmd_record_start (char *args, int from_tty)
> static void
> cmd_record_delete (char *args, int from_tty)
> {
> - if (current_target.to_stratum == record_stratum)
> + require_record_target ();
> +
> + if (!target_record_is_replaying ())
> {
> - 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_following (record_list);
> - }
> - else
> - printf_unfiltered (_("Already at end of record list.\n"));
> + printf_unfiltered (_("Already at end of record list.\n"));
> + return;
> + }
>
> + if (!target_supports_delete_record ())
> + {
> + printf_unfiltered (_("The current record target does not support "
> + "this operation.\n"));
> + return;
> }
> - else
> - printf_unfiltered (_("Process record is not started.\n"));
> +
> + if (!from_tty || query (_("Delete the log from this point forward "
> + "and begin to record the running message "
> + "at current PC?")))
> + target_delete_record ();
> }
>
> /* Implement the "stoprecord" or "record stop" command. */
> @@ -89,36 +110,18 @@ cmd_record_delete (char *args, int from_tty)
> static void
> cmd_record_stop (char *args, int from_tty)
> {
> - if (current_target.to_stratum == record_stratum)
> - {
> - unpush_target (&record_ops);
> - printf_unfiltered (_("Process record is stopped and all execution "
> - "logs are deleted.\n"));
> + struct target_ops *t;
>
> - observer_notify_record_changed (current_inferior (), 0);
> - }
> - else
> - printf_unfiltered (_("Process record is not started.\n"));
> -}
> + t = require_record_target ();
> + unpush_target (t);
>
> -/* Set upper limit of record log size. */
> + printf_unfiltered (_("Process record is stopped and all execution "
> + "logs are deleted.\n"));
>
> -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)
> - {
> - /* Count down record_insn_num while releasing records from list. */
> - while (record_insn_num > record_insn_max_num)
> - {
> - record_list_release_first ();
> - record_insn_num--;
> - }
> - }
> + observer_notify_record_changed (current_inferior (), 0);
> }
>
> -static struct cmd_list_element *record_cmdlist, *set_record_cmdlist,
> - *show_record_cmdlist, *info_record_cmdlist;
> +/* The "set record" command. */
>
> static void
> set_record_command (char *args, int from_tty)
> @@ -128,65 +131,53 @@ set_record_command (char *args, int from_tty)
> help_list (set_record_cmdlist, "set record ", all_commands, gdb_stdout);
> }
>
> +/* The "show record" command. */
> +
> static void
> show_record_command (char *args, int from_tty)
> {
> cmd_show_list (show_record_cmdlist, from_tty, "");
> }
>
> -/* Display some statistics about the execution log. */
> +/* The "info record" command. */
>
> static void
> info_record_command (char *args, int from_tty)
> {
> - struct record_entry *p;
> + struct target_ops *t;
>
> - if (current_target.to_stratum == record_stratum)
> + t = find_record_target ();
> + if (t == NULL)
> {
> - if (RECORD_IS_REPLAY)
> - printf_filtered (_("Replay mode:\n"));
> - else
> - printf_filtered (_("Record mode:\n"));
> -
> - /* Find entry for first actual instruction in the log. */
> - for (p = record_first.next;
> - p != NULL && p->type != record_end;
> - p = p->next)
> - ;
> -
> - /* Do we have a log at all? */
> - if (p != NULL && p->type == record_end)
> - {
> - /* Display instruction number for first instruction in the log. */
> - printf_filtered (_("Lowest recorded instruction number is %s.\n"),
> - pulongest (p->u.end.insn_num));
> -
> - /* If in replay mode, display where we are in the log. */
> - if (RECORD_IS_REPLAY)
> - printf_filtered (_("Current instruction number is %s.\n"),
> - pulongest (record_list->u.end.insn_num));
> -
> - /* Display instruction number for last instruction in the log. */
> - printf_filtered (_("Highest recorded instruction number is %s.\n"),
> - pulongest (record_insn_count));
> -
> - /* Display log count. */
> - printf_filtered (_("Log contains %d instructions.\n"),
> - record_insn_num);
> - }
> - else
> - {
> - printf_filtered (_("No instructions have been logged.\n"));
> - }
> + printf_filtered (_("No record target is currently active.\n"));
> + return;
> }
> +
> + printf_filtered (_("Active record target: %s\n"), t->to_shortname);
> + if (t->to_info_record)

nit: According to the new GDB coding style rules it should be:
if (t->to_info_record != NULL)

> + t->to_info_record ();
> +}
> +
> +/* The "record save" command. */
> +
> +static void
> +cmd_record_save (char *args, int from_tty)
> +{
> + char *recfilename, recfilename_buffer[40];
> +
> + require_record_target ();
> +
> + if (args && *args)
> + recfilename = args;
> else
> {
> - printf_filtered (_("target record is not active.\n"));
> + /* Default recfile name is "gdb_record.PID". */
> + snprintf (recfilename_buffer, sizeof (recfilename_buffer),
> + "gdb_record.%d", PIDGET (inferior_ptid));

xsnprintf, intended for better OS compatibility.

> + recfilename = recfilename_buffer;
> }
>
> - /* Display max log size. */
> - printf_filtered (_("Max logged instructions is %d.\n"),
> - record_insn_max_num);
> + target_save_record (recfilename);
> }
>
> /* "record goto" command. Argument is an instruction number,
> @@ -194,65 +185,26 @@ info_record_command (char *args, int from_tty)
>
> Rewinds the recording (forward or backward) to the given instruction. */
>
> -static void
> +void
> cmd_record_goto (char *arg, int from_tty)
> {
> - struct record_entry *p = NULL;
> - ULONGEST target_insn = 0;
> + require_record_target ();
>
> if (arg == NULL || *arg == '\0')
> error (_("Command requires an argument (insn number to go to)."));
>
> if (strncmp (arg, "start", strlen ("start")) == 0
> || strncmp (arg, "begin", strlen ("begin")) == 0)
> - {
> - /* Special case. Find first insn. */
> - for (p = &record_first; p != NULL; p = p->next)
> - if (p->type == record_end)
> - break;
> - if (p)
> - target_insn = p->u.end.insn_num;
> - }
> + target_goto_record_begin ();
> else if (strncmp (arg, "end", strlen ("end")) == 0)
> - {
> - /* Special case. Find last insn. */
> - for (p = record_list; p->next != NULL; p = p->next)
> - ;
> - for (; p!= NULL; p = p->prev)
> - if (p->type == record_end)
> - break;
> - if (p)
> - target_insn = p->u.end.insn_num;
> - }
> + target_goto_record_end ();
> else
> {
> - /* General case. Find designated insn. */
> - target_insn = parse_and_eval_long (arg);
> + ULONGEST insn;
>
> - for (p = &record_first; p != NULL; p = p->next)
> - if (p->type == record_end && p->u.end.insn_num == target_insn)
> - break;
> + insn = parse_and_eval_long (arg);
> + target_goto_record (insn);
> }
> -
> - if (p == NULL)
> - error (_("Target insn '%s' not found."), arg);
> - else if (p == record_list)
> - error (_("Already at insn '%s'."), arg);
> - else if (p->u.end.insn_num > record_list->u.end.insn_num)
> - {
> - printf_filtered (_("Go forward to insn number %s\n"),
> - pulongest (target_insn));
> - record_goto_insn (p, EXEC_FORWARD);
> - }
> - else
> - {
> - printf_filtered (_("Go backward to insn number %s\n"),
> - pulongest (target_insn));
> - record_goto_insn (p, EXEC_REVERSE);
> - }
> - registers_changed ();
> - reinit_frame_cache ();
> - print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC);
> }
>
> /* Provide a prototype to silence -Wmissing-prototypes. */
> @@ -263,16 +215,6 @@ _initialize_record (void)
> {
> struct cmd_list_element *c;
>
> - /* Init record_first. */
> - record_first.prev = NULL;
> - record_first.next = NULL;
> - record_first.type = record_end;
> -
> - init_record_ops ();
> - add_target (&record_ops);
> - init_record_core_ops ();
> - add_target (&record_core_ops);
> -
> add_setshow_zuinteger_cmd ("record", no_class, &record_debug,
> _("Set debugging of record/replay feature."),
> _("Show debugging of record/replay feature."),
> @@ -282,7 +224,7 @@ _initialize_record (void)
> &showdebuglist);
>
> c = add_prefix_cmd ("record", class_obscure, cmd_record_start,
> - _("Abbreviated form of \"target record\" command."),
> + _("Start recording."),
> &record_cmdlist, "record ", 0, &cmdlist);
> set_cmd_completer (c, filename_completer);
>
> @@ -307,12 +249,6 @@ Default filename is 'gdb_record.<process_id>'."),
> &record_cmdlist);
> set_cmd_completer (c, filename_completer);
>
> - c = add_cmd ("restore", class_obscure, cmd_record_restore,
> - _("Restore the execution log from a file.\n\
> -Argument is filename. File must be created with 'record save'."),
> - &record_cmdlist);
> - set_cmd_completer (c, filename_completer);
> -
> add_cmd ("delete", class_obscure, cmd_record_delete,
> _("Delete the rest of execution log and start recording it anew."),
> &record_cmdlist);
> @@ -324,40 +260,8 @@ Argument is filename. File must be created with 'record save'."),
> &record_cmdlist);
> add_alias_cmd ("s", "stop", class_obscure, 1, &record_cmdlist);
>
> - /* Record instructions number limit command. */
> - add_setshow_boolean_cmd ("stop-at-limit", no_class,
> - &record_stop_at_limit, _("\
> -Set whether record/replay stops when record/replay buffer becomes full."), _("\
> -Show whether record/replay stops when record/replay buffer becomes full."),
> - _("Default is ON.\n\
> -When ON, if the record/replay buffer becomes full, ask user what to do.\n\
> -When OFF, if the record/replay buffer becomes full,\n\
> -delete the oldest recorded instruction to make room for each new one."),
> - NULL, NULL,
> - &set_record_cmdlist, &show_record_cmdlist);
> - add_setshow_uinteger_cmd ("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 is 200000."),
> - set_record_insn_max_num,
> - NULL, &set_record_cmdlist, &show_record_cmdlist);
> -
> add_cmd ("goto", class_obscure, cmd_record_goto, _("\
> Restore the program to its state at instruction number N.\n\
> Argument is instruction number, as shown by 'info record'."),
> &record_cmdlist);
> -
> - add_setshow_boolean_cmd ("memory-query", no_class,
> - &record_memory_query, _("\
> -Set whether query if PREC cannot record memory change of next instruction."),
> - _("\
> -Show whether query if PREC cannot record memory change of next instruction."),
> - _("\
> -Default is OFF.\n\
> -When ON, query if PREC cannot record memory change of next instruction."),
> - NULL, NULL,
> - &set_record_cmdlist, &show_record_cmdlist);
> -
> }
> diff --git a/gdb/record.h b/gdb/record.h
> index 280f4ec..dfd8361 100644
> --- a/gdb/record.h
> +++ b/gdb/record.h
> @@ -20,8 +20,18 @@
> #ifndef _RECORD_H_
> #define _RECORD_H_
>
> +struct cmd_list_element;
> +
> #define RECORD_IS_USED (current_target.to_stratum == record_stratum)
>
> extern unsigned int record_debug;
>
> +/* Allow record targets to add their own sub-commands. */
> +extern struct cmd_list_element *record_cmdlist;
> +extern struct cmd_list_element *set_record_cmdlist;
> +extern struct cmd_list_element *show_record_cmdlist;
> +extern struct cmd_list_element *info_record_cmdlist;
> +
> +extern void cmd_record_goto (char *arg, int from_tty);
> +
> #endif /* _RECORD_H_ */
> diff --git a/gdb/target.c b/gdb/target.c
> index 25f4629..e71ab96 100644
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -4231,6 +4231,135 @@ target_read_btrace (struct btrace_target_info *btinfo)
> return NULL;
> }
>
> +/* See target.h. */
> +
> +void
> +target_info_record (void)
> +{
> + struct target_ops *t;
> +
> + for (t = current_target.beneath; t != NULL; t = t->beneath)
> + if (t->to_info_record != NULL)
> + {
> + t->to_info_record ();
> + return;
> + }
> +
> + tcomplain ();
> +}
> +
> +/* See target.h. */
> +
> +void
> +target_save_record (char *filename)
> +{
> + struct target_ops *t;
> +
> + for (t = current_target.beneath; t != NULL; t = t->beneath)
> + if (t->to_save_record != NULL)
> + {
> + t->to_save_record (filename);
> + return;
> + }
> +
> + tcomplain ();
> +}
> +
> +/* See target.h. */
> +
> +int
> +target_supports_delete_record (void)
> +{
> + struct target_ops *t;
> +
> + for (t = current_target.beneath; t != NULL; t = t->beneath)
> + if (t->to_delete_record != NULL)
> + return 1;
> +
> + return 0;
> +}
> +
> +/* See target.h. */
> +
> +void
> +target_delete_record (void)
> +{
> + struct target_ops *t;
> +
> + for (t = current_target.beneath; t != NULL; t = t->beneath)
> + if (t->to_delete_record != NULL)
> + {
> + t->to_delete_record ();
> + return;
> + }
> +
> + tcomplain ();
> +}
> +
> +/* See target.h. */
> +
> +int
> +target_record_is_replaying (void)
> +{
> + struct target_ops *t;
> +
> + for (t = current_target.beneath; t != NULL; t = t->beneath)
> + if (t->to_record_is_replaying != NULL)
> + return t->to_record_is_replaying ();
> +
> + return 0;
> +}
> +
> +/* See target.h. */
> +
> +void
> +target_goto_record_begin (void)
> +{
> + struct target_ops *t;
> +
> + for (t = current_target.beneath; t != NULL; t = t->beneath)
> + if (t->to_goto_record_begin != NULL)
> + {
> + t->to_goto_record_begin ();
> + return;
> + }
> +
> + tcomplain ();
> +}
> +
> +/* See target.h. */
> +
> +void
> +target_goto_record_end (void)
> +{
> + struct target_ops *t;
> +
> + for (t = current_target.beneath; t != NULL; t = t->beneath)
> + if (t->to_goto_record_end != NULL)
> + {
> + t->to_goto_record_end ();
> + return;
> + }
> +
> + tcomplain ();
> +}
> +
> +/* See target.h. */
> +
> +void
> +target_goto_record (ULONGEST insn)
> +{
> + struct target_ops *t;
> +
> + for (t = current_target.beneath; t != NULL; t = t->beneath)
> + if (t->to_goto_record != NULL)
> + {
> + t->to_goto_record (insn);
> + return;
> + }
> +
> + tcomplain ();
> +}
>
> static void
> debug_to_prepare_to_store (struct regcache *regcache)
> diff --git a/gdb/target.h b/gdb/target.h
> index 1d73336..e4fe5da 100644
> --- a/gdb/target.h
> +++ b/gdb/target.h
> @@ -876,6 +876,27 @@ struct target_ops
> /* Read branch trace data. */
> VEC (btrace_block_s) *(*to_read_btrace) (struct btrace_target_info *);
>
> + /* Print information about the recording. */
> + void (*to_info_record) (void);
> +
> + /* Save the recorded execution trace into a file. */
> + void (*to_save_record) (char *filename);
> +
> + /* Delete the recorded execution trace from the current position onwards. */
> + void (*to_delete_record) (void);
> +
> + /* Query if the record target is currently replaying. */
> + int (*to_record_is_replaying) (void);
> +
> + /* Go to the begin of the execution trace. */
> + void (*to_goto_record_begin) (void);
> +
> + /* Go to the end of the execution trace. */
> + void (*to_goto_record_end) (void);
> +
> + /* Go to a specific location in the recorded execution trace. */
> + void (*to_goto_record) (ULONGEST);

Just a nit, here could be 'ULONGEST insn'.

> +
> int to_magic;
> /* Need sub-structure for target machine related rather than comm related?
> */
> @@ -1938,5 +1959,28 @@ extern int target_btrace_has_changed (struct btrace_target_info *btinfo);
> Returns a vector of branch trace blocks with the latest entry at index 0. */
> extern VEC (btrace_block_s) *target_read_btrace (struct btrace_target_info *);
>
> +/* Print record information for this record target. */
> +extern void target_info_record (void);

Please do not use comments for these target_* declarations, they duplicate
those of their to_* fields and they could become out of sync. As you
commented the to_* fields just write:
/* See to_info_record in struct target_ops. */

> +
> +/* Save the recorded execution trace into a file. */
> +extern void target_save_record (char *filename);
> +
> +/* Query if the target supports deleting the execution log. */
> +extern int target_supports_delete_record (void);
> +
> +/* Delete the recorded execution trace from the current position onwards. */
> +extern void target_delete_record (void);
> +
> +/* Query if the record target is currently replaying. */
> +extern int target_record_is_replaying (void);
> +
> +/* Go to the begin of the execution trace. */
> +extern void target_goto_record_begin (void);
> +
> +/* Go to the end of the execution trace. */
> +extern void target_goto_record_end (void);
> +
> +/* Go to a specific location in the recorded execution trace. */
> +extern void target_goto_record (ULONGEST);

Just a nit, here could be 'ULONGEST insn'.

>
> #endif /* !defined (TARGET_H) */
> --
> 1.7.0.7