diff --git a/lldb/docs/lldb-gdb-remote.txt b/lldb/docs/lldb-gdb-remote.txt --- a/lldb/docs/lldb-gdb-remote.txt +++ b/lldb/docs/lldb-gdb-remote.txt @@ -562,10 +562,6 @@ // Core id in decimal if the data belongs to a CPU core. // "tid"?: , // Tid in decimal if the data belongs to a thread. -// "offset": , -// Offset of the data in bytes. -// "size": , -// Number of bytes in to read starting from the offset. // } // // INTEL PT diff --git a/lldb/include/lldb/Target/Trace.h b/lldb/include/lldb/Target/Trace.h --- a/lldb/include/lldb/Target/Trace.h +++ b/lldb/include/lldb/Target/Trace.h @@ -327,6 +327,12 @@ /// If it's not a live process session, return an empty list. llvm::ArrayRef GetPostMortemProcesses(); + /// Dispatcher for live trace data requests with some additional error + /// checking. + llvm::Expected> + GetLiveTraceBinaryData(const TraceGetBinaryDataRequest &request, + uint64_t expected_size); + /// Implementation of \a OnThreadBinaryDataRead() for live threads. llvm::Error OnLiveThreadBinaryDataRead(lldb::tid_t tid, llvm::StringRef kind, OnBinaryDataReadCallback callback); @@ -532,19 +538,19 @@ /// \{ /// tid -> data kind -> size - llvm::DenseMap> + llvm::DenseMap> live_thread_data; /// core id -> data kind -> size - llvm::DenseMap> + llvm::DenseMap> live_core_data_sizes; /// core id -> data kind -> bytes llvm::DenseMap>> + llvm::DenseMap>> live_core_data; /// data kind -> size - std::unordered_map live_process_data; + llvm::DenseMap live_process_data; /// \} /// The list of cores being traced. Might be \b None depending on the @@ -556,11 +562,11 @@ /// \{ /// tid -> data kind -> file - llvm::DenseMap> + llvm::DenseMap> postmortem_thread_data; /// core id -> data kind -> file - llvm::DenseMap> + llvm::DenseMap> postmortem_core_data; /// \} diff --git a/lldb/include/lldb/Utility/TraceGDBRemotePackets.h b/lldb/include/lldb/Utility/TraceGDBRemotePackets.h --- a/lldb/include/lldb/Utility/TraceGDBRemotePackets.h +++ b/lldb/include/lldb/Utility/TraceGDBRemotePackets.h @@ -155,10 +155,6 @@ llvm::Optional tid; /// Optional core id if the data is related to a cpu core. llvm::Optional core_id; - /// Offset in bytes from where to start reading the data. - uint64_t offset; - /// Number of bytes to read. - uint64_t size; }; bool fromJSON(const llvm::json::Value &value, diff --git a/lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp b/lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp --- a/lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp +++ b/lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp @@ -158,9 +158,8 @@ formatv("Core {0} is not being traced", *request.core_id)); if (request.kind == IntelPTDataKinds::kTraceBuffer) - return it->second.first.GetTraceBuffer(request.offset, request.size); + return it->second.first.GetTraceBuffer(); if (request.kind == IntelPTDataKinds::kPerfContextSwitchTrace) - return it->second.second.ReadFlushedOutDataCyclicBuffer(request.offset, - request.size); + return it->second.second.GetReadOnlyDataBuffer(); return None; } diff --git a/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h b/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h --- a/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h +++ b/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h @@ -64,18 +64,9 @@ /// underlying perf_event is paused during read, and later it's returned to /// its initial state. /// - /// \param[in] offset - /// Offset of the data to read. - /// - /// \param[in] size - /// Number of bytes to read. - /// /// \return - /// A vector with the requested binary data. The vector will have the - /// size of the requested \a size. Non-available positions will be - /// filled with zeroes. - llvm::Expected> GetTraceBuffer(size_t offset, - size_t size); + /// A vector with the requested binary data. + llvm::Expected> GetTraceBuffer(); /// \return /// The total the size in bytes used by the trace buffer managed by this diff --git a/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp b/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp --- a/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp +++ b/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp @@ -213,8 +213,7 @@ return m_perf_event.EnableWithIoctl(); } -Expected> -IntelPTSingleBufferTrace::GetTraceBuffer(size_t offset, size_t size) { +Expected> IntelPTSingleBufferTrace::GetTraceBuffer() { // Disable the perf event to force a flush out of the CPU's internal buffer. // Besides, we can guarantee that the CPU won't override any data as we are // reading the buffer. @@ -229,7 +228,7 @@ // // This is achieved by the PERF_EVENT_IOC_DISABLE ioctl request, as // mentioned in the man page of perf_event_open. - return m_perf_event.ReadFlushedOutAuxCyclicBuffer(offset, size); + return m_perf_event.GetReadOnlyAuxBuffer(); } Expected diff --git a/lldb/source/Plugins/Process/Linux/IntelPTThreadTraceCollection.cpp b/lldb/source/Plugins/Process/Linux/IntelPTThreadTraceCollection.cpp --- a/lldb/source/Plugins/Process/Linux/IntelPTThreadTraceCollection.cpp +++ b/lldb/source/Plugins/Process/Linux/IntelPTThreadTraceCollection.cpp @@ -85,7 +85,7 @@ if (Expected trace = GetTracedThread(*request.tid)) - return trace->GetTraceBuffer(request.offset, request.size); + return trace->GetTraceBuffer(); else return trace.takeError(); } diff --git a/lldb/source/Plugins/Process/Linux/Perf.h b/lldb/source/Plugins/Process/Linux/Perf.h --- a/lldb/source/Plugins/Process/Linux/Perf.h +++ b/lldb/source/Plugins/Process/Linux/Perf.h @@ -200,43 +200,29 @@ /// \a ArrayRef extending \a aux_size bytes from \a aux_offset. llvm::ArrayRef GetAuxBuffer() const; - /// Read the aux buffer managed by this perf event. To ensure that the - /// data is up-to-date and is not corrupted by read-write race conditions, the - /// underlying perf_event is paused during read, and later it's returned to - /// its initial state. The returned data will be linear, i.e. it will fix the - /// circular wrapping the might exist int he buffer. - /// - /// \param[in] offset - /// Offset of the data to read. - /// - /// \param[in] size - /// Number of bytes to read. + /// Read the aux buffer managed by this perf event assuming it was configured + /// with PROT_READ permissions only, which indicates that the buffer is + /// automatically wrapped and overwritten by the kernel or hardware. To ensure + /// that the data is up-to-date and is not corrupted by read-write race + /// conditions, the underlying perf_event is paused during read, and later + /// it's returned to its initial state. The returned data will be linear, i.e. + /// it will fix the circular wrapping the might exist in the buffer. /// /// \return - /// A vector with the requested binary data. The vector will have the - /// size of the requested \a size. Non-available positions will be - /// filled with zeroes. - llvm::Expected> - ReadFlushedOutAuxCyclicBuffer(size_t offset, size_t size); + /// A vector with the requested binary data. + llvm::Expected> GetReadOnlyAuxBuffer(); - /// Read the data buffer managed by this perf event. To ensure that the - /// data is up-to-date and is not corrupted by read-write race conditions, the - /// underlying perf_event is paused during read, and later it's returned to - /// its initial state. The returned data will be linear, i.e. it will fix the - /// circular wrapping the might exist int he buffer. - /// - /// \param[in] offset - /// Offset of the data to read. - /// - /// \param[in] size - /// Number of bytes to read. + /// Read the data buffer managed by this perf even assuming it was configured + /// with PROT_READ permissions only, which indicates that the buffer is + /// automatically wrapped and overwritten by the kernel or hardware. To ensure + /// that the data is up-to-date and is not corrupted by read-write race + /// conditions, the underlying perf_event is paused during read, and later + /// it's returned to its initial state. The returned data will be linear, i.e. + /// it will fix the circular wrapping the might exist int he buffer. /// /// \return - /// A vector with the requested binary data. The vector will have the - /// size of the requested \a size. Non-available positions will be - /// filled with zeroes. - llvm::Expected> - ReadFlushedOutDataCyclicBuffer(size_t offset, size_t size); + /// A vector with the requested binary data. + llvm::Expected> GetReadOnlyDataBuffer(); /// Use the ioctl API to disable the perf event and all the events in its /// group. This doesn't terminate the perf event. diff --git a/lldb/source/Plugins/Process/Linux/Perf.cpp b/lldb/source/Plugins/Process/Linux/Perf.cpp --- a/lldb/source/Plugins/Process/Linux/Perf.cpp +++ b/lldb/source/Plugins/Process/Linux/Perf.cpp @@ -178,8 +178,7 @@ static_cast(mmap_metadata.aux_size)}; } -Expected> -PerfEvent::ReadFlushedOutDataCyclicBuffer(size_t offset, size_t size) { +Expected> PerfEvent::GetReadOnlyDataBuffer() { // The following code assumes that the protection level of the DATA page // is PROT_READ. If PROT_WRITE is used, then reading would require that // this piece of code updates some pointers. See more about data_tail @@ -191,8 +190,8 @@ /** * The data buffer and aux buffer have different implementations - * with respect to their definition of head pointer. In the case - * of Aux data buffer the head always wraps around the aux buffer + * with respect to their definition of head pointer when using PROD_READ only. + * In the case of Aux data buffer the head always wraps around the aux buffer * and we don't need to care about it, whereas the data_head keeps * increasing and needs to be wrapped by modulus operator */ @@ -202,25 +201,18 @@ uint64_t data_head = mmap_metadata.data_head; uint64_t data_size = mmap_metadata.data_size; std::vector output; - output.reserve(size); + output.reserve(data.size()); if (data_head > data_size) { uint64_t actual_data_head = data_head % data_size; // The buffer has wrapped - for (uint64_t i = actual_data_head + offset; - i < data_size && output.size() < size; i++) + for (uint64_t i = actual_data_head; i < data_size; i++) output.push_back(data[i]); - // We need to find the starting position for the left part if the offset was - // too big - uint64_t left_part_start = actual_data_head + offset >= data_size - ? actual_data_head + offset - data_size - : 0; - for (uint64_t i = left_part_start; - i < actual_data_head && output.size() < size; i++) + for (uint64_t i = 0; i < actual_data_head; i++) output.push_back(data[i]); } else { - for (uint64_t i = offset; i < data_head && output.size() < size; i++) + for (uint64_t i = 0; i < data_head; i++) output.push_back(data[i]); } @@ -229,17 +221,10 @@ return std::move(err); } - if (output.size() != size) - return createStringError(inconvertibleErrorCode(), - formatv("Requested {0} bytes of perf_event data " - "buffer but only {1} are available", - size, output.size())); - return output; } -Expected> -PerfEvent::ReadFlushedOutAuxCyclicBuffer(size_t offset, size_t size) { +Expected> PerfEvent::GetReadOnlyAuxBuffer() { // The following code assumes that the protection level of the AUX page // is PROT_READ. If PROT_WRITE is used, then reading would require that // this piece of code updates some pointers. See more about aux_tail @@ -255,7 +240,7 @@ uint64_t aux_head = mmap_metadata.aux_head; uint64_t aux_size = mmap_metadata.aux_size; std::vector output; - output.reserve(size); + output.reserve(data.size()); /** * When configured as ring buffer, the aux buffer keeps wrapping around @@ -269,15 +254,10 @@ * * */ - for (uint64_t i = aux_head + offset; i < aux_size && output.size() < size; - i++) + for (uint64_t i = aux_head; i < aux_size; i++) output.push_back(data[i]); - // We need to find the starting position for the left part if the offset was - // too big - uint64_t left_part_start = - aux_head + offset >= aux_size ? aux_head + offset - aux_size : 0; - for (uint64_t i = left_part_start; i < aux_head && output.size() < size; i++) + for (uint64_t i = 0; i < aux_head; i++) output.push_back(data[i]); if (was_enabled) { @@ -285,12 +265,6 @@ return std::move(err); } - if (output.size() != size) - return createStringError(inconvertibleErrorCode(), - formatv("Requested {0} bytes of perf_event aux " - "buffer but only {1} are available", - size, output.size())); - return output; } diff --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h --- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h +++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h @@ -170,7 +170,7 @@ /// \param[in] session /// The definition file for the postmortem session. /// - /// \param[in] traces_proceses + /// \param[in] traced_processes /// The processes traced in the live session. /// /// \param[in] trace_threads @@ -202,7 +202,7 @@ struct Storage { llvm::Optional multicore_decoder; /// These decoders are used for the non-per-core case - std::map> thread_decoders; + llvm::DenseMap> thread_decoders; /// Helper variable used to track long running operations for telemetry. TaskTimer task_timer; /// It is provided by either a session file or a live process to convert TSC diff --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp --- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp +++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp @@ -102,7 +102,7 @@ m_storage.multicore_decoder.emplace(*this); } else { for (const ThreadPostMortemTraceSP &thread : traced_threads) { - m_storage.thread_decoders.emplace( + m_storage.thread_decoders.try_emplace( thread->GetID(), std::make_unique(thread, *this)); if (const Optional &trace_file = thread->GetTraceFile()) { SetPostMortemThreadDataFile( @@ -323,7 +323,7 @@ Error TraceIntelPT::DoRefreshLiveProcessState(TraceGetStateResponse state, StringRef json_response) { - m_storage = {}; + m_storage = Storage(); Expected intelpt_state = json::parse(json_response, @@ -337,7 +337,7 @@ for (const TraceThreadState &thread_state : state.traced_threads) { ThreadSP thread_sp = GetLiveProcess()->GetThreadList().FindThreadByID(thread_state.tid); - m_storage.thread_decoders.emplace( + m_storage.thread_decoders.try_emplace( thread_state.tid, std::make_unique(thread_sp, *this)); } } else { diff --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp --- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp +++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp @@ -87,9 +87,9 @@ bool fromJSON(const json::Value &value, JSONCore &core, Path path) { ObjectMapper o(value, path); uint64_t core_id; - if (!o || !o.map("coreId", core_id) || - !o.map("traceBuffer", core.trace_buffer) || - !o.map("contextSwitchTrace", core.context_switch_trace)) + if (!(o && o.map("coreId", core_id) && + o.map("traceBuffer", core.trace_buffer) && + o.map("contextSwitchTrace", core.context_switch_trace))) return false; core.core_id = core_id; return true; @@ -108,8 +108,8 @@ ObjectMapper o(value, path); std::string vendor; uint64_t family, model, stepping; - if (!o || !o.map("vendor", vendor) || !o.map("family", family) || - !o.map("model", model) || !o.map("stepping", stepping)) + if (!(o && o.map("vendor", vendor) && o.map("family", family) && + o.map("model", model) && o.map("stepping", stepping))) return false; cpu_info.vendor = vendor == "GenuineIntel" ? pcv_intel : pcv_unknown; cpu_info.family = family; @@ -130,9 +130,9 @@ bool fromJSON(const json::Value &value, JSONTraceSession &session, Path path) { ObjectMapper o(value, path); - if (!o || !o.map("processes", session.processes) || - !o.map("type", session.type) || !o.map("cores", session.cores) || - !o.map("tscPerfZeroConversion", session.tsc_perf_zero_conversion)) + if (!(o && o.map("processes", session.processes) && + o.map("type", session.type) && o.map("cores", session.cores) && + o.map("tscPerfZeroConversion", session.tsc_perf_zero_conversion))) return false; if (session.cores && !session.tsc_perf_zero_conversion) { path.report( diff --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h --- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h +++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h @@ -52,17 +52,21 @@ /// errors, return a null pointer. llvm::Expected Parse(); - llvm::Expected - CreateTraceIntelPTInstance(JSONTraceSession &session, - std::vector &parsed_processes); - private: /// Resolve non-absolute paths relative to the session file folder. FileSpec NormalizePath(const std::string &path); - lldb::ThreadPostMortemTraceSP ParseThread(lldb::ProcessSP &process_sp, + /// Create a post-mortem thread associated with the given \p process + /// using the definition from \p thread. + lldb::ThreadPostMortemTraceSP ParseThread(Process &process, const JSONThread &thread); + /// Given a session description and a list of fully parsed processes, + /// create an actual Trace instance that "traces" these processes. + llvm::Expected + CreateTraceIntelPTInstance(JSONTraceSession &session, + std::vector &parsed_processes); + /// Create the corresponding Threads and Process objects given the JSON /// process definition. /// @@ -70,7 +74,9 @@ /// The JSON process definition llvm::Expected ParseProcess(const JSONProcess &process); - llvm::Error ParseModule(lldb::TargetSP &target_sp, const JSONModule &module); + /// Create a moddule associated with the given \p target + /// using the definition from \p module. + llvm::Error ParseModule(Target &target, const JSONModule &module); /// Create a user-friendly error message upon a JSON-parsing failure using the /// \a json::ObjectMapper functionality. @@ -106,7 +112,7 @@ Debugger &m_debugger; const llvm::json::Value &m_trace_session_file; - std::string m_session_file_dir; + const std::string m_session_file_dir; }; } // namespace trace_intel_pt diff --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp --- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp +++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp @@ -29,7 +29,7 @@ return file_spec; } -Error TraceIntelPTSessionFileParser::ParseModule(lldb::TargetSP &target_sp, +Error TraceIntelPTSessionFileParser::ParseModule(Target &target, const JSONModule &module) { auto do_parse = [&]() -> Error { FileSpec system_file_spec(module.system_path); @@ -46,13 +46,13 @@ Status error; ModuleSP module_sp = - target_sp->GetOrCreateModule(module_spec, /*notify*/ false, &error); + target.GetOrCreateModule(module_spec, /*notify*/ false, &error); if (error.Fail()) return error.ToError(); bool load_addr_changed = false; - module_sp->SetLoadAddress(*target_sp, module.load_address, false, + module_sp->SetLoadAddress(target, module.load_address, false, load_addr_changed); return Error::success(); }; @@ -74,7 +74,7 @@ } ThreadPostMortemTraceSP -TraceIntelPTSessionFileParser::ParseThread(ProcessSP &process_sp, +TraceIntelPTSessionFileParser::ParseThread(Process &process, const JSONThread &thread) { lldb::tid_t tid = static_cast(thread.tid); @@ -83,8 +83,8 @@ trace_file = FileSpec(*thread.trace_buffer); ThreadPostMortemTraceSP thread_sp = - std::make_shared(*process_sp, tid, trace_file); - process_sp->GetThreadList().AddThread(thread_sp); + std::make_shared(process, tid, trace_file); + process.GetThreadList().AddThread(thread_sp); return thread_sp; } @@ -110,10 +110,10 @@ process_sp->SetID(static_cast(process.pid)); for (const JSONThread &thread : process.threads) - parsed_process.threads.push_back(ParseThread(process_sp, thread)); + parsed_process.threads.push_back(ParseThread(*process_sp, thread)); for (const JSONModule &module : process.modules) - if (Error err = ParseModule(target_sp, module)) + if (Error err = ParseModule(*target_sp, module)) return std::move(err); if (!process.threads.empty()) diff --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp --- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp +++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp @@ -30,6 +30,8 @@ using namespace lldb_private::trace_intel_pt; using namespace llvm; +/// Write a stream of bytes from \p data to the given output file. +/// It creates or overwrites the output file, but not append. static llvm::Error WriteBytesToDisk(FileSpec &output_file, ArrayRef data) { std::basic_fstream out_fs = std::fstream( @@ -63,7 +65,7 @@ FileSpec trace_path = directory; trace_path.AppendPathComponent("trace.json"); std::ofstream os(trace_path.GetPath()); - os << std::string(formatv("{0:2}", trace_session_json)); + os << formatv("{0:2}", trace_session_json).str(); os.close(); if (!os) return createStringError(inconvertibleErrorCode(), @@ -91,7 +93,6 @@ FileSpec threads_dir = directory; threads_dir.AppendPathComponent("threads"); - FileSystem::Instance().Resolve(threads_dir); sys::fs::create_directories(threads_dir.GetCString()); for (ThreadSP thread_sp : process.Threads()) { @@ -129,7 +130,6 @@ std::vector json_cores; FileSpec cores_dir = directory; cores_dir.AppendPathComponent("cores"); - FileSystem::Instance().Resolve(cores_dir); sys::fs::create_directories(cores_dir.GetCString()); for (lldb::core_id_t core_id : trace_ipt.GetTracedCores()) { @@ -217,7 +217,6 @@ if (load_addr == LLDB_INVALID_ADDRESS) continue; - FileSystem::Instance().Resolve(directory); FileSpec path_to_copy_module = directory; path_to_copy_module.AppendPathComponent("modules"); path_to_copy_module.AppendPathComponent(system_path); @@ -291,6 +290,8 @@ if (!cpu_info) return cpu_info.takeError(); + FileSystem::Instance().Resolve(directory); + Expected> json_processes = BuildProcessesSection(trace_ipt, directory); diff --git a/lldb/source/Target/Trace.cpp b/lldb/source/Target/Trace.cpp --- a/lldb/source/Target/Trace.cpp +++ b/lldb/source/Target/Trace.cpp @@ -42,6 +42,44 @@ } // namespace json } // namespace llvm +/// Helper functions for fetching data in maps and returning Optionals or +/// pointers instead of iterators for simplicity. It's worth mentioning that the +/// Optionals version can't return the inner data by reference because of +/// limitations in move constructors. +/// \{ +template +static Optional Lookup(DenseMap &map, K k) { + auto it = map.find(k); + if (it == map.end()) + return None; + return it->second; +} + +template +static V *LookupAsPtr(DenseMap &map, K k) { + auto it = map.find(k); + if (it == map.end()) + return nullptr; + return &it->second; +} + +template +static Optional Lookup2(DenseMap> &map, K1 k1, K2 k2) { + auto it = map.find(k1); + if (it == map.end()) + return None; + return Lookup(it->second, k2); +} + +template +static V *Lookup2AsPtr(DenseMap> &map, K1 k1, K2 k2) { + auto it = map.find(k1); + if (it == map.end()) + return nullptr; + return LookupAsPtr(it->second, k2); +} +/// \} + static Error createInvalidPlugInError(StringRef plugin_name) { return createStringError( std::errc::invalid_argument, @@ -88,71 +126,82 @@ Error Trace::Start(const llvm::json::Value &request) { if (!m_live_process) - return createStringError(inconvertibleErrorCode(), - "Tracing requires a live process."); + return createStringError( + inconvertibleErrorCode(), + "Attempted to start tracing without a live process."); return m_live_process->TraceStart(request); } Error Trace::Stop() { if (!m_live_process) - return createStringError(inconvertibleErrorCode(), - "Tracing requires a live process."); + return createStringError( + inconvertibleErrorCode(), + "Attempted to stop tracing without a live process."); return m_live_process->TraceStop(TraceStopRequest(GetPluginName())); } Error Trace::Stop(llvm::ArrayRef tids) { if (!m_live_process) - return createStringError(inconvertibleErrorCode(), - "Tracing requires a live process."); + return createStringError( + inconvertibleErrorCode(), + "Attempted to stop tracing without a live process."); return m_live_process->TraceStop(TraceStopRequest(GetPluginName(), tids)); } Expected Trace::GetLiveProcessState() { if (!m_live_process) - return createStringError(inconvertibleErrorCode(), - "Tracing requires a live process."); + return createStringError( + inconvertibleErrorCode(), + "Attempted to fetch live trace information without a live process."); return m_live_process->TraceGetState(GetPluginName()); } Optional Trace::GetLiveThreadBinaryDataSize(lldb::tid_t tid, llvm::StringRef kind) { Storage &storage = GetUpdatedStorage(); - auto it = storage.live_thread_data.find(tid); - if (it == storage.live_thread_data.end()) - return None; - std::unordered_map &single_thread_data = it->second; - auto single_thread_data_it = single_thread_data.find(kind.str()); - if (single_thread_data_it == single_thread_data.end()) - return None; - return single_thread_data_it->second; + return Lookup2(storage.live_thread_data, tid, ConstString(kind)); } Optional Trace::GetLiveCoreBinaryDataSize(lldb::core_id_t core_id, llvm::StringRef kind) { Storage &storage = GetUpdatedStorage(); - auto it = storage.live_core_data_sizes.find(core_id); - if (it == storage.live_core_data_sizes.end()) - return None; - std::unordered_map &single_core_data = it->second; - auto single_thread_data_it = single_core_data.find(kind.str()); - if (single_thread_data_it == single_core_data.end()) - return None; - return single_thread_data_it->second; + return Lookup2(storage.live_core_data_sizes, core_id, ConstString(kind)); } Optional Trace::GetLiveProcessBinaryDataSize(llvm::StringRef kind) { Storage &storage = GetUpdatedStorage(); - auto data_it = storage.live_process_data.find(kind.str()); - if (data_it == storage.live_process_data.end()) - return None; - return data_it->second; + return Lookup(storage.live_process_data, ConstString(kind)); } Expected> -Trace::GetLiveThreadBinaryData(lldb::tid_t tid, llvm::StringRef kind) { +Trace::GetLiveTraceBinaryData(const TraceGetBinaryDataRequest &request, + uint64_t expected_size) { if (!m_live_process) - return createStringError(inconvertibleErrorCode(), - "Tracing requires a live process."); + return createStringError( + inconvertibleErrorCode(), + formatv("Attempted to fetch live trace data without a live process. " + "Data kind = {0}, tid = {1}, core id = {2}.", + request.kind, request.tid, request.core_id)); + + Expected> data = + m_live_process->TraceGetBinaryData(request); + + if (!data) + return data.takeError(); + + if (data->size() != expected_size) + return createStringError( + inconvertibleErrorCode(), + formatv("Got incomplete live trace data. Data kind = {0}, expected " + "size = {1}, actual size = {2}, tid = {3}, core id = {4}", + request.kind, expected_size, data->size(), request.tid, + request.core_id)); + + return data; +} + +Expected> +Trace::GetLiveThreadBinaryData(lldb::tid_t tid, llvm::StringRef kind) { llvm::Optional size = GetLiveThreadBinaryDataSize(tid, kind); if (!size) return createStringError( @@ -160,16 +209,17 @@ "Tracing data \"%s\" is not available for thread %" PRIu64 ".", kind.data(), tid); - TraceGetBinaryDataRequest request{GetPluginName().str(), kind.str(), tid, - /*core_id=*/None, /*offset=*/0, *size}; - return m_live_process->TraceGetBinaryData(request); + TraceGetBinaryDataRequest request{GetPluginName().str(), kind.str(), tid, + /*core_id=*/None}; + return GetLiveTraceBinaryData(request, *size); } Expected> Trace::GetLiveCoreBinaryData(lldb::core_id_t core_id, llvm::StringRef kind) { if (!m_live_process) - return createStringError(inconvertibleErrorCode(), - "Tracing requires a live process."); + return createStringError( + inconvertibleErrorCode(), + "Attempted to fetch live cpu data without a live process."); llvm::Optional size = GetLiveCoreBinaryDataSize(core_id, kind); if (!size) return createStringError( @@ -178,16 +228,12 @@ kind.data(), core_id); TraceGetBinaryDataRequest request{GetPluginName().str(), kind.str(), - /*tid=*/None, core_id, - /*offset=*/0, *size}; + /*tid=*/None, core_id}; return m_live_process->TraceGetBinaryData(request); } Expected> Trace::GetLiveProcessBinaryData(llvm::StringRef kind) { - if (!m_live_process) - return createStringError(inconvertibleErrorCode(), - "Tracing requires a live process."); llvm::Optional size = GetLiveProcessBinaryDataSize(kind); if (!size) return createStringError( @@ -195,9 +241,8 @@ "Tracing data \"%s\" is not available for the process.", kind.data()); TraceGetBinaryDataRequest request{GetPluginName().str(), kind.str(), - /*tid=*/None, /*core_id*/ None, - /*offset=*/0, *size}; - return m_live_process->TraceGetBinaryData(request); + /*tid=*/None, /*core_id*/ None}; + return GetLiveTraceBinaryData(request, *size); } Trace::Storage &Trace::GetUpdatedStorage() { @@ -219,53 +264,55 @@ m_stop_id = new_stop_id; m_storage = Trace::Storage(); - auto HandleError = [&](Error &&err) -> const char * { - m_storage.live_refresh_error = toString(std::move(err)); - return m_storage.live_refresh_error->c_str(); - }; - - Expected json_string = GetLiveProcessState(); - if (!json_string) - return HandleError(json_string.takeError()); + auto do_refresh = [&]() -> Error { + Expected json_string = GetLiveProcessState(); + if (!json_string) + return json_string.takeError(); - Expected live_process_state = - json::parse(*json_string, "TraceGetStateResponse"); - if (!live_process_state) - return HandleError(live_process_state.takeError()); + Expected live_process_state = + json::parse(*json_string, + "TraceGetStateResponse"); + if (!live_process_state) + return live_process_state.takeError(); - if (live_process_state->warnings) { - for (std::string &warning : *live_process_state->warnings) - LLDB_LOG(log, "== Warning when fetching the trace state: {0}", warning); - } - - for (const TraceThreadState &thread_state : - live_process_state->traced_threads) { - for (const TraceBinaryData &item : thread_state.binary_data) - m_storage.live_thread_data[thread_state.tid][item.kind] = item.size; - } + if (live_process_state->warnings) { + for (std::string &warning : *live_process_state->warnings) + LLDB_LOG(log, "== Warning when fetching the trace state: {0}", warning); + } - LLDB_LOG(log, "== Found {0} threads being traced", - live_process_state->traced_threads.size()); + for (const TraceThreadState &thread_state : + live_process_state->traced_threads) { + for (const TraceBinaryData &item : thread_state.binary_data) + m_storage.live_thread_data[thread_state.tid].insert( + {ConstString(item.kind), item.size}); + } - if (live_process_state->cores) { - m_storage.cores.emplace(); - for (const TraceCoreState &core_state : *live_process_state->cores) { - m_storage.cores->push_back(core_state.core_id); - for (const TraceBinaryData &item : core_state.binary_data) - m_storage.live_core_data_sizes[core_state.core_id][item.kind] = - item.size; + LLDB_LOG(log, "== Found {0} threads being traced", + live_process_state->traced_threads.size()); + + if (live_process_state->cores) { + m_storage.cores.emplace(); + for (const TraceCoreState &core_state : *live_process_state->cores) { + m_storage.cores->push_back(core_state.core_id); + for (const TraceBinaryData &item : core_state.binary_data) + m_storage.live_core_data_sizes[core_state.core_id].insert( + {ConstString(item.kind), item.size}); + } + LLDB_LOG(log, "== Found {0} cpu cores being traced", + live_process_state->cores->size()); } - LLDB_LOG(log, "== Found {0} cpu cores being traced", - live_process_state->cores->size()); - } + for (const TraceBinaryData &item : live_process_state->process_binary_data) + m_storage.live_process_data.insert({ConstString(item.kind), item.size}); - for (const TraceBinaryData &item : live_process_state->process_binary_data) - m_storage.live_process_data[item.kind] = item.size; + return DoRefreshLiveProcessState(std::move(*live_process_state), + *json_string); + }; - if (Error err = DoRefreshLiveProcessState(std::move(*live_process_state), - *json_string)) - return HandleError(std::move(err)); + if (Error err = do_refresh()) { + m_storage.live_refresh_error = toString(std::move(err)); + return m_storage.live_refresh_error->c_str(); + } return nullptr; } @@ -296,60 +343,42 @@ llvm::Expected Trace::GetPostMortemThreadDataFile(lldb::tid_t tid, llvm::StringRef kind) { - auto NotFoundError = [&]() { + Storage &storage = GetUpdatedStorage(); + if (Optional file = + Lookup2(storage.postmortem_thread_data, tid, ConstString(kind))) + return *file; + else return createStringError( inconvertibleErrorCode(), formatv("The thread with tid={0} doesn't have the tracing data {1}", tid, kind)); - }; - - Storage &storage = GetUpdatedStorage(); - auto it = storage.postmortem_thread_data.find(tid); - if (it == storage.postmortem_thread_data.end()) - return NotFoundError(); - - std::unordered_map &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; } llvm::Expected Trace::GetPostMortemCoreDataFile(lldb::core_id_t core_id, llvm::StringRef kind) { - auto NotFoundError = [&]() { + Storage &storage = GetUpdatedStorage(); + if (Optional file = + Lookup2(storage.postmortem_core_data, core_id, ConstString(kind))) + return *file; + else return createStringError( inconvertibleErrorCode(), formatv("The core with id={0} doesn't have the tracing data {1}", core_id, kind)); - }; - - Storage &storage = GetUpdatedStorage(); - auto it = storage.postmortem_core_data.find(core_id); - if (it == storage.postmortem_core_data.end()) - return NotFoundError(); - - std::unordered_map &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) { Storage &storage = GetUpdatedStorage(); - storage.postmortem_thread_data[tid][kind.str()] = file_spec; + storage.postmortem_thread_data[tid].insert({ConstString(kind), file_spec}); } void Trace::SetPostMortemCoreDataFile(lldb::core_id_t core_id, llvm::StringRef kind, FileSpec file_spec) { Storage &storage = GetUpdatedStorage(); - storage.postmortem_core_data[core_id][kind.str()] = file_spec; + storage.postmortem_core_data[core_id].insert({ConstString(kind), file_spec}); } llvm::Error @@ -365,18 +394,15 @@ llvm::StringRef kind, OnBinaryDataReadCallback callback) { Storage &storage = GetUpdatedStorage(); - auto core_data_entries = storage.live_core_data.find(core_id); - if (core_data_entries != storage.live_core_data.end()) { - auto core_data = core_data_entries->second.find(kind.str()); - if (core_data != core_data_entries->second.end()) - return callback(core_data->second); - } + if (std::vector *core_data = + Lookup2AsPtr(storage.live_core_data, core_id, ConstString(kind))) + return callback(*core_data); Expected> data = GetLiveCoreBinaryData(core_id, kind); if (!data) return data.takeError(); - auto it = - storage.live_core_data[core_id].insert({kind.str(), std::move(*data)}); + auto it = storage.live_core_data[core_id].insert( + {ConstString(kind), std::move(*data)}); return callback(it.first->second); } @@ -399,20 +425,20 @@ llvm::Error Trace::OnPostMortemThreadBinaryDataRead(lldb::tid_t tid, llvm::StringRef kind, OnBinaryDataReadCallback callback) { - Expected file = GetPostMortemThreadDataFile(tid, kind); - if (!file) + if (Expected file = GetPostMortemThreadDataFile(tid, kind)) + return OnDataFileRead(*file, callback); + else return file.takeError(); - return OnDataFileRead(*file, callback); } llvm::Error Trace::OnPostMortemCoreBinaryDataRead(lldb::core_id_t core_id, llvm::StringRef kind, OnBinaryDataReadCallback callback) { - Expected file = GetPostMortemCoreDataFile(core_id, kind); - if (!file) + if (Expected file = GetPostMortemCoreDataFile(core_id, kind)) + return OnDataFileRead(*file, callback); + else return file.takeError(); - return OnDataFileRead(*file, callback); } llvm::Error Trace::OnThreadBinaryDataRead(lldb::tid_t tid, llvm::StringRef kind, diff --git a/lldb/source/Utility/TraceGDBRemotePackets.cpp b/lldb/source/Utility/TraceGDBRemotePackets.cpp --- a/lldb/source/Utility/TraceGDBRemotePackets.cpp +++ b/lldb/source/Utility/TraceGDBRemotePackets.cpp @@ -121,8 +121,8 @@ json::Path path) { ObjectMapper o(value, path); uint64_t core_id; - if (!o || !o.map("coreId", core_id) || - !o.map("binaryData", packet.binary_data)) + if (!(o && o.map("coreId", core_id) && + o.map("binaryData", packet.binary_data))) return false; packet.core_id = static_cast(core_id); return true; @@ -139,19 +139,16 @@ json::Value toJSON(const TraceGetBinaryDataRequest &packet) { return json::Value(Object{{"type", packet.type}, {"kind", packet.kind}, - {"offset", packet.offset}, {"tid", packet.tid}, - {"coreId", packet.core_id}, - {"size", packet.size}}); + {"coreId", packet.core_id}}); } bool fromJSON(const json::Value &value, TraceGetBinaryDataRequest &packet, Path path) { ObjectMapper o(value, path); Optional core_id; - if (!o || !o.map("type", packet.type) || !o.map("kind", packet.kind) || - !o.map("tid", packet.tid) || !o.map("offset", packet.offset) || - !o.map("size", packet.size) || !o.map("coreId", core_id)) + if (!(o && o.map("type", packet.type) && o.map("kind", packet.kind) && + o.map("tid", packet.tid) && o.map("coreId", core_id))) return false; if (core_id)