[lto] Make sure that ctors are added to the combined module.

[silvas]

Summary

This is actually a bit too broad and will result in issues with intrinsics.
That will be addressed in the next patch.

[ruiu]

Can you describe what this patch is for?

[silvas]

Without this patch, llvm.global_ctors is not added to the combined module and so the final output contains no ctors. Programs that need these are thus mis-linked/mis-compiled.

[silvas]

The need for ExtraKeeps is because llvm.global_ctors and the other SF_FormatSpecific have names in IR but those names are not symbols for the linker.

[ruiu]

ELF/InputFiles.cpp

454 ↗

(On Diff #50198)

Do you need to copy strings? Because Symtab has unique_ptrs to BitcodeFiles, they are not freed until the program ends.

[silvas]

ELF/InputFiles.cpp

454 ↗

(On Diff #50198)

Yes. M goes away at the end of this function.

[ruiu]

LGTM with a nit.

ELF/InputFiles.h

192 ↗

(On Diff #50198)

Add a comment for this member (for those like me who don't know why we need this.)

[silvas]

Update for Rui's comments.

[silvas]

@rafael, could you comment on the high level design here? I think this is fairly reasonable but as with the other patches I think you may have a specific idea of the "right" way to do things.

[• rafael]

looking.

[• rafael]

So, the horrible special case is appending linkage. Long term we should delete it. For now you can check just for it instead of all SF_FormatSpecific.

The other issue is how we propagate information on which symbols are needed. Currently we create SymbolBodies. There are a few problem with that

• As you point out, we don't want to create them for appending linkage.
• It requires doing a hash lookup to get the GV on the other side.

There are two possible solutions that fix both issues

• Use a single LLVMContext and keep the modules alive.
• Create a IRObjectFile again addBitcodeFile instead of using getLazyBitcodeModule. That would let us match GV and SymbolBody by the index instead of name.

[silvas]

Address Rafael's comment. Don't check SF_FormatSpecific for now.

lld/trunk/ELF/InputFiles.h

 };
 
 class BitcodeFile : public InputFile {
 public:
   explicit BitcodeFile(MemoryBufferRef M);
   static bool classof(const InputFile *F);
   void parse(llvm::DenseSet<StringRef> &ComdatGroups);
   ArrayRef<SymbolBody *> getSymbols() { return SymbolBodies; }
+  ArrayRef<StringRef> getExtraKeeps() { return ExtraKeeps; }
 
 private:
   std::vector<SymbolBody *> SymbolBodies;
+  // Some symbols like llvm.global_ctors are internal to the IR and so
+  // don't show up in SymbolBodies, but must be kept when creating the
+  // combined LTO module. We track them here.
+  // We currently use a different Module for creating SymbolBody's vs when
+  // we are creating the combined LTO module, and so we can't store IR
+  // pointers directly and must rely on the IR names.
+  std::vector<StringRef> ExtraKeeps;
   llvm::BumpPtrAllocator Alloc;
   llvm::StringSaver Saver{Alloc};
 };
 
 // .so file.
 template <class ELFT> class SharedFile : public ELFFileBase<ELFT> {
   typedef ELFFileBase<ELFT> Base;
   typedef typename llvm::object::ELFFile<ELFT>::Elf_Shdr Elf_Shdr;

lld/trunk/ELF/InputFiles.cpp

for (const BasicSymbolRef &Sym : Obj->symbols()) {
     const GlobalValue *GV = Obj->getSymbolGV(Sym.getRawDataRefImpl());
     assert(GV);
     uint32_t Flags = Sym.getFlags();
     if (const Comdat *C = GV->getComdat())
       if (!KeptComdats.count(C))
         continue;
     if (!(Flags & BasicSymbolRef::SF_Global))
         continue;
-    if (Flags & BasicSymbolRef::SF_FormatSpecific)
+    if (GV->hasAppendingLinkage()) {
+      ExtraKeeps.push_back(GV->getName().copy(Alloc));
       continue;
+    }
     uint8_t Visibility = getGvVisibility(GV);
 
     SmallString<64> Name;
     raw_svector_ostream OS(Name);
     Sym.printName(OS);
     StringRef NameRef = Saver.save(StringRef(Name));
 
     SymbolBody *Body;

lld/trunk/ELF/SymbolTable.cpp

if (B->repl() != B)
       continue;
     auto *DB = dyn_cast<DefinedBitcode>(B);
     if (!DB)
       continue;
     GlobalValue *GV = M->getNamedValue(B->getName());
     assert(GV);
     Keep.push_back(GV);
   }
+  for (StringRef S : F.getExtraKeeps()) {
+    GlobalValue *GV = M->getNamedValue(S);
+    assert(GV);
+    Keep.push_back(GV);
+  }
   Mover.move(std::move(M), Keep, [](GlobalValue &, IRMover::ValueAdder) {});
 }
 
 // This is for use when debugging LTO.
 static void saveBCFile(Module &M) {
   std::error_code EC;
   raw_fd_ostream OS(Config->OutputFile.str() + ".lto.bc", EC,
                     sys::fs::OpenFlags::F_None);

lld/trunk/test/ELF/lto/ctors.ll

+; REQUIRES: x86
+; RUN: llvm-as %s -o %t.o
+; RUN: ld.lld -m elf_x86_64 %t.o -o %t.so -shared
+; RUN: llvm-readobj -sections %t.so | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
[e-mail]_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @ctor, i8* null }]
+define void @ctor() {
+  ret void
+}
+
+; The llvm.global_ctors should end up producing constructors.
+; CHECK: Name: .ctors