Skip to content

Commit

Permalink
Bug where insn-based unwind plans on arm64 could be wrong (#1082)
Browse files Browse the repository at this point in the history 
Fix a bug where UnwindAssemblyInstEmulation would confuse which
register is used to compute the Canonical Frame Address after it
had branched over a mid-function epilogue (where the CFA reg changes
from $fp to $sp in the process of epiloguing).  Reinstate the
correct CFA register after we forward the unwind rule for branch
targets.  The failure mode was that UnwindAssemblyInstEmulation
would think CFA was set in terms of $sp after one of these epilogues,
and if it sees modifications to $sp after the branch target, it would
change the CFA offset in the unwind rule -- even though the CFA is
defined in terms of $fp and the $sp changes are irrelevant to correct
calculation.

<rdar://problem/60300528>

Differential Revision: https://reviews.llvm.org/D78077

(cherry picked from commit 1cd92e4)
  • Loading branch information
@jasonmolenda
jasonmolenda committed Apr 15, 2020
1 parent 01397d1 commit 1be5d83
Show file tree
Hide file tree
Showing 2 changed files with 137 additions and 9 deletions.
46 changes: 37 additions & 9 deletions lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,18 +123,12 @@ bool UnwindAssemblyInstEmulation::GetNonCallSiteUnwindPlanFromAssembly(
// 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.
Expand Down Expand Up @@ -165,6 +159,23 @@ bool UnwindAssemblyInstEmulation::GetNonCallSiteUnwindPlanFromAssembly(
*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(),
Expand Down Expand Up @@ -195,6 +206,23 @@ bool UnwindAssemblyInstEmulation::GetNonCallSiteUnwindPlanFromAssembly(
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
Expand Down
100 changes: 100 additions & 0 deletions lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -679,3 +679,103 @@ TEST_F(TestArm64InstEmulation, TestRegisterDoubleSpills) {
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());
}

0 comments on commit 1be5d83

Please sign in to comment.