Re: [patch v8 17/24] record-btrace: provide xfer_partial target method

[Pedro Alves <palves@redhat.com>]

On 12/12/2013 09:15 AM, Markus Metzger wrote:
> Provide the xfer_partial target method for the btrace record target.
> 
> Only allow memory accesses to readonly memory while we're replaying.
> 
> 2013-12-12  Markus Metzger  <markus.t.metzger@intel.com>
> 
> 	* record-btrace.c (record_btrace_xfer_partial): New.
> 	(init_record_btrace_ops): Initialize xfer_partial.
> 
> 
> ---
>  gdb/record-btrace.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
> index cf30e17..ec99ba2 100644
> --- a/gdb/record-btrace.c
> +++ b/gdb/record-btrace.c
> @@ -753,6 +753,56 @@ record_btrace_is_replaying (void)
>    return 0;
>  }
>  
> +/* The to_xfer_partial method of target record-btrace.  */
> +
> +static LONGEST
> +record_btrace_xfer_partial (struct target_ops *ops, enum target_object object,
> +			    const char *annex, gdb_byte *readbuf,
> +			    const gdb_byte *writebuf, ULONGEST offset,
> +			    LONGEST len)
> +{
> +  struct target_ops *t;
> +
> +  /* Filter out requests that don't make sense during replay.  */

This allows writes to pass through?

> +  if (record_btrace_is_replaying ())
> +    {
> +      switch (object)
> +	{
> +	case TARGET_OBJECT_MEMORY:
> +	case TARGET_OBJECT_RAW_MEMORY:
> +	case TARGET_OBJECT_STACK_MEMORY:

I don't think we actually ever see TARGET_OBJECT_RAW_MEMORY
and TARGET_OBJECT_STACK_MEMORY here?  Those are supposedly
handled by target.c, and translated to TARGET_OBJECT_MEMORY
before reaching the actual targets.

> +	  {
> +	    /* We allow reading readonly memory.  */
> +	    struct target_section *section;

A comment on this being an heuristic would be good.  The _current_
readonly sections might well not be the same as the ones the
target had when the trace was recorded.

> +
> +	    section = target_section_by_addr (ops, offset);
> +	    if (section != NULL)
> +	      {
> +		/* Check if the section we found is readonly.  */
> +		if ((bfd_get_section_flags (section->the_bfd_section->owner,
> +					    section->the_bfd_section)
> +		     & SEC_READONLY) != 0)
> +		  {
> +		    /* Truncate the request to fit into this section.  */
> +		    len = min (len, section->endaddr - offset);
> +		    break;
> +		  }
> +	      }
> +
> +	    return -1;

I must be missing something, but won't raw_memory_xfer_partial
keep trying in the target beneath, anyway?

> +	  }
> +	}
> +    }
> +
> +  /* Forward the request.  */
> +  for (t = ops->beneath; t != NULL; t = t->beneath)
> +    if (t->to_xfer_partial != NULL)
> +      return t->to_xfer_partial (t, object, annex, readbuf, writebuf,
> +				 offset, len);
> +
> +  return -1;
> +}
> +
>  /* The to_fetch_registers method of target record-btrace.  */
>  
>  static void
> @@ -930,6 +980,7 @@ init_record_btrace_ops (void)
>    ops->to_call_history_from = record_btrace_call_history_from;
>    ops->to_call_history_range = record_btrace_call_history_range;
>    ops->to_record_is_replaying = record_btrace_is_replaying;
> +  ops->to_xfer_partial = record_btrace_xfer_partial;
>    ops->to_fetch_registers = record_btrace_fetch_registers;
>    ops->to_store_registers = record_btrace_store_registers;
>    ops->to_prepare_to_store = record_btrace_prepare_to_store;
> 


-- 
Pedro Alves