jasonmolenda created this revision.
jasonmolenda added reviewers: clayborg, labath.
jasonmolenda added a project: LLDB.
Herald added subscribers: danielkiss, kristof.beyls.
I found this on an AAarch64 target, where we use a frame pointer register on
Darwin. UnwindAssemblyInstEmulation iterates over the instructions building up
an UnwindPlan row by row. When UnwindAssemblyInstEmulation sees a branch
instruction, it "forwards" the unwind state to the target offset; if we have a
mid-function epilogue that restores the spilled registers and returns, when we
hit the branch target, the saved unwind state is reinstated.
The bug comes in that UnwindAssemblyInstEmulation maintains both the "current
row" object and it tracks the register that the Canonical Frame Address is
defined in terms of, and whether this register is the frame pointer or not.
After the prologue we have unwind state defining the CFA as $fp+16; then during
a mid-function epilogue we restore the registers and switch to the CFA as $sp
based. At this point the UnwindAssemblyInstEmulation ivars (CFA register,
is-it-fp-or-not) are updated to recognize the switch to $sp. After the
epilogue, we restore the current-row but we leave the other ivars to their
epilogue setting.
The failure then comes in if the instructions after the mid-function epilogue
modify $sp, UnwindAssemblyInstEmulation changes the current row's CFA offset
value to track that change, because it thinks the CFA is defined in terms of
the stack pointer. But the current row is still defining it in terms of $fp.
So everything fails.
It's a sneaky bug because it's easy to miss if lldb silently "falls back" to
the architectural default unwind plan when it can't find the caller. So we're
missing spilled registers from this frame. It takes some specific
circumstances to hit it -- modifying $sp after that mid-function epilogue.
I know not a lot of people have touched this code in ages (including me!), but
let's put it up for review in case anyone has questions.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D78077
Files:
lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp
Index: lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp
===================================================================
--- lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp
+++ lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp
@@ -678,3 +678,103 @@
EXPECT_TRUE(regloc.IsSame());
}
}
+
+TEST_F(TestArm64InstEmulation, TestCFARegisterTrackedAcrossJumps) {
+ ArchSpec arch("arm64-apple-ios10");
+ std::unique_ptr<UnwindAssemblyInstEmulation> engine(
+ static_cast<UnwindAssemblyInstEmulation *>(
+ UnwindAssemblyInstEmulation::CreateInstance(arch)));
+ ASSERT_NE(nullptr, engine);
+
+ UnwindPlan::RowSP row_sp;
+ AddressRange sample_range;
+ UnwindPlan unwind_plan(eRegisterKindLLDB);
+ UnwindPlan::Row::RegisterLocation regloc;
+
+ uint8_t data[] = {
+ // prologue
+ 0xf4, 0x4f, 0xbe, 0xa9, // 0: 0xa9be4ff4 stp x20, x19, [sp, #-0x20]!
+ 0xfd, 0x7b, 0x01, 0xa9, // 4: 0xa9017bfd stp x29, x30, [sp, #0x10]
+ 0xfd, 0x43, 0x00, 0x91, // 8: 0x910043fd add x29, sp, #0x10
+ 0xff, 0x43, 0x00, 0xd1, // 12: 0xd10043ff sub sp, sp, #0x10
+ // conditional branch over a mid-function epilogue
+ 0xeb, 0x00, 0x00, 0x54, // 16: 0x540000eb b.lt <+44>
+ // mid-function epilogue
+ 0x1f, 0x20, 0x03, 0xd5, // 20: 0xd503201f nop
+ 0xe0, 0x03, 0x13, 0xaa, // 24: 0xaa1303e0 mov x0, x19
+ 0xbf, 0x43, 0x00, 0xd1, // 28: 0xd10043bf sub sp, x29, #0x10
+ 0xfd, 0x7b, 0x41, 0xa9, // 32: 0xa9417bfd ldp x29, x30, [sp, #0x10]
+ 0xf4, 0x4f, 0xc2, 0xa8, // 36: 0xa8c24ff4 ldp x20, x19, [sp], #0x20
+ 0xc0, 0x03, 0x5f, 0xd6, // 40: 0xd65f03c0 ret
+ // unwind state restored, we're using a frame pointer, let's change the
+ // stack pointer and see no change in how the CFA is computed
+ 0x1f, 0x20, 0x03, 0xd5, // 44: 0xd503201f nop
+ 0xff, 0x43, 0x00, 0xd1, // 48: 0xd10043ff sub sp, sp, #0x10
+ 0x1f, 0x20, 0x03, 0xd5, // 52: 0xd503201f nop
+ // final epilogue
+ 0xe0, 0x03, 0x13, 0xaa, // 56: 0xaa1303e0 mov x0, x19
+ 0xbf, 0x43, 0x00, 0xd1, // 60: 0xd10043bf sub sp, x29, #0x10
+ 0xfd, 0x7b, 0x41, 0xa9, // 64: 0xa9417bfd ldp x29, x30, [sp, #0x10]
+ 0xf4, 0x4f, 0xc2, 0xa8, // 68: 0xa8c24ff4 ldp x20, x19, [sp], #0x20
+ 0xc0, 0x03, 0x5f, 0xd6, // 72: 0xd65f03c0 ret
+
+ 0x1f, 0x20, 0x03, 0xd5, // 52: 0xd503201f nop
+ };
+
+ // UnwindPlan we expect:
+ // row[0]: 0: CFA=sp +0 =>
+ // row[1]: 4: CFA=sp+32 => x19=[CFA-24] x20=[CFA-32]
+ // row[2]: 8: CFA=sp+32 => x19=[CFA-24] x20=[CFA-32] fp=[CFA-16] lr=[CFA-8]
+ // row[3]: 12: CFA=fp+16 => x19=[CFA-24] x20=[CFA-32] fp=[CFA-16] lr=[CFA-8]
+ // row[4]: 32: CFA=sp+32 => x19=[CFA-24] x20=[CFA-32] fp=[CFA-16] lr=[CFA-8]
+ // row[5]: 36: CFA=sp+32 => x19=[CFA-24] x20=[CFA-32] fp= <same> lr= <same>
+ // row[6]: 40: CFA=sp +0 => x19= <same> x20= <same> fp= <same> lr= <same>
+ // row[7]: 44: CFA=fp+16 => x19=[CFA-24] x20=[CFA-32] fp=[CFA-16] lr=[CFA-8]
+ // row[8]: 64: CFA=sp+32 => x19=[CFA-24] x20=[CFA-32] fp=[CFA-16] lr=[CFA-8]
+ // row[9]: 68: CFA=sp+32 => x19=[CFA-24] x20=[CFA-32] fp= <same> lr= <same>
+ // row[10]: 72: CFA=sp +0 => x19= <same> x20= <same> fp= <same> lr= <same>
+
+ // The specific bug we're looking for is this incorrect CFA definition,
+ // where the InstEmulation is using the $sp value mixed in with $fp,
+ // it looks like this:
+ //
+ // row[7]: 44: CFA=fp+16 => x19=[CFA-24] x20=[CFA-32] fp=[CFA-16] lr=[CFA-8]
+ // row[8]: 52: CFA=fp+64 => x19=[CFA-24] x20=[CFA-32] fp=[CFA-16] lr=[CFA-8]
+ // row[9]: 68: CFA=fp+64 => x19=[CFA-24] x20=[CFA-32] fp= <same> lr= <same>
+
+ sample_range = AddressRange(0x1000, sizeof(data));
+
+ EXPECT_TRUE(engine->GetNonCallSiteUnwindPlanFromAssembly(
+ sample_range, data, sizeof(data), unwind_plan));
+
+ // Confirm CFA at mid-func epilogue 'ret' is $sp+0
+ row_sp = unwind_plan.GetRowForFunctionOffset(40);
+ EXPECT_EQ(40ull, row_sp->GetOffset());
+ EXPECT_TRUE(row_sp->GetCFAValue().GetRegisterNumber() == gpr_sp_arm64);
+ EXPECT_TRUE(row_sp->GetCFAValue().IsRegisterPlusOffset() == true);
+ EXPECT_EQ(0, row_sp->GetCFAValue().GetOffset());
+
+ // After the 'ret', confirm we're back to the correct CFA of $fp+16
+ row_sp = unwind_plan.GetRowForFunctionOffset(44);
+ EXPECT_EQ(44ull, row_sp->GetOffset());
+ EXPECT_TRUE(row_sp->GetCFAValue().GetRegisterNumber() == gpr_fp_arm64);
+ EXPECT_TRUE(row_sp->GetCFAValue().IsRegisterPlusOffset() == true);
+ EXPECT_EQ(16, row_sp->GetCFAValue().GetOffset());
+
+ // Confirm that we have no additional UnwindPlan rows before the
+ // real epilogue -- we still get the Row at offset 44.
+ row_sp = unwind_plan.GetRowForFunctionOffset(60);
+ EXPECT_EQ(44ull, row_sp->GetOffset());
+ EXPECT_TRUE(row_sp->GetCFAValue().GetRegisterNumber() == gpr_fp_arm64);
+ EXPECT_TRUE(row_sp->GetCFAValue().IsRegisterPlusOffset() == true);
+ EXPECT_EQ(16, row_sp->GetCFAValue().GetOffset());
+
+ // And in the epilogue, confirm that we start by switching back to
+ // defining the CFA in terms of $sp.
+ row_sp = unwind_plan.GetRowForFunctionOffset(64);
+ EXPECT_EQ(64ull, row_sp->GetOffset());
+ EXPECT_TRUE(row_sp->GetCFAValue().GetRegisterNumber() == gpr_sp_arm64);
+ EXPECT_TRUE(row_sp->GetCFAValue().IsRegisterPlusOffset() == true);
+ EXPECT_EQ(32, row_sp->GetCFAValue().GetOffset());
+}
+
Index: lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
===================================================================
--- lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
+++ lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
@@ -138,6 +138,13 @@
m_inst_emulator_up->GetRegisterInfo(
eRegisterKindGeneric, LLDB_REGNUM_GENERIC_RA, ra_reg_info);
+ // cache the stack pointer register number (in whatever register
+ // numbering this UnwindPlan uses) for quick reference during
+ // instruction parsing.
+ RegisterInfo sp_reg_info;
+ m_inst_emulator_up->GetRegisterInfo(
+ eRegisterKindGeneric, LLDB_REGNUM_GENERIC_SP, ra_reg_info);
+
// The architecture dependent condition code of the last processed
// instruction.
EmulateInstruction::InstructionCondition last_condition =
@@ -167,6 +174,23 @@
*newrow = *it->second.first;
m_curr_row.reset(newrow);
m_register_values = it->second.second;
+ // re-set the CFA register ivars to match the
+ // new m_curr_row.
+ if (sp_reg_info.name &&
+ m_curr_row->GetCFAValue().IsRegisterPlusOffset()) {
+ uint32_t row_cfa_regnum =
+ m_curr_row->GetCFAValue().GetRegisterNumber();
+ lldb::RegisterKind row_kind =
+ m_unwind_plan_ptr->GetRegisterKind();
+ // set m_cfa_reg_info to the row's CFA reg.
+ m_inst_emulator_up->GetRegisterInfo(row_kind, row_cfa_regnum,
+ m_cfa_reg_info);
+ // set m_fp_is_cfa.
+ if (sp_reg_info.kinds[row_kind] == row_cfa_regnum)
+ m_fp_is_cfa = false;
+ else
+ m_fp_is_cfa = true;
+ }
}
m_inst_emulator_up->SetInstruction(inst->GetOpcode(),
@@ -197,6 +221,23 @@
std::make_shared<UnwindPlan::Row>(*saved_state.first);
m_curr_row->SetOffset(current_offset);
m_register_values = saved_state.second;
+ // re-set the CFA register ivars to match the
+ // new m_curr_row.
+ if (sp_reg_info.name &&
+ m_curr_row->GetCFAValue().IsRegisterPlusOffset()) {
+ uint32_t row_cfa_regnum =
+ m_curr_row->GetCFAValue().GetRegisterNumber();
+ lldb::RegisterKind row_kind =
+ m_unwind_plan_ptr->GetRegisterKind();
+ // set m_cfa_reg_info to the row's CFA reg.
+ m_inst_emulator_up->GetRegisterInfo(row_kind, row_cfa_regnum,
+ m_cfa_reg_info);
+ // set m_fp_is_cfa.
+ if (sp_reg_info.kinds[row_kind] == row_cfa_regnum)
+ m_fp_is_cfa = false;
+ else
+ m_fp_is_cfa = true;
+ }
bool replace_existing =
true; // The last instruction might already
// created a row for this offset and
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
