Do not destroy external linkage when deleting function body

[petarj]

Summary

The function deleteBody() converts the linkage to external and thus destroys
original linkage type value. Lack of correct linkage type causes wrong
relocations to be emitted later.
Calling dropAllReferences() instead of deleteBody() will fix the issue.

[petarj]

Related thread at llvm-commit list: http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20140915/thread.html#235838

[mseaborn]

LGTM. Is there a way to test this, though? Should there be a unit test to cover BitcodeReader::Dematerialize()?

[petarj]

Added a test to unittests/Bitcode/BitReaderTest.cpp. PTAL.

[petarj]

Minor update.

[mseaborn]

LGTM

unittests/Bitcode/BitReaderTest.cpp

61 ↗

(On Diff #13935)

Nit: how about "DematerializeFunctionPreservesLinkageType" to capture the intent a little better?

77 ↗

(On Diff #13935)

Nit: change to "Check that..." (otherwise the comment doesn't make it clear whether the test wants the linkage preserved or not)

[petarj]

Nits addressed.

unittests/Bitcode/BitReaderTest.cpp

61 ↗

(On Diff #13935)

Done.

77 ↗

(On Diff #13935)

Done.

[petarj]

Closed by commit rL218302 (authored by @petarj).

llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp

void BitcodeReader::Dematerialize(GlobalValue *GV) {
   Function *F = dyn_cast<Function>(GV);
   // If this function isn't dematerializable, this is a noop.
   if (!F || !isDematerializable(F))
     return;
 
   assert(DeferredFunctionInfo.count(F) && "No info to read function later?");
 
   // Just forget the function body, we can remat it later.
-  F->deleteBody();
+  F->dropAllReferences();
 }
 
 std::error_code BitcodeReader::MaterializeModule(Module *M) {
   assert(M == TheModule &&
          "Can only Materialize the Module this BitcodeReader is attached to.");
 
   // Promise to materialize all forward references.
   WillMaterializeAllForwardRefs = true;

llvm/trunk/unittests/Bitcode/BitReaderTest.cpp

static std::unique_ptr<Module> getLazyModuleFromAssembly(LLVMContext &Context,
   writeModuleToBuffer(parseAssembly(Assembly), Mem);
   std::unique_ptr<MemoryBuffer> Buffer =
       MemoryBuffer::getMemBuffer(Mem.str(), "test", false);
   ErrorOr<Module *> ModuleOrErr =
       getLazyBitcodeModule(std::move(Buffer), Context);
   return std::unique_ptr<Module>(ModuleOrErr.get());
 }
 
+TEST(BitReaderTest, DematerializeFunctionPreservesLinkageType) {
+  SmallString<1024> Mem;
+
+  LLVMContext Context;
+  std::unique_ptr<Module> M = getLazyModuleFromAssembly(
+      Context, Mem, "define internal i32 @func() {\n"
+                      "ret i32 0\n"
+                    "}\n");
+
+  EXPECT_FALSE(verifyModule(*M, &dbgs()));
+
+  M->getFunction("func")->Materialize();
+  EXPECT_FALSE(M->getFunction("func")->empty());
+  EXPECT_TRUE(M->getFunction("func")->getLinkage() ==
+              GlobalValue::InternalLinkage);
+
+  // Check that the linkage type is preserved after dematerialization.
+  M->getFunction("func")->Dematerialize();
+  EXPECT_TRUE(M->getFunction("func")->empty());
+  EXPECT_TRUE(M->getFunction("func")->getLinkage() ==
+              GlobalValue::InternalLinkage);
+  EXPECT_FALSE(verifyModule(*M, &dbgs()));
+}
+
 TEST(BitReaderTest, MaterializeFunctionsForBlockAddr) { // PR11677
   SmallString<1024> Mem;
 
   LLVMContext Context;
   std::unique_ptr<Module> M = getLazyModuleFromAssembly(
       Context, Mem, "@table = constant i8* blockaddress(@func, %bb)\n"
                     "define void @func() {\n"
                     "  unreachable\n"