This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Added support of linkerscript's "/DISCARD/" for --emit-relocs
ClosedPublic

Authored by grimar on Jan 30 2017, 3:08 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

grimar created this revision.Jan 30 2017, 3:08 AM
grimar edited the summary of this revision. (Show Details)
ruiu added inline comments.Jan 31 2017, 2:21 PM
ELF/LinkerScript.cpp
266 ↗(On Diff #86262)

Please describe not what we are doing but why we are doing.

grimar updated this revision to Diff 86598.Feb 1 2017, 2:35 AM
grimar edited the summary of this revision. (Show Details)
  • Addressed review comment.
grimar added inline comments.Feb 1 2017, 2:39 AM
ELF/LinkerScript.cpp
266 ↗(On Diff #86262)

Done.

tpimh added a subscriber: tpimh.Feb 1 2017, 11:16 AM
ruiu added inline comments.Feb 1 2017, 5:54 PM
ELF/Driver.cpp
238 ↗(On Diff #86598)

This comment can be improved.

// -emit-relocs and -gc-sections don't conflict in theory, but LLD currently
// does not support the combination due to an implementation limitation.
ELF/InputFiles.cpp
409–410 ↗(On Diff #86598)

So, why? Please explain for those who will read this code.

ELF/LinkerScript.cpp
266 ↗(On Diff #86262)

I still do not understand why you need this. Well, I do understand what you are trying to do here and what your intention is, but I wonder why you specifically have to care about this case.

Let's say you have sections A and B. A defines symbol X, and B has an undefined symbol X. Naturally, the B's undefined symbol will be resolved using A. But, what if you discard section A using a linker script? What will happen? Isn't this essentially the same situation as this?

grimar added inline comments.Feb 2 2017, 1:09 AM
ELF/InputFiles.cpp
409–410 ↗(On Diff #86598)

Will "pointing to .eh_frame is not supported due to an implementation limitation" sound fine for you ?
The only reason for that place is that I want to prepare support for this feature in following patch to simplify review of this.
It is also used in linux kernel, so this fatal is kinda temporarily stub.

ELF/LinkerScript.cpp
266 ↗(On Diff #86262)

Linux kernel contains exactly the situation I show in testcase.
It discards all debug sections:

/DISCARD/ : { *(.debug*) }

And without that code we just crash for this situation. Because we will try to emit .rela.debug* and crash.
That is the only real situation I care here about.

ruiu added inline comments.Feb 2 2017, 10:00 AM
ELF/LinkerScript.cpp
266 ↗(On Diff #86262)

Do not focus too much on the Linux kernel. Please attempt to find the fundamental issue and a generic solution. If you reach a conclusion that this still needs special handling after a careful examination, that is fine, but adding a piece of code just to link the Linux kernel is not ok.

grimar added inline comments.Feb 6 2017, 5:09 AM
ELF/LinkerScript.cpp
266 ↗(On Diff #86262)

Let's say you have sections A and B. A defines symbol X, and B has an undefined symbol X. Naturally, the B's undefined symbol will be resolved >using A. But, what if you discard section A using a linker script? What will happen? Isn't this essentially the same situation as this?

So we have next.

SECTIONS { . = 0x1000; .A : { *(.A) } /DISCARD/ : { *(.B*) } }
.section .A,"a"
.quad foo

.section .B,"a"
foo:

What will happen? We will not fail on inputs above. BFD will, gold also does not. We resolve symbol VA to zero in that case.

But how it is can be relative to this patch ?

Both sections A and B apply relocations using InputSectionBase<ELFT>::relocate. If A refers to symbol defined in B and we discard B, it still should be possible to know about "issue", because we apply relocations and can find out that symbol is in discarded section during relocation loop.

In my case for --emit-relocs lets say I have A and .rela.A. And script discards A.
How .rela.A will know that it should be discarded then ?
I can iterate over all SHT_REL[A] sections separatelly and check that target sections are still alive. But is it better than use of DependentSection approach ?

One another solution I though about was:
copyRelocations() is called too late for SHT_REL[A] sections. At this step all I can do while iterating the relocations is
to switch them into R_X86_64_NONE, for example. That way we end up with relocation section with NONE relocations,
but this section should reference some target section. And that target section is discarded. That does not look an option.

grimar updated this revision to Diff 88324.Feb 14 2017, 12:40 AM
grimar retitled this revision from [ELF] - Added partial support for --emit-relocs (no --gc-section case) to [ELF] - Added support of linkerscript's "/DISCARD/" for --emit-relocs.
grimar edited the summary of this revision. (Show Details)
  • Rebased.
grimar updated this revision to Diff 88743.Feb 16 2017, 8:45 AM
grimar edited the summary of this revision. (Show Details)
  • Rebased.
  • Addred support for discarding .eh_frame

To support discarding of .eh_frame when in has relocation section,
I had to change where DependentSections lives, now it is a member of
InputSectionBase<ELFT>

grimar updated this revision to Diff 88745.Feb 16 2017, 8:51 AM
  • Renamed testcases.