LTOCodeGenerator: add linkonce to llvm.used when in "MustPreserve" set

[mehdi_amini]

Summary

If the linker requested to preserve a linkonce function, we should
honor this even if we drop all uses.

[mehdi_amini]

Fix typo in the test

[david2050]

lib/LTO/LTOCodeGenerator.cpp

378

Is there a missing return here?

test/tools/lto/hide-linkonce-odr.ll

5–8

longer

[mehdi_amini]

Address David's comment

[mehdi_amini]

Thanks for the comments.

lib/LTO/LTOCodeGenerator.cpp

378

Yes, good catch. It turns out that the next variable declaration was unused, and was taken as body for the if block, which is why the code was still compiling...

[• rafael]

In gold we just change it to weak_odr.

[mehdi_amini]

Address Duncan's comments: turn into weak(_odr) instead of using the llvm.used global

[dexonsmith]

LGTM.

[mehdi_amini]

(mark as approved)

[mehdi_amini]

r267644

lib/LTO/LTOCodeGenerator.cpp

 }
 
 std::unique_ptr<TargetMachine> LTOCodeGenerator::createTargetMachine() {
   return std::unique_ptr<TargetMachine>(
       MArch->createTargetMachine(TripleStr, MCpu, FeatureStr, Options,
                                  RelocModel, CodeModel::Default, CGOptLevel));
 }
 
+// If a linkonce global is present in the MustPreserveSymbols, we need to make
+// sure we honor this. To force the compiler to not drop it, we turn its linkage
+// to the weak equivalent.
+static void
+preserveDiscardableGVs(Module &TheModule,
+                       function_ref<bool(const GlobalValue &)> mustPreserveGV) {
+  auto mayPreserveGlobal = [&](GlobalValue &GV) {
+    if (!GV.isDiscardableIfUnused() || GV.isDeclaration())
+      return;
+    if (!mustPreserveGV(GV))
+      return;
+    if (GV.hasAvailableExternallyLinkage() || GV.hasLocalLinkage())
+      report_fatal_error("The linker asked LTO to preserve a symbol with an"
+                         "unexpected linkage");
+    GV.setLinkage(GlobalValue::getWeakLinkage(GV.hasLinkOnceODRLinkage()));
+  };
+
+  for (auto &GV : TheModule)
+    mayPreserveGlobal(GV);
+  for (auto &GV : TheModule.globals())
+    mayPreserveGlobal(GV);
+  for (auto &GV : TheModule.aliases())
+    mayPreserveGlobal(GV);
+}
+
 void LTOCodeGenerator::applyScopeRestrictions() {
-  if (ScopeRestrictionsDone || !ShouldInternalize)
+  if (ScopeRestrictionsDone)
+    return;
+
+  // Declare a callback for the internalize pass that will ask for every
+  // candidate GlobalValue if it can be internalized or not.
+  SmallString<64> MangledName;
+  auto mustPreserveGV = [&](const GlobalValue &GV) -> bool {
+    // Need to mangle the GV as the "MustPreserveSymbols" StringSet is filled
+    // with the linker supplied name, which on Darwin includes a leading
+    // underscore.
+    MangledName.clear();
+    MangledName.reserve(GV.getName().size() + 1);
+    Mangler::getNameWithPrefix(MangledName, GV.getName(),
+                               MergedModule->getDataLayout());
+    return MustPreserveSymbols.count(MangledName);
+  };
+
+  // Preserve linkonce value on linker request
+  preserveDiscardableGVs(*MergedModule, mustPreserveGV);
+
+  if (!ShouldInternalize)
     return;
 
   if (ShouldRestoreGlobalsLinkage) {
     // Record the linkage type of non-local symbols so they can be restored
-    // prior
-    // to module splitting.
+    // prior to module splitting.
     auto RecordLinkage = [&](const GlobalValue &GV) {
       if (!GV.hasAvailableExternallyLinkage() && !GV.hasLocalLinkage() &&
           GV.hasName())
         ExternalSymbols.insert(std::make_pair(GV.getName(), GV.getLinkage()));
     };
     for (auto &GV : *MergedModule)
       RecordLinkage(GV);
     for (auto &GV : MergedModule->globals())
       RecordLinkage(GV);
     for (auto &GV : MergedModule->aliases())
       RecordLinkage(GV);
   }
 
   // Update the llvm.compiler_used globals to force preserving libcalls and
   // symbols referenced from asm
   UpdateCompilerUsed(*MergedModule, *TargetMach, AsmUndefinedRefs);
 
-  // Declare a callback for the internalize pass that will ask for every
-  // candidate GlobalValue if it can be internalized or not.
-  Mangler Mangler;
-  SmallString<64> MangledName;
-  auto MustPreserveGV = [&](const GlobalValue &GV) -> bool {
-    // Need to mangle the GV as the "MustPreserveSymbols" StringSet is filled
-    // with the linker supplied name, which on Darwin includes a leading
-    // underscore.
-    MangledName.clear();
-    MangledName.reserve(GV.getName().size() + 1);
-    Mangler::getNameWithPrefix(MangledName, GV.getName(),
-                               MergedModule->getDataLayout());
-    return MustPreserveSymbols.count(MangledName);
-  };
-
-  internalizeModule(*MergedModule, MustPreserveGV);
+  internalizeModule(*MergedModule, mustPreserveGV);
 
   ScopeRestrictionsDone = true;
 }
 
 /// Restore original linkage for symbols that may have been internalized
 void LTOCodeGenerator::restoreLinkageForExternals() {
   if (!ShouldInternalize || !ShouldRestoreGlobalsLinkage)
     return;

test/tools/lto/hide-linkonce-odr.ll

 ; RUN: llvm-as %s -o %t.o
-; RUN: %ld64 -lto_library %llvmshlibdir/libLTO.dylib -dylib -arch x86_64 -macosx_version_min 10.10.0 -lSystem -o %t.dylib %t.o -save-temps  -undefined dynamic_lookup
+; RUN: %ld64 -lto_library %llvmshlibdir/libLTO.dylib -dylib -arch x86_64 -macosx_version_min 10.10.0 -lSystem -o %t.dylib %t.o -save-temps  -undefined dynamic_lookup -exported_symbol _c -exported_symbol _b
 
 ; RUN: llvm-dis %t.dylib.lto.opt.bc -o - | FileCheck --check-prefix=IR %s
-; check that @a is still a linkonce_odr definition
-; IR: define linkonce_odr void @a()
+; check that @a is no longer a linkonce_odr definition
+; IR-NOT: define linkonce_odr void @a()
+; check that @b is turned into weak because it is exported
+; IR: define weak_odr void @b() #1 {
 
 ; RUN: llvm-nm %t.dylib | FileCheck --check-prefix=NM %s
 ; check that the linker can hide @a but not @b
 ; NM: 0000000000000f10 t _a
 ; NM: 0000000000000f20 T _b
 ; NM: 0000000000000f00 T _c
 
 target triple = "x86_64-apple-macosx10.10.0"