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

Mon Oct 12 02:11:41 PDT 2020

grimar marked an inline comment as done.
grimar added inline comments.

================
Comment at: llvm/test/tools/llvm-readobj/ELF/hash-table.test:69
+
+# NOWARN: warning: '[[FILE]]': string table was not found
+
----------------
jhenderson wrote:
> 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).
Perhaps we can adjust the logic of `llvm-readobj` to stop printing `LoadName` field when there is no
`DT_SONAME` tag. And we also can delay getting the `SOName` until that moment.
With it `llvm-readelf` will never report a warning, as it does not use the `SOName` at all and `llvm-readobj`s output will be probably more reasonable even without adding any additional options.

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

https://reviews.llvm.org/D88817