[PATCH] D71803: [llvm-nm] Display STT_GNU_IFUNC as 'i'

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: llvm/tools/llvm-nm/llvm-nm.cpp:1137
+  else {
+    if (elf_symbol_iterator(I)->getELFType() == ELF::STT_GNU_IFUNC)
+      return 'i';
----------------
MaskRay wrote:
> grimar wrote:
> > Why not to place the new logic inside `getSymbolNMTypeChar` ?
> > It would be consistent with `STB_GNU_UNIQUE` I think.
> Place the new logic inside getSymbolNMTypeChar will be more complex.
> 
> For both global and local symbols, the key is the lower case `i`...
> 
> 
Ok, that behavior makes sence I think.
What about handling the `STB_GNU_UNIQUE` here too?
(Because at this point this looks like it solves the same thing as `STB_GNU_UNIQUE`,
but in the different way, what looks inconsistent).

Also, would it be better to do the following?

```
  else if (WasmObjectFile *Wasm = dyn_cast<WasmObjectFile>(&Obj))
    Ret = getSymbolNMTypeChar(*Wasm, I);
  else if (ELFObjectFileBase *ELF = dyn_cast<ELFObjectFileBase>(&Obj)) {
   ...
  }
  else
   llvm_unreachable(...);
```

(for me it wasn't obvious that is is OK to use `elf_symbol_iterator(I)` before
`cast<ELFObjectFileBase>(Obj)`. It was not clear that iterator have the same safe cast
inside).



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71803