Skip to content

Commit 80a6823

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 (cherry picked from commit 4e4763a)
1 parent dba9d76 commit 80a6823

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
@@ -879,7 +879,7 @@ TEST_F(TestArm64InstEmulation, TestMidFunctionEpilogueAndBackwardsJump) {
879879
0xfd, 0x7b, 0x42, 0xa9, // <+20>: ldp x29, x30, [sp, #0x20]
880880
0xff, 0xc3, 0x00, 0x91, // <+24>: add sp, sp, #0x30
881881
0xc0, 0x03, 0x5f, 0xd6, // <+28>: ret
882-
// AFTER_EPILOGUE: LLDB computes the next 5 unwind states incorrectly.
882+
// AFTER_EPILOGUE
883883
0x37, 0x00, 0x80, 0xd2, // <+32>: mov x23, #0x1
884884
0xf6, 0x5f, 0x41, 0xa9, // <+36>: ldp x22, x23, [sp, #0x10]
885885
0xfd, 0x7b, 0x42, 0xa9, // <+40>: ldp x29, x30, [sp, #0x20]
@@ -946,12 +946,19 @@ TEST_F(TestArm64InstEmulation, TestMidFunctionEpilogueAndBackwardsJump) {
946946
EXPECT_TRUE(row->GetCFAValue().GetRegisterNumber() == gpr_sp_arm64);
947947
EXPECT_EQ(row->GetCFAValue().GetOffset(), 0);
948948

949-
// FIXME: Row for offset +32 incorrectly inherits the state of the `ret`
950-
// instruction, but +32 _never_ executes after the `ret`.
949+
// Row for offset +32 should not inherit the state of the `ret` instruction
950+
// in +28. Instead, it should inherit the state of the branch in +64.
951+
// Check for register x22, which is available in row +64.
951952
// <+28>: ret
952953
// <+32>: mov x23, #0x1
953954
row = unwind_plan.GetRowForFunctionOffset(32);
954-
// FIXME: EXPECT_NE(28, row->GetOffset());
955+
EXPECT_EQ(32, row->GetOffset());
956+
{
957+
UnwindPlan::Row::AbstractRegisterLocation loc;
958+
EXPECT_TRUE(row->GetRegisterInfo(gpr_x22_arm64, loc));
959+
EXPECT_TRUE(loc.IsAtCFAPlusOffset());
960+
EXPECT_EQ(loc.GetOffset(), -32);
961+
}
955962

956963
// Check that the state of this branch
957964
// <+16>: b.ne ; <+52> DO_SOMETHING_AND_GOTO_AFTER_EPILOGUE
@@ -962,4 +969,12 @@ TEST_F(TestArm64InstEmulation, TestMidFunctionEpilogueAndBackwardsJump) {
962969
EXPECT_TRUE(row->GetCFAValue().IsRegisterPlusOffset());
963970
EXPECT_EQ(row->GetCFAValue().GetRegisterNumber(), gpr_fp_arm64);
964971
EXPECT_EQ(row->GetCFAValue().GetOffset(), 16);
972+
973+
row = unwind_plan.GetRowForFunctionOffset(64);
974+
{
975+
UnwindPlan::Row::AbstractRegisterLocation loc;
976+
EXPECT_TRUE(row->GetRegisterInfo(gpr_x22_arm64, loc));
977+
EXPECT_TRUE(loc.IsAtCFAPlusOffset());
978+
EXPECT_EQ(loc.GetOffset(), -32);
979+
}
965980
}

0 commit comments

Comments
 (0)