Re: [RFA/RFC] Add dump and load command to process record and replay

[Michael Snyder <msnyder@vmware.com>]

Hui Zhu wrote:

> +/* Load the execution log from a file.  */
> +
> +static void
> +cmd_record_load (char *args, int from_tty)
> +{
> +  int recfd;
> +  uint32_t magic;
> +  struct cleanup *old_cleanups;
> +  struct cleanup *old_cleanups2;
> +  struct record_entry *rec;
> +  int insn_number = 0;
> +
> +  if (current_target.to_stratum != record_stratum)
> +    {
> +      cmd_record_start (NULL, from_tty);
> +      printf_unfiltered (_("Auto start process record.\n"));
> +    }
> +
> +  if (!args || (args && !*args))
> +    error (_("Argument for filename required.\n"));
> +
> +  /* Open the load file.  */
> +  recfd = open (args, O_RDONLY | O_BINARY);
> +  if (recfd < 0)
> +    error (_("Failed to open '%s' for loading execution records: %s"),
> +           args, strerror (errno));
> +  old_cleanups = make_cleanup (cmd_record_fd_cleanups, &recfd);
> +
> +  /* Check the magic code.  */
> +  record_read_dump (args, recfd, &magic, 4);
> +  if (magic != RECORD_FILE_MAGIC)
> +    error (_("'%s' is not a valid dump of execution records."), args);
> +
> +  /* Load the entries in recfd to the record_arch_list_head and
> +     record_arch_list_tail.  */
> +  record_arch_list_head = NULL;
> +  record_arch_list_tail = NULL;

Hi Hui,

It seems to me that you didn't discard the old recording entries
before loading the new ones -- you just added the new ones into
the existing list.

I don't think that is safe.  We can't be sure that there is
any relationship between any existing recording entries and
the new entries from the file.  We certainly can't assume
that they are consecutive.

I think you have to completely clear the record log at this point,
before you begin reading new entries from the dump file.

What I would suggest is that we add these lines to
record_list_release -- and then we can just call that here.

@@ -159,6 +159,11 @@ record_list_release (struct record_entry

    if (rec != &record_first)
      xfree (rec);
+
+  record_list = &record_first;
+  record_arch_list_tail = NULL;
+  record_arch_list_tail = NULL;
+  record_insn_num = 0;
  }