[PATCH] D61117: Fix Bug 41353 - unique symbols printed as D instead of u

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: llvm/test/tools/llvm-nm/X86/unique.test:1
+# RUN: clang -c %p/Inputs/unique.s -o %t
+# RUN: llvm-nm %t | FileCheck %s
----------------
rupprecht wrote:
> Instead of clang, use `llvm-mc -filetype=obj -triple=x86_64-pc-linux` to directly handle the assembly
> 
> Actually, yaml2obj might be even better, now that it can handle unique symbols.
+1 to use yaml2obj.


================
Comment at: llvm/test/tools/llvm-nm/X86/unique.test:1
+# Fixes https://bugs.llvm.org/show_bug.cgi?id=41353
+# RUN: llvm-mc %s -filetype=obj -triple=x86_64-pc-linux -o %t.o
----------------
I would write in plain English what this test checks (instead of adding a link).
This is s more common way to describe what is going on in test and much more convenient for readers.


================
Comment at: llvm/tools/llvm-nm/llvm-nm.cpp:932
+  return ELFSym.getBinding() == ELF::STB_GNU_UNIQUE;
+}
+
----------------
Since `getSymbolNMTypeChar` starts from comment "// OK, this is ELF",
and has `ELFObjectFileBase` argument (i.e. it is called only for ELF)
I suggest to simplify and inline this method instead.
Something like:

```
case (ELF::SHF_TLS | ELF::SHF_ALLOC | ELF::SHF_WRITE):
case (ELF::SHF_ALLOC | ELF::SHF_WRITE):
  return ELFSymbolRef(Sym).getBinding() == ELF::STB_GNU_UNIQUE ? 'u' : 'd';
...
if ((Symflags & object::SymbolRef::SF_Global) && 
  ELFSymbolRef(Sym).getBinding() != ELF::STB_GNU_UNIQUE)
    Ret = toupper(Ret);

```


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61117/new/

https://reviews.llvm.org/D61117