[PATCH] D88561: [llvm-readobj] - Fix possible crashes related to dumping gnu hash symbols.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/hash-symbols.test:662
+
+## Case A: a hashed symbol can't be read, because of reading past the EOF attempt.
+# RUN: yaml2obj --docnum=11 -DVALUE=0x17 %s -o %t11.past.eof.so
----------------
It's not clear from this comment what is causing the attempt to read past the end of file. Please could you clarify.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/hash-symbols.test:672-673
+
+## Case B.1: a hash value entry can't be read because we have a broken value in the hash buckets array and trying to read
+##         a data past the end of the hash values array. The VALUE used is equal to the value from the case A minus 1.
+# RUN: yaml2obj --docnum=11 -DVALUE=0x16 %s -o %t11.value1.so
----------------



================
Comment at: llvm/test/tools/llvm-readobj/ELF/hash-symbols.test:679-680
+# BUCKET-READ-VALUE-A:      Num Buc:    Value          Size   Type   Bind Vis      Ndx Name
+## Note: we are trying to dump a dynamic symbol using a wrong index and so reading some arbitrary data as symbol data.
+##       That is why the following warning is reported.
+# BUCKET-READ-VALUE-A-NEXT: warning: '[[FILE]]': st_name (0x36) is past the end of the string table of size 0x5
----------------
Surely we should be diagnosing the attempt to read using an invalid dynamic symbol index in the first place, so that we don't see semi-random warnings?


================
Comment at: llvm/test/tools/llvm-readobj/ELF/hash-symbols.test:691
+# BUCKET-READ-VALUE-B:      Num Buc:    Value          Size   Type   Bind Vis      Ndx Name
+## Note: we are trying to dump a dynamic symbol using a wrong index and so reading some arbitrary data as symbol data.
+##       That is why the following warning is reported.
----------------
Ditto.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:4088
+            "unable to read the hash value: index (" + Twine(GnuHashable) +
+            ") goes past the end of the hash values array of size " +
+            Twine(Values.size())));
----------------
Perhaps this - "length" implies the number of entries in the array, whereas "size" could mean either that or the total size taken up by the array (e.g. 4 times the length, if the elements are 4 bytes in size each).


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

https://reviews.llvm.org/D88561