[trace][intel pt] Implement the Intel PT cursor

[wallace]

Summary

D104422 added the interface for TraceCursor, which is the main way to traverse instructions in a trace. This diff implements the corresponding cursor class for Intel PT and deletes the now obsolete code.

Besides that, the logic for the "thread trace dump instructions" was adapted to use this cursor (pretty much I ended up moving code from Trace.cpp to TraceCursor.cpp). The command by default traverses the instructions backwards, and if the user passes --forwards, then it's not forwards. More information about that is in the Options.td file. An interesting change is that I'm printing positive indices when traversing forwards, and negative ones when doing it backwards. They do help readability and don't require to know the total instruction count. You can check the tests to see that.

Regarding the Intel PT cursor. All Intel PT cursors for the same thread share the same DecodedThread instance. I'm not yet implementing lazy decoding because we don't need it. That'll be for later. For the time being, the entire thread trace is decoded when the first cursor for that thread is requested.

[wallace]

nit

[wallace]

improve documentation

[clayborg]

lldb/include/lldb/Target/TraceCursor.h

68–69

move this above destructor above.

108–119

Some of these are out of order (skip and count), or missing (direction).

120

Should we just use a bool for direction? There can't be any direction other than forward or backward.

120–121

Remove the "size_t skip" from this API. It isn't needed because the user can move the cursor around before they call this API to skip or position the cursor as needed, then call this function.

130–137

This description is pretty vague. It should probably be removed or fully documented to state exactly what is expected of trace clients. This also seems like something that might be returned when moving the cursor around instead of a separate method? From the description, I am unclear as to what I should expect from traversing backwards. What happens when I call SeekToEnd(), or SeekToBegin()? What happens if I call Next(...) or Prev(...) when the granularity is set in a specific way?

134

Probably best to store this as a: ExecutionContextRef m_exe_ref. See description of ExecutionContextRef in ExecutionContext.h for reasons why. The main issue is we don't want a TraceCursor to keep a strong reference to a thread and keep the thread and possibly the process object alive. Keeping a weak reference is safer and allows you to try to fetch the thread when needed by calling ExecutionContextRef::GetThreadSP()

lldb/include/lldb/lldb-enumerations.h

962–966 ↗

(On Diff #356875)

Unless there is another direction, I would just switch this to a bool and remove this from the public API. Fine to use it internally if needed.

lldb/source/Commands/CommandObjectThread.cpp

2103–2107

take care of skip on the cursor _before_ calling this API, and remove skip argument.

2104–2105

just pass a bool? Enum seems a bit much for a simple bool argument. Unless there is going to be another direction in the future, but I can't think of one.

lldb/source/Plugins/Trace/intel-pt/DecodedThread.h

121

You only need to inherit from "std::enable_shared_from_this" if you ever need to take a "DecodedThread *" and get ahold of the original std::shared_ptr<DecodedThread>. Is that the case here?

[wallace]

lldb/source/Plugins/Trace/intel-pt/DecodedThread.h

121

i'm using it for calling shared_from_this inside DecodedThread.cpp, which is very handy

[wallace]

Addressed all issues

• Created a TraceInstructionDumper class that keeps the index to print, which leaves the TraceCursor unaffected. It also keeps some other useful state.

[clayborg]

lldb/source/Commands/CommandObjectThread.cpp

2111

"Thread *" isn't a stable thing to use, I would suggest using the "tid" of the thread. Any reason you are making a map of thread to unique pointers? Can't this just be:

std::map<lldb::tid_t, TraceInstructionDumper> m_dumpers;

lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp

41

Shouldn't the m_pt_insn.ip be set to LLDB_INVALID_ADDRESS already?

lldb/source/Target/TraceInstructionDumper.cpp

1

Filename should be TraceInstructionDumper.cpp

22

Seems like the "forwards" and "skip" parameter shouldn't be in this class at all? Just setup the cursor before giving it to this class?

25–30

Remove and have user set this up prior to creating one of these classes?

35

Should we modify the TraceCursor API for Next(...) to be able to take a count? Seems like that might be much more efficient than recursively calling this function?

41

Ditto, should Prev(...) be able to take a count as well?

156–159

to match llvm coding guidelines, add '=' after variable name

167–171

Ditto about adding '=', same as above comment.

291

You should be watching the return value of this right? What if this returns false?

[wallace]

lldb/source/Commands/CommandObjectThread.cpp

2111

I would suggest using the "tid" of the thread -> makes sense

The cursor is a Unique pointer, which renders the TraceInstructionDumper not copyable.

lldb/source/Target/TraceInstructionDumper.cpp

22

the direction is useful to knowing whether to call Next or Prev in the iterations

the skip is also useful to have inside, because that way it's easier to know if all the data has been consumed by the skip. That's stored in the m_no_more_data flag. When a trace has been fully consumed, it's still pointing to the last instruction, so another variable is needed to prevent more iterations

35

that's a pretty good idea

[wallace]

lldb/source/Target/TraceInstructionDumper.cpp

291

that's taken care of by

if (m_no_more_data) {
   s.Printf("    no more data\n");
   break;
 }

in the beginning of the iteration

[wallace]

Address comments.

• I ended up creating new method Next(int count) and Prev(int count) in the cursor for the skip operation. It might be useful for other things. I didn't add the count parameter to the existing Prev and Next methods because it makes it ambiguous because of the
• I got skip out of the constructor of the Dumper and now the dumper doesn't reset the initial position of the cursor. The cursor is set up outside.
• Other improvements here and there

[clayborg]

Check my current comments before I proceed to look at the rest. Now that we are using the API for something, it is showing some issues with the usability of the API in TraceCursor. Let me know what you think

lldb/include/lldb/Target/TraceCursor.h

85–94

Since we haven't made the trace cursor API public yet, I wonder if this would be a better interface:

virtual void SetGranularity(lldb::TraceInstructionControlFlowType granularity);
virtual void SetIgnoreErrors(bool ignore_errors);
virtual void SetDirection(bool forward);
virtual size_t Move(size_t count);

Then we don't need Next(...) and Prev(...) now since the direction is set via an accessor. The Move(...) function would advance "count" times and it would obey the the granularity that is currently active. It also simplifies all of the arguments that used to be needed to be passed to the Next(...) and Prev(...) functions and gets rid of the default parameters.

94–100

Wondering if we want to replace these with more of a file like seek with a offset and whence?:

enum class SeekType { Set, Current, End };
bool Seek(int64_t offset, SeekType whence);

Then SeekToEnd() would become:

Seek(0, SeekType::End);

And SeekToBegin():

Seek(0, SeekType::Set);

And to skip some instructions:

Seek(10, SeekType::Current); // Advance in the direction by 10 granularity units
Seek(-10, SeekType::Current); // Reverse in the direction by 10 granularity units

97

clarify that this would move "count" times the granularity right?

[wallace]

Followed the recommendations. Now I'm making sure that TraceInstructionDumper uses the state of the Cursor and doesn't modify it, this makes the class reusable for other purposes. Having the direction and granularity as part of the cursor makes it more useful.

I also made the Seek method resemble the fseek function.

[clayborg]

Very close! Just a few nits.

lldb/include/lldb/Target/TraceCursor.h

77

If there is no other direction other than forward to backward, should this just be a bool? Then we won't have to add the enum to the public API when we expose this.

81

bool? Fine if you think the enum should be used.

101–102

If we get an error, we should return less than the requested amount. If you ask for 10, and only get 7, but get an error, you don't want to return 8. So I would vote for documentation and implementation like:

The number of successful moves that occurred. If the returned value is less than \a count, then check for an error condition by calling XXX. If there is no error and less than \a count was returned, then the cursor is at the end or beginnning of the data.

lldb/source/Target/TraceCursor.cpp

20–35

All these can be moved to the header file. We don't do this in the public API, but we can do this in private classes.

[wallace]

• Nwo using a single-step iterator method Next
• Changed the direction to a boolean flag called "forwards"

[clayborg]

Just switch all "-f" to the long option format as mentioned in inlined comments and this is good to go.

lldb/test/API/commands/trace/TestTraceDumpInstructions.py

38–40

use long form of option for clarity

63

use long form of option for clarity

146–147

use long form of option for clarity

165–168

use long form of option for clarity

173–175

use long form of option for clarity

195–196

use long form of option for clarity

340

use long form of option for clarity

lldb/test/API/commands/trace/TestTraceStartStop.py

114–115

use long form of option for clarity

121–122

use long form of option for clarity

152

use long form of option for clarity

[wallace]

fix nits

lldb/include/lldb/Target/Trace.h

 /// this information.
 ///
 /// In order to support live tracing, the name of the plug-in should match the
 /// name of the tracing type returned by the gdb-remote packet
 /// \a jLLDBTraceSupported.
 class Trace : public PluginInterface,
               public std::enable_shared_from_this<Trace> {
 public:
-  enum class TraceDirection {
-    Forwards = 0,
-    Backwards,
-  };
-
   /// Dump the trace data that this plug-in has access to.
   ///
   /// This function will dump all of the trace data for all threads in a user
   /// readable format. Options for dumping can be added as this API is iterated
   /// on.
   ///
   /// \param[in] s
   ///     A stream object to dump the information to.
public:
   /// Get the command handle for the "thread trace start" command.
   virtual lldb::CommandObjectSP
   GetThreadTraceStartCommand(CommandInterpreter &interpreter) = 0;
 
   /// \return
   ///     The JSON schema of this Trace plug-in.
   virtual llvm::StringRef GetSchema() = 0;
 
-  /// Dump \a count instructions of the given thread's trace ending at the
-  /// given \a end_position position.
-  ///
-  /// The instructions are printed along with their indices or positions, which
-  /// are increasing chronologically. This means that the \a index 0 represents
-  /// the oldest instruction of the trace chronologically.
-  ///
-  /// \param[in] thread
-  ///     The thread whose trace will be dumped.
-  ///
-  /// \param[in] s
-  ///     The stream object where the instructions are printed.
-  ///
-  /// \param[in] count
-  ///     The number of instructions to print.
-  ///
-  /// \param[in] end_position
-  ///     The position of the last instruction to print.
-  ///
-  /// \param[in] raw
-  ///     Dump only instruction addresses without disassembly nor symbol
-  ///     information.
-  void DumpTraceInstructions(Thread &thread, Stream &s, size_t count,
-                             size_t end_position, bool raw);
-
-  /// Run the provided callback on the instructions of the trace of the given
-  /// thread.
-  ///
-  /// The instructions will be traversed starting at the given \a position
-  /// sequentially until the callback returns \b false, in which case no more
-  /// instructions are inspected.
-  ///
-  /// The purpose of this method is to allow inspecting traced instructions
-  /// without exposing the internal representation of how they are stored on
-  /// memory.
-  ///
-  /// \param[in] thread
-  ///     The thread whose trace will be traversed.
-  ///
-  /// \param[in] position
-  ///     The instruction position to start iterating on.
-  ///
-  /// \param[in] direction
-  ///     If \b TraceDirection::Forwards, then then instructions will be
-  ///     traversed forwards chronologically, i.e. with incrementing indices. If
-  ///     \b TraceDirection::Backwards, the traversal is done backwards
-  ///     chronologically, i.e. with decrementing indices.
-  ///
-  /// \param[in] callback
-  ///     The callback to execute on each instruction. If it returns \b false,
-  ///     the iteration stops.
-  virtual void TraverseInstructions(
-      Thread &thread, size_t position, TraceDirection direction,
-      std::function<bool(size_t index, llvm::Expected<lldb::addr_t> load_addr)>
-          callback) = 0;
-
   /// Get a \a TraceCursor for the given thread's trace.
   ///
   /// \return
   ///     A \a TraceCursorUP. If the thread is not traced or its trace
   ///     information failed to load, the corresponding error is embedded in the
   ///     trace.
   virtual lldb::TraceCursorUP GetCursor(Thread &thread) = 0;
 
-  /// Get the number of available instructions in the trace of the given thread.
-  ///
-  /// \param[in] thread
-  ///     The thread whose trace will be inspected.
-  ///
-  /// \return
-  ///     The total number of instructions in the trace, or \a llvm::None if the
-  ///     thread is not being traced.
-  virtual llvm::Optional<size_t> GetInstructionCount(Thread &thread) = 0;
-
   /// Check if a thread is currently traced by this object.
   ///
   /// \param[in] thread
   ///     The thread in question.
   ///
   /// \return
   ///     \b true if the thread is traced by this instance, \b false otherwise.
   virtual bool IsTraced(const Thread &thread) = 0;

lldb/include/lldb/Target/TraceCursor.h

 //===-- TraceCursor.h -------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_TARGET_TRACE_CURSOR_H
 #define LLDB_TARGET_TRACE_CURSOR_H
 
 #include "lldb/lldb-private.h"
 
+#include "lldb/Target/ExecutionContext.h"
+
 namespace lldb_private {
 
 /// Class used for iterating over the instructions of a thread's trace.
 ///
 /// This class attempts to be a generic interface for accessing the instructions
 /// of the trace so that each Trace plug-in can reconstruct, represent and store
 /// the instruction data in an flexible way that is efficient for the given
 /// technology.

 ///       std::endl;
 ///     else if (cursor->GetInstructionControlFlowType() &
 ///     eTraceInstructionControlFlowTypeReturn)
 ///       std::cout << "return found at " << cursor->GetLoadAddress() <<
 ///       std::endl;
 ///  } while(cursor->Prev(granularity));
 class TraceCursor {
 public:
+  TraceCursor(lldb::ThreadSP thread_sp);
+
   virtual ~TraceCursor() = default;
 
   /// Move the cursor to the next instruction more recent chronologically in the
   /// trace given the provided granularity. If such instruction is not found,
   /// the cursor doesn't move.
   ///
   /// \param[in] granularity
   ///     Bitmask granularity filter. The cursor stops at the next
   ///     instruction that matches the specified granularity.
   ///
   /// \param[in] ignore_errors
   ///     If \b false, the cursor stops as soon as it finds a failure in the
   ///     trace and points at it.
   ///
   /// \return
   ///     \b true if the cursor effectively moved and now points to a different
   ///     item in the trace, including errors when \b ignore_errors is \b false.
   ///     In other words, if \b false is returned, then the trace is pointing at
   ///     the same item in the trace as before.
   virtual bool Next(lldb::TraceInstructionControlFlowType granularity =
                         lldb::eTraceInstructionControlFlowTypeInstruction,
                     bool ignore_errors = false) = 0;
 
   /// Similar to \a TraceCursor::Next(), but moves backwards chronologically.
   virtual bool Prev(lldb::TraceInstructionControlFlowType granularity =
                         lldb::eTraceInstructionControlFlowTypeInstruction,
                     bool ignore_errors = false) = 0;
 
   /// Force the cursor to point to the end of the trace, i.e. the most recent
   /// item.
   virtual void SeekToEnd() = 0;
 
   /// Force the cursor to point to the beginning of the trace, i.e. the oldest
   /// item.
   virtual void SeekToBegin() = 0;
 
   /// \return
-  ///   \b true if the trace corresponds to a live process who has resumed after
-  ///   the trace cursor was created. Otherwise, including the case in which the
-  ///   process is a post-mortem one, return \b false.
-  bool IsStale();
+  ///   The \a ExecutionContextRef of the backing thread from the creation time
+  ///   of this cursor.
+  ExecutionContextRef &GetExecutionContextRef();
 
   /// Instruction or error information
   /// \{
 
   /// Get the corresponding error message if the cursor points to an error in
   /// the trace.
   ///
   /// \return
   ///     \b llvm::Error::success if the cursor is not pointing to an error in
   ///     the trace. Otherwise return an \a llvm::Error describing the issue.
   virtual llvm::Error GetError() = 0;
 
   /// \return
   ///     The load address of the instruction the cursor is pointing at. If the
   ///     cursor points to an error in the trace, return \b
   ///     LLDB_INVALID_ADDRESS.
   virtual lldb::addr_t GetLoadAddress() = 0;
 
   /// \return
   ///     The \a lldb::TraceInstructionControlFlowType categories the
   ///     instruction the cursor is pointing at falls into. If the cursor points
   ///     to an error in the trace, return \b 0.
   virtual lldb::TraceInstructionControlFlowType
   GetInstructionControlFlowType() = 0;
-
   /// \}
 
-private:
-  /// The stop ID when the cursor was created.
-  uint32_t m_stop_id = 0;
-  /// The trace that owns this cursor.
-  lldb::TraceSP m_trace_sp;
+protected:
+  ExecutionContextRef m_exe_ctx_ref;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_TARGET_TRACE_CURSOR_H

lldb/include/lldb/Target/TraceInstructionDumper.h

+//===-- TraceInstructionDumper.h --------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Target/TraceCursor.h"
+
+#ifndef LLDB_TARGET_TRACE_INSTRUCTION_DUMPER_H
+#define LLDB_TARGET_TRACE_INSTRUCTION_DUMPER_H
+
+namespace lldb_private {
+
+class TraceInstructionDumper {
+public:
+  /// \param[in] cursor
+  ///     The cursor whose instruction will be dumped
+  ///
+  /// \param[in] forwards
+  ///     If \b true, the instructions are traversed forwards from the oldest.
+  ///     Otherwise, backwards from the most recent.
+  ///
+  /// \param[in] skip
+  ///     The number of instruction to skip before printing.
+  ///
+  /// \param[in] raw
+  ///     Dump only instruction addresses without disassembly nor symbol
+  ///     information.
+  TraceInstructionDumper(lldb::TraceCursorUP &&cursor_up, bool forwards,
+                         size_t skip, bool raw);
+
+  /// Dump \a count instructions of the thread trace starting at the current
+  /// cursor position.
+  ///
+  /// This effectively moves the cursor to the next unvisited position.
+  ///
+  /// \param[in] s
+  ///     The stream object where the instructions are printed.
+  ///
+  /// \param[in] count
+  ///     The number of instructions to print.
+  void DumpInstructions(Stream &s, size_t count);
+
+private:
+  bool TryMoveOneInstruction();
+
+  /// Skip \p count instructions in the direction specified in the constructor.
+  void Skip(size_t count);
+
+  lldb::TraceCursorUP m_cursor_up;
+  /// Whether to traverse the trace forwards of backwards.
+  bool m_forwards;
+  bool m_raw;
+  /// If \b true, all the instructions have been traversed.
+  bool m_no_more_data = false;
+  /// Instruction index for presentation purposes. It increases if the traversal
+  /// is done forwards, and decreases otherwise.
+  int m_index = 0;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_TARGET_TRACE_INSTRUCTION_DUMPER_H

lldb/source/Commands/CommandObjectThread.cpp

 //===-- CommandObjectThread.cpp -------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "CommandObjectThread.h"
 
+#include <memory>
 #include <sstream>
 
 #include "CommandObjectThreadUtil.h"
 #include "CommandObjectTrace.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Host/OptionParser.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Interpreter/OptionArgParser.h"
 #include "lldb/Interpreter/OptionGroupPythonClassWithDict.h"
 #include "lldb/Interpreter/Options.h"
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/LineEntry.h"
 #include "lldb/Symbol/LineTable.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/RegisterContext.h"
 #include "lldb/Target/SystemRuntime.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/Thread.h"
 #include "lldb/Target/ThreadPlan.h"
 #include "lldb/Target/ThreadPlanStepInRange.h"
 #include "lldb/Target/Trace.h"
+#include "lldb/Target/TraceInstructionDumper.h"
 #include "lldb/Utility/State.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 // CommandObjectThreadBacktrace
 #define LLDB_OPTIONS_thread_backtrace
 #include "CommandOptions.inc"
Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
             count < 0)
           error.SetErrorStringWithFormat(
               "invalid integer value for option '%s'",
               option_arg.str().c_str());
         else
           m_count = count;
         break;
       }
-      case 'p': {
-        int32_t position;
-        if (option_arg.empty() || option_arg.getAsInteger(0, position) ||
-            position < 0)
+      case 's': {
+        int32_t skip;
+        if (option_arg.empty() || option_arg.getAsInteger(0, skip) || skip < 0)
           error.SetErrorStringWithFormat(
               "invalid integer value for option '%s'",
               option_arg.str().c_str());
         else
-          m_position = position;
+          m_skip = skip;
         break;
       }
       case 'r': {
         m_raw = true;
         break;
       }
+      case 'f': {
+        m_forwards = true;
+        break;
+      }
       default:
         llvm_unreachable("Unimplemented option");
       }
       return error;
     }
 
     void OptionParsingStarting(ExecutionContext *execution_context) override {
       m_count = kDefaultCount;
-      m_position = llvm::None;
+      m_skip = 0;
       m_raw = false;
+      m_forwards = false;
     }
 
     llvm::ArrayRef<OptionDefinition> GetDefinitions() override {
       return llvm::makeArrayRef(g_thread_trace_dump_instructions_options);
     }
 
     static const size_t kDefaultCount = 20;
 
     // Instance variables to hold the values for command options.
     size_t m_count;
-    llvm::Optional<ssize_t> m_position;
+    size_t m_skip;
     bool m_raw;
+    bool m_forwards;
   };
 
   CommandObjectTraceDumpInstructions(CommandInterpreter &interpreter)
       : CommandObjectIterateOverThreads(
             interpreter, "thread trace dump instructions",
-            "Dump the traced instructions for one or more threads.  If no "
+            "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,
             eCommandRequiresProcess | eCommandTryTargetAPILock |
                 eCommandProcessMustBeLaunched | eCommandProcessMustBePaused |
                 eCommandProcessMustBeTraced),
         m_options(), m_create_repeat_command_just_invoked(false) {}
 
   ~CommandObjectTraceDumpInstructions() override = default;
 
   Options *GetOptions() override { return &m_options; }
 
   const char *GetRepeatCommand(Args &current_command_args,
                                uint32_t index) override {
     current_command_args.GetCommandString(m_repeat_command);
     m_create_repeat_command_just_invoked = true;
-    m_consecutive_repetitions = 0;
     return m_repeat_command.c_str();
   }
 
 protected:
   bool DoExecute(Args &args, CommandReturnObject &result) override {
-    if (IsRepeatCommand())
-      m_consecutive_repetitions++;
+    if (!IsRepeatCommand())
+      m_dumpers.clear();
+
     bool status = CommandObjectIterateOverThreads::DoExecute(args, result);
 
     m_create_repeat_command_just_invoked = false;
     return status;
   }
 
   bool IsRepeatCommand() {
     return !m_repeat_command.empty() && !m_create_repeat_command_just_invoked;
   }
 
   bool HandleOneThread(lldb::tid_t tid, CommandReturnObject &result) override {
     const TraceSP &trace_sp = m_exe_ctx.GetTargetSP()->GetTrace();
     ThreadSP thread_sp =
         m_exe_ctx.GetProcessPtr()->GetThreadList().FindThreadByID(tid);
 
-    if (llvm::Optional<size_t> insn_count =
-            trace_sp->GetInstructionCount(*thread_sp)) {
-      size_t count = m_options.m_count;
-      ssize_t position =
-          m_options.m_position.getValueOr((ssize_t)*insn_count - 1) -
-          m_consecutive_repetitions * count;
-      if (position < 0)
-        result.AppendError("error: no more data");
-      else
-        trace_sp->DumpTraceInstructions(*thread_sp, result.GetOutputStream(),
-                                        count, position, m_options.m_raw);
-    } else {
-      result.AppendError("error: not traced");
-    }
+    if (!m_dumpers.count(thread_sp.get()))
+      m_dumpers[thread_sp.get()] = std::make_unique<TraceInstructionDumper>(
+          trace_sp->GetCursor(*thread_sp), m_options.m_forwards,
+          m_options.m_skip, m_options.m_raw);
+
+    m_dumpers[thread_sp.get()]->DumpInstructions(result.GetOutputStream(),
+                                                 m_options.m_count);
     return true;
   }
 
   CommandOptions m_options;
 
   // Repeat command helpers
   std::string m_repeat_command;
   bool m_create_repeat_command_just_invoked;
-  size_t m_consecutive_repetitions = 0;
+  std::map<const Thread *, std::unique_ptr<TraceInstructionDumper>> m_dumpers;
 };
 
 // CommandObjectMultiwordTraceDump
 class CommandObjectMultiwordTraceDump : public CommandObjectMultiword {
 public:
   CommandObjectMultiwordTraceDump(CommandInterpreter &interpreter)
       : CommandObjectMultiword(
             interpreter, "dump",

lldb/source/Commands/Options.td

let Command = "thread plan list" in {
   def thread_plan_list_thread_id : Option<"thread-id", "t">, Group<1>,
     Arg<"ThreadID">, Desc<"List the thread plans for this TID, can be "
     "specified more than once.">;
   def thread_plan_list_unreported : Option<"unreported", "u">, Group<1>,
     Desc<"Display thread plans for unreported threads">;
 }
 
 let Command = "thread trace dump instructions" in {
+  def thread_trace_dump_instructions_forwards: Option<"forwards", "f">, Group<1>,
+    Desc<"If specified, the trace is traversed forwards chronologically "
+    "starting at the oldest instruction. Otherwise, it starts at the most "
+    "recent one and the traversal is backwards.">;
   def thread_trace_dump_instructions_count : Option<"count", "c">, Group<1>,
     Arg<"Count">,
-    Desc<"The number of instructions to display ending at the current position.">;
-  def thread_trace_dump_instructions_position : Option<"position", "p">,
+    Desc<"The number of instructions to display starting at the most recent "
+    "instruction, or the oldest if --forwards is provided.">;
+  def thread_trace_dump_instructions_skip: Option<"skip", "s">,
     Group<1>,
     Arg<"Index">,
-    Desc<"The position to use instead of the current position of the trace.">;
+    Desc<"How many instruction to skip from the end of the trace to start "
+    "dumping instructions, or from the beginning if --forwards is provided">;
   def thread_trace_dump_instructions_raw : Option<"raw", "r">,
     Group<1>,
     Desc<"Dump only instruction address without disassembly nor symbol information.">;
 }
 
 let Command = "type summary add" in {
   def type_summary_add_category : Option<"category", "w">, Arg<"Name">,
     Desc<"Add this to the given category instead of the default one.">;

lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt

 lldb_tablegen(TraceIntelPTCommandOptions.inc -gen-lldb-option-defs
   SOURCE TraceIntelPTOptions.td
   TARGET TraceIntelPTOptionsGen)
 
 add_lldb_library(lldbPluginTraceIntelPT PLUGIN
   CommandObjectTraceStartIntelPT.cpp
   DecodedThread.cpp
   IntelPTDecoder.cpp
+  TraceCursorIntelPT.cpp
   TraceIntelPT.cpp
   TraceIntelPTSessionFileParser.cpp
 
   LINK_LIBS
     lldbCore
     lldbSymbol
     lldbTarget
     ${LIBIPT_LIBRARY}
   LINK_COMPONENTS
     Support
   )
 
 
 add_dependencies(lldbPluginTraceIntelPT TraceIntelPTOptionsGen)

lldb/source/Plugins/Trace/intel-pt/DecodedThread.h

 //===-- DecodedThread.h -----------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_DECODEDTHREAD_H
 #define LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_DECODEDTHREAD_H
 
 #include <vector>
 
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
 
 #include "lldb/Target/Trace.h"
 #include "lldb/Utility/TraceIntelPTGDBRemotePackets.h"
 
 #include "intel-pt.h"
 
 namespace lldb_private {
 namespace trace_intel_pt {
 
 /// Class for representing a libipt decoding error.
public:
 
   /// Check if this object represents an error (i.e. a gap).
   ///
   /// \return
   ///     Whether this object represents an error.
   bool IsError() const;
 
   /// \return
-  ///     The instruction pointer address, or an \a llvm::Error if it is an
-  ///     error.
-  llvm::Expected<lldb::addr_t> GetLoadAddress() const;
+  ///     The instruction pointer address, or \a LLDB_INVALID_ADDRESS if it is
+  ///     an error.
+  lldb::addr_t GetLoadAddress() const;
 
   /// \return
   ///     An \a llvm::Error object if this class corresponds to an Error, or an
   ///     \a llvm::Error::success otherwise.
   llvm::Error ToError() const;
 
+  /// Get the \a lldb::TraceInstructionControlFlowType categories of the
+  /// instruction.
+  ///
+  /// \param[in] next_load_address
+  ///     The address of the next instruction in the trace or \b
+  ///     LLDB_INVALID_ADDRESS if not available.
+  ///
+  /// \return
+  ///     The control flow categories, or \b 0 if the instruction is an error.
+  lldb::TraceInstructionControlFlowType
+  GetControlFlowType(lldb::addr_t next_load_address) const;
+
   IntelPTInstruction(IntelPTInstruction &&other) = default;
 
 private:
   IntelPTInstruction(const IntelPTInstruction &other) = delete;
   const IntelPTInstruction &operator=(const IntelPTInstruction &other) = delete;
 
   pt_insn m_pt_insn;
   std::unique_ptr<llvm::ErrorInfoBase> m_error;
 };
 
 /// \class DecodedThread
 /// Class holding the instructions and function call hierarchy obtained from
 /// decoding a trace, as well as a position cursor used when reverse debugging
 /// the trace.
 ///
 /// Each decoded thread contains a cursor to the current position the user is
 /// stopped at. See \a Trace::GetCursorPosition for more information.
-class DecodedThread {
+class DecodedThread : public std::enable_shared_from_this<DecodedThread> {
 public:
-  DecodedThread(std::vector<IntelPTInstruction> &&instructions)
-      : m_instructions(std::move(instructions)), m_position(GetLastPosition()) {
-  }
+  DecodedThread(lldb::ThreadSP thread_sp,
+                std::vector<IntelPTInstruction> &&instructions);
 
   /// Constructor with a single error signaling a complete failure of the
   /// decoding process.
-  DecodedThread(llvm::Error error);
+  DecodedThread(lldb::ThreadSP thread_sp, llvm::Error error);
 
   /// Get the instructions from the decoded trace. Some of them might indicate
   /// errors (i.e. gaps) in the trace.
   ///
   /// \return
   ///   The instructions of the trace.
   llvm::ArrayRef<IntelPTInstruction> GetInstructions() const;
 
-  /// \return
-  ///   The current position of the cursor of this trace, or 0 if there are no
-  ///   instructions.
-  size_t GetCursorPosition() const;
-
-  /// Change the position of the cursor of this trace. If this value is to high,
-  /// the new position will be set as the last instruction of the trace.
-  ///
-  /// \return
-  ///     The effective new position.
-  size_t SetCursorPosition(size_t new_position);
-  /// \}
+  /// Get a new cursor for the decoded thread.
+  lldb::TraceCursorUP GetCursor();
 
 private:
-  /// \return
-  ///     The index of the last element of the trace, or 0 if empty.
-  size_t GetLastPosition() const;
-
+  lldb::ThreadSP m_thread_sp;
   std::vector<IntelPTInstruction> m_instructions;
-  size_t m_position;
 };
 
+using DecodedThreadSP = std::shared_ptr<DecodedThread>;
+
 } // namespace trace_intel_pt
 } // namespace lldb_private
 
 #endif // LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_DECODEDTHREAD_H

lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp

 //===-- DecodedThread.cpp -------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "DecodedThread.h"
 
+#include <intel-pt.h>
+#include <memory>
+
+#include "TraceCursorIntelPT.h"
 #include "lldb/Utility/StreamString.h"
 
+using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::trace_intel_pt;
 using namespace llvm;
 
 char IntelPTError::ID;
 
 IntelPTError::IntelPTError(int libipt_error_code, lldb::addr_t address)
     : m_libipt_error_code(libipt_error_code), m_address(address) {
   assert(libipt_error_code < 0);
 }
 
 void IntelPTError::log(llvm::raw_ostream &OS) const {
   const char *libipt_error_message = pt_errstr(pt_errcode(m_libipt_error_code));
   if (m_address != LLDB_INVALID_ADDRESS && m_address > 0) {
     write_hex(OS, m_address, HexPrintStyle::PrefixLower, 18);
     OS << "    ";
   }
   OS << "error: " << libipt_error_message;
 }
 
 bool IntelPTInstruction::IsError() const { return (bool)m_error; }
 
-Expected<lldb::addr_t> IntelPTInstruction::GetLoadAddress() const {
-  if (IsError())
-    return ToError();
-  return m_pt_insn.ip;
+lldb::addr_t IntelPTInstruction::GetLoadAddress() const {
+  return IsError() ? LLDB_INVALID_ADDRESS : m_pt_insn.ip;
 }
 
 Error IntelPTInstruction::ToError() const {
   if (!IsError())
     return Error::success();
 
   if (m_error->isA<IntelPTError>())
     return make_error<IntelPTError>(static_cast<IntelPTError &>(*m_error));
   return make_error<StringError>(m_error->message(),
                                  m_error->convertToErrorCode());
 }
 
-size_t DecodedThread::GetLastPosition() const {
-  return m_instructions.empty() ? 0 : m_instructions.size() - 1;
+TraceInstructionControlFlowType
+IntelPTInstruction::GetControlFlowType(lldb::addr_t next_load_address) const {
+  if (IsError())
+    return (TraceInstructionControlFlowType)0;
+
+  TraceInstructionControlFlowType mask =
+      eTraceInstructionControlFlowTypeInstruction;
+
+  switch (m_pt_insn.iclass) {
+  case ptic_cond_jump:
+  case ptic_jump:
+  case ptic_far_jump:
+    mask |= eTraceInstructionControlFlowTypeBranch;
+    if (m_pt_insn.ip + m_pt_insn.size != next_load_address)
+      mask |= eTraceInstructionControlFlowTypeTakenBranch;
+    break;
+  case ptic_return:
+  case ptic_far_return:
+    mask |= eTraceInstructionControlFlowTypeReturn;
+    break;
+  case ptic_call:
+  case ptic_far_call:
+    mask |= eTraceInstructionControlFlowTypeCall;
+    break;
+  default:
+    break;
+  }
+
+  return mask;
 }
 
 ArrayRef<IntelPTInstruction> DecodedThread::GetInstructions() const {
   return makeArrayRef(m_instructions);
 }
 
-size_t DecodedThread::GetCursorPosition() const { return m_position; }
+DecodedThread::DecodedThread(ThreadSP thread_sp, Error error)
+    : m_thread_sp(thread_sp) {
+  m_instructions.emplace_back(std::move(error));
+}
 
-size_t DecodedThread::SetCursorPosition(size_t new_position) {
-  m_position = std::min(new_position, GetLastPosition());
-  return m_position;
+DecodedThread::DecodedThread(ThreadSP thread_sp,
+                             std::vector<IntelPTInstruction> &&instructions)
+    : m_thread_sp(thread_sp), m_instructions(std::move(instructions)) {
+  if (m_instructions.empty())
+    m_instructions.emplace_back(
+        createStringError(inconvertibleErrorCode(), "empty trace"));
 }
 
-DecodedThread::DecodedThread(Error error) {
-  m_instructions.emplace_back(std::move(error));
-  m_position = GetLastPosition();
+lldb::TraceCursorUP DecodedThread::GetCursor() {
+  return std::make_unique<TraceCursorIntelPT>(m_thread_sp, shared_from_this());
 }

lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.h

 //===-- IntelPTDecoder.h --======--------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_DECODER_H
 #define LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_DECODER_H
 
 #include "intel-pt.h"
 
 #include "DecodedThread.h"
 #include "forward-declarations.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Utility/FileSpec.h"
 
 namespace lldb_private {
 namespace trace_intel_pt {
 
 /// Base class that handles the decoding of a thread and caches the result.
 class ThreadDecoder {
 public:
   virtual ~ThreadDecoder() = default;
 
   ThreadDecoder() = default;
 
   /// Decode the thread and store the result internally, to avoid
   /// recomputations.
   ///
   /// \return
   ///     A \a DecodedThread instance.
-  const DecodedThread &Decode();
+  DecodedThreadSP Decode();
 
   ThreadDecoder(const ThreadDecoder &other) = delete;
   ThreadDecoder &operator=(const ThreadDecoder &other) = delete;
 
 protected:
   /// Decode the thread.
   ///
   /// \return
   ///     A \a DecodedThread instance.
-  virtual DecodedThread DoDecode() = 0;
+  virtual DecodedThreadSP DoDecode() = 0;
 
-  llvm::Optional<DecodedThread> m_decoded_thread;
+  llvm::Optional<DecodedThreadSP> m_decoded_thread;
 };
 
 /// Decoder implementation for \a lldb_private::ThreadPostMortemTrace, which are
 /// non-live processes that come trace session files.
 class PostMortemThreadDecoder : public ThreadDecoder {
 public:
   /// \param[in] trace_thread
   ///     The thread whose trace file will be decoded.
   ///
   /// \param[in] trace
   ///     The main Trace object who owns this decoder and its data.
   PostMortemThreadDecoder(const lldb::ThreadPostMortemTraceSP &trace_thread,
                           TraceIntelPT &trace);
 
 private:
-  DecodedThread DoDecode() override;
+  DecodedThreadSP DoDecode() override;
 
   lldb::ThreadPostMortemTraceSP m_trace_thread;
   TraceIntelPT &m_trace;
 };
 
 class LiveThreadDecoder : public ThreadDecoder {
 public:
   /// \param[in] thread
   ///     The thread whose traces will be decoded.
   ///
   /// \param[in] trace
   ///     The main Trace object who owns this decoder and its data.
   LiveThreadDecoder(Thread &thread, TraceIntelPT &trace);
 
 private:
-  DecodedThread DoDecode() override;
+  DecodedThreadSP DoDecode() override;
 
   lldb::ThreadSP m_thread_sp;
   TraceIntelPT &m_trace;
 };
 
 } // namespace trace_intel_pt
 } // namespace lldb_private
 
 #endif // LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_DECODER_H

lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp

if (!buffer)
     return buffer.takeError();
   if (Expected<pt_cpu> cpu_info = trace.GetCPUInfo())
     return DecodeInMemoryTrace(*thread.GetProcess(), trace,
                                MutableArrayRef<uint8_t>(*buffer));
   else
     return cpu_info.takeError();
 }
 
-const DecodedThread &ThreadDecoder::Decode() {
+DecodedThreadSP ThreadDecoder::Decode() {
   if (!m_decoded_thread.hasValue())
     m_decoded_thread = DoDecode();
   return *m_decoded_thread;
 }
 
 PostMortemThreadDecoder::PostMortemThreadDecoder(
     const lldb::ThreadPostMortemTraceSP &trace_thread, TraceIntelPT &trace)
     : m_trace_thread(trace_thread), m_trace(trace) {}
 
-DecodedThread PostMortemThreadDecoder::DoDecode() {
+DecodedThreadSP PostMortemThreadDecoder::DoDecode() {
   if (Expected<std::vector<IntelPTInstruction>> instructions =
           DecodeTraceFile(*m_trace_thread->GetProcess(), m_trace,
                           m_trace_thread->GetTraceFile()))
-    return DecodedThread(std::move(*instructions));
+    return std::make_shared<DecodedThread>(m_trace_thread->shared_from_this(),
+                                           std::move(*instructions));
   else
-    return DecodedThread(instructions.takeError());
+    return std::make_shared<DecodedThread>(m_trace_thread->shared_from_this(),
+                                           instructions.takeError());
 }
 
 LiveThreadDecoder::LiveThreadDecoder(Thread &thread, TraceIntelPT &trace)
     : m_thread_sp(thread.shared_from_this()), m_trace(trace) {}
 
-DecodedThread LiveThreadDecoder::DoDecode() {
+DecodedThreadSP LiveThreadDecoder::DoDecode() {
   if (Expected<std::vector<IntelPTInstruction>> instructions =
           DecodeLiveThread(*m_thread_sp, m_trace))
-    return DecodedThread(std::move(*instructions));
+    return std::make_shared<DecodedThread>(m_thread_sp,
+                                           std::move(*instructions));
   else
-    return DecodedThread(instructions.takeError());
+    return std::make_shared<DecodedThread>(m_thread_sp,
+                                           instructions.takeError());
 }

lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h

+//===-- TraceCursorIntelPT.h ------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACECURSORINTELPT_H
+#define LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACECURSORINTELPT_H
+
+#include "IntelPTDecoder.h"
+#include "TraceIntelPTSessionFileParser.h"
+
+namespace lldb_private {
+namespace trace_intel_pt {
+
+class TraceCursorIntelPT : public TraceCursor {
+public:
+  TraceCursorIntelPT(lldb::ThreadSP thread_sp,
+                     DecodedThreadSP decoded_thread_sp);
+
+  bool Next(lldb::TraceInstructionControlFlowType granularity,
+            bool ignore_errors) override;
+
+  bool Prev(lldb::TraceInstructionControlFlowType granularity,
+            bool ignore_errors) override;
+
+  void SeekToEnd() override;
+
+  void SeekToBegin() override;
+
+  llvm::Error GetError() override;
+
+  lldb::addr_t GetLoadAddress() override;
+
+  lldb::TraceInstructionControlFlowType
+  GetInstructionControlFlowType() override;
+
+private:
+  bool IsError();
+
+  /// Storage of the actual instructions
+  DecodedThreadSP m_decoded_thread_sp;
+  /// Internal instruction index currently pointing at.
+  size_t m_pos;
+};
+
+} // namespace trace_intel_pt
+} // namespace lldb_private
+
+#endif // LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACECURSORINTELPT_H

lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp

+//===-- TraceCursorIntelPT.cpp --------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "TraceCursorIntelPT.h"
+#include "DecodedThread.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::trace_intel_pt;
+using namespace llvm;
+
+TraceCursorIntelPT::TraceCursorIntelPT(ThreadSP thread_sp,
+                                       DecodedThreadSP decoded_thread_sp)
+    : TraceCursor(thread_sp), m_decoded_thread_sp(decoded_thread_sp) {
+  assert(!m_decoded_thread_sp->GetInstructions().empty() &&
+         "a trace should have at least one instruction or error");
+  m_pos = m_decoded_thread_sp->GetInstructions().size() - 1;
+}
+
+bool TraceCursorIntelPT::Next(TraceInstructionControlFlowType granularity,
+                              bool ignore_errors) {
+  size_t size = m_decoded_thread_sp->GetInstructions().size();
+  while (m_pos + 1 < size) {
+    m_pos++;
+    if ((GetInstructionControlFlowType() & granularity) ||
+        (!ignore_errors && IsError()))
+      return true;
+  }
+  return false;
+}
+
+bool TraceCursorIntelPT::Prev(TraceInstructionControlFlowType granularity,
+                              bool ignore_errors) {
+  while (m_pos > 0) {
+    m_pos--;
+    if ((GetInstructionControlFlowType() & granularity) ||
+        (!ignore_errors && IsError()))
+      return true;
+  }
+  return false;
+}
+
+void TraceCursorIntelPT::SeekToEnd() {
+  m_pos = m_decoded_thread_sp->GetInstructions().size() - 1;
+}
+
+void TraceCursorIntelPT::SeekToBegin() { m_pos = 0; }
+
+bool TraceCursorIntelPT::IsError() {
+  return m_decoded_thread_sp->GetInstructions()[m_pos].IsError();
+}
+
+Error TraceCursorIntelPT::GetError() {
+  return m_decoded_thread_sp->GetInstructions()[m_pos].ToError();
+}
+
+lldb::addr_t TraceCursorIntelPT::GetLoadAddress() {
+  return m_decoded_thread_sp->GetInstructions()[m_pos].GetLoadAddress();
+}
+
+TraceInstructionControlFlowType
+TraceCursorIntelPT::GetInstructionControlFlowType() {
+  lldb::addr_t next_load_address =
+      m_pos + 1 < m_decoded_thread_sp->GetInstructions().size()
+          ? m_decoded_thread_sp->GetInstructions()[m_pos + 1].GetLoadAddress()
+          : LLDB_INVALID_ADDRESS;
+  return m_decoded_thread_sp->GetInstructions()[m_pos].GetControlFlowType(
+      next_load_address);
+}

lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h

public:
   lldb::CommandObjectSP
   GetProcessTraceStartCommand(CommandInterpreter &interpreter) override;
 
   lldb::CommandObjectSP
   GetThreadTraceStartCommand(CommandInterpreter &interpreter) override;
 
   llvm::StringRef GetSchema() override;
 
-  void TraverseInstructions(
-      Thread &thread, size_t position, TraceDirection direction,
-      std::function<bool(size_t index, llvm::Expected<lldb::addr_t> load_addr)>
-          callback) override;
-
-  llvm::Optional<size_t> GetInstructionCount(Thread &thread) override;
-
   lldb::TraceCursorUP GetCursor(Thread &thread) override;
 
   void DoRefreshLiveProcessState(
       llvm::Expected<TraceGetStateResponse> state) override;
 
   bool IsTraced(const Thread &thread) override;
 
   const char *GetStartConfigurationHelp() override;
TraceIntelPT(
       const pt_cpu &cpu_info,
       const std::vector<lldb::ThreadPostMortemTraceSP> &traced_threads);
 
   /// Constructor for live processes
   TraceIntelPT(Process &live_process)
       : Trace(live_process), m_thread_decoders(){};
 
   /// Decode the trace of the given thread that, i.e. recontruct the traced
-  /// instructions. That trace must be managed by this class.
+  /// instructions.
   ///
   /// \param[in] thread
   ///     If \a thread is a \a ThreadTrace, then its internal trace file will be
   ///     decoded. Live threads are not currently supported.
   ///
   /// \return
-  ///     A \a DecodedThread instance if decoding was successful, or a \b
-  ///     nullptr if the thread's trace is not managed by this class.
-  const DecodedThread *Decode(Thread &thread);
+  ///     A \a DecodedThread shared pointer with the decoded instructions. Any
+  ///     errors are embedded in the instruction list.
+  DecodedThreadSP Decode(Thread &thread);
 
   /// It is provided by either a session file or a live process' "cpuInfo"
   /// binary data.
   llvm::Optional<pt_cpu> m_cpu_info;
   std::map<const Thread *, std::unique_ptr<ThreadDecoder>> m_thread_decoders;
-  /// Dummy DecodedThread used when decoding threads after there were errors
-  /// when refreshing the live process state.
-  llvm::Optional<DecodedThread> m_failed_live_threads_decoder;
+  /// Error gotten after a failed live process update, if any.
+  llvm::Optional<std::string> m_live_refresh_error;
 };
 
 } // namespace trace_intel_pt
 } // namespace lldb_private
 
 #endif // LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACEINTELPT_H

lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp

 //===-- TraceIntelPT.cpp --------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "TraceIntelPT.h"
 
 #include "CommandObjectTraceStartIntelPT.h"
+#include "DecodedThread.h"
 #include "TraceIntelPTConstants.h"
 #include "TraceIntelPTSessionFileParser.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/ThreadPostMortemTrace.h"
 
 using namespace lldb;
TraceIntelPT::TraceIntelPT(
     const pt_cpu &cpu_info,
     const std::vector<ThreadPostMortemTraceSP> &traced_threads)
     : m_cpu_info(cpu_info) {
   for (const ThreadPostMortemTraceSP &thread : traced_threads)
     m_thread_decoders.emplace(
         thread.get(), std::make_unique<PostMortemThreadDecoder>(thread, *this));
 }
 
-const DecodedThread *TraceIntelPT::Decode(Thread &thread) {
+DecodedThreadSP TraceIntelPT::Decode(Thread &thread) {
   RefreshLiveProcessState();
-  if (m_failed_live_threads_decoder.hasValue())
-    return &*m_failed_live_threads_decoder;
+  if (m_live_refresh_error.hasValue())
+    return std::make_shared<DecodedThread>(
+        thread.shared_from_this(),
+        createStringError(inconvertibleErrorCode(), *m_live_refresh_error));
 
   auto it = m_thread_decoders.find(&thread);
   if (it == m_thread_decoders.end())
-    return nullptr;
-  return &it->second->Decode();
+    return std::make_shared<DecodedThread>(
+        thread.shared_from_this(),
+        createStringError(inconvertibleErrorCode(), "thread not traced"));
+  return it->second->Decode();
 }
 
 lldb::TraceCursorUP TraceIntelPT::GetCursor(Thread &thread) {
-  // TODO: to implement
-  return nullptr;
-}
-
-void TraceIntelPT::TraverseInstructions(
-    Thread &thread, size_t position, TraceDirection direction,
-    std::function<bool(size_t index, Expected<lldb::addr_t> load_addr)>
-        callback) {
-  const DecodedThread *decoded_thread = Decode(thread);
-  if (!decoded_thread)
-    return;
-
-  ArrayRef<IntelPTInstruction> instructions = decoded_thread->GetInstructions();
-
-  ssize_t delta = direction == TraceDirection::Forwards ? 1 : -1;
-  for (ssize_t i = position; i < (ssize_t)instructions.size() && i >= 0;
-       i += delta)
-    if (!callback(i, instructions[i].GetLoadAddress()))
-      break;
-}
-
-Optional<size_t> TraceIntelPT::GetInstructionCount(Thread &thread) {
-  if (const DecodedThread *decoded_thread = Decode(thread))
-    return decoded_thread->GetInstructions().size();
-  else
-    return None;
+  return Decode(thread)->GetCursor();
 }
 
 Expected<pt_cpu> TraceIntelPT::GetCPUInfoForLiveProcess() {
   Expected<std::vector<uint8_t>> cpu_info = GetLiveProcessBinaryData("cpuInfo");
   if (!cpu_info)
     return cpu_info.takeError();
 
   int64_t cpu_family = -1;
Expected<pt_cpu> TraceIntelPT::GetCPUInfo() {
   return *m_cpu_info;
 }
 
 void TraceIntelPT::DoRefreshLiveProcessState(
     Expected<TraceGetStateResponse> state) {
   m_thread_decoders.clear();
 
   if (!state) {
-    m_failed_live_threads_decoder = DecodedThread(state.takeError());
+    m_live_refresh_error = toString(state.takeError());
     return;
   }
 
   for (const TraceThreadState &thread_state : state->tracedThreads) {
     Thread &thread =
         *m_live_process->GetThreadList().FindThreadByID(thread_state.tid);
     m_thread_decoders.emplace(
         &thread, std::make_unique<LiveThreadDecoder>(thread, *this));

lldb/source/Target/CMakeLists.txt

add_lldb_library(lldbTarget
   ThreadPlanStepThrough.cpp
   ThreadPlanStepUntil.cpp
   ThreadPlanTracer.cpp
   ThreadPlanStack.cpp
   ThreadSpec.cpp
   ThreadPostMortemTrace.cpp
   Trace.cpp
   TraceCursor.cpp
+  TraceInstructionDumper.cpp
   TraceSessionFileParser.cpp
   UnixSignals.cpp
   UnwindAssembly.cpp
   UnwindLLDB.cpp
 
   LINK_LIBS
     lldbBreakpoint
     lldbCore

lldb/source/Target/Trace.cpp

Expected<StringRef> Trace::FindPluginSchema(StringRef name) {
   ConstString plugin_name(name);
   StringRef schema = PluginManager::GetTraceSchema(plugin_name);
   if (!schema.empty())
     return schema;
 
   return createInvalidPlugInError(name);
 }
 
-static int GetNumberOfDigits(size_t num) {
-  return num == 0 ? 1 : static_cast<int>(log10(num)) + 1;
-}
-
-/// \return
-///     \b true if the provided line entries match line, column and source file.
-///     This function assumes that the line entries are valid.
-static bool FileLineAndColumnMatches(const LineEntry &a, const LineEntry &b) {
-  if (a.line != b.line)
-    return false;
-  if (a.column != b.column)
-    return false;
-  return a.file == b.file;
-}
-
-// This custom LineEntry validator is neded because some line_entries have
-// 0 as line, which is meaningless. Notice that LineEntry::IsValid only
-// checks that line is not LLDB_INVALID_LINE_NUMBER, i.e. UINT32_MAX.
-static bool IsLineEntryValid(const LineEntry &line_entry) {
-  return line_entry.IsValid() && line_entry.line > 0;
-}
-
-/// Helper structure for \a TraverseInstructionsWithSymbolInfo.
-struct InstructionSymbolInfo {
-  SymbolContext sc;
-  Address address;
-  lldb::addr_t load_address;
-  lldb::DisassemblerSP disassembler;
-  lldb::InstructionSP instruction;
-  lldb_private::ExecutionContext exe_ctx;
-};
-
-/// InstructionSymbolInfo object with symbol information for the given
-/// instruction, calculated efficiently.
-///
-/// \param[in] symbol_scope
-///     If not \b 0, then the \a InstructionSymbolInfo will have its
-///     SymbolContext calculated up to that level.
-///
-/// \param[in] include_disassembler
-///     If \b true, then the \a InstructionSymbolInfo will have the
-///     \a disassembler and \a instruction objects calculated.
-static void TraverseInstructionsWithSymbolInfo(
-    Trace &trace, Thread &thread, size_t position,
-    Trace::TraceDirection direction, SymbolContextItem symbol_scope,
-    bool include_disassembler,
-    std::function<bool(size_t index, Expected<InstructionSymbolInfo> insn)>
-        callback) {
-  InstructionSymbolInfo prev_insn;
-
-  Target &target = thread.GetProcess()->GetTarget();
-  ExecutionContext exe_ctx;
-  target.CalculateExecutionContext(exe_ctx);
-  const ArchSpec &arch = target.GetArchitecture();
-
-  // Find the symbol context for the given address reusing the previous
-  // instruction's symbol context when possible.
-  auto calculate_symbol_context = [&](const Address &address) {
-    AddressRange range;
-    if (prev_insn.sc.GetAddressRange(symbol_scope, 0,
-                                     /*inline_block_range*/ false, range) &&
-        range.Contains(address))
-      return prev_insn.sc;
-
-    SymbolContext sc;
-    address.CalculateSymbolContext(&sc, symbol_scope);
-    return sc;
-  };
-
-  // Find the disassembler for the given address reusing the previous
-  // instruction's disassembler when possible.
-  auto calculate_disass = [&](const Address &address, const SymbolContext &sc) {
-    if (prev_insn.disassembler) {
-      if (InstructionSP instruction =
-              prev_insn.disassembler->GetInstructionList()
-                  .GetInstructionAtAddress(address))
-        return std::make_tuple(prev_insn.disassembler, instruction);
-    }
-
-    if (sc.function) {
-      if (DisassemblerSP disassembler =
-              sc.function->GetInstructions(exe_ctx, nullptr)) {
-        if (InstructionSP instruction =
-                disassembler->GetInstructionList().GetInstructionAtAddress(
-                    address))
-          return std::make_tuple(disassembler, instruction);
-      }
-    }
-    // We fallback to a single instruction disassembler
-    AddressRange range(address, arch.GetMaximumOpcodeByteSize());
-    DisassemblerSP disassembler =
-        Disassembler::DisassembleRange(arch, /*plugin_name*/ nullptr,
-                                       /*flavor*/ nullptr, target, range);
-    return std::make_tuple(disassembler,
-                           disassembler ? disassembler->GetInstructionList()
-                                              .GetInstructionAtAddress(address)
-                                        : InstructionSP());
-  };
-
-  trace.TraverseInstructions(
-      thread, position, direction,
-      [&](size_t index, Expected<lldb::addr_t> load_address) -> bool {
-        if (!load_address)
-          return callback(index, load_address.takeError());
-
-        InstructionSymbolInfo insn;
-        insn.load_address = *load_address;
-        insn.exe_ctx = exe_ctx;
-        insn.address.SetLoadAddress(*load_address, &target);
-        if (symbol_scope != 0)
-          insn.sc = calculate_symbol_context(insn.address);
-        if (include_disassembler)
-          std::tie(insn.disassembler, insn.instruction) =
-              calculate_disass(insn.address, insn.sc);
-        prev_insn = insn;
-        return callback(index, insn);
-      });
-}
-
-/// Compare the symbol contexts of the provided \a InstructionSymbolInfo
-/// objects.
-///
-/// \return
-///     \a true if both instructions belong to the same scope level analized
-///     in the following order:
-///       - module
-///       - symbol
-///       - function
-///       - line
-static bool
-IsSameInstructionSymbolContext(const InstructionSymbolInfo &prev_insn,
-                               const InstructionSymbolInfo &insn) {
-  // module checks
-  if (insn.sc.module_sp != prev_insn.sc.module_sp)
-    return false;
-
-  // symbol checks
-  if (insn.sc.symbol != prev_insn.sc.symbol)
-    return false;
-
-  // function checks
-  if (!insn.sc.function && !prev_insn.sc.function)
-    return true;
-  else if (insn.sc.function != prev_insn.sc.function)
-    return false;
-
-  // line entry checks
-  const bool curr_line_valid = IsLineEntryValid(insn.sc.line_entry);
-  const bool prev_line_valid = IsLineEntryValid(prev_insn.sc.line_entry);
-  if (curr_line_valid && prev_line_valid)
-    return FileLineAndColumnMatches(insn.sc.line_entry,
-                                    prev_insn.sc.line_entry);
-  return curr_line_valid == prev_line_valid;
-}
-
-/// Dump the symbol context of the given instruction address if it's different
-/// from the symbol context of the previous instruction in the trace.
-///
-/// \param[in] prev_sc
-///     The symbol context of the previous instruction in the trace.
-///
-/// \param[in] address
-///     The address whose symbol information will be dumped.
-///
-/// \return
-///     The symbol context of the current address, which might differ from the
-///     previous one.
-static void
-DumpInstructionSymbolContext(Stream &s,
-                             Optional<InstructionSymbolInfo> prev_insn,
-                             InstructionSymbolInfo &insn) {
-  if (prev_insn && IsSameInstructionSymbolContext(*prev_insn, insn))
-    return;
-
-  s.Printf("  ");
-
-  if (!insn.sc.module_sp)
-    s.Printf("(none)");
-  else if (!insn.sc.function && !insn.sc.symbol)
-    s.Printf("%s`(none)",
-             insn.sc.module_sp->GetFileSpec().GetFilename().AsCString());
-  else
-    insn.sc.DumpStopContext(&s, insn.exe_ctx.GetTargetPtr(), insn.address,
-                            /*show_fullpath*/ false,
-                            /*show_module*/ true, /*show_inlined_frames*/ false,
-                            /*show_function_arguments*/ true,
-                            /*show_function_name*/ true);
-  s.Printf("\n");
-}
-
-static void DumpInstructionDisassembly(Stream &s, InstructionSymbolInfo &insn) {
-  if (!insn.instruction)
-    return;
-  s.Printf("    ");
-  insn.instruction->Dump(&s, /*show_address*/ false, /*show_bytes*/ false,
-                         /*max_opcode_byte_size*/ 0, &insn.exe_ctx, &insn.sc,
-                         /*prev_sym_ctx*/ nullptr,
-                         /*disassembly_addr_format*/ nullptr,
-                         /*max_address_text_size*/ 0);
-}
-
-void Trace::DumpTraceInstructions(Thread &thread, Stream &s, size_t count,
-                                  size_t end_position, bool raw) {
-  Optional<size_t> instructions_count = GetInstructionCount(thread);
-  if (!instructions_count) {
-    s.Printf("thread #%u: tid = %" PRIu64 ", not traced\n", thread.GetIndexID(),
-             thread.GetID());
-    return;
-  }
-
-  s.Printf("thread #%u: tid = %" PRIu64 ", total instructions = %zu\n",
-           thread.GetIndexID(), thread.GetID(), *instructions_count);
-
-  if (count == 0 || end_position >= *instructions_count)
-    return;
-
-  int digits_count = GetNumberOfDigits(end_position);
-  size_t start_position =
-      end_position + 1 < count ? 0 : end_position + 1 - count;
-  auto printInstructionIndex = [&](size_t index) {
-    s.Printf("    [%*zu] ", digits_count, index);
-  };
-
-  bool was_prev_instruction_an_error = false;
-  Optional<InstructionSymbolInfo> prev_insn;
-
-  TraverseInstructionsWithSymbolInfo(
-      *this, thread, start_position, TraceDirection::Forwards,
-      eSymbolContextEverything, /*disassembler*/ true,
-      [&](size_t index, Expected<InstructionSymbolInfo> insn) -> bool {
-        if (!insn) {
-          printInstructionIndex(index);
-          s << toString(insn.takeError());
-
-          prev_insn = None;
-          was_prev_instruction_an_error = true;
-        } else {
-          if (was_prev_instruction_an_error)
-            s.Printf("    ...missing instructions\n");
-
-          if (!raw)
-            DumpInstructionSymbolContext(s, prev_insn, *insn);
-
-          printInstructionIndex(index);
-          s.Printf("0x%016" PRIx64, insn->load_address);
-
-          if (!raw)
-            DumpInstructionDisassembly(s, *insn);
-
-          prev_insn = *insn;
-          was_prev_instruction_an_error = false;
-        }
-
-        s.Printf("\n");
-        return index < end_position;
-      });
-}
-
 Error Trace::Start(const llvm::json::Value &request) {
   if (!m_live_process)
     return createStringError(inconvertibleErrorCode(),
                              "Tracing requires a live process.");
   return m_live_process->TraceStart(request);
 }
 
 Error Trace::Stop() {

lldb/source/Target/TraceCursor.cpp

 //===-- TraceCursor.cpp -----------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Target/TraceCursor.h"
 
-#include "lldb/Target/Trace.h"
+#include "lldb/Target/ExecutionContext.h"
 
+using namespace lldb;
 using namespace lldb_private;
+using namespace llvm;
 
-bool TraceCursor::IsStale() { return m_stop_id != m_trace_sp->GetStopID(); }
+TraceCursor::TraceCursor(lldb::ThreadSP thread_sp)
+    : m_exe_ctx_ref(ExecutionContext(thread_sp)) {}
+
+ExecutionContextRef &TraceCursor::GetExecutionContextRef() {
+  return m_exe_ctx_ref;
+}

lldb/source/Target/TraceInstructionDumper.cpp

+//===-- TraceCursor.cpp ---------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Target/TraceInstructionDumper.h"
+
+#include "lldb/Core/Module.h"
+#include "lldb/Symbol/Function.h"
+#include "lldb/Target/ExecutionContext.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/SectionLoadList.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace llvm;
+
+TraceInstructionDumper::TraceInstructionDumper(lldb::TraceCursorUP &&cursor_up,
+                                               bool forwards, size_t skip,
+                                               bool raw)
+    : m_cursor_up(std::move(cursor_up)), m_forwards(forwards), m_raw(raw) {
+  if (forwards)
+    m_cursor_up->SeekToBegin();
+  else
+    m_cursor_up->SeekToEnd();
+
+  Skip(skip);
+}
+
+bool TraceInstructionDumper::TryMoveOneInstruction() {
+  if (m_forwards) {
+    if (!m_cursor_up->Next()) {
+      m_no_more_data = true;
+      return false;
+    }
+    m_index++;
+  } else {
+    if (!m_cursor_up->Prev()) {
+      m_no_more_data = true;
+      return false;
+    }
+    m_index--;
+  }
+  return true;
+}
+
+void TraceInstructionDumper::Skip(size_t count) {
+  for (size_t i = 0; i < count; i++) {
+    if (!TryMoveOneInstruction())
+      break;
+  }
+}
+
+static int GetNumberOfDigits(int num) {
+  if (num == 0)
+    return 1;
+  return (num < 0 ? 1 : 0) + static_cast<int>(log10(abs(num))) + 1;
+}
+
+/// Helper struct that holds symbol, disassembly and address information of an
+/// instruction.
+struct InstructionSymbolInfo {
+  SymbolContext sc;
+  Address address;
+  lldb::addr_t load_address;
+  lldb::DisassemblerSP disassembler;
+  lldb::InstructionSP instruction;
+  lldb_private::ExecutionContext exe_ctx;
+};
+
+// This custom LineEntry validator is neded because some line_entries have
+// 0 as line, which is meaningless. Notice that LineEntry::IsValid only
+// checks that line is not LLDB_INVALID_LINE_NUMBER, i.e. UINT32_MAX.
+static bool IsLineEntryValid(const LineEntry &line_entry) {
+  return line_entry.IsValid() && line_entry.line > 0;
+}
+
+/// \return
+///     \b true if the provided line entries match line, column and source file.
+///     This function assumes that the line entries are valid.
+static bool FileLineAndColumnMatches(const LineEntry &a, const LineEntry &b) {
+  if (a.line != b.line)
+    return false;
+  if (a.column != b.column)
+    return false;
+  return a.file == b.file;
+}
+
+/// Compare the symbol contexts of the provided \a InstructionSymbolInfo
+/// objects.
+///
+/// \return
+///     \a true if both instructions belong to the same scope level analized
+///     in the following order:
+///       - module
+///       - symbol
+///       - function
+///       - line
+static bool
+IsSameInstructionSymbolContext(const InstructionSymbolInfo &prev_insn,
+                               const InstructionSymbolInfo &insn) {
+  // module checks
+  if (insn.sc.module_sp != prev_insn.sc.module_sp)
+    return false;
+
+  // symbol checks
+  if (insn.sc.symbol != prev_insn.sc.symbol)
+    return false;
+
+  // function checks
+  if (!insn.sc.function && !prev_insn.sc.function)
+    return true;
+  else if (insn.sc.function != prev_insn.sc.function)
+    return false;
+
+  // line entry checks
+  const bool curr_line_valid = IsLineEntryValid(insn.sc.line_entry);
+  const bool prev_line_valid = IsLineEntryValid(prev_insn.sc.line_entry);
+  if (curr_line_valid && prev_line_valid)
+    return FileLineAndColumnMatches(insn.sc.line_entry,
+                                    prev_insn.sc.line_entry);
+  return curr_line_valid == prev_line_valid;
+}
+
+/// Dump the symbol context of the given instruction address if it's different
+/// from the symbol context of the previous instruction in the trace.
+///
+/// \param[in] prev_sc
+///     The symbol context of the previous instruction in the trace.
+///
+/// \param[in] address
+///     The address whose symbol information will be dumped.
+///
+/// \return
+///     The symbol context of the current address, which might differ from the
+///     previous one.
+static void
+DumpInstructionSymbolContext(Stream &s,
+                             Optional<InstructionSymbolInfo> prev_insn,
+                             InstructionSymbolInfo &insn) {
+  if (prev_insn && IsSameInstructionSymbolContext(*prev_insn, insn))
+    return;
+
+  s.Printf("  ");
+
+  if (!insn.sc.module_sp)
+    s.Printf("(none)");
+  else if (!insn.sc.function && !insn.sc.symbol)
+    s.Printf("%s`(none)",
+             insn.sc.module_sp->GetFileSpec().GetFilename().AsCString());
+  else
+    insn.sc.DumpStopContext(&s, insn.exe_ctx.GetTargetPtr(), insn.address,
+                            /*show_fullpath*/ false,
+                            /*show_module*/ true, /*show_inlined_frames*/ false,
+                            /*show_function_arguments*/ true,
+                            /*show_function_name*/ true);
+  s.Printf("\n");
+}
+
+static void DumpInstructionDisassembly(Stream &s, InstructionSymbolInfo &insn) {
+  if (!insn.instruction)
+    return;
+  s.Printf("    ");
+  insn.instruction->Dump(&s, /*show_address*/ false, /*show_bytes*/ false,
+                         /*max_opcode_byte_size*/ 0, &insn.exe_ctx, &insn.sc,
+                         /*prev_sym_ctx*/ nullptr,
+                         /*disassembly_addr_format*/ nullptr,
+                         /*max_address_text_size*/ 0);
+}
+
+void TraceInstructionDumper::DumpInstructions(Stream &s, size_t count) {
+  ThreadSP thread_sp = m_cursor_up->GetExecutionContextRef().GetThreadSP();
+  if (!thread_sp) {
+    s.Printf("invalid thread");
+    return;
+  }
+
+  s.Printf("thread #%u: tid = %" PRIu64 "\n", thread_sp->GetIndexID(),
+           thread_sp->GetID());
+
+  int digits_count =
+      GetNumberOfDigits(m_forwards ? m_index + count - 1 : m_index - count + 1);
+  bool was_prev_instruction_an_error = false;
+
+  auto printMissingInstructionsMessage = [&]() {
+    s.Printf("    ...missing instructions\n");
+  };
+
+  auto printInstructionIndex = [&]() {
+    s.Printf("    [%*d] ", digits_count, m_index);
+  };
+
+  InstructionSymbolInfo prev_insn_info;
+
+  Target &target = thread_sp->GetProcess()->GetTarget();
+  ExecutionContext exe_ctx;
+  target.CalculateExecutionContext(exe_ctx);
+  const ArchSpec &arch = target.GetArchitecture();
+
+  // Find the symbol context for the given address reusing the previous
+  // instruction's symbol context when possible.
+  auto calculateSymbolContext = [&](const Address &address) {
+    AddressRange range;
+    if (prev_insn_info.sc.GetAddressRange(eSymbolContextEverything, 0,
+                                          /*inline_block_range*/ false,
+                                          range) &&
+        range.Contains(address))
+      return prev_insn_info.sc;
+
+    SymbolContext sc;
+    address.CalculateSymbolContext(&sc, eSymbolContextEverything);
+    return sc;
+  };
+
+  // Find the disassembler for the given address reusing the previous
+  // instruction's disassembler when possible.
+  auto calculateDisass = [&](const Address &address, const SymbolContext &sc) {
+    if (prev_insn_info.disassembler) {
+      if (InstructionSP instruction =
+              prev_insn_info.disassembler->GetInstructionList()
+                  .GetInstructionAtAddress(address))
+        return std::make_tuple(prev_insn_info.disassembler, instruction);
+    }
+
+    if (sc.function) {
+      if (DisassemblerSP disassembler =
+              sc.function->GetInstructions(exe_ctx, nullptr)) {
+        if (InstructionSP instruction =
+                disassembler->GetInstructionList().GetInstructionAtAddress(
+                    address))
+          return std::make_tuple(disassembler, instruction);
+      }
+    }
+    // We fallback to a single instruction disassembler
+    AddressRange range(address, arch.GetMaximumOpcodeByteSize());
+    DisassemblerSP disassembler =
+        Disassembler::DisassembleRange(arch, /*plugin_name*/ nullptr,
+                                       /*flavor*/ nullptr, target, range);
+    return std::make_tuple(disassembler,
+                           disassembler ? disassembler->GetInstructionList()
+                                              .GetInstructionAtAddress(address)
+                                        : InstructionSP());
+  };
+
+  for (size_t i = 0; i < count; i++) {
+    if (m_no_more_data) {
+      s.Printf("    no more data\n");
+      break;
+    }
+
+    if (Error err = m_cursor_up->GetError()) {
+      if (!m_forwards && !was_prev_instruction_an_error)
+        printMissingInstructionsMessage();
+
+      was_prev_instruction_an_error = true;
+
+      printInstructionIndex();
+      s << toString(std::move(err));
+    } else {
+      if (m_forwards && was_prev_instruction_an_error)
+        printMissingInstructionsMessage();
+
+      was_prev_instruction_an_error = false;
+
+      InstructionSymbolInfo insn_info;
+
+      if (!m_raw) {
+        insn_info.load_address = m_cursor_up->GetLoadAddress();
+        insn_info.exe_ctx = exe_ctx;
+        insn_info.address.SetLoadAddress(insn_info.load_address, &target);
+        insn_info.sc = calculateSymbolContext(insn_info.address);
+        std::tie(insn_info.disassembler, insn_info.instruction) =
+            calculateDisass(insn_info.address, insn_info.sc);
+
+        DumpInstructionSymbolContext(s, prev_insn_info, insn_info);
+      }
+
+      printInstructionIndex();
+      s.Printf("0x%016" PRIx64, m_cursor_up->GetLoadAddress());
+
+      if (!m_raw)
+        DumpInstructionDisassembly(s, insn_info);
+
+      prev_insn_info = insn_info;
+    }
+
+    s.Printf("\n");
+    TryMoveOneInstruction();
+  }
+}

lldb/test/API/commands/trace/TestTraceDumpInstructions.py

def testErrorMessages(self):
             substrs=["error: Process is not being traced"],
             error=True)
 
     def testRawDumpInstructions(self):
         self.expect("trace load -v " +
             os.path.join(self.getSourceDir(), "intelpt-trace", "trace.json"),
             substrs=["intel-pt"])
 
-        self.expect("thread trace dump instructions --raw",
-            substrs=['''thread #1: tid = 3842849, total instructions = 21
+        self.expect("thread trace dump instructions --raw --count 21 -f",
+            substrs=['''thread #1: tid = 3842849
+    [ 0] 0x0000000000400511
     [ 1] 0x0000000000400518
     [ 2] 0x000000000040051f
     [ 3] 0x0000000000400529
     [ 4] 0x000000000040052d
     [ 5] 0x0000000000400521
     [ 6] 0x0000000000400525
     [ 7] 0x0000000000400529
     [ 8] 0x000000000040052d
     [ 9] 0x0000000000400521
     [10] 0x0000000000400525
     [11] 0x0000000000400529
     [12] 0x000000000040052d
     [13] 0x0000000000400521
     [14] 0x0000000000400525
     [15] 0x0000000000400529
     [16] 0x000000000040052d
     [17] 0x0000000000400521
     [18] 0x0000000000400525
     [19] 0x0000000000400529
     [20] 0x000000000040052d'''])
 
-        # We check if we can pass count and position
-        self.expect("thread trace dump instructions --count 5 --position 10 --raw",
-            substrs=['''thread #1: tid = 3842849, total instructions = 21
+        # We check if we can pass count and skip
+        self.expect("thread trace dump instructions --count 5 --skip 6 --raw -f",
+            substrs=['''thread #1: tid = 3842849
     [ 6] 0x0000000000400525
     [ 7] 0x0000000000400529
     [ 8] 0x000000000040052d
     [ 9] 0x0000000000400521
     [10] 0x0000000000400525'''])
 
+        self.expect("thread trace dump instructions --count 5 --skip 6 --raw",
+            substrs=['''thread #1: tid = 3842849
+    [ -6] 0x0000000000400525
+    [ -7] 0x0000000000400521
+    [ -8] 0x000000000040052d
+    [ -9] 0x0000000000400529
+    [-10] 0x0000000000400525'''])
+
         # We check if we can access the thread by index id
         self.expect("thread trace dump instructions 1 --raw",
-            substrs=['''thread #1: tid = 3842849, total instructions = 21
-    [ 1] 0x0000000000400518'''])
+            substrs=['''thread #1: tid = 3842849
+    [  0] 0x000000000040052d'''])
 
         # We check that we get an error when using an invalid thread index id
         self.expect("thread trace dump instructions 10", error=True,
             substrs=['error: no thread with index: "10"'])
 
     def testDumpFullInstructionsWithMultipleThreads(self):
         # We load a trace with two threads
         self.expect("trace load -v " +
             os.path.join(self.getSourceDir(), "intelpt-trace", "trace_2threads.json"))
 
         # We print the instructions of two threads simultaneously
         self.expect("thread trace dump instructions 1 2 --count 2",
-            substrs=['''thread #1: tid = 3842849, total instructions = 21
-  a.out`main + 28 at main.cpp:4
-    [19] 0x0000000000400529    cmpl   $0x3, -0x8(%rbp)
-    [20] 0x000000000040052d    jle    0x400521                  ; <+20> at main.cpp:5
-thread #2: tid = 3842850, total instructions = 21
-  a.out`main + 28 at main.cpp:4
-    [19] 0x0000000000400529    cmpl   $0x3, -0x8(%rbp)
-    [20] 0x000000000040052d    jle    0x400521                  ; <+20> at main.cpp:5'''])
-
-        # We use custom --count and --position, saving the command to history for later
-        self.expect("thread trace dump instructions 1 2 --count 2 --position 20", inHistory=True,
-            substrs=['''thread #1: tid = 3842849, total instructions = 21
-  a.out`main + 28 at main.cpp:4
-    [19] 0x0000000000400529    cmpl   $0x3, -0x8(%rbp)
-    [20] 0x000000000040052d    jle    0x400521                  ; <+20> at main.cpp:5
-thread #2: tid = 3842850, total instructions = 21
-  a.out`main + 28 at main.cpp:4
-    [19] 0x0000000000400529    cmpl   $0x3, -0x8(%rbp)
-    [20] 0x000000000040052d    jle    0x400521                  ; <+20> at main.cpp:5'''])
+            substrs=['''thread #1: tid = 3842849
+  a.out`main + 32 at main.cpp:4
+    [ 0] 0x000000000040052d    jle    0x400521                  ; <+20> at main.cpp:5
+    [-1] 0x0000000000400529    cmpl   $0x3, -0x8(%rbp)
+thread #2: tid = 3842850
+  a.out`main + 32 at main.cpp:4
+    [ 0] 0x000000000040052d    jle    0x400521                  ; <+20> at main.cpp:5
+    [-1] 0x0000000000400529    cmpl   $0x3, -0x8(%rbp)'''])
+
+        # We use custom --count and --skip, saving the command to history for later
+        self.expect("thread trace dump instructions 1 2 --count 2 --skip 2", inHistory=True,
+            substrs=['''thread #1: tid = 3842849
+  a.out`main + 24 at main.cpp:4
+    [-2] 0x0000000000400525    addl   $0x1, -0x8(%rbp)
+  a.out`main + 20 at main.cpp:5
+    [-3] 0x0000000000400521    xorl   $0x1, -0x4(%rbp)
+thread #2: tid = 3842850
+  a.out`main + 24 at main.cpp:4
+    [-2] 0x0000000000400525    addl   $0x1, -0x8(%rbp)
+  a.out`main + 20 at main.cpp:5
+    [-3] 0x0000000000400521    xorl   $0x1, -0x4(%rbp)'''])
 
         # We use a repeat command twice and ensure the previous count is used and the
         # start position moves with each command.
         self.expect("", inHistory=True,
-            substrs=['''thread #1: tid = 3842849, total instructions = 21
-  a.out`main + 20 at main.cpp:5
-    [17] 0x0000000000400521    xorl   $0x1, -0x4(%rbp)
+            substrs=['''thread #1: tid = 3842849
+  a.out`main + 32 at main.cpp:4
+    [-4] 0x000000000040052d    jle    0x400521                  ; <+20> at main.cpp:5
+    [-5] 0x0000000000400529    cmpl   $0x3, -0x8(%rbp)
+thread #2: tid = 3842850
+  a.out`main + 32 at main.cpp:4
+    [-4] 0x000000000040052d    jle    0x400521                  ; <+20> at main.cpp:5
+    [-5] 0x0000000000400529    cmpl   $0x3, -0x8(%rbp)'''])
+
+        self.expect("", inHistory=True,
+            substrs=['''thread #1: tid = 3842849
   a.out`main + 24 at main.cpp:4
-    [18] 0x0000000000400525    addl   $0x1, -0x8(%rbp)
-thread #2: tid = 3842850, total instructions = 21
+    [-6] 0x0000000000400525    addl   $0x1, -0x8(%rbp)
   a.out`main + 20 at main.cpp:5
-    [17] 0x0000000000400521    xorl   $0x1, -0x4(%rbp)
+    [-7] 0x0000000000400521    xorl   $0x1, -0x4(%rbp)
+thread #2: tid = 3842850
   a.out`main + 24 at main.cpp:4
-    [18] 0x0000000000400525    addl   $0x1, -0x8(%rbp)'''])
-
-        self.expect("", inHistory=True,
-            substrs=['''thread #1: tid = 3842849, total instructions = 21
-  a.out`main + 28 at main.cpp:4
-    [15] 0x0000000000400529    cmpl   $0x3, -0x8(%rbp)
-    [16] 0x000000000040052d    jle    0x400521                  ; <+20> at main.cpp:5
-thread #2: tid = 3842850, total instructions = 21
-  a.out`main + 28 at main.cpp:4
-    [15] 0x0000000000400529    cmpl   $0x3, -0x8(%rbp)
-    [16] 0x000000000040052d    jle    0x400521                  ; <+20> at main.cpp:5'''])
+    [-6] 0x0000000000400525    addl   $0x1, -0x8(%rbp)
+  a.out`main + 20 at main.cpp:5
+    [-7] 0x0000000000400521    xorl   $0x1, -0x4(%rbp)'''])
 
     def testInvalidBounds(self):
         self.expect("trace load -v " +
             os.path.join(self.getSourceDir(), "intelpt-trace", "trace.json"))
 
         # The output should be work when too many instructions are asked
-        self.expect("thread trace dump instructions --count 20 --position 2",
-            substrs=['''thread #1: tid = 3842849, total instructions = 21
+        self.expect("thread trace dump instructions --count 20 -f",
+            substrs=['''thread #1: tid = 3842849
   a.out`main + 4 at main.cpp:2
-    [0] 0x0000000000400511    movl   $0x0, -0x4(%rbp)
+    [ 0] 0x0000000000400511    movl   $0x0, -0x4(%rbp)
   a.out`main + 11 at main.cpp:4
-    [1] 0x0000000000400518    movl   $0x0, -0x8(%rbp)
-    [2] 0x000000000040051f    jmp    0x400529                  ; <+28> at main.cpp:4'''])
+    [ 1] 0x0000000000400518    movl   $0x0, -0x8(%rbp)
+    [ 2] 0x000000000040051f    jmp    0x400529                  ; <+28> at main.cpp:4'''])
 
         # Should print no instructions if the position is out of bounds
-        self.expect("thread trace dump instructions --position 23",
-            endstr='thread #1: tid = 3842849, total instructions = 21\n')
+        self.expect("thread trace dump instructions --skip 23",
+            endstr='no more data\n')
 
         # Should fail with negative bounds
-        self.expect("thread trace dump instructions --position -1", error=True)
+        self.expect("thread trace dump instructions --skip -1", error=True)
         self.expect("thread trace dump instructions --count -1", error=True)
 
     def testWrongImage(self):
         self.expect("trace load " +
             os.path.join(self.getSourceDir(), "intelpt-trace", "trace_bad_image.json"))
-        self.expect("thread trace dump instructions",
-            substrs=['''thread #1: tid = 3842849, total instructions = 2
-    [0] 0x0000000000400511    error: no memory mapped at this address
-    [1] 0x0000000000400518    error: no memory mapped at this address'''])
+        self.expect("thread trace dump instructions -f",
+            substrs=['''thread #1: tid = 3842849
+    [ 0] 0x0000000000400511    error: no memory mapped at this address
+    [ 1] 0x0000000000400518    error: no memory mapped at this address'''])
 
     def testWrongCPU(self):
         self.expect("trace load " +
             os.path.join(self.getSourceDir(), "intelpt-trace", "trace_wrong_cpu.json"))
-        self.expect("thread trace dump instructions",
-            substrs=['''thread #1: tid = 3842849, total instructions = 1
-    [0] error: unknown cpu'''])
+        self.expect("thread trace dump instructions -f",
+            substrs=['''thread #1: tid = 3842849
+    [ 0] error: unknown cpu'''])
 
     def testMultiFileTraceWithMissingModule(self):
         self.expect("trace load " +
             os.path.join(self.getSourceDir(), "intelpt-trace-multi-file", "multi-file-no-ld.json"))
 
         # This instructions in this test covers the following flow:
         #
         # - The trace starts with a call to libfoo, which triggers the dynamic
         #   linker, but the dynamic linker is not included in the JSON file,
         #   thus the trace reports a set of missing instructions after
         #   instruction [6].
         # - Then, the dump continues in the next synchronization point showing
         #   a call to an inlined function, which is displayed as [inlined].
         # - Finally, a call to libfoo is performed, which invokes libbar inside.
         #
         # Whenever there's a line or symbol change, including the inline case, a
         # line is printed showing the symbol context change.
         #
         # Finally, the instruction disassembly is included in the dump.
-        self.expect("thread trace dump instructions --count 50",
-            substrs=['''thread #1: tid = 815455, total instructions = 46
+        self.expect("thread trace dump instructions --count 50 -f",
+            substrs=['''thread #1: tid = 815455
   a.out`main + 15 at main.cpp:10
     [ 0] 0x000000000040066f    callq  0x400540                  ; symbol stub for: foo()
   a.out`symbol stub for: foo()
     [ 1] 0x0000000000400540    jmpq   *0x200ae2(%rip)           ; _GLOBAL_OFFSET_TABLE_ + 40
     [ 2] 0x0000000000400546    pushq  $0x2
     [ 3] 0x000000000040054b    jmp    0x400510
   a.out`(none)
     [ 4] 0x0000000000400510    pushq  0x200af2(%rip)            ; _GLOBAL_OFFSET_TABLE_ + 8
libfoo.so`foo() + 27 at foo.cpp:6
     [39] 0x00007ffff7bd96fb    movl   -0x4(%rbp), %eax
     [40] 0x00007ffff7bd96fe    addq   $0x10, %rsp
     [41] 0x00007ffff7bd9702    popq   %rbp
     [42] 0x00007ffff7bd9703    retq''',
   '''a.out`main + 68 at main.cpp:16
     [43] 0x00000000004006a4    movl   -0xc(%rbp), %ecx
     [44] 0x00000000004006a7    addl   %eax, %ecx
     [45] 0x00000000004006a9    movl   %ecx, -0xc(%rbp)'''])
+
+
+        self.expect("thread trace dump instructions --count 50",
+            substrs=['''thread #1: tid = 815455
+  a.out`main + 73 at main.cpp:16
+    [  0] 0x00000000004006a9    movl   %ecx, -0xc(%rbp)
+    [ -1] 0x00000000004006a7    addl   %eax, %ecx
+    [ -2] 0x00000000004006a4    movl   -0xc(%rbp), %ecx
+  libfoo.so`foo() + 35 at foo.cpp:6
+    [ -3] 0x00007ffff7bd9703    retq''',
+    '''[ -4] 0x00007ffff7bd9702    popq   %rbp
+    [ -5] 0x00007ffff7bd96fe    addq   $0x10, %rsp
+    [ -6] 0x00007ffff7bd96fb    movl   -0x4(%rbp), %eax
+  libfoo.so`foo() + 24 at foo.cpp:5
+    [ -7] 0x00007ffff7bd96f8    movl   %eax, -0x4(%rbp)
+    [ -8] 0x00007ffff7bd96f3    addl   $0x1, %eax
+    [ -9] 0x00007ffff7bd96f0    movl   -0x4(%rbp), %eax
+  libfoo.so`foo() + 13 at foo.cpp:4
+    [-10] 0x00007ffff7bd96ed    movl   %eax, -0x4(%rbp)
+  libbar.so`bar() + 26 at bar.cpp:4
+    [-11] 0x00007ffff79d76aa    retq''',
+    '''[-12] 0x00007ffff79d76a9    popq   %rbp
+    [-13] 0x00007ffff79d76a6    movl   -0x4(%rbp), %eax
+  libbar.so`bar() + 19 at bar.cpp:3
+    [-14] 0x00007ffff79d76a3    movl   %eax, -0x4(%rbp)
+    [-15] 0x00007ffff79d769e    addl   $0x1, %eax
+    [-16] 0x00007ffff79d769b    movl   -0x4(%rbp), %eax
+  libbar.so`bar() + 4 at bar.cpp:2
+    [-17] 0x00007ffff79d7694    movl   $0x1, -0x4(%rbp)
+  libbar.so`bar() + 1 at bar.cpp:1
+    [-18] 0x00007ffff79d7691    movq   %rsp, %rbp
+    [-19] 0x00007ffff79d7690    pushq  %rbp
+  libfoo.so`symbol stub for: bar()
+    [-20] 0x00007ffff7bd95d0    jmpq   *0x200a4a(%rip)           ; _GLOBAL_OFFSET_TABLE_ + 32
+  libfoo.so`foo() + 8 at foo.cpp:4
+    [-21] 0x00007ffff7bd96e8    callq  0x7ffff7bd95d0            ; symbol stub for: bar()
+    [-22] 0x00007ffff7bd96e4    subq   $0x10, %rsp
+  libfoo.so`foo() + 1 at foo.cpp:3
+    [-23] 0x00007ffff7bd96e1    movq   %rsp, %rbp
+    [-24] 0x00007ffff7bd96e0    pushq  %rbp
+  a.out`symbol stub for: foo()
+    [-25] 0x0000000000400540    jmpq   *0x200ae2(%rip)           ; _GLOBAL_OFFSET_TABLE_ + 40
+  a.out`main + 63 at main.cpp:16
+    [-26] 0x000000000040069f    callq  0x400540                  ; symbol stub for: foo()
+  a.out`main + 60 at main.cpp:14
+    [-27] 0x000000000040069c    movl   %ecx, -0xc(%rbp)
+    [-28] 0x000000000040069a    addl   %eax, %ecx
+    [-29] 0x0000000000400697    movl   -0xc(%rbp), %ecx
+  a.out`main + 52 [inlined] inline_function() + 18 at main.cpp:6
+    [-30] 0x0000000000400694    movl   -0x4(%rbp), %eax
+  a.out`main + 49 [inlined] inline_function() + 15 at main.cpp:5
+    [-31] 0x0000000000400691    movl   %eax, -0x4(%rbp)
+    [-32] 0x000000000040068c    addl   $0x1, %eax
+    [-33] 0x0000000000400689    movl   -0x4(%rbp), %eax
+  a.out`main + 34 [inlined] inline_function() at main.cpp:4
+    [-34] 0x0000000000400682    movl   $0x0, -0x4(%rbp)
+  a.out`main + 31 at main.cpp:12
+    [-35] 0x000000000040067f    movl   %eax, -0xc(%rbp)
+    [-36] 0x000000000040067a    addl   $0x1, %eax
+    [-37] 0x0000000000400677    movl   -0xc(%rbp), %eax
+  a.out`main + 20 at main.cpp:10
+    [-38] 0x0000000000400674    movl   %eax, -0xc(%rbp)
+    ...missing instructions
+    [-39] 0x00007ffff7df1950    error: no memory mapped at this address
+  a.out`(none)
+    [-40] 0x0000000000400516    jmpq   *0x200af4(%rip)           ; _GLOBAL_OFFSET_TABLE_ + 16
+    [-41] 0x0000000000400510    pushq  0x200af2(%rip)            ; _GLOBAL_OFFSET_TABLE_ + 8
+  a.out`symbol stub for: foo() + 11
+    [-42] 0x000000000040054b    jmp    0x400510
+    [-43] 0x0000000000400546    pushq  $0x2
+    [-44] 0x0000000000400540    jmpq   *0x200ae2(%rip)           ; _GLOBAL_OFFSET_TABLE_ + 40
+  a.out`main + 15 at main.cpp:10
+    [-45] 0x000000000040066f    callq  0x400540                  ; symbol stub for: foo()'''])
+
+        self.expect("thread trace dump instructions --skip 100 -f", inHistory=True,
+            substrs=['''thread #1: tid = 815455
+    no more data'''])
+
+        self.expect("", substrs=['''thread #1: tid = 815455
+    no more data'''])

lldb/test/API/commands/trace/TestTraceStartStop.py

class TestTraceStartStop(TraceIntelPTTestCaseBase):
 
     @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
     def testStoppingAThread(self):
         self.expect("file " + os.path.join(self.getSourceDir(), "intelpt-trace", "a.out"))
         self.expect("b main")
         self.expect("r")
         self.expect("thread trace start")
         self.expect("n")
-        self.expect("thread trace dump instructions", substrs=["total instructions"])
+        self.expect("thread trace dump instructions", substrs=["""0x0000000000400511    movl   $0x0, -0x4(%rbp)
+    no more data"""])
         # process stopping should stop the thread
         self.expect("process trace stop")
         self.expect("n")
-        self.expect("thread trace dump instructions", error=True, substrs=["not traced"])
+        self.expect("thread trace dump instructions", substrs=["not traced"])
 
 
     @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
     def testStartStopLiveThreads(self):
         # The help command should be the generic one if there's no process running
         self.expectGenericHelpMessageForStartCommand()
 
         self.expect("thread trace start", error=True,
def testStartStopLiveThreads(self):
         self.expect("thread trace start 1 --size 4096")
 
         # We fail if we try to trace again
         self.expect("thread trace start", error=True,
             substrs=["error: Thread ", "already traced"])
 
         # We can reconstruct the single instruction executed in the first line
         self.expect("n")
-        self.expect("thread trace dump instructions",
-            patterns=[f'''thread #1: tid = .*, total instructions = 1
+        self.expect("thread trace dump instructions -f",
+            patterns=[f'''thread #1: tid = .*
   a.out`main \+ 4 at main.cpp:2
-    \[0\] {ADDRESS_REGEX}    movl'''])
+    \[ 0\] {ADDRESS_REGEX}    movl'''])
 
         # We can reconstruct the instructions up to the second line
         self.expect("n")
-        self.expect("thread trace dump instructions",
-            patterns=[f'''thread #1: tid = .*, total instructions = 5
+        self.expect("thread trace dump instructions -f",
+            patterns=[f'''thread #1: tid = .*
   a.out`main \+ 4 at main.cpp:2
-    \[0\] {ADDRESS_REGEX}    movl .*
+    \[ 0\] {ADDRESS_REGEX}    movl .*
   a.out`main \+ 11 at main.cpp:4
-    \[1\] {ADDRESS_REGEX}    movl .*
-    \[2\] {ADDRESS_REGEX}    jmp  .* ; <\+28> at main.cpp:4
-    \[3\] {ADDRESS_REGEX}    cmpl .*
-    \[4\] {ADDRESS_REGEX}    jle  .* ; <\+20> at main.cpp:5'''])
+    \[ 1\] {ADDRESS_REGEX}    movl .*
+    \[ 2\] {ADDRESS_REGEX}    jmp  .* ; <\+28> at main.cpp:4
+    \[ 3\] {ADDRESS_REGEX}    cmpl .*
+    \[ 4\] {ADDRESS_REGEX}    jle  .* ; <\+20> at main.cpp:5'''])
 
+        self.expect("thread trace dump instructions",
+            patterns=[f'''thread #1: tid = .*
+  a.out`main \+ 32 at main.cpp:4
+    \[  0\] {ADDRESS_REGEX}    jle  .* ; <\+20> at main.cpp:5
+    \[ -1\] {ADDRESS_REGEX}    cmpl .*
+    \[ -2\] {ADDRESS_REGEX}    jmp  .* ; <\+28> at main.cpp:4
+    \[ -3\] {ADDRESS_REGEX}    movl .*
+  a.out`main \+ 4 at main.cpp:2
+    \[ -4\] {ADDRESS_REGEX}    movl .* '''])
+
         # We stop tracing
         self.expect("thread trace stop")
 
         # We can't stop twice
         self.expect("thread trace stop", error=True,
             substrs=["error: Thread ", "not currently traced"])
 
         # We trace again from scratch, this time letting LLDB to pick the current
         # thread
         self.expect("thread trace start")
         self.expect("n")
+        self.expect("thread trace dump instructions -f",
+            patterns=[f'''thread #1: tid = .*
+  a.out`main \+ 20 at main.cpp:5
+    \[ 0\] {ADDRESS_REGEX}    xorl'''])
+
         self.expect("thread trace dump instructions",
-            patterns=[f'''thread #1: tid = .*, total instructions = 1
+            patterns=[f'''thread #1: tid = .*
   a.out`main \+ 20 at main.cpp:5
-    \[0\] {ADDRESS_REGEX}    xorl'''])
+    \[  0\] {ADDRESS_REGEX}    xorl'''])
 
         self.expect("c")
         # Now the process has finished, so the commands should fail
         self.expect("thread trace start", error=True,
             substrs=["error: Process must be launched"])
 
         self.expect("thread trace stop", error=True,
             substrs=["error: Process must be launched"])

lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py

def testStartMultipleLiveThreadsWithStops(self):
 
         # The trace is still in memory
         self.expect("thread trace dump instructions 2", substrs=['main.cpp:9'])
 
         # We'll stop at the next breakpoint in thread 3, thread 2 and 3 will be alive, but only 3 traced.
         self.expect("continue")
         self.expect("thread trace dump instructions", substrs=['main.cpp:4'])
         self.expect("thread trace dump instructions 3", substrs=['main.cpp:4'])
-        self.expect("thread trace dump instructions 1", error=True, substrs=['not traced'])
-        self.expect("thread trace dump instructions 2", error=True, substrs=['not traced'])
+        self.expect("thread trace dump instructions 1", substrs=['not traced'])
+        self.expect("thread trace dump instructions 2", substrs=['not traced'])
 
     @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
     def testStartMultipleLiveThreadsWithThreadStartAll(self):
         self.build()
         exe = self.getBuildArtifact("a.out")
         target = self.dbg.CreateTarget(exe)
 
         self.expect("b main")
def testStartMultipleLiveThreadsWithThreadStartAll(self):
         # We stop tracing all
         self.runCmd("thread trace stop all")
 
         # The trace is still in memory
         self.expect("thread trace dump instructions 2", substrs=['main.cpp:11'])
 
         # We'll stop at the next breakpoint in thread 3, and nothing should be traced
         self.expect("continue")
-        self.expect("thread trace dump instructions 3", error=True, substrs=['not traced'])
-        self.expect("thread trace dump instructions 1", error=True, substrs=['not traced'])
-        self.expect("thread trace dump instructions 2", error=True, substrs=['not traced'])
+        self.expect("thread trace dump instructions 3", substrs=['not traced'])
+        self.expect("thread trace dump instructions 1", substrs=['not traced'])
+        self.expect("thread trace dump instructions 2", substrs=['not traced'])
 
     @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
     @testSBAPIAndCommands
     def testStartMultipleLiveThreadsWithSmallTotalLimit(self):
         self.build()
         exe = self.getBuildArtifact("a.out")
 
         self.dbg.CreateTarget(exe)