wallace created this revision. wallace added reviewers: jj10306, zrthxn. Herald added a project: All. wallace requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits.
Some parts of the code have to distinguish between live and postmortem threads to figure out how to get some data, e.g. thread trace buffers. This makes the code less generic and more error prone. An example of that is that we have two different decoders: LiveThreadDecoder and PostMortemThreadDecoder. They exist because getting the trace bufer is different for each case. The problem doesn't stop there. Soon we'll have even more kinds of data, like the context switch trace, whose fetching will be different for live and post- mortem processes. As a way to fix this, I'm creating a common API for accessing thread data, which is able to figure out how to handle the postmortem and live cases on behalf of the caller. As a result of that, I was able to eliminate the two decoders and unify them into a simpler one. Not only that, our TraceSave functionality only worked for live threads, but now it can also work for postmortem processes, which might be useful now, but it might in the future. This common API is OnThreadBinaryDataRead. More information in the inline documentation. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D123281 Files: lldb/include/lldb/Target/Trace.h lldb/source/Plugins/Trace/common/TraceSessionSaver.cpp lldb/source/Plugins/Trace/common/TraceSessionSaver.h lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.cpp lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.h lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp lldb/source/Target/Trace.cpp
Index: lldb/source/Target/Trace.cpp =================================================================== --- lldb/source/Target/Trace.cpp +++ lldb/source/Target/Trace.cpp @@ -215,3 +215,67 @@ RefreshLiveProcessState(); return m_stop_id; } + +llvm::Expected<FileSpec> +Trace::GetPostMortemThreadDataFile(lldb::tid_t tid, llvm::StringRef kind) { + auto NotFoundError = [&]() { + return createStringError( + inconvertibleErrorCode(), + formatv("The thread with tid={0} doesn't have the tracing data {1}", + tid, kind)); + }; + + auto it = m_postmortem_thread_data.find(tid); + if (it == m_postmortem_thread_data.end()) + return NotFoundError(); + + std::unordered_map<std::string, FileSpec> &data_kind_to_file_spec_map = + it->second; + auto it2 = data_kind_to_file_spec_map.find(kind.str()); + if (it2 == data_kind_to_file_spec_map.end()) + return NotFoundError(); + return it2->second; +} + +void Trace::SetPostMortemThreadDataFile(lldb::tid_t tid, llvm::StringRef kind, + FileSpec file_spec) { + m_postmortem_thread_data[tid][kind.str()] = file_spec; +} + +llvm::Error Trace::OnLiveThreadBinaryDataRead( + lldb::tid_t tid, llvm::StringRef kind, + std::function<llvm::Error(llvm::MutableArrayRef<uint8_t> data)> callback) { + Expected<std::vector<uint8_t>> data = GetLiveThreadBinaryData(tid, kind); + if (!data) + return data.takeError(); + return callback(*data); +} + +llvm::Error Trace::OnPostMortemThreadBinaryDataRead( + lldb::tid_t tid, llvm::StringRef kind, + std::function<llvm::Error(llvm::MutableArrayRef<uint8_t> data)> callback) { + Expected<FileSpec> file = GetPostMortemThreadDataFile(tid, kind); + if (!file) + return file.takeError(); + ErrorOr<std::unique_ptr<MemoryBuffer>> trace_or_error = + MemoryBuffer::getFile(file->GetPath()); + if (std::error_code err = trace_or_error.getError()) + return errorCodeToError(err); + + MemoryBuffer &trace = **trace_or_error; + MutableArrayRef<uint8_t> trace_data( + // The libipt library does not modify the trace buffer, hence the + // following cast is safe. + reinterpret_cast<uint8_t *>(const_cast<char *>(trace.getBufferStart())), + trace.getBufferSize()); + return callback(trace_data); +} + +llvm::Error Trace::OnThreadBinaryDataRead( + lldb::tid_t tid, llvm::StringRef kind, + std::function<llvm::Error(llvm::MutableArrayRef<uint8_t> data)> callback) { + if (m_live_process) + return OnLiveThreadBinaryDataRead(tid, kind, callback); + else + return OnPostMortemThreadBinaryDataRead(tid, kind, callback); +} Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp =================================================================== --- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp +++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp @@ -48,15 +48,8 @@ return json_intel_pt_trace.takeError(); llvm::Expected<JSONTraceSessionBase> json_session_description = - TraceSessionSaver::BuildProcessesSection( - *live_process, - [&](lldb::tid_t tid) - -> llvm::Expected<llvm::Optional<std::vector<uint8_t>>> { - if (!trace_ipt.IsTraced(tid)) - return None; - return trace_ipt.GetLiveThreadBuffer(tid); - }, - directory); + TraceSessionSaver::BuildProcessesSection(*live_process, + "threadTraceBuffer", directory); if (!json_session_description) return json_session_description.takeError(); 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 @@ -137,8 +137,11 @@ StructuredData::ObjectSP configuration = StructuredData::ObjectSP()) override; - /// Get the thread buffer content for a live thread - llvm::Expected<std::vector<uint8_t>> GetLiveThreadBuffer(lldb::tid_t tid); + /// See \a Trace::OnThreadBinaryDataRead(). + llvm::Error OnThreadBufferRead( + lldb::tid_t tid, + std::function<llvm::Error(llvm::MutableArrayRef<uint8_t> callback)> + callback); llvm::Expected<pt_cpu> GetCPUInfo(); 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 @@ -78,10 +78,12 @@ const pt_cpu &cpu_info, const std::vector<ThreadPostMortemTraceSP> &traced_threads) : m_cpu_info(cpu_info) { - for (const ThreadPostMortemTraceSP &thread : traced_threads) - m_thread_decoders.emplace( - thread->GetID(), - std::make_unique<PostMortemThreadDecoder>(thread, *this)); + for (const ThreadPostMortemTraceSP &thread : traced_threads) { + m_thread_decoders.emplace(thread->GetID(), + std::make_unique<ThreadDecoder>(thread, *this)); + SetPostMortemThreadDataFile(thread->GetID(), "threadTraceBuffer", + thread->GetTraceFile()); + } } DecodedThreadSP TraceIntelPT::Decode(Thread &thread) { @@ -213,10 +215,10 @@ } for (const TraceThreadState &thread_state : state->tracedThreads) { - Thread &thread = - *m_live_process->GetThreadList().FindThreadByID(thread_state.tid); + ThreadSP thread_sp = + m_live_process->GetThreadList().FindThreadByID(thread_state.tid); m_thread_decoders.emplace( - thread_state.tid, std::make_unique<LiveThreadDecoder>(thread, *this)); + thread_state.tid, std::make_unique<ThreadDecoder>(thread_sp, *this)); } } @@ -352,7 +354,9 @@ return Start(tids, thread_buffer_size, enable_tsc, psb_period); } -Expected<std::vector<uint8_t>> -TraceIntelPT::GetLiveThreadBuffer(lldb::tid_t tid) { - return Trace::GetLiveThreadBinaryData(tid, "threadTraceBuffer"); +Error TraceIntelPT::OnThreadBufferRead( + lldb::tid_t tid, + std::function<llvm::Error(llvm::MutableArrayRef<uint8_t> callback)> + callback) { + return OnThreadBinaryDataRead(tid, "threadTraceBuffer", callback); } Index: lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.h =================================================================== --- lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.h +++ lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.h @@ -19,12 +19,15 @@ namespace lldb_private { namespace trace_intel_pt { -/// Base class that handles the decoding of a thread and caches the result. +/// Class that handles the decoding of a thread and caches the result. class ThreadDecoder { public: - virtual ~ThreadDecoder() = default; - - ThreadDecoder() = default; + /// \param[in] thread_sp + /// The thread whose trace buffer will be decoded. + /// + /// \param[in] trace + /// The main Trace object who owns this decoder and its data. + ThreadDecoder(const lldb::ThreadSP &thread_sp, TraceIntelPT &trace); /// Decode the thread and store the result internally, to avoid /// recomputations. @@ -36,49 +39,12 @@ ThreadDecoder(const ThreadDecoder &other) = delete; ThreadDecoder &operator=(const ThreadDecoder &other) = delete; -protected: - /// Decode the thread. - /// - /// \return - /// A \a DecodedThread instance. - virtual DecodedThreadSP DoDecode() = 0; - - llvm::Optional<DecodedThreadSP> m_decoded_thread; -}; - -/// Decoder implementation for \a lldb_private::ThreadPostMortemTrace, which are -/// non-live processes that come trace session files. -class PostMortemThreadDecoder : public ThreadDecoder { -public: - /// \param[in] trace_thread - /// The thread whose trace file will be decoded. - /// - /// \param[in] trace - /// The main Trace object who owns this decoder and its data. - PostMortemThreadDecoder(const lldb::ThreadPostMortemTraceSP &trace_thread, - TraceIntelPT &trace); - private: - DecodedThreadSP DoDecode() override; - - lldb::ThreadPostMortemTraceSP m_trace_thread; - TraceIntelPT &m_trace; -}; - -class LiveThreadDecoder : public ThreadDecoder { -public: - /// \param[in] thread - /// The thread whose traces will be decoded. - /// - /// \param[in] trace - /// The main Trace object who owns this decoder and its data. - LiveThreadDecoder(Thread &thread, TraceIntelPT &trace); - -private: - DecodedThreadSP DoDecode() override; + DecodedThreadSP DoDecode(); lldb::ThreadSP m_thread_sp; TraceIntelPT &m_trace; + llvm::Optional<DecodedThreadSP> m_decoded_thread; }; } // namespace trace_intel_pt Index: lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.cpp =================================================================== --- lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.cpp +++ lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.cpp @@ -20,7 +20,8 @@ using namespace lldb_private::trace_intel_pt; using namespace llvm; -// ThreadDecoder ==================== +ThreadDecoder::ThreadDecoder(const ThreadSP &thread_sp, TraceIntelPT &trace) + : m_thread_sp(thread_sp), m_trace(trace) {} DecodedThreadSP ThreadDecoder::Decode() { if (!m_decoded_thread.hasValue()) @@ -28,53 +29,16 @@ return *m_decoded_thread; } -// LiveThreadDecoder ==================== - -LiveThreadDecoder::LiveThreadDecoder(Thread &thread, TraceIntelPT &trace) - : m_thread_sp(thread.shared_from_this()), m_trace(trace) {} - -DecodedThreadSP LiveThreadDecoder::DoDecode() { +DecodedThreadSP ThreadDecoder::DoDecode() { DecodedThreadSP decoded_thread_sp = std::make_shared<DecodedThread>(m_thread_sp); - Expected<std::vector<uint8_t>> buffer = - m_trace.GetLiveThreadBuffer(m_thread_sp->GetID()); - if (!buffer) { - decoded_thread_sp->AppendError(buffer.takeError()); - return decoded_thread_sp; - } - - decoded_thread_sp->SetRawTraceSize(buffer->size()); - DecodeInMemoryTrace(*decoded_thread_sp, m_trace, - MutableArrayRef<uint8_t>(*buffer)); - return decoded_thread_sp; -} - -// PostMortemThreadDecoder ======================= - -PostMortemThreadDecoder::PostMortemThreadDecoder( - const lldb::ThreadPostMortemTraceSP &trace_thread, TraceIntelPT &trace) - : m_trace_thread(trace_thread), m_trace(trace) {} - -DecodedThreadSP PostMortemThreadDecoder::DoDecode() { - DecodedThreadSP decoded_thread_sp = - std::make_shared<DecodedThread>(m_trace_thread); - - ErrorOr<std::unique_ptr<MemoryBuffer>> trace_or_error = - MemoryBuffer::getFile(m_trace_thread->GetTraceFile().GetPath()); - if (std::error_code err = trace_or_error.getError()) { - decoded_thread_sp->AppendError(errorCodeToError(err)); - return decoded_thread_sp; - } - - MemoryBuffer &trace = **trace_or_error; - MutableArrayRef<uint8_t> trace_data( - // The libipt library does not modify the trace buffer, hence the - // following cast is safe. - reinterpret_cast<uint8_t *>(const_cast<char *>(trace.getBufferStart())), - trace.getBufferSize()); - decoded_thread_sp->SetRawTraceSize(trace_data.size()); - - DecodeInMemoryTrace(*decoded_thread_sp, m_trace, trace_data); + Error err = m_trace.OnThreadBufferRead( + m_thread_sp->GetID(), [&](llvm::MutableArrayRef<uint8_t> data) { + DecodeInMemoryTrace(*decoded_thread_sp, m_trace, data); + return Error::success(); + }); + if (err) + decoded_thread_sp->AppendError(std::move(err)); return decoded_thread_sp; } Index: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp =================================================================== --- lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp +++ lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp @@ -288,6 +288,8 @@ void lldb_private::trace_intel_pt::DecodeInMemoryTrace( DecodedThread &decoded_thread, TraceIntelPT &trace_intel_pt, MutableArrayRef<uint8_t> buffer) { + decoded_thread.SetRawTraceSize(buffer.size()); + Expected<PtInsnDecoderUP> decoder_up = CreateInstructionDecoder(decoded_thread, trace_intel_pt, buffer); if (!decoder_up) Index: lldb/source/Plugins/Trace/common/TraceSessionSaver.h =================================================================== --- lldb/source/Plugins/Trace/common/TraceSessionSaver.h +++ lldb/source/Plugins/Trace/common/TraceSessionSaver.h @@ -39,11 +39,8 @@ /// \param[in] live_process /// The process being traced. /// - /// \param[in] raw_trace_fetcher - /// Callback function that receives a thread ID and returns its raw trace. - /// This callback should return \a None if the thread is not being traced. - /// Otherwise, it should return the raw trace in bytes or an - /// \a llvm::Error in case of failures. + /// \param[in] raw_thread_trace_data_kind + /// Identifier for the data kind of the raw trace for each thread. /// /// \param[in] directory /// The directory where files will be saved when building the processes @@ -51,12 +48,10 @@ /// /// \return /// The processes section or \a llvm::Error in case of failures. - static llvm::Expected<JSONTraceSessionBase> BuildProcessesSection( - Process &live_process, - std::function< - llvm::Expected<llvm::Optional<std::vector<uint8_t>>>(lldb::tid_t tid)> - raw_trace_fetcher, - FileSpec directory); + static llvm::Expected<JSONTraceSessionBase> + BuildProcessesSection(Process &live_process, + llvm::StringRef raw_thread_trace_data_kind, + FileSpec directory); /// Build the threads sub-section of the trace session description file. /// For each traced thread, its raw trace is also written to the file @@ -65,11 +60,8 @@ /// \param[in] live_process /// The process being traced. /// - /// \param[in] raw_trace_fetcher - /// Callback function that receives a thread ID and returns its raw trace. - /// This callback should return \a None if the thread is not being traced. - /// Otherwise, it should return the raw trace in bytes or an - /// \a llvm::Error in case of failures. + /// \param[in] raw_thread_trace_data_kind + /// Identifier for the data kind of the raw trace for each thread. /// /// \param[in] directory /// The directory where files will be saved when building the threads @@ -77,12 +69,10 @@ /// /// \return /// The threads section or \a llvm::Error in case of failures. - static llvm::Expected<std::vector<JSONThread>> BuildThreadsSection( - Process &live_process, - std::function< - llvm::Expected<llvm::Optional<std::vector<uint8_t>>>(lldb::tid_t tid)> - raw_trace_fetcher, - FileSpec directory); + static llvm::Expected<std::vector<JSONThread>> + BuildThreadsSection(Process &live_process, + llvm::StringRef raw_thread_trace_data_kind, + FileSpec directory); /// Build modules sub-section of the trace's session. The original modules /// will be copied over to the \a <directory/modules> folder. Invalid modules Index: lldb/source/Plugins/Trace/common/TraceSessionSaver.cpp =================================================================== --- lldb/source/Plugins/Trace/common/TraceSessionSaver.cpp +++ lldb/source/Plugins/Trace/common/TraceSessionSaver.cpp @@ -39,15 +39,12 @@ } llvm::Expected<JSONTraceSessionBase> TraceSessionSaver::BuildProcessesSection( - Process &live_process, - std::function< - llvm::Expected<llvm::Optional<std::vector<uint8_t>>>(lldb::tid_t tid)> - raw_trace_fetcher, + Process &live_process, llvm::StringRef raw_thread_trace_data_kind, FileSpec directory) { JSONTraceSessionBase json_session_description; Expected<std::vector<JSONThread>> json_threads = - BuildThreadsSection(live_process, raw_trace_fetcher, directory); + BuildThreadsSection(live_process, raw_thread_trace_data_kind, directory); if (!json_threads) return json_threads.takeError(); @@ -64,39 +61,41 @@ } llvm::Expected<std::vector<JSONThread>> TraceSessionSaver::BuildThreadsSection( - Process &live_process, - std::function< - llvm::Expected<llvm::Optional<std::vector<uint8_t>>>(lldb::tid_t tid)> - raw_trace_fetcher, + Process &live_process, llvm::StringRef raw_thread_trace_data_kind, FileSpec directory) { std::vector<JSONThread> json_threads; for (ThreadSP thread_sp : live_process.Threads()) { + TraceSP trace_sp = live_process.GetTarget().GetTrace(); + lldb::tid_t tid = thread_sp->GetID(); + if (!trace_sp->IsTraced(tid)) + continue; + // resolve the directory just in case FileSystem::Instance().Resolve(directory); FileSpec raw_trace_path = directory; - raw_trace_path.AppendPathComponent(std::to_string(thread_sp->GetID()) + - ".trace"); - json_threads.push_back(JSONThread{static_cast<int64_t>(thread_sp->GetID()), + raw_trace_path.AppendPathComponent(std::to_string(tid) + ".trace"); + json_threads.push_back(JSONThread{static_cast<int64_t>(tid), raw_trace_path.GetPath().c_str()}); - llvm::Expected<llvm::Optional<std::vector<uint8_t>>> raw_trace = - raw_trace_fetcher(thread_sp->GetID()); - - if (!raw_trace) - return raw_trace.takeError(); - if (!raw_trace.get()) - continue; - - std::basic_fstream<char> raw_trace_fs = std::fstream( - raw_trace_path.GetPath().c_str(), std::ios::out | std::ios::binary); - raw_trace_fs.write(reinterpret_cast<const char *>(&raw_trace.get()->at(0)), - raw_trace.get()->size() * sizeof(uint8_t)); - raw_trace_fs.close(); - if (!raw_trace_fs) { - return createStringError(inconvertibleErrorCode(), - formatv("couldn't write to the file {0}", - raw_trace_path.GetPath().c_str())); - } + llvm::Error err = + live_process.GetTarget().GetTrace()->OnThreadBinaryDataRead( + tid, raw_thread_trace_data_kind, + [&](llvm::MutableArrayRef<uint8_t> data) -> llvm::Error { + std::basic_fstream<char> raw_trace_fs = + std::fstream(raw_trace_path.GetPath().c_str(), + std::ios::out | std::ios::binary); + raw_trace_fs.write(reinterpret_cast<const char *>(&data[0]), + data.size() * sizeof(uint8_t)); + raw_trace_fs.close(); + if (!raw_trace_fs) + return createStringError( + inconvertibleErrorCode(), + formatv("couldn't write to the file {0}", + raw_trace_path.GetPath().c_str())); + return Error::success(); + }); + if (err) + return std::move(err); } return json_threads; } Index: lldb/include/lldb/Target/Trace.h =================================================================== --- lldb/include/lldb/Target/Trace.h +++ lldb/include/lldb/Target/Trace.h @@ -233,15 +233,76 @@ /// \a llvm::Error otherwise. llvm::Error Stop(); - /// Get the trace file of the given post mortem thread. - llvm::Expected<const FileSpec &> GetPostMortemTraceFile(lldb::tid_t tid); - /// \return /// The stop ID of the live process being traced, or an invalid stop ID /// if the trace is in an error or invalid state. uint32_t GetStopID(); + /// Fetch binary data associated with a thread, either live or postmortem, and + /// pass it to the given callback. The reason of having a callback is to free + /// the caller from having to manage the life cycle of the data and to hide + /// the different data fetching procedures that exist for live and post mortem + /// threads. + /// + /// The fetched data is not persisted after the callback is invoked. + /// + /// \param[in] tid + /// The tid who owns the data. + /// + /// \param[in] kind + /// The kind of data to read. + /// + /// \param[in] callback + /// The callback to be invoked once the data was successfully read. Its + /// return value, which is an \a llvm::Error, is returned by this + /// function. + /// + /// \return + /// An \a llvm::Error if the data couldn't be fetched, or the return value + /// of the callback, otherwise. + llvm::Error OnThreadBinaryDataRead( + lldb::tid_t tid, llvm::StringRef kind, + std::function<llvm::Error(llvm::MutableArrayRef<uint8_t> data)> callback); + protected: + /// Implementation of \a OnThreadBinaryDataRead() for live threads. + llvm::Error OnLiveThreadBinaryDataRead( + lldb::tid_t tid, llvm::StringRef kind, + std::function<llvm::Error(llvm::MutableArrayRef<uint8_t> data)> callback); + + /// Implementation of \a OnThreadBinaryDataRead() for post mortem threads. + llvm::Error OnPostMortemThreadBinaryDataRead( + lldb::tid_t tid, llvm::StringRef kind, + std::function<llvm::Error(llvm::MutableArrayRef<uint8_t> data)> callback); + + /// Get the file path containing data of a postmortem thread given a data + /// identifier. + /// + /// \param[in] tid + /// The thread whose data is requested. + /// + /// \param[in] kind + /// The kind of data requested. + /// + /// \return + /// The file spec containing the requested data, or an \a llvm::Error in + /// case of failures. + llvm::Expected<FileSpec> GetPostMortemThreadDataFile(lldb::tid_t tid, + llvm::StringRef kind); + + /// Associate a given thread with a data file using a data identifier. + /// + /// \param[in] tid + /// The thread associated with the data file. + /// + /// \param[in] kind + /// The kind of data being registered. + /// + /// \param[in] file_spec + /// The path of the data file. + void SetPostMortemThreadDataFile(lldb::tid_t tid, llvm::StringRef kind, + FileSpec file_spec); + /// Get binary data of a live thread given a data identifier. /// /// \param[in] tid @@ -320,6 +381,9 @@ m_live_thread_data; /// data kind -> size std::unordered_map<std::string, size_t> m_live_process_data; + /// tid -> data kind -> file + std::map<lldb::tid_t, std::unordered_map<std::string, FileSpec>> + m_postmortem_thread_data; }; } // namespace lldb_private
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits