[lld] [ELF] Orphan placement: remove hasInputSections condition (PR #93761)

Fangrui Song via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 11 22:41:04 PDT 2024


MaskRay wrote:

> I have sympathy with MaskRay's position in that it is a somewhat arbitrary rule that OutputSections can't affect orphan placement, and may result in surprise given the existing GNU ld documentation (https://sourceware.org/binutils/docs/ld/Orphan-Sections.html). 

Thanks!

> Having said that I think we may want to define what type and flags our OutputSections without InputSections get their type to given people more stability.
> 
> My understanding of GNU ld is that it will by default give the OutputSection SHT_PROGBITS type and "RAW" flags. This can be altered to READONLY with recent releses, although I don't think LLD supports that.
> ```
> READONLY ( TYPE = type )
> This form of the syntax combines the READONLY type with the type specified by type.
> ```

GNU ld since Sep 2020 defaults to "RA" flags. Older versions used "RAW".
`. += expr` adds "W". BYTE/QUAD don't.
https://sourceware.org/bugzilla/show_bug.cgi?id=26378

Orphan sections use the default type `SHT_PROGBITS`.

GNU ld introduced "READONLY" in July 2021 (https://sourceware.org/pipermail/binutils/2021-July/117492.html)
"READONLY" gives an explicit flags signal, which can be a nice thing, though we could probably have different syntax (e.g. `(FLAGS=aw)`) resembling `(TYPE=xxx)`. 

> Empiricially LLD seems to inherit the type and flags from a previous OutputSection (I've not checked the code). This largely seems to do the right thing although I'm not convinced that inheriting the "x" flag is the right thing to do.
> 
> My, little thought, opening bid would be something like:
> * By default, inherit type and flags from the previous output section excluding SHF_EXECINSTR
> * If any explicit OutputSection type is used https://sourceware.org/binutils/docs/ld/Output-Section-Type.html default to GNU  ld behaviour of "awr" unless READONLY is used.

We inherit `SHF_ALLOC|SHF_WRITE` flags around LinkerScript.cpp:1253. We used to inherit `SHF_EXECINSTR`, but I removed it in #70911.
The section type is not inherited.

I have also considered that flags setting syntax can help guide orphan placement.
While a more complex condition like `osd && (osd->osec.hasInputSections || osd->osec.hasExplicitFlags)` could be used,
opting for the simpler `osd` condition seems easier to understand.

> However that is separate from this pull request. To progress this one do we have an example of one of these sample linker scripts having a problematic Output Section acting as an anchor for orphans, or is it a theoretical risk?

> So far, the risk is theoretical. I just don't like it when data can be written to an executable segment unexpectedly. Since LinkerScript::adjustOutputSections() propagates SHF_WRITE but not SHF_EXECINSTR, the problematic case could be a linker script that defines read-only sections before executable sections, and among the executable sections there is an empty output section with symbol assignments, data values, or other similar instructions; in this scenario, read-only orphan sections are placed in the executable segment. I can't say how likely this case is, nor how harmful or harmless it might be.

This (`.rodata; .text sec_without_input .text1`) is valid concern, though this might be rare and I believe it doesn't justify a particular orphan placement behavior.
I think this is rare because: `. += expr` leads to a writable section in GNU ld, which makes `.` padding not useful for code padding (#70911).

Considering this and our shift towards explicit flags syntax, removing `hasInputSections` seems viable, simplifying the code.


https://github.com/llvm/llvm-project/pull/93761


More information about the llvm-commits mailing list