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

[MaskRay]

Summary

• Handle initial relocation types R_RISCV_32, R_RISCV_64, R_RISCV_CALL_PLT and R_RISCV_GOT_HI20.
• Produce dynamic relocation types: R_RISCV_COPY, R_RISCV_RELATIVE, R_RISCV_JUMP_SLOT, and R_RISCV_{32,64}.
• Generate PLT header: it is used by glibc lazy binding PLT.

Simple -no-pie/-pie/-shared programs linked against musl or glibc should
work with this patch.

[ruiu]

How did you verify the correctness of this patch? Did you set up a virtual machine? If that's easy to do, I'd like to set it up too.

[MaskRay]

musl is about to add support for riscv64 (there is a draft at https://github.com/riscv/riscv-musl).

Get a riscv cross gcc (clang is still lacking many stuff, and you'll need libgcc anyway even if use clang), say, ~/.local/opt/riscv-none-gcc

qemu-riscv64-static (in the package "qemu-user-static" in several distros)

# clone musl
mkdir riscv64; cd riscv64
../configure --target riscv64 CROSS_COMPILE=~/.local/opt/riscv-none-gcc/bin/riscv-none-embed- CFLAGS='-march=rv64g -mabi=lp64d'
make

# hack obj/musl-gcc
exec ~/.local/opt/riscv-none/gcc/bin/riscv-none-embed-gcc "$@" -specs $HOME/musl/riscv64/lib/musl-gcc.specs

# hack lib/musl-gcc.specs
*cpp_options:
-nostdinc -isystem /path/to/musl/arch/riscv64 -isystem /path/to/musl/arch/generic -isystem /path/to/musl/include -isystem /path/to/musl/riscv64/obj/include -isystem include%s %(old_cpp_options)

*cc1:
%(cc1_cpu) -nostdinc -isystem /path/to/musl/arch/riscv64 -isystem /path/to/musl/arch/generic -isystem /path/to/musl/include -isystem /path/to/musl/riscv64/obj/include -isystem include%s

*link:
 -nostdlib %{shared:-shared} %{static:-static ;:-dynamic-linker /path/to/musl/riscv64/lib/libc.so} %{rdynamic:-export-dynamic}

gcc<9 doesn't support -fuse-ld=lld. So I did another horrible hack... I placed a script ld.gold in ~/.local/opt/riscv-none-gcc/bin and made it call /path/to/lld -flavor gnu "$@". Then:

/path/to/musl/riscv64/obj/musl-gcc -fuse-ld=gold -march=rv64g -mabi=lp64d a.c -o a
./a  # /usr/bin/qemu-riscv64-static has been registered to binfmt. this can be directly executed

glibc is a pain... https://wiki.qemu.org/Documentation/Platforms/RISCV Fedora has a prebuilt riscv64 image, however, gdb is not available. So it is not better than qemu-riscv64-static.

I manually downloaded https://packages.debian.org/sid/gcc-8-riscv64-linux-gnu and its dependencies (libgcc1-riscv64-cross libgcc-8-dev-riscv64-cross libc6_2.28-10_riscv64 cpp-8-riscv64-linux-gnu etc), extracted these .deb files under ~/.local/opt/riscv64-linux-gnu-gcc, then I run:

LD_LIBRARY_PATH=~/.local/opt/riscv64-linux-gnu-gcc/usr/lib/x86_64-linux-gnu ~/.local/opt/riscv64-linux-gnu-gcc/usr/bin/riscv64-linux-gnu-gcc -fuse-ld=gold a.c -o a

Unpack a riscv64 libc6 somewhere, say, /tmp/debian-riscv64, then run qemu-riscv64-static -L /tmp/debian-riscv64 ./a

[ruiu]

ELF/Arch/RISCV.cpp

48

nit: add a blank line

59–67

Nice little assembly routines.

71

Does this really support R_RISCV_32?

118

For example, it looks like this code assumes RISCV64.

126–127

What is RVE?

142

nit: -Target->PltHeaderSize - 12

[jrtc27]

ELF/Arch/RISCV.cpp

126–127

RV32E, as opposed to RV32I; it only has 16 GPRs available.

test/ELF/riscv-plt.s

3

Can we put this in a file in Inputs rather than creating it dynamically?

73

This should never use a PLT stub.

[MaskRay]

ELF/Arch/RISCV.cpp

71

This should be correct. glibc/sysdeps/riscv/dl-machine.h says:

case R_RISCV_JUMP_SLOT:
case __WORDSIZE == 64 ? R_RISCV_64 : R_RISCV_32:
  *addr_field = value;
  break;

On RISC-V, there is no SymbolRel GotRel difference as there is on other targets. (R_X86_64_GLOB_DAT is an unnecessary dynamic relocation; R_X86_64_64 can be used instead)

126–127

The issue is that RV32E doesn't have the register t3 so this PLT stub doesn't work.

test/ELF/riscv-plt.s

3

I prefer inlined assembly if it is simple. This one-liner makes it easy for a reader.

I wanted to delete .type bar,@function; .type weak,@function but unfortunately neither binutils as nor llvm-mc marks a symbol STT_FUNC if there is a call instruction.

73

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.

[jrtc27]

test/ELF/riscv-plt.s

73

Yes, and I'm 99% sure that's an unintended bug. If you delete the call foo@plt, then the call bar *doesn't* use a PLT stub.

73

I mean delete call bar@plt, of course...

[MaskRay]

test/ELF/riscv-plt.s

73

Yes, and I'm 99% sure that's an unintended bug. If you delete the call bar@plt, then the call bar *doesn't* use a PLT stub.

I think you misread it..

bar@plt will be created, even if I delete call bar@plt. The behavior is consistent with ld.bfd.

Because bar is not local, a PLT has to be created.

[MaskRay]

Address review comments.

Dispatch write32le write64le on Config->Is64
Rebase after the SymbolRel change.
Check .got.plt and .rela.plt in the test.

[ruiu]

ELF/Arch/RISCV.cpp

118

We have writeInt and writeUint to write word-size integers. Maybe you should use that?

[MaskRay]

ELF/Arch/RISCV.cpp

118

The functions are in SyntheticSections.cpp and are static:

static uint64_t readUint(uint8_t *Buf) {
  return Config->Is64 ? read64(Buf) : read32(Buf);
}

static void writeUint(uint8_t *Buf, uint64_t Val) {
  if (Config->Is64)
    write64(Buf, Val);
  else
    write32(Buf, Val);
}

I don't expect there will be many other dispatches like these. So the duplication is probably ok? (If there are going to have more, we can create such helper functions in this file)

[MaskRay]

• [WIP][ELF][RISCV] Support GD/LD TLS models

[MaskRay]

Restore the original diff. Made an unintended change while creating another revision

[ruiu]

ELF/Arch/RISCV.cpp

132

Can you add a comment saying that RV32E/RV64E is fewer number of registers and PLTs are not supported on them because some registers that the PLT stub uses don't exist?

133

Is RVE something that average RISC-V users can understand? That acronym is too short that it is not easy to google. Maybe we can refer them as RV32E/RV64E?

[MaskRay]

Delete RVE related code and tests

[MaskRay]

ELF/Arch/RISCV.cpp

132

I just deleted this code.

I read from somewhere that RV32E may not support dynamic linking. If that is true, we can move the error checking to an appropriate place to check !SharedFiles.empty() || Config->Shared instead. rv32 hasn't been upstreamed to glibc yet. I think rv32e is not stable now.

[jrtc27]

test/ELF/riscv-plt.s

73

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@plt
    call baz@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@plt>:
 310:   00002e17            auipc   t3,0x2
 314:   d00e3e03            ld  t3,-768(t3) # 2010 <baz>
 318:   000e0367            jalr    t1,t3
 31c:   00000013            nop

0000000000000320 <bar@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@plt>
 33c:   fe5ff0ef            jal ra,320 <bar@plt>
 340:   fd1ff0ef            jal ra,310 <baz@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.

[MaskRay]

test/ELF/riscv-plt.s

73

call bar (R_RISCV_CALL) and call bar@plt (R_RISCV_CALL_PLT) can both appear in an object file as a result of ld -r.

Making call bar and call bar@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?

[jrtc27]

test/ELF/riscv-plt.s

73

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.

[jrtc27]

test/ELF/riscv-plt.s

73

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?

[MaskRay]

test/ELF/riscv-plt.s

73

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 ...

[MaskRay]

Handle R_RISCV_CALL the same way as R_RISCV_CALL_PLT

The binutils issue was reported at https://sourceware.org/bugzilla/show_bug.cgi?id=24685

[jrtc27]

Still disagree about the behaviour of R_RISCV_CALL. You can't just choose the ABI that you want. Please at least put it back to R_PC. @ruiu do you have an opinion on this?

test/ELF/riscv-plt.s

51

You mean ## foo - . = 0x11020-0x11000 = 32.

54

You mean ## bar@plt - . = 0x11050-0x11008 = 72.

57

You mean ## bar@plt - . = 0x11050-0x11010 = 64.

59

No comment for ## weak@plt - . = 0x11060-0x11018 = 72?

[ruiu]

Does that make a visible difference?

[jrtc27]

In 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.

[MaskRay]

Still disagree about the behaviour of R_RISCV_CALL. You can't just choose the ABI that you want. Please at least put it back to R_PC. @ruiu do you have an opinion on this?

@jrtc27 R_PC and R_PLT_PC do not behave in the way you thought. In fact, when used as a PLT relocation, R_PC and R_PLT_PC work almost the same way. The difference is: R_PLT_PC is considered a PLT-generating relocation but R_PC isn't.

• If the target symbol is non-preemptable, an R_PLT_PC gets optimized to an R_PC, and the R_PC is considered a link-time constant.
• Otherwise, R_PLT_PC is still a link-time constant. However, R_PC will trigger the creation of a canonical PLT and behave as if an R_PLT_PC was used:

// Relocations.cpp:1026
  if (Sym.isFunc()) {
    if (Config->Pie && Config->EMachine == EM_386)
      errorOrWarn("symbol '" + toString(Sym) +
                  "' cannot be preempted; recompile with -fPIE" +
                  getLocation(Sec, Sym, Offset));
    if (!Sym.isInPlt())
      addPltEntry<ELFT>(In.Plt, In.GotPlt, In.RelaPlt, Target->PltRel, Sym);
    if (!Sym.isDefined())
      replaceWithDefined(
          Sym, In.Plt,
          Target->PltHeaderSize + Target->PltEntrySize * Sym.PltIndex, 0);
    Sym.NeedsPltAddr = true;  /// marks the creation of a canonical PLT
    Sec.Relocations.push_back({Expr, Type, Offset, Addend, &Sym});
    return;
  }

@ruiu There is visible difference, but the difference is related to whether a canonical PLT is created, not as @jrtc27 thought that it changed the bindings. I reported the ld.bfd bug at https://sourceware.org/bugzilla/show_bug.cgi?id=24685

% riscv-ld -m elf64lriscv a.o b.so -o bfd; riscv-objdump -d bfd

0000000000010250 <_start>:
   10250:       00000097                auipc   ra,0x0
   10254:       020080e7                jalr    32(ra) # 10270 <foo>
   10258:       00000097                auipc   ra,0x0
   1025c:       fe8080e7                jalr    -24(ra) # 10240 <bar@plt>
   10260:       00000097                auipc   ra,0x0
   10264:       fe0080e7                jalr    -32(ra) # 10240 <bar@plt>
   10268:       00000097                auipc   ra,0x0
   1026c:       fc8080e7                jalr    -56(ra) # 10230 <weak@plt>
% readelf --dyn-s bfd
...
     1: 0000000000000000     0 FUNC    WEAK   DEFAULT  UND weak
     2: 0000000000010240     0 FUNC    GLOBAL DEFAULT  UND bar  # this is a canonical PLT, it is however not needed. See 
...

With this patch, the 4 calls bind the symbols the same as ld.bfd:

% myld.lld a.o b.so -o a; riscv-objdump -d a
0000000000011000 <_start>:                                                                                     
   11000:       00000097                auipc   ra,0x0                  
   11004:       020080e7                jalr    32(ra) # 11020 <foo>                
   11008:       00000097                auipc   ra,0x0                   
   1100c:       048080e7                jalr    72(ra) # 11050 <bar@plt>
   11010:       00000097                auipc   ra,0x0   
   11014:       040080e7                jalr    64(ra) # 11050 <bar@plt>                                       
   11018:       00000097                auipc   ra,0x0
   1101c:       048080e7                jalr    72(ra) # 11060 <weak@plt>

The call sites are: <foo>, <bar@plt>, <bar@plt>, <weak@plt>, the same as ld.bfd. This is the behavior that matters..

The difference is that we don't create the unnecessary canonical PLT:

% readelf --dyn-s a
     1: 0000000000000000     0 FUNC    GLOBAL DEFAULT  UND bar
     2: 0000000000000000     0 FUNC    WEAK   DEFAULT  UND weak

@jrtc27 I agree I am not following the ABI wrt R_RISCV_CALL, but that case is not checked in my added tests. Given that ld.bfd doesn't follow the ABI either https://sourceware.org/bugzilla/show_bug.cgi?id=24683, and the semantics of R_RISCV_CALL may be broken, I'll not implement the behavior in this patch.

Actually, whether I use R_PC or R_PLT_PC for R_RISCV_CALL does not change the function binding: <foo>, <bar@plt>, <bar@plt>, <weak@plt>. If I use R_PC, lld will also create the unnecessary canonical PLT. That is why in the updated diff I added these lines to the test:

## A canonical PLT has a non-zero st_value. bar and weak are called but their
## addresses are not taken, so a canonical PLT is not necessary.
# NM: {{0*}}00000000 0 FUNC GLOBAL DEFAULT UND bar
# NM: {{0*}}00000000 0 FUNC WEAK   DEFAULT UND weak

[MaskRay]

Fix some ## comments as reported by jrtc27

[MaskRay]

test/ELF/riscv-plt.s

59

Thanks! I didn't update the comments while adding more instructions..

[MaskRay]

In D63076#1545905, @jrtc27 wrote:

In 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_PLT_PC.

[jimw]

The stage4 image that the qemu wiki points at is old and obsolete. gdb has been available since late last year. I did the work for that last summer/fall. Use the info here instead

https://fedoraproject.org/wiki/Architectures/RISC-V/Installing

The new name for the koji build server if you want an up-to-date image is

http://fedora.riscv.rocks/koji/

They are in the process of moving from the old server to the new server, so there may be some temporary instability.

There is no expectation that dynamic linking works with the current RV32E abi. The GNU toolchain doesn't support this. There may be a new embedded ABI defined at some point, but since that ABI is only intended for embedded systems (not embedded linux), this may not support dynamic linking with RV32E either.

I'm taking a look at the CALL versus CALL_PLT stuff and trying to make sense of it in the context of binutils and bfd.

[MaskRay]

In D63076#1556616, @jimw wrote:

The stage4 image that the qemu wiki points at is old and obsolete. gdb has been available since late last year. I did the work for that last summer/fall. Use the info here instead

https://fedoraproject.org/wiki/Architectures/RISC-V/Installing

The new name for the koji build server if you want an up-to-date image is

http://fedora.riscv.rocks/koji/

They are in the process of moving from the old server to the new server, so there may be some temporary instability.

There is no expectation that dynamic linking works with the current RV32E abi. The GNU toolchain doesn't support this. There may be a new embedded ABI defined at some point, but since that ABI is only intended for embedded systems (not embedded linux), this may not support dynamic linking with RV32E either.

I'm taking a look at the CALL versus CALL_PLT stuff and trying to make sense of it in the context of binutils and bfd.

In D63076#1556616, @jimw wrote:

The stage4 image that the qemu wiki points at is old and obsolete. gdb has been available since late last year. I did the work for that last summer/fall. Use the info here instead

https://fedoraproject.org/wiki/Architectures/RISC-V/Installing

The new name for the koji build server if you want an up-to-date image is

http://fedora.riscv.rocks/koji/

They are in the process of moving from the old server to the new server, so there may be some temporary instability.

There is no expectation that dynamic linking works with the current RV32E abi. The GNU toolchain doesn't support this. There may be a new embedded ABI defined at some point, but since that ABI is only intended for embedded systems (not embedded linux), this may not support dynamic linking with RV32E either.

I'm taking a look at the CALL versus CALL_PLT stuff and trying to make sense of it in the context of binutils and bfd.

@jimw Thank you for weighing in (as binutils maintainer and an effective overlord of the ABI)! llvm 9.0.0 will be branched on 18 July 2019. It will be good to get the support in before that day. R_ARM_JUMP24/R_AARCH64_JUMP26/R_X86_64_PLT32 have similar behaviors.

@ruiu @jrtc27 @jimw Do you have questions regarding parts other than the CALL versus CALL_PLT nuance? (It seems after https://sourceware.org/bugzilla/show_bug.cgi?id=24685 is fixed, the behavior of R_RISCV_CALL as implemented in this patch will be compatible with it)

[jimw]

After spending some time looking at this, I have to agree with MaskRay that there is no meaningful distinction between R_RISCV_CALL and R_RISCV_CALL_PLT and they can be implemented the same. As a test, I tried modifying GNU ld to handle R_RISCV_CALL exactly the same as R_RISCV_CALL_PLT, did native linux and cross newlib/linux builds and checks, and nothing broke. The native gcc testsuite run is still going, but I don't expect that anything will fail. I found the aarch64 ABI to be convincing. It is a relatively recently designed ABI and has only R_AARCH64_CALL26 for calls. An aarch64 toolchain emits the same assembly source for pic and non-pic calls, and creates the same object file code and relocs. It isn't until link time where you see a difference, depending on the type of linking, and symbol info, at which point the R_RISCV_CALL reloc is modified to point directly at the target symbol or the PLT for it. Older ABIs have separate relocs and separate assembly syntax for pic versus non-pic because older toolchains needed that, but current toolchains don't need that anymore. It looks like a mistake designing the RISC-V ABI that we ended up with some old style assembler syntax and relocs that we don't need.

I haven't studied the full patch. I'm already working on too many different projects and don't want to increase the number of them. I'm just trying to comment on the stuff that appears to be in question.

[MaskRay]

Jim is not against my treatment of CALL and CALL_PLT. Does anyone want to give me a stamp? :) I really want to see this in lld 9.0.0

[jrtc27]

I don't like the mismatch between LLD and BFD, but this is a definite improvement over the current state. I'm happy for this to be merged, with the intent that discussions about CALL/CALL_PLT continue in the RISC-V ABI world to get a consensus, specify it and then fix any implementations that deviate from it, whether that be BFD, LLD or both.

ELF/Arch/RISCV.cpp

 //===- RISCV.cpp ----------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "InputFiles.h"
+#include "SyntheticSections.h"
 #include "Target.h"
 
 using namespace llvm;
 using namespace llvm::object;
 using namespace llvm::support::endian;
 using namespace llvm::ELF;
 using namespace lld;
 using namespace lld::elf;
 
 namespace {
 
 class RISCV final : public TargetInfo {
 public:
   RISCV();
   uint32_t calcEFlags() const override;
+  void writeGotHeader(uint8_t *Buf) const override;
+  void writeGotPlt(uint8_t *Buf, const Symbol &S) const override;
+  void writePltHeader(uint8_t *Buf) const override;
+  void writePlt(uint8_t *Buf, uint64_t GotPltEntryAddr, uint64_t PltEntryAddr,
+                int32_t Index, unsigned RelOff) const override;
+  RelType getDynRel(RelType Type) const override;
   RelExpr getRelExpr(RelType Type, const Symbol &S,
                      const uint8_t *Loc) const override;
   void relocateOne(uint8_t *Loc, RelType Type, uint64_t Val) const override;
 };
 
 } // end anonymous namespace
 
-RISCV::RISCV() { NoneRel = R_RISCV_NONE; }
+enum Op {
+  ADDI = 0x13,
+  AUIPC = 0x17,
+  JALR = 0x67,
+  LD = 0x3003,
+  LW = 0x2003,
+  SRLI = 0x5013,
+  SUB = 0x40000033,
+};
+
+enum Reg {
+  X_RA = 1,
+  X_T0 = 5,
+  X_T1 = 6,
+  X_T2 = 7,
+  X_T3 = 28,
+};
+
+static uint32_t hi20(uint32_t Val) { return (Val + 0x800) >> 12; }
+static uint32_t lo12(uint32_t Val) { return Val & 4095; }
+
+static uint32_t itype(uint32_t Op, uint32_t Rd, uint32_t Rs1, uint32_t Imm) {
+  return Op | (Rd << 7) | (Rs1 << 15) | (Imm << 20);
+}
+static uint32_t rtype(uint32_t Op, uint32_t Rd, uint32_t Rs1, uint32_t Rs2) {
+  return Op | (Rd << 7) | (Rs1 << 15) | (Rs2 << 20);
+}
+static uint32_t utype(uint32_t Op, uint32_t Rd, uint32_t Imm) {
+  return Op | (Rd << 7) | (Imm << 12);
+}
+
+RISCV::RISCV() {
+  CopyRel = R_RISCV_COPY;
+  NoneRel = R_RISCV_NONE;
+  PltRel = R_RISCV_JUMP_SLOT;
+  RelativeRel = R_RISCV_RELATIVE;
+  SymbolicRel = Config->Is64 ? R_RISCV_64 : R_RISCV_32;
+  GotRel = SymbolicRel;
+
+  // .got[0] = _DYNAMIC
+  GotBaseSymInGotPlt = false;
+  GotHeaderEntriesNum = 1;
+
+  // .got.plt[0] = _dl_runtime_resolve, .got.plt[1] = link_map
+  GotPltHeaderEntriesNum = 2;
+
+  PltEntrySize = 16;
+  PltHeaderSize = 32;
+}
 
 static uint32_t getEFlags(InputFile *F) {
   if (Config->Is64)
     return cast<ObjFile<ELF64LE>>(F)->getObj().getHeader()->e_flags;
   return cast<ObjFile<ELF32LE>>(F)->getObj().getHeader()->e_flags;
 }
 
 uint32_t RISCV::calcEFlags() const {
for (InputFile *F : ObjectFiles) {
     if ((EFlags & EF_RISCV_RVE) != (Target & EF_RISCV_RVE))
       error(toString(F) +
             ": cannot link object files with different EF_RISCV_RVE");
   }
 
   return Target;
 }
 
+void RISCV::writeGotHeader(uint8_t *Buf) const {
+  if (Config->Is64)
+    write64le(Buf, Main->Dynamic->getVA());
+  else
+    write32le(Buf, Main->Dynamic->getVA());
+}
+
+void RISCV::writeGotPlt(uint8_t *Buf, const Symbol &S) const {
+  if (Config->Is64)
+    write64le(Buf, In.Plt->getVA());
+  else
+    write32le(Buf, In.Plt->getVA());
+}
+
+void RISCV::writePltHeader(uint8_t *Buf) const {
+  if (Config->EFlags & EF_RISCV_RVE)
+    error("PLT is not supported for RVE");
+
+  // 1: auipc t2, %pcrel_hi(.got.plt)
+  // sub t1, t1, t3
+  // l[wd] t3, %pcrel_lo(1b)(t2); t3 = _dl_runtime_resolve
+  // addi t1, t1, -PltHeaderSize-12; t1 = &.plt[i] - &.plt[0]
+  // addi t0, t2, %pcrel_lo(1b)
+  // srli t1, t1, (rv64?1:2); t1 = &.got.plt[i] - &.got.plt[0]
+  // l[wd] t0, Wordsize(t0); t0 = link_map
+  // jr t3
+  uint32_t Offset = In.GotPlt->getVA() - In.Plt->getVA();
+  uint32_t Load = Config->Is64 ? LD : LW;
+  write32le(Buf + 0, utype(AUIPC, X_T2, hi20(Offset)));
+  write32le(Buf + 4, rtype(SUB, X_T1, X_T1, X_T3));
+  write32le(Buf + 8, itype(Load, X_T3, X_T2, lo12(Offset)));
+  write32le(Buf + 12, itype(ADDI, X_T1, X_T1, -Target->PltHeaderSize - 12));
+  write32le(Buf + 16, itype(ADDI, X_T0, X_T2, lo12(Offset)));
+  write32le(Buf + 20, itype(SRLI, X_T1, X_T1, Config->Is64 ? 1 : 2));
+  write32le(Buf + 24, itype(Load, X_T0, X_T0, Config->Wordsize));
+  write32le(Buf + 28, itype(JALR, 0, X_T3, 0));
+}
+
+void RISCV::writePlt(uint8_t *Buf, uint64_t GotPltEntryAddr,
+                     uint64_t PltEntryAddr, int32_t Index,
+                     unsigned RelOff) const {
+  // 1: auipc t3, %pcrel_hi(f@.got.plt)
+  // l[wd] t3, %pcrel_lo(1b)(t3)
+  // jalr t1, t3
+  // nop
+  uint32_t Offset = GotPltEntryAddr - PltEntryAddr;
+  write32le(Buf + 0, utype(AUIPC, X_T3, hi20(Offset)));
+  write32le(Buf + 4, itype(Config->Is64 ? LD : LW, X_T3, X_T3, lo12(Offset)));
+  write32le(Buf + 8, itype(JALR, X_T1, X_T3, 0));
+  write32le(Buf + 12, itype(ADDI, 0, 0, 0));
+}
+
+RelType RISCV::getDynRel(RelType Type) const {
+  return Type == Target->GotRel ? Type : R_RISCV_NONE;
+}
+
 RelExpr RISCV::getRelExpr(const RelType Type, const Symbol &S,
                           const uint8_t *Loc) const {
   switch (Type) {
   case R_RISCV_JAL:
   case R_RISCV_BRANCH:
   case R_RISCV_CALL:
   case R_RISCV_PCREL_HI20:
   case R_RISCV_RVC_BRANCH:
   case R_RISCV_RVC_JUMP:
   case R_RISCV_32_PCREL:
     return R_PC;
+  case R_RISCV_CALL_PLT:
+    return R_PLT_PC;
+  case R_RISCV_GOT_HI20:
+    return R_GOT_PC;
   case R_RISCV_PCREL_LO12_I:
   case R_RISCV_PCREL_LO12_S:
     return R_RISCV_PC_INDIRECT;
   case R_RISCV_RELAX:
   case R_RISCV_ALIGN:
     return R_HINT;
   default:
     return R_ABS;
case R_RISCV_BRANCH: {
     uint32_t Imm11 = extractBits(Val, 11, 11) << 7;
     Insn |= Imm12 | Imm10_5 | Imm4_1 | Imm11;
 
     write32le(Loc, Insn);
     return;
   }
 
   // auipc + jalr pair
-  case R_RISCV_CALL: {
+  case R_RISCV_CALL:
+  case R_RISCV_CALL_PLT: {
     int64_t Hi = SignExtend64(Val + 0x800, Bits) >> 12;
     checkInt(Loc, Hi, 20, Type);
     if (isInt<20>(Hi)) {
       relocateOne(Loc, R_RISCV_PCREL_HI20, Val);
       relocateOne(Loc + 4, R_RISCV_PCREL_LO12_I, Val);
     }
     return;
   }
 
+  case R_RISCV_GOT_HI20:
   case R_RISCV_PCREL_HI20:
   case R_RISCV_HI20: {
     uint64_t Hi = Val + 0x800;
     checkInt(Loc, SignExtend64(Hi, Bits) >> 12, 20, Type);
     write32le(Loc, (read32le(Loc) & 0xFFF) | (Hi & 0xFFFFF000));
     return;
   }
 

ELF/InputSection.cpp

static Relocation *getRISCVPCRelHi20(const Symbol *Sym, uint64_t Addend) {
   R.Offset = D->Value;
   auto Range =
       std::equal_range(IS->Relocations.begin(), IS->Relocations.end(), R,
                        [](const Relocation &LHS, const Relocation &RHS) {
                          return LHS.Offset < RHS.Offset;
                        });
 
   for (auto It = Range.first; It != Range.second; ++It)
-    if (It->Expr == R_PC)
+    if (It->Type == R_RISCV_PCREL_HI20 || It->Type == R_RISCV_GOT_HI20)
       return &*It;
 
   error("R_RISCV_PCREL_LO12 relocation points to " + IS->getObjMsg(D->Value) +
         " without an associated R_RISCV_PCREL_HI20 relocation");
   return nullptr;
 }
 
 // A TLS symbol's virtual address is relative to the TLS segment. Add a

test/ELF/riscv-plt.s

+# REQUIRES: riscv
+# RUN: echo '.globl bar, weak; .type bar,@function; .type weak,@function; bar: weak:' > %t1.s
+
+# RUN: llvm-mc -filetype=obj -triple=riscv32 %t1.s -o %t1.32.o
+# RUN: ld.lld -shared %t1.32.o -o %t1.32.so
+# RUN: llvm-mc -filetype=obj -triple=riscv32 %s -o %t.32.o
+# RUN: ld.lld %t.32.o %t1.32.so -o %t.32
+# RUN: llvm-readelf -S %t.32 | FileCheck --check-prefix=SEC %s
+# RUN: llvm-readobj -r %t.32 | FileCheck --check-prefix=RELOC32 %s
+# RUN: llvm-readelf -x .got.plt %t.32 | FileCheck --check-prefix=GOTPLT32 %s
+# RUN: llvm-objdump -d --no-show-raw-insn %t.32 | FileCheck --check-prefixes=DIS,DIS32 %s
+
+# RUN: llvm-mc -filetype=obj -triple=riscv64 %t1.s -o %t1.64.o
+# RUN: ld.lld -shared %t1.64.o -o %t1.64.so
+# RUN: llvm-mc -filetype=obj -triple=riscv64 %s -o %t.64.o
+# RUN: ld.lld %t.64.o %t1.64.so -o %t.64
+# RUN: llvm-readelf -S %t.64 | FileCheck --check-prefix=SEC %s
+# RUN: llvm-readobj -r %t.64 | FileCheck --check-prefix=RELOC64 %s
+# RUN: llvm-readelf -x .got.plt %t.64 | FileCheck --check-prefix=GOTPLT64 %s
+# RUN: llvm-objdump -d --no-show-raw-insn %t.64 | FileCheck --check-prefixes=DIS,DIS64 %s
+
+# RUN: llvm-mc -filetype=obj -triple=riscv32 -mattr=+e %t1.s -o %t1.32e.o
+# RUN: ld.lld -shared %t1.32e.o -o %t1.32e.so
+# RUN: llvm-mc -filetype=obj -triple=riscv32 -mattr=+e %s -o %t.32e.o
+# RUN: not ld.lld %t.32e.o %t1.32e.so -o %t.32e 2>&1 | FileCheck --check-prefix=ERR %s
+# ERR: PLT is not supported for RVE
+
+# SEC: .plt PROGBITS {{0*}}00011030
+
+## The .got.plt slots relocated by .rela.plt point to .plt
+## This is required by glibc.
+# RELOC32:      .rela.plt {
+# RELOC32-NEXT:   0x13008 R_RISCV_JUMP_SLOT bar 0x0
+# RELOC32-NEXT:   0x1300C R_RISCV_JUMP_SLOT weak 0x0
+# RELOC32-NEXT: }
+# GOTPLT32:      section '.got.plt'
+# GOTPLT32-NEXT: 0x00013000 00000000 00000000 30100100 30100100
+
+# RELOC64:      .rela.plt {
+# RELOC64-NEXT:   0x13010 R_RISCV_JUMP_SLOT bar 0x0
+# RELOC64-NEXT:   0x13018 R_RISCV_JUMP_SLOT weak 0x0
+# RELOC64-NEXT: }
+# GOTPLT64:      section '.got.plt'
+# GOTPLT64-NEXT: 0x00013000 00000000 00000000 00000000 00000000
+# GOTPLT64-NEXT: 0x00013010 30100100 00000000 30100100 00000000
+
+# DIS:      _start:
+## Direct call
+# DIS-NEXT:          auipc ra, 0
+# DIS-NEXT:   11004: jalr ra, ra, 32
+## foo@plt - . = 0x11050-0x1100c = 72
+# DIS-NEXT:          auipc ra, 0
+# DIS-NEXT:   1100c: jalr ra, ra, 72
+## foo@plt - . = 0x11050-0x1100c = 72
+# DIS-NEXT:          auipc ra, 0
+# DIS-NEXT:   11014: jalr ra, ra, 64
+## bar@plt - . = 0x11060-0x1101c = 76
+# DIS-NEXT:          auipc ra, 0
+# DIS-NEXT:   1101c: jalr ra, ra, 72
+# DIS:      foo:
+# DIS-NEXT:   11020:
+
+# DIS:      Disassembly of section .plt:
+# DIS:      .plt:
+# DIS-NEXT:     auipc t2, 2
+# DIS-NEXT:     sub t1, t1, t3
+## .got.plt - .plt = 0x13000 - 0x11030 = 4096*2-48
+# DIS32-NEXT:   lw t3, -48(t2)
+# DIS64-NEXT:   ld t3, -48(t2)
+# DIS-NEXT:     addi t1, t1, -44
+# DIS-NEXT:     addi t0, t2, -48
+# DIS32-NEXT:   srli t1, t1, 2
+# DIS64-NEXT:   srli t1, t1, 1
+# DIS32-NEXT:   lw t0, 4(t0)
+# DIS64-NEXT:   ld t0, 8(t0)
+# DIS-NEXT:     jr t3
+
+## 32-bit: &.got.plt[bar]-. = 0x13008-0x11050 = 4096*2-72
+# DIS:        11050: auipc t3, 2
+# DIS32-NEXT:   lw t3, -72(t3)
+# DIS64-NEXT:   ld t3, -64(t3)
+# DIS-NEXT:     jalr t1, t3, 0
+# DIS-NEXT:     nop
+
+## 32-bit: &.got.plt[weak]-. = 0x1300c-0x11060 = 4096*2-84
+# DIS:        11060: auipc t3, 2
+# DIS32-NEXT:   lw t3, -84(t3)
+# DIS64-NEXT:   ld t3, -72(t3)
+# DIS-NEXT:     jalr t1, t3, 0
+# DIS-NEXT:     nop
+
+.global _start, foo, bar
+.weak weak
+
+_start:
+  call foo
+  call bar
+  call bar@plt
+  call weak
+
+## foo is local and non-preemptale, no PLT is generated.
+foo:
+  ret

test/ELF/riscv-reloc-64-pic.s

+# REQUIRES: riscv
+# RUN: llvm-mc -filetype=obj -triple=riscv64 %s -o %t.o
+# RUN: not ld.lld -shared %t.o -o %t.so 2>&1 | FileCheck %s
+
+# CHECK: error: relocation R_RISCV_32 cannot be used against symbol a
+
+.globl a
+
+.data
+.long a

test/ELF/riscv-reloc-copy.s

+# REQUIRES: riscv
+# RUN: llvm-mc -filetype=obj -triple=riscv32 %p/Inputs/relocation-copy.s -o %t1.o
+# RUN: ld.lld -shared %t1.o -o %t1.so
+# RUN: llvm-mc -filetype=obj -triple=riscv32 %s -o %t.o
+# RUN: ld.lld %t.o %t1.so -o %t
+# RUN: llvm-readobj -r %t | FileCheck --check-prefix=RELOC %s
+# RUN: llvm-nm -S %t | FileCheck --check-prefix=NM32 %s
+
+# RUN: llvm-mc -filetype=obj -triple=riscv64 %p/Inputs/relocation-copy.s -o %t1.o
+# RUN: ld.lld -shared %t1.o -o %t1.so
+# RUN: llvm-mc -filetype=obj -triple=riscv64 %s -o %t.o
+# RUN: ld.lld %t.o %t1.so -o %t
+# RUN: llvm-readobj -r %t | FileCheck --check-prefix=RELOC %s
+# RUN: llvm-nm -S %t | FileCheck --check-prefix=NM64 %s
+
+# RELOC:      .rela.dyn {
+# RELOC-NEXT:   0x13000 R_RISCV_COPY x 0x0
+# RELOC-NEXT: }
+
+# NM32: 00013000 00000004 B x
+# NM64: 0000000000013000 0000000000000004 B x
+
+la a0, x

test/ELF/riscv-reloc-got.s

+# REQUIRES: riscv
+# RUN: echo '.globl b; b:' | llvm-mc -filetype=obj -triple=riscv32 - -o %t1.o
+# RUN: ld.lld -shared %t1.o -o %t1.so
+
+# RUN: llvm-mc -filetype=obj -triple=riscv32 -position-independent %s -o %t.o
+# RUN: ld.lld %t.o %t1.so -o %t
+# RUN: llvm-readobj -r %t | FileCheck --check-prefix=RELOC32 %s
+# RUN: llvm-nm %t | FileCheck --check-prefix=NM %s
+# RUN: llvm-readobj -x .got %t | FileCheck --check-prefix=HEX32 %s
+# RUN: llvm-objdump -d --no-show-raw-insn %t | FileCheck --check-prefix=DIS32 %s
+
+# RUN: echo '.globl b; b:' | llvm-mc -filetype=obj -triple=riscv64 - -o %t1.o
+# RUN: ld.lld -shared %t1.o -o %t1.so
+
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -position-independent %s -o %t.o
+# RUN: ld.lld %t.o %t1.so -o %t
+# RUN: llvm-readobj -r %t | FileCheck --check-prefix=RELOC64 %s
+# RUN: llvm-nm %t | FileCheck --check-prefix=NM %s
+# RUN: llvm-readobj -x .got %t | FileCheck --check-prefix=HEX64 %s
+# RUN: llvm-objdump -d --no-show-raw-insn %t | FileCheck --check-prefix=DIS64 %s
+
+# SEC: .got PROGBITS 00012060 020060 00000c
+
+# RELOC32:      .rela.dyn {
+# RELOC32-NEXT:   0x12068 R_RISCV_32 b 0x0
+# RELOC32-NEXT: }
+
+# RELOC64:      .rela.dyn {
+# RELOC64-NEXT:   0x120D0 R_RISCV_64 b 0x0
+# RELOC64-NEXT: }
+
+# NM: 00013000 d a
+
+## .got[0] = _DYNAMIC
+## .got[1] = a (filled at link time)
+## .got[2] = 0 (relocated by R_RISCV_64 at runtime)
+# HEX32: section '.got':
+# HEX32: 0x00012060 00200100 00300100 00000000
+
+# HEX64: section '.got':
+# HEX64: 0x000120c0 00200100 00000000 00300100 00000000
+# HEX64: 0x000120d0 00000000 00000000
+
+## &.got[1]-. = 0x12060-0x11000 = 4096*1+100
+# DIS32:      11000: auipc a0, 1
+# DIS32-NEXT:        lw a0, 100(a0)
+## &.got[2]-. = 0x12064-0x11008 = 4096*1+96
+# DIS32:      11008: auipc a0, 1
+# DIS32-NEXT:        lw a0, 96(a0)
+
+## &.got[1]-. = 0x120c8-0x11000 = 4096*1+100
+# DIS64:      11000: auipc a0, 1
+# DIS64-NEXT:        ld a0, 200(a0)
+## &.got[2]-. = 0x120d0-0x11008 = 4096*1+200
+# DIS64:      11008: auipc a0, 1
+# DIS64-NEXT:        ld a0, 200(a0)
+
+la a0,a
+la a0,b
+
+.data
+a:
+## An undefined reference of _GLOBAL_OFFSET_TABLE_ causes .got[0] to be
+## allocated to store _DYNAMIC.
+.long _GLOBAL_OFFSET_TABLE_ - .

test/ELF/riscv32-reloc-32-pic.s

+# REQUIRES: riscv
+# RUN: llvm-mc -filetype=obj -triple=riscv32 %s -o %t.o
+# RUN: ld.lld -shared %t.o -o %t.so
+# RUN: llvm-nm %t.so | FileCheck --check-prefix=NM %s
+# RUN: llvm-readobj -r %t.so | FileCheck --check-prefix=RELOC %s
+
+## R_RISCV_32 is an absolute relocation type.
+## In PIC mode, it creates a relative relocation if the symbol is non-preemptable.
+
+# NM: 00002004 d b
+
+# RELOC:      .rela.dyn {
+# RELOC-NEXT:   0x2004 R_RISCV_RELATIVE - 0x2004
+# RELOC-NEXT:   0x2000 R_RISCV_32 a 0
+# RELOC-NEXT: }
+
+.globl a, b
+.hidden b
+
+.data
+.long a
+b:
+.long b

test/ELF/riscv64-reloc-64-pic.s

+# REQUIRES: riscv
+# RUN: llvm-mc -filetype=obj -triple=riscv64 %s -o %t.o
+# RUN: ld.lld -shared %t.o -o %t.so
+# RUN: llvm-nm %t.so | FileCheck --check-prefix=NM %s
+# RUN: llvm-readobj -r %t.so | FileCheck --check-prefix=RELOC %s
+
+## R_RISCV_64 is an absolute relocation type.
+## In PIC mode, it creates a relative relocation if the symbol is non-preemptable.
+
+# NM: 0000000000002008 d b
+
+# RELOC:      .rela.dyn {
+# RELOC-NEXT:   0x2008 R_RISCV_RELATIVE - 0x2008
+# RELOC-NEXT:   0x2000 R_RISCV_64 a 0
+# RELOC-NEXT: }
+
+.globl a, b
+.hidden b
+
+.data
+.quad a
+b:
+.quad b