[PATCH] D76410: [ELF] Don't combine SHF_LINK_ORDER sections linking different output sections

[Fangrui Song via Phabricator via llvm-commits]

MaskRay abandoned this revision.
MaskRay added a comment.

In D76410#1950006 <https://reviews.llvm.org/D76410#1950006>, @psmith wrote:

> In D76410#1948314 <https://reviews.llvm.org/D76410#1948314>, @MaskRay wrote:
>
> > Reading the comprehensive feedback, I think we should make this change. There are probably really two difference uses of SHF_LINK_ORDER: one is monolithic (`__start_`/DT_INIT_ARRAY/...) and the other is not (`__patchable_function_entries`/`.stack_sizes`). If pcc has future monolithic use cases, we can cross the bridge when we come to it.
>
>
> I'm less convinced that this is the right thing to do. Looking at the thread the last response is important:
>
>   SHF_LINK_ORDER says nothing about separate output sections. If you
>   need the metadata sections for .text.foo to be in a separate output
>   section from those for .text.bar, you should name the metadata
>   sections accordingly, too.
>
>
> If the metadata sections are appropriately named then the separate OutputSections drop out without us needing to make any changes. Splitting up identically named OutputSections that would normally be collated as one risks breaking them. I'm personally inclined to view the use of SHF_LINK_ORDER outside of constructing a single table as the special case given the origin story of SHF_LINK_ORDER. I think we'll have to agree to disagree on this one. Having said that we can handle the special cases when needed so I needn't be a blocker if the consensus is the other way.


I am also less convinced after I created D77007 <https://reviews.llvm.org/D77007> and put more thoughts on this change. However, my viewpoint is more from

- We can't (not impossible but not motivated enough) extend the name based linker script input section descriptions.
- The implementation consistency

but less from the existing use cases of SHF_LINK_ORDER because I am not convinced that we should necessarily restrict SHF_LINK_ORDER just by the current (admittedly dominating) single table use cases.

We don't label outSecOff in sortSections() for -r. This leads to the following rule in addOrphanSections.

  // In -r links, SHF_LINK_ORDER sections are added while adding their parent
  // sections because we need to know the parent's output section before we
  // can select an output section for the SHF_LINK_ORDER section.
  if (config->relocatable && (isec->flags & SHF_LINK_ORDER))

This is related to the rule this patch intends to simplify. Simplifying one place is not worthwhile.

I am abandoning this patch but I'll improve the test case.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76410/new/

https://reviews.llvm.org/D76410