This is an archive of the discontinued LLVM Phabricator instance.

[obj2yaml] - Teach obj2yaml to dump SHT_NOBITS sections when dumping program headers.
ClosedPublic

Authored by grimar on Apr 9 2020, 7:23 AM.

Details

Summary

SHT_NOBITS are a bit special because occupy no physical space.
This patch adds support for them.

Diff Detail

Event Timeline

grimar planned changes to this revision.Apr 9 2020, 7:30 AM

I'll add one more test case.

grimar added a comment.Apr 9 2020, 7:47 AM

I think I've found an issue in yaml2obj revealed by the test case I'd like to add here.
I'll investigate it in my tomorrow and update this patch after.

I feel that the dependency can be reversed, i.e. D77652 will depend on D77805.

Using file offsets for SHT_NOBITS is the only way. For an empty SHT_PROGBITS, we need to discuss several malformed cases. It is less clear what we should do.

I think I've found an issue in yaml2obj revealed by the test case I'd like to add here.
I'll investigate it in my tomorrow and update this patch after.

It needs D78005 to be landed to proced.

jhenderson added inline comments.Apr 16 2020, 1:43 AM
llvm/test/tools/obj2yaml/program-headers.yaml
482

in a different

493

Could you add some comments to these sections saying why the Address and Size values were chosen. It's not particularly clear to me how they're important to the test.

llvm/tools/obj2yaml/elf2yaml.cpp
307

"SHT_NOBITS sections usually occupy no physical space in a file. Such sections belong to a segment when they reside in the segment's virtual address space."

I added the "usually" because I think a SHT_NOBITS section sandwiched between two non NOBITS sections might need to be allocated file space to avoid things pointing to the wrong place.

grimar updated this revision to Diff 258286.EditedApr 17 2020, 4:13 AM
grimar marked 3 inline comments as done.
  • Addressed review comments.

Ready for review:

I feel that the dependency can be reversed, i.e. D77652 will depend on D77805.

Done.

In D77805#1972126, @grimar wrote:
I think I've found an issue in yaml2obj revealed by the test case I'd like to add here.
I'll investigate it in my tomorrow and update this patch after.

It needs D78005 to be landed to proceed.

Let's go without an additional test case for now, the issue I was talking about is
related to trailing multiple SHT_NOBITS sections (fixed in D78005).
We can add it later.

jhenderson accepted this revision.Apr 20 2020, 1:00 AM

LGTM with grammar and typo fixes.

llvm/test/tools/obj2yaml/program-headers.yaml
493

arbitraty -> arbitrary

499

the size which -> a size that

504

arbitraty -> arbitrary

517

arbitraty -> arbitrary
different from -> different to

This revision is now accepted and ready to land.Apr 20 2020, 1:00 AM
This revision was automatically updated to reflect the committed changes.
grimar marked 4 inline comments as done.
Herald added a project: Restricted Project. · View Herald TranscriptApr 20 2020, 4:48 AM