Change how symbol sizes are handled in lib/Object.

COFF and MachO only define symbol sizes for common symbols. Reflect that
in the class hierarchy by having a method for common symbols only in the base
and a general one in ELF.

This avoids the need of using a magic value for the size, which had a few
problems
* Most callers didn't check for it.
* The ones that did could not tell the magic value from a file actually having
  that value.

llvm-svn: 240529

llvm/include/llvm/Object/COFF.h

@@ -636,7 +636,7 @@ protected:
                                 StringRef &Res) const override;
   std::error_code getSymbolAddress(DataRefImpl Symb,
                                    uint64_t &Res) const override;
-  uint64_t getSymbolSize(DataRefImpl Symb) const override;
+  uint64_t getCommonSymbolSizeImpl(DataRefImpl Symb) const override;
   uint32_t getSymbolFlags(DataRefImpl Symb) const override;
   std::error_code getSymbolType(DataRefImpl Symb,
                                 SymbolRef::Type &Res) const override;

llvm/include/llvm/Object/ELFObjectFile.h

@@ -52,6 +52,8 @@ public:
   virtual uint64_t getSectionFlags(SectionRef Sec) const = 0;
   virtual uint32_t getSectionType(SectionRef Sec) const = 0;
 
+  virtual uint64_t getSymbolSize(SymbolRef Symb) const = 0;
+
   static inline bool classof(const Binary *v) { return v->isELF(); }
 };
 
@@ -72,6 +74,8 @@ public:
   typedef typename ELFFile<ELFT>::Elf_Shdr_Iter Elf_Shdr_Iter;
   typedef typename ELFFile<ELFT>::Elf_Dyn_Iter Elf_Dyn_Iter;
 
+  uint64_t getSymbolSize(SymbolRef Symb) const override;
+
 protected:
   ELFFile<ELFT> EF;
 
@@ -81,7 +85,7 @@ protected:
   std::error_code getSymbolAddress(DataRefImpl Symb,
                                    uint64_t &Res) const override;
   uint32_t getSymbolAlignment(DataRefImpl Symb) const override;
-  uint64_t getSymbolSize(DataRefImpl Symb) const override;
+  uint64_t getCommonSymbolSizeImpl(DataRefImpl Symb) const override;
   uint32_t getSymbolFlags(DataRefImpl Symb) const override;
   std::error_code getSymbolOther(DataRefImpl Symb, uint8_t &Res) const override;
   std::error_code getSymbolType(DataRefImpl Symb,
@@ -277,7 +281,7 @@ std::error_code ELFObjectFile<ELFT>::getSymbolAddress(DataRefImpl Symb,
   switch (EF.getSymbolTableIndex(ESym)) {
   case ELF::SHN_COMMON:
   case ELF::SHN_UNDEF:
-    Result = UnknownAddressOrSize;
+    Result = UnknownAddress;
     return std::error_code();
   case ELF::SHN_ABS:
     Result = ESym->st_value;
@@ -312,7 +316,12 @@ uint32_t ELFObjectFile<ELFT>::getSymbolAlignment(DataRefImpl Symb) const {
 }
 
 template <class ELFT>
-uint64_t ELFObjectFile<ELFT>::getSymbolSize(DataRefImpl Symb) const {
+uint64_t ELFObjectFile<ELFT>::getSymbolSize(SymbolRef Symb) const {
+  return toELFSymIter(Symb.getRawDataRefImpl())->st_size;
+}
+
+template <class ELFT>
+uint64_t ELFObjectFile<ELFT>::getCommonSymbolSizeImpl(DataRefImpl Symb) const {
   return toELFSymIter(Symb)->st_size;
 }
 

llvm/include/llvm/Object/MachO.h

@@ -207,7 +207,7 @@ public:
   std::error_code getSymbolAddress(DataRefImpl Symb,
                                    uint64_t &Res) const override;
   uint32_t getSymbolAlignment(DataRefImpl Symb) const override;
-  uint64_t getSymbolSize(DataRefImpl Symb) const override;
+  uint64_t getCommonSymbolSizeImpl(DataRefImpl Symb) const override;
   std::error_code getSymbolType(DataRefImpl Symb,
                                 SymbolRef::Type &Res) const override;
   uint32_t getSymbolFlags(DataRefImpl Symb) const override;

llvm/include/llvm/Object/ObjectFile.h

@@ -141,7 +141,7 @@ public:
   std::error_code getAddress(uint64_t &Result) const;
   /// @brief Get the alignment of this symbol as the actual value (not log 2).
   uint32_t getAlignment() const;
-  uint64_t getSize() const;
+  uint64_t getCommonSize() const;
   std::error_code getType(SymbolRef::Type &Result) const;
   std::error_code getOther(uint8_t &Result) const;
 
@@ -201,7 +201,7 @@ protected:
   virtual std::error_code getSymbolAddress(DataRefImpl Symb,
                                            uint64_t &Res) const = 0;
   virtual uint32_t getSymbolAlignment(DataRefImpl Symb) const;
-  virtual uint64_t getSymbolSize(DataRefImpl Symb) const = 0;
+  virtual uint64_t getCommonSymbolSizeImpl(DataRefImpl Symb) const = 0;
   virtual std::error_code getSymbolType(DataRefImpl Symb,
                                         SymbolRef::Type &Res) const = 0;
   virtual std::error_code getSymbolSection(DataRefImpl Symb,
@@ -252,6 +252,11 @@ protected:
   }
 
 public:
+  uint64_t getCommonSymbolSize(DataRefImpl Symb) const {
+    assert(getSymbolFlags(Symb) & SymbolRef::SF_Common);
+    return getCommonSymbolSizeImpl(Symb);
+  }
+
   typedef iterator_range<symbol_iterator> symbol_iterator_range;
   symbol_iterator_range symbols() const {
     return symbol_iterator_range(symbol_begin(), symbol_end());
@@ -326,8 +331,8 @@ inline uint32_t SymbolRef::getAlignment() const {
   return getObject()->getSymbolAlignment(getRawDataRefImpl());
 }
 
-inline uint64_t SymbolRef::getSize() const {
-  return getObject()->getSymbolSize(getRawDataRefImpl());
+inline uint64_t SymbolRef::getCommonSize() const {
+  return getObject()->getCommonSymbolSize(getRawDataRefImpl());
 }
 
 inline std::error_code SymbolRef::getSection(section_iterator &Result) const {

llvm/include/llvm/Object/SymbolicFile.h

@@ -115,7 +115,7 @@ public:
 
 typedef content_iterator<BasicSymbolRef> basic_symbol_iterator;
 
-const uint64_t UnknownAddressOrSize = ~0ULL;
+const uint64_t UnknownAddress = ~0ULL;
 
 class SymbolicFile : public Binary {
 public:

llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp

@@ -118,8 +118,8 @@ static std::error_code getOffset(const SymbolRef &Sym, uint64_t &Result) {
   if (std::error_code EC = Sym.getAddress(Address))
     return EC;
 
-  if (Address == UnknownAddressOrSize) {
-    Result = UnknownAddressOrSize;
+  if (Address == UnknownAddress) {
+    Result = UnknownAddress;
     return std::error_code();
   }
 
@@ -129,7 +129,7 @@ static std::error_code getOffset(const SymbolRef &Sym, uint64_t &Result) {
     return EC;
 
   if (SecI == Obj->section_end()) {
-    Result = UnknownAddressOrSize;
+    Result = UnknownAddress;
     return std::error_code();
   }
 
@@ -387,7 +387,7 @@ void RuntimeDyldImpl::computeTotalAllocSize(const ObjectFile &Obj,
     uint32_t Flags = I->getFlags();
     if (Flags & SymbolRef::SF_Common) {
       // Add the common symbols to a list.  We'll allocate them all below.
-      uint64_t Size = I->getSize();
+      uint64_t Size = I->getCommonSize();
       CommonSize += Size;
     }
   }
@@ -494,7 +494,7 @@ void RuntimeDyldImpl::emitCommonSymbols(const ObjectFile &Obj,
     }
 
     uint32_t Align = Sym.getAlignment();
-    uint64_t Size = Sym.getSize();
+    uint64_t Size = Sym.getCommonSize();
 
     CommonSize += Align + Size;
     SymbolsToAllocate.push_back(Sym);
@@ -517,7 +517,7 @@ void RuntimeDyldImpl::emitCommonSymbols(const ObjectFile &Obj,
   for (auto &Sym : SymbolsToAllocate) {
     uint32_t Align = Sym.getAlignment();
     StringRef Name;
-    uint64_t Size = Sym.getSize();
+    uint64_t Size = Sym.getCommonSize();
     Check(Sym.getName(Name));
     if (Align) {
       // This symbol has an alignment requirement.

llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.cpp

@@ -64,18 +64,18 @@ RuntimeDyldCOFF::loadObject(const object::ObjectFile &O) {
 uint64_t RuntimeDyldCOFF::getSymbolOffset(const SymbolRef &Sym) {
   uint64_t Address;
   if (Sym.getAddress(Address))
-    return UnknownAddressOrSize;
+    return UnknownAddress;
 
-  if (Address == UnknownAddressOrSize)
-    return UnknownAddressOrSize;
+  if (Address == UnknownAddress)
+    return UnknownAddress;
 
   const ObjectFile *Obj = Sym.getObject();
   section_iterator SecI(Obj->section_end());
   if (Sym.getSection(SecI))
-    return UnknownAddressOrSize;
+    return UnknownAddress;
 
   if (SecI == Obj->section_end())
-    return UnknownAddressOrSize;
+    return UnknownAddress;
 
   uint64_t SectionAddress = SecI->getAddress();
   return Address - SectionAddress;

llvm/lib/Object/COFFObjectFile.cpp

@@ -155,11 +155,11 @@ std::error_code COFFObjectFile::getSymbolAddress(DataRefImpl Ref,
   COFFSymbolRef Symb = getCOFFSymbol(Ref);
 
   if (Symb.isAnyUndefined()) {
-    Result = UnknownAddressOrSize;
+    Result = UnknownAddress;
     return std::error_code();
   }
   if (Symb.isCommon()) {
-    Result = UnknownAddressOrSize;
+    Result = UnknownAddress;
     return std::error_code();
   }
   int32_t SectionNumber = Symb.getSectionNumber();
@@ -236,12 +236,9 @@ uint32_t COFFObjectFile::getSymbolFlags(DataRefImpl Ref) const {
   return Result;
 }
 
-uint64_t COFFObjectFile::getSymbolSize(DataRefImpl Ref) const {
+uint64_t COFFObjectFile::getCommonSymbolSizeImpl(DataRefImpl Ref) const {
   COFFSymbolRef Symb = getCOFFSymbol(Ref);
-
-  if (Symb.isCommon())
-    return Symb.getValue();
-  return UnknownAddressOrSize;
+  return Symb.getValue();
 }
 
 std::error_code

llvm/lib/Object/MachOObjectFile.cpp

@@ -375,14 +375,14 @@ std::error_code MachOObjectFile::getSymbolAddress(DataRefImpl Symb,
     MachO::nlist_64 Entry = getSymbol64TableEntry(Symb);
     if ((Entry.n_type & MachO::N_TYPE) == MachO::N_UNDF &&
         Entry.n_value == 0)
-      Res = UnknownAddressOrSize;
+      Res = UnknownAddress;
     else
       Res = Entry.n_value;
   } else {
     MachO::nlist Entry = getSymbolTableEntry(Symb);
     if ((Entry.n_type & MachO::N_TYPE) == MachO::N_UNDF &&
         Entry.n_value == 0)
-      Res = UnknownAddressOrSize;
+      Res = UnknownAddress;
     else
       Res = Entry.n_value;
   }
@@ -398,13 +398,10 @@ uint32_t MachOObjectFile::getSymbolAlignment(DataRefImpl DRI) const {
   return 0;
 }
 
-uint64_t MachOObjectFile::getSymbolSize(DataRefImpl DRI) const {
+uint64_t MachOObjectFile::getCommonSymbolSizeImpl(DataRefImpl DRI) const {
   uint64_t Value;
   getSymbolAddress(DRI, Value);
-  uint32_t flags = getSymbolFlags(DRI);
-  if (flags & SymbolRef::SF_Common)
-    return Value;
-  return UnknownAddressOrSize;
+  return Value;
 }
 
 std::error_code MachOObjectFile::getSymbolType(DataRefImpl Symb,
@@ -453,7 +450,7 @@ uint32_t MachOObjectFile::getSymbolFlags(DataRefImpl DRI) const {
     if ((MachOType & MachO::N_TYPE) == MachO::N_UNDF) {
       uint64_t Value;
       getSymbolAddress(DRI, Value);
-      if (Value && Value != UnknownAddressOrSize)
+      if (Value && Value != UnknownAddress)
         Result |= SymbolRef::SF_Common;
     }
 

llvm/lib/Object/Object.cpp

@@ -187,7 +187,7 @@ uint64_t LLVMGetSymbolAddress(LLVMSymbolIteratorRef SI) {
 }
 
 uint64_t LLVMGetSymbolSize(LLVMSymbolIteratorRef SI) {
-  return (*unwrap(SI))->getSize();
+  return (*unwrap(SI))->getCommonSize();
 }
 
 // RelocationRef accessors

llvm/lib/Object/SymbolSize.cpp

@@ -41,10 +41,9 @@ ErrorOr<std::vector<std::pair<SymbolRef, uint64_t>>>
 llvm::object::computeSymbolSizes(const ObjectFile &O) {
   std::vector<std::pair<SymbolRef, uint64_t>> Ret;
 
-  if (isa<ELFObjectFileBase>(&O)) {
-    for (SymbolRef Sym : O.symbols()) {
-      Ret.push_back({Sym, Sym.getSize()});
-    }
+  if (const auto *E = dyn_cast<ELFObjectFileBase>(&O)) {
+    for (SymbolRef Sym : E->symbols())
+      Ret.push_back({Sym, E->getSymbolSize(Sym)});
     return Ret;
   }
 

llvm/lib/Target/X86/MCTargetDesc/X86ELFRelocationInfo.cpp

@@ -31,8 +31,8 @@ public:
 
     StringRef SymName; SymI->getName(SymName);
     uint64_t  SymAddr; SymI->getAddress(SymAddr);
-    uint64_t SymSize = SymI->getSize();
     auto *Obj = cast<ELFObjectFileBase>(Rel.getObjectFile());
+    uint64_t SymSize = Obj->getSymbolSize(*SymI);
     int64_t Addend = *Obj->getRelocationAddend(Rel.getRawDataRefImpl());
 
     MCSymbol *Sym = Ctx.getOrCreateSymbol(SymName);

llvm/test/Object/X86/nm-print-size.s

@@ -0,0 +1,7 @@
+// RUN: llvm-mc %s -o %t -filetype=obj -triple=x86_64-pc-linux
+// RUN: llvm-nm --print-size %t | FileCheck %s
+
+// CHECK: 0000000000000000 ffffffffffffffff t a
+
+a:
+        .size a, 0xffffffffffffffff

llvm/test/Object/lit.local.cfg

@@ -1 +1 @@
-config.suffixes = ['.test', '.ll', '.yaml']
+config.suffixes = ['.test', '.ll', '.s', '.yaml']

llvm/tools/dsymutil/MachODebugMapParser.cpp

@@ -160,7 +160,7 @@ void MachODebugMapParser::handleStabSymbolTableEntry(uint32_t StringIndex,
     // symbol table to find its address as it might not be in the
     // debug map (for common symbols).
     Value = getMainBinarySymbolAddress(Name);
-    if (Value == UnknownAddressOrSize)
+    if (Value == UnknownAddress)
       return;
     break;
   case MachO::N_FUN:
@@ -199,8 +199,7 @@ void MachODebugMapParser::loadCurrentObjectFileSymbols() {
   for (auto Sym : CurrentObjectHolder.Get().symbols()) {
     StringRef Name;
     uint64_t Addr;
-    if (Sym.getAddress(Addr) || Addr == UnknownAddressOrSize ||
-        Sym.getName(Name))
+    if (Sym.getAddress(Addr) || Addr == UnknownAddress || Sym.getName(Name))
       continue;
     CurrentObjectAddresses[Name] = Addr;
   }
@@ -212,7 +211,7 @@ void MachODebugMapParser::loadCurrentObjectFileSymbols() {
 uint64_t MachODebugMapParser::getMainBinarySymbolAddress(StringRef Name) {
   auto Sym = MainBinarySymbolAddresses.find(Name);
   if (Sym == MainBinarySymbolAddresses.end())
-    return UnknownAddressOrSize;
+    return UnknownAddress;
   return Sym->second;
 }
 
@@ -233,7 +232,7 @@ void MachODebugMapParser::loadMainBinarySymbols() {
     // are the only ones that need to be queried because the address
     // of common data won't be described in the debug map. All other
     // addresses should be fetched for the debug map.
-    if (Sym.getAddress(Addr) || Addr == UnknownAddressOrSize ||
+    if (Sym.getAddress(Addr) || Addr == UnknownAddress ||
         !(Sym.getFlags() & SymbolRef::SF_Global) || Sym.getSection(Section) ||
         Section->isText() || Sym.getName(Name) || Name.size() == 0 ||
         Name[0] == '\0')

llvm/tools/llvm-nm/llvm-nm.cpp

@@ -569,7 +569,7 @@ static void sortAndPrintSymbolList(SymbolicFile &Obj, bool printName,
       continue;
     if ((I->TypeChar == 'U') && DefinedOnly)
       continue;
-    if (SizeSort && !PrintAddress && I->Size == UnknownAddressOrSize)
+    if (SizeSort && !PrintAddress)
       continue;
     if (PrintFileName) {
       if (!ArchitectureName.empty())
@@ -586,16 +586,15 @@ static void sortAndPrintSymbolList(SymbolicFile &Obj, bool printName,
     char SymbolAddrStr[18] = "";
     char SymbolSizeStr[18] = "";
 
-    if (OutputFormat == sysv || I->Address == UnknownAddressOrSize)
+    if (OutputFormat == sysv || I->Address == UnknownAddress)
       strcpy(SymbolAddrStr, printBlanks);
     if (OutputFormat == sysv)
       strcpy(SymbolSizeStr, printBlanks);
 
-    if (I->Address != UnknownAddressOrSize)
+    if (I->Address != UnknownAddress)
       format(printFormat, I->Address)
           .print(SymbolAddrStr, sizeof(SymbolAddrStr));
-    if (I->Size != UnknownAddressOrSize)
-      format(printFormat, I->Size).print(SymbolSizeStr, sizeof(SymbolSizeStr));
+    format(printFormat, I->Size).print(SymbolSizeStr, sizeof(SymbolSizeStr));
 
     // If OutputFormat is darwin or we are printing Mach-O symbols in hex and
     // we have a MachOObjectFile, call darwinPrintSymbol to print as darwin's
@@ -613,8 +612,7 @@ static void sortAndPrintSymbolList(SymbolicFile &Obj, bool printName,
         outs() << SymbolAddrStr << ' ';
       if (PrintSize) {
         outs() << SymbolSizeStr;
-        if (I->Size != UnknownAddressOrSize)
-          outs() << ' ';
+        outs() << ' ';
       }
       outs() << I->TypeChar;
       if (I->TypeChar == '-' && MachO)
@@ -930,11 +928,13 @@ static void dumpSymbolNamesFromObject(SymbolicFile &Obj, bool printName,
     if (Nsect && Nsect != getNsectInMachO(*MachO, I))
       continue;
     NMSymbol S;
-    S.Size = UnknownAddressOrSize;
-    S.Address = UnknownAddressOrSize;
-    if (PrintSize && isa<ELFObjectFileBase>(Obj)) {
-      symbol_iterator SymI = I;
-      S.Size = SymI->getSize();
+    S.Size = 0;
+    S.Address = UnknownAddress;
+    if (PrintSize) {
+      if (auto *E = dyn_cast<ELFObjectFileBase>(&Obj)) {
+        symbol_iterator SymI = I;
+        S.Size = E->getSymbolSize(*SymI);
+      }
     }
     if (PrintAddress && isa<ObjectFile>(Obj))
       if (error(symbol_iterator(I)->getAddress(S.Address)))

llvm/tools/llvm-objdump/MachODump.cpp

@@ -2417,10 +2417,9 @@ static const char *get_pointer_32(uint32_t Address, uint32_t &offset,
 // for the specified section offset in the specified section reference.
 // If no relocation information is found and a non-zero ReferenceValue for the
 // symbol is passed, look up that address in the info's AddrMap.
-static const char *
-get_symbol_64(uint32_t sect_offset, SectionRef S, DisassembleInfo *info,
-              uint64_t &n_value,
-              uint64_t ReferenceValue = UnknownAddressOrSize) {
+static const char *get_symbol_64(uint32_t sect_offset, SectionRef S,
+                                 DisassembleInfo *info, uint64_t &n_value,
+                                 uint64_t ReferenceValue = UnknownAddress) {
   n_value = 0;
   if (!info->verbose)
     return nullptr;
@@ -2454,7 +2453,7 @@ get_symbol_64(uint32_t sect_offset, SectionRef S, DisassembleInfo *info,
   const char *SymbolName = nullptr;
   if (reloc_found && isExtern) {
     Symbol.getAddress(n_value);
-    if (n_value == UnknownAddressOrSize)
+    if (n_value == UnknownAddress)
       n_value = 0;
     StringRef name;
     Symbol.getName(name);
@@ -2475,7 +2474,7 @@ get_symbol_64(uint32_t sect_offset, SectionRef S, DisassembleInfo *info,
 
   // We did not find an external relocation entry so look up the ReferenceValue
   // as an address of a symbol and if found return that symbol's name.
-  if (ReferenceValue != UnknownAddressOrSize)
+  if (ReferenceValue != UnknownAddress)
     SymbolName = GuessSymbolName(ReferenceValue, info->AddrMap);
 
   return SymbolName;

llvm/tools/llvm-objdump/llvm-objdump.cpp

@@ -784,7 +784,7 @@ static void DisassembleObject(const ObjectFile *Obj, bool InlineRelocs) {
         uint64_t Address;
         if (error(Symbol.getAddress(Address)))
           break;
-        if (Address == UnknownAddressOrSize)
+        if (Address == UnknownAddress)
           continue;
         Address -= SectionAddr;
         if (Address >= SectSize)
@@ -1101,9 +1101,9 @@ void llvm::PrintSymbolTable(const ObjectFile *o) {
     bool Hidden = Flags & SymbolRef::SF_Hidden;
 
     if (Common)
-      Address = Symbol.getSize();
+      Address = Symbol.getCommonSize();
 
-    if (Address == UnknownAddressOrSize)
+    if (Address == UnknownAddress)
       Address = 0;
     char GlobLoc = ' ';
     if (Type != SymbolRef::ST_Unknown)
@@ -1149,7 +1149,8 @@ void llvm::PrintSymbolTable(const ObjectFile *o) {
 
     outs() << '\t';
     if (Common || isa<ELFObjectFileBase>(o)) {
-      uint64_t Val = Common ? Symbol.getAlignment() :  Symbol.getSize();
+      uint64_t Val = Common ? Symbol.getAlignment()
+                            : cast<ELFObjectFileBase>(o)->getSymbolSize(Symbol);
       outs() << format("\t %08" PRIx64 " ", Val);
     }
 

llvm/tools/llvm-symbolizer/LLVMSymbolize.cpp

@@ -94,7 +94,7 @@ void ModuleInfo::addSymbol(const SymbolRef &Symbol, DataExtractor *OpdExtractor,
     return;
   uint64_t SymbolAddress;
   if (error(Symbol.getAddress(SymbolAddress)) ||
-      SymbolAddress == UnknownAddressOrSize)
+      SymbolAddress == UnknownAddress)
     return;
   if (OpdExtractor) {
     // For big-endian PowerPC64 ELF, symbols in the .opd section refer to
@@ -109,15 +109,12 @@ void ModuleInfo::addSymbol(const SymbolRef &Symbol, DataExtractor *OpdExtractor,
       SymbolAddress = OpdExtractor->getAddress(&OpdOffset32);
   }
   uint64_t SymbolSize;
-  // Getting symbol size is linear for Mach-O files, so assume that symbol
-  // occupies the memory range up to the following symbol.
-  if (isa<MachOObjectFile>(Module))
+  // Onyl ELF has a size for every symbol so assume that symbol occupies the
+  // memory range up to the following symbol.
+  if (auto *E = dyn_cast<ELFObjectFileBase>(Module))
+    SymbolSize = E->getSymbolSize(Symbol);
+  else
     SymbolSize = 0;
-  else {
-    SymbolSize = Symbol.getSize();
-    if (SymbolSize == UnknownAddressOrSize)
-      return;
-  }
   StringRef SymbolName;
   if (error(Symbol.getName(SymbolName)))
     return;