[llvm-nm] Fix for BZ41711 - Class character for a symbol with undefined binding does not match class assigned by GNU nm

[chrisjackson]

Summary

A symbol with a binding of 0x5 that is present in a .text section is assigned the class character 'T'. However, gnu-nm assigns the character '?'. I have added logic that mimics that present in GNU nm's bfd_decode_symclass() from bfd\symc.c.

See 41711.

The associated test was XFailing, but I also had to make a slight correction of the symbol names.

[mtrent]

Respectfully, I am not a good reviewer for ELF-specific file format changes. I assume you added me as a proxy for 'enderby', who touched some of the lines of this routine. Looking at the SVN history I believe Kevin Enderby's involvement was limited to promulgating Lang Hames' "Expected<>" as the preferred error handling idiom in libObject and friends. So, again, respectfully, neither Kevin nor I are what I would consider good, authoritative reviewers for ELF.

The changes here look reasonable to me from a generic programming / code flow point-of-view. If you need something domain specific, I suggest you look elsewhere?

[MaskRay]

LG, but you may want to wait for other reviewers' opinions.

[jhenderson]

tools/llvm-nm/llvm-nm.cpp

901

My immediate thought when looking at the code here was "what about STB_WEAK"? I know this is handled elsewhere, so maybe it's worth an assert that the binding isn't STB_WEAK, along with a comment explaining why it can't be here.

I also wonder about symbols in the STB_LOOS/STB_HIOS ranges STB_LOPROC/STB_HIPROC. Does GNU in these cases always emit '?'? Please add test-cases for these.

[chrisjackson]

Tidied up the test and added some additional cases. Added an assert explaining the tested binding values.

[jhenderson]

LGTM.

[chrisjackson]

This was closed with commit 41e20d210, svn r364559.

test/tools/llvm-nm/format-sysv-binding.test

-# XFAIL: *
-# For a symbol in a text section the class character for an unrecognised binding
-# value is '?' in gnu-nm but llvm-nm prints 'T'. Filed as:
-# https://bugs.llvm.org/show_bug.cgi?id=41711
-
-# RUN: yaml2obj %s > %t.o
+# RUN: yaml2obj %s -o %t.o
 # RUN: llvm-nm %t.o --format=sysv | FileCheck %s
 
 !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_REL
   Machine: EM_X86_64
 Sections:
   - Name:  .text
     Type:  SHT_PROGBITS
     Flags: [ SHF_ALLOC, SHF_EXECINSTR ]
 Symbols:
-  - Name:    symbol_local
+  - Name:    local_binding
     Binding: STB_LOCAL
     Section: .text
-    Value:   0x101
-    Size:    8
-  - Name:    symbol_weak
+    Value:   0xbeef
+  - Name:    global_binding
+    Binding: STB_GLOBAL
+    Section: .text
+  - Name:    weak_binding
     Binding: STB_WEAK
     Section: .text
-    Value:   0x102
-    Size:    4
-  - Name:    symbol_undefined_binding
+  - Name:    unrecognised_binding
     Binding: 5
     Section: .text
-    Value:   0x1004
-    Size:    32
-  - Name:    symbol_global
-    Binding: STB_GLOBAL
+  - Name:    gnu_unique_binding
+    Binding: 10
+    Section: .text
+  - Name:    os_binding
+    Binding: 11
+    Section: .text
+  - Name:    proc_binding
+    Binding: 14
     Section: .text
-    Value:   0x103
-    Size:    16
 
-# CHECK:      symbol_global {{.*}}|   T  |
-# CHECK-NEXT: symbol_local  {{.*}}|   t  |
-# CHECK-NEXT: symbol_unrecognised_binding {{.*}}| ? |
-# CHECK-NEXT: symbol_weak {{.*}}|   W  |
+# CHECK:      global_binding      {{.*}}|   T  |
+# CHECK-NEXT: gnu_unique_binding  {{.*}}|   u  |
+# CHECK-NEXT: local_binding       {{.*}}|   t  |
+# CHECK-NEXT: os_binding          {{.*}}|   ?  |
+# CHECK-NEXT: proc_binding        {{.*}}|   ?  |
+# CHECK-NEXT: unrecognised_binding{{.*}}|   ?  |
+# CHECK-NEXT: weak_binding        {{.*}}|   W  |

tools/llvm-nm/llvm-nm.cpp

static char getSymbolNMTypeChar(ELFObjectFileBase &Obj,
   elf_symbol_iterator SymI(I);
 
   Expected<elf_section_iterator> SecIOrErr = SymI->getSection();
   if (!SecIOrErr) {
     consumeError(SecIOrErr.takeError());
     return '?';
   }
 
-  if (SymI->getBinding() == ELF::STB_GNU_UNIQUE)
+  uint8_t Binding = SymI->getBinding();
+  if (Binding == ELF::STB_GNU_UNIQUE)
     return 'u';
 
+  assert(Binding != ELF::STB_WEAK && "STB_WEAK not tested in calling function");
+  if (Binding != ELF::STB_GLOBAL && Binding != ELF::STB_LOCAL)
+    return '?';
+
   elf_section_iterator SecI = *SecIOrErr;
   if (SecI != Obj.section_end()) {
     uint32_t Type = SecI->getType();
     uint64_t Flags = SecI->getFlags();
     if (Flags & ELF::SHF_EXECINSTR)
       return 't';
     if (Type == ELF::SHT_NOBITS)
       return 'b';