jasonmolenda created this revision.
jasonmolenda added a reviewer: labath.
jasonmolenda added a project: LLDB.
Herald added subscribers: JDevlieghere, pengfei, atanasyan, jrtc27, kbarton,
kristof.beyls, nemanjai, sdardis.
jasonmolenda requested review of this revision.
The ABI plugins can provide an architectural default unwind plan -- an
UnwindPlan that will get you up the stack when you don't have any unwind
information or function start addresses. The unwinder uses these arch default
unwind plans in two cases: No information for this stack frame, or as a fast
unwind plan to walk the stack quickly, when you're not retrieving additional
registers. (the unwinder will often also try this as a "fallback" unwind plan
when it thinks there might be more stack to discover, giving us the
ever-awesome double-start() that we see on Darwin processes at the end of the
backtrace, sigh)
The big important use of an arch default unwind plan is jitted code -- where
lldb doesn't have any start addresses for the functions or unwind information.
We can't detect where registers are spilled to the stack, but at least we can
walk the stack.
However, when we have a stack 5 frames deep,
0 func1()
1 func2()
2 <no function, jitted code>
3 func3()
4 main()
If we navigate to frame 3 (func3) and say "p/x $r12", lldb will descend down
the stack (ultimately to frame 0, func1) looking for a place where r12 was
saved to stack before it was reused. If no saves/spills are found, the live
r12 value in frame 0 is used.
The problem here is that we have no view into what happened in frame 2, the
jitted code. It may have spilled r12 and modified the register. The rule in
lldb is that we don't provide register values that are *possibly* modified --
the same reason why lldb won't pass caller-spilled aka volatile register values
up the stack, e.g. rax on SysV x86_64, where a function is free to modify it
without saving the old value. We cannot be sure that the value of these
registers haven't been changed, so lldb will refuse to print $rax for frame 3
in this backtrace.
This patchset adds a new bool to UnwindPlan::Row which specifies that any
register not explicitly defined should return as Undefined. This means that
the saved-register-finder method should NOT continue down the stack looking for
a saved value - we cannot know what happened in this stack frame, so it's not
safe to continue searching and pass a register value up the stack.
The second use case of the arch default UnwindPlan is as a "fast Unwind Plan"
when lldb only needs to retrieve frame pointer & pc values when doing a quick
stack walk. In this case, when lldb is searching for a register in
RegisterContextUnwind::SavedLocationForRegister, it first tries the fast unwind
plan, and if the register isn't mentioned there, it tries to find a location in
the Full unwind plan. I have one small modification here to treat an Undefined
register in the *fast* unwind plan as meaning that we should look in the full
unwind plan, instead of stopping here. The full unwind plan may have a real
register save location; if it says the register is Undefined, then that will be
used.
The big risk with this patch, obviously, is that I'm updating all of the
architectural default unwind plans to mark undefined registers as unavailable.
I worry that there's some target that is relying on architectural default
unwind plans heavily and previously the frame 0 register values were passed up
the stack ... and maybe it was right sometimes, but now lldb won't send them up
any more. I'll need to watch the bots closely when this lands. I could have
done the cowardly thing and only updated the SysV x86_64 and Darwin AArch64 ABI
plugins, but this is a change I've been wanting to make for a while.
Testing this is something I don't have a good answer for. I could unit-test
that the SysV x86_64 ABI's arch default UnwindPlan sets
SetUnspecifiedRegistersAreUndefined, but that's not anything really useful.
Until recently you could jit an expression that stopped at a breakpoint and
you'd have a wonderfully unsymbolicated stack frame in the middle of the
backtrace but some rascal like Raphael seems to have changed it so we have a
Module for the jitted code with a function start address recently. :/ Short of
a corefile or a test using a scripted process plugin (basically the same
thing), I don't think I can simulate the use case here. On the up side, bugs
show up real fast in the testsuite run as I found when I didn't originally
remember how the fast unwind plan fallback needed to work when I wrote this
patch. :)
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D96829
Files:
lldb/include/lldb/Symbol/UnwindPlan.h
lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
lldb/source/Plugins/ABI/ARM/ABIMacOSX_arm.cpp
lldb/source/Plugins/ABI/ARM/ABISysV_arm.cpp
lldb/source/Plugins/ABI/Hexagon/ABISysV_hexagon.cpp
lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp
lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp
lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc.cpp
lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc64.cpp
lldb/source/Plugins/ABI/X86/ABIMacOSX_i386.cpp
lldb/source/Plugins/ABI/X86/ABISysV_i386.cpp
lldb/source/Plugins/ABI/X86/ABISysV_x86_64.cpp
lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp
lldb/source/Symbol/UnwindPlan.cpp
lldb/source/Target/RegisterContextUnwind.cpp
Index: lldb/source/Target/RegisterContextUnwind.cpp
===================================================================
--- lldb/source/Target/RegisterContextUnwind.cpp
+++ lldb/source/Target/RegisterContextUnwind.cpp
@@ -542,6 +542,8 @@
StreamString active_row_strm;
active_row->Dump(active_row_strm, m_fast_unwind_plan_sp.get(), &m_thread,
m_start_pc.GetLoadAddress(exe_ctx.GetTargetPtr()));
+ UnwindLogMsg("Using fast unwind plan '%s'",
+ m_fast_unwind_plan_sp->GetSourceName().AsCString());
UnwindLogMsg("active row: %s", active_row_strm.GetData());
}
} else {
@@ -556,6 +558,8 @@
active_row->Dump(active_row_strm, m_full_unwind_plan_sp.get(),
&m_thread,
m_start_pc.GetLoadAddress(exe_ctx.GetTargetPtr()));
+ UnwindLogMsg("Using full unwind plan '%s'",
+ m_full_unwind_plan_sp->GetSourceName().AsCString());
UnwindLogMsg("active row: %s", active_row_strm.GetData());
}
}
@@ -662,13 +666,6 @@
*m_thread.CalculateTarget(), m_thread);
if (unwind_plan_sp) {
if (unwind_plan_sp->PlanValidAtAddress(m_current_pc)) {
- Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_UNWIND));
- if (log && log->GetVerbose()) {
- if (m_fast_unwind_plan_sp)
- UnwindLogMsgVerbose("frame, and has a fast UnwindPlan");
- else
- UnwindLogMsgVerbose("frame");
- }
m_frame_type = eNormalFrame;
return unwind_plan_sp;
} else {
@@ -1147,6 +1144,7 @@
RegisterContextUnwind::SavedLocationForRegister(
uint32_t lldb_regnum, lldb_private::UnwindLLDB::RegisterLocation ®loc) {
RegisterNumber regnum(m_thread, eRegisterKindLLDB, lldb_regnum);
+ Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_UNWIND));
// Have we already found this register location?
if (!m_registers.empty()) {
@@ -1179,8 +1177,17 @@
(int)unwindplan_registerkind);
return UnwindLLDB::RegisterSearchResult::eRegisterNotFound;
}
+ // The architecture default unwind plan marks unknown registers as
+ // Undefined so that we don't forward them up the stack when a
+ // jitted stack frame may have overwritten them. But when the
+ // arch default unwind plan is used as the Fast Unwind Plan, we
+ // need to recognize this & switch over to the Full Unwind Plan
+ // to see what unwind rule that (more knoweldgeable, probably)
+ // UnwindPlan has. If the full UnwindPlan says the register
+ // location is Undefined, then it really is.
if (active_row->GetRegisterInfo(regnum.GetAsKind(unwindplan_registerkind),
- unwindplan_regloc)) {
+ unwindplan_regloc) &&
+ !unwindplan_regloc.IsUndefined()) {
UnwindLogMsg(
"supplying caller's saved %s (%d)'s location using FastUnwindPlan",
regnum.GetName(), regnum.GetAsKind(eRegisterKindLLDB));
@@ -1191,8 +1198,11 @@
if (!have_unwindplan_regloc) {
// m_full_unwind_plan_sp being NULL means that we haven't tried to find a
// full UnwindPlan yet
- if (!m_full_unwind_plan_sp)
+ bool got_new_full_unwindplan = false;
+ if (!m_full_unwind_plan_sp) {
m_full_unwind_plan_sp = GetFullUnwindPlanForFrame();
+ got_new_full_unwindplan = true;
+ }
if (m_full_unwind_plan_sp) {
RegisterNumber pc_regnum(m_thread, eRegisterKindGeneric,
@@ -1202,6 +1212,16 @@
m_full_unwind_plan_sp->GetRowForFunctionOffset(m_current_offset);
unwindplan_registerkind = m_full_unwind_plan_sp->GetRegisterKind();
+ if (got_new_full_unwindplan && active_row.get() && log) {
+ StreamString active_row_strm;
+ ExecutionContext exe_ctx(m_thread.shared_from_this());
+ active_row->Dump(active_row_strm, m_full_unwind_plan_sp.get(),
+ &m_thread,
+ m_start_pc.GetLoadAddress(exe_ctx.GetTargetPtr()));
+ UnwindLogMsg("Using full unwind plan '%s'",
+ m_full_unwind_plan_sp->GetSourceName().AsCString());
+ UnwindLogMsg("active row: %s", active_row_strm.GetData());
+ }
RegisterNumber return_address_reg;
// If we're fetching the saved pc and this UnwindPlan defines a
Index: lldb/source/Symbol/UnwindPlan.cpp
===================================================================
--- lldb/source/Symbol/UnwindPlan.cpp
+++ lldb/source/Symbol/UnwindPlan.cpp
@@ -217,6 +217,7 @@
m_cfa_value.SetUnspecified();
m_afa_value.SetUnspecified();
m_offset = 0;
+ m_unspecified_registers_are_undefined = false;
m_register_locations.clear();
}
@@ -242,11 +243,11 @@
idx->second.Dump(s, unwind_plan, this, thread, verbose);
s.PutChar(' ');
}
- s.EOL();
}
UnwindPlan::Row::Row()
- : m_offset(0), m_cfa_value(), m_afa_value(), m_register_locations() {}
+ : m_offset(0), m_cfa_value(), m_afa_value(), m_register_locations(),
+ m_unspecified_registers_are_undefined(false) {}
bool UnwindPlan::Row::GetRegisterInfo(
uint32_t reg_num,
@@ -256,6 +257,10 @@
register_location = pos->second;
return true;
}
+ if (m_unspecified_registers_are_undefined) {
+ register_location.SetUndefined();
+ return true;
+ }
return false;
}
@@ -348,10 +353,11 @@
}
bool UnwindPlan::Row::operator==(const UnwindPlan::Row &rhs) const {
- return m_offset == rhs.m_offset &&
- m_cfa_value == rhs.m_cfa_value &&
- m_afa_value == rhs.m_afa_value &&
- m_register_locations == rhs.m_register_locations;
+ return m_offset == rhs.m_offset && m_cfa_value == rhs.m_cfa_value &&
+ m_afa_value == rhs.m_afa_value &&
+ m_unspecified_registers_are_undefined ==
+ rhs.m_unspecified_registers_are_undefined &&
+ m_register_locations == rhs.m_register_locations;
}
void UnwindPlan::AppendRow(const UnwindPlan::RowSP &row_sp) {
@@ -552,6 +558,7 @@
for (pos = begin; pos != end; ++pos) {
s.Printf("row[%u]: ", (uint32_t)std::distance(begin, pos));
(*pos)->Dump(s, this, thread, base_addr);
+ s.Printf("\n");
}
}
Index: lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp
===================================================================
--- lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp
+++ lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp
@@ -767,6 +767,7 @@
const int32_t ptr_size = 8;
row->GetCFAValue().SetIsRegisterPlusOffset(dwarf_rbp, 2 * ptr_size);
row->SetOffset(0);
+ row->SetUnspecifiedRegistersAreUndefined(true);
row->SetRegisterLocationToAtCFAPlusOffset(fp_reg_num, ptr_size * -2, true);
row->SetRegisterLocationToAtCFAPlusOffset(pc_reg_num, ptr_size * -1, true);
Index: lldb/source/Plugins/ABI/X86/ABISysV_x86_64.cpp
===================================================================
--- lldb/source/Plugins/ABI/X86/ABISysV_x86_64.cpp
+++ lldb/source/Plugins/ABI/X86/ABISysV_x86_64.cpp
@@ -887,6 +887,7 @@
const int32_t ptr_size = 8;
row->GetCFAValue().SetIsRegisterPlusOffset(dwarf_rbp, 2 * ptr_size);
row->SetOffset(0);
+ row->SetUnspecifiedRegistersAreUndefined(true);
row->SetRegisterLocationToAtCFAPlusOffset(fp_reg_num, ptr_size * -2, true);
row->SetRegisterLocationToAtCFAPlusOffset(pc_reg_num, ptr_size * -1, true);
Index: lldb/source/Plugins/ABI/X86/ABISysV_i386.cpp
===================================================================
--- lldb/source/Plugins/ABI/X86/ABISysV_i386.cpp
+++ lldb/source/Plugins/ABI/X86/ABISysV_i386.cpp
@@ -652,6 +652,7 @@
row->GetCFAValue().SetIsRegisterPlusOffset(fp_reg_num, 2 * ptr_size);
row->SetOffset(0);
+ row->SetUnspecifiedRegistersAreUndefined(true);
row->SetRegisterLocationToAtCFAPlusOffset(fp_reg_num, ptr_size * -2, true);
row->SetRegisterLocationToAtCFAPlusOffset(pc_reg_num, ptr_size * -1, true);
Index: lldb/source/Plugins/ABI/X86/ABIMacOSX_i386.cpp
===================================================================
--- lldb/source/Plugins/ABI/X86/ABIMacOSX_i386.cpp
+++ lldb/source/Plugins/ABI/X86/ABIMacOSX_i386.cpp
@@ -389,6 +389,7 @@
row->GetCFAValue().SetIsRegisterPlusOffset(fp_reg_num, 2 * ptr_size);
row->SetOffset(0);
+ row->SetUnspecifiedRegistersAreUndefined(true);
row->SetRegisterLocationToAtCFAPlusOffset(fp_reg_num, ptr_size * -2, true);
row->SetRegisterLocationToAtCFAPlusOffset(pc_reg_num, ptr_size * -1, true);
Index: lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc64.cpp
===================================================================
--- lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc64.cpp
+++ lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc64.cpp
@@ -1003,6 +1003,7 @@
UnwindPlan::RowSP row(new UnwindPlan::Row);
const int32_t ptr_size = 8;
+ row->SetUnspecifiedRegistersAreUndefined(true);
row->GetCFAValue().SetIsRegisterDereferenced(sp_reg_num);
row->SetRegisterLocationToAtCFAPlusOffset(pc_reg_num, ptr_size * 2, true);
Index: lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc.cpp
===================================================================
--- lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc.cpp
+++ lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc.cpp
@@ -900,6 +900,7 @@
UnwindPlan::RowSP row(new UnwindPlan::Row);
const int32_t ptr_size = 4;
+ row->SetUnspecifiedRegistersAreUndefined(true);
row->GetCFAValue().SetIsRegisterDereferenced(sp_reg_num);
row->SetRegisterLocationToAtCFAPlusOffset(pc_reg_num, ptr_size * 1, true);
Index: lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp
===================================================================
--- lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp
+++ lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp
@@ -1156,6 +1156,7 @@
UnwindPlan::RowSP row(new UnwindPlan::Row);
+ row->SetUnspecifiedRegistersAreUndefined(true);
row->GetCFAValue().SetIsRegisterPlusOffset(dwarf_r29, 0);
row->SetRegisterLocationToRegister(dwarf_pc, dwarf_r31, true);
Index: lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp
===================================================================
--- lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp
+++ lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp
@@ -985,6 +985,7 @@
UnwindPlan::RowSP row(new UnwindPlan::Row);
+ row->SetUnspecifiedRegistersAreUndefined(true);
row->GetCFAValue().SetIsRegisterPlusOffset(dwarf_r29, 0);
row->SetRegisterLocationToRegister(dwarf_pc, dwarf_r31, true);
Index: lldb/source/Plugins/ABI/Hexagon/ABISysV_hexagon.cpp
===================================================================
--- lldb/source/Plugins/ABI/Hexagon/ABISysV_hexagon.cpp
+++ lldb/source/Plugins/ABI/Hexagon/ABISysV_hexagon.cpp
@@ -1225,6 +1225,7 @@
UnwindPlan::RowSP row(new UnwindPlan::Row);
+ row->SetUnspecifiedRegistersAreUndefined(true);
row->GetCFAValue().SetIsRegisterPlusOffset(LLDB_REGNUM_GENERIC_FP, 8);
row->SetRegisterLocationToAtCFAPlusOffset(fp_reg_num, -8, true);
Index: lldb/source/Plugins/ABI/ARM/ABISysV_arm.cpp
===================================================================
--- lldb/source/Plugins/ABI/ARM/ABISysV_arm.cpp
+++ lldb/source/Plugins/ABI/ARM/ABISysV_arm.cpp
@@ -1940,6 +1940,7 @@
row->GetCFAValue().SetIsRegisterPlusOffset(fp_reg_num, 2 * ptr_size);
row->SetOffset(0);
+ row->SetUnspecifiedRegistersAreUndefined(true);
row->SetRegisterLocationToAtCFAPlusOffset(fp_reg_num, ptr_size * -2, true);
row->SetRegisterLocationToAtCFAPlusOffset(pc_reg_num, ptr_size * -1, true);
Index: lldb/source/Plugins/ABI/ARM/ABIMacOSX_arm.cpp
===================================================================
--- lldb/source/Plugins/ABI/ARM/ABIMacOSX_arm.cpp
+++ lldb/source/Plugins/ABI/ARM/ABIMacOSX_arm.cpp
@@ -1824,6 +1824,7 @@
row->GetCFAValue().SetIsRegisterPlusOffset(fp_reg_num, 2 * ptr_size);
row->SetOffset(0);
+ row->SetUnspecifiedRegistersAreUndefined(true);
row->SetRegisterLocationToAtCFAPlusOffset(fp_reg_num, ptr_size * -2, true);
row->SetRegisterLocationToAtCFAPlusOffset(pc_reg_num, ptr_size * -1, true);
Index: lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
===================================================================
--- lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
+++ lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
@@ -356,6 +356,7 @@
row->GetCFAValue().SetIsRegisterPlusOffset(fp_reg_num, 2 * ptr_size);
row->SetOffset(0);
+ row->SetUnspecifiedRegistersAreUndefined(true);
row->SetRegisterLocationToAtCFAPlusOffset(fp_reg_num, ptr_size * -2, true);
row->SetRegisterLocationToAtCFAPlusOffset(pc_reg_num, ptr_size * -1, true);
Index: lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
===================================================================
--- lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
+++ lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
@@ -384,6 +384,7 @@
row->GetCFAValue().SetIsRegisterPlusOffset(fp_reg_num, 2 * ptr_size);
row->SetOffset(0);
+ row->SetUnspecifiedRegistersAreUndefined(true);
row->SetRegisterLocationToAtCFAPlusOffset(fp_reg_num, ptr_size * -2, true);
row->SetRegisterLocationToAtCFAPlusOffset(pc_reg_num, ptr_size * -1, true);
Index: lldb/include/lldb/Symbol/UnwindPlan.h
===================================================================
--- lldb/include/lldb/Symbol/UnwindPlan.h
+++ lldb/include/lldb/Symbol/UnwindPlan.h
@@ -360,6 +360,25 @@
bool SetRegisterLocationToSame(uint32_t reg_num, bool must_replace);
+ // When this UnspecifiedRegistersAreUndefined mode is
+ // set, any register that is not specified by this Row will
+ // be described as Undefined.
+ // This will prevent the unwinder from iterating down the
+ // stack looking for a spill location, or a live register value
+ // at frame 0.
+ // It would be used for an UnwindPlan row where we can't track
+ // spilled registers -- for instance a jitted stack frame where
+ // we have no unwind information or start address -- and registers
+ // MAY have been spilled and overwritten, so providing the
+ // spilled/live value from a newer frame may show an incorrect value.
+ void SetUnspecifiedRegistersAreUndefined(bool unspec_is_undef) {
+ m_unspecified_registers_are_undefined = unspec_is_undef;
+ }
+
+ bool GetUnspecifiedRegistersAreUndefined() {
+ return m_unspecified_registers_are_undefined;
+ }
+
void Clear();
void Dump(Stream &s, const UnwindPlan *unwind_plan, Thread *thread,
@@ -372,6 +391,7 @@
FAValue m_cfa_value;
FAValue m_afa_value;
collection m_register_locations;
+ bool m_unspecified_registers_are_undefined;
}; // class Row
typedef std::shared_ptr<Row> RowSP;
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
