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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/tools/llvm-nm/llvm-nm.cpp:911-912
       return 'b';
+    if (Binding == ELF::STB_GNU_UNIQUE)
+      return 'u';
     if (Flags & ELF::SHF_ALLOC)
----------------
mmpozulp wrote:
> jhenderson wrote:
> > I don't know how realistic this is, but is this the right order of precedence for unique symbols, compared with GNU nm? In other words, do executable and NOBITS sections containing these symbols result in 't'/'b'?
> STB_GNU_UNIQUE precedence is high. I tested 5 different cases with GNU nm version 2.27-34.base.el7 to make sure llvm-nm agrees (see below), and added those tests to this patch. What do you think, @jhenderson?
> 
> ```
> --- !ELF
> FileHeader:    
>   Class:   ELFCLASS64
>   Data:    ELFDATA2LSB
>   Type:    ET_REL
>   Machine: EM_X86_64
> Sections:    
>   - Name:  .data
>     Type:  XXX
> Symbols:    
>   - Name:    foo 
>     Section: .data
>     Binding: STB_GNU_UNIQUE
> ```
> ```
> XXX             nm  llvm-nm
> SHT_PROGBITS    u   u
> SHT_NOBITS      u   u
> ```
> 
> ```
> --- !ELF
> FileHeader:    
>   Class:   ELFCLASS64
>   Data:    ELFDATA2LSB
>   Type:    ET_REL
>   Machine: EM_X86_64
> Sections:    
>   - Name:  .data
>     Type:  SHT_PROGBITS
>     Flags: [YYY]
> Symbols:    
>   - Name:    foo 
>     Section: .data
>     Binding: STB_GNU_UNIQUE
> ```
> ```
> YYY                     nm  llvm-nm
> SHF_EXECINSTR           u   u
> SHF_ALLOC               u   u
> SHF_ALLOC, SHF_WRITE    u   u
> ```
You may move the `'u'` check before `if (SecI != Obj.section_end()) {`

See
https://sourceware.org/git/?p=binutils-gdb.git;a=blob;hb=9d6d4be89d12747a92629ed1bde1d423e2831de1;f=bfd/syms.c#l699

The `if (SecI != Obj.section_end()) {` blocks roughly corresponds to `decode_section_type (symbol->section);`.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61117