[trace][intelpt] Remove code smell when printing the raw trace size

[wallace]

Summary

Something ugly I did was to report the trace buffer size to the DecodedThread,
which is later used as part of the dump info command. Instead of doing that,
we can just directly ask the trace for the raw buffer and print its size.

I thought about not asking for the entire trace but instead just for its size,
but in this case, as our traces as not extremely big, I prefer to ask for the
entire trace, ensuring it could be fetched, and then print its size.

[jj10306]

Looks good. Any reason not to have GetRawTraceSize() at the Trace interface level instead of being specific to TraceIntelPT?

[wallace]

Well, not all theoretical trace plugins might have raw trace sizes.

[wallace]

Committed as 44103c96fa6b00e7824319de1b10ce26781e3852

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

public:
   /// \return
   ///   The error message of \b nullptr if the given index
   ///   points to a valid instruction.
   const char *GetErrorByInstructionIndex(size_t ins_idx);
 
   /// Get a new cursor for the decoded thread.
   lldb::TraceCursorUP GetCursor();
 
-  /// Set the size in bytes of the corresponding Intel PT raw trace.
-  void SetRawTraceSize(size_t size);
-
-  /// Get the size in bytes of the corresponding Intel PT raw trace.
-  ///
-  /// \return
-  ///   The size of the trace, or \b llvm::None if not available.
-  llvm::Optional<size_t> GetRawTraceSize() const;
-
   /// Return the number of TSC decoding errors that happened. A TSC error
   /// is not a fatal error and doesn't create gaps in the trace. Instead
   /// we only keep track of them as a statistic.
   ///
   /// \return
   ///   The number of TSC decoding errors.
   const LibiptErrors &GetTscErrors() const;
 

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

void IntelPTError::log(llvm::raw_ostream &OS) const {
   const char *libipt_error_message = pt_errstr(pt_errcode(m_libipt_error_code));
   if (m_address != LLDB_INVALID_ADDRESS && m_address > 0) {
     write_hex(OS, m_address, HexPrintStyle::PrefixLower, 18);
     OS << "    ";
   }
   OS << "error: " << libipt_error_message;
 }
 
-Optional<size_t> DecodedThread::GetRawTraceSize() const {
-  return m_raw_trace_size;
-}
-
 size_t DecodedThread::GetInstructionsCount() const {
   return m_instruction_ips.size();
 }
 
 lldb::addr_t DecodedThread::GetInstructionLoadAddress(size_t insn_index) const {
   return m_instruction_ips[insn_index];
 }
 

 
 DecodedThread::DecodedThread(ThreadSP thread_sp) : m_thread_sp(thread_sp) {}
 
 DecodedThread::DecodedThread(ThreadSP thread_sp, Error &&error)
     : m_thread_sp(thread_sp) {
   AppendError(std::move(error));
 }
 
-void DecodedThread::SetRawTraceSize(size_t size) { m_raw_trace_size = size; }
-
 lldb::TraceCursorUP DecodedThread::GetCursor() {
   // We insert a fake error signaling an empty trace if needed becasue the
   // TraceCursor requires non-empty traces.
   if (m_instruction_ips.empty())
     AppendError(createStringError(inconvertibleErrorCode(), "empty trace"));
   return std::make_unique<TraceCursorIntelPT>(m_thread_sp, shared_from_this());
 }
 

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

if (IsLibiptError(
           status = pt_image_set_callback(image, ReadProcessMemory, process)))
     return make_error<IntelPTError>(status);
   return decoder_up;
 }
 
 void lldb_private::trace_intel_pt::DecodeTrace(DecodedThread &decoded_thread,
                                                TraceIntelPT &trace_intel_pt,
                                                ArrayRef<uint8_t> buffer) {
-  decoded_thread.SetRawTraceSize(buffer.size());
-
   Expected<PtInsnDecoderUP> decoder_up =
       CreateInstructionDecoder(decoded_thread, trace_intel_pt, buffer);
   if (!decoder_up)
     return decoded_thread.AppendError(decoder_up.takeError());
 
   LibiptDecoder libipt_decoder(*decoder_up.get(), decoded_thread);
   libipt_decoder.DecodeUntilEndOfTrace();
 }

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

public:
   GetThreadTraceStartCommand(CommandInterpreter &interpreter) override;
 
   llvm::StringRef GetSchema() override;
 
   lldb::TraceCursorUP GetCursor(Thread &thread) override;
 
   void DumpTraceInfo(Thread &thread, Stream &s, bool verbose) override;
 
-  llvm::Optional<size_t> GetRawTraceSize(Thread &thread);
+  llvm::Expected<size_t> GetRawTraceSize(Thread &thread);
 
   void DoRefreshLiveProcessState(
       llvm::Expected<TraceGetStateResponse> state) override;
 
   bool IsTraced(lldb::tid_t tid) override;
 
   const char *GetStartConfigurationHelp() override;
 

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

DecodedThreadSP TraceIntelPT::Decode(Thread &thread) {
   return it->second->Decode();
 }
 
 lldb::TraceCursorUP TraceIntelPT::GetCursor(Thread &thread) {
   return Decode(thread)->GetCursor();
 }
 
 void TraceIntelPT::DumpTraceInfo(Thread &thread, Stream &s, bool verbose) {
-  Optional<size_t> raw_size = GetRawTraceSize(thread);
+  lldb::tid_t tid = thread.GetID();
   s.Format("\nthread #{0}: tid = {1}", thread.GetIndexID(), thread.GetID());
-  if (!raw_size) {
+  if (!IsTraced(tid)) {
     s << ", not traced\n";
     return;
   }
   s << "\n";
+
+  Expected<size_t> raw_size = GetRawTraceSize(thread);
+  if (!raw_size) {
+    s.Format("  {0}\n", toString(raw_size.takeError()));
+    return;
+  }
+
   DecodedThreadSP decoded_trace_sp = Decode(thread);
   size_t insn_len = decoded_trace_sp->GetInstructionsCount();
   size_t mem_used = decoded_trace_sp->CalculateApproximateMemoryUsage();
 
   s.Format("  Total number of instructions: {0}\n", insn_len);
 
-  s.PutCString("\n  Memory usage:\n");
+  s << "\n  Memory usage:\n";
   s.Format("    Raw trace size: {0} KiB\n", *raw_size / 1024);
   s.Format(
       "    Total approximate memory usage (excluding raw trace): {0:2} KiB\n",
       (double)mem_used / 1024);
   if (insn_len != 0)
     s.Format("    Average memory usage per instruction (excluding raw trace): "
              "{0:2} bytes\n",
              (double)mem_used / insn_len);
 
-  s.PutCString("\n  Timing:\n");
-  GetTimer()
-      .ForThread(thread.GetID())
-      .ForEachTimedTask(
-          [&](const std::string &name, std::chrono::milliseconds duration) {
-            s.Format("    {0}: {1:2}s\n", name, duration.count() / 1000.0);
-          });
+  s << "\n  Timing:\n";
+  GetTimer().ForThread(tid).ForEachTimedTask(
 
-  s.PutCString("\n  Errors:\n");
+  s << "\n  Errors:\n";
   const DecodedThread::LibiptErrors &tsc_errors =
       decoded_trace_sp->GetTscErrors();
   s.Format("    Number of TSC decoding errors: {0}\n", tsc_errors.total_count);
   for (const auto &error_message_to_count : tsc_errors.libipt_errors) {
     s.Format("      {0}: {1}\n", error_message_to_count.first,
              error_message_to_count.second);
   }
 }
 
-Optional<size_t> TraceIntelPT::GetRawTraceSize(Thread &thread) {
-  if (IsTraced(thread.GetID()))
-    return Decode(thread)->GetRawTraceSize();
-  else
-    return None;
+llvm::Expected<size_t> TraceIntelPT::GetRawTraceSize(Thread &thread) {
+  size_t size;
+  auto callback = [&](llvm::ArrayRef<uint8_t> data) {
+    size = data.size();
+    return Error::success();
+  };
+  if (Error err = OnThreadBufferRead(thread.GetID(), callback))
+    return std::move(err);
+
+  return size;
 }
 
 Expected<pt_cpu> TraceIntelPT::GetCPUInfoForLiveProcess() {
   Expected<std::vector<uint8_t>> cpu_info = GetLiveProcessBinaryData("cpuInfo");
   if (!cpu_info)
     return cpu_info.takeError();
 
   int64_t cpu_family = -1;