wallace created this revision.
wallace added a reviewer: jj10306.
Herald added a project: All.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This improves several things and addresses comments up to the diff [11] in this 
stack.

- Simplify many functions to receive less parameters that they can identify 
easily
- Create Storage classes for Trace and TraceIntelPT that can make it easier to 
reason about what can change with live process refreshes and what cannot.
- Don't cache the perf zero conversion numbers in lldb-server to make sure we 
get the most up-to-date numbers.
- Move the thread identifaction from context switches to the bundle parser, to 
leave TraceIntelPT simpler. This also makes sure that the constructor of 
TraceIntelPT is invoked when the entire data has been checked to be correct.
- Normalize all bundle paths before the Processes, Threads and Modules are 
created, so that they can assume that all paths are correct and absolute
- Fix some issues in the tests. Now they all pass.
- return the specific instance when constructing PerThread and MultiCore 
processor tracers.
- Properly implement IntelPTMultiCoreTrace::TraceStart.
- Improve some comments.
- Use the typedef ContextSwitchTrace more often for clarity.
- Move CreateContextSwitchTracePerfEvent to Perf.h as a utility function.
- Synchronize better the state of the context switch and the intel pt

perf events.

- Use a booblean instead of an enum for the PerfEvent state.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127456

Files:
  lldb/include/lldb/Target/Trace.h
  lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
  lldb/source/Plugins/Process/Linux/IntelPTCollector.h
  lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
  lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.h
  lldb/source/Plugins/Process/Linux/IntelPTPerThreadProcessTrace.cpp
  lldb/source/Plugins/Process/Linux/IntelPTPerThreadProcessTrace.h
  lldb/source/Plugins/Process/Linux/IntelPTProcessTrace.h
  lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h
  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.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.h
  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/test/API/commands/trace/intelpt-multi-core-trace/trace.json
  
lldb/test/API/commands/trace/intelpt-multi-core-trace/trace_missing_threads.json

Index: lldb/test/API/commands/trace/intelpt-multi-core-trace/trace_missing_threads.json
===================================================================
--- lldb/test/API/commands/trace/intelpt-multi-core-trace/trace_missing_threads.json
+++ lldb/test/API/commands/trace/intelpt-multi-core-trace/trace_missing_threads.json
@@ -1,14 +1,14 @@
 {
   "cores": [
     {
-      "contextSwitchTrace": "/tmp/trace8/cores/45.perf_context_switch_trace",
+      "contextSwitchTrace": "cores/45.perf_context_switch_trace",
       "coreId": 45,
-      "traceBuffer": "/tmp/trace8/cores/45.intelpt_trace"
+      "traceBuffer": "cores/45.intelpt_trace"
     },
     {
-      "contextSwitchTrace": "/tmp/trace8/cores/51.perf_context_switch_trace",
+      "contextSwitchTrace": "cores/51.perf_context_switch_trace",
       "coreId": 51,
-      "traceBuffer": "/tmp/trace8/cores/51.intelpt_trace"
+      "traceBuffer": "cores/51.intelpt_trace"
     }
   ],
   "cpuInfo": {
Index: lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
===================================================================
--- lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
+++ lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
@@ -1,14 +1,14 @@
 {
   "cores": [
     {
-      "contextSwitchTrace": "/tmp/trace8/cores/45.perf_context_switch_trace",
+      "contextSwitchTrace": "cores/45.perf_context_switch_trace",
       "coreId": 45,
-      "traceBuffer": "/tmp/trace8/cores/45.intelpt_trace"
+      "traceBuffer": "cores/45.intelpt_trace"
     },
     {
-      "contextSwitchTrace": "/tmp/trace8/cores/51.perf_context_switch_trace",
+      "contextSwitchTrace": "cores/51.perf_context_switch_trace",
       "coreId": 51,
-      "traceBuffer": "/tmp/trace8/cores/51.intelpt_trace"
+      "traceBuffer": "cores/51.intelpt_trace"
     }
   ],
   "cpuInfo": {
Index: lldb/source/Target/Trace.cpp
===================================================================
--- lldb/source/Target/Trace.cpp
+++ lldb/source/Target/Trace.cpp
@@ -116,8 +116,9 @@
 
 Optional<uint64_t> Trace::GetLiveThreadBinaryDataSize(lldb::tid_t tid,
                                                       llvm::StringRef kind) {
-  auto it = m_live_thread_data.find(tid);
-  if (it == m_live_thread_data.end())
+  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());
@@ -128,8 +129,9 @@
 
 Optional<uint64_t> Trace::GetLiveCoreBinaryDataSize(lldb::core_id_t core_id,
                                                     llvm::StringRef kind) {
-  auto it = m_live_core_data_sizes.find(core_id);
-  if (it == m_live_core_data_sizes.end())
+  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());
@@ -139,8 +141,9 @@
 }
 
 Optional<uint64_t> Trace::GetLiveProcessBinaryDataSize(llvm::StringRef kind) {
-  auto data_it = m_live_process_data.find(kind.str());
-  if (data_it == m_live_process_data.end())
+  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;
 }
@@ -197,6 +200,11 @@
   return m_live_process->TraceGetBinaryData(request);
 }
 
+Trace::Storage &Trace::GetUpdatedStorage() {
+  RefreshLiveProcessState();
+  return m_storage;
+}
+
 const char *Trace::RefreshLiveProcessState() {
   if (!m_live_process)
     return nullptr;
@@ -209,15 +217,11 @@
   LLDB_LOG(log, "Trace::RefreshLiveProcessState invoked");
 
   m_stop_id = new_stop_id;
-  m_live_thread_data.clear();
-  m_live_refresh_error.reset();
-  m_live_core_data_sizes.clear();
-  m_live_core_data.clear();
-  m_cores.reset();
+  m_storage = Trace::Storage();
 
   auto HandleError = [&](Error &&err) -> const char * {
-    m_live_refresh_error = toString(std::move(err));
-    return m_live_refresh_error->c_str();
+    m_storage.live_refresh_error = toString(std::move(err));
+    return m_storage.live_refresh_error->c_str();
   };
 
   Expected<std::string> json_string = GetLiveProcessState();
@@ -237,18 +241,19 @@
   for (const TraceThreadState &thread_state :
        live_process_state->traced_threads) {
     for (const TraceBinaryData &item : thread_state.binary_data)
-      m_live_thread_data[thread_state.tid][item.kind] = item.size;
+      m_storage.live_thread_data[thread_state.tid][item.kind] = item.size;
   }
 
   LLDB_LOG(log, "== Found {0} threads being traced",
            live_process_state->traced_threads.size());
 
   if (live_process_state->cores) {
-    m_cores.emplace();
+    m_storage.cores.emplace();
     for (const TraceCoreState &core_state : *live_process_state->cores) {
-      m_cores->push_back(core_state.core_id);
+      m_storage.cores->push_back(core_state.core_id);
       for (const TraceBinaryData &item : core_state.binary_data)
-        m_live_core_data_sizes[core_state.core_id][item.kind] = item.size;
+        m_storage.live_core_data_sizes[core_state.core_id][item.kind] =
+            item.size;
     }
     LLDB_LOG(log, "== Found {0} cpu cores being traced",
             live_process_state->cores->size());
@@ -256,7 +261,7 @@
 
 
   for (const TraceBinaryData &item : live_process_state->process_binary_data)
-    m_live_process_data[item.kind] = item.size;
+    m_storage.live_process_data[item.kind] = item.size;
 
   if (Error err = DoRefreshLiveProcessState(std::move(*live_process_state),
                                             *json_string))
@@ -268,14 +273,14 @@
 Trace::Trace(ArrayRef<ProcessSP> postmortem_processes,
              Optional<std::vector<lldb::core_id_t>> postmortem_cores) {
   for (ProcessSP process_sp : postmortem_processes)
-    m_postmortem_processes.push_back(process_sp.get());
-  m_cores = postmortem_cores;
+    m_storage.postmortem_processes.push_back(process_sp.get());
+  m_storage.cores = postmortem_cores;
 }
 
 Process *Trace::GetLiveProcess() { return m_live_process; }
 
 ArrayRef<Process *> Trace::GetPostMortemProcesses() {
-  return m_postmortem_processes;
+  return m_storage.postmortem_processes;
 }
 
 std::vector<Process *> Trace::GetAllProcesses() {
@@ -298,8 +303,9 @@
                 tid, kind));
   };
 
-  auto it = m_postmortem_thread_data.find(tid);
-  if (it == m_postmortem_thread_data.end())
+  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 =
@@ -320,8 +326,9 @@
                 core_id, kind));
   };
 
-  auto it = m_postmortem_core_data.find(core_id);
-  if (it == m_postmortem_core_data.end())
+  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 =
@@ -334,13 +341,15 @@
 
 void Trace::SetPostMortemThreadDataFile(lldb::tid_t tid, llvm::StringRef kind,
                                         FileSpec file_spec) {
-  m_postmortem_thread_data[tid][kind.str()] = file_spec;
+  Storage &storage = GetUpdatedStorage();
+  storage.postmortem_thread_data[tid][kind.str()] = file_spec;
 }
 
 void Trace::SetPostMortemCoreDataFile(lldb::core_id_t core_id,
                                       llvm::StringRef kind,
                                       FileSpec file_spec) {
-  m_postmortem_core_data[core_id][kind.str()] = file_spec;
+  Storage &storage = GetUpdatedStorage();
+  storage.postmortem_core_data[core_id][kind.str()] = file_spec;
 }
 
 llvm::Error
@@ -355,8 +364,9 @@
 llvm::Error Trace::OnLiveCoreBinaryDataRead(lldb::core_id_t core_id,
                                             llvm::StringRef kind,
                                             OnBinaryDataReadCallback callback) {
-  auto core_data_entries = m_live_core_data.find(core_id);
-  if (core_data_entries != m_live_core_data.end()) {
+  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);
@@ -365,7 +375,8 @@
   Expected<std::vector<uint8_t>> data = GetLiveCoreBinaryData(core_id, kind);
   if (!data)
     return data.takeError();
-  auto it = m_live_core_data[core_id].insert({kind.str(), std::move(*data)});
+  auto it =
+      storage.live_core_data[core_id].insert({kind.str(), std::move(*data)});
   return callback(it.first->second);
 }
 
@@ -374,7 +385,9 @@
   ErrorOr<std::unique_ptr<MemoryBuffer>> trace_or_error =
       MemoryBuffer::getFile(file.GetPath());
   if (std::error_code err = trace_or_error.getError())
-    return errorCodeToError(err);
+    return createStringError(
+        inconvertibleErrorCode(), "Failed fetching trace-related file %s. %s",
+        file.GetCString(), toString(errorCodeToError(err)).c_str());
 
   MemoryBuffer &data = **trace_or_error;
   ArrayRef<uint8_t> array_ref(
@@ -404,7 +417,6 @@
 
 llvm::Error Trace::OnThreadBinaryDataRead(lldb::tid_t tid, llvm::StringRef kind,
                                           OnBinaryDataReadCallback callback) {
-  RefreshLiveProcessState();
   if (m_live_process)
     return OnLiveThreadBinaryDataRead(tid, kind, callback);
   else
@@ -412,14 +424,16 @@
 }
 
 llvm::Error
-Trace::OnCoresBinaryDataRead(const std::set<lldb::core_id_t> core_ids,
-                             llvm::StringRef kind,
-                             OnCoresBinaryDataReadCallback callback) {
+Trace::OnAllCoresBinaryDataRead(llvm::StringRef kind,
+                                OnCoresBinaryDataReadCallback callback) {
   DenseMap<core_id_t, ArrayRef<uint8_t>> buffers;
+  Storage &storage = GetUpdatedStorage();
+  if (!storage.cores)
+    return Error::success();
 
-  std::function<Error(std::set<core_id_t>::iterator)> process_core =
-      [&](std::set<core_id_t>::iterator core_id) -> Error {
-    if (core_id == core_ids.end())
+  std::function<Error(std::vector<core_id_t>::iterator)> process_core =
+      [&](std::vector<core_id_t>::iterator core_id) -> Error {
+    if (core_id == storage.cores->end())
       return callback(buffers);
 
     return OnCoreBinaryDataRead(*core_id, kind,
@@ -430,13 +444,12 @@
                                   return process_core(next_id);
                                 });
   };
-  return process_core(core_ids.begin());
+  return process_core(storage.cores->begin());
 }
 
 llvm::Error Trace::OnCoreBinaryDataRead(lldb::core_id_t core_id,
                                         llvm::StringRef kind,
                                         OnBinaryDataReadCallback callback) {
-  RefreshLiveProcessState();
   if (m_live_process)
     return OnLiveCoreBinaryDataRead(core_id, kind, callback);
   else
@@ -444,16 +457,17 @@
 }
 
 ArrayRef<lldb::core_id_t> Trace::GetTracedCores() {
-  RefreshLiveProcessState();
-  if (m_cores)
-    return *m_cores;
+  Storage &storage = GetUpdatedStorage();
+  if (storage.cores)
+    return *storage.cores;
   return {};
 }
 
-std::vector<Process *> Trace::GetTracedProcesses() const {
+std::vector<Process *> Trace::GetTracedProcesses() {
   std::vector<Process *> processes;
+  Storage &storage = GetUpdatedStorage();
 
-  for (Process *proc : m_postmortem_processes)
+  for (Process *proc : storage.postmortem_processes)
     processes.push_back(proc);
 
   if (m_live_process)
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
@@ -262,7 +262,7 @@
     return json_modules.takeError();
 
   return JSONProcess{
-      static_cast<int64_t>(process.GetID()),
+      process.GetID(),
       process.GetTarget().GetArchitecture().GetTriple().getTriple(),
       json_threads.get(), json_modules.get()};
 }
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
@@ -57,9 +57,8 @@
                              std::vector<ParsedProcess> &parsed_processes);
 
 private:
-  /// Resolve non-absolute paths relative to the session file folder. It
-  /// modifies the given file_spec.
-  void NormalizePath(lldb_private::FileSpec &file_spec);
+  /// Resolve non-absolute paths relative to the session file folder.
+  FileSpec NormalizePath(const std::string &path);
 
   lldb::ThreadPostMortemTraceSP ParseThread(lldb::ProcessSP &process_sp,
                                             const JSONThread &thread);
@@ -92,6 +91,19 @@
   llvm::Expected<std::vector<ParsedProcess>>
   ParseSessionFile(const JSONTraceSession &session);
 
+  /// When applicable, augment the list of threads in the session file by
+  /// inspecting the context switch trace. This only applies for threads of
+  /// processes already specified in this session file.
+  ///
+  /// \return
+  ///   An \a llvm::Error in case if failures, or \a llvm::Error::success
+  ///   otherwise.
+  llvm::Error AugmentThreadsFromContextSwitches(JSONTraceSession &session);
+
+  /// Modifiy the session file by normalizing all the paths relative to the
+  /// session file directory.
+  void NormalizeAllPaths(JSONTraceSession &session);
+
   Debugger &m_debugger;
   const llvm::json::Value &m_trace_session_file;
   std::string m_session_file_dir;
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
@@ -22,39 +22,45 @@
 using namespace lldb_private::trace_intel_pt;
 using namespace llvm;
 
-void TraceIntelPTSessionFileParser::NormalizePath(
-    lldb_private::FileSpec &file_spec) {
+FileSpec TraceIntelPTSessionFileParser::NormalizePath(const std::string &path) {
+  FileSpec file_spec(path);
   if (file_spec.IsRelative())
     file_spec.PrependPathComponent(m_session_file_dir);
+  return file_spec;
 }
 
 Error TraceIntelPTSessionFileParser::ParseModule(lldb::TargetSP &target_sp,
                                                  const JSONModule &module) {
-  FileSpec system_file_spec(module.system_path);
-  NormalizePath(system_file_spec);
+  auto do_parse = [&]() -> Error {
+    FileSpec system_file_spec(module.system_path);
 
-  FileSpec local_file_spec(module.file.hasValue() ? *module.file
-                                                  : module.system_path);
-  NormalizePath(local_file_spec);
+    FileSpec local_file_spec(module.file.hasValue() ? *module.file
+                                                    : module.system_path);
 
-  ModuleSpec module_spec;
-  module_spec.GetFileSpec() = local_file_spec;
-  module_spec.GetPlatformFileSpec() = system_file_spec;
+    ModuleSpec module_spec;
+    module_spec.GetFileSpec() = local_file_spec;
+    module_spec.GetPlatformFileSpec() = system_file_spec;
 
-  if (module.uuid.hasValue())
-    module_spec.GetUUID().SetFromStringRef(*module.uuid);
+    if (module.uuid.hasValue())
+      module_spec.GetUUID().SetFromStringRef(*module.uuid);
 
-  Status error;
-  ModuleSP module_sp =
-      target_sp->GetOrCreateModule(module_spec, /*notify*/ false, &error);
+    Status error;
+    ModuleSP module_sp =
+        target_sp->GetOrCreateModule(module_spec, /*notify*/ false, &error);
 
-  if (error.Fail())
-    return error.ToError();
+    if (error.Fail())
+      return error.ToError();
 
-  bool load_addr_changed = false;
-  module_sp->SetLoadAddress(*target_sp, module.load_address, false,
-                            load_addr_changed);
-  return llvm::Error::success();
+    bool load_addr_changed = false;
+    module_sp->SetLoadAddress(*target_sp, module.load_address, false,
+                              load_addr_changed);
+    return Error::success();
+  };
+  if (Error err = do_parse())
+    return createStringError(
+        inconvertibleErrorCode(), "Error when parsing module %s. %s",
+        module.system_path.c_str(), toString(std::move(err)).c_str());
+  return Error::success();
 }
 
 Error TraceIntelPTSessionFileParser::CreateJSONError(json::Path::Root &root,
@@ -73,10 +79,8 @@
   lldb::tid_t tid = static_cast<lldb::tid_t>(thread.tid);
 
   Optional<FileSpec> trace_file;
-  if (thread.trace_buffer) {
-    trace_file.emplace(*thread.trace_buffer);
-    NormalizePath(*trace_file);
-  }
+  if (thread.trace_buffer)
+    trace_file = FileSpec(*thread.trace_buffer);
 
   ThreadPostMortemTraceSP thread_sp =
       std::make_shared<ThreadPostMortemTrace>(*process_sp, tid, trace_file);
@@ -225,6 +229,54 @@
   return schema;
 }
 
+Error TraceIntelPTSessionFileParser::AugmentThreadsFromContextSwitches(
+    JSONTraceSession &session) {
+  if (!session.cores)
+    return Error::success();
+
+  if (!session.tsc_perf_zero_conversion)
+    return createStringError(inconvertibleErrorCode(),
+                             "TSC to nanos conversion values are needed when "
+                             "context switch information is provided.");
+
+  DenseMap<lldb::pid_t, JSONProcess *> indexed_processes;
+  DenseMap<JSONProcess *, DenseSet<tid_t>> indexed_threads;
+
+  for (JSONProcess &process : session.processes) {
+    indexed_processes[process.pid] = &process;
+    for (JSONThread &thread : process.threads)
+      indexed_threads[&process].insert(thread.tid);
+  }
+
+  auto on_thread_seen = [&](lldb::pid_t pid, tid_t tid) {
+    auto proc = indexed_processes.find(pid);
+    if (proc == indexed_processes.end())
+      return;
+    if (indexed_threads[proc->second].count(tid))
+      return;
+    indexed_threads[proc->second].insert(tid);
+    proc->second->threads.push_back({tid, /*trace_buffer=*/None});
+  };
+
+  for (const JSONCore &core : *session.cores) {
+    Error err = Trace::OnDataFileRead(
+        FileSpec(core.context_switch_trace),
+        [&](ArrayRef<uint8_t> data) -> Error {
+          Expected<std::vector<ThreadContinuousExecution>> executions =
+              DecodePerfContextSwitchTrace(data, core.core_id,
+                                           *session.tsc_perf_zero_conversion);
+          if (!executions)
+            return executions.takeError();
+          for (const ThreadContinuousExecution &execution : *executions)
+            on_thread_seen(execution.pid, execution.tid);
+          return Error::success();
+        });
+    if (err)
+      return err;
+  }
+  return Error::success();
+}
+
 Expected<TraceSP> TraceIntelPTSessionFileParser::CreateTraceIntelPTInstance(
     JSONTraceSession &session, std::vector<ParsedProcess> &parsed_processes) {
   std::vector<ThreadPostMortemTraceSP> threads;
@@ -235,17 +287,33 @@
                    parsed_process.threads.end());
   }
 
-  TraceIntelPTSP trace_instance(new TraceIntelPT(
-      session, FileSpec(m_session_file_dir), processes, threads));
+  TraceSP trace_instance(new TraceIntelPT(session, processes, threads));
   for (const ParsedProcess &parsed_process : parsed_processes)
     parsed_process.target_sp->SetTrace(trace_instance);
 
+  return trace_instance;
+}
+
+void TraceIntelPTSessionFileParser::NormalizeAllPaths(
+    JSONTraceSession &session) {
+  for (JSONProcess &process : session.processes) {
+    for (JSONModule &module : process.modules) {
+      module.system_path = NormalizePath(module.system_path).GetPath();
+      if (module.file)
+        module.file = NormalizePath(*module.file).GetPath();
+    }
+    for (JSONThread &thread : process.threads) {
+      if (thread.trace_buffer)
+        thread.trace_buffer = NormalizePath(*thread.trace_buffer).GetPath();
+    }
+  }
   if (session.cores) {
-    if (Error err = trace_instance->CreateThreadsFromContextSwitches())
-      return std::move(err);
+    for (JSONCore &core : *session.cores) {
+      core.context_switch_trace =
+          NormalizePath(core.context_switch_trace).GetPath();
+      core.trace_buffer = NormalizePath(core.trace_buffer).GetPath();
+    }
   }
-
-  return trace_instance;
 }
 
 Expected<TraceSP> TraceIntelPTSessionFileParser::Parse() {
@@ -254,6 +322,11 @@
   if (!fromJSON(m_trace_session_file, session, root))
     return CreateJSONError(root, m_trace_session_file);
 
+  NormalizeAllPaths(session);
+
+  if (Error err = AugmentThreadsFromContextSwitches(session))
+    return std::move(err);
+
   if (Expected<std::vector<ParsedProcess>> parsed_processes =
           ParseSessionFile(session))
     return CreateTraceIntelPTInstance(session, *parsed_processes);
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.h
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.h
@@ -24,20 +24,14 @@
 /// to contention or race conditions. Finally, it assumes that a tid is not
 /// repeated twice for two different threads because of the shortness of the
 /// intel pt trace.
+///
+/// This object should be recreated after every stop in the case of live
+/// processes.
 class TraceIntelPTMultiCoreDecoder {
 public:
-  /// \param[in] core_ids
-  ///   The list of cores where the traced programs were running on.
-  ///
-  /// \param[in] tids
-  ///   The full list of tids that were traced.
-  ///
-  /// \param[in] tsc_conversion
-  ///   The conversion values for converting between nanoseconds and TSCs.
-  TraceIntelPTMultiCoreDecoder(
-      TraceIntelPT &trace, llvm::ArrayRef<lldb::core_id_t> core_ids,
-      llvm::ArrayRef<lldb::tid_t> tids,
-      const LinuxPerfZeroTscConversion &tsc_conversion);
+  /// \param[in] TraceIntelPT
+  ///   The trace object to be decoded
+  TraceIntelPTMultiCoreDecoder(TraceIntelPT &trace);
 
   /// \return
   ///   A \a DecodedThread for the \p thread by decoding its instructions on all
@@ -68,16 +62,14 @@
       llvm::DenseMap<lldb::tid_t, std::vector<IntelPTThreadContinousExecution>>>
   CorrelateContextSwitchesAndIntelPtTraces();
 
-  TraceIntelPT &m_trace;
-  std::set<lldb::core_id_t> m_cores;
+  TraceIntelPT *m_trace;
   std::set<lldb::tid_t> m_tids;
   llvm::Optional<
       llvm::DenseMap<lldb::tid_t, std::vector<IntelPTThreadContinousExecution>>>
       m_continuous_executions_per_thread;
   llvm::DenseMap<lldb::tid_t, DecodedThreadSP> m_decoded_threads;
-  LinuxPerfZeroTscConversion m_tsc_conversion;
   /// This variable will be non-None if a severe error happened during the setup
-  /// of the decoder.
+  /// of the decoder and we don't want decoding to be reattempted.
   llvm::Optional<std::string> m_setup_error;
   uint64_t m_unattributed_intelpt_subtraces;
 };
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.cpp
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.cpp
@@ -17,11 +17,14 @@
 using namespace lldb_private::trace_intel_pt;
 using namespace llvm;
 
-TraceIntelPTMultiCoreDecoder::TraceIntelPTMultiCoreDecoder(
-    TraceIntelPT &trace, ArrayRef<core_id_t> core_ids, ArrayRef<tid_t> tids,
-    const LinuxPerfZeroTscConversion &tsc_conversion)
-    : m_trace(trace), m_cores(core_ids.begin(), core_ids.end()),
-      m_tids(tids.begin(), tids.end()), m_tsc_conversion(tsc_conversion) {}
+TraceIntelPTMultiCoreDecoder::TraceIntelPTMultiCoreDecoder(TraceIntelPT &trace)
+    : m_trace(&trace) {
+  for (Process *proc : trace.GetAllProcesses()) {
+    for (ThreadSP thread_sp : proc->GetThreadList().Threads()) {
+      m_tids.insert(thread_sp->GetID());
+    }
+  }
+}
 
 bool TraceIntelPTMultiCoreDecoder::TracesThread(lldb::tid_t tid) const {
   return m_tids.count(tid);
@@ -38,12 +41,12 @@
   DecodedThreadSP decoded_thread_sp =
       std::make_shared<DecodedThread>(thread.shared_from_this());
 
-  Error err = m_trace.OnCoresBinaryDataRead(
-      m_cores, IntelPTDataKinds::kTraceBuffer,
+  Error err = m_trace->OnAllCoresBinaryDataRead(
+      IntelPTDataKinds::kTraceBuffer,
       [&](const DenseMap<core_id_t, ArrayRef<uint8_t>> buffers) -> Error {
         auto it = m_continuous_executions_per_thread->find(thread.GetID());
         if (it != m_continuous_executions_per_thread->end())
-          DecodeTrace(*decoded_thread_sp, m_trace, buffers, it->second);
+          DecodeTrace(*decoded_thread_sp, *m_trace, buffers, it->second);
 
         return Error::success();
       });
@@ -60,14 +63,23 @@
   llvm::DenseMap<lldb::tid_t, std::vector<IntelPTThreadContinousExecution>>
       continuous_executions_per_thread;
 
-  for (core_id_t core_id : m_cores) {
+  Optional<LinuxPerfZeroTscConversion> conv_opt =
+      m_trace->GetPerfZeroTscConversion();
+  if (!conv_opt)
+    return createStringError(
+        inconvertibleErrorCode(),
+        "TSC to nanoseconds conversion values were not found");
+
+  LinuxPerfZeroTscConversion tsc_conversion = *conv_opt;
+
+  for (core_id_t core_id : m_trace->GetTracedCores()) {
     std::vector<IntelPTThreadSubtrace> intel_pt_executions;
 
-    Error err = m_trace.OnCoreBinaryDataRead(
+    Error err = m_trace->OnCoreBinaryDataRead(
         core_id, IntelPTDataKinds::kTraceBuffer,
         [&](ArrayRef<uint8_t> data) -> Error {
           Expected<std::vector<IntelPTThreadSubtrace>> split_trace =
-              SplitTraceInContinuousExecutions(m_trace, data);
+              SplitTraceInContinuousExecutions(*m_trace, data);
           if (!split_trace)
             return split_trace.takeError();
 
@@ -96,11 +108,11 @@
           continuous_executions_per_thread[thread_execution.tid].push_back(
               execution);
         };
-    err = m_trace.OnCoreBinaryDataRead(
+    err = m_trace->OnCoreBinaryDataRead(
         core_id, IntelPTDataKinds::kPerfContextSwitchTrace,
         [&](ArrayRef<uint8_t> data) -> Error {
           Expected<std::vector<ThreadContinuousExecution>> executions =
-              DecodePerfContextSwitchTrace(data, core_id, m_tsc_conversion);
+              DecodePerfContextSwitchTrace(data, core_id, tsc_conversion);
           if (!executions)
             return executions.takeError();
           for (const ThreadContinuousExecution &exec : *executions)
@@ -125,7 +137,7 @@
   if (m_continuous_executions_per_thread)
     return Error::success();
 
-  Error err = m_trace.GetTimer().ForGlobal().TimeTask<Error>(
+  Error err = m_trace->GetTimer().ForGlobal().TimeTask<Error>(
       "Context switch and Intel PT traces correlation", [&]() -> Error {
         if (auto correlation = CorrelateContextSwitchesAndIntelPtTraces()) {
           m_continuous_executions_per_thread.emplace(std::move(*correlation));
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
@@ -30,12 +30,12 @@
 };
 
 struct JSONThread {
-  int64_t tid;
+  uint64_t tid;
   llvm::Optional<std::string> trace_buffer;
 };
 
 struct JSONProcess {
-  int64_t pid;
+  uint64_t pid;
   llvm::Optional<std::string> triple;
   std::vector<JSONThread> threads;
   std::vector<JSONModule> modules;
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
@@ -163,15 +163,6 @@
 private:
   friend class TraceIntelPTSessionFileParser;
 
-  /// Create post-mortem threads associated with the processes traced by this
-  /// instance using the context switch traces.
-  ///
-  /// This does nothing if the threads already exist.
-  ///
-  /// \return
-  ///   An \a llvm::Error in case of failures.
-  llvm::Error CreateThreadsFromContextSwitches();
-
   llvm::Expected<pt_cpu> GetCPUInfoForLiveProcess();
 
   /// Postmortem trace constructor
@@ -185,13 +176,12 @@
   /// \param[in] trace_threads
   ///     The threads traced in the live session. They must belong to the
   ///     processes mentioned above.
-  TraceIntelPT(JSONTraceSession &session, const FileSpec &session_file_dir,
+  TraceIntelPT(JSONTraceSession &session,
                llvm::ArrayRef<lldb::ProcessSP> traced_processes,
                llvm::ArrayRef<lldb::ThreadPostMortemTraceSP> traced_threads);
 
   /// Constructor for live processes
-  TraceIntelPT(Process &live_process)
-      : Trace(live_process), m_thread_decoders(){};
+  TraceIntelPT(Process &live_process) : Trace(live_process){};
 
   /// Decode the trace of the given thread that, i.e. recontruct the traced
   /// instructions.
@@ -205,20 +195,29 @@
   ///     errors are embedded in the instruction list.
   DecodedThreadSP Decode(Thread &thread);
 
+  /// We package all the data that can change upon process stops to make sure
+  /// this contract is very visible.
+  /// This variable should only be accessed directly by constructores or live
+  /// process data refreshers.
+  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;
+    /// 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
+    /// counters to and from nanos. It might not be available on all hosts.
+    llvm::Optional<LinuxPerfZeroTscConversion> tsc_conversion;
+  } m_storage;
+
   /// It is provided by either a session file or a live process' "cpuInfo"
-  /// binary data.
+  /// binary data. We don't put it in the Storage because this variable doesn't
+  /// change.
   llvm::Optional<pt_cpu> m_cpu_info;
-  llvm::Optional<TraceIntelPTMultiCoreDecoder> m_multicore_decoder;
-  /// These decoders are used for the non-per-core case
-  std::map<lldb::tid_t, std::unique_ptr<ThreadDecoder>> m_thread_decoders;
-  /// Helper variable used to track long running operations for telemetry.
-  TaskTimer m_task_timer;
-  /// It is provided by either a session file or a live process to convert TSC
-  /// counters to and from nanos. It might not be available on all hosts.
-  llvm::Optional<LinuxPerfZeroTscConversion> m_tsc_conversion;
-};
 
-using TraceIntelPTSP = std::shared_ptr<TraceIntelPT>;
+  /// Get the storage after refreshing the data in the case of a live process.
+  Storage &GetUpdatedStorage();
+};
 
 } // namespace trace_intel_pt
 } // namespace lldb_private
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
@@ -75,37 +75,22 @@
 }
 
 TraceIntelPT::TraceIntelPT(JSONTraceSession &session,
-                           const FileSpec &session_file_dir,
                            ArrayRef<ProcessSP> traced_processes,
                            ArrayRef<ThreadPostMortemTraceSP> traced_threads)
     : Trace(traced_processes, session.GetCoreIds()),
-      m_cpu_info(session.cpu_info),
-      m_tsc_conversion(session.tsc_perf_zero_conversion) {
-  for (const ThreadPostMortemTraceSP &thread : traced_threads) {
-    m_thread_decoders.emplace(thread->GetID(),
-                              std::make_unique<ThreadDecoder>(thread, *this));
-    if (const Optional<FileSpec> &trace_file = thread->GetTraceFile()) {
-      SetPostMortemThreadDataFile(thread->GetID(),
-                                  IntelPTDataKinds::kTraceBuffer, *trace_file);
-    }
-  }
+      m_cpu_info(session.cpu_info) {
+  m_storage.tsc_conversion = session.tsc_perf_zero_conversion;
+
   if (session.cores) {
     std::vector<core_id_t> cores;
 
     for (const JSONCore &core : *session.cores) {
-      FileSpec trace_buffer(core.trace_buffer);
-      if (trace_buffer.IsRelative())
-        trace_buffer.PrependPathComponent(session_file_dir);
-
       SetPostMortemCoreDataFile(core.core_id, IntelPTDataKinds::kTraceBuffer,
-                                trace_buffer);
+                                FileSpec(core.trace_buffer));
 
-      FileSpec context_switch(core.context_switch_trace);
-      if (context_switch.IsRelative())
-        context_switch.PrependPathComponent(session_file_dir);
       SetPostMortemCoreDataFile(core.core_id,
                                 IntelPTDataKinds::kPerfContextSwitchTrace,
-                                context_switch);
+                                FileSpec(core.context_switch_trace));
       cores.push_back(core.core_id);
     }
 
@@ -114,8 +99,16 @@
       for (const JSONThread &thread : process.threads)
         tids.push_back(thread.tid);
 
-    m_multicore_decoder.emplace(*this, cores, tids,
-                                *session.tsc_perf_zero_conversion);
+    m_storage.multicore_decoder.emplace(*this);
+  } else {
+    for (const ThreadPostMortemTraceSP &thread : traced_threads) {
+      m_storage.thread_decoders.emplace(
+          thread->GetID(), std::make_unique<ThreadDecoder>(thread, *this));
+      if (const Optional<FileSpec> &trace_file = thread->GetTraceFile()) {
+        SetPostMortemThreadDataFile(
+            thread->GetID(), IntelPTDataKinds::kTraceBuffer, *trace_file);
+      }
+    }
   }
 }
 
@@ -125,11 +118,12 @@
         thread.shared_from_this(),
         createStringError(inconvertibleErrorCode(), error));
 
-  if (m_multicore_decoder)
-    return m_multicore_decoder->Decode(thread);
+  Storage &storage = GetUpdatedStorage();
+  if (storage.multicore_decoder)
+    return storage.multicore_decoder->Decode(thread);
 
-  auto it = m_thread_decoders.find(thread.GetID());
-  if (it == m_thread_decoders.end())
+  auto it = storage.thread_decoders.find(thread.GetID());
+  if (it == storage.thread_decoders.end())
     return std::make_shared<DecodedThread>(
         thread.shared_from_this(),
         createStringError(inconvertibleErrorCode(), "thread not traced"));
@@ -141,6 +135,8 @@
 }
 
 void TraceIntelPT::DumpTraceInfo(Thread &thread, Stream &s, bool verbose) {
+  Storage &storage = GetUpdatedStorage();
+
   lldb::tid_t tid = thread.GetID();
   s.Format("\nthread #{0}: tid = {1}", thread.GetIndexID(), thread.GetID());
   if (!IsTraced(tid)) {
@@ -209,12 +205,13 @@
   }
 
   // Multicode decoding stats
-  if (m_multicore_decoder) {
+  if (storage.multicore_decoder) {
     s << "\n  Multi-core decoding:\n";
     s.Format("    Total number of continuous executions found: {0}\n",
-             m_multicore_decoder->GetTotalContinuousExecutionsCount());
-    s.Format("    Number of continuous executions for this thread: {0}\n",
-             m_multicore_decoder->GetNumContinuousExecutionsForThread(tid));
+             storage.multicore_decoder->GetTotalContinuousExecutionsCount());
+    s.Format(
+        "    Number of continuous executions for this thread: {0}\n",
+        storage.multicore_decoder->GetNumContinuousExecutionsForThread(tid));
   }
 
   // Errors
@@ -234,7 +231,7 @@
 
 llvm::Expected<Optional<uint64_t>>
 TraceIntelPT::GetRawTraceSize(Thread &thread) {
-  if (m_multicore_decoder)
+  if (GetUpdatedStorage().multicore_decoder)
     return None; // TODO: calculate the amount of intel pt raw trace associated
                  // with the given thread.
   if (GetLiveProcess())
@@ -316,15 +313,17 @@
 
 llvm::Optional<LinuxPerfZeroTscConversion>
 TraceIntelPT::GetPerfZeroTscConversion() {
+  return GetUpdatedStorage().tsc_conversion;
+}
+
+TraceIntelPT::Storage &TraceIntelPT::GetUpdatedStorage() {
   RefreshLiveProcessState();
-  return m_tsc_conversion;
+  return m_storage;
 }
 
 Error TraceIntelPT::DoRefreshLiveProcessState(TraceGetStateResponse state,
                                               StringRef json_response) {
-  m_thread_decoders.clear();
-  m_tsc_conversion.reset();
-  m_multicore_decoder.reset();
+  m_storage = {};
 
   Expected<TraceIntelPTGetStateResponse> intelpt_state =
       json::parse<TraceIntelPTGetStateResponse>(json_response,
@@ -332,11 +331,13 @@
   if (!intelpt_state)
     return intelpt_state.takeError();
 
+  m_storage.tsc_conversion = intelpt_state->tsc_perf_zero_conversion;
+
   if (!intelpt_state->cores) {
     for (const TraceThreadState &thread_state : state.traced_threads) {
       ThreadSP thread_sp =
           GetLiveProcess()->GetThreadList().FindThreadByID(thread_state.tid);
-      m_thread_decoders.emplace(
+      m_storage.thread_decoders.emplace(
           thread_state.tid, std::make_unique<ThreadDecoder>(thread_sp, *this));
     }
   } else {
@@ -351,12 +352,10 @@
     if (!intelpt_state->tsc_perf_zero_conversion)
       return createStringError(inconvertibleErrorCode(),
                                "Missing perf time_zero conversion values");
-    m_multicore_decoder.emplace(*this, cores, tids,
-                                *intelpt_state->tsc_perf_zero_conversion);
+    m_storage.multicore_decoder.emplace(*this);
   }
 
-  m_tsc_conversion = intelpt_state->tsc_perf_zero_conversion;
-  if (m_tsc_conversion) {
+  if (m_storage.tsc_conversion) {
     Log *log = GetLog(LLDBLog::Target);
     LLDB_LOG(log, "TraceIntelPT found TSC conversion information");
   }
@@ -364,10 +363,10 @@
 }
 
 bool TraceIntelPT::IsTraced(lldb::tid_t tid) {
-  RefreshLiveProcessState();
-  if (m_multicore_decoder)
-    return m_multicore_decoder->TracesThread(tid);
-  return m_thread_decoders.count(tid);
+  Storage &storage = GetUpdatedStorage();
+  if (storage.multicore_decoder)
+    return storage.multicore_decoder->TracesThread(tid);
+  return storage.thread_decoders.count(tid);
 }
 
 // The information here should match the description of the intel-pt section
@@ -481,46 +480,4 @@
   return OnThreadBinaryDataRead(tid, IntelPTDataKinds::kTraceBuffer, callback);
 }
 
-TaskTimer &TraceIntelPT::GetTimer() { return m_task_timer; }
-
-Error TraceIntelPT::CreateThreadsFromContextSwitches() {
-  DenseMap<lldb::pid_t, DenseSet<lldb::tid_t>> pids_to_tids;
-
-  for (core_id_t core_id : GetTracedCores()) {
-    Error err = OnCoreBinaryDataRead(
-        core_id, IntelPTDataKinds::kPerfContextSwitchTrace,
-        [&](ArrayRef<uint8_t> data) -> Error {
-          Expected<std::vector<ThreadContinuousExecution>> executions =
-              DecodePerfContextSwitchTrace(data, core_id, *m_tsc_conversion);
-          if (!executions)
-            return executions.takeError();
-          for (const ThreadContinuousExecution &execution : *executions)
-            pids_to_tids[execution.pid].insert(execution.tid);
-          return Error::success();
-        });
-    if (err)
-      return err;
-  }
-
-  DenseMap<lldb::pid_t, Process *> processes;
-  for (Process *proc : GetTracedProcesses())
-    processes.try_emplace(proc->GetID(), proc);
-
-  for (const auto &pid_to_tids : pids_to_tids) {
-    lldb::pid_t pid = pid_to_tids.first;
-    auto it = processes.find(pid);
-    if (it == processes.end())
-      continue;
-
-    Process &process = *it->second;
-    ThreadList &thread_list = process.GetThreadList();
-
-    for (lldb::tid_t tid : pid_to_tids.second) {
-      if (!thread_list.FindThreadByID(tid)) {
-        thread_list.AddThread(std::make_shared<ThreadPostMortemTrace>(
-            process, tid, /*trace_file*/ None));
-      }
-    }
-  }
-  return Error::success();
-}
+TaskTimer &TraceIntelPT::GetTimer() { return GetUpdatedStorage().task_timer; }
Index: lldb/source/Plugins/Process/Linux/Perf.h
===================================================================
--- lldb/source/Plugins/Process/Linux/Perf.h
+++ lldb/source/Plugins/Process/Linux/Perf.h
@@ -80,11 +80,6 @@
 /// Handles the management of the event's file descriptor and mmap'ed
 /// regions.
 class PerfEvent {
-  enum class CollectionState {
-    Enabled,
-    Disabled,
-  };
-
 public:
   /// Create a new performance monitoring event via the perf_event_open syscall.
   ///
@@ -116,7 +111,7 @@
   static llvm::Expected<PerfEvent> Init(perf_event_attr &attr,
                                         llvm::Optional<lldb::pid_t> pid,
                                         llvm::Optional<lldb::core_id_t> cpu,
-                                        llvm::Optional<int> group_fd,
+                                        llvm::Optional<long> group_fd,
                                         unsigned long flags);
 
   /// Create a new performance monitoring event via the perf_event_open syscall
@@ -266,17 +261,21 @@
   ///   data.
   size_t GetEffectiveDataBufferSize() const;
 
+  /// \return
+  ///   \b true if and only the perf event is enabled and collecting.
+  bool IsEnabled() const;
+
 private:
   /// Create new \a PerfEvent.
   ///
   /// \param[in] fd
   ///   File descriptor of the perf event.
   ///
-  /// \param[in] initial_state
+  /// \param[in] enabled
   ///   Initial collection state configured for this perf_event.
-  PerfEvent(long fd, CollectionState initial_state)
+  PerfEvent(long fd, bool enabled)
       : m_fd(new long(fd), resource_handle::FileDescriptorDeleter()),
-        m_collection_state(initial_state) {}
+        m_enabled(enabled) {}
 
   /// Wrapper for \a mmap to provide custom error messages.
   ///
@@ -319,9 +318,21 @@
   /// such as IntelPT.
   resource_handle::MmapUP m_aux_base;
   /// The state of the underlying perf_event.
-  CollectionState m_collection_state;
+  bool m_enabled;
 };
 
+/// Create a perf event that tracks context switches on a cpu.
+///
+/// \param[in] core_id
+///   The core to trace.
+///
+/// \param[in] parent_perf_event
+///   An optional perf event that will be grouped with the
+///   new perf event.
+llvm::Expected<PerfEvent>
+CreateContextSwitchTracePerfEvent(lldb::core_id_t core_id,
+                                  const PerfEvent *parent_perf_event = nullptr);
+
 /// Load \a PerfTscConversionParameters from \a perf_event_mmap_page, if
 /// available.
 llvm::Expected<LinuxPerfZeroTscConversion> LoadPerfTscConversionParameters();
Index: lldb/source/Plugins/Process/Linux/Perf.cpp
===================================================================
--- lldb/source/Plugins/Process/Linux/Perf.cpp
+++ lldb/source/Plugins/Process/Linux/Perf.cpp
@@ -74,7 +74,7 @@
 llvm::Expected<PerfEvent> PerfEvent::Init(perf_event_attr &attr,
                                           Optional<lldb::pid_t> pid,
                                           Optional<lldb::core_id_t> cpu,
-                                          Optional<int> group_fd,
+                                          Optional<long> group_fd,
                                           unsigned long flags) {
   errno = 0;
   long fd = syscall(SYS_perf_event_open, &attr, pid.getValueOr(-1),
@@ -84,8 +84,7 @@
         llvm::formatv("perf event syscall failed: {0}", std::strerror(errno));
     return llvm::createStringError(llvm::inconvertibleErrorCode(), err_msg);
   }
-  return PerfEvent(fd, attr.disabled ? CollectionState::Disabled
-                                     : CollectionState::Enabled);
+  return PerfEvent(fd, !attr.disabled);
 }
 
 llvm::Expected<PerfEvent> PerfEvent::Init(perf_event_attr &attr,
@@ -181,7 +180,12 @@
 
 Expected<std::vector<uint8_t>>
 PerfEvent::ReadFlushedOutDataCyclicBuffer(size_t offset, size_t size) {
-  CollectionState previous_state = m_collection_state;
+  // 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
+  // in https://man7.org/linux/man-pages/man2/perf_event_open.2.html.
+
+  bool was_enabled = m_enabled;
   if (Error err = DisableWithIoctl())
     return std::move(err);
 
@@ -220,7 +224,7 @@
       output.push_back(data[i]);
   }
 
-  if (previous_state == CollectionState::Enabled) {
+  if (was_enabled) {
     if (Error err = EnableWithIoctl())
       return std::move(err);
   }
@@ -236,7 +240,12 @@
 
 Expected<std::vector<uint8_t>>
 PerfEvent::ReadFlushedOutAuxCyclicBuffer(size_t offset, size_t size) {
-  CollectionState previous_state = m_collection_state;
+  // 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
+  // in https://man7.org/linux/man-pages/man2/perf_event_open.2.html.
+
+  bool was_enabled = m_enabled;
   if (Error err = DisableWithIoctl())
     return std::move(err);
 
@@ -271,7 +280,7 @@
   for (uint64_t i = left_part_start; i < aux_head && output.size() < size; i++)
     output.push_back(data[i]);
 
-  if (previous_state == CollectionState::Enabled) {
+  if (was_enabled) {
     if (Error err = EnableWithIoctl())
       return std::move(err);
   }
@@ -286,7 +295,7 @@
 }
 
 Error PerfEvent::DisableWithIoctl() {
-  if (m_collection_state == CollectionState::Disabled)
+  if (!m_enabled)
     return Error::success();
 
   if (ioctl(*m_fd, PERF_EVENT_IOC_DISABLE, PERF_IOC_FLAG_GROUP) < 0)
@@ -294,12 +303,14 @@
                              "Can't disable perf event. %s",
                              std::strerror(errno));
 
-  m_collection_state = CollectionState::Disabled;
+  m_enabled = false;
   return Error::success();
 }
 
+bool PerfEvent::IsEnabled() const { return m_enabled; }
+
 Error PerfEvent::EnableWithIoctl() {
-  if (m_collection_state == CollectionState::Enabled)
+  if (m_enabled)
     return Error::success();
 
   if (ioctl(*m_fd, PERF_EVENT_IOC_ENABLE, PERF_IOC_FLAG_GROUP) < 0)
@@ -307,7 +318,7 @@
                              "Can't enable perf event. %s",
                              std::strerror(errno));
 
-  m_collection_state = CollectionState::Enabled;
+  m_enabled = true;
   return Error::success();
 }
 
@@ -318,3 +329,53 @@
   else
     return mmap_metadata.data_size; // The buffer has wrapped.
 }
+
+Expected<PerfEvent>
+lldb_private::process_linux::CreateContextSwitchTracePerfEvent(
+    lldb::core_id_t core_id, const PerfEvent *parent_perf_event) {
+  Log *log = GetLog(POSIXLog::Trace);
+#ifndef PERF_ATTR_SIZE_VER5
+  return createStringError(inconvertibleErrorCode(),
+                           "Intel PT Linux perf event not supported");
+#else
+  perf_event_attr attr;
+  memset(&attr, 0, sizeof(attr));
+  attr.size = sizeof(attr);
+  attr.sample_type = PERF_SAMPLE_TID | PERF_SAMPLE_TIME;
+  attr.type = PERF_TYPE_SOFTWARE;
+  attr.context_switch = 1;
+  attr.exclude_kernel = 1;
+  attr.sample_id_all = 1;
+  attr.exclude_hv = 1;
+  attr.disabled = parent_perf_event ? !parent_perf_event->IsEnabled() : false;
+
+  // The given perf configuration will produce context switch records of 32
+  // bytes each. Assuming that every context switch will be emitted twice (one
+  // for context switch ins and another one for context switch outs), and that a
+  // context switch will happen at least every half a millisecond per core, we
+  // need 500 * 32 bytes (~16 KB) for a trace of one second, which is much more
+  // than what a regular intel pt trace can get. Pessimistically we pick as
+  // 32KiB for the size of our context switch trace.
+
+  uint64_t data_buffer_size = 32768;
+  uint64_t data_buffer_numpages = data_buffer_size / getpagesize();
+
+  LLDB_LOG(log, "Will create context switch trace buffer of size {0}",
+           data_buffer_size);
+
+  Optional<long> group_fd;
+  if (parent_perf_event)
+    group_fd = parent_perf_event->GetFd();
+
+  if (Expected<PerfEvent> perf_event =
+          PerfEvent::Init(attr, /*pid=*/None, core_id, group_fd, /*flags=*/0)) {
+    if (Error mmap_err = perf_event->MmapMetadataAndBuffers(
+            data_buffer_numpages, 0, /*data_buffer_write=*/false)) {
+      return std::move(mmap_err);
+    }
+    return perf_event;
+  } else {
+    return perf_event.takeError();
+  }
+#endif
+}
Index: lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h
===================================================================
--- lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h
+++ lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h
@@ -40,7 +40,9 @@
   ///     The CPU core id where to trace. If \b None, then this traces all CPUs.
   ///
   /// \param[in] disabled
-  ///     Whether to start the tracing paused.
+  ///     If \b true, then no data is collected until \a Resume is invoked.
+  ///     Similarly, if \b false, data is collected right away until \a Pause is
+  ///     invoked.
   ///
   /// \return
   ///   A \a IntelPTSingleBufferTrace instance if tracing was successful, or
Index: lldb/source/Plugins/Process/Linux/IntelPTProcessTrace.h
===================================================================
--- lldb/source/Plugins/Process/Linux/IntelPTProcessTrace.h
+++ lldb/source/Plugins/Process/Linux/IntelPTProcessTrace.h
@@ -16,7 +16,8 @@
 namespace lldb_private {
 namespace process_linux {
 
-// Abstract class to be inherited by all the process tracing strategies.
+/// Interface to be implemented by each 'process trace' strategy (per core, per
+/// thread, etc).
 class IntelPTProcessTrace {
 public:
   virtual ~IntelPTProcessTrace() = default;
Index: lldb/source/Plugins/Process/Linux/IntelPTPerThreadProcessTrace.h
===================================================================
--- lldb/source/Plugins/Process/Linux/IntelPTPerThreadProcessTrace.h
+++ lldb/source/Plugins/Process/Linux/IntelPTPerThreadProcessTrace.h
@@ -32,7 +32,7 @@
   /// \return
   ///   An \a IntelPTMultiCoreTrace instance if tracing was successful, or
   ///   an \a llvm::Error otherwise.
-  static llvm::Expected<IntelPTProcessTraceUP>
+  static llvm::Expected<std::unique_ptr<IntelPTPerThreadProcessTrace>>
   Start(const TraceIntelPTStartRequest &request,
         llvm::ArrayRef<lldb::tid_t> current_tids);
 
Index: lldb/source/Plugins/Process/Linux/IntelPTPerThreadProcessTrace.cpp
===================================================================
--- lldb/source/Plugins/Process/Linux/IntelPTPerThreadProcessTrace.cpp
+++ lldb/source/Plugins/Process/Linux/IntelPTPerThreadProcessTrace.cpp
@@ -52,10 +52,11 @@
   return m_thread_traces.TryGetBinaryData(request);
 }
 
-Expected<IntelPTProcessTraceUP>
+Expected<std::unique_ptr<IntelPTPerThreadProcessTrace>>
 IntelPTPerThreadProcessTrace::Start(const TraceIntelPTStartRequest &request,
                                     ArrayRef<lldb::tid_t> current_tids) {
-  IntelPTProcessTraceUP trace(new IntelPTPerThreadProcessTrace(request));
+  std::unique_ptr<IntelPTPerThreadProcessTrace> trace(
+      new IntelPTPerThreadProcessTrace(request));
 
   Error error = Error::success();
   for (lldb::tid_t tid : current_tids)
Index: lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.h
===================================================================
--- lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.h
+++ lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.h
@@ -38,7 +38,7 @@
   /// \return
   ///   An \a IntelPTMultiCoreTrace instance if tracing was successful, or
   ///   an \a llvm::Error otherwise.
-  static llvm::Expected<IntelPTProcessTraceUP>
+  static llvm::Expected<std::unique_ptr<IntelPTMultiCoreTrace>>
   StartOnAllCores(const TraceIntelPTStartRequest &request,
                   NativeProcessProtocol &process);
 
@@ -65,7 +65,7 @@
   ///   The perf event collecting context switches for the given core.
   void ForEachCore(std::function<void(lldb::core_id_t core_id,
                                       IntelPTSingleBufferTrace &intelpt_trace,
-                                      PerfEvent &context_switch_trace)>
+                                      ContextSwitchTrace &context_switch_trace)>
                        callback);
 
   void ProcessDidStop() override;
Index: lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
===================================================================
--- lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
+++ lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
@@ -33,55 +33,7 @@
       toString(std::move(error)).c_str());
 }
 
-static Expected<PerfEvent> CreateContextSwitchTracePerfEvent(
-    bool disabled, lldb::core_id_t core_id,
-    IntelPTSingleBufferTrace &intelpt_core_trace) {
-  Log *log = GetLog(POSIXLog::Trace);
-#ifndef PERF_ATTR_SIZE_VER5
-  return createStringError(inconvertibleErrorCode(),
-                           "Intel PT Linux perf event not supported");
-#else
-  perf_event_attr attr;
-  memset(&attr, 0, sizeof(attr));
-  attr.size = sizeof(attr);
-  attr.sample_period = 0;
-  attr.sample_type = PERF_SAMPLE_TID | PERF_SAMPLE_TIME;
-  attr.type = PERF_TYPE_SOFTWARE;
-  attr.context_switch = 1;
-  attr.exclude_kernel = 1;
-  attr.sample_id_all = 1;
-  attr.exclude_hv = 1;
-  attr.disabled = disabled;
-
-  // The given perf configuration will product context switch records of 32
-  // bytes each. Assuming that every context switch will be emitted twice (one
-  // for context switch ins and another one for context switch outs), and that a
-  // context switch will happen at least every half a millisecond per core, we
-  // need 500 * 32 bytes (~16 KB) for a trace of one second, which is much more
-  // than what a regular intel pt trace can get. Pessimistically we pick as
-  // 32KiB for the size of our context switch trace.
-
-  uint64_t data_buffer_size = 32768;
-  uint64_t data_buffer_numpages = data_buffer_size / getpagesize();
-
-  LLDB_LOG(log, "Will create context switch trace buffer of size {0}",
-           data_buffer_size);
-
-  if (Expected<PerfEvent> perf_event = PerfEvent::Init(
-          attr, /*pid=*/None, core_id,
-          intelpt_core_trace.GetPerfEvent().GetFd(), /*flags=*/0)) {
-    if (Error mmap_err = perf_event->MmapMetadataAndBuffers(
-            data_buffer_numpages, 0, /*data_buffer_write=*/false)) {
-      return std::move(mmap_err);
-    }
-    return perf_event;
-  } else {
-    return perf_event.takeError();
-  }
-#endif
-}
-
-Expected<IntelPTProcessTraceUP>
+Expected<std::unique_ptr<IntelPTMultiCoreTrace>>
 IntelPTMultiCoreTrace::StartOnAllCores(const TraceIntelPTStartRequest &request,
                                        NativeProcessProtocol &process) {
   Expected<ArrayRef<core_id_t>> core_ids = GetAvailableLogicalCoreIDs();
@@ -105,8 +57,8 @@
       return IncludePerfEventParanoidMessageInError(core_trace.takeError());
 
     if (Expected<PerfEvent> context_switch_trace =
-            CreateContextSwitchTracePerfEvent(/*disabled=*/true, core_id,
-                                              core_trace.get())) {
+            CreateContextSwitchTracePerfEvent(core_id,
+                                              &core_trace->GetPerfEvent())) {
       traces.try_emplace(core_id,
                          std::make_pair(std::move(*core_trace),
                                         std::move(*context_switch_trace)));
@@ -115,7 +67,7 @@
     }
   }
 
-  return IntelPTProcessTraceUP(
+  return std::unique_ptr<IntelPTMultiCoreTrace>(
       new IntelPTMultiCoreTrace(std::move(traces), process));
 }
 
@@ -129,7 +81,7 @@
 void IntelPTMultiCoreTrace::ForEachCore(
     std::function<void(core_id_t core_id,
                        IntelPTSingleBufferTrace &intelpt_trace,
-                       PerfEvent &context_switch_trace)>
+                       ContextSwitchTrace &context_switch_trace)>
         callback) {
   for (auto &it : m_traces_per_core)
     callback(it.first, it.second.first, it.second.second);
@@ -163,7 +115,7 @@
   state.cores.emplace();
   ForEachCore([&](lldb::core_id_t core_id,
                   const IntelPTSingleBufferTrace &core_trace,
-                  const PerfEvent &context_switch_trace) {
+                  const ContextSwitchTrace &context_switch_trace) {
     state.cores->push_back(
         {core_id,
          {{IntelPTDataKinds::kTraceBuffer, core_trace.GetTraceBufferSize()},
@@ -180,8 +132,12 @@
 }
 
 llvm::Error IntelPTMultiCoreTrace::TraceStart(lldb::tid_t tid) {
-  // This instance is already tracing all threads automatically.
-  return llvm::Error::success();
+  // All the process' threads are being traced automatically.
+  if (!TracesThread(tid))
+    return createStringError(
+        inconvertibleErrorCode(),
+        "Thread %" PRIu64 " is not part of the target process", tid);
+  return Error::success();
 }
 
 Error IntelPTMultiCoreTrace::TraceStop(lldb::tid_t tid) {
Index: lldb/source/Plugins/Process/Linux/IntelPTCollector.h
===================================================================
--- lldb/source/Plugins/Process/Linux/IntelPTCollector.h
+++ lldb/source/Plugins/Process/Linux/IntelPTCollector.h
@@ -74,8 +74,7 @@
                          const TraceIntelPTStartRequest &request);
 
   /// \return
-  ///   The conversion object between TSC and wall time. It caches the result
-  ///   upon success.
+  ///   The conversion object between TSC and wall time.
   llvm::Expected<LinuxPerfZeroTscConversion &>
   FetchPerfTscConversionParameters();
 
@@ -87,9 +86,6 @@
   /// Only one instance of "process trace" can be active at a given time.
   /// It might be \b nullptr.
   IntelPTProcessTraceUP m_process_trace_up;
-
-  /// Cached TSC to and from wall time conversion.
-  llvm::Optional<LinuxPerfZeroTscConversion> m_cached_tsc_conversion;
 };
 
 } // namespace process_linux
Index: lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
===================================================================
--- lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
+++ lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
@@ -37,16 +37,13 @@
 
 llvm::Expected<LinuxPerfZeroTscConversion &>
 IntelPTCollector::FetchPerfTscConversionParameters() {
-  if (!m_cached_tsc_conversion) {
-    if (Expected<LinuxPerfZeroTscConversion> tsc_conversion =
-            LoadPerfTscConversionParameters())
-      m_cached_tsc_conversion = std::move(*tsc_conversion);
-    else
-      return createStringError(inconvertibleErrorCode(),
-                               "Unable to load TSC to wall time conversion: %s",
-                               toString(tsc_conversion.takeError()).c_str());
-  }
-  return *m_cached_tsc_conversion;
+  if (Expected<LinuxPerfZeroTscConversion> tsc_conversion =
+          LoadPerfTscConversionParameters())
+    return *tsc_conversion;
+  else
+    return createStringError(inconvertibleErrorCode(),
+                             "Unable to load TSC to wall time conversion: %s",
+                             toString(tsc_conversion.takeError()).c_str());
 }
 
 Error IntelPTCollector::TraceStop(lldb::tid_t tid) {
Index: lldb/include/lldb/Target/Trace.h
===================================================================
--- lldb/include/lldb/Target/Trace.h
+++ lldb/include/lldb/Target/Trace.h
@@ -296,10 +296,9 @@
                                    OnBinaryDataReadCallback callback);
 
   /// Similar to \a OnCoreBinaryDataRead but this is able to fetch the same data
-  /// from multiple cores at once.
-  llvm::Error OnCoresBinaryDataRead(const std::set<lldb::core_id_t> core_ids,
-                                    llvm::StringRef kind,
-                                    OnCoresBinaryDataReadCallback callback);
+  /// from all cores at once.
+  llvm::Error OnAllCoresBinaryDataRead(llvm::StringRef kind,
+                                       OnCoresBinaryDataReadCallback callback);
 
   /// \return
   ///     All the currently traced processes.
@@ -310,6 +309,11 @@
   ///     plugin.
   llvm::ArrayRef<lldb::core_id_t> GetTracedCores();
 
+  /// Helper method for reading a data file and passing its data to the given
+  /// callback.
+  static llvm::Error OnDataFileRead(FileSpec file,
+                                    OnBinaryDataReadCallback callback);
+
 protected:
   /// Get the currently traced live process.
   ///
@@ -342,10 +346,6 @@
                                              llvm::StringRef kind,
                                              OnBinaryDataReadCallback callback);
 
-  /// Helper method for reading a data file and passing its data to the given
-  /// callback.
-  llvm::Error OnDataFileRead(FileSpec file, OnBinaryDataReadCallback callback);
-
   /// Get the file path containing data of a postmortem thread given a data
   /// identifier.
   ///
@@ -498,7 +498,7 @@
 
   /// Return the list of processes traced by this instance. None of the returned
   /// pointers are invalid.
-  std::vector<Process *> GetTracedProcesses() const;
+  std::vector<Process *> GetTracedProcesses();
 
   /// Method to be invoked by the plug-in to refresh the live process state. It
   /// will invoked DoRefreshLiveProcessState at some point, which should be
@@ -513,52 +513,63 @@
 
 private:
   uint32_t m_stop_id = LLDB_INVALID_STOP_ID;
+
   /// Process traced by this object if doing live tracing. Otherwise it's null.
   Process *m_live_process = nullptr;
 
-  /// Portmortem processes traced by this object if doing non-live tracing.
-  /// Otherwise it's empty.
-  std::vector<Process *> m_postmortem_processes;
-
-  /// These data kinds are returned by lldb-server when fetching the state of
-  /// the tracing session. The size in bytes can be used later for fetching the
-  /// data in batches.
-  /// \{
-
-  /// tid -> data kind -> size
-  llvm::DenseMap<lldb::tid_t, std::unordered_map<std::string, uint64_t>>
-      m_live_thread_data;
-
-  /// core id -> data kind -> size
-  llvm::DenseMap<lldb::core_id_t, std::unordered_map<std::string, uint64_t>>
-      m_live_core_data_sizes;
-  /// core id -> data kind -> bytes
-  llvm::DenseMap<lldb::core_id_t,
-                 std::unordered_map<std::string, std::vector<uint8_t>>>
-      m_live_core_data;
-
-  /// data kind -> size
-  std::unordered_map<std::string, uint64_t> m_live_process_data;
-  /// \}
-
-  /// The list of cores being traced. Might be \b None depending on the plug-in.
-  llvm::Optional<std::vector<lldb::core_id_t>> m_cores;
-
-  /// Postmortem traces can specific additional data files, which are
-  /// represented in this variable using a data kind identifier for each file.
-  /// \{
-
-  /// tid -> data kind -> file
-  llvm::DenseMap<lldb::tid_t, std::unordered_map<std::string, FileSpec>>
-      m_postmortem_thread_data;
-
-  /// core id -> data kind -> file
-  llvm::DenseMap<lldb::core_id_t, std::unordered_map<std::string, FileSpec>>
-      m_postmortem_core_data;
-
-  /// \}
-
-  llvm::Optional<std::string> m_live_refresh_error;
+  /// We package all the data that can change upon process stops to make sure
+  /// this contract is very visible.
+  /// This variable should only be accessed directly by constructores or live
+  /// process data refreshers.
+  struct Storage {
+    /// Portmortem processes traced by this object if doing non-live tracing.
+    /// Otherwise it's empty.
+    std::vector<Process *> postmortem_processes;
+
+    /// These data kinds are returned by lldb-server when fetching the state of
+    /// the tracing session. The size in bytes can be used later for fetching
+    /// the data in batches.
+    /// \{
+
+    /// tid -> data kind -> size
+    llvm::DenseMap<lldb::tid_t, std::unordered_map<std::string, uint64_t>>
+        live_thread_data;
+
+    /// core id -> data kind -> size
+    llvm::DenseMap<lldb::core_id_t, std::unordered_map<std::string, 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>>>
+        live_core_data;
+
+    /// data kind -> size
+    std::unordered_map<std::string, uint64_t> live_process_data;
+    /// \}
+
+    /// The list of cores being traced. Might be \b None depending on the
+    /// plug-in.
+    llvm::Optional<std::vector<lldb::core_id_t>> cores;
+
+    /// Postmortem traces can specific additional data files, which are
+    /// represented in this variable using a data kind identifier for each file.
+    /// \{
+
+    /// tid -> data kind -> file
+    llvm::DenseMap<lldb::tid_t, std::unordered_map<std::string, FileSpec>>
+        postmortem_thread_data;
+
+    /// core id -> data kind -> file
+    llvm::DenseMap<lldb::core_id_t, std::unordered_map<std::string, FileSpec>>
+        postmortem_core_data;
+
+    /// \}
+
+    llvm::Optional<std::string> live_refresh_error;
+  } m_storage;
+
+  /// Get the storage after refreshing the data in the case of a live process.
+  Storage &GetUpdatedStorage();
 };
 
 } // namespace lldb_private
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to