llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Felipe de Azevedo Piovezan (felipepiovezan) <details> <summary>Changes</summary> This allows the unwinder to handle code with mid-function epilogues where the subsequent code is reachable through a backwards branch. Two changes are required to accomplish this: 1. Do not enqueue the subsequent instruction if the current instruction is a barrier(*). 2. When processing an instruction, stop ignoring branches with negative offsets. (*) As per the definition in LLVM's MC layer, a barrier is any instruction that "stops control flow from executing the instruction immediately following it". See `MCInstrDesc::isBarrier` in MCInstrDesc.h commit-id:fd266c13 --- Full diff: https://github.com/llvm/llvm-project/pull/169633.diff 3 Files Affected: - (modified) lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp (+8-4) - (modified) lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.h (+1-1) - (modified) lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp (+18-3) ``````````diff diff --git a/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp b/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp index 21a01cc9126a0..c911b98539aec 100644 --- a/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp +++ b/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp @@ -227,6 +227,10 @@ bool UnwindAssemblyInstEmulation::GetNonCallSiteUnwindPlanFromAssembly( } } + // If inst is a barrier, do not propagate state to the next instruction. + if (inst.IsBarrier()) + continue; + // Were there any changes to the CFI while evaluating this instruction? if (m_curr_row_modified) { // Save the modified row if we don't already have a CFI row in the @@ -530,20 +534,20 @@ bool UnwindAssemblyInstEmulation::WriteRegister( case EmulateInstruction::eContextAbsoluteBranchRegister: case EmulateInstruction::eContextRelativeBranchImmediate: { if (context.GetInfoType() == EmulateInstruction::eInfoTypeISAAndImmediate && - context.info.ISAAndImmediate.unsigned_data32 > 0) { + context.info.ISAAndImmediate.unsigned_data32 != 0) { m_branch_offset = context.info.ISAAndImmediate.unsigned_data32; } else if (context.GetInfoType() == EmulateInstruction::eInfoTypeISAAndImmediateSigned && - context.info.ISAAndImmediateSigned.signed_data32 > 0) { + context.info.ISAAndImmediateSigned.signed_data32 != 0) { m_branch_offset = context.info.ISAAndImmediateSigned.signed_data32; } else if (context.GetInfoType() == EmulateInstruction::eInfoTypeImmediate && - context.info.unsigned_immediate > 0) { + context.info.unsigned_immediate != 0) { m_branch_offset = context.info.unsigned_immediate; } else if (context.GetInfoType() == EmulateInstruction::eInfoTypeImmediateSigned && - context.info.signed_immediate > 0) { + context.info.signed_immediate != 0) { m_branch_offset = context.info.signed_immediate; } } break; diff --git a/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.h b/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.h index 1c80199235b4b..97f1093ba50bd 100644 --- a/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.h +++ b/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.h @@ -152,7 +152,7 @@ class UnwindAssemblyInstEmulation : public lldb_private::UnwindAssembly { bool m_curr_row_modified; // The instruction is branching forward with the given offset. 0 value means // no branching. - uint32_t m_branch_offset = 0; + int32_t m_branch_offset = 0; }; #endif // LLDB_SOURCE_PLUGINS_UNWINDASSEMBLY_INSTEMULATION_UNWINDASSEMBLYINSTEMULATION_H diff --git a/lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp b/lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp index 6c74860971674..b006103359801 100644 --- a/lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp +++ b/lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp @@ -1054,12 +1054,19 @@ TEST_F(TestArm64InstEmulation, TestMidFunctionEpilogueAndBackwardsJump) { EXPECT_TRUE(row->GetCFAValue().GetRegisterNumber() == gpr_sp_arm64); EXPECT_EQ(row->GetCFAValue().GetOffset(), 0); - // FIXME: Row for offset +32 incorrectly inherits the state of the `ret` - // instruction, but +32 _never_ executes after the `ret`. + // Row for offset +32 should not inherits the state of the `ret` instruction + // in +28. Instead, it should inherit the state of the branch in +64. + // Check for register x22, which is available in row +64. // <+28>: ret // <+32>: mov x23, #0x1 row = unwind_plan.GetRowForFunctionOffset(32); - // FIXME: EXPECT_NE(28, row->GetOffset()); + EXPECT_EQ(32, row->GetOffset()); + { + UnwindPlan::Row::AbstractRegisterLocation loc; + EXPECT_TRUE(row->GetRegisterInfo(gpr_x22_arm64, loc)); + EXPECT_TRUE(loc.IsAtCFAPlusOffset()); + EXPECT_EQ(loc.GetOffset(), -32); + } // Check that the state of this branch // <+16>: b.ne ; <+52> DO_SOMETHING_AND_GOTO_AFTER_EPILOGUE @@ -1070,4 +1077,12 @@ TEST_F(TestArm64InstEmulation, TestMidFunctionEpilogueAndBackwardsJump) { EXPECT_TRUE(row->GetCFAValue().IsRegisterPlusOffset()); EXPECT_EQ(row->GetCFAValue().GetRegisterNumber(), gpr_fp_arm64); EXPECT_EQ(row->GetCFAValue().GetOffset(), 16); + + row = unwind_plan.GetRowForFunctionOffset(64); + { + UnwindPlan::Row::AbstractRegisterLocation loc; + EXPECT_TRUE(row->GetRegisterInfo(gpr_x22_arm64, loc)); + EXPECT_TRUE(loc.IsAtCFAPlusOffset()); + EXPECT_EQ(loc.GetOffset(), -32); + } } `````````` </details> https://github.com/llvm/llvm-project/pull/169633 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
