Skip to content

Commit 32ebf63

Browse files
authored
[LLDB] Fix debuginfo ELF files overwriting Unified Section List (llvm#166635)
Recently I've been deep diving ELF cores in LLDB, aspiring to move LLDB closer to GDB in capability. One issue I encountered was a system lib losing it's unwind plan when loading the debuginfo. The reason for this was the debuginfo has the eh_frame section stripped and the main executable did not. The root cause of this was this line in [ObjectFileElf](https://github.com/llvm/llvm-project/blob/163933e9e7099f352ff8df1973f9a9c3d7def6c5/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp#L1972) ``` // For eTypeDebugInfo files, the Symbol Vendor will take care of updating the // unified section list. if (GetType() != eTypeDebugInfo) unified_section_list = *m_sections_up; ``` This would always be executed because CalculateType can never return an eTypeDebugInfo ``` ObjectFile::Type ObjectFileELF::CalculateType() { switch (m_header.e_type) { case llvm::ELF::ET_NONE: // 0 - No file type return eTypeUnknown; case llvm::ELF::ET_REL: // 1 - Relocatable file return eTypeObjectFile; case llvm::ELF::ET_EXEC: // 2 - Executable file return eTypeExecutable; case llvm::ELF::ET_DYN: // 3 - Shared object file return eTypeSharedLibrary; case ET_CORE: // 4 - Core file return eTypeCoreFile; default: break; } return eTypeUnknown; } ``` This makes sense as there isn't a explicit sh_type to denote that this file is a debuginfo. After some discussion with @clayborg and @GeorgeHuyubo we settled on joining the exciting unified section list with whatever new sections were being added. Adding each new unique section, or taking the section with the maximum file size. We picked this strategy to pick the section with the most information. In most scenarios, LHS should be SHT_NOBITS and RHS would be SHT_PROGBITS. Here is a diagram documenting the existing vs proposed new way. <img width="1666" height="1093" alt="image" src="https://github.com/user-attachments/assets/73ba9620-c737-439e-9934-ac350d88a3b5" />
1 parent 2fd3bf3 commit 32ebf63

File tree

10 files changed

+544
-4
lines changed

10 files changed

+544
-4
lines changed

lldb/include/lldb/Core/Section.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ class SectionList {
4646
/// Create an empty list.
4747
SectionList() = default;
4848

49+
SectionList(const SectionList &lhs);
50+
4951
SectionList &operator=(const SectionList &rhs);
5052

5153
size_t AddSection(const lldb::SectionSP &section_sp);
@@ -96,6 +98,17 @@ class SectionList {
9698
/// information.
9799
uint64_t GetDebugInfoSize() const;
98100

101+
// Callback to decide which of two matching sections should be used in the
102+
// merged output.
103+
using MergeCallback =
104+
std::function<lldb::SectionSP(lldb::SectionSP, lldb::SectionSP)>;
105+
106+
// Function that merges two different sections into a new output list. All
107+
// unique sections will be checked for conflict and resolved using the
108+
// supplied merging callback.
109+
static SectionList Merge(SectionList &lhs, SectionList &rhs,
110+
MergeCallback filter);
111+
99112
protected:
100113
collection m_sections;
101114
};

lldb/source/Core/Section.cpp

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,8 @@ bool Section::IsGOTSection() const {
477477

478478
#pragma mark SectionList
479479

480+
SectionList::SectionList(const SectionList &rhs) : m_sections(rhs.m_sections) {}
481+
480482
SectionList &SectionList::operator=(const SectionList &rhs) {
481483
if (this != &rhs)
482484
m_sections = rhs.m_sections;
@@ -687,6 +689,33 @@ uint64_t SectionList::GetDebugInfoSize() const {
687689
return debug_info_size;
688690
}
689691

692+
SectionList SectionList::Merge(SectionList &lhs, SectionList &rhs,
693+
MergeCallback filter) {
694+
SectionList output_list;
695+
696+
// Iterate through all the sections in lhs and see if we have matches in
697+
// the rhs list.
698+
for (const auto &lhs_section : lhs) {
699+
auto rhs_section = rhs.FindSectionByName(lhs_section->GetName());
700+
if (rhs_section)
701+
output_list.AddSection(filter(lhs_section, rhs_section));
702+
else
703+
output_list.AddSection(lhs_section);
704+
}
705+
706+
// Now that we've visited all possible duplicates, we can iterate over
707+
// the rhs and take any values not in lhs.
708+
for (const auto &rhs_section : rhs) {
709+
auto lhs_section = lhs.FindSectionByName(rhs_section->GetName());
710+
// Because we already visited everything overlapping between rhs
711+
// and lhs, any section not in lhs is unique and can be output.
712+
if (!lhs_section)
713+
output_list.AddSection(rhs_section);
714+
}
715+
716+
return output_list;
717+
}
718+
690719
namespace llvm {
691720
namespace json {
692721

lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,29 @@ class ELFRelocation {
130130

131131
RelocUnion reloc;
132132
};
133+
134+
lldb::SectionSP MergeSections(lldb::SectionSP lhs, lldb::SectionSP rhs) {
135+
assert(lhs && rhs);
136+
137+
lldb::ModuleSP lhs_module_parent = lhs->GetModule();
138+
lldb::ModuleSP rhs_module_parent = rhs->GetModule();
139+
assert(lhs_module_parent && rhs_module_parent);
140+
141+
// Do a sanity check, these should be the same.
142+
if (lhs->GetFileAddress() != rhs->GetFileAddress())
143+
lhs_module_parent->ReportWarning(
144+
"Mismatch addresses for section {0} when "
145+
"merging with {1}, expected: {2:x}, "
146+
"actual: {3:x}",
147+
lhs->GetTypeAsCString(),
148+
rhs_module_parent->GetFileSpec().GetPathAsConstString().GetCString(),
149+
lhs->GetByteSize(), rhs->GetByteSize());
150+
151+
// We want to take the greater of two sections. If LHS and RHS are both
152+
// SHT_NOBITS, we should default to LHS. If RHS has a bigger section,
153+
// indicating it has data that wasn't stripped, we should take that instead.
154+
return rhs->GetFileSize() > lhs->GetFileSize() ? rhs : lhs;
155+
}
133156
} // end anonymous namespace
134157

135158
ELFRelocation::ELFRelocation(unsigned type) {
@@ -1967,10 +1990,10 @@ void ObjectFileELF::CreateSections(SectionList &unified_section_list) {
19671990
provider.AddSection(std::move(*InfoOr), std::move(section_sp));
19681991
}
19691992

1970-
// For eTypeDebugInfo files, the Symbol Vendor will take care of updating the
1971-
// unified section list.
1972-
if (GetType() != eTypeDebugInfo)
1973-
unified_section_list = *m_sections_up;
1993+
// Merge the two adding any new sections, and overwriting any existing
1994+
// sections that are SHT_NOBITS
1995+
unified_section_list =
1996+
SectionList::Merge(unified_section_list, *m_sections_up, MergeSections);
19741997

19751998
// If there's a .gnu_debugdata section, we'll try to read the .symtab that's
19761999
// embedded in there and replace the one in the original object file (if any).
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
CXX_SOURCES := main.cpp
2+
3+
SPLIT_DEBUG_SYMBOLS := YES
4+
5+
include Makefile.rules

0 commit comments

Comments
 (0)