[PATCH] D70826: [llvm-readobj/llvm-readelf] - Reimplement dumping of the SHT_GNU_verneed section.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/elf-verneed-invalid.test:92
 # GNU-NOLINK-NEXT:  Addr: 0000000000000000  Offset: 0x000044  Link: 0 ()
-# GNU-NOLINK-NEXT:   0x0000: Version: 1  File: <invalid>  Cnt: 1
-# GNU-NOLINK-NEXT:   0x0010:   Name: <invalid>  Flags: none Version: 2
+# GNU-NOLINK-EMPTY:
+# GNU-NOLINK-NEXT:  warning: '[[FILE]]': invalid string table linked to SHT_GNU_verneed section with index 2: invalid sh_type for string table section [index 0]: expected SHT_STRTAB, but got SHT_NULL
----------------
grimar wrote:
> jhenderson wrote:
> > No need for this EMPTY line.
> It comes from `reportWarning`:
> https://github.com/llvm-mirror/llvm/blob/master/tools/llvm-readobj/llvm-readobj.cpp#L393
> 
> And we also print an additional '\n' when reporting errors:
> https://github.com/llvm-mirror/llvm/blob/master/tools/llvm-readobj/llvm-readobj.cpp#L369
> 
> I guess removing might cause issues like "error/warning is printed on the same like with the
> normal output",  but haven't tried yet.
> 
> What do you think if I'll try to remove it in a follow-up?
> (I had plan to investigate this issue to address the similar comment for D70665 anyways).
Ah, okay. It might be nice to remove it, but if it's not sensible, it's fine to leave it completely. I'm happy for the attempt to be a follow-up.


================
Comment at: llvm/test/tools/llvm-readobj/elf-verneed-invalid.test:372
+    ShOffset: 0xFFFFFFFF
+    Dependencies:
+      - Version: 1
----------------
grimar wrote:
> jhenderson wrote:
> > Ditto
> Done.
Huh, just realised: how come 0xFFFFFFFF + 0 can't be represented?


================
Comment at: llvm/test/tools/llvm-readobj/elf-verneed-invalid.test:396-397
+    ShOffset: 0xFFFFFFFF
+DynamicSymbols:
+  - Name: foo
+
----------------
I believe this can't be removed because you need a .dynstr, right? Can you add a comment please.


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

https://reviews.llvm.org/D70826