[Lldb-commits] [lldb] bcf1978 - [intelpt] Refactoring instruction decoding for flexibility

Walter Erquinigo via lldb-commits lldb-commits at lists.llvm.org
Sat Mar 26 11:36:31 PDT 2022


Author: Alisamar Husain
Date: 2022-03-26T11:34:47-07:00
New Revision: bcf1978a871535e297c965195afe467134164413

URL: https://github.com/llvm/llvm-project/commit/bcf1978a871535e297c965195afe467134164413
DIFF: https://github.com/llvm/llvm-project/commit/bcf1978a871535e297c965195afe467134164413.diff

LOG: [intelpt] Refactoring instruction decoding for flexibility

Now the decoded thread has Append methods that provide more flexibility
in terms of the underlying data structure that represents the
instructions. In this case, we are able to represent the sporadic errors
as map and thus reduce the size of each instruction.

Differential Revision: https://reviews.llvm.org/D122293

Added: 
    

Modified: 
    lldb/include/lldb/Target/TraceCursor.h
    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/TraceCursorIntelPT.cpp
    lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
    lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
    lldb/source/Target/TraceInstructionDumper.cpp
    lldb/test/API/commands/trace/TestTraceDumpInfo.py
    lldb/test/API/commands/trace/TestTraceLoad.py

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Target/TraceCursor.h b/lldb/include/lldb/Target/TraceCursor.h
index 83ab22b367c53..2e3c4eb22b04e 100644
--- a/lldb/include/lldb/Target/TraceCursor.h
+++ b/lldb/include/lldb/Target/TraceCursor.h
@@ -170,9 +170,9 @@ class TraceCursor {
   /// the trace.
   ///
   /// \return
-  ///     \b llvm::Error::success if the cursor is not pointing to an error in
-  ///     the trace. Otherwise return an \a llvm::Error describing the issue.
-  virtual llvm::Error GetError() = 0;
+  ///     \b nullptr if the cursor is not pointing to an error in
+  ///     the trace. Otherwise return the actual error message.
+  virtual const char *GetError() = 0;
 
   /// \return
   ///     The load address of the instruction the cursor is pointing at. If the

diff  --git a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
index a81a779302605..3c39c4d9a96d3 100644
--- a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
@@ -35,35 +35,27 @@ void IntelPTError::log(llvm::raw_ostream &OS) const {
   OS << "error: " << libipt_error_message;
 }
 
-IntelPTInstruction::IntelPTInstruction(llvm::Error err) {
-  llvm::handleAllErrors(std::move(err),
-                        [&](std::unique_ptr<llvm::ErrorInfoBase> info) {
-                          m_error = std::move(info);
-                        });
+IntelPTInstruction::IntelPTInstruction() {
   m_pt_insn.ip = LLDB_INVALID_ADDRESS;
   m_pt_insn.iclass = ptic_error;
+  m_is_error = true;
 }
 
-bool IntelPTInstruction::IsError() const { return (bool)m_error; }
+bool IntelPTInstruction::IsError() const { return m_is_error; }
 
 lldb::addr_t IntelPTInstruction::GetLoadAddress() const { return m_pt_insn.ip; }
 
-size_t IntelPTInstruction::GetNonErrorMemoryUsage() { return sizeof(IntelPTInstruction); }
+size_t IntelPTInstruction::GetMemoryUsage() {
+  return sizeof(IntelPTInstruction);
+}
 
 Optional<uint64_t> IntelPTInstruction::GetTimestampCounter() const {
   return m_timestamp;
 }
 
-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());
+Optional<size_t> DecodedThread::GetRawTraceSize() const {
+  return m_raw_trace_size;
 }
-size_t DecodedThread::GetRawTraceSize() const { return m_raw_trace_size; }
 
 TraceInstructionControlFlowType
 IntelPTInstruction::GetControlFlowType(lldb::addr_t next_load_address) const {
@@ -96,31 +88,44 @@ IntelPTInstruction::GetControlFlowType(lldb::addr_t next_load_address) const {
   return mask;
 }
 
+ThreadSP DecodedThread::GetThread() { return m_thread_sp; }
+
+void DecodedThread::AppendError(llvm::Error &&error) {
+  m_errors.try_emplace(m_instructions.size(), toString(std::move(error)));
+  m_instructions.emplace_back();
+}
+
 ArrayRef<IntelPTInstruction> DecodedThread::GetInstructions() const {
   return makeArrayRef(m_instructions);
 }
 
-DecodedThread::DecodedThread(ThreadSP thread_sp, Error error)
-    : m_thread_sp(thread_sp) {
-  m_instructions.emplace_back(std::move(error));
+const char *DecodedThread::GetErrorByInstructionIndex(uint64_t idx) {
+  auto it = m_errors.find(idx);
+  if (it == m_errors.end())
+    return nullptr;
+
+  return it->second.c_str();
 }
 
-DecodedThread::DecodedThread(ThreadSP thread_sp,
-                             std::vector<IntelPTInstruction> &&instructions,
-                             size_t raw_trace_size)
-    : m_thread_sp(thread_sp), m_instructions(std::move(instructions)),
-      m_raw_trace_size(raw_trace_size) {
-  if (m_instructions.empty())
-    m_instructions.emplace_back(
-        createStringError(inconvertibleErrorCode(), "empty trace"));
+DecodedThread::DecodedThread(ThreadSP thread_sp) : m_thread_sp(thread_sp) {}
+
+DecodedThread::DecodedThread(ThreadSP thread_sp, Error &&error)
+    : m_thread_sp(thread_sp) {
+  AppendError(std::move(error));
 }
 
+void DecodedThread::SetRawTraceSize(size_t size) { m_raw_trace_size = size; }
+
 lldb::TraceCursorUP DecodedThread::GetCursor() {
+  // We insert a fake error signaling an empty trace if needed becasue the
+  // TraceCursor requires non-empty traces.
+  if (m_instructions.empty())
+    AppendError(createStringError(inconvertibleErrorCode(), "empty trace"));
   return std::make_unique<TraceCursorIntelPT>(m_thread_sp, shared_from_this());
 }
 
 size_t DecodedThread::CalculateApproximateMemoryUsage() const {
-  return m_raw_trace_size
-    + IntelPTInstruction::GetNonErrorMemoryUsage() * m_instructions.size()
-    + sizeof(DecodedThread);
+  return m_raw_trace_size.getValueOr(0) +
+         IntelPTInstruction::GetMemoryUsage() * m_instructions.size() +
+         m_errors.getMemorySize();
 }

diff  --git a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
index 4063012997045..bda5d046c3a0d 100644
--- a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
+++ b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
@@ -9,6 +9,7 @@
 #ifndef LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_DECODEDTHREAD_H
 #define LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_DECODEDTHREAD_H
 
+#include <utility>
 #include <vector>
 
 #include "llvm/Support/Errc.h"
@@ -62,14 +63,13 @@ class IntelPTError : public llvm::ErrorInfo<IntelPTError> {
 class IntelPTInstruction {
 public:
   IntelPTInstruction(const pt_insn &pt_insn, uint64_t timestamp)
-      : m_pt_insn(pt_insn), m_timestamp(timestamp) {}
+      : m_pt_insn(pt_insn), m_timestamp(timestamp), m_is_error(false) {}
 
-  IntelPTInstruction(const pt_insn &pt_insn) : m_pt_insn(pt_insn) {}
+  IntelPTInstruction(const pt_insn &pt_insn)
+      : m_pt_insn(pt_insn), m_is_error(false) {}
 
   /// Error constructor
-  ///
-  /// libipt errors should use the underlying \a IntelPTError class.
-  IntelPTInstruction(llvm::Error err);
+  IntelPTInstruction();
 
   /// Check if this object represents an error (i.e. a gap).
   ///
@@ -81,14 +81,9 @@ class IntelPTInstruction {
   ///     The instruction pointer address, or \a LLDB_INVALID_ADDRESS if it is
   ///     an error.
   lldb::addr_t GetLoadAddress() const;
-  
-  /// Get the size in bytes of a non-error instance of this class
-  static size_t GetNonErrorMemoryUsage();
 
-  /// \return
-  ///     An \a llvm::Error object if this class corresponds to an Error, or an
-  ///     \a llvm::Error::success otherwise.
-  llvm::Error ToError() const;
+  /// Get the size in bytes of an instance of this class
+  static size_t GetMemoryUsage();
 
   /// Get the timestamp associated with the current instruction. The timestamp
   /// is similar to what a rdtsc instruction would return.
@@ -115,11 +110,11 @@ class IntelPTInstruction {
   IntelPTInstruction(const IntelPTInstruction &other) = delete;
   const IntelPTInstruction &operator=(const IntelPTInstruction &other) = delete;
 
-  // When adding new members to this class, make sure to update 
+  // When adding new members to this class, make sure to update
   // IntelPTInstruction::GetNonErrorMemoryUsage() if needed.
   pt_insn m_pt_insn;
   llvm::Optional<uint64_t> m_timestamp;
-  std::unique_ptr<llvm::ErrorInfoBase> m_error;
+  bool m_is_error;
 };
 
 /// \class DecodedThread
@@ -131,40 +126,60 @@ class IntelPTInstruction {
 /// stopped at. See \a Trace::GetCursorPosition for more information.
 class DecodedThread : public std::enable_shared_from_this<DecodedThread> {
 public:
-  DecodedThread(lldb::ThreadSP thread_sp,
-                std::vector<IntelPTInstruction> &&instructions,
-                size_t raw_trace_size);
+  DecodedThread(lldb::ThreadSP thread_sp);
 
-  /// Constructor with a single error signaling a complete failure of the
-  /// decoding process.
-  DecodedThread(lldb::ThreadSP thread_sp, llvm::Error error);
+  /// Utility constructor that initializes the trace with a provided error.
+  DecodedThread(lldb::ThreadSP thread_sp, llvm::Error &&err);
 
   /// Get the instructions from the decoded trace. Some of them might indicate
-  /// errors (i.e. gaps) in the trace.
+  /// errors (i.e. gaps) in the trace. For an instruction error, you can access
+  /// its underlying error message with the \a GetErrorByInstructionIndex()
+  /// method.
   ///
   /// \return
   ///   The instructions of the trace.
   llvm::ArrayRef<IntelPTInstruction> GetInstructions() const;
 
+  /// Get the error associated with a given instruction index.
+  ///
+  /// \return
+  ///   The error message of \b nullptr if the given index
+  ///   points to a valid instruction.
+  const char *GetErrorByInstructionIndex(uint64_t ins_idx);
+
+  /// Append a successfully decoded instruction.
+  template <typename... Ts> void AppendInstruction(Ts... instruction_args) {
+    m_instructions.emplace_back(instruction_args...);
+  }
+
+  /// Append a decoding error (i.e. an instruction that failed to be decoded).
+  void AppendError(llvm::Error &&error);
+
   /// Get a new cursor for the decoded thread.
   lldb::TraceCursorUP GetCursor();
 
-  /// Get the size in bytes of the corresponding Intel PT raw trace
+  /// Set the size in bytes of the corresponding Intel PT raw trace.
+  void SetRawTraceSize(size_t size);
+
+  /// Get the size in bytes of the corresponding Intel PT raw trace.
   ///
   /// \return
-  ///   The size of the trace.
-  size_t GetRawTraceSize() const;
+  ///   The size of the trace, or \b llvm::None if not available.
+  llvm::Optional<size_t> GetRawTraceSize() const;
 
-  /// The approximate size in bytes used by this instance, 
+  /// The approximate size in bytes used by this instance,
   /// including all the already decoded instructions.
   size_t CalculateApproximateMemoryUsage() const;
 
+  lldb::ThreadSP GetThread();
+
 private:
-  /// When adding new members to this class, make sure 
+  /// When adding new members to this class, make sure
   /// to update \a CalculateApproximateMemoryUsage() accordingly.
   lldb::ThreadSP m_thread_sp;
   std::vector<IntelPTInstruction> m_instructions;
-  size_t m_raw_trace_size;
+  llvm::DenseMap<uint64_t, std::string> m_errors;
+  llvm::Optional<size_t> m_raw_trace_size;
 };
 
 using DecodedThreadSP = std::shared_ptr<DecodedThread>;

diff  --git a/lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp b/lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
index 3827881454c7c..9ca366bb57746 100644
--- a/lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
@@ -16,6 +16,7 @@
 #include "lldb/Core/Section.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/StringExtractor.h"
+#include <utility>
 
 using namespace lldb;
 using namespace lldb_private;
@@ -101,20 +102,15 @@ static int ProcessPTEvents(pt_insn_decoder &decoder, int errcode) {
 ///
 /// \param[in] decoder
 ///   A configured libipt \a pt_insn_decoder.
-///
-/// \return
-///   The decoded instructions.
-static std::vector<IntelPTInstruction>
-DecodeInstructions(pt_insn_decoder &decoder) {
-  std::vector<IntelPTInstruction> instructions;
-
+static void DecodeInstructions(pt_insn_decoder &decoder,
+                               DecodedThreadSP &decoded_thread_sp) {
   while (true) {
     int errcode = FindNextSynchronizationPoint(decoder);
     if (errcode == -pte_eos)
       break;
 
     if (errcode < 0) {
-      instructions.emplace_back(make_error<IntelPTError>(errcode));
+      decoded_thread_sp->AppendError(make_error<IntelPTError>(errcode));
       break;
     }
 
@@ -123,17 +119,18 @@ DecodeInstructions(pt_insn_decoder &decoder) {
     while (true) {
       errcode = ProcessPTEvents(decoder, errcode);
       if (errcode < 0) {
-        instructions.emplace_back(make_error<IntelPTError>(errcode));
+        decoded_thread_sp->AppendError(make_error<IntelPTError>(errcode));
         break;
       }
-      pt_insn insn;
 
+      pt_insn insn;
       errcode = pt_insn_next(&decoder, &insn, sizeof(insn));
       if (errcode == -pte_eos)
         break;
 
       if (errcode < 0) {
-        instructions.emplace_back(make_error<IntelPTError>(errcode, insn.ip));
+        decoded_thread_sp->AppendError(
+            make_error<IntelPTError>(errcode, insn.ip));
         break;
       }
 
@@ -142,22 +139,21 @@ DecodeInstructions(pt_insn_decoder &decoder) {
       if (time_error == -pte_invalid) {
         // This happens if we invoke the pt_insn_time method incorrectly,
         // but the instruction is good though.
-        instructions.emplace_back(
+        decoded_thread_sp->AppendError(
             make_error<IntelPTError>(time_error, insn.ip));
-        instructions.emplace_back(insn);
+        decoded_thread_sp->AppendInstruction(insn);
         break;
       }
+
       if (time_error == -pte_no_time) {
         // We simply don't have time information, i.e. None of TSC, MTC or CYC
         // was enabled.
-        instructions.emplace_back(insn);
+        decoded_thread_sp->AppendInstruction(insn);
       } else {
-        instructions.emplace_back(insn, time);
+        decoded_thread_sp->AppendInstruction(insn, time);
       }
     }
   }
-
-  return instructions;
 }
 
 /// Callback used by libipt for reading the process memory.
@@ -176,70 +172,40 @@ static int ReadProcessMemory(uint8_t *buffer, size_t size,
   return bytes_read;
 }
 
-static Expected<std::vector<IntelPTInstruction>>
-DecodeInMemoryTrace(Process &process, TraceIntelPT &trace_intel_pt,
-                    MutableArrayRef<uint8_t> buffer) {
+static void DecodeInMemoryTrace(DecodedThreadSP &decoded_thread_sp,
+                                TraceIntelPT &trace_intel_pt,
+                                MutableArrayRef<uint8_t> buffer) {
   Expected<pt_cpu> cpu_info = trace_intel_pt.GetCPUInfo();
-  if (!cpu_info)
-    return cpu_info.takeError();
+  if (!cpu_info) {
+    return decoded_thread_sp->AppendError(cpu_info.takeError());
+  }
 
   pt_config config;
   pt_config_init(&config);
   config.cpu = *cpu_info;
 
   if (int errcode = pt_cpu_errata(&config.errata, &config.cpu))
-    return make_error<IntelPTError>(errcode);
+    return decoded_thread_sp->AppendError(make_error<IntelPTError>(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 decoded_thread_sp->AppendError(make_error<IntelPTError>(-pte_nomem));
 
   pt_image *image = pt_insn_get_image(decoder);
 
-  int errcode = pt_image_set_callback(image, ReadProcessMemory, &process);
+  int errcode =
+      pt_image_set_callback(image, ReadProcessMemory,
+                            decoded_thread_sp->GetThread()->GetProcess().get());
   assert(errcode == 0);
   (void)errcode;
 
-  std::vector<IntelPTInstruction> instructions = DecodeInstructions(*decoder);
-
+  DecodeInstructions(*decoder, decoded_thread_sp);
   pt_insn_free_decoder(decoder);
-  return instructions;
-}
-
-static Expected<std::vector<IntelPTInstruction>>
-DecodeTraceFile(Process &process, TraceIntelPT &trace_intel_pt,
-                const FileSpec &trace_file, size_t &raw_trace_size) {
-  ErrorOr<std::unique_ptr<MemoryBuffer>> trace_or_error =
-      MemoryBuffer::getFile(trace_file.GetPath());
-  if (std::error_code err = trace_or_error.getError())
-    return errorCodeToError(err);
-
-  MemoryBuffer &trace = **trace_or_error;
-  MutableArrayRef<uint8_t> trace_data(
-      // The libipt library does not modify the trace buffer, hence the
-      // following cast is safe.
-      reinterpret_cast<uint8_t *>(const_cast<char *>(trace.getBufferStart())),
-      trace.getBufferSize());
-  raw_trace_size = trace_data.size();
-  return DecodeInMemoryTrace(process, trace_intel_pt, trace_data);
-}
-
-static Expected<std::vector<IntelPTInstruction>>
-DecodeLiveThread(Thread &thread, TraceIntelPT &trace, size_t &raw_trace_size) {
-  Expected<std::vector<uint8_t>> buffer =
-      trace.GetLiveThreadBuffer(thread.GetID());
-  if (!buffer)
-    return buffer.takeError();
-  raw_trace_size = buffer->size();
-  if (Expected<pt_cpu> cpu_info = trace.GetCPUInfo())
-    return DecodeInMemoryTrace(*thread.GetProcess(), trace,
-                               MutableArrayRef<uint8_t>(*buffer));
-  else
-    return cpu_info.takeError();
 }
+// ---------------------------
 
 DecodedThreadSP ThreadDecoder::Decode() {
   if (!m_decoded_thread.hasValue())
@@ -247,33 +213,53 @@ DecodedThreadSP ThreadDecoder::Decode() {
   return *m_decoded_thread;
 }
 
+// LiveThreadDecoder ====================
+
+LiveThreadDecoder::LiveThreadDecoder(Thread &thread, TraceIntelPT &trace)
+    : m_thread_sp(thread.shared_from_this()), m_trace(trace) {}
+
+DecodedThreadSP LiveThreadDecoder::DoDecode() {
+  DecodedThreadSP decoded_thread_sp =
+      std::make_shared<DecodedThread>(m_thread_sp);
+
+  Expected<std::vector<uint8_t>> buffer =
+      m_trace.GetLiveThreadBuffer(m_thread_sp->GetID());
+  if (!buffer) {
+    decoded_thread_sp->AppendError(buffer.takeError());
+    return decoded_thread_sp;
+  }
+
+  decoded_thread_sp->SetRawTraceSize(buffer->size());
+  DecodeInMemoryTrace(decoded_thread_sp, m_trace,
+                      MutableArrayRef<uint8_t>(*buffer));
+  return decoded_thread_sp;
+}
+
+// PostMortemThreadDecoder =======================
+
 PostMortemThreadDecoder::PostMortemThreadDecoder(
     const lldb::ThreadPostMortemTraceSP &trace_thread, TraceIntelPT &trace)
     : m_trace_thread(trace_thread), m_trace(trace) {}
 
 DecodedThreadSP PostMortemThreadDecoder::DoDecode() {
-  size_t raw_trace_size = 0;
-  if (Expected<std::vector<IntelPTInstruction>> instructions =
-          DecodeTraceFile(*m_trace_thread->GetProcess(), m_trace,
-                          m_trace_thread->GetTraceFile(), raw_trace_size))
-    return std::make_shared<DecodedThread>(m_trace_thread->shared_from_this(),
-                                           std::move(*instructions),
-                                           raw_trace_size);
-  else
-    return std::make_shared<DecodedThread>(m_trace_thread->shared_from_this(),
-                                           instructions.takeError());
-}
+  DecodedThreadSP decoded_thread_sp =
+      std::make_shared<DecodedThread>(m_trace_thread);
 
-LiveThreadDecoder::LiveThreadDecoder(Thread &thread, TraceIntelPT &trace)
-    : m_thread_sp(thread.shared_from_this()), m_trace(trace) {}
+  ErrorOr<std::unique_ptr<MemoryBuffer>> trace_or_error =
+      MemoryBuffer::getFile(m_trace_thread->GetTraceFile().GetPath());
+  if (std::error_code err = trace_or_error.getError()) {
+    decoded_thread_sp->AppendError(errorCodeToError(err));
+    return decoded_thread_sp;
+  }
 
-DecodedThreadSP LiveThreadDecoder::DoDecode() {
-  size_t raw_trace_size = 0;
-  if (Expected<std::vector<IntelPTInstruction>> instructions =
-          DecodeLiveThread(*m_thread_sp, m_trace, raw_trace_size))
-    return std::make_shared<DecodedThread>(
-        m_thread_sp, std::move(*instructions), raw_trace_size);
-  else
-    return std::make_shared<DecodedThread>(m_thread_sp,
-                                           instructions.takeError());
+  MemoryBuffer &trace = **trace_or_error;
+  MutableArrayRef<uint8_t> trace_data(
+      // The libipt library does not modify the trace buffer, hence the
+      // following cast is safe.
+      reinterpret_cast<uint8_t *>(const_cast<char *>(trace.getBufferStart())),
+      trace.getBufferSize());
+  decoded_thread_sp->SetRawTraceSize(trace_data.size());
+
+  DecodeInMemoryTrace(decoded_thread_sp, m_trace, trace_data);
+  return decoded_thread_sp;
 }

diff  --git a/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp b/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
index f6b26e0231e98..b335e5e4774f7 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -77,8 +77,8 @@ bool TraceCursorIntelPT::IsError() {
   return m_decoded_thread_sp->GetInstructions()[m_pos].IsError();
 }
 
-Error TraceCursorIntelPT::GetError() {
-  return m_decoded_thread_sp->GetInstructions()[m_pos].ToError();
+const char *TraceCursorIntelPT::GetError() {
+  return m_decoded_thread_sp->GetErrorByInstructionIndex(m_pos);
 }
 
 lldb::addr_t TraceCursorIntelPT::GetLoadAddress() {

diff  --git a/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h b/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
index 8ec55941f005f..cda524b98c69f 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
@@ -24,7 +24,7 @@ class TraceCursorIntelPT : public TraceCursor {
 
   virtual bool Next() override;
 
-  llvm::Error GetError() override;
+  const char *GetError() override;
 
   lldb::addr_t GetLoadAddress() override;
 

diff  --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
index 63318031f8893..8ba6c1c51db72 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -112,12 +112,15 @@ void TraceIntelPT::DumpTraceInfo(Thread &thread, Stream &s, bool verbose) {
   }
   s.Printf("\n");
 
+  size_t insn_len = Decode(thread)->GetInstructions().size();
   size_t mem_used = Decode(thread)->CalculateApproximateMemoryUsage();
+
   s.Printf("  Raw trace size: %zu KiB\n", *raw_size / 1024);
-  s.Printf("  Total number of instructions: %zu\n",
-           Decode(thread)->GetInstructions().size());
+  s.Printf("  Total number of instructions: %zu\n", insn_len);
   s.Printf("  Total approximate memory usage: %0.2lf KiB\n",
            (double)mem_used / 1024);
+  s.Printf("  Average memory usage per instruction: %zu bytes\n",
+           mem_used / insn_len);
   return;
 }
 

diff  --git a/lldb/source/Target/TraceInstructionDumper.cpp b/lldb/source/Target/TraceInstructionDumper.cpp
index 20b3a4733dd37..52a97babd0a26 100644
--- a/lldb/source/Target/TraceInstructionDumper.cpp
+++ b/lldb/source/Target/TraceInstructionDumper.cpp
@@ -250,14 +250,14 @@ void TraceInstructionDumper::DumpInstructions(Stream &s, size_t count) {
       break;
     }
 
-    if (Error err = m_cursor_up->GetError()) {
+    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;
 
       printInstructionIndex();
-      s << toString(std::move(err));
+      s << err;
     } else {
       if (m_cursor_up->IsForwards() && was_prev_instruction_an_error)
         printMissingInstructionsMessage();

diff  --git a/lldb/test/API/commands/trace/TestTraceDumpInfo.py b/lldb/test/API/commands/trace/TestTraceDumpInfo.py
index bc50b5274195a..a90a28cf7fea1 100644
--- a/lldb/test/API/commands/trace/TestTraceDumpInfo.py
+++ b/lldb/test/API/commands/trace/TestTraceDumpInfo.py
@@ -40,4 +40,5 @@ def testDumpRawTraceSize(self):
 thread #1: tid = 3842849
   Raw trace size: 4 KiB
   Total number of instructions: 21
-  Total approximate memory usage: 5.38 KiB'''])
+  Total approximate memory usage: 5.31 KiB
+  Average memory usage per instruction: 259 bytes'''])

diff  --git a/lldb/test/API/commands/trace/TestTraceLoad.py b/lldb/test/API/commands/trace/TestTraceLoad.py
index f901e2b2d9eb6..a68aa71bcedfe 100644
--- a/lldb/test/API/commands/trace/TestTraceLoad.py
+++ b/lldb/test/API/commands/trace/TestTraceLoad.py
@@ -38,7 +38,8 @@ def testLoadTrace(self):
 thread #1: tid = 3842849
   Raw trace size: 4 KiB
   Total number of instructions: 21
-  Total approximate memory usage: 5.38 KiB'''])
+  Total approximate memory usage: 5.31 KiB
+  Average memory usage per instruction: 259 bytes'''])
 
     def testLoadInvalidTraces(self):
         src_dir = self.getSourceDir()


        


More information about the lldb-commits mailing list