[PATCH] D26349: [ELF] Convert .got.plt section to input section

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 8 17:29:49 PST 2016


ruiu added a comment.

Overall looking good.



================
Comment at: ELF/InputSection.cpp:86
+    return S->getSize();
+  } else if (auto *D = dyn_cast<InputSection<ELFT>>(this)) {
     if (D->getThunksSize() > 0)
----------------
Remove `else` because the last `if` ends with `return`.


================
Comment at: ELF/InputSection.h:49
+    MipsAbiFlags,
+    Synthetic
+  };
----------------
nit: add ',' at end of the last enum.


================
Comment at: ELF/OutputSections.h:599
     };
-    enum KindT { SecAddr, SecSize, SymAddr, PlainInt } Kind;
+    enum KindT { SecAddr, SecSize, SymAddr, PlainInt, InSecAddr } Kind;
     Entry(int32_t Tag, OutputSectionBase<ELFT> *OutSec, KindT Kind = SecAddr)
----------------
You are not using InSecAddr. What is this?


================
Comment at: ELF/SyntheticSections.h:27
+
+  virtual void writeTo(uint8_t *Buf) {}
+  virtual size_t getSize() const { return this->Data.size(); }
----------------
If all derived classes define this method, use `= 0` instead of providing the default empty function.


================
Comment at: ELF/Writer.cpp:857
+    addInputSec(In<ELFT>::GotPlt);
+    In<ELFT>::GotPlt->OutSec->assignOffsets();
+  }
----------------
I want to avoid calling `assignOffsets` more than once to redo it. It is not only inefficient but also confusing. Could you add this section earlier than this to avoid this function call?


https://reviews.llvm.org/D26349





More information about the llvm-commits mailing list