[Lldb-commits] [PATCH] D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages.

[Walter via lldb-commits]

I totally agree with you, I didn't think about creating custom callbacks
=P. I'll refactor the code accordingly. Thanks, man

Il giorno ven 2 ott 2020 alle ore 01:51 Pavel Labath <pavel at labath.sk> ha
scritto:

> On 01/10/2020 20:57, Walter wrote:
> >> - I am surprised that it was not necessary to create a special process
> > plugin for this purpose. I have a feeling one will be necessary sooner
> > or later because of the need to customize the step/continue/etc. flows.
> > Currently, this will probably produce very bizarre if one tries to
> > execute those commands. The reason I'm bringing this up is because of
> > the `Target::GetTrace` method being added in the next patch. If there is
> > a trace-specific process class, then it might make sense for this class
> > to hold the trace object instead of adding the GetTrace method on every
> > Target object out there even though most targets will have that null. I
> > don't know if that will really be the case, but I think it's something
> > worth keeping in mind as you work on the subsequent patches.
> >
> > Very good remark. Probably we'll end up doing that when we start
> > implementing reverse debugging. The tricky thing we'll need to solve in
> > the future is seamlessly transition between a trace and a live process.
> > For example, when reverse debugging a live process, you might be stopped
> > at a breakpoint in the trace, then you do "continue", it reaches the end
> > of the trace, and then it resumes the live process. I still haven't
> > decided what we'll propose for this, but probably we'll have to change a
> > lot of the current code to make that happen nicely.
>
> Yes, that will be interesting. Even more interesting will be the
> question of how to communicate to the user the fact that even though we
> have "unwound" the PC to the previous location, variables and memory
> still contain the "live" values. Particularly, once we support rr-style
> reverse debugging which will "unwind" memory as well..
>
> >
> >> - I am puzzled by the TraceXXX vs. TraceXXXSettingsParser duality. The
> > two classes are very tightly coupled, so it's not clear to me what is
> > the advantage of separating it out this way (one could just move all the
> > relevant methods directly into the Trace class. What's the reason for
> > this design?
> >
> > Well, there's definitely coupling, but there are two reasons. One is
> > that the Trace of a live process won't need any parsing. Parsing is only
> > used when loading a trace from a json file. That makes parsing one of
> > the two main ways to create a Trace. And besides, I think that the
> > single responsibility principle is good to follow. The Parser class does
> > the parsing, and the Trace class holds an actual trace.
>
> Yeah, I'm all for the single responsibility principle, but the way it is
> implemented gives me pause. Like, if the Parser is supposed to "create"
> the Trace, then why does the code do it the other way around
> (Trace::CreateParser)? For one, that means that it will be possible to
> "parse" (by calling CreateParser()->...) any trace that you can get your
> hands on, even that of a live process. Ideally, the Parser would be the
> one creating a Trace object (in it's Parse function, or something), and
> after the Trace is created, it should no longer be possible to obtain
> the parser. All of this could be encapsulated in the "create_callback"
> that the plugin registers, and so the fact that there is a parser class
> involved would be completely unknown to the rest of the code. And then
> there could be a different create_callback which would create a Trace
> object suitable for tracing a live process...
>
> pl
>


-- 
- Walter Erquínigo Pezo
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20201002/c93ae2a2/attachment.html>