[PATCH] D65537: Print reasonable representations of type names in llvm-nm, readelf and readobj.

[Sunil Srivastava via Phabricator via llvm-commits]

Sunil_Srivastava marked an inline comment as done.
Sunil_Srivastava added inline comments.


================
Comment at: test/tools/llvm-nm/format-sysv-type.test:44
 
-# CHECK:      os_specific   {{.*}}|                  |                |     |*UND*
-# CHECK-NEXT: proc_specific {{.*}}|                  |                |     |*UND*
-# CHECK-NEXT: symbol_common {{.*}}|            COMMON|                |     |*COM*
-# CHECK-NEXT: symbol_file   {{.*}}|              FILE|                |     |*UND*
-# CHECK-NEXT: symbol_func   {{.*}}|              FUNC|                |     |*UND*
-# CHECK-NEXT: symbol_ifunc  {{.*}}|             IFUNC|                |     |*UND*
-# CHECK-NEXT: symbol_notype {{.*}}|            NOTYPE|                |     |*UND*
-# CHECK-NEXT: symbol_obj    {{.*}}|            OBJECT|                |     |*UND*
-# CHECK-NEXT: symbol_tls    {{.*}}|               TLS|                |     |*UND*
-# CHECK-NEXT: unknown       {{.*}}|                  |                |     |*UND*
+# CHECK:      os_specific_10      |                |   U  |             IFUNC|                |     |*UND*
+# CHECK-NEXT: os_specific_11      |                |   U  | <OS specific>: 11|                |     |*UND*
----------------
Sunil_Srivastava wrote:
> jhenderson wrote:
> > jhenderson wrote:
> > > When looking at this offline, I missed this. It isn't great that we have IFUNC printed here, because not all ABIs support such symbol types and it actually makes us less compatible with GNU nm. I think we should change the table to print "<OS specific>: 10" here, and put a (hopefully) temporary patch in llvm-readobj to convert that particular case to IFUNC, pending the resolution of https://bugs.llvm.org/show_bug.cgi?id=42686.
> > Sorry, got mixed up. This comment should probably be on the readelf test, not the llvm-nm test. Please double-check versus GNU readelf though!
> OK. Will update the proposal after checking GNU readelf.
To clarify:

For type 10,
  - GNU readelf prints IFUNC. So does llvm-nm with this proposal.
  - GNU "nm --format=sysv" prints "<OS specific>: 10". This proposal prints IFUNC.

So you want llvm-nm to print "<OS specific>: 10" , at least for now. Right ? That will change llvm-nm test.

Looking at the code of both, it is easier to leave the ElfSymbolTypes table with IFUNC and make a special case in llvm-nm.cpp.

Will wait for confirmation before making change.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65537