[PATCH] D72904: [ELF] Allow SHF_LINK_ORDER sections to have sh_link=0

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added a comment.

In D72904#2164880 <https://reviews.llvm.org/D72904#2164880>, @MaskRay wrote:

> See also @jhenderson's latest reply to the generic-abi thread "SHF_LINK_ORDER's original semantics make upgrade difficult" <https://groups.google.com/forum/#!topic/generic-abi/hgx_m1aXqUo>
>
>   # input.o:
>   <table header> [common]
>   - DW_TAG_compile_unit [common]
>   -- DW_TAG_variable [.data.foo]
>   -- DW_TAG_namespace [common]
>   --- DW_TAG_subprogram [.text.bar]
>   --- DW_TAG_variable [.data.baz]
>   <table footer> [common]
>
>
> @pcc I sent the patches since you expressed a preference on more work on SHF_LINK_ORDER. Though, @jhenderson's needs (and my header/footer argument in that thread) make me think more whether we should continue patching SHF_LINK_ORDER or start pursuing another section flag.
>
> Without metadata sections's override, SHF_LINK_ORDER will have very few use cases. The problems still exist but we probably don't really need to address them.


I think it is too premature to consider adding a section flag, since they are a limited resource. Furthermore, adding a flag whose absence means "use the linker's default input section ordering", and creating compiler features that rely on said default input section order, may constrain us in the future if we want to change the default order (consider features like ICF). Ideally, we would create a flag that means "instead of using the normal SHF_LINK_ORDER ordering, use this other ordering". But that would require specifying exactly what that means so that future linker changes don't break it.



================
Comment at: lld/test/ELF/linkorder-mixed.test:1
+## Test that we allow SHF_LINK_ORDER sections with sh_link=0.
+## SHF_LINK_ORDER sections with sh_link!=0 are ordered before others.
----------------
This can become a .s if we implement the parser changes in D72899, no?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72904