Skip to content

Commit ddf528b

Browse files
npmillermstorsjo
npmiller
authored and
mstorsjo
committed
[llvm-objcopy][COFF] Fix section name encoding
The section name encoding for `llvm-objcopy` had two main issues, the first is that the size used for the `snprintf` in the original code is incorrect because `snprintf` adds a null byte, so this code was only able to encode offsets of 6 digits - `/`, `\0` and 6 digits of the offset - rather than the 7 digits it should support. And the second part is that it didn't support the base64 encoding for offsets larger than 7 digits. This issue specifically showed up when using the `clang-offload-bundler` with a binary containing a lot of symbols/sections, since it uses `llvm-objcopy` to add the sections containing the offload code. Reviewed By: jhenderson Differential Revision: https://reviews.llvm.org/D118692
1 parent 85f4023 commit ddf528b

File tree

4 files changed

+112
-7
lines changed

4 files changed

+112
-7
lines changed

‎llvm/lib/ObjCopy/COFF/Writer.cpp

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ void COFFWriter::layoutSections() {
116116
}
117117
}
118118

119-
size_t COFFWriter::finalizeStringTable() {
119+
Expected<size_t> COFFWriter::finalizeStringTable() {
120120
for (const auto &S : Obj.getSections())
121121
if (S.Name.size() > COFF::NameSize)
122122
StrTabBuilder.add(S.Name);
@@ -129,11 +129,16 @@ size_t COFFWriter::finalizeStringTable() {
129129

130130
for (auto &S : Obj.getMutableSections()) {
131131
memset(S.Header.Name, 0, sizeof(S.Header.Name));
132-
if (S.Name.size() > COFF::NameSize) {
133-
snprintf(S.Header.Name, sizeof(S.Header.Name), "/%d",
134-
(int)StrTabBuilder.getOffset(S.Name));
135-
} else {
132+
if (S.Name.size() <= COFF::NameSize) {
133+
// Short names can go in the field directly.
136134
memcpy(S.Header.Name, S.Name.data(), S.Name.size());
135+
} else {
136+
// Offset of the section name in the string table.
137+
size_t Offset = StrTabBuilder.getOffset(S.Name);
138+
if (!COFF::encodeSectionName(S.Header.Name, Offset))
139+
return createStringError(object_error::invalid_section_index,
140+
"COFF string table is greater than 64GB, "
141+
"unable to encode section name offset");
137142
}
138143
}
139144
for (auto &S : Obj.getMutableSymbols()) {
@@ -219,7 +224,11 @@ Error COFFWriter::finalize(bool IsBigObj) {
219224
Obj.PeHeader.CheckSum = 0;
220225
}
221226

222-
size_t StrTabSize = finalizeStringTable();
227+
Expected<size_t> StrTabSizeOrErr = finalizeStringTable();
228+
if (!StrTabSizeOrErr)
229+
return StrTabSizeOrErr.takeError();
230+
231+
size_t StrTabSize = *StrTabSizeOrErr;
223232

224233
size_t PointerToSymbolTable = FileSize;
225234
// StrTabSize <= 4 is the size of an empty string table, only consisting

‎llvm/lib/ObjCopy/COFF/Writer.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ class COFFWriter {
3535
Error finalizeRelocTargets();
3636
Error finalizeSymbolContents();
3737
void layoutSections();
38-
size_t finalizeStringTable();
38+
Expected<size_t> finalizeStringTable();
3939

4040
Error finalize(bool IsBigObj);
4141

‎llvm/test/tools/llvm-objcopy/COFF/section-name-encoding.s

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
## Check that COFF section names of sections added by llvm-objcopy are properly
2+
## encoded.
3+
##
4+
## Encodings for different name lengths and string table index:
5+
## [0, 8]: raw name
6+
## (8, 999999]: base 10 string table index (/9999999)
7+
## (999999, 0xFFFFFFFF]: base 64 string table index (##AAAAAA)
8+
##
9+
## Note: the names in the string table will be sorted in reverse
10+
## lexicographical order. Use a suffix letter (z, y, x, ...) to
11+
## get the preferred ordering of names in the test.
12+
##
13+
# REQUIRES: x86-registered-target
14+
##
15+
# RUN: echo DEADBEEF > %t.sec
16+
# RUN: llvm-mc -triple x86_64-pc-win32 -filetype=obj %s -o %t.obj
17+
# RUN: llvm-objcopy --add-section=s1234567=%t.sec \
18+
# RUN: --add-section=s1234567z=%t.sec \
19+
# RUN: --add-section=sevendigitx=%t.sec \
20+
# RUN: --add-section=doubleslashv=%t.sec \
21+
# RUN: %t.obj %t
22+
# RUN: llvm-readobj --sections %t | FileCheck %s
23+
24+
## Raw encoding
25+
26+
# CHECK: Section {
27+
# CHECK: Number: 14
28+
# CHECK: Name: s1234567 (73 31 32 33 34 35 36 37)
29+
# CHECK: }
30+
31+
## Base 10 encoding with a small offset, section name at the beginning of the
32+
## string table.
33+
34+
## /4
35+
##
36+
# CHECK: Section {
37+
# CHECK: Number: 15
38+
# CHECK: Name: s1234567z (2F 34 00 00 00 00 00 00)
39+
# CHECK: }
40+
41+
## Base 10 encoding with a 7 digit offset, section name after the y padding in
42+
## the string table.
43+
44+
## /1000029 == 4 + 10 + (5 * (2 + (20 * 10 * 1000) + 1))
45+
## v | | v ~~~~~~~~~~~~~~ v
46+
## table size v v "p0" y pad NULL separator
47+
## "s1234567z\0" # of pad sections
48+
##
49+
# CHECK: Section {
50+
# CHECK: Number: 16
51+
# CHECK: Name: sevendigitx (2F 31 30 30 30 30 32 39)
52+
# CHECK: }
53+
54+
## Base 64 encoding, section name after the w padding in the string table.
55+
56+
## //AAmJa4 == 1000029 + 12 + (5 * (2 + (9 * 20 * 10 * 1000) + 1)) == 38*64^3 + 9*64^2 + 26*64 + 56
57+
## v | | v ~~~~~~~~~~~~~~~~~~ v
58+
## sevendigitx offset v v "p0" w pad NULL separator
59+
## "sevendigitx\0" # of pad sections
60+
##
61+
## "2F 2F 41 41 6D 4A 61 34" is "//AAmJa4", which decodes to "0 0 38 9 26 56".
62+
##
63+
# CHECK: Section {
64+
# CHECK: Number: 17
65+
# CHECK: Name: doubleslashv (2F 2F 41 41 6D 4A 61 34)
66+
# CHECK: }
67+
68+
## Generate padding sections to increase the string table size to at least
69+
## 1,000,000 bytes.
70+
.macro pad_sections2 pad
71+
## 10x \pad
72+
.section p0\pad\pad\pad\pad\pad\pad\pad\pad\pad\pad; .long 1
73+
.section p1\pad\pad\pad\pad\pad\pad\pad\pad\pad\pad; .long 1
74+
.section p2\pad\pad\pad\pad\pad\pad\pad\pad\pad\pad; .long 1
75+
.section p3\pad\pad\pad\pad\pad\pad\pad\pad\pad\pad; .long 1
76+
.section p4\pad\pad\pad\pad\pad\pad\pad\pad\pad\pad; .long 1
77+
.endm
78+
79+
.macro pad_sections pad
80+
## 20x \pad
81+
pad_sections2 \pad\pad\pad\pad\pad\pad\pad\pad\pad\pad\pad\pad\pad\pad\pad\pad\pad\pad\pad\pad
82+
.endm
83+
84+
## 1000x 'y'
85+
pad_sections yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy
86+
87+
## Generate padding sections to increase the string table size to at least
88+
## 10,000,000 bytes.
89+
.macro pad_sections_ex pad
90+
## 9x \pad
91+
pad_sections \pad\pad\pad\pad\pad\pad\pad\pad\pad
92+
.endm
93+
94+
## 1000x 'w'
95+
pad_sections_ex wwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwww

‎llvm/tools/llvm-objcopy/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ set(LLVM_LINK_COMPONENTS
44
Option
55
Support
66
MC
7+
BinaryFormat
78
)
89

910
set(LLVM_TARGET_DEFINITIONS ObjcopyOpts.td)

0 commit comments

Comments
 (0)