Skip to content

Commit 2f81505

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 93a1327 commit 2f81505

File tree

1 file changed

+107
-0
lines changed

1 file changed

+107
-0
lines changed

lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -964,3 +964,110 @@ TEST_F(TestArm64InstEmulation, TestPrologueStartsWithStrD8) {
964964
EXPECT_TRUE(regloc.IsSame());
965965
}
966966
}
967+
968+
TEST_F(TestArm64InstEmulation, TestMidFunctionEpilogueAndBackwardsJump) {
969+
ArchSpec arch("arm64-apple-ios15");
970+
std::unique_ptr<UnwindAssemblyInstEmulation> engine(
971+
static_cast<UnwindAssemblyInstEmulation *>(
972+
UnwindAssemblyInstEmulation::CreateInstance(arch)));
973+
ASSERT_NE(nullptr, engine);
974+
975+
const UnwindPlan::Row *row;
976+
AddressRange sample_range;
977+
UnwindPlan unwind_plan(eRegisterKindLLDB);
978+
UnwindPlan::Row::AbstractRegisterLocation regloc;
979+
980+
// clang-format off
981+
uint8_t data[] = {
982+
0xff, 0xc3, 0x00, 0xd1, // <+0>: sub sp, sp, #0x30
983+
0xfd, 0x7b, 0x02, 0xa9, // <+4>: stp x29, x30, [sp, #0x20]
984+
0xfd, 0x83, 0x00, 0x91, // <+8>: add x29, sp, #0x20
985+
0x1f, 0x04, 0x00, 0xf1, // <+12>: cmp x0, #0x1
986+
0x21, 0x01, 0x00, 0x54, // <+16>: b.ne ; <+52> DO_SOMETHING_AND_GOTO_AFTER_EPILOGUE
987+
0xfd, 0x7b, 0x42, 0xa9, // <+20>: ldp x29, x30, [sp, #0x20]
988+
0xff, 0xc3, 0x00, 0x91, // <+24>: add sp, sp, #0x30
989+
0xc0, 0x03, 0x5f, 0xd6, // <+28>: ret
990+
// AFTER_EPILOGUE: LLDB computes the next 5 unwind states incorrectly.
991+
0x37, 0x00, 0x80, 0xd2, // <+32>: mov x23, #0x1
992+
0xf6, 0x5f, 0x41, 0xa9, // <+36>: ldp x22, x23, [sp, #0x10]
993+
0xfd, 0x7b, 0x42, 0xa9, // <+40>: ldp x29, x30, [sp, #0x20]
994+
0xff, 0xc3, 0x00, 0x91, // <+44>: add sp, sp, #0x30
995+
0xc0, 0x03, 0x5f, 0xd6, // <+48>: ret
996+
// DO_SOMETHING_AND_GOTO_AFTER_EPILOGUE
997+
0xf6, 0x5f, 0x01, 0xa9, // <+52>: stp x22, x23, [sp, #0x10]
998+
0x36, 0x00, 0x80, 0xd2, // <+56>: mov x22, #0x1
999+
0x37, 0x00, 0x80, 0xd2, // <+60>: mov x23, #0x1
1000+
0xf8, 0xff, 0xff, 0x17, // <+64>: b ; <+32> AFTER_EPILOGUE
1001+
};
1002+
1003+
// UnwindPlan we expect:
1004+
// row[0]: 0: CFA=sp +0 =>
1005+
// row[1]: 4: CFA=sp+48 =>
1006+
// row[2]: 8: CFA=sp+16 => fp=[CFA-16] lr=[CFA-8]
1007+
// row[3]: 12: CFA=fp+16 => fp=[CFA-16] lr=[CFA-8]
1008+
// row[4]: 24: CFA=sp+48 => fp=<same> lr=<same>
1009+
//
1010+
// This must come from +56
1011+
// row[5]: 32: CFA=fp+16 => fp=[CFA-16] lr=[CFA-8] x22=[CFA-24], x23=[CFA-32]
1012+
// row[6]: 40: CFA=fp+16 => fp=[CFA-16] lr=[CFA-8] x22=same, x23 = same
1013+
// row[6]: 44: CFA=sp+48 => fp=same lr=same x22=same, x23 = same
1014+
// row[6]: 48: CFA=sp0 => fp=same lr=same x22=same, x23 = same
1015+
//
1016+
// row[x]: 52: CFA=fp+16 => fp=[CFA-16] lr=[CFA-8]
1017+
// row[x]: 56: CFA=fp+16 => fp=[CFA-16] lr=[CFA-8] x22=[CFA-24], x23=[CFA-32]
1018+
// clang-format on
1019+
1020+
sample_range = AddressRange(0x1000, sizeof(data));
1021+
1022+
EXPECT_TRUE(engine->GetNonCallSiteUnwindPlanFromAssembly(
1023+
sample_range, data, sizeof(data), unwind_plan));
1024+
1025+
// At the end of prologue (+12), CFA = fp + 16.
1026+
// <+0>: sub sp, sp, #0x30
1027+
// <+4>: stp x29, x30, [sp, #0x20]
1028+
// <+8>: add x29, sp, #0x20
1029+
row = unwind_plan.GetRowForFunctionOffset(12);
1030+
EXPECT_EQ(12, row->GetOffset());
1031+
EXPECT_TRUE(row->GetCFAValue().IsRegisterPlusOffset());
1032+
EXPECT_EQ(row->GetCFAValue().GetRegisterNumber(), gpr_fp_arm64);
1033+
EXPECT_EQ(row->GetCFAValue().GetOffset(), 16);
1034+
1035+
// +16 and +20 are the same as +12.
1036+
// <+12>: cmp x0, #0x1
1037+
// <+16>: b.ne ; <+52> DO_SOMETHING_AND_GOTO_AFTER_EPILOGUE
1038+
EXPECT_EQ(12, unwind_plan.GetRowForFunctionOffset(16)->GetOffset());
1039+
EXPECT_EQ(12, unwind_plan.GetRowForFunctionOffset(20)->GetOffset());
1040+
1041+
// After restoring $fp to caller's value, CFA = $sp + 48
1042+
// <+20>: ldp x29, x30, [sp, #0x20]
1043+
row = unwind_plan.GetRowForFunctionOffset(24);
1044+
EXPECT_EQ(24, row->GetOffset());
1045+
EXPECT_TRUE(row->GetCFAValue().IsRegisterPlusOffset());
1046+
EXPECT_TRUE(row->GetCFAValue().GetRegisterNumber() == gpr_sp_arm64);
1047+
EXPECT_EQ(row->GetCFAValue().GetOffset(), 48);
1048+
1049+
// $sp has been restored
1050+
// <+24>: add sp, sp, #0x30
1051+
row = unwind_plan.GetRowForFunctionOffset(28);
1052+
EXPECT_EQ(28, row->GetOffset());
1053+
EXPECT_TRUE(row->GetCFAValue().IsRegisterPlusOffset());
1054+
EXPECT_TRUE(row->GetCFAValue().GetRegisterNumber() == gpr_sp_arm64);
1055+
EXPECT_EQ(row->GetCFAValue().GetOffset(), 0);
1056+
1057+
// FIXME: Row for offset +32 incorrectly inherits the state of the `ret`
1058+
// instruction, but +32 _never_ executes after the `ret`.
1059+
// <+28>: ret
1060+
// <+32>: mov x23, #0x1
1061+
row = unwind_plan.GetRowForFunctionOffset(32);
1062+
// FIXME: EXPECT_NE(32, row->GetOffset());
1063+
1064+
// Check that the state of this branch
1065+
// <+16>: b.ne ; <+52> DO_SOMETHING_AND_GOTO_AFTER_EPILOGUE
1066+
// was forwarded to the branch target:
1067+
// <+52>: stp x22, x23, [sp, #0x10]
1068+
row = unwind_plan.GetRowForFunctionOffset(52);
1069+
EXPECT_EQ(52, row->GetOffset());
1070+
EXPECT_TRUE(row->GetCFAValue().IsRegisterPlusOffset());
1071+
EXPECT_EQ(row->GetCFAValue().GetRegisterNumber(), gpr_fp_arm64);
1072+
EXPECT_EQ(row->GetCFAValue().GetOffset(), 16);
1073+
}

0 commit comments

Comments
 (0)