From 50331d64f1080c2c9957fb608e0af236b96c1a41 Mon Sep 17 00:00:00 2001 From: Nelson Chu Date: Tue, 22 Jun 2021 12:02:52 +0800 Subject: [PATCH] RISC-V: Clarify the addends of pc-relative access. The original discussion was here, https://github.com/riscv/riscv-elf-psabi-doc/issues/184 After discussing with Kito Cheng, I think the addends of %pcrel_hi and %pcrel_lo are both allowed in GNU toolchain. However, both of the them mean the offset of symbols, rather than the pc address. But the addends of %got_pcrel_hi and it's %pcrel_lo do not look reasonable. I believe gcc won't generate the got patterns with addends, so linker should report dangerous relocation errors, in case the assembly code use them. Another issue was here, https://sourceware.org/pipermail/binutils/2021-June/116983.html At the beginnig, I suppose %pcrel_hi and %pcrel_lo are valid only when they are in the same input section. But Jim Wilson points out that gcc may generate %hi and %lo in the different input sections, when -freorder-blocks-and-partition option is used. So that a memory references for a loop may have the %hi outside the loop, but the %lo remain in the loop. However, it is hard to create the testcases, to see if %pcrel_hi and %pcrel_lo have the same behavior. Unfortunately, I notice that the current pcrel resolver cannot work for the above case. For now we build a hash table for pcrel at the start of riscv_elf_relocate_section, and then free the hash at the end. But riscv_elf_relocate_section only handles an input section at a time, so that means we can only resolve the %pcrel_hi and %pcrel_lo which are in the same input section. Otherwise, like the above case, we will report "%pcrel_lo missing matching %pcrel_hi" for them. I have no plan to improve this in the short-term, so maybe we can wait until someone meets the problem before we deal with it. bfd/ * elfnn-riscv.c (riscv_pcrel_hi_reloc): Added field to store the original relocation type, in case the type is converted to R_RISCV_HI20. (riscv_pcrel_lo_reloc): Removed unused name field. (riscv_pcrel_relocs): Added comments. (riscv_zero_pcrel_hi_reloc): Removed unused input_bfd. (riscv_record_pcrel_hi_reloc): Updated. (riscv_record_pcrel_lo_reloc): Likewise. (riscv_resolve_pcrel_lo_relocs): Likewise. Check the original type of auipc, to make sure the %pcrel_lo without any addends. Otherwise, report dangerous relocation error. (riscv_elf_relocate_section): Updated above functions are changed. For R_RISCV_GOT_HI20, report dangerous relocation error when addend isn't zero. ld/ * testsuite/ld-riscv-elf/ld-riscv-elf.exp: Updated. * testsuite/ld-riscv-elf/pcrel-lo-addend-3a.d: New testcase. * testsuite/ld-riscv-elf/pcrel-lo-addend-3a.s: Likewise. * testsuite/ld-riscv-elf/pcrel-lo-addend-3b.d: New testcase. Should report error since the %pcrel_lo with addend refers to %got_pcrel_hi. * testsuite/ld-riscv-elf/pcrel-lo-addend-3b.s: Likewise. * testsuite/ld-riscv-elf/pcrel-lo-addend-3c.d: New testcase. Should report error since the %got_pcrel_hi with addend. * testsuite/ld-riscv-elf/pcrel-lo-addend-3c.s: Likewise. * testsuite/ld-riscv-elf/pcrel-lo-addend-3.ld: Likewise. --- bfd/ChangeLog | 17 ++ bfd/elfnn-riscv.c | 160 ++++++++++-------- ld/ChangeLog | 14 ++ ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp | 3 + .../ld-riscv-elf/pcrel-lo-addend-3.ld | 13 ++ .../ld-riscv-elf/pcrel-lo-addend-3a.d | 18 ++ .../ld-riscv-elf/pcrel-lo-addend-3a.s | 21 +++ .../ld-riscv-elf/pcrel-lo-addend-3b.d | 4 + .../ld-riscv-elf/pcrel-lo-addend-3b.s | 13 ++ .../ld-riscv-elf/pcrel-lo-addend-3c.d | 4 + .../ld-riscv-elf/pcrel-lo-addend-3c.s | 13 ++ 11 files changed, 212 insertions(+), 68 deletions(-) create mode 100644 ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3.ld create mode 100644 ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3a.d create mode 100644 ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3a.s create mode 100644 ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3b.d create mode 100644 ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3b.s create mode 100644 ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3c.d create mode 100644 ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3c.s diff --git a/bfd/ChangeLog b/bfd/ChangeLog index 212fe332e2d..f18e16549da 100644 --- a/bfd/ChangeLog +++ b/bfd/ChangeLog @@ -1,3 +1,20 @@ +2021-06-22 Nelson Chu + + * elfnn-riscv.c (riscv_pcrel_hi_reloc): Added field to store + the original relocation type, in case the type is converted to + R_RISCV_HI20. + (riscv_pcrel_lo_reloc): Removed unused name field. + (riscv_pcrel_relocs): Added comments. + (riscv_zero_pcrel_hi_reloc): Removed unused input_bfd. + (riscv_record_pcrel_hi_reloc): Updated. + (riscv_record_pcrel_lo_reloc): Likewise. + (riscv_resolve_pcrel_lo_relocs): Likewise. Check the original + type of auipc, to make sure the %pcrel_lo without any addends. + Otherwise, report dangerous relocation error. + (riscv_elf_relocate_section): Updated above functions are changed. + For R_RISCV_GOT_HI20, report dangerous relocation error when addend + isn't zero. + 2021-06-19 H.J. Lu PR ld/27998 diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c index eef1e800221..f206708a9f3 100644 --- a/bfd/elfnn-riscv.c +++ b/bfd/elfnn-riscv.c @@ -1739,25 +1739,41 @@ perform_relocation (const reloc_howto_type *howto, typedef struct { + /* PC value. */ bfd_vma address; + /* Relocation value with addend. */ bfd_vma value; + /* Original reloc type. */ + int type; } riscv_pcrel_hi_reloc; typedef struct riscv_pcrel_lo_reloc { + /* PC value of auipc. */ + bfd_vma address; + /* Internal relocation. */ + const 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 + free the hash at the end. But riscv_elf_relocate_section only + handles an input section at a time, so that means we can only + resolve the %pcrel_hi and %pcrel_lo which are in the same input + section. Otherwise, we will report dangerous relocation errors + for those %pcrel which are not in the same input section. */ asection *input_section; struct bfd_link_info *info; reloc_howto_type *howto; - const Elf_Internal_Rela *reloc; - bfd_vma addr; - const char *name; bfd_byte *contents; + /* The next riscv_pcrel_lo_reloc. */ struct riscv_pcrel_lo_reloc *next; } riscv_pcrel_lo_reloc; typedef struct { + /* Hash table for riscv_pcrel_hi_reloc. */ htab_t hi_relocs; + /* Linked list for riscv_pcrel_lo_reloc. */ riscv_pcrel_lo_reloc *lo_relocs; } riscv_pcrel_relocs; @@ -1805,8 +1821,7 @@ riscv_zero_pcrel_hi_reloc (Elf_Internal_Rela *rel, bfd_vma pc, bfd_vma addr, bfd_byte *contents, - const reloc_howto_type *howto, - bfd *input_bfd ATTRIBUTE_UNUSED) + const reloc_howto_type *howto) { /* We may need to reference low addreses in PC-relative modes even when the PC is far away from these addresses. For example, undefweak references @@ -1839,11 +1854,14 @@ riscv_zero_pcrel_hi_reloc (Elf_Internal_Rela *rel, } static bool -riscv_record_pcrel_hi_reloc (riscv_pcrel_relocs *p, bfd_vma addr, - bfd_vma value, bool absolute) +riscv_record_pcrel_hi_reloc (riscv_pcrel_relocs *p, + bfd_vma addr, + bfd_vma value, + int type, + bool absolute) { bfd_vma offset = absolute ? value : value - addr; - riscv_pcrel_hi_reloc entry = {addr, offset}; + riscv_pcrel_hi_reloc entry = {addr, offset, type}; riscv_pcrel_hi_reloc **slot = (riscv_pcrel_hi_reloc **) htab_find_slot (p->hi_relocs, &entry, INSERT); @@ -1857,20 +1875,19 @@ riscv_record_pcrel_hi_reloc (riscv_pcrel_relocs *p, bfd_vma addr, static bool riscv_record_pcrel_lo_reloc (riscv_pcrel_relocs *p, + bfd_vma addr, + const Elf_Internal_Rela *reloc, asection *input_section, struct bfd_link_info *info, reloc_howto_type *howto, - const Elf_Internal_Rela *reloc, - bfd_vma addr, - const char *name, bfd_byte *contents) { riscv_pcrel_lo_reloc *entry; entry = (riscv_pcrel_lo_reloc *) bfd_malloc (sizeof (riscv_pcrel_lo_reloc)); if (entry == NULL) return false; - *entry = (riscv_pcrel_lo_reloc) {input_section, info, howto, reloc, addr, - name, contents, p->lo_relocs}; + *entry = (riscv_pcrel_lo_reloc) {addr, reloc, input_section, info, + howto, contents, p->lo_relocs}; p->lo_relocs = entry; return true; } @@ -1884,26 +1901,34 @@ riscv_resolve_pcrel_lo_relocs (riscv_pcrel_relocs *p) { bfd *input_bfd = r->input_section->owner; - riscv_pcrel_hi_reloc search = {r->addr, 0}; + riscv_pcrel_hi_reloc search = {r->address, 0, 0}; riscv_pcrel_hi_reloc *entry = htab_find (p->hi_relocs, &search); - if (entry == NULL - /* Check the overflow when adding reloc addend. */ - || (RISCV_CONST_HIGH_PART (entry->value) - != RISCV_CONST_HIGH_PART (entry->value + r->reloc->r_addend))) + /* There may be a risk if the %pcrel_lo with addend refers to + an IFUNC symbol. The %pcrel_hi has been relocated to plt, + so the corresponding %pcrel_lo with addend looks wrong. */ + char *string = NULL; + if (entry == NULL) + string = _("%pcrel_lo missing matching %pcrel_hi"); + else if (entry->type == R_RISCV_GOT_HI20 + && r->reloc->r_addend != 0) + string = _("%pcrel_lo with addend isn't allowed for R_RISCV_GOT_HI20"); + else if (RISCV_CONST_HIGH_PART (entry->value) + != RISCV_CONST_HIGH_PART (entry->value + r->reloc->r_addend)) { - char *string; - if (entry == NULL) - string = _("%pcrel_lo missing matching %pcrel_hi"); - else if (asprintf (&string, - _("%%pcrel_lo overflow with an addend, the " - "value of %%pcrel_hi is 0x%" PRIx64 " without " - "any addend, but may be 0x%" PRIx64 " after " - "adding the %%pcrel_lo addend"), - (int64_t) RISCV_CONST_HIGH_PART (entry->value), - (int64_t) RISCV_CONST_HIGH_PART + /* Check the overflow when adding reloc addend. */ + if (asprintf (&string, + _("%%pcrel_lo overflow with an addend, the " + "value of %%pcrel_hi is 0x%" PRIx64 " without " + "any addend, but may be 0x%" PRIx64 " after " + "adding the %%pcrel_lo addend"), + (int64_t) RISCV_CONST_HIGH_PART (entry->value), + (int64_t) RISCV_CONST_HIGH_PART (entry->value + r->reloc->r_addend)) == -1) string = _("%pcrel_lo overflow with an addend"); + } + if (string != NULL) + { (*r->info->callbacks->reloc_dangerous) (r->info, string, input_bfd, r->input_section, r->reloc->r_offset); return true; @@ -2225,12 +2250,9 @@ riscv_elf_relocate_section (bfd *output_bfd, relocation = base_got->output_section->vma + base_got->output_offset + off; - r_type = ELFNN_R_TYPE (rel->r_info); - howto = riscv_elf_rtype_to_howto (input_bfd, r_type); - if (howto == NULL) - r = bfd_reloc_notsupported; - else if (!riscv_record_pcrel_hi_reloc (&pcrel_relocs, pc, - relocation, false)) + if (!riscv_record_pcrel_hi_reloc (&pcrel_relocs, pc, + relocation, r_type, + false)) r = bfd_reloc_overflow; goto do_relocation; @@ -2242,12 +2264,9 @@ riscv_elf_relocate_section (bfd *output_bfd, goto do_relocation; case R_RISCV_PCREL_HI20: - r_type = ELFNN_R_TYPE (rel->r_info); - howto = riscv_elf_rtype_to_howto (input_bfd, r_type); - if (howto == NULL) - r = bfd_reloc_notsupported; - else if (!riscv_record_pcrel_hi_reloc (&pcrel_relocs, pc, - relocation, false)) + if (!riscv_record_pcrel_hi_reloc (&pcrel_relocs, pc, + relocation, r_type, + false)) r = bfd_reloc_overflow; goto do_relocation; @@ -2384,21 +2403,29 @@ riscv_elf_relocate_section (bfd *output_bfd, local_got_offsets[r_symndx] |= 1; } } - relocation = sec_addr (htab->elf.sgot) + off; - absolute = riscv_zero_pcrel_hi_reloc (rel, - info, - pc, - relocation, - contents, - howto, - input_bfd); - r_type = ELFNN_R_TYPE (rel->r_info); - howto = riscv_elf_rtype_to_howto (input_bfd, r_type); - if (howto == NULL) - r = bfd_reloc_notsupported; - else if (!riscv_record_pcrel_hi_reloc (&pcrel_relocs, pc, - relocation, absolute)) - r = bfd_reloc_overflow; + + if (rel->r_addend != 0) + { + msg = _("The addend isn't allowed for R_RISCV_GOT_HI20"); + r = bfd_reloc_dangerous; + } + else + { + /* Address of got entry. */ + relocation = sec_addr (htab->elf.sgot) + off; + 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, + ELFNN_R_TYPE (rel->r_info)); + if (howto == NULL) + r = bfd_reloc_notsupported; + else if (!riscv_record_pcrel_hi_reloc (&pcrel_relocs, pc, + relocation, r_type, + absolute)) + r = bfd_reloc_overflow; + } break; case R_RISCV_ADD8: @@ -2499,20 +2526,16 @@ riscv_elf_relocate_section (bfd *output_bfd, } case R_RISCV_PCREL_HI20: - absolute = riscv_zero_pcrel_hi_reloc (rel, - info, - pc, - relocation, - contents, - howto, - input_bfd); - r_type = ELFNN_R_TYPE (rel->r_info); - howto = riscv_elf_rtype_to_howto (input_bfd, r_type); + 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, + ELFNN_R_TYPE (rel->r_info)); if (howto == NULL) r = bfd_reloc_notsupported; else if (!riscv_record_pcrel_hi_reloc (&pcrel_relocs, pc, relocation + rel->r_addend, - absolute)) + r_type, absolute)) r = bfd_reloc_overflow; break; @@ -2532,8 +2555,8 @@ riscv_elf_relocate_section (bfd *output_bfd, break; } - if (riscv_record_pcrel_lo_reloc (&pcrel_relocs, input_section, info, - howto, rel, relocation, name, + if (riscv_record_pcrel_lo_reloc (&pcrel_relocs, relocation, rel, + input_section, info, howto, contents)) continue; r = bfd_reloc_overflow; @@ -2726,7 +2749,8 @@ riscv_elf_relocate_section (bfd *output_bfd, BFD_ASSERT (off < (bfd_vma) -2); relocation = sec_addr (htab->elf.sgot) + off + (is_ie ? ie_off : 0); if (!riscv_record_pcrel_hi_reloc (&pcrel_relocs, pc, - relocation, false)) + relocation, r_type, + false)) r = bfd_reloc_overflow; unresolved_reloc = false; break; diff --git a/ld/ChangeLog b/ld/ChangeLog index dc08ec9f0a0..b4c49cc6513 100644 --- a/ld/ChangeLog +++ b/ld/ChangeLog @@ -1,3 +1,17 @@ +2021-06-22 Nelson Chu + + * testsuite/ld-riscv-elf/ld-riscv-elf.exp: Updated. + * testsuite/ld-riscv-elf/pcrel-lo-addend-3a.d: New testcase. + * testsuite/ld-riscv-elf/pcrel-lo-addend-3a.s: Likewise. + * testsuite/ld-riscv-elf/pcrel-lo-addend-3b.d: New testcase. + Should report error since the %pcrel_lo with addend refers to + %got_pcrel_hi. + * testsuite/ld-riscv-elf/pcrel-lo-addend-3b.s: Likewise. + * testsuite/ld-riscv-elf/pcrel-lo-addend-3c.d: New testcase. + Should report error since the %got_pcrel_hi with addend. + * testsuite/ld-riscv-elf/pcrel-lo-addend-3c.s: Likewise. + * testsuite/ld-riscv-elf/pcrel-lo-addend-3.ld: Likewise. + 2021-06-19 H.J. Lu PR ld/27998 diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp index 0b49ddcacf6..1f1245af707 100644 --- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp +++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp @@ -87,6 +87,9 @@ if [istarget "riscv*-*-*"] { run_dump_test "pcrel-lo-addend" 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-3b" + run_dump_test "pcrel-lo-addend-3c" run_dump_test "restart-relax" run_dump_test "attr-merge-arch-01" run_dump_test "attr-merge-arch-02" diff --git a/ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3.ld b/ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3.ld new file mode 100644 index 00000000000..fabb65bdc82 --- /dev/null +++ b/ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3.ld @@ -0,0 +1,13 @@ +ENTRY(_start) +SECTIONS +{ + .got 0x1000 : { + *(.got) + } + .data 0x2000: { + *(.data) + } + .text 0x900000000 : { + *(.text) + } +} diff --git a/ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3a.d b/ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3a.d new file mode 100644 index 00000000000..1f77e20d52e --- /dev/null +++ b/ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3a.d @@ -0,0 +1,18 @@ +#source: pcrel-lo-addend-3a.s +#as: -march=rv64i -mabi=lp64 -mno-relax +#ld: -m[riscv_choose_lp64_emul] -Tpcrel-lo-addend-3.ld +#objdump: -d + +#... +Disassembly of section .text: + +0+900000000 <_start>: +.*:[ ]+[0-9a-f]+[ ]+lui[ ]+a5,0x2 +.*:[ ]+[0-9a-f]+[ ]+ld[ ]+a0,0\(a5\) # 2000 +.*:[ ]+[0-9a-f]+[ ]+ld[ ]+a0,4\(a5\) +.*:[ ]+[0-9a-f]+[ ]+lui[ ]+a5,0x2 +.*:[ ]+[0-9a-f]+[ ]+ld[ ]+a0,4\(a5\) # 2004 +.*:[ ]+[0-9a-f]+[ ]+ld[ ]+a0,8\(a5\) +.*:[ ]+[0-9a-f]+[ ]+lui[ ]+a5,0x1 +.*:[ ]+[0-9a-f]+[ ]+ld[ ]+a0,8\(a5\) # 1008 <_GLOBAL_OFFSET_TABLE_\+0x8> +#pass diff --git a/ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3a.s b/ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3a.s new file mode 100644 index 00000000000..14dc596dca9 --- /dev/null +++ b/ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3a.s @@ -0,0 +1,21 @@ + .text + .globl _start + .align 3 +_start: +.LA0: auipc a5, %pcrel_hi (ll) + ld a0, %pcrel_lo (.LA0)(a5) + ld a0, %pcrel_lo (.LA0 + 0x4)(a5) + +.LA1: auipc a5, %pcrel_hi (ll + 0x4) + ld a0, %pcrel_lo (.LA1)(a5) + ld a0, %pcrel_lo (.LA1 + 0x4)(a5) + +.LA2: auipc a5, %got_pcrel_hi (ll) + ld a0, %pcrel_lo (.LA2)(a5) + + .globl ll + .data +ll: + .dword 0 + .dword 0 + .dword 0 diff --git a/ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3b.d b/ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3b.d new file mode 100644 index 00000000000..9c87c165a58 --- /dev/null +++ b/ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3b.d @@ -0,0 +1,4 @@ +#source: pcrel-lo-addend-3b.s +#as: -march=rv64i -mabi=lp64 -mno-relax +#ld: -m[riscv_choose_lp64_emul] -Tpcrel-lo-addend-3.ld +#error: .*dangerous relocation: The addend isn't allowed for R_RISCV_GOT_HI20 diff --git a/ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3b.s b/ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3b.s new file mode 100644 index 00000000000..0b7ebd63a20 --- /dev/null +++ b/ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3b.s @@ -0,0 +1,13 @@ + .text + .globl _start + .align 3 +_start: +.LA0: auipc a5, %got_pcrel_hi (ll + 0x4) + ld a0, %pcrel_lo (.LA0)(a5) + + .globl ll + .data +ll: + .dword 0 + .dword 0 + .dword 0 diff --git a/ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3c.d b/ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3c.d new file mode 100644 index 00000000000..295895b8267 --- /dev/null +++ b/ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3c.d @@ -0,0 +1,4 @@ +#source: pcrel-lo-addend-3c.s +#as: -march=rv64i -mabi=lp64 -mno-relax +#ld: -m[riscv_choose_lp64_emul] -Tpcrel-lo-addend-3.ld +#error: .*dangerous relocation: %pcrel_lo with addend isn't allowed for R_RISCV_GOT_HI20 diff --git a/ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3c.s b/ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3c.s new file mode 100644 index 00000000000..0495851cf93 --- /dev/null +++ b/ld/testsuite/ld-riscv-elf/pcrel-lo-addend-3c.s @@ -0,0 +1,13 @@ + .text + .globl _start + .align 3 +_start: +.LA0: auipc a5, %got_pcrel_hi (ll) + ld a0, %pcrel_lo (.LA0 + 0x4)(a5) + + .globl ll + .data +ll: + .dword 0 + .dword 0 + .dword 0 -- 2.39.5