labath created this revision. labath added reviewers: clayborg, jingham. Herald added a subscriber: mgorny.
lld r367537 changed the way the linker organizes sections and segments. This exposed an lldb bug and caused some tests to fail. In all of the failing tests the root cause was the same -- when we were trying to resolve the last address in the line_table section, we failed because it pointed past the end of the section. This is similar to the problem encountered in D32022 <https://reviews.llvm.org/D32022>, where we had a problem with unwinding of functions near the end of the section/module. In that patch, we added a flag which allowed these end addresses to resolve to the section, but we only did so for load address computations (SectionLoadList). line table uses file addresses, and so we are not able to take advantage of that here. This patch adds an equivalent flag to the SectionList class, and changes the line table resolving code to use it for end-of-sequence entries (as those are the only ones that can validly point outside of a section). I also revert the linker flags which were added to the failing tests to restore previous behavior, and add a unit test to exercise the new API more directly. https://reviews.llvm.org/D65647 Files: include/lldb/Core/Module.h include/lldb/Core/Section.h lit/SymbolFile/DWARF/debug-line-basic.s lit/SymbolFile/DWARF/dir-separator-no-comp-dir-relative-name.s lit/SymbolFile/DWARF/dir-separator-no-comp-dir.s lit/SymbolFile/DWARF/dir-separator-posix.s lit/SymbolFile/DWARF/dir-separator-windows.s source/Core/Module.cpp source/Core/Section.cpp source/Symbol/LineTable.cpp unittests/Core/CMakeLists.txt unittests/Core/Inputs/resolve-file-address.yaml unittests/Core/ModuleTest.cpp
Index: unittests/Core/ModuleTest.cpp =================================================================== --- /dev/null +++ unittests/Core/ModuleTest.cpp @@ -0,0 +1,69 @@ +//===-- ModuleTest.cpp ------------------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "lldb/Core/Module.h" +#include "Plugins/ObjectFile/ELF/ObjectFileELF.h" +#include "Plugins/SymbolFile/Symtab/SymbolFileSymtab.h" +#include "TestingSupport/TestUtilities.h" +#include "lldb/Core/Section.h" +#include "lldb/Host/HostInfo.h" +#include "llvm/Support/FileUtilities.h" +#include "llvm/Testing/Support/Error.h" +#include "gtest/gtest.h" + +using namespace lldb_private; +using namespace lldb; + +namespace { +class ModuleTest : public testing::Test { +public: + static void SetUpTestCase() { + FileSystem::Initialize(); + HostInfo::Initialize(); + ObjectFileELF::Initialize(); + SymbolFileSymtab::Initialize(); + } + + static void TearDownTestCase() { + SymbolFileSymtab::Terminate(); + ObjectFileELF::Terminate(); + HostInfo::Terminate(); + FileSystem::Terminate(); + } +}; +} // namespace + +llvm::Optional<Address> resolveFileAddress(Module &M, addr_t FileAddr, + bool AllowSectionEnd) { + Address A; + if (M.ResolveFileAddress(FileAddr, A, AllowSectionEnd)) + return A; + return llvm::None; +} + +TEST_F(ModuleTest, ResolveFileAddress) { + llvm::SmallString<128> Obj; + ASSERT_EQ(llvm::sys::fs::createTemporaryFile("resolve-file-address-%%%%%%", + "obj", Obj), + std::error_code()); + llvm::FileRemover Deleter(Obj); + ASSERT_THAT_ERROR(ReadYAMLObjectFile("resolve-file-address.yaml", Obj), + llvm::Succeeded()); + + ModuleSpec Spec{FileSpec(Obj)}; + Spec.GetSymbolFileSpec().SetFile(Obj, FileSpec::Style::native); + auto M = std::make_shared<Module>(Spec); + + ASSERT_TRUE(M->GetSectionList()); + SectionSP S = M->GetSectionList()->FindSectionByName(ConstString(".text")); + ASSERT_TRUE(S); + + EXPECT_EQ(resolveFileAddress(*M, 0x1000, false), Address(S, 0)); + EXPECT_EQ(resolveFileAddress(*M, 0x1100, false), llvm::None); + EXPECT_EQ(resolveFileAddress(*M, 0x1100, true), Address(S, 0x100)); +} Index: unittests/Core/Inputs/resolve-file-address.yaml =================================================================== --- /dev/null +++ unittests/Core/Inputs/resolve-file-address.yaml @@ -0,0 +1,12 @@ +--- !ELF +FileHeader: + Class: ELFCLASS64 + Data: ELFDATA2LSB + Type: ET_EXEC + Machine: EM_X86_64 +Sections: + - Name: .text + Type: SHT_PROGBITS + Flags: [ SHF_ALLOC, SHF_EXECINSTR ] + Address: 0x1000 + Size: 0x100 Index: unittests/Core/CMakeLists.txt =================================================================== --- unittests/Core/CMakeLists.txt +++ unittests/Core/CMakeLists.txt @@ -1,5 +1,6 @@ add_lldb_unittest(LLDBCoreTests MangledTest.cpp + ModuleTest.cpp RichManglingContextTest.cpp StreamCallbackTest.cpp UniqueCStringMapTest.cpp @@ -18,5 +19,6 @@ set(test_inputs mangled-function-names.yaml + resolve-file-address.yaml ) add_unittest_inputs(LLDBCoreTests "${test_inputs}") Index: source/Symbol/LineTable.cpp =================================================================== --- source/Symbol/LineTable.cpp +++ source/Symbol/LineTable.cpp @@ -244,9 +244,9 @@ if (idx < m_entries.size()) { const Entry &entry = m_entries[idx]; ModuleSP module_sp(m_comp_unit->GetModule()); - if (module_sp && - module_sp->ResolveFileAddress(entry.file_addr, - line_entry.range.GetBaseAddress())) { + if (module_sp && module_sp->ResolveFileAddress( + entry.file_addr, line_entry.range.GetBaseAddress(), + /*allow_section_end = */ entry.is_terminal_entry)) { if (!entry.is_terminal_entry && idx + 1 < m_entries.size()) line_entry.range.SetByteSize(m_entries[idx + 1].file_addr - entry.file_addr); Index: source/Core/Section.cpp =================================================================== --- source/Core/Section.cpp +++ source/Core/Section.cpp @@ -267,12 +267,13 @@ return true; } -bool Section::ContainsFileAddress(addr_t vm_addr) const { +bool Section::ContainsFileAddress(addr_t vm_addr, + bool allow_section_end) const { const addr_t file_addr = GetFileAddress(); if (file_addr != LLDB_INVALID_ADDRESS && !IsThreadSpecific()) { if (file_addr <= vm_addr) { const addr_t offset = (vm_addr - file_addr) * m_target_byte_size; - return offset < GetByteSize(); + return offset < GetByteSize() + (allow_section_end ? 1 : 0); } } return false; @@ -586,6 +587,20 @@ return sect_sp; } +llvm::Optional<Address> +SectionList::ResolveFileAddress(addr_t file_addr, + bool allow_section_end) const { + for (const SectionSP §ion_sp : m_sections) { + if (section_sp->ContainsFileAddress(file_addr, allow_section_end)) { + Address addr; + section_sp->ResolveContainedAddress( + file_addr - section_sp->GetFileAddress(), addr, allow_section_end); + return addr; + } + } + return llvm::None; +} + bool SectionList::ContainsSection(user_id_t sect_id) const { return FindSectionByID(sect_id).get() != nullptr; } Index: source/Core/Module.cpp =================================================================== --- source/Core/Module.cpp +++ source/Core/Module.cpp @@ -423,15 +423,21 @@ return cu_sp; } -bool Module::ResolveFileAddress(lldb::addr_t vm_addr, Address &so_addr) { +bool Module::ResolveFileAddress(lldb::addr_t vm_addr, Address &so_addr, + bool allow_section_end) { std::lock_guard<std::recursive_mutex> guard(m_mutex); static Timer::Category func_cat(LLVM_PRETTY_FUNCTION); Timer scoped_timer(func_cat, "Module::ResolveFileAddress (vm_addr = 0x%" PRIx64 ")", vm_addr); SectionList *section_list = GetSectionList(); - if (section_list) - return so_addr.ResolveAddressUsingFileSections(vm_addr, section_list); + if (!section_list) + return false; + if (llvm::Optional<Address> addr = + section_list->ResolveFileAddress(vm_addr, allow_section_end)) { + so_addr = *addr; + return true; + } return false; } Index: lit/SymbolFile/DWARF/dir-separator-windows.s =================================================================== --- lit/SymbolFile/DWARF/dir-separator-windows.s +++ lit/SymbolFile/DWARF/dir-separator-windows.s @@ -4,7 +4,7 @@ # REQUIRES: lld, x86 # RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj > %t.o -# RUN: ld.lld -z separate-code %t.o -o %t +# RUN: ld.lld %t.o -o %t # RUN: %lldb %t -s %S/Inputs/dir-separator-windows.lldbinit -o exit | FileCheck %s # CHECK-LABEL: image dump line-table a.c Index: lit/SymbolFile/DWARF/dir-separator-posix.s =================================================================== --- lit/SymbolFile/DWARF/dir-separator-posix.s +++ lit/SymbolFile/DWARF/dir-separator-posix.s @@ -4,7 +4,7 @@ # REQUIRES: lld, x86 # RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj > %t.o -# RUN: ld.lld -z separate-code %t.o -o %t +# RUN: ld.lld %t.o -o %t # RUN: %lldb %t -s %S/Inputs/dir-separator-posix.lldbinit -o exit | FileCheck %s # CHECK-LABEL: image dump line-table a.c Index: lit/SymbolFile/DWARF/dir-separator-no-comp-dir.s =================================================================== --- lit/SymbolFile/DWARF/dir-separator-no-comp-dir.s +++ lit/SymbolFile/DWARF/dir-separator-no-comp-dir.s @@ -4,7 +4,7 @@ # REQUIRES: lld, x86 # RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj > %t.o -# RUN: ld.lld -z separate-code %t.o -o %t +# RUN: ld.lld %t.o -o %t # RUN: %lldb %t -s %S/Inputs/dir-separator-windows.lldbinit -o exit | FileCheck %s # CHECK-LABEL: image dump line-table a.c Index: lit/SymbolFile/DWARF/dir-separator-no-comp-dir-relative-name.s =================================================================== --- lit/SymbolFile/DWARF/dir-separator-no-comp-dir-relative-name.s +++ lit/SymbolFile/DWARF/dir-separator-no-comp-dir-relative-name.s @@ -5,7 +5,7 @@ # REQUIRES: lld, x86 # RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj > %t.o -# RUN: ld.lld -z separate-code %t.o -o %t +# RUN: ld.lld %t.o -o %t # RUN: %lldb %t -s %S/Inputs/dir-separator-no-comp-dir-relative-name.lldbinit -o exit | FileCheck %s # CHECK-LABEL: image dump line-table a.c Index: lit/SymbolFile/DWARF/debug-line-basic.s =================================================================== --- lit/SymbolFile/DWARF/debug-line-basic.s +++ lit/SymbolFile/DWARF/debug-line-basic.s @@ -1,7 +1,7 @@ # REQUIRES: lld, x86 # RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj > %t.o -# RUN: ld.lld -z separate-code %t.o -o %t +# RUN: ld.lld %t.o -o %t # RUN: %lldb %t -o "image dump line-table -v a.c" -o exit | FileCheck %s Index: include/lldb/Core/Section.h =================================================================== --- include/lldb/Core/Section.h +++ include/lldb/Core/Section.h @@ -71,6 +71,14 @@ FindSectionContainingFileAddress(lldb::addr_t addr, uint32_t depth = UINT32_MAX) const; + /// Look up the given file address in the contained sections. Return a + /// section-relative Address object if a section is found. if + /// allow_section_end is true, an address pointing past the end of a section + /// is permitted and resolved relative to the section which ends at the given + /// address. + llvm::Optional<Address> ResolveFileAddress(lldb::addr_t file_addr, + bool allow_section_end) const; + // Get the number of sections in this list only size_t GetSize() const { return m_sections.size(); } @@ -122,7 +130,11 @@ static int Compare(const Section &a, const Section &b); - bool ContainsFileAddress(lldb::addr_t vm_addr) const; + /// Return true iff this section contains the given file address. If + /// allow_section_end is true, the function returns true also for address + /// pointing past the end of the section. + bool ContainsFileAddress(lldb::addr_t vm_addr, + bool allow_section_end = false) const; SectionList &GetChildren() { return m_children; } Index: include/lldb/Core/Module.h =================================================================== --- include/lldb/Core/Module.h +++ include/lldb/Core/Module.h @@ -680,7 +680,8 @@ /// data the symbol vendor provides. void ParseAllDebugSymbols(); - bool ResolveFileAddress(lldb::addr_t vm_addr, Address &so_addr); + bool ResolveFileAddress(lldb::addr_t vm_addr, Address &so_addr, + bool allow_section_end = false); /// Resolve the symbol context for the given address. ///
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
