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

Alan Hayward
2017-06-22T16:02:19Z

> 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.