[ELF] - Fix combination of -script and relocatable output.

[grimar]

Summary

Previously this was completely broken. If -script and -r were used together,
section headers had size 0, because assignOffsets or assignAddresses() were not called.

This patch fixes that, testcase demonstrates that linkerscript section rules (if any)
also works correctly.

[ruiu]

Shouldn't we simply report an error if both -relocatable and linker script SECTIONS command are used? It is hard to imagine that there's a valid use case for that combination.

[grimar]

In D24801#548662, @ruiu wrote:

Shouldn't we simply report an error if both -relocatable and linker script SECTIONS command are used? It is hard to imagine that there's a valid use case for that combination.

That was mentioned in summary for D24032 that we have such issue and also in comments for D24782. So I guess there is a use case. I fixed it because zero sized sections is too obvious issue and it was very simple to support the correct scripted behavior.

[• rafael]

ELF/LinkerScript.cpp

638

So Out<ELFT>::ProgramHeaders is the only outsection with a non 0 VA?

[grimar]

• Reimplemented.

ELF/LinkerScript.cpp

638

Relocatable output does not have program headers table, it probably was copypaste mistake.

And patch was not correct. We are calculating file offsets using VA now. It has also incorrect calculation
of output section size because of that, what is noticable in testcase.
I think if we want to support such functionality with minimal changes, the best way I can imagine is to let
script set VA and do what it do, but then set VA to 0 for all sections. That what I do in updated diff.

[• rafael]

The implementation is fine by me.

The only big question is: do we need this?

Peter, you mentioned the combination of -r and script on another review. Do you guys need this?

[ruiu]

ELF/Writer.cpp

276–277

I do not understand this piece of code. VAs are initialized to zero by default, no?

[ruiu]

That's what I was thinking. Linker scripts are generally to create final outputs. The behavior of the linker script in combination with -r is not well defined, I guess. I think we should rather ban the combination of -r and linker scripts so that we don't have to care about that.

[peter.smith]

I don't have a personal use case for ld -r and scripts, it is just one of the possible combinations that someone can use, and make exceptions more difficult.

The only use case that I know of from ld.bfd is the creation of linux kernel modules.
The script in question seems to be: https://github.com/torvalds/linux/blob/master/scripts/module-common.lds

As to whether we need them, I think anyone using linux will happily use ld.bfd or gold. For BSD there may not be an option other that lld. From what I can dig up via searching the web I found the makefile for BSD kernel modules: http://fxr.watson.org/fxr/source/conf/kmod.mk
This seems to use ld -r but does not use an ldscript:
.if ${__KLD_SHARED} == yes

232 ${KMOD}.kld: ${OBJS}
233 .else
234 ${FULLPROG}: ${OBJS}
235 .endif
236         ${LD} ${_LDFLAGS} -r -d -o ${.TARGET} ${OBJS}
237 .if ${MK_CTF} != "no"
238         ${CTFMERGE} ${CTFFLAGS} -o ${.TARGET} ${OBJS}
239 .endif

I'm not a Linux or BSD expert, my background is tools for bare-metal embedded systems. This may be out of date.

In summary I think a case could be made for not supporting the combination until someone actually needs it.

[grimar]

Closing as we seems do not plan to support this until have a reason to.

ELF/LinkerScript.h

public:
   std::vector<PhdrEntry<ELFT>> createPhdrs();
   bool ignoreInterpSection();
 
   ArrayRef<uint8_t> getFiller(StringRef Name);
   void writeDataBytes(StringRef Name, uint8_t *Buf);
   bool hasLMA(StringRef Name);
   bool shouldKeep(InputSectionBase<ELFT> *S);
   void assignOffsets(OutputSectionCommand *Cmd);
-  void assignAddresses(std::vector<PhdrEntry<ELFT>> &Phdrs);
+  void assignAddresses(std::vector<PhdrEntry<ELFT>> *Phdrs);
   bool hasPhdrsCommands();
   uint64_t getOutputSectionAddress(StringRef Name) override;
   uint64_t getOutputSectionSize(StringRef Name) override;
   uint64_t getOutputSectionAlign(StringRef Name) override;
   uint64_t getOutputSectionLMA(StringRef Name) override;
   uint64_t getHeaderSize() override;
   uint64_t getSymbolValue(StringRef S) override;
   bool isDefined(StringRef S) override;

ELF/LinkerScript.cpp

if (isa<OutputSectionCommand>(Cmd))
     return false;
   const auto *Assign = dyn_cast<SymbolAssignment>(&Cmd);
   if (!Assign)
     return true;
   return Assign->Name != ".";
 }
 
 template <class ELFT>
-void LinkerScript<ELFT>::assignAddresses(std::vector<PhdrEntry<ELFT>> &Phdrs) {
+void LinkerScript<ELFT>::assignAddresses(std::vector<PhdrEntry<ELFT>> *Phdrs) {
   // Orphan sections are sections present in the input files which
   // are not explicitly placed into the output file by the linker script.
   // We place orphan sections at end of file.
   // Other linkers places them using some heuristics as described in
   // https://sourceware.org/binutils/docs/ld/Orphan-Sections.html#Orphan-Sections.
 
   // The OutputSections are already in the correct order.
   // This loops creates or moves commands as needed so that they are in the
for (const std::unique_ptr<BaseCommand> &Base : Opt.Commands) {
     auto *Cmd = cast<OutputSectionCommand>(Base.get());
 
     if (Cmd->AddrExpr)
       Dot = Cmd->AddrExpr(Dot);
 
     assignOffsets(Cmd);
   }
 
+  if (Config->Relocatable)
+    return;
+
   uintX_t MinVA = std::numeric_limits<uintX_t>::max();
   for (OutputSectionBase<ELFT> *Sec : *OutputSections) {
     if (Sec->getFlags() & SHF_ALLOC)
       MinVA = std::min(MinVA, Sec->getVA());
     else
       Sec->setVA(0);
   }
 
   uintX_t HeaderSize = getHeaderSize();
   auto FirstPTLoad =
-      std::find_if(Phdrs.begin(), Phdrs.end(), [](const PhdrEntry<ELFT> &E) {
+      std::find_if(Phdrs->begin(), Phdrs->end(), [](const PhdrEntry<ELFT> &E) {
         return E.H.p_type == PT_LOAD;
       });
-  if (HeaderSize <= MinVA && FirstPTLoad != Phdrs.end()) {
+  if (HeaderSize <= MinVA && FirstPTLoad != Phdrs->end()) {
     // ELF and Program headers need to be right before the first section in
     // memory. Set their addresses accordingly.
     MinVA = alignDown(MinVA - HeaderSize, Target->PageSize);
     Out<ELFT>::ElfHeader->setVA(MinVA);
     Out<ELFT>::ProgramHeaders->setVA(Out<ELFT>::ElfHeader->getSize() + MinVA);
     FirstPTLoad->First = Out<ELFT>::ElfHeader;
     if (!FirstPTLoad->Last)
       FirstPTLoad->Last = Out<ELFT>::ProgramHeaders;

ELF/Writer.cpp

template <class ELFT> void Writer<ELFT>::run() {
   if (Config->Discard != DiscardPolicy::All)
     copyLocalSymbols();
 
   finalizeSections();
   if (HasError)
     return;
 
   if (Config->Relocatable) {
+    if (ScriptConfig->HasSections)
+      Script<ELFT>::X->assignAddresses(nullptr);
     assignFileOffsets();
+    for (OutputSectionBase<ELFT> *Sec : OutputSections)
+      Sec->setVA(0);
   } else {
     Phdrs = Script<ELFT>::X->hasPhdrsCommands() ? Script<ELFT>::X->createPhdrs()
                                                 : createPhdrs();
     fixHeaders();
     if (ScriptConfig->HasSections) {
-      Script<ELFT>::X->assignAddresses(Phdrs);
+      Script<ELFT>::X->assignAddresses(&Phdrs);
     } else {
       fixSectionAlignments();
       assignAddresses();
     }
 
     if (!Config->OFormatBinary)
       assignFileOffsets();
     else

test/ELF/linkerscript/relocatable.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
+
+# RUN: echo "SECTIONS { foo : { *(.foo*) } }" > %t.script
+# RUN: ld.lld -r -o %t.out --script %t.script %t
+# RUN: llvm-objdump -section-headers %t.out | FileCheck %s
+
+# CHECK:      Sections:
+# CHECK-NEXT: Idx Name          Size      Address          Type
+# CHECK-NEXT:   0               00000000 0000000000000000
+# CHECK-NEXT:   1 foo           00000008 0000000000000000 DATA
+# CHECK-NEXT:   2 .text         00000001 0000000000000000 TEXT DATA
+# CHECK-NEXT:   3 .bar1         00000004 0000000000000000 DATA
+# CHECK-NEXT:   4 .bar2         00000004 0000000000000000 DATA
+
+.global _start
+_start:
+  nop
+
+.section .foo1, "a"
+foo1:
+ .long 1
+
+.section .foo2, "a"
+foo2:
+ .long 1
+
+.section .bar1, "aw"
+bar1:
+ .long 1
+
+.section .bar2, "aw"
+bar2:
+ .long 1