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

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D85705#2211073 <https://reviews.llvm.org/D85705#2211073>, @vsk wrote:

> This looks very cool, thanks @clayborg! I think using JSON to describe the trace data (what kind of trace is this, what's in it, etc.) sounds reasonable.
>
>> For "trace load", I get the plugin for the JSON file by matching it up with the "name" field in the JSON, but I don't store the "trace_sp" anywhere. We will need to store it with the target that we create, or for later commands add it to a target that is stopped when the trace data is loaded via the process interface (through lldb-server is the current thinking for this).
>
> Have you considered what might happen if there are multiple targets covered by a single trace? Strawman proposal: would it make sense to register the trace with a Debugger instance? This can be a list of traces if it makes sense to support debugging more than one trace at a time.

I hadn't thought of that but that does make sense!

We can work this all into the JSON format. We should actually make a schema for the common parts of information that should be represented in the JSON and also allow each plug-in to supply a schema for the parts that is requires in the JSON.

Some ideas that this information should contain:

- array of process infos dictionaries
- process info dictionary
  - pid
  - array of shared library dictionaries
- shared library dictionary:
  - original path
  - UUID if available
  - MD5 of file if no UUID
  - load location
  - optional URL to download

And LLDB will easily be able to load up N targets with everything setup correctly.

>> "trace dump" does nothing for now, but this is what we can use to test that "trace load" worked and was able to create a target.
>
> It'd be great to have some test for this, even if all 'trace load' does at this point is print an error about bad JSON input.

I agree! If we like this format, Walter Erquinigo can commandeer this revision and fill in actual Intel PT guts and have this work. The idea is I am going to get the infrastructure in place and once we work this out, I will let Walter take over the patch and actually fill it in with real Intel PT stuff and test it fully.



================
Comment at: lldb/source/Commands/Options.td:206
+    Required, Completion<"SourceFile">,
+    Desc<"A specifier in the form filename:line[:column] for setting file & line breakpoints.">;
   /* Don't add this option till it actually does something useful...
----------------
vsk wrote:
> nit: seems like an accidental change here?
yeah, my editor removes trailing spaces. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85705/new/

https://reviews.llvm.org/D85705