[PATCH] D38361: [ELF] Stop setting output section size early

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In https://reviews.llvm.org/D38361#890717, @peter.smith wrote:

> Apologies for the drive by comment, I need to leave the office fairly soon. In this specific case might it be simpler to only early calculate the size of non SHF_ALLOC content such as debug sections as the OutputSection doesn't get an address, just an offset and doesn't affect the addresses of any other SHF_ALLOC content, it is therefore safe to calculate the OutputSection size early.


Unfortunately, it isn't safe: non-alloc sections can still be manipulated in linker scripts. For example, you could do something like `.foobar : { *(.foo); . += 16; *(.bar) }` in your script, which leaves a 16-byte gap in the output section, whose size is not accounted for in the early calculation, and is only later fixed by assignAddresses. This isn't an issue for compressed debug sections, because we explicitly ignore custom layout in scripts for such sections.

> In the general case I think the late addition and removal of content such as Thunks, Mips GOT entries, errata fixes and I'm guessing at RiscV relaxation are going to require some kind of iteration to a fixed point. I usually find iterating to a fixed point better than trying to predict addresses/sizes as it is easy to miss hard to predict corner cases. I'm wondering if it is possible to make this kind of iteration clearer by having a kind of Relaxation() stage that we can add functions that may need to update addresses and sizes to, this should hopefully make such code easier to understand. Probably a separate series of refactorings though.

I completely agree with this general idea, although "Relaxation" is another term that is getting a bit too over-loaded for my taste (like "finalize"), so I'd call it something else.

> I recommend splitting out the SHF_LINK_ORDER change into a separate patch if it is unrelated.

It's not entirely unrelated, but it could be a separate prerequisite patch. I'll look at splitting it off.


https://reviews.llvm.org/D38361