wallace created this revision.
wallace added a reviewer: clayborg.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
wallace requested review of this revision.
Herald added a subscriber: JDevlieghere.

Depends on D86670 <https://reviews.llvm.org/D86670>.

I first just want to get feedback about the structures of the class in this 
patch, later I con polish it.

This adds the skeleton for decoding traces of threads. The first step is having 
the plug-in implementation decode its trace into raw instructions. Individual 
instructions can sometimes fail to be decoded, so the instruction class should 
have an is_error flag. Error are quite sporadic though.
I'm adding a TraceThread class that contains all the information needed to 
decode a trace. I'm creating another class TraceDecodedThread will is obtained 
by calling TraceThread::Decode and its existance indicates that decoding didn't 
fail and you can use that class for inspecting the decoding trace.
This means that TraceThread is like an initialization object, and then the 
analysis is done by the TraceDecodedTrace class, which splits responsibilities 
and makes error handling easier.

I added some comments in Trace.h as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86899

Files:
  lldb/include/lldb/Target/Trace.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Target/Trace.cpp

Index: lldb/source/Target/Trace.cpp
===================================================================
--- lldb/source/Target/Trace.cpp
+++ lldb/source/Target/Trace.cpp
@@ -13,8 +13,11 @@
 
 #include "llvm/Support/Format.h"
 
+#include "lldb/Core/Debugger.h"
 #include "lldb/Core/PluginManager.h"
+#include "lldb/Target/Process.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Target/ThreadList.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -53,14 +56,26 @@
 
   for (auto target_sp : GetTargets())
     target_sp->SetTrace(shared_from_this());
+
+  for (auto process_it : GetParser().m_thread_to_trace_file_map) {
+    lldb::pid_t pid = process_it.first;
+    ThreadList &thread_list =
+        debugger.FindTargetWithProcessID(pid)->GetProcessSP()->GetThreadList();
+    for (auto thread_it : process_it.second) {
+      lldb::tid_t tid = thread_it.first;
+      m_trace_threads[pid].emplace(
+          tid,
+          CreateTraceThread(thread_list.FindThreadByID(tid), thread_it.second));
+    }
+  }
   return llvm::Error::success();
 }
 
 llvm::StringRef Trace::GetSchema() { return GetParser().GetSchema(); }
 
-const std::map<lldb::pid_t, std::map<lldb::tid_t, lldb_private::FileSpec>> &
-Trace::GetThreadToTraceFileMap() const {
-  return GetParser().m_thread_to_trace_file_map;
+const std::map<lldb::pid_t, std::map<lldb::tid_t, std::unique_ptr<TraceThread>>>
+    &Trace::GetTraceThreads() const {
+  return m_trace_threads;
 }
 
 const llvm::json::Object &Trace::GetSettings() const {
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
@@ -16,6 +16,18 @@
 #include "lldb/Target/Trace.h"
 #include "lldb/lldb-private.h"
 
+class TraceIntelPTThread : public lldb_private::TraceThread {
+public:
+  ~TraceIntelPTThread() override = default;
+
+  TraceIntelPTThread(const lldb::ThreadSP &thread_sp,
+                     const lldb_private::FileSpec &trace_file)
+      : lldb_private::TraceThread(thread_sp, trace_file) {}
+
+  llvm::Expected<std::vector<lldb_private::TraceInstruction>>
+  DecodeInstructions() override;
+};
+
 class TraceIntelPT : public lldb_private::Trace {
 public:
   void Dump(lldb_private::Stream &s, lldb_private::Process *process,
@@ -45,6 +57,10 @@
 
   TraceIntelPTSettingsParser &GetParser() override;
 
+  std::unique_ptr<lldb_private::TraceThread>
+  CreateTraceThread(const lldb::ThreadSP &thread,
+                    const lldb_private::FileSpec &trace_file) override;
+
 private:
   const pt_cpu &GetPTCPU() const;
 
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
@@ -51,6 +51,15 @@
 
 uint32_t TraceIntelPT::GetPluginVersion() { return 1; }
 
+lldb::TraceSP TraceIntelPT::CreateInstance(const llvm::json::Object &settings,
+                                           StringRef settings_dir) {
+  return lldb::TraceSP(new TraceIntelPT(settings, settings_dir));
+}
+
+//------------------------------------------------------------------
+// TraceIntelPT
+//------------------------------------------------------------------
+
 void TraceIntelPT::Dump(Stream &s, Process *process, Thread *thread,
                         bool verbose) const {
   if (verbose) {
@@ -73,20 +82,31 @@
 
   // We go through all the processes and threads even when there are filters as
   // a way to simplify the implementation.
-  for (const auto &process_it : GetThreadToTraceFileMap()) {
+  for (const auto &process_it : GetTraceThreads()) {
     if (!process || process->GetID() == process_it.first) {
       for (const auto &thread_it : process_it.second) {
         if (!thread || thread->GetID() == thread_it.first) {
           s.Printf("\npid: '%" PRIu64 "', tid: '%" PRIu64 "' -> ",
                    process_it.first, thread_it.first);
-          s << thread_it.second;
+          s << thread_it.second->GetTraceFile();
         }
       }
     }
   }
 }
 
-lldb::TraceSP TraceIntelPT::CreateInstance(const llvm::json::Object &settings,
-                                           StringRef settings_dir) {
-  return lldb::TraceSP(new TraceIntelPT(settings, settings_dir));
+std::unique_ptr<TraceThread>
+TraceIntelPT::CreateTraceThread(const ThreadSP &thread,
+                                const FileSpec &trace_file) {
+  return std::make_unique<TraceIntelPTThread>(thread, trace_file);
+}
+
+//------------------------------------------------------------------
+// TraceIntelPTThread
+//------------------------------------------------------------------
+
+llvm::Expected<std::vector<TraceInstruction>>
+TraceIntelPTThread::DecodeInstructions() {
+  std::vector<TraceInstruction> v;
+  return std::move(v);
 }
Index: lldb/include/lldb/Target/Trace.h
===================================================================
--- lldb/include/lldb/Target/Trace.h
+++ lldb/include/lldb/Target/Trace.h
@@ -14,10 +14,96 @@
 #include "lldb/Core/PluginInterface.h"
 #include "lldb/Target/TraceSettingsParser.h"
 #include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/Status.h"
 #include "lldb/lldb-private.h"
 
 namespace lldb_private {
 
+/// Base class that represents an instructions. As decoding a trace can fail to
+/// decode a instruction, we keep a flag to signal it is an error or not. For
+/// intel-pt, this happens sporadically when there's an internal buffer
+/// overflow, for example.
+class TraceInstruction {
+public:
+  TraceInstruction(uint64_t ip) : m_ip(ip), m_is_error(false) {}
+  TraceInstruction() : m_is_error(true) {}
+
+  uint64_t GetIP() const { return m_ip; }
+
+  bool IsError() const { return m_is_error; }
+
+private:
+  /// instruction address at runtime
+  uint64_t m_ip;
+  /// Useful to signal a decoding error
+  bool m_is_error;
+};
+
+/// A decoded thread trace ready to be analized or traversed. An instance of
+/// this object indicates that the entirity of decoding didn't fail. Individual
+/// instructions could have failed to decode, though.
+class TraceDecodedThread {
+public:
+  TraceDecodedThread() : m_instructions(){};
+  TraceDecodedThread(const std::vector<TraceInstruction> &instructions)
+      : m_instructions(std::move(instructions)) {}
+
+  const std::vector<TraceInstruction> &GetInstructions() const {
+    return m_instructions;
+  }
+
+private:
+  std::vector<TraceInstruction> m_instructions;
+};
+
+/// A class that represents a trace of a given thread. In order to use the
+/// decoded trace, you need to first do
+///
+/// if (llvm::Expected<TraceDecodedThread &> decoded_thread =
+/// trace_thread::Decode()) {
+///   // do stuff
+/// }
+class TraceThread {
+public:
+  TraceThread(const lldb::ThreadSP &thread_sp, const FileSpec &trace_file)
+      : m_thread_sp(thread_sp), m_trace_file(trace_file), m_decoded_thread(),
+        m_decoding_status(), m_already_decoded(false) {}
+
+  virtual ~TraceThread(){};
+
+  /// Functions that performs the decoding and caches the result value or error.
+  llvm::Expected<TraceDecodedThread &> Decode() {
+    if (!m_already_decoded) {
+      if (llvm::Expected<std::vector<TraceInstruction>> instructions =
+              DecodeInstructions()) {
+        m_decoding_status.Clear();
+        m_decoded_thread = TraceDecodedThread(std::move(*instructions));
+      } else {
+        m_decoding_status = instructions.takeError();
+      }
+      m_already_decoded = true;
+    }
+    if (m_decoding_status.Success())
+      return m_decoded_thread;
+    return m_decoding_status.ToError();
+  }
+
+  const FileSpec &GetTraceFile() const { return m_trace_file; }
+
+protected:
+  virtual llvm::Expected<std::vector<TraceInstruction>>
+  DecodeInstructions() = 0;
+
+private:
+  lldb::ThreadSP m_thread_sp;
+  FileSpec m_trace_file;
+
+  TraceDecodedThread m_decoded_thread;
+  Status m_decoding_status;
+  bool m_already_decoded;
+};
+
 /// \class Trace Trace.h "lldb/Target/Trace.h"
 /// A plug-in interface definition class for trace information.
 ///
@@ -112,14 +198,19 @@
 protected:
   Trace() {}
 
+  virtual std::unique_ptr<TraceThread>
+  CreateTraceThread(const lldb::ThreadSP &thread,
+                    const FileSpec &trace_file) = 0;
+
   /// The actual plug-in should define its own implementaiton of \a
   /// TraceSettingsParser for doing any custom parsing.
   virtual const TraceSettingsParser &GetParser() const = 0;
 
   virtual TraceSettingsParser &GetParser() = 0;
 
-  const std::map<lldb::pid_t, std::map<lldb::tid_t, lldb_private::FileSpec>> &
-  GetThreadToTraceFileMap() const;
+  const std::map<lldb::pid_t,
+                 std::map<lldb::tid_t, std::unique_ptr<TraceThread>>> &
+  GetTraceThreads() const;
 
   const llvm::json::Object &GetSettings() const;
 
@@ -130,6 +221,9 @@
 private:
   Trace(const Trace &) = delete;
   const Trace &operator=(const Trace &) = delete;
+
+  std::map<lldb::pid_t, std::map<lldb::tid_t, std::unique_ptr<TraceThread>>>
+      m_trace_threads;
 };
 
 } // namespace lldb_private
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
  • [Lldb-commits] [PATCH] D... walter erquinigo via Phabricator via lldb-commits

Reply via email to