This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by grimar on Sep 6 2017, 7:40 AM.

Details

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.

Diff Detail

Event Timeline

grimar created this revision.Sep 6 2017, 7:40 AM
ruiu edited edge metadata.Sep 6 2017, 10:15 AM

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

grimar added a comment.Sep 7 2017, 1:36 AM
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 added a comment.Sep 7 2017, 12:26 PM

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 added a comment.Sep 8 2017, 1:49 AM
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 this revision to Diff 115392.Sep 15 2017, 4:11 AM
  • Updated according to my last comment.
ruiu added a comment.Sep 15 2017, 10:56 AM

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

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

grimar updated this revision to Diff 115610.Sep 18 2017, 1:56 AM
  • Addressed review comments.
ruiu added a comment.Sep 18 2017, 12:23 PM

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 added a comment.EditedSep 19 2017, 2:10 AM
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 added a comment.Sep 19 2017, 1:13 PM

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 added a comment.EditedSep 22 2017, 5:51 AM
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 accepted this revision.Sep 22 2017, 11:29 AM

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.

This revision is now accepted and ready to land.Sep 22 2017, 11:29 AM
grimar edited the summary of this revision. (Show Details)Sep 25 2017, 2:42 AM
This revision was automatically updated to reflect the committed changes.