release/21.x: [lldb] Implement DW_CFA_val_offset and DW_CFA_val_offset_sf (#150732)

[llvmbot]

Backport c455c4e

Requested by: @dsandersllvm

[llvmbot]

@labath What do you think about merging this PR to the release branch?

[llvmbot]

@llvm/pr-subscribers-lldb

Author: None (llvmbot)

Changes

Backport c455c4e

Requested by: @dsandersllvm

---

Full diff: https://github.com/llvm/llvm-project/pull/166611.diff

6 Files Affected:

• (modified) lldb/include/lldb/Target/UnwindLLDB.h (+8)
• (modified) lldb/source/Symbol/DWARFCallFrameInfo.cpp (+26-2)
• (modified) lldb/source/Target/RegisterContextUnwind.cpp (+29-1)
• (added) lldb/test/Shell/Unwind/Inputs/eh-frame-dwarf-unwind-val-offset.s (+50)
• (added) lldb/test/Shell/Unwind/eh-frame-dwarf-unwind-val-offset.test (+58)
• (modified) lldb/unittests/Symbol/TestDWARFCallFrameInfo.cpp (+124)

diff --git a/lldb/include/lldb/Target/UnwindLLDB.h b/lldb/include/lldb/Target/UnwindLLDB.h index f2f65e67a7640..88180b37fd93a 100644 --- a/lldb/include/lldb/Target/UnwindLLDB.h +++ b/lldb/include/lldb/Target/UnwindLLDB.h @@ -49,6 +49,9 @@ class UnwindLLDB : public lldb_private::Unwind { // target mem (target_memory_location) eRegisterInRegister, // register is available in a (possible other) // register (register_number) + eRegisterIsRegisterPlusOffset, // register is available in a (possible + // other) register (register_number) with + // an offset applied eRegisterSavedAtHostMemoryLocation, // register is saved at a word in // lldb's address space eRegisterValueInferred, // register val was computed (and is in @@ -64,6 +67,11 @@ class UnwindLLDB : public lldb_private::Unwind { void *host_memory_location; uint64_t inferred_value; // eRegisterValueInferred - e.g. stack pointer == // cfa + offset + struct { + uint32_t + register_number; // in eRegisterKindLLDB register numbering system + uint64_t offset; + } reg_plus_offset; } location; }; diff --git a/lldb/source/Symbol/DWARFCallFrameInfo.cpp b/lldb/source/Symbol/DWARFCallFrameInfo.cpp index cb8aa8a26c3f1..057a765b6cbdb 100644 --- a/lldb/source/Symbol/DWARFCallFrameInfo.cpp +++ b/lldb/source/Symbol/DWARFCallFrameInfo.cpp @@ -766,8 +766,32 @@ DWARFCallFrameInfo::ParseFDE(dw_offset_t dwarf_offset, break; } - case DW_CFA_val_offset: // 0x14 - case DW_CFA_val_offset_sf: // 0x15 + case DW_CFA_val_offset: { // 0x14 + // takes two unsigned LEB128 operands representing a register number + // and a factored offset. The required action is to change the rule + // for the register indicated by the register number to be a + // val_offset(N) rule where the value of N is factored_offset* + // data_alignment_factor + uint32_t reg_num = (uint32_t)m_cfi_data.GetULEB128(&offset); + int32_t op_offset = + (int32_t)m_cfi_data.GetULEB128(&offset) * data_align; + reg_location.SetIsCFAPlusOffset(op_offset); + row.SetRegisterInfo(reg_num, reg_location); + break; + } + case DW_CFA_val_offset_sf: { // 0x15 + // takes two operands: an unsigned LEB128 value representing a + // register number and a signed LEB128 factored offset. This + // instruction is identical to DW_CFA_val_offset except that the + // second operand is signed and factored. The resulting offset is + // factored_offset* data_alignment_factor. + uint32_t reg_num = (uint32_t)m_cfi_data.GetULEB128(&offset); + int32_t op_offset = + (int32_t)m_cfi_data.GetSLEB128(&offset) * data_align; + reg_location.SetIsCFAPlusOffset(op_offset); + row.SetRegisterInfo(reg_num, reg_location); + break; + } default: break; } diff --git a/lldb/source/Target/RegisterContextUnwind.cpp b/lldb/source/Target/RegisterContextUnwind.cpp index 880300d0637fb..9e9e2d86958f3 100644 --- a/lldb/source/Target/RegisterContextUnwind.cpp +++ b/lldb/source/Target/RegisterContextUnwind.cpp @@ -1118,6 +1118,27 @@ bool RegisterContextUnwind::ReadRegisterValueFromRegisterLocation( success = GetNextFrame()->ReadRegister(other_reg_info, value); } } break; + case UnwindLLDB::ConcreteRegisterLocation::eRegisterIsRegisterPlusOffset: { + auto regnum = regloc.location.reg_plus_offset.register_number; + const RegisterInfo *other_reg_info = + GetRegisterInfoAtIndex(regloc.location.reg_plus_offset.register_number); + + if (!other_reg_info) + return false; + + if (IsFrameZero()) { + success = + m_thread.GetRegisterContext()->ReadRegister(other_reg_info, value); + } else { + success = GetNextFrame()->ReadRegister(other_reg_info, value); + } + if (success) { + UnwindLogMsg("read (%d)'s location", regnum); + value = value.GetAsUInt64(~0ull, &success) + + regloc.location.reg_plus_offset.offset; + UnwindLogMsg("success %s", success ? "yes" : "no"); + } + } break; case UnwindLLDB::ConcreteRegisterLocation::eRegisterValueInferred: success = value.SetUInt(regloc.location.inferred_value, reg_info->byte_size); @@ -1164,6 +1185,7 @@ bool RegisterContextUnwind::WriteRegisterValueToRegisterLocation( success = GetNextFrame()->WriteRegister(other_reg_info, value); } } break; + case UnwindLLDB::ConcreteRegisterLocation::eRegisterIsRegisterPlusOffset: case UnwindLLDB::ConcreteRegisterLocation::eRegisterValueInferred: case UnwindLLDB::ConcreteRegisterLocation::eRegisterNotSaved: break; @@ -1959,6 +1981,7 @@ bool RegisterContextUnwind::ReadFrameAddress( switch (fa.GetValueType()) { case UnwindPlan::Row::FAValue::isRegisterDereferenced: { + UnwindLogMsg("CFA value via dereferencing reg"); RegisterNumber cfa_reg(m_thread, row_register_kind, fa.GetRegisterNumber()); if (ReadGPRValue(cfa_reg, cfa_reg_contents)) { @@ -1991,6 +2014,7 @@ bool RegisterContextUnwind::ReadFrameAddress( break; } case UnwindPlan::Row::FAValue::isRegisterPlusOffset: { + UnwindLogMsg("CFA value via register plus offset"); RegisterNumber cfa_reg(m_thread, row_register_kind, fa.GetRegisterNumber()); if (ReadGPRValue(cfa_reg, cfa_reg_contents)) { @@ -2012,10 +2036,13 @@ bool RegisterContextUnwind::ReadFrameAddress( address, cfa_reg.GetName(), cfa_reg.GetAsKind(eRegisterKindLLDB), cfa_reg_contents, fa.GetOffset()); return true; - } + } else + UnwindLogMsg("unable to read CFA register %s (%d)", cfa_reg.GetName(), + cfa_reg.GetAsKind(eRegisterKindLLDB)); break; } case UnwindPlan::Row::FAValue::isDWARFExpression: { + UnwindLogMsg("CFA value via DWARF expression"); ExecutionContext exe_ctx(m_thread.shared_from_this()); Process *process = exe_ctx.GetProcessPtr(); DataExtractor dwarfdata(fa.GetDWARFExpressionBytes(), @@ -2042,6 +2069,7 @@ bool RegisterContextUnwind::ReadFrameAddress( break; } case UnwindPlan::Row::FAValue::isRaSearch: { + UnwindLogMsg("CFA value via heuristic search"); Process &process = *m_thread.GetProcess(); lldb::addr_t return_address_hint = GetReturnAddressHint(fa.GetOffset()); if (return_address_hint == LLDB_INVALID_ADDRESS) diff --git a/lldb/test/Shell/Unwind/Inputs/eh-frame-dwarf-unwind-val-offset.s b/lldb/test/Shell/Unwind/Inputs/eh-frame-dwarf-unwind-val-offset.s new file mode 100644 index 0000000000000..273921cc9c549 --- /dev/null +++ b/lldb/test/Shell/Unwind/Inputs/eh-frame-dwarf-unwind-val-offset.s @@ -0,0 +1,50 @@ + .text + .globl bar +bar: + .cfi_startproc + leal (%edi, %edi), %eax + ret + .cfi_endproc + + .globl foo + # This function uses a non-standard calling convention (return address + # needs to be adjusted) to force lldb to use eh_frame/debug_frame + # instead of reading the code directly. +foo: + .cfi_startproc + .cfi_escape 0x16, 0x10, 0x06, 0x38, 0x1c, 0x06, 0x08, 0x47, 0x1c + # Clobber r12 and record that it's reconstructable from CFA + .cfi_val_offset %r12, 32 + movq $0x456, %r12 + call bar + addl $1, %eax + # Reconstruct %r12 + movq %rsp, %r12 + addq %r12, 8 + popq %rdi + subq $0x47, %rdi + jmp *%rdi # Return + .cfi_endproc + + .globl asm_main +asm_main: + .cfi_startproc + pushq %rbp + .cfi_def_cfa_offset 16 + .cfi_offset %rbp, -16 + movq %rsp, %rbp + movq %rsp, %r12 + addq $32, %r12 + .cfi_def_cfa_register %rbp + movl $47, %edi + + # Non-standard calling convention. The real return address must be + # decremented by 0x47. + leaq 0x47+1f(%rip), %rax + pushq %rax + jmp foo # call +1: + popq %rbp + .cfi_def_cfa %rsp, 8 + ret + .cfi_endproc diff --git a/lldb/test/Shell/Unwind/eh-frame-dwarf-unwind-val-offset.test b/lldb/test/Shell/Unwind/eh-frame-dwarf-unwind-val-offset.test new file mode 100644 index 0000000000000..a6a17d081970d --- /dev/null +++ b/lldb/test/Shell/Unwind/eh-frame-dwarf-unwind-val-offset.test @@ -0,0 +1,58 @@ +# Test handing of the dwarf val_offset() rule which can be used to reconstruct +# the value of a register that is neither in a live register or saved on the +# stack but is computable with CFA + offset. + +# UNSUPPORTED: system-windows, ld_new-bug +# REQUIRES: target-x86_64, native + +# RUN: %clang_host %p/Inputs/call-asm.c %p/Inputs/eh-frame-dwarf-unwind-val-offset.s -o %t +# RUN: %lldb %t -s %s -o exit | FileCheck %s + +breakpoint set -n asm_main +# CHECK: Breakpoint 1: where = {{.*}}`asm_main + +breakpoint set -n bar +# CHECK: Breakpoint 2: where = {{.*}}`bar + +process launch +# CHECK: stop reason = breakpoint 1.1 + +stepi +stepi +stepi +register read -G x r12 +# CHECK: r12 = 0x[[#%.16x,R12:]]{{$}} + +continue +# CHECK: stop reason = breakpoint 2.1 + +thread backtrace +# CHECK: frame #0: {{.*}}`bar +# CHECK: frame #1: {{.*}}`foo + 12 +# CHECK: frame #2: {{.*}}`asm_main + 29 + +target modules show-unwind -n bar +# CHECK: eh_frame UnwindPlan: +# CHECK: row[0]: 0: CFA=rsp +8 => rip=[CFA-8] + +target modules show-unwind -n foo +# CHECK: eh_frame UnwindPlan: +# CHECK: row[0]: 0: CFA=rsp +8 => r12=CFA+32 rip=DW_OP_lit8, DW_OP_minus, DW_OP_deref, DW_OP_const1u 0x47, DW_OP_minus + +target modules show-unwind -n asm_main +# CHECK: eh_frame UnwindPlan: +# CHECK: row[0]: 0: CFA=rsp +8 => rip=[CFA-8] +# CHECK: row[1]: 1: CFA=rsp+16 => rbp=[CFA-16] rip=[CFA-8] +# CHECK: row[2]: 11: CFA=rbp+16 => rbp=[CFA-16] rip=[CFA-8] +# CHECK: row[3]: 30: CFA=rsp +8 => rbp=[CFA-16] rip=[CFA-8] + +register read -G x r12 +# CHECK: r12 = 0x0000000000000456 + +frame select 1 +register read -G x r12 +# CHECK: r12 = 0x0000000000000456 + +frame select 2 +register read -G x r12 +# CHECK: r12 = 0x[[#R12 + 32]] diff --git a/lldb/unittests/Symbol/TestDWARFCallFrameInfo.cpp b/lldb/unittests/Symbol/TestDWARFCallFrameInfo.cpp index c1dcab02227da..e113b8ca99341 100644 --- a/lldb/unittests/Symbol/TestDWARFCallFrameInfo.cpp +++ b/lldb/unittests/Symbol/TestDWARFCallFrameInfo.cpp @@ -39,6 +39,7 @@ class DWARFCallFrameInfoTest : public testing::Test { protected: void TestBasic(DWARFCallFrameInfo::Type type, llvm::StringRef symbol); + void TestValOffset(DWARFCallFrameInfo::Type type, llvm::StringRef symbol); }; namespace lldb_private { @@ -256,3 +257,126 @@ TEST_F(DWARFCallFrameInfoTest, Basic_dwarf4) { TEST_F(DWARFCallFrameInfoTest, Basic_eh) { TestBasic(DWARFCallFrameInfo::EH, "eh_frame"); } + +static UnwindPlan::Row GetValOffsetExpectedRow0() { + UnwindPlan::Row row; + row.SetOffset(0); + row.GetCFAValue().SetIsRegisterPlusOffset(dwarf_rsp_x86_64, 16); + row.SetRegisterLocationToAtCFAPlusOffset(dwarf_rip_x86_64, -8, false); + row.SetRegisterLocationToIsCFAPlusOffset(dwarf_rbp_x86_64, -16, false); + return row; +} + +void DWARFCallFrameInfoTest::TestValOffset(DWARFCallFrameInfo::Type type, + llvm::StringRef symbol) { + // This test is artificial as X86 does not use DW_CFA_val_offset but this + // test verifies that we can successfully interpret them if they do occur. + // Note the distinction between RBP and RIP in this part of the DWARF dump: + // 0x0: CFA=RSP+16: RBP=CFA-16, RIP=[CFA-8] + // Whereas RIP is stored in the memory CFA-8 points at, RBP is reconstructed + // from the CFA without any memory access. + auto ExpectedFile = TestFile::fromYaml(R"( +--- !ELF +FileHeader: + Class: ELFCLASS64 + Data: ELFDATA2LSB + Type: ET_REL + Machine: EM_X86_64 + SectionHeaderStringTable: .strtab +Sections: + - Name: .text + Type: SHT_PROGBITS + Flags: [ SHF_ALLOC, SHF_EXECINSTR ] + AddressAlign: 0x4 + Content: 0F1F00 + - Name: .debug_frame + Type: SHT_PROGBITS + AddressAlign: 0x8 +#00000000 00000014 ffffffff CIE +# Format: DWARF32 +# Version: 4 +# Augmentation: "" +# Address size: 8 +# Segment desc size: 0 +# Code alignment factor: 1 +# Data alignment factor: -8 +# Return address column: 16 +# +# DW_CFA_def_cfa: RSP +8 +# DW_CFA_offset: RIP -8 +# DW_CFA_nop: +# DW_CFA_nop: +# DW_CFA_nop: +# DW_CFA_nop: +# +# CFA=RSP+8: RIP=[CFA-8] +# +#00000018 0000001c 00000000 FDE cie=00000000 pc=00000000...00000003 +# Format: DWARF32 +# DW_CFA_def_cfa_offset: +16 +# DW_CFA_val_offset: RBP -16 +# DW_CFA_nop: +# DW_CFA_nop: +# DW_CFA_nop: +# +# 0x0: CFA=RSP+16: RBP=CFA-16, RIP=[CFA-8] + Content: 14000000FFFFFFFF040008000178100C07089001000000001C00000000000000000000000000000003000000000000000E10140602000000 + - Name: .rela.debug_frame + Type: SHT_RELA + Flags: [ SHF_INFO_LINK ] + Link: .symtab + AddressAlign: 0x8 + Info: .debug_frame + Relocations: + - Offset: 0x1C + Symbol: .debug_frame + Type: R_X86_64_32 + - Offset: 0x20 + Symbol: .text + Type: R_X86_64_64 + - Type: SectionHeaderTable + Sections: + - Name: .strtab + - Name: .text + - Name: .debug_frame + - Name: .rela.debug_frame + - Name: .symtab +Symbols: + - Name: .text + Type: STT_SECTION + Section: .text + - Name: debug_frame3 + Section: .text + - Name: .debug_frame + Type: STT_SECTION + Section: .debug_frame +... +)"); + ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded()); + + auto module_sp = std::make_shared<Module>(ExpectedFile->moduleSpec()); + SectionList *list = module_sp->GetSectionList(); + ASSERT_NE(nullptr, list); + + auto section_sp = list->FindSectionByType(type == DWARFCallFrameInfo::EH + ? eSectionTypeEHFrame + : eSectionTypeDWARFDebugFrame, + false); + ASSERT_NE(nullptr, section_sp); + + DWARFCallFrameInfo cfi(*module_sp->GetObjectFile(), section_sp, type); + + const Symbol *sym = module_sp->FindFirstSymbolWithNameAndType( + ConstString(symbol), eSymbolTypeAny); + ASSERT_NE(nullptr, sym); + + std::unique_ptr<UnwindPlan> plan_up = cfi.GetUnwindPlan(sym->GetAddress()); + ASSERT_TRUE(plan_up); + ASSERT_EQ(1, plan_up->GetRowCount()); + EXPECT_THAT(plan_up->GetRowAtIndex(0), + testing::Pointee(GetValOffsetExpectedRow0())); +} + +TEST_F(DWARFCallFrameInfoTest, ValOffset_dwarf3) { + TestValOffset(DWARFCallFrameInfo::DWARF, "debug_frame3"); +} 

[dyung]

At this time we are only accepting release branch patches for major bugs or regressions, and this does not seem to be either to me (although admittedly I'm not very familiar with this area). @dsandersllvm can you explain why this meets the criteria for inclusion in the release branch if you still feel it does?