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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D159082#4641158 <https://reviews.llvm.org/D159082#4641158>, @jobnoorman wrote:

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

Opening an https://sourceware.org/ issue is useful but I do believe R_RISCV_RELAX=>R_RISCV_NONE on our side is unnecessary.

--emit-relocs is a debug or advanced feature. Giving more information by preserving the original type has some value. The R_RISCV_RELAX=>R_RISCV_NONE write is unneeded complexity, albeit small.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159082