Re: [PREC/RFA] Add not_replay to make precord support release memory better

[Hui Zhu <teawater@gmail.com>]

Thanks.

Hui

On Wed, Aug 5, 2009 at 02:18, Michael Snyder<msnyder@vmware.com> wrote:
> Hui Zhu wrote:
>>
>> On Tue, Aug 4, 2009 at 01:30, Michael Snyder<msnyder@vmware.com> wrote:
>>>
>>> Hui Zhu wrote:
>>>>
>>>> The record_exec_entry don't have big contact with core dump patch.
>>>>
>>>> I think the new version is more clear.
>>>
>>> But we had this patch all worked out, and now you've added
>>> a bunch of new stuff to it.  It's not a "new version" of the
>>> "add not_replay" patch, it's a whole new change.
>>>
>>> That's not the way we do it.  If you want to add new stuff,
>>> you start a new thread, you don't add it into a thread that's
>>> already in review.
>>>
>>>
>>
>> OK.  I got it.
>>
>> So the patch http://sourceware.org/ml/gdb-patches/2009-08/msg00006.html
>> looks OK.
>> Could you check it in?
>>
>> I will update dump patch after this patch in.
>>
>> Thanks,
>> Hui
>
> Thanks.  Added some comments, and it's in as attached here.
>
>
>
> 2009-08-04  Hui Zhu  <teawater@gmail.com>
>            Michael Snyder  <msnyder@vmware.com>
>
>        * record.c (record_mem_entry): New field 'mem_entry_not_accessible'.
>        (record_arch_list_add_mem): Initialize 'mem_entry_not_accessible'.
>        (record_wait): Set 'mem_entry_not_accessible' flag if target
>        memory not readable.  Don't try to change target memory if
>        'mem_entry_not_accessible' is set.
>
> Index: record.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/record.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 record.c
> --- record.c    22 Jul 2009 05:31:26 -0000      1.9
> +++ record.c    4 Aug 2009 18:17:15 -0000
> @@ -51,6 +51,9 @@ struct record_mem_entry
>  {
>   CORE_ADDR addr;
>   int len;
> +  /* Set this flag if target memory for this entry
> +     can no longer be accessed.  */
> +  int mem_entry_not_accessible;
>   gdb_byte *val;
>  };
>
> @@ -275,6 +278,7 @@ record_arch_list_add_mem (CORE_ADDR addr
>   rec->type = record_mem;
>   rec->u.mem.addr = addr;
>   rec->u.mem.len = len;
> +  rec->u.mem.mem_entry_not_accessible = 0;
>
>   if (target_read_memory (addr, rec->u.mem.val, len))
>     {
> @@ -727,32 +731,55 @@ record_wait (struct target_ops *ops,
>          else if (record_list->type == record_mem)
>            {
>              /* mem */
> -             gdb_byte *mem = alloca (record_list->u.mem.len);
> -             if (record_debug > 1)
> -               fprintf_unfiltered (gdb_stdlog,
> -                                   "Process record: record_mem %s to "
> -                                   "inferior addr = %s len = %d.\n",
> -                                   host_address_to_string (record_list),
> -                                   paddress (gdbarch,
> record_list->u.mem.addr),
> -                                   record_list->u.mem.len);
> -
> -             if (target_read_memory
> -                 (record_list->u.mem.addr, mem, record_list->u.mem.len))
> -               error (_("Process record: error reading memory at "
> -                        "addr = %s len = %d."),
> -                      paddress (gdbarch, record_list->u.mem.addr),
> -                      record_list->u.mem.len);
> -
> -             if (target_write_memory
> -                 (record_list->u.mem.addr, record_list->u.mem.val,
> -                  record_list->u.mem.len))
> -               error (_
> -                      ("Process record: error writing memory at "
> -                       "addr = %s len = %d."),
> -                      paddress (gdbarch, record_list->u.mem.addr),
> -                      record_list->u.mem.len);
> -
> -             memcpy (record_list->u.mem.val, mem, record_list->u.mem.len);
> +             /* Nothing to do if the entry is flagged not_accessible.  */
> +             if (!record_list->u.mem.mem_entry_not_accessible)
> +               {
> +                 gdb_byte *mem = alloca (record_list->u.mem.len);
> +                 if (record_debug > 1)
> +                   fprintf_unfiltered (gdb_stdlog,
> +                                       "Process record: record_mem %s to "
> +                                       "inferior addr = %s len = %d.\n",
> +                                       host_address_to_string
> (record_list),
> +                                       paddress (gdbarch,
> +                                                 record_list->u.mem.addr),
> +                                       record_list->u.mem.len);
> +
> +                 if (target_read_memory (record_list->u.mem.addr, mem,
> +                                         record_list->u.mem.len))
> +                   {
> +                     if (execution_direction != EXEC_REVERSE)
> +                       error (_("Process record: error reading memory at "
> +                                "addr = %s len = %d."),
> +                              paddress (gdbarch, record_list->u.mem.addr),
> +                              record_list->u.mem.len);
> +                     else
> +                       /* Read failed --
> +                          flag entry as not_accessible.  */
> +                       record_list->u.mem.mem_entry_not_accessible = 1;
> +                   }
> +                 else
> +                   {
> +                     if (target_write_memory (record_list->u.mem.addr,
> +                                              record_list->u.mem.val,
> +                                              record_list->u.mem.len))
> +                       {
> +                         if (execution_direction != EXEC_REVERSE)
> +                           error (_("Process record: error writing memory
> at "
> +                                    "addr = %s len = %d."),
> +                                  paddress (gdbarch,
> record_list->u.mem.addr),
> +                                  record_list->u.mem.len);
> +                         else
> +                           /* Write failed --
> +                              flag entry as not_accessible.  */
> +                           record_list->u.mem.mem_entry_not_accessible = 1;
> +                       }
> +                     else
> +                       {
> +                         memcpy (record_list->u.mem.val, mem,
> +                                 record_list->u.mem.len);
> +                       }
> +                   }
> +               }
>            }
>          else
>            {
>
>