[PATCH] D14090: [ELF2] R_X86_64_COPY relocation implemented

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/Writer.cpp:417
@@ +416,3 @@
+    Off = RoundUpToAlignment(Off, Sym.st_size);
+    C->OffsetInBSS = Off;
+    Off += Sym.st_size;
----------------
ruiu wrote:
> 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.
Yes, "min" certainly.
Ok, I`ll align to 16 then.


http://reviews.llvm.org/D14090