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

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 14 22:06:54 PDT 2019


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


================
Comment at: test/ELF/riscv-plt.s:72
+  call foo
+  call bar
+  call bar at plt
----------------
jrtc27 wrote:
> jrtc27 wrote:
> > MaskRay wrote:
> > > jrtc27 wrote:
> > > > MaskRay wrote:
> > > > > jrtc27 wrote:
> > > > > > jrtc27 wrote:
> > > > > > > MaskRay wrote:
> > > > > > > > jrtc27 wrote:
> > > > > > > > > This should never use a PLT stub.
> > > > > > > > > This should never use a PLT stub.
> > > > > > > > 
> > > > > > > > Which function should not use a PLT stub? I think the behavior is consistent with ld.bfd, foo doesn't get a PLT but weak and bar get PLT stubs.
> > > > > > > Yes, and I'm 99% sure that's an unintended bug. If you delete the `call foo at plt`, then the `call bar` *doesn't* use a PLT stub.
> > > > > > I mean delete `call bar at plt`, of course...
> > > > > > Yes, and I'm 99% sure that's an unintended bug. If you delete the call bar at plt, then the call bar *doesn't* use a PLT stub.
> > > > > 
> > > > > I think you misread it..
> > > > > 
> > > > > bar at plt will be created, even if I delete `call bar at plt`. The behavior is consistent with ld.bfd.
> > > > > 
> > > > > Because bar is not local, a PLT has to be created.
> > > > No, the whole point is that by *not* adding `@plt` you forcefully bypass the PLT and don't allow bar to be preempted in this instance:
> > > > 
> > > > ```
> > > > burn:snippets jrtc4% cat call-plt.s
> > > > .global test, foo, bar, baz
> > > > 
> > > > test:
> > > >     call foo
> > > >     call bar
> > > >     call bar at plt
> > > >     call baz at plt
> > > > burn:snippets jrtc4% riscv64-linux-gnu-gcc -o call-plt-pic.so -shared -fPIC call-plt.s -nostdlib
> > > > burn:snippets jrtc4% riscv64-linux-gnu-objdump -d call-plt-pic.so                               
> > > > 
> > > > call-plt-pic.so:     file format elf64-littleriscv
> > > > 
> > > > 
> > > > Disassembly of section .plt:
> > > > 
> > > > 00000000000002f0 <.plt>:
> > > >  2f0:   00002397            auipc   t2,0x2
> > > >  2f4:   41c30333            sub t1,t1,t3
> > > >  2f8:   d103be03            ld  t3,-752(t2) # 2000 <.got>
> > > >  2fc:   fd430313            addi    t1,t1,-44
> > > >  300:   d1038293            addi    t0,t2,-752
> > > >  304:   00135313            srli    t1,t1,0x1
> > > >  308:   0082b283            ld  t0,8(t0)
> > > >  30c:   000e0067            jr  t3
> > > > 
> > > > 0000000000000310 <baz at plt>:
> > > >  310:   00002e17            auipc   t3,0x2
> > > >  314:   d00e3e03            ld  t3,-768(t3) # 2010 <baz>
> > > >  318:   000e0367            jalr    t1,t3
> > > >  31c:   00000013            nop
> > > > 
> > > > 0000000000000320 <bar at plt>:
> > > >  320:   00002e17            auipc   t3,0x2
> > > >  324:   cf8e3e03            ld  t3,-776(t3) # 2018 <bar>
> > > >  328:   000e0367            jalr    t1,t3
> > > >  32c:   00000013            nop
> > > > 
> > > > Disassembly of section .text:
> > > > 
> > > > 0000000000000330 <test>:
> > > >  330:   00000097            auipc   ra,0x0
> > > >  334:   cd0080e7            jalr    -816(ra) # 0 <_PROCEDURE_LINKAGE_TABLE_-0x2f0>
> > > >  338:   fe9ff0ef            jal ra,320 <bar at plt>
> > > >  33c:   fe5ff0ef            jal ra,320 <bar at plt>
> > > >  340:   fd1ff0ef            jal ra,310 <baz at plt>
> > > > ```
> > > > 
> > > > `foo` and `baz` behave as expected, with the former not using a PLT stub but the latter using a PLT stub (based on whether or not they were suffixed with `@plt`). However, *both* calls to `bar` end up using a PLT stub, despite only one of them having a `@plt` suffix. This is the BFD bug I was talking about.
> > > `call bar` (`R_RISCV_CALL`) and `call bar at plt` (`R_RISCV_CALL_PLT`) can both appear in an object file as a result of ld -r.
> > > 
> > > Making `call bar` and `call bar at plt` resolve to different targets (`R_RISCV_CALL`->`__ehdr_start`; `R_RISCV_CALL_PLT`->plt stub) is nonsensical. The preemptability is the property of the symbol, not the property of the relocation type. The preemptability decides whether a PLT stub should be the target. This is both simple and consistent.
> > > 
> > > Preemptability, as a property of the symbol, is mergeable. If you have an undefined reference to foo in a.o and a defined foo in b.o, after ld -r, you get a defined foo, which is non-preemptable if you eventually use it in a -no-pie/-pie link. In contrast, preemptability on relocation types is not mergeable. If you place the preemptability information on the relocation types, you can get things that aren't consistent after ld -r.
> > > 
> > > I don't think `R_RISCV_CALL` is meaningful so I filed https://github.com/riscv/riscv-elf-psabi-doc/issues/98 The examples I raised (`R_X86_64_PLT32` `R_PPC_LOCAL24PC` `R_PPC_PLTREL24`) were to demonstrate how the PLT relocation types evolved on other architectures.
> > > 
> > > @jrtc27 Have said all that I've no objections to make them distinct in the future if RISC-V folks can convince me the distinction between `R_RISCV_CALL` and `R_RISCV_CALL_PLT` is useful :) Does this patch look good from your perspective minus this difference from ld.bfd?
> > > 
> > Software relies on the difference in behaviour between `CALL` and `CALL_PLT` as implemented by BFD. You are free to campaign for the ABI to change and to have them be the same, but that is not the case currently, and so LLD should implement the current behaviour if it wants to support linking things like glibc and FreeBSD's libc.
> Looking more closely, `CALL` is returning `R_PC` and `CALL_PLT` is returning `R_PLT_PC`, so does this not in fact already match BFD's behaviour?
> Software relies on the difference in behaviour between CALL and CALL_PLT as implemented by BFD. You are free to campaign for the ABI to change and to have them be the same, but that is not the case currently, and so LLD should implement the current behaviour if it wants to support linking things like glibc and FreeBSD's libc.

I don't have a FreeBSD box to test, but my patches make programs linked against musl/glibc work.

> Software relies on the difference in behaviour between CALL and CALL_PLT as implemented by BFD

An example will be welcomed. Just as a program working under glibc doesn't guarantee it will work under musl, a program linked by ld.bfd may have issues when linked by gold or lld. In few cases the issues are caused by improper support in gold or lld, but more commonly, they are because programs are making use of unsupported features. Working under one specific version of binutils ld.bfd doesn't guarantee it will continue working in a future version.

For CALL vs CALL_PLT, I'm confident to say the distinction is totally artificially and I expect CALL will be deprecated. I don't want to implement its specific weird behavior in lld while its own fortune is unclear.

> Looking more closely, CALL is returning R_PC and CALL_PLT is returning R_PLT_PC, so does this not in fact already match BFD's behaviour?

It already doesn't match BFD:) lld makes use of these generic relocation expressions: R_PC, R_PLT_PC, etc, so the behavior is general. If R_PLT_PC works some way on target A, you can almost reliably expect it to work the same way on target B. I find it super helpful. The several issues I recently raised are all due to binutils elf*-*.c behaviors are duplicated on every targt I think :) So implementers made mistakes while copying code from mips/arm/...

Give you some examples I find lld has different behaviors:

* PC relative relocations to SHN_ABS symbols are errors on lld but not on ld.bfd (at least on x86/x86-64/aarch64. haven't checked others)
* The rule that absolute relocation types are resolved to R_*_RELATIVE is different. I fixed one issue yesterday (matching GNU ld is nice but not my main motivation to do something).
* Weak undefined symbols are different.
* Whether weak undefined symbols show up in .dynsym is different. D59549
* ld.bfd has more TLS relaxation checks but we don't.

I think for all the above, BFD isn't consistent across its elf*-*.c ...



Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D63076





More information about the llvm-commits mailing list