jasonmolenda updated this revision to Diff 257557.
jasonmolenda added a comment.
Update to address mistake Greg identified; also remove two unused variables
that were in this method before my changes.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78077/new/
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
@@ -125,18 +125,12 @@
// Add the initial state to the save list with offset 0.
saved_unwind_states.insert({0, {last_row, m_register_values}});
- // cache the pc register number (in whatever register numbering this
- // UnwindPlan uses) for quick reference during instruction parsing.
- RegisterInfo pc_reg_info;
- m_inst_emulator_up->GetRegisterInfo(
- eRegisterKindGeneric, LLDB_REGNUM_GENERIC_PC, pc_reg_info);
-
- // cache the return address register number (in whatever register
+ // cache the stack pointer register number (in whatever register
// numbering this UnwindPlan uses) for quick reference during
// instruction parsing.
- RegisterInfo ra_reg_info;
+ RegisterInfo sp_reg_info;
m_inst_emulator_up->GetRegisterInfo(
- eRegisterKindGeneric, LLDB_REGNUM_GENERIC_RA, ra_reg_info);
+ eRegisterKindGeneric, LLDB_REGNUM_GENERIC_SP, sp_reg_info);
// The architecture dependent condition code of the last processed
// instruction.
@@ -167,6 +161,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 +208,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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
