[PATCH 2/2] RISC-V: Renumber internal-only [GT]PREL_[IS] reloc
Tsukasa OI
research_trasio@irq.a4lg.com
Tue Oct 17 04:36:03 GMT 2023
Thanks for your precious review!
On 2023/10/17 10:34, Nelson Chu wrote:
>
>
> On Mon, Oct 16, 2023 at 2:03 PM Tsukasa OI <research_trasio@irq.a4lg.com
> <mailto:research_trasio@irq.a4lg.com>> wrote:
>
> From: Tsukasa OI <research_trasio@irq.a4lg.com
> <mailto:research_trasio@irq.a4lg.com>>
>
> After ratification of the RISC-V psABI specification (version 1.0), it
> is getting enhanced and improved.
>
> Some commits include new relocation types:
>
> [1]
> <https://github.com/riscv-non-isa/riscv-elf-psabi-doc/commit/eaf0b30db01255e145fd173d8a0946a4b7cce90f <https://github.com/riscv-non-isa/riscv-elf-psabi-doc/commit/eaf0b30db01255e145fd173d8a0946a4b7cce90f>>
> [2]
> <https://github.com/riscv-non-isa/riscv-elf-psabi-doc/commit/d49e48097bcad410e7e6d96ebf9d94d6d41dc9c0 <https://github.com/riscv-non-isa/riscv-elf-psabi-doc/commit/d49e48097bcad410e7e6d96ebf9d94d6d41dc9c0>>
>
> The latest draft of the RISC-V psABI specification started to use
> relocation types 47-49 [2] but some of them conflict with Binutils'
> internal-only relocation types:
>
> | N | psABI type (draft) | Binutils type |
> | -- | ------------------- | --------------- |
> | 47 | GPREL_LO12_I | R_RISCV_GPREL_I |
> | 48 | GPREL_LO12_S | R_RISCV_GPREL_S |
> | 49 | GPREL_HI20 | R_RISCV_TPREL_I |
> | 50 | (reserved) | R_RISCV_TPREL_S |
>
> Note that R_RISCV_GPREL_[IS] cannot be used for GPREL_LO12_[IS] because
> GPREL_LO12_[IS] do not allow rewrite to rd and internal
> R_RISCV_GPREL_[IS]
> are for single instruction sequence only (that's why we have both
> internal
> TPREL_[IS] and external TPREL_LO12_[IS]).
>
> We have to move at least 47-49 but for locality, we move
> R_RISCV_GPREL_[IS] from 47-48 to 41-42 and R_RISCV_TPREL_[IS] from 49-50
> to 66-67 (note that 62-65 are reserved for other relocation types in the
> latest draft of RISC-V psABI [1]).
>
>
> Seems like we will need to renumber these internally relocations in the
> future frequently, so probably need other solutions.
I agree that we need a long term solution and I have to admit that this
is only a short term one. Meanwhile, PATCH 1/2 can be a part of a long
term solution and approving that (alone) would be helpful.
For instance, PATCH 1/2 also rejects vacant relocation type in the HOWTO
table (reserved and not used by internal use only relocations either
[but may be used in the future]).
>
> For now I think we only have five internal relocations,
> R_RISCV_GPREL_I/S
> R_RISCV_TPREL_I/S
> R_RISCV_DELETE
To be precise, there are six (R_RISCV_RVC_LUI as in the PATCH 1/2, is
also internal use only [only appears in the linker relaxation]).
<https://github.com/riscv-non-isa/riscv-elf-psabi-doc/issues/398>
<https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/400>
>
> There are two ways to go,
>
> 1. Renumber these internal relocations from last (R_RISCV_max) to
> forward (R_RISCV_max-1, R_RISCV_max -2).
> This will ensure we have a safe time until these internal relocations
> overlap with the regular ones defined in the psabi.
>
> 2. Don't define these internal relocations in the howto table
> The howto of GPREL_I/S and TPREL_I/S are basically the same as the howto
> of GPREL_LO12_I/S and TPREL_LO12_I/S. Therefore, we probably can define
> them from R_RISCV_max to R_RISCV_max + n, just like what we did for
> R_RISCV_DELETE.
I'll give it a shot (so I'll withdraw PATCH 2/2 for now). My first
attempt was to separate internal only relocations somehow and failed
multiple times... but forgot to record "failed how".
So the ways you suggested might work.
I don't want to disrupt relocation types 192-255 (reserved for vendor)
but... that might not be possible and may be a job for another time.
>
> Currently, for R_RISCV_TPREL_I/S, we converted it from
> R_RISCV_TPREL_LO12_I/S in the relax_section, and changed the base
> register to tp in relocate_section. We probably can convert the reloc
> types back to R_RISCV_TPREL_I/S after changing the base register
> (https://github.com/bminor/binutils-gdb/blob/master/bfd/elfnn-riscv.c#L2736 <https://github.com/bminor/binutils-gdb/blob/master/bfd/elfnn-riscv.c#L2736>), so that the perform_relocation will still do the right thing, and also make sure that --emit-reloc is still working.
I forgot about --emit-reloc and will investigate interactions between
this option and internal relocation type changes.
>
> For R_RISCV_GPREL_I/S, since they should do the same thing with the
> regular R_RISCV_GPREL_LO12_I/S, we could wait until the regular relocs
> are supported, and then renumber and dealt with the internal ones.
I don't agree with it (if we have to move internal ones, we *have to*
move them too) because of two reasons:
1. While GPREL_LO12_I/S is for two instruction sequence,
GPREL_I/S is for one instruction only (it limits relative offsets).
GPREL_LO12_I/S don't allow rewriting rd field to gp/zero either.
2. %gprel_hi and %gprel_lo are too easy to implement and I don't want
to disrupt a developer who wish to implement those in GAS.
Note that I withdraw PATCH 2/2 for now but not PATCH 1/2. Reviewing
PATCH 1/2 is appreciated.
Thanks,
Tsukasa
>
> Thanks
> Nelson
>
>
>
> It also notes all reserved relocation types as defined in the latest
> draft of RISC-V psABI.
>
> bfd/ChangeLog:
>
> * elfxx-riscv.c (howto_table): Reserve all defined relocation
> types as defined in the latest draft of RISC-V psABI. Move
> R_RISCV_[GT]PREL_[IS] to the empty spaces.
>
> include/ChangeLog:
>
> * elf/riscv.h (elf_riscv_reloc_type): Renumber
> R_RISCV_[GT]PREL_[IS] from 47-50 to 41, 42, 66 and 67. Comment
> all defined relocation types as defined in the latest draft
> of RISC-V psABI.
> ---
> bfd/elfxx-riscv.c | 138 +++++++++++++++++++++++++-------------------
> include/elf/riscv.h | 15 +++--
> 2 files changed, 90 insertions(+), 63 deletions(-)
>
> diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> index ffcdae341b2f..18fc638d05cb 100644
> --- a/bfd/elfxx-riscv.c
> +++ b/bfd/elfxx-riscv.c
> @@ -601,9 +601,35 @@ static reloc_howto_type howto_table[] =
> MINUS_ONE, /* dst_mask */
> false), /* pcrel_offset */
>
> - /* 41 and 42 are reserved. */
> - EMPTY_HOWTO (0),
> - EMPTY_HOWTO (0),
> + /* GP-relative load. */
> + HOWTO (R_RISCV_GPREL_I, /* type */
> + 0, /* rightshift */
> + 4, /* size */
> + 32, /* bitsize */
> + false, /* pc_relative */
> + 0, /* bitpos */
> + complain_overflow_dont, /* complain_on_overflow */
> + bfd_elf_generic_reloc, /* special_function */
> + "R_RISCV_GPREL_I", /* name */
> + false, /* partial_inplace */
> + 0, /* src_mask */
> + ENCODE_ITYPE_IMM (-1U), /* dst_mask */
> + false), /* pcrel_offset */
> +
> + /* GP-relative store. */
> + HOWTO (R_RISCV_GPREL_S, /* type */
> + 0, /* rightshift */
> + 4, /* size */
> + 32, /* bitsize */
> + false, /* pc_relative */
> + 0, /* bitpos */
> + complain_overflow_dont, /* complain_on_overflow */
> + bfd_elf_generic_reloc, /* special_function */
> + "R_RISCV_GPREL_S", /* name */
> + false, /* partial_inplace */
> + 0, /* src_mask */
> + ENCODE_STYPE_IMM (-1U), /* dst_mask */
> + false), /* pcrel_offset */
>
> /* Indicates an alignment statement. The addend field encodes
> how many
> bytes of NOPs follow the statement. The desired alignment is the
> @@ -667,65 +693,17 @@ static reloc_howto_type howto_table[] =
> ENCODE_CITYPE_IMM (-1U), /* dst_mask */
> false), /* pcrel_offset */
>
> - /* GP-relative load. */
> - HOWTO (R_RISCV_GPREL_I, /* type */
> - 0, /* rightshift */
> - 4, /* size */
> - 32, /* bitsize */
> - false, /* pc_relative */
> - 0, /* bitpos */
> - complain_overflow_dont, /* complain_on_overflow */
> - bfd_elf_generic_reloc, /* special_function */
> - "R_RISCV_GPREL_I", /* name */
> - false, /* partial_inplace */
> - 0, /* src_mask */
> - ENCODE_ITYPE_IMM (-1U), /* dst_mask */
> - false), /* pcrel_offset */
> + /* Reserved for R_RISCV_GPREL_LO12_I. */
> + EMPTY_HOWTO (47),
>
> - /* GP-relative store. */
> - HOWTO (R_RISCV_GPREL_S, /* type */
> - 0, /* rightshift */
> - 4, /* size */
> - 32, /* bitsize */
> - false, /* pc_relative */
> - 0, /* bitpos */
> - complain_overflow_dont, /* complain_on_overflow */
> - bfd_elf_generic_reloc, /* special_function */
> - "R_RISCV_GPREL_S", /* name */
> - false, /* partial_inplace */
> - 0, /* src_mask */
> - ENCODE_STYPE_IMM (-1U), /* dst_mask */
> - false), /* pcrel_offset */
> + /* Reserved for R_RISCV_GPREL_LO12_S. */
> + EMPTY_HOWTO (48),
>
> - /* TP-relative TLS LE load. */
> - HOWTO (R_RISCV_TPREL_I, /* type */
> - 0, /* rightshift */
> - 4, /* size */
> - 32, /* bitsize */
> - false, /* pc_relative */
> - 0, /* bitpos */
> - complain_overflow_signed, /* complain_on_overflow */
> - bfd_elf_generic_reloc, /* special_function */
> - "R_RISCV_TPREL_I", /* name */
> - false, /* partial_inplace */
> - 0, /* src_mask */
> - ENCODE_ITYPE_IMM (-1U), /* dst_mask */
> - false), /* pcrel_offset */
> + /* Reserved for R_RISCV_GPREL_HI20. */
> + EMPTY_HOWTO (49),
>
> - /* TP-relative TLS LE store. */
> - HOWTO (R_RISCV_TPREL_S, /* type */
> - 0, /* rightshift */
> - 4, /* size */
> - 32, /* bitsize */
> - false, /* pc_relative */
> - 0, /* bitpos */
> - complain_overflow_signed, /* complain_on_overflow */
> - bfd_elf_generic_reloc, /* special_function */
> - "R_RISCV_TPREL_S", /* name */
> - false, /* partial_inplace */
> - 0, /* src_mask */
> - ENCODE_STYPE_IMM (-1U), /* dst_mask */
> - false), /* pcrel_offset */
> + /* 50 is reserved. */
> + EMPTY_HOWTO (50),
>
> /* The paired relocation may be relaxed. */
> HOWTO (R_RISCV_RELAX, /* type */
> @@ -879,6 +857,48 @@ static reloc_howto_type howto_table[] =
> 0, /* src_mask */
> 0, /* dst_mask */
> false), /* pcrel_offset */
> +
> + /* Reserved for R_RISCV_TLSDESC_HI20. */
> + EMPTY_HOWTO (62),
> +
> + /* Reserved for R_RISCV_TLSDESC_LOAD_LO12. */
> + EMPTY_HOWTO (63),
> +
> + /* Reserved for R_RISCV_TLSDESC_ADD_LO12. */
> + EMPTY_HOWTO (64),
> +
> + /* Reserved for R_RISCV_TLSDESC_CALL. */
> + EMPTY_HOWTO (65),
> +
> + /* TP-relative TLS LE load. */
> + HOWTO (R_RISCV_TPREL_I, /* type */
> + 0, /* rightshift */
> + 4, /* size */
> + 32, /* bitsize */
> + false, /* pc_relative */
> + 0, /* bitpos */
> + complain_overflow_signed, /* complain_on_overflow */
> + bfd_elf_generic_reloc, /* special_function */
> + "R_RISCV_TPREL_I", /* name */
> + false, /* partial_inplace */
> + 0, /* src_mask */
> + ENCODE_ITYPE_IMM (-1U), /* dst_mask */
> + false), /* pcrel_offset */
> +
> + /* TP-relative TLS LE store. */
> + HOWTO (R_RISCV_TPREL_S, /* type */
> + 0, /* rightshift */
> + 4, /* size */
> + 32, /* bitsize */
> + false, /* pc_relative */
> + 0, /* bitpos */
> + complain_overflow_signed, /* complain_on_overflow */
> + bfd_elf_generic_reloc, /* special_function */
> + "R_RISCV_TPREL_S", /* name */
> + false, /* partial_inplace */
> + 0, /* src_mask */
> + ENCODE_STYPE_IMM (-1U), /* dst_mask */
> + false), /* pcrel_offset */
> };
>
> /* A mapping from BFD reloc types to RISC-V ELF reloc types. */
> diff --git a/include/elf/riscv.h b/include/elf/riscv.h
> index 0aa8b3359c4c..6ae31f6a969a 100644
> --- a/include/elf/riscv.h
> +++ b/include/elf/riscv.h
> @@ -71,14 +71,15 @@ START_RELOC_NUMBERS (elf_riscv_reloc_type)
> RELOC_NUMBER (R_RISCV_SUB16, 38)
> RELOC_NUMBER (R_RISCV_SUB32, 39)
> RELOC_NUMBER (R_RISCV_SUB64, 40)
> + RELOC_NUMBER (R_RISCV_GPREL_I, 41)
> + RELOC_NUMBER (R_RISCV_GPREL_S, 42)
> RELOC_NUMBER (R_RISCV_ALIGN, 43)
> RELOC_NUMBER (R_RISCV_RVC_BRANCH, 44)
> RELOC_NUMBER (R_RISCV_RVC_JUMP, 45)
> RELOC_NUMBER (R_RISCV_RVC_LUI, 46)
> - RELOC_NUMBER (R_RISCV_GPREL_I, 47)
> - RELOC_NUMBER (R_RISCV_GPREL_S, 48)
> - RELOC_NUMBER (R_RISCV_TPREL_I, 49)
> - RELOC_NUMBER (R_RISCV_TPREL_S, 50)
> + /* Reserved 47 for R_RISCV_GPREL_LO12_I. */
> + /* Reserved 48 for R_RISCV_GPREL_LO12_S. */
> + /* Reserved 49 for R_RISCV_GPREL_HI20. */
> RELOC_NUMBER (R_RISCV_RELAX, 51)
> RELOC_NUMBER (R_RISCV_SUB6, 52)
> RELOC_NUMBER (R_RISCV_SET6, 53)
> @@ -90,6 +91,12 @@ START_RELOC_NUMBERS (elf_riscv_reloc_type)
> /* Reserved 59 for R_RISCV_PLT32. */
> RELOC_NUMBER (R_RISCV_SET_ULEB128, 60)
> RELOC_NUMBER (R_RISCV_SUB_ULEB128, 61)
> + /* Reserved 62 for R_RISCV_TLSDESC_HI20. */
> + /* Reserved 63 for R_RISCV_TLSDESC_LOAD_LO12. */
> + /* Reserved 64 for R_RISCV_TLSDESC_ADD_LO12. */
> + /* Reserved 65 for R_RISCV_TLSDESC_CALL. */
> + RELOC_NUMBER (R_RISCV_TPREL_I, 66)
> + RELOC_NUMBER (R_RISCV_TPREL_S, 67)
> END_RELOC_NUMBERS (R_RISCV_max)
>
> /* Processor specific flags for the ELF header e_flags field. */
> --
> 2.42.0
>
More information about the Binutils
mailing list