Index: lld/trunk/COFF/PDB.h =================================================================== --- lld/trunk/COFF/PDB.h +++ lld/trunk/COFF/PDB.h @@ -27,7 +27,7 @@ void createPDB(SymbolTable *Symtab, llvm::ArrayRef OutputSections, llvm::ArrayRef SectionTable, - const llvm::codeview::DebugInfo *DI); + const llvm::codeview::DebugInfo &BuildId); } } Index: lld/trunk/COFF/PDB.cpp =================================================================== --- lld/trunk/COFF/PDB.cpp +++ lld/trunk/COFF/PDB.cpp @@ -76,7 +76,7 @@ IDTable(Alloc) {} /// Emit the basic PDB structure: initial streams, headers, etc. - void initialize(const llvm::codeview::DebugInfo *DI); + void initialize(const llvm::codeview::DebugInfo &BuildId); /// Link CodeView from each object file in the symbol table into the PDB. void addObjectsToPDB(); @@ -808,15 +808,15 @@ void coff::createPDB(SymbolTable *Symtab, ArrayRef OutputSections, ArrayRef SectionTable, - const llvm::codeview::DebugInfo *DI) { + const llvm::codeview::DebugInfo &BuildId) { PDBLinker PDB(Symtab); - PDB.initialize(DI); + PDB.initialize(BuildId); PDB.addObjectsToPDB(); PDB.addSections(OutputSections, SectionTable); PDB.commit(); } -void PDBLinker::initialize(const llvm::codeview::DebugInfo *DI) { +void PDBLinker::initialize(const llvm::codeview::DebugInfo &BuildId) { ExitOnErr(Builder.initialize(4096)); // 4096 is blocksize // Create streams in MSF for predefined streams, namely @@ -826,17 +826,17 @@ // Add an Info stream. auto &InfoBuilder = Builder.getInfoBuilder(); - InfoBuilder.setAge(DI ? DI->PDB70.Age : 0); + InfoBuilder.setAge(BuildId.PDB70.Age); - GUID uuid{}; - if (DI) - memcpy(&uuid, &DI->PDB70.Signature, sizeof(uuid)); + GUID uuid; + memcpy(&uuid, &BuildId.PDB70.Signature, sizeof(uuid)); InfoBuilder.setGuid(uuid); InfoBuilder.setSignature(time(nullptr)); InfoBuilder.setVersion(pdb::PdbRaw_ImplVer::PdbImplVC70); // Add an empty DBI stream. pdb::DbiStreamBuilder &DbiBuilder = Builder.getDbiBuilder(); + DbiBuilder.setAge(BuildId.PDB70.Age); DbiBuilder.setVersionHeader(pdb::PdbDbiV70); ExitOnErr(DbiBuilder.addDbgStream(pdb::DbgHeaderType::NewFPO, {})); } Index: lld/trunk/COFF/Writer.cpp =================================================================== --- lld/trunk/COFF/Writer.cpp +++ lld/trunk/COFF/Writer.cpp @@ -20,6 +20,7 @@ #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/StringSwitch.h" +#include "llvm/Support/BinaryStreamReader.h" #include "llvm/Support/Debug.h" #include "llvm/Support/Endian.h" #include "llvm/Support/FileOutputBuffer.h" @@ -92,19 +93,17 @@ void writeTo(uint8_t *B) const override { // Save off the DebugInfo entry to backfill the file signature (build id) // in Writer::writeBuildId - DI = reinterpret_cast(B + OutputSectionOff); - - DI->Signature.CVSignature = OMF::Signature::PDB70; + BuildId = reinterpret_cast(B + OutputSectionOff); // variable sized field (PDB Path) - auto *P = reinterpret_cast(B + OutputSectionOff + sizeof(*DI)); + char *P = reinterpret_cast(B + OutputSectionOff + sizeof(*BuildId)); if (!PDBAbsPath.empty()) memcpy(P, PDBAbsPath.data(), PDBAbsPath.size()); P[PDBAbsPath.size()] = '\0'; } SmallString<128> PDBAbsPath; - mutable codeview::DebugInfo *DI = nullptr; + mutable codeview::DebugInfo *BuildId = nullptr; }; // The writer writes a SymbolTable result to a file. @@ -126,8 +125,8 @@ void fixSafeSEHSymbols(); void setSectionPermissions(); void writeSections(); - void sortExceptionTable(); void writeBuildId(); + void sortExceptionTable(); llvm::Optional createSymbol(Defined *D); size_t addEntryToStringTable(StringRef Str); @@ -153,6 +152,7 @@ Chunk *DebugDirectory = nullptr; std::vector DebugRecords; CVDebugRecordChunk *BuildId = nullptr; + Optional PreviousBuildId; ArrayRef SectionTable; uint64_t FileSize; @@ -220,6 +220,65 @@ } // namespace coff } // namespace lld +// PDBs are matched against executables using a build id which consists of three +// components: +// 1. A 16-bit GUID +// 2. An age +// 3. A time stamp. +// +// Debuggers and symbol servers match executables against debug info by checking +// each of these components of the EXE/DLL against the corresponding value in +// the PDB and failing a match if any of the components differ. In the case of +// symbol servers, symbols are cached in a folder that is a function of the +// GUID. As a result, in order to avoid symbol cache pollution where every +// incremental build copies a new PDB to the symbol cache, we must try to re-use +// the existing GUID if one exists, but bump the age. This way the match will +// fail, so the symbol cache knows to use the new PDB, but the GUID matches, so +// it overwrites the existing item in the symbol cache rather than making a new +// one. +static Optional loadExistingBuildId(StringRef Path) { + // We don't need to incrementally update a previous build id if we're not + // writing codeview debug info. + if (!Config->Debug) + return None; + + auto ExpectedBinary = llvm::object::createBinary(Path); + if (!ExpectedBinary) { + consumeError(ExpectedBinary.takeError()); + return None; + } + + auto Binary = std::move(*ExpectedBinary); + if (!Binary.getBinary()->isCOFF()) + return None; + + std::error_code EC; + COFFObjectFile File(Binary.getBinary()->getMemoryBufferRef(), EC); + if (EC) + return None; + + // If the machine of the binary we're outputting doesn't match the machine + // of the existing binary, don't try to re-use the build id. + if (File.is64() != Config->is64() || File.getMachine() != Config->Machine) + return None; + + for (const auto &DebugDir : File.debug_directories()) { + if (DebugDir.Type != IMAGE_DEBUG_TYPE_CODEVIEW) + continue; + + const codeview::DebugInfo *ExistingDI = nullptr; + StringRef PDBFileName; + if (auto EC = File.getDebugPDBInfo(ExistingDI, PDBFileName)) + return None; + // We only support writing PDBs in v70 format. So if this is not a build + // id that we recognize / support, ignore it. + if (ExistingDI->Signature.CVSignature != OMF::Signature::PDB70) + return None; + return *ExistingDI; + } + return None; +} + // The main function of the writer. void Writer::run() { createSections(); @@ -232,6 +291,10 @@ removeEmptySections(); setSectionPermissions(); createSymbolAndStringTable(); + + // We must do this before opening the output file, as it depends on being able + // to read the contents of the existing output file. + PreviousBuildId = loadExistingBuildId(Config->OutputFile); openFile(Config->OutputFile); if (Config->is64()) { writeHeader(); @@ -244,10 +307,9 @@ writeBuildId(); if (!Config->PDBPath.empty() && Config->Debug) { - const llvm::codeview::DebugInfo *DI = nullptr; - if (Config->DebugTypes & static_cast(coff::DebugType::CV)) - DI = BuildId->DI; - createPDB(Symtab, OutputSections, SectionTable, DI); + + assert(BuildId); + createPDB(Symtab, OutputSections, SectionTable, *BuildId->BuildId); } writeMapFile(OutputSections); @@ -311,13 +373,13 @@ if (Config->Debug) { DebugDirectory = make(DebugRecords); - // TODO(compnerd) create a coffgrp entry if DebugType::CV is not enabled - if (Config->DebugTypes & static_cast(coff::DebugType::CV)) { - auto *Chunk = make(); - - BuildId = Chunk; - DebugRecords.push_back(Chunk); - } + // Make a CVDebugRecordChunk even when /DEBUG:CV is not specified. We + // output a PDB no matter what, and this chunk provides the only means of + // allowing a debugger to match a PDB and an executable. So we need it even + // if we're ultimately not going to write CodeView data to the PDB. + auto *CVChunk = make(); + BuildId = CVChunk; + DebugRecords.push_back(CVChunk); RData->addChunk(DebugDirectory); for (Chunk *C : DebugRecords) @@ -782,6 +844,25 @@ } } +void Writer::writeBuildId() { + // If we're not writing a build id (e.g. because /debug is not specified), + // then just return; + if (!Config->Debug) + return; + + assert(BuildId && "BuildId is not set!"); + + if (PreviousBuildId.hasValue()) { + *BuildId->BuildId = *PreviousBuildId; + BuildId->BuildId->PDB70.Age = BuildId->BuildId->PDB70.Age + 1; + return; + } + + BuildId->BuildId->Signature.CVSignature = OMF::Signature::PDB70; + BuildId->BuildId->PDB70.Age = 1; + llvm::getRandomBytes(BuildId->BuildId->PDB70.Signature, 16); +} + // Sort .pdata section contents according to PE/COFF spec 5.5. void Writer::sortExceptionTable() { OutputSection *Sec = findSection(".pdata"); @@ -805,26 +886,6 @@ errs() << "warning: don't know how to handle .pdata.\n"; } -// Backfill the CVSignature in a PDB70 Debug Record. This backfilling allows us -// to get reproducible builds. -void Writer::writeBuildId() { - // There is nothing to backfill if BuildId was not setup. - if (BuildId == nullptr) - return; - - assert(BuildId->DI->Signature.CVSignature == OMF::Signature::PDB70 && - "only PDB 7.0 is supported"); - assert(sizeof(BuildId->DI->PDB70.Signature) == 16 && - "signature size mismatch"); - - // Compute an MD5 hash. - ArrayRef Buf(Buffer->getBufferStart(), Buffer->getBufferEnd()); - memcpy(BuildId->DI->PDB70.Signature, MD5::hash(Buf).data(), 16); - - // TODO(compnerd) track the Age - BuildId->DI->PDB70.Age = 1; -} - OutputSection *Writer::findSection(StringRef Name) { for (OutputSection *Sec : OutputSections) if (Sec->getName() == Name) Index: lld/trunk/test/COFF/pdb-diff.test =================================================================== --- lld/trunk/test/COFF/pdb-diff.test +++ lld/trunk/test/COFF/pdb-diff.test @@ -4,8 +4,11 @@ file with LLD and compare the two PDBs. Since the baseline object file and PDB are already checked in, we just run LLD on the object file. -RUN: lld-link /debug /pdb:%T/pdb-diff-lld.pdb /nodefaultlib /entry:main %S/Inputs/pdb-diff.obj -RUN: llvm-pdbutil diff -result -values=false -left-bin-root=%S -right-bin-root=D:/src/llvm-mono/lld/test/COFF/ %T/pdb-diff-lld.pdb %S/Inputs/pdb-diff-cl.pdb | FileCheck %s +RUN: rm -f %T/pdb-diff-lld.pdb %T/pdb-diff-lld.exe +RUN: lld-link /debug /pdb:%T/pdb-diff-lld.pdb /out:%T/pdb-diff-lld.exe /nodefaultlib \ +RUN: /entry:main %S/Inputs/pdb-diff.obj +RUN: llvm-pdbutil diff -result -values=false -left-bin-root=%S -right-bin-root=D:/src/llvm-mono/lld/test/COFF/ \ +RUN: %T/pdb-diff-lld.pdb %S/Inputs/pdb-diff-cl.pdb | FileCheck %s CHECK: ---------------------- CHECK-NEXT: | MSF Super Block | Index: lld/trunk/test/COFF/pdb-none.test =================================================================== --- lld/trunk/test/COFF/pdb-none.test +++ lld/trunk/test/COFF/pdb-none.test @@ -1,12 +1,13 @@ # RUN: yaml2obj < %p/Inputs/pdb1.yaml > %t1.obj # RUN: yaml2obj < %p/Inputs/pdb2.yaml > %t2.obj +# RUN: rm -f %t.pdb %t.dll # RUN: lld-link /debug /debugtype:pdata /pdb:%t.pdb /dll /out:%t.dll /entry:main /nodefaultlib \ # RUN: %t1.obj %t2.obj # RUN: llvm-pdbutil pdb2yaml -pdb-stream %t.pdb | FileCheck %s # CHECK: PdbStream: -# CHECK-NEXT: Age: 0 +# CHECK-NEXT: Age: 1 # CHECK-NEXT: Guid: # CHECK-NEXT: Signature: # CHECK-NEXT: Features: [ VC140 ] Index: lld/trunk/test/COFF/pdb-source-lines.test =================================================================== --- lld/trunk/test/COFF/pdb-source-lines.test +++ lld/trunk/test/COFF/pdb-source-lines.test @@ -19,10 +19,11 @@ RUN: yaml2obj %S/Inputs/pdb_lines_1.yaml -o %t.pdb_lines_1.obj RUN: yaml2obj %S/Inputs/pdb_lines_2.yaml -o %t.pdb_lines_2.obj +RUN: rm -f %t.exe %t.pdb RUN: lld-link -debug -entry:main -nodefaultlib -out:%t.exe -pdb:%t.pdb %t.pdb_lines_1.obj %t.pdb_lines_2.obj RUN: llvm-pdbutil pdb2yaml -modules -module-files -subsections=lines,fc %t.pdb | FileCheck %s -CHECK-LABEL: DbiStream: +CHECK-LABEL: DbiStream: CHECK-NEXT: VerHeader: V70 CHECK-NEXT: Age: 1 CHECK-NEXT: BuildNumber: 0 @@ -30,22 +31,22 @@ CHECK-NEXT: PdbDllRbld: 0 CHECK-NEXT: Flags: 0 CHECK-NEXT: MachineType: x86 -CHECK-NEXT: Modules: +CHECK-NEXT: Modules: CHECK-LABEL: - Module: {{.*}}pdb_lines_1.obj CHECK-NEXT: ObjFile: {{.*}}pdb_lines_1.obj -CHECK-NEXT: SourceFiles: +CHECK-NEXT: SourceFiles: CHECK-NEXT: - '{{.*}}pdb_lines_1.c' CHECK-NEXT: - '{{.*}}foo.h' -CHECK-NEXT: Subsections: +CHECK-NEXT: Subsections: CHECK-NEXT: - !Lines CHECK-NEXT: CodeSize: 19 CHECK-NEXT: Flags: [ ] CHECK-NEXT: RelocOffset: 0 CHECK-NEXT: RelocSegment: 2 -CHECK-NEXT: Blocks: +CHECK-NEXT: Blocks: CHECK-NEXT: - FileName: '{{.*}}pdb_lines_1.c' -CHECK-NEXT: Lines: +CHECK-NEXT: Lines: CHECK-NEXT: - Offset: 0 CHECK-NEXT: LineStart: 2 CHECK-NEXT: IsStatement: true @@ -62,9 +63,9 @@ CHECK-NEXT: LineStart: 5 CHECK-NEXT: IsStatement: true CHECK-NEXT: EndDelta: 0 -CHECK-NEXT: Columns: +CHECK-NEXT: Columns: CHECK-NEXT: - !FileChecksums -CHECK-NEXT: Checksums: +CHECK-NEXT: Checksums: CHECK-NEXT: - FileName: '{{.*}}pdb_lines_1.c' CHECK-NEXT: Kind: MD5 CHECK-NEXT: Checksum: 4EB19DCD86C3BA2238A255C718572E7B @@ -76,9 +77,9 @@ CHECK-NEXT: Flags: [ ] CHECK-NEXT: RelocOffset: 32 CHECK-NEXT: RelocSegment: 2 -CHECK-NEXT: Blocks: +CHECK-NEXT: Blocks: CHECK-NEXT: - FileName: '{{.*}}foo.h' -CHECK-NEXT: Lines: +CHECK-NEXT: Lines: CHECK-NEXT: - Offset: 0 CHECK-NEXT: LineStart: 2 CHECK-NEXT: IsStatement: true @@ -91,21 +92,21 @@ CHECK-NEXT: LineStart: 4 CHECK-NEXT: IsStatement: true CHECK-NEXT: EndDelta: 0 -CHECK-NEXT: Columns: +CHECK-NEXT: Columns: CHECK-LABEL: - Module: {{.*}}pdb_lines_2.obj CHECK-NEXT: ObjFile: {{.*}}pdb_lines_2.obj -CHECK-NEXT: SourceFiles: +CHECK-NEXT: SourceFiles: CHECK-NEXT: - '{{.*}}pdb_lines_2.c' -CHECK-NEXT: Subsections: +CHECK-NEXT: Subsections: CHECK-NEXT: - !Lines CHECK-NEXT: CodeSize: 1 CHECK-NEXT: Flags: [ ] CHECK-NEXT: RelocOffset: 48 CHECK-NEXT: RelocSegment: 2 -CHECK-NEXT: Blocks: +CHECK-NEXT: Blocks: CHECK-NEXT: - FileName: '{{.*}}pdb_lines_2.c' -CHECK-NEXT: Lines: +CHECK-NEXT: Lines: CHECK-NEXT: - Offset: 0 CHECK-NEXT: LineStart: 1 CHECK-NEXT: IsStatement: true @@ -114,9 +115,9 @@ CHECK-NEXT: LineStart: 2 CHECK-NEXT: IsStatement: true CHECK-NEXT: EndDelta: 0 -CHECK-NEXT: Columns: +CHECK-NEXT: Columns: CHECK-NEXT: - !FileChecksums -CHECK-NEXT: Checksums: +CHECK-NEXT: Checksums: CHECK-NEXT: - FileName: '{{.*}}pdb_lines_2.c' CHECK-NEXT: Kind: MD5 CHECK-NEXT: Checksum: DF91CB3A2B8D917486574BB50CAC4CC7 Index: lld/trunk/test/COFF/pdb.test =================================================================== --- lld/trunk/test/COFF/pdb.test +++ lld/trunk/test/COFF/pdb.test @@ -1,5 +1,6 @@ # RUN: yaml2obj < %p/Inputs/pdb1.yaml > %t1.obj # RUN: yaml2obj < %p/Inputs/pdb2.yaml > %t2.obj +# RUN: rm -f %t.dll %t.pdb # RUN: lld-link /debug /pdb:%t.pdb /dll /out:%t.dll /entry:main /nodefaultlib \ # RUN: %t1.obj %t2.obj Index: lld/trunk/test/COFF/rsds.test =================================================================== --- lld/trunk/test/COFF/rsds.test +++ lld/trunk/test/COFF/rsds.test @@ -1,11 +1,20 @@ # RUN: yaml2obj %s > %t.obj +# RUN: rm -f %t.dll %t.pdb # RUN: lld-link /debug /dll /out:%t.dll /entry:DllMain %t.obj -# RUN: llvm-readobj -coff-debug-directory %t.dll | FileCheck %s +# RUN: llvm-readobj -coff-debug-directory %t.dll > %t.1.txt +# RUN: lld-link /debug /dll /out:%t.dll /entry:DllMain %t.obj +# RUN: llvm-readobj -coff-debug-directory %t.dll > %t.2.txt +# RUN: cat %t.1.txt %t.2.txt | FileCheck %s +# RUN: rm -f %t.dll %t.pdb +# RUN: lld-link /debug /pdb:%t.pdb /dll /out:%t.dll /entry:DllMain %t.obj +# RUN: llvm-readobj -coff-debug-directory %t.dll > %t.3.txt # RUN: lld-link /debug /pdb:%t.pdb /dll /out:%t.dll /entry:DllMain %t.obj -# RUN: llvm-readobj -coff-debug-directory %t.dll | FileCheck %s +# RUN: llvm-readobj -coff-debug-directory %t.dll > %t.4.txt +# RUN: cat %t.3.txt %t.4.txt | FileCheck %s +# CHECK: File: [[FILE:.*]].dll # CHECK: DebugDirectory [ # CHECK: DebugEntry { # CHECK: Characteristics: 0x0 @@ -18,12 +27,31 @@ # CHECK: PointerToRawData: 0x{{[^0]}} # CHECK: PDBInfo { # CHECK: PDBSignature: 0x53445352 -# CHECK: PDBGUID: +# CHECK: PDBGUID: [[GUID:\(([A-Za-z0-9]{2} ?){16}\)]] # CHECK: PDBAge: 1 # CHECK: PDBFileName: {{.*}}.pdb # CHECK: } # CHECK: } # CHECK: ] +# CHECK: File: [[FILE]].dll +# CHECK: DebugDirectory [ +# CHECK: DebugEntry { +# CHECK: Characteristics: 0x0 +# CHECK: TimeDateStamp: 1970-01-01 00:00:00 (0x0) +# CHECK: MajorVersion: 0x0 +# CHECK: MinorVersion: 0x0 +# CHECK: Type: CodeView (0x2) +# CHECK: SizeOfData: 0x{{[^0]}} +# CHECK: AddressOfRawData: 0x{{[^0]}} +# CHECK: PointerToRawData: 0x{{[^0]}} +# CHECK: PDBInfo { +# CHECK: PDBSignature: 0x53445352 +# CHECK: PDBGUID: [[GUID]] +# CHECK: PDBAge: 2 +# CHECK: PDBFileName: {{.*}}.pdb +# CHECK: } +# CHECK: } +# CHECK: ] --- !COFF header: