From: "Palmer Dabbelt via binutils" <binutils@sourceware.org>
To: kuanlinchentw@gmail.com
Cc: binutils@sourceware.org
Subject: Re: [PATCH] [RISCV] Support subtraction of .uleb128.
Date: Thu, 09 Jan 2020 23:20:00 -0000 [thread overview]
Message-ID: <mhng-bf7b317b-75dc-4000-b583-9074021d73fe@palmerdabbelt-glaptop> (raw)
In-Reply-To: <CAJr6u0hFBQSRzatQbU_B-rPwj1Ja9a6qQJWtEeZQM6tvfkRDBg@mail.gmail.com>
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
next prev parent reply other threads:[~2020-01-09 23:20 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-27 8:12 Kuan-Lin Chen
2019-11-27 10:36 ` Andreas Schwab
2019-12-02 1:18 ` Kuan-Lin Chen
2019-12-02 22:27 ` Jim Wilson
2019-12-06 23:48 ` Palmer Dabbelt via binutils
2019-12-10 5:43 ` Kuan-Lin Chen
2019-12-17 23:59 ` Jim Wilson
2019-12-18 1:50 ` Nelson Chu
2020-01-06 8:18 ` Kuan-Lin Chen
2020-01-06 8:37 ` Kito Cheng
2020-01-06 8:44 ` Kuan-Lin Chen
2020-01-09 23:20 ` Palmer Dabbelt via binutils [this message]
2020-01-22 7:26 ` Kuan-Lin Chen
2020-09-25 7:14 ` Kito Cheng
2020-09-25 15:55 ` Jozef Lawrynowicz
2020-09-01 12:56 Jozef Lawrynowicz
2020-09-02 1:29 ` Kuan-Lin Chen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=mhng-bf7b317b-75dc-4000-b583-9074021d73fe@palmerdabbelt-glaptop \
--to=binutils@sourceware.org \
--cc=kuanlinchentw@gmail.com \
--cc=palmerdabbelt@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).