[PATCH] D64750: [llvm-readelf] - Remove the precompiled binary from gnu-hash-symbols.test.
Fangrui Song via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 16 03:05:39 PDT 2019
MaskRay added inline comments.
================
Comment at: test/tools/llvm-readobj/elf-hash-symbols.test:3
+
+# RUN: yaml2obj --docnum=1 %s -o %t1-386.o
+# RUN: llvm-readelf --hash-symbols %t1-386.o | FileCheck %s --check-prefix HASH-I386
----------------
`%t1-386.o` -> `%t1-386.so`
It is weird to name an `ET_DYN` `*.o`
================
Comment at: test/tools/llvm-readobj/elf-hash-symbols.test:48
+ - Tag: DT_GNU_HASH
+## 0x2C + PT_LOAD's p_offset == offset of .gnu.hash
+ Value: 0x000000000000002C
----------------
jhenderson wrote:
> Perhaps you should explain what 0x2C is (I think it's the size of .hash, right?)
> PT_LOAD's p_offset
Nit: I think the usage here is a bit unusual. Normally a `PT_DYNAMIC` is created to contain `.dynamic` and at runtime the rtld decodes `.dynamic` entries by finding the `PT_DYNAMIC` address.
Maybe you can create a PT_DYNAMIC to make the test look more reasonable?
If you decide to do that, delete `- Section: .dynamic` from the `PT_LOAD`.
================
Comment at: test/tools/llvm-readobj/elf-hash-symbols.test:78
+
+## Show that if there are no hash sections, we do not print anything.
+# RUN: yaml2obj --docnum=2 %s -o %t2.o
----------------
jhenderson wrote:
> Do you think there's value for testing the case with exactly one of .hash or .gnu.hash?
I think it is fine not to test that...
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64750/new/
https://reviews.llvm.org/D64750
More information about the llvm-commits
mailing list