ELF: Merge UndefinedBitcode and UndefinedElf.

[pcc]

Summary

Depends on D19560

[ruiu]

It's generally looking good.

ELF/InputFiles.cpp

320 ↗

(On Diff #55114)

This may be a silly question, but I don't remember why we need to save st_size for undefined symbols. Are we using that information?

[pcc]

ELF/InputFiles.cpp

320 ↗

(On Diff #55114)

The size shows up in the output symbol table, but I can't really think of a good reason why we need to keep it there. I'll change this patch to drop it unless @rafael can see a reason not to.

[ruiu]

LGTM. You can keep it for now and remove in a follow-up patch. Probably we shouldn't mix them together.

lld/trunk/ELF/InputFiles.cpp

 }
 
 template <class ELFT>
 SymbolBody *elf::ObjectFile<ELFT>::createSymbolBody(const Elf_Sym *Sym) {
   unsigned char Binding = Sym->getBinding();
   InputSectionBase<ELFT> *Sec = getSection(*Sym);
   if (Binding == STB_LOCAL) {
     if (Sym->st_shndx == SHN_UNDEF)
-      return new (Alloc) UndefinedElf<ELFT>(*Sym);
+      return new (Alloc)
+          Undefined(Sym->st_name, Sym->st_other, Sym->getType(), Sym->st_size);
     return new (Alloc) DefinedRegular<ELFT>(*Sym, Sec);
   }
 
   StringRef Name = check(Sym->getName(this->StringTable));
 
   switch (Sym->st_shndx) {
   case SHN_UNDEF:
-    return new (Alloc) UndefinedElf<ELFT>(Name, *Sym);
+    return new (Alloc)
+        Undefined(Name, Binding, Sym->st_other, Sym->getType(), Sym->st_size,
+                  /*IsBitcode*/ false);
   case SHN_COMMON:
     return new (Alloc) DefinedCommon(Name, Sym->st_size, Sym->st_value, Binding,
                                      Sym->st_other, Sym->getType());
   }
 
   switch (Binding) {
   default:
     fatal("unexpected binding");
   case STB_GLOBAL:
   case STB_WEAK:
   case STB_GNU_UNIQUE:
     if (Sec == &InputSection<ELFT>::Discarded)
-      return new (Alloc) UndefinedElf<ELFT>(Name, *Sym);
+      return new (Alloc)
+          Undefined(Name, Binding, Sym->st_other, Sym->getType(), Sym->st_size,
+                    /*IsBitcode*/ false);
     return new (Alloc) DefinedRegular<ELFT>(Name, *Sym, Sec);
   }
 }
 
 void ArchiveFile::parse() {
   File = check(Archive::create(MB), "failed to parse archive");
 
   // Allocate a buffer for Lazy objects.
BitcodeFile::createBody(const DenseSet<const Comdat *> &KeptComdats,
   SmallString<64> Name;
   raw_svector_ostream OS(Name);
   Sym.printName(OS);
   StringRef NameRef = Saver.save(StringRef(Name));
   SymbolBody *Body;
 
   uint32_t Flags = Sym.getFlags();
   bool IsWeak = Flags & BasicSymbolRef::SF_Weak;
+  uint32_t Binding = IsWeak ? STB_WEAK : STB_GLOBAL;
 
   uint8_t Visibility;
   if (GV)
     Visibility = getGvVisibility(GV);
   else
     // FIXME: Set SF_Hidden flag correctly for module asm symbols, and expose
     // protected visibility.
     Visibility = STV_DEFAULT;
 
   if (GV)
     if (const Comdat *C = GV->getComdat())
       if (!KeptComdats.count(C)) {
-        Body = new (Alloc)
-          UndefinedBitcode(NameRef, IsWeak, Visibility);
+        Body = new (Alloc) Undefined(NameRef, Binding, Visibility, /*Type*/ 0,
+                                     /*Size*/ 0, /*IsBitcode*/ true);
         return Body;
       }
 
   const Module &M = Obj.getModule();
   if (Flags & BasicSymbolRef::SF_Undefined)
-    return new (Alloc) UndefinedBitcode(NameRef, IsWeak, Visibility);
+    return new (Alloc) Undefined(NameRef, Binding, Visibility, /*Type*/ 0,
+                                 /*Size*/ 0, /*IsBitcode*/ true);
   if (Flags & BasicSymbolRef::SF_Common) {
     // FIXME: Set SF_Common flag correctly for module asm symbols, and expose
     // size and alignment.
     assert(GV);
     const DataLayout &DL = M.getDataLayout();
     uint64_t Size = DL.getTypeAllocSize(GV->getValueType());
-    return new (Alloc)
-        DefinedCommon(NameRef, Size, GV->getAlignment(),
-                      IsWeak ? STB_WEAK : STB_GLOBAL, Visibility, /*Type*/ 0);
+    return new (Alloc) DefinedCommon(NameRef, Size, GV->getAlignment(), Binding,
+                                     Visibility, /*Type*/ 0);
   }
   return new (Alloc) DefinedBitcode(NameRef, IsWeak, Visibility);
 }
 
 SymbolBody *
 BitcodeFile::createSymbolBody(const DenseSet<const Comdat *> &KeptComdats,
                               const IRObjectFile &Obj,
                               const BasicSymbolRef &Sym) {

lld/trunk/ELF/OutputSections.cpp

case SymbolBody::DefinedRegularKind: {
     break;
   }
   case SymbolBody::DefinedCommonKind:
     return Out<ELFT>::Bss;
   case SymbolBody::SharedKind:
     if (cast<SharedSymbol<ELFT>>(Sym)->needsCopy())
       return Out<ELFT>::Bss;
     break;
-  case SymbolBody::UndefinedElfKind:
-  case SymbolBody::UndefinedBitcodeKind:
+  case SymbolBody::UndefinedKind:
   case SymbolBody::LazyArchiveKind:
   case SymbolBody::LazyObjectKind:
     break;
   case SymbolBody::DefinedBitcodeKind:
     llvm_unreachable("should have been replaced");
   }
   return nullptr;
 }

lld/trunk/ELF/SymbolTable.cpp

for (auto &IF : IFs) {
     ObjectFiles.emplace_back(Obj);
   }
 }
 
 // Add an undefined symbol.
 template <class ELFT>
 SymbolBody *SymbolTable<ELFT>::addUndefined(StringRef Name) {
   auto *Sym = new (Alloc)
-      UndefinedElf<ELFT>(Name, STB_GLOBAL, STV_DEFAULT, /*Type*/ 0);
+      Undefined(Name, STB_GLOBAL, STV_DEFAULT, /*Type*/ 0, /*Size*/ 0,
+                /*IsBitcode*/ false);
   resolve(Sym);
   return Sym;
 }
 
 template <class ELFT>
 DefinedRegular<ELFT> *SymbolTable<ELFT>::addAbsolute(StringRef Name,
                                                      uint8_t Visibility) {
   // Pass nullptr because absolute symbols have no corresponding input sections.
template <class ELFT> Symbol *SymbolTable<ELFT>::insert(SymbolBody *New) {
   // in the output.
   if (!New->isShared())
     Sym->Visibility = getMinVisibility(Sym->Visibility, New->getVisibility());
   Sym->ExportDynamic = Sym->ExportDynamic || shouldExport(New);
   SymbolBody::Kind K = New->kind();
   if (K == SymbolBody::DefinedRegularKind ||
       K == SymbolBody::DefinedCommonKind ||
       K == SymbolBody::DefinedSyntheticKind ||
-      K == SymbolBody::UndefinedElfKind)
+      (K == SymbolBody::UndefinedKind && !New->IsUndefinedBitcode))
     Sym->IsUsedInRegularObj = true;
   return Sym;
 }
 
 template <class ELFT> SymbolBody *SymbolTable<ELFT>::find(StringRef Name) {
   auto It = Symtab.find(Name);
   if (It == Symtab.end())
     return nullptr;

lld/trunk/ELF/Symbols.h

public:
   enum Kind {
     DefinedFirst,
     DefinedRegularKind = DefinedFirst,
     SharedKind,
     DefinedCommonKind,
     DefinedBitcodeKind,
     DefinedSyntheticKind,
     DefinedLast = DefinedSyntheticKind,
-    UndefinedElfKind,
-    UndefinedBitcodeKind,
+    UndefinedKind,
     LazyArchiveKind,
     LazyObjectKind,
   };
 
   Kind kind() const { return static_cast<Kind>(SymbolKind); }
 
   bool isWeak() const { return Binding == llvm::ELF::STB_WEAK; }
-  bool isUndefined() const {
-    return SymbolKind == UndefinedBitcodeKind || SymbolKind == UndefinedElfKind;
-  }
+  bool isUndefined() const { return SymbolKind == UndefinedKind; }
   bool isDefined() const { return SymbolKind <= DefinedLast; }
   bool isCommon() const { return SymbolKind == DefinedCommonKind; }
   bool isLazy() const {
     return SymbolKind == LazyArchiveKind || SymbolKind == LazyObjectKind;
   }
   bool isShared() const { return SymbolKind == SharedKind; }
   bool isLocal() const { return Binding == llvm::ELF::STB_LOCAL; }
   bool isPreemptible() const;
public:
   // LTO, but we could add the feature to ELF. It would be similar to
   // MachO's .weak_def_can_be_hidden.
   unsigned CanOmitFromDynSym : 1;
 
   // True if the linker has to generate a copy relocation for this shared
   // symbol or if the symbol should point to its plt entry.
   unsigned NeedsCopyOrPltAddr : 1;
 
+  // True if the symbol is undefined and comes from a bitcode file. We need to
+  // keep track of this because undefined symbols only prevent internalization
+  // of bitcode symbols if they did not come from a bitcode file.
+  unsigned IsUndefinedBitcode : 1;
+
   // The following fields have the same meaning as the ELF symbol attributes.
   uint8_t Type;    // symbol type
   uint8_t Binding; // symbol binding
   uint8_t StOther; // st_other field value
 
   bool isSection() const { return Type == llvm::ELF::STT_SECTION; }
   bool isTls() const { return Type == llvm::ELF::STT_TLS; }
   bool isFunc() const { return Type == llvm::ELF::STT_FUNC; }
public:
   // Special value designates that the symbol 'points'
   // to the end of the section.
   static const uintX_t SectionEnd = uintX_t(-1);
 
   uintX_t Value;
   const OutputSectionBase<ELFT> &Section;
 };
 
-class UndefinedBitcode : public SymbolBody {
+class Undefined : public SymbolBody {
 public:
-  UndefinedBitcode(StringRef N, bool IsWeak, uint8_t StOther);
+  Undefined(StringRef Name, uint8_t Binding, uint8_t StOther, uint8_t Type,
+            uint64_t Size, bool IsBitcode);
+  Undefined(uint32_t NameOffset, uint8_t StOther, uint8_t Type, uint64_t Size);
 
-  static bool classof(const SymbolBody *S) {
-    return S->kind() == UndefinedBitcodeKind;
-  }
-};
-
-template <class ELFT> class UndefinedElf : public SymbolBody {
-  typedef typename ELFT::uint uintX_t;
-  typedef typename ELFT::Sym Elf_Sym;
-
-public:
-  UndefinedElf(StringRef N, const Elf_Sym &Sym);
-  UndefinedElf(const Elf_Sym &Sym);
-  UndefinedElf(StringRef Name, uint8_t Binding, uint8_t StOther, uint8_t Type);
-
-  uintX_t Size;
+  uint64_t Size;
 
   static bool classof(const SymbolBody *S) {
-    return S->kind() == SymbolBody::UndefinedElfKind;
+    return S->kind() == UndefinedKind;
   }
 };
 
 template <class ELFT> class SharedSymbol : public Defined {
   typedef typename ELFT::Sym Elf_Sym;
   typedef typename ELFT::uint uintX_t;
 
 public:

lld/trunk/ELF/Symbols.cpp

static typename ELFT::uint getSymVA(const SymbolBody &Body,
   case SymbolBody::SharedKind: {
     auto &SS = cast<SharedSymbol<ELFT>>(Body);
     if (!SS.NeedsCopyOrPltAddr)
       return 0;
     if (SS.isFunc())
       return Body.getPltVA<ELFT>();
     return Out<ELFT>::Bss->getVA() + SS.OffsetInBss;
   }
-  case SymbolBody::UndefinedElfKind:
-  case SymbolBody::UndefinedBitcodeKind:
+  case SymbolBody::UndefinedKind:
     return 0;
   case SymbolBody::LazyArchiveKind:
   case SymbolBody::LazyObjectKind:
     assert(Body.Backref->IsUsedInRegularObj && "lazy symbol reached writer");
     return 0;
   case SymbolBody::DefinedBitcodeKind:
     llvm_unreachable("should have been replaced");
   }

 
 template <class ELFT> typename ELFT::uint SymbolBody::getSize() const {
   if (const auto *C = dyn_cast<DefinedCommon>(this))
     return C->Size;
   if (const auto *DR = dyn_cast<DefinedRegular<ELFT>>(this))
     return DR->Size;
   if (const auto *S = dyn_cast<SharedSymbol<ELFT>>(this))
     return S->Sym.st_size;
-  if (const auto *U = dyn_cast<UndefinedElf<ELFT>>(this))
+  if (const auto *U = dyn_cast<Undefined>(this))
     return U->Size;
   return 0;
 }
 
 // Returns 1, 0 or -1 if this symbol should take precedence
 // over the Other, tie or lose, respectively.
 int SymbolBody::compare(SymbolBody *Other) {
   assert(!isLazy() && !Other->isLazy());

 DefinedBitcode::DefinedBitcode(StringRef Name, bool IsWeak, uint8_t StOther)
     : Defined(DefinedBitcodeKind, Name, IsWeak ? STB_WEAK : STB_GLOBAL,
               StOther, 0 /* Type */) {}
 
 bool DefinedBitcode::classof(const SymbolBody *S) {
   return S->kind() == DefinedBitcodeKind;
 }
 
-UndefinedBitcode::UndefinedBitcode(StringRef N, bool IsWeak, uint8_t StOther)
-    : SymbolBody(SymbolBody::UndefinedBitcodeKind, N,
-                 IsWeak ? STB_WEAK : STB_GLOBAL, StOther, 0 /* Type */) {}
-
-template <typename ELFT>
-UndefinedElf<ELFT>::UndefinedElf(StringRef N, const Elf_Sym &Sym)
-    : SymbolBody(SymbolBody::UndefinedElfKind, N, Sym.getBinding(),
-                 Sym.st_other, Sym.getType()),
-      Size(Sym.st_size) {}
-
-template <typename ELFT>
-UndefinedElf<ELFT>::UndefinedElf(StringRef Name, uint8_t Binding,
-                                 uint8_t StOther, uint8_t Type)
-    : SymbolBody(SymbolBody::UndefinedElfKind, Name, Binding, StOther, Type) {}
+Undefined::Undefined(StringRef Name, uint8_t Binding, uint8_t StOther,
+                     uint8_t Type, uint64_t Size, bool IsBitcode)
+    : SymbolBody(SymbolBody::UndefinedKind, Name, Binding, StOther, Type),
+      Size(Size) {
+  this->IsUndefinedBitcode = IsBitcode;
+}
 
-template <typename ELFT>
-UndefinedElf<ELFT>::UndefinedElf(const Elf_Sym &Sym)
-    : SymbolBody(SymbolBody::UndefinedElfKind, Sym.st_name, Sym.st_other,
-                 Sym.getType()),
-      Size(Sym.st_size) {
-  assert(Sym.getBinding() == STB_LOCAL);
+Undefined::Undefined(uint32_t NameOffset, uint8_t StOther, uint8_t Type,
+                     uint64_t Size)
+    : SymbolBody(SymbolBody::UndefinedKind, NameOffset, StOther, Type),
+      Size(Size) {
+  this->IsUndefinedBitcode = false;
 }
 
 template <typename ELFT>
 DefinedSynthetic<ELFT>::DefinedSynthetic(StringRef N, uintX_t Value,
                                          OutputSectionBase<ELFT> &Section)
     : Defined(SymbolBody::DefinedSyntheticKind, N, STB_GLOBAL, STV_HIDDEN,
               0 /* Type */),
       Value(Value), Section(Section) {}

 template uint64_t SymbolBody::template getSize<ELF64LE>() const;
 template uint64_t SymbolBody::template getSize<ELF64BE>() const;
 
 template uint32_t SymbolBody::template getThunkVA<ELF32LE>() const;
 template uint32_t SymbolBody::template getThunkVA<ELF32BE>() const;
 template uint64_t SymbolBody::template getThunkVA<ELF64LE>() const;
 template uint64_t SymbolBody::template getThunkVA<ELF64BE>() const;
 
-template class elf::UndefinedElf<ELF32LE>;
-template class elf::UndefinedElf<ELF32BE>;
-template class elf::UndefinedElf<ELF64LE>;
-template class elf::UndefinedElf<ELF64BE>;
-
 template class elf::DefinedSynthetic<ELF32LE>;
 template class elf::DefinedSynthetic<ELF32BE>;
 template class elf::DefinedSynthetic<ELF64LE>;
 template class elf::DefinedSynthetic<ELF64BE>;