[llvm-nm][ELF] - Use @@ prefix when printing default versions.

[grimar]

Summary

llvm-readelf prints default versions with @@ prefix.
This patch does the same for llvm-nm.

Depends on https://reviews.llvm.org/D95219

[MaskRay]

llvm/test/tools/llvm-nm/dynamic.test

64–65

A default version (@@) is only available for defined symbols.

Undefined symbols should use @ everywhere.

(I reported a similar bug to GNU readelf --dyn-syms some time last year)

[jhenderson]

Seems reasonable in principle.

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

1689

I was under the impression that anonymous namespaces were purely for hiding structs and classes, and not functions, based on the coding standards?

[grimar]

llvm/test/tools/llvm-nm/dynamic.test

64–65

Seems we have an issue with llvm-readelf too then (with ELFFile<ELFT>::getSymbolVersionByIndex, which returns IsDefault actually).
It should be fixed first, I'll take a look a bit later.

[grimar]

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

1689

True (https://llvm.org/docs/CodingStandards.html#anonymous-namespaces). Fixed.

[grimar]

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

1689

Posted this a bit too early. Will be fixed in update.

[grimar]

• Addressed review comments.

[MaskRay]

Looks great!

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

1691

StringRef can be used.

[grimar]

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

1691

No, because this string comes from

Expected<SmallVector<Optional<VersionEntry>, 0>> MapOrErr =
    Obj.loadVersionMap(SymVerNeedSec, SymVerDefSec);

which internally builds a string.

llvm/test/tools/llvm-nm/dynamic.test

FileHeader:
   Data:    ELFDATA2LSB
   Type:    ET_DYN
   Machine: EM_X86_64
 Sections:
   - Name: .dynsym
     Type: SHT_DYNSYM
     Size: 0
 
 ## Check we print symbol versions, when they are available.
+## A default version is one that is contained in the version table (SHT_GNU_versym)
+## and only available for defined symbols. Check we use the "@@" prefix to print it.
 
-# RUN: yaml2obj --docnum=4 %s -o %t4.o
-# RUN: llvm-nm --dynamic %t4.o 2>&1 | \
-# RUN:   FileCheck %s -DFILE=%t4.o --check-prefix=VERSIONED-SYMS
-
-# VERSIONED-SYMS:      U globalversym
-# VERSIONED-SYMS-NEXT: U localversym
-# VERSIONED-SYMS-NEXT: U version2sym@v2
-# VERSIONED-SYMS-NEXT: U version3sym@v3hidden
-# VERSIONED-SYMS-NEXT: U version4sym@v4
-# VERSIONED-SYMS-NEXT: U version5sym@v5hidden
+# RUN: yaml2obj --docnum=4 %s -o %t4-undef.o
+# RUN: llvm-nm --dynamic %t4-undef.o 2>&1 | \
+# RUN:   FileCheck %s -DFILE=%t4-undef.o -DTYPE=U \
+# RUN:     --check-prefixes=VERSIONED-SYMS,VERSIONED-UNDEF-SYMS
+
+# RUN: yaml2obj --docnum=4 -DINDEX=0x1 %s -o %t4-def.o
+# RUN: llvm-nm --dynamic %t4-def.o 2>&1 | \
+# RUN:   FileCheck %s -DFILE=%t4-def.o -DTYPE=r \
+# RUN:     --check-prefixes=VERSIONED-SYMS,VERSIONED-DEF-SYMS
+
+#            VERSIONED-SYMS: [[TYPE]] globalversym
+#       VERSIONED-SYMS-NEXT: [[TYPE]] localversym
+# VERSIONED-UNDEF-SYMS-NEXT: [[TYPE]] version2sym@v2
+#   VERSIONED-DEF-SYMS-NEXT: [[TYPE]] version2sym@@v2
+#       VERSIONED-SYMS-NEXT: [[TYPE]] version3sym@v3hidden
+#       VERSIONED-SYMS-NEXT: [[TYPE]] version4sym@v4
+#       VERSIONED-SYMS-NEXT: [[TYPE]] version5sym@v5hidden
 
 --- !ELF
 FileHeader:
   Class: ELFCLASS64
   Data:  ELFDATA2LSB
   Type:  ET_DYN
 Sections:
   - Name:    .gnu.version
Dependencies:
             Other: 4
       - Version: 1
         File:    file2.0
         Entries:
           - Name:  v5hidden
             Hash:  0
             Flags: 0
             Other: 5
+  - Name:    .dynsym
+    Type:    SHT_DYNSYM
+    EntSize: [[ENTSIZE=<none>]]
 DynamicSymbols:
-  - Name: localversym
-  - Name: globalversym
-  - Name: version2sym
-  - Name: version3sym
-  - Name: version4sym
-  - Name: version5sym
+  - Name:  localversym
+    Index: [[INDEX=<none>]]
+  - Name:  globalversym
+    Index: [[INDEX=<none>]]
+  - Name:  version2sym
+    Index: [[INDEX=<none>]]
+  - Name:  version3sym
+    Index: [[INDEX=<none>]]
+  - Name:  version4sym
+    Index: [[INDEX=<none>]]
+  - Name:  version5sym
+    Index: [[INDEX=<none>]]
 
 ## In the following cases we check we report warnings when unable to read symbol version.
 ## Check that we still print unversioned symbol names.
 
 ## Case 1: check we report a warning when unable to read symbol versions
 ## from a broken SHT_GNU_verdef section. In this case its sh_offset
 ## field has a too large value that goes past the EOF.
 
 # RUN: yaml2obj --docnum=4 -DVERDEFOFFSET=0xffffffff %s -o %t4-broken-verdef.o
 # RUN: llvm-nm --dynamic %t4-broken-verdef.o 2>&1 | \
-# RUN:   FileCheck %s -DFILE=%t4-broken-verdef.o --check-prefixes=VERSION-ERR,VERSION-ERR1
+# RUN:   FileCheck %s --check-prefixes=VERSION-ERR,VERSION-ERR1
 
 # VERSION-ERR1: warning: unable to read symbol versions: cannot read content of SHT_GNU_verdef section with index 2: section [index 2] has a sh_offset (0xffffffff) + sh_size (0x38) that is greater than the file size (0x438)
 # VERSION-ERR2: warning: unable to read symbol versions: unable to read an entry with index 1 from SHT_GNU_versym section with index 1: section [index 1] has an invalid sh_size (255) which is not a multiple of its sh_entsize (2)
 # VERSION-ERR3: warning: unable to read symbol versions: unable to get a version for entry 3 of SHT_GNU_versym section with index 1: SHT_GNU_versym section refers to a version index 255 which is missing
 
 # VERSION-ERR-NEXT: U globalversym{{$}}
 # VERSION-ERR-NEXT: U localversym{{$}}
 # VERSION-ERR-NEXT: U version2sym{{$}}
 # VERSION-ERR-NEXT: U version3sym{{$}}
 # VERSION-ERR-NEXT: U version4sym{{$}}
 # VERSION-ERR-NEXT: U version5sym{{$}}
 
 ## Case 2: check we report a warning when we are unable to read a SHT_GNU_versym section entry.
 ## In this case, the section has a size that is not a multiple of its sh_entsize.
 
 # RUN: yaml2obj --docnum=4 -DVERSYMSIZE=0xff %s -o %t4-broken-versym.o
 # RUN: llvm-nm --dynamic %t4-broken-versym.o 2>&1 | \
-# RUN:   FileCheck %s -DFILE=%t4-broken-versym.o --check-prefixes=VERSION-ERR,VERSION-ERR2
+# RUN:   FileCheck %s --check-prefixes=VERSION-ERR,VERSION-ERR2
 
-## Case 3: check we report a warning when we are unable to get a vesrion for a SHT_GNU_versym section entry.
+## Case 3: check we report a warning when we are unable to get a version for a SHT_GNU_versym section entry.
 ## In this case the SHT_GNU_versym section refers to a version index 255 which is missing.
 
 # RUN: yaml2obj --docnum=4 -DVERSYMENTRY=0xff %s -o %t4-broken-index.o
 # RUN: llvm-nm --dynamic %t4-broken-index.o 2>&1 | \
-# RUN:   FileCheck %s -DFILE=%t4-broken-index.o --check-prefixes=VERSION-ERR,VERSION-ERR3
+# RUN:   FileCheck %s --check-prefixes=VERSION-ERR,VERSION-ERR3
+
+## Case 4: check we report a warning when we are unable to get symbol flags.
+## In this case the dynamic symbol table has a wrong sh_entsize and we can't read a symbol.
+
+# RUN: yaml2obj --docnum=4 -DENTSIZE=0xff %s -o %t4-broken-dynsym.o
+# RUN: not llvm-nm --dynamic %t4-broken-dynsym.o 2>&1 | \
+# RUN:   FileCheck %s -DFILE=%t4-broken-dynsym.o --check-prefix=VERSION-ERR4
+
+# VERSION-ERR4: warning: unable to read symbol versions: unable to read flags for symbol with index 1: section [index 4] has invalid sh_entsize: expected 24, but got 255
+# VERSION-ERR4: error: [[FILE]] section [index 4] has invalid sh_entsize: expected 24, but got 255

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

if (FunctionStartsAdded) {
           Q += strlen(Q) + 1;
         }
         I++;
       }
     }
   }
 }
 
+namespace {
+struct SymbolVersion {
+  std::string Name;
+  bool IsDefault;
+};
+}; // namespace
+
 template <class ELFT>
-static Expected<std::vector<std::string>>
+static Expected<std::vector<SymbolVersion>>
 readSymbolVersionsELF(const ELFFile<ELFT> &Obj, StringRef FileName,
                       ELFObjectFileBase::elf_symbol_iterator_range Symbols) {
   using Elf_Shdr = typename ELFT::Shdr;
 
   // We called sections() earlier, so can't fail here.
   typename ELFT::ShdrRange SectionsOrErr = cantFail(Obj.sections());
   const Elf_Shdr *SymVerSec = nullptr;
   const Elf_Shdr *SymVerNeedSec = nullptr;
   const Elf_Shdr *SymVerDefSec = nullptr;
   for (const Elf_Shdr &Sec : SectionsOrErr) {
     if (Sec.sh_type == ELF::SHT_GNU_versym)
       SymVerSec = &Sec;
     else if (Sec.sh_type == ELF::SHT_GNU_verdef)
       SymVerDefSec = &Sec;
     else if (Sec.sh_type == ELF::SHT_GNU_verneed)
       SymVerNeedSec = &Sec;
   }
 
   if (!SymVerSec)
-    return std::vector<std::string>{};
+    return std::vector<SymbolVersion>{};
 
   Expected<SmallVector<Optional<VersionEntry>, 0>> MapOrErr =
       Obj.loadVersionMap(SymVerNeedSec, SymVerDefSec);
   if (!MapOrErr)
     return MapOrErr.takeError();
 
-  std::vector<std::string> Ret;
+  std::vector<SymbolVersion> Ret;
   size_t I = 0;
   for (auto It = Symbols.begin(), E = Symbols.end(); It != E; ++It) {
     ++I;
     Expected<const typename ELFT::Versym *> VerEntryOrErr =
         Obj.template getEntry<typename ELFT::Versym>(*SymVerSec, I);
     if (!VerEntryOrErr)
       return createError("unable to read an entry with index " + Twine(I) +
                          " from " + describe(Obj, *SymVerSec) + ": " +
                          toString(VerEntryOrErr.takeError()));
 
+    Expected<uint32_t> FlagsOrErr = It->getFlags();
+    if (!FlagsOrErr)
+      return createError("unable to read flags for symbol with index " +
+                         Twine(I) + ": " + toString(FlagsOrErr.takeError()));
+
     bool IsDefault;
     Expected<StringRef> VerOrErr = Obj.getSymbolVersionByIndex(
-        (*VerEntryOrErr)->vs_index, IsDefault, *MapOrErr, /*IsSymHidden=*/None);
+        (*VerEntryOrErr)->vs_index, IsDefault, *MapOrErr,
+        (*FlagsOrErr) & SymbolRef::SF_Undefined);
     if (!VerOrErr)
       return createError("unable to get a version for entry " + Twine(I) +
                          " of " + describe(Obj, *SymVerSec) + ": " +
                          toString(VerOrErr.takeError()));
 
-    Ret.push_back((*VerOrErr).str());
+    Ret.push_back({(*VerOrErr).str(), IsDefault});
   }
 
   return Ret;
 }
 
-static Expected<std::vector<std::string>>
+static Expected<std::vector<SymbolVersion>>
 readSymbolVersionsELF(const ELFObjectFileBase &Obj,
                       ELFObjectFileBase::elf_symbol_iterator_range Symbols) {
   if (const auto *ELF = dyn_cast<ELF32LEObjectFile>(&Obj))
     return readSymbolVersionsELF(ELF->getELFFile(), Obj.getFileName(), Symbols);
   else if (const auto *ELF = dyn_cast<ELF32BEObjectFile>(&Obj))
     return readSymbolVersionsELF(ELF->getELFFile(), Obj.getFileName(), Symbols);
   else if (const auto *ELF = dyn_cast<ELF64LEObjectFile>(&Obj))
     return readSymbolVersionsELF(ELF->getELFFile(), Obj.getFileName(), Symbols);
   return readSymbolVersionsELF(cast<ELF64BEObjectFile>(&Obj)->getELFFile(),
                                Obj.getFileName(), Symbols);
 }
 
 static void dumpSymbolNamesFromObject(SymbolicFile &Obj, bool printName,
                                       StringRef ArchiveName = {},
                                       StringRef ArchitectureName = {}) {
   auto Symbols = Obj.symbols();
-  std::vector<std::string> SymbolVersions;
+  std::vector<SymbolVersion> SymbolVersions;
   if (DynamicSyms) {
     const auto *E = dyn_cast<ELFObjectFileBase>(&Obj);
     if (!E) {
       error("File format has no dynamic symbol table", Obj.getFileName());
       return;
     }
     Symbols = E->getDynamicSymbolIterators();
 
-    if (Expected<std::vector<std::string>> VersionsOrErr =
+    if (Expected<std::vector<SymbolVersion>> VersionsOrErr =
             readSymbolVersionsELF(*E, Symbols))
       SymbolVersions = std::move(*VersionsOrErr);
     else
       WithColor::warning(errs(), ToolName)
           << "unable to read symbol versions: "
           << toString(VersionsOrErr.takeError()) << "\n";
   }
 
for (BasicSymbolRef Sym : Symbols) {
       raw_string_ostream OS(S.Name);
       if (Error E = Sym.printName(OS)) {
         if (MachO) {
           OS << "bad string index";
           consumeError(std::move(E));
         } else
           error(std::move(E), Obj.getFileName());
       }
-      if (!SymbolVersions.empty() && !SymbolVersions[I].empty())
-        S.Name += "@" + SymbolVersions[I];
+      if (!SymbolVersions.empty() && !SymbolVersions[I].Name.empty())
+        S.Name +=
+            (SymbolVersions[I].IsDefault ? "@@" : "@") + SymbolVersions[I].Name;
 
       S.Sym = Sym;
       SymbolList.push_back(S);
     }
   }
 
   // If this is a Mach-O file where the nlist symbol table is out of sync
   // with the dyld export trie then look through exports and fake up symbols