[PATCH] D26498: [ELF] Convert .got section to input section

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 10 16:34:08 PST 2016


ruiu accepted this revision.
ruiu added a comment.
This revision is now accepted and ready to land.

LGTM. Thank you for doing this!



================
Comment at: ELF/SyntheticSections.cpp:568
+
+template <class ELFT> void GotSection<ELFT>::writeTo(uint8_t *Buf) {
+  if (Config->EMachine == EM_MIPS) {
----------------
This is not related to this patch, but probably we should make the MIPS GOT a separate class than the GOT for all the other archs, because MIPS seems too different. (I'm just writing what I'm thinking, so no need to take an action.)


================
Comment at: ELF/SyntheticSections.h:74
+  uintX_t getVA() const {
+    return this->OutSec ? this->OutSec->Addr + this->OutSecOff : 0;
+  }
----------------
Isn't it an error to call getVA before setting OutSec? If so, use assert.


================
Comment at: ELF/Writer.cpp:561
                                     uint8_t StOther = STV_HIDDEN) {
-  SymbolBody *S = Symtab<ELFT>::X->find(Name);
-  if (!S)
-    return nullptr;
-  if (!S->isUndefined() && !S->isShared())
-    return S->symbol();
-  return Symtab<ELFT>::X->addSynthetic(Name, Sec, Val, StOther);
+  Symbol *Ret;
+  if (shouldAddOptional<ELFT>(Name, Ret))
----------------
This function seems to be doing the same thing as before (am I right?). The new code seems a bit hard to understand to me. Can you keep this function as is?


================
Comment at: ELF/Writer.cpp:582
+  Symbol *Ret;
+  if (shouldAddOptional<ELFT>(Name, Ret))
+    Ret = addRegular(Name, S, Value);
----------------
And inline this function here.


Repository:
  rL LLVM

https://reviews.llvm.org/D26498





More information about the llvm-commits mailing list