[trace] Add an option to dump instructions in json and to a file

[wallace]

Summary

In order to provide simple scripting support on top of instruction traces, a simple solution is to enhance the dump instructions command and allow printing in json and directly to a file. The format is verbose and not space efficient, but it's not supposed to be used for really large traces, in which case the TraceCursor API is the way to go.

• add a -j option for printing the dump in json
• add a -J option for pretty printing the json output
• add a -F option for specifying an output file
• add a -a option for dumping all the instructions available starting at the initial point configured with the other flags. This is useful for dumping all the instructions to a file in one go
• add tests for all cases
• refactored the instruction dumper and abstracted the actual "printing" logic. There are two writer implementations: CLI and JSON. This made the dumper itself much more readable and maintanable

sample output:

(lldb) thread trace dump instructions  -t -a --id 100 -J
[
  {
    "id": 100,
    "tsc": "43591204528448966"
    "loadAddress": "0x407a91",
    "module": "a.out",
    "symbol": "void std::deque<Foo, std::allocator<Foo>>::_M_push_back_aux<Foo>(Foo&&)",
    "mnemonic": "movq",
    "source": "/usr/include/c++/8/bits/deque.tcc",
    "line": 492,
    "column": 30
  },
  ...

[wallace]

add a test for -a

[jj10306]

looks great overall, just a couple minor comments!

lldb/include/lldb/Target/TraceInstructionDumper.h

46

nit: wdyt ab renaming this to TraceInstructionsDumper since now we have the TraceInstructionWriter which is responsible for displaying a single instruction? With how things are currently named it's slightly confusing at first glance because there names sound like they are both doing the same thing (something related to a single instruction).

75

since the JSON subclass doesn't need to implement this, consider providing a default stubbed out implementation at the super class level so that subclasses aren't necessarily required to implement it?

120–122

nit: to be consistent with the --raw flag of the thread trace dump instructions command which says:

-r ( --raw )
     Dump only instruction address without disassembly nor symbol information.

lldb/source/Commands/CommandObjectThread.cpp

2210

Why is this stored on the command object and not the TraceinstructionDumperOptions?
Can you explain the separation of responsibility of the dumper options versus the options here?

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

121–122

nit: maybe use the new HasId method here?

lldb/source/Target/TraceInstructionDumper.cpp

90–124

Move declarations to header?

193

219–224

the values being passed to the attribute method are c strings. Unsure how this method handles a nullptr, but consider checking if these are nullptr before calling attribute if necessary?

[wallace]

lldb/include/lldb/Target/TraceInstructionDumper.h

46

I'm renaming TraceInstructionWriter to OutputWriter, just because that one is an internal class

lldb/source/Commands/CommandObjectThread.cpp

2210

basically just because this field is not used in the dumper at all. The dumper requires a Stream object, which is configured by the caller, either a file stream or a standard output stream (or maybe a python string stream). So in this case, because this field is only used by this command handler to create the actual Stream to pass to the dumper, I'm restricting this variable to here.

lldb/source/Target/TraceInstructionDumper.cpp

90–124

in this case I'd rather not do that because I don't want anyone to see these classes. They are very restricted to the use case of the dumper and no one should try to use them for any reason. So I'm making them very private intentionally

219–224

good catch!

[wallace]

address comments

[jj10306]

lldb/source/Target/TraceInstructionDumper.cpp

190–191

the inline comment should be next to the second parameter, right?

[wallace]

lldb/source/Target/TraceInstructionDumper.cpp

190–191

silly me

lldb/include/lldb/Target/TraceCursor.h

public:
   ///
   /// \return
   ///     \b true if the given identifier exists and the cursor effectively
   ///     moved. Otherwise, \b false is returned and the cursor doesn't change
   ///     its position.
   virtual bool GoToId(lldb::user_id_t id) = 0;
 
   /// \return
+  ///     \b true if and only if there's an instruction item with the given \p
+  ///     id.
+  virtual bool HasId(lldb::user_id_t id) const = 0;
+
+  /// \return
   ///     A unique identifier for the instruction or error this cursor is
   ///     pointing to.
   virtual lldb::user_id_t GetId() const = 0;
   /// \}
 
   /// Make the cursor point to an item in the trace based on an origin point and
   /// an offset. This API doesn't distinguishes instruction types nor errors in
   /// the trace, unlike the \a TraceCursor::Next() method.

lldb/include/lldb/Target/TraceInstructionDumper.h

 
 #include "lldb/Symbol/SymbolContext.h"
 
 #ifndef LLDB_TARGET_TRACE_INSTRUCTION_DUMPER_H
 #define LLDB_TARGET_TRACE_INSTRUCTION_DUMPER_H
 
 namespace lldb_private {
 
-/// 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;
-};
-
 /// Class that holds the configuration used by \a TraceInstructionDumper for
 /// traversing and dumping instructions.
 struct TraceInstructionDumperOptions {
   /// If \b true, the cursor will be iterated forwards starting from the
   /// oldest instruction. Otherwise, the iteration starts from the most
   /// recent instruction.
   bool forwards = false;
   /// Dump only instruction addresses without disassembly nor symbol
   /// information.
   bool raw = false;
+  /// Dump in json format.
+  bool json = false;
+  /// When dumping in JSON format, pretty print the output.
+  bool pretty_print_json = false;
   /// For each instruction, print the corresponding timestamp counter if
   /// available.
   bool show_tsc = false;
   /// Dump the events that happened between instructions.
   bool show_events = false;
   /// Optional custom id to start traversing from.
   llvm::Optional<uint64_t> id = llvm::None;
   /// Optional number of instructions to skip from the starting position
   /// of the cursor.
   llvm::Optional<size_t> skip = llvm::None;
 };
 
 /// Class used to dump the instructions of a \a TraceCursor using its current
 /// state and granularity.
 class TraceInstructionDumper {
 public:
+  /// Helper struct that holds symbol, disassembly and address information of an
+  /// instruction.
+  struct SymbolInfo {
+    SymbolContext sc;
+    Address address;
+    lldb::DisassemblerSP disassembler;
+    lldb::InstructionSP instruction;
+    lldb_private::ExecutionContext exe_ctx;
+  };
+
+  /// Helper struct that holds all the information we know about an instruction
+  struct InstructionEntry {
+    lldb::user_id_t id;
+    lldb::addr_t load_address;
+    llvm::Optional<uint64_t> tsc;
+    llvm::Optional<llvm::StringRef> error;
+    llvm::Optional<SymbolInfo> symbol_info;
+    llvm::Optional<SymbolInfo> prev_symbol_info;
+  };
+
+  /// Interface used to abstract away the format in which the instruction
+  /// information will be dumped.
+  class TraceInstructionWriter {
+  public:
+    virtual ~TraceInstructionWriter() = default;
+
+    /// Indicate a user-level info message. It's not part of the actual trace.
+    virtual void InfoMessage(llvm::StringRef text) = 0;
+
+    /// Dump a trace event.
+    virtual void Event(llvm::StringRef text) = 0;
+
+    /// Dump an instruction or a trace error.
+    virtual void Instruction(const InstructionEntry &insn) = 0;
+  };
+
   /// Create a instruction dumper for the cursor.
   ///
   /// \param[in] cursor
   ///     The cursor whose instructions will be dumped.
   ///
   /// \param[in] s
   ///     The stream where to dump the instructions to.
   ///
public:
   ///     The instruction id of the last traversed instruction, or \b llvm::None
   ///     if no instructions were visited.
   llvm::Optional<lldb::user_id_t> DumpInstructions(size_t count);
 
   /// \return
   ///     \b true if there's still more data to traverse in the trace.
   bool HasMoreData();
 
-private:
   /// Indicate to the dumper that no more data is available in the trace.
   /// This will prevent further iterations.
   void SetNoMoreData();
 
-  /// Move the cursor one step.
-  ///
-  /// \return
-  ///     \b true if the cursor moved.
-  bool TryMoveOneStep();
+private:
+  /// Create an instruction entry for the current position without symbol
+  /// information.
+  InstructionEntry CreateBasicInstructionEntry();
 
   void PrintEvents();
 
-  void PrintMissingInstructionsMessage();
-
-  void PrintInstructionHeader();
-
-  void DumpInstructionDisassembly(const InstructionSymbolInfo &insn);
-
-  /// 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.
-  void DumpInstructionSymbolContext(
-      const llvm::Optional<InstructionSymbolInfo> &prev_insn,
-      const InstructionSymbolInfo &insn);
-
   lldb::TraceCursorUP m_cursor_up;
   TraceInstructionDumperOptions m_options;
-  Stream &m_s;
   /// If \b true, all the instructions have been traversed.
   bool m_no_more_data = false;
+  std::unique_ptr<TraceInstructionWriter> m_writer_up;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_TARGET_TRACE_INSTRUCTION_DUMPER_H

lldb/source/Commands/CommandObjectThread.cpp

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 'a': {
+        m_count = std::numeric_limits<decltype(m_count)>::max();
+        break;
+      }
       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_dumper_options.skip = skip;
         break;
       }
       case 'i': {
         uint64_t id;
         if (option_arg.empty() || option_arg.getAsInteger(0, id))
           error.SetErrorStringWithFormat(
               "invalid integer value for option '%s'",
               option_arg.str().c_str());
         else
           m_dumper_options.id = id;
         break;
       }
+      case 'F': {
+        m_output_file.emplace(option_arg);
+        break;
+      }
       case 'r': {
         m_dumper_options.raw = true;
         break;
       }
       case 'f': {
         m_dumper_options.forwards = true;
         break;
       }
       case 't': {
         m_dumper_options.show_tsc = true;
         break;
       }
       case 'e': {
         m_dumper_options.show_events = true;
         break;
       }
+      case 'j': {
+        m_dumper_options.json = true;
+        break;
+      }
+      case 'J': {
+        m_dumper_options.pretty_print_json = true;
+        m_dumper_options.json = true;
+        break;
+      }
       case 'C': {
         m_continue = true;
         break;
       }
       default:
         llvm_unreachable("Unimplemented option");
       }
       return error;
     }
 
     void OptionParsingStarting(ExecutionContext *execution_context) override {
       m_count = kDefaultCount;
       m_continue = false;
+      m_output_file = llvm::None;
       m_dumper_options = {};
     }
 
     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;
     size_t m_continue;
+    llvm::Optional<FileSpec> m_output_file;
     TraceInstructionDumperOptions m_dumper_options;
   };
 
   CommandObjectTraceDumpInstructions(CommandInterpreter &interpreter)
       : CommandObjectParsed(
             interpreter, "thread trace dump instructions",
             "Dump the traced instructions for one thread. If no "
             "thread is specified, show the current thread.",
ThreadSP thread_sp =
             thread_idx);
     if (!thread_sp)
       result.AppendErrorWithFormat("no thread with index: \"%s\"\n", arg);
     return thread_sp;
   }
 
   bool DoExecute(Args &args, CommandReturnObject &result) override {
     ThreadSP thread_sp = GetThread(args, result);
-    if (!thread_sp)
+    if (!thread_sp) {
+      result.AppendError("invalid thread\n");
       return false;
-
-    Stream &s = result.GetOutputStream();
-    s.Printf("thread #%u: tid = %" PRIu64 "\n", thread_sp->GetIndexID(),
-             thread_sp->GetID());
-
-    if (m_options.m_continue) {
-      if (!m_last_id) {
-        result.AppendMessage("    no more data\n");
-        return true;
-      }
+
+    if (m_options.m_continue && m_last_id) {
       // We set up the options to continue one instruction past where
       // the previous iteration stopped.
       m_options.m_dumper_options.skip = 1;
       m_options.m_dumper_options.id = m_last_id;
     }
 
-    const TraceSP &trace_sp = m_exe_ctx.GetTargetSP()->GetTrace();
-    TraceInstructionDumper dumper(trace_sp->GetCursor(*thread_sp), s,
-                                  m_options.m_dumper_options);
+    TraceCursorUP cursor_up =
+        m_exe_ctx.GetTargetSP()->GetTrace()->GetCursor(*thread_sp);
+
+    if (m_options.m_dumper_options.id &&
+        !cursor_up->HasId(*m_options.m_dumper_options.id)) {
+      result.AppendError("invalid instruction id\n");
+      return false;
+    }
+
+    llvm::Optional<StreamFile> out_file;
+    if (m_options.m_output_file) {
+      out_file.emplace(m_options.m_output_file->GetPath().c_str(),
+                       File::eOpenOptionWriteOnly | File::eOpenOptionCanCreate,
+                       lldb::eFilePermissionsFileDefault);
+    }
+
+    TraceInstructionDumper dumper(
+        std::move(cursor_up), out_file ? *out_file : result.GetOutputStream(),
+
+    if (m_options.m_continue && !m_last_id) {
+      // We need to tell the dumper to stop processing data when
+      // we already ran out of instructions in a previous command
+      dumper.SetNoMoreData();
+    }
+
     m_last_id = dumper.DumpInstructions(m_options.m_count);
     return true;
   }
 
   CommandOptions m_options;
   // Last traversed id used to continue a repeat command. None means
   // that all the trace has been consumed.
   llvm::Optional<lldb::user_id_t> m_last_id;

lldb/source/Commands/Options.td

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 starting at the most recent "
     "instruction, or the oldest if --forwards is provided.">;
+  def thread_trace_dump_instructions_all : Option<"all", "a">, Group<1>,
+    Desc<"From the starting point of the trace, dump all instructions "
+    "available.">;
   def thread_trace_dump_instructions_id: Option<"id", "i">, Group<1>,
     Arg<"Index">,
     Desc<"Custom starting instruction id from where to start traversing. This "
     "id can be provided in decimal or hexadecimal representation.">;
   def thread_trace_dump_instructions_skip: Option<"skip", "s">, Group<1>,
     Arg<"Index">,
     Desc<"How many instruction to skip from the starting position of the trace "
     "before starting the traversal.">;
-  def thread_trace_dump_instructions_raw : Option<"raw", "r">,
-    Group<1>,
+  def thread_trace_dump_instructions_raw : Option<"raw", "r">, Group<1>,
     Desc<"Dump only instruction address without disassembly nor symbol "
     "information.">;
+  def thread_trace_dump_instructions_file : Option<"file", "F">, Group<1>,
+    Arg<"Filename">,
+    Desc<"Dump the instruction to a file instead of the standard output.">;
+  def thread_trace_dump_instructions_json: Option<"json", "j">,
+    Group<1>,
+    Desc<"Dump in simple JSON format.">;
+  def thread_trace_dump_instructions_pretty_print: Option<"pretty-json", "J">,
+    Group<1>,
+    Desc<"Dump in JSON format but pretty printing the output for easier readability.">;
   def thread_trace_dump_instructions_show_tsc : Option<"tsc", "t">, Group<1>,
     Desc<"For each instruction, print the corresponding timestamp counter if "
     "available.">;
-  def thread_trace_dump_instructions_hide_events : Option<"events", "e">,
+  def thread_trace_dump_instructions_show_events : Option<"events", "e">,
     Group<1>,
     Desc<"Dump the events that happened during the execution of the target.">;
   def thread_trace_dump_instructions_continue: Option<"continue", "C">,
     Group<1>,
     Desc<"Continue dumping instructions right where the previous invocation of "
     "this command was left, or from the beginning if this is the first "
     "invocation. The --skip argument is discarded and the other arguments are "
     "preserved from the previous invocation when possible.">;

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

public:
   GetInstructionControlFlowType() override;
 
   bool IsError() override;
 
   bool GoToId(lldb::user_id_t id) override;
 
   lldb::user_id_t GetId() const override;
 
+  bool HasId(lldb::user_id_t id) const override;
+
 private:
   size_t GetInternalInstructionSize();
 
   /// Storage of the actual instructions
   DecodedThreadSP m_decoded_thread_sp;
   /// Internal instruction index currently pointing at.
   size_t m_pos;
   /// Tsc range covering the current instruction.
   llvm::Optional<DecodedThread::TscRange> m_tsc_range;
 };
 
 } // namespace trace_intel_pt
 } // namespace lldb_private
 
 #endif // LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACECURSORINTELPT_H

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

 }
 
 TraceInstructionControlFlowType
 TraceCursorIntelPT::GetInstructionControlFlowType() {
   return m_decoded_thread_sp->GetInstructionControlFlowType(m_pos);
 }
 
 bool TraceCursorIntelPT::GoToId(user_id_t id) {
   if (m_decoded_thread_sp->GetInstructionsCount() <= id)
     return false;
   m_pos = id;
   m_tsc_range = m_decoded_thread_sp->CalculateTscRange(m_pos, m_tsc_range);
 
   return true;
 }
 
+bool TraceCursorIntelPT::HasId(lldb::user_id_t id) const {
+  return id < m_decoded_thread_sp->GetInstructionsCount();
+}
+
 user_id_t TraceCursorIntelPT::GetId() const { return m_pos; }

lldb/source/Target/TraceInstructionDumper.cpp

 //===-- TraceInstructionDumper.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/CompileUnit.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, Stream &s,
-    const TraceInstructionDumperOptions &options)
-    : m_cursor_up(std::move(cursor_up)), m_options(options), m_s(s) {
-  // We first set the cursor in its initial position
-  if (m_options.id) {
-    if (!m_cursor_up->GoToId(*m_options.id)) {
-      s.PutCString("    invalid instruction id\n");
-      SetNoMoreData();
-      return;
-    }
-  } else if (m_options.forwards) {
-    m_cursor_up->Seek(0, TraceCursor::SeekType::Beginning);
-  } else {
-    m_cursor_up->Seek(0, TraceCursor::SeekType::End);
-  }
-
-  m_cursor_up->SetForwards(m_options.forwards);
-  if (m_options.skip) {
-    uint64_t to_skip = m_options.skip.getValue();
-    if (m_cursor_up->Seek((m_options.forwards ? 1 : -1) * to_skip,
-                          TraceCursor::SeekType::Current) < to_skip) {
-      // This happens when the skip value was more than the number of
-      // available instructions.
-      SetNoMoreData();
-    }
-  }
-}
-
-bool TraceInstructionDumper::TryMoveOneStep() {
-  if (!m_cursor_up->Next()) {
-    SetNoMoreData();
-    return false;
-  }
-  return true;
+/// \return
+///   The module name (basename if the module is a file, or the actual name if
+///   it's a virtual module), or \b nullptr if no name nor module was found.
+static const char *
+GetModuleName(const TraceInstructionDumper::InstructionEntry &insn) {
+  if (!insn.symbol_info)
+    return nullptr;
+  if (!insn.symbol_info->sc.module_sp)
+    return nullptr;
+  return insn.symbol_info->sc.module_sp->GetFileSpec()
+      .GetFilename()
+      .AsCString();
 }
 
 // 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
+/// Compare the symbol contexts of the provided \a SymbolInfo
 /// 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) {
+static bool IsSameInstructionSymbolContext(
+    const TraceInstructionDumper::SymbolInfo &prev_insn,
+    const TraceInstructionDumper::SymbolInfo &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;
 }
 
-void TraceInstructionDumper::DumpInstructionSymbolContext(
-    const Optional<InstructionSymbolInfo> &prev_insn,
-    const InstructionSymbolInfo &insn) {
-  if (prev_insn && IsSameInstructionSymbolContext(*prev_insn, insn))
-    return;
+class TraceInstructionWriterCLI
+    : public TraceInstructionDumper::TraceInstructionWriter {
+public:
+  TraceInstructionWriterCLI(Stream &s,
+                            const TraceInstructionDumperOptions &options)
+      : m_s(s), m_options(options){};
 
-  m_s << "  ";
+  void InfoMessage(StringRef text) override { m_s << "    " << text << "\n"; }
+
+  void Event(StringRef text) override { m_s.Format("  [{0}]\n", text); }
 
-  if (!insn.sc.module_sp)
+  void
+  Instruction(const TraceInstructionDumper::InstructionEntry &insn) override {
+    if (insn.symbol_info) {
+      if (!insn.prev_symbol_info ||
+          !IsSameInstructionSymbolContext(*insn.prev_symbol_info,
+                                          *insn.symbol_info)) {
+        m_s << "  ";
+        const char *module_name = GetModuleName(insn);
+        if (!module_name)
           m_s << "(none)";
-  else if (!insn.sc.function && !insn.sc.symbol)
-    m_s.Format("{0}`(none)",
-               insn.sc.module_sp->GetFileSpec().GetFilename().AsCString());
+        else if (!insn.symbol_info->sc.function && !insn.symbol_info->sc.symbol)
+          m_s.Format("{0}`(none)", module_name);
         else
-    insn.sc.DumpStopContext(&m_s, insn.exe_ctx.GetTargetPtr(), insn.address,
-                            /*show_fullpaths=*/false,
-                            /*show_module=*/true, /*show_inlined_frames=*/false,
-                            /*show_function_arguments=*/true,
-                            /*show_function_name=*/true);
+          insn.symbol_info->sc.DumpStopContext(
+              &m_s, insn.symbol_info->exe_ctx.GetTargetPtr(),
+              insn.symbol_info->address,
         m_s << "\n";
       }
+    }
 
-void TraceInstructionDumper::DumpInstructionDisassembly(
-    const InstructionSymbolInfo &insn) {
-  if (!insn.instruction)
-    return;
+    if (insn.error && !m_was_prev_instruction_an_error)
+      InfoMessage("...missing instructions");
+
+    m_s.Format("    {0}: ", insn.id);
+
+    if (m_options.show_tsc) {
+      m_s << "[tsc=";
+
+      if (insn.tsc)
+        m_s.Format("{0}", *insn.tsc);
+      else
+        m_s << "unavailable";
+
+      m_s << "] ";
+    }
+
+    if (insn.error) {
+      m_s << *insn.error;
+      m_was_prev_instruction_an_error = true;
+    } else {
+      m_s.Format("{0:x+16}", insn.load_address);
+      if (insn.symbol_info) {
         m_s << "    ";
-  insn.instruction->Dump(&m_s, /*max_opcode_byte_size=*/0,
+        insn.symbol_info->instruction->Dump(&m_s, /*max_opcode_byte_size=*/0,
                                             /*show_address=*/false,
-                         /*show_bytes=*/false, &insn.exe_ctx, &insn.sc,
+                                            /*show_bytes=*/false,
+                                            &insn.symbol_info->exe_ctx,
+                                            &insn.symbol_info->sc,
                                             /*prev_sym_ctx=*/nullptr,
                                             /*disassembly_addr_format=*/nullptr,
                                             /*max_address_text_size=*/0);
       }
+      m_was_prev_instruction_an_error = false;
+    }
+    m_s << "\n";
+  }
 
-void TraceInstructionDumper::SetNoMoreData() { m_no_more_data = true; }
+private:
+  Stream &m_s;
+  TraceInstructionDumperOptions m_options;
+  bool m_was_prev_instruction_an_error = false;
+};
+
+class TraceInstructionWriterJSON
+    : public TraceInstructionDumper::TraceInstructionWriter {
+  /* schema:
+    error_message: string
+    | {
+      "event": string
+    } | {
+      "id": decimal,
+      "tsc"?: string decimal,
+      "error": string,
+    | {
+      "id": decimal,
+      "tsc"?: string decimal,
+      "module"?: string,
+      "symbol"?: string,
+      "line"?: decimal,
+      "column"?: decimal,
+      "source"?: string,
+      "mnemonic"?: string,
+    }
+  */
+public:
+  TraceInstructionWriterJSON(Stream &s,
+                             const TraceInstructionDumperOptions &options)
+      : m_s(s), m_options(options),
+        m_j(m_s.AsRawOstream(), options.pretty_print_json ? 2 : 0) {
+    m_j.arrayBegin();
+  };
 
-bool TraceInstructionDumper::HasMoreData() { return !m_no_more_data; }
+  ~TraceInstructionWriterJSON() { m_j.arrayEnd(); }
+
+  void InfoMessage(StringRef text) override {}
 
-void TraceInstructionDumper::PrintMissingInstructionsMessage() {
-  m_s << "    ...missing instructions\n";
+  void Event(StringRef text) override {
+    m_j.object([&] { m_j.attribute("event", text); });
   }
 
-void TraceInstructionDumper::PrintInstructionHeader() {
-  m_s.Format("    {0}: ", m_cursor_up->GetId());
+  void
+  Instruction(const TraceInstructionDumper::InstructionEntry &insn) override {
+    m_j.object([&] {
+      m_j.attribute("id", insn.id);
+      if (m_options.show_tsc)
+        m_j.attribute("tsc", insn.tsc ? Optional<std::string>(std::to_string(*insn.tsc)) : None);
 
-  if (m_options.show_tsc) {
-    m_s << "[tsc=";
+      if (insn.error) {
+        m_j.attribute("error", *insn.error);
+        return;
+      }
 
-    if (Optional<uint64_t> timestamp =
-            m_cursor_up->GetCounter(lldb::eTraceCounterTSC))
-      m_s.Format("{0:x+16}", *timestamp);
-    else
-      m_s << "unavailable";
+      m_j.attribute("loadAddress", formatv("{0:x}", insn.load_address));
+      if (insn.symbol_info) {
+        m_j.attribute("module", GetModuleName(insn));
+        m_j.attribute("symbol",
+                      insn.symbol_info->sc.GetFunctionName().AsCString());
+        m_j.attribute("mnemonic", insn.symbol_info->instruction->GetMnemonic(
+                                      &insn.symbol_info->exe_ctx));
+
+        if (IsLineEntryValid(insn.symbol_info->sc.line_entry)) {
+          m_j.attribute("source",
+                        insn.symbol_info->sc.line_entry.file.GetPath());
+          m_j.attribute("line", insn.symbol_info->sc.line_entry.line);
+          m_j.attribute("column", insn.symbol_info->sc.line_entry.column);
+        }
+      }
+    });
+  }
+
+private:
+  Stream &m_s;
+  TraceInstructionDumperOptions m_options;
+  json::OStream m_j;
+};
+
+static std::unique_ptr<TraceInstructionDumper::TraceInstructionWriter>
+CreateWriter(Stream &s, const TraceInstructionDumperOptions &options) {
+  if (options.json)
+    return std::unique_ptr<TraceInstructionDumper::TraceInstructionWriter>(
+        new TraceInstructionWriterJSON(s, options));
+    return std::unique_ptr<TraceInstructionDumper::TraceInstructionWriter>(
+        new TraceInstructionWriterCLI(s, options));
+}
 
-    m_s << "] ";
+TraceInstructionDumper::TraceInstructionDumper(
+    lldb::TraceCursorUP &&cursor_up, Stream &s,
+    const TraceInstructionDumperOptions &options)
+    : m_cursor_up(std::move(cursor_up)), m_options(options),
+      m_writer_up(CreateWriter(s, m_options)) {
+
+  if (m_options.id) {
+    if (!m_cursor_up->GoToId(*m_options.id)) {
+      m_writer_up->InfoMessage("invalid instruction id");
+      SetNoMoreData();
     }
+  } else if (m_options.forwards) {
+    m_cursor_up->Seek(0, TraceCursor::SeekType::Beginning);
+  } else {
+    m_cursor_up->Seek(0, TraceCursor::SeekType::End);
+  }
+
+  m_cursor_up->SetForwards(m_options.forwards);
+  if (m_options.skip) {
+    uint64_t to_skip = m_options.skip.getValue();
+    if (m_cursor_up->Seek((m_options.forwards ? 1 : -1) * to_skip,
+                          TraceCursor::SeekType::Current) < to_skip) {
+      // This happens when the skip value was more than the number of
+      // available instructions.
+      SetNoMoreData();
+    }
+  }
+}
+
+void TraceInstructionDumper::SetNoMoreData() { m_no_more_data = true; }
+
+bool TraceInstructionDumper::HasMoreData() { return !m_no_more_data; }
+
+TraceInstructionDumper::InstructionEntry
+TraceInstructionDumper::CreateBasicInstructionEntry() {
+  InstructionEntry insn;
+  insn.id = m_cursor_up->GetId();
+
+  if (m_options.show_tsc)
+    insn.tsc = m_cursor_up->GetCounter(lldb::eTraceCounterTSC);
+  return insn;
 }
 
 void TraceInstructionDumper::PrintEvents() {
   if (!m_options.show_events)
     return;
 
   trace_event_utils::ForEachEvent(
       m_cursor_up->GetEvents(), [&](TraceEvents event) {
-        m_s.Format("  [{0}]\n", trace_event_utils::EventToDisplayString(event));
+        m_writer_up->Event(trace_event_utils::EventToDisplayString(event));
       });
 }
 
 /// Find the symbol context for the given address reusing the previous
 /// instruction's symbol context when possible.
-static SymbolContext
-CalculateSymbolContext(const Address &address,
-                       const InstructionSymbolInfo &prev_insn_info) {
+static SymbolContext CalculateSymbolContext(
+    const Address &address,
+    const TraceInstructionDumper::SymbolInfo &prev_symbol_info) {
   AddressRange range;
-  if (prev_insn_info.sc.GetAddressRange(eSymbolContextEverything, 0,
-                                        /*inline_block_range*/ false, range) &&
+  if (prev_symbol_info.sc.GetAddressRange(eSymbolContextEverything, 0,
+                                          /*inline_block_range*/ false,
+                                          range) &&
       range.Contains(address))
-    return prev_insn_info.sc;
+    return prev_symbol_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.
 static std::tuple<DisassemblerSP, InstructionSP>
-CalculateDisass(const InstructionSymbolInfo &insn_info,
-                const InstructionSymbolInfo &prev_insn_info,
+CalculateDisass(const TraceInstructionDumper::SymbolInfo &symbol_info,
+                const TraceInstructionDumper::SymbolInfo &prev_symbol_info,
                 const ExecutionContext &exe_ctx) {
-  if (prev_insn_info.disassembler) {
+  if (prev_symbol_info.disassembler) {
     if (InstructionSP instruction =
-            prev_insn_info.disassembler->GetInstructionList()
-                .GetInstructionAtAddress(insn_info.address))
-      return std::make_tuple(prev_insn_info.disassembler, instruction);
+            prev_symbol_info.disassembler->GetInstructionList()
+                .GetInstructionAtAddress(symbol_info.address))
+      return std::make_tuple(prev_symbol_info.disassembler, instruction);
   }
 
-  if (insn_info.sc.function) {
+  if (symbol_info.sc.function) {
     if (DisassemblerSP disassembler =
-            insn_info.sc.function->GetInstructions(exe_ctx, nullptr)) {
+            symbol_info.sc.function->GetInstructions(exe_ctx, nullptr)) {
       if (InstructionSP instruction =
               disassembler->GetInstructionList().GetInstructionAtAddress(
-                  insn_info.address))
+                  symbol_info.address))
         return std::make_tuple(disassembler, instruction);
     }
   }
   // We fallback to a single instruction disassembler
   Target &target = exe_ctx.GetTargetRef();
   const ArchSpec arch = target.GetArchitecture();
-  AddressRange range(insn_info.address, arch.GetMaximumOpcodeByteSize());
+  AddressRange range(symbol_info.address, arch.GetMaximumOpcodeByteSize());
   DisassemblerSP disassembler =
       Disassembler::DisassembleRange(arch, /*plugin_name*/ nullptr,
                                      /*flavor*/ nullptr, target, range);
   return std::make_tuple(
       disassembler,
       disassembler ? disassembler->GetInstructionList().GetInstructionAtAddress(
-                         insn_info.address)
+                         symbol_info.address)
                    : InstructionSP());
 }
 
 Optional<lldb::user_id_t>
 TraceInstructionDumper::DumpInstructions(size_t count) {
   ThreadSP thread_sp = m_cursor_up->GetExecutionContextRef().GetThreadSP();
-  if (!thread_sp) {
-    m_s << "invalid thread";
-    return None;
-  }
 
-  bool was_prev_instruction_an_error = false;
-  InstructionSymbolInfo prev_insn_info;
+  m_writer_up->InfoMessage(formatv("thread #{0}: tid = {1}",
+                                   thread_sp->GetIndexID(), thread_sp->GetID())
+                               .str());
+
+  SymbolInfo prev_symbol_info;
   Optional<lldb::user_id_t> last_id;
 
   ExecutionContext exe_ctx;
   thread_sp->GetProcess()->GetTarget().CalculateExecutionContext(exe_ctx);
 
   for (size_t i = 0; i < count; i++) {
     if (!HasMoreData()) {
-      m_s << "    no more data\n";
+      m_writer_up->InfoMessage("no more data");
       break;
     }
     last_id = m_cursor_up->GetId();
+
     if (m_options.forwards) {
       // When moving forwards, we first print the event before printing
       // the actual instruction.
       PrintEvents();
     }
 
-    if (const char *err = m_cursor_up->GetError()) {
-      if (!m_cursor_up->IsForwards() && !was_prev_instruction_an_error)
-        PrintMissingInstructionsMessage();
-
-      was_prev_instruction_an_error = true;
+    InstructionEntry insn = CreateBasicInstructionEntry();
 
-      PrintInstructionHeader();
-      m_s << err;
+    if (const char *err = m_cursor_up->GetError()) {
+      insn.error = err;
+      m_writer_up->Instruction(insn);
     } else {
-      if (m_cursor_up->IsForwards() && was_prev_instruction_an_error)
-        PrintMissingInstructionsMessage();
-
-      was_prev_instruction_an_error = false;
-
-      InstructionSymbolInfo insn_info;
+      insn.load_address = m_cursor_up->GetLoadAddress();
 
       if (!m_options.raw) {
-        insn_info.load_address = m_cursor_up->GetLoadAddress();
-        insn_info.exe_ctx = exe_ctx;
-        insn_info.address.SetLoadAddress(insn_info.load_address,
+        SymbolInfo symbol_info;
+        symbol_info.exe_ctx = exe_ctx;
+        symbol_info.address.SetLoadAddress(insn.load_address,
                                            exe_ctx.GetTargetPtr());
-        insn_info.sc =
-            CalculateSymbolContext(insn_info.address, prev_insn_info);
-        std::tie(insn_info.disassembler, insn_info.instruction) =
-            CalculateDisass(insn_info, prev_insn_info, exe_ctx);
-
-        DumpInstructionSymbolContext(prev_insn_info, insn_info);
+        symbol_info.sc =
+            CalculateSymbolContext(symbol_info.address, prev_symbol_info);
+        std::tie(symbol_info.disassembler, symbol_info.instruction) =
+            CalculateDisass(symbol_info, prev_symbol_info, exe_ctx);
+        insn.prev_symbol_info = prev_symbol_info;
+        insn.symbol_info = symbol_info;
+        prev_symbol_info = symbol_info;
       }
-
-      PrintInstructionHeader();
-      m_s.Format("{0:x+16}", m_cursor_up->GetLoadAddress());
-
-      if (!m_options.raw)
-        DumpInstructionDisassembly(insn_info);
-
-      prev_insn_info = insn_info;
+      m_writer_up->Instruction(insn);
     }
 
-    m_s << "\n";
-
     if (!m_options.forwards) {
       // If we move backwards, we print the events after printing
       // the actual instruction so that reading chronologically
       // makes sense.
       PrintEvents();
     }
-    TryMoveOneStep();
+
+    if (!m_cursor_up->Next())
+      SetNoMoreData();
   }
   return last_id;
 }

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

def testErrorMessages(self):
         # Now we check the output when there's a running target without a trace
         self.expect("b main")
         self.expect("run")
 
         self.expect("thread trace dump instructions",
             substrs=["error: Process is not being traced"],
             error=True)
 
+    def testRawDumpInstructionsInJSON(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 --count 5 --forwards --json",
+            substrs=['''[{"id":0,"loadAddress":"0x400511"},{"id":1,"loadAddress":"0x400518"},{"id":2,"loadAddress":"0x40051f"},{"id":3,"loadAddress":"0x400529"},{"id":4,"loadAddress":"0x40052d"}]'''])
+
+        self.expect("thread trace dump instructions --raw --count 5 --forwards --pretty-json",
+            substrs=['''[
+  {
+    "id": 0,
+    "loadAddress": "0x400511"
+  },
+  {
+    "id": 1,
+    "loadAddress": "0x400518"
+  },
+  {
+    "id": 2,
+    "loadAddress": "0x40051f"
+  },
+  {
+    "id": 3,
+    "loadAddress": "0x400529"
+  },
+  {
+    "id": 4,
+    "loadAddress": "0x40052d"
+  }
+]'''])
+
+    def testRawDumpInstructionsInJSONToFile(self):
+        self.expect("trace load -v " +
+            os.path.join(self.getSourceDir(), "intelpt-trace", "trace.json"),
+            substrs=["intel-pt"])
+
+        outfile = os.path.join(self.getBuildDir(), "output.json")
+
+        self.expect("thread trace dump instructions --raw --count 5 --forwards --pretty-json --file " + outfile)
+
+        with open(outfile, "r") as out:
+            self.assertEqual(out.read(), '''[
+  {
+    "id": 0,
+    "loadAddress": "0x400511"
+  },
+  {
+    "id": 1,
+    "loadAddress": "0x400518"
+  },
+  {
+    "id": 2,
+    "loadAddress": "0x40051f"
+  },
+  {
+    "id": 3,
+    "loadAddress": "0x400529"
+  },
+  {
+    "id": 4,
+    "loadAddress": "0x40052d"
+  }
+]''')
+
     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 --count 21 --forwards",
             substrs=['''thread #1: tid = 3842849
     0: 0x0000000000400511
2: 0x000000000040051f    jmp    0x400529                  ; <+28> at main.cpp:4'''])
         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 --forwards",
             substrs=['''thread #1: tid = 3842849
+    ...missing instructions
     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 --forwards",
             substrs=['''thread #1: tid = 3842849
+    ...missing instructions
     0: error: unknown cpu'''])
 
+    def testMultiFileTraceWithMissingModuleInJSON(self):
+        self.expect("trace load " +
+            os.path.join(self.getSourceDir(), "intelpt-trace-multi-file", "multi-file-no-ld.json"))
+
+        self.expect("thread trace dump instructions --count 3 --id 4 --forwards --pretty-json",
+            substrs=['''[
+  {
+    "id": 4,
+    "loadAddress": "0x400510",
+    "module": "a.out",
+    "symbol": "",
+    "mnemonic": "pushq"
+  },
+  {
+    "id": 5,
+    "loadAddress": "0x400516",
+    "module": "a.out",
+    "symbol": "",
+    "mnemonic": "jmpq"
+  },
+  {
+    "id": 6,
+    "error": "0x00007ffff7df1950    error: no memory mapped at this address"
+  }
+]'''])
+
+        self.expect("thread trace dump instructions --count 4 --id 20 --forwards --pretty-json",
+                substrs=['''[
+  {
+    "id": 20,
+    "loadAddress": "0x400540",
+    "module": "a.out",
+    "symbol": "foo()",
+    "mnemonic": "jmpq"
+  },
+  {
+    "id": 21,
+    "loadAddress": "0x7ffff7bd96e0",
+    "module": "libfoo.so",
+    "symbol": "foo()",
+    "mnemonic": "pushq",
+    "source": "/home/wallace/llvm-sand/external/llvm-project/lldb/test/API/commands/trace/intelpt-trace-multi-file/foo.cpp",
+    "line": 3,
+    "column": 0
+  },
+  {
+    "id": 22,
+    "loadAddress": "0x7ffff7bd96e1",
+    "module": "libfoo.so",
+    "symbol": "foo()",
+    "mnemonic": "movq",
+    "source": "/home/wallace/llvm-sand/external/llvm-project/lldb/test/API/commands/trace/intelpt-trace-multi-file/foo.cpp",
+    "line": 3,
+    "column": 0
+  },
+  {
+    "id": 23,
+    "loadAddress": "0x7ffff7bd96e4",
+    "module": "libfoo.so",
+    "symbol": "foo()",
+    "mnemonic": "subq",
+    "source": "/home/wallace/llvm-sand/external/llvm-project/lldb/test/API/commands/trace/intelpt-trace-multi-file/foo.cpp",
+    "line": 4,
+    "column": 0
+  }
+]'''])
+
     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,
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
     5: 0x0000000000400516    jmpq   *0x200af4(%rip)           ; _GLOBAL_OFFSET_TABLE_ + 16
-    6: 0x00007ffff7df1950    error: no memory mapped at this address
     ...missing instructions
+    6: 0x00007ffff7df1950    error: no memory mapped at this address
   a.out`main + 20 at main.cpp:10
     7: 0x0000000000400674    movl   %eax, -0xc(%rbp)
   a.out`main + 23 at main.cpp:12
     8: 0x0000000000400677    movl   -0xc(%rbp), %eax
     9: 0x000000000040067a    addl   $0x1, %eax
     10: 0x000000000040067f    movl   %eax, -0xc(%rbp)
   a.out`main + 34 [inlined] inline_function() at main.cpp:4
     11: 0x0000000000400682    movl   $0x0, -0x4(%rbp)

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

 import lldb
 from intelpt_testcase import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 from lldbsuite.test.decorators import *
 
 class TestTraceLoad(TraceIntelPTTestCaseBase):
     NO_DEBUG_INFO_TESTCASE = True
 
     @testSBAPIAndCommands
     def testLoadMultiCoreTrace(self):
         src_dir = self.getSourceDir()
         trace_description_file_path = os.path.join(src_dir, "intelpt-multi-core-trace", "trace.json")
         self.traceLoad(traceDescriptionFilePath=trace_description_file_path, substrs=["intel-pt"])
         self.expect("thread trace dump instructions 2 -t",
-          substrs=["19521: [tsc=0x008fb5211c143fd8] error: expected tracing enabled event",
+          substrs=["19521: [tsc=40450075479261144] error: expected tracing enabled event",
                    "m.out`foo() + 65 at multi_thread.cpp:12:21",
-                   "19520: [tsc=0x008fb5211bfbc69e] 0x0000000000400ba7    jg     0x400bb3"])
+                   "19520: [tsc=40450075477657246] 0x0000000000400ba7    jg     0x400bb3"])
         self.expect("thread trace dump instructions 3 -t",
-          substrs=["67910: [tsc=0x008fb5211bfdf270] 0x0000000000400bd7    addl   $0x1, -0x4(%rbp)",
+          substrs=["67910: [tsc=40450075477799536] 0x0000000000400bd7    addl   $0x1, -0x4(%rbp)",
                    "m.out`bar() + 26 at multi_thread.cpp:20:6"])
 
     @testSBAPIAndCommands
     def testLoadMultiCoreTraceWithStringNumbers(self):
         src_dir = self.getSourceDir()
         trace_description_file_path = os.path.join(src_dir, "intelpt-multi-core-trace", "trace_with_string_numbers.json")
         self.traceLoad(traceDescriptionFilePath=trace_description_file_path, substrs=["intel-pt"])
         self.expect("thread trace dump instructions 2 -t",
-          substrs=["19521: [tsc=0x008fb5211c143fd8] error: expected tracing enabled event",
+          substrs=["19521: [tsc=40450075479261144] error: expected tracing enabled event",
                    "m.out`foo() + 65 at multi_thread.cpp:12:21",
-                   "19520: [tsc=0x008fb5211bfbc69e] 0x0000000000400ba7    jg     0x400bb3"])
+                   "19520: [tsc=40450075477657246] 0x0000000000400ba7    jg     0x400bb3"])
         self.expect("thread trace dump instructions 3 -t",
-          substrs=["67910: [tsc=0x008fb5211bfdf270] 0x0000000000400bd7    addl   $0x1, -0x4(%rbp)",
+          substrs=["67910: [tsc=40450075477799536] 0x0000000000400bd7    addl   $0x1, -0x4(%rbp)",
                    "m.out`bar() + 26 at multi_thread.cpp:20:6"])
 
     @testSBAPIAndCommands
     def testLoadMultiCoreTraceWithMissingThreads(self):
         src_dir = self.getSourceDir()
         trace_description_file_path = os.path.join(src_dir, "intelpt-multi-core-trace", "trace_missing_threads.json")
         self.traceLoad(traceDescriptionFilePath=trace_description_file_path, substrs=["intel-pt"])
         self.expect("thread trace dump instructions 3 -t",
-          substrs=["19521: [tsc=0x008fb5211c143fd8] error: expected tracing enabled event",
+          substrs=["19521: [tsc=40450075479261144] error: expected tracing enabled event",
                    "m.out`foo() + 65 at multi_thread.cpp:12:21",
-                   "19520: [tsc=0x008fb5211bfbc69e] 0x0000000000400ba7    jg     0x400bb3"])
+                   "19520: [tsc=40450075477657246] 0x0000000000400ba7    jg     0x400bb3"])
         self.expect("thread trace dump instructions 2 -t",
-          substrs=["67910: [tsc=0x008fb5211bfdf270] 0x0000000000400bd7    addl   $0x1, -0x4(%rbp)",
+          substrs=["67910: [tsc=40450075477799536] 0x0000000000400bd7    addl   $0x1, -0x4(%rbp)",
                    "m.out`bar() + 26 at multi_thread.cpp:20:6"])
 
     @testSBAPIAndCommands
     def testLoadTrace(self):
         src_dir = self.getSourceDir()
         trace_description_file_path = os.path.join(src_dir, "intelpt-trace", "trace.json")
         self.traceLoad(traceDescriptionFilePath=trace_description_file_path, substrs=["intel-pt"])
 

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

def testTscPerThread(self):
         self.expect("file " + os.path.join(self.getSourceDir(), "intelpt-trace", "a.out"))
         self.expect("b main")
         self.expect("r")
 
         self.traceStartThread(enableTsc=True)
 
         self.expect("n")
         self.expect("thread trace dump instructions --tsc -c 1",
-            patterns=["0: \[tsc=0x[0-9a-fA-F]+\] 0x0000000000400511    movl"])
+            patterns=["0: \[tsc=\d+\] 0x0000000000400511    movl"])
 
     @testSBAPIAndCommands
     @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
     def testMultipleTscsPerThread(self):
         self.expect("file " + os.path.join(self.getSourceDir(), "intelpt-trace", "a.out"))
         self.expect("b main")
         self.expect("r")
 
def testTscPerProcess(self):
         self.expect("file " + os.path.join(self.getSourceDir(), "intelpt-trace", "a.out"))
         self.expect("b main")
         self.expect("r")
 
         self.traceStartProcess(enableTsc=True)
 
         self.expect("n")
         self.expect("thread trace dump instructions --tsc -c 1",
-            patterns=["0: \[tsc=0x[0-9a-fA-F]+\] 0x0000000000400511    movl"])
+            patterns=["0: \[tsc=\d+\] 0x0000000000400511    movl"])
+
+        self.expect("thread trace dump instructions --tsc -c 1 --pretty-json",
+            patterns=['''"tsc": "\d+"'''])
 
     @testSBAPIAndCommands
     @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
     def testDumpingAfterTracingWithoutTsc(self):
         self.expect("file " + os.path.join(self.getSourceDir(), "intelpt-trace", "a.out"))
         self.expect("b main")
         self.expect("r")
 
         self.traceStartThread(enableTsc=False)
 
         self.expect("n")
         self.expect("thread trace dump instructions --tsc -c 1",
             patterns=["0: \[tsc=unavailable\] 0x0000000000400511    movl"])
 
+        self.expect("thread trace dump instructions --tsc -c 1 --json",
+            patterns=['''"tsc":null'''])
+
     @testSBAPIAndCommands
     @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
     def testPSBPeriod(self):
         def isPSBSupported():
             caps_file = "/sys/bus/event_source/devices/intel_pt/caps/psb_cyc"
             if not os.path.exists(caps_file):
                 return False
             with open(caps_file, "r") as f: