zrthxn updated this revision to Diff 418327.
zrthxn added a comment.

Updated tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122293

Files:
  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/TraceIntelPT.cpp
  lldb/test/API/commands/trace/TestTraceDumpInfo.py
  lldb/test/API/commands/trace/TestTraceLoad.py

Index: lldb/test/API/commands/trace/TestTraceLoad.py
===================================================================
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -37,8 +37,9 @@
 
 thread #1: tid = 3842849
   Raw trace size: 4 KiB
-  Total number of instructions: 21
-  Total approximate memory usage: 5.38 KiB'''])
+  Total number of instructions: 22
+  Total approximate memory usage: 6.38 KiB
+  Average memory usage per instruction: 296 bytes'''])
 
     def testLoadInvalidTraces(self):
         src_dir = self.getSourceDir()
Index: lldb/test/API/commands/trace/TestTraceDumpInfo.py
===================================================================
--- lldb/test/API/commands/trace/TestTraceDumpInfo.py
+++ lldb/test/API/commands/trace/TestTraceDumpInfo.py
@@ -39,5 +39,6 @@
 
 thread #1: tid = 3842849
   Raw trace size: 4 KiB
-  Total number of instructions: 21
-  Total approximate memory usage: 5.38 KiB'''])
+  Total number of instructions: 22
+  Total approximate memory usage: 6.38 KiB
+  Average memory usage per instruction: 296 bytes'''])
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
@@ -112,12 +112,15 @@
   }
   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;
 }
 
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -78,7 +78,7 @@
 }
 
 Error TraceCursorIntelPT::GetError() {
-  return m_decoded_thread_sp->GetInstructions()[m_pos].ToError();
+  return m_decoded_thread_sp->GetErrorByInstructionIndex(m_pos);
 }
 
 lldb::addr_t TraceCursorIntelPT::GetLoadAddress() {
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
@@ -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;
@@ -104,9 +105,11 @@
 ///
 /// \return
 ///   The decoded instructions.
-static std::vector<IntelPTInstruction>
-DecodeInstructions(pt_insn_decoder &decoder) {
-  std::vector<IntelPTInstruction> instructions;
+static DecodedThreadSP DecodeInstructions(pt_insn_decoder &decoder,
+                                          const ThreadSP &thread_sp,
+                                          const size_t raw_trace_size) {
+  DecodedThreadSP decoded_thread = std::make_shared<DecodedThread>(
+      thread_sp, std::vector<IntelPTInstruction>(), raw_trace_size);
 
   while (true) {
     int errcode = FindNextSynchronizationPoint(decoder);
@@ -114,7 +117,7 @@
       break;
 
     if (errcode < 0) {
-      instructions.emplace_back(make_error<IntelPTError>(errcode));
+      decoded_thread->AppendError<IntelPTError>(errcode);
       break;
     }
 
@@ -123,17 +126,17 @@
     while (true) {
       errcode = ProcessPTEvents(decoder, errcode);
       if (errcode < 0) {
-        instructions.emplace_back(make_error<IntelPTError>(errcode));
+        decoded_thread->AppendError<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->AppendError<IntelPTError>(errcode, insn.ip);
         break;
       }
 
@@ -142,22 +145,22 @@
       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(
-            make_error<IntelPTError>(time_error, insn.ip));
-        instructions.emplace_back(insn);
+        decoded_thread->AppendError<IntelPTError>(time_error, insn.ip);
+        decoded_thread->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->AppendInstruction(insn);
       } else {
-        instructions.emplace_back(insn, time);
+        decoded_thread->AppendInstruction(insn, time);
       }
     }
   }
 
-  return instructions;
+  return decoded_thread;
 }
 
 /// Callback used by libipt for reading the process memory.
@@ -176,8 +179,8 @@
   return bytes_read;
 }
 
-static Expected<std::vector<IntelPTInstruction>>
-DecodeInMemoryTrace(Process &process, TraceIntelPT &trace_intel_pt,
+static Expected<DecodedThreadSP>
+DecodeInMemoryTrace(const ThreadSP &thread_sp, TraceIntelPT &trace_intel_pt,
                     MutableArrayRef<uint8_t> buffer) {
   Expected<pt_cpu> cpu_info = trace_intel_pt.GetCPUInfo();
   if (!cpu_info)
@@ -199,81 +202,71 @@
 
   pt_image *image = pt_insn_get_image(decoder);
 
-  int errcode = pt_image_set_callback(image, ReadProcessMemory, &process);
+  // ProcessSP process = thread_sp->GetProcess();
+  int errcode = pt_image_set_callback(image, ReadProcessMemory,
+                                      thread_sp->GetProcess().get());
   assert(errcode == 0);
   (void)errcode;
 
-  std::vector<IntelPTInstruction> instructions = DecodeInstructions(*decoder);
+  DecodedThreadSP decoded_thread =
+      DecodeInstructions(*decoder, thread_sp, buffer.size());
 
   pt_insn_free_decoder(decoder);
-  return instructions;
+  return decoded_thread;
 }
+// ---------------------------
 
-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);
+DecodedThreadSP ThreadDecoder::Decode() {
+  if (!m_decoded_thread.hasValue())
+    m_decoded_thread = DoDecode();
+  return *m_decoded_thread;
 }
 
-static Expected<std::vector<IntelPTInstruction>>
-DecodeLiveThread(Thread &thread, TraceIntelPT &trace, size_t &raw_trace_size) {
+// LiveThreadDecoder ====================
+
+LiveThreadDecoder::LiveThreadDecoder(Thread &thread, TraceIntelPT &trace)
+    : m_thread_sp(thread.shared_from_this()), m_trace(trace) {}
+
+DecodedThreadSP LiveThreadDecoder::DoDecode() {
   Expected<std::vector<uint8_t>> buffer =
-      trace.GetLiveThreadBuffer(thread.GetID());
+      m_trace.GetLiveThreadBuffer(m_thread_sp->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));
+    return std::make_shared<DecodedThread>(m_thread_sp, buffer.takeError());
+
+  if (Expected<pt_cpu> cpu_info = m_trace.GetCPUInfo())
+    if (Expected<DecodedThreadSP> dtsp = DecodeInMemoryTrace(
+            m_thread_sp, m_trace, MutableArrayRef<uint8_t>(*buffer)))
+      return *dtsp;
+    else
+      return std::make_shared<DecodedThread>(m_thread_sp, dtsp.takeError());
   else
-    return cpu_info.takeError();
+    return std::make_shared<DecodedThread>(m_thread_sp, cpu_info.takeError());
 }
 
-DecodedThreadSP ThreadDecoder::Decode() {
-  if (!m_decoded_thread.hasValue())
-    m_decoded_thread = DoDecode();
-  return *m_decoded_thread;
-}
+// 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());
-}
+  ErrorOr<std::unique_ptr<MemoryBuffer>> trace_or_error =
+      MemoryBuffer::getFile(m_trace_thread->GetTraceFile().GetPath());
+  if (std::error_code err = trace_or_error.getError())
+    return std::make_shared<DecodedThread>(m_trace_thread,
+                                           errorCodeToError(err));
 
-LiveThreadDecoder::LiveThreadDecoder(Thread &thread, TraceIntelPT &trace)
-    : m_thread_sp(thread.shared_from_this()), m_trace(trace) {}
+  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());
 
-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);
+  if (Expected<DecodedThreadSP> dtsp =
+          DecodeInMemoryTrace(m_trace_thread, m_trace, trace_data))
+    return *dtsp;
   else
-    return std::make_shared<DecodedThread>(m_thread_sp,
-                                           instructions.takeError());
+    return std::make_shared<DecodedThread>(m_trace_thread, dtsp.takeError());
 }
Index: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
+++ 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"
@@ -69,7 +70,7 @@
   /// Error constructor
   ///
   /// libipt errors should use the underlying \a IntelPTError class.
-  IntelPTInstruction(llvm::Error err);
+  IntelPTInstruction(const llvm::Error &err);
 
   /// Check if this object represents an error (i.e. a gap).
   ///
@@ -81,14 +82,9 @@
   ///     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 +111,11 @@
   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
@@ -140,12 +136,36 @@
   DecodedThread(lldb::ThreadSP thread_sp, llvm::Error error);
 
   /// Get the instructions from the decoded trace. Some of them might indicate
-  /// errors (i.e. gaps) in the trace.
+  /// errors (i.e. gaps) in the trace. For an instruction error, you can access
+  /// its underlying Error object 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 or \a llvm::Error::success if the given index
+  ///   points to a valid instruction.
+  llvm::Error GetErrorByInstructionIndex(uint64_t ins_idx);
+
+  /// Append a successfully decoded instruction.
+  template <typename... Ts> void AppendInstruction(Ts... __args) {
+    m_instructions.emplace_back(std::move(IntelPTInstruction(__args...)));
+  }
+
+  /// Append a decoding error (i.e. an instruction that failed to be decoded).
+  template <class ErrT, typename... Ts> void AppendError(Ts... __args) {
+    llvm::Error err = llvm::make_error<ErrT>(__args...);
+    m_instructions.emplace_back(err);
+    handleAllErrors(
+        std::move(err), [&](std::unique_ptr<llvm::ErrorInfoBase> info) {
+          m_errors.try_emplace(m_instructions.size(), std::move(info));
+        });
+  }
+
   /// Get a new cursor for the decoded thread.
   lldb::TraceCursorUP GetCursor();
 
@@ -155,15 +175,16 @@
   ///   The size of the trace.
   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;
 
 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;
+  llvm::DenseMap<uint64_t, std::unique_ptr<llvm::ErrorInfoBase>> m_errors;
   size_t m_raw_trace_size;
 };
 
Index: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
+++ lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
@@ -35,34 +35,24 @@
   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(const Error &err) {
   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());
-}
 size_t DecodedThread::GetRawTraceSize() const { return m_raw_trace_size; }
 
 TraceInstructionControlFlowType
@@ -100,9 +90,21 @@
   return makeArrayRef(m_instructions);
 }
 
+Error DecodedThread::GetErrorByInstructionIndex(uint64_t idx) {
+  auto it = m_errors.find(idx);
+  if (it == m_errors.end())
+    return Error::success();
+
+  auto &err = it->second;
+  if (err->isA<IntelPTError>())
+    return make_error<IntelPTError>(static_cast<IntelPTError &>(*err));
+
+  return make_error<StringError>(err->message(), err->convertToErrorCode());
+}
+
 DecodedThread::DecodedThread(ThreadSP thread_sp, Error error)
     : m_thread_sp(thread_sp) {
-  m_instructions.emplace_back(std::move(error));
+  m_instructions.emplace_back(error);
 }
 
 DecodedThread::DecodedThread(ThreadSP thread_sp,
@@ -111,8 +113,7 @@
     : 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"));
+    AppendError<StringError>(inconvertibleErrorCode(), "empty trace");
 }
 
 lldb::TraceCursorUP DecodedThread::GetCursor() {
@@ -120,7 +121,7 @@
 }
 
 size_t DecodedThread::CalculateApproximateMemoryUsage() const {
-  return m_raw_trace_size
-    + IntelPTInstruction::GetNonErrorMemoryUsage() * m_instructions.size()
-    + sizeof(DecodedThread);
+  return m_raw_trace_size +
+         IntelPTInstruction::GetMemoryUsage() * m_instructions.size() +
+         m_errors.getMemorySize();
 }
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to