wallace updated this revision to Diff 351344.
wallace edited the summary of this revision.
wallace added a comment.

nits


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,30 @@
   std::unique_ptr<llvm::ErrorInfoBase> m_error;
 };
 
-/// \class DecodedThread
-/// Class holding the instructions and function call hierarchy obtained from
-/// decoding a trace, as well as a position cursor used when reverse debugging
-/// the trace.
-///
-/// Each decoded thread contains a cursor to the current position the user is
-/// stopped at. See \a Trace::GetCursorPosition for more information.
-class DecodedThread {
+class 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,13 +8,13 @@
 #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;
@@ -222,9 +222,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 +237,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