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

["Metzger, Markus T" <markus.t.metzger@intel.com>]

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Monday, May 19, 2014 7:44 PM
> To: Metzger, Markus T; Eli Zaretskii

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

We must be able to read code sections.  Without that, btrace
will not work.

One more option that I can think of is reading from object
files.

I'll use an enum for the sake of flexibility.


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

At the moment it can't.  I plan to add core file support, though.

Regards,
Markus.

> >> 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
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052