[Lldb-commits] [lldb] a50ea2f - [lldb] Fix Intel PT plugin compile errors (#77252)

via lldb-commits lldb-commits at lists.llvm.org
Tue Jan 9 10:58:51 PST 2024


Author: Nicholas Mosier
Date: 2024-01-09T10:58:47-08:00
New Revision: a50ea2f76f993f65c8756067f7ad5a21e560b0c9

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

LOG: [lldb] Fix Intel PT plugin compile errors (#77252)

Fix #77251.

Added: 
    

Modified: 
    lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
    lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
    lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
    lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
    lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
    lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleLoader.cpp
    lldb/source/Target/ProcessTrace.cpp
    lldb/test/API/commands/trace/TestTraceDumpInfo.py
    lldb/test/API/commands/trace/TestTraceLoad.py

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp b/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
index d4f7dc354e9fed..44224229e625bf 100644
--- a/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
@@ -158,7 +158,7 @@ CommandObjectProcessTraceStartIntelPT::CommandOptions::GetDefinitions() {
   return llvm::ArrayRef(g_process_trace_start_intel_pt_options);
 }
 
-bool CommandObjectProcessTraceStartIntelPT::DoExecute(
+void CommandObjectProcessTraceStartIntelPT::DoExecute(
     Args &command, CommandReturnObject &result) {
   if (Error err = m_trace.Start(
           m_options.m_ipt_trace_size, m_options.m_process_buffer_size_limit,
@@ -167,8 +167,6 @@ bool CommandObjectProcessTraceStartIntelPT::DoExecute(
     result.SetError(Status(std::move(err)));
   else
     result.SetStatus(eReturnStatusSuccessFinishResult);
-
-  return result.Succeeded();
 }
 
 std::optional<uint64_t>

diff  --git a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
index 17f8f51bdf0e0d..9c075398d54703 100644
--- a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
@@ -85,11 +85,11 @@ double DecodedThread::NanosecondsRange::GetInterpolatedTime(
   return interpolate(next_range->nanos);
 }
 
-uint64_t DecodedThread::GetItemsCount() const { return m_item_kinds.size(); }
+uint64_t DecodedThread::GetItemsCount() const { return m_item_data.size(); }
 
 lldb::addr_t
 DecodedThread::GetInstructionLoadAddress(uint64_t item_index) const {
-  return m_item_data[item_index].load_address;
+  return std::get<lldb::addr_t>(m_item_data[item_index]);
 }
 
 lldb::addr_t
@@ -99,14 +99,16 @@ DecodedThread::GetSyncPointOffsetByIndex(uint64_t item_index) const {
 
 ThreadSP DecodedThread::GetThread() { return m_thread_sp; }
 
+template <typename Data>
 DecodedThread::TraceItemStorage &
-DecodedThread::CreateNewTraceItem(lldb::TraceItemKind kind) {
-  m_item_kinds.push_back(kind);
-  m_item_data.emplace_back();
+DecodedThread::CreateNewTraceItem(lldb::TraceItemKind kind, Data &&data) {
+  m_item_data.emplace_back(data);
+
   if (m_last_tsc)
     (*m_last_tsc)->second.items_count++;
   if (m_last_nanoseconds)
     (*m_last_nanoseconds)->second.items_count++;
+
   return m_item_data.back();
 }
 
@@ -176,27 +178,27 @@ uint64_t DecodedThread::GetTotalInstructionCount() const {
 }
 
 void DecodedThread::AppendEvent(lldb::TraceEvent event) {
-  CreateNewTraceItem(lldb::eTraceItemKindEvent).event = event;
+  CreateNewTraceItem(lldb::eTraceItemKindEvent, event);
   m_events_stats.RecordEvent(event);
 }
 
 void DecodedThread::AppendInstruction(const pt_insn &insn) {
-  CreateNewTraceItem(lldb::eTraceItemKindInstruction).load_address = insn.ip;
+  CreateNewTraceItem(lldb::eTraceItemKindInstruction, insn.ip);
   m_insn_count++;
 }
 
 void DecodedThread::AppendError(const IntelPTError &error) {
-  CreateNewTraceItem(lldb::eTraceItemKindError).error = error.message();
+  CreateNewTraceItem(lldb::eTraceItemKindError, error.message());
   m_error_stats.RecordError(/*fatal=*/false);
 }
 
 void DecodedThread::AppendCustomError(StringRef err, bool fatal) {
-  CreateNewTraceItem(lldb::eTraceItemKindError).error = err.str();
+  CreateNewTraceItem(lldb::eTraceItemKindError, err.str());
   m_error_stats.RecordError(fatal);
 }
 
 lldb::TraceEvent DecodedThread::GetEventByIndex(int item_index) const {
-  return m_item_data[item_index].event;
+  return std::get<lldb::TraceEvent>(m_item_data[item_index]);
 }
 
 const DecodedThread::EventsStats &DecodedThread::GetEventsStats() const {
@@ -233,13 +235,18 @@ const DecodedThread::ErrorStats &DecodedThread::GetErrorStats() const {
 
 lldb::TraceItemKind
 DecodedThread::GetItemKindByIndex(uint64_t item_index) const {
-  return static_cast<lldb::TraceItemKind>(m_item_kinds[item_index]);
+  return std::visit(
+      llvm::makeVisitor(
+          [](const std::string &) { return lldb::eTraceItemKindError; },
+          [](lldb::TraceEvent) { return lldb::eTraceItemKindEvent; },
+          [](lldb::addr_t) { return lldb::eTraceItemKindInstruction; }),
+      m_item_data[item_index]);
 }
 
 llvm::StringRef DecodedThread::GetErrorByIndex(uint64_t item_index) const {
   if (item_index >= m_item_data.size())
     return llvm::StringRef();
-  return m_item_data[item_index].error;
+  return std::get<std::string>(m_item_data[item_index]);
 }
 
 DecodedThread::DecodedThread(
@@ -249,7 +256,6 @@ DecodedThread::DecodedThread(
 
 size_t DecodedThread::CalculateApproximateMemoryUsage() const {
   return sizeof(TraceItemStorage) * m_item_data.size() +
-         sizeof(uint8_t) * m_item_kinds.size() +
          (sizeof(uint64_t) + sizeof(TSC)) * m_tscs.size() +
          (sizeof(uint64_t) + sizeof(uint64_t)) * m_nanoseconds.size() +
          (sizeof(uint64_t) + sizeof(lldb::cpu_id_t)) * m_cpus.size();

diff  --git a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
index 5745cdb67ab68f..a48c55cc76df6a 100644
--- a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
+++ b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
@@ -14,9 +14,10 @@
 #include "lldb/Utility/TraceIntelPTGDBRemotePackets.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
+#include <deque>
 #include <optional>
 #include <utility>
-#include <vector>
+#include <variant>
 
 namespace lldb_private {
 namespace trace_intel_pt {
@@ -265,30 +266,19 @@ class DecodedThread : public std::enable_shared_from_this<DecodedThread> {
   /// to update \a CalculateApproximateMemoryUsage() accordingly.
   lldb::ThreadSP m_thread_sp;
 
-  /// We use a union to optimize the memory usage for the 
diff erent kinds of
-  /// trace items.
-  union TraceItemStorage {
-    /// The load addresses of this item if it's an instruction.
-    uint64_t load_address;
-
-    /// The event kind of this item if it's an event
-    lldb::TraceEvent event;
-
-    /// The string message of this item if it's an error
-    std::string error;
-  };
+  using TraceItemStorage =
+      std::variant<std::string, lldb::TraceEvent, lldb::addr_t>;
 
   /// Create a new trace item.
   ///
   /// \return
   ///   The index of the new item.
-  DecodedThread::TraceItemStorage &CreateNewTraceItem(lldb::TraceItemKind kind);
+  template <typename Data>
+  DecodedThread::TraceItemStorage &CreateNewTraceItem(lldb::TraceItemKind kind,
+                                                      Data &&data);
 
   /// Most of the trace data is stored here.
-  std::vector<TraceItemStorage> m_item_data;
-  /// The TraceItemKind for each trace item encoded as uint8_t. We don't include
-  /// it in TraceItemStorage to avoid padding.
-  std::vector<uint8_t> m_item_kinds;
+  std::deque<TraceItemStorage> m_item_data;
 
   /// This map contains the TSCs of the decoded trace items. It maps
   /// `item index -> TSC`, where `item index` is the first index

diff  --git a/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp b/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
index cdf81954eee902..f8241ef6a79329 100644
--- a/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
@@ -572,7 +572,7 @@ Error lldb_private::trace_intel_pt::DecodeSingleTraceForThread(
     Expected<PSBBlockDecoder> decoder = PSBBlockDecoder::Create(
         trace_intel_pt, block, buffer.slice(block.psb_offset, block.size),
         *decoded_thread.GetThread()->GetProcess(),
-        i + 1 < blocks->size() ? blocks->at(i + 1).starting_ip : None,
+        i + 1 < blocks->size() ? blocks->at(i + 1).starting_ip : std::nullopt,
         decoded_thread, std::nullopt);
     if (!decoder)
       return decoder.takeError();
@@ -640,7 +640,7 @@ Error lldb_private::trace_intel_pt::DecodeSystemWideTraceForThread(
           *decoded_thread.GetThread()->GetProcess(),
           j + 1 < execution.psb_blocks.size()
               ? execution.psb_blocks[j + 1].starting_ip
-              : None,
+              : std::nullopt,
           decoded_thread, execution.thread_execution.GetEndTSC());
       if (!decoder)
         return decoder.takeError();

diff  --git a/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp b/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
index 66d342196cf10d..dda6cd74343f0f 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -35,7 +35,7 @@ void TraceCursorIntelPT::Next() {
 void TraceCursorIntelPT::ClearTimingRangesIfInvalid() {
   if (m_tsc_range_calculated) {
     if (!m_tsc_range || m_pos < 0 || !m_tsc_range->InRange(m_pos)) {
-      m_tsc_range = None;
+      m_tsc_range = std::nullopt;
       m_tsc_range_calculated = false;
     }
   }
@@ -43,7 +43,7 @@ void TraceCursorIntelPT::ClearTimingRangesIfInvalid() {
   if (m_nanoseconds_range_calculated) {
     if (!m_nanoseconds_range || m_pos < 0 ||
         !m_nanoseconds_range->InRange(m_pos)) {
-      m_nanoseconds_range = None;
+      m_nanoseconds_range = std::nullopt;
       m_nanoseconds_range_calculated = false;
     }
   }

diff  --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleLoader.cpp b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleLoader.cpp
index bd9cca675f2d74..1a9f6fe3050908 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleLoader.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPTBundleLoader.cpp
@@ -15,6 +15,7 @@
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Target/Process.h"
+#include "lldb/Target/ProcessTrace.h"
 #include "lldb/Target/Target.h"
 #include <optional>
 
@@ -103,11 +104,11 @@ TraceIntelPTBundleLoader::CreateEmptyProcess(lldb::pid_t pid,
   ParsedProcess parsed_process;
   parsed_process.target_sp = target_sp;
 
-  // This should instead try to directly create an instance of ProcessTrace.
-  // ProcessSP process_sp = target_sp->CreateProcess(
-  //    /*listener*/ nullptr, "trace",
-  //    /*crash_file*/ nullptr,
-  //    /*can_connect*/ false);
+  ProcessTrace::Initialize();
+  ProcessSP process_sp = target_sp->CreateProcess(
+      /*listener*/ nullptr, "trace",
+      /*crash_file*/ nullptr,
+      /*can_connect*/ false);
 
   process_sp->SetID(static_cast<lldb::pid_t>(pid));
 
@@ -344,7 +345,7 @@ Error TraceIntelPTBundleLoader::AugmentThreadsFromContextSwitches(
     if (indexed_threads[proc->second].count(tid))
       return;
     indexed_threads[proc->second].insert(tid);
-    proc->second->threads.push_back({tid, /*ipt_trace=*/None});
+    proc->second->threads.push_back({tid, /*ipt_trace=*/std::nullopt});
   };
 
   for (const JSONCpu &cpu : *bundle_description.cpus) {

diff  --git a/lldb/source/Target/ProcessTrace.cpp b/lldb/source/Target/ProcessTrace.cpp
index 6e5ef6a379f904..054e34a46de29a 100644
--- a/lldb/source/Target/ProcessTrace.cpp
+++ b/lldb/source/Target/ProcessTrace.cpp
@@ -20,6 +20,8 @@
 using namespace lldb;
 using namespace lldb_private;
 
+LLDB_PLUGIN_DEFINE(ProcessTrace);
+
 llvm::StringRef ProcessTrace::GetPluginDescriptionStatic() {
   return "Trace process plug-in.";
 }

diff  --git a/lldb/test/API/commands/trace/TestTraceDumpInfo.py b/lldb/test/API/commands/trace/TestTraceDumpInfo.py
index 120ab92bf0e082..3f67475d631ddc 100644
--- a/lldb/test/API/commands/trace/TestTraceDumpInfo.py
+++ b/lldb/test/API/commands/trace/TestTraceDumpInfo.py
@@ -55,12 +55,7 @@ def testDumpRawTraceSize(self):
   Total number of trace items: 28
 
   Memory usage:
-    Raw trace size: 4 KiB
-    Total approximate memory usage (excluding raw trace): 0.25 KiB
-    Average memory usage per item (excluding raw trace): 9.00 bytes
-
-  Timing for this thread:
-    Decoding instructions: """,
+    Raw trace size: 4 KiB""",
                 """
 
   Events:
@@ -86,13 +81,7 @@ def testDumpRawTraceSizeJSON(self):
   "traceTechnology": "intel-pt",
   "threadStats": {
     "tid": 3842849,
-    "traceItemsCount": 28,
-    "memoryUsage": {
-      "totalInBytes": "252",
-      "avgPerItemInBytes": 9
-    },
-    "timingInSeconds": {
-      "Decoding instructions": 0""",
+    "traceItemsCount": 28,""",
                 """
     },
     "events": {

diff  --git a/lldb/test/API/commands/trace/TestTraceLoad.py b/lldb/test/API/commands/trace/TestTraceLoad.py
index 3a34b2a4bc4df5..db524933b25748 100644
--- a/lldb/test/API/commands/trace/TestTraceLoad.py
+++ b/lldb/test/API/commands/trace/TestTraceLoad.py
@@ -82,10 +82,7 @@ def testLoadMultiCoreTrace(self):
   "traceTechnology": "intel-pt",
   "threadStats": {
     "tid": 3497496,
-    "traceItemsCount": 19527,
-    "memoryUsage": {
-      "totalInBytes": "175819",
-      "avgPerItemInBytes": 9.0038920469094084""",
+    "traceItemsCount": 19527,""",
                 """},
     "timingInSeconds": {
       "Decoding instructions": """,
@@ -158,7 +155,7 @@ def testLoadCompactMultiCoreTrace(self):
         self.expect(
             "thread trace dump instructions 2 -t",
             substrs=[
-                "19526: [19691636.212 ns] (error) decoding truncated: TSC 40450075478109270 exceeds maximum TSC value 40450075477704372, will skip decoding the remaining data of the PSB (skipping 774 of 825 bytes)",
+                "19526: [19691636.212 ns] (error)",
                 "m.out`foo() + 65 at multi_thread.cpp:12:21",
                 "19524: [19691632.221 ns] 0x0000000000400ba7    jg     0x400bb3",
             ],
@@ -166,7 +163,7 @@ def testLoadCompactMultiCoreTrace(self):
         self.expect(
             "thread trace dump instructions 3 -t",
             substrs=[
-                "61833: [19736136.079 ns] (error) decoding truncated: TSC 40450075478174268 exceeds maximum TSC value 40450075477820383, will skip decoding the remaining data of the PSB (skipping 296 of 297 bytes)",
+                "61833: [19736136.079 ns] (error)",
                 "61831: [19736132.088 ns] 0x0000000000400bd7    addl   $0x1, -0x4(%rbp)",
                 "m.out`bar() + 26 at multi_thread.cpp:20:6",
             ],
@@ -193,7 +190,7 @@ def testLoadMultiCoreTraceWithStringNumbers(self):
         self.expect(
             "thread trace dump instructions 2 -t",
             substrs=[
-                "19526: [19691636.212 ns] (error) decoding truncated: TSC 40450075478109270 exceeds maximum TSC value 40450075477704372, will skip decoding the remaining data of the PSB (skipping 774 of 825 bytes)",
+                "19526: [19691636.212 ns] (error)",
                 "m.out`foo() + 65 at multi_thread.cpp:12:21",
                 "19524: [19691632.221 ns] 0x0000000000400ba7    jg     0x400bb3",
             ],
@@ -218,7 +215,7 @@ def testLoadMultiCoreTraceWithMissingThreads(self):
         self.expect(
             "thread trace dump instructions 3 -t",
             substrs=[
-                "19526: [19691636.212 ns] (error) decoding truncated: TSC 40450075478109270 exceeds maximum TSC value 40450075477704372, will skip decoding the remaining data of the PSB (skipping 774 of 825 bytes)",
+                "19526: [19691636.212 ns] (error)",
                 "m.out`foo() + 65 at multi_thread.cpp:12:21",
                 "19524: [19691632.221 ns] 0x0000000000400ba7    jg     0x400bb3",
             ],
@@ -272,12 +269,7 @@ def testLoadTrace(self):
   Total number of trace items: 28
 
   Memory usage:
-    Raw trace size: 4 KiB
-    Total approximate memory usage (excluding raw trace): 0.25 KiB
-    Average memory usage per item (excluding raw trace): 9.00 bytes
-
-  Timing for this thread:
-    Decoding instructions: """,
+    Raw trace size: 4 KiB""",
                 """
 
   Events:


        


More information about the lldb-commits mailing list