This is an archive of the discontinued LLVM Phabricator instance.

[yaml2obj] - Do not assert when .dynsym is specified explicitly, but .dynstr is not present.
ClosedPublic

Authored by grimar on Jun 7 2019, 3:13 AM.

Details

Summary

We have a code in buildSectionIndex() that adds implicit sections:

// Add special sections after input sections, if necessary.
for (StringRef Name : implicitSectionNames())
  if (SN2I.addName(Name, SecNo)) {
    // Account for this section, since it wasn't in the Doc
    ++SecNo;
    DotShStrtab.add(Name);
  }

The problem arises when .dynsym is specified explicitly and no
DynamicSymbols is used. In that case, we do not add
.dynstr implicitly and will assert later when will try to set Link
for .dynsym.

Seems, in this case, reasonable behavior is to allow Link field to be zero.
This is what this patch does.

Diff Detail

Event Timeline

grimar created this revision.Jun 7 2019, 3:13 AM
jhenderson added inline comments.Jun 7 2019, 7:29 AM
test/tools/yaml2obj/explicit-dynsym-no-dynstr.yaml
7–9

Could you file/find a bug for this and include the bug URL here?

Also report -> reports

tools/yaml2obj/yaml2elf.cpp
371

in document -> in the document

372

omit "DynamicSymbols" -> omit the "DynamicSymbols"
You don't need the ',' after "case"

373

described -> defined?

375–379

It looks like this doesn't allow for a custom-specified sh_link field. It might be nice to add that later.

grimar updated this revision to Diff 203788.Jun 10 2019, 2:58 AM
grimar marked 5 inline comments as done.
  • Addressed review comments.
tools/yaml2obj/yaml2elf.cpp
375–379

Yes, it will be needed to make a complete test case for https://bugs.llvm.org/show_bug.cgi?id=42215

This revision is now accepted and ready to land.Jun 10 2019, 3:39 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJun 10 2019, 4:36 AM