[ELF] Emit DT_TEXTREL dynamic table flag

[atanasyan]

Summary

If one or more dynamic relocation might modify a read-only section, dynamic table should contain DT_TEXTREL tag.

The patch introduces new RelocationTable::canModifyReadonlySection() method. This method checks through the relocations to see if any modifies a read-only section. The DynamicTable class calls this method and emits the DT_TEXTREL tag if necessary.

[• shankar.easwaran]

This patch LGTM but do you think it's better to have a pass ?

[atanasyan]

Good point. I will send new version of the patch soon.

[atanasyan]

In this version of thу patch the RelocationTable::canModifyReadonlySection() method iterates over relocations to see if any one modifies a read-only section each time when we need this information.

[ruiu]

LGTM

lib/ReaderWriter/ELF/SectionChunks.h

959

can you expand "auto"?

960

nit: fit in one line

[atanasyan]

lib/ReaderWriter/ELF/SectionChunks.h

959

The type name of rel is rather long - std::pair<const DefinedAtom *, const Reference *>. It is possible to make it shorter using typedefs but I cannot invent a good short name.

960

Yeah, the expression does not look good. But how can we fit it into one line?

[ruiu]

lib/ReaderWriter/ELF/SectionChunks.h

959

I'd probably write like this

for (const auto &rel : _relocs) {

const DefinedAtom *atom = rel.first
...

But I feel it might be a bit verbose. It's up to you.

960

It looked me that it would fit in 80 chars. It actually isn't. Sorry.

[joerg]

Can you also add the corresponding --warn-shared-textrel option?
It would also be nice to indicate which section at what offset by which relocation created the text relocation.
The lack of any hint towards what triggered the text relocation is one of the most annoying "features" of GNU ld.

[atanasyan]

Sure, I plan to add the --warn-shared-textrel option in a separate patch.

[atanasyan]

Thanks for review.

Closed by commit rL208670.

lib/ReaderWriter/ELF/SectionChunks.h

bool getRelocationIndex(const Reference &r, uint32_t &res) {
     res = std::distance(_relocs.begin(), rel);
     return true;
   }
 
   void setSymbolTable(const DynamicSymbolTable<ELFT> *symbolTable) {
     _symbolTable = symbolTable;
   }
 
+  /// \brief Check if any relocation modifies a read-only section.
+  bool canModifyReadonlySection() const {
+    for (const auto &rel : _relocs)
+      if ((rel.first->permissions() & DefinedAtom::permRW_) !=
+          DefinedAtom::permRW_)
+        return true;
+    return false;
+  }
+
   virtual void finalize() {
     this->_link = _symbolTable ? _symbolTable->ordinal() : 0;
     if (this->_parent)
       this->_parent->setLink(this->_link);
   }
 
   virtual void write(ELFWriter *writer, TargetLayout<ELFT> &layout,
                      llvm::FileOutputBuffer &buffer) {
virtual void createDefaultEntries() {
     _dt_fini_arraysz = addEntry(dyn);
     if (_layout.hasDynamicRelocationTable()) {
       dyn.d_tag = isRela ? DT_RELA : DT_REL;
       _dt_rela = addEntry(dyn);
       dyn.d_tag = isRela ? DT_RELASZ : DT_RELSZ;
       _dt_relasz = addEntry(dyn);
       dyn.d_tag = isRela ? DT_RELAENT : DT_RELENT;
       _dt_relaent = addEntry(dyn);
+
+      if (_layout.getDynamicRelocationTable()->canModifyReadonlySection()) {
+        dyn.d_tag = DT_TEXTREL;
+        _dt_textrel = addEntry(dyn);
+      }
     }
     if (_layout.hasPLTRelocationTable()) {
       dyn.d_tag = DT_PLTRELSZ;
       _dt_pltrelsz = addEntry(dyn);
       dyn.d_tag = getGotPltTag();
       _dt_pltgot = addEntry(dyn);
       dyn.d_tag = DT_PLTREL;
       dyn.d_un.d_val = isRela ? DT_RELA : DT_REL;
private:
   std::size_t _dt_strsz;
   std::size_t _dt_syment;
   std::size_t _dt_pltrelsz;
   std::size_t _dt_pltgot;
   std::size_t _dt_pltrel;
   std::size_t _dt_jmprel;
   std::size_t _dt_fini_array;
   std::size_t _dt_fini_arraysz;
+  std::size_t _dt_textrel;
   TargetLayout<ELFT> &_layout;
   DynamicSymbolTable<ELFT> *_dynamicSymbolTable;
   HashSection<ELFT> *_hashTable;
 };
 
 template <class ELFT> class InterpSection : public Section<ELFT> {
 public:
   InterpSection(const ELFLinkingContext &context, StringRef str, int32_t order,

test/elf/Mips/dt-textrel.test

+# Check that if a dynamic relocation modify a read-only section,
+# .dynamic section contains the DT_TEXTREL tag.
+
+# RUN: yaml2obj -format=elf %S/Inputs/pic-obj.yaml > %t-so-obj
+# RUN: lld -flavor gnu -target mipsel -shared -o %t-so %t-so-obj
+# RUN: yaml2obj -format=elf %s > %t-obj
+# RUN: lld -flavor gnu -target mipsel -e T0 -o %t-exe %t-obj %t-so
+# RUN: llvm-readobj -dynamic-table %t-exe | FileCheck %s
+
+# CHECK: 0x{{[0-9A-F]+}} TEXTREL
+
+FileHeader:
+  Class:           ELFCLASS32
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_MIPS
+  Flags:           [ EF_MIPS_NOREORDER, EF_MIPS_PIC, EF_MIPS_CPIC,
+                     EF_MIPS_ABI_O32, EF_MIPS_ARCH_32 ]
+Sections:
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    AddressAlign:    0x04
+    Content:         '00000000'
+
+  - Name:            .rel.text
+    Type:            SHT_REL
+    Link:            .symtab
+    Info:            .text
+    AddressAlign:    0x04
+    Relocations:
+      - Offset:      0
+        Symbol:      T1
+        Type:        R_MIPS_32
+
+Symbols:
+  Local:
+    - Name:            .text
+      Type:            STT_SECTION
+      Section:         .text
+
+  Global:
+    - Name:            T0
+      Type:            STT_FUNC
+      Section:         .text
+      Size:            0x04
+    - Name:            T1