Compute and Print Type and Section columns in "llvm-nm -f sysv" output. Round 2.

[Sunil_Srivastava]

Summary

This is a re-submission of https://reviews.llvm.org/D58263 or https://reviews.llvm.org/rL354833, for PR39998 and PR39999.

That checkin was reverted due to an Asan failure. This patch fixes that failure.

In the process of fixing the Asan failure, the overall patch has become much smaller, so I am submitting it as a separate review.

The major simplification vs D58263 is that the new virtual function introduced in in that has now been removed. The code in llvm-nm.cpp now relies on Symbol flags already computed earlier.

Other than that there are

1. minor stylistic changes, and
2. a test Object/X86/nm-print-size.s has been enhanced to test "-f sysv" output

[jhenderson]

tools/llvm-nm/llvm-nm.cpp

888–889

Looks like the indentation got messed up here?

1113

Could you add a comment explaining why we throw away the error rather than reporting it here, please?

[Sunil_Srivastava]

Replaced tab with spaces.

tools/llvm-nm/llvm-nm.cpp

888–889

Yes, there is a tab there. Fixing.

1113

I have copied the usage from other places in the same file. I am assuming that consumeError is handling the error.

[Sunil_Srivastava]

tools/llvm-nm/llvm-nm.cpp

1113

consumeError() is calling handleAllErrors which handles errors.

[Sunil_Srivastava]

tools/llvm-nm/llvm-nm.cpp

1113

I see now that consumeError is 'throwing away' errors in some sense.

This function, getNMSectionTagAndName is the rename of getNMTypeChar. getNMTypeChar calls getSymbolNMTypeChar for different kinds of object files.

Since I needed to do something special for ELF, I guarded my code for ELF, and mimicked the code in getSymbolNMTypeChar, the ELF version. getSymbolNMTypeChar for ELF (and also for COFF) has the same call to getSection and consumeError, without any comment indicating what they are doing.

If you have any suggestion as to what comment should go here, I will be happy to add it.

[jhenderson]

LGTM.

tools/llvm-nm/llvm-nm.cpp

1113

Ah, I missed that this is how it is handled elsewhere. No need to worry for now. I might suggest some error handling improvements in the future, but they should be wider and not part of this change then.

include/llvm/Object/ELFObjectFile.h

 #include "llvm/Support/MemoryBuffer.h"
 #include <cassert>
 #include <cstdint>
 #include <system_error>
 
 namespace llvm {
 namespace object {
 
+constexpr int NumElfSymbolTypes = 8;
+extern const llvm::EnumEntry<unsigned> ElfSymbolTypes[NumElfSymbolTypes];
+
 class elf_symbol_iterator;
 
 class ELFObjectFileBase : public ObjectFile {
   friend class ELFRelocationRef;
   friend class ELFSectionRef;
   friend class ELFSymbolRef;
 
 protected:
public:
 
   uint8_t getOther() const {
     return getObject()->getSymbolOther(getRawDataRefImpl());
   }
 
   uint8_t getELFType() const {
     return getObject()->getSymbolELFType(getRawDataRefImpl());
   }
+
+  StringRef getELFTypeName() const {
+    uint8_t Type = getELFType();
+    for (auto &EE : ElfSymbolTypes) {
+      if (EE.Value == Type) {
+        return EE.AltName;
+      }
+    }
+    return "";
+  }
 };
 
 class elf_symbol_iterator : public symbol_iterator {
 public:
   elf_symbol_iterator(const basic_symbol_iterator &B)
       : symbol_iterator(SymbolRef(B->getRawDataRefImpl(),
                                   cast<ELFObjectFileBase>(B->getObject()))) {}
 

lib/Object/ELFObjectFile.cpp

 #include <memory>
 #include <string>
 #include <system_error>
 #include <utility>
 
 using namespace llvm;
 using namespace object;
 
+const EnumEntry<unsigned> llvm::object::ElfSymbolTypes[NumElfSymbolTypes] = {
+    {"None", "NOTYPE", ELF::STT_NOTYPE},
+    {"Object", "OBJECT", ELF::STT_OBJECT},
+    {"Function", "FUNC", ELF::STT_FUNC},
+    {"Section", "SECTION", ELF::STT_SECTION},
+    {"File", "FILE", ELF::STT_FILE},
+    {"Common", "COMMON", ELF::STT_COMMON},
+    {"TLS", "TLS", ELF::STT_TLS},
+    {"GNU_IFunc", "IFUNC", ELF::STT_GNU_IFUNC}};
+
 ELFObjectFileBase::ELFObjectFileBase(unsigned int Type, MemoryBufferRef Source)
     : ObjectFile(Type, Source) {}
 
 template <class ELFT>
 static Expected<std::unique_ptr<ELFObjectFile<ELFT>>>
 createPtr(MemoryBufferRef Object) {
   auto Ret = ELFObjectFile<ELFT>::create(Object);
   if (Error E = Ret.takeError())

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

 // RUN: llvm-mc %s -o %t -filetype=obj -triple=x86_64-pc-linux
 // RUN: llvm-nm --print-size %t | FileCheck %s
+// RUN: llvm-nm -f sysv %t | FileCheck -check-prefix=SYSV %s
 
 // CHECK: 0000000000000000 ffffffffffffffff n a
 // CHECK: 0000000000000000 0000000000000000 N b
 // CHECK: 0000000000000004 0000000000000004 C c
 // CHECK: ffffffffffffffff 0000000000000000 a d
 
+// SYSV: a                   |0000000000000000|   n  |            NOTYPE|ffffffffffffffff|     |foo
+// SYSV: b                   |0000000000000000|   N  |            NOTYPE|0000000000000000|     |foo
+// SYSV: c                   |0000000000000004|   C  |            OBJECT|0000000000000004|     |*COM*
+// SYSV: d                   |ffffffffffffffff|   a  |            NOTYPE|0000000000000000|     |*ABS*
+
         .section foo
 a:
         .size a, 0xffffffffffffffff
 
         .global b
 b:
 
         .comm c,4,8
 
 d = 0xffffffffffffffff

test/tools/llvm-nm/X86/sysv-i386.test

+RUN: llvm-nm -f sysv %p/Inputs/hello.obj.elf-i386 
 RUN: llvm-nm -f sysv %p/Inputs/hello.obj.elf-i386 | FileCheck %s --strict-whitespace
 
 CHECK: Name                  Value   Class        Type         Size     Line  Section
-CHECK: .L.str              |00000000|   r  |                  |0000000f|     |
-CHECK: main                |00000000|   T  |                  |00000015|     |
-CHECK: puts                |        |   U  |                  |        |     |
+CHECK: .L.str              |00000000|   r  |            OBJECT|0000000f|     |.rodata.str1.1
+CHECK: main                |00000000|   T  |              FUNC|00000015|     |.text
+CHECK: puts                |        |   U  |            NOTYPE|        |     |*UND*

test/tools/llvm-nm/X86/sysv-x86_64.test

 RUN: llvm-nm -f sysv %p/Inputs/hello.obj.elf-x86_64 | FileCheck %s --check-prefix=ELF --strict-whitespace
 RUN: llvm-nm -f sysv %p/Inputs/hello.obj.macho-x86_64 2>&1 | FileCheck %s --check-prefix=MACHO --strict-whitespace
 
 ELF: Name                  Value           Class        Type         Size             Line  Section
-ELF: main                |0000000000000000|   T  |                  |0000000000000015|     |
-ELF: puts                |                |   U  |                  |                |     |
+ELF: main                |0000000000000000|   T  |              FUNC|0000000000000015|     |.text
+ELF: puts                |                |   U  |            NOTYPE|                |     |*UND*
 
 MACHO: Name                  Value           Class        Type         Size             Line  Section
 MACHO: EH_frame0           |0000000000000068|   s  |                  |0000000000000000|     |
 MACHO: L_.str              |000000000000003b|   s  |                  |0000000000000000|     |
 MACHO: _main               |0000000000000000|   T  |                  |0000000000000000|     |
 MACHO: _main.eh            |0000000000000080|   S  |                  |0000000000000000|     |
 MACHO: _printf             |                |   U  |                  |                |     |

tools/llvm-nm/llvm-nm.cpp

 }
 
 namespace {
 struct NMSymbol {
   uint64_t Address;
   uint64_t Size;
   char TypeChar;
   StringRef Name;
+  StringRef SectionName;
+  StringRef TypeName;
   BasicSymbolRef Sym;
   // The Sym field above points to the native symbol in the object file,
   // for Mach-O when we are creating symbols from the dyld info the above
   // pointer is null as there is no native symbol.  In these cases the fields
   // below are filled in to represent what would have been a Mach-O nlist
   // native symbol.
   uint32_t SymFlags;
   SectionRef Section;
if ((OutputFormat == darwin || FormatMachOasHex) && (MachO || Obj.isIR())) {
         } else
           outs() << I->IndirectName << ")";
       }
       outs() << "\n";
     } else if (OutputFormat == sysv) {
       std::string PaddedName(Name);
       while (PaddedName.length() < 20)
         PaddedName += " ";
+      std::string TypeName = I->TypeName;
+      if (TypeName.size() < 18)
+        TypeName = std::string(18-TypeName.size(), ' ') + TypeName;
       outs() << PaddedName << "|" << SymbolAddrStr << "|   " << I->TypeChar
-             << "  |                  |" << SymbolSizeStr << "|     |\n";
+             << "  |" << TypeName << "|" << SymbolSizeStr << "|     |"
+             << I->SectionName << "\n";
     }
   }
 
   SymbolList.clear();
 }
 
 static char getSymbolNMTypeChar(ELFObjectFileBase &Obj,
                                 basic_symbol_iterator I) {

 }
 
 static bool isObject(SymbolicFile &Obj, basic_symbol_iterator I) {
   return !dyn_cast<ELFObjectFileBase>(&Obj)
              ? false
              : elf_symbol_iterator(I)->getELFType() == ELF::STT_OBJECT;
 }
 
-static char getNMTypeChar(SymbolicFile &Obj, basic_symbol_iterator I) {
+// For ELF object files, Set TypeName to the symbol typename, to be printed
+// in the 'Type' column of the SYSV format output.
+static StringRef getNMTypeName(SymbolicFile &Obj, basic_symbol_iterator I) {
+  if (isa<ELFObjectFileBase>(&Obj)) {
+    elf_symbol_iterator SymI(I);
+    return SymI->getELFTypeName();
+  }
+  return "";
+}
+
+// Return Posix nm class type tag (single letter), but also set SecName and
+// section and name, to be used in format=sysv output.
+static char getNMSectionTagAndName(SymbolicFile &Obj, basic_symbol_iterator I,
+                                   StringRef &SecName) {
   uint32_t Symflags = I->getFlags();
+  if (isa<ELFObjectFileBase>(&Obj)) {
+    if (Symflags & object::SymbolRef::SF_Absolute)
+      SecName = "*ABS*";
+    else if (Symflags & object::SymbolRef::SF_Common)
+      SecName = "*COM*";
+    else if (Symflags & object::SymbolRef::SF_Undefined)
+      SecName = "*UND*";
+    else {
+      elf_symbol_iterator SymI(I);
+      Expected<elf_section_iterator> SecIOrErr = SymI->getSection();
+      if (!SecIOrErr) {
+        consumeError(SecIOrErr.takeError());
+        return '?';
+      }
+      elf_section_iterator secT = *SecIOrErr;
+      secT->getName(SecName);
+    }
+  }
+
   if ((Symflags & object::SymbolRef::SF_Weak) && !isa<MachOObjectFile>(Obj)) {
     char Ret = isObject(Obj, I) ? 'v' : 'w';
     return (!(Symflags & object::SymbolRef::SF_Undefined)) ? toupper(Ret) : Ret;
   }
 
   if (Symflags & object::SymbolRef::SF_Undefined)
     return 'U';
 
for (BasicSymbolRef Sym : Symbols) {
         SymbolRef SymRef(Sym);
         Expected<uint64_t> AddressOrErr = SymRef.getAddress();
         if (!AddressOrErr) {
           consumeError(AddressOrErr.takeError());
           break;
         }
         S.Address = *AddressOrErr;
       }
-      S.TypeChar = getNMTypeChar(Obj, Sym);
+      S.TypeName = getNMTypeName(Obj, Sym);
+      S.TypeChar = getNMSectionTagAndName(Obj, Sym, S.SectionName);
       std::error_code EC = Sym.printName(OS);
       if (EC && MachO)
         OS << "bad string index";
       else
         error(EC);
       OS << '\0';
       S.Sym = Sym;
       SymbolList.push_back(S);

tools/llvm-readobj/ELFDumper.cpp

static const EnumEntry<unsigned> ElfSymbolBindings[] = {
     {"Unique", "UNIQUE", ELF::STB_GNU_UNIQUE}};
 
 static const EnumEntry<unsigned> ElfSymbolVisibilities[] = {
     {"DEFAULT",   "DEFAULT",   ELF::STV_DEFAULT},
     {"INTERNAL",  "INTERNAL",  ELF::STV_INTERNAL},
     {"HIDDEN",    "HIDDEN",    ELF::STV_HIDDEN},
     {"PROTECTED", "PROTECTED", ELF::STV_PROTECTED}};
 
-static const EnumEntry<unsigned> ElfSymbolTypes[] = {
-    {"None",      "NOTYPE",  ELF::STT_NOTYPE},
-    {"Object",    "OBJECT",  ELF::STT_OBJECT},
-    {"Function",  "FUNC",    ELF::STT_FUNC},
-    {"Section",   "SECTION", ELF::STT_SECTION},
-    {"File",      "FILE",    ELF::STT_FILE},
-    {"Common",    "COMMON",  ELF::STT_COMMON},
-    {"TLS",       "TLS",     ELF::STT_TLS},
-    {"GNU_IFunc", "IFUNC",   ELF::STT_GNU_IFUNC}};
-
 static const EnumEntry<unsigned> AMDGPUSymbolTypes[] = {
   { "AMDGPU_HSA_KERNEL",            ELF::STT_AMDGPU_HSA_KERNEL }
 };
 
 static const char *getGroupType(uint32_t Flag) {
   if (Flag & ELF::GRP_COMDAT)
     return "COMDAT";
   else