[lldb] Make GetRowForFunctionOffset compatible with discontinuous functions

The function had special handling for -1, but that is incompatible with
functions whose entry point is not the first address. Use std::nullopt
instead.

Author: labath

lldb/include/lldb/Symbol/UnwindPlan.h

@@ -450,11 +450,12 @@ class UnwindPlan {
   void InsertRow(Row row, bool replace_existing = false);
 
   // Returns a pointer to the best row for the given offset into the function's
-  // instructions. If offset is -1 it indicates that the function start is
-  // unknown - the final row in the UnwindPlan is returned. In practice, the
-  // UnwindPlan for a function with no known start address will be the
-  // architectural default UnwindPlan which will only have one row.
-  const UnwindPlan::Row *GetRowForFunctionOffset(int offset) const;
+  // instructions. If offset is std::nullopt it indicates that the function
+  // start is unknown - the final row in the UnwindPlan is returned. In
+  // practice, the UnwindPlan for a function with no known start address will be
+  // the architectural default UnwindPlan which will only have one row.
+  const UnwindPlan::Row *
+  GetRowForFunctionOffset(std::optional<int> offset) const;
 
   lldb::RegisterKind GetRegisterKind() const { return m_register_kind; }
 

lldb/include/lldb/Target/RegisterContextUnwind.h

@@ -230,18 +230,17 @@ class RegisterContextUnwind : public lldb_private::RegisterContext {
   lldb_private::Address m_start_pc;
   lldb_private::Address m_current_pc;
 
-  int m_current_offset; // how far into the function we've executed; -1 if
-                        // unknown
-                        // 0 if no instructions have been executed yet.
-
-  // 0 if no instructions have been executed yet.
-  // On architectures where the return address on the stack points
-  // to the instruction after the CALL, this value will have 1
-  // subtracted from it.  Else a function that ends in a CALL will
-  // have an offset pointing into the next function's address range.
+  /// How far into the function we've executed. 0 if no instructions have been
+  /// executed yet, std::nullopt if unknown.
+  std::optional<int> m_current_offset;
+
+  // How far into the function we've executed. 0 if no instructions have been
+  // executed yet, std::nullopt if unknown. On architectures where the return
+  // address on the stack points to the instruction after the CALL, this value
+  // will have 1 subtracted from it. Otherwise, a function that ends in a CALL
+  // will have an offset pointing into the next function's address range.
   // m_current_pc has the actual address of the "current" pc.
-  int m_current_offset_backed_up_one; // how far into the function we've
-                                      // executed; -1 if unknown
+  std::optional<int> m_current_offset_backed_up_one;
 
   bool m_behaves_like_zeroth_frame; // this frame behaves like frame zero
 

lldb/source/Symbol/UnwindPlan.cpp

@@ -417,9 +417,10 @@ void UnwindPlan::InsertRow(Row row, bool replace_existing) {
   }
 }
 
-const UnwindPlan::Row *UnwindPlan::GetRowForFunctionOffset(int offset) const {
-  auto it = offset == -1 ? m_row_list.end()
-                         : llvm::upper_bound(m_row_list, offset, RowLess());
+const UnwindPlan::Row *
+UnwindPlan::GetRowForFunctionOffset(std::optional<int> offset) const {
+  auto it = offset ? llvm::upper_bound(m_row_list, *offset, RowLess())
+                   : m_row_list.end();
   if (it == m_row_list.begin())
     return nullptr;
   // upper_bound returns the row strictly greater than our desired offset, which

lldb/source/Target/RegisterContextUnwind.cpp

@@ -94,8 +94,9 @@ bool RegisterContextUnwind::IsUnwindPlanValidForCurrentPC(
     return true;
   }
 
-  // if m_current_offset <= 0, we've got nothing else to try
-  if (m_current_offset <= 0)
+  // If don't have an offset or we're at the start of the function, we've got
+  // nothing else to try.
+  if (!m_current_offset || m_current_offset == 0)
     return false;
 
   // check pc - 1 to see if it's valid
@@ -199,8 +200,8 @@ void RegisterContextUnwind::InitializeZerothFrame() {
     m_current_offset_backed_up_one = m_current_offset;
   } else {
     m_start_pc = m_current_pc;
-    m_current_offset = -1;
-    m_current_offset_backed_up_one = -1;
+    m_current_offset = std::nullopt;
+    m_current_offset_backed_up_one = std::nullopt;
   }
 
   // We've set m_frame_type and m_sym_ctx before these calls.
@@ -439,8 +440,8 @@ void RegisterContextUnwind::InitializeNonZerothFrame() {
         m_frame_type = eNormalFrame;
       }
       m_all_registers_available = false;
-      m_current_offset = -1;
-      m_current_offset_backed_up_one = -1;
+      m_current_offset = std::nullopt;
+      m_current_offset_backed_up_one = std::nullopt;
       RegisterKind row_register_kind = m_full_unwind_plan_sp->GetRegisterKind();
       if (const UnwindPlan::Row *row =
               m_full_unwind_plan_sp->GetRowForFunctionOffset(0)) {
@@ -571,16 +572,16 @@ void RegisterContextUnwind::InitializeNonZerothFrame() {
     m_current_offset = pc - m_start_pc.GetLoadAddress(&process->GetTarget());
     m_current_offset_backed_up_one = m_current_offset;
     if (decr_pc_and_recompute_addr_range &&
-        m_current_offset_backed_up_one > 0) {
-      m_current_offset_backed_up_one--;
+        m_current_offset_backed_up_one != 0) {
+      --*m_current_offset_backed_up_one;
       if (m_sym_ctx_valid) {
         m_current_pc.SetLoadAddress(pc - 1, &process->GetTarget());
       }
     }
   } else {
     m_start_pc = m_current_pc;
-    m_current_offset = -1;
-    m_current_offset_backed_up_one = -1;
+    m_current_offset = std::nullopt;
+    m_current_offset_backed_up_one = std::nullopt;
   }
 
   if (IsTrapHandlerSymbol(process, m_sym_ctx)) {
@@ -748,7 +749,7 @@ bool RegisterContextUnwind::BehavesLikeZerothFrame() const {
 //   2. m_sym_ctx should already be filled in, and
 //   3. m_current_pc should have the current pc value for this frame
 //   4. m_current_offset_backed_up_one should have the current byte offset into
-//   the function, maybe backed up by 1, -1 if unknown
+//   the function, maybe backed up by 1, std::nullopt if unknown
 
 std::shared_ptr<const UnwindPlan>
 RegisterContextUnwind::GetFastUnwindPlanForFrame() {
@@ -790,7 +791,7 @@ RegisterContextUnwind::GetFastUnwindPlanForFrame() {
 //   2. m_sym_ctx should already be filled in, and
 //   3. m_current_pc should have the current pc value for this frame
 //   4. m_current_offset_backed_up_one should have the current byte offset into
-//   the function, maybe backed up by 1, -1 if unknown
+//   the function, maybe backed up by 1, std::nullopt if unknown
 
 std::shared_ptr<const UnwindPlan>
 RegisterContextUnwind::GetFullUnwindPlanForFrame() {