[PATCH] D68462: [llvm-readobj/llvm-readelf] - Add checks for GNU-style to "all.test" test case.
George Rimar via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 7 04:15:45 PDT 2019
grimar marked 14 inline comments as done.
grimar added inline comments.
================
Comment at: test/tools/llvm-readobj/all.test:27-28
+# GNU-ALL: Symbol table '.symtab' contains {{.*}} entries:
+# GNU-ALL: EH_FRAME Header [
+# GNU-ALL: Dynamic section at offset {{.*}} contains {{.*}} entries:
+# GNU-ALL: Program Headers:
----------------
jhenderson wrote:
> These two appear to be missing from the LLVM list above?
Yes. LLVM output seems to be inconsistent, incomplete and ugly sometimes. I'd review and refine it separatelly.
================
Comment at: test/tools/llvm-readobj/all.test:33-34
+# GNU-ALL: Version needs section '.gnu.version_r' contains {{.*}} entries:
+# GNU-ALL: COMDAT group section [ {{.*}}] `.group' [foo] contains {{.*}} sections:
+# GNU-ALL: Histogram for bucket list length (total of 1 buckets)
+# GNU-ALL: Displaying notes found at file offset {{.*}} with length {{.*}}:
----------------
jhenderson wrote:
> These two are also missing from the LLVM list.
The same.
================
Comment at: test/tools/llvm-readobj/all.test:49
+ Relocations:
+ - Name: .gnu.version
+ Type: SHT_GNU_versym
----------------
jhenderson wrote:
> I take it the version stuff is needed to make GNU mode print anything?
Yep, GNU style prints nothing if there is no version section.
See: https://github.com/llvm-mirror/llvm/blob/master/tools/llvm-readobj/ELFDumper.cpp#L3785
================
Comment at: test/tools/llvm-readobj/all.test:63
+ Entries: []
+ - Name: .group
+ Type: SHT_GROUP
----------------
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
================
Comment at: test/tools/llvm-readobj/all.test:75-76
+ Entries:
+ - Tag: DT_HASH
+ Value: 0x1100
+ - Tag: DT_NULL
----------------
jhenderson wrote:
> Can we just get away with the DT_NULL tag, or is DT_HASH required for the hash histogram behaviour?
> is DT_HASH required for the hash histogram behaviour?
Yes, we need it to get the `DT_HASH` content:
https://github.com/llvm-mirror/llvm/blob/master/tools/llvm-readobj/ELFDumper.cpp#L1712
================
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
----------------
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).
================
Comment at: test/tools/llvm-readobj/all.test:100
+## An arbitrary linker-generated valid content.
+ Content: 040000001000000003000000474E55004FCB712AA6387724A9F465A32CD8C14B
+Symbols:
----------------
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?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68462/new/
https://reviews.llvm.org/D68462
More information about the llvm-commits
mailing list