[ELF] - DT_TEXTREL/DF_TEXTREL flags implemented.

[grimar]

Summary

This patch implements DT_TEXTREL/DF_TEXTREL flags. Actually I am not sure in current implementation and would be happy to know the side opinion about it.
Problem of that feature is that somehow we should know that relocation should cause a modification of read only segment.

To do that I am checking that relocation OKind field is DynamicReloc<ELFT>::Off_Sec. Looks it is safe and correct way to use it for that ?
Also for simplification I am not iterating through sections in LOADs and just checking the read/write flag of output sections when adding relocations during Writer<ELFT>::scanRelocs().

DT_TEXTREL description:
This member's absence signifies that no relocation entry should cause a modification to a non-writable segment, as specified by the segment permissions in the program header table. If this member is present, one or more relocation entries might request modifications to a non-writable segment, and the dynamic linker can prepare accordingly. This entry is at level 2. Its use has been superseded by the DF_TEXTREL flag.

[davide]

ELF/OutputSections.cpp

245 ↗

(On Diff #49750)

I'm not entirely sure this is correct. Ideally you should scan through the list of segments that have dynamic rels and set the flag if flags & PF_W and type == PT_LOAD.

621 ↗

(On Diff #49750)

So, DT_TEXTREL tag is not needed. It's for compatibility with older loaders, which is unlikely we're going to support anyway. I'd like to keep it away until somebody asks for it.

[davide]

Also, very nitpicky comment, DT_* are generally referred as tags and not only flags in ELF jargon.
You might want to adjust your commit message accordingly.

[• rafael]

Is this actually needed?

It was noticed in freebsd because we were incorrectly creating text
relocations. Short of bugs, is it actually common to have relocations
pointing to text these days?

Cheers,
Rafael

[grimar]

In D17857#367522, @rafael wrote:

Is this actually needed?

It was noticed in freebsd because we were incorrectly creating text
relocations. Short of bugs, is it actually common to have relocations
pointing to text these days?

Cheers,
Rafael

Doesn't all R_*_RELATIVE relocations are doing that ? I think they are used still at least for some targets. But I dont know actually how much it is needed.

ELF/OutputSections.cpp

245 ↗

(On Diff #49750)

Yep, I mentioned that simplification in commit message. I think it can work and does not need the one more iteration. I am not completely sure though.

[grimar]

I am sorry, that was closed by mistake.

I placed wrong "Differential revision: " ID when commited another patch:
http://reviews.llvm.org/D17817, r262650.

Per discussion in llvm-mails, this one will not be sumbited until a real need comes up,
so that patch should have be abandoned for now anyways.

lld/trunk/ELF/Writer.cpp

 
 // Used for relocatable output (-r). In this case we create only ELF file
 // header, do not create program headers. Also assign of section addresses
 // is very straightforward: we just put all sections sequentually to the file.
 template <class ELFT> void Writer<ELFT>::assignAddressesRelocatable() {
   Out<ELFT>::ElfHeader->setSize(sizeof(Elf_Ehdr));
   uintX_t FileOff = 0;
   for (OutputSectionBase<ELFT> *Sec : OutputSections) {
+    if (Sec->getType() != SHT_NOBITS)
       FileOff = alignTo(FileOff, Sec->getAlign());
     Sec->setFileOffset(FileOff);
+    if (Sec->getType() != SHT_NOBITS)
       FileOff += Sec->getSize();
   }
   SectionHeaderOff = alignTo(FileOff, sizeof(uintX_t));
   FileSize = SectionHeaderOff + getNumSections() * sizeof(Elf_Shdr);
 }
 
 // Visits all headers in PhdrTable and assigns the adresses to
 // the output sections. Also creates common and special headers.
 template <class ELFT> void Writer<ELFT>::assignAddresses() {

lld/trunk/test/ELF/relocatable-bss.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t1.o
+# RUN: ld.lld -r %t1.o -o %t
+# RUN: llvm-readobj -file-headers -sections -program-headers -symbols -r %t | FileCheck %s
+
+## We check here that .bss does not occupy the space in file.
+## If it would, the SectionHeaderOffset would have offset about 5 megabytes.
+# CHECK:       ElfHeader {
+# CHECK-NEXT:  Ident {
+# CHECK-NEXT:    Magic: (7F 45 4C 46)
+# CHECK-NEXT:    Class: 64-bit
+# CHECK-NEXT:    DataEncoding: LittleEndian
+# CHECK-NEXT:    FileVersion: 1
+# CHECK-NEXT:    OS/ABI: SystemV
+# CHECK-NEXT:    ABIVersion: 0
+# CHECK-NEXT:    Unused: (00 00 00 00 00 00 00)
+# CHECK-NEXT:  }
+# CHECK-NEXT:  Type: Relocatable
+# CHECK-NEXT:  Machine: EM_X86_64
+# CHECK-NEXT:  Version:
+# CHECK-NEXT:  Entry:
+# CHECK-NEXT:   ProgramHeaderOffset:
+# CHECK-NEXT:   SectionHeaderOffset: 0xA8
+# CHECK-NEXT:  Flags [
+# CHECK-NEXT:  ]
+# CHECK-NEXT:  HeaderSize:
+# CHECK-NEXT:  ProgramHeaderEntrySize:
+# CHECK-NEXT:  ProgramHeaderCount:
+# CHECK-NEXT:  SectionHeaderEntrySize:
+# CHECK-NEXT:  SectionHeaderCount:
+# CHECK-NEXT:  StringTableSectionIndex:
+# CHECK-NEXT:  }
+
+.text
+.globl _start;
+_start:
+ nop
+
+.bss
+ .space 5242880

lld/trunk/test/ELF/relocatable.s

 # CHECK-NEXT:    ABIVersion: 0
 # CHECK-NEXT:    Unused: (00 00 00 00 00 00 00)
 # CHECK-NEXT:  }
 # CHECK-NEXT:  Type: Relocatable
 # CHECK-NEXT:  Machine: EM_X86_64
 # CHECK-NEXT:  Version: 1
 # CHECK-NEXT:  Entry: 0x0
 # CHECK-NEXT:  ProgramHeaderOffset: 0x0
-# CHECK-NEXT:  SectionHeaderOffset: 0x2D8
+# CHECK-NEXT:  SectionHeaderOffset: 0x2C0
 # CHECK-NEXT:  Flags [
 # CHECK-NEXT:  ]
 # CHECK-NEXT:  HeaderSize: 64
 # CHECK-NEXT:  ProgramHeaderEntrySize: 0
 # CHECK-NEXT:  ProgramHeaderCount: 0
 # CHECK-NEXT:  SectionHeaderEntrySize: 64
 # CHECK-NEXT:  SectionHeaderCount: 7
 # CHECK-NEXT:  StringTableSectionIndex: 5