Put sections into expected offset in binary format.
ClosedPublic
Actions

Authored by ray on Sep 27 2019, 10:09 PM.

Details

Reviewers

dim
emaste
markj

Commits

rS356082: MFC r352875:
rS352875: ections into expected offset in binary format.

Summary

Calculate binary file offset using address field, bacause software know only offset to known data, not where to load segment.
With that patch, kernel .data section can have any alignment - kernel works.

PR: 235391

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ray created this revision.Sep 27 2019, 10:09 PM

Herald added a subscriber: imp. · View Herald TranscriptSep 27 2019, 10:09 PM

Harbormaster completed remote builds in B26745: Diff 62669.Sep 27 2019, 10:09 PM

ray edited the summary of this revision. (Show Details)Sep 27 2019, 10:11 PM

markj added inline comments.Sep 27 2019, 10:19 PM

contrib/elftoolchain/elfcopy/binary.c
81 ↗	(On Diff #62669)	Why keep "base"?

ray added inline comments.Sep 27 2019, 10:21 PM

contrib/elftoolchain/elfcopy/binary.c
81 ↗	(On Diff #62669)	As flags. Because sh_addr must not use special values.

markj added inline comments.Sep 27 2019, 11:18 PM

contrib/elftoolchain/elfcopy/binary.c
81 ↗	(On Diff #62669)	That claim should apply to sh_offset as well, shouldn't it? For position-independent objects we will have sh_offset == sh_addr.

ray added inline comments.Sep 27 2019, 11:23 PM

contrib/elftoolchain/elfcopy/binary.c
81 ↗	(On Diff #62669)	After `objcopy -O binary` done, nobody know it is PIC, loadable or anything else.

Can you show the readelf -S output for the input file where this change matters? The change looks ok to me, but I do not understand why it is useful.

In D21827#476548, @markj wrote:

Can you show the readelf -S output for the input file where this change matters? The change looks ok to me, but I do not understand why it is useful.

https://gist.github.com/rayddteam/270b5e564566c7a64841d58a9b34d7cd

ray edited the summary of this revision. (Show Details)Sep 28 2019, 7:55 AM

markj accepted this revision.Sep 28 2019, 5:09 PM

markj added inline comments.

contrib/elftoolchain/elfcopy/binary.c
81 ↗	(On Diff #62669)	Sure, but here we are talking about the input file, not the output file. Anyway, if you insist on keeping both variables, I would rename `base` to `baseoff`.
116 ↗	(On Diff #62669)	Should be `baseaddr` now.

This revision is now accepted and ready to land.Sep 28 2019, 5:09 PM

Rename base to baseoff. Fix comment.

This revision now requires review to proceed.Sep 29 2019, 9:17 PM

Harbormaster completed remote builds in B26776: Diff 62724.Sep 29 2019, 9:17 PM

ray marked 2 inline comments as done.Sep 29 2019, 9:17 PM

ray added inline comments.Sep 29 2019, 9:25 PM

contrib/elftoolchain/elfcopy/binary.c
81 ↗	(On Diff #62669)	About claim: not exact same. baseoff(base before) is off_t, which is big enough to never reach '-1' and in that case used on value, that can't be negative. So, not clearly, but can be used with special value. About Elf64_Addr, we can't be sure what address will be selected to start kernel on platform. We can use special value 0xff..f, but it will looks more magic than just -1 :)