[ELF] Linkerscript: Try to put section to the last PT_LOAD, in case there is no exact segment specification

[evgeny777]

Summary

Some scripts may look like this:

PHDRS { all  PT_LOAD; }
SECTIONS {
   .aaa : { *(.aaa) } : all;
   .bbb : { *(.bbb) }
}

This patch will try to put section .bbb to segment 'all'

[ruiu]

ELF/LinkerScript.cpp

670–680

I do not understand what this code does. Can you elaborate?

[evgeny777]

I've rewritten it and added comments. Hope it's more clear now.

[evgeny777]

Simplified

[evgeny777]

Ping

[evgeny777]

Ping

[evgeny777]

Addressed comments from Rafael

[• rafael]

ELF/LinkerScript.cpp

652

Add a comment saying what you are doing. If I understand it, this just puts the first PT_LOAD from PHDRS in the vector. Also add a comment saying what the vector is.

660

This if is not necessary anymore, no?

[evgeny777]

ELF/LinkerScript.cpp

652

Yes you're right. If we don't have phdr spec for section and hasn't added any section yet, then we'll simply put section to the very first PT_LOAD found. Here is an example:

PHDRS { segment PT_LOAD; }
SECTIONS {
   .aaa : { *(.aaa) }
   .bbb : { *(.bbb) } : segment
}

Of course this script is malformed, but this happens.

660

Not exactly. You may have no PT_LOAD segments in PHDRS directive. GNU gold (bunutils 2.26) crashes in such case. I think we shouldn't

[evgeny777]

ELF/LinkerScript.cpp

660

On the second look I see that you're right. Will update diff.

[evgeny777]

Posted new diff to wrong review and got green mark
https://reviews.llvm.org/D25765

ELF/LinkerScript.h

private:
 
   void addSection(OutputSectionFactory<ELFT> &Factory,
                   InputSectionBase<ELFT> *Sec, StringRef Name);
   void discard(ArrayRef<InputSectionBase<ELFT> *> V);
 
   std::vector<InputSectionBase<ELFT> *>
   createInputSectionList(OutputSectionCommand &Cmd);
 
+  bool canAdjustPhdrFlags(unsigned PhdrId);
+
   // "ScriptConfig" is a bit too long, so define a short name for it.
   ScriptConfiguration &Opt = *ScriptConfig;
 
   std::vector<size_t> getPhdrIndices(StringRef SectionName);
   size_t getPhdrIndex(StringRef PhdrName);
 
   uintX_t Dot;
   OutputSectionBase<ELFT> *CurOutSec = nullptr;

ELF/LinkerScript.cpp

template <class ELFT> void LinkerScript<ELFT>::assignAddresses() {
 
   // 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);
 }
 
+template <class ELFT>
+bool LinkerScript<ELFT>::canAdjustPhdrFlags(unsigned PhdrId) {
+  return Opt.PhdrsCommands.size() > PhdrId &&
+         Opt.PhdrsCommands[PhdrId].Flags == UINT_MAX;
+}
+
 // Creates program headers as instructed by PHDRS linker script command.
 template <class ELFT>
 std::vector<PhdrEntry<ELFT>> LinkerScript<ELFT>::createPhdrs() {
   std::vector<PhdrEntry<ELFT>> Ret;
 
   // Process PHDRS and FILEHDR keywords because they are not
   // real output sections and cannot be added in the following loop.
   for (const PhdrsCommand &Cmd : Opt.PhdrsCommands) {
     Ret.emplace_back(Cmd.Type, Cmd.Flags == UINT_MAX ? PF_R : Cmd.Flags);
     PhdrEntry<ELFT> &Phdr = Ret.back();
 
     if (Cmd.HasFilehdr)
       Phdr.add(Out<ELFT>::ElfHeader);
     if (Cmd.HasPhdrs)
       Phdr.add(Out<ELFT>::ProgramHeaders);
 
     if (Cmd.LMAExpr) {
       Phdr.H.p_paddr = Cmd.LMAExpr(0);
       Phdr.HasLMA = true;
     }
   }
 
   // Add output sections to program headers.
-  PhdrEntry<ELFT> *Load = nullptr;
-  uintX_t Flags = PF_R;
+  unsigned LoadId = UINT_MAX;
   for (OutputSectionBase<ELFT> *Sec : *OutputSections) {
     if (!(Sec->getFlags() & SHF_ALLOC))
       break;
 
     std::vector<size_t> PhdrIds = getPhdrIndices(Sec->getName());
     if (!PhdrIds.empty()) {
       // Assign headers specified by linker script
       for (size_t Id : PhdrIds) {
         Ret[Id].add(Sec);
         if (Opt.PhdrsCommands[Id].Flags == UINT_MAX)
           Ret[Id].H.p_flags |= Sec->getPhdrFlags();
+        if (Ret[Id].H.p_type == PT_LOAD)
+          LoadId = Id;
       }
     } else {
-      // If we have no load segment or flags've changed then we want new load
-      // segment.
-      uintX_t NewFlags = Sec->getPhdrFlags();
-      if (Load == nullptr || Flags != NewFlags) {
-        Load = &*Ret.emplace(Ret.end(), PT_LOAD, NewFlags);
-        Flags = NewFlags;
+      uintX_t Flags = Sec->getPhdrFlags();
+      // Try to put section to previously created segment.
+      // We can do this in the following cases:
+      //  a) Previous segment flags and section flags are identical.
+      //  b) Previous segment is listed in PHDRS block with no flag
+      //     specification. In such case we're ORing the segment flags.
+      if (LoadId == UINT_MAX ||
+          ((Flags != Ret[LoadId].H.p_flags) && !canAdjustPhdrFlags(LoadId))) {
+        Ret.emplace_back(PT_LOAD, 0);
+        LoadId = Ret.size() - 1;
       }
-      Load->add(Sec);
+      Ret[LoadId].add(Sec);
+      Ret[LoadId].H.p_flags |= Flags;
     }
   }
   return Ret;
 }
 
 template <class ELFT> bool LinkerScript<ELFT>::ignoreInterpSection() {
   // Ignore .interp section in case we have PHDRS specification
   // and PT_INTERP isn't listed.

test/ELF/linkerscript/phdrs-flags.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
 # RUN: echo "PHDRS {all PT_LOAD FILEHDR PHDRS FLAGS (1 | 1 + 0x1);} \
 # RUN:       SECTIONS { \
 # RUN:           . = 0x10000200; \
 # RUN:           .text : {*(.text*)} :all \
 # RUN:           .foo : {*(.foo.*)} :all \
 # RUN:           .data : {*(.data.*)} :all}" > %t.script
-
 # RUN: ld.lld -o %t1 --script %t.script %t
 # RUN: llvm-readobj -program-headers %t1 | FileCheck %s
+
+# RUN: echo "PHDRS {all PT_LOAD FILEHDR PHDRS FLAGS (0x1);} \
+# RUN:       SECTIONS { \
+# RUN:           . = 0x10000200; \
+# RUN:           .text : {*(.text*)} :all \
+# RUN:           .foo : {*(.foo.*)}  \
+# RUN:           .data : {*(.data.*)} }" > %t.script
+# RUN: ld.lld -o %t1 --script %t.script %t
+# RUN: llvm-readobj -program-headers %t1 | FileCheck --check-prefix=DEFHDR %s
+
 # CHECK:     ProgramHeaders [
 # CHECK-NEXT:  ProgramHeader {
 # CHECK-NEXT:    Type: PT_LOAD (0x1)
 # CHECK-NEXT:    Offset: 0x0
 # CHECK-NEXT:    VirtualAddress: 0x10000000
 # CHECK-NEXT:    PhysicalAddress: 0x10000000
 # CHECK-NEXT:    FileSize: 521
 # CHECK-NEXT:    MemSize: 521
 # CHECK-NEXT:    Flags [
 # CHECK-NEXT:      PF_W (0x2)
 # CHECK-NEXT:      PF_X (0x1)
 # CHECK-NEXT:    ]
 
+# DEFHDR:     ProgramHeaders [
+# DEFHDR-NEXT:  ProgramHeader {
+# DEFHDR-NEXT:    Type: PT_LOAD (0x1)
+# DEFHDR-NEXT:    Offset: 0x0
+# DEFHDR-NEXT:    VirtualAddress: 0x10000000
+# DEFHDR-NEXT:    PhysicalAddress: 0x10000000
+# DEFHDR-NEXT:    FileSize: 513
+# DEFHDR-NEXT:    MemSize: 513
+# DEFHDR-NEXT:    Flags [ (0x1)
+# DEFHDR-NEXT:      PF_X (0x1)
+# DEFHDR-NEXT:    ]
+# DEFHDR-NEXT:    Alignment: 4096
+# DEFHDR-NEXT:  }
+# DEFHDR-NEXT:  ProgramHeader {
+# DEFHDR-NEXT:    Type: PT_LOAD (0x1)
+# DEFHDR-NEXT:    Offset: 0x201
+# DEFHDR-NEXT:    VirtualAddress: 0x10000201
+# DEFHDR-NEXT:    PhysicalAddress: 0x10000201
+# DEFHDR-NEXT:    FileSize: 8
+# DEFHDR-NEXT:    MemSize: 8
+# DEFHDR-NEXT:    Flags [ (0x6)
+# DEFHDR-NEXT:      PF_R (0x4)
+# DEFHDR-NEXT:      PF_W (0x2)
+# DEFHDR-NEXT:    ]
+# DEFHDR-NEXT:    Alignment: 4096
+# DEFHDR-NEXT:  }
+# DEFHDR-NEXT: ]
+
 .global _start
 _start:
  nop
 
 .section .foo.1,"a"
 foo1:
  .long 0
 
 .section .foo.2,"aw"
 foo2:
  .long 0

test/ELF/linkerscript/phdrs.s

 # RUN:       SECTIONS { \
 # RUN:           . = 0x10000200; \
 # RUN:           .text : {*(.text*)} :all \
 # RUN:           .foo : {*(.foo.*)} :all \
 # RUN:           .data : {*(.data.*)} :all}" > %t.script
 # RUN: ld.lld -o %t1 --script %t.script %t
 # RUN: llvm-readobj -program-headers %t1 | FileCheck --check-prefix=AT %s
 
+# RUN: echo "PHDRS {all PT_LOAD FILEHDR PHDRS ;} \
+# RUN:       SECTIONS { \
+# RUN:           . = 0x10000200; \
+# RUN:           .text : {*(.text*)} :all \
+# RUN:           .foo : {*(.foo.*)}  \
+# RUN:           .data : {*(.data.*)} }" > %t.script
+# RUN: ld.lld -o %t1 --script %t.script %t
+# RUN: llvm-readobj -program-headers %t1 | FileCheck --check-prefix=DEFHDR %s
+
 # CHECK:     ProgramHeaders [
 # CHECK-NEXT:  ProgramHeader {
 # CHECK-NEXT:    Type: PT_LOAD (0x1)
 # CHECK-NEXT:    Offset: 0x0
 # CHECK-NEXT:    VirtualAddress: 0x10000000
 # CHECK-NEXT:    PhysicalAddress: 0x10000000
 # CHECK-NEXT:    FileSize: 521
 # CHECK-NEXT:    MemSize: 521

 # AT-NEXT:      FileSize: 521
 # AT-NEXT:      MemSize: 521
 # AT-NEXT:      Flags [ (0x7)
 # AT-NEXT:        PF_R (0x4)
 # AT-NEXT:        PF_W (0x2)
 # AT-NEXT:        PF_X (0x1)
 # AT-NEXT:      ]
 
+# DEFHDR:     ProgramHeaders [
+# DEFHDR-NEXT:  ProgramHeader {
+# DEFHDR-NEXT:    Type: PT_LOAD (0x1)
+# DEFHDR-NEXT:    Offset: 0x0
+# DEFHDR-NEXT:    VirtualAddress: 0x10000000
+# DEFHDR-NEXT:    PhysicalAddress: 0x10000000
+# DEFHDR-NEXT:    FileSize: 521
+# DEFHDR-NEXT:    MemSize: 521
+# DEFHDR-NEXT:    Flags [ (0x7)
+# DEFHDR-NEXT:      PF_R (0x4)
+# DEFHDR-NEXT:      PF_W (0x2)
+# DEFHDR-NEXT:      PF_X (0x1)
+# DEFHDR-NEXT:    ]
+
 .global _start
 _start:
  nop
 
 .section .foo.1,"a"
 foo1:
  .long 0
 
 .section .foo.2,"aw"
 foo2:
  .long 0