Skip to content

Commit 09b533f

Browse files
[lldb] Add test showing UnwindAssemblyInstEmulation can't handle backwards branches (llvm#168398)
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 assumes 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). (cherry picked from commit cd13d9f)
1 parent daf7162 commit 09b533f

File tree

1 file changed

+215
-0
lines changed

1 file changed

+215
-0
lines changed

lldb/unittests/UnwindAssembly/ARM64/TestArm64InstEmulation.cpp

Lines changed: 215 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -856,3 +856,218 @@ 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, TestPrologueStartsWithStrD8) {
861+
ArchSpec arch("aarch64");
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+
// The sample function is built with 'clang --target aarch64 -O1':
873+
//
874+
// int bar(float x);
875+
// int foo(float x) {
876+
// return bar(x) + bar(x);
877+
// }
878+
//
879+
// The function uses one floating point register and spills it with
880+
// 'str d8, [sp, #-0x20]!'.
881+
882+
// clang-format off
883+
uint8_t data[] = {
884+
// prologue
885+
0xe8, 0x0f, 0x1e, 0xfc, // 0: fc1e0fe8 str d8, [sp, #-0x20]!
886+
0xfd, 0xfb, 0x00, 0xa9, // 4: a900fbfd stp x29, x30, [sp, #0x8]
887+
0xf3, 0x0f, 0x00, 0xf9, // 8: f9000ff3 str x19, [sp, #0x18]
888+
0xfd, 0x23, 0x00, 0x91, // 12: 910023fd add x29, sp, #0x8
889+
890+
// epilogue
891+
0xfd, 0xfb, 0x40, 0xa9, // 16: a940fbfd ldp x29, x30, [sp, #0x8]
892+
0xf3, 0x0f, 0x40, 0xf9, // 20: f9400ff3 ldr x19, [sp, #0x18]
893+
0xe8, 0x07, 0x42, 0xfc, // 24: fc4207e8 ldr d8, [sp], #0x20
894+
0xc0, 0x03, 0x5f, 0xd6, // 28: d65f03c0 ret
895+
};
896+
// clang-format on
897+
898+
// UnwindPlan we expect:
899+
// 0: CFA=sp +0 =>
900+
// 4: CFA=sp+32 => d8=[CFA-32]
901+
// 8: CFA=sp+32 => fp=[CFA-24] lr=[CFA-16] d8=[CFA-32]
902+
// 12: CFA=sp+32 => x19=[CFA-8] fp=[CFA-24] lr=[CFA-16] d8=[CFA-32]
903+
// 16: CFA=fp+24 => x19=[CFA-8] fp=[CFA-24] lr=[CFA-16] d8=[CFA-32]
904+
// 20: CFA=sp+32 => x19=[CFA-8] fp=<same> lr=<same> d8=[CFA-32]
905+
// 24: CFA=sp+32 => x19=<same> fp=<same> lr=<same> d8=[CFA-32]
906+
// 28: CFA=sp +0 => x19=<same> fp=<same> lr=<same> d8=<same>
907+
908+
sample_range = AddressRange(0x1000, sizeof(data));
909+
910+
EXPECT_TRUE(engine->GetNonCallSiteUnwindPlanFromAssembly(
911+
sample_range, data, sizeof(data), unwind_plan));
912+
913+
// 4: CFA=sp+32 => d8=[CFA-32]
914+
row = unwind_plan.GetRowForFunctionOffset(4);
915+
EXPECT_EQ(4, row->GetOffset());
916+
EXPECT_TRUE(row->GetCFAValue().GetRegisterNumber() == gpr_sp_arm64);
917+
EXPECT_TRUE(row->GetCFAValue().IsRegisterPlusOffset() == true);
918+
EXPECT_EQ(32, row->GetCFAValue().GetOffset());
919+
920+
EXPECT_TRUE(row->GetRegisterInfo(fpu_d8_arm64, regloc));
921+
EXPECT_TRUE(regloc.IsAtCFAPlusOffset());
922+
EXPECT_EQ(-32, regloc.GetOffset());
923+
924+
// 16: CFA=fp+24 => x19=[CFA-8] fp=[CFA-24] lr=[CFA-16] d8=[CFA-32]
925+
row = unwind_plan.GetRowForFunctionOffset(16);
926+
EXPECT_EQ(16, row->GetOffset());
927+
EXPECT_TRUE(row->GetCFAValue().GetRegisterNumber() == gpr_fp_arm64);
928+
EXPECT_TRUE(row->GetCFAValue().IsRegisterPlusOffset() == true);
929+
EXPECT_EQ(24, row->GetCFAValue().GetOffset());
930+
931+
EXPECT_TRUE(row->GetRegisterInfo(gpr_x19_arm64, regloc));
932+
EXPECT_TRUE(regloc.IsAtCFAPlusOffset());
933+
EXPECT_EQ(-8, regloc.GetOffset());
934+
935+
EXPECT_TRUE(row->GetRegisterInfo(gpr_fp_arm64, regloc));
936+
EXPECT_TRUE(regloc.IsAtCFAPlusOffset());
937+
EXPECT_EQ(-24, regloc.GetOffset());
938+
939+
EXPECT_TRUE(row->GetRegisterInfo(gpr_lr_arm64, regloc));
940+
EXPECT_TRUE(regloc.IsAtCFAPlusOffset());
941+
EXPECT_EQ(-16, regloc.GetOffset());
942+
943+
EXPECT_TRUE(row->GetRegisterInfo(fpu_d8_arm64, regloc));
944+
EXPECT_TRUE(regloc.IsAtCFAPlusOffset());
945+
EXPECT_EQ(-32, regloc.GetOffset());
946+
947+
// 28: CFA=sp +0 => x19=<same> fp=<same> lr=<same> d8=<same>
948+
row = unwind_plan.GetRowForFunctionOffset(28);
949+
EXPECT_EQ(28, row->GetOffset());
950+
EXPECT_TRUE(row->GetCFAValue().GetRegisterNumber() == gpr_sp_arm64);
951+
EXPECT_TRUE(row->GetCFAValue().IsRegisterPlusOffset() == true);
952+
EXPECT_EQ(0, row->GetCFAValue().GetOffset());
953+
954+
if (row->GetRegisterInfo(gpr_x19_arm64, regloc)) {
955+
EXPECT_TRUE(regloc.IsSame());
956+
}
957+
if (row->GetRegisterInfo(gpr_fp_arm64, regloc)) {
958+
EXPECT_TRUE(regloc.IsSame());
959+
}
960+
if (row->GetRegisterInfo(gpr_lr_arm64, regloc)) {
961+
EXPECT_TRUE(regloc.IsSame());
962+
}
963+
if (row->GetRegisterInfo(fpu_d8_arm64, regloc)) {
964+
EXPECT_TRUE(regloc.IsSame());
965+
}
966+
}
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)