wallace updated this revision to Diff 351329.
wallace added a comment.
Herald added a subscriber: mgorny.

Following @vsk 's feedback, I made the following changes:

- Create a ThreadTrace class, that contains the trace for a specific thread and 
offers the TraverseInstructions method and other getters like 
GetInstructionType and GetInstructionSize. This effectively moves some code 
from the Trace.h class to this new class, which has a few advantanges:
  - It's easier to expose an SBAPI for instructions and its data storage with a 
corresponding SBThreadTrace class. Having only SBTrace is not enough for that.
  - A ThreadTrace can potentially outlive a stop id in the case of a live 
process, which could be useful if the user wants to compare traces or two 
different points in the execution of a process.
- I'm keeping the vector<IntelPTInstruction> class for now as an internal 
representation, but now there's more freedom to change it because the top level 
API in ThreadTrace makes no assumptions on the data, except that it's indexed.
- I removed some deprecated code and improved some comments.

As a side note, I was chatting with some teammates and we'll explore storing 
decoded intel pt traces on disk, which later lldb will mmap and use as its 
storage. The accessors exposed in this diff are generic enough to handle that 
case as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103588

Files:
  lldb/include/lldb/Target/ThreadTrace.h
  lldb/include/lldb/Target/Trace.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.h
  lldb/source/Plugins/Trace/intel-pt/ThreadTraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/ThreadTraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Target/Trace.cpp
  lldb/test/API/commands/trace/TestTraceDumpInstructions.py

Index: lldb/test/API/commands/trace/TestTraceDumpInstructions.py
===================================================================
--- lldb/test/API/commands/trace/TestTraceDumpInstructions.py
+++ lldb/test/API/commands/trace/TestTraceDumpInstructions.py
@@ -160,8 +160,7 @@
         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'''])
+            substrs=['''thread #1: tid = 3842849, error: unknown cpu'''])
 
     def testMultiFileTraceWithMissingModule(self):
         self.expect("trace load " +
Index: lldb/source/Target/Trace.cpp
===================================================================
--- lldb/source/Target/Trace.cpp
+++ lldb/source/Target/Trace.cpp
@@ -141,8 +141,8 @@
 ///     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,
+    ThreadTraceSP thread_trace_sp, Thread &thread, size_t position,
+    TraceDirection direction, SymbolContextItem symbol_scope,
     bool include_disassembler,
     std::function<bool(size_t index, Expected<InstructionSymbolInfo> insn)>
         callback) {
@@ -197,8 +197,8 @@
                                         : InstructionSP());
   };
 
-  trace.TraverseInstructions(
-      thread, position, direction,
+  thread_trace_sp->TraverseInstructions(
+      position, direction,
       [&](size_t index, Expected<lldb::addr_t> load_address) -> bool {
         if (!load_address)
           return callback(index, load_address.takeError());
@@ -301,59 +301,59 @@
 
 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, thread.GetIndexID(), thread.GetID());
+
+  if (Expected<ThreadTraceSP> thread_trace_sp = GetThreadTrace(thread)) {
+    size_t instructions_count = thread_trace_sp.get()->GetInstructionCount();
+    s.Printf(", total instructions = %zu\n", 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(
+        *thread_trace_sp, 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;
+        });
+
+  } else {
+    s.Printf(", %s\n", llvm::toString(thread_trace_sp.takeError()).c_str());
   }
-
-  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) {
@@ -439,27 +439,35 @@
   return m_live_process->TraceGetBinaryData(request);
 }
 
-void Trace::RefreshLiveProcessState() {
+Error Trace::RefreshLiveProcessState() {
   if (!m_live_process)
-    return;
+    return Error::success();
 
   uint32_t new_stop_id = m_live_process->GetStopID();
-  if (new_stop_id == m_stop_id)
-    return;
+  if (new_stop_id == m_stop_id) {
+    if (m_live_process_refresh_error)
+      return createStringError(inconvertibleErrorCode(),
+                               *m_live_process_refresh_error);
+    else
+      return Error::success();
+  }
 
   m_stop_id = new_stop_id;
   m_live_thread_data.clear();
+  m_live_process_refresh_error = None;
 
   Expected<std::string> json_string = GetLiveProcessState();
   if (!json_string) {
-    DoRefreshLiveProcessState(json_string.takeError());
-    return;
+    m_live_process_refresh_error = toString(json_string.takeError());
+    return createStringError(inconvertibleErrorCode(),
+                             *m_live_process_refresh_error);
   }
   Expected<TraceGetStateResponse> live_process_state =
       json::parse<TraceGetStateResponse>(*json_string, "TraceGetStateResponse");
   if (!live_process_state) {
-    DoRefreshLiveProcessState(live_process_state.takeError());
-    return;
+    m_live_process_refresh_error = toString(live_process_state.takeError());
+    return createStringError(inconvertibleErrorCode(),
+                             *m_live_process_refresh_error);
   }
 
   for (const TraceThreadState &thread_state :
@@ -471,5 +479,5 @@
   for (const TraceBinaryData &item : live_process_state->processBinaryData)
     m_live_process_data[item.kind] = item.size;
 
-  DoRefreshLiveProcessState(std::move(live_process_state));
+  return DoRefreshLiveProcessState(*live_process_state);
 }
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
@@ -65,19 +65,10 @@
 
   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::Error
+  DoRefreshLiveProcessState(const TraceGetStateResponse &state) override;
 
-  llvm::Optional<size_t> GetInstructionCount(Thread &thread) override;
-
-  size_t GetCursorPosition(Thread &thread) override;
-
-  void DoRefreshLiveProcessState(
-      llvm::Expected<TraceGetStateResponse> state) override;
-
-  bool IsTraced(const Thread &thread) override;
+  llvm::Expected<lldb::ThreadTraceSP> GetThreadTrace(Thread &thread) override;
 
   const char *GetStartConfigurationHelp() override;
 
@@ -144,25 +135,19 @@
   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.
-  ///
-  /// \param[in] thread
-  ///     If \a thread is a \a ThreadTrace, then its internal trace file will be
-  ///     decoded. Live threads are not currently supported.
+  /// Decode the trace of the given thread, i.e. recontruct the traced
+  /// instructions.
   ///
   /// \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 ThreadTraceIntelPT instance if decoding was successful, or an \a
+  ///     llvm::Error if the thread's trace is not managed by this class or if
+  ///     there was an error setting up the the decoder or the raw trace data.
+  llvm::Expected<ThreadTraceIntelPTSP> 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;
 };
 
 } // namespace trace_intel_pt
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -88,46 +88,18 @@
         thread.get(), std::make_unique<PostMortemThreadDecoder>(thread, *this));
 }
 
-const DecodedThread *TraceIntelPT::Decode(Thread &thread) {
-  RefreshLiveProcessState();
-  if (m_failed_live_threads_decoder.hasValue())
-    return &*m_failed_live_threads_decoder;
-
-  auto it = m_thread_decoders.find(&thread);
-  if (it == m_thread_decoders.end())
-    return nullptr;
-  return &it->second->Decode();
+llvm::Expected<ThreadTraceSP> TraceIntelPT::GetThreadTrace(Thread &thread) {
+  return Decode(thread);
 }
 
-size_t TraceIntelPT::GetCursorPosition(Thread &thread) {
-  const DecodedThread *decoded_thread = Decode(thread);
-  if (!decoded_thread)
-    return 0;
-  return decoded_thread->GetCursorPosition();
-}
+llvm::Expected<ThreadTraceIntelPTSP> TraceIntelPT::Decode(Thread &thread) {
+  if (llvm::Error err = RefreshLiveProcessState())
+    return std::move(err);
 
-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;
+  auto it = m_thread_decoders.find(&thread);
+  if (it == m_thread_decoders.end())
+    return createStringError(inconvertibleErrorCode(), "thread not traced");
+  return it->second->Decode();
 }
 
 Expected<pt_cpu> TraceIntelPT::GetCPUInfoForLiveProcess() {
@@ -192,26 +164,16 @@
   return *m_cpu_info;
 }
 
-void TraceIntelPT::DoRefreshLiveProcessState(
-    Expected<TraceGetStateResponse> state) {
+Error TraceIntelPT::DoRefreshLiveProcessState(
+    const TraceGetStateResponse &state) {
   m_thread_decoders.clear();
-
-  if (!state) {
-    m_failed_live_threads_decoder = DecodedThread(state.takeError());
-    return;
-  }
-
-  for (const TraceThreadState &thread_state : state->tracedThreads) {
+  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));
   }
-}
-
-bool TraceIntelPT::IsTraced(const Thread &thread) {
-  RefreshLiveProcessState();
-  return m_thread_decoders.count(&thread);
+  return Error::success();
 }
 
 const char *TraceIntelPT::GetStartConfigurationHelp() {
Index: lldb/source/Plugins/Trace/intel-pt/ThreadTraceIntelPT.h
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/ThreadTraceIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/ThreadTraceIntelPT.h
@@ -1,4 +1,4 @@
-//===-- DecodedThread.h -----------------------------------------*- C++ -*-===//
+//===-- ThreadTraceIntelPT.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.
@@ -6,12 +6,11 @@
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_DECODEDTHREAD_H
-#define LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_DECODEDTHREAD_H
+#ifndef LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_THREAD_TRACE_H
+#define LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_THREAD_TRACE_H
 
 #include <vector>
 
-#include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
 
 #include "lldb/Target/Trace.h"
@@ -84,6 +83,10 @@
   ///     error.
   llvm::Expected<lldb::addr_t> GetLoadAddress() const;
 
+  lldb::TraceInstructionType GetInstructionType() const;
+
+  size_t GetInstructionSize() const;
+
   /// \return
   ///     An \a llvm::Error object if this class corresponds to an Error, or an
   ///     \a llvm::Error::success otherwise.
@@ -99,53 +102,37 @@
   std::unique_ptr<llvm::ErrorInfoBase> m_error;
 };
 
-/// \class DecodedThread
+/// \class ThreadTraceIntelPT
 /// 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 ThreadTraceIntelPT : public ThreadTrace {
 public:
-  DecodedThread(std::vector<IntelPTInstruction> &&instructions)
-      : m_instructions(std::move(instructions)), m_position(GetLastPosition()) {
-  }
+  ThreadTraceIntelPT(std::vector<IntelPTInstruction> &&instructions)
+      : m_instructions(std::move(instructions)) {}
 
-  /// Constructor with a single error signaling a complete failure of the
-  /// decoding process.
-  DecodedThread(llvm::Error error);
+  void TraverseInstructions(
+      size_t start_position, TraceDirection direction,
+      std::function<bool(size_t index,
+                         llvm::Expected<lldb::addr_t> load_address)>
+          callback) override;
 
-  /// 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;
+  size_t GetInstructionCount() override;
 
-  /// \return
-  ///   The current position of the cursor of this trace, or 0 if there are no
-  ///   instructions.
-  size_t GetCursorPosition() const;
+  lldb::TraceInstructionType GetInstructionType(size_t index) override;
 
-  /// 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);
-  /// \}
+  size_t GetInstructionSize(size_t index) override;
 
 private:
-  /// \return
-  ///     The index of the last element of the trace, or 0 if empty.
-  size_t GetLastPosition() const;
-
   std::vector<IntelPTInstruction> m_instructions;
-  size_t m_position;
 };
 
+using ThreadTraceIntelPTSP = std::shared_ptr<ThreadTraceIntelPT>;
+
 } // namespace trace_intel_pt
 } // namespace lldb_private
 
-#endif // LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_DECODEDTHREAD_H
+#endif // LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_THREAD_TRACE_H
Index: lldb/source/Plugins/Trace/intel-pt/ThreadTraceIntelPT.cpp
===================================================================
--- /dev/null
+++ lldb/source/Plugins/Trace/intel-pt/ThreadTraceIntelPT.cpp
@@ -0,0 +1,98 @@
+//===-- ThreadTraceIntelPT.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 "ThreadTraceIntelPT.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;
+}
+
+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());
+}
+
+TraceInstructionType IntelPTInstruction::GetInstructionType() const {
+  switch (m_pt_insn.iclass) {
+  case ptic_other:
+    return lldb::eTraceInstructionOther;
+  case ptic_call:
+  case ptic_far_call:
+    return lldb::eTraceInstructionCall;
+  case ptic_return:
+  case ptic_far_return:
+    return lldb::eTraceInstructionReturn;
+  case ptic_jump:
+  case ptic_far_jump:
+    return lldb::eTraceInstructionJump;
+  case ptic_cond_jump:
+    return lldb::eTraceInstructionCondJump;
+  case ptic_ptwrite:
+    return lldb::eTraceInstructionTraceWrite;
+  default:
+    return lldb::eTraceInstructionUnknown;
+  }
+}
+
+size_t IntelPTInstruction::GetInstructionSize() const { return m_pt_insn.size; }
+
+void ThreadTraceIntelPT::TraverseInstructions(
+    size_t start_position, TraceDirection direction,
+    std::function<bool(size_t index, llvm::Expected<lldb::addr_t> load_address)>
+        callback) {
+  ssize_t delta = direction == TraceDirection::Forwards ? 1 : -1;
+  for (ssize_t i = start_position; i < (ssize_t)m_instructions.size() && i >= 0;
+       i += delta)
+    if (!callback(i, m_instructions[i].GetLoadAddress()))
+      break;
+}
+
+size_t ThreadTraceIntelPT::GetInstructionCount() {
+  return m_instructions.size();
+}
+
+lldb::TraceInstructionType
+ThreadTraceIntelPT::GetInstructionType(size_t index) {
+  return m_instructions[index].GetInstructionType();
+}
+
+size_t ThreadTraceIntelPT::GetInstructionSize(size_t index) {
+  return m_instructions[index].GetInstructionSize();
+}
Index: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.h
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.h
+++ lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.h
@@ -11,7 +11,7 @@
 
 #include "intel-pt.h"
 
-#include "DecodedThread.h"
+#include "ThreadTraceIntelPT.h"
 #include "forward-declarations.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Utility/FileSpec.h"
@@ -31,7 +31,7 @@
   ///
   /// \return
   ///     A \a DecodedThread instance.
-  const DecodedThread &Decode();
+  llvm::Expected<ThreadTraceIntelPTSP> Decode();
 
   ThreadDecoder(const ThreadDecoder &other) = delete;
   ThreadDecoder &operator=(const ThreadDecoder &other) = delete;
@@ -41,9 +41,9 @@
   ///
   /// \return
   ///     A \a DecodedThread instance.
-  virtual DecodedThread DoDecode() = 0;
+  virtual llvm::Expected<ThreadTraceIntelPTSP> DoDecode() = 0;
 
-  llvm::Optional<DecodedThread> m_decoded_thread;
+  llvm::Optional<ThreadTraceIntelPTSP> m_decoded_thread;
 };
 
 /// Decoder implementation for \a lldb_private::ThreadPostMortemTrace, which are
@@ -59,7 +59,7 @@
                           TraceIntelPT &trace);
 
 private:
-  DecodedThread DoDecode() override;
+  llvm::Expected<ThreadTraceIntelPTSP> DoDecode() override;
 
   lldb::ThreadPostMortemTraceSP m_trace_thread;
   TraceIntelPT &m_trace;
@@ -75,7 +75,7 @@
   LiveThreadDecoder(Thread &thread, TraceIntelPT &trace);
 
 private:
-  DecodedThread DoDecode() override;
+  llvm::Expected<ThreadTraceIntelPTSP> DoDecode() override;
 
   lldb::ThreadSP m_thread_sp;
   TraceIntelPT &m_trace;
Index: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
+++ lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
@@ -8,19 +8,32 @@
 #include "IntelPTDecoder.h"
 
 #include "llvm/Support/MemoryBuffer.h"
+#include <intel-pt.h>
 
 #include "TraceIntelPT.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/ThreadPostMortemTrace.h"
-#include "lldb/Utility/StringExtractor.h"
 
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::trace_intel_pt;
 using namespace llvm;
 
+static llvm::Error
+CreateIntelPTError(int libipt_error_code,
+                   lldb::addr_t address = LLDB_INVALID_ADDRESS) {
+  const char *libipt_error_message = pt_errstr(pt_errcode(libipt_error_code));
+  if (address != LLDB_INVALID_ADDRESS && address > 0) {
+    return createStringError(inconvertibleErrorCode(),
+                             "0x%16.16" PRIx64 "    error: %s", address,
+                             libipt_error_message);
+  }
+  return createStringError(inconvertibleErrorCode(), "error: %s",
+                           libipt_error_message);
+}
+
 /// Move the decoder forward to the next synchronization point (i.e. next PSB
 /// packet).
 ///
@@ -113,7 +126,7 @@
       break;
 
     if (errcode < 0) {
-      instructions.emplace_back(make_error<IntelPTError>(errcode));
+      instructions.emplace_back(CreateIntelPTError(errcode));
       break;
     }
 
@@ -122,7 +135,7 @@
     while (true) {
       errcode = ProcessPTEvents(decoder, errcode);
       if (errcode < 0) {
-        instructions.emplace_back(make_error<IntelPTError>(errcode));
+        instructions.emplace_back(CreateIntelPTError(errcode));
         break;
       }
       pt_insn insn;
@@ -132,7 +145,7 @@
         break;
 
       if (errcode < 0) {
-        instructions.emplace_back(make_error<IntelPTError>(errcode, insn.ip));
+        instructions.emplace_back(CreateIntelPTError(errcode, insn.ip));
         break;
       }
 
@@ -171,14 +184,14 @@
   config.cpu = *cpu_info;
 
   if (int errcode = pt_cpu_errata(&config.errata, &config.cpu))
-    return make_error<IntelPTError>(errcode);
+    return CreateIntelPTError(errcode);
 
   config.begin = buffer.data();
   config.end = buffer.data() + buffer.size();
 
   pt_insn_decoder *decoder = pt_insn_alloc_decoder(&config);
   if (!decoder)
-    return make_error<IntelPTError>(-pte_nomem);
+    return CreateIntelPTError(-pte_nomem);
 
   pt_image *image = pt_insn_get_image(decoder);
 
@@ -222,9 +235,14 @@
     return cpu_info.takeError();
 }
 
-const DecodedThread &ThreadDecoder::Decode() {
-  if (!m_decoded_thread.hasValue())
-    m_decoded_thread = DoDecode();
+Expected<ThreadTraceIntelPTSP> ThreadDecoder::Decode() {
+  if (!m_decoded_thread.hasValue()) {
+    if (Expected<ThreadTraceIntelPTSP> thread_trace_sp = DoDecode()) {
+      m_decoded_thread = *thread_trace_sp;
+    } else {
+      return thread_trace_sp.takeError();
+    }
+  }
   return *m_decoded_thread;
 }
 
@@ -232,22 +250,22 @@
     const lldb::ThreadPostMortemTraceSP &trace_thread, TraceIntelPT &trace)
     : m_trace_thread(trace_thread), m_trace(trace) {}
 
-DecodedThread PostMortemThreadDecoder::DoDecode() {
+Expected<ThreadTraceIntelPTSP> 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<ThreadTraceIntelPT>(std::move(*instructions));
   else
-    return DecodedThread(instructions.takeError());
+    return instructions.takeError();
 }
 
 LiveThreadDecoder::LiveThreadDecoder(Thread &thread, TraceIntelPT &trace)
     : m_thread_sp(thread.shared_from_this()), m_trace(trace) {}
 
-DecodedThread LiveThreadDecoder::DoDecode() {
+Expected<ThreadTraceIntelPTSP> LiveThreadDecoder::DoDecode() {
   if (Expected<std::vector<IntelPTInstruction>> instructions =
           DecodeLiveThread(*m_thread_sp, m_trace))
-    return DecodedThread(std::move(*instructions));
+    return std::make_shared<ThreadTraceIntelPT>(std::move(*instructions));
   else
-    return DecodedThread(instructions.takeError());
+    return instructions.takeError();
 }
Index: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
+++ /dev/null
@@ -1,69 +0,0 @@
-//===-- 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 "lldb/Utility/StreamString.h"
-
-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;
-}
-
-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;
-}
-
-ArrayRef<IntelPTInstruction> DecodedThread::GetInstructions() const {
-  return makeArrayRef(m_instructions);
-}
-
-size_t DecodedThread::GetCursorPosition() const { return m_position; }
-
-size_t DecodedThread::SetCursorPosition(size_t new_position) {
-  m_position = std::min(new_position, GetLastPosition());
-  return m_position;
-}
-
-DecodedThread::DecodedThread(Error error) {
-  m_instructions.emplace_back(std::move(error));
-  m_position = GetLastPosition();
-}
Index: lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
+++ lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
@@ -15,7 +15,7 @@
 
 add_lldb_library(lldbPluginTraceIntelPT PLUGIN
   CommandObjectTraceStartIntelPT.cpp
-  DecodedThread.cpp
+  ThreadTraceIntelPT.cpp
   IntelPTDecoder.cpp
   TraceIntelPT.cpp
   TraceIntelPTSessionFileParser.cpp
Index: lldb/source/Commands/CommandObjectThread.cpp
===================================================================
--- lldb/source/Commands/CommandObjectThread.cpp
+++ lldb/source/Commands/CommandObjectThread.cpp
@@ -2140,16 +2140,25 @@
     const TraceSP &trace_sp = m_exe_ctx.GetTargetSP()->GetTrace();
     ThreadSP thread_sp =
         m_exe_ctx.GetProcessPtr()->GetThreadList().FindThreadByID(tid);
+    if (llvm::Expected<ThreadTraceSP> thread_trace_sp =
+            trace_sp->GetThreadTrace(*thread_sp)) {
+      size_t count = m_options.m_count;
+      ssize_t position =
+          m_options.m_position.getValueOr(
+              (ssize_t)thread_trace_sp.get()->GetInstructionCount() - 1) -
+          m_consecutive_repetitions * count;
+      if (position < 0)
+        result.SetError("error: no more data");
+      else
+        trace_sp->DumpTraceInstructions(*thread_sp, result.GetOutputStream(),
+                                        count, position, m_options.m_raw);
+    } else {
+      result.GetOutputStream().Printf(
+          "thread #%u: tid = %" PRIu64 ", %s", thread_sp->GetIndexID(),
+          thread_sp->GetID(),
+          llvm::toString(thread_trace_sp.takeError()).c_str());
+    }
 
-    size_t count = m_options.m_count;
-    ssize_t position = m_options.m_position.getValueOr(
-                           trace_sp->GetCursorPosition(*thread_sp)) -
-                       m_consecutive_repetitions * count;
-    if (position < 0)
-      result.SetError("error: no more data");
-    else
-      trace_sp->DumpTraceInstructions(*thread_sp, result.GetOutputStream(),
-                                      count, position, m_options.m_raw);
     return true;
   }
 
Index: lldb/include/lldb/lldb-forward.h
===================================================================
--- lldb/include/lldb/lldb-forward.h
+++ lldb/include/lldb/lldb-forward.h
@@ -226,8 +226,9 @@
 class ThreadPlanStepRange;
 class ThreadPlanStepThrough;
 class ThreadPlanTracer;
-class ThreadSpec;
 class ThreadPostMortemTrace;
+class ThreadSpec;
+class ThreadTrace;
 class Trace;
 class TraceSessionFileParser;
 class Type;
@@ -440,6 +441,7 @@
     ThreadPostMortemTraceSP;
 typedef std::weak_ptr<lldb_private::ThreadPlan> ThreadPlanWP;
 typedef std::shared_ptr<lldb_private::ThreadPlanTracer> ThreadPlanTracerSP;
+typedef std::shared_ptr<lldb_private::ThreadTrace> ThreadTraceSP;
 typedef std::shared_ptr<lldb_private::Trace> TraceSP;
 typedef std::shared_ptr<lldb_private::Type> TypeSP;
 typedef std::weak_ptr<lldb_private::Type> TypeWP;
Index: lldb/include/lldb/lldb-enumerations.h
===================================================================
--- lldb/include/lldb/lldb-enumerations.h
+++ lldb/include/lldb/lldb-enumerations.h
@@ -9,6 +9,7 @@
 #ifndef LLDB_LLDB_ENUMERATIONS_H
 #define LLDB_LLDB_ENUMERATIONS_H
 
+#include <cstdint>
 #include <type_traits>
 
 #ifndef SWIG
@@ -773,12 +774,23 @@
   eBasicTypeOther
 };
 
-/// Deprecated
-enum TraceType {
-  eTraceTypeNone = 0,
-
-  /// Intel Processor Trace
-  eTraceTypeProcessorTrace
+/// Architecture-agnostic categorization of instructions. Useful for doing
+/// analysis on traces.
+enum TraceInstructionType : uint8_t {
+  /// The instruction is not recognized by LLDB
+  eTraceInstructionUnknown = 0,
+  /// The instruction is something not listed below
+  eTraceInstructionOther,
+  /// The instruction is a (function) call
+  eTraceInstructionCall,
+  /// The instruction is a (function) return
+  eTraceInstructionReturn,
+  /// The instruction is a unconditional jump
+  eTraceInstructionJump,
+  /// The instruction is a conditional jump
+  eTraceInstructionCondJump,
+  /// The instruction writes custom data to the trace, e.g. Intel's PTWRITE.
+  eTraceInstructionTraceWrite,
 };
 
 enum StructuredDataType {
Index: lldb/include/lldb/Target/Trace.h
===================================================================
--- lldb/include/lldb/Target/Trace.h
+++ lldb/include/lldb/Target/Trace.h
@@ -15,6 +15,7 @@
 
 #include "lldb/Core/PluginInterface.h"
 #include "lldb/Target/Thread.h"
+#include "lldb/Target/ThreadTrace.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/TraceGDBRemotePackets.h"
 #include "lldb/Utility/UnimplementedError.h"
@@ -44,11 +45,6 @@
 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
@@ -136,18 +132,6 @@
   ///     The JSON schema of this Trace plug-in.
   virtual llvm::StringRef GetSchema() = 0;
 
-  /// Each decoded thread contains a cursor to the current position the user is
-  /// stopped at. When reverse debugging, each operation like reverse-next or
-  /// reverse-continue will move this cursor, which is then picked by any
-  /// subsequent dump or reverse operation.
-  ///
-  /// The initial position for this cursor is the last element of the thread,
-  /// which is the most recent chronologically.
-  ///
-  /// \return
-  ///     The current position of the thread's trace or \b 0 if empty.
-  virtual size_t GetCursorPosition(Thread &thread) = 0;
-
   /// Dump \a count instructions of the given thread's trace ending at the
   /// given \a end_position position.
   ///
@@ -173,55 +157,15 @@
   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 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.
+  /// Get the trace corresponding to a specific thread.
   ///
   /// \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;
+  ///     The requested trace or an \a llvm::Error if the thread is not being
+  ///     traced or if there was an error loading or fetching the trace.
+  virtual llvm::Expected<lldb::ThreadTraceSP>
+  GetThreadTrace(Thread &thread) = 0;
 
   /// \return
   ///     A description of the parameters to use for the \a Trace::Start method.
@@ -342,13 +286,13 @@
   ///
   /// \param[in] state
   ///     The jLLDBTraceGetState response.
-  virtual void
-  DoRefreshLiveProcessState(llvm::Expected<TraceGetStateResponse> state) = 0;
+  virtual llvm::Error
+  DoRefreshLiveProcessState(const TraceGetStateResponse &state) = 0;
 
   /// Method to be invoked by the plug-in to refresh the live process state.
   ///
   /// The result is cached through the same process stop.
-  void RefreshLiveProcessState();
+  llvm::Error RefreshLiveProcessState();
 
   /// Process traced by this object if doing live tracing. Otherwise it's null.
   int64_t m_stop_id = -1;
@@ -358,6 +302,8 @@
       m_live_thread_data;
   /// data kind -> size
   std::unordered_map<std::string, size_t> m_live_process_data;
+  /// Cached error if \a Trace::RefreshLiveProcessState fails.
+  llvm::Optional<std::string> m_live_process_refresh_error;
 };
 
 } // namespace lldb_private
Index: lldb/include/lldb/Target/ThreadTrace.h
===================================================================
--- /dev/null
+++ lldb/include/lldb/Target/ThreadTrace.h
@@ -0,0 +1,88 @@
+//===-- ThreadTrace.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_THREAD_TRACE_H
+#define LLDB_TARGET_THREAD_TRACE_H
+
+#include "llvm/Support/Error.h"
+
+#include "lldb/lldb-private.h"
+
+namespace lldb_private {
+
+/// Helper enumeration for traversing the instructions of a processor trace.
+enum class TraceDirection {
+  Forwards = 0,
+  Backwards,
+};
+
+/// Class that represents a trace for a specific thread.
+///
+/// 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 the best possible way for the given technology.
+///
+/// Consumers of this class should make no assumptions on whether the trace is
+/// fully loaded on memory or not, as the plug-in might load or reconstruct
+/// instruction lazily.
+class ThreadTrace {
+public:
+  virtual ~ThreadTrace() = default;
+
+  /// Run the provided callback on the instructions of the trace.
+  ///
+  /// The instructions will be traversed starting at the given \a start_position
+  /// sequentially until the callback returns \b false, in which case no more
+  /// instructions are inspected.
+  ///
+  /// \param[in] start_position
+  ///     The instruction position to start iterating on, where 0 corresponds to
+  ///     the oldest instruction chronologically.
+  ///
+  /// \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.
+  ///
+  ///     If the \b load_address input of this callback is an \a llvm::Error,
+  ///     then that means that the instruction failed to be reconstructed.
+  ///     Otherwise, the load address of that instruction is returned.
+  virtual void TraverseInstructions(
+      size_t position, TraceDirection direction,
+      std::function<bool(size_t index,
+                         llvm::Expected<lldb::addr_t> load_address)>
+          callback) = 0;
+
+  /// \return
+  ///       The number of available instructions in the trace.
+  virtual size_t GetInstructionCount() = 0;
+
+  /// Return an architecture-agnostic category for the given instruction.
+  ///
+  /// \param[in] index
+  ///      The index of the instruction in question. It must be valid.
+  ///
+  /// \return
+  ///      The instruction type.
+  virtual lldb::TraceInstructionType GetInstructionType(size_t index) = 0;
+
+  /// Return the size in bytes of a given instruction.
+  ///
+  /// \param[in] index
+  ///      The index of the instruction in question. It must be valid.
+  virtual size_t GetInstructionSize(size_t index) = 0;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_TARGET_THREAD_TRACE_H
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to