Skip to content

Commit 87e9249

Browse files
JlalondJlalond
committed
Implement feedback from Greg and test the variety of merge cases with yamlized elf files
1 parent bf0c366 commit 87e9249

File tree

6 files changed

+325
-21
lines changed

6 files changed

+325
-21
lines changed

‎lldb/source/Core/Section.cpp‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -691,10 +691,9 @@ SectionList::Merge(SectionList &lhs, SectionList &rhs, MergeCallback filter) {
691691
// the rhs list.
692692
for (const auto &lhs_section : lhs) {
693693
auto rhs_section = rhs.FindSectionByName(lhs_section->GetName());
694-
if (rhs_section) {
695-
assert(lhs_section->GetName() == rhs_section->GetName());
694+
if (rhs_section)
696695
output_sp->AddSection(filter(lhs_section, rhs_section));
697-
} else
696+
else
698697
output_sp->AddSection(lhs_section);
699698
}
700699

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

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -134,27 +134,11 @@ class ELFRelocation {
134134
lldb::SectionSP MergeSections(lldb::SectionSP lhs, lldb::SectionSP rhs) {
135135
assert(lhs && rhs);
136136

137-
// We only take the RHS is the LHS is SHT_NOBITS, which would be
138-
// represented as a size of 0. Where we can take the rhs.
139-
if (lhs->GetByteSize() == 0)
140-
return rhs;
141-
142137
lldb::ModuleSP lhs_module_parent = lhs->GetModule();
143138
lldb::ModuleSP rhs_module_parent = rhs->GetModule();
144139
assert(lhs_module_parent && rhs_module_parent);
145140

146-
// Now that we're taking the lhs, we should do some sanity checks to warn the
147-
// user if this debuginfo/dwp looks invalid. However if RHS is SHT_NO_BITS
148-
// we want to ignore the size comparison.
149-
if (rhs->GetByteSize() > 0 && lhs->GetByteSize() != rhs->GetByteSize())
150-
lhs_module_parent->ReportWarning(
151-
"Mismatched size for section {0} when merging with {1}, expected: "
152-
"{2:x}, "
153-
"actual: {3:x}",
154-
lhs->GetTypeAsCString(),
155-
rhs_module_parent->GetFileSpec().GetPathAsConstString().GetCString(),
156-
lhs->GetByteSize(), rhs->GetByteSize());
157-
141+
// Do a sanity check, these should be the same.
158142
if (lhs->GetFileAddress() != rhs->GetFileAddress())
159143
lhs_module_parent->ReportWarning(
160144
"Mismatch addresses for section {0} when "
@@ -164,7 +148,10 @@ lldb::SectionSP MergeSections(lldb::SectionSP lhs, lldb::SectionSP rhs) {
164148
rhs_module_parent->GetFileSpec().GetPathAsConstString().GetCString(),
165149
lhs->GetByteSize(), rhs->GetByteSize());
166150

167-
return lhs;
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 lhs->GetFileSize() > rhs->GetFileSize() ? lhs : rhs;
168155
}
169156
} // end anonymous namespace
170157

‎lldb/test/API/python_api/unified_section_list/TestModuleUnifiedSectionList.py‎

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,3 +49,184 @@ def test_unified_section_list(self):
4949
debug_info_section = main_exe_module.FindSection(".debug_info")
5050
self.assertTrue(debug_info_section.IsValid())
5151
self.assertGreater(debug_info_section.file_size, 0)
52+
53+
def test_unified_section_list_overwrite_larger_section(self):
54+
"""
55+
Test the merging of an ELF file with another ELF File where all the new sections are bigger, validating we
56+
overwrite .comment from SHT_NOBITS to the new SHT_PROGBITS section and the smaller .text with the larger
57+
.text
58+
"""
59+
exe = self.getBuildArtifact("a.out")
60+
self.yaml2obj("main.yaml", exe)
61+
62+
target = self.dbg.CreateTarget(exe)
63+
self.assertTrue(target, VALID_TARGET)
64+
main_exe_module = target.GetModuleAtIndex(0)
65+
66+
# First we verify out .text section is the expected BEC0FFEE
67+
text_before_merge = main_exe_module.FindSection(".text")
68+
self.assertTrue(text_before_merge.IsValid())
69+
error = lldb.SBError()
70+
section_content = text_before_merge.data.ReadRawData(
71+
error, 0, text_before_merge.data.size
72+
)
73+
self.assertTrue(error.Success())
74+
self.assertEqual(section_content, bytes.fromhex("BEC0FFEE"))
75+
76+
# .comment in main.yaml should be SHT_NOBITS, and size 0
77+
comment_before_merge = main_exe_module.FindSection(".comment")
78+
self.assertTrue(comment_before_merge.IsValid())
79+
self.assertEqual(comment_before_merge.data.size, 0)
80+
81+
# yamlize the main.largertext.yaml and force symbol loading
82+
debug_info = self.getBuildArtifact("a.out.debug")
83+
self.yaml2obj("main.largertext.yaml", debug_info)
84+
85+
ci = self.dbg.GetCommandInterpreter()
86+
res = lldb.SBCommandReturnObject()
87+
ci.HandleCommand(f"target symbols add {debug_info}", res)
88+
self.assertTrue(res.Succeeded())
89+
90+
# verify we took the larger .text section
91+
main_exe_module_after_merge = target.GetModuleAtIndex(0)
92+
text_after_merge = main_exe_module_after_merge.FindSection(".text")
93+
self.assertTrue(text_after_merge.IsValid())
94+
self.assertGreater(text_after_merge.data.size, text_before_merge.data.size)
95+
section_content_after_merge = text_after_merge.data.ReadRawData(
96+
error, 0, text_after_merge.data.size
97+
)
98+
self.assertTrue(error.Success())
99+
self.assertEqual(section_content_after_merge, bytes.fromhex("BEC0FFEEEEFF0CEB"))
100+
101+
# in main.largertext.yaml comment is not SHT_NOBITS, and so we should see
102+
# the size > 0 and equal to BAADF00D
103+
comment_after_merge = main_exe_module_after_merge.FindSection(".comment")
104+
self.assertTrue(comment_after_merge.IsValid())
105+
comment_content_after_merge = comment_after_merge.data.ReadRawData(
106+
error, 0, comment_after_merge.data.size
107+
)
108+
109+
self.assertTrue(error.Success())
110+
self.assertEqual(comment_content_after_merge, bytes.fromhex("BAADF00D"))
111+
112+
def test_unified_section_list_overwrite_smaller_section(self):
113+
"""
114+
Test the merging of an ELF file with another ELF File where all the existing sections are bigger, validating we don't
115+
overwrite with the SHT_NOBITS for .comment or the smaller .text section.
116+
"""
117+
exe = self.getBuildArtifact("a.out")
118+
self.yaml2obj("main.largertext.yaml", exe)
119+
120+
target = self.dbg.CreateTarget(exe)
121+
self.assertTrue(target, VALID_TARGET)
122+
main_exe_module = target.GetModuleAtIndex(0)
123+
124+
# Same as above test but inverse, verify our larger .text section
125+
# is the expected BEC0FFEE palindrome
126+
text_before_merge = main_exe_module.FindSection(".text")
127+
self.assertTrue(text_before_merge.IsValid())
128+
error = lldb.SBError()
129+
section_content = text_before_merge.data.ReadRawData(
130+
error, 0, text_before_merge.data.size
131+
)
132+
self.assertTrue(error.Success())
133+
self.assertEqual(section_content, bytes.fromhex("BEC0FFEEEEFF0CEB"))
134+
135+
# Comment is SHT_PROGBITS on the larger yaml and should remain
136+
# the same after merge.
137+
comment_before_merge = main_exe_module.FindSection(".comment")
138+
self.assertTrue(comment_before_merge.IsValid())
139+
comment_content = comment_before_merge.data.ReadRawData(
140+
error, 0, comment_before_merge.data.size
141+
)
142+
143+
self.assertTrue(error.Success())
144+
self.assertEqual(comment_content, bytes.fromhex("BAADF00D"))
145+
146+
debug_info = self.getBuildArtifact("a.out.debug")
147+
self.yaml2obj("main.yaml", debug_info)
148+
149+
ci = self.dbg.GetCommandInterpreter()
150+
res = lldb.SBCommandReturnObject()
151+
ci.HandleCommand(f"target symbols add {debug_info}", res)
152+
self.assertTrue(res.Succeeded())
153+
154+
# Verify we didn't replace the sections after merge.s
155+
main_exe_module_after_merge = target.GetModuleAtIndex(0)
156+
text_after_merge = main_exe_module_after_merge.FindSection(".text")
157+
self.assertTrue(text_after_merge.IsValid())
158+
self.assertEqual(text_after_merge.data.size, text_before_merge.data.size)
159+
section_content_after_merge = text_after_merge.data.ReadRawData(
160+
error, 0, text_after_merge.data.size
161+
)
162+
self.assertTrue(error.Success())
163+
self.assertEqual(section_content_after_merge, bytes.fromhex("BEC0FFEEEEFF0CEB"))
164+
165+
comment_after_merge = main_exe_module_after_merge.FindSection(".comment")
166+
self.assertTrue(comment_after_merge.IsValid())
167+
comment_content_after_merge = comment_after_merge.data.ReadRawData(
168+
error, 0, comment_after_merge.data.size
169+
)
170+
171+
self.assertTrue(error.Success())
172+
self.assertEqual(comment_content_after_merge, bytes.fromhex("BAADF00D"))
173+
174+
def test_unified_section_list_overwrite_mixed_merge(self):
175+
"""
176+
Test the merging of an ELF file with another ELF File where the lhs has a larger .comment section
177+
and the RHS has a larger .text section.
178+
"""
179+
exe = self.getBuildArtifact("a.out")
180+
self.yaml2obj("main.largercomment.yaml", exe)
181+
182+
target = self.dbg.CreateTarget(exe)
183+
self.assertTrue(target, VALID_TARGET)
184+
main_exe_module = target.GetModuleAtIndex(0)
185+
186+
# Verify we have the expected smaller BEC0FFEE
187+
text_before_merge = main_exe_module.FindSection(".text")
188+
self.assertTrue(text_before_merge.IsValid())
189+
error = lldb.SBError()
190+
section_content = text_before_merge.data.ReadRawData(
191+
error, 0, text_before_merge.data.size
192+
)
193+
self.assertTrue(error.Success())
194+
self.assertEqual(section_content, bytes.fromhex("BEC0FFEE"))
195+
196+
# Verify we have the larger palindromic comment
197+
comment_before_merge = main_exe_module.FindSection(".comment")
198+
self.assertTrue(comment_before_merge.IsValid())
199+
comment_content = comment_before_merge.data.ReadRawData(
200+
error, 0, comment_before_merge.data.size
201+
)
202+
203+
self.assertTrue(error.Success())
204+
self.assertEqual(comment_content, bytes.fromhex("BAADF00DF00DBAAD"))
205+
206+
debug_info = self.getBuildArtifact("a.out.debug")
207+
self.yaml2obj("main.largertext.yaml", debug_info)
208+
209+
ci = self.dbg.GetCommandInterpreter()
210+
res = lldb.SBCommandReturnObject()
211+
ci.HandleCommand(f"target symbols add {debug_info}", res)
212+
self.assertTrue(res.Succeeded())
213+
214+
# Verify we replaced .text
215+
main_exe_module_after_merge = target.GetModuleAtIndex(0)
216+
text_after_merge = main_exe_module_after_merge.FindSection(".text")
217+
self.assertTrue(text_after_merge.IsValid())
218+
section_content_after_merge = text_after_merge.data.ReadRawData(
219+
error, 0, text_after_merge.data.size
220+
)
221+
self.assertTrue(error.Success())
222+
self.assertEqual(section_content_after_merge, bytes.fromhex("BEC0FFEEEEFF0CEB"))
223+
224+
# Verify .comment is still the same.
225+
comment_after_merge = main_exe_module_after_merge.FindSection(".comment")
226+
self.assertTrue(comment_after_merge.IsValid())
227+
comment_content_after_merge = comment_after_merge.data.ReadRawData(
228+
error, 0, comment_after_merge.data.size
229+
)
230+
231+
self.assertTrue(error.Success())
232+
self.assertEqual(comment_content_after_merge, bytes.fromhex("BAADF00DF00DBAAD"))

‎lldb/test/API/python_api/unified_section_list/main.largercomment.yaml‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
--- !ELF
2+
FileHeader:
3+
Class: ELFCLASS64
4+
Data: ELFDATA2LSB
5+
Type: ET_DYN
6+
Machine: EM_X86_64
7+
Entry: 0x1040
8+
ProgramHeaders:
9+
- Type: PT_PHDR
10+
Flags: [ PF_R ]
11+
VAddr: 0x40
12+
Align: 0x8
13+
Offset: 0x40
14+
- Type: PT_LOAD
15+
Flags: [ PF_R ]
16+
FirstSec: .text
17+
LastSec: .fini
18+
Align: 0x1000
19+
Offset: 0x0
20+
Sections:
21+
- Name: .text
22+
Type: SHT_PROGBITS
23+
Flags: [ SHF_ALLOC, SHF_EXECINSTR ]
24+
Address: 0x1040
25+
AddressAlign: 0x10
26+
Content: BEC0FFEE
27+
- Name: .fini
28+
Type: SHT_PROGBITS
29+
Flags: [ SHF_ALLOC, SHF_EXECINSTR ]
30+
Address: 0x1140
31+
AddressAlign: 0x4
32+
Content: DEADBEEF
33+
- Name: .comment
34+
Type: SHT_PROGBITS
35+
Flags: [ SHF_ALLOC ]
36+
Address: 0x3140
37+
AddressAlign: 0x4
38+
Content: BAADF00DF00DBAAD
39+
Symbols:
40+
- Name: main
41+
Type: STT_FUNC
42+
Section: .text
43+
Binding: STB_GLOBAL
44+
Value: 0x1130
45+
Size: 0xF
46+
...

‎lldb/test/API/python_api/unified_section_list/main.largertext.yaml‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
--- !ELF
2+
FileHeader:
3+
Class: ELFCLASS64
4+
Data: ELFDATA2LSB
5+
Type: ET_DYN
6+
Machine: EM_X86_64
7+
Entry: 0x1040
8+
ProgramHeaders:
9+
- Type: PT_PHDR
10+
Flags: [ PF_R ]
11+
VAddr: 0x40
12+
Align: 0x8
13+
Offset: 0x40
14+
- Type: PT_LOAD
15+
Flags: [ PF_R ]
16+
FirstSec: .text
17+
LastSec: .fini
18+
Align: 0x1000
19+
Offset: 0x0
20+
Sections:
21+
- Name: .text
22+
Type: SHT_PROGBITS
23+
Flags: [ SHF_ALLOC, SHF_EXECINSTR ]
24+
Address: 0x1040
25+
AddressAlign: 0x10
26+
Content: BEC0FFEEEEFF0CEB
27+
- Name: .fini
28+
Type: SHT_PROGBITS
29+
Flags: [ SHF_ALLOC, SHF_EXECINSTR ]
30+
Address: 0x1140
31+
AddressAlign: 0x4
32+
Content: DEADBEEF
33+
- Name: .comment
34+
Type: SHT_PROGBITS
35+
Flags: [ SHF_ALLOC ]
36+
Address: 0x3140
37+
AddressAlign: 0x4
38+
Content: BAADF00D
39+
Symbols:
40+
- Name: main
41+
Type: STT_FUNC
42+
Section: .text
43+
Binding: STB_GLOBAL
44+
Value: 0x1130
45+
Size: 0xF
46+
...

‎lldb/test/API/python_api/unified_section_list/main.yaml‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
--- !ELF
2+
FileHeader:
3+
Class: ELFCLASS64
4+
Data: ELFDATA2LSB
5+
Type: ET_DYN
6+
Machine: EM_X86_64
7+
Entry: 0x1040
8+
ProgramHeaders:
9+
- Type: PT_PHDR
10+
Flags: [ PF_R ]
11+
VAddr: 0x40
12+
Align: 0x8
13+
Offset: 0x40
14+
- Type: PT_LOAD
15+
Flags: [ PF_R ]
16+
FirstSec: .text
17+
LastSec: .fini
18+
Align: 0x1000
19+
Offset: 0x0
20+
Sections:
21+
- Name: .text
22+
Type: SHT_PROGBITS
23+
Flags: [ SHF_ALLOC, SHF_EXECINSTR ]
24+
Address: 0x1040
25+
AddressAlign: 0x10
26+
Content: BEC0FFEE
27+
- Name: .fini
28+
Type: SHT_PROGBITS
29+
Flags: [ SHF_ALLOC, SHF_EXECINSTR ]
30+
Address: 0x1140
31+
AddressAlign: 0x4
32+
Content: DEADBEEF
33+
- Name: .comment
34+
Type: SHT_NOBITS
35+
Flags: [ SHF_ALLOC ]
36+
Address: 0x3140
37+
AddressAlign: 0x4
38+
Symbols:
39+
- Name: main
40+
Type: STT_FUNC
41+
Section: .text
42+
Binding: STB_GLOBAL
43+
Value: 0x1130
44+
Size: 0xF
45+
...

0 commit comments

Comments
 (0)