From c55f4fb8055591bee9ed577794299c0dd3ff791e Mon Sep 17 00:00:00 2001 From: Rafael Espindola Date: Fri, 4 Dec 2015 22:08:53 +0000 Subject: [PATCH] Always pass a diagnostic handler to the linker. Before this patch the diagnostic handler was optional. If it was not passed, the one in the LLVMContext was used. That is probably not a pattern we want to follow. If each area has an optional callback, there is a sea of callbacks and it is hard to follow which one is called. Doing this also found cases where the callback is a nice addition, like testing that no errors or warnings are reported. The other option is to always use the diagnostic handler in the LLVMContext. That has a few problems * To implement the C API we would have to set the diag handler and then set it back to the original value. * Code that creates the context might be far away from code that wants the diagnostics. I do have a patch that implements the second option and will send that as an RFC. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@254777 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Linker/Linker.h | 6 +++--- lib/LTO/LTOCodeGenerator.cpp | 10 ++++++---- lib/Linker/LinkModules.cpp | 10 ---------- tools/bugpoint/BugDriver.cpp | 9 ++++++++- tools/bugpoint/Miscompilation.cpp | 19 +++++++++++++++---- tools/gold/gold-plugin.cpp | 2 +- unittests/Linker/LinkModulesTest.cpp | 8 +++++--- 7 files changed, 38 insertions(+), 26 deletions(-) diff --git a/include/llvm/Linker/Linker.h b/include/llvm/Linker/Linker.h index f9890935126..f0c8ad979ab 100644 --- a/include/llvm/Linker/Linker.h +++ b/include/llvm/Linker/Linker.h @@ -69,7 +69,6 @@ public: }; Linker(Module &M, DiagnosticHandlerFunction DiagnosticHandler); - Linker(Module &M); /// \brief Link \p Src into the composite. The source is destroyed. /// @@ -88,8 +87,9 @@ public: DiagnosticHandlerFunction DiagnosticHandler, unsigned Flags = Flags::None); - static bool linkModules(Module &Dest, Module &Src, - unsigned Flags = Flags::None); + DiagnosticHandlerFunction getDiagnosticHandler() const { + return DiagnosticHandler; + } private: Module &Composite; diff --git a/lib/LTO/LTOCodeGenerator.cpp b/lib/LTO/LTOCodeGenerator.cpp index b0dae74c13d..25c150b2784 100644 --- a/lib/LTO/LTOCodeGenerator.cpp +++ b/lib/LTO/LTOCodeGenerator.cpp @@ -65,9 +65,10 @@ const char* LTOCodeGenerator::getVersionString() { } LTOCodeGenerator::LTOCodeGenerator(LLVMContext &Context) - : Context(Context), - MergedModule(new Module("ld-temp.o", Context)), - IRLinker(new Linker(*MergedModule)) { + : Context(Context), MergedModule(new Module("ld-temp.o", Context)), + IRLinker(new Linker(*MergedModule, [this](const DiagnosticInfo &DI) { + MergedModule->getContext().diagnose(DI); + })) { initializeLTOPasses(); } @@ -123,7 +124,8 @@ void LTOCodeGenerator::setModule(std::unique_ptr Mod) { AsmUndefinedRefs.clear(); MergedModule = Mod->takeModule(); - IRLinker = make_unique(*MergedModule); + IRLinker = + make_unique(*MergedModule, IRLinker->getDiagnosticHandler()); const std::vector &Undefs = Mod->getAsmUndefinedRefs(); for (int I = 0, E = Undefs.size(); I != E; ++I) diff --git a/lib/Linker/LinkModules.cpp b/lib/Linker/LinkModules.cpp index 55ab1824740..88b8e443c48 100644 --- a/lib/Linker/LinkModules.cpp +++ b/lib/Linker/LinkModules.cpp @@ -2030,11 +2030,6 @@ Linker::Linker(Module &M, DiagnosticHandlerFunction DiagnosticHandler) } } -Linker::Linker(Module &M) - : Linker(M, [this](const DiagnosticInfo &DI) { - Composite.getContext().diagnose(DI); - }) {} - bool Linker::linkInModule(Module &Src, unsigned Flags, const FunctionInfoIndex *Index, DenseSet *FunctionsToImport) { @@ -2061,11 +2056,6 @@ bool Linker::linkModules(Module &Dest, Module &Src, return L.linkInModule(Src, Flags); } -bool Linker::linkModules(Module &Dest, Module &Src, unsigned Flags) { - Linker L(Dest); - return L.linkInModule(Src, Flags); -} - //===----------------------------------------------------------------------===// // C API. //===----------------------------------------------------------------------===// diff --git a/tools/bugpoint/BugDriver.cpp b/tools/bugpoint/BugDriver.cpp index 39887d5d59d..9edc242d470 100644 --- a/tools/bugpoint/BugDriver.cpp +++ b/tools/bugpoint/BugDriver.cpp @@ -15,6 +15,7 @@ #include "BugDriver.h" #include "ToolRunner.h" +#include "llvm/IR/DiagnosticPrinter.h" #include "llvm/IR/Module.h" #include "llvm/IR/Verifier.h" #include "llvm/IRReader/IRReader.h" @@ -112,6 +113,12 @@ std::unique_ptr llvm::parseInputFile(StringRef Filename, return Result; } +static void diagnosticHandler(const DiagnosticInfo &DI) { + DiagnosticPrinterRawOStream DP(errs()); + DI.print(DP); + errs() << '\n'; +} + // This method takes the specified list of LLVM input files, attempts to load // them, either as assembly or bitcode, then link them together. It returns // true on failure (if, for example, an input bitcode file could not be @@ -132,7 +139,7 @@ bool BugDriver::addSources(const std::vector &Filenames) { if (!M.get()) return true; outs() << "Linking in input file: '" << Filenames[i] << "'\n"; - if (Linker::linkModules(*Program, *M)) + if (Linker::linkModules(*Program, *M, diagnosticHandler)) return true; } diff --git a/tools/bugpoint/Miscompilation.cpp b/tools/bugpoint/Miscompilation.cpp index e7eae40ec95..0b61b096985 100644 --- a/tools/bugpoint/Miscompilation.cpp +++ b/tools/bugpoint/Miscompilation.cpp @@ -18,6 +18,7 @@ #include "llvm/Config/config.h" // for HAVE_LINK_R #include "llvm/IR/Constants.h" #include "llvm/IR/DerivedTypes.h" +#include "llvm/IR/DiagnosticPrinter.h" #include "llvm/IR/Instructions.h" #include "llvm/IR/Module.h" #include "llvm/IR/Verifier.h" @@ -207,6 +208,14 @@ namespace { }; } +static void diagnosticHandler(const DiagnosticInfo &DI) { + DiagnosticPrinterRawOStream DP(errs()); + DI.print(DP); + errs() << '\n'; + if (DI.getSeverity() == DS_Error) + exit(1); +} + /// TestMergedProgram - Given two modules, link them together and run the /// program, checking to see if the program matches the diff. If there is /// an error, return NULL. If not, return the merged module. The Broken argument @@ -222,7 +231,7 @@ static Module *TestMergedProgram(const BugDriver &BD, Module *M1, Module *M2, M1 = CloneModule(M1); M2 = CloneModule(M2); } - if (Linker::linkModules(*M1, *M2)) + if (Linker::linkModules(*M1, *M2, diagnosticHandler)) exit(1); delete M2; // We are done with this module. @@ -390,7 +399,8 @@ static bool ExtractLoops(BugDriver &BD, MisCompFunctions.emplace_back(F->getName(), F->getFunctionType()); } - if (Linker::linkModules(*ToNotOptimize, *ToOptimizeLoopExtracted)) + if (Linker::linkModules(*ToNotOptimize, *ToOptimizeLoopExtracted, + diagnosticHandler)) exit(1); MiscompiledFunctions.clear(); @@ -418,7 +428,8 @@ static bool ExtractLoops(BugDriver &BD, // extraction both didn't break the program, and didn't mask the problem. // Replace the current program with the loop extracted version, and try to // extract another loop. - if (Linker::linkModules(*ToNotOptimize, *ToOptimizeLoopExtracted)) + if (Linker::linkModules(*ToNotOptimize, *ToOptimizeLoopExtracted, + diagnosticHandler)) exit(1); delete ToOptimizeLoopExtracted; @@ -594,7 +605,7 @@ static bool ExtractBlocks(BugDriver &BD, if (!I->isDeclaration()) MisCompFunctions.emplace_back(I->getName(), I->getFunctionType()); - if (Linker::linkModules(*ProgClone, *Extracted)) + if (Linker::linkModules(*ProgClone, *Extracted, diagnosticHandler)) exit(1); // Set the new program and delete the old one. diff --git a/tools/gold/gold-plugin.cpp b/tools/gold/gold-plugin.cpp index 1bd2f8afb29..8eacdc3ff23 100644 --- a/tools/gold/gold-plugin.cpp +++ b/tools/gold/gold-plugin.cpp @@ -938,7 +938,7 @@ static ld_plugin_status allSymbolsReadHook(raw_fd_ostream *ApiFile) { } std::unique_ptr Combined(new Module("ld-temp.o", Context)); - Linker L(*Combined); + Linker L(*Combined, diagnosticHandler); std::string DefaultTriple = sys::getDefaultTargetTriple(); diff --git a/unittests/Linker/LinkModulesTest.cpp b/unittests/Linker/LinkModulesTest.cpp index 4eba718e266..e56a692125e 100644 --- a/unittests/Linker/LinkModulesTest.cpp +++ b/unittests/Linker/LinkModulesTest.cpp @@ -71,6 +71,8 @@ protected: BasicBlock *ExitBB; }; +static void expectNoDiags(const DiagnosticInfo &DI) { EXPECT_TRUE(false); } + TEST_F(LinkModuleTest, BlockAddress) { IRBuilder<> Builder(EntryBB); @@ -93,7 +95,7 @@ TEST_F(LinkModuleTest, BlockAddress) { Builder.CreateRet(ConstantPointerNull::get(Type::getInt8PtrTy(Ctx))); Module *LinkedModule = new Module("MyModuleLinked", Ctx); - Linker::linkModules(*LinkedModule, *M); + Linker::linkModules(*LinkedModule, *M, expectNoDiags); // Delete the original module. M.reset(); @@ -169,13 +171,13 @@ static Module *getInternal(LLVMContext &Ctx) { TEST_F(LinkModuleTest, EmptyModule) { std::unique_ptr InternalM(getInternal(Ctx)); std::unique_ptr EmptyM(new Module("EmptyModule1", Ctx)); - Linker::linkModules(*EmptyM, *InternalM); + Linker::linkModules(*EmptyM, *InternalM, expectNoDiags); } TEST_F(LinkModuleTest, EmptyModule2) { std::unique_ptr InternalM(getInternal(Ctx)); std::unique_ptr EmptyM(new Module("EmptyModule1", Ctx)); - Linker::linkModules(*InternalM, *EmptyM); + Linker::linkModules(*InternalM, *EmptyM, expectNoDiags); } TEST_F(LinkModuleTest, TypeMerge) { -- 2.34.1