Thanks, committed first. If there still are some concerns or changes, we can fix them in the future patches. Nelson On Thu, May 18, 2023 at 5:31 PM Kito Cheng wrote: > LGTM, thanks! > > This patch actually has been verified within SiFive internally for > years and works well, and finally the psABI spec part has settled down > now, it's really good to see this supported in binutils. > > NOTE: LLVM/LLD patch is review in progress: > https://reviews.llvm.org/D142879 https://reviews.llvm.org/D142880 > > On Thu, May 18, 2023 at 5:23 PM Nelson Chu wrote: > > > > From: Kuan-Lin Chen > > > > > https://github.com/riscv-non-isa/riscv-elf-psabi-doc/commit/96d6e190e9fc04a8517f9ff7fb9aed3e9876cbd6 > > > > There are some known limitations for now, > > > > * Do not shrink the length of the uleb128 value, even if the value is > reduced > > after relaxations. Also reports error if the length grows up. > > > > * The R_RISCV_SET_ULEB128 needs to be paired with and be placed before > the > > R_RISCV_SUB_ULEB128. > > > > bfd/ > > * bfd-in2.h: Regenerated. > > * elfnn-riscv.c (perform_relocation): Perform > R_RISCV_SUB_ULEB128 and > > R_RISCV_SET_ULEB128 relocations. Do not shrink the length of the > > uleb128 value, and report error if the length grows up. Called > the > > generic functions, _bfd_read_unsigned_leb128 and > _bfd_write_unsigned_leb128, > > to encode the uleb128 into the section contents. > > (riscv_elf_relocate_section): Make sure that the > R_RISCV_SET_ULEB128 > > must be paired with and be placed before the R_RISCV_SUB_ULEB128. > > * elfxx-riscv.c (howto_table): Added R_RISCV_SUB_ULEB128 and > > R_RISCV_SET_ULEB128. > > (riscv_reloc_map): Likewise. > > (riscv_elf_ignore_reloc): New function. > > * libbfd.h: Regenerated. > > * reloc.c (BFD_RELOC_RISCV_SET_ULEB128, > BFD_RELOC_RISCV_SUB_ULEB128): > > New relocations to support .uleb128 subtraction. > > gas/ > > * config/tc-riscv.c (md_apply_fix): Added > BFD_RELOC_RISCV_SET_ULEB128 > > and BFD_RELOC_RISCV_SUB_ULEB128. > > (s_riscv_leb128): Updated to allow uleb128 subtraction. > > (riscv_insert_uleb128_fixes): New function, scan uleb128 > subtraction > > expressions and insert fixups for them. > > (riscv_md_finish): Called riscv_insert_uleb128_fixes for all > sections. > > include/ > > * elf/riscv.h ((R_RISCV_SET_ULEB128, (R_RISCV_SUB_ULEB128): > Defined. > > ld/ > > * testsuite/ld-riscv-elf/ld-riscv-elf.exp: Updated. > > * testsuite/ld-riscv-elf/uleb128*: New testcase for uleb128 > subtraction. > > binutils/ > > * testsuite/binutils-all/nm.exp: Updated since RISCV supports > .uleb128. > > --- > > bfd/bfd-in2.h | 2 + > > bfd/elfnn-riscv.c | 83 ++++++++++++++++++++++ > > bfd/elfxx-riscv.c | 54 ++++++++++++++ > > bfd/libbfd.h | 2 + > > bfd/reloc.c | 4 ++ > > binutils/testsuite/binutils-all/nm.exp | 2 - > > gas/config/tc-riscv.c | 56 ++++++++++++++- > > include/elf/riscv.h | 3 + > > ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp | 1 + > > ld/testsuite/ld-riscv-elf/uleb128.d | 18 +++++ > > ld/testsuite/ld-riscv-elf/uleb128.s | 18 +++++ > > 11 files changed, 239 insertions(+), 4 deletions(-) > > create mode 100644 ld/testsuite/ld-riscv-elf/uleb128.d > > create mode 100644 ld/testsuite/ld-riscv-elf/uleb128.s > > > > diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h > > index 7be18db20a8..363a6b35a5c 100644 > > --- a/bfd/bfd-in2.h > > +++ b/bfd/bfd-in2.h > > @@ -5394,6 +5394,8 @@ number for the SBIC, SBIS, SBI and CBI > instructions */ > > BFD_RELOC_RISCV_SET16, > > BFD_RELOC_RISCV_SET32, > > BFD_RELOC_RISCV_32_PCREL, > > + BFD_RELOC_RISCV_SET_ULEB128, > > + BFD_RELOC_RISCV_SUB_ULEB128, > > > > /* Renesas RL78 Relocations. */ > > BFD_RELOC_RL78_NEG8, > > diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c > > index a23b91ac15c..75af040cf92 100644 > > --- a/bfd/elfnn-riscv.c > > +++ b/bfd/elfnn-riscv.c > > @@ -1792,6 +1792,55 @@ perform_relocation (const reloc_howto_type *howto, > > value = ENCODE_CITYPE_LUI_IMM (RISCV_CONST_HIGH_PART (value)); > > break; > > > > + /* SUB_ULEB128 must be applied after SET_ULEB128, so we only write > the > > + value back for SUB_ULEB128 should be enough. */ > > + case R_RISCV_SET_ULEB128: > > + break; > > + case R_RISCV_SUB_ULEB128: > > + { > > + unsigned int len = 0; > > + _bfd_read_unsigned_leb128 (input_bfd, contents + rel->r_offset, > &len); > > + > > + /* Clean the contents value to zero (0x80), but keep the original > > + length. */ > > + bfd_byte *p = contents + rel->r_offset; > > + bfd_byte *endp = p + len - 1; > > + memset (p, 0x80, len - 1); > > + *(endp) = 0; > > + > > + /* Make sure the length of the new uleb128 value within the > > + original (available) length. */ > > + unsigned int new_len = 0; > > + unsigned int val_t = value; > > + do > > + { > > + new_len++; > > + val_t >>= 7; > > + } > > + while (val_t); > > + if (new_len > len) > > + { > > + _bfd_error_handler > > + (_("final size of uleb128 value at offset 0x%lx in %pA > from " > > + "%pB exceeds available space"), > > + (long) rel->r_offset, input_section, input_bfd); > > + return bfd_reloc_dangerous; > > + } > > + else > > + { > > + p = _bfd_write_unsigned_leb128 (p, endp, value); > > + BFD_ASSERT (p); > > + > > + /* If the length of the value is reduced and shorter than the > > + original uleb128 length, then _bfd_write_unsigned_leb128 > may > > + clear the 0x80 to 0x0 for the last byte that was written. > > + So reset it to keep the the original uleb128 length. */ > > + if (--p < endp) > > + *p |= 0x80; > > + } > > + return bfd_reloc_ok; > > + } > > + > > case R_RISCV_32: > > case R_RISCV_64: > > case R_RISCV_ADD8: > > @@ -2086,6 +2135,8 @@ riscv_elf_relocate_section (bfd *output_bfd, > > Elf_Internal_Shdr *symtab_hdr = &elf_symtab_hdr (input_bfd); > > struct elf_link_hash_entry **sym_hashes = elf_sym_hashes (input_bfd); > > bfd_vma *local_got_offsets = elf_local_got_offsets (input_bfd); > > + bfd_vma uleb128_set_vma = 0; > > + Elf_Internal_Rela *uleb128_set_rel = NULL; > > bool absolute; > > > > if (!riscv_init_pcrel_relocs (&pcrel_relocs)) > > @@ -2427,6 +2478,38 @@ riscv_elf_relocate_section (bfd *output_bfd, > > /* These require no special handling beyond > perform_relocation. */ > > break; > > > > + case R_RISCV_SET_ULEB128: > > + if (uleb128_set_rel == NULL) > > + { > > + /* Saved for later usage. */ > > + uleb128_set_vma = relocation; > > + uleb128_set_rel = rel; > > + continue; > > + } > > + else > > + { > > + msg = ("Mismatched R_RISCV_SET_ULEB128, it must be paired > with" > > + "and applied before R_RISCV_SUB_ULEB128"); > > + r = bfd_reloc_dangerous; > > + } > > + break; > > + > > + case R_RISCV_SUB_ULEB128: > > + if (uleb128_set_rel != NULL > > + && uleb128_set_rel->r_offset == rel->r_offset) > > + { > > + relocation = uleb128_set_vma - relocation; > > + uleb128_set_vma = 0; > > + uleb128_set_rel = NULL; > > + } > > + else > > + { > > + msg = ("Mismatched R_RISCV_SUB_ULEB128, it must be paired > with" > > + "and applied after R_RISCV_SET_ULEB128"); > > + r = bfd_reloc_dangerous; > > + } > > + break; > > + > > case R_RISCV_GOT_HI20: > > if (h != NULL) > > { > > diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c > > index bd0c7c81329..7f453246449 100644 > > --- a/bfd/elfxx-riscv.c > > +++ b/bfd/elfxx-riscv.c > > @@ -38,6 +38,8 @@ > > relocations for the debug info. */ > > static bfd_reloc_status_type riscv_elf_add_sub_reloc > > (bfd *, arelent *, asymbol *, void *, asection *, bfd *, char **); > > +static bfd_reloc_status_type riscv_elf_ignore_reloc > > + (bfd *, arelent *, asymbol *, void *, asection *, bfd *, char **); > > > > /* The relocation table used for SHT_RELA sections. */ > > > > @@ -844,6 +846,39 @@ static reloc_howto_type howto_table[] = > > 0, /* src_mask */ > > 0xffffffff, /* dst_mask */ > > false), /* pcrel_offset */ > > + > > + /* Reserved for R_RISCV_PLT32. */ > > + EMPTY_HOWTO (59), > > + > > + /* N-bit in-place setting, for unsigned-leb128 local label > subtraction. */ > > + HOWTO (R_RISCV_SET_ULEB128, /* type */ > > + 0, /* rightshift */ > > + 0, /* size */ > > + 0, /* bitsize */ > > + false, /* pc_relative */ > > + 0, /* bitpos */ > > + complain_overflow_dont, /* complain_on_overflow */ > > + riscv_elf_ignore_reloc, /* special_function */ > > + "R_RISCV_SET_ULEB128", /* name */ > > + false, /* partial_inplace */ > > + 0, /* src_mask */ > > + 0, /* dst_mask */ > > + false), /* pcrel_offset */ > > + > > + /* N-bit in-place addition, for unsigned-leb128 local label > subtraction. */ > > + HOWTO (R_RISCV_SUB_ULEB128, /* type */ > > + 0, /* rightshift */ > > + 0, /* size */ > > + 0, /* bitsize */ > > + false, /* pc_relative */ > > + 0, /* bitpos */ > > + complain_overflow_dont, /* complain_on_overflow */ > > + riscv_elf_ignore_reloc, /* special_function */ > > + "R_RISCV_SUB_ULEB128", /* name */ > > + false, /* partial_inplace */ > > + 0, /* src_mask */ > > + 0, /* dst_mask */ > > + false), /* pcrel_offset */ > > }; > > > > /* A mapping from BFD reloc types to RISC-V ELF reloc types. */ > > @@ -905,6 +940,8 @@ static const struct elf_reloc_map riscv_reloc_map[] = > > { BFD_RELOC_RISCV_SET16, R_RISCV_SET16 }, > > { BFD_RELOC_RISCV_SET32, R_RISCV_SET32 }, > > { BFD_RELOC_RISCV_32_PCREL, R_RISCV_32_PCREL }, > > + { BFD_RELOC_RISCV_SET_ULEB128, R_RISCV_SET_ULEB128 }, > > + { BFD_RELOC_RISCV_SUB_ULEB128, R_RISCV_SUB_ULEB128 }, > > }; > > > > /* Given a BFD reloc type, return a howto structure. */ > > @@ -1010,6 +1047,23 @@ riscv_elf_add_sub_reloc (bfd *abfd, > > return bfd_reloc_ok; > > } > > > > +/* Special handler for relocations which don't have to be relocated. > > + This function just simply return bfd_reloc_ok. */ > > + > > +static bfd_reloc_status_type > > +riscv_elf_ignore_reloc (bfd *abfd ATTRIBUTE_UNUSED, > > + arelent *reloc_entry, > > + asymbol *symbol ATTRIBUTE_UNUSED, > > + void *data ATTRIBUTE_UNUSED, > > + asection *input_section, > > + bfd *output_bfd, > > + char **error_message ATTRIBUTE_UNUSED) > > +{ > > + if (output_bfd != NULL) > > + reloc_entry->address += input_section->output_offset; > > + return bfd_reloc_ok; > > +} > > + > > /* Always add the IMPLICIT for the SUBSET. */ > > > > static bool > > diff --git a/bfd/libbfd.h b/bfd/libbfd.h > > index 05508c986ad..f29a65a4b22 100644 > > --- a/bfd/libbfd.h > > +++ b/bfd/libbfd.h > > @@ -2417,6 +2417,8 @@ static const char *const > bfd_reloc_code_real_names[] = { "@@uninitialized@@", > > "BFD_RELOC_RISCV_SET16", > > "BFD_RELOC_RISCV_SET32", > > "BFD_RELOC_RISCV_32_PCREL", > > + "BFD_RELOC_RISCV_SET_ULEB128", > > + "BFD_RELOC_RISCV_SUB_ULEB128", > > "BFD_RELOC_RL78_NEG8", > > "BFD_RELOC_RL78_NEG16", > > "BFD_RELOC_RL78_NEG24", > > diff --git a/bfd/reloc.c b/bfd/reloc.c > > index aab5d49bdb3..c7d11cf5f40 100644 > > --- a/bfd/reloc.c > > +++ b/bfd/reloc.c > > @@ -5048,6 +5048,10 @@ ENUMX > > BFD_RELOC_RISCV_SET32 > > ENUMX > > BFD_RELOC_RISCV_32_PCREL > > +ENUMX > > + BFD_RELOC_RISCV_SET_ULEB128 > > +ENUMX > > + BFD_RELOC_RISCV_SUB_ULEB128 > > ENUMDOC > > RISC-V relocations. > > > > diff --git a/binutils/testsuite/binutils-all/nm.exp > b/binutils/testsuite/binutils-all/nm.exp > > index d0f4c3e275c..91b519d9074 100644 > > --- a/binutils/testsuite/binutils-all/nm.exp > > +++ b/binutils/testsuite/binutils-all/nm.exp > > @@ -246,8 +246,6 @@ if [is_elf_format] { > > # Test nm --line-numbers on DWARF-4 debug info. > > set testname "nm --line-numbers on DWARF-4 debug info" > > > > - # The RISCV target does not (currently) support .uleb128. > > - setup_xfail "riscv*-*-*" > > # The SH targets complain that the pseudo-ops used to construct > > # the DWARF data are misaligned. > > setup_xfail "sh*-*-*" > > diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c > > index 0cc2484b049..fceb53e54a6 100644 > > --- a/gas/config/tc-riscv.c > > +++ b/gas/config/tc-riscv.c > > @@ -3950,6 +3950,9 @@ md_apply_fix (fixS *fixP, valueT *valP, segT seg > ATTRIBUTE_UNUSED) > > case BFD_RELOC_RISCV_SUB32: > > case BFD_RELOC_RISCV_SUB64: > > case BFD_RELOC_RISCV_RELAX: > > + /* cvt_frag_to_fill () has called output_leb128 (). */ > > + case BFD_RELOC_RISCV_SET_ULEB128: > > + case BFD_RELOC_RISCV_SUB_ULEB128: > > break; > > > > case BFD_RELOC_RISCV_TPREL_HI20: > > @@ -4714,8 +4717,11 @@ s_riscv_leb128 (int sign) > > char *save_in = input_line_pointer; > > > > expression (&exp); > > - if (exp.X_op != O_constant) > > - as_bad (_("non-constant .%cleb128 is not supported"), sign ? 's' : > 'u'); > > + if (sign && exp.X_op != O_constant) > > + as_bad (_("non-constant .sleb128 is not supported")); > > + else if (!sign && exp.X_op != O_constant && exp.X_op != O_subtract) > > + as_bad (_(".uleb128 only supports constant or subtract > expressions")); > > + > > demand_empty_rest_of_line (); > > > > input_line_pointer = save_in; > > @@ -4835,12 +4841,58 @@ riscv_set_public_attributes (void) > > riscv_write_out_attrs (); > > } > > > > +/* Scan uleb128 subtraction expressions and insert fixups for them. > > + e.g., .uleb128 .L1 - .L0 > > + Because relaxation may change the value of the subtraction, we > > + must resolve them at link-time. */ > > + > > +static void > > +riscv_insert_uleb128_fixes (bfd *abfd ATTRIBUTE_UNUSED, > > + asection *sec, void *xxx ATTRIBUTE_UNUSED) > > +{ > > + segment_info_type *seginfo = seg_info (sec); > > + struct frag *fragP; > > + > > + subseg_set (sec, 0); > > + > > + for (fragP = seginfo->frchainP->frch_root; > > + fragP; fragP = fragP->fr_next) > > + { > > + expressionS *exp, *exp_dup; > > + > > + if (fragP->fr_type != rs_leb128 || fragP->fr_symbol == NULL) > > + continue; > > + > > + exp = symbol_get_value_expression (fragP->fr_symbol); > > + > > + if (exp->X_op != O_subtract) > > + continue; > > + > > + /* Only unsigned leb128 can be handled. */ > > + gas_assert (fragP->fr_subtype == 0); > > + exp_dup = xmemdup (exp, sizeof (*exp), sizeof (*exp)); > > + exp_dup->X_op = O_symbol; > > + exp_dup->X_op_symbol = NULL; > > + > > + /* Insert relocations to resolve the subtraction at link-time. > > + Emit the SET relocation first in riscv. */ > > + exp_dup->X_add_symbol = exp->X_add_symbol; > > + fix_new_exp (fragP, fragP->fr_fix, 0, > > + exp_dup, 0, BFD_RELOC_RISCV_SET_ULEB128); > > + exp_dup->X_add_symbol = exp->X_op_symbol; > > + fix_new_exp (fragP, fragP->fr_fix, 0, > > + exp_dup, 0, BFD_RELOC_RISCV_SUB_ULEB128); > > + } > > +} > > + > > /* Called after all assembly has been done. */ > > > > void > > riscv_md_finish (void) > > { > > riscv_set_public_attributes (); > > + if (riscv_opts.relax) > > + bfd_map_over_sections (stdoutput, riscv_insert_uleb128_fixes, NULL); > > } > > > > /* Adjust the symbol table. */ > > diff --git a/include/elf/riscv.h b/include/elf/riscv.h > > index aabc71cf979..0aa8b3359c4 100644 > > --- a/include/elf/riscv.h > > +++ b/include/elf/riscv.h > > @@ -87,6 +87,9 @@ START_RELOC_NUMBERS (elf_riscv_reloc_type) > > RELOC_NUMBER (R_RISCV_SET32, 56) > > RELOC_NUMBER (R_RISCV_32_PCREL, 57) > > RELOC_NUMBER (R_RISCV_IRELATIVE, 58) > > + /* Reserved 59 for R_RISCV_PLT32. */ > > + RELOC_NUMBER (R_RISCV_SET_ULEB128, 60) > > + RELOC_NUMBER (R_RISCV_SUB_ULEB128, 61) > > END_RELOC_NUMBERS (R_RISCV_max) > > > > /* Processor specific flags for the ELF header e_flags field. */ > > diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp > b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp > > index 9e103b283f7..947a266ba72 100644 > > --- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp > > +++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp > > @@ -172,6 +172,7 @@ if [istarget "riscv*-*-*"] { > > run_dump_test "attr-merge-priv-spec-failed-06" > > run_dump_test "attr-phdr" > > run_dump_test "relax-max-align-gp" > > + run_dump_test "uleb128" > > run_ld_link_tests [list \ > > [list "Weak reference 32" "-T weakref.ld > -m[riscv_choose_ilp32_emul]" "" \ > > "-march=rv32i -mabi=ilp32" {weakref32.s} \ > > diff --git a/ld/testsuite/ld-riscv-elf/uleb128.d > b/ld/testsuite/ld-riscv-elf/uleb128.d > > new file mode 100644 > > index 00000000000..a921478e988 > > --- /dev/null > > +++ b/ld/testsuite/ld-riscv-elf/uleb128.d > > @@ -0,0 +1,18 @@ > > +#source: uleb128.s > > +#as: -march=rv32ic > > +#ld: -melf32lriscv > > +#objdump: -d > > + > > +.*:[ ]+file format .* > > + > > +Disassembly of section .text: > > + > > +.* <_start>: > > +.*jal.* > > +.*jal.* > > +.*jal.* > > +.*jal.* > > +.*jal.* > > +.*jal.* > > +.*:[ ]+0e0c.* > > +#pass > > diff --git a/ld/testsuite/ld-riscv-elf/uleb128.s > b/ld/testsuite/ld-riscv-elf/uleb128.s > > new file mode 100644 > > index 00000000000..f7d23be1635 > > --- /dev/null > > +++ b/ld/testsuite/ld-riscv-elf/uleb128.s > > @@ -0,0 +1,18 @@ > > +.text > > +.globl bar > > +.globl _start > > +.option rvc > > +.align 2 > > +_start: > > +.L0: > > + .rept 6 > > + call bar > > + .endr > > +.align 2 > > +.L1: > > + .uleb128 .L1 - .L0 > > + .uleb128 .L2 - .L0 > > +.L2: > > +.align 2 > > +bar: > > + nop > > -- > > 2.39.2 (Apple Git-143) > > >