wallace updated this revision to Diff 290605.
wallace added a comment.
Addresses Greg's comments.
As a side note, I'll work on a RFC for the LLDB mailing list to get feedback on
the overall design of the feature, as @labath suggested. However, Greg and I
want to get these patches in, as we need this functionality soon. We can easily
make modifications whenever suggestions come, following an iterative approach
to development.
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
@@ -2964,6 +2964,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
@@ -1179,6 +1179,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,13 @@
namespace lldb_private {
+/// Helper class that holds the configuration of a Dump action.
+struct TraceDumpOptions {
+ Process *process = nullptr; // If NULL, dump all processes
+ std::set<lldb::tid_t> tids; // Thread IDs, if empty dump all threads
+ bool verbose = false;
+};
+
/// \class Trace Trace.h "lldb/Target/Trace.h"
/// A plug-in interface definition class for trace information.
///
@@ -33,19 +40,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 +112,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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
