Index: include/llvm/Linker/IRMover.h =================================================================== --- include/llvm/Linker/IRMover.h +++ include/llvm/Linker/IRMover.h @@ -15,6 +15,7 @@ #include namespace llvm { +class Error; class GlobalValue; class Metadata; class Module; @@ -22,6 +23,13 @@ class TrackingMDRef; class Type; +enum class irmover_error { + appending = 1, + module_flags, +}; + +const std::error_category &irmover_category(); + class IRMover { struct StructTypeKeyInfo { struct KeyTy { @@ -70,10 +78,8 @@ /// not present in ValuesToLink. The GlobalValue and a ValueAdder callback /// are passed as an argument, and the callback is expected to be called /// if the GlobalValue needs to be added to the \p ValuesToLink and linked. - /// - /// Returns true on error. - bool move(std::unique_ptr Src, ArrayRef ValuesToLink, - std::function AddLazyFor); + Error move(std::unique_ptr Src, ArrayRef ValuesToLink, + std::function AddLazyFor); Module &getModule() { return Composite; } private: Index: lib/Linker/IRMover.cpp =================================================================== --- lib/Linker/IRMover.cpp +++ lib/Linker/IRMover.cpp @@ -17,9 +17,40 @@ #include "llvm/IR/DiagnosticPrinter.h" #include "llvm/IR/GVMaterializer.h" #include "llvm/IR/TypeFinder.h" +#include "llvm/Support/Error.h" +#include "llvm/Support/ManagedStatic.h" #include "llvm/Transforms/Utils/Cloning.h" using namespace llvm; +namespace { + +// FIXME: This class is only here to support the transition to llvm::Error. It +// will be removed once this transition is complete. Clients should prefer to +// deal with the Error value directly, rather than converting to error_code. +class irmover_error_category : public std::error_category { + const char *name() const LLVM_NOEXCEPT override { + return "llvm.irmover"; + } + std::string message(int IE) const override { + irmover_error E = static_cast(IE); + switch (E) { + case irmover_error::appending: + return "Error linking variables with appending linkage"; + case irmover_error::module_flags: + return "Module flags mismatch"; + } + llvm_unreachable("invalid irmover_error"); + } +}; + +static ManagedStatic error_category; + +} // end anonymous namespace + +const std::error_category &llvm::irmover_category() { + return *error_category; +} + //===----------------------------------------------------------------------===// // TypeMap implementation. //===----------------------------------------------------------------------===// @@ -401,8 +432,6 @@ /// references. bool DoneLinkingBodies = false; - bool HasError = false; - /// Entry point for mapping values and alternate context for mapping aliases. ValueMapper Mapper; unsigned AliasMCID; @@ -412,12 +441,13 @@ GlobalValue *copyGlobalValueProto(const GlobalValue *SGV, bool ForDefinition); /// Helper method for setting a message and returning an error code. - bool emitError(const Twine &Message) { + void emitError(irmover_error E, const Twine &Message) { SrcM->getContext().diagnose(LinkDiagnosticInfo(DS_Error, Message)); - HasError = true; - return true; + ErrorCode = std::error_code(static_cast(E), irmover_category()); } + std::error_code ErrorCode; + void emitWarning(const Twine &Message) { SrcM->getContext().diagnose(LinkDiagnosticInfo(DS_Warning, Message)); } @@ -457,7 +487,7 @@ bool shouldLink(GlobalValue *DGV, GlobalValue &SGV); Constant *linkGlobalValueProto(GlobalValue *GV, bool ForAlias); - bool linkModuleFlagsMetadata(); + void linkModuleFlagsMetadata(); void linkGlobalInit(GlobalVariable &Dst, GlobalVariable &Src); bool linkFunctionBody(Function &Dst, Function &Src); @@ -489,7 +519,7 @@ } ~IRLinker() { SharedMDs = std::move(*ValueMap.getMDMap()); } - bool run(); + std::error_code run(); Value *materializeDeclFor(Value *V, bool ForAlias); void materializeInitFor(GlobalValue *New, GlobalValue *Old, bool ForAlias); }; @@ -752,42 +782,49 @@ DstNumElements = DstTy->getNumElements(); if (!SrcGV->hasAppendingLinkage() || !DstGV->hasAppendingLinkage()) { - emitError( - "Linking globals named '" + SrcGV->getName() + - "': can only link appending global with another appending global!"); + emitError(irmover_error::appending, + "Linking globals named '" + SrcGV->getName() + + "': can only link appending global with another appending " + "global!"); return nullptr; } // Check to see that they two arrays agree on type. if (EltTy != DstTy->getElementType()) { - emitError("Appending variables with different element types!"); + emitError(irmover_error::appending, + "Appending variables with different element types!"); return nullptr; } if (DstGV->isConstant() != SrcGV->isConstant()) { - emitError("Appending variables linked with different const'ness!"); + emitError(irmover_error::appending, + "Appending variables linked with different const'ness!"); return nullptr; } if (DstGV->getAlignment() != SrcGV->getAlignment()) { emitError( + irmover_error::appending, "Appending variables with different alignment need to be linked!"); return nullptr; } if (DstGV->getVisibility() != SrcGV->getVisibility()) { emitError( + irmover_error::appending, "Appending variables with different visibility need to be linked!"); return nullptr; } if (DstGV->hasUnnamedAddr() != SrcGV->hasUnnamedAddr()) { emitError( + irmover_error::appending, "Appending variables with different unnamed_addr need to be linked!"); return nullptr; } if (DstGV->getSection() != SrcGV->getSection()) { emitError( + irmover_error::appending, "Appending variables with different section name need to be linked!"); return nullptr; } @@ -943,8 +980,11 @@ assert(Dst.isDeclaration() && !Src.isDeclaration()); // Materialize if needed. - if (std::error_code EC = Src.materialize()) - return emitError(EC.message()); + if (std::error_code EC = Src.materialize()) { + SrcM->getContext().diagnose(LinkDiagnosticInfo(DS_Error, EC.message())); + ErrorCode = EC; + return true; + } // Link in the operands without remapping. if (Src.hasPrefixData()) @@ -999,11 +1039,11 @@ } /// Merge the linker flags in Src into the Dest module. -bool IRLinker::linkModuleFlagsMetadata() { +void IRLinker::linkModuleFlagsMetadata() { // If the source module has no module flags, we are done. const NamedMDNode *SrcModFlags = SrcM->getModuleFlagsMetadata(); if (!SrcModFlags) - return false; + return; // If the destination module doesn't have module flags yet, then just copy // over the source module's flags. @@ -1012,7 +1052,7 @@ for (unsigned I = 0, E = SrcModFlags->getNumOperands(); I != E; ++I) DstModFlags->addOperand(SrcModFlags->getOperand(I)); - return false; + return; } // First build a map of the existing module flags and requirements. @@ -1069,8 +1109,9 @@ // Diagnose inconsistent flags which both have override behavior. if (SrcBehaviorValue == Module::Override && SrcOp->getOperand(2) != DstOp->getOperand(2)) { - emitError("linking module flags '" + ID->getString() + - "': IDs have conflicting override values"); + emitError(irmover_error::module_flags, + "linking module flags '" + ID->getString() + + "': IDs have conflicting override values"); } continue; } else if (SrcBehaviorValue == Module::Override) { @@ -1082,8 +1123,9 @@ // Diagnose inconsistent merge behavior types. if (SrcBehaviorValue != DstBehaviorValue) { - emitError("linking module flags '" + ID->getString() + - "': IDs have conflicting behaviors"); + emitError(irmover_error::module_flags, + "linking module flags '" + ID->getString() + + "': IDs have conflicting behaviors"); continue; } @@ -1102,8 +1144,9 @@ case Module::Error: { // Emit an error if the values differ. if (SrcOp->getOperand(2) != DstOp->getOperand(2)) { - emitError("linking module flags '" + ID->getString() + - "': IDs have conflicting values"); + emitError(irmover_error::module_flags, + "linking module flags '" + ID->getString() + + "': IDs have conflicting values"); } continue; } @@ -1148,13 +1191,12 @@ MDNode *Op = Flags[Flag].first; if (!Op || Op->getOperand(2) != ReqValue) { - emitError("linking module flags '" + Flag->getString() + - "': does not have the required value"); + emitError(irmover_error::module_flags, + "linking module flags '" + Flag->getString() + + "': does not have the required value"); continue; } } - - return HasError; } // This function returns true if the triples match. @@ -1178,10 +1220,11 @@ return DstTriple.str(); } -bool IRLinker::run() { +std::error_code IRLinker::run() { // Ensure metadata materialized before value mapping. - if (SrcM->getMaterializer() && SrcM->getMaterializer()->materializeMetadata()) - return true; + if (SrcM->getMaterializer()) + if (std::error_code EC = SrcM->getMaterializer()->materializeMetadata()) + return EC; // Inherit the target data from the source module if the destination module // doesn't have one already. @@ -1235,8 +1278,8 @@ assert(!GV->isDeclaration()); Mapper.mapValue(*GV); - if (HasError) - return true; + if (ErrorCode) + return ErrorCode; } // Note that we are done linking global value bodies. This prevents @@ -1250,10 +1293,9 @@ linkNamedMDNodes(); // Merge the module flags into the DstM module. - if (linkModuleFlagsMetadata()) - return true; + linkModuleFlagsMetadata(); - return false; + return ErrorCode; } IRMover::StructTypeKeyInfo::KeyTy::KeyTy(ArrayRef E, bool P) @@ -1357,12 +1399,12 @@ } } -bool IRMover::move( +Error IRMover::move( std::unique_ptr Src, ArrayRef ValuesToLink, std::function AddLazyFor) { IRLinker TheIRLinker(Composite, SharedMDs, IdentifiedStructTypes, std::move(Src), ValuesToLink, AddLazyFor); - bool RetCode = TheIRLinker.run(); + std::error_code EC = TheIRLinker.run(); Composite.dropTriviallyDeadConstantArrays(); - return RetCode; + return errorCodeToError(EC); } Index: lib/Linker/LinkModules.cpp =================================================================== --- lib/Linker/LinkModules.cpp +++ lib/Linker/LinkModules.cpp @@ -18,6 +18,7 @@ #include "llvm/IR/DiagnosticPrinter.h" #include "llvm/IR/LLVMContext.h" #include "llvm/Linker/Linker.h" +#include "llvm/Support/Error.h" #include "llvm/Transforms/Utils/FunctionImportUtils.h" using namespace llvm; @@ -574,11 +575,24 @@ Internalize.insert(GV->getName()); } - if (Mover.move(std::move(SrcM), ValuesToLink.getArrayRef(), - [this](GlobalValue &GV, IRMover::ValueAdder Add) { - addLazyFor(GV, Add); - })) + // FIXME: Once we no longer need to be able to losslessly convert from + // llvm::Error to std::error_code, we should teach IRMover to encode error + // messages directly as Errors and return them here. Until then, an Error from + // IRMover causes us to return true here, and we rely on IRMover to send a + // more descriptive error message to the DiagnosticHandler. + bool HasErrors = false; + if (Error E = Mover.move(std::move(SrcM), ValuesToLink.getArrayRef(), + [this](GlobalValue &GV, IRMover::ValueAdder Add) { + addLazyFor(GV, Add); + })) { + handleAllErrors(std::move(E), [&](ErrorInfoBase &EIB) { + HasErrors = true; + return Error::success(); + }); + } + if (HasErrors) return true; + for (auto &P : Internalize) { GlobalValue *GV = DstM.getNamedValue(P.first()); GV->setLinkage(GlobalValue::InternalLinkage); Index: tools/gold/gold-plugin.cpp =================================================================== --- tools/gold/gold-plugin.cpp +++ tools/gold/gold-plugin.cpp @@ -1097,8 +1097,8 @@ } /// Links the module in \p View from file \p F into the combined module -/// saved in the IRMover \p L. Returns true on error, false on success. -static bool linkInModule(LLVMContext &Context, IRMover &L, claimed_file &F, +/// saved in the IRMover \p L. +static void linkInModule(LLVMContext &Context, IRMover &L, claimed_file &F, const void *View, ld_plugin_input_file &File, raw_fd_ostream *ApiFile, StringSet<> &Internalize, StringSet<> &Maybe) { @@ -1107,7 +1107,7 @@ std::unique_ptr M = getModuleForFile( Context, F, View, File, ApiFile, Internalize, Maybe, Keep, Realign); if (!M.get()) - return false; + return; if (!options::triple.empty()) M->setTargetTriple(options::triple.c_str()); else if (M->getTargetTriple().empty()) { @@ -1115,7 +1115,7 @@ } if (L.move(std::move(M), Keep, [](GlobalValue &, IRMover::ValueAdder) {})) - return true; + message(LDPL_FATAL, "Failed to link module"); for (const auto &I : Realign) { GlobalValue *Dst = L.getModule().getNamedValue(I.first()); @@ -1123,8 +1123,6 @@ continue; cast(Dst)->setAlignment(I.second); } - - return false; } /// Perform the ThinLTO backend on a single module, invoking the LTO and codegen @@ -1145,8 +1143,7 @@ IRMover L(*NewModule.get()); StringSet<> Dummy; - if (linkInModule(Context, L, F, View, File, ApiFile, Dummy, Dummy)) - message(LDPL_FATAL, "Failed to rename module for ThinLTO"); + linkInModule(Context, L, F, View, File, ApiFile, Dummy, Dummy); if (renameModuleForThinLTO(*NewModule, CombinedIndex)) message(LDPL_FATAL, "Failed to rename module for ThinLTO"); @@ -1368,9 +1365,8 @@ const void *View = getSymbolsAndView(F); if (!View) continue; - if (linkInModule(Context, L, F, View, InputFile.file(), ApiFile, - Internalize, Maybe)) - message(LDPL_FATAL, "Failed to link module"); + linkInModule(Context, L, F, View, InputFile.file(), ApiFile, Internalize, + Maybe); } for (const auto &Name : Internalize) {