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

[James Clarke via Phabricator via llvm-commits]

jrtc27 added a comment.

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. Non-CALL(_PLT) PC-relative relocations *must not* be mislinked like this. Either do the same rewriting as in John’s patch that BFD does, or produce a suitable error, but it is not acceptable to silently mis-link this code. And I would really like LLD to support linking GCC-produced code; we should not be going off and doing our own incompatible thing. Maybe you don’t want to support `gcc -fuse-ld=lld`, but what about when using Clang and LLD and you’re statically-linking against a library compiled by GCC? That means coordination; either support what GCC produces (which a patch already exists for), or convince GCC to change.

@ruiu do you agree with this assessment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72046