wallace created this revision. wallace added reviewers: labath, clayborg. Herald added subscribers: lldb-commits, mgorny. Herald added a project: LLDB. wallace requested review of this revision. Herald added a subscriber: JDevlieghere.
Renamed ThreadIntelPT to TreaceThread, making it a top-level class. I noticed that this class can and shuld work for any trace plugin and there's nothing intel-pt specific in it. With that TraceThread change, I was able to move most of the json file parsing logic to the base class TraceSessionFileParser, which makes adding new plug-ins easier. This originally was part of https://reviews.llvm.org/D89283 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D89408 Files: lldb/include/lldb/Target/TraceSessionFileParser.h lldb/include/lldb/Target/TraceThread.h lldb/include/lldb/lldb-forward.h lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt lldb/source/Plugins/Trace/intel-pt/ThreadIntelPT.cpp lldb/source/Plugins/Trace/intel-pt/ThreadIntelPT.h lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h lldb/source/Target/CMakeLists.txt lldb/source/Target/TraceSessionFileParser.cpp lldb/source/Target/TraceThread.cpp
Index: lldb/source/Target/TraceThread.cpp =================================================================== --- lldb/source/Target/TraceThread.cpp +++ lldb/source/Target/TraceThread.cpp @@ -1,4 +1,4 @@ -//===-- ThreadIntelPT.cpp -------------------------------------------------===// +//===-- TraceThread.cpp ---------------------------------------------------===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -6,7 +6,7 @@ // //===----------------------------------------------------------------------===// -#include "ThreadIntelPT.h" +#include "lldb/Target/TraceThread.h" #include <memory> @@ -16,11 +16,10 @@ using namespace lldb; using namespace lldb_private; -using namespace lldb_private::trace_intel_pt; -void ThreadIntelPT::RefreshStateAfterStop() {} +void TraceThread::RefreshStateAfterStop() {} -RegisterContextSP ThreadIntelPT::GetRegisterContext() { +RegisterContextSP TraceThread::GetRegisterContext() { if (!m_reg_context_sp) m_reg_context_sp = CreateRegisterContextForFrame(nullptr); @@ -28,11 +27,13 @@ } RegisterContextSP -ThreadIntelPT::CreateRegisterContextForFrame(StackFrame *frame) { +TraceThread::CreateRegisterContextForFrame(StackFrame *frame) { // Eventually this will calculate the register context based on the current // trace position. return std::make_shared<RegisterContextHistory>( *this, 0, GetProcess()->GetAddressByteSize(), LLDB_INVALID_ADDRESS); } -bool ThreadIntelPT::CalculateStopInfo() { return false; } +bool TraceThread::CalculateStopInfo() { return false; } + +const FileSpec &TraceThread::GetTraceFile() const { return m_trace_file; } Index: lldb/source/Target/TraceSessionFileParser.cpp =================================================================== --- lldb/source/Target/TraceSessionFileParser.cpp +++ lldb/source/Target/TraceSessionFileParser.cpp @@ -10,8 +10,11 @@ #include <sstream> +#include "lldb/Core/Debugger.h" #include "lldb/Core/Module.h" +#include "lldb/Target/Process.h" #include "lldb/Target/Target.h" +#include "lldb/Target/TraceThread.h" using namespace lldb; using namespace lldb_private; @@ -34,7 +37,6 @@ ModuleSpec module_spec; module_spec.GetFileSpec() = local_file_spec; module_spec.GetPlatformFileSpec() = system_file_spec; - module_spec.SetObjectOffset(module.load_address.value); if (module.uuid.hasValue()) module_spec.GetUUID().SetFromStringRef(*module.uuid); @@ -42,7 +44,14 @@ Status error; ModuleSP module_sp = target_sp->GetOrCreateModule(module_spec, /*notify*/ false, &error); - return error.ToError(); + + if (error.Fail()) + return error.ToError(); + + bool load_addr_changed = false; + module_sp->SetLoadAddress(*target_sp, module.load_address.value, false, + load_addr_changed); + return llvm::Error::success(); } Error TraceSessionFileParser::CreateJSONError(json::Path::Root &root, @@ -87,6 +96,55 @@ return schema_builder.str(); } +void TraceSessionFileParser::ParseThread(ProcessSP &process_sp, + const JSONThread &thread) { + lldb::tid_t tid = static_cast<lldb::tid_t>(thread.tid); + + FileSpec trace_file(thread.trace_file); + NormalizePath(trace_file); + + ThreadSP thread_sp = + std::make_shared<TraceThread>(*process_sp, tid, trace_file); + process_sp->GetThreadList().AddThread(thread_sp); +} + +Expected<TargetSP> +TraceSessionFileParser::ParseProcess(const JSONProcess &process) { + TargetSP target_sp; + Status error = m_debugger.GetTargetList().CreateTarget( + m_debugger, /*user_exe_path*/ StringRef(), process.triple, + eLoadDependentsNo, + /*platform_options*/ nullptr, target_sp); + + if (!target_sp) + return error.ToError(); + + m_debugger.GetTargetList().SetSelectedTarget(target_sp.get()); + + ProcessSP process_sp(target_sp->CreateProcess( + /*listener*/ nullptr, "trace", + /*crash_file*/ nullptr)); + process_sp->SetID(static_cast<lldb::pid_t>(process.pid)); + + for (const JSONThread &thread : process.threads) + ParseThread(process_sp, thread); + + for (const JSONModule &module : process.modules) { + if (Error err = ParseModule(target_sp, module)) + return std::move(err); + } + + if (!process.threads.empty()) + process_sp->GetThreadList().SetSelectedThreadByIndexID(0); + + // We invoke DidAttach to create a correct stopped state for the process and + // its threads. + ArchSpec process_arch; + process_sp->DidAttach(process_arch); + + return target_sp; +} + namespace llvm { namespace json { Index: lldb/source/Target/CMakeLists.txt =================================================================== --- lldb/source/Target/CMakeLists.txt +++ lldb/source/Target/CMakeLists.txt @@ -67,6 +67,7 @@ ThreadSpec.cpp Trace.cpp TraceSessionFileParser.cpp + TraceThread.cpp UnixSignals.cpp UnwindAssembly.cpp UnwindLLDB.cpp Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h =================================================================== --- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h +++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h @@ -9,8 +9,6 @@ #ifndef LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACEINTELPTSESSIONFILEPARSER_H #define LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACEINTELPTSESSIONFILEPARSER_H -#include "intel-pt.h" - #include "TraceIntelPT.h" #include "lldb/Target/TraceSessionFileParser.h" @@ -38,8 +36,8 @@ TraceIntelPTSessionFileParser(Debugger &debugger, const llvm::json::Value &trace_session_file, llvm::StringRef session_file_dir) - : TraceSessionFileParser(session_file_dir, GetSchema()), - m_debugger(debugger), m_trace_session_file(trace_session_file) {} + : TraceSessionFileParser(debugger, session_file_dir, GetSchema()), + m_trace_session_file(trace_session_file) {} /// \return /// The JSON schema for the session data. @@ -53,24 +51,14 @@ /// errors, return a null pointer. llvm::Expected<lldb::TraceSP> Parse(); -private: - llvm::Error ParseImpl(); - - llvm::Error ParseProcess(const TraceSessionFileParser::JSONProcess &process); + lldb::TraceSP + CreateTraceIntelPTInstance(const pt_cpu &pt_cpu, + std::vector<lldb::TargetSP> &targets); - void ParseThread(lldb::ProcessSP &process_sp, - const TraceSessionFileParser::JSONThread &thread); - - void ParsePTCPU(const JSONPTCPU &pt_cpu); +private: + pt_cpu ParsePTCPU(const JSONPTCPU &pt_cpu); - Debugger &m_debugger; const llvm::json::Value &m_trace_session_file; - - /// Objects created as product of the parsing - /// \{ - pt_cpu m_pt_cpu; - std::vector<lldb::TargetSP> m_targets; - /// \} }; } // namespace trace_intel_pt Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp =================================================================== --- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp +++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp @@ -8,10 +8,11 @@ #include "TraceIntelPTSessionFileParser.h" -#include "ThreadIntelPT.h" #include "lldb/Core/Debugger.h" #include "lldb/Target/Process.h" #include "lldb/Target/Target.h" +#include "lldb/Target/ThreadList.h" +#include "lldb/Target/TraceThread.h" using namespace lldb; using namespace lldb_private; @@ -34,88 +35,54 @@ return schema; } -void TraceIntelPTSessionFileParser::ParseThread( - ProcessSP &process_sp, const TraceSessionFileParser::JSONThread &thread) { - lldb::tid_t tid = static_cast<lldb::tid_t>(thread.tid); - - FileSpec trace_file(thread.trace_file); - NormalizePath(trace_file); - - ThreadSP thread_sp = - std::make_shared<ThreadIntelPT>(*process_sp, tid, trace_file); - process_sp->GetThreadList().AddThread(thread_sp); +pt_cpu TraceIntelPTSessionFileParser::ParsePTCPU(const JSONPTCPU &pt_cpu) { + return {pt_cpu.vendor.compare("intel") == 0 ? pcv_intel : pcv_unknown, + static_cast<uint16_t>(pt_cpu.family), + static_cast<uint8_t>(pt_cpu.model), + static_cast<uint8_t>(pt_cpu.stepping)}; } -Error TraceIntelPTSessionFileParser::ParseProcess( - const TraceSessionFileParser::JSONProcess &process) { - TargetSP target_sp; - Status error = m_debugger.GetTargetList().CreateTarget( - m_debugger, /*user_exe_path*/ StringRef(), process.triple, - eLoadDependentsNo, - /*platform_options*/ nullptr, target_sp); - - if (!target_sp) - return error.ToError(); - - m_targets.push_back(target_sp); - m_debugger.GetTargetList().SetSelectedTarget(target_sp.get()); - - ProcessSP process_sp(target_sp->CreateProcess( - /*listener*/ nullptr, "trace", - /*crash_file*/ nullptr)); - process_sp->SetID(static_cast<lldb::pid_t>(process.pid)); - - for (const TraceSessionFileParser::JSONThread &thread : process.threads) - ParseThread(process_sp, thread); - - for (const TraceSessionFileParser::JSONModule &module : process.modules) { - if (Error err = ParseModule(target_sp, module)) - return err; +TraceSP TraceIntelPTSessionFileParser::CreateTraceIntelPTInstance( + const pt_cpu &pt_cpu, std::vector<TargetSP> &targets) { + std::vector<std::shared_ptr<TraceThread>> threads; + for (TargetSP &target_sp : targets) { + ThreadList &thread_list = target_sp->GetProcessSP()->GetThreadList(); + for (size_t i = 0; i < thread_list.GetSize(); i++) { + // The top-level parser creates TraceThreads, so this is safe + threads.push_back(std::static_pointer_cast<TraceThread>( + thread_list.GetThreadAtIndex(i))); + } } - if (!process.threads.empty()) - process_sp->GetThreadList().SetSelectedThreadByIndexID(0); - - // We invoke DidAttach to create a correct stopped state for the process and - // its threads. - ArchSpec process_arch; - process_sp->DidAttach(process_arch); - - return llvm::Error::success(); -} + TraceSP trace_instance(new TraceIntelPT(pt_cpu, threads)); + for (const TargetSP &target_sp : targets) + target_sp->SetTrace(trace_instance); -void TraceIntelPTSessionFileParser::ParsePTCPU(const JSONPTCPU &pt_cpu) { - m_pt_cpu = {pt_cpu.vendor.compare("intel") == 0 ? pcv_intel : pcv_unknown, - static_cast<uint16_t>(pt_cpu.family), - static_cast<uint8_t>(pt_cpu.model), - static_cast<uint8_t>(pt_cpu.stepping)}; + return trace_instance; } -Error TraceIntelPTSessionFileParser::ParseImpl() { +Expected<TraceSP> TraceIntelPTSessionFileParser::Parse() { json::Path::Root root("traceSession"); TraceSessionFileParser::JSONTraceSession<JSONTraceIntelPTSettings> session; - if (!json::fromJSON(m_trace_session_file, session, root)) { + if (!json::fromJSON(m_trace_session_file, session, root)) return CreateJSONError(root, m_trace_session_file); - } - - ParsePTCPU(session.trace.pt_cpu); - for (const TraceSessionFileParser::JSONProcess &process : session.processes) { - if (Error err = ParseProcess(process)) - return err; - } - return Error::success(); -} -Expected<TraceSP> TraceIntelPTSessionFileParser::Parse() { - if (Error err = ParseImpl()) { - // Delete all targets that were created - for (auto target_sp : m_targets) + std::vector<TargetSP> targets; + auto onError = [this, &targets]() { + // Delete all targets that were created so far in case of failures + for (TargetSP &target_sp : targets) m_debugger.GetTargetList().DeleteTarget(target_sp); - m_targets.clear(); - return std::move(err); - } + }; - return TraceIntelPT::CreateInstance(m_pt_cpu, m_targets); + for (const TraceSessionFileParser::JSONProcess &process : session.processes) { + if (Expected<TargetSP> target_sp = ParseProcess(process)) + targets.push_back(*target_sp); + else { + onError(); + return target_sp.takeError(); + } + } + return CreateTraceIntelPTInstance(ParsePTCPU(session.trace.pt_cpu), targets); } namespace llvm { 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 @@ -52,21 +52,6 @@ CreateInstance(const llvm::json::Value &trace_session_file, llvm::StringRef session_file_dir, Debugger &debugger); - /// Create an instance of this class. - /// - /// \param[in] pt_cpu - /// The libipt.h cpu information needed for decoding correctling the - /// traces. - /// - /// \param[in] targets - /// The list of targets to associate with this trace instance - /// - /// \return - /// An intel-pt trace instance. - static lldb::TraceSP - CreateInstance(const pt_cpu &pt_cpu, - const std::vector<lldb::TargetSP> &targets); - static ConstString GetPluginNameStatic(); uint32_t GetPluginVersion() override; @@ -75,14 +60,17 @@ llvm::StringRef GetSchema() override; private: - TraceIntelPT(const pt_cpu &pt_cpu, const std::vector<lldb::TargetSP> &targets) - : m_pt_cpu(pt_cpu) { - for (const lldb::TargetSP &target_sp : targets) - m_targets.push_back(target_sp); - } + friend class TraceIntelPTSessionFileParser; + + /// \param[in] trace_threads + /// TraceThread instances, which are not live-processes and whose trace + /// files are fixed. + TraceIntelPT(const pt_cpu &pt_cpu, + const std::vector<std::shared_ptr<TraceThread>> &traced_threads); pt_cpu m_pt_cpu; - std::vector<std::weak_ptr<Target>> m_targets; + std::map<std::pair<lldb::pid_t, lldb::tid_t>, std::shared_ptr<TraceThread>> + m_trace_threads; }; } // namespace trace_intel_pt 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 @@ -10,7 +10,9 @@ #include "TraceIntelPTSessionFileParser.h" #include "lldb/Core/PluginManager.h" +#include "lldb/Target/Process.h" #include "lldb/Target/Target.h" +#include "lldb/Target/TraceThread.h" using namespace lldb; using namespace lldb_private; @@ -56,11 +58,11 @@ .Parse(); } -TraceSP TraceIntelPT::CreateInstance(const pt_cpu &pt_cpu, - const std::vector<TargetSP> &targets) { - TraceSP trace_instance(new TraceIntelPT(pt_cpu, targets)); - for (const TargetSP &target_sp : targets) - target_sp->SetTrace(trace_instance); - - return trace_instance; +TraceIntelPT::TraceIntelPT( + const pt_cpu &pt_cpu, + const std::vector<std::shared_ptr<TraceThread>> &traced_threads) + : m_pt_cpu(pt_cpu) { + for (const std::shared_ptr<TraceThread> &thread : traced_threads) + m_trace_threads.emplace( + std::make_pair(thread->GetProcess()->GetID(), thread->GetID()), thread); } Index: lldb/source/Plugins/Trace/intel-pt/ThreadIntelPT.h =================================================================== --- lldb/source/Plugins/Trace/intel-pt/ThreadIntelPT.h +++ /dev/null @@ -1,54 +0,0 @@ -//===-- ThreadIntelPT.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_SOURCE_PLUGINS_TRACE_INTEL_PT_THREADINTELPT_H -#define LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_THREADINTELPT_H - -#include "lldb/Target/Thread.h" - -namespace lldb_private { -namespace trace_intel_pt { - -class ThreadIntelPT : public Thread { -public: - /// Create an Intel PT-traced thread. - /// - /// \param[in] process - /// The process that owns this thread. - /// - /// \param[in] tid - /// The thread id of this thread. - /// - /// \param[in] trace_file - /// The trace file for this thread. - /// - /// \param[in] pt_cpu - /// The Intel CPU information required to decode the \a trace_file. - ThreadIntelPT(Process &process, lldb::tid_t tid, const FileSpec &trace_file) - : Thread(process, tid), m_trace_file(trace_file) {} - - void RefreshStateAfterStop() override; - - lldb::RegisterContextSP GetRegisterContext() override; - - lldb::RegisterContextSP - CreateRegisterContextForFrame(StackFrame *frame) override; - -protected: - bool CalculateStopInfo() override; - - lldb::RegisterContextSP m_thread_reg_ctx_sp; - -private: - FileSpec m_trace_file; -}; - -} // namespace trace_intel_pt -} // namespace lldb_private - -#endif // LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_THREADINTELPT_H Index: lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt =================================================================== --- lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt +++ lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt @@ -12,7 +12,6 @@ add_lldb_library(lldbPluginTraceIntelPT PLUGIN TraceIntelPT.cpp TraceIntelPTSessionFileParser.cpp - ThreadIntelPT.cpp LINK_LIBS lldbCore Index: lldb/include/lldb/lldb-forward.h =================================================================== --- lldb/include/lldb/lldb-forward.h +++ lldb/include/lldb/lldb-forward.h @@ -228,6 +228,7 @@ class ThreadSpec; class Trace; class TraceSessionFileParser; +class TraceThread; class TraceOptions; class Type; class TypeAndOrName; Index: lldb/include/lldb/Target/TraceThread.h =================================================================== --- /dev/null +++ lldb/include/lldb/Target/TraceThread.h @@ -0,0 +1,59 @@ +//===-- TraceThread.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_TARGET_TRACETHREAD_H +#define LLDB_TARGET_TRACETHREAD_H + +#include "lldb/Target/Thread.h" + +namespace lldb_private { + +/// \class TraceThread TraceThread.h +/// +/// Thread implementation used for representing threads gotten from trace +/// session files, which are similar to threads from core files. +/// +/// See \a TraceSessionFileParser for more information regarding trace session +/// files. +class TraceThread : public Thread { +public: + /// \param[in] process + /// The process who owns this thread. + /// + /// \param[in] tid + /// The tid of this thread. + /// + /// \param[in] trace_file. + /// The file that contains the list of instructions that were traced when + /// this thread was being executed. + TraceThread(Process &process, lldb::tid_t tid, const FileSpec trace_file) + : Thread(process, tid), m_trace_file(trace_file) {} + + void RefreshStateAfterStop() override; + + lldb::RegisterContextSP GetRegisterContext() override; + + lldb::RegisterContextSP + CreateRegisterContextForFrame(StackFrame *frame) override; + + /// \return + /// The trace file of this thread. + const FileSpec &GetTraceFile() const; + +protected: + bool CalculateStopInfo() override; + + lldb::RegisterContextSP m_thread_reg_ctx_sp; + +private: + FileSpec m_trace_file; +}; + +} // namespace lldb_private + +#endif // LLDB_TARGET_TRACETHREAD_H Index: lldb/include/lldb/Target/TraceSessionFileParser.h =================================================================== --- lldb/include/lldb/Target/TraceSessionFileParser.h +++ lldb/include/lldb/Target/TraceSessionFileParser.h @@ -61,9 +61,10 @@ }; /// \} - TraceSessionFileParser(llvm::StringRef session_file_dir, + TraceSessionFileParser(Debugger &debugger, llvm::StringRef session_file_dir, llvm::StringRef schema) - : m_session_file_dir(session_file_dir), m_schema(schema) {} + : m_debugger(debugger), m_session_file_dir(session_file_dir), + m_schema(schema) {} /// Build the full schema for a Trace plug-in. /// @@ -80,6 +81,10 @@ /// modifies the given file_spec. void NormalizePath(lldb_private::FileSpec &file_spec); + void ParseThread(lldb::ProcessSP &process_sp, const JSONThread &thread); + + llvm::Expected<lldb::TargetSP> ParseProcess(const JSONProcess &process); + llvm::Error ParseModule(lldb::TargetSP &target_sp, const JSONModule &module); /// Create a user-friendly error message upon a JSON-parsing failure using the @@ -96,6 +101,7 @@ llvm::Error CreateJSONError(llvm::json::Path::Root &root, const llvm::json::Value &value); + Debugger &m_debugger; std::string m_session_file_dir; llvm::StringRef m_schema; };
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
