[PATCH] D14090: [ELF2] R_X86_64_COPY relocation implemented

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 28 03:09:33 PDT 2015


grimar added inline comments.

================
Comment at: ELF/Symbols.h:96
@@ -95,2 +95,3 @@
   uint32_t PltIndex = -1;
+  bool InRelaBss = false;
   bool isInGot() const { return GotIndex != -1U; }
----------------
ruiu wrote:
> Can you move this to SharedSymbol? This increases the size of SymbolBody at least one byte, but this field is not going to be used except for SharedSymbol.
I just removed it at all. SharedSymbol has NeedsCopy flag which is sufficient to replace the single use of InRelaBss.

================
Comment at: ELF/Writer.cpp:209
@@ +208,3 @@
+          continue;
+        else
+          Body->InRelaBss = true;
----------------
ruiu wrote:
> No `else` after `continue`.
Done.

================
Comment at: ELF/Writer.cpp:415
@@ +414,3 @@
+  for (SharedSymbol<ELFT> *C : Syms) {
+    const Elf_Sym &Sym = C->Sym;
+    Off = RoundUpToAlignment(Off, 16);
----------------
ruiu wrote:
> Add a brief comment here to say that we don't know the exact alignment requirement for the data copied by a copy relocation, so align that to 16 byte boundaries (which we think is large enough) unconditionally.
Done.


http://reviews.llvm.org/D14090





More information about the llvm-commits mailing list