From 91a6bfed61631b23deabad9986ac29b74d7491ea Mon Sep 17 00:00:00 2001 From: Reid Kleckner Date: Wed, 15 Apr 2020 09:54:22 -0700 Subject: [PATCH] [COFF] Assign unique identifiers to ObjFiles from LTO Use the unique filenames that are used when /lldsavetemps is passed. After this change, module names for LTO blobs in PDBs will be unique. Visual Studio and probably other debuggers expect module names to be unique. Revert some changes from 1e0b158db (2017) that are no longer necessary after removing MSVC LTO support. Reviewed By: MaskRay Differential Revision: https://reviews.llvm.org/D78221 --- lld/COFF/LTO.cpp | 38 ++++++++++++++++++++++++-------------- lld/COFF/LTO.h | 2 +- lld/COFF/SymbolTable.cpp | 14 +++++--------- lld/COFF/SymbolTable.h | 1 - lld/test/COFF/lto-parallel.ll | 22 +++++++++++++++++----- lld/test/COFF/pdb-thinlto.ll | 38 ++++++++++++++++++++++++++++++++++++++ lld/test/COFF/thinlto.ll | 8 ++++---- lld/test/COFF/weak-external.test | 4 ++-- lld/test/COFF/weak-external3.test | 4 ++-- 9 files changed, 93 insertions(+), 38 deletions(-) create mode 100644 lld/test/COFF/pdb-thinlto.ll diff --git a/lld/COFF/LTO.cpp b/lld/COFF/LTO.cpp index 94906e5..c88ce0e 100644 --- a/lld/COFF/LTO.cpp +++ b/lld/COFF/LTO.cpp @@ -144,7 +144,7 @@ void BitcodeCompiler::add(BitcodeFile &f) { // Merge all the bitcode files we have seen, codegen the result // and return the resulting objects. -std::vector BitcodeCompiler::compile() { +std::vector BitcodeCompiler::compile() { unsigned maxTasks = ltoObj->getMaxTasks(); buf.resize(maxTasks); files.resize(maxTasks); @@ -188,22 +188,32 @@ std::vector BitcodeCompiler::compile() { if (!config->ltoCache.empty()) pruneCache(config->ltoCache, config->ltoCachePolicy); - std::vector ret; + std::vector ret; for (unsigned i = 0; i != maxTasks; ++i) { - if (buf[i].empty()) + // Assign unique names to LTO objects. This ensures they have unique names + // in the PDB if one is produced. The names should look like: + // - foo.exe.lto.obj + // - foo.exe.lto.1.obj + // - ... + StringRef ltoObjName = + saver.save(Twine(config->outputFile) + ".lto" + + (i == 0 ? Twine("") : Twine('.') + Twine(i)) + ".obj"); + + // Get the native object contents either from the cache or from memory. Do + // not use the cached MemoryBuffer directly, or the PDB will not be + // deterministic. + StringRef objBuf; + if (files[i]) + objBuf = files[i]->getBuffer(); + else + objBuf = buf[i]; + if (objBuf.empty()) continue; - if (config->saveTemps) { - if (i == 0) - saveBuffer(buf[i], config->outputFile + ".lto.obj"); - else - saveBuffer(buf[i], config->outputFile + Twine(i) + ".lto.obj"); - } - ret.emplace_back(buf[i].data(), buf[i].size()); - } - for (std::unique_ptr &file : files) - if (file) - ret.push_back(file->getBuffer()); + if (config->saveTemps) + saveBuffer(buf[i], ltoObjName); + ret.push_back(make(MemoryBufferRef(objBuf, ltoObjName))); + } return ret; } diff --git a/lld/COFF/LTO.h b/lld/COFF/LTO.h index 2a0cfa0..a2b321d 100644 --- a/lld/COFF/LTO.h +++ b/lld/COFF/LTO.h @@ -45,7 +45,7 @@ public: ~BitcodeCompiler(); void add(BitcodeFile &f); - std::vector compile(); + std::vector compile(); private: std::unique_ptr ltoObj; diff --git a/lld/COFF/SymbolTable.cpp b/lld/COFF/SymbolTable.cpp index 8c5556a..f846af8 100644 --- a/lld/COFF/SymbolTable.cpp +++ b/lld/COFF/SymbolTable.cpp @@ -789,20 +789,16 @@ Symbol *SymbolTable::addUndefined(StringRef name) { return addUndefined(name, nullptr, false); } -std::vector SymbolTable::compileBitcodeFiles() { - lto.reset(new BitcodeCompiler); - for (BitcodeFile *f : BitcodeFile::instances) - lto->add(*f); - return lto->compile(); -} - void SymbolTable::addCombinedLTOObjects() { if (BitcodeFile::instances.empty()) return; ScopedTimer t(ltoTimer); - for (StringRef object : compileBitcodeFiles()) { - auto *obj = make(MemoryBufferRef(object, "lto.tmp")); + lto.reset(new BitcodeCompiler); + for (BitcodeFile *f : BitcodeFile::instances) + lto->add(*f); + for (InputFile *newObj : lto->compile()) { + ObjFile *obj = cast(newObj); obj->parse(); ObjFile::instances.push_back(obj); } diff --git a/lld/COFF/SymbolTable.h b/lld/COFF/SymbolTable.h index cd8a53d..870a715 100644 --- a/lld/COFF/SymbolTable.h +++ b/lld/COFF/SymbolTable.h @@ -77,7 +77,6 @@ public: // BitcodeFiles and add them to the symbol table. Called after all files are // added and before the writer writes results to a file. void addCombinedLTOObjects(); - std::vector compileBitcodeFiles(); // Creates an Undefined symbol for a given name. Symbol *addUndefined(StringRef name); diff --git a/lld/test/COFF/lto-parallel.ll b/lld/test/COFF/lto-parallel.ll index 8e1f44a..3ba1126 100644 --- a/lld/test/COFF/lto-parallel.ll +++ b/lld/test/COFF/lto-parallel.ll @@ -1,23 +1,35 @@ ; REQUIRES: x86 ; RUN: llvm-as -o %t.obj %s -; RUN: lld-link -opt:noicf /out:%t.exe /entry:foo /include:bar /opt:lldltopartitions=2 /subsystem:console /lldmap:%t.map %t.obj +; RUN: lld-link -opt:noicf /out:%t.exe /entry:foo /include:bar /opt:lldltopartitions=2 /subsystem:console /lldmap:%t.map %t.obj /debug /pdb:%t.pdb ; RUN: FileCheck %s < %t.map +; RUN: llvm-pdbutil dump %t.pdb --modules | FileCheck %s --check-prefix=PDB target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" target triple = "x86_64-pc-windows-msvc" -; CHECK: lto.tmp -; CHECK: lto.tmp +; CHECK: lto-parallel.ll.tmp.exe.lto.obj: +; CHECK: lto-parallel.ll.tmp.exe.lto.obj: ; CHECK-NEXT: foo define void @foo() { call void @bar() ret void } -; CHECK: lto.tmp -; CHECK: lto.tmp +; CHECK: lto-parallel.ll.tmp.exe.lto.1.obj: +; CHECK: lto-parallel.ll.tmp.exe.lto.1.obj: ; CHECK: bar define void @bar() { call void @foo() ret void } + + +; Objects in the PDB should receive distinct names. + +; PDB: Modules +; PDB: Mod 0000 | `{{.*}}lto-parallel.ll.tmp.exe.lto.obj`: +; PDB: Obj: `{{.*}}lto-parallel.ll.tmp.exe.lto.obj`: +; PDB: Mod 0001 | `{{.*}}lto-parallel.ll.tmp.exe.lto.1.obj`: +; PDB: Obj: `{{.*}}lto-parallel.ll.tmp.exe.lto.1.obj`: +; PDB: Mod 0002 | `* Linker *`: +; PDB: Obj: ``: diff --git a/lld/test/COFF/pdb-thinlto.ll b/lld/test/COFF/pdb-thinlto.ll new file mode 100644 index 0000000..de0eef2 --- /dev/null +++ b/lld/test/COFF/pdb-thinlto.ll @@ -0,0 +1,38 @@ +; REQUIRES: x86 +; RUN: rm -rf %t && mkdir -p %t && cd %t +; RUN: opt -thinlto-bc -o main.bc %s +; RUN: opt -thinlto-bc -o foo.bc %S/Inputs/lto-dep.ll + +; Even if the native object is cached, the PDB must be the same. +; RUN: rm -rf thinltocachedir && mkdir thinltocachedir + +; RUN: lld-link /lldltocache:thinltocachedir /out:main.exe /entry:main /subsystem:console main.bc foo.bc /debug /pdb:main.pdb + +; RUN: llvm-pdbutil dump --modules main.pdb | FileCheck %s + +; Run again with the cache. Make sure we get the same object names. + +; RUN: lld-link /lldltocache:thinltocachedir /out:main.exe /entry:main /subsystem:console main.bc foo.bc /debug /pdb:main.pdb + +; RUN: llvm-pdbutil dump --modules main.pdb | FileCheck %s + + +target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-pc-windows-msvc" + +define i32 @main() { + call void @foo() + ret i32 0 +} + +declare void @foo() + +; CHECK: Modules +; CHECK: ============================================================ +; CHECK: Mod 0000 | `{{.*}}main.exe.lto.obj`: +; CHECK: Obj: `{{.*}}main.exe.lto.obj`: +; CHECK: Mod 0001 | `{{.*}}main.exe.lto.1.obj`: +; CHECK: Obj: `{{.*}}main.exe.lto.1.obj`: +; CHECK: Mod 0002 | `{{.*}}main.exe.lto.2.obj`: +; CHECK: Obj: `{{.*}}main.exe.lto.2.obj`: +; CHECK: Mod 0003 | `* Linker *`: diff --git a/lld/test/COFF/thinlto.ll b/lld/test/COFF/thinlto.ll index 8727ac0..1a3ba23 100644 --- a/lld/test/COFF/thinlto.ll +++ b/lld/test/COFF/thinlto.ll @@ -4,15 +4,15 @@ ; RUN: opt -thinlto-bc -o %T/thinlto/main.obj %s ; RUN: opt -thinlto-bc -o %T/thinlto/foo.obj %S/Inputs/lto-dep.ll ; RUN: lld-link /lldsavetemps /out:%T/thinlto/main.exe /entry:main /subsystem:console %T/thinlto/main.obj %T/thinlto/foo.obj -; RUN: llvm-nm %T/thinlto/main.exe1.lto.obj | FileCheck %s +; RUN: llvm-nm %T/thinlto/main.exe.lto.1.obj | FileCheck %s ; Test various possible options for /opt:lldltojobs ; RUN: lld-link /lldsavetemps /out:%T/thinlto/main.exe /entry:main /subsystem:console %T/thinlto/main.obj %T/thinlto/foo.obj /opt:lldltojobs=1 -; RUN: llvm-nm %T/thinlto/main.exe1.lto.obj | FileCheck %s +; RUN: llvm-nm %T/thinlto/main.exe.lto.1.obj | FileCheck %s ; RUN: lld-link /lldsavetemps /out:%T/thinlto/main.exe /entry:main /subsystem:console %T/thinlto/main.obj %T/thinlto/foo.obj /opt:lldltojobs=all -; RUN: llvm-nm %T/thinlto/main.exe1.lto.obj | FileCheck %s +; RUN: llvm-nm %T/thinlto/main.exe.lto.1.obj | FileCheck %s ; RUN: lld-link /lldsavetemps /out:%T/thinlto/main.exe /entry:main /subsystem:console %T/thinlto/main.obj %T/thinlto/foo.obj /opt:lldltojobs=100 -; RUN: llvm-nm %T/thinlto/main.exe1.lto.obj | FileCheck %s +; RUN: llvm-nm %T/thinlto/main.exe.lto.1.obj | FileCheck %s ; RUN: not lld-link /lldsavetemps /out:%T/thinlto/main.exe /entry:main /subsystem:console %T/thinlto/main.obj %T/thinlto/foo.obj /opt:lldltojobs=foo 2>&1 | FileCheck %s --check-prefix=BAD-JOBS ; BAD-JOBS: error: /opt:lldltojobs: invalid job count: foo diff --git a/lld/test/COFF/weak-external.test b/lld/test/COFF/weak-external.test index 352b14f..2d65011 100644 --- a/lld/test/COFF/weak-external.test +++ b/lld/test/COFF/weak-external.test @@ -5,8 +5,8 @@ # RUN: lld-link /out:%t2.exe /entry:g /subsystem:console /lldmap:%t2.map %t.obj %t.lto.obj # RUN: FileCheck %s < %t2.map -# CHECK: lto.tmp -# CHECK-NEXT: lto.tmp +# CHECK: .lto.obj: +# CHECK-NEXT: .lto.obj: # CHECK-NEXT: 0 g --- !COFF diff --git a/lld/test/COFF/weak-external3.test b/lld/test/COFF/weak-external3.test index 480b09b..8a4afdb 100644 --- a/lld/test/COFF/weak-external3.test +++ b/lld/test/COFF/weak-external3.test @@ -6,8 +6,8 @@ # RUN: lld-link /out:%t2.exe /entry:f /subsystem:console /lldmap:%t2.map %t.obj %t.lto.obj # RUN: FileCheck --check-prefix=CHECK2 %s < %t2.map -# CHECK1: lto.tmp -# CHECK1: lto.tmp +# CHECK1: .lto.obj: +# CHECK1-NEXT: .lto.obj: # CHECK1-NEXT: 0 g # CHECK2: weak-external3.test.tmp.obj -- 2.7.4