Skip to content

Commit

Permalink
[PDB] Correctly link S_FILESTATIC records.
Browse files Browse the repository at this point in the history 
This is not a record type that clang currently generates,
but it is a record that is encountered in object files generated
by cl.  This record is unusual in that it refers directly to
the string table instead of indirectly to the string table via
the FileChecksums table.  Because of this, it was previously
overlooked and we weren't remapping the string indices at all.
This would lead to crashes in MSVC when trying to display a
variable whose debug info involved an S_FILESTATIC.

Original bug report by Alexander Ganea

Differential Revision: https://reviews.llvm.org/D41718

llvm-svn: 321883
  • Loading branch information
Zachary Turner committed Jan 5, 2018
1 parent 5b6aacf commit 6047858
Show file tree
Hide file tree
Showing 9 changed files with 3,557 additions and 39 deletions.
111 changes: 90 additions & 21 deletions lld/COFF/PDB.cpp
View file
Expand Up @@ -47,6 +47,7 @@
#include "llvm/Object/COFF.h"
#include "llvm/Support/BinaryByteStream.h"
#include "llvm/Support/Endian.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/JamCRC.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/ScopedPrinter.h"
Expand Down Expand Up @@ -412,6 +413,36 @@ static void remapTypesInSymbolRecord(ObjFile *File, SymbolKind SymKind,
}
}

static void
recordStringTableReferenceAtOffset(MutableArrayRef<uint8_t> Contents,
uint32_t Offset,
std::vector<ulittle32_t *> &StrTableRefs) {
Contents =
Contents.drop_front(Offset).take_front(sizeof(support::ulittle32_t));
ulittle32_t *Index = reinterpret_cast<ulittle32_t *>(Contents.data());
StrTableRefs.push_back(Index);
}

static void
recordStringTableReferences(SymbolKind Kind, MutableArrayRef<uint8_t> Contents,
std::vector<ulittle32_t *> &StrTableRefs) {
// For now we only handle S_FILESTATIC, but we may need the same logic for
// S_DEFRANGE and S_DEFRANGE_SUBFIELD. However, I cannot seem to generate any
// PDBs that contain these types of records, so because of the uncertainty
// they are omitted here until we can prove that it's necessary.
switch (Kind) {
case SymbolKind::S_FILESTATIC:
// FileStaticSym::ModFileOffset
recordStringTableReferenceAtOffset(Contents, 4, StrTableRefs);
break;
case SymbolKind::S_DEFRANGE:
case SymbolKind::S_DEFRANGE_SUBFIELD:
log("Not fixing up string table reference in S_DEFRANGE / "
"S_DEFRANGE_SUBFIELD record");
break;
}
}

static SymbolKind symbolKind(ArrayRef<uint8_t> RecordData) {
const RecordPrefix *Prefix =
reinterpret_cast<const RecordPrefix *>(RecordData.data());
Expand Down Expand Up @@ -628,6 +659,7 @@ static void mergeSymbolRecords(BumpPtrAllocator &Alloc, ObjFile *File,
pdb::GSIStreamBuilder &GsiBuilder,
const CVIndexMap &IndexMap,
TypeCollection &IDTable,
std::vector<ulittle32_t *> &StringTableRefs,
BinaryStreamRef SymData) {
// FIXME: Improve error recovery by warning and skipping records when
// possible.
Expand Down Expand Up @@ -656,6 +688,10 @@ static void mergeSymbolRecords(BumpPtrAllocator &Alloc, ObjFile *File,
// "real" symbols in a PDB.
translateIdSymbols(NewData, IDTable);

// If this record refers to an offset in the object file's string table,
// add that item to the global PDB string table and re-write the index.
recordStringTableReferences(Sym.kind(), Contents, StringTableRefs);

SymbolKind NewKind = symbolKind(NewData);

// Fill in "Parent" and "End" fields by maintaining a stack of scopes.
Expand Down Expand Up @@ -710,6 +746,9 @@ void PDBLinker::addObjFile(ObjFile *File) {
const CVIndexMap &IndexMap = mergeDebugT(File, ObjectIndexMap);

// Now do all live .debug$S sections.
DebugStringTableSubsectionRef CVStrTab;
DebugChecksumsSubsectionRef Checksums;
std::vector<ulittle32_t *> StringTableReferences;
for (SectionChunk *DebugChunk : File->getDebugChunks()) {
if (!DebugChunk->isLive() || DebugChunk->getSectionName() != ".debug$S")
continue;
Expand All @@ -723,14 +762,20 @@ void PDBLinker::addObjFile(ObjFile *File) {
BinaryStreamReader Reader(RelocatedDebugContents, support::little);
ExitOnErr(Reader.readArray(Subsections, RelocatedDebugContents.size()));

DebugStringTableSubsectionRef CVStrTab;
DebugChecksumsSubsectionRef Checksums;
for (const DebugSubsectionRecord &SS : Subsections) {
switch (SS.kind()) {
case DebugSubsectionKind::StringTable:
case DebugSubsectionKind::StringTable: {
auto Data = SS.getRecordData();
ArrayRef<uint8_t> Buffer;
cantFail(Data.readLongestContiguousChunk(0, Buffer));
assert(!CVStrTab.valid() &&
"Encountered multiple string table subsections!");
ExitOnErr(CVStrTab.initialize(SS.getRecordData()));
break;
}
case DebugSubsectionKind::FileChecksums:
assert(!Checksums.valid() &&
"Encountered multiple checksum subsections!");
ExitOnErr(Checksums.initialize(SS.getRecordData()));
break;
case DebugSubsectionKind::Lines:
Expand All @@ -741,36 +786,60 @@ void PDBLinker::addObjFile(ObjFile *File) {
case DebugSubsectionKind::Symbols:
if (Config->DebugGHashes) {
mergeSymbolRecords(Alloc, File, Builder.getGsiBuilder(), IndexMap,
GlobalIDTable, SS.getRecordData());
GlobalIDTable, StringTableReferences,
SS.getRecordData());
} else {
mergeSymbolRecords(Alloc, File, Builder.getGsiBuilder(), IndexMap,
IDTable, SS.getRecordData());
IDTable, StringTableReferences,
SS.getRecordData());
}
break;
default:
// FIXME: Process the rest of the subsections.
break;
}
}
}

if (Checksums.valid()) {
// Make a new file checksum table that refers to offsets in the PDB-wide
// string table. Generally the string table subsection appears after the
// checksum table, so we have to do this after looping over all the
// subsections.
if (!CVStrTab.valid())
fatal(".debug$S sections must have both a string table subsection "
"and a checksum subsection table or neither");
auto NewChecksums = make_unique<DebugChecksumsSubsection>(PDBStrTab);
for (FileChecksumEntry &FC : Checksums) {
StringRef FileName = ExitOnErr(CVStrTab.getString(FC.FileNameOffset));
ExitOnErr(Builder.getDbiBuilder().addModuleSourceFile(*File->ModuleDBI,
FileName));
NewChecksums->addChecksum(FileName, FC.Kind, FC.Checksum);
}
File->ModuleDBI->addDebugSubsection(std::move(NewChecksums));
// We should have seen all debug subsections across the entire object file now
// which means that if a StringTable subsection and Checksums subsection were
// present, now is the time to handle them.
if (!CVStrTab.valid()) {
if (Checksums.valid())
fatal(".debug$S sections with a checksums subsection must also contain a "
"string table subsection");

if (!StringTableReferences.empty())
warn("No StringTable subsection was encountered, but there are string "
"table references");
return;
}

// Rewrite each string table reference based on the value that the string
// assumes in the final PDB.
for (ulittle32_t *Ref : StringTableReferences) {
auto ExpectedString = CVStrTab.getString(*Ref);
if (!ExpectedString) {
warn("Invalid string table reference");
consumeError(ExpectedString.takeError());
continue;
}

*Ref = PDBStrTab.insert(*ExpectedString);
}

// Make a new file checksum table that refers to offsets in the PDB-wide
// string table. Generally the string table subsection appears after the
// checksum table, so we have to do this after looping over all the
// subsections.
auto NewChecksums = make_unique<DebugChecksumsSubsection>(PDBStrTab);
for (FileChecksumEntry &FC : Checksums) {
StringRef FileName = ExitOnErr(CVStrTab.getString(FC.FileNameOffset));
ExitOnErr(Builder.getDbiBuilder().addModuleSourceFile(*File->ModuleDBI,
FileName));
NewChecksums->addChecksum(FileName, FC.Kind, FC.Checksum);
}
File->ModuleDBI->addDebugSubsection(std::move(NewChecksums));
}

static PublicSym32 createPublic(Defined *Def) {
Expand Down

0 comments on commit 6047858

Please sign in to comment.