[PATCH] D18960: [lld] Initial implementation of TLSDESC relocation handling

[George Rimar via llvm-commits]

grimar added a subscriber: grimar.

================
Comment at: ELF/OutputSections.cpp:64
@@ -62,2 +63,3 @@
 
 template <class ELFT> void GotPltSection<ELFT>::addEntry(SymbolBody &Sym) {
+  Sym.GotPltIndex = Target->GotPltHeaderEntriesNum +
----------------
Method does not seem to be clang formatted.

================
Comment at: ELF/OutputSections.cpp:76
@@ -66,1 +75,3 @@
+  Entries[DynReloc_TlsDesc].push_back(nullptr);
+  Entries[DynReloc_TlsDesc].push_back(nullptr);
 }
----------------
I would write:

```
Entries[DynReloc_TlsDesc].insert(Entries[DynReloc_TlsDesc].end(), { nullptr, nullptr });
```
As in fact this is single entry

================
Comment at: ELF/OutputSections.cpp:84
@@ -72,1 +83,3 @@
 template <class ELFT> void GotPltSection<ELFT>::finalize() {
+  size_t entriesSize = Entries[DynReloc_JumpSlot].size() +
+                       Entries[DynReloc_TlsDesc].size();
----------------
```
EntriesSize 
```

================
Comment at: ELF/OutputSections.cpp:335
@@ -291,3 +334,3 @@
     : OutputSectionBase<ELFT>(Name, Config->Rela ? SHT_RELA : SHT_REL,
-                              SHF_ALLOC) {
+                              SHF_ALLOC), Relocs(DynReloc_NumKind) {
   this->Header.sh_entsize = Config->Rela ? sizeof(Elf_Rela) : sizeof(Elf_Rel);
----------------
Not formatted.

================
Comment at: ELF/OutputSections.cpp:351
@@ -302,11 +350,3 @@
 template <class ELFT> void RelocationSection<ELFT>::writeTo(uint8_t *Buf) {
-  for (const DynamicReloc<ELFT> &Rel : Relocs) {
-    auto *P = reinterpret_cast<Elf_Rela *>(Buf);
-    Buf += Config->Rela ? sizeof(Elf_Rela) : sizeof(Elf_Rel);
-    SymbolBody *Sym = Rel.Sym;
-
-    if (Config->Rela)
-      P->r_addend = Rel.UseSymVA ? Sym->getVA<ELFT>(Rel.Addend) : Rel.Addend;
-    P->r_offset = Rel.OffsetInSec + Rel.OffsetSec->getVA();
-    uint32_t SymIdx = (!Rel.UseSymVA && Sym) ? Sym->DynsymIndex : 0;
-    P->setSymbolAndType(SymIdx, Rel.Type, Config->Mips64EL);
+  for (auto RelocVec : Relocs) {
+    for (const DynamicReloc<ELFT> &Rel : RelocVec) {
----------------
I don't think we use auto in such cases as type is not obvious here. So I would use

```
  for (std::vector<DynamicReloc<ELFT>> &Vec : Relocs) {
    for (const DynamicReloc<ELFT> &Rel : Vec) {
```

================
Comment at: ELF/OutputSections.h:153
@@ +152,3 @@
+  DynReloc_NumKind = DynReloc_TlsDesc + 1
+};
+
----------------
Do you really need direct assignmens here ? Why don't write like:

```
  DynReloc_JumpSlot,
  DynReloc_TlsDesc,
  DynReloc_NumKind
```

================
Comment at: ELF/Symbols.cpp:171
@@ +170,3 @@
+template <class ELFT>
+typename ELFT::uint SymbolBody::getGotPltTlsDescVA() const {
+  return Out<ELFT>::GotPlt->getVA() +
----------------
This one not formatted either. Please use clang-format for this patch.

================
Comment at: ELF/Target.cpp:177
@@ -176,2 +176,3 @@
                    uint64_t SA) const override;
+
   size_t relaxTlsGdToLe(uint8_t *Loc, uint8_t *BufEnd, uint32_t Type,
----------------
Remove the empty line please.


Repository:
  rL LLVM

http://reviews.llvm.org/D18960