This is an archive of the discontinued LLVM Phabricator instance.

[yaml2obj] - Allow setting custom sh_info for RawContentSection sections.
ClosedPublic

Authored by grimar on Feb 28 2019, 5:37 AM.

Details

Summary

I need this for tweaking SHT_SYMTAB sections.
Their sh_info contains the (number of symbols + 1) usually. But for
creating invalid inputs for test cases it would be convenient
to allow explicitly override this field from YAML.

This change allowed to remove 4 precompiled binaries from
LLD test cases set: https://reviews.llvm.org/D58780,
https://reviews.llvm.org/D58783

Diff Detail

Event Timeline

grimar created this revision.Feb 28 2019, 5:37 AM
grimar planned changes to this revision.Feb 28 2019, 5:52 AM

Planning change.

grimar requested review of this revision.Feb 28 2019, 5:58 AM

No, it's OK as is :)

jhenderson added inline comments.Feb 28 2019, 6:03 AM
test/tools/yaml2obj/elf-symtab-shinfo.yaml
45

This looks like it belongs in a different test, as it's not to do with the sh_info field.

tools/yaml2obj/yaml2elf.cpp
347

If -> If the

348

want to be able to -> should

grimar marked an inline comment as done.Feb 28 2019, 6:07 AM
grimar added inline comments.
test/tools/yaml2obj/elf-symtab-shinfo.yaml
45

It is for line 352 of yaml2elf.cpp:

This line: if (auto S = dyn_cast<ELFYAML::RawContentSection>(Sec)) is needed,
without if this test would crash.

grimar marked an inline comment as done.Feb 28 2019, 6:09 AM
grimar added inline comments.
test/tools/yaml2obj/elf-symtab-shinfo.yaml
45

So it's technically relative with how we set sh_info. Want me to move this out?

jhenderson added inline comments.Feb 28 2019, 6:58 AM
test/tools/yaml2obj/elf-symtab-shinfo.yaml
45

Yes, I think so. It can be in this patch, but should be a different test, as although it is relevant to the current implementation, you could imagine it being more generally useful.

grimar updated this revision to Diff 188854.Mar 1 2019, 12:30 AM
grimar marked 3 inline comments as done.
  • Addressed review comments.
jhenderson added inline comments.Mar 1 2019, 1:44 AM
test/tools/yaml2obj/elf-symtab-shtype.yaml
2 ↗(On Diff #188854)

Maybe it's worth showing what the output type of .symtab actually is?

grimar updated this revision to Diff 188865.Mar 1 2019, 1:52 AM
grimar marked an inline comment as done.
  • Addressed review comment.
test/tools/yaml2obj/elf-symtab-shtype.yaml
2 ↗(On Diff #188854)

Done.

This revision is now accepted and ready to land.Mar 1 2019, 1:56 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 1 2019, 2:20 AM