[PATCH] D13651: [ELF2] - Initial lazy loading support (x86_x64)
George Rimar via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 13 06:41:12 PDT 2015
grimar added inline comments.
================
Comment at: ELF/OutputSections.cpp:39-41
@@ +38,5 @@
+ // .got.plt has 3 reserved entry
+ Entries.push_back(nullptr);
+ Entries.push_back(nullptr);
+ Entries.push_back(nullptr);
+}
----------------
ruiu wrote:
> Entries.resize(3);
Done.
================
Comment at: ELF/OutputSections.cpp:65-70
@@ +64,8 @@
+ for (const SymbolBody *B : Entries) {
+ uint8_t *Entry = Buf;
+ Buf += sizeof(uintX_t);
+ if (B) {
+ uint64_t Plt = Out<ELFT>::Plt->getEntryAddr(*B);
+ Target->writeGotPltEntry(Entry, Plt);
+ }
+ }
----------------
ruiu wrote:
> if (B)
> Target->writeGotPltEntry(Buf, Out<ELFT>::Plt->getEntryAddr(*B);
> Buf += sizeof(uintX_t);
>
Done.
================
Comment at: ELF/OutputSections.cpp:115
@@ +114,3 @@
+
+ // first write PLT[0] entry which is special
+ Target->writePltBaseEntry(Buf, Out<ELFT>::GotPlt->getVA(), this->getVA());
----------------
ruiu wrote:
> Start with an uppercase character and end with a period.
Done.
================
Comment at: ELF/OutputSections.cpp:116
@@ +115,3 @@
+ // first write PLT[0] entry which is special
+ Target->writePltBaseEntry(Buf, Out<ELFT>::GotPlt->getVA(), this->getVA());
+ Buf += Target->getPltBaseSize();
----------------
ruiu wrote:
> Is PLT base is a common terminology? I prefer writePltZeroEntry as the name of the function.
Changed to writePltZeroEntry. Also changed PltBaseSize to PltZeroEntrySize.
================
Comment at: ELF/OutputSections.cpp:174-208
@@ -125,31 +173,37 @@
bool CanBePreempted = canBePreempted(Body);
uintX_t Addend = 0;
if (!CanBePreempted) {
if (IsRela) {
if (Body)
Addend += getSymVA<ELFT>(cast<ELFSymbolBody<ELFT>>(*Body));
else
Addend += getLocalSymVA(
Obj.getRelocationSymbol(&RI, File.getSymbolTable()), File);
}
P->setSymbolAndType(0, Target->getRelativeReloc(), IsMips64EL);
}
if (Body && Target->relocNeedsGot(Type, *Body)) {
- P->r_offset = Out<ELFT>::Got->getEntryAddr(*Body);
+ bool NeedsPlt = Target->relocNeedsPlt(Type, *Body);
+ if (NeedsPlt)
+ P->r_offset = Out<ELFT>::GotPlt->getEntryAddr(*Body);
+ else
+ P->r_offset = Out<ELFT>::Got->getEntryAddr(*Body);
if (CanBePreempted)
P->setSymbolAndType(Body->getDynamicSymbolTableIndex(),
- Target->getGotReloc(), IsMips64EL);
+ NeedsPlt ? Target->getPltReloc()
+ : Target->getGotReloc(),
+ IsMips64EL);
} else {
if (IsRela)
Addend += static_cast<const Elf_Rela &>(RI).r_addend;
P->r_offset = RI.r_offset + C.getOutputSectionOff() + OutSec->getVA();
if (CanBePreempted)
P->setSymbolAndType(Body->getDynamicSymbolTableIndex(), Type,
IsMips64EL);
}
if (IsRela)
static_cast<Elf_Rela *>(P)->r_addend = Addend;
}
----------------
ruiu wrote:
> This code needs explaining. I'm not actually sure that this code is correct overall.
I added some comments above, under each of 2 code sections of my changes.
================
Comment at: ELF/OutputSections.cpp:192
@@ -141,1 +191,3 @@
+ else
+ P->r_offset = Out<ELFT>::Got->getEntryAddr(*Body);
if (CanBePreempted)
----------------
Each symbol that needs plt relocation is placed to Plt and GotPlt, otherwise to Got.
Also symbol can be placed both to Got and Plt+GotPlt, for example when we take address of function from DSO and also make a call to it.
So here depending on what type of relocation is we switch from which table to take the offset from.
================
Comment at: ELF/OutputSections.cpp:198
@@ -144,2 +197,3 @@
+ IsMips64EL);
} else {
if (IsRela)
----------------
Relocations of symbols in ".rela.dyn" and ".rela.plt" are different.
Symbols that NeedsPlt placed to .rela.plt and requred to be R_X86_64_JUMP_SLOT.
================
Comment at: ELF/OutputSections.cpp:762-766
@@ -696,2 +761,7 @@
+template class GotPltSection<ELF32LE>;
+template class GotPltSection<ELF32BE>;
+template class GotPltSection<ELF64LE>;
+template class GotPltSection<ELF64BE>;
+
template class PltSection<ELF32LE>;
----------------
ruiu wrote:
> Move this before template class GotSection<ELF32LE>;
Done.
================
Comment at: ELF/OutputSections.h:305
@@ -287,2 +304,3 @@
static GotSection<ELFT> *Got;
+ static GotPltSection<ELFT> *GotPlt;
static HashTableSection<ELFT> *HashTab;
----------------
ruiu wrote:
> Sort.
Done.
================
Comment at: ELF/OutputSections.h:320
@@ -300,2 +319,3 @@
template <class ELFT> GotSection<ELFT> *Out<ELFT>::Got;
+template <class ELFT> GotPltSection<ELFT> *Out<ELFT>::GotPlt;
template <class ELFT> HashTableSection<ELFT> *Out<ELFT>::HashTab;
----------------
ruiu wrote:
> Sort.
Done.
================
Comment at: ELF/Target.cpp:127-136
@@ +126,12 @@
+
+ uint64_t NextPC = PltEntryAddr + 6;
+ int64_t Delta = GotEntryAddr - NextPC + 8; //GOT + 8
+ assert(isInt<32>(Delta));
+ write32le(Buf + 2, Delta);
+ Buf += 6;
+
+ NextPC += 6;
+ Delta = GotEntryAddr - NextPC + 16; //GOT + 16
+ write32le(Buf + 2, Delta);
+ Buf += 6;
+}
----------------
ruiu wrote:
> write32le(Buf + 2, GotEntryAddr - PltEntryAddr + 2); // GOT+8
> write32le(Buf + 8, GotEntryAddr - PltEntryAddr + 4); // GOT+16
>
> No assert() needed.
Done.
================
Comment at: ELF/Target.cpp:147-161
@@ -109,7 +146,17 @@
memcpy(Buf, Inst, sizeof(Inst));
uint64_t NextPC = PltEntryAddr + 6;
int64_t Delta = GotEntryAddr - NextPC;
assert(isInt<32>(Delta));
write32le(Buf + 2, Delta);
+ Buf += 6;
+
+ NextPC += 5;
+ write32le(Buf + 1, Index);
+ Buf += 5;
+
+ NextPC += 5;
+ Delta = PltEntryAddr - Index * PltEntrySize - PltBaseSize - NextPC;
+ assert(isInt<32>(Delta));
+ write32le(Buf + 1, Delta);
}
----------------
ruiu wrote:
> write32le(Buf + 2, GotEntryAdrr - PltEntryAddr - 6);
> write32le(Buf + 7, Index);
> write32le(Buf + 11, ...);
>
Done. Just note: Buf + 12, not + 11.
http://reviews.llvm.org/D13651
More information about the llvm-commits
mailing list