[Lldb-commits] [PATCH] D104422: [trace] Add a TraceCursor class

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

Just one issue regarding the stop ID for post mortem. Once this is resolved, we will be good to go!



================
Comment at: lldb/include/lldb/Target/Trace.h:292
+  /// \return
+  ///     The stop ID of the live process being traced, or \b 0 if this is a
+  ///     post-mortem trace session.
----------------
zero is invalid. We should just have post mortem use a valid stop ID like 1


================
Comment at: lldb/include/lldb/Target/Trace.h:367
 
+  uint32_t m_stop_id = 0;
   /// Process traced by this object if doing live tracing. Otherwise it's null.
----------------
As per my comment above, we will need to set this to 1 for post mortem, or maybe the process in post mortem already has its stop ID set to 1 so this will fix itself?


================
Comment at: lldb/include/lldb/Target/TraceCursor.h:77
+  /// \return
+  ///     \b true if the cursor moved, \b false otherwise.
+  virtual bool Next(lldb::TraceInstructionControlFlowType granularity =
----------------
Is the cursor in an error state in the case where we return false? It should be and we should document it


================
Comment at: lldb/source/Target/Trace.cpp:478
+uint32_t Trace::GetStopID() {
+  RefreshLiveProcessState();
+  return m_stop_id;
----------------
We should make sure for core file that the m_stop_id is set correctly somehow


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104422/new/

https://reviews.llvm.org/D104422