[PATCH] [RISCV] Support subtraction of .uleb128.

Kito Cheng kito.cheng@gmail.com
Fri Sep 25 07:14:01 GMT 2020


Hi Jim, Palmer:

ping, I think this patch should be ready to land after rebase and
adjust reloc number?

On Wed, Jan 22, 2020 at 3:26 PM Kuan-Lin Chen <kuanlinchentw@gmail.com> wrote:
>
> Hi,
>
> This version patch updates as follows:
> Fix the warning about uninitialized variable.
> Set a limit to .uleb128 expressions as Palmer's suggestion.
> Thanks.
>
> Palmer Dabbelt <palmerdabbelt@google.com> 於 2020年1月10日 週五 上午7:19寫道:
> >
> > On Mon, 09 Dec 2019 21:42:58 PST (-0800), kuanlinchentw@gmail.com wrote:
> > > Hi Palmer,
> > >
> > >> GCC gives me -Wmaybe-uninitialized for both of these.
> > > May I know your gcc version?
> > > I tried gcc-7.3.0 and gcc-8.2.0, and I didn't get the warning.
> >
> > $ gcc --version
> > gcc (Debian 8.3.0-6) 8.3.0
> > Copyright (C) 2018 Free Software Foundation, Inc.
> > This is free software; see the source for copying conditions.  There is NO
> > warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> >
> > >> I don't understand how this could possibly work: uleb128s can be long, so we
> > >> can't just treat them as a single byte.
> > > Yes, uleb128s can be long so I implement riscv_elf_ignore_reloc to
> > > ignore the overflow checking.
> > > And I relocate uleb128s by myself in perform_relocation.
> > >
> > >> I'm having trouble testing it, though,
> > >> as I can't figure out how to emit R_RISCV_SET_ULEB128 alone.  For example, when
> > >> I try to insert a uleb128 in the assembler I'm not even getting a relocation:
> > > .uleb128 only support subtract format. (.uleb128 .L1 - .L0)
> > > It is impossible to get final the value and the length for ".uleb128
> > > symbol" in assemble-time.
> >
> > Well, you know symbols are at most 64-bit on rv64 and 32-bit on rv32, so you
> > can just emit enough space to encode any symbol.  That's better than emitting 7
> > bits, which is unlikely to encode any symbol.
> >
> > > I think ".uleb128 symbol" is meaningless.
> > > Therefore, all targets just fill the value as assemble-time value
> > > without relocations.
> >
> > If the assembler can't handle something then it needs to provide an error, it
> > can't just generate incorrect code.
> >
> > >>I can get a R_RISCV_SUB_ULEB128, but it appears to be both producing the wrong
> > >>value and garbaging up bytes after the relocation.  For example:
> > >
> > > I get the same result in the object file but the different result in executable.
> > > pal.o:     file format elf64-littleriscv
> > >
> > > Disassembly of section .text:
> > >
> > > 0000000000000000 <_start>:
> > >    0:   00000013                nop
> > >
> > > 0000000000000004 <.L0>:
> > >    4:   00000013                nop
> > >
> > > 0000000000000008 <uleb_l1_l0>:
> > >    8:   00100013                li      zero,1
> > >    c:   aa04                    fsd     fs1,16(a2)
> > >                         c: R_RISCV_SET_ULEB128  .L1
> > >                         c: R_RISCV_SUB_ULEB128  .L0
> > >    e:   aaaa                    fsd     fa0,336(sp)
> > >   10:   00200013                li      zero,2
> > >
> > >
> > > a.out:     file format elf64-littleriscv
> > >
> > >
> > > Disassembly of section .text:
> > >
> > > 0000000000010078 <_start>:
> > >    10078:       00000013                nop
> > >    1007c:       00000013                nop
> > >
> > > 0000000000010080 <uleb_l1_l0>:
> > >    10080:       00100013                li      zero,1
> > >    10084:       aa04                    fsd     fs1,16(a2)
> > >    10086:       aaaa                    fsd     fa0,336(sp)
> > >    10088:       00200013                li      zero,2
> > >
> > > Maybe there is something mismatched. But I don't have any idea currently.
> > > Could you please give me more information about your flow?
> >
> > IIRC I just applied the patch on top of whatever was HEAD of binutils-gdb at
> > the time, but I don't have the working directory around any more.
> >
> > > Thanks.
> > >
> > >> Presumably that's why I can't get a _SET alone.
> > > I think it's meaningless to use _SET alone in assembly.
> > >
> > >> It looks like alignment is broken here after uleb128s, but it's not actually
> > >> triggering an error in all cases.  Having two of them in this test case doesn't
> > >> trigger the issue, but an odd number does.  For example, the following source
> > > I think this alignment issue can be fixed by the following patch:
> > > https://sourceware.org/ml/binutils/2019-12/msg00024.html
> >
> > Ya, thanks -- I think there's some issues there as well, IIRC I commented on
> > it.
> >
> > >
> > > Thanks for your review.
> > >
> > > Palmer Dabbelt <palmerdabbelt@google.com> 於 2019年12月7日 週六 上午7:47寫道:
> > >>
> > >> On Wed, 27 Nov 2019 00:11:50 PST (-0800), kuanlinchentw@gmail.com wrote:
> > >> > The data length of uleb128 is variable.  So linker must recalculate the
> > >> > value of the subtraction.  The patch leave relocations in object files
> > >> > so that linker can relocate again after relaxation.
> > >> >
> > >> > bfd/ChangeLog:
> > >> > * bfd-in2.h: Regenerated.
> > >> > * elfnn-riscv.c (write_uleb128): New function.
> > >> > (perform_relocation): Perform R_RISCV_SUB_ULEB128 and
> > >> > R_RISCV_SET_ULEB128 relocation.
> > >> > (riscv_elf_relocate_section): Likewise.
> > >> > * elfxx-riscv.c (howto_table): Add R_RISCV_SUB_ULEB128 and
> > >> > R_RISCV_SET_ULEB128.
> > >> > (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.
> > >> >
> > >> > gas/ChangeLog:
> > >> > * config/tc-riscv.c (md_apply_fix): Add BFD_RELOC_RISCV_SET_ULEB128 and
> > >> > BFD_RELOC_RISCV_SUB_ULEB128.
> > >> > (riscv_insert_uleb128_fixes): New function.
> > >> > (riscv_md_end): Scan rs_leb128 fragments.
> > >> > (riscv_pseudo_table): Remove uleb128.
> > >> >
> > >> > include/ChangeLog:
> > >> > * elf/riscv.h ((R_RISCV_SET_ULEB128, (R_RISCV_SUB_ULEB128): Define.
> > >> >
> > >> > ld/ChangeLog:
> > >> > * testsuite/ld-riscv-elf/ld-riscv-elf.exp: Add uleb128.
> > >> > * testsuite/ld-riscv-elf/uleb128.d: New test.
> > >> > * testsuite/ld-riscv-elf/uleb128.s: New file.
> > >> >
> > >> > OK to commit?
> > >>
> > >> I'm having a lot of trouble trying to figure out how this works.  LMK if I'm
> > >> missing something, there's some comments in line:
> > >>
> > >> > From 9f7f0aaa484dee2b32c550f8cca4de18959f01e1 Mon Sep 17 00:00:00 2001
> > >> > From: Kuan-Lin Chen <rufus@andestech.com>
> > >> > Date: Thu, 14 Nov 2019 14:24:22 +0800
> > >> > Subject: [PATCH] RISC-V: Support subtraction of .uleb128.
> > >> >
> > >> > The data length of uleb128 is variable.  So linker must recalculate the
> > >> > value of the subtraction.  The patch leave relocations in object files
> > >> > so that linker can relocate again after relaxation.
> > >> >
> > >> > bfd/ChangeLog:
> > >> >         * bfd-in2.h: Regenerated.
> > >> >         * elfnn-riscv.c (write_uleb128): New function.
> > >> >         (perform_relocation): Perform R_RISCV_SUB_ULEB128 and
> > >> >          R_RISCV_SET_ULEB128 relocation.
> > >> >         (riscv_elf_relocate_section): Likewise.
> > >> >         * elfxx-riscv.c (howto_table): Add R_RISCV_SUB_ULEB128 and
> > >> >         R_RISCV_SET_ULEB128.
> > >> >         (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.
> > >> >
> > >> > gas/ChangeLog:
> > >> >         * config/tc-riscv.c (md_apply_fix): Add BFD_RELOC_RISCV_SET_ULEB128 and
> > >> >         BFD_RELOC_RISCV_SUB_ULEB128.
> > >> >         (riscv_insert_uleb128_fixes): New function.
> > >> >         (riscv_md_end): Scan rs_leb128 fragments.
> > >> >         (riscv_pseudo_table): Remove uleb128.
> > >> >
> > >> > include/ChangeLog:
> > >> >         * elf/riscv.h ((R_RISCV_SET_ULEB128, (R_RISCV_SUB_ULEB128): Define.
> > >> >
> > >> > ld/ChangeLog:
> > >> >         * testsuite/ld-riscv-elf/ld-riscv-elf.exp: Add uleb128.
> > >> >         * testsuite/ld-riscv-elf/uleb128.d: New test.
> > >> >         * testsuite/ld-riscv-elf/uleb128.s: New file.
> > >> > ---
> > >> >  bfd/ChangeLog                              | 14 ++++
> > >> >  bfd/bfd-in2.h                              |  2 +
> > >> >  bfd/elfnn-riscv.c                          | 82 ++++++++++++++++++++++
> > >> >  bfd/elfxx-riscv.c                          | 51 ++++++++++++++
> > >> >  bfd/libbfd.h                               |  2 +
> > >> >  bfd/reloc.c                                |  4 ++
> > >> >  gas/ChangeLog                              |  8 +++
> > >> >  gas/config/tc-riscv.c                      | 47 ++++++++++++-
> > >> >  include/ChangeLog                          |  4 ++
> > >> >  include/elf/riscv.h                        |  2 +
> > >> >  ld/ChangeLog                               |  6 ++
> > >> >  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 +++++
> > >> >  14 files changed, 258 insertions(+), 1 deletion(-)
> > >> >  create mode 100644 ld/testsuite/ld-riscv-elf/uleb128.d
> > >> >  create mode 100644 ld/testsuite/ld-riscv-elf/uleb128.s
> > >> >
> > >> > diff --git a/bfd/ChangeLog b/bfd/ChangeLog
> > >> > index 4a0852e577..d11ffde74c 100644
> > >> > --- a/bfd/ChangeLog
> > >> > +++ b/bfd/ChangeLog
> > >> > @@ -1,3 +1,17 @@
> > >> > +2019-11-27  Kuan-Lin Chen  <kuanlinchentw@gmail.com>
> > >> > +
> > >> > +        * bfd-in2.h: Regenerated.
> > >> > +        * elfnn-riscv.c (write_uleb128): New function.
> > >> > +        (perform_relocation): Perform R_RISCV_SUB_ULEB128 and
> > >> > +         R_RISCV_SET_ULEB128 relocation.
> > >> > +        (riscv_elf_relocate_section): Likewise.
> > >> > +        * elfxx-riscv.c (howto_table): Add R_RISCV_SUB_ULEB128 and
> > >> > +        R_RISCV_SET_ULEB128.
> > >> > +        (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.
> > >> > +
> > >> >  2019-11-27  Alan Modra  <amodra@gmail.com>
> > >> >
> > >> >          PR 23652
> > >> > diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
> > >> > index 44902fc8d0..a32708ebc2 100644
> > >> > --- a/bfd/bfd-in2.h
> > >> > +++ b/bfd/bfd-in2.h
> > >> > @@ -4380,6 +4380,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 997f786602..075ef6b82d 100644
> > >> > --- a/bfd/elfnn-riscv.c
> > >> > +++ b/bfd/elfnn-riscv.c
> > >> > @@ -1419,6 +1419,26 @@ riscv_global_pointer_value (struct bfd_link_info *info)
> > >> >    return h->u.def.value + sec_addr (h->u.def.section);
> > >> >  }
> > >> >
> > >> > +/* Write VAL in uleb128 format to P, returning a pointer to the
> > >> > +   following byte.
> > >> > +   This code is copied from elf-attr.c.  */
> > >> > +
> > >> > +static bfd_byte *
> > >> > +write_uleb128 (bfd_byte *p, unsigned int val)
> > >> > +{
> > >> > +  bfd_byte c;
> > >> > +  do
> > >> > +    {
> > >> > +      c = val & 0x7f;
> > >> > +      val >>= 7;
> > >> > +      if (val)
> > >> > +        c |= 0x80;
> > >> > +      *(p++) = c;
> > >> > +    }
> > >> > +  while (val);
> > >> > +  return p;
> > >> > +}
> > >> > +
> > >> >  /* Emplace a static relocation.  */
> > >> >
> > >> >  static bfd_reloc_status_type
> > >> > @@ -1512,6 +1532,25 @@ perform_relocation (const reloc_howto_type *howto,
> > >> >          value = ENCODE_RVC_LUI_IMM (RISCV_CONST_HIGH_PART (value));
> > >> >        break;
> > >> >
> > >> > +    case R_RISCV_SET_ULEB128:
> > >> > +    case R_RISCV_SUB_ULEB128:
> > >> > +      {
> > >> > +        unsigned int len = 0;
> > >> > +        bfd_byte *endp, *p;
> > >> > +
> > >> > +        _bfd_read_unsigned_leb128 (input_bfd, contents + rel->r_offset, &len);
> > >> > +
> > >> > +        /* Clean the contents value to zero.  Do not reduce the length.  */
> > >> > +        p = contents + rel->r_offset;
> > >> > +        endp = p + len -1;
> > >> > +        memset (p, 0x80, len);
> > >> > +        *(endp) = 0;
> > >> > +        p = write_uleb128 (p, value) - 1;
> > >> > +        if (p < endp)
> > >> > +          *p |= 0x80;
> > >> > +        return bfd_reloc_ok;
> > >> > +      }
> > >> > +
> > >> >      case R_RISCV_32:
> > >> >      case R_RISCV_64:
> > >> >      case R_RISCV_ADD8:
> > >> > @@ -1767,6 +1806,8 @@ riscv_elf_relocate_section (bfd *output_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_boolean absolute;
> > >> > +  bfd_vma uleb128_vma;
> > >> > +  Elf_Internal_Rela *uleb128_rel = NULL;
> > >> >
> > >> >    if (!riscv_init_pcrel_relocs (&pcrel_relocs))
> > >> >      return FALSE;
> > >> > @@ -1871,6 +1912,47 @@ riscv_elf_relocate_section (bfd *output_bfd,
> > >> >          case R_RISCV_DELETE:
> > >> >            /* These require no special handling beyond perform_relocation.  */
> > >> >            break;
> > >> > +        case R_RISCV_SET_ULEB128:
> > >> > +          if (!uleb128_rel)
> > >> > +            {
> > >> > +              /* Save the minuend to use later.  */
> > >> > +              uleb128_vma = relocation;
> > >> > +              uleb128_rel = rel;
> > >> > +              continue;
> > >> > +            }
> > >> > +          else
> > >> > +            {
> > >> > +              if (uleb128_rel->r_offset != rel->r_offset)
> > >> > +                {
> > >> > +                  (*_bfd_error_handler) (_("%pB: relocation %s mismatched. "),
> > >> > +                                         input_bfd, howto->name);
> > >> > +                  bfd_set_error (bfd_error_bad_value);
> > >> > +                }
> > >> > +              relocation = relocation - uleb128_vma;
> > >> > +              uleb128_rel = NULL;
> > >> > +              break;
> > >> > +            }
> > >> > +
> > >> > +        case R_RISCV_SUB_ULEB128:
> > >> > +          if (uleb128_rel)
> > >> > +            {
> > >> > +              if (uleb128_rel->r_offset != rel->r_offset)
> > >> > +                {
> > >> > +                  (*_bfd_error_handler) (_("%pB: relocation %s mismatched. "),
> > >> > +                                         input_bfd, howto->name);
> > >> > +                  bfd_set_error (bfd_error_bad_value);
> > >> > +                }
> > >> > +              relocation = uleb128_vma - relocation;
> > >>
> > >> GCC gives me -Wmaybe-uninitialized for both of these.
> > >>
> > >> > +              uleb128_rel = NULL;
> > >> > +              break;
> > >> > +            }
> > >> > +          else
> > >> > +            {
> > >> > +              /* Save the subtrahend to use later.  */
> > >> > +              uleb128_vma = relocation;
> > >> > +              uleb128_rel = rel;
> > >> > +              continue;
> > >> > +            }
> > >> >
> > >> >          case R_RISCV_GOT_HI20:
> > >> >            if (h != NULL)
> > >> > diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> > >> > index 245717f70f..c136ba2207 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.  */
> > >> >
> > >> > @@ -855,8 +857,41 @@ static reloc_howto_type howto_table[] =
> > >> >           0,                                /* src_mask */
> > >> >           MINUS_ONE,                        /* dst_mask */
> > >> >           FALSE),                        /* pcrel_offset */
> > >> > +
> > >> > +  /* The length of unsigned-leb128 is variant, just assume the
> > >> > +     size is one byte here.  */
> > >> > +  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 */
> > >>
> > >> I don't understand how this could possibly work: uleb128s can be long, so we
> > >> can't just treat them as a single byte.  I'm having trouble testing it, though,
> > >> as I can't figure out how to emit R_RISCV_SET_ULEB128 alone.  For example, when
> > >> I try to insert a uleb128 in the assembler I'm not even getting a relocation:
> > >>
> > >>     .text
> > >>     .globl _start
> > >>     _start:
> > >>             nop
> > >>     .L0:
> > >>             nop
> > >>     .L1:
> > >>     uleb_l0:
> > >>             addi x0, x0, 1
> > >>             .uleb128 .L0
> > >>             # Manually fix up the alignment, as .align is broken.
> > >>             .rept 3
> > >>             .byte 0xAA
> > >>             .endr
> > >>             addi x0, x0, 2
> > >>
> > >>     $ objdump -dr
> > >>     test.o:     file format elf64-littleriscv
> > >>
> > >>
> > >>     Disassembly of section .text:
> > >>
> > >>     0000000000000000 <_start>:
> > >>        0:   00000013                nop
> > >>        4:   00000013                nop
> > >>
> > >>     0000000000000008 <uleb_l0>:
> > >>        8:   00100013                li      zero,1
> > >>        c:   aa04                    fsd     fs1,16(a2)
> > >>        e:   aaaa                    fsd     fa0,336(sp)
> > >>       10:   00200013                li      zero,2
> > >>
> > >> so then when I link I'm just getting the wrong value entirely.
> > >>
> > >>     test:     file format elf64-littleriscv
> > >>
> > >>
> > >>     Disassembly of section .text:
> > >>
> > >>     0000000000010078 <_start>:
> > >>        10078:       00000013                nop
> > >>        1007c:       00000013                nop
> > >>
> > >>     0000000000010080 <uleb_l0>:
> > >>        10080:       00100013                li      zero,1
> > >>        10084:       aa04                    fsd     fs1,16(a2)
> > >>        10086:       aaaa                    fsd     fa0,336(sp)
> > >>        10088:       00200013                li      zero,2
> > >>
> > >> > +  /* The length of unsigned-leb128 is variant, just assume the
> > >> > +     size is one byte here.  */
> > >> > +  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 */
> > >> >  };
> > >>
> > >> I can get a R_RISCV_SUB_ULEB128, but it appears to be both producing the wrong
> > >> value and garbaging up bytes after the relocation.  For example:
> > >>
> > >>     .text
> > >>     .globl _start
> > >>     _start:
> > >>             nop
> > >>     .L0:
> > >>             nop
> > >>     .L1:
> > >>     uleb_l1_l0:
> > >>             addi x0, x0, 1
> > >>             .uleb128 .L1 - .L0
> > >>             # Manually fix up the alignment, as .align is broken
> > >>             .rept 3
> > >>             .byte 0xAA
> > >>             .endr
> > >>             addi x0, x0, 2
> > >>
> > >> produces the relocation
> > >>
> > >>     test.o:     file format elf64-littleriscv
> > >>
> > >>
> > >>     Disassembly of section .text:
> > >>
> > >>     0000000000000000 <_start>:
> > >>        0:   00000013                nop
> > >>
> > >>     0000000000000004 <.L0>:
> > >>        4:   00000013                nop
> > >>
> > >>     0000000000000008 <uleb_l1_l0>:
> > >>        8:   00100013                li      zero,1
> > >>        c:   aa04                    fsd     fs1,16(a2)
> > >>                             c: R_RISCV_SET_ULEB128  .L1
> > >>                             c: R_RISCV_SUB_ULEB128  .L0
> > >>        e:   aaaa                    fsd     fa0,336(sp)
> > >>       10:   00200013                li      zero,2
> > >>
> > >> but then goes and corrupts the resulting executable
> > >>
> > >>
> > >>     test:     file format elf64-littleriscv
> > >>     Disassembly of section .text:
> > >>
> > >>     0000000000010078 <_start>:
> > >>        10078:       00000013                nop
> > >>        1007c:       00000013                nop
> > >>
> > >>     0000000000010080 <uleb_l1_l0>:
> > >>        10080:       00100013                li      zero,1
> > >>        10084:       ff84                    sd      s1,56(a5)
> > >>        10086:       000ffffb                0xffffb
> > >>        1008a:       0020                    addi    s0,sp,8
> > >>
> > >> I can't think of a way to make this work aside from just emitting uleb128s at
> > >> their full length, which would make them work but would really defeat the
> > >> purpose of the format.  I guess we could then relax these, but that seems like
> > >> a lot of work.
> > >>
> > >> > +
> > >> >  /* A mapping from BFD reloc types to RISC-V ELF reloc types.  */
> > >> >
> > >> >  struct elf_reloc_map
> > >> > @@ -917,6 +952,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.  */
> > >> > @@ -1011,6 +1048,20 @@ 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;
> > >> > +}
> > >> > +
> > >> >  /* Parsing subset version.
> > >> >
> > >> >     Return Value:
> > >> > diff --git a/bfd/libbfd.h b/bfd/libbfd.h
> > >> > index 77b732ee4b..7aa8f91504 100644
> > >> > --- a/bfd/libbfd.h
> > >> > +++ b/bfd/libbfd.h
> > >> > @@ -2327,6 +2327,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 b00b79f319..9b8b75ad21 100644
> > >> > --- a/bfd/reloc.c
> > >> > +++ b/bfd/reloc.c
> > >> > @@ -5212,6 +5212,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/gas/ChangeLog b/gas/ChangeLog
> > >> > index 09991524da..e53ac0895b 100644
> > >> > --- a/gas/ChangeLog
> > >> > +++ b/gas/ChangeLog
> > >> > @@ -1,3 +1,11 @@
> > >> > +2019-11-27  Kuan-Lin Chen  <kuanlinchentw@gmail.com>
> > >> > +
> > >> > +        * config/tc-riscv.c (md_apply_fix): Add BFD_RELOC_RISCV_SET_ULEB128 and
> > >> > +        BFD_RELOC_RISCV_SUB_ULEB128.
> > >> > +        (riscv_insert_uleb128_fixes): New function.
> > >> > +        (riscv_md_end): Scan rs_leb128 fragments.
> > >> > +        (riscv_pseudo_table): Remove uleb128.
> > >> > +
> > >> >  2019-11-25  Andrew Pinski  <apinski@marvell.com>
> > >> >
> > >> >          * config/tc-aarch64.c (md_begin): Use correct
> > >> > diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> > >> > index e50505138e..c35c591c0a 100644
> > >> > --- a/gas/config/tc-riscv.c
> > >> > +++ b/gas/config/tc-riscv.c
> > >> > @@ -2392,6 +2392,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:
> > >> > @@ -3127,11 +3130,54 @@ riscv_set_public_attributes (void)
> > >> >      riscv_write_out_arch_attr ();
> > >> >  }
> > >> >
> > >> > +/* Scan uleb128 subtraction expressions and insert fixups for them.
> > >> > +   e.g., .uleb128 .L1 - .L0
> > >> > +   Becuase relaxation may change the value of the subtraction, we
> > >> > +   must resolve them in 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;
> > >>
> > >> Presumably that's why I can't get a _SET alone.
> > >>
> > >> > +
> > >> > +      /* Only unsigned leb128 can be handle.  */
> > >> > +      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 in link-time.  */
> > >> > +      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_end (void)
> > >> >  {
> > >> > +  bfd_map_over_sections (stdoutput, riscv_insert_uleb128_fixes, NULL);
> > >> >    riscv_set_public_attributes ();
> > >> >  }
> > >> >
> > >> > @@ -3215,7 +3261,6 @@ static const pseudo_typeS riscv_pseudo_table[] =
> > >> >    {"dtprelword", s_dtprel, 4},
> > >> >    {"dtpreldword", s_dtprel, 8},
> > >> >    {"bss", s_bss, 0},
> > >> > -  {"uleb128", s_riscv_leb128, 0},
> > >> >    {"sleb128", s_riscv_leb128, 1},
> > >> >    {"insn", s_riscv_insn, 0},
> > >> >    {"attribute", s_riscv_attribute, 0},
> > >> > diff --git a/include/ChangeLog b/include/ChangeLog
> > >> > index 47bb86cf71..c290b245c6 100644
> > >> > --- a/include/ChangeLog
> > >> > +++ b/include/ChangeLog
> > >> > @@ -1,3 +1,7 @@
> > >> > +2019-11-27  Kuan-Lin Chen  <kuanlinchentw@gmail.com>
> > >> > +
> > >> > +        * elf/riscv.h ((R_RISCV_SET_ULEB128, (R_RISCV_SUB_ULEB128): Define.
> > >> > +
> > >> >  2019-11-25  Alan Modra  <amodra@gmail.com>
> > >> >
> > >> >          * coff/ti.h (GET_SCNHDR_SIZE, PUT_SCNHDR_SIZE, GET_SCN_SCNLEN),
> > >> > diff --git a/include/elf/riscv.h b/include/elf/riscv.h
> > >> > index 2f98aa4a3e..030679b35f 100644
> > >> > --- a/include/elf/riscv.h
> > >> > +++ b/include/elf/riscv.h
> > >> > @@ -88,6 +88,8 @@ START_RELOC_NUMBERS (elf_riscv_reloc_type)
> > >> >    RELOC_NUMBER (R_RISCV_SET16, 55)
> > >> >    RELOC_NUMBER (R_RISCV_SET32, 56)
> > >> >    RELOC_NUMBER (R_RISCV_32_PCREL, 57)
> > >> > +  RELOC_NUMBER (R_RISCV_SET_ULEB128, 58)
> > >> > +  RELOC_NUMBER (R_RISCV_SUB_ULEB128, 59)
> > >> >  END_RELOC_NUMBERS (R_RISCV_max)
> > >> >
> > >> >  /* Processor specific flags for the ELF header e_flags field.  */
> > >> > diff --git a/ld/ChangeLog b/ld/ChangeLog
> > >> > index 969ab78035..5c2e406101 100644
> > >> > --- a/ld/ChangeLog
> > >> > +++ b/ld/ChangeLog
> > >> > @@ -1,3 +1,9 @@
> > >> > +2019-11-27  Kuan-Lin Chen  <kuanlinchentw@gmail.com>
> > >> > +
> > >> > +        * testsuite/ld-riscv-elf/ld-riscv-elf.exp: Add uleb128.
> > >> > +        * testsuite/ld-riscv-elf/uleb128.d: New test.
> > >> > +        * testsuite/ld-riscv-elf/uleb128.s: New file.
> > >> > +
> > >> >  2019-11-26  Martin Liska  <mliska@suse.cz>
> > >> >
> > >> >          * scripttempl/arclinux.sc: Add .text.sorted.* which is sorted
> > >> > diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> > >> > index 7aabbdd641..809e40f08d 100644
> > >> > --- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> > >> > +++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> > >> > @@ -38,6 +38,7 @@ if [istarget "riscv*-*-*"] {
> > >> >      run_dump_test "attr-merge-priv-spec"
> > >> >      run_dump_test "attr-merge-arch-failed-01"
> > >> >      run_dump_test "attr-merge-stack-align-failed"
> > >> > +    run_dump_test "uleb128"
> > >> >      run_ld_link_tests {
> > >> >          { "Weak reference 32" "-T weakref.ld -melf32lriscv" ""
> > >> >              "-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 0000000000..a921478e98
> > >> > --- /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.*<bar>
> > >> > +.*jal.*<bar>
> > >> > +.*jal.*<bar>
> > >> > +.*jal.*<bar>
> > >> > +.*jal.*<bar>
> > >> > +.*jal.*<bar>
> > >> > +.*:[         ]+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 0000000000..f7d23be163
> > >> > --- /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
> > >>
> > >> It looks like alignment is broken here after uleb128s, but it's not actually
> > >> triggering an error in all cases.  Having two of them in this test case doesn't
> > >> trigger the issue, but an odd number does.  For example, the following source
> > >>
> > >>     .text
> > >>     .globl _start
> > >>     .align 2
> > >>     _start:
> > >>             .uleb128 _start
> > >>     .align 2
> > >>     _end:
> > >>             nop
> > >>
> > >> links _end to a misaligned symbol
> > >>
> > >>     Disassembly of section .text:
> > >>
> > >>     0000000000010078 <_start>:
> > >>             ...
> > >>
> > >>     0000000000010079 <_end>:
> > >>        10079:       00000013                nop
> > >>        1007d:       0000                    unimp
> > >>             ...
> > >>
> > >> I'm assuming that's because norvc assumes 4-byte alignment in text sections, so
> > >> it's just eliding the alingment directives.  With RVC I do get an error message
> > >>
> > >>     .text
> > >>     .globl _start
> > >>     .option rvc
> > >>     .align 2
> > >>     _start:
> > >>             .uleb128 _start
> > >>     .align 2
> > >>     _end:
> > >>             nop
> > >>
> > >>     ./install/bin/riscv64-unknown-linux-gnu-ld: test.o(.text+0x1): 3 bytes required for alignment to 4-byte boundary, but only 2 present
> > >>     ./install/bin/riscv64-unknown-linux-gnu-ld: can't relax section: bad value
> > >>
> > >>
> > >
> > >
> > > --
> > > Best regards,
> > > Kuan-Lin Chen.
> > > kuanlinchentw@gmail.com
>
>
>
> --
> Best regards,
> Kuan-Lin Chen.
> kuanlinchentw@gmail.com


More information about the Binutils mailing list