Fix the unwinding plan augmentation from x86 assembly

Unwind plan augmentation should compute the plan row at offset x from
the instruction before offset x, but currently we compute it from the
instruction at offset x. Note that this behavior is a regression
introduced when moving the x86 assembly inspection engine to its own
file
(1c9858b298 (diff-375a2be066db6f34bb9a71442c9b71fcL913));
the original version handled this properly by copying the previous
instruction out before advancing the instruction pointer.

The relevant bug with more info is here: https://bugs.llvm.org/show_bug.cgi?id=43561

Differential Revision: https://reviews.llvm.org/D68454
Patch by Jaroslav Sevcik <jarin@google.com>.

llvm-svn: 374342
This commit is contained in:
Pavel Labath 2019-10-10 13:23:09 +00:00
parent c8c71e6f76
commit 5070322332
2 changed files with 105 additions and 15 deletions

View File

@ -1371,7 +1371,6 @@ bool x86AssemblyInspectionEngine::AugmentUnwindPlanFromCallSite(
int row_id = 1;
bool unwind_plan_updated = false;
UnwindPlan::RowSP row(new UnwindPlan::Row(*first_row));
m_cur_insn = data + offset;
// After a mid-function epilogue we will need to re-insert the original
// unwind rules so unwinds work for the remainder of the function. These
@ -1381,19 +1380,17 @@ bool x86AssemblyInspectionEngine::AugmentUnwindPlanFromCallSite(
while (offset < size) {
m_cur_insn = data + offset;
int insn_len;
if (!instruction_length(m_cur_insn, insn_len, size - offset)
|| insn_len == 0
|| insn_len > kMaxInstructionByteSize) {
if (!instruction_length(m_cur_insn, insn_len, size - offset) ||
insn_len == 0 || insn_len > kMaxInstructionByteSize) {
// An unrecognized/junk instruction.
break;
}
// Advance offsets.
offset += insn_len;
m_cur_insn = data + offset;
// offset is pointing beyond the bounds of the function; stop looping.
if (offset >= size)
if (offset >= size)
continue;
if (reinstate_unwind_state) {
@ -1547,16 +1544,18 @@ bool x86AssemblyInspectionEngine::AugmentUnwindPlanFromCallSite(
// [0x5d] pop %rbp/%ebp
// => [0xc3] ret
if (pop_rbp_pattern_p() || leave_pattern_p()) {
offset += 1;
row->SetOffset(offset);
row->GetCFAValue().SetIsRegisterPlusOffset(
first_row->GetCFAValue().GetRegisterNumber(), m_wordsize);
m_cur_insn++;
if (ret_pattern_p()) {
row->SetOffset(offset);
row->GetCFAValue().SetIsRegisterPlusOffset(
first_row->GetCFAValue().GetRegisterNumber(), m_wordsize);
UnwindPlan::RowSP new_row(new UnwindPlan::Row(*row));
unwind_plan.InsertRow(new_row);
unwind_plan_updated = true;
reinstate_unwind_state = true;
continue;
UnwindPlan::RowSP new_row(new UnwindPlan::Row(*row));
unwind_plan.InsertRow(new_row);
unwind_plan_updated = true;
reinstate_unwind_state = true;
continue;
}
}
} else {
// CFA register is not sp or fp.

View File

@ -2199,6 +2199,97 @@ TEST_F(Testx86AssemblyInspectionEngine, TestSpillRegToStackViaMOVi386) {
EXPECT_EQ(-40, regloc.GetOffset());
}
TEST_F(Testx86AssemblyInspectionEngine, TestSpArithx86_64Augmented) {
UnwindPlan::Row::RegisterLocation regloc;
UnwindPlan::RowSP row_sp;
AddressRange sample_range;
UnwindPlan unwind_plan(eRegisterKindLLDB);
std::unique_ptr<x86AssemblyInspectionEngine> engine64 = Getx86_64Inspector();
uint8_t data[] = {
0x55, // pushq %rbp
0x48, 0x89, 0xe5, // movq %rsp, %rbp
// x86AssemblyInspectionEngine::AugmentUnwindPlanFromCallSite
// has a bug where it can't augment a function that is just
// prologue+epilogue - it needs at least one other instruction
// in between.
0x90, // nop
0x48, 0x81, 0xec, 0x88, 0, 0, 0, // subq $0x88, %rsp
0x90, // nop
0x48, 0x81, 0xc4, 0x88, 0, 0, 0, // addq $0x88, %rsp
0x5d, // popq %rbp
0xc3 // retq
};
sample_range = AddressRange(0x1000, sizeof(data));
unwind_plan.SetSourceName("unit testing hand-created unwind plan");
unwind_plan.SetPlanValidAddressRange(sample_range);
unwind_plan.SetRegisterKind(eRegisterKindLLDB);
row_sp = std::make_shared<UnwindPlan::Row>();
// Describe offset 0
row_sp->SetOffset(0);
row_sp->GetCFAValue().SetIsRegisterPlusOffset(k_rsp, 8);
regloc.SetAtCFAPlusOffset(-8);
row_sp->SetRegisterInfo(k_rip, regloc);
unwind_plan.AppendRow(row_sp);
// Allocate a new Row, populate it with the existing Row contents.
UnwindPlan::Row *new_row = new UnwindPlan::Row;
*new_row = *row_sp.get();
row_sp.reset(new_row);
// Describe offset 1
row_sp->SetOffset(1);
row_sp->GetCFAValue().SetIsRegisterPlusOffset(k_rsp, 16);
regloc.SetAtCFAPlusOffset(-16);
row_sp->SetRegisterInfo(k_rbp, regloc);
unwind_plan.AppendRow(row_sp);
// Allocate a new Row, populate it with the existing Row contents.
new_row = new UnwindPlan::Row;
*new_row = *row_sp.get();
row_sp.reset(new_row);
// Describe offset 4
row_sp->SetOffset(4);
row_sp->GetCFAValue().SetIsRegisterPlusOffset(k_rsp, 16);
unwind_plan.AppendRow(row_sp);
RegisterContextSP reg_ctx_sp;
EXPECT_TRUE(engine64->AugmentUnwindPlanFromCallSite(
data, sizeof(data), sample_range, unwind_plan, reg_ctx_sp));
// Before we touch the stack pointer, we should still refer to the
// row from after the prologue.
row_sp = unwind_plan.GetRowForFunctionOffset(5);
EXPECT_EQ(4ull, row_sp->GetOffset());
// Check the first stack pointer update.
row_sp = unwind_plan.GetRowForFunctionOffset(12);
EXPECT_EQ(12ull, row_sp->GetOffset());
EXPECT_TRUE(row_sp->GetCFAValue().GetRegisterNumber() == k_rsp);
EXPECT_EQ(152, row_sp->GetCFAValue().GetOffset());
// After the nop, we should still refer to the same row.
row_sp = unwind_plan.GetRowForFunctionOffset(13);
EXPECT_EQ(12ull, row_sp->GetOffset());
// Check that the second stack pointer update is reflected in the
// unwind plan.
row_sp = unwind_plan.GetRowForFunctionOffset(20);
EXPECT_EQ(20ull, row_sp->GetOffset());
EXPECT_TRUE(row_sp->GetCFAValue().GetRegisterNumber() == k_rsp);
EXPECT_EQ(16, row_sp->GetCFAValue().GetOffset());
}
TEST_F(Testx86AssemblyInspectionEngine, TestSimplex86_64Augmented) {
UnwindPlan::Row::RegisterLocation regloc;
UnwindPlan::RowSP row_sp;