[llvm-nm][ELF] - Make -D display symbol versions.

[grimar]

Summary

This fixes https://bugs.llvm.org/show_bug.cgi?id=48670.

Since binutils 2.35, nm -D displays symbol versions by default.
This patch teaches llvm-nm to do the same.

[grimar]

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

75

Note: I think we should print @@ for default versions and @ for hidden versions.
It is not what GNU nm does, but it would be more consistent with other tools that print versioned symbols.
I am going to post a follow-up patch for this.

[grimar]

• Format.

[jhenderson]

As our downstream toolchain doesn't use symbol versioning, I'm not really familiar enough with this to review the details (and don't have the time to read back up on it currently), so you'll need @MaskRay or someone else to give input too.

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

131

134

154

Adding "wrong" here felt wrong :-)

161

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

1718–1719

Seems like if you're not using Sym, this should be an older style for loop?

1788

Is the case actually necessary? I'm sure I've seen size_t X = -1 in other places before without warnings.

[grimar]

• Addressed review comments.

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

1718–1719

Done. Unfortunately I can't take size(), because Symbols is ELFObjectFileBase::elf_symbol_iterator_range.

1788

Yeah, looks like it is not needed.

[jhenderson]

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

153

Here the thing you can't read is the section, not the section entry, right? If so, delete the "entry" in this sentence.

[grimar]

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

153

The reported warning is:

# 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)

I.e. I can't read a particular entry because there is an issue with extracting all section entries.

The test case tries to trigger the following code in readSymbolVersionsELF:

if (!VerEntryOrErr)
  return createError("unable to read an entry with index " + Twine(I) +
                     " from " + describe(Obj, *SymVerSec) + ": " +
                     toString(VerEntryOrErr.takeError()));

[jhenderson]

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

153

The next line of this comment refers to "it" which would mean the unreadable "section entry" mentioned at the end of the previous comment, so you either need to change the first line to talk about the section, not the section entry, or the second line to not use "it" (e.g. "In this case, the section has...")

[grimar]

• Addressed review comment.

[jhenderson]

LGTM.

[grimar]

@MaskRay, are you OK with it?

[MaskRay]

LGTM.

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

146

Perhaps append {{$}} or use FileCheck --match-full-lines

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

1789–1790

Perhaps llvm::enumerate

[MaskRay]

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

1831

Sorry, I did not read carefully.

This is wrong. D94912 should just be folded into D94907. We should not have pushed D94907 leaving @ in the output where the correct output uses @@ and the legacy output does not use @ at all.

[MaskRay]

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

1831

This caused https://github.com/ClangBuiltLinux/linux/issues/1266 @nathanchance

Though it is probably not too urgent.

[grimar]

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

1831

I was under impression that GNU nm never prints @@. But seems it just doesn't do it for undefined symbols, and I guess I didn't try other cases previously.
Anyways I am going to look into this issue today.

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

FileHeader:
   Class:   ELFCLASS64
   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.
+
+# 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
+
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data:  ELFDATA2LSB
+  Type:  ET_DYN
+Sections:
+  - Name:    .gnu.version
+    Type:    SHT_GNU_versym
+    Flags:   [ SHF_ALLOC ]
+## 0x8000 is a special VERSYM_HIDDEN bit.
+    Entries: [ 0, 0, 1, [[VERSYMENTRY=2]], 0x8003, 4, 0x8005 ]
+    ShSize:  [[VERSYMSIZE=<none>]]
+  - Name:         .gnu.version_d
+    Type:         SHT_GNU_verdef
+    Flags:        [ SHF_ALLOC ]
+    Link:         .dynstr
+    AddressAlign: 0x4
+    Info:         0x2
+    ShOffset:     [[VERDEFOFFSET=<none>]]
+    Entries:
+      - VersionNdx: 2
+        Names:
+          - v2
+      - VersionNdx: 3
+        Names:
+          - v3hidden
+  - Name:  .gnu.version_r
+    Type:  SHT_GNU_verneed
+    Flags: [ SHF_ALLOC ]
+    Link:  .dynstr
+    Info:  0x2
+    Dependencies:
+      - Version: 1
+        File:    file1.so
+        Entries:
+          - Name:  v4
+            Hash:  0
+            Flags: 0
+            Other: 4
+      - Version: 1
+        File:    file2.0
+        Entries:
+          - Name:  v5hidden
+            Hash:  0
+            Flags: 0
+            Other: 5
+DynamicSymbols:
+  - Name: localversym
+  - Name: globalversym
+  - Name: version2sym
+  - Name: version3sym
+  - Name: version4sym
+  - Name: version5sym
+
+## 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
+
+# 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
+
+## Case 3: check we report a warning when we are unable to get a vesrion 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

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

if (FunctionStartsAdded) {
           Q += strlen(Q) + 1;
         }
         I++;
       }
     }
   }
 }
 
+template <class ELFT>
+static Expected<std::vector<std::string>>
+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>{};
+
+  Expected<SmallVector<Optional<VersionEntry>, 0>> MapOrErr =
+      Obj.loadVersionMap(SymVerNeedSec, SymVerDefSec);
+  if (!MapOrErr)
+    return MapOrErr.takeError();
+
+  std::vector<std::string> 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()));
+
+    bool IsDefault;
+    Expected<StringRef> VerOrErr = Obj.getSymbolVersionByIndex(
+        (*VerEntryOrErr)->vs_index, IsDefault, *MapOrErr);
+    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());
+  }
+
+  return Ret;
+}
+
+static Expected<std::vector<std::string>>
+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;
   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 =
+            readSymbolVersionsELF(*E, Symbols))
+      SymbolVersions = std::move(*VersionsOrErr);
+    else
+      WithColor::warning(errs(), ToolName)
+          << "unable to read symbol versions: "
+          << toString(VersionsOrErr.takeError()) << "\n";
   }
 
   // If a "-s segname sectname" option was specified and this is a Mach-O
   // file get the section number for that section in this object file.
   unsigned int Nsect = 0;
   MachOObjectFile *MachO = dyn_cast<MachOObjectFile>(&Obj);
   if (!SegSect.empty() && MachO) {
     Nsect = getNsectForSegSect(MachO);
     // If this section is not in the object file no symbols are printed.
     if (Nsect == 0)
       return;
   }
   if (!(MachO && DyldInfoOnly)) {
+    size_t I = -1;
     for (BasicSymbolRef Sym : Symbols) {
+      ++I;
       Expected<uint32_t> SymFlagsOrErr = Sym.getFlags();
       if (!SymFlagsOrErr) {
         error(SymFlagsOrErr.takeError(), Obj.getFileName());
         return;
       }
       if (!DebugSyms && (*SymFlagsOrErr & SymbolRef::SF_FormatSpecific))
         continue;
       if (WithoutAliases && (*SymFlagsOrErr & SymbolRef::SF_Indirect))
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];
+
       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
   // for the ones that are missing (also done with the -add-dyldinfo flag).