[PATCH] D93362: [llvm-elfabi] Support ELF file that lacks .gnu.hash section

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: llvm/include/llvm/Object/ELF.h:566
+  // Locate the end of the chain to find the last symbol index.
+  while ((*It & 1) == 0) {
+    LastSymIdx++;
----------------
jhenderson wrote:
> haowei wrote:
> > jhenderson wrote:
> > > Seems like this might enter an infinite loop, if there's a corrupt section without a 0 value at the end of the chain.
> > You are right, but what would be the best way to prevent that? I thought about to determine the max bucket capacity by calculating the differences between indexes in the bucket. However, it is not guaranteed that hashes will evenly distributed in the buckets. It also won't work if this is only one bucket.
> I know @grimar spent some time fixing loops in hash table code like this in llvm-readelf. Maybe he can provide some guidance there. If nothing else, we can at least make sure the hash table doesn't run off the end of the file.
My understanding is that this loop might either run past the end of the section or past the end of the file. It seems that saying "infinite loop" is not really correct then: it should crash sooner ol later, or return a wrong result.

There are 2 cases:
1) When we have a section header table, we can get the size of the hash table from there.
2) When we don't have a section header table, we have no information about the hash table size and only can check that we don't go past the end of the file I believe.

Perhaps, `getDynSymtabSizeFromGnuHash` could also accept one more argument, like `void *End` or something alike,
which will either point past the end of the section or past the end of the file. The code could check that no overrun happens then.

May be it worth just to pass both `Optional<uint64_t> SecSize` and `void *BufEnd` for better diagnostics.


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

https://reviews.llvm.org/D93362