[Lldb-commits] [PATCH] D88769: [trace] Scaffold "thread trace dump instructions"

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG26d861cbbd5f: [trace] Scaffold thread trace dump instructions (authored by Walter Erquinigo wall...@fb.com). Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D88769: [trace] Scaffold "thread trace dump instructions"

labath accepted this revision. labath added inline comments. Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp:61 + const std::vector ) { + TraceSP trace_instance(new TraceIntelPT(pt_cpu, targets)); + for (const TargetSP _sp :

[Lldb-commits] [PATCH] D88769: [trace] Scaffold "thread trace dump instructions"

wallace updated this revision to Diff 297373. wallace added a comment. - Moved the thread dumping logic to Trace.h. - Did the small fixes that were requested. - Left a comment regarding the shared_ptr instantiation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D88769: [trace] Scaffold "thread trace dump instructions"

wallace marked 3 inline comments as done. wallace added inline comments. Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp:61 + const std::vector ) { + TraceSP trace_instance(new TraceIntelPT(pt_cpu, targets)); + for (const

[Lldb-commits] [PATCH] D88769: [trace] Scaffold "thread trace dump instructions"

wallace added inline comments. Comment at: lldb/source/Target/Thread.cpp:1617-1627 +void Thread::DumpTraceInstructions(Stream , size_t count, + size_t start_position) const { + if (!GetProcess()->GetTarget().GetTrace()) { +s << "error: no

[Lldb-commits] [PATCH] D88769: [trace] Scaffold "thread trace dump instructions"

labath added a comment. I was waiting for the dependant patch to take shape. This looks ok to me -- just one small design question. Comment at: lldb/include/lldb/Target/Target.h:1120 + /// The trace object. It might be undefined. + lldb::TraceSP (); +

[Lldb-commits] [PATCH] D88769: [trace] Scaffold "thread trace dump instructions"

wallace updated this revision to Diff 297127. wallace added a comment. Rebase and made some cosmetic fixes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88769/new/ https://reviews.llvm.org/D88769 Files: lldb/include/lldb/Target/Target.h

[Lldb-commits] [PATCH] D88769: [trace] Scaffold "thread trace dump instructions"

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. LGTM. Pavel please chime in if you have any issues. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88769/new/

[Lldb-commits] [PATCH] D88769: [trace] Scaffold "thread trace dump instructions"

wallace updated this revision to Diff 296866. wallace edited the summary of this revision. wallace added a comment. Addressed comments, which includse: - Implemented the requested repeat command logic, including tests - Now showing the total number of instructions as part of the dump command,

[Lldb-commits] [PATCH] D88769: [trace] Scaffold "thread trace dump instructions"

wallace added inline comments. Comment at: lldb/source/Plugins/Process/Trace/ProcessTrace.h:18 + +class ProcessTrace : public lldb_private::Process { +public: tatyana-krasnukha wrote: > clayborg wrote: > > So one issue is how do we eventually deal with debugging

[Lldb-commits] [PATCH] D88769: [trace] Scaffold "thread trace dump instructions"

tatyana-krasnukha added inline comments. Comment at: lldb/source/Plugins/Process/Trace/ProcessTrace.h:18 + +class ProcessTrace : public lldb_private::Process { +public: clayborg wrote: > So one issue is how do we eventually deal with debugging a live process

[Lldb-commits] [PATCH] D88769: [trace] Scaffold "thread trace dump instructions"

labath added a comment. In D88769#2314164 , @labath wrote: > In D88769#2312482 , @wallace wrote: > >> - Comments on the architecture of classes are in that diff > > Where would that be? I couldn't find anything

[Lldb-commits] [PATCH] D88769: [trace] Scaffold "thread trace dump instructions"

labath added a comment. In D88769#2312482 , @wallace wrote: > - Comments on the architecture of classes are in that diff Where would that be? I couldn't find anything that would answer my question from the previous comment? The reason I want to know

[Lldb-commits] [PATCH] D88769: [trace] Scaffold "thread trace dump instructions"

clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed. Comment at: lldb/source/Commands/CommandObjectThread.cpp:2231-2240 + CommandObjectTraceDumpInstructions(CommandInterpreter ) + :

[Lldb-commits] [PATCH] D88769: [trace] Scaffold "thread trace dump instructions"

wallace updated this revision to Diff 296254. wallace added a comment. Rebased on top of https://reviews.llvm.org/D88841 - Comments on the architecture of classes are in that diff - I was able to get rid of cross-plug-in dependencies Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[Lldb-commits] [PATCH] D88769: [trace] Scaffold "thread trace dump instructions"

labath added a comment. Before going into the code details, could you give a high-level overview of the intended interactions between the Trace and ProcessTrace classes? I am thinking about how to organize this so that we don't have too many criss-cross dependencies. My main question is

[Lldb-commits] [PATCH] D88769: [trace] Scaffold "thread trace dump instructions"

wallace updated this revision to Diff 296106. wallace added a comment. Herald added a subscriber: dexonsmith. Address comments: - Created a basic ThreadTrace class instead of using HistoryThread. I'll modify this class later when I add the decoding functionality and refactor the json file

[Lldb-commits] [PATCH] D88769: [trace] Scaffold "thread trace dump instructions"

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. See inline comments about "thread trace dump instructions" as I am not quite sure what --offset means. After we load a trace file, what is the default "offset" value? Is there a

[Lldb-commits] [PATCH] D88769: [trace] Scaffold "thread trace dump instructions"

wallace created this revision. wallace added reviewers: clayborg, labath. Herald added subscribers: lldb-commits, dang, mgorny. Herald added a reviewer: JDevlieghere. Herald added a project: LLDB. wallace requested review of this revision. As per the discussion in the RFC, we'll implement both