wallace updated this revision to Diff 289300.
wallace added a comment.

- Now using the TraceDumpOptions struct


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86670

Files:
  lldb/include/lldb/Target/Target.h
  lldb/include/lldb/Target/Trace.h
  lldb/source/Commands/CommandObjectTrace.cpp
  lldb/source/Commands/Options.td
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Target/Target.cpp
  lldb/source/Target/Trace.cpp
  lldb/test/API/commands/trace/TestTraceDump.py
  lldb/test/API/commands/trace/TestTraceLoad.py
  lldb/test/API/commands/trace/intelpt-trace/trace2.json

Index: lldb/test/API/commands/trace/intelpt-trace/trace2.json
===================================================================
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace/trace2.json
@@ -0,0 +1,53 @@
+{
+  "trace": {
+    "type": "intel-pt",
+    "pt_cpu": {
+      "vendor": "intel",
+      "family": 6,
+      "model": 79,
+      "stepping": 1
+    }
+  },
+  "processes": [
+    {
+      "pid": 1,
+      "triple": "x86_64-*-linux",
+      "threads": [
+        {
+          "tid": 11,
+          "traceFile": "3842849.trace"
+        }
+      ],
+      "modules": [
+        {
+          "file": "a.out",
+          "systemPath": "a.out",
+          "loadAddress": "0x0000000000400000",
+          "uuid": "6AA9A4E2-6F28-2F33-377D-59FECE874C71-5B41261A"
+        }
+      ]
+    },
+    {
+      "pid": 2,
+      "triple": "x86_64-*-linux",
+      "threads": [
+        {
+          "tid": 21,
+          "traceFile": "3842849.trace"
+        },
+        {
+          "tid": 22,
+          "traceFile": "3842849.trace"
+        }
+      ],
+      "modules": [
+        {
+          "file": "a.out",
+          "systemPath": "a.out",
+          "loadAddress": "0x0000000000400000",
+          "uuid": "6AA9A4E2-6F28-2F33-377D-59FECE874C71-5B41261A"
+        }
+      ]
+    }
+  ]
+}
Index: lldb/test/API/commands/trace/TestTraceLoad.py
===================================================================
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -55,3 +55,5 @@
         self.expect("trace load -v " + trace_definition_file2, error=True,
             substrs=['error: JSON object is missing the "pid" field.'])
         self.assertEqual(self.dbg.GetNumTargets(), 0)
+
+        self.expect("trace dump", substrs=["error: no trace data in this target"])
Index: lldb/test/API/commands/trace/TestTraceDump.py
===================================================================
--- /dev/null
+++ lldb/test/API/commands/trace/TestTraceDump.py
@@ -0,0 +1,48 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbsuite.test.decorators import *
+
+class TestTraceDump(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+    NO_DEBUG_INFO_TESTCASE = True
+
+    def setUp(self):
+        TestBase.setUp(self)
+        if 'intel-pt' not in configuration.enabled_plugins:
+            self.skipTest("The intel-pt test plugin is not enabled")
+
+
+    def testDumpTrace(self):
+        self.expect("trace dump", substrs=["error: no trace data in this target"])
+
+        src_dir = self.getSourceDir()
+        trace_definition_file = os.path.join(src_dir, "intelpt-trace", "trace2.json")
+        self.expect("trace load " + trace_definition_file)
+
+        # An invalid thread id should show an error message
+        self.expect("trace dump -t 5678", substrs=['error: the thread id "5678" does not exist in this target'])
+
+        # Only the specified thread should be printed
+        self.expect("trace dump -t 22", substrs=["Trace files"],
+          patterns=["pid: '2', tid: '22' -> .*/test/API/commands/trace/intelpt-trace/3842849.trace"])
+
+        # Verbose should output the entire JSON settings besides the thread specific information
+        self.expect("trace dump -t 22 -v",
+          substrs=["Settings", "3842849.trace", "intel-pt", "Settings directory"],
+          patterns=["pid: '2', tid: '22' -> .*/test/API/commands/trace/intelpt-trace/3842849.trace"])
+
+        # In this case all threads should be printed
+        self.expect("trace dump", substrs=["Trace files"],
+          patterns=["pid: '2', tid: '21' -> .*/test/API/commands/trace/intelpt-trace/3842849.trace",
+            "pid: '2', tid: '22' -> .*/test/API/commands/trace/intelpt-trace/3842849.trace"])
+
+        self.expect("trace dump -t 21 --thread-id 22", substrs=["Trace files"],
+          patterns=["pid: '2', tid: '21' -> .*/test/API/commands/trace/intelpt-trace/3842849.trace",
+            "pid: '2', tid: '22' -> .*/test/API/commands/trace/intelpt-trace/3842849.trace"])
+
+        # We switch targets and check the threads of this target
+        self.dbg.SetSelectedTarget(self.dbg.FindTargetWithProcessID(1))
+        self.expect("trace dump", substrs=["Trace files"],
+          patterns=["pid: '1', tid: '11' -> .*/test/API/commands/trace/intelpt-trace/3842849.trace"])
Index: lldb/source/Target/Trace.cpp
===================================================================
--- lldb/source/Target/Trace.cpp
+++ lldb/source/Target/Trace.cpp
@@ -14,6 +14,7 @@
 #include "llvm/Support/Format.h"
 
 #include "lldb/Core/PluginManager.h"
+#include "lldb/Target/Target.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -49,12 +50,27 @@
 llvm::Error Trace::ParseSettings(Debugger &debugger) {
   if (llvm::Error err = GetParser().ParseSettings(debugger))
     return err;
+
+  for (auto target_sp : GetTargets())
+    target_sp->SetTrace(shared_from_this());
   return llvm::Error::success();
 }
 
 llvm::StringRef Trace::GetSchema() { return GetParser().GetSchema(); }
 
-std::map<lldb::pid_t, std::map<lldb::tid_t, lldb_private::FileSpec>> &
-Trace::GetThreadToTraceFileMap() {
+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 llvm::json::Object &Trace::GetSettings() const {
+  return GetParser().m_settings;
+}
+
+const llvm::StringRef Trace::GetSettingsDir() const {
+  return GetParser().m_settings_dir;
+}
+
+const std::vector<lldb::TargetSP> &Trace::GetTargets() const {
+  return GetParser().m_targets;
+}
Index: lldb/source/Target/Target.cpp
===================================================================
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -2965,6 +2965,31 @@
   return error;
 }
 
+void Target::SetTrace(const lldb::TraceSP &trace_sp) { m_trace_sp = trace_sp; }
+
+lldb::TraceSP &Target::GetTrace() { return m_trace_sp; }
+
+void Target::DumpTrace(Stream &s, TraceDumpOptions &options) {
+  if (!m_trace_sp) {
+    s << "error: no trace data in this target";
+    return;
+  }
+  if (!m_process_sp) {
+    s << "error: no processes in this target";
+    return;
+  }
+  options.process = m_process_sp.get();
+  ThreadSP thread_sp;
+  for (lldb::tid_t tid : options.tids) {
+    if (!m_process_sp->GetThreadList().FindThreadByID(tid)) {
+      s.Printf("error: the thread id \"%" PRIu64
+               "\" does not exist in this target",
+               tid);
+    }
+  }
+  m_trace_sp->Dump(s, options);
+}
+
 Status Target::Attach(ProcessAttachInfo &attach_info, Stream *stream) {
   auto state = eStateInvalid;
   auto process_sp = GetProcessSP();
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
@@ -18,7 +18,8 @@
 
 class TraceIntelPT : public lldb_private::Trace {
 public:
-  void Dump(lldb_private::Stream *s) const override;
+  void Dump(lldb_private::Stream &s,
+            const lldb_private::TraceDumpOptions &options) const override;
 
   /// PluginInterface protocol
   /// \{
@@ -40,10 +41,12 @@
   TraceIntelPT(const llvm::json::Object &settings, llvm::StringRef settings_dir)
       : Trace(), m_parser(settings, settings_dir) {}
 
+  const TraceIntelPTSettingsParser &GetParser() const override;
+
   TraceIntelPTSettingsParser &GetParser() override;
 
 private:
-  pt_cpu &GetPTCPU();
+  const pt_cpu &GetPTCPU() const;
 
   TraceIntelPTSettingsParser m_parser;
 };
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
@@ -8,8 +8,12 @@
 
 #include "TraceIntelPT.h"
 
+#include "llvm/Support/Format.h"
+#include "llvm/Support/FormatVariadic.h"
+
 #include "TraceIntelPTSettingsParser.h"
 #include "lldb/Core/PluginManager.h"
+#include "lldb/Target/Process.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -31,9 +35,13 @@
   return g_name;
 }
 
+const TraceIntelPTSettingsParser &TraceIntelPT::GetParser() const {
+  return m_parser;
+}
+
 TraceIntelPTSettingsParser &TraceIntelPT::GetParser() { return m_parser; }
 
-pt_cpu &TraceIntelPT::GetPTCPU() { return GetParser().m_pt_cpu; }
+const pt_cpu &TraceIntelPT::GetPTCPU() const { return GetParser().m_pt_cpu; }
 
 //------------------------------------------------------------------
 // PluginInterface protocol
@@ -43,7 +51,40 @@
 
 uint32_t TraceIntelPT::GetPluginVersion() { return 1; }
 
-void TraceIntelPT::Dump(lldb_private::Stream *s) const {}
+void TraceIntelPT::Dump(Stream &s, const TraceDumpOptions &options) const {
+  if (options.verbose) {
+    s << "Settings:\n";
+    s.Indent();
+    std::string str;
+    llvm::raw_string_ostream OS(str);
+    json::Object obj = GetSettings();
+    OS << llvm::formatv("{0:2}", json::Value(std::move(obj)));
+    OS.flush();
+    s << OS.str();
+    s.IndentLess();
+
+    s << "\n\nSettings directory:\n";
+    s.Indent();
+    s << GetSettingsDir() << "\n\n";
+    s.IndentLess();
+  }
+  s << "Trace files:";
+
+  // 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()) {
+    lldb::pid_t pid = process_it.first;
+    if (!options.process || options.process->GetID() == pid) {
+      for (const auto &thread_trace_file : process_it.second) {
+        lldb::tid_t tid = thread_trace_file.first;
+        if (options.tids.empty() || options.tids.count(tid)) {
+          s.Printf("\npid: '%" PRIu64 "', tid: '%" PRIu64 "' -> ", pid, tid);
+          s << thread_trace_file.second;
+        }
+      }
+    }
+  }
+}
 
 lldb::TraceSP TraceIntelPT::CreateInstance(const llvm::json::Object &settings,
                                            StringRef settings_dir) {
Index: lldb/source/Commands/Options.td
===================================================================
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -1171,6 +1171,8 @@
 let Command = "trace dump" in {
   def trace_dump_verbose : Option<"verbose", "v">, Group<1>,
     Desc<"Show verbose trace information.">;
+  def trace_dump_thread_id : Option<"thread-id", "t">, Group<1>,
+    Arg<"ThreadID">, Desc<"The thread id to dump trace information of.">;
 }
 
 let Command = "trace schema" in {
Index: lldb/source/Commands/CommandObjectTrace.cpp
===================================================================
--- lldb/source/Commands/CommandObjectTrace.cpp
+++ lldb/source/Commands/CommandObjectTrace.cpp
@@ -149,7 +149,16 @@
 
       switch (short_option) {
       case 'v': {
-        m_verbose = true;
+        m_dump_options.verbose = true;
+        break;
+      }
+      case 't': {
+        lldb::tid_t tid;
+        if (option_arg.empty() || option_arg.getAsInteger(0, tid))
+          error.SetErrorStringWithFormat("invalid thread id string '%s'",
+                                         option_arg.str().c_str());
+        else
+          m_dump_options.tids.insert(tid);
         break;
       }
       default:
@@ -159,20 +168,21 @@
     }
 
     void OptionParsingStarting(ExecutionContext *execution_context) override {
-      m_verbose = false;
+      m_dump_options = TraceDumpOptions();
     }
 
     llvm::ArrayRef<OptionDefinition> GetDefinitions() override {
       return llvm::makeArrayRef(g_trace_dump_options);
     }
 
-    bool m_verbose; // Enable verbose logging for debugging purposes.
+    TraceDumpOptions m_dump_options;
   };
 
   CommandObjectTraceDump(CommandInterpreter &interpreter)
-      : CommandObjectParsed(interpreter, "trace dump",
-                            "Dump the loaded processor trace data.",
-                            "trace dump"),
+      : CommandObjectParsed(
+            interpreter, "trace dump",
+            "Dump the loaded processor trace data from the current target.",
+            "trace dump"),
         m_options() {}
 
   ~CommandObjectTraceDump() override = default;
@@ -182,8 +192,11 @@
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
     Status error;
-    // TODO: fill in the dumping code here!
+    Target &target = GetSelectedOrDummyTarget();
+    target.DumpTrace(result.GetOutputStream(), m_options.m_dump_options);
+
     if (error.Success()) {
+      result.AppendMessage("\n");
       result.SetStatus(eReturnStatusSuccessFinishResult);
     } else {
       result.AppendErrorWithFormat("%s\n", error.AsCString());
Index: lldb/include/lldb/Target/Trace.h
===================================================================
--- lldb/include/lldb/Target/Trace.h
+++ lldb/include/lldb/Target/Trace.h
@@ -18,6 +18,14 @@
 
 namespace lldb_private {
 
+/// Helper class that holds the configuration of a Dump action.
+struct TraceDumpOptions {
+  Stream *s;
+  Process *process;           // If NULL, dump all processes
+  std::set<lldb::tid_t> tids; // Thread IDs, if empty dump all threads
+  bool verbose;
+};
+
 /// \class Trace Trace.h "lldb/Target/Trace.h"
 /// A plug-in interface definition class for trace information.
 ///
@@ -33,19 +41,23 @@
 /// Processor trace information can also be fetched through the process
 /// interfaces during a live debug session if your process supports gathering
 /// this information.
-class Trace : public PluginInterface {
+class Trace : public PluginInterface,
+              public std::enable_shared_from_this<Trace> {
 public:
   ~Trace() override = default;
 
   /// 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
-  /// readable format. Options for dumping can be added as this API is iterated
-  /// on.
+  /// readable format.
   ///
   /// \param[in] s
   ///     A stream object to dump the information to.
-  virtual void Dump(Stream *s) const = 0;
+  ///
+  /// \param[in] options
+  ///   The trace dump options. See \a TraceDumpOptions for more information.
+  virtual void Dump(lldb_private::Stream &s,
+                    const TraceDumpOptions &options) const = 0;
 
   /// Find a trace plug-in using JSON data.
   ///
@@ -101,10 +113,18 @@
 
   /// 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;
 
-  std::map<lldb::pid_t, std::map<lldb::tid_t, lldb_private::FileSpec>> &
-  GetThreadToTraceFileMap();
+  const std::map<lldb::pid_t, std::map<lldb::tid_t, lldb_private::FileSpec>> &
+  GetThreadToTraceFileMap() const;
+
+  const llvm::json::Object &GetSettings() const;
+
+  const llvm::StringRef GetSettingsDir() const;
+
+  const std::vector<lldb::TargetSP> &GetTargets() const;
 
 private:
   Trace(const Trace &) = delete;
Index: lldb/include/lldb/Target/Target.h
===================================================================
--- lldb/include/lldb/Target/Target.h
+++ lldb/include/lldb/Target/Target.h
@@ -28,6 +28,7 @@
 #include "lldb/Target/ExecutionContextScope.h"
 #include "lldb/Target/PathMappingList.h"
 #include "lldb/Target/SectionLoadHistory.h"
+#include "lldb/Target/Trace.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/Broadcaster.h"
 #include "lldb/Utility/LLDBAssert.h"
@@ -209,7 +210,7 @@
   bool GetInjectLocalVariables(ExecutionContext *exe_ctx) const;
 
   void SetInjectLocalVariables(ExecutionContext *exe_ctx, bool b);
-  
+
   void SetRequireHardwareBreakpoints(bool b);
 
   bool GetRequireHardwareBreakpoints() const;
@@ -1102,6 +1103,29 @@
 
   void ClearAllLoadedSections();
 
+  /// Set the \a Trace object containing processor trace information of this
+  /// target.
+  ///
+  /// \param[in] trace_sp
+  ///   The trace object.
+  void SetTrace(const lldb::TraceSP &trace_sp);
+
+  /// Get the \a Trace object containing processor trace information of this
+  /// target.
+  ///
+  /// \return
+  ///   The trace object. It might be undefined.
+  lldb::TraceSP &GetTrace();
+
+  /// Dump the \a Trace information.
+  ///
+  /// \param[in] s
+  ///   The stream object where the information is printed.
+  ///
+  /// \param[in] options
+  ///   The trace dump options. See \a TraceDumpOptions for more information.
+  void DumpTrace(Stream &s, lldb_private::TraceDumpOptions &options);
+
   // Since expressions results can persist beyond the lifetime of a process,
   // and the const expression results are available after a process is gone, we
   // provide a way for expressions to be evaluated from the Target itself. If
@@ -1294,12 +1318,12 @@
   lldb::PlatformSP m_platform_sp; ///< The platform for this target.
   std::recursive_mutex m_mutex; ///< An API mutex that is used by the lldb::SB*
                                 /// classes make the SB interface thread safe
-  /// When the private state thread calls SB API's - usually because it is 
+  /// When the private state thread calls SB API's - usually because it is
   /// running OS plugin or Python ThreadPlan code - it should not block on the
   /// API mutex that is held by the code that kicked off the sequence of events
-  /// that led us to run the code.  We hand out this mutex instead when we 
+  /// that led us to run the code.  We hand out this mutex instead when we
   /// detect that code is running on the private state thread.
-  std::recursive_mutex m_private_mutex; 
+  std::recursive_mutex m_private_mutex;
   Arch m_arch;
   ModuleList m_images; ///< The list of images for this process (shared
                        /// libraries and anything dynamically loaded).
@@ -1334,6 +1358,9 @@
   bool m_suppress_stop_hooks;
   bool m_is_dummy_target;
   unsigned m_next_persistent_variable_index = 0;
+  /// An optional \a lldb_private::Trace object containing processor trace
+  /// information of this target.
+  lldb::TraceSP m_trace_sp;
   /// Stores the frame recognizers of this target.
   lldb::StackFrameRecognizerManagerUP m_frame_recognizer_manager_up;
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to