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() !=
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() !=
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
