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

[grimar]

Summary

Previously LLD crashed on on provided testcase because "/DISCARD/" was
not supported. Patch implements that.

[ruiu]

ELF/LinkerScript.cpp

266 ↗

(On Diff #86262)

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

[grimar]

• Addressed review comment.

[grimar]

ELF/LinkerScript.cpp

266 ↗

(On Diff #86262)

Done.

[ruiu]

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]

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]

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]

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]

Prepared alternative solution: D29636, is it better ?

[grimar]

• Rebased.

[grimar]

• Rebased.
• Addred support for discarding .eh_frame

[grimar]

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]

• Renamed testcases.

lld/trunk/ELF/InputFiles.cpp

case SHT_REL: {
     }
     assert(isUInt<31>(NumRelocations));
     Target->NumRelocations = NumRelocations;
 
     // Relocation sections processed by the linker are usually removed
     // from the output, so returning `nullptr` for the normal case.
     // However, if -emit-relocs is given, we need to leave them in the output.
     // (Some post link analysis tools need this information.)
-    if (Config->EmitRelocs)
-      return make<InputSection<ELFT>>(this, &Sec, Name);
+    if (Config->EmitRelocs) {
+      InputSection<ELFT> *RelocSec = make<InputSection<ELFT>>(this, &Sec, Name);
+      // We will not emit relocation section if target was discarded.
+      Target->DependentSections.push_back(RelocSec);
+      return RelocSec;
+    }
     return nullptr;
   }
   }
 
   // .note.GNU-stack is a marker section to control the presence of
   // PT_GNU_STACK segment in outputs. Since the presence of the segment
   // is controlled only by the command line option (-z execstack) in LLD,
   // .note.GNU-stack is ignored.

lld/trunk/test/ELF/linkerscript/emit-relocs-discard.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
+# RUN: echo "SECTIONS { /DISCARD/ : { *(.bbb) } }" > %t.script
+# RUN: ld.lld --emit-relocs --script %t.script %t.o -o %t1
+# RUN: llvm-readobj -r %t1 | FileCheck %s
+
+# CHECK:      Relocations [
+# CHECK-NEXT: ]
+
+.section .aaa,"",@progbits
+.Lfoo:
+
+.section .bbb,"",@progbits
+.long .Lfoo

lld/trunk/test/ELF/linkerscript/emit-relocs-ehframe-discard.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t1.o
+# RUN: echo "SECTIONS { /DISCARD/ : { *(.eh_frame) } }" > %t.script
+# RUN: ld.lld --emit-relocs --script %t.script %t1.o -o %t
+# RUN: llvm-objdump -section-headers %t | FileCheck %s
+
+# CHECK-NOT: .rela.eh_frame
+
+.section .foo,"ax",@progbits
+.cfi_startproc
+.cfi_endproc