[PATCH] RISC-V: PR27180, Update relocation for riscv_zero_pcrel_hi_reloc.
Nelson Chu
nelson@rivosinc.com
Fri Jun 28 05:53:14 GMT 2024
Committed after passing regression of riscv-gnu-toolchian and build-all/gdb.
Thanks
Nelson
On Thu, Jun 13, 2024 at 3:58 PM Nelson Chu <nelson@rivosinc.com> wrote:
> When pcrel access overflow, the riscv_zero_pcrel_hi_reloc may convert pcrel
> relocation to absolutly access if possible at the relocate stage. We used
> to
> encode the target address into r_sym of R_RISCV_HI20 if it is converted
> from
> R_RISCV_PCREL_HI20. But that may cause segfault if --emit-relocs is set,
> since r_sym becomes an address rather than a symbol index. Although the
> relocate result is correct, it does not meet the definition, so may cause
> unexpected behaviors.
>
> This patch encodes the target address into r_addend, rather than r_sym, if
> riscv_zero_pcrel_hi_reloc converts the relocation. Besdies, since the
> corresponding pcrel_lo relocation are also changed to absolutly access,
> we should also update them to R_RISCV_LO12_I/S.
>
> bfd/
> PR 27180
> * elfnn-riscv.c (riscv_pcrel_hi_reloc): New boolean `absolute', to
> inform corresponding pcrel_lo that the pcrel_hi relocation was
> already
> converted to hi20 relocation.
> (riscv_record_pcrel_hi_reloc): Likewise, record `absolute'.
> (riscv_pcrel_lo_reloc): Removed `const' for Elf_Internal_Rela
> *reloc,
> since we may need to convert it from pcrel_lo to lo relocation.
> (riscv_record_pcrel_lo_reloc): Likewise. Convert pcrel_lo to lo
> relocation if corresponding pcrel_hi was converted to hi
> relocation.
> (riscv_zero_pcrel_hi_reloc): Encode target absolute address into
> r_addend rather than r_sym. Clear the `addr' to avoid duplicate
> relocate in the perform_relocation.
> (riscv_elf_relocate_section): Updated.
> ld/
> PR 27180
> * testsuite/ld-riscv-elf/pcrel-lo-addend-3a-emit-relocs.d: New
> testcase.
> Segfault without applying this patch.
> * testsuite/ld-riscv-elf/ld-riscv-elf.exp: Updated.
> ---
> bfd/elfnn-riscv.c | 52 ++++++++++++++-----
> ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp | 1 +
> .../pcrel-lo-addend-3a-emit-relocs.d | 26 ++++++++++
> 3 files changed, 67 insertions(+), 12 deletions(-)
> create mode 100644
> ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3a-emit-relocs.d
>
> diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
> index 7591968ca9c..90ecc276f31 100644
> --- a/bfd/elfnn-riscv.c
> +++ b/bfd/elfnn-riscv.c
> @@ -1969,6 +1969,8 @@ typedef struct
> bfd_vma value;
> /* Original reloc type. */
> int type;
> + /* True if changed to R_RISCV_HI20. */
> + bool absolute;
> } riscv_pcrel_hi_reloc;
>
> typedef struct riscv_pcrel_lo_reloc
> @@ -1976,7 +1978,7 @@ typedef struct riscv_pcrel_lo_reloc
> /* PC value of auipc. */
> bfd_vma address;
> /* Internal relocation. */
> - const Elf_Internal_Rela *reloc;
> + Elf_Internal_Rela *reloc;
> /* Record the following information helps to resolve the %pcrel
> which cross different input section. For now we build a hash
> for pcrel at the start of riscv_elf_relocate_section, and then
> @@ -2043,7 +2045,7 @@ static bool
> riscv_zero_pcrel_hi_reloc (Elf_Internal_Rela *rel,
> struct bfd_link_info *info,
> bfd_vma pc,
> - bfd_vma addr,
> + bfd_vma *addr,
> bfd_byte *contents,
> const reloc_howto_type *howto)
> {
> @@ -2059,17 +2061,22 @@ riscv_zero_pcrel_hi_reloc (Elf_Internal_Rela *rel,
>
> /* If it's possible to reference the symbol using auipc we do so, as
> that's
> more in the spirit of the PC-relative relocations we're processing.
> */
> - bfd_vma offset = addr - pc;
> + bfd_vma offset = *addr - pc;
> if (ARCH_SIZE == 32 || VALID_UTYPE_IMM (RISCV_CONST_HIGH_PART (offset)))
> return false;
>
> /* If it's impossible to reference this with a LUI-based offset then
> don't
> bother to convert it at all so users still see the PC-relative
> relocation
> in the truncation message. */
> - if (ARCH_SIZE > 32 && !VALID_UTYPE_IMM (RISCV_CONST_HIGH_PART (addr)))
> + if (ARCH_SIZE > 32 && !VALID_UTYPE_IMM (RISCV_CONST_HIGH_PART (*addr)))
> return false;
>
> - rel->r_info = ELFNN_R_INFO (addr, R_RISCV_HI20);
> + /* PR27180, encode target absolute address into r_addend rather than
> + r_sym. Clear the ADDR to avoid duplicate relocate in the
> + perform_relocation. */
> + rel->r_info = ELFNN_R_INFO (0, R_RISCV_HI20);
> + rel->r_addend += *addr;
> + *addr = 0;
>
> bfd_vma insn = riscv_get_insn (howto->bitsize, contents +
> rel->r_offset);
> insn = (insn & ~MASK_AUIPC) | MATCH_LUI;
> @@ -2085,7 +2092,7 @@ riscv_record_pcrel_hi_reloc (riscv_pcrel_relocs *p,
> bool absolute)
> {
> bfd_vma offset = absolute ? value : value - addr;
> - riscv_pcrel_hi_reloc entry = {addr, offset, type};
> + riscv_pcrel_hi_reloc entry = {addr, offset, type, absolute};
> riscv_pcrel_hi_reloc **slot =
> (riscv_pcrel_hi_reloc **) htab_find_slot (p->hi_relocs, &entry,
> INSERT);
>
> @@ -2100,7 +2107,7 @@ riscv_record_pcrel_hi_reloc (riscv_pcrel_relocs *p,
> static bool
> riscv_record_pcrel_lo_reloc (riscv_pcrel_relocs *p,
> bfd_vma addr,
> - const Elf_Internal_Rela *reloc,
> + Elf_Internal_Rela *reloc,
> asection *input_section,
> struct bfd_link_info *info,
> reloc_howto_type *howto,
> @@ -2125,7 +2132,7 @@ riscv_resolve_pcrel_lo_relocs (riscv_pcrel_relocs *p)
> {
> bfd *input_bfd = r->input_section->owner;
>
> - riscv_pcrel_hi_reloc search = {r->address, 0, 0};
> + riscv_pcrel_hi_reloc search = {r->address, 0, 0, 0};
> riscv_pcrel_hi_reloc *entry = htab_find (p->hi_relocs, &search);
> /* There may be a risk if the %pcrel_lo with addend refers to
> an IFUNC symbol. The %pcrel_hi has been relocated to plt,
> @@ -2160,6 +2167,27 @@ riscv_resolve_pcrel_lo_relocs (riscv_pcrel_relocs
> *p)
>
> perform_relocation (r->howto, r->reloc, entry->value,
> r->input_section,
> input_bfd, r->contents);
> +
> + /* The corresponding R_RISCV_GOT_PCREL_HI20 and R_RISCV_PCREL_HI20
> are
> + converted to R_RISCV_HI20, so try to convert
> R_RISCV_PCREL_LO12_I/S
> + to R_RISCV_LO12_I/S. */
> + if (entry->absolute)
> + {
> + switch (ELFNN_R_TYPE (r->reloc->r_info))
> + {
> + case R_RISCV_PCREL_LO12_I:
> + r->reloc->r_info = ELFNN_R_INFO (0, R_RISCV_LO12_I);
> + r->reloc->r_addend += entry->value;
> + break;
> + case R_RISCV_PCREL_LO12_S:
> + r->reloc->r_info = ELFNN_R_INFO (0, R_RISCV_LO12_S);
> + r->reloc->r_addend += entry->value;
> + break;
> + default:
> + /* This shouldn't happen, so just skip it. */
> + break;
> + }
> + }
> }
>
> return true;
> @@ -2698,7 +2726,7 @@ riscv_elf_relocate_section (bfd *output_bfd,
> /* Address of got entry. */
> relocation = sec_addr (htab->elf.sgot) + off;
> absolute = riscv_zero_pcrel_hi_reloc (rel, info, pc,
> - relocation, contents,
> + &relocation, contents,
> howto);
> /* Update howto if relocation is changed. */
> howto = riscv_elf_rtype_to_howto (input_bfd,
> @@ -2706,8 +2734,8 @@ riscv_elf_relocate_section (bfd *output_bfd,
> if (howto == NULL)
> r = bfd_reloc_notsupported;
> else if (!riscv_record_pcrel_hi_reloc (&pcrel_relocs, pc,
> - relocation, r_type,
> - absolute))
> + relocation +
> rel->r_addend,
> + r_type, absolute))
> r = bfd_reloc_overflow;
> }
> break;
> @@ -2849,7 +2877,7 @@ riscv_elf_relocate_section (bfd *output_bfd,
> }
>
> case R_RISCV_PCREL_HI20:
> - absolute = riscv_zero_pcrel_hi_reloc (rel, info, pc, relocation,
> + absolute = riscv_zero_pcrel_hi_reloc (rel, info, pc, &relocation,
> contents, howto);
> /* Update howto if relocation is changed. */
> howto = riscv_elf_rtype_to_howto (input_bfd,
> diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> index 669ac5d506d..0ee20255551 100644
> --- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> +++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> @@ -131,6 +131,7 @@ if [istarget "riscv*-*-*"] {
> run_dump_test "pcrel-lo-addend-2a"
> run_dump_test "pcrel-lo-addend-2b"
> run_dump_test "pcrel-lo-addend-3a"
> + run_dump_test "pcrel-lo-addend-3a-emit-relocs"
> run_dump_test "pcrel-lo-addend-3b"
> run_dump_test "pcrel-lo-addend-3c"
> run_dump_test "code-model-medlow-01"
> diff --git a/ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3a-emit-relocs.d
> b/ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3a-emit-relocs.d
> new file mode 100644
> index 00000000000..d5a667c3279
> --- /dev/null
> +++ b/ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3a-emit-relocs.d
> @@ -0,0 +1,26 @@
> +#source: pcrel-lo-addend-3a.s
> +#as: -march=rv64i -mabi=lp64 -mno-relax
> +#ld: -m[riscv_choose_lp64_emul] -Tpcrel-lo-addend-3.ld --emit-relocs
> +#objdump: -dr
> +
> +#...
> +Disassembly of section .text:
> +
> +0+900000000 <_start>:
> +.*:[ ]+[0-9a-f]+[ ]+lui[ ]+a5,0x2
> +.*:[ ]+R_RISCV_HI20[ ]+\*ABS\*\+0x2000
> +.*:[ ]+[0-9a-f]+[ ]+ld[ ]+a0,0\(a5\) # 2000 <ll>
> +.*:[ ]+R_RISCV_LO12_I[ ]+\*ABS\*\+0x2000
> +.*:[ ]+[0-9a-f]+[ ]+ld[ ]+a0,4\(a5\)
> +.*:[ ]+R_RISCV_LO12_I[ ]+\*ABS\*\+0x2004
> +.*:[ ]+[0-9a-f]+[ ]+lui[ ]+a5,0x2
> +.*:[ ]+R_RISCV_HI20[ ]+\*ABS\*\+0x2004
> +.*:[ ]+[0-9a-f]+[ ]+ld[ ]+a0,4\(a5\) # 2004 <ll\+0x4>
> +.*:[ ]+R_RISCV_LO12_I[ ]+\*ABS\*\+0x2004
> +.*:[ ]+[0-9a-f]+[ ]+ld[ ]+a0,8\(a5\)
> +.*:[ ]+R_RISCV_LO12_I[ ]+\*ABS\*\+0x2008
> +.*:[ ]+[0-9a-f]+[ ]+lui[ ]+a5,0x1
> +.*:[ ]+R_RISCV_HI20[ ]+\*ABS\*\+0x1008
> +.*:[ ]+[0-9a-f]+[ ]+ld[ ]+a0,8\(a5\) # 1008
> <_GLOBAL_OFFSET_TABLE_\+0x8>
> +.*:[ ]+R_RISCV_LO12_I[ ]+\*ABS\*\+0x1008
> +#pass
> --
> 2.39.3 (Apple Git-146)
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://sourceware.org/pipermail/binutils/attachments/20240628/1c5b072b/attachment-0001.htm>
More information about the Binutils
mailing list