[lldb] Defer checking CFA memory region (swiftlang#7877)

To support stepping in Swift async code, LLDB uses the memory region (ie stack vs heap) 
of the Canonical Frame Address (CFA) to identify when an async function calls a 
synchronous function.

But this has resulted in a performance regression, as determining the memory region of 
the CFA is not free, and eagerly performing this query for every `StackID`, 
particularly during backtracing/unwinding – when stepping is not yet occurring, is 
wasteful.

This change proposes a solution that defers the memory region query until required to 
perform stepping logic. The logic that checks heaps vs stacks was moved into 
`ThreadPlan`, the only place that currently needs to make that determination. 
Performing this check in `ThreadPlan` provides access to the process, allowing 
stack/heap determination to happen as needed. Previously, the stack/heap determination 
was made eagerly when the `StackID` was constructed. 

rdar://117505613

Author: kastiglione

lldb/include/lldb/Target/StackID.h

@@ -36,7 +36,7 @@ class StackID {
   void Clear() {
     m_pc = LLDB_INVALID_ADDRESS;
     m_cfa = LLDB_INVALID_ADDRESS;
-    m_cfa_on_stack = true;
+    m_cfa_on_stack = eLazyBoolCalculate;
     m_symbol_scope = nullptr;
   }
 
@@ -57,15 +57,18 @@ class StackID {
     return *this;
   }
 
-  bool IsCFAOnStack() const { return m_cfa_on_stack; }
+  /// Check if the CFA is on the stack, or elsewhere in the process, such as on
+  /// the heap.
+  bool IsCFAOnStack(Process &process) const;
+
+  /// Determine if the first StackID is "younger" than the second.
+  static bool IsYounger(const StackID &lhs, const StackID &rhs,
+                        Process &process);
 
 protected:
   friend class StackFrame;
 
-  explicit StackID(lldb::addr_t pc, lldb::addr_t cfa, lldb::ThreadSP thread_sp)
-      : m_pc(pc), m_cfa(cfa), m_cfa_on_stack(IsStackAddress(cfa, thread_sp)) {}
-
-  bool IsStackAddress(lldb::addr_t addr, lldb::ThreadSP thread_sp) const;
+  explicit StackID(lldb::addr_t pc, lldb::addr_t cfa) : m_pc(pc), m_cfa(cfa) {}
 
   void SetPC(lldb::addr_t pc) { m_pc = pc; }
 
@@ -84,7 +87,7 @@ class StackID {
                             // below)
   // True if the CFA is an address on the stack, false if it's an address
   // elsewhere (ie heap).
-  bool m_cfa_on_stack = true;
+  mutable LazyBool m_cfa_on_stack = eLazyBoolCalculate;
   SymbolContextScope *m_symbol_scope =
       nullptr; // If nullptr, there is no block or symbol for this frame.
                // If not nullptr, this will either be the scope for the
@@ -98,9 +101,6 @@ class StackID {
 bool operator==(const StackID &lhs, const StackID &rhs);
 bool operator!=(const StackID &lhs, const StackID &rhs);
 
-// frame_id_1 < frame_id_2 means "frame_id_1 is YOUNGER than frame_id_2"
-bool operator<(const StackID &lhs, const StackID &rhs);
-
 } // namespace lldb_private
 
 #endif // LLDB_TARGET_STACKID_H

lldb/include/lldb/Target/ThreadPlan.h

@@ -555,6 +555,28 @@ class ThreadPlan : public std::enable_shared_from_this<ThreadPlan>,
 
   bool IsUsuallyUnexplainedStopReason(lldb::StopReason);
 
+  /// Determine if the first StackID is younger than the second.
+  ///
+  /// A callee is considered younger than its caller, and is also younger than
+  /// all ancestor frames leading up to the caller. Consider this stack:
+  ///
+  ///   +------------------+
+  ///   |        Fa        |
+  ///   +------------------+
+  ///   |        Fb        |
+  ///   +------------------+
+  ///   |        ...       |
+  ///   +------------------+
+  ///   |        Fy        |
+  ///   +------------------+
+  ///   |        Fz        |
+  ///   +------------------+
+  ///
+  /// In this case Fz is younger than each of Fy, ..., Fb, and Fa. Fy is not
+  /// younger than Fz, but is younger than all frames above it in the stack,
+  /// including Fa and Fb.
+  bool IsYounger(const StackID &lhs, const StackID &rhs) const;
+
   Status m_status;
   Process &m_process;
   lldb::tid_t m_tid;

lldb/source/Target/StackFrame.cpp

@@ -57,8 +57,8 @@ StackFrame::StackFrame(const ThreadSP &thread_sp, user_id_t frame_idx,
                        const SymbolContext *sc_ptr)
     : m_thread_wp(thread_sp), m_frame_index(frame_idx),
       m_concrete_frame_index(unwind_frame_index), m_reg_context_sp(),
-      m_id(pc, cfa, thread_sp), m_frame_code_addr(pc), m_sc(), m_flags(),
-      m_frame_base(), m_frame_base_error(), m_cfa_is_valid(cfa_is_valid),
+      m_id(pc, cfa), m_frame_code_addr(pc), m_sc(), m_flags(), m_frame_base(),
+      m_frame_base_error(), m_cfa_is_valid(cfa_is_valid),
       m_stack_frame_kind(kind),
       m_behaves_like_zeroth_frame(behaves_like_zeroth_frame),
       m_variable_list_sp(), m_variable_list_value_objects(),
@@ -83,10 +83,9 @@ StackFrame::StackFrame(const ThreadSP &thread_sp, user_id_t frame_idx,
                        const SymbolContext *sc_ptr)
     : m_thread_wp(thread_sp), m_frame_index(frame_idx),
       m_concrete_frame_index(unwind_frame_index),
-      m_reg_context_sp(reg_context_sp), m_id(pc, cfa, thread_sp),
-      m_frame_code_addr(pc), m_sc(), m_flags(), m_frame_base(),
-      m_frame_base_error(), m_cfa_is_valid(true),
-      m_stack_frame_kind(StackFrame::Kind::Regular),
+      m_reg_context_sp(reg_context_sp), m_id(pc, cfa), m_frame_code_addr(pc),
+      m_sc(), m_flags(), m_frame_base(), m_frame_base_error(),
+      m_cfa_is_valid(true), m_stack_frame_kind(StackFrame::Kind::Regular),
       m_behaves_like_zeroth_frame(behaves_like_zeroth_frame),
       m_variable_list_sp(), m_variable_list_value_objects(),
       m_recognized_frame_sp(), m_disassembly(), m_mutex() {
@@ -110,8 +109,7 @@ StackFrame::StackFrame(const ThreadSP &thread_sp, user_id_t frame_idx,
     : m_thread_wp(thread_sp), m_frame_index(frame_idx),
       m_concrete_frame_index(unwind_frame_index),
       m_reg_context_sp(reg_context_sp),
-      m_id(pc_addr.GetLoadAddress(thread_sp->CalculateTarget().get()), cfa,
-           thread_sp),
+      m_id(pc_addr.GetLoadAddress(thread_sp->CalculateTarget().get()), cfa),
       m_frame_code_addr(pc_addr), m_sc(), m_flags(), m_frame_base(),
       m_frame_base_error(), m_cfa_is_valid(true),
       m_stack_frame_kind(StackFrame::Kind::Regular),

lldb/source/Target/StackID.cpp

@@ -17,18 +17,17 @@
 
 using namespace lldb_private;
 
-bool StackID::IsStackAddress(lldb::addr_t addr,
-                             lldb::ThreadSP thread_sp) const {
-  if (addr == LLDB_INVALID_ADDRESS)
-    return false;
-
-  if (thread_sp)
-    if (auto process_sp = thread_sp->GetProcess()) {
+bool StackID::IsCFAOnStack(Process &process) const {
+  if (m_cfa_on_stack == eLazyBoolCalculate) {
+    m_cfa_on_stack = eLazyBoolNo;
+    if (m_cfa != LLDB_INVALID_ADDRESS) {
       MemoryRegionInfo mem_info;
-      if (process_sp->GetMemoryRegionInfo(addr, mem_info).Success())
-        return mem_info.IsStackMemory() == MemoryRegionInfo::eYes;
+      if (process.GetMemoryRegionInfo(m_cfa, mem_info).Success())
+        if (mem_info.IsStackMemory() == MemoryRegionInfo::eYes)
+          m_cfa_on_stack = eLazyBoolYes;
     }
-  return true; // assumed
+  }
+  return m_cfa_on_stack == eLazyBoolYes;
 }
 
 void StackID::Dump(Stream *s) {
@@ -74,19 +73,20 @@ bool lldb_private::operator!=(const StackID &lhs, const StackID &rhs) {
   return lhs_scope != rhs_scope;
 }
 
-bool lldb_private::operator<(const StackID &lhs, const StackID &rhs) {
-  const lldb::addr_t lhs_cfa = lhs.GetCallFrameAddress();
-  const lldb::addr_t rhs_cfa = rhs.GetCallFrameAddress();
-
+bool StackID::IsYounger(const StackID &lhs, const StackID &rhs,
+                        Process &process) {
   // FIXME: rdar://76119439
-  // Heuristic: At the boundary between an async parent frame calling a regular
-  // child frame, the CFA of the parent async function is a heap addresses, and
-  // the CFA of concrete child function is a stack addresses. Therefore, if lhs
-  // is on stack, and rhs is not, lhs is considered less than rhs (regardless of
-  // address values).
-  if (lhs.IsCFAOnStack() && !rhs.IsCFAOnStack())
+  // At the boundary between an async parent frame calling a regular child
+  // frame, the CFA of the parent async function is a heap addresses, and the
+  // CFA of concrete child function is a stack address. Therefore, if lhs is
+  // on stack, and rhs is not, lhs is considered less than rhs, independent of
+  // address values.
+  if (lhs.IsCFAOnStack(process) && !rhs.IsCFAOnStack(process))
     return true;
 
+  const lldb::addr_t lhs_cfa = lhs.GetCallFrameAddress();
+  const lldb::addr_t rhs_cfa = rhs.GetCallFrameAddress();
+
   // FIXME: We are assuming that the stacks grow downward in memory.  That's not
   // necessary, but true on
   // all the machines we care about at present.  If this changes, we'll have to

lldb/source/Target/ThreadPlan.cpp

@@ -180,6 +180,10 @@ bool ThreadPlan::IsUsuallyUnexplainedStopReason(lldb::StopReason reason) {
   }
 }
 
+bool ThreadPlan::IsYounger(const StackID &lhs, const StackID &rhs) const {
+  return StackID::IsYounger(lhs, rhs, m_process);
+}
+
 // ThreadPlanNull
 
 ThreadPlanNull::ThreadPlanNull(Thread &thread)

lldb/source/Target/ThreadPlanStepInstruction.cpp

@@ -110,7 +110,7 @@ bool ThreadPlanStepInstruction::IsPlanStale() {
       SetPlanComplete();
     }
     return (thread.GetRegisterContext()->GetPC(0) != m_instruction_addr);
-  } else if (cur_frame_id < m_stack_id) {
+  } else if (IsYounger(cur_frame_id, m_stack_id)) {
     // If the current frame is younger than the start frame and we are stepping
     // over, then we need to continue, but if we are doing just one step, we're
     // done.
@@ -140,7 +140,8 @@ bool ThreadPlanStepInstruction::ShouldStop(Event *event_ptr) {
 
     StackID cur_frame_zero_id = cur_frame_sp->GetStackID();
 
-    if (cur_frame_zero_id == m_stack_id || m_stack_id < cur_frame_zero_id) {
+    if (cur_frame_zero_id == m_stack_id ||
+        IsYounger(m_stack_id, cur_frame_zero_id)) {
       if (thread.GetRegisterContext()->GetPC(0) != m_instruction_addr) {
         if (--m_iteration_count <= 0) {
           SetPlanComplete();

lldb/source/Target/ThreadPlanStepOut.cpp

@@ -334,12 +334,12 @@ bool ThreadPlanStepOut::DoPlanExplainsStop(Event *event_ptr) {
 
         if (m_step_out_to_id == frame_zero_id)
           done = true;
-        else if (m_step_out_to_id < frame_zero_id) {
+        else if (IsYounger(m_step_out_to_id, frame_zero_id)) {
           // Either we stepped past the breakpoint, or the stack ID calculation
           // was incorrect and we should probably stop.
           done = true;
         } else {
-          done = (m_immediate_step_from_id < frame_zero_id);
+          done = IsYounger(m_immediate_step_from_id, frame_zero_id);
         }
 
         if (done) {
@@ -398,7 +398,7 @@ bool ThreadPlanStepOut::ShouldStop(Event *event_ptr) {
 
   if (!done) {
     StackID frame_zero_id = GetThread().GetStackFrameAtIndex(0)->GetStackID();
-    done = !(frame_zero_id < m_step_out_to_id);
+    done = !IsYounger(frame_zero_id, m_step_out_to_id);
   }
 
   // The normal step out computations think we are done, so all we need to do
@@ -608,5 +608,5 @@ bool ThreadPlanStepOut::IsPlanStale() {
   // then there's something for us to do.  Otherwise, we're stale.
 
   StackID frame_zero_id = GetThread().GetStackFrameAtIndex(0)->GetStackID();
-  return !(frame_zero_id < m_step_out_to_id);
+  return !IsYounger(frame_zero_id, m_step_out_to_id);
 }

lldb/source/Target/ThreadPlanStepRange.cpp

@@ -220,7 +220,7 @@ lldb::FrameComparison ThreadPlanStepRange::CompareCurrentFrameToStartFrame() {
 
   if (cur_frame_id == m_stack_id) {
     frame_order = eFrameCompareEqual;
-  } else if (cur_frame_id < m_stack_id) {
+  } else if (IsYounger(cur_frame_id, m_stack_id)) {
     frame_order = eFrameCompareYounger;
   } else {
     StackFrameSP cur_parent_frame = thread.GetStackFrameAtIndex(1);

lldb/source/Target/ThreadPlanStepUntil.cpp

@@ -174,7 +174,7 @@ void ThreadPlanStepUntil::AnalyzeStop() {
         bool done;
         StackID cur_frame_zero_id;
 
-        done = (m_stack_id < cur_frame_zero_id);
+        done = IsYounger(m_stack_id, cur_frame_zero_id);
 
         if (done) {
           m_stepped_out = true;
@@ -200,7 +200,7 @@ void ThreadPlanStepUntil::AnalyzeStop() {
 
             if (frame_zero_id == m_stack_id)
               done = true;
-            else if (frame_zero_id < m_stack_id)
+            else if (IsYounger(frame_zero_id, m_stack_id))
               done = false;
             else {
               StackFrameSP older_frame_sp = thread.GetStackFrameAtIndex(1);