Skip to content

Commit 022df2e

Browse files
[lldb] Add test showing UnwindAssemblyInstEmulation can't handle backwards branches
If we have a conditional branch, followed by an epilogue, followed by more code, LLDB will incorrectly compute unwind information through instruction emulation. Consider this: ``` // ... <+16>: b.ne ; <+52> DO_SOMETHING_AND_GOTO_AFTER_EPILOGUE // epilogue start <+20>: ldp x29, x30, [sp, #0x20] <+24>: add sp, sp, #0x30 <+28>: ret // epilogue end AFTER_EPILOGUE: <+32>: do something // ... <+48>: ret DO_SOMETHING_AND_GOTO_AFTER_EPILOGUE: <+52>: stp x22, x23, [sp, #0x10] <+56>: mov x22, #0x1 <+64>: b ; <+32> AFTER_EPILOGUE ``` LLDB will think that the unwind state of +32 is the same as +28. This is false, as +32 _never_ executes after +28. The root cause of the problem is the order in which instructions are visited; they are visited in the order they appear in the text, with unwind state always being forwarded to positive branch offsets, but never to negative offsets. In the example above, `AFTER_EPILOGUE` should inherit the state of the branch in +64, but it doesn't because `AFTER_EPILOGUE` is visited right after the `ret` in +28. Fixing this should be simple: maintain a stack of instructions to visit. While the stack is not empty, take the next instruction on stack and visit it. * After visiting a non-branching instruction, push the next instruction and forward unwind state to it. * After visiting a branch with one or more known targets, push the known branch targets and forward state to them. * In all other cases (ret, or branch to register), don't push nor forward anything. Never push an instruction already on the stack. Like the algorithm today, this new algorithm also assume that, if two instructions branch to the same target, the unwind state in both better be the same. (Note: yes, branch to register is also handled incorrectly today, and will still be incorrect).
1 parent e69d2bf commit 022df2e

File tree

1 file changed

+97
-0
lines changed

1 file changed

+97
-0
lines changed

lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -856,3 +856,100 @@ TEST_F(TestArm64InstEmulation, TestCFAResetToSP) {
856856
EXPECT_TRUE(row->GetCFAValue().GetRegisterNumber() == gpr_sp_arm64);
857857
EXPECT_TRUE(row->GetCFAValue().IsRegisterPlusOffset() == true);
858858
}
859+
860+
TEST_F(TestArm64InstEmulation, TestMidFunctionEpilogueAndBackwardsJump) {
861+
ArchSpec arch("arm64-apple-ios15");
862+
std::unique_ptr<UnwindAssemblyInstEmulation> engine(
863+
static_cast<UnwindAssemblyInstEmulation *>(
864+
UnwindAssemblyInstEmulation::CreateInstance(arch)));
865+
ASSERT_NE(nullptr, engine);
866+
867+
const UnwindPlan::Row *row;
868+
AddressRange sample_range;
869+
UnwindPlan unwind_plan(eRegisterKindLLDB);
870+
UnwindPlan::Row::AbstractRegisterLocation regloc;
871+
872+
uint8_t data[] = {
873+
0xff, 0xc3, 0x00, 0xd1, // <+0>: sub sp, sp, #0x30
874+
0xfd, 0x7b, 0x02, 0xa9, // <+4>: stp x29, x30, [sp, #0x20]
875+
0xfd, 0x83, 0x00, 0x91, // <+8>: add x29, sp, #0x20
876+
0x1f, 0x04, 0x00, 0xf1, // <+12>: cmp x0, #0x1
877+
0x21, 0x01, 0x00, 0x54, // <+16>: b.ne ; <+52> DO_SOMETHING_AND_GOTO_AFTER_EPILOGUE
878+
0xfd, 0x7b, 0x42, 0xa9, // <+20>: ldp x29, x30, [sp, #0x20]
879+
0xff, 0xc3, 0x00, 0x91, // <+24>: add sp, sp, #0x30
880+
0xc0, 0x03, 0x5f, 0xd6, // <+28>: ret
881+
// AFTER_EPILOGUE: LLDB computes the next 5 unwind states incorrectly.
882+
0x37, 0x00, 0x80, 0xd2, // <+32>: mov x23, #0x1
883+
0xf6, 0x5f, 0x41, 0xa9, // <+36>: ldp x22, x23, [sp, #0x10]
884+
0xfd, 0x7b, 0x42, 0xa9, // <+40>: ldp x29, x30, [sp, #0x20]
885+
0xff, 0xc3, 0x00, 0x91, // <+44>: add sp, sp, #0x30
886+
0xc0, 0x03, 0x5f, 0xd6, // <+48>: ret
887+
// DO_SOMETHING_AND_GOTO_AFTER_EPILOGUE
888+
0xf6, 0x5f, 0x01, 0xa9, // <+52>: stp x22, x23, [sp, #0x10]
889+
0x36, 0x00, 0x80, 0xd2, // <+56>: mov x22, #0x1
890+
0x37, 0x00, 0x80, 0xd2, // <+60>: mov x23, #0x1
891+
0xf8, 0xff, 0xff, 0x17, // <+64>: b ; <+32> AFTER_EPILOGUE
892+
};
893+
894+
// UnwindPlan we expect:
895+
// row[0]: 0: CFA=sp +0 =>
896+
// row[1]: 4: CFA=sp+48 =>
897+
// row[2]: 8: CFA=sp+16 => fp=[CFA-16] lr=[CFA-8]
898+
// row[3]: 12: CFA=fp+16 => fp=[CFA-16] lr=[CFA-8]
899+
// row[5]: 24: CFA=sp+48 => fp=<same> lr=<same>
900+
// row[5]: 28: CFA=sp+ 0 => fp=<same> lr=<same>
901+
902+
sample_range = AddressRange(0x1000, sizeof(data));
903+
904+
EXPECT_TRUE(engine->GetNonCallSiteUnwindPlanFromAssembly(
905+
sample_range, data, sizeof(data), unwind_plan));
906+
907+
// At the end of prologue (+12), CFA = fp + 16.
908+
// <+0>: sub sp, sp, #0x30
909+
// <+4>: stp x29, x30, [sp, #0x20]
910+
// <+8>: add x29, sp, #0x20
911+
row = unwind_plan.GetRowForFunctionOffset(12);
912+
EXPECT_EQ(12, row->GetOffset());
913+
EXPECT_TRUE(row->GetCFAValue().IsRegisterPlusOffset());
914+
EXPECT_EQ(row->GetCFAValue().GetRegisterNumber(), gpr_fp_arm64);
915+
EXPECT_EQ(row->GetCFAValue().GetOffset(), 16);
916+
917+
// +16 and +20 are the same as +12.
918+
// <+12>: cmp x0, #0x1
919+
// <+16>: b.ne ; <+52> DO_SOMETHING_AND_GOTO_AFTER_EPILOGUE
920+
EXPECT_EQ(12, unwind_plan.GetRowForFunctionOffset(16)->GetOffset());
921+
EXPECT_EQ(12, unwind_plan.GetRowForFunctionOffset(20)->GetOffset());
922+
923+
// After restoring $fp to caller's value, CFA = $sp + 48
924+
// <+20>: ldp x29, x30, [sp, #0x20]
925+
row = unwind_plan.GetRowForFunctionOffset(24);
926+
EXPECT_EQ(24, row->GetOffset());
927+
EXPECT_TRUE(row->GetCFAValue().IsRegisterPlusOffset());
928+
EXPECT_TRUE(row->GetCFAValue().GetRegisterNumber() == gpr_sp_arm64);
929+
EXPECT_EQ(row->GetCFAValue().GetOffset(), 48);
930+
931+
// $sp has been restored
932+
// <+24>: add sp, sp, #0x30
933+
row = unwind_plan.GetRowForFunctionOffset(28);
934+
EXPECT_EQ(28, row->GetOffset());
935+
EXPECT_TRUE(row->GetCFAValue().IsRegisterPlusOffset());
936+
EXPECT_TRUE(row->GetCFAValue().GetRegisterNumber() == gpr_sp_arm64);
937+
EXPECT_EQ(row->GetCFAValue().GetOffset(), 0);
938+
939+
// FIXME: Row for offset +32 incorrectly inherits the state of the `ret`
940+
// instruction, but +32 _never_ executes after the `ret`.
941+
// <+28>: ret
942+
// <+32>: mov x23, #0x1
943+
row = unwind_plan.GetRowForFunctionOffset(32);
944+
// FIXME: EXPECT_NE(32, row->GetOffset());
945+
946+
// Check that the state of this branch
947+
// <+16>: b.ne ; <+52> DO_SOMETHING_AND_GOTO_AFTER_EPILOGUE
948+
// was forwarded to the branch target:
949+
// <+52>: stp x22, x23, [sp, #0x10]
950+
row = unwind_plan.GetRowForFunctionOffset(52);
951+
EXPECT_EQ(52, row->GetOffset());
952+
EXPECT_TRUE(row->GetCFAValue().IsRegisterPlusOffset());
953+
EXPECT_EQ(row->GetCFAValue().GetRegisterNumber(), gpr_fp_arm64);
954+
EXPECT_EQ(row->GetCFAValue().GetOffset(), 16);
955+
}

0 commit comments

Comments
 (0)