[PATCH] D18960: [lld] Initial implementation of TLSDESC relocation handling
George Rimar via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 11 08:46:28 PDT 2016
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
More information about the llvm-commits
mailing list