[Lldb-commits] [PATCH] D124957: When picking a row from an UnwindPlan on a mid-stack frame, decr pc to get within bounds of the CALL instruction

[Jason Molenda via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb6388e4a0050: Decr return pc mid-stack when picking 
UnwindPlan row (authored by jasonmolenda).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124957

Files:
  lldb/include/lldb/Target/RegisterContextUnwind.h
  lldb/source/Target/RegisterContextUnwind.cpp


Index: lldb/source/Target/RegisterContextUnwind.cpp
===================================================================
--- lldb/source/Target/RegisterContextUnwind.cpp
+++ lldb/source/Target/RegisterContextUnwind.cpp
@@ -82,14 +82,13 @@
 }
 
 bool RegisterContextUnwind::IsUnwindPlanValidForCurrentPC(
-    lldb::UnwindPlanSP unwind_plan_sp, int &valid_pc_offset) {
+    lldb::UnwindPlanSP unwind_plan_sp) {
   if (!unwind_plan_sp)
     return false;
 
   // check if m_current_pc is valid
   if (unwind_plan_sp->PlanValidAtAddress(m_current_pc)) {
     // yes - current offset can be used as is
-    valid_pc_offset = m_current_offset;
     return true;
   }
 
@@ -101,8 +100,6 @@
   Address pc_minus_one(m_current_pc);
   pc_minus_one.SetOffset(m_current_pc.GetOffset() - 1);
   if (unwind_plan_sp->PlanValidAtAddress(pc_minus_one)) {
-    // *valid_pc_offset = m_current_offset - 1;
-    valid_pc_offset = m_current_pc.GetOffset() - 1;
     return true;
   }
 
@@ -514,9 +511,12 @@
   } else if (!addr_range.GetBaseAddress().IsValid() ||
              addr_range.GetBaseAddress().GetSection() != m_current_pc.GetSection() ||
              addr_range.GetBaseAddress().GetOffset() != m_current_pc.GetOffset()) {
-    // If our "current" pc isn't the start of a function, no need
-    // to decrement and recompute.
-    decr_pc_and_recompute_addr_range = false;
+    // If our "current" pc isn't the start of a function, decrement the pc
+    // if we're up the stack.
+    if (m_behaves_like_zeroth_frame)
+      decr_pc_and_recompute_addr_range = false;
+    else
+      decr_pc_and_recompute_addr_range = true;
   } else if (IsTrapHandlerSymbol(process, m_sym_ctx)) {
     // Signal dispatch may set the return address of the handler it calls to
     // point to the first byte of a return trampoline (like __kernel_rt_sigreturn),
@@ -636,9 +636,9 @@
     }
   } else {
     m_full_unwind_plan_sp = GetFullUnwindPlanForFrame();
-    int valid_offset = -1;
-    if (IsUnwindPlanValidForCurrentPC(m_full_unwind_plan_sp, valid_offset)) {
-      active_row = m_full_unwind_plan_sp->GetRowForFunctionOffset(valid_offset);
+    if (IsUnwindPlanValidForCurrentPC(m_full_unwind_plan_sp)) {
+      active_row = m_full_unwind_plan_sp->GetRowForFunctionOffset(
+          m_current_offset_backed_up_one);
       row_register_kind = m_full_unwind_plan_sp->GetRegisterKind();
       PropagateTrapHandlerFlagFromUnwindPlan(m_full_unwind_plan_sp);
       if (active_row.get() && log) {
@@ -1007,8 +1007,7 @@
     unwind_plan_sp = func_unwinders_sp->GetUnwindPlanAtCallSite(
         process->GetTarget(), m_thread);
   }
-  int valid_offset = -1;
-  if (IsUnwindPlanValidForCurrentPC(unwind_plan_sp, valid_offset)) {
+  if (IsUnwindPlanValidForCurrentPC(unwind_plan_sp)) {
     UnwindLogMsgVerbose("frame uses %s for full UnwindPlan because this "
                         "is the call-site unwind plan",
                         unwind_plan_sp->GetSourceName().GetCString());
@@ -1047,7 +1046,7 @@
     }
   }
 
-  if (IsUnwindPlanValidForCurrentPC(unwind_plan_sp, valid_offset)) {
+  if (IsUnwindPlanValidForCurrentPC(unwind_plan_sp)) {
     UnwindLogMsgVerbose("frame uses %s for full UnwindPlan because we "
                         "failed to find a call-site unwind plan that would work",
                         unwind_plan_sp->GetSourceName().GetCString());
@@ -1313,7 +1312,8 @@
                                LLDB_REGNUM_GENERIC_PC);
 
       UnwindPlan::RowSP active_row =
-          m_full_unwind_plan_sp->GetRowForFunctionOffset(m_current_offset);
+          m_full_unwind_plan_sp->GetRowForFunctionOffset(
+              m_current_offset_backed_up_one);
       unwindplan_registerkind = m_full_unwind_plan_sp->GetRegisterKind();
 
       if (got_new_full_unwindplan && active_row.get() && log) {
@@ -1770,7 +1770,8 @@
   m_full_unwind_plan_sp = m_fallback_unwind_plan_sp;
 
   UnwindPlan::RowSP active_row =
-      m_fallback_unwind_plan_sp->GetRowForFunctionOffset(m_current_offset);
+      m_fallback_unwind_plan_sp->GetRowForFunctionOffset(
+          m_current_offset_backed_up_one);
 
   if (active_row &&
       active_row->GetCFAValue().GetValueType() !=
Index: lldb/include/lldb/Target/RegisterContextUnwind.h
===================================================================
--- lldb/include/lldb/Target/RegisterContextUnwind.h
+++ lldb/include/lldb/Target/RegisterContextUnwind.h
@@ -203,8 +203,7 @@
   void UnwindLogMsgVerbose(const char *fmt, ...)
       __attribute__((format(printf, 2, 3)));
 
-  bool IsUnwindPlanValidForCurrentPC(lldb::UnwindPlanSP unwind_plan_sp,
-                                     int &valid_pc_offset);
+  bool IsUnwindPlanValidForCurrentPC(lldb::UnwindPlanSP unwind_plan_sp);
 
   lldb::addr_t GetReturnAddressHint(int32_t plan_offset);
 

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits