[LTO] Include in .symtab/.dynsym symbols introduced by optimizations

[davide]

Summary

Some optimizations, e.g. SimplifyLibCalls, can replace functions with others as part of the lowering, e.g. printf => puts.
The new symbols don't have the isUsedInRegularObj flag set so they don't get included in the final symbol table (and dynamic symbol table), and the dynamic linker gets confused.

davide@foogoo:~/llvm/build/bin % ./clang blah.c -o blah -flto -fuse-ld=lld -Wl,-L/lib
davide@foogoo:~/llvm/build/bin % ./blah
Segmentation fault (core dumped)
davide@foogoo:~/llvm/build/bin % ./llvm-nm ./blah | grep puts

% cat blah.c
#include <stdio.h>

int main(void)
{

printf("blah\n");
return (0);

}

My proposed fix is to set the flag in addCombinedLtoObject() once we see the new symbols. I'm working on a testcase now, but I would like to hear if this sounds reasonable to you.

[• rafael]

I think this is fine with a test, but please upload a version that includes one.

[davide]

Add testcase.

[ruiu]

Maybe we should rename IsUsedInRegularObj -> MustBeInSymtab?

[• rafael]

ELF/SymbolTable.cpp

111 ↗

(On Diff #51810)

git-clang-format issue?

[davide]

clang-format clean.,

[• rafael]

ELF/SymbolTable.cpp

112 ↗

(On Diff #51810)

You can just unconditionally set this, no?
That would be simpler and a bit more like what happens during regular symbol resolution.

test/ELF/lto/undefined-puts.ll

20 ↗

(On Diff #51810)

It is also used in a relocation, no? Can you check that for completeness?

[davide]

Rafael's comments.

[• rafael]

Lgtm.

Thanks
Rafael

lld/trunk/ELF/SymbolTable.cpp

for (const std::unique_ptr<BitcodeFile> &F : BitcodeFiles)
     Lto->add(*F);
   std::unique_ptr<ObjectFile<ELFT>> Obj = Lto->compile<ELFT>();
 
   // Replace bitcode symbols.
   llvm::DenseSet<StringRef> DummyGroups;
   Obj->parse(DummyGroups);
   for (SymbolBody *Body : Obj->getNonLocalSymbols()) {
     Symbol *Sym = insert(Body);
+    Sym->Body->setUsedInRegularObj();
     if (!Sym->Body->isUndefined() && Body->isUndefined())
       continue;
     Sym->Body = Body;
   }
   ObjectFiles.push_back(std::move(Obj));
 }
 
 // Add an undefined symbol.

lld/trunk/ELF/Symbols.h

public:
   uint32_t GlobalDynIndex = -1;
   uint32_t GotIndex = -1;
   uint32_t GotPltIndex = -1;
   uint32_t PltIndex = -1;
   bool hasGlobalDynIndex() { return GlobalDynIndex != uint32_t(-1); }
   bool isInGot() const { return GotIndex != -1U; }
   bool isInPlt() const { return PltIndex != -1U; }
 
+  void setUsedInRegularObj() { IsUsedInRegularObj = true; }
+
   template <class ELFT>
   typename ELFT::uint getVA(typename ELFT::uint Addend = 0) const;
 
   template <class ELFT> typename ELFT::uint getGotVA() const;
   template <class ELFT> typename ELFT::uint getGotPltVA() const;
   template <class ELFT> typename ELFT::uint getPltVA() const;
   template <class ELFT> typename ELFT::uint getSize() const;
 
public:
 
   static bool classof(const SymbolBody *S) { return S->kind() == LazyKind; }
 
   // Returns an object file for this symbol, or a nullptr if the file
   // was already returned.
   std::unique_ptr<InputFile> getMember();
 
   void setWeak() { IsWeak = true; }
-  void setUsedInRegularObj() { IsUsedInRegularObj = true; }
 
 private:
   ArchiveFile *File;
   const llvm::object::Archive::Symbol Sym;
 };
 
 // Some linker-generated symbols need to be created as
 // DefinedRegular symbols, so they need Elf_Sym symbols.

lld/trunk/test/ELF/lto/Inputs/shared.s

+.globl  printf
+printf:
+
+.globl  puts
+puts:

lld/trunk/test/ELF/lto/undefined-puts.ll

+; REQUIRES: x86
+; RUN: llvm-mc %p/Inputs/shared.s -o %t1.o -filetype=obj -triple=x86_64-unknown-linux
+; RUN: ld.lld %t1.o -o %t1.so -shared
+; RUN: llvm-as %s -o %t2.o
+; RUN: ld.lld %t1.so %t2.o -m elf_x86_64 -o %t
+; RUN: llvm-readobj -dyn-symbols -dyn-relocations %t | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@.str = private unnamed_addr constant [6 x i8] c"blah\0A\00", align 1
+
+define i32 @_start() {
+  %str = call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([6 x i8], [6 x i8]* @.str, i32 0, i32 0))
+  ret i32 0
+}
+
+declare i32 @printf(i8*, ...)
+
+; Check that puts symbol is present in the dynamic symbol table and
+; there's a relocation for it.
+; CHECK: Dynamic Relocations {
+; CHECK-NEXT:  0x11007 R_X86_64_PC32 puts 0xFFFFFFFFFFFFFFFC
+; CHECK-NEXT: }
+
+; CHECK: DynamicSymbols [
+; CHECK: Symbol {
+; CHECK:    Name: puts@