Re: [rfc] btrace: control memory access during replay

[Pedro Alves <palves at redhat dot com>]

On 05/19/2014 08:51 AM, Metzger, Markus T wrote:
>> -----Original Message-----
>> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
>> owner@sourceware.org] On Behalf Of Pedro Alves
>> Sent: Wednesday, May 14, 2014 5:35 PM
> 
> 
>> On 04/04/2014 10:48 AM, Eli Zaretskii wrote:
> 
>>>>> Other than that, the documentation parts are approved.  However, I
>>>>> wonder whether "allow-memory-access" is a good name for a setting
>>>>> which actually allows access to writable portion of the memory.  IOW,
>>>>> even when the value is OFF, we do allow access to memory, just not the
>>>>> writable portion of it.
>>>>
>>>> Agreed; allow-access-to-writable-memory-while-replaying is a bit long,
>> though.
>>>
>>> How about access-writable-memory?
>>
>> Sounds fine to me.
>>
>> What's the likelihood of another variant appearing?  That is,
>> I'm mildly wondering if it should be an enum from the get go:
>>
>>  set record btrace replay-memory-access read-only|read-write|...|...
> 
> I don't see another variant right now but I also don't see why it
> shouldn't be an enum.

The kind of variant I was considering was disabling the fallback
of reading read only regions as tagged in the binary from live/core
memory.  But maybe btrace gets completely useless that way.
If we can't think of another useful variant, then I'm fine with
a boolean, if it sounds more natural.  Your choice.

>> I also got a little confused with:
>>
>> "The accessed memory corresponds to the end of the recorded
>> execution trace."
>>
>> Maybe we should say "live program" instead ?
> 
> Would "live program" still be OK for core files?

Hmm.  I didn't think btrace could work with core files,
unlike record full?  Are you adding support for dumping/restoring
the btrace like "record save" does?

>> I actually didn't see anything in the patch that actually makes the
>> setting work.
> 
> The patch is using an existing variable to guard writable memory
> access.  We already allow write-access for breakpoints during
> replay.  This patch is now adding a CLI for the guard variable.

Ah.  Thanks.

>> Also, please install a show hook in the command, so that i18n
>> can work.
> 
> I'm using the default set/show functions with _("") descriptions
> for both set and show.  Isn't that enough for i18n?

Nope.  See deprecated_show_value_hack:

  /* Print doc minus "show" at start.  */
  print_doc_line (gdb_stdout, c->doc + 5);

That can only work in English.

> 
> In case it isn't, would I need a set function, as well?

Nope, just the show function is enough.

-- 
Pedro Alves