[PATCH] D63076: [ELF][RISCV] Support PLT, GOT, copy and relative relocations

[Fangrui Song via Phabricator via llvm-commits]

MaskRay marked an inline comment as done.
MaskRay added a comment.

In D63076#1545905 <https://reviews.llvm.org/D63076#1545905>, @jrtc27 wrote:

> In D63076#1545858 <https://reviews.llvm.org/D63076#1545858>, @ruiu wrote:
>
> > Does that make a visible difference?
>
>
> It'll give you `ld.lld: error: relocation R_RISCV_CALL cannot be used against symbol foo; recompile with -fPIC`. Obviously not the correct behaviour, but an error seems better than silently doing the wrong thing in a way that will break glibc and FreeBSD's libc.
>
> At least that's what it does currently without this patch, where it currently returns `R_PC`.


Yes:

- with `case R_RISCV_CALL: return R_PC;`, `ld.lld -shared %t.64.o %t1.64.so` will give you `error: relocation R_RISCV_CALL cannot be used against symbol foo; recompile with -fPIC`.
- with `case R_RISCV_CALL: return R_PLT_PC;`, this is accepted.

But I don't agree with your statement "silently doing the wrong thing in a way that will break glibc and FreeBSD's libc.`

ld.bfd doesn't error on `foo` or `bar`, either. Its warning on `weak` is confusing.

  % ~/projects/binutils-gdb/riscv/ld/ld-new  -m elf64lriscv -shared %t.64.o %t1.64.so -o %t.64
  ...: a.o(.text+0x18): unresolvable R_RISCV_CALL relocation against symbol `weak'

@jrtc27 I still don't understand your definition of `R_RISCV_CALL`, but obviously, with the problem I just described, and binutils PR24683 and PR24685, no matter how we interpret it, the semantics are never properly implemented in binutils.

So I just want to choose a way that make things work, and provide the most sensible semantics (don't create an unnecessary canonical PLT), i.e. making `R_RISCV_CALL` and `R_RISCV_CALL_PLT` the same: `R_PC`.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D63076