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 &interpreter)
+ : CommandObjectIterateOverThreads(
+ interpreter, "thread trace dump instructions",
+ "Dump the traced instructions for one or more threads. If no "
+ "threads are specified, show the current thread. Use the "
+ "thread-index \"all\" to see all threads.",
+ nullptr,
----------------
This command does seem hard to abstract over all flavors of trace. Would it be
better to have the trace plug-ins vend a command objects for "thread trace
dump"? The idea would be something like "hey trace plug-in, do you have any
commands you support for the 'thread trace dump' multiword command?". It could
say "yes, I have one called 'instructions' and here is the CommandObjectSP
================
Comment at: lldb/source/Commands/CommandObjectThread.cpp:2245
+ Options *GetOptions() override { return &m_options; }
+
+protected:
----------------
We should override the repeat command method here:
```
const char *GetRepeatCommand(Args ¤t_command_args, uint32_t index)
override;
```
This way if you type:
```
(lldb) thread trace dump instruction --offset 0 --count 32
```
And then hit enter, the next command it should run is:
```
(lldb) thread trace dump instruction --offset 32 --count 32
```
That you can keep dumping more instructions by just hitting ENTER.
================
Comment at: lldb/source/Commands/Options.td:1008
+let Command = "thread trace dump instructions" in {
+ def thread_trace_dump_instructions_count : Option<"count", "c">, Group<1>,
----------------
Do we add a "--summary" option to do this command so that when this option is
specified would dump the number of instructions that any threads from the might
have? Like:
```
(lldb) thread trace dump instructions --summary
thread #1 has 4096 instructions
thread #2 has 234 instructions
```
================
Comment at: lldb/source/Commands/Options.td:1017-1020
+ Desc<"The position of the first instruction to print. Defaults to the "
+ "current position. The instructions are indexed in reverse order, which "
+ "means that a start position of 0 refers to the last instruction "
+ "chronologically.">;
----------------
Is there a "current position" that is maintained? Or is the current position
where TraceThread currently is stopped?
================
Comment at: lldb/source/Plugins/Process/Trace/ProcessTrace.h:18
+
+class ProcessTrace : public lldb_private::Process {
+public:
----------------
So one issue is how do we eventually deal with debugging a live process that
enables tracing. In that case we already have a real process class:
ProcessGDBRemote most likely. We should avoid putting anything custom that is
required from a process in this ProcessTrace class for when we actually have a
real process class already. If we need to add anything, we will need to have
virtual functions on the lldb_private::Process class that can call through to
the Trace plug-in via its virtual functions as well to implement any
functionality we might need.
Is this class solely going to be used for "trace load"?
================
Comment at: lldb/test/API/commands/trace/TestTraceDumpInstructions.py:46
+ self.expect("thread trace dump instructions --count 5 --start-position
10",
+ substrs=['thread #1: tid = 3842849',
+ 'would print 5 instructions from position 10'])
----------------
need to test the repeat command stuff here if we add support for:
```
const char *GetRepeatCommand(Args ¤t_command_args, uint32_t index)
override {
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88769/new/
https://reviews.llvm.org/D88769
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
