[ELF] Generalize ELF symbol type handling

[davide]

Summary

SymbolBody constructor and friends take isFunc and isTLS boolean arguments. ELF symbols have already a type so than be easily passed down. This makes code a little bit more readable, I think.
The next patch in this series will remove isTLS.

[davide]

Note (to future self and others): this minor yak was motivated by a bug I found.
Some symbols are not emitted in the symbol table with the appropriate type, e.g. STT_OBJECT.
The current code logic would require passing another bool isObject around. Up to two additional argument, this stretching exercise was a little bit goofy but still acceptable, but with more types to support, is just too much, IMHO.

[ruiu]

This looks good to me, but do you want to do isTLS in the same patch? This patch itself does not reduce the number of parameters, so I think this patch more sense if you do both at once.

[davide]

In D17871#367706, @ruiu wrote:

This looks good to me, but do you want to do isTLS in the same patch? This patch itself does not reduce the number of parameters, so I think this patch more sense if you do both at once.

Sure, why not.

[davide]

Addressed Rui's comments. Removed isTls sooner rather than later.

[ruiu]

LGTM

[grimar]

Hi Davide,

Change itself looks good for me, but I have few thoughts about possible implementation and prepared a little patch here: http://reviews.llvm.org/D17879.

ELF/Symbols.h

public:
   bool isUndefined() const {
     return SymbolKind == UndefinedKind || SymbolKind == UndefinedElfKind;
   }
   bool isDefined() const { return SymbolKind <= DefinedLast; }
   bool isCommon() const { return SymbolKind == DefinedCommonKind; }
   bool isLazy() const { return SymbolKind == LazyKind; }
   bool isShared() const { return SymbolKind == SharedKind; }
   bool isUsedInRegularObj() const { return IsUsedInRegularObj; }
-  bool isFunc() const { return IsFunc; }
+  bool isFunc() const { return Type == llvm::ELF::STT_FUNC; }
 
   // Returns the symbol name.
   StringRef getName() const { return Name; }
 
   uint8_t getVisibility() const { return Visibility; }
 
   unsigned DynsymIndex = 0;
   uint32_t GlobalDynIndex = -1;
public:
 
   // Decides which symbol should "win" in the symbol table, this or
   // the Other. Returns 1 if this wins, -1 if the Other wins, or 0 if
   // they are duplicate (conflicting) symbols.
   template <class ELFT> int compare(SymbolBody *Other);
 
 protected:
   SymbolBody(Kind K, StringRef Name, bool IsWeak, uint8_t Visibility,
-             bool IsTls, bool IsFunc)
+             uint8_t Type, bool IsTls)
       : SymbolKind(K), IsWeak(IsWeak), Visibility(Visibility),
         MustBeInDynSym(false), NeedsCopyOrPltAddr(false), IsTls(IsTls),
-        IsFunc(IsFunc), Name(Name) {
+        Type(Type), Name(Name) {
     IsUsedInRegularObj = K != SharedKind && K != LazyKind;
   }
 
   const unsigned SymbolKind : 8;
   unsigned IsWeak : 1;
   unsigned Visibility : 2;
 
   // True if the symbol was used for linking and thus need to be
   // added to the output file's symbol table. It is usually true,
   // but if it is a shared symbol that were not referenced by anyone,
   // it can be false.
   unsigned IsUsedInRegularObj : 1;
 
 public:
   // If true, the symbol is added to .dynsym symbol table.
   unsigned MustBeInDynSym : 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;
 
   unsigned IsTls : 1;
 
 protected:
-  unsigned IsFunc : 1;
+  uint8_t Type;
 
   StringRef Name;
   Symbol *Backref = nullptr;
 };
 
 // The base class for any defined symbols.
 class Defined : public SymbolBody {
 public:
-  Defined(Kind K, StringRef Name, bool IsWeak, uint8_t Visibility, bool IsTls,
-          bool IsFunction);
+  Defined(Kind K, StringRef Name, bool IsWeak, uint8_t Visibility,
+          uint8_t Type, bool IsTls);
   static bool classof(const SymbolBody *S) { return S->isDefined(); }
 };
 
 // Any defined symbol from an ELF file.
 template <class ELFT> class DefinedElf : public Defined {
 protected:
   typedef typename llvm::object::ELFFile<ELFT>::Elf_Sym Elf_Sym;
 
 public:
   DefinedElf(Kind K, StringRef N, const Elf_Sym &Sym)
       : Defined(K, N, Sym.getBinding() == llvm::ELF::STB_WEAK,
-                Sym.getVisibility(), Sym.getType() == llvm::ELF::STT_TLS,
-                Sym.getType() == llvm::ELF::STT_FUNC),
+                Sym.getVisibility(), Sym.getType(),
+                Sym.getType() == llvm::ELF::STT_TLS),
         Sym(Sym) {}
 
   const Elf_Sym &Sym;
   static bool classof(const SymbolBody *S) {
     return S->kind() <= DefinedElfLast;
   }
 };
 
public:
   SharedSymbol(SharedFile<ELFT> *F, StringRef Name, const Elf_Sym &Sym)
       : DefinedElf<ELFT>(SymbolBody::SharedKind, Name, Sym), File(F) {}
 
   SharedFile<ELFT> *File;
 
   // OffsetInBss is significant only when needsCopy() is true.
   uintX_t OffsetInBss = 0;
 
-  bool needsCopy() const { return this->NeedsCopyOrPltAddr && !this->IsFunc; }
+  bool needsCopy() const { return this->NeedsCopyOrPltAddr && !this->isFunc(); }
 };
 
 // This class represents a symbol defined in an archive file. It is
 // created from an archive file header, and it knows how to load an
 // object file from an archive to replace itself with a defined
 // symbol. If the resolver finds both Undefined and Lazy for
 // the same name, it will ask the Lazy to load a file.
 class Lazy : public SymbolBody {

ELF/Symbols.cpp

case DefinedRegularKind: {
     return SC->OutSec->getVA() + SC->getOffset(D->Sym);
   }
   case DefinedCommonKind:
     return Out<ELFT>::Bss->getVA() + cast<DefinedCommon>(this)->OffsetInBss;
   case SharedKind: {
     auto *SS = cast<SharedSymbol<ELFT>>(this);
     if (!SS->NeedsCopyOrPltAddr)
       return 0;
-    if (SS->IsFunc)
+    if (SS->isFunc())
       return getPltVA<ELFT>();
     else
       return Out<ELFT>::Bss->getVA() + SS->OffsetInBss;
   }
   case UndefinedElfKind:
   case UndefinedKind:
     return 0;
   case LazyKind:
if (isCommon()) {
     return -1;
   }
   if (Other->isCommon())
     return 1;
   return 0;
 }
 
 Defined::Defined(Kind K, StringRef Name, bool IsWeak, uint8_t Visibility,
-                 bool IsTls, bool IsFunction)
-    : SymbolBody(K, Name, IsWeak, Visibility, IsTls, IsFunction) {}
+                 uint8_t Type, bool IsTls)
+    : SymbolBody(K, Name, IsWeak, Visibility, Type, IsTls) {}
 
 DefinedBitcode::DefinedBitcode(StringRef Name, bool IsWeak)
     : Defined(DefinedBitcodeKind, Name, IsWeak, STV_DEFAULT, false, false) {}
 
 bool DefinedBitcode::classof(const SymbolBody *S) {
   return S->kind() == DefinedBitcodeKind;
 }
 
 Undefined::Undefined(SymbolBody::Kind K, StringRef N, bool IsWeak,
                      uint8_t Visibility, bool IsTls)
-    : SymbolBody(K, N, IsWeak, Visibility, IsTls, /*IsFunction*/ false),
+    : SymbolBody(K, N, IsWeak, Visibility, 0 /* Type */, IsTls),
       CanKeepUndefined(false) {}
 
 Undefined::Undefined(StringRef N, bool IsWeak, uint8_t Visibility,
                      bool CanKeepUndefined)
     : Undefined(SymbolBody::UndefinedKind, N, IsWeak, Visibility,
                 /*IsTls*/ false) {
   this->CanKeepUndefined = CanKeepUndefined;
 }
 
 template <typename ELFT>
 UndefinedElf<ELFT>::UndefinedElf(StringRef N, const Elf_Sym &Sym)
     : Undefined(SymbolBody::UndefinedElfKind, N,
                 Sym.getBinding() == llvm::ELF::STB_WEAK, Sym.getVisibility(),
                 Sym.getType() == llvm::ELF::STT_TLS),
       Sym(Sym) {}
 
 template <typename ELFT>
 DefinedSynthetic<ELFT>::DefinedSynthetic(StringRef N, uintX_t Value,
                                          OutputSectionBase<ELFT> &Section,
                                          uint8_t Visibility)
     : Defined(SymbolBody::DefinedSyntheticKind, N, false, Visibility,
-              /*IsTls*/ false, /*IsFunction*/ false),
+              Type, /*IsTls*/ false),
       Value(Value), Section(Section) {}
 
 DefinedCommon::DefinedCommon(StringRef N, uint64_t Size, uint64_t Alignment,
                              bool IsWeak, uint8_t Visibility)
-    : Defined(SymbolBody::DefinedCommonKind, N, IsWeak, Visibility,
-              /*IsTls*/ false, /*IsFunction*/ false) {
+    : Defined(SymbolBody::DefinedCommonKind, N, IsWeak, Visibility, Type,
+              /*IsTls*/ false) {
   MaxAlignment = Alignment;
   this->Size = Size;
 }
 
 std::unique_ptr<InputFile> Lazy::getMember() {
   MemoryBufferRef MBRef = File->getMember(&Sym);
 
   // getMember returns an empty buffer if the member was already