[Patch] Change linkInModule to take a std::unique_ptr
Rafael Espíndola via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 8 17:07:21 PST 2015
Passing in a std::unique_ptr should help find errors when the module
is used after being linked into another module.
This changes a C api, but changes it to what it is documented to do:
* Links the source module into the destination module, taking ownership
* of the source module away from the caller.
Cheers,
Rafael
-------------- next part --------------
diff --git a/include/llvm/Linker/Linker.h b/include/llvm/Linker/Linker.h
index aa43009..2153856 100644
--- a/include/llvm/Linker/Linker.h
+++ b/include/llvm/Linker/Linker.h
@@ -70,7 +70,7 @@ public:
Linker(Module &M, DiagnosticHandlerFunction DiagnosticHandler);
- /// \brief Link \p Src into the composite. The source is destroyed.
+ /// \brief Link \p Src into the composite.
///
/// Passing OverrideSymbols as true will have symbols from Src
/// shadow those in the Dest.
@@ -79,11 +79,11 @@ public:
/// are part of the set will be imported from the source module.
///
/// Returns true on error.
- bool linkInModule(Module &Src, unsigned Flags = Flags::None,
+ bool linkInModule(std::unique_ptr<Module> Src, unsigned Flags = Flags::None,
const FunctionInfoIndex *Index = nullptr,
DenseSet<const GlobalValue *> *FunctionsToImport = nullptr);
- static bool linkModules(Module &Dest, Module &Src,
+ static bool linkModules(Module &Dest, std::unique_ptr<Module> Src,
DiagnosticHandlerFunction DiagnosticHandler,
unsigned Flags = Flags::None);
diff --git a/lib/LTO/LTOCodeGenerator.cpp b/lib/LTO/LTOCodeGenerator.cpp
index bf3cde5..03a2c74 100644
--- a/lib/LTO/LTOCodeGenerator.cpp
+++ b/lib/LTO/LTOCodeGenerator.cpp
@@ -108,7 +108,7 @@ bool LTOCodeGenerator::addModule(LTOModule *Mod) {
assert(&Mod->getModule().getContext() == &Context &&
"Expected module in same context");
- bool ret = IRLinker->linkInModule(Mod->getModule());
+ bool ret = IRLinker->linkInModule(Mod->takeModule());
const std::vector<const char *> &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 a9fcee7..a907f54 100644
--- a/lib/Linker/LinkModules.cpp
+++ b/lib/Linker/LinkModules.cpp
@@ -1969,10 +1969,10 @@ Linker::Linker(Module &M, DiagnosticHandlerFunction DiagnosticHandler)
}
}
-bool Linker::linkInModule(Module &Src, unsigned Flags,
+bool Linker::linkInModule(std::unique_ptr<Module> Src, unsigned Flags,
const FunctionInfoIndex *Index,
DenseSet<const GlobalValue *> *FunctionsToImport) {
- ModuleLinker TheLinker(Composite, IdentifiedStructTypes, Src,
+ ModuleLinker TheLinker(Composite, IdentifiedStructTypes, *Src,
DiagnosticHandler, Flags, Index, FunctionsToImport);
bool RetCode = TheLinker.run();
Composite.dropTriviallyDeadConstantArrays();
@@ -1988,11 +1988,11 @@ bool Linker::linkInModule(Module &Src, unsigned Flags,
/// true is returned and ErrorMsg (if not null) is set to indicate the problem.
/// Upon failure, the Dest module could be in a modified state, and shouldn't be
/// relied on to be consistent.
-bool Linker::linkModules(Module &Dest, Module &Src,
+bool Linker::linkModules(Module &Dest, std::unique_ptr<Module> Src,
DiagnosticHandlerFunction DiagnosticHandler,
unsigned Flags) {
Linker L(Dest, DiagnosticHandler);
- return L.linkInModule(Src, Flags);
+ return L.linkInModule(std::move(Src), Flags);
}
std::unique_ptr<Module>
@@ -2002,7 +2002,7 @@ llvm::renameModuleForThinLTO(std::unique_ptr<Module> &M,
std::unique_ptr<llvm::Module> RenamedModule(
new llvm::Module(M->getModuleIdentifier(), M->getContext()));
Linker L(*RenamedModule.get(), DiagnosticHandler);
- if (L.linkInModule(*M.get(), llvm::Linker::Flags::None, Index))
+ if (L.linkInModule(std::move(M), llvm::Linker::Flags::None, Index))
return nullptr;
return RenamedModule;
}
@@ -2017,9 +2017,10 @@ LLVMBool LLVMLinkModules(LLVMModuleRef Dest, LLVMModuleRef Src,
std::string Message;
raw_string_ostream Stream(Message);
DiagnosticPrinterRawOStream DP(Stream);
+ std::unique_ptr<Module> M(unwrap(Src));
LLVMBool Result = Linker::linkModules(
- *D, *unwrap(Src), [&](const DiagnosticInfo &DI) { DI.print(DP); });
+ *D, std::move(M), [&](const DiagnosticInfo &DI) { DI.print(DP); });
if (OutMessages && Result) {
Stream.flush();
diff --git a/lib/Transforms/IPO/FunctionImport.cpp b/lib/Transforms/IPO/FunctionImport.cpp
index b585a86..394aa83 100644
--- a/lib/Transforms/IPO/FunctionImport.cpp
+++ b/lib/Transforms/IPO/FunctionImport.cpp
@@ -165,8 +165,8 @@ static unsigned ProcessImportWorklist(
// Link in the specified function.
DenseSet<const GlobalValue *> FunctionsToImport;
FunctionsToImport.insert(F);
- if (TheLinker.linkInModule(*SrcModule, Linker::Flags::None, &Index,
- &FunctionsToImport))
+ if (TheLinker.linkInModule(std::move(SrcModule), Linker::Flags::None,
+ &Index, &FunctionsToImport))
report_fatal_error("Function Import: link error");
// Process the newly imported function and add callees to the worklist.
diff --git a/tools/bugpoint/BugDriver.cpp b/tools/bugpoint/BugDriver.cpp
index 9edc242..4f7a747 100644
--- a/tools/bugpoint/BugDriver.cpp
+++ b/tools/bugpoint/BugDriver.cpp
@@ -139,7 +139,7 @@ bool BugDriver::addSources(const std::vector<std::string> &Filenames) {
if (!M.get()) return true;
outs() << "Linking in input file: '" << Filenames[i] << "'\n";
- if (Linker::linkModules(*Program, *M, diagnosticHandler))
+ if (Linker::linkModules(*Program, std::move(M), diagnosticHandler))
return true;
}
diff --git a/tools/bugpoint/Miscompilation.cpp b/tools/bugpoint/Miscompilation.cpp
index 5c9f027..5d704a8 100644
--- a/tools/bugpoint/Miscompilation.cpp
+++ b/tools/bugpoint/Miscompilation.cpp
@@ -230,7 +230,7 @@ static std::unique_ptr<Module> testMergedProgram(const BugDriver &BD,
std::unique_ptr<Module> M2,
std::string &Error,
bool &Broken) {
- if (Linker::linkModules(*M1, *M2, diagnosticHandler))
+ if (Linker::linkModules(*M1, std::move(M2), diagnosticHandler))
exit(1);
// Execute the program.
@@ -396,7 +396,8 @@ static bool ExtractLoops(BugDriver &BD,
MisCompFunctions.emplace_back(F->getName(), F->getFunctionType());
}
- if (Linker::linkModules(*ToNotOptimize, *ToOptimizeLoopExtracted,
+ if (Linker::linkModules(*ToNotOptimize,
+ std::move(ToOptimizeLoopExtracted),
diagnosticHandler))
exit(1);
@@ -424,7 +425,7 @@ 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, std::move(ToOptimizeLoopExtracted),
diagnosticHandler))
exit(1);
@@ -593,7 +594,7 @@ static bool ExtractBlocks(BugDriver &BD,
if (!I->isDeclaration())
MisCompFunctions.emplace_back(I->getName(), I->getFunctionType());
- if (Linker::linkModules(*ProgClone, *Extracted, diagnosticHandler))
+ if (Linker::linkModules(*ProgClone, std::move(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 8eacdc3..f7b65c5 100644
--- a/tools/gold/gold-plugin.cpp
+++ b/tools/gold/gold-plugin.cpp
@@ -956,7 +956,7 @@ static ld_plugin_status allSymbolsReadHook(raw_fd_ostream *ApiFile) {
M->setTargetTriple(DefaultTriple);
}
- if (L.linkInModule(*M))
+ if (L.linkInModule(std::move(M)))
message(LDPL_FATAL, "Failed to link module");
if (release_input_file(F.handle) != LDPS_OK)
message(LDPL_FATAL, "Failed to release file information");
diff --git a/tools/llvm-link/llvm-link.cpp b/tools/llvm-link/llvm-link.cpp
index 6f90f40..3b91109 100644
--- a/tools/llvm-link/llvm-link.cpp
+++ b/tools/llvm-link/llvm-link.cpp
@@ -200,7 +200,7 @@ static bool importFunctions(const char *argv0, LLVMContext &Context,
// Link in the specified function.
DenseSet<const GlobalValue *> FunctionsToImport;
FunctionsToImport.insert(F);
- if (L.linkInModule(*M, Linker::Flags::None, Index.get(),
+ if (L.linkInModule(std::move(M), Linker::Flags::None, Index.get(),
&FunctionsToImport))
return false;
}
@@ -241,7 +241,7 @@ static bool linkFiles(const char *argv0, LLVMContext &Context, Linker &L,
if (Verbose)
errs() << "Linking in '" << File << "'\n";
- if (L.linkInModule(*M, ApplicableFlags, Index.get()))
+ if (L.linkInModule(std::move(M), ApplicableFlags, Index.get()))
return false;
// All linker flags apply to linking of subsequent files.
ApplicableFlags = Flags;
diff --git a/unittests/Linker/LinkModulesTest.cpp b/unittests/Linker/LinkModulesTest.cpp
index e56a692..891eb46 100644
--- a/unittests/Linker/LinkModulesTest.cpp
+++ b/unittests/Linker/LinkModulesTest.cpp
@@ -95,10 +95,7 @@ TEST_F(LinkModuleTest, BlockAddress) {
Builder.CreateRet(ConstantPointerNull::get(Type::getInt8PtrTy(Ctx)));
Module *LinkedModule = new Module("MyModuleLinked", Ctx);
- Linker::linkModules(*LinkedModule, *M, expectNoDiags);
-
- // Delete the original module.
- M.reset();
+ Linker::linkModules(*LinkedModule, std::move(M), expectNoDiags);
// Check that the global "@switch.bas" is well-formed.
const GlobalVariable *LinkedGV = LinkedModule->getNamedGlobal("switch.bas");
@@ -171,13 +168,13 @@ static Module *getInternal(LLVMContext &Ctx) {
TEST_F(LinkModuleTest, EmptyModule) {
std::unique_ptr<Module> InternalM(getInternal(Ctx));
std::unique_ptr<Module> EmptyM(new Module("EmptyModule1", Ctx));
- Linker::linkModules(*EmptyM, *InternalM, expectNoDiags);
+ Linker::linkModules(*EmptyM, std::move(InternalM), expectNoDiags);
}
TEST_F(LinkModuleTest, EmptyModule2) {
std::unique_ptr<Module> InternalM(getInternal(Ctx));
std::unique_ptr<Module> EmptyM(new Module("EmptyModule1", Ctx));
- Linker::linkModules(*InternalM, *EmptyM, expectNoDiags);
+ Linker::linkModules(*InternalM, std::move(EmptyM), expectNoDiags);
}
TEST_F(LinkModuleTest, TypeMerge) {
@@ -192,7 +189,7 @@ TEST_F(LinkModuleTest, TypeMerge) {
"@t2 = weak global %t zeroinitializer\n";
std::unique_ptr<Module> M2 = parseAssemblyString(M2Str, Err, C);
- Linker::linkModules(*M1, *M2, [](const llvm::DiagnosticInfo &) {});
+ Linker::linkModules(*M1, std::move(M2), [](const llvm::DiagnosticInfo &) {});
EXPECT_EQ(M1->getNamedGlobal("t1")->getType(),
M1->getNamedGlobal("t2")->getType());
@@ -202,7 +199,7 @@ TEST_F(LinkModuleTest, CAPISuccess) {
std::unique_ptr<Module> DestM(getExternal(Ctx, "foo"));
std::unique_ptr<Module> SourceM(getExternal(Ctx, "bar"));
char *errout = nullptr;
- LLVMBool result = LLVMLinkModules(wrap(DestM.get()), wrap(SourceM.get()),
+ LLVMBool result = LLVMLinkModules(wrap(DestM.get()), wrap(SourceM.release()),
LLVMLinkerDestroySource, &errout);
EXPECT_EQ(0, result);
EXPECT_EQ(nullptr, errout);
@@ -215,7 +212,7 @@ TEST_F(LinkModuleTest, CAPIFailure) {
std::unique_ptr<Module> DestM(getExternal(Ctx, "foo"));
std::unique_ptr<Module> SourceM(getExternal(Ctx, "foo"));
char *errout = nullptr;
- LLVMBool result = LLVMLinkModules(wrap(DestM.get()), wrap(SourceM.get()),
+ LLVMBool result = LLVMLinkModules(wrap(DestM.get()), wrap(SourceM.release()),
LLVMLinkerDestroySource, &errout);
EXPECT_EQ(1, result);
EXPECT_STREQ("Linking globals named 'foo': symbol multiply defined!", errout);
@@ -269,7 +266,8 @@ TEST_F(LinkModuleTest, MoveDistinctMDs) {
// Link into destination module.
auto Dst = llvm::make_unique<Module>("Linked", C);
ASSERT_TRUE(Dst.get());
- Linker::linkModules(*Dst, *Src, [](const llvm::DiagnosticInfo &) {});
+ Linker::linkModules(*Dst, std::move(Src),
+ [](const llvm::DiagnosticInfo &) {});
// Check that distinct metadata was moved, not cloned. Even !4, the uniqued
// node, should effectively be moved, since its only operand hasn't changed.
More information about the llvm-commits
mailing list