Skip to content

Commit 2935e91

Browse files
jasonmolendajasonmolenda
authored
Merge pull request #9376 from jasonmolenda/cp/r137153323-handle-non-epilogue-instruction-mid-epilogue
[lldb] Improve mid-function epilogue scanning for x86 (llvm#110965)
2 parents 94ecd6e + c5b4778 commit 2935e91

File tree

2 files changed

+37
-18
lines changed

2 files changed

+37
-18
lines changed

‎lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -809,7 +809,7 @@ bool x86AssemblyInspectionEngine::local_branch_p (
809809
// Branch target is before the start of this function
810810
return false;
811811
}
812-
if (offset + next_pc_value > func_range.GetByteSize()) {
812+
if (offset + next_pc_value >= func_range.GetByteSize()) {
813813
// Branch targets outside this function's bounds
814814
return false;
815815
}
@@ -967,6 +967,8 @@ bool x86AssemblyInspectionEngine::GetNonCallSiteUnwindPlanFromAssembly(
967967

968968
bool in_epilogue = false; // we're in the middle of an epilogue sequence
969969
bool row_updated = false; // The UnwindPlan::Row 'row' has been updated
970+
bool current_sp_offset_updated =
971+
false; // current_sp_bytes_offset_from_fa has been updated this insn
970972

971973
m_cur_insn = data + current_func_text_offset;
972974
if (!instruction_length(m_cur_insn, insn_len, size - current_func_text_offset)
@@ -1013,8 +1015,10 @@ bool x86AssemblyInspectionEngine::GetNonCallSiteUnwindPlanFromAssembly(
10131015
afa_value.SetUnspecified();
10141016
row_updated = true;
10151017
}
1016-
if (fa_value_ptr->GetRegisterNumber() == m_lldb_fp_regnum)
1018+
if (fa_value_ptr->GetRegisterNumber() == m_lldb_fp_regnum) {
10171019
current_sp_bytes_offset_from_fa = fa_value_ptr->GetOffset();
1020+
current_sp_offset_updated = true;
1021+
}
10181022
}
10191023

10201024
else if (mov_rbx_rsp_pattern_p()) {
@@ -1025,8 +1029,10 @@ bool x86AssemblyInspectionEngine::GetNonCallSiteUnwindPlanFromAssembly(
10251029
afa_value.SetUnspecified();
10261030
row_updated = true;
10271031
}
1028-
if (fa_value_ptr->GetRegisterNumber() == m_lldb_alt_fp_regnum)
1032+
if (fa_value_ptr->GetRegisterNumber() == m_lldb_alt_fp_regnum) {
10291033
current_sp_bytes_offset_from_fa = fa_value_ptr->GetOffset();
1034+
current_sp_offset_updated = true;
1035+
}
10301036
}
10311037

10321038
// This is the start() function (or a pthread equivalent), it starts with a
@@ -1039,6 +1045,7 @@ bool x86AssemblyInspectionEngine::GetNonCallSiteUnwindPlanFromAssembly(
10391045

10401046
else if (push_reg_p(machine_regno)) {
10411047
current_sp_bytes_offset_from_fa += m_wordsize;
1048+
current_sp_offset_updated = true;
10421049
// the PUSH instruction has moved the stack pointer - if the FA is set
10431050
// in terms of the stack pointer, we need to add a new row of
10441051
// instructions.
@@ -1064,6 +1071,7 @@ bool x86AssemblyInspectionEngine::GetNonCallSiteUnwindPlanFromAssembly(
10641071

10651072
else if (pop_reg_p(machine_regno)) {
10661073
current_sp_bytes_offset_from_fa -= m_wordsize;
1074+
current_sp_offset_updated = true;
10671075

10681076
if (nonvolatile_reg_p(machine_regno) &&
10691077
machine_regno_to_lldb_regno(machine_regno, lldb_regno) &&
@@ -1091,6 +1099,7 @@ bool x86AssemblyInspectionEngine::GetNonCallSiteUnwindPlanFromAssembly(
10911099

10921100
else if (pop_misc_reg_p()) {
10931101
current_sp_bytes_offset_from_fa -= m_wordsize;
1102+
current_sp_offset_updated = true;
10941103
if (fa_value_ptr->GetRegisterNumber() == m_lldb_sp_regnum) {
10951104
fa_value_ptr->SetIsRegisterPlusOffset(
10961105
m_lldb_sp_regnum, current_sp_bytes_offset_from_fa);
@@ -1126,6 +1135,7 @@ bool x86AssemblyInspectionEngine::GetNonCallSiteUnwindPlanFromAssembly(
11261135
}
11271136

11281137
current_sp_bytes_offset_from_fa -= m_wordsize;
1138+
current_sp_offset_updated = true;
11291139

11301140
if (fa_value_ptr->GetRegisterNumber() == m_lldb_sp_regnum) {
11311141
fa_value_ptr->SetIsRegisterPlusOffset(
@@ -1161,6 +1171,7 @@ bool x86AssemblyInspectionEngine::GetNonCallSiteUnwindPlanFromAssembly(
11611171

11621172
else if (sub_rsp_pattern_p(stack_offset)) {
11631173
current_sp_bytes_offset_from_fa += stack_offset;
1174+
current_sp_offset_updated = true;
11641175
if (fa_value_ptr->GetRegisterNumber() == m_lldb_sp_regnum) {
11651176
fa_value_ptr->SetOffset(current_sp_bytes_offset_from_fa);
11661177
row_updated = true;
@@ -1169,6 +1180,7 @@ bool x86AssemblyInspectionEngine::GetNonCallSiteUnwindPlanFromAssembly(
11691180

11701181
else if (add_rsp_pattern_p(stack_offset)) {
11711182
current_sp_bytes_offset_from_fa -= stack_offset;
1183+
current_sp_offset_updated = true;
11721184
if (fa_value_ptr->GetRegisterNumber() == m_lldb_sp_regnum) {
11731185
fa_value_ptr->SetOffset(current_sp_bytes_offset_from_fa);
11741186
row_updated = true;
@@ -1179,6 +1191,7 @@ bool x86AssemblyInspectionEngine::GetNonCallSiteUnwindPlanFromAssembly(
11791191
else if (push_extended_pattern_p() || push_imm_pattern_p() ||
11801192
push_misc_reg_p()) {
11811193
current_sp_bytes_offset_from_fa += m_wordsize;
1194+
current_sp_offset_updated = true;
11821195
if (fa_value_ptr->GetRegisterNumber() == m_lldb_sp_regnum) {
11831196
fa_value_ptr->SetOffset(current_sp_bytes_offset_from_fa);
11841197
row_updated = true;
@@ -1187,6 +1200,7 @@ bool x86AssemblyInspectionEngine::GetNonCallSiteUnwindPlanFromAssembly(
11871200

11881201
else if (lea_rsp_pattern_p(stack_offset)) {
11891202
current_sp_bytes_offset_from_fa -= stack_offset;
1203+
current_sp_offset_updated = true;
11901204
if (fa_value_ptr->GetRegisterNumber() == m_lldb_sp_regnum) {
11911205
fa_value_ptr->SetOffset(current_sp_bytes_offset_from_fa);
11921206
row_updated = true;
@@ -1206,6 +1220,7 @@ bool x86AssemblyInspectionEngine::GetNonCallSiteUnwindPlanFromAssembly(
12061220
if (fa_value_ptr->GetRegisterNumber() == m_lldb_fp_regnum) {
12071221
current_sp_bytes_offset_from_fa =
12081222
fa_value_ptr->GetOffset() - stack_offset;
1223+
current_sp_offset_updated = true;
12091224
}
12101225
}
12111226

@@ -1219,6 +1234,7 @@ bool x86AssemblyInspectionEngine::GetNonCallSiteUnwindPlanFromAssembly(
12191234
}
12201235
if (fa_value_ptr->GetRegisterNumber() == m_lldb_alt_fp_regnum) {
12211236
current_sp_bytes_offset_from_fa = fa_value_ptr->GetOffset() - stack_offset;
1237+
current_sp_offset_updated = true;
12221238
}
12231239
}
12241240

@@ -1251,6 +1267,7 @@ bool x86AssemblyInspectionEngine::GetNonCallSiteUnwindPlanFromAssembly(
12511267
row.reset(newrow);
12521268
current_sp_bytes_offset_from_fa =
12531269
prologue_completed_sp_bytes_offset_from_cfa;
1270+
current_sp_offset_updated = true;
12541271
is_aligned = prologue_completed_is_aligned;
12551272

12561273
saved_registers.clear();
@@ -1272,6 +1289,7 @@ bool x86AssemblyInspectionEngine::GetNonCallSiteUnwindPlanFromAssembly(
12721289
// global data
12731290
else if (call_next_insn_pattern_p()) {
12741291
current_sp_bytes_offset_from_fa += m_wordsize;
1292+
current_sp_offset_updated = true;
12751293
if (fa_value_ptr->GetRegisterNumber() == m_lldb_sp_regnum) {
12761294
fa_value_ptr->SetOffset(current_sp_bytes_offset_from_fa);
12771295
row_updated = true;
@@ -1304,7 +1322,7 @@ bool x86AssemblyInspectionEngine::GetNonCallSiteUnwindPlanFromAssembly(
13041322

13051323
// We may change the sp value without adding a new Row necessarily -- keep
13061324
// track of it either way.
1307-
if (!in_epilogue) {
1325+
if (!in_epilogue && current_sp_offset_updated) {
13081326
prologue_completed_sp_bytes_offset_from_cfa =
13091327
current_sp_bytes_offset_from_fa;
13101328
prologue_completed_is_aligned = is_aligned;

‎lldb/unittests/UnwindAssembly/x86/Testx86AssemblyInspectionEngine.cpp

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2859,16 +2859,17 @@ TEST_F(Testx86AssemblyInspectionEngine, TestDisassemblyMidFunctionEpilogues) {
28592859

28602860
0x90, // <+18>: nop // prologue setup back
28612861

2862-
0x74, 0x7, // <+19>: je 6 <+27>
2862+
0x74, 0x8, // <+19>: je 7 <+28>
28632863
0x48, 0x83, 0xc4, 0x70, // <+21>: addq $0x70, %rsp
28642864
0x5d, // <+25>: popq %rbp
2865-
0xc3, // <+26>: retq // epilogue completed
2865+
0x90, // <+26>: nop // mid-epilogue non-epilogue
2866+
0xc3, // <+27>: retq // epilogue completed
28662867

2867-
0x90, // <+27>: nop // prologue setup back
2868+
0x90, // <+28>: nop // prologue setup back
28682869

2869-
0x48, 0x83, 0xc4, 0x70, // <+28>: addq $0x70, %rsp
2870-
0x5d, // <+32>: popq %rbp
2871-
0xc3, // <+33>: retq // epilogue completed
2870+
0x48, 0x83, 0xc4, 0x70, // <+29>: addq $0x70, %rsp
2871+
0x5d, // <+33>: popq %rbp
2872+
0xc3, // <+34>: retq // epilogue completed
28722873

28732874
};
28742875

@@ -2898,17 +2899,17 @@ TEST_F(Testx86AssemblyInspectionEngine, TestDisassemblyMidFunctionEpilogues) {
28982899
// Check that we've reinstated the stack frame setup
28992900
// unwind instructions after a mid-function retq
29002901
// row: CFA=ebp +8 => esp=CFA+0 eip=[CFA-8]
2901-
row_sp = unwind_plan.GetRowForFunctionOffset(27);
2902-
EXPECT_EQ(27ull, row_sp->GetOffset());
2902+
row_sp = unwind_plan.GetRowForFunctionOffset(28);
2903+
EXPECT_EQ(28ull, row_sp->GetOffset());
29032904
EXPECT_TRUE(row_sp->GetCFAValue().GetRegisterNumber() == k_ebp);
29042905
EXPECT_TRUE(row_sp->GetCFAValue().IsRegisterPlusOffset() == true);
29052906
EXPECT_EQ(wordsize * 2, row_sp->GetCFAValue().GetOffset());
29062907

29072908
// After last instruction in the function, verify that
29082909
// the stack frame has been unwound
29092910
// row: CFA=esp +4 => esp=CFA+0 eip=[CFA-4]
2910-
row_sp = unwind_plan.GetRowForFunctionOffset(33);
2911-
EXPECT_EQ(33ull, row_sp->GetOffset());
2911+
row_sp = unwind_plan.GetRowForFunctionOffset(34);
2912+
EXPECT_EQ(34ull, row_sp->GetOffset());
29122913
EXPECT_TRUE(row_sp->GetCFAValue().GetRegisterNumber() == k_esp);
29132914
EXPECT_TRUE(row_sp->GetCFAValue().IsRegisterPlusOffset() == true);
29142915
EXPECT_EQ(wordsize, row_sp->GetCFAValue().GetOffset());
@@ -2940,17 +2941,17 @@ TEST_F(Testx86AssemblyInspectionEngine, TestDisassemblyMidFunctionEpilogues) {
29402941
// Check that we've reinstated the stack frame setup
29412942
// unwind instructions after a mid-function retq
29422943
// row: CFA=rbp+16 => rsp=CFA+0 rip=[CFA-16]
2943-
row_sp = unwind_plan.GetRowForFunctionOffset(27);
2944-
EXPECT_EQ(27ull, row_sp->GetOffset());
2944+
row_sp = unwind_plan.GetRowForFunctionOffset(28);
2945+
EXPECT_EQ(28ull, row_sp->GetOffset());
29452946
EXPECT_TRUE(row_sp->GetCFAValue().GetRegisterNumber() == k_rbp);
29462947
EXPECT_TRUE(row_sp->GetCFAValue().IsRegisterPlusOffset() == true);
29472948
EXPECT_EQ(wordsize * 2, row_sp->GetCFAValue().GetOffset());
29482949

29492950
// After last instruction in the function, verify that
29502951
// the stack frame has been unwound
29512952
// row: CFA=rsp +8 => esp=CFA+0 rip=[CFA-8]
2952-
row_sp = unwind_plan.GetRowForFunctionOffset(33);
2953-
EXPECT_EQ(33ull, row_sp->GetOffset());
2953+
row_sp = unwind_plan.GetRowForFunctionOffset(34);
2954+
EXPECT_EQ(34ull, row_sp->GetOffset());
29542955
EXPECT_TRUE(row_sp->GetCFAValue().GetRegisterNumber() == k_rsp);
29552956
EXPECT_TRUE(row_sp->GetCFAValue().IsRegisterPlusOffset() == true);
29562957
EXPECT_EQ(wordsize, row_sp->GetCFAValue().GetOffset());

0 commit comments

Comments
 (0)