Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [RFA] replay bookmarks
@ 2009-11-01 21:13 Michael Snyder
  2009-11-01 21:23 ` Eli Zaretskii
  0 siblings, 1 reply; 23+ messages in thread
From: Michael Snyder @ 2009-11-01 21:13 UTC (permalink / raw)
  To: gdb-patches, Greg Law, Jakob Engblom, Hui Zhu

[-- Attachment #1: Type: text/plain, Size: 1088 bytes --]

Here's a first cut at the "bookmark" functionality discussed earlier.

This should be target-agnostic -- I've added two target methods,
to_get_bookmark and to_goto_bookmark.  For remote targets, we use
q/Q query syntax.

Seems to work well in process record.

Summary:
   bookmark (no argument)
     Save a reference to the current position in the replay log.
     Doesn't care whether replay log is precord or arbitrary remote
     target.
   delete bookmark <N>
     Delete the bookmark numbered N (just like breakpoint numbers)
   goto-bookmark <N>
     Return the replay log to the state of the bookmark numbered N.
   info bookmarks
     List bookmarks.


Bookmarks are saved internally in the form of a string, which is
provided by and returned to the target.

There is a back-door -- if the user understands the format of
the string that is used by the target to represent bookmarks,
he can supply a quoted string (single or double quotes) eg.:

   (gdb) bookmark '12345'

which will be passed along to the target, even if no corresponding
bookmark exists in gdb's internal list.


[-- Attachment #2: bookmark.txt --]
[-- Type: text/plain, Size: 21202 bytes --]

2009-10-25  Michael Snyder  <msnyder@vmware.com>

        * target.h (struct target_ops): New methods to_get_bookmark
        and to_goto_bookmark.
        (target_get_bookmark): New macro.
        (target_goto_bookmark): New macro.
        * target.c (dummy_get_bookmark): New function, default implementation.
        (dummy_goto_bookmark): New function, default implementation.
        (update_current_target): Inherit new methods.
        * record.c (record_get_bookmark): New function.
        (record_goto_bookmark): New function.
        (init_record_ops): Set to_get_bookmark and to_goto_bookmark methods.
        * reverse.c (struct bookmark): New type.
        (save_bookmark_command): New function (command).
        (delete_bookmark_command): New function (command).
        (goto_bookmark_command): New function (command).
        (bookmarks_info): New function (command).
        (_initialize_reverse): Add new bookmark commands.
        * remote.c (remote_get_bookmark): New target method.
        (remote_goto_bookmark): New target method.
	* command.h (enum command_class): Add class_bookmark.
	* NEWS: Mention bookmark commands.

Index: gdb/record.c
===================================================================
--- gdb.orig/record.c	2009-11-01 12:35:54.000000000 -0800
+++ gdb/record.c	2009-11-01 12:39:42.000000000 -0800
@@ -1523,6 +1523,57 @@
   return 1;
 }
 
+/* "to_get_bookmark" method for process record and prec over core.  */
+
+static gdb_byte *
+record_get_bookmark (char *args, int from_tty)
+{
+  gdb_byte *ret = NULL;
+
+  /* Return stringified form of instruction count.  */
+  if (record_list && record_list->type == record_end)
+    ret = xstrdup (pulongest (record_list->u.end.insn_num));
+
+  if (record_debug)
+    {
+      if (ret)
+	fprintf_unfiltered (gdb_stdlog,
+			    "record_get_bookmark returns %s\n", ret);
+      else
+	fprintf_unfiltered (gdb_stdlog,
+			    "record_get_bookmark returns NULL\n");
+    }
+  return ret;
+}
+
+/* The implementation of the command "record goto".  */
+static void cmd_record_goto (char *, int);
+
+/* "to_goto_bookmark" method for process record and prec over core.  */
+
+static void
+record_goto_bookmark (gdb_byte *bookmark, int from_tty)
+{
+  if (record_debug)
+    fprintf_unfiltered (gdb_stdlog,
+			"record_goto_bookmark receives %s\n", bookmark);
+
+  if (bookmark[0] == '\'' || bookmark[0] == '\"')
+    {
+      if (bookmark[strlen (bookmark) - 1] != bookmark[0])
+	error (_("Unbalanced quotes: %s"), bookmark);
+
+      /* Strip trailing quote.  */
+      bookmark[strlen (bookmark) - 1] = '\0';
+      /* Strip leading quote.  */
+      bookmark++;
+      /* Pass along to cmd_record_goto.  */
+    }
+
+  cmd_record_goto ((char *) bookmark, from_tty);
+  return;
+}
+
 static void
 init_record_ops (void)
 {
@@ -1545,6 +1596,9 @@
   record_ops.to_remove_breakpoint = record_remove_breakpoint;
   record_ops.to_can_execute_reverse = record_can_execute_reverse;
   record_ops.to_stratum = record_stratum;
+  /* Add bookmark target methods.  */
+  record_ops.to_get_bookmark = record_get_bookmark;
+  record_ops.to_goto_bookmark = record_goto_bookmark;
   record_ops.to_magic = OPS_MAGIC;
 }
 
@@ -1750,6 +1804,9 @@
   record_core_ops.to_can_execute_reverse = record_can_execute_reverse;
   record_core_ops.to_has_execution = record_core_has_execution;
   record_core_ops.to_stratum = record_stratum;
+  /* Add bookmark target methods.  */
+  record_core_ops.to_get_bookmark = record_get_bookmark;
+  record_core_ops.to_goto_bookmark = record_goto_bookmark;
   record_core_ops.to_magic = OPS_MAGIC;
 }
 
@@ -2407,6 +2464,99 @@
 		   recfilename);
 }
 
+/* record_goto_insn -- rewind the record log (forward or backward,
+   depending on DIR) to the given entry, changing the program state
+   correspondingly.  */
+
+static void
+record_goto_insn (struct record_entry *entry,
+		  enum exec_direction_kind dir)
+{
+  struct cleanup *set_cleanups = record_gdb_operation_disable_set ();
+  struct regcache *regcache = get_current_regcache ();
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+
+  /* Assume everything is valid: we will hit the entry,
+     and we will not hit the end of the recording.  */
+
+  if (dir == EXEC_FORWARD)
+    record_list = record_list->next;
+
+  do
+    {
+      record_exec_insn (regcache, gdbarch, record_list);
+      if (dir == EXEC_REVERSE)
+	record_list = record_list->prev;
+      else
+	record_list = record_list->next;
+    } while (record_list != entry);
+  do_cleanups (set_cleanups);
+}
+
+/* "record goto" command.  Argument is an instruction number,
+   as given by "info record".
+
+   Rewinds the recording (forward or backward) to the given instruction.  */
+
+static void
+cmd_record_goto (char *arg, int from_tty)
+{
+  struct record_entry *p = NULL;
+  ULONGEST target_insn = 0;
+
+  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;
+    }
+  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;
+    }
+  else
+    {
+      /* General case.  Find designated insn.  */
+      target_insn = parse_and_eval_long (arg);
+
+      for (p = &record_first; p != NULL; p = p->next)
+	if (p->type == record_end && p->u.end.insn_num == target_insn)
+	  break;
+    }
+
+  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 %d\n", (int) target_insn);
+      record_goto_insn (p, EXEC_FORWARD);
+    }
+  else
+    {
+      printf_filtered ("Go backward to insn number %d\n", (int) target_insn);
+      record_goto_insn (p, EXEC_REVERSE);
+    }
+  registers_changed ();
+  reinit_frame_cache ();
+  print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC);
+}
+
 void
 _initialize_record (void)
 {
@@ -2492,4 +2642,9 @@
 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);
 }
Index: gdb/remote.c
===================================================================
--- gdb.orig/remote.c	2009-11-01 12:35:54.000000000 -0800
+++ gdb/remote.c	2009-11-01 12:39:42.000000000 -0800
@@ -8882,6 +8882,46 @@
   return rs->cond_tracepoints;
 }
 
+/* "to_get_bookmark" target method.
+
+   Return a string from the target that uniquely identifies the
+   current machine state, such that the target will be able to
+   return to this machine state at a later request.
+
+   Only expected to work with record/replay targets.  */
+
+static gdb_byte *
+remote_get_bookmark (char *args, int from_tty)
+{
+  struct remote_state *rs = get_remote_state ();
+
+  putpkt ("qBookmark");
+  getpkt (&rs->buf, &rs->buf_size, 0);
+  if (rs->buf[0] == 'Q' && rs->buf[1] == 'B')
+    return &rs->buf[2];
+  else
+    return NULL;
+}
+
+/* "to_goto_bookmark" target method.
+
+   Restore the target to an earlier-recorded machine state.  */
+
+static void
+remote_goto_bookmark (gdb_byte *bookmark, int from_tty)
+{
+  struct remote_state *rs = get_remote_state ();
+  char *p = rs->buf;
+
+  xsnprintf (rs->buf, rs->buf_size, "QBookmark:%s", bookmark);
+
+  putpkt (p);
+  getpkt (&rs->buf, &rs->buf_size, 0);
+  if (rs->buf[0] != 'O' && rs->buf[1] != 'K')
+    error (_("remote goto bookmark not implemented."));
+}
+
+
 static void
 init_remote_ops (void)
 {
@@ -8931,7 +8971,6 @@
   remote_ops.to_has_execution = default_child_has_execution;
   remote_ops.to_has_thread_control = tc_schedlock;	/* can lock scheduler */
   remote_ops.to_can_execute_reverse = remote_can_execute_reverse;
-  remote_ops.to_magic = OPS_MAGIC;
   remote_ops.to_memory_map = remote_memory_map;
   remote_ops.to_flash_erase = remote_flash_erase;
   remote_ops.to_flash_done = remote_flash_done;
@@ -8945,6 +8984,9 @@
   remote_ops.to_terminal_ours = remote_terminal_ours;
   remote_ops.to_supports_non_stop = remote_supports_non_stop;
   remote_ops.to_supports_multi_process = remote_supports_multi_process;
+  remote_ops.to_get_bookmark = remote_get_bookmark;
+  remote_ops.to_goto_bookmark = remote_goto_bookmark;
+  remote_ops.to_magic = OPS_MAGIC;
 }
 
 /* Set up the extended remote vector by making a copy of the standard
Index: gdb/reverse.c
===================================================================
--- gdb.orig/reverse.c	2009-11-01 12:35:54.000000000 -0800
+++ gdb/reverse.c	2009-11-01 12:49:32.000000000 -0800
@@ -24,6 +24,7 @@
 #include "cli/cli-cmds.h"
 #include "cli/cli-decode.h"
 #include "inferior.h"
+#include "regcache.h"
 
 /* User interface:
    reverse-step, reverse-next etc.  */
@@ -101,6 +102,221 @@
   exec_reverse_once ("finish", args, from_tty);
 }
 
+/* Data structures for a bookmark list.  */
+
+struct bookmark {
+  struct bookmark *next;
+  int number;
+  CORE_ADDR pc;
+  struct symtab_and_line sal;
+  gdb_byte *opaque_data;
+};
+
+struct bookmark *bookmark_chain;
+int bookmark_count;
+
+#define ALL_BOOKMARKS(B) for ((B) = bookmark_chain; (B); (B) = (B)->next)
+
+#define ALL_BOOKMARKS_SAFE(B,TMP)           \
+     for ((B) = bookmark_chain;             \
+          (B) ? ((TMP) = (B)->next, 1) : 0; \
+          (B) = (TMP))
+
+/* save_bookmark_command -- implement "bookmark" command.
+   Call target method to get a bookmark identifier.
+   Insert bookmark identifier into list.
+
+   Identifier will be a malloc string (gdb_byte *).
+   Up to us to free it as required.  */
+
+static void
+save_bookmark_command (char *args, int from_tty)
+{
+  /* Get target's idea of a bookmark.  */
+  gdb_byte *bookmark_id = target_get_bookmark (args, from_tty);
+  struct bookmark *b, *b1;
+  struct gdbarch *gdbarch = get_regcache_arch (get_current_regcache ());
+
+  /* CR should not cause another identical bookmark.  */
+  dont_repeat ();
+
+  if (bookmark_id == NULL)
+    error (_("target_get_bookmark failed."));
+
+  /* Set up a bookmark struct.  */
+  b = xcalloc (1, sizeof (struct bookmark));
+  b->number = ++bookmark_count;
+  init_sal (&b->sal);
+  b->pc = regcache_read_pc (get_current_regcache ());
+  b->sal = find_pc_line (b->pc, 0);
+  b->sal.pspace = get_frame_program_space (get_current_frame ());
+  b->opaque_data = bookmark_id;
+  b->next = NULL;
+
+  /* Add this bookmark to the end of the chain, so that a list
+     of bookmarks will come out in order of increasing numbers.  */
+
+  b1 = bookmark_chain;
+  if (b1 == 0)
+    bookmark_chain = b;
+  else
+    {
+      while (b1->next)
+	b1 = b1->next;
+      b1->next = b;
+    }
+  printf_filtered (_("Saved bookmark %d at %s\n"), b->number,
+		     paddress (gdbarch, b->sal.pc));
+}
+
+/* Implement "delete bookmark" command.  */
+
+static int
+delete_one_bookmark (struct bookmark *b)
+{
+  struct bookmark *b1;
+
+  /* Special case, first item in list.  */
+  if (b == bookmark_chain)
+    bookmark_chain = b->next;
+
+  /* Find bookmark preceeding "marked" one, so we can unlink.  */
+  /* FIXME what about end cases (first and last)?  */
+  if (b)
+    {
+      ALL_BOOKMARKS (b1)
+	if (b1->next == b)
+	  {
+	    /* Found designated bookmark.  Unlink and delete.  */
+	    b1->next = b->next;
+	    break;
+	  }
+      xfree (b->opaque_data);
+      xfree (b);
+      return 1;		/* success */
+    }
+  return 0;		/* failure */
+}
+
+static void
+delete_all_bookmarks (void)
+{
+  struct bookmark *b, *b1;
+
+  ALL_BOOKMARKS_SAFE (b, b1)
+    {
+      xfree (b->opaque_data);
+      xfree (b);
+    }
+  bookmark_chain = NULL;
+}
+
+static void
+delete_bookmark_command (char *args, int from_tty)
+{
+  struct bookmark *b, *b1;
+  unsigned long num;
+
+  if (bookmark_chain == NULL)
+    {
+      warning ("No bookmarks.");
+      return;
+    }
+
+  if (args == NULL || args[0] == '\0')
+    {
+      if (from_tty && !query (_("Delete all bookmarks? ")))
+	return;
+      delete_all_bookmarks ();
+      return;
+    }
+
+  num = strtoul (args, NULL, 0);
+  /* Find bookmark with corresponding number.  */
+  ALL_BOOKMARKS (b)
+    if (b->number == num)
+      break;
+
+  if (!delete_one_bookmark (b))
+    /* Not found.  */
+    error (_("delete bookmark: no bookmark found for '%s'."), args);
+}
+
+/* Implement "goto-bookmark" command.  */
+
+static void
+goto_bookmark_command (char *args, int from_tty)
+{
+  struct bookmark *b;
+  unsigned long num;
+
+  if (args == NULL || args[0] == '\0')
+    error (_("Command requires an argument."));
+
+  if (strncmp (args, "start", strlen ("start")) == 0 ||
+      strncmp (args, "begin", strlen ("begin")) == 0 ||
+      strncmp (args, "end",   strlen ("end")) == 0)
+    {
+      /* Special case.  Give target opportunity to handle.  */
+      target_goto_bookmark (args, from_tty);
+      return;
+    }
+
+  if (args[0] == '\'' || args[0] == '\"')
+    {
+      /* Special case -- quoted string.  Pass on to target.  */
+      if (args[strlen (args) - 1] != args[0])
+	error (_("Unbalanced quotes: %s"), args);
+      target_goto_bookmark (args, from_tty);
+      return;
+    }
+
+  /* General case.  Bookmark identified by bookmark number.  */
+  num = strtoul (args, NULL, 0);
+  ALL_BOOKMARKS (b)
+    if (b->number == num)
+      break;
+
+  if (b)
+    {
+      /* Found.  Send to target method.  */
+      target_goto_bookmark (b->opaque_data, from_tty);
+      return;
+    }
+  /* Not found.  */
+  error (_("goto-bookmark: no bookmark found for '%s'."), args);
+}
+
+/* Implement "info bookmarks" command.  */
+
+static void
+bookmarks_info (char *args, int from_tty)
+{
+  struct bookmark *b;
+  int bnum = -1;
+  struct gdbarch *gdbarch;
+
+  if (args)
+    bnum = parse_and_eval_long (args);
+
+  if (!bookmark_chain)
+    {
+      printf_filtered (_("No bookmarks.\n"));
+      return;
+    }
+
+  gdbarch = get_regcache_arch (get_current_regcache ());
+  printf_filtered (_("Bookmark    Address     Opaque\n"));
+  printf_filtered (_("   ID                    Data \n"));
+
+  ALL_BOOKMARKS (b)
+    printf_filtered ("   %d       %s    '%s'\n",
+		     b->number,
+		     paddress (gdbarch, b->pc),
+		     b->opaque_data);
+}
+
+
 /* Provide a prototype to silence -Wmissing-prototypes.  */
 extern initialize_file_ftype _initialize_reverse;
 
@@ -142,4 +358,20 @@
 
   add_com ("reverse-finish", class_run, reverse_finish, _("\
 Execute backward until just before selected stack frame is called."));
+
+  add_com ("bookmark", class_bookmark, save_bookmark_command, _("\
+Set a bookmark.\n\
+A bookmark represents a point in the execution history \n\
+that can be returned to at a later point in the debug session."));
+  add_info ("bookmarks", bookmarks_info, _("\
+Status of user-settable bookmarks.\n\
+Bookmarks are user-settable markers representing a point in the \n\
+execution history that can be returned to later in the same debug \n\
+session."));
+  add_cmd ("bookmark", class_bookmark, delete_bookmark_command, _("\
+Delete a bookmark.\n"), &deletelist);
+  add_com ("goto-bookmark", class_bookmark, goto_bookmark_command, _("\
+Go to bookmark <n>\n\
+Return to the point in the execution history that was saved previously\n\
+by using the 'bookmark' command."));
 }
Index: gdb/target.c
===================================================================
--- gdb.orig/target.c	2009-11-01 12:35:54.000000000 -0800
+++ gdb/target.c	2009-11-01 12:39:42.000000000 -0800
@@ -674,6 +674,8 @@
       INHERIT (to_async_mask, t);
       INHERIT (to_find_memory_regions, t);
       INHERIT (to_make_corefile_notes, t);
+      INHERIT (to_get_bookmark, t);
+      INHERIT (to_goto_bookmark, t);
       /* Do not inherit to_get_thread_local_address.  */
       INHERIT (to_can_execute_reverse, t);
       INHERIT (to_thread_architecture, t);
@@ -2767,6 +2769,21 @@
   return NULL;
 }
 
+/* Error-catcher for target_get_bookmark.  */
+static gdb_byte *
+dummy_get_bookmark (char *ignore1, int ignore2)
+{
+  tcomplain ();
+  return NULL;
+}
+
+/* Error-catcher for target_goto_bookmark.  */
+static void
+dummy_goto_bookmark (gdb_byte *ignore, int from_tty)
+{
+  tcomplain ();
+}
+
 /* Set up the handful of non-empty slots needed by the dummy target
    vector.  */
 
@@ -2787,6 +2804,8 @@
   dummy_target.to_stratum = dummy_stratum;
   dummy_target.to_find_memory_regions = dummy_find_memory_regions;
   dummy_target.to_make_corefile_notes = dummy_make_corefile_notes;
+  dummy_target.to_get_bookmark = dummy_get_bookmark;
+  dummy_target.to_goto_bookmark = dummy_goto_bookmark;
   dummy_target.to_xfer_partial = default_xfer_partial;
   dummy_target.to_has_all_memory = (int (*) (struct target_ops *)) return_zero;
   dummy_target.to_has_memory = (int (*) (struct target_ops *)) return_zero;
Index: gdb/target.h
===================================================================
--- gdb.orig/target.h	2009-11-01 12:35:54.000000000 -0800
+++ gdb/target.h	2009-11-01 12:39:42.000000000 -0800
@@ -459,13 +459,18 @@
     void (*to_async) (void (*) (enum inferior_event_type, void *), void *);
     int (*to_async_mask) (int);
     int (*to_supports_non_stop) (void);
+    /* find_memory_regions support method for gcore */
     int (*to_find_memory_regions) (int (*) (CORE_ADDR,
 					    unsigned long,
 					    int, int, int,
 					    void *),
 				   void *);
+    /* make_corefile_notes support method for gcore */
     char * (*to_make_corefile_notes) (bfd *, int *);
-
+    /* get_bookmark support method for bookmarks */
+    gdb_byte * (*to_get_bookmark) (char *, int);
+    /* goto_bookmark support method for bookmarks */
+    void (*to_goto_bookmark) (gdb_byte *, int);
     /* Return the thread-local address at OFFSET in the
        thread-local storage for the thread PTID and the shared library
        or executable file given by OBJFILE.  If that block of
@@ -1141,6 +1146,13 @@
 #define target_make_corefile_notes(BFD, SIZE_P) \
      (current_target.to_make_corefile_notes) (BFD, SIZE_P)
 
+/* Bookmark interfaces.  */
+#define target_get_bookmark(ARGS, FROM_TTY) \
+     (current_target.to_get_bookmark) (ARGS, FROM_TTY)
+
+#define target_goto_bookmark(ARG, FROM_TTY) \
+     (current_target.to_goto_bookmark) (ARG, FROM_TTY)
+
 /* Hardware watchpoint interfaces.  */
 
 /* Returns non-zero if we were stopped by a hardware watchpoint (memory read or
Index: gdb/breakpoint.c
===================================================================
--- gdb.orig/breakpoint.c	2009-11-01 12:35:54.000000000 -0800
+++ gdb/breakpoint.c	2009-11-01 12:39:42.000000000 -0800
@@ -10086,6 +10086,8 @@
 Convenience variable \"$bpnum\" contains the number of the last\n\
 breakpoint set."));
 
+  add_info_alias ("b", "breakpoints", 1);
+
   if (xdb_commands)
     add_com ("lb", class_breakpoint, breakpoints_info, _("\
 Status of user-settable breakpoints, or breakpoint number NUMBER.\n\
Index: gdb/NEWS
===================================================================
--- gdb.orig/NEWS	2009-11-01 12:53:32.000000000 -0800
+++ gdb/NEWS	2009-11-01 12:59:46.000000000 -0800
@@ -24,6 +24,23 @@
 
 * New commands (for set/show, see "New options" below)
 
+bookmark
+  Save a reference to the current position in the execution log (for
+  replay targets such as process record/replay).  This execution
+  position can then be returned to later with the 'goto-bookmark' 
+  command.
+
+goto-bookmark [<N>]
+  Return the replay target to the excution position represented by the
+  Nth bookmark.  Resume replay debugging from that point.
+
+info bookmarks
+  List the bookmarks presently known to gdb.  Each bookmark can be
+  returned to using the 'goto-bookmark' command.
+
+delete bookmark [<N>]
+  Delete the Nth bookmark, or all bookmarks.
+
 record save [<FILENAME>]
   Save a file (in core file format) containing the process record 
   execution log for replay debugging at a later time.
Index: gdb/command.h
===================================================================
--- gdb.orig/command.h	2009-08-13 07:56:19.000000000 -0700
+++ gdb/command.h	2009-11-01 12:45:16.000000000 -0800
@@ -32,8 +32,8 @@
   /* Classes of commands */
   no_class = -1, class_run = 0, class_vars, class_stack,
   class_files, class_support, class_info, class_breakpoint, class_trace,
-  class_alias, class_obscure, class_user, class_maintenance,
-  class_pseudo, class_tui, class_xdb
+  class_bookmark, class_alias, class_obscure, class_user, 
+  class_maintenance, class_pseudo, class_tui, class_xdb
 };
 
 /* FIXME: cagney/2002-03-17: Once cmd_type() has been removed, ``enum

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFA] replay bookmarks
  2009-11-01 21:13 [RFA] replay bookmarks Michael Snyder
@ 2009-11-01 21:23 ` Eli Zaretskii
  2009-11-02  0:29   ` Michael Snyder
  0 siblings, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2009-11-01 21:23 UTC (permalink / raw)
  To: Michael Snyder; +Cc: gdb-patches, glaw, jakob, teawater

> Date: Sun, 01 Nov 2009 13:13:24 -0800
> From: Michael Snyder <msnyder@vmware.com>
> 
> +  add_com ("bookmark", class_bookmark, save_bookmark_command, _("\
> +Set a bookmark.\n\

This is way too short.  Suggest to expand a bit:

  Set a bookmark in the inferior's execution history

> +  add_com ("goto-bookmark", class_bookmark, goto_bookmark_command, _("\
> +Go to bookmark <n>\n\

Similarly here: such a short description is hardly useful, as it just
repeats the name of the command.

TIA


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFA] replay bookmarks
  2009-11-01 21:23 ` Eli Zaretskii
@ 2009-11-02  0:29   ` Michael Snyder
  2009-11-02  4:01     ` Eli Zaretskii
                       ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: Michael Snyder @ 2009-11-02  0:29 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches, glaw, jakob, teawater

[-- Attachment #1: Type: text/plain, Size: 550 bytes --]

Eli Zaretskii wrote:
>> Date: Sun, 01 Nov 2009 13:13:24 -0800
>> From: Michael Snyder <msnyder@vmware.com>
>>
>> +  add_com ("bookmark", class_bookmark, save_bookmark_command, _("\
>> +Set a bookmark.\n\
> 
> This is way too short.  Suggest to expand a bit:
> 
>   Set a bookmark in the inferior's execution history
> 
>> +  add_com ("goto-bookmark", class_bookmark, goto_bookmark_command, _("\
>> +Go to bookmark <n>\n\
> 
> Similarly here: such a short description is hardly useful, as it just
> repeats the name of the command.

How about this?



[-- Attachment #2: bookmark.txt --]
[-- Type: text/plain, Size: 21365 bytes --]

2009-10-25  Michael Snyder  <msnyder@vmware.com>

        * target.h (struct target_ops): New methods to_get_bookmark
        and to_goto_bookmark.
        (target_get_bookmark): New macro.
        (target_goto_bookmark): New macro.
        * target.c (dummy_get_bookmark): New function, default implementation.
        (dummy_goto_bookmark): New function, default implementation.
        (update_current_target): Inherit new methods.
        * record.c (record_get_bookmark): New function.
        (record_goto_bookmark): New function.
        (init_record_ops): Set to_get_bookmark and to_goto_bookmark methods.
        * reverse.c (struct bookmark): New type.
        (save_bookmark_command): New function (command).
        (delete_bookmark_command): New function (command).
        (goto_bookmark_command): New function (command).
        (bookmarks_info): New function (command).
        (_initialize_reverse): Add new bookmark commands.
        * remote.c (remote_get_bookmark): New target method.
        (remote_goto_bookmark): New target method.
	* command.h (enum command_class): Add class_bookmark.
	* NEWS: Mention bookmark commands.

Index: gdb/record.c
===================================================================
--- gdb.orig/record.c	2009-11-01 14:05:02.000000000 -0800
+++ gdb/record.c	2009-11-01 16:26:37.000000000 -0800
@@ -1523,6 +1523,57 @@
   return 1;
 }
 
+/* "to_get_bookmark" method for process record and prec over core.  */
+
+static gdb_byte *
+record_get_bookmark (char *args, int from_tty)
+{
+  gdb_byte *ret = NULL;
+
+  /* Return stringified form of instruction count.  */
+  if (record_list && record_list->type == record_end)
+    ret = xstrdup (pulongest (record_list->u.end.insn_num));
+
+  if (record_debug)
+    {
+      if (ret)
+	fprintf_unfiltered (gdb_stdlog,
+			    "record_get_bookmark returns %s\n", ret);
+      else
+	fprintf_unfiltered (gdb_stdlog,
+			    "record_get_bookmark returns NULL\n");
+    }
+  return ret;
+}
+
+/* The implementation of the command "record goto".  */
+static void cmd_record_goto (char *, int);
+
+/* "to_goto_bookmark" method for process record and prec over core.  */
+
+static void
+record_goto_bookmark (gdb_byte *bookmark, int from_tty)
+{
+  if (record_debug)
+    fprintf_unfiltered (gdb_stdlog,
+			"record_goto_bookmark receives %s\n", bookmark);
+
+  if (bookmark[0] == '\'' || bookmark[0] == '\"')
+    {
+      if (bookmark[strlen (bookmark) - 1] != bookmark[0])
+	error (_("Unbalanced quotes: %s"), bookmark);
+
+      /* Strip trailing quote.  */
+      bookmark[strlen (bookmark) - 1] = '\0';
+      /* Strip leading quote.  */
+      bookmark++;
+      /* Pass along to cmd_record_goto.  */
+    }
+
+  cmd_record_goto ((char *) bookmark, from_tty);
+  return;
+}
+
 static void
 init_record_ops (void)
 {
@@ -1545,6 +1596,9 @@
   record_ops.to_remove_breakpoint = record_remove_breakpoint;
   record_ops.to_can_execute_reverse = record_can_execute_reverse;
   record_ops.to_stratum = record_stratum;
+  /* Add bookmark target methods.  */
+  record_ops.to_get_bookmark = record_get_bookmark;
+  record_ops.to_goto_bookmark = record_goto_bookmark;
   record_ops.to_magic = OPS_MAGIC;
 }
 
@@ -1750,6 +1804,9 @@
   record_core_ops.to_can_execute_reverse = record_can_execute_reverse;
   record_core_ops.to_has_execution = record_core_has_execution;
   record_core_ops.to_stratum = record_stratum;
+  /* Add bookmark target methods.  */
+  record_core_ops.to_get_bookmark = record_get_bookmark;
+  record_core_ops.to_goto_bookmark = record_goto_bookmark;
   record_core_ops.to_magic = OPS_MAGIC;
 }
 
@@ -2407,6 +2464,99 @@
 		   recfilename);
 }
 
+/* record_goto_insn -- rewind the record log (forward or backward,
+   depending on DIR) to the given entry, changing the program state
+   correspondingly.  */
+
+static void
+record_goto_insn (struct record_entry *entry,
+		  enum exec_direction_kind dir)
+{
+  struct cleanup *set_cleanups = record_gdb_operation_disable_set ();
+  struct regcache *regcache = get_current_regcache ();
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+
+  /* Assume everything is valid: we will hit the entry,
+     and we will not hit the end of the recording.  */
+
+  if (dir == EXEC_FORWARD)
+    record_list = record_list->next;
+
+  do
+    {
+      record_exec_insn (regcache, gdbarch, record_list);
+      if (dir == EXEC_REVERSE)
+	record_list = record_list->prev;
+      else
+	record_list = record_list->next;
+    } while (record_list != entry);
+  do_cleanups (set_cleanups);
+}
+
+/* "record goto" command.  Argument is an instruction number,
+   as given by "info record".
+
+   Rewinds the recording (forward or backward) to the given instruction.  */
+
+static void
+cmd_record_goto (char *arg, int from_tty)
+{
+  struct record_entry *p = NULL;
+  ULONGEST target_insn = 0;
+
+  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;
+    }
+  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;
+    }
+  else
+    {
+      /* General case.  Find designated insn.  */
+      target_insn = parse_and_eval_long (arg);
+
+      for (p = &record_first; p != NULL; p = p->next)
+	if (p->type == record_end && p->u.end.insn_num == target_insn)
+	  break;
+    }
+
+  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 %d\n", (int) target_insn);
+      record_goto_insn (p, EXEC_FORWARD);
+    }
+  else
+    {
+      printf_filtered ("Go backward to insn number %d\n", (int) target_insn);
+      record_goto_insn (p, EXEC_REVERSE);
+    }
+  registers_changed ();
+  reinit_frame_cache ();
+  print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC);
+}
+
 void
 _initialize_record (void)
 {
@@ -2492,4 +2642,9 @@
 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);
 }
Index: gdb/remote.c
===================================================================
--- gdb.orig/remote.c	2009-11-01 14:05:02.000000000 -0800
+++ gdb/remote.c	2009-11-01 14:08:31.000000000 -0800
@@ -8882,6 +8882,46 @@
   return rs->cond_tracepoints;
 }
 
+/* "to_get_bookmark" target method.
+
+   Return a string from the target that uniquely identifies the
+   current machine state, such that the target will be able to
+   return to this machine state at a later request.
+
+   Only expected to work with record/replay targets.  */
+
+static gdb_byte *
+remote_get_bookmark (char *args, int from_tty)
+{
+  struct remote_state *rs = get_remote_state ();
+
+  putpkt ("qBookmark");
+  getpkt (&rs->buf, &rs->buf_size, 0);
+  if (rs->buf[0] == 'Q' && rs->buf[1] == 'B')
+    return &rs->buf[2];
+  else
+    return NULL;
+}
+
+/* "to_goto_bookmark" target method.
+
+   Restore the target to an earlier-recorded machine state.  */
+
+static void
+remote_goto_bookmark (gdb_byte *bookmark, int from_tty)
+{
+  struct remote_state *rs = get_remote_state ();
+  char *p = rs->buf;
+
+  xsnprintf (rs->buf, rs->buf_size, "QBookmark:%s", bookmark);
+
+  putpkt (p);
+  getpkt (&rs->buf, &rs->buf_size, 0);
+  if (rs->buf[0] != 'O' && rs->buf[1] != 'K')
+    error (_("remote goto bookmark not implemented."));
+}
+
+
 static void
 init_remote_ops (void)
 {
@@ -8931,7 +8971,6 @@
   remote_ops.to_has_execution = default_child_has_execution;
   remote_ops.to_has_thread_control = tc_schedlock;	/* can lock scheduler */
   remote_ops.to_can_execute_reverse = remote_can_execute_reverse;
-  remote_ops.to_magic = OPS_MAGIC;
   remote_ops.to_memory_map = remote_memory_map;
   remote_ops.to_flash_erase = remote_flash_erase;
   remote_ops.to_flash_done = remote_flash_done;
@@ -8945,6 +8984,9 @@
   remote_ops.to_terminal_ours = remote_terminal_ours;
   remote_ops.to_supports_non_stop = remote_supports_non_stop;
   remote_ops.to_supports_multi_process = remote_supports_multi_process;
+  remote_ops.to_get_bookmark = remote_get_bookmark;
+  remote_ops.to_goto_bookmark = remote_goto_bookmark;
+  remote_ops.to_magic = OPS_MAGIC;
 }
 
 /* Set up the extended remote vector by making a copy of the standard
Index: gdb/reverse.c
===================================================================
--- gdb.orig/reverse.c	2009-11-01 14:05:02.000000000 -0800
+++ gdb/reverse.c	2009-11-01 16:28:40.000000000 -0800
@@ -24,6 +24,7 @@
 #include "cli/cli-cmds.h"
 #include "cli/cli-decode.h"
 #include "inferior.h"
+#include "regcache.h"
 
 /* User interface:
    reverse-step, reverse-next etc.  */
@@ -101,6 +102,221 @@
   exec_reverse_once ("finish", args, from_tty);
 }
 
+/* Data structures for a bookmark list.  */
+
+struct bookmark {
+  struct bookmark *next;
+  int number;
+  CORE_ADDR pc;
+  struct symtab_and_line sal;
+  gdb_byte *opaque_data;
+};
+
+struct bookmark *bookmark_chain;
+int bookmark_count;
+
+#define ALL_BOOKMARKS(B) for ((B) = bookmark_chain; (B); (B) = (B)->next)
+
+#define ALL_BOOKMARKS_SAFE(B,TMP)           \
+     for ((B) = bookmark_chain;             \
+          (B) ? ((TMP) = (B)->next, 1) : 0; \
+          (B) = (TMP))
+
+/* save_bookmark_command -- implement "bookmark" command.
+   Call target method to get a bookmark identifier.
+   Insert bookmark identifier into list.
+
+   Identifier will be a malloc string (gdb_byte *).
+   Up to us to free it as required.  */
+
+static void
+save_bookmark_command (char *args, int from_tty)
+{
+  /* Get target's idea of a bookmark.  */
+  gdb_byte *bookmark_id = target_get_bookmark (args, from_tty);
+  struct bookmark *b, *b1;
+  struct gdbarch *gdbarch = get_regcache_arch (get_current_regcache ());
+
+  /* CR should not cause another identical bookmark.  */
+  dont_repeat ();
+
+  if (bookmark_id == NULL)
+    error (_("target_get_bookmark failed."));
+
+  /* Set up a bookmark struct.  */
+  b = xcalloc (1, sizeof (struct bookmark));
+  b->number = ++bookmark_count;
+  init_sal (&b->sal);
+  b->pc = regcache_read_pc (get_current_regcache ());
+  b->sal = find_pc_line (b->pc, 0);
+  b->sal.pspace = get_frame_program_space (get_current_frame ());
+  b->opaque_data = bookmark_id;
+  b->next = NULL;
+
+  /* Add this bookmark to the end of the chain, so that a list
+     of bookmarks will come out in order of increasing numbers.  */
+
+  b1 = bookmark_chain;
+  if (b1 == 0)
+    bookmark_chain = b;
+  else
+    {
+      while (b1->next)
+	b1 = b1->next;
+      b1->next = b;
+    }
+  printf_filtered (_("Saved bookmark %d at %s\n"), b->number,
+		     paddress (gdbarch, b->sal.pc));
+}
+
+/* Implement "delete bookmark" command.  */
+
+static int
+delete_one_bookmark (struct bookmark *b)
+{
+  struct bookmark *b1;
+
+  /* Special case, first item in list.  */
+  if (b == bookmark_chain)
+    bookmark_chain = b->next;
+
+  /* Find bookmark preceeding "marked" one, so we can unlink.  */
+  /* FIXME what about end cases (first and last)?  */
+  if (b)
+    {
+      ALL_BOOKMARKS (b1)
+	if (b1->next == b)
+	  {
+	    /* Found designated bookmark.  Unlink and delete.  */
+	    b1->next = b->next;
+	    break;
+	  }
+      xfree (b->opaque_data);
+      xfree (b);
+      return 1;		/* success */
+    }
+  return 0;		/* failure */
+}
+
+static void
+delete_all_bookmarks (void)
+{
+  struct bookmark *b, *b1;
+
+  ALL_BOOKMARKS_SAFE (b, b1)
+    {
+      xfree (b->opaque_data);
+      xfree (b);
+    }
+  bookmark_chain = NULL;
+}
+
+static void
+delete_bookmark_command (char *args, int from_tty)
+{
+  struct bookmark *b, *b1;
+  unsigned long num;
+
+  if (bookmark_chain == NULL)
+    {
+      warning ("No bookmarks.");
+      return;
+    }
+
+  if (args == NULL || args[0] == '\0')
+    {
+      if (from_tty && !query (_("Delete all bookmarks? ")))
+	return;
+      delete_all_bookmarks ();
+      return;
+    }
+
+  num = strtoul (args, NULL, 0);
+  /* Find bookmark with corresponding number.  */
+  ALL_BOOKMARKS (b)
+    if (b->number == num)
+      break;
+
+  if (!delete_one_bookmark (b))
+    /* Not found.  */
+    error (_("delete bookmark: no bookmark found for '%s'."), args);
+}
+
+/* Implement "goto-bookmark" command.  */
+
+static void
+goto_bookmark_command (char *args, int from_tty)
+{
+  struct bookmark *b;
+  unsigned long num;
+
+  if (args == NULL || args[0] == '\0')
+    error (_("Command requires an argument."));
+
+  if (strncmp (args, "start", strlen ("start")) == 0 ||
+      strncmp (args, "begin", strlen ("begin")) == 0 ||
+      strncmp (args, "end",   strlen ("end")) == 0)
+    {
+      /* Special case.  Give target opportunity to handle.  */
+      target_goto_bookmark (args, from_tty);
+      return;
+    }
+
+  if (args[0] == '\'' || args[0] == '\"')
+    {
+      /* Special case -- quoted string.  Pass on to target.  */
+      if (args[strlen (args) - 1] != args[0])
+	error (_("Unbalanced quotes: %s"), args);
+      target_goto_bookmark (args, from_tty);
+      return;
+    }
+
+  /* General case.  Bookmark identified by bookmark number.  */
+  num = strtoul (args, NULL, 0);
+  ALL_BOOKMARKS (b)
+    if (b->number == num)
+      break;
+
+  if (b)
+    {
+      /* Found.  Send to target method.  */
+      target_goto_bookmark (b->opaque_data, from_tty);
+      return;
+    }
+  /* Not found.  */
+  error (_("goto-bookmark: no bookmark found for '%s'."), args);
+}
+
+/* Implement "info bookmarks" command.  */
+
+static void
+bookmarks_info (char *args, int from_tty)
+{
+  struct bookmark *b;
+  int bnum = -1;
+  struct gdbarch *gdbarch;
+
+  if (args)
+    bnum = parse_and_eval_long (args);
+
+  if (!bookmark_chain)
+    {
+      printf_filtered (_("No bookmarks.\n"));
+      return;
+    }
+
+  gdbarch = get_regcache_arch (get_current_regcache ());
+  printf_filtered (_("Bookmark    Address     Opaque\n"));
+  printf_filtered (_("   ID                    Data \n"));
+
+  ALL_BOOKMARKS (b)
+    printf_filtered ("   %d       %s    '%s'\n",
+		     b->number,
+		     paddress (gdbarch, b->pc),
+		     b->opaque_data);
+}
+
+
 /* Provide a prototype to silence -Wmissing-prototypes.  */
 extern initialize_file_ftype _initialize_reverse;
 
@@ -142,4 +358,19 @@
 
   add_com ("reverse-finish", class_run, reverse_finish, _("\
 Execute backward until just before selected stack frame is called."));
+
+  add_com ("bookmark", class_bookmark, save_bookmark_command, _("\
+Set a bookmark in the program's execution history.\n\
+A bookmark represents a point in the execution history \n\
+that can be returned to at a later point in the debug session."));
+  add_info ("bookmarks", bookmarks_info, _("\
+Status of user-settable bookmarks.\n\
+Bookmarks are user-settable markers representing a point in the \n\
+execution history that can be returned to later in the same debug \n\
+session."));
+  add_cmd ("bookmark", class_bookmark, delete_bookmark_command, _("\
+Delete a bookmark from the bookmark list.\n"), &deletelist);
+  add_com ("goto-bookmark", class_bookmark, goto_bookmark_command, _("\
+Go to an earlier-bookmarked point in the program's execution history.\n\
+The bookmark must be saved earlier by using the 'bookmark' command."));
 }
Index: gdb/target.c
===================================================================
--- gdb.orig/target.c	2009-11-01 14:05:02.000000000 -0800
+++ gdb/target.c	2009-11-01 14:08:31.000000000 -0800
@@ -674,6 +674,8 @@
       INHERIT (to_async_mask, t);
       INHERIT (to_find_memory_regions, t);
       INHERIT (to_make_corefile_notes, t);
+      INHERIT (to_get_bookmark, t);
+      INHERIT (to_goto_bookmark, t);
       /* Do not inherit to_get_thread_local_address.  */
       INHERIT (to_can_execute_reverse, t);
       INHERIT (to_thread_architecture, t);
@@ -2767,6 +2769,21 @@
   return NULL;
 }
 
+/* Error-catcher for target_get_bookmark.  */
+static gdb_byte *
+dummy_get_bookmark (char *ignore1, int ignore2)
+{
+  tcomplain ();
+  return NULL;
+}
+
+/* Error-catcher for target_goto_bookmark.  */
+static void
+dummy_goto_bookmark (gdb_byte *ignore, int from_tty)
+{
+  tcomplain ();
+}
+
 /* Set up the handful of non-empty slots needed by the dummy target
    vector.  */
 
@@ -2787,6 +2804,8 @@
   dummy_target.to_stratum = dummy_stratum;
   dummy_target.to_find_memory_regions = dummy_find_memory_regions;
   dummy_target.to_make_corefile_notes = dummy_make_corefile_notes;
+  dummy_target.to_get_bookmark = dummy_get_bookmark;
+  dummy_target.to_goto_bookmark = dummy_goto_bookmark;
   dummy_target.to_xfer_partial = default_xfer_partial;
   dummy_target.to_has_all_memory = (int (*) (struct target_ops *)) return_zero;
   dummy_target.to_has_memory = (int (*) (struct target_ops *)) return_zero;
Index: gdb/target.h
===================================================================
--- gdb.orig/target.h	2009-11-01 14:05:02.000000000 -0800
+++ gdb/target.h	2009-11-01 14:08:31.000000000 -0800
@@ -459,13 +459,18 @@
     void (*to_async) (void (*) (enum inferior_event_type, void *), void *);
     int (*to_async_mask) (int);
     int (*to_supports_non_stop) (void);
+    /* find_memory_regions support method for gcore */
     int (*to_find_memory_regions) (int (*) (CORE_ADDR,
 					    unsigned long,
 					    int, int, int,
 					    void *),
 				   void *);
+    /* make_corefile_notes support method for gcore */
     char * (*to_make_corefile_notes) (bfd *, int *);
-
+    /* get_bookmark support method for bookmarks */
+    gdb_byte * (*to_get_bookmark) (char *, int);
+    /* goto_bookmark support method for bookmarks */
+    void (*to_goto_bookmark) (gdb_byte *, int);
     /* Return the thread-local address at OFFSET in the
        thread-local storage for the thread PTID and the shared library
        or executable file given by OBJFILE.  If that block of
@@ -1141,6 +1146,13 @@
 #define target_make_corefile_notes(BFD, SIZE_P) \
      (current_target.to_make_corefile_notes) (BFD, SIZE_P)
 
+/* Bookmark interfaces.  */
+#define target_get_bookmark(ARGS, FROM_TTY) \
+     (current_target.to_get_bookmark) (ARGS, FROM_TTY)
+
+#define target_goto_bookmark(ARG, FROM_TTY) \
+     (current_target.to_goto_bookmark) (ARG, FROM_TTY)
+
 /* Hardware watchpoint interfaces.  */
 
 /* Returns non-zero if we were stopped by a hardware watchpoint (memory read or
Index: gdb/breakpoint.c
===================================================================
--- gdb.orig/breakpoint.c	2009-11-01 14:05:02.000000000 -0800
+++ gdb/breakpoint.c	2009-11-01 16:26:37.000000000 -0800
@@ -10086,6 +10086,8 @@
 Convenience variable \"$bpnum\" contains the number of the last\n\
 breakpoint set."));
 
+  add_info_alias ("b", "breakpoints", 1);
+
   if (xdb_commands)
     add_com ("lb", class_breakpoint, breakpoints_info, _("\
 Status of user-settable breakpoints, or breakpoint number NUMBER.\n\
Index: gdb/NEWS
===================================================================
--- gdb.orig/NEWS	2009-11-01 14:05:02.000000000 -0800
+++ gdb/NEWS	2009-11-01 16:26:37.000000000 -0800
@@ -24,8 +24,25 @@
 
 * New commands (for set/show, see "New options" below)
 
+bookmark
+  Save a reference to the current position in the execution log (for
+  replay targets such as process record/replay).  This execution
+  position can then be returned to later with the 'goto-bookmark'
+  command.
+
+goto-bookmark [<N>]
+  Return the replay target to the excution position represented by the
+  Nth bookmark.  Resume replay debugging from that point.
+
+info bookmarks
+  List the bookmarks presently known to gdb.  Each bookmark can be
+  returned to using the 'goto-bookmark' command.
+
+delete bookmark [<N>]
+  Delete the Nth bookmark, or all bookmarks.
+
 record save [<FILENAME>]
-  Save a file (in core file format) containing the process record 
+  Save a file (in core file format) containing the process record
   execution log for replay debugging at a later time.
 
 record restore <FILENAME>
Index: gdb/command.h
===================================================================
--- gdb.orig/command.h	2009-11-01 14:05:02.000000000 -0800
+++ gdb/command.h	2009-11-01 14:08:31.000000000 -0800
@@ -32,8 +32,8 @@
   /* Classes of commands */
   no_class = -1, class_run = 0, class_vars, class_stack,
   class_files, class_support, class_info, class_breakpoint, class_trace,
-  class_alias, class_obscure, class_user, class_maintenance,
-  class_pseudo, class_tui, class_xdb
+  class_bookmark, class_alias, class_obscure, class_user,
+  class_maintenance, class_pseudo, class_tui, class_xdb
 };
 
 /* FIXME: cagney/2002-03-17: Once cmd_type() has been removed, ``enum

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFA] replay bookmarks
  2009-11-02  0:29   ` Michael Snyder
@ 2009-11-02  4:01     ` Eli Zaretskii
  2009-11-03  8:34       ` Jakob Engblom
  2009-11-03  8:35     ` Hui Zhu
  2009-11-09 17:55     ` Tom Tromey
  2 siblings, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2009-11-02  4:01 UTC (permalink / raw)
  To: Michael Snyder; +Cc: gdb-patches, glaw, jakob, teawater

> Date: Sun, 01 Nov 2009 16:29:29 -0800
> From: Michael Snyder <msnyder@vmware.com>
> CC: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>, 
>  "glaw@undo-software.com" <glaw@undo-software.com>,
>  "jakob@virtutech.com" <jakob@virtutech.com>, 
>  "teawater@gmail.com" <teawater@gmail.com>
> 
> Eli Zaretskii wrote:
> >> Date: Sun, 01 Nov 2009 13:13:24 -0800
> >> From: Michael Snyder <msnyder@vmware.com>
> >>
> >> +  add_com ("bookmark", class_bookmark, save_bookmark_command, _("\
> >> +Set a bookmark.\n\
> > 
> > This is way too short.  Suggest to expand a bit:
> > 
> >   Set a bookmark in the inferior's execution history
> > 
> >> +  add_com ("goto-bookmark", class_bookmark, goto_bookmark_command, _("\
> >> +Go to bookmark <n>\n\
> > 
> > Similarly here: such a short description is hardly useful, as it just
> > repeats the name of the command.
> 
> How about this?

Fine with me.

There are a few instances of user messages where the text is not
marked by _().

Also, we will need an update for the manual, once this is approved.

Thanks.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: [RFA] replay bookmarks
  2009-11-02  4:01     ` Eli Zaretskii
@ 2009-11-03  8:34       ` Jakob Engblom
  2009-11-03  8:36         ` Hui Zhu
  2009-11-03 18:42         ` Michael Snyder
  0 siblings, 2 replies; 23+ messages in thread
From: Jakob Engblom @ 2009-11-03  8:34 UTC (permalink / raw)
  To: 'Eli Zaretskii', 'Michael Snyder'
  Cc: gdb-patches, glaw, teawater

> > >> Date: Sun, 01 Nov 2009 13:13:24 -0800
> > >> From: Michael Snyder <msnyder@vmware.com>
> > >>
> > >> +  add_com ("bookmark", class_bookmark, save_bookmark_command, _("\
> > >> +Set a bookmark.\n\
> > >
> > > This is way too short.  Suggest to expand a bit:
> > >
> > >   Set a bookmark in the inferior's execution history
> > >
> > >> +  add_com ("goto-bookmark", class_bookmark, goto_bookmark_command, _("\
> > >> +Go to bookmark <n>\n\
> > >
> > > Similarly here: such a short description is hardly useful, as it just
> > > repeats the name of the command.
> >
> > How about this?
> 
> Fine with me.
> 
> There are a few instances of user messages where the text is not
> marked by _().
> 
> Also, we will need an update for the manual, once this is approved.

Also, will this add corresponding operations to gdb-serial? That is necessary to
get this working for undodb, Simics, and similar remote-exec bookmarkable
targets.  I guess that also means VMWare, right?

/jakob


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFA] replay bookmarks
  2009-11-02  0:29   ` Michael Snyder
  2009-11-02  4:01     ` Eli Zaretskii
@ 2009-11-03  8:35     ` Hui Zhu
  2009-11-05 18:13       ` Michael Snyder
  2009-11-09 17:55     ` Tom Tromey
  2 siblings, 1 reply; 23+ messages in thread
From: Hui Zhu @ 2009-11-03  8:35 UTC (permalink / raw)
  To: Michael Snyder; +Cc: Eli Zaretskii, gdb-patches, glaw, jakob

I try this patch in i386 ubuntu.  It works very good.

Could you add some works in cmd help to talk about "begin" and "end"?

Thanks,
Hui

On Mon, Nov 2, 2009 at 08:29, Michael Snyder <msnyder@vmware.com> wrote:
> Eli Zaretskii wrote:
>>>
>>> Date: Sun, 01 Nov 2009 13:13:24 -0800
>>> From: Michael Snyder <msnyder@vmware.com>
>>>
>>> +  add_com ("bookmark", class_bookmark, save_bookmark_command, _("\
>>> +Set a bookmark.\n\
>>
>> This is way too short.  Suggest to expand a bit:
>>
>>  Set a bookmark in the inferior's execution history
>>
>>> +  add_com ("goto-bookmark", class_bookmark, goto_bookmark_command, _("\
>>> +Go to bookmark <n>\n\
>>
>> Similarly here: such a short description is hardly useful, as it just
>> repeats the name of the command.
>
> How about this?
>
>
>
> 2009-10-25  Michael Snyder  <msnyder@vmware.com>
>
>        * target.h (struct target_ops): New methods to_get_bookmark
>        and to_goto_bookmark.
>        (target_get_bookmark): New macro.
>        (target_goto_bookmark): New macro.
>        * target.c (dummy_get_bookmark): New function, default
> implementation.
>        (dummy_goto_bookmark): New function, default implementation.
>        (update_current_target): Inherit new methods.
>        * record.c (record_get_bookmark): New function.
>        (record_goto_bookmark): New function.
>        (init_record_ops): Set to_get_bookmark and to_goto_bookmark methods.
>        * reverse.c (struct bookmark): New type.
>        (save_bookmark_command): New function (command).
>        (delete_bookmark_command): New function (command).
>        (goto_bookmark_command): New function (command).
>        (bookmarks_info): New function (command).
>        (_initialize_reverse): Add new bookmark commands.
>        * remote.c (remote_get_bookmark): New target method.
>        (remote_goto_bookmark): New target method.
>        * command.h (enum command_class): Add class_bookmark.
>        * NEWS: Mention bookmark commands.
>
> Index: gdb/record.c
> ===================================================================
> --- gdb.orig/record.c   2009-11-01 14:05:02.000000000 -0800
> +++ gdb/record.c        2009-11-01 16:26:37.000000000 -0800
> @@ -1523,6 +1523,57 @@
>   return 1;
>  }
>
> +/* "to_get_bookmark" method for process record and prec over core.  */
> +
> +static gdb_byte *
> +record_get_bookmark (char *args, int from_tty)
> +{
> +  gdb_byte *ret = NULL;
> +
> +  /* Return stringified form of instruction count.  */
> +  if (record_list && record_list->type == record_end)
> +    ret = xstrdup (pulongest (record_list->u.end.insn_num));
> +
> +  if (record_debug)
> +    {
> +      if (ret)
> +       fprintf_unfiltered (gdb_stdlog,
> +                           "record_get_bookmark returns %s\n", ret);
> +      else
> +       fprintf_unfiltered (gdb_stdlog,
> +                           "record_get_bookmark returns NULL\n");
> +    }
> +  return ret;
> +}
> +
> +/* The implementation of the command "record goto".  */
> +static void cmd_record_goto (char *, int);
> +
> +/* "to_goto_bookmark" method for process record and prec over core.  */
> +
> +static void
> +record_goto_bookmark (gdb_byte *bookmark, int from_tty)
> +{
> +  if (record_debug)
> +    fprintf_unfiltered (gdb_stdlog,
> +                       "record_goto_bookmark receives %s\n", bookmark);
> +
> +  if (bookmark[0] == '\'' || bookmark[0] == '\"')
> +    {
> +      if (bookmark[strlen (bookmark) - 1] != bookmark[0])
> +       error (_("Unbalanced quotes: %s"), bookmark);
> +
> +      /* Strip trailing quote.  */
> +      bookmark[strlen (bookmark) - 1] = '\0';
> +      /* Strip leading quote.  */
> +      bookmark++;
> +      /* Pass along to cmd_record_goto.  */
> +    }
> +
> +  cmd_record_goto ((char *) bookmark, from_tty);
> +  return;
> +}
> +
>  static void
>  init_record_ops (void)
>  {
> @@ -1545,6 +1596,9 @@
>   record_ops.to_remove_breakpoint = record_remove_breakpoint;
>   record_ops.to_can_execute_reverse = record_can_execute_reverse;
>   record_ops.to_stratum = record_stratum;
> +  /* Add bookmark target methods.  */
> +  record_ops.to_get_bookmark = record_get_bookmark;
> +  record_ops.to_goto_bookmark = record_goto_bookmark;
>   record_ops.to_magic = OPS_MAGIC;
>  }
>
> @@ -1750,6 +1804,9 @@
>   record_core_ops.to_can_execute_reverse = record_can_execute_reverse;
>   record_core_ops.to_has_execution = record_core_has_execution;
>   record_core_ops.to_stratum = record_stratum;
> +  /* Add bookmark target methods.  */
> +  record_core_ops.to_get_bookmark = record_get_bookmark;
> +  record_core_ops.to_goto_bookmark = record_goto_bookmark;
>   record_core_ops.to_magic = OPS_MAGIC;
>  }
>
> @@ -2407,6 +2464,99 @@
>                   recfilename);
>  }
>
> +/* record_goto_insn -- rewind the record log (forward or backward,
> +   depending on DIR) to the given entry, changing the program state
> +   correspondingly.  */
> +
> +static void
> +record_goto_insn (struct record_entry *entry,
> +                 enum exec_direction_kind dir)
> +{
> +  struct cleanup *set_cleanups = record_gdb_operation_disable_set ();
> +  struct regcache *regcache = get_current_regcache ();
> +  struct gdbarch *gdbarch = get_regcache_arch (regcache);
> +
> +  /* Assume everything is valid: we will hit the entry,
> +     and we will not hit the end of the recording.  */
> +
> +  if (dir == EXEC_FORWARD)
> +    record_list = record_list->next;
> +
> +  do
> +    {
> +      record_exec_insn (regcache, gdbarch, record_list);
> +      if (dir == EXEC_REVERSE)
> +       record_list = record_list->prev;
> +      else
> +       record_list = record_list->next;
> +    } while (record_list != entry);
> +  do_cleanups (set_cleanups);
> +}
> +
> +/* "record goto" command.  Argument is an instruction number,
> +   as given by "info record".
> +
> +   Rewinds the recording (forward or backward) to the given instruction.
>  */
> +
> +static void
> +cmd_record_goto (char *arg, int from_tty)
> +{
> +  struct record_entry *p = NULL;
> +  ULONGEST target_insn = 0;
> +
> +  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;
> +    }
> +  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;
> +    }
> +  else
> +    {
> +      /* General case.  Find designated insn.  */
> +      target_insn = parse_and_eval_long (arg);
> +
> +      for (p = &record_first; p != NULL; p = p->next)
> +       if (p->type == record_end && p->u.end.insn_num == target_insn)
> +         break;
> +    }
> +
> +  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 %d\n", (int)
> target_insn);
> +      record_goto_insn (p, EXEC_FORWARD);
> +    }
> +  else
> +    {
> +      printf_filtered ("Go backward to insn number %d\n", (int)
> target_insn);
> +      record_goto_insn (p, EXEC_REVERSE);
> +    }
> +  registers_changed ();
> +  reinit_frame_cache ();
> +  print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC);
> +}
> +
>  void
>  _initialize_record (void)
>  {
> @@ -2492,4 +2642,9 @@
>  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);
>  }
> Index: gdb/remote.c
> ===================================================================
> --- gdb.orig/remote.c   2009-11-01 14:05:02.000000000 -0800
> +++ gdb/remote.c        2009-11-01 14:08:31.000000000 -0800
> @@ -8882,6 +8882,46 @@
>   return rs->cond_tracepoints;
>  }
>
> +/* "to_get_bookmark" target method.
> +
> +   Return a string from the target that uniquely identifies the
> +   current machine state, such that the target will be able to
> +   return to this machine state at a later request.
> +
> +   Only expected to work with record/replay targets.  */
> +
> +static gdb_byte *
> +remote_get_bookmark (char *args, int from_tty)
> +{
> +  struct remote_state *rs = get_remote_state ();
> +
> +  putpkt ("qBookmark");
> +  getpkt (&rs->buf, &rs->buf_size, 0);
> +  if (rs->buf[0] == 'Q' && rs->buf[1] == 'B')
> +    return &rs->buf[2];
> +  else
> +    return NULL;
> +}
> +
> +/* "to_goto_bookmark" target method.
> +
> +   Restore the target to an earlier-recorded machine state.  */
> +
> +static void
> +remote_goto_bookmark (gdb_byte *bookmark, int from_tty)
> +{
> +  struct remote_state *rs = get_remote_state ();
> +  char *p = rs->buf;
> +
> +  xsnprintf (rs->buf, rs->buf_size, "QBookmark:%s", bookmark);
> +
> +  putpkt (p);
> +  getpkt (&rs->buf, &rs->buf_size, 0);
> +  if (rs->buf[0] != 'O' && rs->buf[1] != 'K')
> +    error (_("remote goto bookmark not implemented."));
> +}
> +
> +
>  static void
>  init_remote_ops (void)
>  {
> @@ -8931,7 +8971,6 @@
>   remote_ops.to_has_execution = default_child_has_execution;
>   remote_ops.to_has_thread_control = tc_schedlock;     /* can lock scheduler
> */
>   remote_ops.to_can_execute_reverse = remote_can_execute_reverse;
> -  remote_ops.to_magic = OPS_MAGIC;
>   remote_ops.to_memory_map = remote_memory_map;
>   remote_ops.to_flash_erase = remote_flash_erase;
>   remote_ops.to_flash_done = remote_flash_done;
> @@ -8945,6 +8984,9 @@
>   remote_ops.to_terminal_ours = remote_terminal_ours;
>   remote_ops.to_supports_non_stop = remote_supports_non_stop;
>   remote_ops.to_supports_multi_process = remote_supports_multi_process;
> +  remote_ops.to_get_bookmark = remote_get_bookmark;
> +  remote_ops.to_goto_bookmark = remote_goto_bookmark;
> +  remote_ops.to_magic = OPS_MAGIC;
>  }
>
>  /* Set up the extended remote vector by making a copy of the standard
> Index: gdb/reverse.c
> ===================================================================
> --- gdb.orig/reverse.c  2009-11-01 14:05:02.000000000 -0800
> +++ gdb/reverse.c       2009-11-01 16:28:40.000000000 -0800
> @@ -24,6 +24,7 @@
>  #include "cli/cli-cmds.h"
>  #include "cli/cli-decode.h"
>  #include "inferior.h"
> +#include "regcache.h"
>
>  /* User interface:
>    reverse-step, reverse-next etc.  */
> @@ -101,6 +102,221 @@
>   exec_reverse_once ("finish", args, from_tty);
>  }
>
> +/* Data structures for a bookmark list.  */
> +
> +struct bookmark {
> +  struct bookmark *next;
> +  int number;
> +  CORE_ADDR pc;
> +  struct symtab_and_line sal;
> +  gdb_byte *opaque_data;
> +};
> +
> +struct bookmark *bookmark_chain;
> +int bookmark_count;
> +
> +#define ALL_BOOKMARKS(B) for ((B) = bookmark_chain; (B); (B) = (B)->next)
> +
> +#define ALL_BOOKMARKS_SAFE(B,TMP)           \
> +     for ((B) = bookmark_chain;             \
> +          (B) ? ((TMP) = (B)->next, 1) : 0; \
> +          (B) = (TMP))
> +
> +/* save_bookmark_command -- implement "bookmark" command.
> +   Call target method to get a bookmark identifier.
> +   Insert bookmark identifier into list.
> +
> +   Identifier will be a malloc string (gdb_byte *).
> +   Up to us to free it as required.  */
> +
> +static void
> +save_bookmark_command (char *args, int from_tty)
> +{
> +  /* Get target's idea of a bookmark.  */
> +  gdb_byte *bookmark_id = target_get_bookmark (args, from_tty);
> +  struct bookmark *b, *b1;
> +  struct gdbarch *gdbarch = get_regcache_arch (get_current_regcache ());
> +
> +  /* CR should not cause another identical bookmark.  */
> +  dont_repeat ();
> +
> +  if (bookmark_id == NULL)
> +    error (_("target_get_bookmark failed."));
> +
> +  /* Set up a bookmark struct.  */
> +  b = xcalloc (1, sizeof (struct bookmark));
> +  b->number = ++bookmark_count;
> +  init_sal (&b->sal);
> +  b->pc = regcache_read_pc (get_current_regcache ());
> +  b->sal = find_pc_line (b->pc, 0);
> +  b->sal.pspace = get_frame_program_space (get_current_frame ());
> +  b->opaque_data = bookmark_id;
> +  b->next = NULL;
> +
> +  /* Add this bookmark to the end of the chain, so that a list
> +     of bookmarks will come out in order of increasing numbers.  */
> +
> +  b1 = bookmark_chain;
> +  if (b1 == 0)
> +    bookmark_chain = b;
> +  else
> +    {
> +      while (b1->next)
> +       b1 = b1->next;
> +      b1->next = b;
> +    }
> +  printf_filtered (_("Saved bookmark %d at %s\n"), b->number,
> +                    paddress (gdbarch, b->sal.pc));
> +}
> +
> +/* Implement "delete bookmark" command.  */
> +
> +static int
> +delete_one_bookmark (struct bookmark *b)
> +{
> +  struct bookmark *b1;
> +
> +  /* Special case, first item in list.  */
> +  if (b == bookmark_chain)
> +    bookmark_chain = b->next;
> +
> +  /* Find bookmark preceeding "marked" one, so we can unlink.  */
> +  /* FIXME what about end cases (first and last)?  */
> +  if (b)
> +    {
> +      ALL_BOOKMARKS (b1)
> +       if (b1->next == b)
> +         {
> +           /* Found designated bookmark.  Unlink and delete.  */
> +           b1->next = b->next;
> +           break;
> +         }
> +      xfree (b->opaque_data);
> +      xfree (b);
> +      return 1;                /* success */
> +    }
> +  return 0;            /* failure */
> +}
> +
> +static void
> +delete_all_bookmarks (void)
> +{
> +  struct bookmark *b, *b1;
> +
> +  ALL_BOOKMARKS_SAFE (b, b1)
> +    {
> +      xfree (b->opaque_data);
> +      xfree (b);
> +    }
> +  bookmark_chain = NULL;
> +}
> +
> +static void
> +delete_bookmark_command (char *args, int from_tty)
> +{
> +  struct bookmark *b, *b1;
> +  unsigned long num;
> +
> +  if (bookmark_chain == NULL)
> +    {
> +      warning ("No bookmarks.");
> +      return;
> +    }
> +
> +  if (args == NULL || args[0] == '\0')
> +    {
> +      if (from_tty && !query (_("Delete all bookmarks? ")))
> +       return;
> +      delete_all_bookmarks ();
> +      return;
> +    }
> +
> +  num = strtoul (args, NULL, 0);
> +  /* Find bookmark with corresponding number.  */
> +  ALL_BOOKMARKS (b)
> +    if (b->number == num)
> +      break;
> +
> +  if (!delete_one_bookmark (b))
> +    /* Not found.  */
> +    error (_("delete bookmark: no bookmark found for '%s'."), args);
> +}
> +
> +/* Implement "goto-bookmark" command.  */
> +
> +static void
> +goto_bookmark_command (char *args, int from_tty)
> +{
> +  struct bookmark *b;
> +  unsigned long num;
> +
> +  if (args == NULL || args[0] == '\0')
> +    error (_("Command requires an argument."));
> +
> +  if (strncmp (args, "start", strlen ("start")) == 0 ||
> +      strncmp (args, "begin", strlen ("begin")) == 0 ||
> +      strncmp (args, "end",   strlen ("end")) == 0)
> +    {
> +      /* Special case.  Give target opportunity to handle.  */
> +      target_goto_bookmark (args, from_tty);
> +      return;
> +    }
> +
> +  if (args[0] == '\'' || args[0] == '\"')
> +    {
> +      /* Special case -- quoted string.  Pass on to target.  */
> +      if (args[strlen (args) - 1] != args[0])
> +       error (_("Unbalanced quotes: %s"), args);
> +      target_goto_bookmark (args, from_tty);
> +      return;
> +    }
> +
> +  /* General case.  Bookmark identified by bookmark number.  */
> +  num = strtoul (args, NULL, 0);
> +  ALL_BOOKMARKS (b)
> +    if (b->number == num)
> +      break;
> +
> +  if (b)
> +    {
> +      /* Found.  Send to target method.  */
> +      target_goto_bookmark (b->opaque_data, from_tty);
> +      return;
> +    }
> +  /* Not found.  */
> +  error (_("goto-bookmark: no bookmark found for '%s'."), args);
> +}
> +
> +/* Implement "info bookmarks" command.  */
> +
> +static void
> +bookmarks_info (char *args, int from_tty)
> +{
> +  struct bookmark *b;
> +  int bnum = -1;
> +  struct gdbarch *gdbarch;
> +
> +  if (args)
> +    bnum = parse_and_eval_long (args);
> +
> +  if (!bookmark_chain)
> +    {
> +      printf_filtered (_("No bookmarks.\n"));
> +      return;
> +    }
> +
> +  gdbarch = get_regcache_arch (get_current_regcache ());
> +  printf_filtered (_("Bookmark    Address     Opaque\n"));
> +  printf_filtered (_("   ID                    Data \n"));
> +
> +  ALL_BOOKMARKS (b)
> +    printf_filtered ("   %d       %s    '%s'\n",
> +                    b->number,
> +                    paddress (gdbarch, b->pc),
> +                    b->opaque_data);
> +}
> +
> +
>  /* Provide a prototype to silence -Wmissing-prototypes.  */
>  extern initialize_file_ftype _initialize_reverse;
>
> @@ -142,4 +358,19 @@
>
>   add_com ("reverse-finish", class_run, reverse_finish, _("\
>  Execute backward until just before selected stack frame is called."));
> +
> +  add_com ("bookmark", class_bookmark, save_bookmark_command, _("\
> +Set a bookmark in the program's execution history.\n\
> +A bookmark represents a point in the execution history \n\
> +that can be returned to at a later point in the debug session."));
> +  add_info ("bookmarks", bookmarks_info, _("\
> +Status of user-settable bookmarks.\n\
> +Bookmarks are user-settable markers representing a point in the \n\
> +execution history that can be returned to later in the same debug \n\
> +session."));
> +  add_cmd ("bookmark", class_bookmark, delete_bookmark_command, _("\
> +Delete a bookmark from the bookmark list.\n"), &deletelist);
> +  add_com ("goto-bookmark", class_bookmark, goto_bookmark_command, _("\
> +Go to an earlier-bookmarked point in the program's execution history.\n\
> +The bookmark must be saved earlier by using the 'bookmark' command."));
>  }
> Index: gdb/target.c
> ===================================================================
> --- gdb.orig/target.c   2009-11-01 14:05:02.000000000 -0800
> +++ gdb/target.c        2009-11-01 14:08:31.000000000 -0800
> @@ -674,6 +674,8 @@
>       INHERIT (to_async_mask, t);
>       INHERIT (to_find_memory_regions, t);
>       INHERIT (to_make_corefile_notes, t);
> +      INHERIT (to_get_bookmark, t);
> +      INHERIT (to_goto_bookmark, t);
>       /* Do not inherit to_get_thread_local_address.  */
>       INHERIT (to_can_execute_reverse, t);
>       INHERIT (to_thread_architecture, t);
> @@ -2767,6 +2769,21 @@
>   return NULL;
>  }
>
> +/* Error-catcher for target_get_bookmark.  */
> +static gdb_byte *
> +dummy_get_bookmark (char *ignore1, int ignore2)
> +{
> +  tcomplain ();
> +  return NULL;
> +}
> +
> +/* Error-catcher for target_goto_bookmark.  */
> +static void
> +dummy_goto_bookmark (gdb_byte *ignore, int from_tty)
> +{
> +  tcomplain ();
> +}
> +
>  /* Set up the handful of non-empty slots needed by the dummy target
>    vector.  */
>
> @@ -2787,6 +2804,8 @@
>   dummy_target.to_stratum = dummy_stratum;
>   dummy_target.to_find_memory_regions = dummy_find_memory_regions;
>   dummy_target.to_make_corefile_notes = dummy_make_corefile_notes;
> +  dummy_target.to_get_bookmark = dummy_get_bookmark;
> +  dummy_target.to_goto_bookmark = dummy_goto_bookmark;
>   dummy_target.to_xfer_partial = default_xfer_partial;
>   dummy_target.to_has_all_memory = (int (*) (struct target_ops *))
> return_zero;
>   dummy_target.to_has_memory = (int (*) (struct target_ops *)) return_zero;
> Index: gdb/target.h
> ===================================================================
> --- gdb.orig/target.h   2009-11-01 14:05:02.000000000 -0800
> +++ gdb/target.h        2009-11-01 14:08:31.000000000 -0800
> @@ -459,13 +459,18 @@
>     void (*to_async) (void (*) (enum inferior_event_type, void *), void *);
>     int (*to_async_mask) (int);
>     int (*to_supports_non_stop) (void);
> +    /* find_memory_regions support method for gcore */
>     int (*to_find_memory_regions) (int (*) (CORE_ADDR,
>                                            unsigned long,
>                                            int, int, int,
>                                            void *),
>                                   void *);
> +    /* make_corefile_notes support method for gcore */
>     char * (*to_make_corefile_notes) (bfd *, int *);
> -
> +    /* get_bookmark support method for bookmarks */
> +    gdb_byte * (*to_get_bookmark) (char *, int);
> +    /* goto_bookmark support method for bookmarks */
> +    void (*to_goto_bookmark) (gdb_byte *, int);
>     /* Return the thread-local address at OFFSET in the
>        thread-local storage for the thread PTID and the shared library
>        or executable file given by OBJFILE.  If that block of
> @@ -1141,6 +1146,13 @@
>  #define target_make_corefile_notes(BFD, SIZE_P) \
>      (current_target.to_make_corefile_notes) (BFD, SIZE_P)
>
> +/* Bookmark interfaces.  */
> +#define target_get_bookmark(ARGS, FROM_TTY) \
> +     (current_target.to_get_bookmark) (ARGS, FROM_TTY)
> +
> +#define target_goto_bookmark(ARG, FROM_TTY) \
> +     (current_target.to_goto_bookmark) (ARG, FROM_TTY)
> +
>  /* Hardware watchpoint interfaces.  */
>
>  /* Returns non-zero if we were stopped by a hardware watchpoint (memory
> read or
> Index: gdb/breakpoint.c
> ===================================================================
> --- gdb.orig/breakpoint.c       2009-11-01 14:05:02.000000000 -0800
> +++ gdb/breakpoint.c    2009-11-01 16:26:37.000000000 -0800
> @@ -10086,6 +10086,8 @@
>  Convenience variable \"$bpnum\" contains the number of the last\n\
>  breakpoint set."));
>
> +  add_info_alias ("b", "breakpoints", 1);
> +
>   if (xdb_commands)
>     add_com ("lb", class_breakpoint, breakpoints_info, _("\
>  Status of user-settable breakpoints, or breakpoint number NUMBER.\n\
> Index: gdb/NEWS
> ===================================================================
> --- gdb.orig/NEWS       2009-11-01 14:05:02.000000000 -0800
> +++ gdb/NEWS    2009-11-01 16:26:37.000000000 -0800
> @@ -24,8 +24,25 @@
>
>  * New commands (for set/show, see "New options" below)
>
> +bookmark
> +  Save a reference to the current position in the execution log (for
> +  replay targets such as process record/replay).  This execution
> +  position can then be returned to later with the 'goto-bookmark'
> +  command.
> +
> +goto-bookmark [<N>]
> +  Return the replay target to the excution position represented by the
> +  Nth bookmark.  Resume replay debugging from that point.
> +
> +info bookmarks
> +  List the bookmarks presently known to gdb.  Each bookmark can be
> +  returned to using the 'goto-bookmark' command.
> +
> +delete bookmark [<N>]
> +  Delete the Nth bookmark, or all bookmarks.
> +
>  record save [<FILENAME>]
> -  Save a file (in core file format) containing the process record
> +  Save a file (in core file format) containing the process record
>   execution log for replay debugging at a later time.
>
>  record restore <FILENAME>
> Index: gdb/command.h
> ===================================================================
> --- gdb.orig/command.h  2009-11-01 14:05:02.000000000 -0800
> +++ gdb/command.h       2009-11-01 14:08:31.000000000 -0800
> @@ -32,8 +32,8 @@
>   /* Classes of commands */
>   no_class = -1, class_run = 0, class_vars, class_stack,
>   class_files, class_support, class_info, class_breakpoint, class_trace,
> -  class_alias, class_obscure, class_user, class_maintenance,
> -  class_pseudo, class_tui, class_xdb
> +  class_bookmark, class_alias, class_obscure, class_user,
> +  class_maintenance, class_pseudo, class_tui, class_xdb
>  };
>
>  /* FIXME: cagney/2002-03-17: Once cmd_type() has been removed, ``enum
>
>


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFA] replay bookmarks
  2009-11-03  8:34       ` Jakob Engblom
@ 2009-11-03  8:36         ` Hui Zhu
  2009-11-03 18:42         ` Michael Snyder
  1 sibling, 0 replies; 23+ messages in thread
From: Hui Zhu @ 2009-11-03  8:36 UTC (permalink / raw)
  To: Jakob Engblom; +Cc: Eli Zaretskii, Michael Snyder, gdb-patches, glaw

On Tue, Nov 3, 2009 at 16:34, Jakob Engblom <jakob@virtutech.com> wrote:
>> > >> Date: Sun, 01 Nov 2009 13:13:24 -0800
>> > >> From: Michael Snyder <msnyder@vmware.com>
>> > >>
>> > >> +  add_com ("bookmark", class_bookmark, save_bookmark_command, _("\
>> > >> +Set a bookmark.\n\
>> > >
>> > > This is way too short.  Suggest to expand a bit:
>> > >
>> > >   Set a bookmark in the inferior's execution history
>> > >
>> > >> +  add_com ("goto-bookmark", class_bookmark, goto_bookmark_command, _("\
>> > >> +Go to bookmark <n>\n\
>> > >
>> > > Similarly here: such a short description is hardly useful, as it just
>> > > repeats the name of the command.
>> >
>> > How about this?
>>
>> Fine with me.
>>
>> There are a few instances of user messages where the text is not
>> marked by _().
>>
>> Also, we will need an update for the manual, once this is approved.
>
> Also, will this add corresponding operations to gdb-serial? That is necessary to
> get this working for undodb, Simics, and similar remote-exec bookmarkable
> targets.  I guess that also means VMWare, right?
>

       * remote.c (remote_get_bookmark): New target method.
       (remote_goto_bookmark): New target method.

Is it you looking for?

Thanks,
Hui


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFA] replay bookmarks
  2009-11-03  8:34       ` Jakob Engblom
  2009-11-03  8:36         ` Hui Zhu
@ 2009-11-03 18:42         ` Michael Snyder
  2009-11-05 11:05           ` Jakob Engblom
  1 sibling, 1 reply; 23+ messages in thread
From: Michael Snyder @ 2009-11-03 18:42 UTC (permalink / raw)
  To: Jakob Engblom; +Cc: 'Eli Zaretskii', gdb-patches, glaw, teawater

Jakob Engblom wrote:
>>>>> Date: Sun, 01 Nov 2009 13:13:24 -0800
>>>>> From: Michael Snyder <msnyder@vmware.com>
>>>>>
>>>>> +  add_com ("bookmark", class_bookmark, save_bookmark_command, _("\
>>>>> +Set a bookmark.\n\
>>>> This is way too short.  Suggest to expand a bit:
>>>>
>>>>   Set a bookmark in the inferior's execution history
>>>>
>>>>> +  add_com ("goto-bookmark", class_bookmark, goto_bookmark_command, _("\
>>>>> +Go to bookmark <n>\n\
>>>> Similarly here: such a short description is hardly useful, as it just
>>>> repeats the name of the command.
>>> How about this?
>> Fine with me.
>>
>> There are a few instances of user messages where the text is not
>> marked by _().
>>
>> Also, we will need an update for the manual, once this is approved.
> 
> Also, will this add corresponding operations to gdb-serial? That is necessary to
> get this working for undodb, Simics, and similar remote-exec bookmarkable
> targets.  I guess that also means VMWare, right?

Right.  But I haven't tested the remote serial bits, so
any feedback that you have will be helpful.  This is the
implementation of the protocol question that I asked you
earlier.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: [RFA] replay bookmarks
  2009-11-03 18:42         ` Michael Snyder
@ 2009-11-05 11:05           ` Jakob Engblom
  0 siblings, 0 replies; 23+ messages in thread
From: Jakob Engblom @ 2009-11-05 11:05 UTC (permalink / raw)
  To: 'Michael Snyder'
  Cc: 'Eli Zaretskii', gdb-patches, glaw, teawater

> Right.  But I haven't tested the remote serial bits, so
> any feedback that you have will be helpful.  This is the
> implementation of the protocol question that I asked you
> earlier.

Thanks for the clarification on what was what. 

I am currently mostly on parental leave with my nine-months-old daughter, so
don't expect too much feedback from me personally over the next quarter or so.
Sorry that I acnnot be of more help, as this is really interesting stuff.

/jakob


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFA] replay bookmarks
  2009-11-03  8:35     ` Hui Zhu
@ 2009-11-05 18:13       ` Michael Snyder
  2009-11-05 18:44         ` Pedro Alves
  2009-11-05 19:17         ` Eli Zaretskii
  0 siblings, 2 replies; 23+ messages in thread
From: Michael Snyder @ 2009-11-05 18:13 UTC (permalink / raw)
  To: Hui Zhu; +Cc: Eli Zaretskii, gdb-patches, glaw, jakob

[-- Attachment #1: Type: text/plain, Size: 200 bytes --]

Hui Zhu wrote:
> I try this patch in i386 ubuntu.  It works very good.
> 
> Could you add some works in cmd help to talk about "begin" and "end"?

Added, plus a few more _()'s, per comment from Eli.


[-- Attachment #2: bookmark.txt --]
[-- Type: text/plain, Size: 21515 bytes --]

2009-10-25  Michael Snyder  <msnyder@vmware.com>

        * target.h (struct target_ops): New methods to_get_bookmark
        and to_goto_bookmark.
        (target_get_bookmark): New macro.
        (target_goto_bookmark): New macro.
        * target.c (dummy_get_bookmark): New function, default implementation.
        (dummy_goto_bookmark): New function, default implementation.
        (update_current_target): Inherit new methods.
        * record.c (record_get_bookmark): New function.
        (record_goto_bookmark): New function.
        (init_record_ops): Set to_get_bookmark and to_goto_bookmark methods.
        * reverse.c (struct bookmark): New type.
        (save_bookmark_command): New function (command).
        (delete_bookmark_command): New function (command).
        (goto_bookmark_command): New function (command).
        (bookmarks_info): New function (command).
        (_initialize_reverse): Add new bookmark commands.
        * remote.c (remote_get_bookmark): New target method.
        (remote_goto_bookmark): New target method.
	* command.h (enum command_class): Add class_bookmark.
	* NEWS: Mention bookmark commands.

Index: gdb/record.c
===================================================================
--- gdb.orig/record.c	2009-11-05 09:49:11.000000000 -0800
+++ gdb/record.c	2009-11-05 09:54:36.000000000 -0800
@@ -1523,6 +1523,57 @@
   return 1;
 }
 
+/* "to_get_bookmark" method for process record and prec over core.  */
+
+static gdb_byte *
+record_get_bookmark (char *args, int from_tty)
+{
+  gdb_byte *ret = NULL;
+
+  /* Return stringified form of instruction count.  */
+  if (record_list && record_list->type == record_end)
+    ret = xstrdup (pulongest (record_list->u.end.insn_num));
+
+  if (record_debug)
+    {
+      if (ret)
+	fprintf_unfiltered (gdb_stdlog,
+			    "record_get_bookmark returns %s\n", ret);
+      else
+	fprintf_unfiltered (gdb_stdlog,
+			    "record_get_bookmark returns NULL\n");
+    }
+  return ret;
+}
+
+/* The implementation of the command "record goto".  */
+static void cmd_record_goto (char *, int);
+
+/* "to_goto_bookmark" method for process record and prec over core.  */
+
+static void
+record_goto_bookmark (gdb_byte *bookmark, int from_tty)
+{
+  if (record_debug)
+    fprintf_unfiltered (gdb_stdlog,
+			"record_goto_bookmark receives %s\n", bookmark);
+
+  if (bookmark[0] == '\'' || bookmark[0] == '\"')
+    {
+      if (bookmark[strlen (bookmark) - 1] != bookmark[0])
+	error (_("Unbalanced quotes: %s"), bookmark);
+
+      /* Strip trailing quote.  */
+      bookmark[strlen (bookmark) - 1] = '\0';
+      /* Strip leading quote.  */
+      bookmark++;
+      /* Pass along to cmd_record_goto.  */
+    }
+
+  cmd_record_goto ((char *) bookmark, from_tty);
+  return;
+}
+
 static void
 init_record_ops (void)
 {
@@ -1545,6 +1596,9 @@
   record_ops.to_remove_breakpoint = record_remove_breakpoint;
   record_ops.to_can_execute_reverse = record_can_execute_reverse;
   record_ops.to_stratum = record_stratum;
+  /* Add bookmark target methods.  */
+  record_ops.to_get_bookmark = record_get_bookmark;
+  record_ops.to_goto_bookmark = record_goto_bookmark;
   record_ops.to_magic = OPS_MAGIC;
 }
 
@@ -1750,6 +1804,9 @@
   record_core_ops.to_can_execute_reverse = record_can_execute_reverse;
   record_core_ops.to_has_execution = record_core_has_execution;
   record_core_ops.to_stratum = record_stratum;
+  /* Add bookmark target methods.  */
+  record_core_ops.to_get_bookmark = record_get_bookmark;
+  record_core_ops.to_goto_bookmark = record_goto_bookmark;
   record_core_ops.to_magic = OPS_MAGIC;
 }
 
@@ -2407,6 +2464,101 @@
 		   recfilename);
 }
 
+/* record_goto_insn -- rewind the record log (forward or backward,
+   depending on DIR) to the given entry, changing the program state
+   correspondingly.  */
+
+static void
+record_goto_insn (struct record_entry *entry,
+		  enum exec_direction_kind dir)
+{
+  struct cleanup *set_cleanups = record_gdb_operation_disable_set ();
+  struct regcache *regcache = get_current_regcache ();
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+
+  /* Assume everything is valid: we will hit the entry,
+     and we will not hit the end of the recording.  */
+
+  if (dir == EXEC_FORWARD)
+    record_list = record_list->next;
+
+  do
+    {
+      record_exec_insn (regcache, gdbarch, record_list);
+      if (dir == EXEC_REVERSE)
+	record_list = record_list->prev;
+      else
+	record_list = record_list->next;
+    } while (record_list != entry);
+  do_cleanups (set_cleanups);
+}
+
+/* "record goto" command.  Argument is an instruction number,
+   as given by "info record".
+
+   Rewinds the recording (forward or backward) to the given instruction.  */
+
+static void
+cmd_record_goto (char *arg, int from_tty)
+{
+  struct record_entry *p = NULL;
+  ULONGEST target_insn = 0;
+
+  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;
+    }
+  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;
+    }
+  else
+    {
+      /* General case.  Find designated insn.  */
+      target_insn = parse_and_eval_long (arg);
+
+      for (p = &record_first; p != NULL; p = p->next)
+	if (p->type == record_end && p->u.end.insn_num == target_insn)
+	  break;
+    }
+
+  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);
+}
+
 void
 _initialize_record (void)
 {
@@ -2492,4 +2644,9 @@
 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);
 }
Index: gdb/remote.c
===================================================================
--- gdb.orig/remote.c	2009-11-05 09:49:11.000000000 -0800
+++ gdb/remote.c	2009-11-05 09:49:30.000000000 -0800
@@ -8882,6 +8882,46 @@
   return rs->cond_tracepoints;
 }
 
+/* "to_get_bookmark" target method.
+
+   Return a string from the target that uniquely identifies the
+   current machine state, such that the target will be able to
+   return to this machine state at a later request.
+
+   Only expected to work with record/replay targets.  */
+
+static gdb_byte *
+remote_get_bookmark (char *args, int from_tty)
+{
+  struct remote_state *rs = get_remote_state ();
+
+  putpkt ("qBookmark");
+  getpkt (&rs->buf, &rs->buf_size, 0);
+  if (rs->buf[0] == 'Q' && rs->buf[1] == 'B')
+    return &rs->buf[2];
+  else
+    return NULL;
+}
+
+/* "to_goto_bookmark" target method.
+
+   Restore the target to an earlier-recorded machine state.  */
+
+static void
+remote_goto_bookmark (gdb_byte *bookmark, int from_tty)
+{
+  struct remote_state *rs = get_remote_state ();
+  char *p = rs->buf;
+
+  xsnprintf (rs->buf, rs->buf_size, "QBookmark:%s", bookmark);
+
+  putpkt (p);
+  getpkt (&rs->buf, &rs->buf_size, 0);
+  if (rs->buf[0] != 'O' && rs->buf[1] != 'K')
+    error (_("remote goto bookmark not implemented."));
+}
+
+
 static void
 init_remote_ops (void)
 {
@@ -8931,7 +8971,6 @@
   remote_ops.to_has_execution = default_child_has_execution;
   remote_ops.to_has_thread_control = tc_schedlock;	/* can lock scheduler */
   remote_ops.to_can_execute_reverse = remote_can_execute_reverse;
-  remote_ops.to_magic = OPS_MAGIC;
   remote_ops.to_memory_map = remote_memory_map;
   remote_ops.to_flash_erase = remote_flash_erase;
   remote_ops.to_flash_done = remote_flash_done;
@@ -8945,6 +8984,9 @@
   remote_ops.to_terminal_ours = remote_terminal_ours;
   remote_ops.to_supports_non_stop = remote_supports_non_stop;
   remote_ops.to_supports_multi_process = remote_supports_multi_process;
+  remote_ops.to_get_bookmark = remote_get_bookmark;
+  remote_ops.to_goto_bookmark = remote_goto_bookmark;
+  remote_ops.to_magic = OPS_MAGIC;
 }
 
 /* Set up the extended remote vector by making a copy of the standard
Index: gdb/reverse.c
===================================================================
--- gdb.orig/reverse.c	2009-11-05 09:49:11.000000000 -0800
+++ gdb/reverse.c	2009-11-05 09:56:58.000000000 -0800
@@ -24,6 +24,7 @@
 #include "cli/cli-cmds.h"
 #include "cli/cli-decode.h"
 #include "inferior.h"
+#include "regcache.h"
 
 /* User interface:
    reverse-step, reverse-next etc.  */
@@ -101,6 +102,221 @@
   exec_reverse_once ("finish", args, from_tty);
 }
 
+/* Data structures for a bookmark list.  */
+
+struct bookmark {
+  struct bookmark *next;
+  int number;
+  CORE_ADDR pc;
+  struct symtab_and_line sal;
+  gdb_byte *opaque_data;
+};
+
+struct bookmark *bookmark_chain;
+int bookmark_count;
+
+#define ALL_BOOKMARKS(B) for ((B) = bookmark_chain; (B); (B) = (B)->next)
+
+#define ALL_BOOKMARKS_SAFE(B,TMP)           \
+     for ((B) = bookmark_chain;             \
+          (B) ? ((TMP) = (B)->next, 1) : 0; \
+          (B) = (TMP))
+
+/* save_bookmark_command -- implement "bookmark" command.
+   Call target method to get a bookmark identifier.
+   Insert bookmark identifier into list.
+
+   Identifier will be a malloc string (gdb_byte *).
+   Up to us to free it as required.  */
+
+static void
+save_bookmark_command (char *args, int from_tty)
+{
+  /* Get target's idea of a bookmark.  */
+  gdb_byte *bookmark_id = target_get_bookmark (args, from_tty);
+  struct bookmark *b, *b1;
+  struct gdbarch *gdbarch = get_regcache_arch (get_current_regcache ());
+
+  /* CR should not cause another identical bookmark.  */
+  dont_repeat ();
+
+  if (bookmark_id == NULL)
+    error (_("target_get_bookmark failed."));
+
+  /* Set up a bookmark struct.  */
+  b = xcalloc (1, sizeof (struct bookmark));
+  b->number = ++bookmark_count;
+  init_sal (&b->sal);
+  b->pc = regcache_read_pc (get_current_regcache ());
+  b->sal = find_pc_line (b->pc, 0);
+  b->sal.pspace = get_frame_program_space (get_current_frame ());
+  b->opaque_data = bookmark_id;
+  b->next = NULL;
+
+  /* Add this bookmark to the end of the chain, so that a list
+     of bookmarks will come out in order of increasing numbers.  */
+
+  b1 = bookmark_chain;
+  if (b1 == 0)
+    bookmark_chain = b;
+  else
+    {
+      while (b1->next)
+	b1 = b1->next;
+      b1->next = b;
+    }
+  printf_filtered (_("Saved bookmark %d at %s\n"), b->number,
+		     paddress (gdbarch, b->sal.pc));
+}
+
+/* Implement "delete bookmark" command.  */
+
+static int
+delete_one_bookmark (struct bookmark *b)
+{
+  struct bookmark *b1;
+
+  /* Special case, first item in list.  */
+  if (b == bookmark_chain)
+    bookmark_chain = b->next;
+
+  /* Find bookmark preceeding "marked" one, so we can unlink.  */
+  /* FIXME what about end cases (first and last)?  */
+  if (b)
+    {
+      ALL_BOOKMARKS (b1)
+	if (b1->next == b)
+	  {
+	    /* Found designated bookmark.  Unlink and delete.  */
+	    b1->next = b->next;
+	    break;
+	  }
+      xfree (b->opaque_data);
+      xfree (b);
+      return 1;		/* success */
+    }
+  return 0;		/* failure */
+}
+
+static void
+delete_all_bookmarks (void)
+{
+  struct bookmark *b, *b1;
+
+  ALL_BOOKMARKS_SAFE (b, b1)
+    {
+      xfree (b->opaque_data);
+      xfree (b);
+    }
+  bookmark_chain = NULL;
+}
+
+static void
+delete_bookmark_command (char *args, int from_tty)
+{
+  struct bookmark *b, *b1;
+  unsigned long num;
+
+  if (bookmark_chain == NULL)
+    {
+      warning (_("No bookmarks."));
+      return;
+    }
+
+  if (args == NULL || args[0] == '\0')
+    {
+      if (from_tty && !query (_("Delete all bookmarks? ")))
+	return;
+      delete_all_bookmarks ();
+      return;
+    }
+
+  num = strtoul (args, NULL, 0);
+  /* Find bookmark with corresponding number.  */
+  ALL_BOOKMARKS (b)
+    if (b->number == num)
+      break;
+
+  if (!delete_one_bookmark (b))
+    /* Not found.  */
+    error (_("delete bookmark: no bookmark found for '%s'."), args);
+}
+
+/* Implement "goto-bookmark" command.  */
+
+static void
+goto_bookmark_command (char *args, int from_tty)
+{
+  struct bookmark *b;
+  unsigned long num;
+
+  if (args == NULL || args[0] == '\0')
+    error (_("Command requires an argument."));
+
+  if (strncmp (args, "start", strlen ("start")) == 0 ||
+      strncmp (args, "begin", strlen ("begin")) == 0 ||
+      strncmp (args, "end",   strlen ("end")) == 0)
+    {
+      /* Special case.  Give target opportunity to handle.  */
+      target_goto_bookmark (args, from_tty);
+      return;
+    }
+
+  if (args[0] == '\'' || args[0] == '\"')
+    {
+      /* Special case -- quoted string.  Pass on to target.  */
+      if (args[strlen (args) - 1] != args[0])
+	error (_("Unbalanced quotes: %s"), args);
+      target_goto_bookmark (args, from_tty);
+      return;
+    }
+
+  /* General case.  Bookmark identified by bookmark number.  */
+  num = strtoul (args, NULL, 0);
+  ALL_BOOKMARKS (b)
+    if (b->number == num)
+      break;
+
+  if (b)
+    {
+      /* Found.  Send to target method.  */
+      target_goto_bookmark (b->opaque_data, from_tty);
+      return;
+    }
+  /* Not found.  */
+  error (_("goto-bookmark: no bookmark found for '%s'."), args);
+}
+
+/* Implement "info bookmarks" command.  */
+
+static void
+bookmarks_info (char *args, int from_tty)
+{
+  struct bookmark *b;
+  int bnum = -1;
+  struct gdbarch *gdbarch;
+
+  if (args)
+    bnum = parse_and_eval_long (args);
+
+  if (!bookmark_chain)
+    {
+      printf_filtered (_("No bookmarks.\n"));
+      return;
+    }
+
+  gdbarch = get_regcache_arch (get_current_regcache ());
+  printf_filtered (_("Bookmark    Address     Opaque\n"));
+  printf_filtered (_("   ID                    Data \n"));
+
+  ALL_BOOKMARKS (b)
+    printf_filtered ("   %d       %s    '%s'\n",
+		     b->number,
+		     paddress (gdbarch, b->pc),
+		     b->opaque_data);
+}
+
+
 /* Provide a prototype to silence -Wmissing-prototypes.  */
 extern initialize_file_ftype _initialize_reverse;
 
@@ -142,4 +358,22 @@
 
   add_com ("reverse-finish", class_run, reverse_finish, _("\
 Execute backward until just before selected stack frame is called."));
+
+  add_com ("bookmark", class_bookmark, save_bookmark_command, _("\
+Set a bookmark in the program's execution history.\n\
+A bookmark represents a point in the execution history \n\
+that can be returned to at a later point in the debug session."));
+  add_info ("bookmarks", bookmarks_info, _("\
+Status of user-settable bookmarks.\n\
+Bookmarks are user-settable markers representing a point in the \n\
+execution history that can be returned to later in the same debug \n\
+session."));
+  add_cmd ("bookmark", class_bookmark, delete_bookmark_command, _("\
+Delete a bookmark from the bookmark list.\n"), &deletelist);
+  add_com ("goto-bookmark", class_bookmark, goto_bookmark_command, _("\
+Go to an earlier-bookmarked point in the program's execution history.\n\
+Argument is a bookmark saved earlier by using the 'bookmark' command,\n\
+or the special arguments:\n\
+  start (beginning of recording)\n\
+  end   (end of recording)\n"));
 }
Index: gdb/target.c
===================================================================
--- gdb.orig/target.c	2009-11-05 09:49:11.000000000 -0800
+++ gdb/target.c	2009-11-05 09:49:30.000000000 -0800
@@ -674,6 +674,8 @@
       INHERIT (to_async_mask, t);
       INHERIT (to_find_memory_regions, t);
       INHERIT (to_make_corefile_notes, t);
+      INHERIT (to_get_bookmark, t);
+      INHERIT (to_goto_bookmark, t);
       /* Do not inherit to_get_thread_local_address.  */
       INHERIT (to_can_execute_reverse, t);
       INHERIT (to_thread_architecture, t);
@@ -2767,6 +2769,21 @@
   return NULL;
 }
 
+/* Error-catcher for target_get_bookmark.  */
+static gdb_byte *
+dummy_get_bookmark (char *ignore1, int ignore2)
+{
+  tcomplain ();
+  return NULL;
+}
+
+/* Error-catcher for target_goto_bookmark.  */
+static void
+dummy_goto_bookmark (gdb_byte *ignore, int from_tty)
+{
+  tcomplain ();
+}
+
 /* Set up the handful of non-empty slots needed by the dummy target
    vector.  */
 
@@ -2787,6 +2804,8 @@
   dummy_target.to_stratum = dummy_stratum;
   dummy_target.to_find_memory_regions = dummy_find_memory_regions;
   dummy_target.to_make_corefile_notes = dummy_make_corefile_notes;
+  dummy_target.to_get_bookmark = dummy_get_bookmark;
+  dummy_target.to_goto_bookmark = dummy_goto_bookmark;
   dummy_target.to_xfer_partial = default_xfer_partial;
   dummy_target.to_has_all_memory = (int (*) (struct target_ops *)) return_zero;
   dummy_target.to_has_memory = (int (*) (struct target_ops *)) return_zero;
Index: gdb/target.h
===================================================================
--- gdb.orig/target.h	2009-11-05 09:49:11.000000000 -0800
+++ gdb/target.h	2009-11-05 09:49:30.000000000 -0800
@@ -459,13 +459,18 @@
     void (*to_async) (void (*) (enum inferior_event_type, void *), void *);
     int (*to_async_mask) (int);
     int (*to_supports_non_stop) (void);
+    /* find_memory_regions support method for gcore */
     int (*to_find_memory_regions) (int (*) (CORE_ADDR,
 					    unsigned long,
 					    int, int, int,
 					    void *),
 				   void *);
+    /* make_corefile_notes support method for gcore */
     char * (*to_make_corefile_notes) (bfd *, int *);
-
+    /* get_bookmark support method for bookmarks */
+    gdb_byte * (*to_get_bookmark) (char *, int);
+    /* goto_bookmark support method for bookmarks */
+    void (*to_goto_bookmark) (gdb_byte *, int);
     /* Return the thread-local address at OFFSET in the
        thread-local storage for the thread PTID and the shared library
        or executable file given by OBJFILE.  If that block of
@@ -1141,6 +1146,13 @@
 #define target_make_corefile_notes(BFD, SIZE_P) \
      (current_target.to_make_corefile_notes) (BFD, SIZE_P)
 
+/* Bookmark interfaces.  */
+#define target_get_bookmark(ARGS, FROM_TTY) \
+     (current_target.to_get_bookmark) (ARGS, FROM_TTY)
+
+#define target_goto_bookmark(ARG, FROM_TTY) \
+     (current_target.to_goto_bookmark) (ARG, FROM_TTY)
+
 /* Hardware watchpoint interfaces.  */
 
 /* Returns non-zero if we were stopped by a hardware watchpoint (memory read or
Index: gdb/breakpoint.c
===================================================================
--- gdb.orig/breakpoint.c	2009-11-05 09:49:11.000000000 -0800
+++ gdb/breakpoint.c	2009-11-05 09:54:36.000000000 -0800
@@ -10086,6 +10086,8 @@
 Convenience variable \"$bpnum\" contains the number of the last\n\
 breakpoint set."));
 
+  add_info_alias ("b", "breakpoints", 1);
+
   if (xdb_commands)
     add_com ("lb", class_breakpoint, breakpoints_info, _("\
 Status of user-settable breakpoints, or breakpoint number NUMBER.\n\
Index: gdb/NEWS
===================================================================
--- gdb.orig/NEWS	2009-11-05 09:49:11.000000000 -0800
+++ gdb/NEWS	2009-11-05 09:54:36.000000000 -0800
@@ -24,8 +24,25 @@
 
 * New commands (for set/show, see "New options" below)
 
+bookmark
+  Save a reference to the current position in the execution log (for
+  replay targets such as process record/replay).  This execution
+  position can then be returned to later with the 'goto-bookmark'
+  command.
+
+goto-bookmark [<N>]
+  Return the replay target to the excution position represented by the
+  Nth bookmark.  Resume replay debugging from that point.
+
+info bookmarks
+  List the bookmarks presently known to gdb.  Each bookmark can be
+  returned to using the 'goto-bookmark' command.
+
+delete bookmark [<N>]
+  Delete the Nth bookmark, or all bookmarks.
+
 record save [<FILENAME>]
-  Save a file (in core file format) containing the process record 
+  Save a file (in core file format) containing the process record
   execution log for replay debugging at a later time.
 
 record restore <FILENAME>
Index: gdb/command.h
===================================================================
--- gdb.orig/command.h	2009-11-05 09:49:11.000000000 -0800
+++ gdb/command.h	2009-11-05 09:49:30.000000000 -0800
@@ -32,8 +32,8 @@
   /* Classes of commands */
   no_class = -1, class_run = 0, class_vars, class_stack,
   class_files, class_support, class_info, class_breakpoint, class_trace,
-  class_alias, class_obscure, class_user, class_maintenance,
-  class_pseudo, class_tui, class_xdb
+  class_bookmark, class_alias, class_obscure, class_user,
+  class_maintenance, class_pseudo, class_tui, class_xdb
 };
 
 /* FIXME: cagney/2002-03-17: Once cmd_type() has been removed, ``enum

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFA] replay bookmarks
  2009-11-05 18:13       ` Michael Snyder
@ 2009-11-05 18:44         ` Pedro Alves
  2009-11-05 19:17         ` Eli Zaretskii
  1 sibling, 0 replies; 23+ messages in thread
From: Pedro Alves @ 2009-11-05 18:44 UTC (permalink / raw)
  To: gdb-patches; +Cc: Michael Snyder, Hui Zhu, Eli Zaretskii, glaw, jakob

On Thursday 05 November 2009 18:13:09, Michael Snyder wrote:
> Hui Zhu wrote:
> > I try this patch in i386 ubuntu.  It works very good.
> > 
> > Could you add some works in cmd help to talk about "begin" and "end"?
> 
> Added, plus a few more _()'s, per comment from Eli.

Quick comments:

- any chance you could use packet_ok?

- '||'s at beginning of line, not at end.

- You should be bin2hex/hex2bin'ing the strings that are
  being xfered over the wire.

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFA] replay bookmarks
  2009-11-05 18:13       ` Michael Snyder
  2009-11-05 18:44         ` Pedro Alves
@ 2009-11-05 19:17         ` Eli Zaretskii
  2009-11-05 23:40           ` Michael Snyder
  1 sibling, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2009-11-05 19:17 UTC (permalink / raw)
  To: Michael Snyder; +Cc: teawater, gdb-patches, glaw, jakob

> Date: Thu, 05 Nov 2009 10:13:09 -0800
> From: Michael Snyder <msnyder@vmware.com>
> CC: Eli Zaretskii <eliz@gnu.org>,   "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>,  "glaw@undo-software.com" <glaw@undo-software.com>,   "jakob@virtutech.com" <jakob@virtutech.com>
> 
> > Could you add some works in cmd help to talk about "begin" and "end"?
> 
> Added, plus a few more _()'s, per comment from Eli.

Thanks.  One more nit:

> +  add_com ("goto-bookmark", class_bookmark, goto_bookmark_command, _("\
> +Go to an earlier-bookmarked point in the program's execution history.\n\
> +Argument is a bookmark saved earlier by using the 'bookmark' command,\n\
> +or the special arguments:\n\
> +  start (beginning of recording)\n\
> +  end   (end of recording)\n"));

This says nothing about what kind of argument it expects.  Is it a
number? a name (i.e. string)? something else?


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFA] replay bookmarks
  2009-11-05 19:17         ` Eli Zaretskii
@ 2009-11-05 23:40           ` Michael Snyder
  2009-11-06  8:34             ` Eli Zaretskii
  0 siblings, 1 reply; 23+ messages in thread
From: Michael Snyder @ 2009-11-05 23:40 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: teawater, gdb-patches, glaw, jakob

[-- Attachment #1: Type: text/plain, Size: 946 bytes --]

Eli Zaretskii wrote:
>> Date: Thu, 05 Nov 2009 10:13:09 -0800
>> From: Michael Snyder <msnyder@vmware.com>
>> CC: Eli Zaretskii <eliz@gnu.org>,   "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>,  "glaw@undo-software.com" <glaw@undo-software.com>,   "jakob@virtutech.com" <jakob@virtutech.com>
>>
>>> Could you add some works in cmd help to talk about "begin" and "end"?
>> Added, plus a few more _()'s, per comment from Eli.
> 
> Thanks.  One more nit:
> 
>> +  add_com ("goto-bookmark", class_bookmark, goto_bookmark_command, _("\
>> +Go to an earlier-bookmarked point in the program's execution history.\n\
>> +Argument is a bookmark saved earlier by using the 'bookmark' command,\n\
>> +or the special arguments:\n\
>> +  start (beginning of recording)\n\
>> +  end   (end of recording)\n"));
> 
> This says nothing about what kind of argument it expects.  Is it a
> number? a name (i.e. string)? something else?

Thanks.  Better?



[-- Attachment #2: bookmark.txt --]
[-- Type: text/plain, Size: 21620 bytes --]

2009-10-25  Michael Snyder  <msnyder@vmware.com>

        * target.h (struct target_ops): New methods to_get_bookmark
        and to_goto_bookmark.
        (target_get_bookmark): New macro.
        (target_goto_bookmark): New macro.
        * target.c (dummy_get_bookmark): New function, default implementation.
        (dummy_goto_bookmark): New function, default implementation.
        (update_current_target): Inherit new methods.
        * record.c (record_get_bookmark): New function.
        (record_goto_bookmark): New function.
        (init_record_ops): Set to_get_bookmark and to_goto_bookmark methods.
        * reverse.c (struct bookmark): New type.
        (save_bookmark_command): New function (command).
        (delete_bookmark_command): New function (command).
        (goto_bookmark_command): New function (command).
        (bookmarks_info): New function (command).
        (_initialize_reverse): Add new bookmark commands.
        * remote.c (remote_get_bookmark): New target method.
        (remote_goto_bookmark): New target method.
	* command.h (enum command_class): Add class_bookmark.
	* NEWS: Mention bookmark commands.

Index: gdb/record.c
===================================================================
--- gdb.orig/record.c	2009-11-05 09:49:11.000000000 -0800
+++ gdb/record.c	2009-11-05 15:36:33.000000000 -0800
@@ -1523,6 +1523,57 @@
   return 1;
 }
 
+/* "to_get_bookmark" method for process record and prec over core.  */
+
+static gdb_byte *
+record_get_bookmark (char *args, int from_tty)
+{
+  gdb_byte *ret = NULL;
+
+  /* Return stringified form of instruction count.  */
+  if (record_list && record_list->type == record_end)
+    ret = xstrdup (pulongest (record_list->u.end.insn_num));
+
+  if (record_debug)
+    {
+      if (ret)
+	fprintf_unfiltered (gdb_stdlog,
+			    "record_get_bookmark returns %s\n", ret);
+      else
+	fprintf_unfiltered (gdb_stdlog,
+			    "record_get_bookmark returns NULL\n");
+    }
+  return ret;
+}
+
+/* The implementation of the command "record goto".  */
+static void cmd_record_goto (char *, int);
+
+/* "to_goto_bookmark" method for process record and prec over core.  */
+
+static void
+record_goto_bookmark (gdb_byte *bookmark, int from_tty)
+{
+  if (record_debug)
+    fprintf_unfiltered (gdb_stdlog,
+			"record_goto_bookmark receives %s\n", bookmark);
+
+  if (bookmark[0] == '\'' || bookmark[0] == '\"')
+    {
+      if (bookmark[strlen (bookmark) - 1] != bookmark[0])
+	error (_("Unbalanced quotes: %s"), bookmark);
+
+      /* Strip trailing quote.  */
+      bookmark[strlen (bookmark) - 1] = '\0';
+      /* Strip leading quote.  */
+      bookmark++;
+      /* Pass along to cmd_record_goto.  */
+    }
+
+  cmd_record_goto ((char *) bookmark, from_tty);
+  return;
+}
+
 static void
 init_record_ops (void)
 {
@@ -1545,6 +1596,9 @@
   record_ops.to_remove_breakpoint = record_remove_breakpoint;
   record_ops.to_can_execute_reverse = record_can_execute_reverse;
   record_ops.to_stratum = record_stratum;
+  /* Add bookmark target methods.  */
+  record_ops.to_get_bookmark = record_get_bookmark;
+  record_ops.to_goto_bookmark = record_goto_bookmark;
   record_ops.to_magic = OPS_MAGIC;
 }
 
@@ -1750,6 +1804,9 @@
   record_core_ops.to_can_execute_reverse = record_can_execute_reverse;
   record_core_ops.to_has_execution = record_core_has_execution;
   record_core_ops.to_stratum = record_stratum;
+  /* Add bookmark target methods.  */
+  record_core_ops.to_get_bookmark = record_get_bookmark;
+  record_core_ops.to_goto_bookmark = record_goto_bookmark;
   record_core_ops.to_magic = OPS_MAGIC;
 }
 
@@ -2407,6 +2464,101 @@
 		   recfilename);
 }
 
+/* record_goto_insn -- rewind the record log (forward or backward,
+   depending on DIR) to the given entry, changing the program state
+   correspondingly.  */
+
+static void
+record_goto_insn (struct record_entry *entry,
+		  enum exec_direction_kind dir)
+{
+  struct cleanup *set_cleanups = record_gdb_operation_disable_set ();
+  struct regcache *regcache = get_current_regcache ();
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+
+  /* Assume everything is valid: we will hit the entry,
+     and we will not hit the end of the recording.  */
+
+  if (dir == EXEC_FORWARD)
+    record_list = record_list->next;
+
+  do
+    {
+      record_exec_insn (regcache, gdbarch, record_list);
+      if (dir == EXEC_REVERSE)
+	record_list = record_list->prev;
+      else
+	record_list = record_list->next;
+    } while (record_list != entry);
+  do_cleanups (set_cleanups);
+}
+
+/* "record goto" command.  Argument is an instruction number,
+   as given by "info record".
+
+   Rewinds the recording (forward or backward) to the given instruction.  */
+
+static void
+cmd_record_goto (char *arg, int from_tty)
+{
+  struct record_entry *p = NULL;
+  ULONGEST target_insn = 0;
+
+  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;
+    }
+  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;
+    }
+  else
+    {
+      /* General case.  Find designated insn.  */
+      target_insn = parse_and_eval_long (arg);
+
+      for (p = &record_first; p != NULL; p = p->next)
+	if (p->type == record_end && p->u.end.insn_num == target_insn)
+	  break;
+    }
+
+  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);
+}
+
 void
 _initialize_record (void)
 {
@@ -2492,4 +2644,9 @@
 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);
 }
Index: gdb/remote.c
===================================================================
--- gdb.orig/remote.c	2009-11-05 09:49:11.000000000 -0800
+++ gdb/remote.c	2009-11-05 09:49:30.000000000 -0800
@@ -8882,6 +8882,46 @@
   return rs->cond_tracepoints;
 }
 
+/* "to_get_bookmark" target method.
+
+   Return a string from the target that uniquely identifies the
+   current machine state, such that the target will be able to
+   return to this machine state at a later request.
+
+   Only expected to work with record/replay targets.  */
+
+static gdb_byte *
+remote_get_bookmark (char *args, int from_tty)
+{
+  struct remote_state *rs = get_remote_state ();
+
+  putpkt ("qBookmark");
+  getpkt (&rs->buf, &rs->buf_size, 0);
+  if (rs->buf[0] == 'Q' && rs->buf[1] == 'B')
+    return &rs->buf[2];
+  else
+    return NULL;
+}
+
+/* "to_goto_bookmark" target method.
+
+   Restore the target to an earlier-recorded machine state.  */
+
+static void
+remote_goto_bookmark (gdb_byte *bookmark, int from_tty)
+{
+  struct remote_state *rs = get_remote_state ();
+  char *p = rs->buf;
+
+  xsnprintf (rs->buf, rs->buf_size, "QBookmark:%s", bookmark);
+
+  putpkt (p);
+  getpkt (&rs->buf, &rs->buf_size, 0);
+  if (rs->buf[0] != 'O' && rs->buf[1] != 'K')
+    error (_("remote goto bookmark not implemented."));
+}
+
+
 static void
 init_remote_ops (void)
 {
@@ -8931,7 +8971,6 @@
   remote_ops.to_has_execution = default_child_has_execution;
   remote_ops.to_has_thread_control = tc_schedlock;	/* can lock scheduler */
   remote_ops.to_can_execute_reverse = remote_can_execute_reverse;
-  remote_ops.to_magic = OPS_MAGIC;
   remote_ops.to_memory_map = remote_memory_map;
   remote_ops.to_flash_erase = remote_flash_erase;
   remote_ops.to_flash_done = remote_flash_done;
@@ -8945,6 +8984,9 @@
   remote_ops.to_terminal_ours = remote_terminal_ours;
   remote_ops.to_supports_non_stop = remote_supports_non_stop;
   remote_ops.to_supports_multi_process = remote_supports_multi_process;
+  remote_ops.to_get_bookmark = remote_get_bookmark;
+  remote_ops.to_goto_bookmark = remote_goto_bookmark;
+  remote_ops.to_magic = OPS_MAGIC;
 }
 
 /* Set up the extended remote vector by making a copy of the standard
Index: gdb/reverse.c
===================================================================
--- gdb.orig/reverse.c	2009-11-05 09:49:11.000000000 -0800
+++ gdb/reverse.c	2009-11-05 15:37:48.000000000 -0800
@@ -24,6 +24,7 @@
 #include "cli/cli-cmds.h"
 #include "cli/cli-decode.h"
 #include "inferior.h"
+#include "regcache.h"
 
 /* User interface:
    reverse-step, reverse-next etc.  */
@@ -101,6 +102,221 @@
   exec_reverse_once ("finish", args, from_tty);
 }
 
+/* Data structures for a bookmark list.  */
+
+struct bookmark {
+  struct bookmark *next;
+  int number;
+  CORE_ADDR pc;
+  struct symtab_and_line sal;
+  gdb_byte *opaque_data;
+};
+
+struct bookmark *bookmark_chain;
+int bookmark_count;
+
+#define ALL_BOOKMARKS(B) for ((B) = bookmark_chain; (B); (B) = (B)->next)
+
+#define ALL_BOOKMARKS_SAFE(B,TMP)           \
+     for ((B) = bookmark_chain;             \
+          (B) ? ((TMP) = (B)->next, 1) : 0; \
+          (B) = (TMP))
+
+/* save_bookmark_command -- implement "bookmark" command.
+   Call target method to get a bookmark identifier.
+   Insert bookmark identifier into list.
+
+   Identifier will be a malloc string (gdb_byte *).
+   Up to us to free it as required.  */
+
+static void
+save_bookmark_command (char *args, int from_tty)
+{
+  /* Get target's idea of a bookmark.  */
+  gdb_byte *bookmark_id = target_get_bookmark (args, from_tty);
+  struct bookmark *b, *b1;
+  struct gdbarch *gdbarch = get_regcache_arch (get_current_regcache ());
+
+  /* CR should not cause another identical bookmark.  */
+  dont_repeat ();
+
+  if (bookmark_id == NULL)
+    error (_("target_get_bookmark failed."));
+
+  /* Set up a bookmark struct.  */
+  b = xcalloc (1, sizeof (struct bookmark));
+  b->number = ++bookmark_count;
+  init_sal (&b->sal);
+  b->pc = regcache_read_pc (get_current_regcache ());
+  b->sal = find_pc_line (b->pc, 0);
+  b->sal.pspace = get_frame_program_space (get_current_frame ());
+  b->opaque_data = bookmark_id;
+  b->next = NULL;
+
+  /* Add this bookmark to the end of the chain, so that a list
+     of bookmarks will come out in order of increasing numbers.  */
+
+  b1 = bookmark_chain;
+  if (b1 == 0)
+    bookmark_chain = b;
+  else
+    {
+      while (b1->next)
+	b1 = b1->next;
+      b1->next = b;
+    }
+  printf_filtered (_("Saved bookmark %d at %s\n"), b->number,
+		     paddress (gdbarch, b->sal.pc));
+}
+
+/* Implement "delete bookmark" command.  */
+
+static int
+delete_one_bookmark (struct bookmark *b)
+{
+  struct bookmark *b1;
+
+  /* Special case, first item in list.  */
+  if (b == bookmark_chain)
+    bookmark_chain = b->next;
+
+  /* Find bookmark preceeding "marked" one, so we can unlink.  */
+  /* FIXME what about end cases (first and last)?  */
+  if (b)
+    {
+      ALL_BOOKMARKS (b1)
+	if (b1->next == b)
+	  {
+	    /* Found designated bookmark.  Unlink and delete.  */
+	    b1->next = b->next;
+	    break;
+	  }
+      xfree (b->opaque_data);
+      xfree (b);
+      return 1;		/* success */
+    }
+  return 0;		/* failure */
+}
+
+static void
+delete_all_bookmarks (void)
+{
+  struct bookmark *b, *b1;
+
+  ALL_BOOKMARKS_SAFE (b, b1)
+    {
+      xfree (b->opaque_data);
+      xfree (b);
+    }
+  bookmark_chain = NULL;
+}
+
+static void
+delete_bookmark_command (char *args, int from_tty)
+{
+  struct bookmark *b, *b1;
+  unsigned long num;
+
+  if (bookmark_chain == NULL)
+    {
+      warning (_("No bookmarks."));
+      return;
+    }
+
+  if (args == NULL || args[0] == '\0')
+    {
+      if (from_tty && !query (_("Delete all bookmarks? ")))
+	return;
+      delete_all_bookmarks ();
+      return;
+    }
+
+  num = strtoul (args, NULL, 0);
+  /* Find bookmark with corresponding number.  */
+  ALL_BOOKMARKS (b)
+    if (b->number == num)
+      break;
+
+  if (!delete_one_bookmark (b))
+    /* Not found.  */
+    error (_("delete bookmark: no bookmark found for '%s'."), args);
+}
+
+/* Implement "goto-bookmark" command.  */
+
+static void
+goto_bookmark_command (char *args, int from_tty)
+{
+  struct bookmark *b;
+  unsigned long num;
+
+  if (args == NULL || args[0] == '\0')
+    error (_("Command requires an argument."));
+
+  if (strncmp (args, "start", strlen ("start")) == 0
+      || strncmp (args, "begin", strlen ("begin")) == 0
+      || strncmp (args, "end",   strlen ("end")) == 0)
+    {
+      /* Special case.  Give target opportunity to handle.  */
+      target_goto_bookmark (args, from_tty);
+      return;
+    }
+
+  if (args[0] == '\'' || args[0] == '\"')
+    {
+      /* Special case -- quoted string.  Pass on to target.  */
+      if (args[strlen (args) - 1] != args[0])
+	error (_("Unbalanced quotes: %s"), args);
+      target_goto_bookmark (args, from_tty);
+      return;
+    }
+
+  /* General case.  Bookmark identified by bookmark number.  */
+  num = strtoul (args, NULL, 0);
+  ALL_BOOKMARKS (b)
+    if (b->number == num)
+      break;
+
+  if (b)
+    {
+      /* Found.  Send to target method.  */
+      target_goto_bookmark (b->opaque_data, from_tty);
+      return;
+    }
+  /* Not found.  */
+  error (_("goto-bookmark: no bookmark found for '%s'."), args);
+}
+
+/* Implement "info bookmarks" command.  */
+
+static void
+bookmarks_info (char *args, int from_tty)
+{
+  struct bookmark *b;
+  int bnum = -1;
+  struct gdbarch *gdbarch;
+
+  if (args)
+    bnum = parse_and_eval_long (args);
+
+  if (!bookmark_chain)
+    {
+      printf_filtered (_("No bookmarks.\n"));
+      return;
+    }
+
+  gdbarch = get_regcache_arch (get_current_regcache ());
+  printf_filtered (_("Bookmark    Address     Opaque\n"));
+  printf_filtered (_("   ID                    Data \n"));
+
+  ALL_BOOKMARKS (b)
+    printf_filtered ("   %d       %s    '%s'\n",
+		     b->number,
+		     paddress (gdbarch, b->pc),
+		     b->opaque_data);
+}
+
+
 /* Provide a prototype to silence -Wmissing-prototypes.  */
 extern initialize_file_ftype _initialize_reverse;
 
@@ -142,4 +358,24 @@
 
   add_com ("reverse-finish", class_run, reverse_finish, _("\
 Execute backward until just before selected stack frame is called."));
+
+  add_com ("bookmark", class_bookmark, save_bookmark_command, _("\
+Set a bookmark in the program's execution history.\n\
+A bookmark represents a point in the execution history \n\
+that can be returned to at a later point in the debug session."));
+  add_info ("bookmarks", bookmarks_info, _("\
+Status of user-settable bookmarks.\n\
+Bookmarks are user-settable markers representing a point in the \n\
+execution history that can be returned to later in the same debug \n\
+session."));
+  add_cmd ("bookmark", class_bookmark, delete_bookmark_command, _("\
+Delete a bookmark from the bookmark list.\n\
+Argument is a bookmark number, or no argument to delete all bookmarks.\n"), 
+	   &deletelist);
+  add_com ("goto-bookmark", class_bookmark, goto_bookmark_command, _("\
+Go to an earlier-bookmarked point in the program's execution history.\n\
+Argument is the bookmark number of a bookmark saved earlier by using \n\
+the 'bookmark' command, or the special arguments:\n\
+  start (beginning of recording)\n\
+  end   (end of recording)\n"));
 }
Index: gdb/target.c
===================================================================
--- gdb.orig/target.c	2009-11-05 09:49:11.000000000 -0800
+++ gdb/target.c	2009-11-05 09:49:30.000000000 -0800
@@ -674,6 +674,8 @@
       INHERIT (to_async_mask, t);
       INHERIT (to_find_memory_regions, t);
       INHERIT (to_make_corefile_notes, t);
+      INHERIT (to_get_bookmark, t);
+      INHERIT (to_goto_bookmark, t);
       /* Do not inherit to_get_thread_local_address.  */
       INHERIT (to_can_execute_reverse, t);
       INHERIT (to_thread_architecture, t);
@@ -2767,6 +2769,21 @@
   return NULL;
 }
 
+/* Error-catcher for target_get_bookmark.  */
+static gdb_byte *
+dummy_get_bookmark (char *ignore1, int ignore2)
+{
+  tcomplain ();
+  return NULL;
+}
+
+/* Error-catcher for target_goto_bookmark.  */
+static void
+dummy_goto_bookmark (gdb_byte *ignore, int from_tty)
+{
+  tcomplain ();
+}
+
 /* Set up the handful of non-empty slots needed by the dummy target
    vector.  */
 
@@ -2787,6 +2804,8 @@
   dummy_target.to_stratum = dummy_stratum;
   dummy_target.to_find_memory_regions = dummy_find_memory_regions;
   dummy_target.to_make_corefile_notes = dummy_make_corefile_notes;
+  dummy_target.to_get_bookmark = dummy_get_bookmark;
+  dummy_target.to_goto_bookmark = dummy_goto_bookmark;
   dummy_target.to_xfer_partial = default_xfer_partial;
   dummy_target.to_has_all_memory = (int (*) (struct target_ops *)) return_zero;
   dummy_target.to_has_memory = (int (*) (struct target_ops *)) return_zero;
Index: gdb/target.h
===================================================================
--- gdb.orig/target.h	2009-11-05 09:49:11.000000000 -0800
+++ gdb/target.h	2009-11-05 09:49:30.000000000 -0800
@@ -459,13 +459,18 @@
     void (*to_async) (void (*) (enum inferior_event_type, void *), void *);
     int (*to_async_mask) (int);
     int (*to_supports_non_stop) (void);
+    /* find_memory_regions support method for gcore */
     int (*to_find_memory_regions) (int (*) (CORE_ADDR,
 					    unsigned long,
 					    int, int, int,
 					    void *),
 				   void *);
+    /* make_corefile_notes support method for gcore */
     char * (*to_make_corefile_notes) (bfd *, int *);
-
+    /* get_bookmark support method for bookmarks */
+    gdb_byte * (*to_get_bookmark) (char *, int);
+    /* goto_bookmark support method for bookmarks */
+    void (*to_goto_bookmark) (gdb_byte *, int);
     /* Return the thread-local address at OFFSET in the
        thread-local storage for the thread PTID and the shared library
        or executable file given by OBJFILE.  If that block of
@@ -1141,6 +1146,13 @@
 #define target_make_corefile_notes(BFD, SIZE_P) \
      (current_target.to_make_corefile_notes) (BFD, SIZE_P)
 
+/* Bookmark interfaces.  */
+#define target_get_bookmark(ARGS, FROM_TTY) \
+     (current_target.to_get_bookmark) (ARGS, FROM_TTY)
+
+#define target_goto_bookmark(ARG, FROM_TTY) \
+     (current_target.to_goto_bookmark) (ARG, FROM_TTY)
+
 /* Hardware watchpoint interfaces.  */
 
 /* Returns non-zero if we were stopped by a hardware watchpoint (memory read or
Index: gdb/breakpoint.c
===================================================================
--- gdb.orig/breakpoint.c	2009-11-05 09:49:11.000000000 -0800
+++ gdb/breakpoint.c	2009-11-05 15:36:33.000000000 -0800
@@ -10086,6 +10086,8 @@
 Convenience variable \"$bpnum\" contains the number of the last\n\
 breakpoint set."));
 
+  add_info_alias ("b", "breakpoints", 1);
+
   if (xdb_commands)
     add_com ("lb", class_breakpoint, breakpoints_info, _("\
 Status of user-settable breakpoints, or breakpoint number NUMBER.\n\
Index: gdb/NEWS
===================================================================
--- gdb.orig/NEWS	2009-11-05 09:49:11.000000000 -0800
+++ gdb/NEWS	2009-11-05 15:36:33.000000000 -0800
@@ -24,8 +24,25 @@
 
 * New commands (for set/show, see "New options" below)
 
+bookmark
+  Save a reference to the current position in the execution log (for
+  replay targets such as process record/replay).  This execution
+  position can then be returned to later with the 'goto-bookmark'
+  command.
+
+goto-bookmark [<N>]
+  Return the replay target to the excution position represented by the
+  Nth bookmark.  Resume replay debugging from that point.
+
+info bookmarks
+  List the bookmarks presently known to gdb.  Each bookmark can be
+  returned to using the 'goto-bookmark' command.
+
+delete bookmark [<N>]
+  Delete the Nth bookmark, or all bookmarks.
+
 record save [<FILENAME>]
-  Save a file (in core file format) containing the process record 
+  Save a file (in core file format) containing the process record
   execution log for replay debugging at a later time.
 
 record restore <FILENAME>
Index: gdb/command.h
===================================================================
--- gdb.orig/command.h	2009-11-05 09:49:11.000000000 -0800
+++ gdb/command.h	2009-11-05 09:49:30.000000000 -0800
@@ -32,8 +32,8 @@
   /* Classes of commands */
   no_class = -1, class_run = 0, class_vars, class_stack,
   class_files, class_support, class_info, class_breakpoint, class_trace,
-  class_alias, class_obscure, class_user, class_maintenance,
-  class_pseudo, class_tui, class_xdb
+  class_bookmark, class_alias, class_obscure, class_user,
+  class_maintenance, class_pseudo, class_tui, class_xdb
 };
 
 /* FIXME: cagney/2002-03-17: Once cmd_type() has been removed, ``enum

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFA] replay bookmarks
  2009-11-05 23:40           ` Michael Snyder
@ 2009-11-06  8:34             ` Eli Zaretskii
  0 siblings, 0 replies; 23+ messages in thread
From: Eli Zaretskii @ 2009-11-06  8:34 UTC (permalink / raw)
  To: Michael Snyder; +Cc: teawater, gdb-patches, glaw, jakob

> Date: Thu, 05 Nov 2009 15:39:18 -0800
> From: Michael Snyder <msnyder@vmware.com>
> CC: "teawater@gmail.com" <teawater@gmail.com>, 
>  "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>,
>  "glaw@undo-software.com" <glaw@undo-software.com>, 
>  "jakob@virtutech.com" <jakob@virtutech.com>
> 
> >> +  add_com ("goto-bookmark", class_bookmark, goto_bookmark_command, _("\
> >> +Go to an earlier-bookmarked point in the program's execution history.\n\
> >> +Argument is a bookmark saved earlier by using the 'bookmark' command,\n\
> >> +or the special arguments:\n\
> >> +  start (beginning of recording)\n\
> >> +  end   (end of recording)\n"));
> > 
> > This says nothing about what kind of argument it expects.  Is it a
> > number? a name (i.e. string)? something else?
> 
> Thanks.  Better?

Yes, much better.  Thanks.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFA] replay bookmarks
  2009-11-02  0:29   ` Michael Snyder
  2009-11-02  4:01     ` Eli Zaretskii
  2009-11-03  8:35     ` Hui Zhu
@ 2009-11-09 17:55     ` Tom Tromey
  2009-11-10 22:09       ` Michael Snyder
  2 siblings, 1 reply; 23+ messages in thread
From: Tom Tromey @ 2009-11-09 17:55 UTC (permalink / raw)
  To: Michael Snyder; +Cc: Eli Zaretskii, gdb-patches, glaw, jakob, teawater

>>>>> "Michael" == Michael Snyder <msnyder@vmware.com> writes:

Michael> How about this?

Michael> +struct bookmark *bookmark_chain;
Michael> +int bookmark_count;

These should be static.

Michael> +static int
Michael> +delete_one_bookmark (struct bookmark *b)
Michael> +{
[...]
Michael> +  /* Find bookmark preceeding "marked" one, so we can unlink.  */
Michael> +  /* FIXME what about end cases (first and last)?  */

We're trying to avoid new FIXMEs.

I think instead of using ALL_BOOKMARKS this code would be simpler and
more obviously correct if it just had an explicit loop.  There's no need
to have a special case for the first element of the list, either.  E.g.,
see the deletion code in cli-decode.c:delete_cmd.

Tom


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFA] replay bookmarks
  2009-11-09 17:55     ` Tom Tromey
@ 2009-11-10 22:09       ` Michael Snyder
  2009-11-10 22:27         ` Tom Tromey
  2009-11-11  4:15         ` Eli Zaretskii
  0 siblings, 2 replies; 23+ messages in thread
From: Michael Snyder @ 2009-11-10 22:09 UTC (permalink / raw)
  To: tromey; +Cc: Eli Zaretskii, gdb-patches, glaw, jakob, teawater

[-- Attachment #1: Type: text/plain, Size: 885 bytes --]

Tom Tromey wrote:
>>>>>> "Michael" == Michael Snyder <msnyder@vmware.com> writes:
> 
> Michael> How about this?
> 
> Michael> +struct bookmark *bookmark_chain;
> Michael> +int bookmark_count;
> 
> These should be static.

OK.

> Michael> +static int
> Michael> +delete_one_bookmark (struct bookmark *b)
> Michael> +{
> [...]
> Michael> +  /* Find bookmark preceeding "marked" one, so we can unlink.  */
> Michael> +  /* FIXME what about end cases (first and last)?  */
> 
> We're trying to avoid new FIXMEs.

Sorry, left over, removed.

> I think instead of using ALL_BOOKMARKS this code would be simpler and
> more obviously correct if it just had an explicit loop.  There's no need
> to have a special case for the first element of the list, either.  E.g.,
> see the deletion code in cli-decode.c:delete_cmd.

Well, my model was breakpoints.  Do you really insist that I change it?


[-- Attachment #2: bookmark.txt --]
[-- Type: text/plain, Size: 21578 bytes --]

2009-10-25  Michael Snyder  <msnyder@vmware.com>

        * target.h (struct target_ops): New methods to_get_bookmark
        and to_goto_bookmark.
        (target_get_bookmark): New macro.
        (target_goto_bookmark): New macro.
        * target.c (dummy_get_bookmark): New function, default implementation.
        (dummy_goto_bookmark): New function, default implementation.
        (update_current_target): Inherit new methods.
        * record.c (record_get_bookmark): New function.
        (record_goto_bookmark): New function.
        (init_record_ops): Set to_get_bookmark and to_goto_bookmark methods.
        * reverse.c (struct bookmark): New type.
        (save_bookmark_command): New function (command).
        (delete_bookmark_command): New function (command).
        (goto_bookmark_command): New function (command).
        (bookmarks_info): New function (command).
        (_initialize_reverse): Add new bookmark commands.
        * remote.c (remote_get_bookmark): New target method.
        (remote_goto_bookmark): New target method.
	* command.h (enum command_class): Add class_bookmark.
	* NEWS: Mention bookmark commands.

Index: gdb/record.c
===================================================================
--- gdb.orig/record.c	2009-11-10 14:00:11.000000000 -0800
+++ gdb/record.c	2009-11-10 14:00:15.000000000 -0800
@@ -1523,6 +1523,57 @@
   return 1;
 }
 
+/* "to_get_bookmark" method for process record and prec over core.  */
+
+static gdb_byte *
+record_get_bookmark (char *args, int from_tty)
+{
+  gdb_byte *ret = NULL;
+
+  /* Return stringified form of instruction count.  */
+  if (record_list && record_list->type == record_end)
+    ret = xstrdup (pulongest (record_list->u.end.insn_num));
+
+  if (record_debug)
+    {
+      if (ret)
+	fprintf_unfiltered (gdb_stdlog,
+			    "record_get_bookmark returns %s\n", ret);
+      else
+	fprintf_unfiltered (gdb_stdlog,
+			    "record_get_bookmark returns NULL\n");
+    }
+  return ret;
+}
+
+/* The implementation of the command "record goto".  */
+static void cmd_record_goto (char *, int);
+
+/* "to_goto_bookmark" method for process record and prec over core.  */
+
+static void
+record_goto_bookmark (gdb_byte *bookmark, int from_tty)
+{
+  if (record_debug)
+    fprintf_unfiltered (gdb_stdlog,
+			"record_goto_bookmark receives %s\n", bookmark);
+
+  if (bookmark[0] == '\'' || bookmark[0] == '\"')
+    {
+      if (bookmark[strlen (bookmark) - 1] != bookmark[0])
+	error (_("Unbalanced quotes: %s"), bookmark);
+
+      /* Strip trailing quote.  */
+      bookmark[strlen (bookmark) - 1] = '\0';
+      /* Strip leading quote.  */
+      bookmark++;
+      /* Pass along to cmd_record_goto.  */
+    }
+
+  cmd_record_goto ((char *) bookmark, from_tty);
+  return;
+}
+
 static void
 init_record_ops (void)
 {
@@ -1545,6 +1596,9 @@
   record_ops.to_remove_breakpoint = record_remove_breakpoint;
   record_ops.to_can_execute_reverse = record_can_execute_reverse;
   record_ops.to_stratum = record_stratum;
+  /* Add bookmark target methods.  */
+  record_ops.to_get_bookmark = record_get_bookmark;
+  record_ops.to_goto_bookmark = record_goto_bookmark;
   record_ops.to_magic = OPS_MAGIC;
 }
 
@@ -1750,6 +1804,9 @@
   record_core_ops.to_can_execute_reverse = record_can_execute_reverse;
   record_core_ops.to_has_execution = record_core_has_execution;
   record_core_ops.to_stratum = record_stratum;
+  /* Add bookmark target methods.  */
+  record_core_ops.to_get_bookmark = record_get_bookmark;
+  record_core_ops.to_goto_bookmark = record_goto_bookmark;
   record_core_ops.to_magic = OPS_MAGIC;
 }
 
@@ -2407,6 +2464,101 @@
 		   recfilename);
 }
 
+/* record_goto_insn -- rewind the record log (forward or backward,
+   depending on DIR) to the given entry, changing the program state
+   correspondingly.  */
+
+static void
+record_goto_insn (struct record_entry *entry,
+		  enum exec_direction_kind dir)
+{
+  struct cleanup *set_cleanups = record_gdb_operation_disable_set ();
+  struct regcache *regcache = get_current_regcache ();
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+
+  /* Assume everything is valid: we will hit the entry,
+     and we will not hit the end of the recording.  */
+
+  if (dir == EXEC_FORWARD)
+    record_list = record_list->next;
+
+  do
+    {
+      record_exec_insn (regcache, gdbarch, record_list);
+      if (dir == EXEC_REVERSE)
+	record_list = record_list->prev;
+      else
+	record_list = record_list->next;
+    } while (record_list != entry);
+  do_cleanups (set_cleanups);
+}
+
+/* "record goto" command.  Argument is an instruction number,
+   as given by "info record".
+
+   Rewinds the recording (forward or backward) to the given instruction.  */
+
+static void
+cmd_record_goto (char *arg, int from_tty)
+{
+  struct record_entry *p = NULL;
+  ULONGEST target_insn = 0;
+
+  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;
+    }
+  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;
+    }
+  else
+    {
+      /* General case.  Find designated insn.  */
+      target_insn = parse_and_eval_long (arg);
+
+      for (p = &record_first; p != NULL; p = p->next)
+	if (p->type == record_end && p->u.end.insn_num == target_insn)
+	  break;
+    }
+
+  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);
+}
+
 void
 _initialize_record (void)
 {
@@ -2492,4 +2644,9 @@
 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);
 }
Index: gdb/remote.c
===================================================================
--- gdb.orig/remote.c	2009-11-10 14:00:11.000000000 -0800
+++ gdb/remote.c	2009-11-10 14:00:15.000000000 -0800
@@ -8882,6 +8882,46 @@
   return rs->cond_tracepoints;
 }
 
+/* "to_get_bookmark" target method.
+
+   Return a string from the target that uniquely identifies the
+   current machine state, such that the target will be able to
+   return to this machine state at a later request.
+
+   Only expected to work with record/replay targets.  */
+
+static gdb_byte *
+remote_get_bookmark (char *args, int from_tty)
+{
+  struct remote_state *rs = get_remote_state ();
+
+  putpkt ("qBookmark");
+  getpkt (&rs->buf, &rs->buf_size, 0);
+  if (rs->buf[0] == 'Q' && rs->buf[1] == 'B')
+    return &rs->buf[2];
+  else
+    return NULL;
+}
+
+/* "to_goto_bookmark" target method.
+
+   Restore the target to an earlier-recorded machine state.  */
+
+static void
+remote_goto_bookmark (gdb_byte *bookmark, int from_tty)
+{
+  struct remote_state *rs = get_remote_state ();
+  char *p = rs->buf;
+
+  xsnprintf (rs->buf, rs->buf_size, "QBookmark:%s", bookmark);
+
+  putpkt (p);
+  getpkt (&rs->buf, &rs->buf_size, 0);
+  if (rs->buf[0] != 'O' && rs->buf[1] != 'K')
+    error (_("remote goto bookmark not implemented."));
+}
+
+
 static void
 init_remote_ops (void)
 {
@@ -8931,7 +8971,6 @@
   remote_ops.to_has_execution = default_child_has_execution;
   remote_ops.to_has_thread_control = tc_schedlock;	/* can lock scheduler */
   remote_ops.to_can_execute_reverse = remote_can_execute_reverse;
-  remote_ops.to_magic = OPS_MAGIC;
   remote_ops.to_memory_map = remote_memory_map;
   remote_ops.to_flash_erase = remote_flash_erase;
   remote_ops.to_flash_done = remote_flash_done;
@@ -8945,6 +8984,9 @@
   remote_ops.to_terminal_ours = remote_terminal_ours;
   remote_ops.to_supports_non_stop = remote_supports_non_stop;
   remote_ops.to_supports_multi_process = remote_supports_multi_process;
+  remote_ops.to_get_bookmark = remote_get_bookmark;
+  remote_ops.to_goto_bookmark = remote_goto_bookmark;
+  remote_ops.to_magic = OPS_MAGIC;
 }
 
 /* Set up the extended remote vector by making a copy of the standard
Index: gdb/reverse.c
===================================================================
--- gdb.orig/reverse.c	2009-11-10 14:00:11.000000000 -0800
+++ gdb/reverse.c	2009-11-10 14:06:45.000000000 -0800
@@ -24,6 +24,7 @@
 #include "cli/cli-cmds.h"
 #include "cli/cli-decode.h"
 #include "inferior.h"
+#include "regcache.h"
 
 /* User interface:
    reverse-step, reverse-next etc.  */
@@ -101,6 +102,220 @@
   exec_reverse_once ("finish", args, from_tty);
 }
 
+/* Data structures for a bookmark list.  */
+
+struct bookmark {
+  struct bookmark *next;
+  int number;
+  CORE_ADDR pc;
+  struct symtab_and_line sal;
+  gdb_byte *opaque_data;
+};
+
+static struct bookmark *bookmark_chain;
+static int bookmark_count;
+
+#define ALL_BOOKMARKS(B) for ((B) = bookmark_chain; (B); (B) = (B)->next)
+
+#define ALL_BOOKMARKS_SAFE(B,TMP)           \
+     for ((B) = bookmark_chain;             \
+          (B) ? ((TMP) = (B)->next, 1) : 0; \
+          (B) = (TMP))
+
+/* save_bookmark_command -- implement "bookmark" command.
+   Call target method to get a bookmark identifier.
+   Insert bookmark identifier into list.
+
+   Identifier will be a malloc string (gdb_byte *).
+   Up to us to free it as required.  */
+
+static void
+save_bookmark_command (char *args, int from_tty)
+{
+  /* Get target's idea of a bookmark.  */
+  gdb_byte *bookmark_id = target_get_bookmark (args, from_tty);
+  struct bookmark *b, *b1;
+  struct gdbarch *gdbarch = get_regcache_arch (get_current_regcache ());
+
+  /* CR should not cause another identical bookmark.  */
+  dont_repeat ();
+
+  if (bookmark_id == NULL)
+    error (_("target_get_bookmark failed."));
+
+  /* Set up a bookmark struct.  */
+  b = xcalloc (1, sizeof (struct bookmark));
+  b->number = ++bookmark_count;
+  init_sal (&b->sal);
+  b->pc = regcache_read_pc (get_current_regcache ());
+  b->sal = find_pc_line (b->pc, 0);
+  b->sal.pspace = get_frame_program_space (get_current_frame ());
+  b->opaque_data = bookmark_id;
+  b->next = NULL;
+
+  /* Add this bookmark to the end of the chain, so that a list
+     of bookmarks will come out in order of increasing numbers.  */
+
+  b1 = bookmark_chain;
+  if (b1 == 0)
+    bookmark_chain = b;
+  else
+    {
+      while (b1->next)
+	b1 = b1->next;
+      b1->next = b;
+    }
+  printf_filtered (_("Saved bookmark %d at %s\n"), b->number,
+		     paddress (gdbarch, b->sal.pc));
+}
+
+/* Implement "delete bookmark" command.  */
+
+static int
+delete_one_bookmark (struct bookmark *b)
+{
+  struct bookmark *b1;
+
+  /* Special case, first item in list.  */
+  if (b == bookmark_chain)
+    bookmark_chain = b->next;
+
+  /* Find bookmark preceeding "marked" one, so we can unlink.  */
+  if (b)
+    {
+      ALL_BOOKMARKS (b1)
+	if (b1->next == b)
+	  {
+	    /* Found designated bookmark.  Unlink and delete.  */
+	    b1->next = b->next;
+	    break;
+	  }
+      xfree (b->opaque_data);
+      xfree (b);
+      return 1;		/* success */
+    }
+  return 0;		/* failure */
+}
+
+static void
+delete_all_bookmarks (void)
+{
+  struct bookmark *b, *b1;
+
+  ALL_BOOKMARKS_SAFE (b, b1)
+    {
+      xfree (b->opaque_data);
+      xfree (b);
+    }
+  bookmark_chain = NULL;
+}
+
+static void
+delete_bookmark_command (char *args, int from_tty)
+{
+  struct bookmark *b, *b1;
+  unsigned long num;
+
+  if (bookmark_chain == NULL)
+    {
+      warning (_("No bookmarks."));
+      return;
+    }
+
+  if (args == NULL || args[0] == '\0')
+    {
+      if (from_tty && !query (_("Delete all bookmarks? ")))
+	return;
+      delete_all_bookmarks ();
+      return;
+    }
+
+  num = strtoul (args, NULL, 0);
+  /* Find bookmark with corresponding number.  */
+  ALL_BOOKMARKS (b)
+    if (b->number == num)
+      break;
+
+  if (!delete_one_bookmark (b))
+    /* Not found.  */
+    error (_("delete bookmark: no bookmark found for '%s'."), args);
+}
+
+/* Implement "goto-bookmark" command.  */
+
+static void
+goto_bookmark_command (char *args, int from_tty)
+{
+  struct bookmark *b;
+  unsigned long num;
+
+  if (args == NULL || args[0] == '\0')
+    error (_("Command requires an argument."));
+
+  if (strncmp (args, "start", strlen ("start")) == 0
+      || strncmp (args, "begin", strlen ("begin")) == 0
+      || strncmp (args, "end",   strlen ("end")) == 0)
+    {
+      /* Special case.  Give target opportunity to handle.  */
+      target_goto_bookmark (args, from_tty);
+      return;
+    }
+
+  if (args[0] == '\'' || args[0] == '\"')
+    {
+      /* Special case -- quoted string.  Pass on to target.  */
+      if (args[strlen (args) - 1] != args[0])
+	error (_("Unbalanced quotes: %s"), args);
+      target_goto_bookmark (args, from_tty);
+      return;
+    }
+
+  /* General case.  Bookmark identified by bookmark number.  */
+  num = strtoul (args, NULL, 0);
+  ALL_BOOKMARKS (b)
+    if (b->number == num)
+      break;
+
+  if (b)
+    {
+      /* Found.  Send to target method.  */
+      target_goto_bookmark (b->opaque_data, from_tty);
+      return;
+    }
+  /* Not found.  */
+  error (_("goto-bookmark: no bookmark found for '%s'."), args);
+}
+
+/* Implement "info bookmarks" command.  */
+
+static void
+bookmarks_info (char *args, int from_tty)
+{
+  struct bookmark *b;
+  int bnum = -1;
+  struct gdbarch *gdbarch;
+
+  if (args)
+    bnum = parse_and_eval_long (args);
+
+  if (!bookmark_chain)
+    {
+      printf_filtered (_("No bookmarks.\n"));
+      return;
+    }
+
+  gdbarch = get_regcache_arch (get_current_regcache ());
+  printf_filtered (_("Bookmark    Address     Opaque\n"));
+  printf_filtered (_("   ID                    Data \n"));
+
+  ALL_BOOKMARKS (b)
+    printf_filtered ("   %d       %s    '%s'\n",
+		     b->number,
+		     paddress (gdbarch, b->pc),
+		     b->opaque_data);
+}
+
+
 /* Provide a prototype to silence -Wmissing-prototypes.  */
 extern initialize_file_ftype _initialize_reverse;
 
@@ -142,4 +357,24 @@
 
   add_com ("reverse-finish", class_run, reverse_finish, _("\
 Execute backward until just before selected stack frame is called."));
+
+  add_com ("bookmark", class_bookmark, save_bookmark_command, _("\
+Set a bookmark in the program's execution history.\n\
+A bookmark represents a point in the execution history \n\
+that can be returned to at a later point in the debug session."));
+  add_info ("bookmarks", bookmarks_info, _("\
+Status of user-settable bookmarks.\n\
+Bookmarks are user-settable markers representing a point in the \n\
+execution history that can be returned to later in the same debug \n\
+session."));
+  add_cmd ("bookmark", class_bookmark, delete_bookmark_command, _("\
+Delete a bookmark from the bookmark list.\n\
+Argument is a bookmark number, or no argument to delete all bookmarks.\n"),
+	   &deletelist);
+  add_com ("goto-bookmark", class_bookmark, goto_bookmark_command, _("\
+Go to an earlier-bookmarked point in the program's execution history.\n\
+Argument is the bookmark number of a bookmark saved earlier by using \n\
+the 'bookmark' command, or the special arguments:\n\
+  start (beginning of recording)\n\
+  end   (end of recording)\n"));
 }
Index: gdb/target.c
===================================================================
--- gdb.orig/target.c	2009-11-10 14:00:11.000000000 -0800
+++ gdb/target.c	2009-11-10 14:00:15.000000000 -0800
@@ -674,6 +674,8 @@
       INHERIT (to_async_mask, t);
       INHERIT (to_find_memory_regions, t);
       INHERIT (to_make_corefile_notes, t);
+      INHERIT (to_get_bookmark, t);
+      INHERIT (to_goto_bookmark, t);
       /* Do not inherit to_get_thread_local_address.  */
       INHERIT (to_can_execute_reverse, t);
       INHERIT (to_thread_architecture, t);
@@ -2767,6 +2769,21 @@
   return NULL;
 }
 
+/* Error-catcher for target_get_bookmark.  */
+static gdb_byte *
+dummy_get_bookmark (char *ignore1, int ignore2)
+{
+  tcomplain ();
+  return NULL;
+}
+
+/* Error-catcher for target_goto_bookmark.  */
+static void
+dummy_goto_bookmark (gdb_byte *ignore, int from_tty)
+{
+  tcomplain ();
+}
+
 /* Set up the handful of non-empty slots needed by the dummy target
    vector.  */
 
@@ -2787,6 +2804,8 @@
   dummy_target.to_stratum = dummy_stratum;
   dummy_target.to_find_memory_regions = dummy_find_memory_regions;
   dummy_target.to_make_corefile_notes = dummy_make_corefile_notes;
+  dummy_target.to_get_bookmark = dummy_get_bookmark;
+  dummy_target.to_goto_bookmark = dummy_goto_bookmark;
   dummy_target.to_xfer_partial = default_xfer_partial;
   dummy_target.to_has_all_memory = (int (*) (struct target_ops *)) return_zero;
   dummy_target.to_has_memory = (int (*) (struct target_ops *)) return_zero;
Index: gdb/target.h
===================================================================
--- gdb.orig/target.h	2009-11-10 14:00:11.000000000 -0800
+++ gdb/target.h	2009-11-10 14:00:16.000000000 -0800
@@ -459,13 +459,18 @@
     void (*to_async) (void (*) (enum inferior_event_type, void *), void *);
     int (*to_async_mask) (int);
     int (*to_supports_non_stop) (void);
+    /* find_memory_regions support method for gcore */
     int (*to_find_memory_regions) (int (*) (CORE_ADDR,
 					    unsigned long,
 					    int, int, int,
 					    void *),
 				   void *);
+    /* make_corefile_notes support method for gcore */
     char * (*to_make_corefile_notes) (bfd *, int *);
-
+    /* get_bookmark support method for bookmarks */
+    gdb_byte * (*to_get_bookmark) (char *, int);
+    /* goto_bookmark support method for bookmarks */
+    void (*to_goto_bookmark) (gdb_byte *, int);
     /* Return the thread-local address at OFFSET in the
        thread-local storage for the thread PTID and the shared library
        or executable file given by OBJFILE.  If that block of
@@ -1141,6 +1146,13 @@
 #define target_make_corefile_notes(BFD, SIZE_P) \
      (current_target.to_make_corefile_notes) (BFD, SIZE_P)
 
+/* Bookmark interfaces.  */
+#define target_get_bookmark(ARGS, FROM_TTY) \
+     (current_target.to_get_bookmark) (ARGS, FROM_TTY)
+
+#define target_goto_bookmark(ARG, FROM_TTY) \
+     (current_target.to_goto_bookmark) (ARG, FROM_TTY)
+
 /* Hardware watchpoint interfaces.  */
 
 /* Returns non-zero if we were stopped by a hardware watchpoint (memory read or
Index: gdb/breakpoint.c
===================================================================
--- gdb.orig/breakpoint.c	2009-11-10 14:00:11.000000000 -0800
+++ gdb/breakpoint.c	2009-11-10 14:00:16.000000000 -0800
@@ -10086,6 +10086,8 @@
 Convenience variable \"$bpnum\" contains the number of the last\n\
 breakpoint set."));
 
+  add_info_alias ("b", "breakpoints", 1);
+
   if (xdb_commands)
     add_com ("lb", class_breakpoint, breakpoints_info, _("\
 Status of user-settable breakpoints, or breakpoint number NUMBER.\n\
Index: gdb/NEWS
===================================================================
--- gdb.orig/NEWS	2009-11-10 14:00:11.000000000 -0800
+++ gdb/NEWS	2009-11-10 14:00:16.000000000 -0800
@@ -24,8 +24,25 @@
 
 * New commands (for set/show, see "New options" below)
 
+bookmark
+  Save a reference to the current position in the execution log (for
+  replay targets such as process record/replay).  This execution
+  position can then be returned to later with the 'goto-bookmark'
+  command.
+
+goto-bookmark [<N>]
+  Return the replay target to the excution position represented by the
+  Nth bookmark.  Resume replay debugging from that point.
+
+info bookmarks
+  List the bookmarks presently known to gdb.  Each bookmark can be
+  returned to using the 'goto-bookmark' command.
+
+delete bookmark [<N>]
+  Delete the Nth bookmark, or all bookmarks.
+
 record save [<FILENAME>]
-  Save a file (in core file format) containing the process record 
+  Save a file (in core file format) containing the process record
   execution log for replay debugging at a later time.
 
 record restore <FILENAME>
Index: gdb/command.h
===================================================================
--- gdb.orig/command.h	2009-11-10 14:00:11.000000000 -0800
+++ gdb/command.h	2009-11-10 14:00:16.000000000 -0800
@@ -32,8 +32,8 @@
   /* Classes of commands */
   no_class = -1, class_run = 0, class_vars, class_stack,
   class_files, class_support, class_info, class_breakpoint, class_trace,
-  class_alias, class_obscure, class_user, class_maintenance,
-  class_pseudo, class_tui, class_xdb
+  class_bookmark, class_alias, class_obscure, class_user,
+  class_maintenance, class_pseudo, class_tui, class_xdb
 };
 
 /* FIXME: cagney/2002-03-17: Once cmd_type() has been removed, ``enum

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFA] replay bookmarks
  2009-11-10 22:09       ` Michael Snyder
@ 2009-11-10 22:27         ` Tom Tromey
  2009-11-11  4:15         ` Eli Zaretskii
  1 sibling, 0 replies; 23+ messages in thread
From: Tom Tromey @ 2009-11-10 22:27 UTC (permalink / raw)
  To: Michael Snyder; +Cc: Eli Zaretskii, gdb-patches, glaw, jakob, teawater

>>>>> "Michael" == Michael Snyder <msnyder@vmware.com> writes:

>> I think instead of using ALL_BOOKMARKS this code would be simpler and
>> more obviously correct if it just had an explicit loop.  There's no need
>> to have a special case for the first element of the list, either.  E.g.,
>> see the deletion code in cli-decode.c:delete_cmd.

Michael> Well, my model was breakpoints.  Do you really insist that I
Michael> change it?

Nope.

Tom


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFA] replay bookmarks
  2009-11-10 22:09       ` Michael Snyder
  2009-11-10 22:27         ` Tom Tromey
@ 2009-11-11  4:15         ` Eli Zaretskii
  2009-11-11 18:33           ` Michael Snyder
  1 sibling, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2009-11-11  4:15 UTC (permalink / raw)
  To: Michael Snyder; +Cc: tromey, gdb-patches, glaw, jakob, teawater

> Date: Tue, 10 Nov 2009 14:08:11 -0800
> From: Michael Snyder <msnyder@vmware.com>
> CC: Eli Zaretskii <eliz@gnu.org>,   "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>,  "glaw@undo-software.com" <glaw@undo-software.com>,   "jakob@virtutech.com" <jakob@virtutech.com>,  "teawater@gmail.com" <teawater@gmail.com>
> 
> +goto-bookmark [<N>]
> +  Return the replay target to the excution position represented by the
> +  Nth bookmark.  Resume replay debugging from that point.

Is the argument really optional here?


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFA] replay bookmarks
  2009-11-11  4:15         ` Eli Zaretskii
@ 2009-11-11 18:33           ` Michael Snyder
  2009-11-11 18:44             ` Eli Zaretskii
  0 siblings, 1 reply; 23+ messages in thread
From: Michael Snyder @ 2009-11-11 18:33 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: tromey, gdb-patches, glaw, jakob, teawater

[-- Attachment #1: Type: text/plain, Size: 586 bytes --]

Eli Zaretskii wrote:
>> Date: Tue, 10 Nov 2009 14:08:11 -0800
>> From: Michael Snyder <msnyder@vmware.com>
>> CC: Eli Zaretskii <eliz@gnu.org>,   "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>,  "glaw@undo-software.com" <glaw@undo-software.com>,   "jakob@virtutech.com" <jakob@virtutech.com>,  "teawater@gmail.com" <teawater@gmail.com>
>>
>> +goto-bookmark [<N>]
>> +  Return the replay target to the excution position represented by the
>> +  Nth bookmark.  Resume replay debugging from that point.
> 
> Is the argument really optional here?

No.  Sorry.  How about this?



[-- Attachment #2: bookmark.txt --]
[-- Type: text/plain, Size: 21631 bytes --]

2009-10-25  Michael Snyder  <msnyder@vmware.com>

        * target.h (struct target_ops): New methods to_get_bookmark
        and to_goto_bookmark.
        (target_get_bookmark): New macro.
        (target_goto_bookmark): New macro.
        * target.c (dummy_get_bookmark): New function, default implementation.
        (dummy_goto_bookmark): New function, default implementation.
        (update_current_target): Inherit new methods.
        * record.c (record_get_bookmark): New function.
        (record_goto_bookmark): New function.
        (init_record_ops): Set to_get_bookmark and to_goto_bookmark methods.
        * reverse.c (struct bookmark): New type.
        (save_bookmark_command): New function (command).
        (delete_bookmark_command): New function (command).
        (goto_bookmark_command): New function (command).
        (bookmarks_info): New function (command).
        (_initialize_reverse): Add new bookmark commands.
        * remote.c (remote_get_bookmark): New target method.
        (remote_goto_bookmark): New target method.
	* command.h (enum command_class): Add class_bookmark.
	* NEWS: Mention bookmark commands.

Index: gdb/record.c
===================================================================
--- gdb.orig/record.c	2009-11-10 15:39:57.000000000 -0800
+++ gdb/record.c	2009-11-11 10:28:58.000000000 -0800
@@ -1523,6 +1523,57 @@
   return 1;
 }
 
+/* "to_get_bookmark" method for process record and prec over core.  */
+
+static gdb_byte *
+record_get_bookmark (char *args, int from_tty)
+{
+  gdb_byte *ret = NULL;
+
+  /* Return stringified form of instruction count.  */
+  if (record_list && record_list->type == record_end)
+    ret = xstrdup (pulongest (record_list->u.end.insn_num));
+
+  if (record_debug)
+    {
+      if (ret)
+	fprintf_unfiltered (gdb_stdlog,
+			    "record_get_bookmark returns %s\n", ret);
+      else
+	fprintf_unfiltered (gdb_stdlog,
+			    "record_get_bookmark returns NULL\n");
+    }
+  return ret;
+}
+
+/* The implementation of the command "record goto".  */
+static void cmd_record_goto (char *, int);
+
+/* "to_goto_bookmark" method for process record and prec over core.  */
+
+static void
+record_goto_bookmark (gdb_byte *bookmark, int from_tty)
+{
+  if (record_debug)
+    fprintf_unfiltered (gdb_stdlog,
+			"record_goto_bookmark receives %s\n", bookmark);
+
+  if (bookmark[0] == '\'' || bookmark[0] == '\"')
+    {
+      if (bookmark[strlen (bookmark) - 1] != bookmark[0])
+	error (_("Unbalanced quotes: %s"), bookmark);
+
+      /* Strip trailing quote.  */
+      bookmark[strlen (bookmark) - 1] = '\0';
+      /* Strip leading quote.  */
+      bookmark++;
+      /* Pass along to cmd_record_goto.  */
+    }
+
+  cmd_record_goto ((char *) bookmark, from_tty);
+  return;
+}
+
 static void
 init_record_ops (void)
 {
@@ -1545,6 +1596,9 @@
   record_ops.to_remove_breakpoint = record_remove_breakpoint;
   record_ops.to_can_execute_reverse = record_can_execute_reverse;
   record_ops.to_stratum = record_stratum;
+  /* Add bookmark target methods.  */
+  record_ops.to_get_bookmark = record_get_bookmark;
+  record_ops.to_goto_bookmark = record_goto_bookmark;
   record_ops.to_magic = OPS_MAGIC;
 }
 
@@ -1750,6 +1804,9 @@
   record_core_ops.to_can_execute_reverse = record_can_execute_reverse;
   record_core_ops.to_has_execution = record_core_has_execution;
   record_core_ops.to_stratum = record_stratum;
+  /* Add bookmark target methods.  */
+  record_core_ops.to_get_bookmark = record_get_bookmark;
+  record_core_ops.to_goto_bookmark = record_goto_bookmark;
   record_core_ops.to_magic = OPS_MAGIC;
 }
 
@@ -2407,6 +2464,101 @@
 		   recfilename);
 }
 
+/* record_goto_insn -- rewind the record log (forward or backward,
+   depending on DIR) to the given entry, changing the program state
+   correspondingly.  */
+
+static void
+record_goto_insn (struct record_entry *entry,
+		  enum exec_direction_kind dir)
+{
+  struct cleanup *set_cleanups = record_gdb_operation_disable_set ();
+  struct regcache *regcache = get_current_regcache ();
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+
+  /* Assume everything is valid: we will hit the entry,
+     and we will not hit the end of the recording.  */
+
+  if (dir == EXEC_FORWARD)
+    record_list = record_list->next;
+
+  do
+    {
+      record_exec_insn (regcache, gdbarch, record_list);
+      if (dir == EXEC_REVERSE)
+	record_list = record_list->prev;
+      else
+	record_list = record_list->next;
+    } while (record_list != entry);
+  do_cleanups (set_cleanups);
+}
+
+/* "record goto" command.  Argument is an instruction number,
+   as given by "info record".
+
+   Rewinds the recording (forward or backward) to the given instruction.  */
+
+static void
+cmd_record_goto (char *arg, int from_tty)
+{
+  struct record_entry *p = NULL;
+  ULONGEST target_insn = 0;
+
+  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;
+    }
+  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;
+    }
+  else
+    {
+      /* General case.  Find designated insn.  */
+      target_insn = parse_and_eval_long (arg);
+
+      for (p = &record_first; p != NULL; p = p->next)
+	if (p->type == record_end && p->u.end.insn_num == target_insn)
+	  break;
+    }
+
+  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);
+}
+
 void
 _initialize_record (void)
 {
@@ -2492,4 +2644,9 @@
 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);
 }
Index: gdb/remote.c
===================================================================
--- gdb.orig/remote.c	2009-11-10 15:39:57.000000000 -0800
+++ gdb/remote.c	2009-11-10 15:40:28.000000000 -0800
@@ -8882,6 +8882,46 @@
   return rs->cond_tracepoints;
 }
 
+/* "to_get_bookmark" target method.
+
+   Return a string from the target that uniquely identifies the
+   current machine state, such that the target will be able to
+   return to this machine state at a later request.
+
+   Only expected to work with record/replay targets.  */
+
+static gdb_byte *
+remote_get_bookmark (char *args, int from_tty)
+{
+  struct remote_state *rs = get_remote_state ();
+
+  putpkt ("qBookmark");
+  getpkt (&rs->buf, &rs->buf_size, 0);
+  if (rs->buf[0] == 'Q' && rs->buf[1] == 'B')
+    return &rs->buf[2];
+  else
+    return NULL;
+}
+
+/* "to_goto_bookmark" target method.
+
+   Restore the target to an earlier-recorded machine state.  */
+
+static void
+remote_goto_bookmark (gdb_byte *bookmark, int from_tty)
+{
+  struct remote_state *rs = get_remote_state ();
+  char *p = rs->buf;
+
+  xsnprintf (rs->buf, rs->buf_size, "QBookmark:%s", bookmark);
+
+  putpkt (p);
+  getpkt (&rs->buf, &rs->buf_size, 0);
+  if (rs->buf[0] != 'O' && rs->buf[1] != 'K')
+    error (_("remote goto bookmark not implemented."));
+}
+
+
 static void
 init_remote_ops (void)
 {
@@ -8931,7 +8971,6 @@
   remote_ops.to_has_execution = default_child_has_execution;
   remote_ops.to_has_thread_control = tc_schedlock;	/* can lock scheduler */
   remote_ops.to_can_execute_reverse = remote_can_execute_reverse;
-  remote_ops.to_magic = OPS_MAGIC;
   remote_ops.to_memory_map = remote_memory_map;
   remote_ops.to_flash_erase = remote_flash_erase;
   remote_ops.to_flash_done = remote_flash_done;
@@ -8945,6 +8984,9 @@
   remote_ops.to_terminal_ours = remote_terminal_ours;
   remote_ops.to_supports_non_stop = remote_supports_non_stop;
   remote_ops.to_supports_multi_process = remote_supports_multi_process;
+  remote_ops.to_get_bookmark = remote_get_bookmark;
+  remote_ops.to_goto_bookmark = remote_goto_bookmark;
+  remote_ops.to_magic = OPS_MAGIC;
 }
 
 /* Set up the extended remote vector by making a copy of the standard
Index: gdb/reverse.c
===================================================================
--- gdb.orig/reverse.c	2009-11-10 15:39:57.000000000 -0800
+++ gdb/reverse.c	2009-11-10 15:40:28.000000000 -0800
@@ -24,6 +24,7 @@
 #include "cli/cli-cmds.h"
 #include "cli/cli-decode.h"
 #include "inferior.h"
+#include "regcache.h"
 
 /* User interface:
    reverse-step, reverse-next etc.  */
@@ -101,6 +102,220 @@
   exec_reverse_once ("finish", args, from_tty);
 }
 
+/* Data structures for a bookmark list.  */
+
+struct bookmark {
+  struct bookmark *next;
+  int number;
+  CORE_ADDR pc;
+  struct symtab_and_line sal;
+  gdb_byte *opaque_data;
+};
+
+static struct bookmark *bookmark_chain;
+static int bookmark_count;
+
+#define ALL_BOOKMARKS(B) for ((B) = bookmark_chain; (B); (B) = (B)->next)
+
+#define ALL_BOOKMARKS_SAFE(B,TMP)           \
+     for ((B) = bookmark_chain;             \
+          (B) ? ((TMP) = (B)->next, 1) : 0; \
+          (B) = (TMP))
+
+/* save_bookmark_command -- implement "bookmark" command.
+   Call target method to get a bookmark identifier.
+   Insert bookmark identifier into list.
+
+   Identifier will be a malloc string (gdb_byte *).
+   Up to us to free it as required.  */
+
+static void
+save_bookmark_command (char *args, int from_tty)
+{
+  /* Get target's idea of a bookmark.  */
+  gdb_byte *bookmark_id = target_get_bookmark (args, from_tty);
+  struct bookmark *b, *b1;
+  struct gdbarch *gdbarch = get_regcache_arch (get_current_regcache ());
+
+  /* CR should not cause another identical bookmark.  */
+  dont_repeat ();
+
+  if (bookmark_id == NULL)
+    error (_("target_get_bookmark failed."));
+
+  /* Set up a bookmark struct.  */
+  b = xcalloc (1, sizeof (struct bookmark));
+  b->number = ++bookmark_count;
+  init_sal (&b->sal);
+  b->pc = regcache_read_pc (get_current_regcache ());
+  b->sal = find_pc_line (b->pc, 0);
+  b->sal.pspace = get_frame_program_space (get_current_frame ());
+  b->opaque_data = bookmark_id;
+  b->next = NULL;
+
+  /* Add this bookmark to the end of the chain, so that a list
+     of bookmarks will come out in order of increasing numbers.  */
+
+  b1 = bookmark_chain;
+  if (b1 == 0)
+    bookmark_chain = b;
+  else
+    {
+      while (b1->next)
+	b1 = b1->next;
+      b1->next = b;
+    }
+  printf_filtered (_("Saved bookmark %d at %s\n"), b->number,
+		     paddress (gdbarch, b->sal.pc));
+}
+
+/* Implement "delete bookmark" command.  */
+
+static int
+delete_one_bookmark (struct bookmark *b)
+{
+  struct bookmark *b1;
+
+  /* Special case, first item in list.  */
+  if (b == bookmark_chain)
+    bookmark_chain = b->next;
+
+  /* Find bookmark preceeding "marked" one, so we can unlink.  */
+  if (b)
+    {
+      ALL_BOOKMARKS (b1)
+	if (b1->next == b)
+	  {
+	    /* Found designated bookmark.  Unlink and delete.  */
+	    b1->next = b->next;
+	    break;
+	  }
+      xfree (b->opaque_data);
+      xfree (b);
+      return 1;		/* success */
+    }
+  return 0;		/* failure */
+}
+
+static void
+delete_all_bookmarks (void)
+{
+  struct bookmark *b, *b1;
+
+  ALL_BOOKMARKS_SAFE (b, b1)
+    {
+      xfree (b->opaque_data);
+      xfree (b);
+    }
+  bookmark_chain = NULL;
+}
+
+static void
+delete_bookmark_command (char *args, int from_tty)
+{
+  struct bookmark *b, *b1;
+  unsigned long num;
+
+  if (bookmark_chain == NULL)
+    {
+      warning (_("No bookmarks."));
+      return;
+    }
+
+  if (args == NULL || args[0] == '\0')
+    {
+      if (from_tty && !query (_("Delete all bookmarks? ")))
+	return;
+      delete_all_bookmarks ();
+      return;
+    }
+
+  num = strtoul (args, NULL, 0);
+  /* Find bookmark with corresponding number.  */
+  ALL_BOOKMARKS (b)
+    if (b->number == num)
+      break;
+
+  if (!delete_one_bookmark (b))
+    /* Not found.  */
+    error (_("delete bookmark: no bookmark found for '%s'."), args);
+}
+
+/* Implement "goto-bookmark" command.  */
+
+static void
+goto_bookmark_command (char *args, int from_tty)
+{
+  struct bookmark *b;
+  unsigned long num;
+
+  if (args == NULL || args[0] == '\0')
+    error (_("Command requires an argument."));
+
+  if (strncmp (args, "start", strlen ("start")) == 0
+      || strncmp (args, "begin", strlen ("begin")) == 0
+      || strncmp (args, "end",   strlen ("end")) == 0)
+    {
+      /* Special case.  Give target opportunity to handle.  */
+      target_goto_bookmark (args, from_tty);
+      return;
+    }
+
+  if (args[0] == '\'' || args[0] == '\"')
+    {
+      /* Special case -- quoted string.  Pass on to target.  */
+      if (args[strlen (args) - 1] != args[0])
+	error (_("Unbalanced quotes: %s"), args);
+      target_goto_bookmark (args, from_tty);
+      return;
+    }
+
+  /* General case.  Bookmark identified by bookmark number.  */
+  num = strtoul (args, NULL, 0);
+  ALL_BOOKMARKS (b)
+    if (b->number == num)
+      break;
+
+  if (b)
+    {
+      /* Found.  Send to target method.  */
+      target_goto_bookmark (b->opaque_data, from_tty);
+      return;
+    }
+  /* Not found.  */
+  error (_("goto-bookmark: no bookmark found for '%s'."), args);
+}
+
+/* Implement "info bookmarks" command.  */
+
+static void
+bookmarks_info (char *args, int from_tty)
+{
+  struct bookmark *b;
+  int bnum = -1;
+  struct gdbarch *gdbarch;
+
+  if (args)
+    bnum = parse_and_eval_long (args);
+
+  if (!bookmark_chain)
+    {
+      printf_filtered (_("No bookmarks.\n"));
+      return;
+    }
+
+  gdbarch = get_regcache_arch (get_current_regcache ());
+  printf_filtered (_("Bookmark    Address     Opaque\n"));
+  printf_filtered (_("   ID                    Data \n"));
+
+  ALL_BOOKMARKS (b)
+    printf_filtered ("   %d       %s    '%s'\n",
+		     b->number,
+		     paddress (gdbarch, b->pc),
+		     b->opaque_data);
+}
+
+
 /* Provide a prototype to silence -Wmissing-prototypes.  */
 extern initialize_file_ftype _initialize_reverse;
 
@@ -142,4 +357,24 @@
 
   add_com ("reverse-finish", class_run, reverse_finish, _("\
 Execute backward until just before selected stack frame is called."));
+
+  add_com ("bookmark", class_bookmark, save_bookmark_command, _("\
+Set a bookmark in the program's execution history.\n\
+A bookmark represents a point in the execution history \n\
+that can be returned to at a later point in the debug session."));
+  add_info ("bookmarks", bookmarks_info, _("\
+Status of user-settable bookmarks.\n\
+Bookmarks are user-settable markers representing a point in the \n\
+execution history that can be returned to later in the same debug \n\
+session."));
+  add_cmd ("bookmark", class_bookmark, delete_bookmark_command, _("\
+Delete a bookmark from the bookmark list.\n\
+Argument is a bookmark number, or no argument to delete all bookmarks.\n"),
+	   &deletelist);
+  add_com ("goto-bookmark", class_bookmark, goto_bookmark_command, _("\
+Go to an earlier-bookmarked point in the program's execution history.\n\
+Argument is the bookmark number of a bookmark saved earlier by using \n\
+the 'bookmark' command, or the special arguments:\n\
+  start (beginning of recording)\n\
+  end   (end of recording)\n"));
 }
Index: gdb/target.c
===================================================================
--- gdb.orig/target.c	2009-11-10 15:39:57.000000000 -0800
+++ gdb/target.c	2009-11-10 15:40:28.000000000 -0800
@@ -674,6 +674,8 @@
       INHERIT (to_async_mask, t);
       INHERIT (to_find_memory_regions, t);
       INHERIT (to_make_corefile_notes, t);
+      INHERIT (to_get_bookmark, t);
+      INHERIT (to_goto_bookmark, t);
       /* Do not inherit to_get_thread_local_address.  */
       INHERIT (to_can_execute_reverse, t);
       INHERIT (to_thread_architecture, t);
@@ -2767,6 +2769,21 @@
   return NULL;
 }
 
+/* Error-catcher for target_get_bookmark.  */
+static gdb_byte *
+dummy_get_bookmark (char *ignore1, int ignore2)
+{
+  tcomplain ();
+  return NULL;
+}
+
+/* Error-catcher for target_goto_bookmark.  */
+static void
+dummy_goto_bookmark (gdb_byte *ignore, int from_tty)
+{
+  tcomplain ();
+}
+
 /* Set up the handful of non-empty slots needed by the dummy target
    vector.  */
 
@@ -2787,6 +2804,8 @@
   dummy_target.to_stratum = dummy_stratum;
   dummy_target.to_find_memory_regions = dummy_find_memory_regions;
   dummy_target.to_make_corefile_notes = dummy_make_corefile_notes;
+  dummy_target.to_get_bookmark = dummy_get_bookmark;
+  dummy_target.to_goto_bookmark = dummy_goto_bookmark;
   dummy_target.to_xfer_partial = default_xfer_partial;
   dummy_target.to_has_all_memory = (int (*) (struct target_ops *)) return_zero;
   dummy_target.to_has_memory = (int (*) (struct target_ops *)) return_zero;
Index: gdb/target.h
===================================================================
--- gdb.orig/target.h	2009-11-10 15:39:57.000000000 -0800
+++ gdb/target.h	2009-11-10 15:40:28.000000000 -0800
@@ -459,13 +459,18 @@
     void (*to_async) (void (*) (enum inferior_event_type, void *), void *);
     int (*to_async_mask) (int);
     int (*to_supports_non_stop) (void);
+    /* find_memory_regions support method for gcore */
     int (*to_find_memory_regions) (int (*) (CORE_ADDR,
 					    unsigned long,
 					    int, int, int,
 					    void *),
 				   void *);
+    /* make_corefile_notes support method for gcore */
     char * (*to_make_corefile_notes) (bfd *, int *);
-
+    /* get_bookmark support method for bookmarks */
+    gdb_byte * (*to_get_bookmark) (char *, int);
+    /* goto_bookmark support method for bookmarks */
+    void (*to_goto_bookmark) (gdb_byte *, int);
     /* Return the thread-local address at OFFSET in the
        thread-local storage for the thread PTID and the shared library
        or executable file given by OBJFILE.  If that block of
@@ -1141,6 +1146,13 @@
 #define target_make_corefile_notes(BFD, SIZE_P) \
      (current_target.to_make_corefile_notes) (BFD, SIZE_P)
 
+/* Bookmark interfaces.  */
+#define target_get_bookmark(ARGS, FROM_TTY) \
+     (current_target.to_get_bookmark) (ARGS, FROM_TTY)
+
+#define target_goto_bookmark(ARG, FROM_TTY) \
+     (current_target.to_goto_bookmark) (ARG, FROM_TTY)
+
 /* Hardware watchpoint interfaces.  */
 
 /* Returns non-zero if we were stopped by a hardware watchpoint (memory read or
Index: gdb/breakpoint.c
===================================================================
--- gdb.orig/breakpoint.c	2009-11-10 15:39:57.000000000 -0800
+++ gdb/breakpoint.c	2009-11-11 10:28:58.000000000 -0800
@@ -10086,6 +10086,8 @@
 Convenience variable \"$bpnum\" contains the number of the last\n\
 breakpoint set."));
 
+  add_info_alias ("b", "breakpoints", 1);
+
   if (xdb_commands)
     add_com ("lb", class_breakpoint, breakpoints_info, _("\
 Status of user-settable breakpoints, or breakpoint number NUMBER.\n\
Index: gdb/NEWS
===================================================================
--- gdb.orig/NEWS	2009-11-10 15:39:57.000000000 -0800
+++ gdb/NEWS	2009-11-11 10:30:28.000000000 -0800
@@ -24,8 +24,26 @@
 
 * New commands (for set/show, see "New options" below)
 
+bookmark
+  Save a reference to the current position in the execution log (for
+  replay targets such as process record/replay).  This execution
+  position can then be returned to later with the 'goto-bookmark'
+  command.
+
+goto-bookmark <N> or <start> or <end>
+  Return the replay target to the excution position represented by
+  the Nth bookmark (or start or end of record log).  Resume replay
+  debugging from that point.
+
+info bookmarks
+  List the bookmarks presently known to gdb.  Each bookmark can be
+  returned to using the 'goto-bookmark' command.
+
+delete bookmark [<N>]
+  Delete the Nth bookmark, or all bookmarks.
+
 record save [<FILENAME>]
-  Save a file (in core file format) containing the process record 
+  Save a file (in core file format) containing the process record
   execution log for replay debugging at a later time.
 
 record restore <FILENAME>
Index: gdb/command.h
===================================================================
--- gdb.orig/command.h	2009-11-10 15:39:57.000000000 -0800
+++ gdb/command.h	2009-11-10 15:40:28.000000000 -0800
@@ -32,8 +32,8 @@
   /* Classes of commands */
   no_class = -1, class_run = 0, class_vars, class_stack,
   class_files, class_support, class_info, class_breakpoint, class_trace,
-  class_alias, class_obscure, class_user, class_maintenance,
-  class_pseudo, class_tui, class_xdb
+  class_bookmark, class_alias, class_obscure, class_user,
+  class_maintenance, class_pseudo, class_tui, class_xdb
 };
 
 /* FIXME: cagney/2002-03-17: Once cmd_type() has been removed, ``enum

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFA] replay bookmarks
  2009-11-11 18:33           ` Michael Snyder
@ 2009-11-11 18:44             ` Eli Zaretskii
  2009-11-20 17:26               ` Michael Snyder
  0 siblings, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2009-11-11 18:44 UTC (permalink / raw)
  To: Michael Snyder; +Cc: tromey, gdb-patches, glaw, jakob, teawater

> Date: Wed, 11 Nov 2009 10:31:35 -0800
> From: Michael Snyder <msnyder@vmware.com>
> CC: "tromey@redhat.com" <tromey@redhat.com>, 
>  "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>,
>  "glaw@undo-software.com" <glaw@undo-software.com>, 
>  "jakob@virtutech.com" <jakob@virtutech.com>,
>  "teawater@gmail.com" <teawater@gmail.com>
> 
> >> +goto-bookmark [<N>]
> >> +  Return the replay target to the excution position represented by the
> >> +  Nth bookmark.  Resume replay debugging from that point.
> > 
> > Is the argument really optional here?
> 
> No.  Sorry.  How about this?

This is good, thanks.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFA] replay bookmarks
  2009-11-11 18:44             ` Eli Zaretskii
@ 2009-11-20 17:26               ` Michael Snyder
  2009-11-23  3:20                 ` Hui Zhu
  0 siblings, 1 reply; 23+ messages in thread
From: Michael Snyder @ 2009-11-20 17:26 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: tromey, gdb-patches, glaw, jakob, teawater

Eli Zaretskii wrote:
>> Date: Wed, 11 Nov 2009 10:31:35 -0800
>> From: Michael Snyder <msnyder@vmware.com>
>> CC: "tromey@redhat.com" <tromey@redhat.com>, 
>>  "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>,
>>  "glaw@undo-software.com" <glaw@undo-software.com>, 
>>  "jakob@virtutech.com" <jakob@virtutech.com>,
>>  "teawater@gmail.com" <teawater@gmail.com>
>>
>>>> +goto-bookmark [<N>]
>>>> +  Return the replay target to the excution position represented by the
>>>> +  Nth bookmark.  Resume replay debugging from that point.
>>> Is the argument really optional here?
>> No.  Sorry.  How about this?
> 
> This is good, thanks.
> 

OK -- since the only remaining questions pertain to the remote
implimentation, and since the VirtuTech people are looking into
that, I've gone ahead and committed the remainder of the patch,
omitting the remote part.

We can add the remote part back in when we converge on it.

Michael


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFA] replay bookmarks
  2009-11-20 17:26               ` Michael Snyder
@ 2009-11-23  3:20                 ` Hui Zhu
  2009-12-04  9:09                   ` Jakob Engblom
  0 siblings, 1 reply; 23+ messages in thread
From: Hui Zhu @ 2009-11-23  3:20 UTC (permalink / raw)
  To: Michael Snyder; +Cc: Eli Zaretskii, tromey, gdb-patches, glaw, jakob

It's really cool.  Wish more and more people use it.  :)

Thanks,
Hui

On Sat, Nov 21, 2009 at 01:22, Michael Snyder <msnyder@vmware.com> wrote:
> Eli Zaretskii wrote:
>>>
>>> Date: Wed, 11 Nov 2009 10:31:35 -0800
>>> From: Michael Snyder <msnyder@vmware.com>
>>> CC: "tromey@redhat.com" <tromey@redhat.com>,
>>>  "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>,
>>>  "glaw@undo-software.com" <glaw@undo-software.com>,
>>>  "jakob@virtutech.com" <jakob@virtutech.com>,
>>>  "teawater@gmail.com" <teawater@gmail.com>
>>>
>>>>> +goto-bookmark [<N>]
>>>>> +  Return the replay target to the excution position represented by the
>>>>> +  Nth bookmark.  Resume replay debugging from that point.
>>>>
>>>> Is the argument really optional here?
>>>
>>> No.  Sorry.  How about this?
>>
>> This is good, thanks.
>>
>
> OK -- since the only remaining questions pertain to the remote
> implimentation, and since the VirtuTech people are looking into
> that, I've gone ahead and committed the remainder of the patch,
> omitting the remote part.
>
> We can add the remote part back in when we converge on it.
>
> Michael
>


^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: [RFA] replay bookmarks
  2009-11-23  3:20                 ` Hui Zhu
@ 2009-12-04  9:09                   ` Jakob Engblom
  0 siblings, 0 replies; 23+ messages in thread
From: Jakob Engblom @ 2009-12-04  9:09 UTC (permalink / raw)
  To: gdb-patches

I have now tried this functionality myself, which required some patches to make
sure that, for example, memory was allocated for the strings used as arguments.


Here is a sample session. On the remote end, we have a Simics ppc-simple machine
that just contains a PowerPC 603e processor, NS16550 serial port, memory, and a
home-made DMA controller that we are controlling.  It runs no OS, just a
bare-board software that accesses hardware directly. 

I managed to set bookmarks both in Simics and gdb, and goto-bookmark
arbitrarily.  Reverse and forward step commands worked (as expected).  To show
how it looked on the Simics side, here is a typical output from
"list-bookmarks", after reversing from the leading bookmark:

simics> list-bookmarks 
 name                                     cycles
------------------------------------------------------
 start                                        0
 gdb_bookmark1                               32
 <current execution point>                86040
 gdb_bookmark2                            86043
------------------------------------------------------

Notes on possible improvements:

* The "bookmark" command accepts a string as an argument but does nothing with
it.

(gdb) bookmark "first_uart_access"
Saved bookmark 1 at 0x10000

It would be nice with a complaint, or that hte given string was sent to the
backend. The Simics side here only sees a bookmark with ID=1 being created, and
uses the internal name "gdb_bookmark1" for it.  


* A "list-bookmarks" command would also be helpful to know what bookmarks there
are. Also, tab-completion for the goto-bookmark command could list the available
bookmark numbers or names (asking it to query the remote backend for all its
named bookmarks might be a bit too much).


* We definitely need the ability for the remote target to tell gdb to update its
internal state. Now I quickly got into situations where gdb and Simics were out
of sync, and getting them in sync essentially involved restarting gdb. Setting a
time breakpoint or device access breakpoint or other funky breakpoint Simics
right now requires you to do ctrl-C in gdb to get back to prompt, and sometimes
the state is not quite right then. 


* goto-bookmark did not always reset the state of gdb, with the execution engine
in Simics thinking that the program was at one point, but gdb still left with a
different idea for where things were.  


* when single-stepping using "si" on an instruction with a gdb execution
breakpoint set on it, gdb just gets stuck repeatedly stepping the same
instruction.  Simics did not progress either, so I am still investigating where
the real problem is. 



Best regards,

/jakob

_______________________________________________________

Jakob Engblom, PhD, Technical Marketing Manager

Virtutech                   Direct: +46 8 690 07 47   
Drottningholmsvägen 22      Mobile: +46 709 242 646  
11243 Stockholm             Web:    www.virtutech.com 
Sweden
________________________________________________________
  




^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2009-12-04  9:09 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-11-01 21:13 [RFA] replay bookmarks Michael Snyder
2009-11-01 21:23 ` Eli Zaretskii
2009-11-02  0:29   ` Michael Snyder
2009-11-02  4:01     ` Eli Zaretskii
2009-11-03  8:34       ` Jakob Engblom
2009-11-03  8:36         ` Hui Zhu
2009-11-03 18:42         ` Michael Snyder
2009-11-05 11:05           ` Jakob Engblom
2009-11-03  8:35     ` Hui Zhu
2009-11-05 18:13       ` Michael Snyder
2009-11-05 18:44         ` Pedro Alves
2009-11-05 19:17         ` Eli Zaretskii
2009-11-05 23:40           ` Michael Snyder
2009-11-06  8:34             ` Eli Zaretskii
2009-11-09 17:55     ` Tom Tromey
2009-11-10 22:09       ` Michael Snyder
2009-11-10 22:27         ` Tom Tromey
2009-11-11  4:15         ` Eli Zaretskii
2009-11-11 18:33           ` Michael Snyder
2009-11-11 18:44             ` Eli Zaretskii
2009-11-20 17:26               ` Michael Snyder
2009-11-23  3:20                 ` Hui Zhu
2009-12-04  9:09                   ` Jakob Engblom

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox