[PATCH] D72046: [ELF][RISCV] Resolve undefined weak R_PC relocations to p+a

Tue Dec 31 18:47:22 PST 2019

jrtc27 added a comment.

In D72046#1800268 <https://reviews.llvm.org/D72046#1800268>, @MaskRay wrote:

> In D72046#1800264 <https://reviews.llvm.org/D72046#1800264>, @jrtc27 wrote:
>
> > For calls, whilst it differs from BFD, it can work just fine as it’s what AArch64 does.
>
>
>
>
> > For data references, however, this is highly inappropriate. They *need* to resolve to 0 as *real world code* relies on that, and it’s what happens on every architecture out there. Code generated by both GCC and LLVM uses PC-relative relocations to refer to external weak symbols. We can argue all we like about whether that’s good or not, but at the end of the day *that is what happens* and we need to not break that.
>
> No. GCC/clang emit either absolute or GOT-generating relocations (R_RISCV_HI20+R_RISCV_LO12_I; R_RISCV_GOT_HI20+R_RISCV_PCREL_LO12_I). Such relocations to weak undefined symbols can be relied on by real world code. The linker only has to resolve absolute relocations to 0 and fill GOT entries with 0.
>
> PC-relative relocations to undefined weak symbols should not be emitted by compilers. Such hand-written assembly is just broken.

No, you’re missing the whole point behind John’s patch, which is to fix LLD so it can link code compiled by either GCC or LLVM. A copy of my comment on his patch so it’s in this discussion chain:

The problem is and always has been `-mcmodel=medany`. Neither GCC nor LLVM use the GOT in this case, as it’s not `-fpic`/`-fpie`. This is the case we care about for the kernel. Would I have made GCC do this? No. But it does, and we have to deal with that in a sensible way. Making it an error and changing GCC and LLVM is one such way. But that hasn’t happened.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72046