[PATCH] D64750: [llvm-readelf] - Remove the precompiled binary from gnu-hash-symbols.test.

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 16 02:26:16 PDT 2019


jhenderson added inline comments.


================
Comment at: test/tools/llvm-readobj/elf-hash-symbols.test:4
+# RUN: yaml2obj --docnum=1 %s -o %t1-386.o
+# RUN: llvm-readelf --hash-symbols %t1-386.o | FileCheck %s --check-prefix HASH-I386
+
----------------
I wonder if we should add --strict-whitespace/--match-full-lines to this. That will ensure spacing is correct. What do you think?


================
Comment at: test/tools/llvm-readobj/elf-hash-symbols.test:20
+# HASH-I386-NEXT:     5   2: 00001001     0 NOTYPE  WEAK   DEFAULT   1 bbb
+
+--- !ELF
----------------
I think you need to test that there's nothing after the last line, to show that there are no more entries in .gnu.hash.


================
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
----------------
Perhaps you should explain what 0x2C is (I think it's the size of .hash, right?)


================
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
----------------
Do you think there's value for testing the case with exactly one of .hash or .gnu.hash?


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

https://reviews.llvm.org/D64750





More information about the llvm-commits mailing list