[PATCH] D159082: [ELF][RISCV] Implement --emit-relocs with relaxation

Thu Sep 7 12:02:24 PDT 2023

jobnoorman added a comment.

In D159082#4639290 <https://reviews.llvm.org/D159082#4639290>, @jrtc27 wrote:

> I think it's still a bit dodgy for R_RISCV_ALIGN specifically, and don't like the divergence from GNU ld -- there should really be consensus between the toolchains there -- but it may be ok.

I definitely see the point here as well, especially about toolchain consensus.

While trying to figure out whether GNU ld's treatment of `R_RISCV_ALIGN` is intentional, I noticed that the behavior of `--emit-relocs --relax` has changed quite a bit since 2.40 (more specifically, since this commit <https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=43025f01a0c99c939e20e90a6e695741c14a925a>). Since then, most `R_RISCV_RELAX` relocations are also changed to `R_RISCV_NONE`. This is the output of GNU ld 2.41 on the test case in this patch:

  $ riscv64-linux-gnu-gcc -c llvm-project/lld/test/ELF/riscv-relax-emit-relocs.s
  $ binutils-gdb/build/ld/ld-new -q -Ttext=0 riscv-relax-emit-relocs.o
  $ riscv64-linux-gnu-objdump -dr a.out

  a.out:     file format elf64-littleriscv

  Disassembly of section .text:

  0000000000000000 <_start>:
     0:	008000ef          	jal	ra,8 <f>
  			0: R_RISCV_JAL	f
     4:	004000ef          	jal	ra,8 <f>
  			4: R_RISCV_NONE	*ABS*+0x4
  			4: R_RISCV_JAL	f

  0000000000000008 <f>:
     8:	8082                	ret
  			8: R_RISCV_NONE	*ABS*+0x4
  			8: R_RISCV_NONE	*ABS*+0x6

I have the feeling this behavior is not intentional at all but rather an artifact of the implementation of relaxation (metadata about necessary code deletion is encoded in relocations which are later overwritten with `R_RISCV_NONE`).

Given the behavior of the two toolchains has diverged, I think it would make sense to sync with the GNU devs to agree on what the behavior should be. I will start with either opening a bug report or proposing a patch for GNU binutils tomorrow.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159082