[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

962

can you expand "auto"?

963

nit: fit in one line

[atanasyan]

lib/ReaderWriter/ELF/SectionChunks.h

962

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.

963

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

[ruiu]

lib/ReaderWriter/ELF/SectionChunks.h

962

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.

963

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

 
 template <class ELFT> class RelocationTable : public Section<ELFT> {
 public:
   typedef llvm::object::Elf_Rel_Impl<ELFT, false> Elf_Rel;
   typedef llvm::object::Elf_Rel_Impl<ELFT, true> Elf_Rela;
 
   RelocationTable(const ELFLinkingContext &context, StringRef str,
                   int32_t order)
-      : Section<ELFT>(context, str), _symbolTable(nullptr) {
+      : Section<ELFT>(context, str), _symbolTable(nullptr),
+        _canModifyReadonlySection(false) {
     this->setOrder(order);
     this->_flags = SHF_ALLOC;
     // Set the alignment properly depending on the target architecture
     if (context.is64Bits())
       this->_align2 = 8;
     else
       this->_align2 = 4;
     if (context.isRelaOutputFormat()) {
       this->_entSize = sizeof(Elf_Rela);
       this->_type = SHT_RELA;
     } else {
       this->_entSize = sizeof(Elf_Rel);
       this->_type = SHT_REL;
     }
   }
 
   /// \returns the index of the relocation added.
   uint32_t addRelocation(const DefinedAtom &da, const Reference &r) {
     _relocs.emplace_back(&da, &r);
     this->_fsize = _relocs.size() * this->_entSize;
     this->_msize = this->_fsize;
+    if ((da.permissions() & DefinedAtom::permRW_) != DefinedAtom::permRW_)
+      _canModifyReadonlySection = true;
     return _relocs.size() - 1;
   }
 
   bool getRelocationIndex(const Reference &r, uint32_t &res) {
     auto rel = std::find_if(
         _relocs.begin(), _relocs.end(),
         [&](const std::pair<const DefinedAtom *, const Reference *> &p) {
       if (p.second == &r)
         return true;
       return false;
     });
     if (rel == _relocs.end())
       return false;
     res = std::distance(_relocs.begin(), rel);
     return true;
   }
 
   void setSymbolTable(const DynamicSymbolTable<ELFT> *symbolTable) {
     _symbolTable = symbolTable;
   }
 
+  /// \brief There is at least one relocation which can modify
+  /// a read-only section.
+  bool canModifyReadonlySection() const { return _canModifyReadonlySection; }
+
   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) {
     uint8_t *chunkBuffer = buffer.getBufferStart();
     uint8_t *dest = chunkBuffer + this->fileOffset();
     for (const auto &rel : _relocs) {
       if (this->_context.isRelaOutputFormat())
         writeRela(writer, *reinterpret_cast<Elf_Rela *>(dest), *rel.first,
                   *rel.second);
       else
         writeRel(writer, *reinterpret_cast<Elf_Rel *>(dest), *rel.first,
                  *rel.second);
       dest += this->_entSize;
     }
   }
 
 private:
   std::vector<std::pair<const DefinedAtom *, const Reference *> > _relocs;
   const DynamicSymbolTable<ELFT> *_symbolTable;
+  bool _canModifyReadonlySection;
 
   void writeRela(ELFWriter *writer, Elf_Rela &r, const DefinedAtom &atom,
                  const Reference &ref) {
     uint32_t index =
         _symbolTable ? _symbolTable->getSymbolTableIndex(ref.target())
                      : (uint32_t)STN_UNDEF;
     r.setSymbolAndType(index, ref.kindValue());
     r.r_offset = writer->addressOfAtom(&atom) + ref.offsetInAtom();
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