Skip to content

Commit 4e4763a

Browse files
[lldb] Handle backwards branches in UnwindAssemblyInstEmulation (llvm#169633)
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 Part of a sequence of PRs: [lldb][NFCI] Rewrite UnwindAssemblyInstEmulation in terms of a CFG visit llvm#169630 [lldb][NFC] Rename forward_branch_offset to branch_offset in UnwindAssemblyInstEmulation llvm#169631 [lldb] Add DisassemblerLLVMC::IsBarrier API llvm#169632 [lldb] Handle backwards branches in UnwindAssemblyInstEmulation llvm#169633 commit-id:fd266c13
1 parent 4977444 commit 4e4763a

File tree

3 files changed

+28
-9
lines changed

3 files changed

+28
-9
lines changed

lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,10 @@ bool UnwindAssemblyInstEmulation::GetNonCallSiteUnwindPlanFromAssembly(
227227
}
228228
}
229229

230+
// If inst is a barrier, do not propagate state to the next instruction.
231+
if (inst.IsBarrier())
232+
continue;
233+
230234
// Were there any changes to the CFI while evaluating this instruction?
231235
if (m_curr_row_modified) {
232236
// Save the modified row if we don't already have a CFI row in the
@@ -530,19 +534,19 @@ bool UnwindAssemblyInstEmulation::WriteRegister(
530534
case EmulateInstruction::eContextAbsoluteBranchRegister:
531535
case EmulateInstruction::eContextRelativeBranchImmediate: {
532536
if (context.GetInfoType() == EmulateInstruction::eInfoTypeISAAndImmediate &&
533-
context.info.ISAAndImmediate.unsigned_data32 > 0) {
537+
context.info.ISAAndImmediate.unsigned_data32 != 0) {
534538
m_branch_offset = context.info.ISAAndImmediate.unsigned_data32;
535539
} else if (context.GetInfoType() ==
536540
EmulateInstruction::eInfoTypeISAAndImmediateSigned &&
537-
context.info.ISAAndImmediateSigned.signed_data32 > 0) {
541+
context.info.ISAAndImmediateSigned.signed_data32 != 0) {
538542
m_branch_offset = context.info.ISAAndImmediateSigned.signed_data32;
539543
} else if (context.GetInfoType() ==
540544
EmulateInstruction::eInfoTypeImmediate &&
541-
context.info.unsigned_immediate > 0) {
545+
context.info.unsigned_immediate != 0) {
542546
m_branch_offset = context.info.unsigned_immediate;
543547
} else if (context.GetInfoType() ==
544548
EmulateInstruction::eInfoTypeImmediateSigned &&
545-
context.info.signed_immediate > 0) {
549+
context.info.signed_immediate != 0) {
546550
m_branch_offset = context.info.signed_immediate;
547551
}
548552
} break;

lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ class UnwindAssemblyInstEmulation : public lldb_private::UnwindAssembly {
152152
bool m_curr_row_modified;
153153
// The instruction is branching forward with the given offset. 0 value means
154154
// no branching.
155-
uint32_t m_branch_offset = 0;
155+
int64_t m_branch_offset = 0;
156156
};
157157

158158
#endif // LLDB_SOURCE_PLUGINS_UNWINDASSEMBLY_INSTEMULATION_UNWINDASSEMBLYINSTEMULATION_H

lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -987,7 +987,7 @@ TEST_F(TestArm64InstEmulation, TestMidFunctionEpilogueAndBackwardsJump) {
987987
0xfd, 0x7b, 0x42, 0xa9, // <+20>: ldp x29, x30, [sp, #0x20]
988988
0xff, 0xc3, 0x00, 0x91, // <+24>: add sp, sp, #0x30
989989
0xc0, 0x03, 0x5f, 0xd6, // <+28>: ret
990-
// AFTER_EPILOGUE: LLDB computes the next 5 unwind states incorrectly.
990+
// AFTER_EPILOGUE
991991
0x37, 0x00, 0x80, 0xd2, // <+32>: mov x23, #0x1
992992
0xf6, 0x5f, 0x41, 0xa9, // <+36>: ldp x22, x23, [sp, #0x10]
993993
0xfd, 0x7b, 0x42, 0xa9, // <+40>: ldp x29, x30, [sp, #0x20]
@@ -1054,12 +1054,19 @@ TEST_F(TestArm64InstEmulation, TestMidFunctionEpilogueAndBackwardsJump) {
10541054
EXPECT_TRUE(row->GetCFAValue().GetRegisterNumber() == gpr_sp_arm64);
10551055
EXPECT_EQ(row->GetCFAValue().GetOffset(), 0);
10561056

1057-
// FIXME: Row for offset +32 incorrectly inherits the state of the `ret`
1058-
// instruction, but +32 _never_ executes after the `ret`.
1057+
// Row for offset +32 should not inherit the state of the `ret` instruction
1058+
// in +28. Instead, it should inherit the state of the branch in +64.
1059+
// Check for register x22, which is available in row +64.
10591060
// <+28>: ret
10601061
// <+32>: mov x23, #0x1
10611062
row = unwind_plan.GetRowForFunctionOffset(32);
1062-
// FIXME: EXPECT_NE(28, row->GetOffset());
1063+
EXPECT_EQ(32, row->GetOffset());
1064+
{
1065+
UnwindPlan::Row::AbstractRegisterLocation loc;
1066+
EXPECT_TRUE(row->GetRegisterInfo(gpr_x22_arm64, loc));
1067+
EXPECT_TRUE(loc.IsAtCFAPlusOffset());
1068+
EXPECT_EQ(loc.GetOffset(), -32);
1069+
}
10631070

10641071
// Check that the state of this branch
10651072
// <+16>: b.ne ; <+52> DO_SOMETHING_AND_GOTO_AFTER_EPILOGUE
@@ -1070,4 +1077,12 @@ TEST_F(TestArm64InstEmulation, TestMidFunctionEpilogueAndBackwardsJump) {
10701077
EXPECT_TRUE(row->GetCFAValue().IsRegisterPlusOffset());
10711078
EXPECT_EQ(row->GetCFAValue().GetRegisterNumber(), gpr_fp_arm64);
10721079
EXPECT_EQ(row->GetCFAValue().GetOffset(), 16);
1080+
1081+
row = unwind_plan.GetRowForFunctionOffset(64);
1082+
{
1083+
UnwindPlan::Row::AbstractRegisterLocation loc;
1084+
EXPECT_TRUE(row->GetRegisterInfo(gpr_x22_arm64, loc));
1085+
EXPECT_TRUE(loc.IsAtCFAPlusOffset());
1086+
EXPECT_EQ(loc.GetOffset(), -32);
1087+
}
10731088
}

0 commit comments

Comments
 (0)