[ELF] - Report orphan sections if -verbose given.

[grimar]

Summary

This ressurects ideas of abandoned (because of being outdated) D24725 patch.

When -verbose is specified, patch outputs names of each input orphan section
assigned to output.

[ruiu]

This may be a good addition, but why do you want this?

[grimar]

In D37517#862294, @ruiu wrote:

This may be a good addition, but why do you want this?

In comments for original patch (D24725) initially implemented was concluded that
--orphan-handling (initially implemented option for reporting) could be extremely helpful
for bringing FreeBSD's kernel up. Now it is already up, but can be used for other projects
in still.

Placing orphans as you know is not well documented and often probably people considers
that there should be no orphans in their linker scripts, but how can they check it for sure
when there is still no way to know about where orphans were placed ? Examining
outputs is a bit painfull usually and not always can reveal orphan if it placed to desided output
occasionally.

I believe this option can be helpful for people who writes/supports all kinds of linkerscripts.
Since LLD aims to improve diagnostics and --verbose itself is an option used for that,
I think it is nice feature to have. It could be part of -Map probably instead, implementing
via -verbose is much simpler though.

[ruiu]

I'm OK if you remove isa<SyntheticSection> check. This is a FYI message and is not guaranteed to correct in either way. Instead of suppressing it for all synthetic sections, I'd just print it out for all synthetic sections.

[grimar]

In D37517#863788, @ruiu wrote:

I'm OK if you remove isa<SyntheticSection> check. This is a FYI message and is not guaranteed to correct in either way. Instead of suppressing it for all synthetic sections, I'd just print it out for all synthetic sections.

Output would be next then:

orphan input section '.text' assigned to output section '.text'
orphan input section '.text.2' assigned to output section '.text'
orphan input section '.comment' assigned to output section '.comment'
orphan input section '.bss' assigned to output section '.bss'
orphan input section '.bss.rel.ro' assigned to output section '.bss.rel.ro'
orphan input section '.dynsym' assigned to output section '.dynsym'
orphan input section '.gnu.version' assigned to output section '.gnu.version'
orphan input section '.gnu.version_r' assigned to output section '.gnu.version_r'
orphan input section '.hash' assigned to output section '.hash'
orphan input section '.dynamic' assigned to output section '.dynamic'
orphan input section '.dynstr' assigned to output section '.dynstr'
orphan input section '.rela.dyn' assigned to output section '.rela.dyn'
orphan input section '.got' assigned to output section '.got'
orphan input section '.got.plt' assigned to output section '.got.plt'
orphan input section '.got.plt' assigned to output section '.got.plt'
orphan input section '.rela.plt' assigned to output section '.rela.plt'
orphan input section '.rela.plt' assigned to output section '.rela.plt'
orphan input section '.plt' assigned to output section '.plt'
orphan input section '.plt' assigned to output section '.plt'
orphan input section '.eh_frame' assigned to output section '.eh_frame'
orphan input section '.symtab' assigned to output section '.symtab'
orphan input section '.shstrtab' assigned to output section '.shstrtab'
orphan input section '.strtab' assigned to output section '.strtab'

Sections:
Idx Name          Size      Address          Type
  0               00000000 0000000000000000
  1 .text         00000005 0000000000000000 TEXT DATA
  2 .dynsym       00000018 0000000000000008
  3 .hash         00000010 0000000000000020
  4 .dynstr       00000001 0000000000000030
  5 .dynamic      00000060 0000000000000038
  6 .comment      00000008 0000000000000000
  7 .symtab       00000030 0000000000000000
  8 .shstrtab     00000049 0000000000000000
  9 .strtab       0000000a 0000000000000000

Would it be better to change "orphan input section" to "orphan internal section" for synthetics ?
So mark them with something to differentiate from regular inputs for possible output proccessing.

[grimar]

• Updated according to my last comment.

[ruiu]

This is still odd. Why don't you use toString?

The message should be "foo.o:.bar is being placed in .foo".

[grimar]

• Addressed review comments.

[ruiu]

log() is a regular function, so its argument is evaluated first and then its value is passed to log(). That means toString(S) is always executed even if it's not going to be written. Is it slow? (I do not request you update this patch, but I'm wondering whether it's negligible or not.)

[grimar]

In D37517#874133, @ruiu wrote:

log() is a regular function, so its argument is evaluated first and then its value is passed to log(). That means toString(S) is always executed even if it's not going to be written. Is it slow? (I do not request you update this patch, but I'm wondering whether it's negligible or not.)

I think answer is "it depends". addOrphanSections called only when Script->Opt.HasSections condition is true. And I think if script has SECTIONS, amount of orphan sections in compare with
amount of explicitly listed should be much lower.

I performed a quck test which tries to simulate the situation (https://ideone.com/p07NYi). It calls some method log() which do nothing with random string argument million of times.
Like if we would have script with million of orphans and verbose set to off. It takes 80 milliseconds in total. That probably can be considered as the approximation of worst possible scenario ?

[ruiu]

I performed a quck test which tries to simulate the situation (https://ideone.com/p07NYi). It calls some method log() which do nothing with random string argument million of times.
Like if we would have script with million of orphans and verbose set to off. It takes 80 milliseconds in total. That probably can be considered as the approximation of worst possible scenario ?

Well, I do not believe that the cost of calling log really matters, but your test doesn't seem to test it correctly. We are talking about the cost of calling toString on InputSectionBase *. I don't think it makes much sense to write some different function and measure its performance on dummy inputs. Why don't you just profile it?

[grimar]

In D37517#875507, @ruiu wrote:

I performed a quck test which tries to simulate the situation (https://ideone.com/p07NYi). It calls some method log() which do nothing with random string argument million of times.
Like if we would have script with million of orphans and verbose set to off. It takes 80 milliseconds in total. That probably can be considered as the approximation of worst possible scenario ?

Well, I do not believe that the cost of calling log really matters, but your test doesn't seem to test it correctly. We are talking about the cost of calling toString on InputSectionBase *. I don't think it makes much sense to write some different function and measure its performance on dummy inputs. Why don't you just profile it?

So benchmarks were performed (results available in mail list for this thread). Looks we can land it ?

[ruiu]

LGTM

So benchmarks were performed (results available in mail list for this thread). Looks we can land it ?

Your benchmark seems somewhat irrelevant, so I don't think that's a convincing number, but orthogonal to that, I think we don't need to worry too much about it.

ELF/LinkerScript.cpp

if (!S->Live || S->Parent)
       continue;
     StringRef Name = getOutputSectionName(S->Name);
     auto End = Opt.Commands.begin() + NumCommands;
     auto I = std::find_if(Opt.Commands.begin(), End, [&](BaseCommand *Base) {
       if (auto *Sec = dyn_cast<OutputSection>(Base))
         return Sec->Name == Name;
       return false;
     });
+    log(toString(S) + " is being placed in '" + Name + "'");
     if (I == End) {
       Factory.addInputSec(S, Name);
       assert(S->getOutputSection()->SectionIndex == INT_MAX);
     } else {
       OutputSection *Sec = cast<OutputSection>(*I);
       Factory.addInputSec(S, Name, Sec);
       unsigned Index = std::distance(Opt.Commands.begin(), I);
       assert(Sec->SectionIndex == INT_MAX || Sec->SectionIndex == Index);

test/ELF/linkerscript/orphan-report.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
+# RUN: echo "SECTIONS { .text : { *(.text.1) } }" > %t.script
+# RUN: ld.lld -shared -o %t.out --script %t.script %t.o --verbose | FileCheck %s
+
+# CHECK:      {{.*}}.o:(.text) is being placed in '.text'
+# CHECK-NEXT: {{.*}}.o:(.text.2) is being placed in '.text'
+# CHECK-NEXT: <internal>:(.comment) is being placed in '.comment'
+# CHECK-NEXT: <internal>:(.bss) is being placed in '.bss'
+# CHECK-NEXT: <internal>:(.bss.rel.ro) is being placed in '.bss.rel.ro'
+# CHECK-NEXT: <internal>:(.dynsym) is being placed in '.dynsym'
+# CHECK-NEXT: <internal>:(.gnu.version) is being placed in '.gnu.version'
+# CHECK-NEXT: <internal>:(.gnu.version_r) is being placed in '.gnu.version_r'
+# CHECK-NEXT: <internal>:(.hash) is being placed in '.hash'
+# CHECK-NEXT: <internal>:(.dynamic) is being placed in '.dynamic'
+# CHECK-NEXT: <internal>:(.dynstr) is being placed in '.dynstr'
+# CHECK-NEXT: <internal>:(.rela.dyn) is being placed in '.rela.dyn'
+# CHECK-NEXT: <internal>:(.got) is being placed in '.got'
+# CHECK-NEXT: <internal>:(.got.plt) is being placed in '.got.plt'
+# CHECK-NEXT: <internal>:(.got.plt) is being placed in '.got.plt'
+# CHECK-NEXT: <internal>:(.rela.plt) is being placed in '.rela.plt'
+# CHECK-NEXT: <internal>:(.rela.plt) is being placed in '.rela.plt'
+# CHECK-NEXT: <internal>:(.plt) is being placed in '.plt'
+# CHECK-NEXT: <internal>:(.plt) is being placed in '.plt'
+# CHECK-NEXT: <internal>:(.eh_frame) is being placed in '.eh_frame'
+# CHECK-NEXT: <internal>:(.symtab) is being placed in '.symtab'
+# CHECK-NEXT: <internal>:(.shstrtab) is being placed in '.shstrtab'
+# CHECK-NEXT: <internal>:(.strtab) is being placed in '.strtab'
+
+.section .text.1,"a"
+ nop
+
+.section .text.2,"a"
+ nop