Re: [PATCH] Replace regbuf with regcache in record-full.c

[Alan Hayward <Alan dot Hayward at arm dot com>]

> On 22 Jun 2017, at 09:44, Yao Qi <qiyaoltc@gmail.com> wrote:
> 
> Alan Hayward <Alan.Hayward@arm.com> writes:
> 
>> In record-full.c, instead of backing up all the registers into a large
>> buffer, duplicate the regcache.
>> This enables the removal of an instance of MAX_REGISTER_SIZE.
>> 
>> Note that regcache_dup() create a read-only copy of a register cache,
>> which ensures the new regcache cannot write back to the target.
>> 
>> Once created, we need to be able to copy registers between the two caches,
>> which we want to do without creating a temporary buffer.
>> 
>> I've added regcache::raw_copy() to allow the copying of raw registers
>> between two regcaches - either of which might be set as read-only.
> 
> Can we name this method raw_supply?

Resulting in:
  void raw_supply (int regnum, const void *buf);
  void raw_supply (int regnum, const struct regcache *src_regcache);

Happy to do that.


> 
>> 
>> Alternatively, I could make the new regcache as writable (by enabling a
>> regcache copy constructor). But, I think this would be dangerous as it
>> it then has the potential to write back to the target if the wrong function
>> is called.
> 
> regcache only interacts with target through ::raw_update and
> ::raw_write.  Can we have a regcache class without raw_update and
> raw_write?  regcache has two set of methods, {raw,cooked}_{read,write}
> and raw_{collect,supply}XXX.  The former interacts with target, but the
> latter doesn't.  We can create a new class regcache_collect_supply which
> has methods raw_{collect,supply}XXXX, regcache extends it.  Then, add a
> method "void raw_supply (int regnum, const regcache_collect_supply &src)"
> and change record_full_core_regbuf to a regcache_collect_supply.  The
> interface looks like this, (regcache_collect_supply is regcache_1 in the
> doxygen doc, because I didn't figure out a reasonable name yesterday),

detached_regcache ?
virtual_regcache ? (Not as keen on that)

> 
> http://people.linaro.org/~yao.qi/gdb/doxy/regcache-split/gdb-xref/classregcache.html
> 
> Do you like this design?  One more thing is that the new class
> regcache_collect_supply can be used in target_ops hooks
> to_fetch_registers, to_store_registers, to_prepare_to_store,
> 
>    void (*to_fetch_registers) (struct target_ops *, regcache_collect_supply *, int)
>      TARGET_DEFAULT_IGNORE ();
>    void (*to_store_registers) (struct target_ops *, regcache_collect_supply *, int)
>      TARGET_DEFAULT_NORETURN (noprocess ());
>    void (*to_prepare_to_store) (struct target_ops *, regcache_collect_supply *)
>      TARGET_DEFAULT_NORETURN (noprocess ());
> 
> so that all the implementations of these methods above can only access
> raw_{collect,supply}XX methods, and they can't access
> {raw,cooked}_{read,write}XXX methods, which in turn may call target_ops
> to_fetch_registers and to_store_registers again.
> 

Following this through, regcache_dup and regcache_cpy should create a detached_regcache ?

Code would be free to write to the detached_regcache and there would be no need to mark
it as “read-only”.

Would there be any requirement to still have a read-only regcache ?


Alan.