[lldb][NFCI] DecodedThread::TraceItemStorage::error should own its own data

[bulbazord]

Summary

The way it works now, it stores a const char * that it does not
explicitly own. It's owned by the ConstString StringPool. This is purely
to manage its lifetime, we don't really benefit from deduplication (nor
should we try to, they are errors). We also don't really benefit from
quick comparisons.

This may make the size of TraceItemStorage larger, but you have to pay
the cost of owning the data somewhere. The ConstString StringPool is an
attractive choice but ultimately a poor one.

[wallace]

lgtm!

lldb/include/lldb/Target/TraceCursor.h

public:
   virtual lldb::TraceItemKind GetItemKind() const = 0;
 
   /// \return
   ///     Whether the cursor points to an error or not.
   bool IsError() const;
 
   /// \return
   ///     The error message the cursor is pointing at.
-  virtual const char *GetError() const = 0;
+  virtual llvm::StringRef GetError() const = 0;
 
   /// \return
   ///     Whether the cursor points to an event or not.
   bool IsEvent() const;
 
   /// \return
   ///     The specific kind of event the cursor is pointing at.
   virtual lldb::TraceEvent GetEventType() const = 0;

lldb/source/Plugins/Trace/intel-pt/DecodedThread.h

DecodedThread(
       const std::optional<LinuxPerfZeroTscConversion> &tsc_conversion);
 
   /// Get the total number of instruction, errors and events from the decoded
   /// trace.
   uint64_t GetItemsCount() const;
 
   /// \return
   ///   The error associated with a given trace item.
-  const char *GetErrorByIndex(uint64_t item_index) const;
+  llvm::StringRef GetErrorByIndex(uint64_t item_index) const;
 
   /// \return
   ///   The trace item kind given an item index.
   lldb::TraceItemKind GetItemKindByIndex(uint64_t item_index) const;
 
   /// \return
   ///   The underlying event type for the given trace item index.
   lldb::TraceEvent GetEventByIndex(int item_index) const;
private:
   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
-    const char *error;
+    std::string error;
   };
 
   /// Create a new trace item.
   ///
   /// \return
   ///   The index of the new item.
   DecodedThread::TraceItemStorage &CreateNewTraceItem(lldb::TraceItemKind kind);
 

lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp

 }
 
 void DecodedThread::AppendInstruction(const pt_insn &insn) {
   CreateNewTraceItem(lldb::eTraceItemKindInstruction).load_address = insn.ip;
   m_insn_count++;
 }
 
 void DecodedThread::AppendError(const IntelPTError &error) {
-  CreateNewTraceItem(lldb::eTraceItemKindError).error =
-      ConstString(error.message()).AsCString();
+  CreateNewTraceItem(lldb::eTraceItemKindError).error = error.message();
   m_error_stats.RecordError(/*fatal=*/false);
 }
 
 void DecodedThread::AppendCustomError(StringRef err, bool fatal) {
-  CreateNewTraceItem(lldb::eTraceItemKindError).error =
-      ConstString(err).AsCString();
+  CreateNewTraceItem(lldb::eTraceItemKindError).error = err.str();
   m_error_stats.RecordError(fatal);
 }
 
 lldb::TraceEvent DecodedThread::GetEventByIndex(int item_index) const {
   return m_item_data[item_index].event;
 }
 
 const DecodedThread::EventsStats &DecodedThread::GetEventsStats() const {
const DecodedThread::ErrorStats &DecodedThread::GetErrorStats() const {
   return m_error_stats;
 }
 
 lldb::TraceItemKind
 DecodedThread::GetItemKindByIndex(uint64_t item_index) const {
   return static_cast<lldb::TraceItemKind>(m_item_kinds[item_index]);
 }
 
-const char *DecodedThread::GetErrorByIndex(uint64_t item_index) const {
+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;
 }
 
 DecodedThread::DecodedThread(
     ThreadSP thread_sp,
     const std::optional<LinuxPerfZeroTscConversion> &tsc_conversion)
     : m_thread_sp(thread_sp), m_tsc_conversion(tsc_conversion) {}
 
 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();
 }

lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h

TraceCursorIntelPT(
       std::optional<uint64_t> beginning_of_time_nanos);
 
   bool Seek(int64_t offset, lldb::TraceCursorSeekType origin) override;
 
   void Next() override;
 
   bool HasValue() const override;
 
-  const char *GetError() const override;
+  llvm::StringRef GetError() const override;
 
   lldb::addr_t GetLoadAddress() const override;
 
   lldb::TraceEvent GetEventType() const override;
 
   lldb::cpu_id_t GetCPU() const override;
 
   std::optional<uint64_t> GetHWClock() const override;

lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp

bool TraceCursorIntelPT::HasValue() const {
   return m_pos >= 0 &&
          static_cast<uint64_t>(m_pos) < m_decoded_thread_sp->GetItemsCount();
 }
 
 lldb::TraceItemKind TraceCursorIntelPT::GetItemKind() const {
   return m_decoded_thread_sp->GetItemKindByIndex(m_pos);
 }
 
-const char *TraceCursorIntelPT::GetError() const {
+llvm::StringRef TraceCursorIntelPT::GetError() const {
   return m_decoded_thread_sp->GetErrorByIndex(m_pos);
 }
 
 lldb::addr_t TraceCursorIntelPT::GetLoadAddress() const {
   return m_decoded_thread_sp->GetInstructionLoadAddress(m_pos);
 }
 
 std::optional<uint64_t> TraceCursorIntelPT::GetHWClock() const {