[PATCH] D68462: [llvm-readobj/llvm-readelf] - Add checks for GNU-style to "all.test" test case.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: test/tools/llvm-readobj/all.test:63
+        Entries: []
+  - Name: .group
+    Type: SHT_GROUP
----------------
grimar wrote:
> jhenderson wrote:
> > Do we need group information to print a header?
> Yes.
> Logic is:
> 1) Collect all group sections:
>     https://github.com/llvm-mirror/llvm/blob/master/tools/llvm-readobj/ELFDumper.cpp#L2842
> 2) Dump them (header is printed on this step):
>     https://github.com/llvm-mirror/llvm/blob/master/tools/llvm-readobj/ELFDumper.cpp#L2886
I see. Could you just check the "There are no section groups in the file" message instead? (https://github.com/llvm-mirror/llvm/blob/master/tools/llvm-readobj/ELFDumper.cpp#L2911)

I think that would be sufficient for this test case.


================
Comment at: test/tools/llvm-readobj/all.test:86-90
+  - Name:    .eh_frame_hdr
+    Type:    SHT_PROGBITS
+## An arbitrary linker-generated valid content.
+    Content: 011b033b140000000100000000f0ffff30000000
+  - Name:         .eh_frame
----------------
grimar wrote:
> jhenderson wrote:
> > Same comments as earlier. Can these be empty?
> No. We need to have something valid here, otherwise any
> error triggered will fail the dumping.
> (e.g. https://github.com/llvm-mirror/llvm/blob/master/tools/llvm-readobj/DwarfCFIEHPrinter.h#L127).
You don't actually need the .eh_frame_hdr at all, looking at the code, I think, just the .eh_frame section. That said, this appears to be different from GNU readelf.

For reference, GNU readelf prints "There are no unwind sections in this file" if there are no .eh_frame_hdr sections even if there is a .eh_frame section (not looked to see what happens if there is a PT_GNU_EH_FRAME program header).


================
Comment at: test/tools/llvm-readobj/all.test:100
+## An arbitrary linker-generated valid content.
+    Content: 040000001000000003000000474E55004FCB712AA6387724A9F465A32CD8C14B
+Symbols:
----------------
grimar wrote:
> jhenderson wrote:
> > This could probably just be an arbitrary note, and much simpler.
> Probably. But it is already short enough and I do not want to spend time on optimising it until we have a way to describe
> it with YAML. Having a raw content is anyways not optimal. What do you think?
Time to implement SHT_NOTE sections in yaml2obj :)

But happy for that to be later.


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

https://reviews.llvm.org/D68462