[PATCH] D14090: [ELF2] R_X86_64_COPY relocation implemented

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/Writer.cpp:203-209
@@ -202,2 +202,9 @@
     if (Body) {
+      bool NeedsCopy = false;
+      if (auto *E = dyn_cast<SharedSymbol<ELFT>>(Body)) {
+        NeedsCopy = Target->relocNeedsCopy(Type, *Body);
+        if (NeedsCopy && E->NeedsCopy)
+          continue;
+        E->NeedsCopy = NeedsCopy;
+      }
       NeedsPlt = Target->relocNeedsPlt(Type, *Body);
----------------
grimar wrote:
> ruiu wrote:
> > grimar wrote:
> > > grimar wrote:
> > > > ruiu wrote:
> > > > > I'm trying to understand this. So, if two or more relocations exist for a shared symbol, the last relocation's NeedsCopy may override the previous results? Is this correct?
> > > > What kind of relocations ? According to current logic plt relocations will never happen because it is not function and so the only possible case is relocNeedsGot() which implemented as
> > > > 
> > > > 
> > > > bool X86_64TargetInfo::relocNeedsGot(uint32_t Type, const SymbolBody &S) const {
> > > >   return Type == R_X86_64_GOTPCREL || relocNeedsPlt(Type, S);
> > > > }
> > > > 
> > > > 
> > > > that gives us the only case is R_X86_64_GOTPCREL.
> > > > Is it possible that symbol will require both R_X86_64_GOTPCREL and copy relocation ? If I understand correctly that case - we can use copy relocation here. So I see nothing wrong, please correct me if I am mistaken.
> > > I am wrong here I think, I need to investigate that case, pelase ignore my comment.
> > Is that the same as this?
> > 
> >   if (auto *E = dyn_cast<SharedSymbol<ELFT>>(Body)) {
> >     E->NeedsCopy = E->NeedsCopy || Target->relocNeedsCopy(Type, *Body);
> >     if (E->NeedsCopy)
> >       continue;
> >   }
> No, its not. Because in your code any relocation that needs copy will lead to continue and that will not add the relocation to reladyn at the bottom of method.
> I am trying to add the copy relocation reladyn entry but only once for symbol.
So I think my code is correct:
if this is copy relocation that if it is the first one, it will pass through and create the reladyn entry.
If this is copy relocation and it is not the first one - nothing shall be done here, it continues;
If this is not copy relocation it will change nothing and continue checking other relocation types.


http://reviews.llvm.org/D14090