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

[Hui Zhu <teawater at gmail dot 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
> ? ? ? ? ? ?{
>
>