[PATCH] D46204: [PPC64] V2 abi: Emit plt call stubs to the text section rather then the plt section.

[Sean Fertile via Phabricator via llvm-commits]

sfertile added inline comments.


================
Comment at: lld/trunk/ELF/InputSection.cpp:746
+      if (Rel.Sym->NeedsTocRestore) {
+        if (BufLoc + 8 > BufEnd || read32(BufLoc + 4) != 0x60000000) {
+          error(getErrorLocation(BufLoc) + "call lacks nop, can't restore toc");
----------------
sfertile wrote:
> MaskRay wrote:
> > sfertile wrote:
> > > MaskRay wrote:
> > > > If the file was compiled with `gcc -Bsymbolic -fPIC`, there may not be a `nop` after `bl`.
> > > > If the symbol also requires a PLT (its `NeedsTocRestore` will be true), this check will fail.
> > > Hi @MaskRay are you using `-Bsymbolic` in the linker options as well?
> > I get a better example.
> > 
> > 1. Download https://packages.debian.org/jessie/ppc64el/libstdc++-4.9-dev/download
> > 2. Unpack with 7z then `tar xf data.tar`
> > 3. ar x ./usr/lib/gcc/powerpc64le-linux-gnu/4.9/libstdc++.a locale.o
> > 
> > llvm-objdump -d -r locale.o
> > 
> > ```
> > _ZNKSt6locale2id5_M_idEv:
> > ......
> >       b8:       01 00 00 48     bl .+0
> >                 00000000000000b8:  R_PPC64_REL24        _ZNKSt6locale2id5_M_idEv
> >       bc:       01 00 23 39     addi 9, 3, 1     //////////// there is no nop here
> > ```
> > 
> > If the symbol `_ZNKSt6locale2id5_M_idEv` has `NeedsTocRestore` set, this will fail to link, right?
> > 
> > I have checked newer versions, e.g. 5.5 6.4 6.8 they don't have such weird `bl` instruction. I don't know if this was a bug when producing locale.o
> Awesome, I'll pull this down and have a closer look.
> 
> > If the symbol _ZNKSt6locale2id5_M_idEv has NeedsTocRestore set, this will fail to link, right?
> 
> Yes it will, but if `_ZNKSt6locale2id5_M_idEv` is interposable then the missing nop is indeed a bug.  For the error to be wrong I //think// we would need to see a situation where the target of the calll isn't in the plt but NeedsTocRestore was set.
> 
> What I observed in testing out various scenarios yesterday was that gold will emit a fatal error when there is no nop after a call to a function which doesn't have a definition in the shared object we are linking. If there is a definition there is no error even if the target of the call is  interposable. In the situation where there is no nop but the target of the call is interposable the  resulting shared-object **is** subtly broken. It will work fine as long as the dynamic loader resolves the target of the call to the definition local to the shared-object, but if it picks a definition from another shared-object or the executable then we can end up returning to this module with the wrong address in r2.
> 
> 
{F7089908}

good_main.c:
```
int test(int);

int main(void) {
  return test(44);
}
```

bad_main.c:
```
int test(int);

int printf(const char*, ...);

void print(void) {
  printf("%s %d\n", __FILE__, __LINE__);
}

int main(void) {
  return test(44);
}
```

Here is a small test where we can assemble a.s and link it into a shared-object with gold  without error, and it runs fine when used with good_main.c, but if used with bad_main.c we segfault on the lwa instruction following the call to `print`.


Repository:
  rL LLVM

https://reviews.llvm.org/D46204