[PATCH] D92301: [LLD][ELF] - Don't keep empty output sections that are explicitly assigned to segment.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

It makes me happy that doing this doesn't seem to be all that complex. Thanks for looking into it!



================
Comment at: lld/ELF/LinkerScript.cpp:990
 
-  // We do not remove empty sections that are explicitly
-  // assigned to any segment.
-  if (!sec.phdrs.empty())
-    return false;
 
   // We do not want to remove OutputSections with expressions that reference
----------------
Nit: delete extra blank line too.


================
Comment at: lld/ELF/LinkerScript.cpp:1016
 
+static void maybePropogatePhdrs(OutputSection &sec,
+                                std::vector<StringRef> &phdrs) {
----------------



================
Comment at: lld/ELF/LinkerScript.cpp:1082
+    // At this point the order of sections is not final, because orphans are not
+    // inserted to their expected positions. We don't propogate program headers
+    // to them. The final program headers propogation will be done later.
----------------



================
Comment at: lld/ELF/LinkerScript.cpp:1083
+    // inserted to their expected positions. We don't propogate program headers
+    // to them. The final program headers propogation will be done later.
+    if (sec->sectionIndex != UINT32_MAX)
----------------



================
Comment at: lld/test/ELF/linkerscript/implicit-program-header.test:10
 # CHECK:      Segment Sections...
-# CHECK-NEXT:   00     .dynsym .hash .dynstr .bar .foo .text .dynamic
-# CHECK-NEXT:   01     .bar .foo
+# CHECK-NEXT:   00     .dynsym .hash .dynstr .foo .text .dynamic
+# CHECK-NEXT:   01     .foo
----------------
Unrelated to this patch, but it looks incorrect that .foo is assigned to this program header? It should be in ph_exec, and only ph_exec, by my understanding.


================
Comment at: lld/test/ELF/linkerscript/implicit-program-header.test:19
 SECTIONS {
   .bar : { *(.bar) } : ph_exec
   .foo : { *(.foo) }
----------------
Is it worth extending this test case to show that ALL program headers are propagated, e.g. if .bar was inside `: ph_exec : ph_other` then .foo is too?


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

https://reviews.llvm.org/D92301