From: Nelson Chu Date: Wed, 20 Dec 2023 02:37:41 +0000 (+0800) Subject: RISC-V: PR31179, The SET/ADD/SUB fix breaks ABI compatibility with 2.41 objects X-Git-Tag: upstream/2.42~367 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=73d931e560059a87d76f528fafbb4270a98746bc;p=platform%2Fupstream%2Fbinutils.git RISC-V: PR31179, The SET/ADD/SUB fix breaks ABI compatibility with 2.41 objects * Problematic fix commit, 2029e13917d53d2289d3ebb390c4f40bd2112d21 RISC-V: Clarify the behaviors of SET/ADD/SUB relocations * Bugzilla, https://sourceware.org/bugzilla/show_bug.cgi?id=31179#c5 The addend of SUB_ULEB128 should be zero if using .uleb128, but we make it non-zero by accident in assembler before. This causes troubles by applying the above commit, since the calculation is changed to support .reloc *SUB* relocations with non-zero addend. We encourage people to rebuild their stuff to get the non-zero addend of SUB_ULEB128, but that might need some times, so report warnings to inform people need to rebuild their stuff if --check-uleb128 is enabled. Since the failed .reloc cases for ADD/SET/SUB/ULEB128 are rarely to use, it may acceptable that stop supproting them until people rebuld their stuff, maybe half-year or a year later. Or maybe we should teach people that don't write the .reloc R_RISCV_SUB* with non-zero constant, and then report warnings/errors in assembler. bfd/ * elfnn-riscv.c (perform_relocation): Ignore the non-zero addend of R_RISCV_SUB_ULEB128. (riscv_elf_relocate_section): Report warnings to inform people need to rebuild their stuff if --check-uleb128 is enabled. So that can get the right non-zero addend of R_RISCV_SUB_ULEB128. * elfxx-riscv.h (struct riscv_elf_params): Added bool check_uleb128. ld/ * NEWS: Updated. * emultempl/riscvelf.em: Added linker risc-v target options, --[no-]check-uleb128, to enable/disable checking if the addend of uleb128 is non-zero or not. So that people will know they need to rebuild the objects with binutils 2.42 and up, to get the right zero addend of SUB_ULEB128 relocation, or they may get troubles if using .reloc. * ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp: Updated. * ld/testsuite/ld-riscv-elf/pr31179*: New test cases. --- diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c index 042266e..509d61e 100644 --- a/bfd/elfnn-riscv.c +++ b/bfd/elfnn-riscv.c @@ -1735,19 +1735,9 @@ perform_relocation (const reloc_howto_type *howto, if (howto->pc_relative) value -= sec_addr (input_section) + rel->r_offset; - switch (ELFNN_R_TYPE (rel->r_info)) - { - case R_RISCV_SUB6: - case R_RISCV_SUB8: - case R_RISCV_SUB16: - case R_RISCV_SUB32: - case R_RISCV_SUB64: - case R_RISCV_SUB_ULEB128: - value -= rel->r_addend; - break; - default: - value += rel->r_addend; - } + /* PR31179, ignore the non-zero addend of R_RISCV_SUB_ULEB128. */ + if (ELFNN_R_TYPE (rel->r_info) != R_RISCV_SUB_ULEB128) + value += rel->r_addend; switch (ELFNN_R_TYPE (rel->r_info)) { @@ -2530,9 +2520,35 @@ riscv_elf_relocate_section (bfd *output_bfd, if (uleb128_set_rel != NULL && uleb128_set_rel->r_offset == rel->r_offset) { - relocation = uleb128_set_vma - relocation + uleb128_set_rel->r_addend; + relocation = uleb128_set_vma - relocation + + uleb128_set_rel->r_addend; uleb128_set_vma = 0; uleb128_set_rel = NULL; + + /* PR31179, the addend of SUB_ULEB128 should be zero if using + .uleb128, but we make it non-zero by accident in assembler, + so just ignore it in perform_relocation, and make assembler + continue doing the right thing. Don't reset the addend of + SUB_ULEB128 to zero here since it will break the --emit-reloc, + even though the non-zero addend is unexpected. + + We encourage people to rebuild their stuff to get the + non-zero addend of SUB_ULEB128, but that might need some + times, so report warnings to inform people need to rebuild + if --check-uleb128 is enabled. However, since the failed + .reloc cases for ADD/SET/SUB/ULEB128 are rarely to use, it + may acceptable that stop supproting them until people rebuld + their stuff, maybe half-year or one year later. I believe + this might be the least harmful option that we should go. + + Or maybe we should teach people that don't write the + .reloc R_RISCV_SUB* with non-zero constant, and report + warnings/errors in assembler. */ + if (htab->params->check_uleb128 + && rel->r_addend != 0) + _bfd_error_handler (_("%pB: warning: R_RISCV_SUB_ULEB128 with" + " non-zero addend, please rebuild by" + " binutils 2.42 or up"), input_bfd); } else { diff --git a/bfd/elfxx-riscv.h b/bfd/elfxx-riscv.h index abcb409..6df2471 100644 --- a/bfd/elfxx-riscv.h +++ b/bfd/elfxx-riscv.h @@ -31,6 +31,8 @@ struct riscv_elf_params { /* Whether to relax code sequences to GP-relative addressing. */ bool relax_gp; + /* Whether to check if SUB_ULEB128 relocation has non-zero addend. */ + bool check_uleb128; }; extern void riscv_elf32_set_options (struct bfd_link_info *, diff --git a/ld/NEWS b/ld/NEWS index 835dc39..adc5c9e 100644 --- a/ld/NEWS +++ b/ld/NEWS @@ -1,5 +1,10 @@ -*- text -*- +* On RISC-V, add ld target option --[no-]check-uleb128. Should rebuild the + objects by binutils 2.42 and up if enabling the option and get warnings, + since the non-zero addend of SUB_ULEB128 shouldn't be generated from .uleb128 + directives. + * Add support for the KVX instruction set. * A new linker script sorting directive has been added: REVERSE. This reverses diff --git a/ld/emultempl/riscvelf.em b/ld/emultempl/riscvelf.em index bb6298d..8aaed1f 100644 --- a/ld/emultempl/riscvelf.em +++ b/ld/emultempl/riscvelf.em @@ -25,7 +25,8 @@ fragment <