[PATCH] D71639: [ELF][PPC64] Improve "call lacks nop" diagnostic and make it compatible with GCC<5.5 and GCC<6.4

Thu Jan 2 17:48:13 PST 2020

MaskRay added a comment.

In D71639#1802237 <https://reviews.llvm.org/D71639#1802237>, @sfertile wrote:

> I can definitely see it from the opposite side: We want LLD to be usable on these systems that shipped with this bug in libstdc++ and gcc/gfortran. And adding a link option can be hard to do in some build setups so its not as simple as I make it sound. What worries me though is that one of the compilers could subtly break and with the linker[s] being sloppy with the error then we don't find out until long after the toolchain has shipped.
>
> I  think you are right that this will only be triggered on handwritten assembly right now. That is still useful even though not many people write assembly now days. Even experts can mess up, for example taking code written for an executable, then linking it into a shared object since it //looks// like its already position independent could trigger this error because a tail-call that was valid in an exec isn't in the shared object. These kinds of details aren't really documented anywhere. They are difficult to figure out with the error, but much much worse when you are diagnosing runtime corruptions.

I am hesitant to call it a gcc/gfortran bug (that's why I don't use the term "bug" to describe the issue). The GCC report was originally motivated by a glibc use case that does dlopen RTLD_LOCAL. RTLD_LOCAL does not work in various situations and it just appears to work in some circumstances. In a C++ context it will likely be an one-definition-rule violation.

For the hand-written assembly case, to trigger an uncaught error after this patch, the code needs to have the following setting:

  // a.so
  bar:
    bl foo
    # no nop
    ...

  .globl foo   # foo defined by a.so is preempted by the executable or another shared object at runtime
  foo:
    ...

The symbol preemption here makes it not a likely scenario. If you still feel strong about this, I can add an option to warn such cases, and make it off by default (if it is on by default, Linux distributions will have to turn off this option. And in the future, when we remove the code, the distributions will have to delete it)

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71639/new/

https://reviews.llvm.org/D71639