Author: Jason Molenda Date: 2020-04-14T16:57:25-07:00 New Revision: 1cd92e480c12c03ab9a381b29e4e3964892afa01
URL: https://github.com/llvm/llvm-project/commit/1cd92e480c12c03ab9a381b29e4e3964892afa01 DIFF: https://github.com/llvm/llvm-project/commit/1cd92e480c12c03ab9a381b29e4e3964892afa01.diff LOG: Bug where insn-based unwind plans on arm64 could be wrong 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 Added: Modified: lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp Removed: ################################################################################ diff --git a/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp b/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp index ba7544fb52dd..1bc071c2b161 100644 --- a/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp +++ b/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp @@ -125,18 +125,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. @@ -167,6 +161,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(), @@ -197,6 +208,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 diff --git a/lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp b/lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp index 3be15c04a203..6c8bfa1a57f6 100644 --- a/lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp +++ b/lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp @@ -678,3 +678,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()); +} + _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
