[Lldb-commits] [PATCH] D107669: [trace] [intel pt] Create a "process trace save" command

[hanbing wang via Phabricator via lldb-commits]

hanbingwang added inline comments.


================
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileSaver.cpp:87
+llvm::Expected<JSONTraceSessionBase>
+TraceIntelPTSessionFileSaver::BuildProcessesSection(lldb::ProcessSP &process_sp,
+                                                    TraceIntelPT &trace_ipt,
----------------
wallace wrote:
> you could move BuildProcessesSection, BuildThreadsSection and BuildModulesSection to TraceSessionSaver.h/cpp if you provide a callback that receives a thread_id and returns a raw trace. Then almost all the code would be reusable by other Trace plug-ins
Sorry I did not make changes here. A bit unsure how to "provide a callback that receives a thread_id and returns a raw trace" and how would it work?


================
Comment at: lldb/test/API/commands/trace/TestTraceSave.py:34-36
+        self.expect("trace load -v " +
+            os.path.join(self.getSourceDir(), "intelpt-trace", "trace.json"),
+            substrs=["intel-pt"])
----------------
wallace wrote:
> now that we are changing to live processes, this won't work. Instead, trace one of the existing binary files in the test folder, run it, get the first and last 20 instructions, save the contents in strings, then save the trace, then load it and dump the instructions again, finally assert that the outputs are the same
I'm not sure how to "save the contents in strings"?


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

https://reviews.llvm.org/D107669