Skip to content

Commit 4d2eda2

Browse files
committed
Revert "[LLD] [COFF] Use StringTableBuilder to optimize the string table"
This reverts commit 9ffeaaa. This fixes debugging large executables with lldb and gdb. When StringTableBuilder is used, the string offsets for any string can point anywhere in the string table - while previously, all strings were inserted in order (without deduplication and tail merging). For symbols, there's no complications in encoding the string offset; the offset is encoded as a raw 32 bit binary number in half of the symbol name field. For sections, the string table offset is written as "/<decimaloffset>", but if the decimal offset would be larger than 7 digits, it's instead written as "//<base64offset>". Tools that operate on object files can handle the base64 offset format, but apparently neither lldb nor gdb expect that syntax when locating the debug information section. Prior to the reverted commit, all long section names were located at the start of the string table, so their offset never exceeded the range for the decimal syntax. Just reverting this change for now, as the actual benefit from it was fairly modest. Longer term, lld could write all long section names unoptimized at the start of the string table, followed by all the strings for symbol names, with deduplication and tail merging. And lldb and gdb could be fixed to handle sections with the base64 offset syntax. This fixes mstorsjo/llvm-mingw#289.
1 parent ab088de commit 4d2eda2

File tree

1 file changed

+20
-30
lines changed

1 file changed

+20
-30
lines changed

lld/COFF/Writer.cpp

Lines changed: 20 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
#include "llvm/ADT/STLExtras.h"
2525
#include "llvm/ADT/StringSet.h"
2626
#include "llvm/ADT/StringSwitch.h"
27-
#include "llvm/MC/StringTableBuilder.h"
2827
#include "llvm/Support/BinaryStreamReader.h"
2928
#include "llvm/Support/Debug.h"
3029
#include "llvm/Support/Endian.h"
@@ -196,9 +195,7 @@ class PartialSectionKey {
196195
// The writer writes a SymbolTable result to a file.
197196
class Writer {
198197
public:
199-
Writer(COFFLinkerContext &c)
200-
: buffer(errorHandler().outputBuffer),
201-
strtab(StringTableBuilder::WinCOFF), ctx(c) {}
198+
Writer(COFFLinkerContext &c) : buffer(errorHandler().outputBuffer), ctx(c) {}
202199
void run();
203200

204201
private:
@@ -243,6 +240,7 @@ class Writer {
243240
PartialSection *findPartialSection(StringRef name, uint32_t outChars);
244241

245242
llvm::Optional<coff_symbol16> createSymbol(Defined *d);
243+
size_t addEntryToStringTable(StringRef str);
246244

247245
OutputSection *findSection(StringRef name);
248246
void addBaserels();
@@ -252,7 +250,7 @@ class Writer {
252250

253251
std::unique_ptr<FileOutputBuffer> &buffer;
254252
std::map<PartialSectionKey, PartialSection *> partialSections;
255-
StringTableBuilder strtab;
253+
std::vector<char> strtab;
256254
std::vector<llvm::object::coff_symbol16> outputSymtab;
257255
IdataContents idata;
258256
Chunk *importTableStart = nullptr;
@@ -1128,6 +1126,14 @@ void Writer::assignOutputSectionIndices() {
11281126
sc->setOutputSectionIdx(mc->getOutputSectionIdx());
11291127
}
11301128

1129+
size_t Writer::addEntryToStringTable(StringRef str) {
1130+
assert(str.size() > COFF::NameSize);
1131+
size_t offsetOfEntry = strtab.size() + 4; // +4 for the size field
1132+
strtab.insert(strtab.end(), str.begin(), str.end());
1133+
strtab.push_back('\0');
1134+
return offsetOfEntry;
1135+
}
1136+
11311137
Optional<coff_symbol16> Writer::createSymbol(Defined *def) {
11321138
coff_symbol16 sym;
11331139
switch (def->kind()) {
@@ -1164,8 +1170,7 @@ Optional<coff_symbol16> Writer::createSymbol(Defined *def) {
11641170
StringRef name = def->getName();
11651171
if (name.size() > COFF::NameSize) {
11661172
sym.Name.Offset.Zeroes = 0;
1167-
sym.Name.Offset.Offset = 0; // Filled in later
1168-
strtab.add(name);
1173+
sym.Name.Offset.Offset = addEntryToStringTable(name);
11691174
} else {
11701175
memset(sym.Name.ShortName, 0, COFF::NameSize);
11711176
memcpy(sym.Name.ShortName, name.data(), name.size());
@@ -1192,7 +1197,6 @@ void Writer::createSymbolAndStringTable() {
11921197
// solution where discardable sections have long names preserved and
11931198
// non-discardable sections have their names truncated, to ensure that any
11941199
// section which is mapped at runtime also has its name mapped at runtime.
1195-
std::vector<OutputSection *> longNameSections;
11961200
for (OutputSection *sec : ctx.outputSections) {
11971201
if (sec->name.size() <= COFF::NameSize)
11981202
continue;
@@ -1203,12 +1207,9 @@ void Writer::createSymbolAndStringTable() {
12031207
" is longer than 8 characters and will use a non-standard string "
12041208
"table");
12051209
}
1206-
1207-
strtab.add(sec->name);
1208-
longNameSections.push_back(sec);
1210+
sec->setStringTableOff(addEntryToStringTable(sec->name));
12091211
}
12101212

1211-
std::vector<std::pair<size_t, StringRef>> longNameSymbols;
12121213
if (config->debugDwarf || config->debugSymtab) {
12131214
for (ObjFile *file : ctx.objFileInstances) {
12141215
for (Symbol *b : file->getSymbols()) {
@@ -1223,33 +1224,20 @@ void Writer::createSymbolAndStringTable() {
12231224
continue;
12241225
}
12251226

1226-
if (Optional<coff_symbol16> sym = createSymbol(d)) {
1227+
if (Optional<coff_symbol16> sym = createSymbol(d))
12271228
outputSymtab.push_back(*sym);
1228-
if (d->getName().size() > COFF::NameSize)
1229-
longNameSymbols.push_back({outputSymtab.size() - 1, d->getName()});
1230-
}
12311229
}
12321230
}
12331231
}
12341232

1235-
strtab.finalize();
1236-
1237-
for (OutputSection *sec : longNameSections)
1238-
sec->setStringTableOff(strtab.getOffset(sec->name));
1239-
1240-
for (auto P : longNameSymbols) {
1241-
coff_symbol16 &sym = outputSymtab[P.first];
1242-
sym.Name.Offset.Offset = strtab.getOffset(P.second);
1243-
}
1244-
1245-
if (outputSymtab.empty() && strtab.getSize() <= 4)
1233+
if (outputSymtab.empty() && strtab.empty())
12461234
return;
12471235

12481236
// We position the symbol table to be adjacent to the end of the last section.
12491237
uint64_t fileOff = fileSize;
12501238
pointerToSymbolTable = fileOff;
12511239
fileOff += outputSymtab.size() * sizeof(coff_symbol16);
1252-
fileOff += strtab.getSize();
1240+
fileOff += 4 + strtab.size();
12531241
fileSize = alignTo(fileOff, config->fileAlign);
12541242
}
12551243

@@ -1524,7 +1512,7 @@ template <typename PEHeaderTy> void Writer::writeHeader() {
15241512
sectionTable = ArrayRef<uint8_t>(
15251513
buf - ctx.outputSections.size() * sizeof(coff_section), buf);
15261514

1527-
if (outputSymtab.empty() && strtab.getSize() <= 4)
1515+
if (outputSymtab.empty() && strtab.empty())
15281516
return;
15291517

15301518
coff->PointerToSymbolTable = pointerToSymbolTable;
@@ -1537,7 +1525,9 @@ template <typename PEHeaderTy> void Writer::writeHeader() {
15371525
// Create the string table, it follows immediately after the symbol table.
15381526
// The first 4 bytes is length including itself.
15391527
buf = reinterpret_cast<uint8_t *>(&symbolTable[numberOfSymbols]);
1540-
strtab.write(buf);
1528+
write32le(buf, strtab.size() + 4);
1529+
if (!strtab.empty())
1530+
memcpy(buf + 4, strtab.data(), strtab.size());
15411531
}
15421532

15431533
void Writer::openFile(StringRef path) {

0 commit comments

Comments
 (0)