[PATCH] D14090: [ELF2] R_X86_64_COPY relocation implemented

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/OutputSections.cpp:203-206
@@ -197,4 +202,6 @@
     uintX_t Addend;
-    if (CanBePreempted)
+    if (NeedsCopy)
+      Addend = 0;
+    else if (CanBePreempted)
       Addend = OrigAddend;
     else if (Body)
----------------
grimar wrote:
> ruiu wrote:
> > This makes me think whether this function as a whole is correct or not. If IsRela is false, we compute an addend and then discard that value? If that information is discardable, what's the point of computing that value here?
> Probably it needs early return after 
> 
> uintX_t OrigAddend = 0; 
> 
> if IsRela false. But that is not for this patch I guess ?
Right, this is not for this patch. It's just that this is something we want to take a look (after this patch).

================
Comment at: ELF/Writer.cpp:417
@@ +416,3 @@
+    Off = RoundUpToAlignment(Off, Sym.st_size);
+    C->OffsetInBSS = Off;
+    Off += Sym.st_size;
----------------
grimar wrote:
> ruiu wrote:
> > st_size is too large to align. (If a copy relocation points to an array, st_size can be very large.) Maybe we should align to some reasonable boundary, say, 32-byte boundaries for now. I don't think we are going to have tons of copy relocations, so doing something simple should suffice.
> I found the next comment in gnu code (http://www.opensource.apple.com/source/gdb/gdb-213/src/bfd/elf64-x86-64.c):
> 
>   /* We need to figure out the alignment required for this symbol.  I
>      have no idea how ELF linkers handle this.	16-bytes is the size
>      of the largest type that requires hard alignment -- long double.  */
>   /* FIXME: This is VERY ugly. Should be fixed for all architectures using
>      this construct.  */
> 
> So it looks we can use 16.
> Also I guess even if we take 32 bytes then alignment should be calculated as max(Sym.st_size, 32) ?
Do you mean min(Sym.st_size, 32)?

There are actually some instructions that require 32 -byte alignment such as some AVX instructions, but we probably don't have to take care of them for the copy relocation.

There seems to be no "right thing to do" here. Saving a few bytes per one copy relocation using a clever technique is very unlikely to worth the effort. How about this: Always align to 16-byte boundaries. This is very easy to understand.


http://reviews.llvm.org/D14090