[PATCH] D13856: [ELF2] - Lazy relocation support for x86_64.
Rui Ueyama via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 19 12:13:37 PDT 2015
ruiu added inline comments.
================
Comment at: ELF/OutputSections.cpp:312
@@ -253,1 +311,3 @@
}
+ // RelaPlt can be missing if target does not supports lazy relocations.
+ if (Out<ELFT>::RelaPlt && Out<ELFT>::RelaPlt->hasRelocs()) {
----------------
Remove comment
================
Comment at: ELF/OutputSections.cpp:386
@@ -320,1 +385,3 @@
}
+ // RelaPlt can be missing if target does not supports lazy relocations.
+ if (Out<ELFT>::RelaPlt && Out<ELFT>::RelaPlt->hasRelocs()) {
----------------
Ditto
================
Comment at: ELF/OutputSections.h:113-114
@@ +112,4 @@
+class GotPltSection final : public OutputSectionBase<ELFT> {
+ typedef OutputSectionBase<ELFT> Base;
+ typedef typename Base::uintX_t uintX_t;
+
----------------
Use ELFFile<ELFT>::uintX_t instead of Base.
================
Comment at: ELF/Target.cpp:219-226
@@ -197,2 +218,10 @@
RelativeReloc = R_X86_64_RELATIVE;
+ LazyRelocations = true;
+ if (LazyRelocations) {
+ PltEntrySize = 16;
+ PltZeroEntrySize = 16;
+ } else {
+ PltEntrySize = 8;
+ PltZeroEntrySize = 0;
+ }
}
----------------
Setting true and immediately use that value in `if` looks very odd.
LazyRelocations = true;
PltEntrySize = 16;
PltZeroEntrySize = 16;
================
Comment at: ELF/Target.cpp:230-231
@@ +229,4 @@
+void X86_64TargetInfo::writeGotPltEntry(uint8_t *Buf, uint64_t Plt) const {
+ if (!LazyRelocations)
+ return;
+ // Skip 6 bytes of "jmpq *got(%rip)"
----------------
Always assume it's true.
================
Comment at: ELF/Target.cpp:238-240
@@ +237,5 @@
+ uint64_t PltEntryAddr) const {
+ if (!LazyRelocations)
+ return;
+
+ const uint8_t PltData[] = {
----------------
Same
================
Comment at: ELF/Target.cpp:254
@@ +253,3 @@
+ int32_t Index) const {
+ if (LazyRelocations) {
+ const uint8_t Inst[] = {
----------------
Ditto.
================
Comment at: ELF/Target.cpp:620
@@ -550,2 +619,3 @@
// GotReloc = FIXME
+ // PltReloc = FIXME
}
----------------
Please do not add this kind of TODO. We should rather remove all useless "= FIXME", but that should be done in other patch.
================
Comment at: ELF/Target.cpp:704
@@ -629,2 +703,3 @@
// GotReloc = FIXME
+ // PltReloc = FIXME
PageSize = 65536;
----------------
Ditto.
================
Comment at: ELF/Writer.cpp:105
@@ -104,1 +104,3 @@
+ GotPltSection<ELFT> GotPlt;
+ Out<ELFT>::GotPlt = Target->supportsLazyRelocations() ? &GotPlt : nullptr;
PltSection<ELFT> Plt;
----------------
Global variables are initialized to nullptr, so
if (Target->supportsLazyRelocations())
Out<ELFT>::GotPlt = &GotPlt;
================
Comment at: ELF/Writer.cpp:118
@@ -114,1 +117,3 @@
+ RelocationSection<ELFT> RelaPlt(IsRela ? ".rela.plt" : ".rel.plt", IsRela);
+ Out<ELFT>::RelaPlt = Target->supportsLazyRelocations() ? &RelaPlt : nullptr;
DynamicSection<ELFT> Dynamic(*Symtab);
----------------
Ditto.
================
Comment at: ELF/Writer.cpp:195
@@ -188,3 +194,3 @@
if (Body) {
- if (Target->relocNeedsPlt(Type, *Body)) {
+ if ((NeedsPlt = Target->relocNeedsPlt(Type, *Body))) {
if (Body->isInPlt())
----------------
Move the assignment out of this `if`.
================
Comment at: ELF/Writer.cpp:200
@@ -193,6 +199,3 @@
}
- NeedsGot = Target->relocNeedsGot(Type, *Body);
- if (NeedsGot) {
- if (Body->isInGot())
- continue;
- Out<ELFT>::Got->addEntry(Body);
+ if ((NeedsGot = Target->relocNeedsGot(Type, *Body))) {
+ if (NeedsPlt && Target->supportsLazyRelocations()) {
----------------
Ditto.
http://reviews.llvm.org/D13856
More information about the llvm-commits
mailing list