Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Phabricator shutdown timeline

[ELF] - Do not ignore discarding of .rela.plt/.rela.dyn, allow doing custom layout for them.

Authored by grimar on Dec 30 2017, 12:50 AM.



Currently when we build input sections list in linker script
we ignore all rel[a] sections. That was done to support
scripts like .rela.dyn : { *( } for emit relocs.

Though as a result following scripts were also silently ignored:

/DISCARD/ : { *(.rela.plt)
/DISCARD/ : { *(.rela.dyn)

and we produced output with this sections. That is not ideal.
Solution this patch suggests is simple: do not ignore synthetic
rel[a] sections. That way we can enable common discarding logic
for them and report proper error.

Diff Detail


Event Timeline

ruiu added inline comments.Jan 8 2018, 1:07 PM
293 ↗(On Diff #128356)

Obviously this needs comment because without it it is very hard to figure out what this isa for.

grimar updated this revision to Diff 129062.Jan 9 2018, 4:54 AM
  • Addressed review comment.
ruiu added inline comments.Jan 9 2018, 3:25 PM
293–295 ↗(On Diff #129062)

I think I don't understand this comment. How is /DISCARD/ related to the synthetic section?

ruiu added inline comments.Jan 9 2018, 5:27 PM
293–295 ↗(On Diff #129062)

Then please update the comment. This comment can be understood only when you know ".rela.dyn" is a linker-synthesized section, but describing that is not the point of this particular comment.

grimar updated this revision to Diff 129228.Jan 10 2018, 3:01 AM
grimar retitled this revision from [ELF] - Do not ignore discarding of .rela.plt/.rela.dyn to [ELF] - Do not ignore discarding of .rela.plt/.rela.dyn, allow doing custom layout for them..
  • Changed comment.
  • Added testcase showing we are able to do custom layout for synthetic .rel[a] sections with this patch.
ruiu added inline comments.Jan 10 2018, 11:24 PM
293–294 ↗(On Diff #129228)

It is not still clear to me what this comment means. It's because you cannot discard linker-synthesized sections, right?

320–321 ↗(On Diff #129228)

Can this just be isa<SyntheticSection>?

grimar added inline comments.Jan 11 2018, 4:44 AM
293–294 ↗(On Diff #129228)

Generally it should be safe to discard some of synthesized sections.

But problem is that with current code we silently ignore script lines that do /DISCARD/ : { *(.rela.plt) for example,
because rela.plt is synthetic SHT_REL[A] section , and all SHT_REL[A] sections currently are always proccessed as orphans.
What also means code like .foo : { *(.rela.dyn) } silently does not work for them either.

Here in comment I mean that we should not ignore doing custom layout for synthetic sections. By custom layout I mean
actions like above (placing to different output section, discarding, etc manipulations allowed for other sections).

Given above should we allow discarding of .rela.plt/.rela.dyn or not is a bit different question. I think it does not makes much sence
to discard them in general, so I think logic implemented is OK. At the other hand we could still allow doing that if script requests it explicitly.
(so we could remove changes from discard-section-err.s and from LinkerScript::discard introduced, leaving all others).

320–321 ↗(On Diff #129228)

I think no. Because for example with -strip-all we discard .symtab and .strtab during regular link.
I think that means we should be able to /DISCARD/ them from script and so should have white list here.

(The same applies for InX::BuildId, for example, it should be safe to discard it, so probably not useful to disallow it).

espindola edited reviewers, added: espindola; removed: rafael.Mar 15 2018, 8:39 AM
espindola accepted this revision.Mar 23 2018, 7:09 PM

LGTM with a nit.

4 ↗(On Diff #129228)

Do you need --emit-relocs in this test?

This revision is now accepted and ready to land.Mar 23 2018, 7:09 PM
This revision was automatically updated to reflect the committed changes.