[PATCH] D92923: [llvm-readelf/obj][WIP] - Add support for multiple SHT_SYMTAB_SHNDX sections.

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

In D92923#2457199 <https://reviews.llvm.org/D92923#2457199>, @jhenderson wrote:

> Sorry, this somehow slipped off my radar, and I've run out of time to do any further significant reviewing today. I've skimmed it briefly, and the general approach looks fine. I'd like ideally to find a way to avoid needing to pass both a pointer and size around for the shndx table, even if it's just to wrap it in a new class, or change to `Optional<ArrayRef<...>>` or something. If you'd like me to take a more detailed look tomorrow, I can certainly do so.
>
> In D92923#2454368 <https://reviews.llvm.org/D92923#2454368>, @grimar wrote:
>
>> In D92923#2454328 <https://reviews.llvm.org/D92923#2454328>, @jhenderson wrote:
>>
>>> Sorry, this somehow slipped off my radar, and I've run out of time to do any further significant reviewing today. I've skimmed it briefly, and the general approach looks fine. I'd like ideally to find a way to avoid needing to pass both a pointer and size around for the shndx table, even if it's just to wrap it in a new class, or change to `Optional<ArrayRef<...>>` or something.
>>
>> Thanks for looking! I am planning to add missing test cases and will update the patch. We can discuss the possible change of signatures after that.
>> Initially the code had `ArrayRef<Elf_Word> ShndxTable`. In theory we can use it, but then we will have an empty (size == 0) `ArrayRef` with a non-zero data.
>> I am just not sure that doing it looks nice.
>
> Hmmm... that's a fair point. Maybe we should write our own class that simply wraps the pointer and size, and it could do the bounds checking itself, and will therefore know that 0 is special for size somehow?

This might look good. I'll try. Instead of `size == 0` it probably can have `Optional<>` size.


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

https://reviews.llvm.org/D92923