[PATCH] D14090: [ELF2] R_X86_64_COPY relocation implemented

[Rui Ueyama via llvm-commits]

ruiu 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:
> 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.
Then how about this?

  if (auto *E = dyn_cast<SharedSymbol<ELFT>>(Body)) {
    if (E->NeedsCopy)
      continue;
    E->NeedsCopy = Target->relocNeedsCopy(Type, *Body);
  }



http://reviews.llvm.org/D14090