Object, COFF: Cleanup symbol type code, improve binutils compatibility

Do a better job classifying symbols.  This increases the consistency
between the COFF handling code and the ELF side of things.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@220952 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/Object/COFF.h

@@ -301,9 +301,26 @@ public:
 
   uint8_t getComplexType() const { return (getType() & 0xF0) >> 4; }
 
+  bool isExternal() const {
+    return getStorageClass() == COFF::IMAGE_SYM_CLASS_EXTERNAL;
+  }
+
+  bool isCommon() const {
+    return isExternal() && getSectionNumber() == COFF::IMAGE_SYM_UNDEFINED &&
+           getValue() != 0;
+  }
+
+  bool isUndefined() const {
+    return isExternal() && getSectionNumber() == COFF::IMAGE_SYM_UNDEFINED &&
+           getValue() == 0;
+  }
+
+  bool isWeakExternal() const {
+    return getStorageClass() == COFF::IMAGE_SYM_CLASS_WEAK_EXTERNAL;
+  }
+
   bool isFunctionDefinition() const {
-    return getStorageClass() == COFF::IMAGE_SYM_CLASS_EXTERNAL &&
-           getBaseType() == COFF::IMAGE_SYM_TYPE_NULL &&
+    return isExternal() && getBaseType() == COFF::IMAGE_SYM_TYPE_NULL &&
            getComplexType() == COFF::IMAGE_SYM_DTYPE_FUNCTION &&
            !COFF::isReservedSectionNumber(getSectionNumber());
   }
@@ -312,10 +329,8 @@ public:
     return getStorageClass() == COFF::IMAGE_SYM_CLASS_FUNCTION;
   }
 
-  bool isWeakExternal() const {
-    return getStorageClass() == COFF::IMAGE_SYM_CLASS_WEAK_EXTERNAL ||
-           (getStorageClass() == COFF::IMAGE_SYM_CLASS_EXTERNAL &&
-            getSectionNumber() == COFF::IMAGE_SYM_UNDEFINED && getValue() == 0);
+  bool isAnyUndefined() const {
+    return isUndefined() || isWeakExternal();
   }
 
   bool isFileRecord() const {
@@ -329,6 +344,8 @@ public:
         getStorageClass() == COFF::IMAGE_SYM_CLASS_EXTERNAL &&
         getSectionNumber() == COFF::IMAGE_SYM_ABSOLUTE;
     bool isOrdinarySection = getStorageClass() == COFF::IMAGE_SYM_CLASS_STATIC;
+    if (!getNumberOfAuxSymbols())
+      return false;
     return isAppdomainGlobal || isOrdinarySection;
   }
 

lib/Object/COFFObjectFile.cpp

@@ -147,39 +147,54 @@ std::error_code COFFObjectFile::getSymbolName(DataRefImpl Ref,
 std::error_code COFFObjectFile::getSymbolAddress(DataRefImpl Ref,
                                                  uint64_t &Result) const {
   COFFSymbolRef Symb = getCOFFSymbol(Ref);
-  const coff_section *Section = nullptr;
-  if (std::error_code EC = getSection(Symb.getSectionNumber(), Section))
-    return EC;
 
-  if (Symb.getSectionNumber() == COFF::IMAGE_SYM_UNDEFINED)
+  if (Symb.isAnyUndefined()) {
     Result = UnknownAddressOrSize;
-  else if (Section)
+    return object_error::success;
+  }
+  if (Symb.isCommon()) {
+    Result = UnknownAddressOrSize;
+    return object_error::success;
+  }
+  int32_t SectionNumber = Symb.getSectionNumber();
+  if (!COFF::isReservedSectionNumber(SectionNumber)) {
+    const coff_section *Section = nullptr;
+    if (std::error_code EC = getSection(SectionNumber, Section))
+      return EC;
+
     Result = Section->VirtualAddress + Symb.getValue();
-  else
-    Result = Symb.getValue();
+    return object_error::success;
+  }
+
+  Result = Symb.getValue();
   return object_error::success;
 }
 
 std::error_code COFFObjectFile::getSymbolType(DataRefImpl Ref,
                                               SymbolRef::Type &Result) const {
   COFFSymbolRef Symb = getCOFFSymbol(Ref);
+  int32_t SectionNumber = Symb.getSectionNumber();
   Result = SymbolRef::ST_Other;
 
-  if (Symb.getStorageClass() == COFF::IMAGE_SYM_CLASS_EXTERNAL &&
-      Symb.getSectionNumber() == COFF::IMAGE_SYM_UNDEFINED) {
+  if (Symb.isAnyUndefined()) {
     Result = SymbolRef::ST_Unknown;
   } else if (Symb.isFunctionDefinition()) {
     Result = SymbolRef::ST_Function;
-  } else {
-      uint32_t Characteristics = 0;
-      if (!COFF::isReservedSectionNumber(Symb.getSectionNumber())) {
-        const coff_section *Section = nullptr;
-        if (std::error_code EC = getSection(Symb.getSectionNumber(), Section))
-          return EC;
-        Characteristics = Section->Characteristics;
-    }
-    if (Characteristics & COFF::IMAGE_SCN_MEM_READ &&
-        ~Characteristics & COFF::IMAGE_SCN_MEM_WRITE) // Read only.
+  } else if (Symb.isCommon()) {
+    Result = SymbolRef::ST_Data;
+  } else if (Symb.isFileRecord()) {
+    Result = SymbolRef::ST_File;
+  } else if (SectionNumber == COFF::IMAGE_SYM_DEBUG) {
+    Result = SymbolRef::ST_Debug;
+  } else if (!COFF::isReservedSectionNumber(SectionNumber)) {
+    const coff_section *Section = nullptr;
+    if (std::error_code EC = getSection(SectionNumber, Section))
+      return EC;
+    uint32_t Characteristics = Section->Characteristics;
+    if (Characteristics & COFF::IMAGE_SCN_CNT_CODE)
+      Result = SymbolRef::ST_Function;
+    else if (Characteristics & (COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
+                                COFF::IMAGE_SCN_CNT_UNINITIALIZED_DATA))
       Result = SymbolRef::ST_Data;
   }
   return object_error::success;
@@ -189,50 +204,66 @@ uint32_t COFFObjectFile::getSymbolFlags(DataRefImpl Ref) const {
   COFFSymbolRef Symb = getCOFFSymbol(Ref);
   uint32_t Result = SymbolRef::SF_None;
 
-  // TODO: Correctly set SF_FormatSpecific, SF_Common
-
-  if (Symb.getSectionNumber() == COFF::IMAGE_SYM_UNDEFINED) {
-    if (Symb.getValue() == 0)
-      Result |= SymbolRef::SF_Undefined;
-    else
-      Result |= SymbolRef::SF_Common;
-  }
-
-
-  // TODO: This are certainly too restrictive.
-  if (Symb.getStorageClass() == COFF::IMAGE_SYM_CLASS_EXTERNAL)
+  if (Symb.isExternal() || Symb.isWeakExternal())
     Result |= SymbolRef::SF_Global;
 
-  if (Symb.getStorageClass() == COFF::IMAGE_SYM_CLASS_WEAK_EXTERNAL)
+  if (Symb.isWeakExternal())
     Result |= SymbolRef::SF_Weak;
 
   if (Symb.getSectionNumber() == COFF::IMAGE_SYM_ABSOLUTE)
     Result |= SymbolRef::SF_Absolute;
 
+  if (Symb.isFileRecord())
+    Result |= SymbolRef::SF_FormatSpecific;
+
+  if (Symb.isSectionDefinition())
+    Result |= SymbolRef::SF_FormatSpecific;
+
+  if (Symb.isCommon())
+    Result |= SymbolRef::SF_Common;
+
+  if (Symb.isAnyUndefined())
+    Result |= SymbolRef::SF_Undefined;
+
   return Result;
 }
 
 std::error_code COFFObjectFile::getSymbolSize(DataRefImpl Ref,
                                               uint64_t &Result) const {
+  COFFSymbolRef Symb = getCOFFSymbol(Ref);
+
+  if (Symb.isAnyUndefined()) {
+    Result = UnknownAddressOrSize;
+    return object_error::success;
+  }
+  if (Symb.isCommon()) {
+    Result = Symb.getValue();
+    return object_error::success;
+  }
+  if (Symb.isFunctionDefinition()) {
+    ArrayRef<uint8_t> AuxData = getSymbolAuxData(Symb);
+    if (!AuxData.empty()) {
+      const auto *CAFD =
+          reinterpret_cast<const coff_aux_function_definition *>(
+              AuxData.data());
+      Result = CAFD->TotalSize;
+      return object_error::success;
+    }
+  }
   // FIXME: Return the correct size. This requires looking at all the symbols
   //        in the same section as this symbol, and looking for either the next
   //        symbol, or the end of the section.
-  COFFSymbolRef Symb = getCOFFSymbol(Ref);
-  const coff_section *Section = nullptr;
-  if (std::error_code EC = getSection(Symb.getSectionNumber(), Section))
-    return EC;
+  int32_t SectionNumber = Symb.getSectionNumber();
+  if (!COFF::isReservedSectionNumber(SectionNumber)) {
+    const coff_section *Section = nullptr;
+    if (std::error_code EC = getSection(SectionNumber, Section))
+      return EC;
 
-  if (Symb.getSectionNumber() == COFF::IMAGE_SYM_UNDEFINED) {
-    if (Symb.getValue() == 0)
-      Result = UnknownAddressOrSize;
-    else
-      Result = Symb.getValue();
-  } else if (Section) {
     Result = Section->SizeOfRawData - Symb.getValue();
-  } else {
-    Result = 0;
+    return object_error::success;
   }
 
+  Result = 0;
   return object_error::success;
 }
 

test/Object/coff-archive-short.test

@@ -5,7 +5,7 @@
 # than 15 characters, thus, unlike coff_archive.lib, it has no string
 # table as the third member.
 #
-RUN: llvm-nm --numeric-sort -M %p/Inputs/coff_archive_short.lib | FileCheck -check-prefix=CHECKIDX %s
+RUN: llvm-nm -a --numeric-sort -M %p/Inputs/coff_archive_short.lib | FileCheck -check-prefix=CHECKIDX %s
 
 CHECKIDX: Archive map
 CHECKIDX: _shortfn1 in short1.obj

test/Object/coff-archive.test

@@ -1,7 +1,7 @@
 #
 # Check if the index is appearing properly in the output file 
 #
-RUN: llvm-nm --numeric-sort -M %p/Inputs/coff_archive.lib | FileCheck -check-prefix=CHECKIDX %s
+RUN: llvm-nm -a --numeric-sort -M %p/Inputs/coff_archive.lib | FileCheck -check-prefix=CHECKIDX %s
 
 CHECKIDX: Archive map
 CHECKIDX: ??0invalid_argument@std@@QAE@PBD@Z in Debug\mymath.obj

test/Object/nm-archive.test

@@ -1,4 +1,4 @@
-RUN: llvm-nm %p/Inputs/archive-test.a-coff-i386 \
+RUN: llvm-nm -a %p/Inputs/archive-test.a-coff-i386 \
 RUN:         | FileCheck %s -check-prefix COFF
 
 COFF: trivial-object-test.coff-i386:
@@ -9,7 +9,7 @@ COFF-NEXT:          U _SomeOtherFunction
 COFF-NEXT: 00000000 T _main
 COFF-NEXT:          U _puts
 
-RUN: llvm-nm -o %p/Inputs/archive-test.a-coff-i386 \
+RUN: llvm-nm -a -o %p/Inputs/archive-test.a-coff-i386 \
 RUN:         | FileCheck %s -check-prefix COFF-o
 
 COFF-o: {{.*}}/archive-test.a-coff-i386:trivial-object-test.coff-i386: 00000000 d .data

test/Object/nm-trivial-object.test

@@ -1,6 +1,6 @@
-RUN: yaml2obj %p/Inputs/COFF/i386.yaml | llvm-nm - \
+RUN: yaml2obj %p/Inputs/COFF/i386.yaml | llvm-nm -a - \
 RUN:         | FileCheck %s -check-prefix COFF
-RUN: yaml2obj %p/Inputs/COFF/x86-64.yaml | llvm-nm - \
+RUN: yaml2obj %p/Inputs/COFF/x86-64.yaml | llvm-nm -a - \
 RUN:         | FileCheck %s -check-prefix COFF
 RUN: llvm-nm %p/Inputs/trivial-object-test.elf-i386 \
 RUN:         | FileCheck %s -check-prefix ELF
@@ -36,7 +36,7 @@ RUN: llvm-nm -p -a %p/Inputs/macho-hello-g.macho-x86_64 \
 RUN:         | FileCheck %s -check-prefix macho-pa
 RUN: llvm-nm -u %p/Inputs/macho-hello-g.macho-x86_64 \
 RUN:         | FileCheck %s -check-prefix macho-u
-RUN: llvm-nm -S %p/Inputs/common.coff-i386 \
+RUN: llvm-nm -S -a %p/Inputs/common.coff-i386 \
 RUN:         | FileCheck %s -check-prefix COFF-COMMON
 RUN: llvm-nm %p/Inputs/relocatable-with-section-address.elf-x86-64 \
 RUN:         | FileCheck %s -check-prefix ELF-SEC-ADDR64

tools/llvm-nm/llvm-nm.cpp

@@ -721,18 +721,14 @@ static char getSymbolNMTypeChar(COFFObjectFile &Obj, symbol_iterator I) {
     // Check section type.
     if (Characteristics & COFF::IMAGE_SCN_CNT_CODE)
       return 't';
-    else if (Characteristics & COFF::IMAGE_SCN_MEM_READ &&
-             ~Characteristics & COFF::IMAGE_SCN_MEM_WRITE) // Read only.
-      return 'r';
-    else if (Characteristics & COFF::IMAGE_SCN_CNT_INITIALIZED_DATA)
-      return 'd';
-    else if (Characteristics & COFF::IMAGE_SCN_CNT_UNINITIALIZED_DATA)
+    if (Characteristics & COFF::IMAGE_SCN_CNT_INITIALIZED_DATA)
+      return Characteristics & COFF::IMAGE_SCN_MEM_WRITE ? 'd' : 'r';
+    if (Characteristics & COFF::IMAGE_SCN_CNT_UNINITIALIZED_DATA)
       return 'b';
-    else if (Characteristics & COFF::IMAGE_SCN_LNK_INFO)
+    if (Characteristics & COFF::IMAGE_SCN_LNK_INFO)
       return 'i';
-
     // Check for section symbol.
-    else if (Symb.isSectionDefinition())
+    if (Symb.isSectionDefinition())
       return 's';
   }
 

tools/llvm-readobj/COFFDumper.cpp

@@ -857,7 +857,7 @@ void COFFDumper::printSymbol(const SymbolRef &Sym) {
       W.printHex("PointerToLineNumber", Aux->PointerToLinenumber);
       W.printHex("PointerToNextFunction", Aux->PointerToNextFunction);
 
-    } else if (Symbol.isWeakExternal()) {
+    } else if (Symbol.isAnyUndefined()) {
       const coff_aux_weak_external *Aux;
       if (error(getSymbolAuxData(Obj, Symbol, I, Aux)))
         break;

tools/obj2yaml/coff2yaml.cpp

@@ -161,7 +161,7 @@ void COFFDumper::dumpSymbols(unsigned NumSymbols) {
             reinterpret_cast<const object::coff_aux_bf_and_ef_symbol *>(
                 AuxData.data());
         dumpbfAndEfLineInfo(&Sym, ObjBES);
-      } else if (Symbol.isWeakExternal()) {
+      } else if (Symbol.isAnyUndefined()) {
         // This symbol represents a weak external definition.
         assert(Symbol.getNumberOfAuxSymbols() == 1 &&
                "Expected a single aux symbol to describe this weak symbol!");