[lld] r307422 - Fix some differences between lld and MSVC generated PDBs.
Zachary Turner via llvm-commits
llvm-commits at lists.llvm.org
Fri Jul 7 11:45:56 PDT 2017
Author: zturner
Date: Fri Jul 7 11:45:56 2017
New Revision: 307422
URL: http://llvm.org/viewvc/llvm-project?rev=307422&view=rev
Log:
Fix some differences between lld and MSVC generated PDBs.
A couple of things were different about our generated PDBs.
1) We were outputting the wrong Version on the PDB Stream.
The version we were setting was newer than what MSVC is setting.
It's not clear what the implications are, but we change LLD
to use PdbImplVC70, as MSVC does.
2) For the optional debug stream indices in the DBI Stream, we
were outputting 0 to mean "the stream is not present". MSVC
outputs uint16_t(-1), which is the "correct" way to specify
that a stream is not present. So we fix that as well.
3) We were setting the PDB Stream signature to 0. This is supposed
to be the result of calling time(nullptr). Although this leads
to non-deterministic builds, a better way to solve that is by
having a command line option explicitly for generating a
reproducible build, and have the default behavior of lld-link
match the default behavior of link.
To test this, I'm making use of the new and improved `pdb diff`
sub command. To make it suitable for writing tests against, I had
to modify the diff subcommand slightly to print less verbose output.
Previously it would always print | <column> | <value1> | <value2> |
which is quite verbose, and the values are fragile. All we really
want to know is "did we produce the same value as link?" So I added
command line options to print a single character representing the
result status (different, identical, equivalent), and another to
hide the value display. Note that just inspecting the diff output
used to write the test, you can see some things that are obviously
wrong. That is just reflective of the fact that this is the state
of affairs today, not that we're asserting that this is "correct".
We can use this as a starting point to discover differences, fix
them, and update the test.
Differential Revision: https://reviews.llvm.org/D35086
Added:
lld/trunk/test/COFF/Inputs/pdb-diff-cl.pdb (with props)
lld/trunk/test/COFF/Inputs/pdb-diff.cpp
lld/trunk/test/COFF/Inputs/pdb-diff.obj (with props)
lld/trunk/test/COFF/pdb-diff.test
Modified:
lld/trunk/COFF/PDB.cpp
lld/trunk/test/COFF/pdb-source-lines.test
lld/trunk/test/COFF/pdb.test
Modified: lld/trunk/COFF/PDB.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/PDB.cpp?rev=307422&r1=307421&r2=307422&view=diff
==============================================================================
--- lld/trunk/COFF/PDB.cpp (original)
+++ lld/trunk/COFF/PDB.cpp Fri Jul 7 11:45:56 2017
@@ -410,6 +410,7 @@ void coff::createPDB(StringRef Path, Sym
// Add an Info stream.
auto &InfoBuilder = Builder.getInfoBuilder();
InfoBuilder.setAge(DI ? DI->PDB70.Age : 0);
+ InfoBuilder.setSignature(time(nullptr));
llvm::SmallString<128> NativePath(Path.begin(), Path.end());
llvm::sys::fs::make_absolute(NativePath);
@@ -425,7 +426,7 @@ void coff::createPDB(StringRef Path, Sym
// Add an empty DBI stream.
pdb::DbiStreamBuilder &DbiBuilder = Builder.getDbiBuilder();
- DbiBuilder.setVersionHeader(pdb::PdbDbiV110);
+ DbiBuilder.setVersionHeader(pdb::PdbDbiV70);
// It's not entirely clear what this is, but the * Linker * module uses it.
uint32_t PdbFilePathNI = DbiBuilder.addECName(NativePath);
Added: lld/trunk/test/COFF/Inputs/pdb-diff-cl.pdb
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/Inputs/pdb-diff-cl.pdb?rev=307422&view=auto
==============================================================================
Binary file - no diff available.
Propchange: lld/trunk/test/COFF/Inputs/pdb-diff-cl.pdb
------------------------------------------------------------------------------
svn:mime-type = application/octet-stream
Added: lld/trunk/test/COFF/Inputs/pdb-diff.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/Inputs/pdb-diff.cpp?rev=307422&view=auto
==============================================================================
--- lld/trunk/test/COFF/Inputs/pdb-diff.cpp (added)
+++ lld/trunk/test/COFF/Inputs/pdb-diff.cpp Fri Jul 7 11:45:56 2017
@@ -0,0 +1,10 @@
+// Build with cl:
+// cl.exe /Z7 pdb-diff.cpp /link /debug /pdb:pdb-diff-cl.pdb
+// /nodefaultlib /entry:main
+// Build with lld (after running the above cl command):
+// lld-link.exe /debug /pdb:pdb-diff-lld.pdb /nodefaultlib
+// /entry:main pdb-diff.obj
+
+void *__purecall = 0;
+
+int main() { return 42; }
Added: lld/trunk/test/COFF/Inputs/pdb-diff.obj
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/Inputs/pdb-diff.obj?rev=307422&view=auto
==============================================================================
Binary file - no diff available.
Propchange: lld/trunk/test/COFF/Inputs/pdb-diff.obj
------------------------------------------------------------------------------
svn:mime-type = application/octet-stream
Added: lld/trunk/test/COFF/pdb-diff.test
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/pdb-diff.test?rev=307422&view=auto
==============================================================================
--- lld/trunk/test/COFF/pdb-diff.test (added)
+++ lld/trunk/test/COFF/pdb-diff.test Fri Jul 7 11:45:56 2017
@@ -0,0 +1,200 @@
+This test verifies that we produce PDBs compatible with MSVC in various ways.
+We check in a cl-generated object file, PDB, and original source which serve
+as the "baseline" for us to measure against. Then we link the same object
+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 %T/pdb-diff-lld.pdb %S/Inputs/pdb-diff-cl.pdb | FileCheck %s
+
+CHECK: ----------------------
+CHECK-NEXT: | MSF Super Block |
+CHECK-NEXT: |----------------+---|
+CHECK-NEXT: | File | |
+CHECK-NEXT: |----------------+---|
+CHECK-NEXT: | Block Size | I |
+CHECK-NEXT: |----------------+---|
+CHECK-NEXT: | Block Count |
+CHECK-NEXT: |----------------+---|
+CHECK-NEXT: | Unknown 1 | I |
+CHECK-NEXT: |----------------+---|
+CHECK-NEXT: | Directory Size |
+CHECK-NEXT: |----------------+---|
+CHECK-NEXT: ------------------------------------
+CHECK-NEXT: | Stream Directory |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: | File | |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: | Stream Count | D |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: | Old MSF Directory | I |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: | PDB Stream | I |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: | TPI Stream | I |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: | DBI Stream | I |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: | IPI Stream | I |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: | Section Header Data | {{[EI]}} |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: | Named Stream "/names" | {{[EI]}} |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: | Named Stream "/LinkInfo" | {{[EI]}} |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: | Named Stream "/src/heade..." | D |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: | Module "{{.*}}pdb-diff.obj" | D |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: | Module "* Linker *" | {{[EI]}} |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: | TPI Hash | {{[EI]}} |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: | IPI Hash | {{[EI]}} |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: | Global Symbol Hash | D |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: | Public Symbol Hash | D |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: | Public Symbol Records | D |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: | Module "{{.*}}pdb-diff.obj" | D |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: | New FPO Data | D |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: ------------------------------------
+CHECK-NEXT: | String Table |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: | File | |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: | Number of Strings | D |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: | Hash Version | I |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: | Byte Size |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: | Signature | I |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: | Empty Strings |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: | {{.*}}pdb-diff.cpp | {{[EI]}} |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: | $T0 $ebp = $...p $T0 8 + = | D |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: | d:\src\llvm-...er internal) | D |
+CHECK-NEXT: |------------------------------+---|
+CHECK-NEXT: ----------------------------
+CHECK-NEXT: | PDB Stream |
+CHECK-NEXT: |----------------------+---|
+CHECK-NEXT: | File | |
+CHECK-NEXT: |----------------------+---|
+CHECK-NEXT: | Stream Size |
+CHECK-NEXT: |----------------------+---|
+CHECK-NEXT: | Age | I |
+CHECK-NEXT: |----------------------+---|
+CHECK-NEXT: | Guid | D |
+CHECK-NEXT: |----------------------+---|
+CHECK-NEXT: | Signature | D |
+CHECK-NEXT: |----------------------+---|
+CHECK-NEXT: | Version | I |
+CHECK-NEXT: |----------------------+---|
+CHECK-NEXT: | Features (set) | I |
+CHECK-NEXT: |----------------------+---|
+CHECK-NEXT: | Feature | I |
+CHECK-NEXT: |----------------------+---|
+CHECK-NEXT: | Named Stream Size | D |
+CHECK-NEXT: |----------------------+---|
+CHECK-NEXT: | Named Streams (map) | D |
+CHECK-NEXT: |----------------------+---|
+CHECK-NEXT: | /names | I |
+CHECK-NEXT: |----------------------+---|
+CHECK-NEXT: | /LinkInfo | E |
+CHECK-NEXT: |----------------------+---|
+CHECK-NEXT: | /src/headerblock | D |
+CHECK-NEXT: |----------------------+---|
+CHECK-NEXT: ----------------------------------------------
+CHECK-NEXT: | DBI Stream |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | File | |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | Dbi Version | I |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | Age | I |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | Machine | I |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | Flags | D |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | Build Major | D |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | Build Minor | D |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | Build Number | D |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | PDB DLL Version | D |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | PDB DLL RBLD | I |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | DBG (FPO) | I |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | DBG (Exception) | I |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | DBG (Fixup) | I |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | DBG (OmapToSrc) | I |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | DBG (OmapFromSrc) | I |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | DBG (SectionHdr) | {{[EI]}} |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | DBG (TokenRidMap) | I |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | DBG (Xdata) | I |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | DBG (Pdata) | I |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | DBG (NewFPO) | D |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | DBG (SectionHdrOrig) | I |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | Globals Stream | D |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | Publics Stream | D |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | Symbol Records | D |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | Has CTypes | I |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | Is Incrementally Linked | D |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | Is Stripped | I |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | Module Count | I |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | Source File Count | I |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: |Module "{{.*}}Inputs/pdb-diff.obj"|
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | - Modi | D |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | Module "* Linker *" |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | - Modi | I |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | - Obj File Name | I |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | - Debug Stream | {{[EI]}} |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | - C11 Byte Size | I |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | - C13 Byte Size | I |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | - # of files | I |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | - Pdb File Path Index | {{[EI]}} |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | - Source File Name Index | {{[EI]}} |
+CHECK-NEXT: |----------------------------------------+---|
+CHECK-NEXT: | - Symbol Byte Size | D |
+CHECK-NEXT: |----------------------------------------+---|
Modified: lld/trunk/test/COFF/pdb-source-lines.test
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/pdb-source-lines.test?rev=307422&r1=307421&r2=307422&view=diff
==============================================================================
--- lld/trunk/test/COFF/pdb-source-lines.test (original)
+++ lld/trunk/test/COFF/pdb-source-lines.test Fri Jul 7 11:45:56 2017
@@ -23,7 +23,7 @@ RUN: lld-link -debug -entry:main -nodefa
RUN: llvm-pdbutil pdb2yaml -modules -module-files -subsections=lines,fc %t.pdb | FileCheck %s
CHECK-LABEL: DbiStream:
-CHECK-NEXT: VerHeader: V110
+CHECK-NEXT: VerHeader: V70
CHECK-NEXT: Age: 1
CHECK-NEXT: BuildNumber: 0
CHECK-NEXT: PdbDllVersion: 0
Modified: lld/trunk/test/COFF/pdb.test
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/pdb.test?rev=307422&r1=307421&r2=307422&view=diff
==============================================================================
--- lld/trunk/test/COFF/pdb.test (original)
+++ lld/trunk/test/COFF/pdb.test Fri Jul 7 11:45:56 2017
@@ -30,7 +30,7 @@
# CHECK-NEXT: Features: [ VC140 ]
# CHECK-NEXT: Version: VC70
# CHECK-NEXT: DbiStream:
-# CHECK-NEXT: VerHeader: V110
+# CHECK-NEXT: VerHeader: V70
# CHECK-NEXT: Age: 1
# CHECK-NEXT: BuildNumber: 0
# CHECK-NEXT: PdbDllVersion: 0
More information about the llvm-commits
mailing list