[PATCH] D88817: [llvm-readobj/elf] - Ignore the hash table when on EM_S390/EM_ALPHA platforms.

[James Henderson via Phabricator via llvm-commits]

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

My comment re. SONAME are unrelated to this patch. LGTM, with nit.



================
Comment at: llvm/test/tools/llvm-readobj/ELF/hash-table.test:47
+## Document that we ignore the sh_entsize value when dumping the hash section.
+## Implementation assumes that the size of entries is 4, this matches the ELF specification.
+
----------------



================
Comment at: llvm/test/tools/llvm-readobj/ELF/hash-table.test:69
+
+# NOWARN: warning: '[[FILE]]': string table was not found
+
----------------
grimar wrote:
> jhenderson wrote:
> > Seems like we shouldn't be getting this warning either, but that's presumably a separate change and more general. Perhaps add a TODO?
> We report the same warning below (--docnum=2). It happens because the code always tries to get the SOName string (`DT_SONAME`, which is `0` by default), but we have no `DT_STRTAB` tag. The way to fix it is to add it, but I am not sure we should, it is probably unrelated to what we test?
I agree it's unrelated here. I'm just thinking that we shouldn't report the warning at all, even if there is no SOName string unless it is actually required. At least for llvm-readelf output, the DT_SONAME tag isn't needed for anything in most cases (in llvm-readobj output I see there's the LoadName field, although I wonder whether we should simply not print things like that without some option being specified - but that's another issue again).


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

https://reviews.llvm.org/D88817