Skip to content

Commit 0921531

Browse files
e-kudllvmbot
e-kud
authored and
llvmbot
committed
[llvm-objcopy][COFF] Update .symidx values after stripping (#153322)
After deleting debug sections, symbol indices are shifted but sections consisting of .symidx directives are completely ignored. Update symbol indices as well. (cherry picked from commit 74d52f9)
1 parent c346c16 commit 0921531

File tree

6 files changed

+441
-0
lines changed

6 files changed

+441
-0
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ using namespace object;
1818
void Object::addSymbols(ArrayRef<Symbol> NewSymbols) {
1919
for (Symbol S : NewSymbols) {
2020
S.UniqueId = NextSymbolUniqueId++;
21+
S.OriginalRawIndex = NextSymbolOriginalIndex;
22+
NextSymbolOriginalIndex += 1 + S.Sym.NumberOfAuxSymbols;
2123
Symbols.emplace_back(S);
2224
}
2325
updateSymbols();

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ struct Symbol {
8989
std::optional<size_t> WeakTargetSymbolId;
9090
size_t UniqueId;
9191
size_t RawIndex;
92+
size_t OriginalRawIndex;
9293
bool Referenced;
9394
};
9495

@@ -140,6 +141,7 @@ struct Object {
140141
DenseMap<size_t, Symbol *> SymbolMap;
141142

142143
size_t NextSymbolUniqueId = 0;
144+
size_t NextSymbolOriginalIndex = 0;
143145

144146
std::vector<Section> Sections;
145147
DenseMap<ssize_t, Section *> SectionMap;

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

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
#include "llvm/ADT/StringRef.h"
1313
#include "llvm/BinaryFormat/COFF.h"
1414
#include "llvm/Object/COFF.h"
15+
#include "llvm/Support/CRC.h"
16+
#include "llvm/Support/Endian.h"
1517
#include "llvm/Support/Errc.h"
1618
#include "llvm/Support/ErrorHandling.h"
1719
#include <cstddef>
@@ -92,6 +94,77 @@ Error COFFWriter::finalizeSymbolContents() {
9294
return Error::success();
9395
}
9496

97+
Error COFFWriter::finalizeSymIdxContents() {
98+
// CFGuards shouldn't be present in PE.
99+
if (Obj.IsPE)
100+
return Error::success();
101+
102+
// Currently handle only sections consisting only of .symidx.
103+
// TODO: other sections such as .impcall and .hybmp$x require more complex
104+
// handling as they have more complex layout.
105+
auto IsSymIdxSection = [](StringRef Name) {
106+
return Name == ".gljmp$y" || Name == ".giats$y" || Name == ".gfids$y" ||
107+
Name == ".gehcont$y";
108+
};
109+
110+
DenseMap<size_t, size_t> SymIdMap;
111+
SmallDenseMap<ssize_t, coff_aux_section_definition *, 4> SecIdMap;
112+
for (Symbol &Sym : Obj.getMutableSymbols()) {
113+
SymIdMap[Sym.OriginalRawIndex] = Sym.RawIndex;
114+
115+
// We collect only definition symbols of the sections to update the
116+
// checksums.
117+
if (Sym.Sym.StorageClass == IMAGE_SYM_CLASS_STATIC &&
118+
Sym.Sym.NumberOfAuxSymbols == 1 && Sym.Sym.Value == 0 &&
119+
IsSymIdxSection(Sym.Name))
120+
SecIdMap[Sym.TargetSectionId] =
121+
reinterpret_cast<coff_aux_section_definition *>(
122+
Sym.AuxData[0].Opaque);
123+
}
124+
125+
for (Section &Sec : Obj.getMutableSections()) {
126+
if (!IsSymIdxSection(Sec.Name))
127+
continue;
128+
129+
ArrayRef<uint8_t> RawIds = Sec.getContents();
130+
// Nothing to do and also the checksum will be -1 instead of 0 if we
131+
// recalculate it on empty input.
132+
if (RawIds.size() == 0)
133+
continue;
134+
135+
auto SecDefIt = SecIdMap.find(Sec.UniqueId);
136+
if (SecDefIt == SecIdMap.end())
137+
return createStringError(object_error::invalid_symbol_index,
138+
"section '%s' does not have the corresponding "
139+
"symbol or the symbol has unexpected format",
140+
Sec.Name.str().c_str());
141+
142+
// Create updated content.
143+
ArrayRef<support::ulittle32_t> Ids(
144+
reinterpret_cast<const support::ulittle32_t *>(RawIds.data()),
145+
RawIds.size() / 4);
146+
std::vector<support::ulittle32_t> NewIds;
147+
for (support::ulittle32_t Id : Ids) {
148+
auto SymIdIt = SymIdMap.find(Id);
149+
if (SymIdIt == SymIdMap.end())
150+
return createStringError(object_error::invalid_symbol_index,
151+
"section '%s' contains a .symidx (%d) that is "
152+
"incorrect or was stripped",
153+
Sec.Name.str().c_str(), Id.value());
154+
NewIds.push_back(support::ulittle32_t(SymIdIt->getSecond()));
155+
}
156+
ArrayRef<uint8_t> NewRawIds(reinterpret_cast<uint8_t *>(NewIds.data()),
157+
RawIds.size());
158+
// Update the checksum.
159+
JamCRC JC(/*Init=*/0);
160+
JC.update(NewRawIds);
161+
SecDefIt->getSecond()->CheckSum = JC.getCRC();
162+
// Set new content.
163+
Sec.setOwnedContents(NewRawIds.vec());
164+
}
165+
return Error::success();
166+
}
167+
95168
void COFFWriter::layoutSections() {
96169
for (auto &S : Obj.getMutableSections()) {
97170
if (S.Header.SizeOfRawData > 0)
@@ -183,6 +256,8 @@ Error COFFWriter::finalize(bool IsBigObj) {
183256
return E;
184257
if (Error E = finalizeSymbolContents())
185258
return E;
259+
if (Error E = finalizeSymIdxContents())
260+
return E;
186261

187262
size_t SizeOfHeaders = 0;
188263
FileAlignment = 1;

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ class COFFWriter {
3434
template <class SymbolTy> std::pair<size_t, size_t> finalizeSymbolTable();
3535
Error finalizeRelocTargets();
3636
Error finalizeSymbolContents();
37+
Error finalizeSymIdxContents();
3738
void layoutSections();
3839
Expected<size_t> finalizeStringTable();
3940

‎llvm/test/tools/llvm-objcopy/COFF/strip-invalid-symidx-section.test‎

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
## Test that we bail out if a section consisting of symidx is invalid.
2+
3+
## In this case, the symbol .gfids$y is not present at all.
4+
# RUN: yaml2obj %s --docnum=1 -o %t1.in.o
5+
# RUN: not llvm-objcopy --strip-debug %t1.in.o %t1.out.o 2>&1 | FileCheck %s --check-prefix=ERROR-NOSYM -DFILE=%t1.out.o
6+
7+
# ERROR-NOSYM: error: '[[FILE]]': section '.gfids$y' does not have the corresponding symbol or the symbol has unexpected format
8+
9+
--- !COFF
10+
header:
11+
Machine: IMAGE_FILE_MACHINE_AMD64
12+
Characteristics: [ ]
13+
sections:
14+
- Name: .text
15+
Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
16+
- Name: '.gfids$y'
17+
Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
18+
Alignment: 4
19+
SectionData: '04000000'
20+
SizeOfRawData: 8
21+
symbols:
22+
- Name: .text
23+
Value: 0
24+
SectionNumber: 1
25+
SimpleType: IMAGE_SYM_TYPE_NULL
26+
ComplexType: IMAGE_SYM_DTYPE_NULL
27+
StorageClass: IMAGE_SYM_CLASS_STATIC
28+
SectionDefinition:
29+
Length: 0
30+
NumberOfRelocations: 4
31+
NumberOfLinenumbers: 0
32+
CheckSum: 0
33+
Number: 1
34+
- Name: foo
35+
Value: 0
36+
SectionNumber: 0
37+
SimpleType: IMAGE_SYM_TYPE_NULL
38+
ComplexType: IMAGE_SYM_DTYPE_NULL
39+
StorageClass: IMAGE_SYM_CLASS_EXTERNAL
40+
...
41+
42+
## In this case, the symbol .giats$y has a non-zero offset.
43+
# RUN: yaml2obj %s --docnum=2 -o %t2.in.o
44+
# RUN: not llvm-objcopy --strip-debug %t2.in.o %t2.out.o 2>&1 | FileCheck %s --check-prefix=ERROR-OFFSET -DFILE=%t2.out.o
45+
46+
# ERROR-OFFSET: error: '[[FILE]]': section '.giats$y' does not have the corresponding symbol or the symbol has unexpected format
47+
48+
--- !COFF
49+
header:
50+
Machine: IMAGE_FILE_MACHINE_AMD64
51+
Characteristics: [ ]
52+
sections:
53+
- Name: .text
54+
Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
55+
- Name: '.giats$y'
56+
Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
57+
Alignment: 4
58+
SectionData: '0600000010000000'
59+
SizeOfRawData: 8
60+
symbols:
61+
- Name: .text
62+
Value: 0
63+
SectionNumber: 1
64+
SimpleType: IMAGE_SYM_TYPE_NULL
65+
ComplexType: IMAGE_SYM_DTYPE_NULL
66+
StorageClass: IMAGE_SYM_CLASS_STATIC
67+
SectionDefinition:
68+
Length: 0
69+
NumberOfRelocations: 0
70+
NumberOfLinenumbers: 0
71+
CheckSum: 0
72+
Number: 1
73+
- Name: '.giats$y'
74+
Value: 42
75+
SectionNumber: 2
76+
SimpleType: IMAGE_SYM_TYPE_NULL
77+
ComplexType: IMAGE_SYM_DTYPE_NULL
78+
StorageClass: IMAGE_SYM_CLASS_STATIC
79+
SectionDefinition:
80+
Length: 8
81+
NumberOfRelocations: 0
82+
NumberOfLinenumbers: 0
83+
CheckSum: 1167279533
84+
Number: 5
85+
- Name: foo
86+
Value: 0
87+
SectionNumber: 0
88+
SimpleType: IMAGE_SYM_TYPE_NULL
89+
ComplexType: IMAGE_SYM_DTYPE_NULL
90+
StorageClass: IMAGE_SYM_CLASS_EXTERNAL
91+
...
92+
93+
## In this case, the symbol .gljmp$y has a non-static storage class.
94+
# RUN: yaml2obj %s --docnum=3 -o %t3.in.o
95+
# RUN: not llvm-objcopy --strip-debug %t3.in.o %t3.out.o 2>&1 | FileCheck %s --check-prefix=ERROR-EXTERNAL -DFILE=%t3.out.o
96+
97+
# ERROR-EXTERNAL: error: '[[FILE]]': section '.gljmp$y' does not have the corresponding symbol or the symbol has unexpected format
98+
99+
--- !COFF
100+
header:
101+
Machine: IMAGE_FILE_MACHINE_AMD64
102+
Characteristics: [ ]
103+
sections:
104+
- Name: .text
105+
Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
106+
- Name: '.gljmp$y'
107+
Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
108+
Alignment: 4
109+
SectionData: '0600000010000000'
110+
SizeOfRawData: 8
111+
symbols:
112+
- Name: .text
113+
Value: 0
114+
SectionNumber: 1
115+
SimpleType: IMAGE_SYM_TYPE_NULL
116+
ComplexType: IMAGE_SYM_DTYPE_NULL
117+
StorageClass: IMAGE_SYM_CLASS_STATIC
118+
SectionDefinition:
119+
Length: 0
120+
NumberOfRelocations: 0
121+
NumberOfLinenumbers: 0
122+
CheckSum: 0
123+
Number: 1
124+
- Name: '.gljmp$y'
125+
Value: 0
126+
SectionNumber: 2
127+
SimpleType: IMAGE_SYM_TYPE_NULL
128+
ComplexType: IMAGE_SYM_DTYPE_NULL
129+
StorageClass: IMAGE_SYM_CLASS_EXTERNAL
130+
- Name: foo
131+
Value: 0
132+
SectionNumber: 0
133+
SimpleType: IMAGE_SYM_TYPE_NULL
134+
ComplexType: IMAGE_SYM_DTYPE_NULL
135+
StorageClass: IMAGE_SYM_CLASS_EXTERNAL
136+
...
137+
138+
## In this case, .gfids$y contains a symbol index that is not present in the
139+
## symbol table. Generally the behavior should be the same for every section consisting
140+
## of .symidx directives, e.g .giats$y, .gljmp$y and .gehcont$y.
141+
# RUN: yaml2obj %s --docnum=4 -o %t4.in.o
142+
# RUN: not llvm-objcopy --strip-debug %t4.in.o %t4.out.o 2>&1 | FileCheck %s --check-prefix=ERROR-SYMIDX -DFILE=%t4.out.o
143+
144+
# ERROR-SYMIDX: error: '[[FILE]]': section '.gfids$y' contains a .symidx (16) that is incorrect or was stripped
145+
--- !COFF
146+
header:
147+
Machine: IMAGE_FILE_MACHINE_AMD64
148+
Characteristics: [ ]
149+
sections:
150+
- Name: .text
151+
Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
152+
- Name: '.gfids$y'
153+
Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
154+
Alignment: 4
155+
SectionData: '0400000010000000'
156+
SizeOfRawData: 8
157+
symbols:
158+
- Name: .text
159+
Value: 0
160+
SectionNumber: 1
161+
SimpleType: IMAGE_SYM_TYPE_NULL
162+
ComplexType: IMAGE_SYM_DTYPE_NULL
163+
StorageClass: IMAGE_SYM_CLASS_STATIC
164+
SectionDefinition:
165+
Length: 0
166+
NumberOfRelocations: 0
167+
NumberOfLinenumbers: 0
168+
CheckSum: 0
169+
Number: 1
170+
- Name: '.gfids$y'
171+
Value: 0
172+
SectionNumber: 2
173+
SimpleType: IMAGE_SYM_TYPE_NULL
174+
ComplexType: IMAGE_SYM_DTYPE_NULL
175+
StorageClass: IMAGE_SYM_CLASS_STATIC
176+
SectionDefinition:
177+
Length: 8
178+
NumberOfRelocations: 0
179+
NumberOfLinenumbers: 0
180+
CheckSum: 1167279533
181+
Number: 5
182+
- Name: foo
183+
Value: 0
184+
SectionNumber: 0
185+
SimpleType: IMAGE_SYM_TYPE_NULL
186+
ComplexType: IMAGE_SYM_DTYPE_NULL
187+
StorageClass: IMAGE_SYM_CLASS_EXTERNAL
188+
...

0 commit comments

Comments
 (0)