[ELF] - Tweak sorting rules for output sections to place orphan sections at end.

[grimar]

Summary

During discussion of D18499 was stated:
"Orphan sections are sections present in the input files which are not explicitly placed into the output file by the linker script. We place orphan sections at end of file."
This patch does not really affect D18499 because order of OutputSections in D18499 patch does not make sence anymore (script do all layout stuff).

But this patch changes sorting predicate to place orphans to the end. At least that is needed to keep OutputSections in sync with output to
set Sec->SectionIndex correctly for futher finalization.

[grimar]

ELF/LinkerScript.cpp

68

Forgot to change the comment.
I think it can be:

// A compartor to sort output sections. Returns -1 or 1 if 
// A or B are mentioned in linker script. Otherwise, returns 0
// to use the default rule which is implemented in Writer.cpp.

[ruiu]

ELF/LinkerScript.cpp

68

Why don't you update the comment?

[grimar]

• Updated comment.
• Fixed name of check prefix in testcase.

ELF/LinkerScript.cpp

68

Done. I assumed it is acceptable to update it during next iteration (which almost always happens) or before commit if "LGTM" is given.

[ruiu]

I'm confused. If you submit D18499, then there is actually no orphan sections internally because you append all orphan sections to end of the SectionOrder vector, no? Instead of updating the if conditional, shouldn't you just remove it or replace it with assertions?

[grimar]

In D19107#402346, @ruiu wrote:

I'm confused. If you submit D18499, then there is actually no orphan sections internally because you append all orphan sections to end of the SectionOrder vector, no? Instead of updating the if conditional, shouldn't you just remove it or replace it with assertions?

D18499 does not touch SectionOrder vector. It appends orphans to Locations list only.
Probably you're right and it should update the SectionOrder vector too.

[grimar]

But we will do this during assignAddresses now (after addressing last round of review). It will be too late to update SectionOrder vector then.

[grimar]

Also compareSections() is always called now, no matter do we have script or not.
So if we don't have script, then

if (I == E && J == E)

line is expected and valid for us, I think.
So I guess even if we upgrade D18499 and let it update SectionOrder, this patch still seems OK.

[grimar]

I`ll prepare another approach for solving that.

[grimar]

And this approach is D19171

ELF/LinkerScript.cpp

 
 ArrayRef<uint8_t> LinkerScript::getFiller(StringRef Name) {
   auto I = Filler.find(Name);
   if (I == Filler.end())
     return {};
   return I->second;
 }
 
-// A compartor to sort output sections. Returns -1 or 1 if both
-// A and B are mentioned in linker scripts. Otherwise, returns 0
+// A compartor to sort output sections. Returns -1 or 1 if
+// A or B are mentioned in linker script. Otherwise, returns 0
 // to use the default rule which is implemented in Writer.cpp.
 int LinkerScript::compareSections(StringRef A, StringRef B) {
   auto E = SectionOrder.end();
   auto I = std::find(SectionOrder.begin(), E, A);
   auto J = std::find(SectionOrder.begin(), E, B);
-  if (I == E || J == E)
+  if (I == E && J == E)
     return 0;
   return I < J ? -1 : 1;
 }
 
 // Returns true if S matches T. S can contain glob meta-characters.
 // The asterisk ('*') matches zero or more characacters, and the question
 // mark ('?') matches one character.
 static bool matchStr(StringRef S, StringRef T) {

test/ELF/linkerscript-orphans.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
+
+# RUN: echo "SECTIONS { .writable : { *(.writable) } }" > %t.script
+# RUN: ld.lld -o %t.out --script %t.script %t
+# RUN: llvm-objdump -section-headers %t.out | \
+# RUN:   FileCheck -check-prefix=TEXTORPHAN %s
+
+# RUN: echo "SECTIONS { .text : { *(.text) } }" > %t.script
+# RUN: ld.lld -o %t.out --script %t.script %t
+# RUN: llvm-objdump -section-headers %t.out | \
+# RUN:   FileCheck -check-prefix=WRITABLEORPHAN %s
+
+# TEXTORPHAN:      Sections:
+# TEXTORPHAN-NEXT: Idx Name          Size      Address          Type
+# TEXTORPHAN-NEXT:   0               00000000 0000000000000000
+# TEXTORPHAN-NEXT:   1 .writable     00000004 0000000000011000 DATA
+# TEXTORPHAN-NEXT:   2 .text         00000001 0000000000012000 TEXT DATA
+
+# WRITABLEORPHAN:      Sections:
+# WRITABLEORPHAN-NEXT: Idx Name          Size      Address          Type
+# WRITABLEORPHAN-NEXT:   0               00000000 0000000000000000
+# WRITABLEORPHAN-NEXT:   1 .text         00000001 0000000000011000 TEXT DATA
+# WRITABLEORPHAN-NEXT:   2 .writable     00000004 0000000000012000 DATA
+
+.global _start
+_start:
+ nop
+
+.section .writable,"aw"
+ .zero 4