[Lldb-commits] [lldb] 561a61f - [trace][intelpt] Support system-wide tracing [18] - some more improvements

Walter Erquinigo via lldb-commits lldb-commits at lists.llvm.org
Thu Jun 16 11:42:29 PDT 2022


Author: Walter Erquinigo
Date: 2022-06-16T11:42:21-07:00
New Revision: 561a61fb261bd3642f3c6187cf9c334502cac17f

URL: https://github.com/llvm/llvm-project/commit/561a61fb261bd3642f3c6187cf9c334502cac17f
DIFF: https://github.com/llvm/llvm-project/commit/561a61fb261bd3642f3c6187cf9c334502cac17f.diff

LOG: [trace][intelpt] Support system-wide tracing [18] - some more improvements

This applies the changes requested for diff 12.

- use DenseMap<ConstString, _> instead of std::unordered_map<ConstString, _>, which is more idiomatic and possibly performant.
- deduplicate some code in Trace.cpp by using helper functions for fetching in maps
- stop using size and offset when fetching binary data, because we in fact read the entire buffers all the time. If we ever need streaming, we can implement it then. Now, the size is used only to check that we are getting the correct amount of data. This is useful because in some cases determining the size doesn't involve fetching the actual data.
- added back the x86_64 macro to the perf tests
- added more documentation
- simplified some file handling
- fixed some comments

Differential Revision: https://reviews.llvm.org/D127752

Added: 
    

Modified: 
    lldb/docs/lldb-gdb-remote.txt
    lldb/include/lldb/Target/Trace.h
    lldb/include/lldb/Utility/TraceGDBRemotePackets.h
    lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
    lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
    lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h
    lldb/source/Plugins/Process/Linux/IntelPTThreadTraceCollection.cpp
    lldb/source/Plugins/Process/Linux/Perf.cpp
    lldb/source/Plugins/Process/Linux/Perf.h
    lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
    lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
    lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp
    lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
    lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
    lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
    lldb/source/Target/Trace.cpp
    lldb/source/Utility/TraceGDBRemotePackets.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/docs/lldb-gdb-remote.txt b/lldb/docs/lldb-gdb-remote.txt
index e62707bd7db4d..4a06964c30cab 100644
--- a/lldb/docs/lldb-gdb-remote.txt
+++ b/lldb/docs/lldb-gdb-remote.txt
@@ -562,10 +562,6 @@ read packet: {...object}/E<error code>;AAAAAAAAA
 //       Core id in decimal if the data belongs to a CPU core.
 //   "tid"?: <Optional decimal>,
 //       Tid in decimal if the data belongs to a thread.
-//   "offset": <decimal>,
-//       Offset of the data in bytes.
-//   "size": <decimal>,
-//      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
index 2cd4d80ba206e..63d8443715931 100644
--- a/lldb/include/lldb/Target/Trace.h
+++ b/lldb/include/lldb/Target/Trace.h
@@ -327,6 +327,12 @@ class Trace : public PluginInterface,
   ///     If it's not a live process session, return an empty list.
   llvm::ArrayRef<Process *> GetPostMortemProcesses();
 
+  /// Dispatcher for live trace data requests with some additional error
+  /// checking.
+  llvm::Expected<std::vector<uint8_t>>
+  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 @@ class Trace : public PluginInterface,
     /// \{
 
     /// tid -> data kind -> size
-    llvm::DenseMap<lldb::tid_t, std::unordered_map<std::string, uint64_t>>
+    llvm::DenseMap<lldb::tid_t, llvm::DenseMap<ConstString, uint64_t>>
         live_thread_data;
 
     /// core id -> data kind -> size
-    llvm::DenseMap<lldb::core_id_t, std::unordered_map<std::string, uint64_t>>
+    llvm::DenseMap<lldb::core_id_t, llvm::DenseMap<ConstString, uint64_t>>
         live_core_data_sizes;
     /// core id -> data kind -> bytes
     llvm::DenseMap<lldb::core_id_t,
-                   std::unordered_map<std::string, std::vector<uint8_t>>>
+                   llvm::DenseMap<ConstString, std::vector<uint8_t>>>
         live_core_data;
 
     /// data kind -> size
-    std::unordered_map<std::string, uint64_t> live_process_data;
+    llvm::DenseMap<ConstString, uint64_t> live_process_data;
     /// \}
 
     /// The list of cores being traced. Might be \b None depending on the
@@ -556,11 +562,11 @@ class Trace : public PluginInterface,
     /// \{
 
     /// tid -> data kind -> file
-    llvm::DenseMap<lldb::tid_t, std::unordered_map<std::string, FileSpec>>
+    llvm::DenseMap<lldb::tid_t, llvm::DenseMap<ConstString, FileSpec>>
         postmortem_thread_data;
 
     /// core id -> data kind -> file
-    llvm::DenseMap<lldb::core_id_t, std::unordered_map<std::string, FileSpec>>
+    llvm::DenseMap<lldb::core_id_t, llvm::DenseMap<ConstString, FileSpec>>
         postmortem_core_data;
 
     /// \}

diff  --git a/lldb/include/lldb/Utility/TraceGDBRemotePackets.h b/lldb/include/lldb/Utility/TraceGDBRemotePackets.h
index b45f1568a48dc..44f566190e3a6 100644
--- a/lldb/include/lldb/Utility/TraceGDBRemotePackets.h
+++ b/lldb/include/lldb/Utility/TraceGDBRemotePackets.h
@@ -155,10 +155,6 @@ struct TraceGetBinaryDataRequest {
   llvm::Optional<lldb::tid_t> tid;
   /// Optional core id if the data is related to a cpu core.
   llvm::Optional<lldb::core_id_t> 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
index 22cd860cfa935..537f8fb575a5b 100644
--- a/lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
+++ b/lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
@@ -158,9 +158,8 @@ IntelPTMultiCoreTrace::TryGetBinaryData(
         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.cpp b/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
index 8f7909b4072c7..d9d88ac6b747c 100644
--- a/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
+++ b/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
@@ -213,8 +213,7 @@ Error IntelPTSingleBufferTrace::Resume() {
   return m_perf_event.EnableWithIoctl();
 }
 
-Expected<std::vector<uint8_t>>
-IntelPTSingleBufferTrace::GetTraceBuffer(size_t offset, size_t size) {
+Expected<std::vector<uint8_t>> 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 @@ IntelPTSingleBufferTrace::GetTraceBuffer(size_t offset, size_t size) {
   //
   // 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<IntelPTSingleBufferTrace>

diff  --git a/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h b/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h
index 278ddd650aaf9..76952376035b3 100644
--- a/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h
+++ b/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h
@@ -64,18 +64,9 @@ class IntelPTSingleBufferTrace {
   /// 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<std::vector<uint8_t>> GetTraceBuffer(size_t offset,
-                                                      size_t size);
+  ///     A vector with the requested binary data.
+  llvm::Expected<std::vector<uint8_t>> GetTraceBuffer();
 
   /// \return
   ///     The total the size in bytes used by the trace buffer managed by this

diff  --git a/lldb/source/Plugins/Process/Linux/IntelPTThreadTraceCollection.cpp b/lldb/source/Plugins/Process/Linux/IntelPTThreadTraceCollection.cpp
index bd06e033f1606..182f822cd8995 100644
--- a/lldb/source/Plugins/Process/Linux/IntelPTThreadTraceCollection.cpp
+++ b/lldb/source/Plugins/Process/Linux/IntelPTThreadTraceCollection.cpp
@@ -85,7 +85,7 @@ IntelPTThreadTraceCollection::TryGetBinaryData(
 
   if (Expected<IntelPTSingleBufferTrace &> 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.cpp b/lldb/source/Plugins/Process/Linux/Perf.cpp
index 8b6b7f0e9ac45..f51e1cadd0c89 100644
--- a/lldb/source/Plugins/Process/Linux/Perf.cpp
+++ b/lldb/source/Plugins/Process/Linux/Perf.cpp
@@ -178,8 +178,7 @@ ArrayRef<uint8_t> PerfEvent::GetAuxBuffer() const {
            static_cast<size_t>(mmap_metadata.aux_size)};
 }
 
-Expected<std::vector<uint8_t>>
-PerfEvent::ReadFlushedOutDataCyclicBuffer(size_t offset, size_t size) {
+Expected<std::vector<uint8_t>> 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 @@ PerfEvent::ReadFlushedOutDataCyclicBuffer(size_t offset, size_t size) {
 
   /**
    * The data buffer and aux buffer have 
diff erent 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 @@ PerfEvent::ReadFlushedOutDataCyclicBuffer(size_t offset, size_t size) {
   uint64_t data_head = mmap_metadata.data_head;
   uint64_t data_size = mmap_metadata.data_size;
   std::vector<uint8_t> 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 @@ PerfEvent::ReadFlushedOutDataCyclicBuffer(size_t offset, size_t size) {
       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<std::vector<uint8_t>>
-PerfEvent::ReadFlushedOutAuxCyclicBuffer(size_t offset, size_t size) {
+Expected<std::vector<uint8_t>> 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 @@ PerfEvent::ReadFlushedOutAuxCyclicBuffer(size_t offset, size_t size) {
   uint64_t aux_head = mmap_metadata.aux_head;
   uint64_t aux_size = mmap_metadata.aux_size;
   std::vector<uint8_t> output;
-  output.reserve(size);
+  output.reserve(data.size());
 
   /**
    * When configured as ring buffer, the aux buffer keeps wrapping around
@@ -269,15 +254,10 @@ PerfEvent::ReadFlushedOutAuxCyclicBuffer(size_t offset, size_t size) {
    *
    * */
 
-  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 @@ PerfEvent::ReadFlushedOutAuxCyclicBuffer(size_t offset, size_t size) {
       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/Process/Linux/Perf.h b/lldb/source/Plugins/Process/Linux/Perf.h
index e400d53b96ff2..b5f968e72b2a1 100644
--- a/lldb/source/Plugins/Process/Linux/Perf.h
+++ b/lldb/source/Plugins/Process/Linux/Perf.h
@@ -200,43 +200,29 @@ class PerfEvent {
   ///   \a ArrayRef<uint8_t> extending \a aux_size bytes from \a aux_offset.
   llvm::ArrayRef<uint8_t> 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<std::vector<uint8_t>>
-  ReadFlushedOutAuxCyclicBuffer(size_t offset, size_t size);
+  ///     A vector with the requested binary data.
+  llvm::Expected<std::vector<uint8_t>> 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<std::vector<uint8_t>>
-  ReadFlushedOutDataCyclicBuffer(size_t offset, size_t size);
+  ///     A vector with the requested binary data.
+  llvm::Expected<std::vector<uint8_t>> 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/Trace/intel-pt/TraceIntelPT.cpp b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
index cdcd6eb6b74c7..2fe57097dda3f 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -102,7 +102,7 @@ TraceIntelPT::TraceIntelPT(JSONTraceSession &session,
     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<ThreadDecoder>(thread, *this));
       if (const Optional<FileSpec> &trace_file = thread->GetTraceFile()) {
         SetPostMortemThreadDataFile(
@@ -323,7 +323,7 @@ TraceIntelPT::Storage &TraceIntelPT::GetUpdatedStorage() {
 
 Error TraceIntelPT::DoRefreshLiveProcessState(TraceGetStateResponse state,
                                               StringRef json_response) {
-  m_storage = {};
+  m_storage = Storage();
 
   Expected<TraceIntelPTGetStateResponse> intelpt_state =
       json::parse<TraceIntelPTGetStateResponse>(json_response,
@@ -337,7 +337,7 @@ Error TraceIntelPT::DoRefreshLiveProcessState(TraceGetStateResponse state,
     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<ThreadDecoder>(thread_sp, *this));
     }
   } else {

diff  --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
index 53eba4914d04a..0fa6be4bfbb9d 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
@@ -170,7 +170,7 @@ class TraceIntelPT : public Trace {
   /// \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 @@ class TraceIntelPT : public Trace {
   struct Storage {
     llvm::Optional<TraceIntelPTMultiCoreDecoder> multicore_decoder;
     /// These decoders are used for the non-per-core case
-    std::map<lldb::tid_t, std::unique_ptr<ThreadDecoder>> thread_decoders;
+    llvm::DenseMap<lldb::tid_t, std::unique_ptr<ThreadDecoder>> 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/TraceIntelPTJSONStructs.cpp b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp
index 45b50e15f4391..5aeeb46831ed7 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp
@@ -87,9 +87,9 @@ json::Value toJSON(const JSONCore &core) {
 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 @@ bool fromJSON(const json::Value &value, pt_cpu &cpu_info, Path path) {
   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 @@ json::Value toJSON(const JSONTraceSession &session) {
 
 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.cpp b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
index 56ef530df2660..73eeda703c57a 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
@@ -29,7 +29,7 @@ FileSpec TraceIntelPTSessionFileParser::NormalizePath(const std::string &path) {
   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 @@ Error TraceIntelPTSessionFileParser::ParseModule(lldb::TargetSP &target_sp,
 
     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 @@ Error TraceIntelPTSessionFileParser::CreateJSONError(json::Path::Root &root,
 }
 
 ThreadPostMortemTraceSP
-TraceIntelPTSessionFileParser::ParseThread(ProcessSP &process_sp,
+TraceIntelPTSessionFileParser::ParseThread(Process &process,
                                            const JSONThread &thread) {
   lldb::tid_t tid = static_cast<lldb::tid_t>(thread.tid);
 
@@ -83,8 +83,8 @@ TraceIntelPTSessionFileParser::ParseThread(ProcessSP &process_sp,
     trace_file = FileSpec(*thread.trace_buffer);
 
   ThreadPostMortemTraceSP thread_sp =
-      std::make_shared<ThreadPostMortemTrace>(*process_sp, tid, trace_file);
-  process_sp->GetThreadList().AddThread(thread_sp);
+      std::make_shared<ThreadPostMortemTrace>(process, tid, trace_file);
+  process.GetThreadList().AddThread(thread_sp);
   return thread_sp;
 }
 
@@ -110,10 +110,10 @@ TraceIntelPTSessionFileParser::ParseProcess(const JSONProcess &process) {
   process_sp->SetID(static_cast<lldb::pid_t>(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/TraceIntelPTSessionFileParser.h b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
index d000ad21b5e73..d53c76048e876 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
@@ -52,17 +52,21 @@ class TraceIntelPTSessionFileParser {
   ///   errors, return a null pointer.
   llvm::Expected<lldb::TraceSP> Parse();
 
-  llvm::Expected<lldb::TraceSP>
-  CreateTraceIntelPTInstance(JSONTraceSession &session,
-                             std::vector<ParsedProcess> &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<lldb::TraceSP>
+  CreateTraceIntelPTInstance(JSONTraceSession &session,
+                             std::vector<ParsedProcess> &parsed_processes);
+
   /// Create the corresponding Threads and Process objects given the JSON
   /// process definition.
   ///
@@ -70,7 +74,9 @@ class TraceIntelPTSessionFileParser {
   ///   The JSON process definition
   llvm::Expected<ParsedProcess> 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 @@ class TraceIntelPTSessionFileParser {
 
   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/TraceIntelPTSessionSaver.cpp b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
index 1cf254d850ea7..a252b332eb5a8 100644
--- 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;
 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<uint8_t> data) {
   std::basic_fstream<char> out_fs = std::fstream(
@@ -63,7 +65,7 @@ WriteSessionToFile(const llvm::json::Value &trace_session_json,
   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 @@ BuildThreadsSection(Process &process, FileSpec directory) {
 
   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 @@ BuildCoresSection(TraceIntelPT &trace_ipt, FileSpec directory) {
   std::vector<JSONCore> 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 @@ BuildModulesSection(Process &process, FileSpec directory) {
     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 @@ Error TraceIntelPTSessionSaver::SaveToDisk(TraceIntelPT &trace_ipt,
   if (!cpu_info)
     return cpu_info.takeError();
 
+  FileSystem::Instance().Resolve(directory);
+
   Expected<std::vector<JSONProcess>> json_processes =
       BuildProcessesSection(trace_ipt, directory);
 

diff  --git a/lldb/source/Target/Trace.cpp b/lldb/source/Target/Trace.cpp
index 24ad72fceb091..a65f23fdb324d 100644
--- a/lldb/source/Target/Trace.cpp
+++ b/lldb/source/Target/Trace.cpp
@@ -42,6 +42,44 @@ bool fromJSON(const Value &value, JSONSimpleTraceSession &session, Path path) {
 } // 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 <typename K, typename V>
+static Optional<V> Lookup(DenseMap<K, V> &map, K k) {
+  auto it = map.find(k);
+  if (it == map.end())
+    return None;
+  return it->second;
+}
+
+template <typename K, typename V>
+static V *LookupAsPtr(DenseMap<K, V> &map, K k) {
+  auto it = map.find(k);
+  if (it == map.end())
+    return nullptr;
+  return &it->second;
+}
+
+template <typename K1, typename K2, typename V>
+static Optional<V> Lookup2(DenseMap<K1, DenseMap<K2, V>> &map, K1 k1, K2 k2) {
+  auto it = map.find(k1);
+  if (it == map.end())
+    return None;
+  return Lookup(it->second, k2);
+}
+
+template <typename K1, typename K2, typename V>
+static V *Lookup2AsPtr(DenseMap<K1, DenseMap<K2, V>> &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 @@ Expected<StringRef> Trace::FindPluginSchema(StringRef name) {
 
 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<lldb::tid_t> 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<std::string> 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<uint64_t> 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<std::string, uint64_t> &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<uint64_t> 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<std::string, uint64_t> &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<uint64_t> 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<std::vector<uint8_t>>
-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<std::vector<uint8_t>> 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<std::vector<uint8_t>>
+Trace::GetLiveThreadBinaryData(lldb::tid_t tid, llvm::StringRef kind) {
   llvm::Optional<uint64_t> size = GetLiveThreadBinaryDataSize(tid, kind);
   if (!size)
     return createStringError(
@@ -160,16 +209,17 @@ Trace::GetLiveThreadBinaryData(lldb::tid_t tid, llvm::StringRef kind) {
         "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<std::vector<uint8_t>>
 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<uint64_t> size = GetLiveCoreBinaryDataSize(core_id, kind);
   if (!size)
     return createStringError(
@@ -178,16 +228,12 @@ Trace::GetLiveCoreBinaryData(lldb::core_id_t core_id, llvm::StringRef kind) {
         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<std::vector<uint8_t>>
 Trace::GetLiveProcessBinaryData(llvm::StringRef kind) {
-  if (!m_live_process)
-    return createStringError(inconvertibleErrorCode(),
-                             "Tracing requires a live process.");
   llvm::Optional<uint64_t> size = GetLiveProcessBinaryDataSize(kind);
   if (!size)
     return createStringError(
@@ -195,9 +241,8 @@ Trace::GetLiveProcessBinaryData(llvm::StringRef kind) {
         "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 @@ const char *Trace::RefreshLiveProcessState() {
   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<std::string> json_string = GetLiveProcessState();
-  if (!json_string)
-    return HandleError(json_string.takeError());
+  auto do_refresh = [&]() -> Error {
+    Expected<std::string> json_string = GetLiveProcessState();
+    if (!json_string)
+      return json_string.takeError();
 
-  Expected<TraceGetStateResponse> live_process_state =
-      json::parse<TraceGetStateResponse>(*json_string, "TraceGetStateResponse");
-  if (!live_process_state)
-    return HandleError(live_process_state.takeError());
+    Expected<TraceGetStateResponse> live_process_state =
+        json::parse<TraceGetStateResponse>(*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 @@ uint32_t Trace::GetStopID() {
 
 llvm::Expected<FileSpec>
 Trace::GetPostMortemThreadDataFile(lldb::tid_t tid, llvm::StringRef kind) {
-  auto NotFoundError = [&]() {
+  Storage &storage = GetUpdatedStorage();
+  if (Optional<FileSpec> 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<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;
 }
 
 llvm::Expected<FileSpec>
 Trace::GetPostMortemCoreDataFile(lldb::core_id_t core_id,
                                  llvm::StringRef kind) {
-  auto NotFoundError = [&]() {
+  Storage &storage = GetUpdatedStorage();
+  if (Optional<FileSpec> 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<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) {
   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::Error Trace::OnLiveCoreBinaryDataRead(lldb::core_id_t core_id,
                                             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<uint8_t> *core_data =
+          Lookup2AsPtr(storage.live_core_data, core_id, ConstString(kind)))
+    return callback(*core_data);
 
   Expected<std::vector<uint8_t>> 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::OnDataFileRead(FileSpec file,
 llvm::Error
 Trace::OnPostMortemThreadBinaryDataRead(lldb::tid_t tid, llvm::StringRef kind,
                                         OnBinaryDataReadCallback callback) {
-  Expected<FileSpec> file = GetPostMortemThreadDataFile(tid, kind);
-  if (!file)
+  if (Expected<FileSpec> 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<FileSpec> file = GetPostMortemCoreDataFile(core_id, kind);
-  if (!file)
+  if (Expected<FileSpec> 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
index 5822ec7435b33..cea4099d1c3f2 100644
--- a/lldb/source/Utility/TraceGDBRemotePackets.cpp
+++ b/lldb/source/Utility/TraceGDBRemotePackets.cpp
@@ -121,8 +121,8 @@ bool fromJSON(const json::Value &value, TraceCoreState &packet,
               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<lldb::core_id_t>(core_id);
   return true;
@@ -139,19 +139,16 @@ json::Value toJSON(const TraceCoreState &packet) {
 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<uint64_t> 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)


        


More information about the lldb-commits mailing list