This is an archive of the discontinued LLVM Phabricator instance.

[obj2yaml,yaml2obj] - Refine how we set/dump the sh_entsize field.
ClosedPublic

Authored by grimar on Dec 28 2020, 5:19 AM.

Details

Summary

This reuses the code from yaml2obj (moves it to ELFYAML.h).
With it we can set the sh_entsize in a single place in obj2yaml.

Note that it also fixes a bug of yaml2obj: we do not
set the sh_entsize field for the SHT_ARM_EXIDX section properly.

Diff Detail

Event Timeline

grimar created this revision.Dec 28 2020, 5:19 AM
grimar requested review of this revision.Dec 28 2020, 5:19 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 28 2020, 5:19 AM

Could you explain why this patch fixes the ARM_EXIDX sh_entsize specifically? I don't see anything immediately that would only fix that section (and not other sections).

llvm/test/tools/obj2yaml/ELF/mips-abi-flags.yaml
54–55

Do you think this deserves a case where the entsize is not default?

llvm/test/tools/obj2yaml/ELF/sht-symtab-shndx.yaml
191

Really, this comment has nothing to do with the first RUN below it. Probably you should move it to the ENTSIZE=4 case.

Could you explain why this patch fixes the ARM_EXIDX sh_entsize specifically? I don't see anything immediately that would only fix that section (and not other sections).

It's because writeSectionContent(..., const ELFYAML::ARMIndexTableSection &Section, ...) doesn't set the EntSize field as it supposed to. This patch
generalizes the logic, now we set this field in the single place, and so the issue was revealed (see arm-exidx-section.yaml).

grimar updated this revision to Diff 316073.Jan 12 2021, 6:16 AM
grimar marked 2 inline comments as done.
grimar edited the summary of this revision. (Show Details)
  • Addressed review comments.
llvm/test/tools/obj2yaml/ELF/mips-abi-flags.yaml
54–55

Done.

This revision is now accepted and ready to land.Jan 13 2021, 12:41 AM