This is an archive of the discontinued LLVM Phabricator instance.

[llvm-readobj/elf] - Change the behavior of handing DT_SONAME.
ClosedPublic

Authored by grimar on Oct 14 2020, 4:41 AM.

Details

Summary

The current situation/behavior is:

  1. llvm-readelf doesn't need a string that is specified by DT_SONAME.
  2. llvm-readobj/elf always tries to read it, even when there is no DT_SONAME tag.
  3. Because of that both tools reports a warning for many our test cases.

This patch delays getting a SOName string and changes the behavior (llvm-readobj) to
only report a warning when there is a DT_SONAME and a string cab't be read.
Warning is not reported for llvm-readelf, as it never tries to dump it.

Diff Detail

Event Timeline

grimar created this revision.Oct 14 2020, 4:41 AM
Herald added a project: Restricted Project. · View Herald Transcript
grimar requested review of this revision.Oct 14 2020, 4:41 AM

llvm-readelf doesn't need a string that is specified by DT_SONAME.
llvm-readobj/elf always tries to read it, even when there is no DT_SONAME tag.

Agree. An absent DT_SONAME is like a value of 0.

Because of that both tools reports a warning for many our test cases.

I am confused by this point. Is it about the behavior when .dynstr is absent? .dynstr should always exist (st_size>=1).

Because of that both tools reports a warning for many our test cases.

I am confused by this point. Is it about the behavior when .dynstr is absent?

Yes.

.dynstr should always exist (st_size>=1).

It is about cases when we need to have the .dynamic section for whatever reason, but don't have the DT_SONAME tag.
Currently we don't recognize that it is absent and trying to read a string from the .dynstr.
In those tests we often don't need to have the .dynstr, so it doesn't exist.

jhenderson accepted this revision.Oct 19 2020, 2:19 AM

Looks good, thanks.

llvm/test/tools/llvm-readobj/ELF/dynamic-tags.test
496–497

Maybe worth a blank line between these two to highlight that the LoadName printing is not part of the dynamic section printing.

This revision is now accepted and ready to land.Oct 19 2020, 2:19 AM