* [PATCH 0/2] RISC-V: Strict relocation handling
@ 2023-10-16 6:02 Tsukasa OI
2023-10-16 6:02 ` [PATCH 1/2] RISC-V: Reject invalid relocation types Tsukasa OI
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Tsukasa OI @ 2023-10-16 6:02 UTC (permalink / raw)
To: Tsukasa OI, Palmer Dabbelt, Andrew Waterman, Jim Wilson,
Nelson Chu, Kito Cheng
Cc: binutils
Hi,
[Background]
RISC-V BFD ELF handler has many relocation types but some of them are
internal use only (must not be placed on either object file or resulting
executable/shared library file).
[Description: PATCH 2/2]
And after RISC-V psABI version 1.0 is ratified, this ABI specification is
getting improved and enhanced.
Tatsuyuki Ishi enhanced the TLS descriptor model with four new relocation
types [1]. Luís Marques added (non-internal) GP-relative relocations (with
three new relocation types) [2].
[1] <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>
Of which, numbers 47-49 conflict with Binutils' internal relocation types.
Note that, though GP-relative relocations are similar to GNU Binutils'
internal ones, psABI ones (proposed by Luís) does not allow changing rd
(destination register) to gp/zero, requiring separate relocation types.
Also, GNU Binutils' ones only accept one instruction relocations.
That's exactly why both psABI's TPREL_LO12_[IS] (corresponding GNU Binutils'
R_RISCV_TPREL_LO12_[IS]) and GNU Binutils' internal R_RISCV_TPREL_[IS] are
still there.
We are not sure whether those new relocation types will get ratified but
at least should be aware of them.
| 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 |
So, we should move those internal only relocation types.
That's the intent of PATCH 2/2 and moves internal ones them to
unused/reserved spaces (41-42 [GPREL] and 66-67 [TPREL]).
[Description: PATCH 1/2]
Also in the first place, this kind of change and the current design reusing
the same relocation type space both for external ones and internal only ones
poses an issue: a new relocation type may be a severe problem on the future.
We haven't rejected such internal use only relocation types such as
R_RISCV_GPREL_I and R_RISCV_RVC_LUI. The effect if internal only type is
accidentally fed into the older linker is pretty much unpredictable.
Before an accident happens, we should start rejecting unknown relocation
types, at least on the linker. PATCH 1/2 allows only known relocation types
and explicitly rejects internal only relocation types (when the object file
is fed to the linker, the check only happens before linker relaxation
occurs, allowing linker relaxation to use internal only relocation types).
By adding separate checks, this patch also rejects unknown relocation types
on other tools such as objdump and objcopy.
Changes on the tools (summary):
They *did* reject unknown *big* relocation types (e.g. relocation type 191
== 0xBF) but not *small* relocation types and internal use only ones.
PATCH 1/2 makes them to reject those small/internal relocations.
Sincerely,
Tsukasa
Tsukasa OI (2):
RISC-V: Reject invalid relocation types
RISC-V: Renumber internal-only [GT]PREL_[IS] reloc
bfd/elfnn-riscv.c | 77 +++++++++++++++++++++++-
bfd/elfxx-riscv.c | 140 +++++++++++++++++++++++++-------------------
include/elf/riscv.h | 15 +++--
3 files changed, 166 insertions(+), 66 deletions(-)
base-commit: 6674b23fe6409e08de9c36f640bd58127eff9dda
--
2.42.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] RISC-V: Reject invalid relocation types
2023-10-16 6:02 [PATCH 0/2] RISC-V: Strict relocation handling Tsukasa OI
@ 2023-10-16 6:02 ` Tsukasa OI
2023-10-17 5:41 ` Nelson Chu
2023-10-16 6:02 ` [PATCH 2/2] RISC-V: Renumber internal-only [GT]PREL_[IS] reloc Tsukasa OI
2023-10-19 0:30 ` [PATCH v2 0/1] RISC-V: Strict relocation handling Tsukasa OI
2 siblings, 1 reply; 10+ messages in thread
From: Tsukasa OI @ 2023-10-16 6:02 UTC (permalink / raw)
To: Tsukasa OI, Palmer Dabbelt, Andrew Waterman, Jim Wilson,
Nelson Chu, Kito Cheng
Cc: binutils
From: Tsukasa OI <research_trasio@irq.a4lg.com>
In RISC-V BFD, there are several internal-only relocation types. Such
relocation fed from the outside can be a cause of unexpected behaviors
and should be rejected before being parsed further.
This commit adds checks to make sure that we only handle known
relocation types. For maintainability, internal-only relocation types
are listed separately.
Changes to riscv_elf_check_relocs applies to the linker (ld) and changes
to riscv_info_to_howto_rela and riscv_elf_rtype_to_howto applies to
other tools such like objdump and objcopy.
bfd/ChangeLog:
* elfnn-riscv.c (riscv_reloc_is_internal_use_only): New to detect
internal use only relocation type.
(riscv_info_to_howto_rela): Reject invalid relocation types
while handling ELF files but linking.
(riscv_elf_check_relocs): Reject invalid relocation types
while linking.
* elfxx-riscv.c (riscv_elf_rtype_to_howto): Also reject types
without name meaning unknown relocation type.
---
bfd/elfnn-riscv.c | 77 +++++++++++++++++++++++++++++++++++++++++++++--
bfd/elfxx-riscv.c | 2 +-
2 files changed, 76 insertions(+), 3 deletions(-)
diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
index 09aa7be225ef..dedfabe131ba 100644
--- a/bfd/elfnn-riscv.c
+++ b/bfd/elfnn-riscv.c
@@ -262,12 +262,37 @@ riscv_elfNN_set_options (struct bfd_link_info *link_info,
riscv_elf_hash_table (link_info)->params = params;
}
+static bool
+riscv_reloc_is_internal_use_only (unsigned int r_type)
+{
+ switch (r_type)
+ {
+ case R_RISCV_RVC_LUI:
+ case R_RISCV_GPREL_I:
+ case R_RISCV_GPREL_S:
+ case R_RISCV_TPREL_I:
+ case R_RISCV_TPREL_S:
+ case R_RISCV_DELETE:
+ return true;
+ default:
+ return false;
+ }
+}
+
static bool
riscv_info_to_howto_rela (bfd *abfd,
arelent *cache_ptr,
Elf_Internal_Rela *dst)
{
- cache_ptr->howto = riscv_elf_rtype_to_howto (abfd, ELFNN_R_TYPE (dst->r_info));
+ unsigned int r_type = ELFNN_R_TYPE (dst->r_info);
+ cache_ptr->howto = riscv_elf_rtype_to_howto (abfd, r_type);
+ if (cache_ptr->howto && riscv_reloc_is_internal_use_only (r_type))
+ {
+ (*_bfd_error_handler) (_("%pB: unsupported relocation type %#x"),
+ abfd, r_type);
+ bfd_set_error (bfd_error_bad_value);
+ cache_ptr->howto = NULL;
+ }
return cache_ptr->howto != NULL;
}
@@ -834,8 +859,53 @@ riscv_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
h->ref_regular = 1;
}
+ /* Explicitly reject internal use only relocation types. */
+ if (riscv_reloc_is_internal_use_only (r_type))
+ {
+ _bfd_error_handler
+ (_("%pB: internal error: unsupported relocation type %#x"),
+ abfd, r_type);
+ return false;
+ }
+
switch (r_type)
{
+ case R_RISCV_NONE:
+ case R_RISCV_TLS_DTPMOD32:
+ case R_RISCV_TLS_DTPMOD64:
+ case R_RISCV_TLS_DTPREL32:
+ case R_RISCV_TLS_DTPREL64:
+ case R_RISCV_TLS_TPREL32:
+ case R_RISCV_TLS_TPREL64:
+ case R_RISCV_PCREL_LO12_I:
+ case R_RISCV_PCREL_LO12_S:
+ case R_RISCV_LO12_I:
+ case R_RISCV_LO12_S:
+ case R_RISCV_TPREL_LO12_I:
+ case R_RISCV_TPREL_LO12_S:
+ case R_RISCV_TPREL_ADD:
+ case R_RISCV_ADD8:
+ case R_RISCV_ADD16:
+ case R_RISCV_ADD32:
+ case R_RISCV_ADD64:
+ case R_RISCV_SUB8:
+ case R_RISCV_SUB16:
+ case R_RISCV_SUB32:
+ case R_RISCV_SUB64:
+ case R_RISCV_ALIGN:
+ case R_RISCV_RELAX:
+ case R_RISCV_SUB6:
+ case R_RISCV_SET6:
+ case R_RISCV_SET8:
+ case R_RISCV_SET16:
+ case R_RISCV_SET32:
+ case R_RISCV_32_PCREL:
+ case R_RISCV_IRELATIVE:
+ case R_RISCV_SET_ULEB128:
+ case R_RISCV_SUB_ULEB128:
+ /* Known relocation types without additional checks here. */
+ break;
+
case R_RISCV_TLS_GD_HI20:
if (!riscv_elf_record_got_reference (abfd, info, h, r_symndx)
|| !riscv_elf_record_tls_type (abfd, h, r_symndx, GOT_TLS_GD))
@@ -1064,7 +1134,10 @@ riscv_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
break;
default:
- break;
+ _bfd_error_handler
+ (_("%pB: internal error: unsupported relocation type %#x"),
+ abfd, r_type);
+ return false;
}
}
diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
index c070394a3667..ffcdae341b2f 100644
--- a/bfd/elfxx-riscv.c
+++ b/bfd/elfxx-riscv.c
@@ -975,7 +975,7 @@ riscv_reloc_name_lookup (bfd *abfd ATTRIBUTE_UNUSED, const char *r_name)
reloc_howto_type *
riscv_elf_rtype_to_howto (bfd *abfd, unsigned int r_type)
{
- if (r_type >= ARRAY_SIZE (howto_table))
+ if (r_type >= ARRAY_SIZE (howto_table) || !howto_table[r_type].name)
{
(*_bfd_error_handler) (_("%pB: unsupported relocation type %#x"),
abfd, r_type);
--
2.42.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] RISC-V: Renumber internal-only [GT]PREL_[IS] reloc
2023-10-16 6:02 [PATCH 0/2] RISC-V: Strict relocation handling Tsukasa OI
2023-10-16 6:02 ` [PATCH 1/2] RISC-V: Reject invalid relocation types Tsukasa OI
@ 2023-10-16 6:02 ` Tsukasa OI
2023-10-17 1:34 ` Nelson Chu
2023-10-19 0:30 ` [PATCH v2 0/1] RISC-V: Strict relocation handling Tsukasa OI
2 siblings, 1 reply; 10+ messages in thread
From: Tsukasa OI @ 2023-10-16 6:02 UTC (permalink / raw)
To: Tsukasa OI, Palmer Dabbelt, Andrew Waterman, Jim Wilson,
Nelson Chu, Kito Cheng
Cc: binutils
From: Tsukasa OI <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>
[2] <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]).
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] RISC-V: Renumber internal-only [GT]PREL_[IS] reloc
2023-10-16 6:02 ` [PATCH 2/2] RISC-V: Renumber internal-only [GT]PREL_[IS] reloc Tsukasa OI
@ 2023-10-17 1:34 ` Nelson Chu
2023-10-17 1:44 ` Nelson Chu
2023-10-17 4:36 ` Tsukasa OI
0 siblings, 2 replies; 10+ messages in thread
From: Nelson Chu @ 2023-10-17 1:34 UTC (permalink / raw)
To: Tsukasa OI
Cc: Palmer Dabbelt, Andrew Waterman, Jim Wilson, Kito Cheng, binutils
[-- Attachment #1: Type: text/plain, Size: 13750 bytes --]
On Mon, Oct 16, 2023 at 2:03 PM Tsukasa OI <research_trasio@irq.a4lg.com>
wrote:
> From: Tsukasa OI <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
> >
> [2] <
> 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.
For now I think we only have five internal relocations,
R_RISCV_GPREL_I/S
R_RISCV_TPREL_I/S
R_RISCV_DELETE
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.
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),
so that the perform_relocation will still do the right thing, and also make
sure that --emit-reloc is still working.
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.
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
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] RISC-V: Renumber internal-only [GT]PREL_[IS] reloc
2023-10-17 1:34 ` Nelson Chu
@ 2023-10-17 1:44 ` Nelson Chu
2023-10-17 4:36 ` Tsukasa OI
1 sibling, 0 replies; 10+ messages in thread
From: Nelson Chu @ 2023-10-17 1:44 UTC (permalink / raw)
To: Tsukasa OI
Cc: Palmer Dabbelt, Andrew Waterman, Jim Wilson, Kito Cheng, binutils
[-- Attachment #1: Type: text/plain, Size: 3458 bytes --]
On Tue, Oct 17, 2023 at 9:34 AM Nelson Chu <nelson@rivosinc.com> wrote:
>
>
> On Mon, Oct 16, 2023 at 2:03 PM Tsukasa OI <research_trasio@irq.a4lg.com>
> wrote:
>
>> From: Tsukasa OI <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
>> >
>> [2] <
>> 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.
>
> For now I think we only have five internal relocations,
> R_RISCV_GPREL_I/S
> R_RISCV_TPREL_I/S
> R_RISCV_DELETE
>
> 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.
>
> 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),
> so that the perform_relocation will still do the right thing, and also make
> sure that --emit-reloc is still working.
>
> 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.
>
Or we still can define these internal relocs from R_RISCV_max to
R_RISCV_max + n, and then define another howto table for them. You can
refer to what nds32 did in the bfd/elf32-nds32.c, they are doing this in a
smart way.
Nelson
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] RISC-V: Renumber internal-only [GT]PREL_[IS] reloc
2023-10-17 1:34 ` Nelson Chu
2023-10-17 1:44 ` Nelson Chu
@ 2023-10-17 4:36 ` Tsukasa OI
1 sibling, 0 replies; 10+ messages in thread
From: Tsukasa OI @ 2023-10-17 4:36 UTC (permalink / raw)
To: Nelson Chu, Binutils
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
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] RISC-V: Reject invalid relocation types
2023-10-16 6:02 ` [PATCH 1/2] RISC-V: Reject invalid relocation types Tsukasa OI
@ 2023-10-17 5:41 ` Nelson Chu
2023-10-17 7:59 ` Tsukasa OI
0 siblings, 1 reply; 10+ messages in thread
From: Nelson Chu @ 2023-10-17 5:41 UTC (permalink / raw)
To: Tsukasa OI
Cc: Palmer Dabbelt, Andrew Waterman, Jim Wilson, Kito Cheng, binutils
[-- Attachment #1: Type: text/plain, Size: 6217 bytes --]
On Mon, Oct 16, 2023 at 2:03 PM Tsukasa OI <research_trasio@irq.a4lg.com>
wrote:
> From: Tsukasa OI <research_trasio@irq.a4lg.com>
>
> In RISC-V BFD, there are several internal-only relocation types. Such
> relocation fed from the outside can be a cause of unexpected behaviors
> and should be rejected before being parsed further.
>
> This commit adds checks to make sure that we only handle known
> relocation types. For maintainability, internal-only relocation types
> are listed separately.
>
> Changes to riscv_elf_check_relocs applies to the linker (ld) and changes
> to riscv_info_to_howto_rela and riscv_elf_rtype_to_howto applies to
> other tools such like objdump and objcopy.
>
> bfd/ChangeLog:
>
> * elfnn-riscv.c (riscv_reloc_is_internal_use_only): New to detect
> internal use only relocation type.
> (riscv_info_to_howto_rela): Reject invalid relocation types
> while handling ELF files but linking.
> (riscv_elf_check_relocs): Reject invalid relocation types
> while linking.
> * elfxx-riscv.c (riscv_elf_rtype_to_howto): Also reject types
> without name meaning unknown relocation type.
> ---
> bfd/elfnn-riscv.c | 77 +++++++++++++++++++++++++++++++++++++++++++++--
> bfd/elfxx-riscv.c | 2 +-
> 2 files changed, 76 insertions(+), 3 deletions(-)
>
> diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
> index 09aa7be225ef..dedfabe131ba 100644
> --- a/bfd/elfnn-riscv.c
> +++ b/bfd/elfnn-riscv.c
> @@ -262,12 +262,37 @@ riscv_elfNN_set_options (struct bfd_link_info
> *link_info,
> riscv_elf_hash_table (link_info)->params = params;
> }
>
> +static bool
> +riscv_reloc_is_internal_use_only (unsigned int r_type)
> +{
> + switch (r_type)
> + {
> + case R_RISCV_RVC_LUI:
> + case R_RISCV_GPREL_I:
> + case R_RISCV_GPREL_S:
> + case R_RISCV_TPREL_I:
> + case R_RISCV_TPREL_S:
> + case R_RISCV_DELETE:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> static bool
> riscv_info_to_howto_rela (bfd *abfd,
> arelent *cache_ptr,
> Elf_Internal_Rela *dst)
> {
> - cache_ptr->howto = riscv_elf_rtype_to_howto (abfd, ELFNN_R_TYPE
> (dst->r_info));
> + unsigned int r_type = ELFNN_R_TYPE (dst->r_info);
> + cache_ptr->howto = riscv_elf_rtype_to_howto (abfd, r_type);
> + if (cache_ptr->howto && riscv_reloc_is_internal_use_only (r_type))
> + {
> + (*_bfd_error_handler) (_("%pB: unsupported relocation type %#x"),
> + abfd, r_type);
> + bfd_set_error (bfd_error_bad_value);
> + cache_ptr->howto = NULL;
> + }
> return cache_ptr->howto != NULL;
> }
>
Maybe we could define these internal relocs from R_RISCV_max, and then
report errors when the number exceeds it? Like sparc did,
https://github.com/bminor/binutils-gdb/blob/master/bfd/elfxx-sparc.c#L637.
But I'm not sure if disallowing internal relocs will cause problems or
not...
>
> @@ -834,8 +859,53 @@ riscv_elf_check_relocs (bfd *abfd, struct
> bfd_link_info *info,
> h->ref_regular = 1;
> }
>
> + /* Explicitly reject internal use only relocation types. */
> + if (riscv_reloc_is_internal_use_only (r_type))
> + {
> + _bfd_error_handler
> + (_("%pB: internal error: unsupported relocation type %#x"),
> + abfd, r_type);
> + return false;
> + }
> +
> switch (r_type)
> {
> + case R_RISCV_NONE:
> + case R_RISCV_TLS_DTPMOD32:
> + case R_RISCV_TLS_DTPMOD64:
> + case R_RISCV_TLS_DTPREL32:
> + case R_RISCV_TLS_DTPREL64:
> + case R_RISCV_TLS_TPREL32:
> + case R_RISCV_TLS_TPREL64:
> + case R_RISCV_PCREL_LO12_I:
> + case R_RISCV_PCREL_LO12_S:
> + case R_RISCV_LO12_I:
> + case R_RISCV_LO12_S:
> + case R_RISCV_TPREL_LO12_I:
> + case R_RISCV_TPREL_LO12_S:
> + case R_RISCV_TPREL_ADD:
> + case R_RISCV_ADD8:
> + case R_RISCV_ADD16:
> + case R_RISCV_ADD32:
> + case R_RISCV_ADD64:
> + case R_RISCV_SUB8:
> + case R_RISCV_SUB16:
> + case R_RISCV_SUB32:
> + case R_RISCV_SUB64:
> + case R_RISCV_ALIGN:
> + case R_RISCV_RELAX:
> + case R_RISCV_SUB6:
> + case R_RISCV_SET6:
> + case R_RISCV_SET8:
> + case R_RISCV_SET16:
> + case R_RISCV_SET32:
> + case R_RISCV_32_PCREL:
> + case R_RISCV_IRELATIVE:
> + case R_RISCV_SET_ULEB128:
> + case R_RISCV_SUB_ULEB128:
> + /* Known relocation types without additional checks here. */
> + break;
> +
>
It seems like other targets don't have this similar check, so it will be
fine here as usual. If something won't cause a problem, then it would be
safe to keep them as usual.
> case R_RISCV_TLS_GD_HI20:
> if (!riscv_elf_record_got_reference (abfd, info, h, r_symndx)
> || !riscv_elf_record_tls_type (abfd, h, r_symndx,
> GOT_TLS_GD))
> @@ -1064,7 +1134,10 @@ riscv_elf_check_relocs (bfd *abfd, struct
> bfd_link_info *info,
> break;
>
> default:
> - break;
> + _bfd_error_handler
> + (_("%pB: internal error: unsupported relocation type %#x"),
> + abfd, r_type);
> + return false;
> }
> }
>
> diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> index c070394a3667..ffcdae341b2f 100644
> --- a/bfd/elfxx-riscv.c
> +++ b/bfd/elfxx-riscv.c
> @@ -975,7 +975,7 @@ riscv_reloc_name_lookup (bfd *abfd ATTRIBUTE_UNUSED,
> const char *r_name)
> reloc_howto_type *
> riscv_elf_rtype_to_howto (bfd *abfd, unsigned int r_type)
> {
> - if (r_type >= ARRAY_SIZE (howto_table))
> + if (r_type >= ARRAY_SIZE (howto_table) || !howto_table[r_type].name)
>
Seems like this will disallow the EMPTY_HOWTO?
Thanks
Nelson
> {
> (*_bfd_error_handler) (_("%pB: unsupported relocation type %#x"),
> abfd, r_type);
> --
> 2.42.0
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] RISC-V: Reject invalid relocation types
2023-10-17 5:41 ` Nelson Chu
@ 2023-10-17 7:59 ` Tsukasa OI
0 siblings, 0 replies; 10+ messages in thread
From: Tsukasa OI @ 2023-10-17 7:59 UTC (permalink / raw)
To: Nelson Chu, binutils
On 2023/10/17 14:41, 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>>
>
> In RISC-V BFD, there are several internal-only relocation types. Such
> relocation fed from the outside can be a cause of unexpected behaviors
> and should be rejected before being parsed further.
>
> This commit adds checks to make sure that we only handle known
> relocation types. For maintainability, internal-only relocation types
> are listed separately.
>
> Changes to riscv_elf_check_relocs applies to the linker (ld) and changes
> to riscv_info_to_howto_rela and riscv_elf_rtype_to_howto applies to
> other tools such like objdump and objcopy.
>
> bfd/ChangeLog:
>
> * elfnn-riscv.c (riscv_reloc_is_internal_use_only): New to
> detect
> internal use only relocation type.
> (riscv_info_to_howto_rela): Reject invalid relocation types
> while handling ELF files but linking.
> (riscv_elf_check_relocs): Reject invalid relocation types
> while linking.
> * elfxx-riscv.c (riscv_elf_rtype_to_howto): Also reject types
> without name meaning unknown relocation type.
> ---
> bfd/elfnn-riscv.c | 77 +++++++++++++++++++++++++++++++++++++++++++++--
> bfd/elfxx-riscv.c | 2 +-
> 2 files changed, 76 insertions(+), 3 deletions(-)
>
> diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
> index 09aa7be225ef..dedfabe131ba 100644
> --- a/bfd/elfnn-riscv.c
> +++ b/bfd/elfnn-riscv.c
> @@ -262,12 +262,37 @@ riscv_elfNN_set_options (struct bfd_link_info
> *link_info,
> riscv_elf_hash_table (link_info)->params = params;
> }
>
> +static bool
> +riscv_reloc_is_internal_use_only (unsigned int r_type)
> +{
> + switch (r_type)
> + {
> + case R_RISCV_RVC_LUI:
> + case R_RISCV_GPREL_I:
> + case R_RISCV_GPREL_S:
> + case R_RISCV_TPREL_I:
> + case R_RISCV_TPREL_S:
> + case R_RISCV_DELETE:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> static bool
> riscv_info_to_howto_rela (bfd *abfd,
> arelent *cache_ptr,
> Elf_Internal_Rela *dst)
> {
> - cache_ptr->howto = riscv_elf_rtype_to_howto (abfd, ELFNN_R_TYPE
> (dst->r_info));
> + unsigned int r_type = ELFNN_R_TYPE (dst->r_info);
> + cache_ptr->howto = riscv_elf_rtype_to_howto (abfd, r_type);
> + if (cache_ptr->howto && riscv_reloc_is_internal_use_only (r_type))
> + {
> + (*_bfd_error_handler) (_("%pB: unsupported relocation type %#x"),
> + abfd, r_type);
> + bfd_set_error (bfd_error_bad_value);
> + cache_ptr->howto = NULL;
> + }
> return cache_ptr->howto != NULL;
> }
>
>
> Maybe we could define these internal relocs from R_RISCV_max, and then
> report errors when the number exceeds it? Like sparc did,
> https://github.com/bminor/binutils-gdb/blob/master/bfd/elfxx-sparc.c#L637 <https://github.com/bminor/binutils-gdb/blob/master/bfd/elfxx-sparc.c#L637>. But I'm not sure if disallowing internal relocs will cause problems or not...
We could. But rejecting internal use only relocs is an absolute *MUST*
for the next release (short term solution should be merged if we don't
succeed to move internal only relocs after R_RISCV_max).
Fortunately, we won't accept R_RISCV_DELETE from outside (that would be
the worst case if it did but it doesn't happen because R_RISCV_DELETE is
defined relative to R_RISCV_max) but others can cause erroneous results.
>
>
>
> @@ -834,8 +859,53 @@ riscv_elf_check_relocs (bfd *abfd, struct
> bfd_link_info *info,
> h->ref_regular = 1;
> }
>
> + /* Explicitly reject internal use only relocation types. */
> + if (riscv_reloc_is_internal_use_only (r_type))
> + {
> + _bfd_error_handler
> + (_("%pB: internal error: unsupported relocation type %#x"),
> + abfd, r_type);
> + return false;
> + }
> +
> switch (r_type)
> {
> + case R_RISCV_NONE:
> + case R_RISCV_TLS_DTPMOD32:
> + case R_RISCV_TLS_DTPMOD64:
> + case R_RISCV_TLS_DTPREL32:
> + case R_RISCV_TLS_DTPREL64:
> + case R_RISCV_TLS_TPREL32:
> + case R_RISCV_TLS_TPREL64:
> + case R_RISCV_PCREL_LO12_I:
> + case R_RISCV_PCREL_LO12_S:
> + case R_RISCV_LO12_I:
> + case R_RISCV_LO12_S:
> + case R_RISCV_TPREL_LO12_I:
> + case R_RISCV_TPREL_LO12_S:
> + case R_RISCV_TPREL_ADD:
> + case R_RISCV_ADD8:
> + case R_RISCV_ADD16:
> + case R_RISCV_ADD32:
> + case R_RISCV_ADD64:
> + case R_RISCV_SUB8:
> + case R_RISCV_SUB16:
> + case R_RISCV_SUB32:
> + case R_RISCV_SUB64:
> + case R_RISCV_ALIGN:
> + case R_RISCV_RELAX:
> + case R_RISCV_SUB6:
> + case R_RISCV_SET6:
> + case R_RISCV_SET8:
> + case R_RISCV_SET16:
> + case R_RISCV_SET32:
> + case R_RISCV_32_PCREL:
> + case R_RISCV_IRELATIVE:
> + case R_RISCV_SET_ULEB128:
> + case R_RISCV_SUB_ULEB128:
> + /* Known relocation types without additional checks here. */
> + break;
> +
>
>
> It seems like other targets don't have this similar check, so it will be
> fine here as usual. If something won't cause a problem, then it would
> be safe to keep them as usual.
I saw elf32_frv_check_relocs from FR-V architecture code which does and
I thought this is valid to do that (even if rare). Also note that raw
reloc entries fed into riscv_elf_check_relocs are not checked by
riscv_elf_rtype_to_howto and if we miss internal use relocation types
here, that will be used (I don't want that).
>
>
> case R_RISCV_TLS_GD_HI20:
> if (!riscv_elf_record_got_reference (abfd, info, h, r_symndx)
> || !riscv_elf_record_tls_type (abfd, h, r_symndx,
> GOT_TLS_GD))
> @@ -1064,7 +1134,10 @@ riscv_elf_check_relocs (bfd *abfd, struct
> bfd_link_info *info,
> break;
>
> default:
> - break;
> + _bfd_error_handler
> + (_("%pB: internal error: unsupported relocation type %#x"),
> + abfd, r_type);
> + return false;
> }
> }
>
> diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> index c070394a3667..ffcdae341b2f 100644
> --- a/bfd/elfxx-riscv.c
> +++ b/bfd/elfxx-riscv.c
> @@ -975,7 +975,7 @@ riscv_reloc_name_lookup (bfd *abfd
> ATTRIBUTE_UNUSED, const char *r_name)
> reloc_howto_type *
> riscv_elf_rtype_to_howto (bfd *abfd, unsigned int r_type)
> {
> - if (r_type >= ARRAY_SIZE (howto_table))
> + if (r_type >= ARRAY_SIZE (howto_table) || !howto_table[r_type].name)
>
>
> Seems like this will disallow the EMPTY_HOWTO?
Yes, I think we need it. I was surprised by the fact that the most
targets don't do that (some of them don't have EMPTY_HOWTO but others
have such entries even if the most of them reject if the relocation type
is too large for the howto table; that's too arbitrary [same unknown
relocation type but different result between the value of the relocation
type; big or small] isn't it?).
Maybe we should note "what are we rejecting" or wrapping the check is
necessary but I'm going to reject EMPTY_HOWTO cases.
I'll report whether "move internal use only relocs after R_RISCV_max"
works (I'm working on it) and if not, please accept PATCH 1/2 (this
version or the next version with a minor change).
Thanks,
Tsukasa
>
> Thanks
> Nelson
>
>
> {
> (*_bfd_error_handler) (_("%pB: unsupported relocation type %#x"),
> abfd, r_type);
> --
> 2.42.0
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 0/1] RISC-V: Strict relocation handling
2023-10-16 6:02 [PATCH 0/2] RISC-V: Strict relocation handling Tsukasa OI
2023-10-16 6:02 ` [PATCH 1/2] RISC-V: Reject invalid relocation types Tsukasa OI
2023-10-16 6:02 ` [PATCH 2/2] RISC-V: Renumber internal-only [GT]PREL_[IS] reloc Tsukasa OI
@ 2023-10-19 0:30 ` Tsukasa OI
2023-10-19 0:30 ` [PATCH v2 1/1] RISC-V: Separate invalid/internal only ELF relocs Tsukasa OI
2 siblings, 1 reply; 10+ messages in thread
From: Tsukasa OI @ 2023-10-19 0:30 UTC (permalink / raw)
To: Tsukasa OI, Palmer Dabbelt, Andrew Waterman, Jim Wilson,
Nelson Chu, Kito Cheng
Cc: binutils
Hi,
This is the PATCH v2 of an attempt to prevent future accidents possibly
caused by future relocation additions.
PATCH v1:
<https://sourceware.org/pipermail/binutils/2023-October/129971.html>
[Added in v2]
1. Internal relocations are no longer emitted from the linker
even if you use ld --emit-relocs option.
[Changes in v2]
1. It became a single large patch since it's harder to separate
semantically (and cleanly).
2. Rejecting unknown relocs in riscv_elf_check_relocs is preserved but made
simpler (no longer need to list all supported public relocations).
I concluded that it was necessary (rare but valid as in FR-V) but there
were a simpler way to perform the same operation.
3. Wraps querying whether the relocation is empty, clarifying that we
want to reject empty relocations (minor).
4. Moves all internal relocations after R_RISCV_max.
I'll describe this in detail.
[Move internal relocations after R_RISCV_max]
This method is suggested by Nelson and I tried. I soon noticed that simply
moving internal relocations after R_RISCV_max doesn't work.
It was due to the fact that handling of R_RISCV_DELETE (the only internal
relocation defined relative to R_RISCV_max) was so special so that moving
other relocations causes problems.
Specifically, riscv_elf_relocate_section function (the function finally
relocates the section) calls riscv_elf_rtype_to_howto and requires generic
howto information. Yes, howto information.
So my second attempt is: let riscv_elf_rtype_to_howto search through the
special howto table only containing internal relocations only when required.
As far as I know, the only location we need the internal relocations is
in the riscv_elf_relocate_section function and new lookup_internal argument
is set to true only here (I think adding this argument is the simplest
because this function calls the error handler if fails and creating separate
internal variant will require duplicating most of the code).
... and that was a success!
Internal relocations are now all relative to R_RISCV_max and if a relocation
type is added after the biggest known relocation, internal relocation type
numbers are automatically increased. I found an interesting example in
libctf/swap.h so I added bonus safety measure to make sure that such
internal relocations are always safe to handle (relocation type numbers must
fit in an 8-bit integer due to constraints in ELF32).
I hope I resolved most of the issues Nelson pointed out.
Sincerely,
Tsukasa
Tsukasa OI (1):
RISC-V: Separate invalid/internal only ELF relocs
bfd/elfnn-riscv.c | 40 +++++----
bfd/elfxx-riscv.c | 202 +++++++++++++++++++++++++++-----------------
bfd/elfxx-riscv.h | 3 +-
include/elf/riscv.h | 37 ++++++--
4 files changed, 183 insertions(+), 99 deletions(-)
base-commit: e734b3e980d8a30ea23b5640d871b59d33720ecf
--
2.42.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/1] RISC-V: Separate invalid/internal only ELF relocs
2023-10-19 0:30 ` [PATCH v2 0/1] RISC-V: Strict relocation handling Tsukasa OI
@ 2023-10-19 0:30 ` Tsukasa OI
0 siblings, 0 replies; 10+ messages in thread
From: Tsukasa OI @ 2023-10-19 0:30 UTC (permalink / raw)
To: Tsukasa OI, Palmer Dabbelt, Andrew Waterman, Jim Wilson,
Nelson Chu, Kito Cheng
Cc: binutils
From: Tsukasa OI <research_trasio@irq.a4lg.com>
After ratification of the RISC-V psABI specification (version 1.0), it
is getting enhanced and improved.
This commit performs following changes.
1. Reject unknown ELF relocation types when fed into a tool
Before this commit, it accepted unknown (but small) relocation types and
relocation types only for internal uses (linker relaxation). More
worryingly, some internal only relocation types conflict with global
relocation types in the latest psABI draft [1].
[1] <https://github.com/riscv-non-isa/riscv-elf-psabi-doc/commit/d49e48097bcad410e7e6d96ebf9d94d6d41dc9c0>
If (a) psABI changes conflict with internal only relocation types and/or
(b) an object (possibly malicious or just from the future) with unknown
relocation type is encountered while linking (by ld) or relocating by
other tools, it can cause a severe failure (with
unpredictable erroneous results).
This commit now rejects small unknown relocation types and internal only
ones when an ELF file with such relocation types is fed into a tool.
2. Move internal only ELF relocation types after all regular ones
Currently, we have six internal only relocation types but only
R_RISCV_DELETE is distinguished from the regular one (others were defined
in between regular relocation types). This design caused the conflict
with regular relocation types *and* made fixing the number of such
internal relocation a non-trivial task.
This commit moves all internal only relocation types (not only
R_RISCV_DELETE) after R_RISCV_max and creates separate howto relocation
table for range (R_RISCV_DELETE + 1..R_RISCV_internal_max).
All internal only relocations are defined relative to R_RISCV_max and
will not conflict with regular ones (if psABI started to use large numbers,
internal relocations are automatically adjusted).
3. Prevent internal only ELF relocation types from emitting
It prevents emitting internal only relocations when the --emit-relocs
option is specified when linking (instead, replaces such internal only
relocations to R_RISCV_NONE).
bfd/ChangeLog:
* elfnn-riscv.c (R_RISCV_DELETE): Move to elf_riscv_reloc_type.
(riscv_info_to_howto_rela, bad_static_reloc): Reflect
riscv_elf_rtype_to_howto changes.
(riscv_elf_check_relocs): Likewise. Also reject unknown relocs
are found. Reuse howto variable.
(riscv_elf_relocate_section): Reflect riscv_elf_rtype_to_howto
changesL but also look up for internal relocs only when necessary.
Delete internal only relocation after the relocation.
* elfxx-riscv.c (HOWTO_ISEMPTY): New macro to query whether the
howto entry is empty.
(howto_table): Reserve all howto entries defined by the latest
RISC-V psABI specification except no actual EMPTY_HOWTO defs
at the end of the list. Move internal only relocs to...
(howto_table_internal): ...here.
(riscv_elf_rtype_to_howto): Add ability to look up internal only
relocation types only when necessary.
* elfxx-riscv.h (riscv_elf_rtype_to_howto): Reflect above.
include/ChangeLog:
* elf/riscv.h (enum elf_riscv_reloc_type): Comment all reserved
relocation types as defined by the latest RISC-V psABI spec.
Move all internal relocation types after R_RISCV_max, first being
R_RISCV_DELETE. Add R_RISCV_internal_max. Add safety guard for
all relocation types on C11 and later.
---
bfd/elfnn-riscv.c | 40 +++++----
bfd/elfxx-riscv.c | 202 +++++++++++++++++++++++++++-----------------
bfd/elfxx-riscv.h | 3 +-
include/elf/riscv.h | 37 ++++++--
4 files changed, 183 insertions(+), 99 deletions(-)
diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
index 09aa7be225ef..f9f3aca0af6a 100644
--- a/bfd/elfnn-riscv.c
+++ b/bfd/elfnn-riscv.c
@@ -130,9 +130,6 @@
} \
while (0)
-/* Internal relocations used exclusively by the relaxation pass. */
-#define R_RISCV_DELETE (R_RISCV_max + 1)
-
#define ARCH_SIZE NN
#define MINUS_ONE ((bfd_vma)0 - 1)
@@ -267,7 +264,8 @@ riscv_info_to_howto_rela (bfd *abfd,
arelent *cache_ptr,
Elf_Internal_Rela *dst)
{
- cache_ptr->howto = riscv_elf_rtype_to_howto (abfd, ELFNN_R_TYPE (dst->r_info));
+ cache_ptr->howto =
+ riscv_elf_rtype_to_howto (abfd, ELFNN_R_TYPE (dst->r_info), false);
return cache_ptr->howto != NULL;
}
@@ -714,7 +712,7 @@ riscv_elf_record_got_reference (bfd *abfd, struct bfd_link_info *info,
static bool
bad_static_reloc (bfd *abfd, unsigned r_type, struct elf_link_hash_entry *h)
{
- reloc_howto_type * r = riscv_elf_rtype_to_howto (abfd, r_type);
+ reloc_howto_type * r = riscv_elf_rtype_to_howto (abfd, r_type, false);
/* We propably can improve the information to tell users that they
should be recompile the code with -fPIC or -fPIE, just like what
@@ -756,11 +754,17 @@ riscv_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
{
unsigned int r_type;
unsigned int r_symndx;
+ reloc_howto_type *howto;
struct elf_link_hash_entry *h;
bool is_abs_symbol = false;
r_symndx = ELFNN_R_SYM (rel->r_info);
r_type = ELFNN_R_TYPE (rel->r_info);
+ howto = riscv_elf_rtype_to_howto (abfd, r_type, false);
+
+ /* Reject unknown relocs. */
+ if (!howto)
+ return false;
if (r_symndx >= NUM_SHDR_ENTRIES (symtab_hdr))
{
@@ -916,12 +920,10 @@ riscv_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
name = bfd_elf_sym_name (abfd, symtab_hdr, sym, NULL);
}
- reloc_howto_type *r_t =
- riscv_elf_rtype_to_howto (abfd, r_type);
_bfd_error_handler
(_("%pB: relocation %s against absolute symbol `%s' can "
"not be used when making a shared object"),
- abfd, r_t ? r_t->name : _("<unknown>"), name);
+ abfd, howto ? howto->name : _("<unknown>"), name);
bfd_set_error (bfd_error_bad_value);
return false;
}
@@ -959,11 +961,10 @@ riscv_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
if (is_abs_symbol)
break;
- reloc_howto_type *r_t = riscv_elf_rtype_to_howto (abfd, r_type);
_bfd_error_handler
(_("%pB: relocation %s against non-absolute symbol `%s' can "
"not be used in RVNN when making a shared object"),
- abfd, r_t ? r_t->name : _("<unknown>"),
+ abfd, howto ? howto->name : _("<unknown>"),
h != NULL ? h->root.root.string : "a local symbol");
bfd_set_error (bfd_error_bad_value);
return false;
@@ -996,8 +997,7 @@ riscv_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
}
}
- reloc_howto_type *r = riscv_elf_rtype_to_howto (abfd, r_type);
- if (RISCV_NEED_DYNAMIC_RELOC (r->pc_relative, info, h, sec))
+ if (RISCV_NEED_DYNAMIC_RELOC (howto->pc_relative, info, h, sec))
{
struct elf_dyn_relocs *p;
struct elf_dyn_relocs **head;
@@ -1058,7 +1058,7 @@ riscv_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
}
p->count += 1;
- p->pc_count += r == NULL ? 0 : r->pc_relative;
+ p->pc_count += howto == NULL ? 0 : howto->pc_relative;
}
break;
@@ -2182,7 +2182,8 @@ riscv_elf_relocate_section (bfd *output_bfd,
bool unresolved_reloc, is_ie = false;
bfd_vma pc = sec_addr (input_section) + rel->r_offset;
int r_type = ELFNN_R_TYPE (rel->r_info), tls_type;
- reloc_howto_type *howto = riscv_elf_rtype_to_howto (input_bfd, r_type);
+ reloc_howto_type *howto =
+ riscv_elf_rtype_to_howto (input_bfd, r_type, true);
const char *msg = NULL;
bool resolved_to_zero;
@@ -2615,7 +2616,8 @@ riscv_elf_relocate_section (bfd *output_bfd,
howto);
/* Update howto if relocation is changed. */
howto = riscv_elf_rtype_to_howto (input_bfd,
- ELFNN_R_TYPE (rel->r_info));
+ ELFNN_R_TYPE (rel->r_info),
+ false);
if (howto == NULL)
r = bfd_reloc_notsupported;
else if (!riscv_record_pcrel_hi_reloc (&pcrel_relocs, pc,
@@ -2766,7 +2768,8 @@ riscv_elf_relocate_section (bfd *output_bfd,
contents, howto);
/* Update howto if relocation is changed. */
howto = riscv_elf_rtype_to_howto (input_bfd,
- ELFNN_R_TYPE (rel->r_info));
+ ELFNN_R_TYPE (rel->r_info),
+ false);
if (howto == NULL)
r = bfd_reloc_notsupported;
else if (!riscv_record_pcrel_hi_reloc (&pcrel_relocs, pc,
@@ -3008,6 +3011,11 @@ riscv_elf_relocate_section (bfd *output_bfd,
r = perform_relocation (howto, rel, relocation, input_section,
input_bfd, contents);
+ /* Delete internal only relocations
+ (overwrite with R_RISCV_NONE). */
+ if (r_type >= R_RISCV_max)
+ rel->r_info = ELFNN_R_INFO (0, R_RISCV_NONE);
+
/* We should have already detected the error and set message before.
If the error message isn't set since the linker runs out of memory
or we don't set it before, then we should set the default message
diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
index c070394a3667..2366eae03abf 100644
--- a/bfd/elfxx-riscv.c
+++ b/bfd/elfxx-riscv.c
@@ -41,6 +41,10 @@ static bfd_reloc_status_type riscv_elf_add_sub_reloc
static bfd_reloc_status_type riscv_elf_ignore_reloc
(bfd *, arelent *, asymbol *, void *, asection *, bfd *, char **);
+/* Check whether the given howto entry is empty (unknown). */
+
+#define HOWTO_ISEMPTY(howto) (!((howto).name))
+
/* The relocation table used for SHT_RELA sections. */
static reloc_howto_type howto_table[] =
@@ -602,8 +606,8 @@ static reloc_howto_type howto_table[] =
false), /* pcrel_offset */
/* 41 and 42 are reserved. */
- EMPTY_HOWTO (0),
- EMPTY_HOWTO (0),
+ EMPTY_HOWTO (41),
+ EMPTY_HOWTO (42),
/* Indicates an alignment statement. The addend field encodes how many
bytes of NOPs follow the statement. The desired alignment is the
@@ -652,80 +656,20 @@ static reloc_howto_type howto_table[] =
ENCODE_CJTYPE_IMM (-1U), /* dst_mask */
true), /* pcrel_offset */
- /* High 6 bits of 18-bit absolute address. */
- HOWTO (R_RISCV_RVC_LUI, /* type */
- 0, /* rightshift */
- 2, /* size */
- 16, /* bitsize */
- false, /* pc_relative */
- 0, /* bitpos */
- complain_overflow_dont, /* complain_on_overflow */
- bfd_elf_generic_reloc, /* special_function */
- "R_RISCV_RVC_LUI", /* name */
- false, /* partial_inplace */
- 0, /* src_mask */
- ENCODE_CITYPE_IMM (-1U), /* dst_mask */
- false), /* pcrel_offset */
+ /* 46 is reserved. */
+ EMPTY_HOWTO (46),
- /* 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 +823,101 @@ 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), */
+};
+
+/* The relocation table used for SHT_RELA sections for internal use.
+ Those relocations must be removed before we finish linking. */
+
+static reloc_howto_type howto_table_internal[] =
+{
+ /* R_RISCV_DELETE is omitted since this is special. */
+
+ /* High 6 bits of 18-bit absolute address. */
+ HOWTO (R_RISCV_RVC_LUI, /* type */
+ 0, /* rightshift */
+ 2, /* size */
+ 16, /* bitsize */
+ false, /* pc_relative */
+ 0, /* bitpos */
+ complain_overflow_dont, /* complain_on_overflow */
+ bfd_elf_generic_reloc, /* special_function */
+ "R_RISCV_RVC_LUI", /* name */
+ false, /* partial_inplace */
+ 0, /* src_mask */
+ 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 */
+
+ /* 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 */
+
+ /* 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. */
@@ -973,16 +1012,27 @@ riscv_reloc_name_lookup (bfd *abfd ATTRIBUTE_UNUSED, const char *r_name)
}
reloc_howto_type *
-riscv_elf_rtype_to_howto (bfd *abfd, unsigned int r_type)
+riscv_elf_rtype_to_howto (bfd *abfd, unsigned int r_type,
+ bool lookup_internal)
{
- if (r_type >= ARRAY_SIZE (howto_table))
+ reloc_howto_type *ret = NULL;
+ if (r_type < ARRAY_SIZE (howto_table))
+ {
+ ret = &howto_table[r_type];
+ if (HOWTO_ISEMPTY (*ret))
+ ret = NULL;
+ }
+ else if (lookup_internal
+ && r_type > R_RISCV_DELETE
+ && r_type < R_RISCV_internal_max)
+ ret = &howto_table_internal[r_type - (R_RISCV_DELETE + 1)];
+ if (!ret)
{
(*_bfd_error_handler) (_("%pB: unsupported relocation type %#x"),
abfd, r_type);
bfd_set_error (bfd_error_bad_value);
- return NULL;
}
- return &howto_table[r_type];
+ return ret;
}
/* Special_function of RISCV_ADD and RISCV_SUB relocations. */
diff --git a/bfd/elfxx-riscv.h b/bfd/elfxx-riscv.h
index abcb409bd78d..9a6f40787110 100644
--- a/bfd/elfxx-riscv.h
+++ b/bfd/elfxx-riscv.h
@@ -45,7 +45,8 @@ extern reloc_howto_type *
riscv_reloc_type_lookup (bfd *, bfd_reloc_code_real_type);
extern reloc_howto_type *
-riscv_elf_rtype_to_howto (bfd *, unsigned int r_type);
+riscv_elf_rtype_to_howto (bfd *, unsigned int r_type,
+ bool lookup_internal);
/* The information of architecture attribute. */
struct riscv_subset_t
diff --git a/include/elf/riscv.h b/include/elf/riscv.h
index 0aa8b3359c4c..0060542f7998 100644
--- a/include/elf/riscv.h
+++ b/include/elf/riscv.h
@@ -74,11 +74,9 @@ START_RELOC_NUMBERS (elf_riscv_reloc_type)
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,7 +88,34 @@ 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)
-END_RELOC_NUMBERS (R_RISCV_max)
+ /* 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. */
+ EMPTY_RELOC (R_RISCV_max)
+
+ /* Internal relocations used exclusively by the relaxation pass.
+ R_RISCV_DELETE must be the first entry. */
+ EMPTY_RELOC (R_RISCV_DELETE)
+ EMPTY_RELOC (R_RISCV_RVC_LUI)
+ EMPTY_RELOC (R_RISCV_GPREL_I)
+ EMPTY_RELOC (R_RISCV_GPREL_S)
+ EMPTY_RELOC (R_RISCV_TPREL_I)
+ EMPTY_RELOC (R_RISCV_TPREL_S)
+END_RELOC_NUMBERS (R_RISCV_internal_max)
+
+/* Safety guard for all relocation types. */
+#ifndef RELOC_MACROS_GEN_FUNC
+#if !defined (__STDC_VERSION__) || __STDC_VERSION__ < 201112L
+#define _Static_assert(cond, err)
+#endif
+_Static_assert (R_RISCV_internal_max <= 256,
+ "All relocation types including internal ones must fit"
+ "into an 8-bit value.");
+#ifdef _Static_assert
+#undef _Static_assert
+#endif
+#endif
/* Processor specific flags for the ELF header e_flags field. */
--
2.42.0
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-10-19 0:31 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-16 6:02 [PATCH 0/2] RISC-V: Strict relocation handling Tsukasa OI
2023-10-16 6:02 ` [PATCH 1/2] RISC-V: Reject invalid relocation types Tsukasa OI
2023-10-17 5:41 ` Nelson Chu
2023-10-17 7:59 ` Tsukasa OI
2023-10-16 6:02 ` [PATCH 2/2] RISC-V: Renumber internal-only [GT]PREL_[IS] reloc Tsukasa OI
2023-10-17 1:34 ` Nelson Chu
2023-10-17 1:44 ` Nelson Chu
2023-10-17 4:36 ` Tsukasa OI
2023-10-19 0:30 ` [PATCH v2 0/1] RISC-V: Strict relocation handling Tsukasa OI
2023-10-19 0:30 ` [PATCH v2 1/1] RISC-V: Separate invalid/internal only ELF relocs Tsukasa OI
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).