[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]

jasonmolenda created this revision.
jasonmolenda added reviewers: labath, clayborg.
jasonmolenda added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.

This is addressing a bit of a big problem we're hitting with the way optimized code is getting generated for kernel binaries today; we have a central function that has a function call with one set of unwind rules, but the return address is a different block with really different unwind rules (maybe it's a noreturn call, I didn't read through the source tbh).

It's the usual problem with this kind of thing - we need to decrement the return pc before looking up the scope for unwind rules.  The patch is straightforward, but I haven't come up with any way to test it short of "have a kernel binary and corefile" and I can't use either of those (no small part of it being that they're enormous).  Not thrilled about this, but I haven't come up with a synthetic repo case for it.

I don't know who would be a good reviewer given how little I can provide here - I'll add Pavel and Greg, but don't feel obligated to give an opinion if you don't have time to look at the code.  I would let this simmer for longer trying to come up with some kind of test case, but it happens to impact a code sequence that is hit by a lot of internal people here, so I need to get going with a fix.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124957

Files:
  lldb/source/Target/RegisterContextUnwind.cpp


Index: lldb/source/Target/RegisterContextUnwind.cpp
===================================================================
--- lldb/source/Target/RegisterContextUnwind.cpp
+++ lldb/source/Target/RegisterContextUnwind.cpp
@@ -514,9 +514,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;
+    // The pc isn't the start of a function. If we're up the stack,
+    // decr pc so it's within bounds of the CALL instruction.
+    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),
@@ -638,7 +641,8 @@
     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);
+      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) {
@@ -1313,7 +1317,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 +1775,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() !=


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D124957.427114.patch
Type: text/x-patch
Size: 2481 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20220504/7e28c87f/attachment.bin>