[trace] Add LoadTraceFromFile to SBDebugger and SBTrace

[jj10306]

Summary

Before we add bindings for the TraceCursor to allow for programatic traversal of traces, we need to bindings to load a trace from a trace session file.
This diff adds trace load functionality to SBDebugger via the LoadTraceFromFile method and updates the intelpt test case class to have testTraceLoad method so we can take advantage of the testApiAndSB decorator to test both the CLI and SB without duplicating code.

[JDevlieghere]

lldb/include/lldb/API/SBDebugger.h

403

Can this take a filespec?

[wallace]

just some minor changes, including requiring the FileSpec as input to the SB function

lldb/include/lldb/API/SBDebugger.h

394–395

403

+1

403

We need to come up with a good public name for the json file. I've been using trace session file, but I dislike. What about trace description file? Then we can have a "trace bundle" that contains "trace description file". Do you have better suggestions?

lldb/include/lldb/API/SBTrace.h

15

+1

24–41

you can also refer to the documentation of the other function if you don't want to repeat yourself

lldb/source/API/SBTrace.cpp

32

you'll need LLDB_INSTRUMENT_VA(this, error, debugger, trace_session_file_path). This LLDB_INSTRUMENT_VA macro is used for diagnosing test failures on some systems. I haven't used this diagnostics systems myself but let's adhere to the convention

33–38

if you want, add using namespace llvm so that you don't have to write llvm:: everywhere. You can also omit lldb:: already

lldb/source/Commands/CommandObjectTrace.cpp

92–105

nice refactor

102

you can drop the trace_sp check because it should be valid because otherwise you'd have gotten an Expected

lldb/source/Target/Trace.cpp

92–114

you can drop all llvm:: and lldb:: if you want

lldb/test/API/commands/trace/TestTraceLoad.py

12

thanks!

102

split into multiple lines

[jj10306]

Address comments
Note: I changed "trace session file" to "trace description file" in the new methods, but saving the project wide rename/adoption of "trace description file" for a separate diff as to not cloud the purpose of this diff.

[jj10306]

lldb/include/lldb/API/SBDebugger.h

403

I think trace description file is a more appropriate name as well.

lldb/packages/Python/lldbsuite/test/tools/intelpt/intelpt_testcase.py

139

the single arg constructor is deprecated, update this to pass resolve flag

[wallace]

nice job!

lldb/packages/Python/lldbsuite/test/tools/intelpt/intelpt_testcase.py

139

i don't think it's deprecated, but you should pass the resolve flag anyone

[jj10306]

rebase and use SBFileSpec constructor with that requires an explicit resolve flag before landing

lldb/packages/Python/lldbsuite/test/tools/intelpt/intelpt_testcase.py

139

You can find the deprecation warning in the comments here:
https://lldb.llvm.org/cpp_reference/SBFileSpec_8cpp_source.html

[wallace]

lldb/packages/Python/lldbsuite/test/tools/intelpt/intelpt_testcase.py

139

TIL!

lldb/bindings/interface/SBDebugger.i

RunCommandInterpreter (bool auto_handle_events,
                            SBCommandInterpreterRunOptions &options,
                            int  &num_errors,
                            bool &quit_requested,
                            bool &stopped_for_crash);
 
     lldb::SBError
     RunREPL (lldb::LanguageType language, const char *repl_options);
 
+    SBTrace LoadTraceFromFile(SBError &error, const char *trace_file_path);
+
 #ifdef SWIGPYTHON
     %pythoncode%{
     def __iter__(self):
         '''Iterate over all targets in a lldb.SBDebugger object.'''
         return lldb_iter(self, 'GetNumTargets', 'GetTargetAtIndex')
 
     def __len__(self):
         '''Return the number of targets in a lldb.SBDebugger object.'''
         return self.GetNumTargets()
     %}
 #endif
 
 }; // class SBDebugger
 
 } // namespace lldb

lldb/include/lldb/API/SBDebugger.h

void RunCommandInterpreter(bool auto_handle_events, bool spawn_thread,
                              int &num_errors, bool &quit_requested,
                              bool &stopped_for_crash);
 
   SBCommandInterpreterRunResult
   RunCommandInterpreter(const SBCommandInterpreterRunOptions &options);
 
   SBError RunREPL(lldb::LanguageType language, const char *repl_options);
 
+  /// Load trace from trace session file and create Targets based on the
+  /// contents of the file.
+  ///
+  /// \param[out] error
+  ///   An error if the trace could not be created.
+  ///
+  /// \param[in] trace_session_file_path
+  ///     The full or partial path of the trace session file describing the
+  ///     trace session.
+  SBTrace LoadTraceFromFile(SBError &error, const char *trace_file_path);
+
 private:
   friend class SBCommandInterpreter;
   friend class SBInputReader;
   friend class SBListener;
   friend class SBProcess;
   friend class SBSourceManager;
   friend class SBTarget;
+  friend class SBTrace;
 
   lldb::SBTarget FindTargetWithLLDBProcess(const lldb::ProcessSP &processSP);
 
   void reset(const lldb::DebuggerSP &debugger_sp);
 
   lldb_private::Debugger *get() const;
 
   lldb_private::Debugger &ref() const;

lldb/include/lldb/API/SBTrace.h

 //===-- SBTrace.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_API_SBTRACE_H
 #define LLDB_API_SBTRACE_H
 
 #include "lldb/API/SBDefines.h"
 #include "lldb/API/SBError.h"
 
-class TraceImpl;
-
 namespace lldb {
 
 class LLDB_API SBTrace {
 public:
   /// Default constructor for an invalid Trace object.
   SBTrace();
 
   SBTrace(const lldb::TraceSP &trace_sp);
 
+  /// Load trace from trace session file and create Targets based on the
+  /// contents of the file.
+  ///
+  /// \param[out] error
+  ///   An error if the trace could not be created.
+  ///
+  /// \param[in] debugger
+  ///     The debugger instance where new Targets will be created as part of the
+  ///     JSON data parsing.
+  ///
+  /// \param[in] trace_session_file_path
+  ///     The full or partial path of the trace session file describing the
+  ///     trace session.  static SBTrace LoadTraceFromFile(SBDebugger &debugger,
+  ///     const char *trace_session_file_path);
+  ///
+  /// \return
+  ///     A \a TraceSP instance, or an \a llvm::Error if loading the trace
+  ///     from file fails.
+  static SBTrace LoadTraceFromFile(SBError &error, SBDebugger &debugger,
+                                   const char *trace_session_file_path);
+
   /// \return
   ///     A description of the parameters to use for the \a SBTrace::Start
   ///     method, or \b null if the object is invalid.
   const char *GetStartConfigurationHelp();
 
   /// Start tracing all current and future threads in a live process using a
   /// provided configuration. This is referred as "process tracing" in the
   /// documentation.

lldb/include/lldb/Target/Trace.h

public:
 
   /// Get the schema of a Trace plug-in given its name.
   ///
   /// \param[in] plugin_name
   ///     Name of the trace plugin.
   static llvm::Expected<llvm::StringRef>
   FindPluginSchema(llvm::StringRef plugin_name);
 
+  /// Load trace from trace session file and create Targets based on the
+  /// contents of the file.
+  ///
+  /// \param[in] debugger
+  ///     The debugger instance where new Targets will be created as part of the
+  ///     JSON data parsing.
+  ///
+  /// \param[in] trace_session_file_path
+  ///     The full or partial path of the trace session file describing the
+  ///     trace session.
+  ///
+  /// \return
+  ///     A \a TraceSP instance, or an \a llvm::Error if loading the trace
+  ///     from file fails.
+  static llvm::Expected<lldb::TraceSP>
+  LoadPostMortemTraceFromFile(Debugger &debugger,
+                              llvm::StringRef trace_session_file_path);
+
   /// Get the command handle for the "process trace start" command.
   virtual lldb::CommandObjectSP
   GetProcessTraceStartCommand(CommandInterpreter &interpreter) = 0;
 
   /// Get the command handle for the "thread trace start" command.
   virtual lldb::CommandObjectSP
   GetThreadTraceStartCommand(CommandInterpreter &interpreter) = 0;
 

lldb/packages/Python/lldbsuite/test/tools/intelpt/intelpt_testcase.py

def traceStopThread(self, thread=None, error=False, substrs=None):
             thread = thread if thread is not None else self.thread()
             self.assertSBError(self.target().GetTrace().Stop(thread), error)
 
         else:
             command = "thread trace stop"
             if thread is not None:
                 command += " " + str(thread.GetIndexID())
             self.expect(command, error=error, substrs=substrs)
+
+    def traceLoad(self, traceSessionFilePath="trace.json", error=False, substrs=None):
+        if self.USE_SB_API:
+            loadTraceError = lldb.SBError()
+            _trace = self.dbg.LoadTraceFromFile(loadTraceError, traceSessionFilePath)
+            self.assertSBError(loadTraceError, error)
+        else:
+            command = f"trace load -v {traceSessionFilePath}"
+            self.expect(command, error=error, substrs=substrs)

lldb/source/API/SBDebugger.cpp

 #include "lldb/API/SBListener.h"
 #include "lldb/API/SBProcess.h"
 #include "lldb/API/SBSourceManager.h"
 #include "lldb/API/SBStream.h"
 #include "lldb/API/SBStringList.h"
 #include "lldb/API/SBStructuredData.h"
 #include "lldb/API/SBTarget.h"
 #include "lldb/API/SBThread.h"
+#include "lldb/API/SBTrace.h"
 #include "lldb/API/SBTypeCategory.h"
 #include "lldb/API/SBTypeFilter.h"
 #include "lldb/API/SBTypeFormat.h"
 #include "lldb/API/SBTypeNameSpecifier.h"
 #include "lldb/API/SBTypeSummary.h"
 #include "lldb/API/SBTypeSynthetic.h"
 
 #include "lldb/Core/Debugger.h"

 void SBDebugger::SetLoggingCallback(lldb::LogOutputCallback log_callback,
                                     void *baton) {
   LLDB_INSTRUMENT_VA(this, log_callback, baton);
 
   if (m_opaque_sp) {
     return m_opaque_sp->SetLoggingCallback(log_callback, baton);
   }
 }
+
+SBTrace SBDebugger::LoadTraceFromFile(SBError &error,
+                                      const char *trace_file_path) {
+  return SBTrace::LoadTraceFromFile(error, *this, trace_file_path);
+}

lldb/source/API/SBTrace.cpp

 //===-- SBTrace.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 "lldb/Target/Process.h"
 #include "lldb/Utility/Instrumentation.h"
 
+#include "lldb/API/SBDebugger.h"
 #include "lldb/API/SBStructuredData.h"
 #include "lldb/API/SBThread.h"
 #include "lldb/API/SBTrace.h"
 
 #include "lldb/Core/StructuredDataImpl.h"
 
 #include <memory>
 
 using namespace lldb;
 using namespace lldb_private;
 
 SBTrace::SBTrace() { LLDB_INSTRUMENT_VA(this); }
 
 SBTrace::SBTrace(const lldb::TraceSP &trace_sp) : m_opaque_sp(trace_sp) {
   LLDB_INSTRUMENT_VA(this, trace_sp);
 }
 
+SBTrace SBTrace::LoadTraceFromFile(SBError &error, SBDebugger &debugger,
+                                   const char *trace_session_file_path) {
+
+  llvm::Expected<lldb::TraceSP> trace_or_err =
+      Trace::LoadPostMortemTraceFromFile(debugger.ref(),
+                                         trace_session_file_path);
+
+  if (!trace_or_err) {
+    error.SetErrorString(llvm::toString(trace_or_err.takeError()).c_str());
+    return SBTrace();
+  }
+
+  return SBTrace(trace_or_err.get());
+}
+
 const char *SBTrace::GetStartConfigurationHelp() {
   LLDB_INSTRUMENT_VA(this);
   return m_opaque_sp ? m_opaque_sp->GetStartConfigurationHelp() : nullptr;
 }
 
 SBError SBTrace::Start(const SBStructuredData &configuration) {
   LLDB_INSTRUMENT_VA(this, configuration);
   SBError error;

lldb/source/Commands/CommandObjectTrace.cpp

public:
 
   Options *GetOptions() override { return &m_options; }
 
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
     if (command.size() != 1) {
       result.AppendError(
           "a single path to a JSON file containing a trace session"
-          "is required");
+          " is required");
       return false;
     }
 
-    auto end_with_failure = [&result](llvm::Error err) -> bool {
-      result.AppendErrorWithFormat("%s\n",
-                                   llvm::toString(std::move(err)).c_str());
-      return false;
-    };
+    llvm::Expected<lldb::TraceSP> trace_or_err =
+        Trace::LoadPostMortemTraceFromFile(GetDebugger(), command[0].ref());
 
-    FileSpec json_file(command[0].ref());
+    if (!trace_or_err) {
+      result.AppendErrorWithFormat(
+          "%s\n", llvm::toString(trace_or_err.takeError()).c_str());
+      return false;
+    }
 
-    auto buffer_or_error = llvm::MemoryBuffer::getFile(json_file.GetPath());
-    if (!buffer_or_error) {
-      return end_with_failure(llvm::createStringError(
-          std::errc::invalid_argument, "could not open input file: %s - %s.",
-          json_file.GetPath().c_str(),
-          buffer_or_error.getError().message().c_str()));
-    }
-
-    llvm::Expected<json::Value> session_file =
-        json::parse(buffer_or_error.get()->getBuffer().str());
-    if (!session_file)
-      return end_with_failure(session_file.takeError());
-
-    if (Expected<lldb::TraceSP> traceOrErr =
-            Trace::FindPluginForPostMortemProcess(
-                GetDebugger(), *session_file,
-                json_file.GetDirectory().AsCString())) {
-      lldb::TraceSP trace_sp = traceOrErr.get();
-      if (m_options.m_verbose && trace_sp)
-        result.AppendMessageWithFormatv("loading trace with plugin {0}\n",
-                                        trace_sp->GetPluginName());
-    } else
-      return end_with_failure(traceOrErr.takeError());
+    lldb::TraceSP trace_sp = trace_or_err.get();
+    if (m_options.m_verbose && trace_sp) {
+    }
 
     result.SetStatus(eReturnStatusSuccessFinishResult);
     return true;
   }
 
   CommandOptions m_options;
 };
 

lldb/source/Target/Trace.cpp

 
 static Error createInvalidPlugInError(StringRef plugin_name) {
   return createStringError(
       std::errc::invalid_argument,
       "no trace plug-in matches the specified type: \"%s\"",
       plugin_name.data());
 }
 
+llvm::Expected<lldb::TraceSP>
+Trace::LoadPostMortemTraceFromFile(Debugger &debugger,
+                                   llvm::StringRef trace_session_file_path) {
+
+  FileSpec json_file(trace_session_file_path);
+
+  auto buffer_or_error = llvm::MemoryBuffer::getFile(json_file.GetPath());
+  if (!buffer_or_error) {
+    return llvm::createStringError(
+        std::errc::invalid_argument, "could not open input file: %s - %s.",
+        json_file.GetPath().c_str(),
+        buffer_or_error.getError().message().c_str());
+  }
+
+  llvm::Expected<json::Value> session_file =
+      json::parse(buffer_or_error.get()->getBuffer().str());
+  if (!session_file) {
+    return session_file.takeError();
+  }
+
+  return Trace::FindPluginForPostMortemProcess(
+      debugger, *session_file, json_file.GetDirectory().AsCString());
+}
+
 Expected<lldb::TraceSP>
 Trace::FindPluginForPostMortemProcess(Debugger &debugger,
                                       const json::Value &trace_session_file,
                                       StringRef session_file_dir) {
   JSONSimpleTraceSession json_session;
   json::Path::Root root("traceSession");
   if (!json::fromJSON(trace_session_file, json_session, root))
     return root.getError();

lldb/test/API/commands/trace/TestTraceLoad.py

 import lldb
 from intelpt_testcase import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 from lldbsuite.test.decorators import *
 
 class TestTraceLoad(TraceIntelPTTestCaseBase):
 
     mydir = TestBase.compute_mydir(__file__)
     NO_DEBUG_INFO_TESTCASE = True
 
+    @testSBAPIAndCommands
     def testLoadMultiCoreTrace(self):
         src_dir = self.getSourceDir()
         trace_definition_file = os.path.join(src_dir, "intelpt-multi-core-trace", "trace.json")
-        self.expect("trace load -v " + trace_definition_file, substrs=["intel-pt"])
+        self.traceLoad(traceSessionFilePath=trace_definition_file, substrs=["intel-pt"])
         self.expect("thread trace dump instructions 2 -t",
           substrs=["19521: [tsc=0x008fb5211c143fd8] error: expected tracing enabled event",
                    "m.out`foo() + 65 at multi_thread.cpp:12:21",
                    "19520: [tsc=0x008fb5211bfbc69e] 0x0000000000400ba7    jg     0x400bb3"])
         self.expect("thread trace dump instructions 3 -t",
           substrs=["67910: [tsc=0x008fb5211bfdf270] 0x0000000000400bd7    addl   $0x1, -0x4(%rbp)",
                    "m.out`bar() + 26 at multi_thread.cpp:20:6"])
 
+    @testSBAPIAndCommands
     def testLoadMultiCoreTraceWithStringNumbers(self):
         src_dir = self.getSourceDir()
         trace_definition_file = os.path.join(src_dir, "intelpt-multi-core-trace", "trace_with_string_numbers.json")
-        self.expect("trace load -v " + trace_definition_file, substrs=["intel-pt"])
+        self.traceLoad(traceSessionFilePath=trace_definition_file, substrs=["intel-pt"])
         self.expect("thread trace dump instructions 2 -t",
           substrs=["19521: [tsc=0x008fb5211c143fd8] error: expected tracing enabled event",
                    "m.out`foo() + 65 at multi_thread.cpp:12:21",
                    "19520: [tsc=0x008fb5211bfbc69e] 0x0000000000400ba7    jg     0x400bb3"])
         self.expect("thread trace dump instructions 3 -t",
           substrs=["67910: [tsc=0x008fb5211bfdf270] 0x0000000000400bd7    addl   $0x1, -0x4(%rbp)",
                    "m.out`bar() + 26 at multi_thread.cpp:20:6"])
 
+    @testSBAPIAndCommands
     def testLoadMultiCoreTraceWithMissingThreads(self):
         src_dir = self.getSourceDir()
         trace_definition_file = os.path.join(src_dir, "intelpt-multi-core-trace", "trace_missing_threads.json")
-        self.expect("trace load -v " + trace_definition_file, substrs=["intel-pt"])
+        self.traceLoad(traceSessionFilePath=trace_definition_file, substrs=["intel-pt"])
         self.expect("thread trace dump instructions 3 -t",
           substrs=["19521: [tsc=0x008fb5211c143fd8] error: expected tracing enabled event",
                    "m.out`foo() + 65 at multi_thread.cpp:12:21",
                    "19520: [tsc=0x008fb5211bfbc69e] 0x0000000000400ba7    jg     0x400bb3"])
         self.expect("thread trace dump instructions 2 -t",
           substrs=["67910: [tsc=0x008fb5211bfdf270] 0x0000000000400bd7    addl   $0x1, -0x4(%rbp)",
                    "m.out`bar() + 26 at multi_thread.cpp:20:6"])
 
+    @testSBAPIAndCommands
     def testLoadTrace(self):
         src_dir = self.getSourceDir()
         trace_definition_file = os.path.join(src_dir, "intelpt-trace", "trace.json")
-        self.expect("trace load -v " + trace_definition_file, substrs=["intel-pt"])
+        self.traceLoad(traceSessionFilePath=trace_definition_file, substrs=["intel-pt"])
 
         target = self.dbg.GetSelectedTarget()
         process = target.GetProcess()
         self.assertEqual(process.GetProcessID(), 1234)
 
         self.assertEqual(process.GetNumThreads(), 1)
         self.assertEqual(process.GetThreadAtIndex(0).GetThreadID(), 3842849)
 
thread #1: tid = 3842849
   Events:
     Number of instructions with events: 1
     Number of individual events: 1
       paused: 1
 
   Errors:
     Number of TSC decoding errors: 0'''])
 
+    @testSBAPIAndCommands
     def testLoadInvalidTraces(self):
         src_dir = self.getSourceDir()
+        trace_definition_file = os.path.join(src_dir, "intelpt-trace", "trace_bad.json")
         # We test first an invalid type
-        self.expect("trace load -v " + os.path.join(src_dir, "intelpt-trace", "trace_bad.json"), error=True,
-          substrs=['''error: expected object at traceSession.processes[0]
+        self.traceLoad(traceSessionFilePath=trace_definition_file, error=True, substrs=['''error: expected object at traceSession.processes[0]
 
 Context:
 {
   "cpuInfo": { ... },
   "processes": [
     /* error: expected object */
     123
   ],