[PATCH v1 1/3] aarch64: Fix IMAGE_REL_ARM64_PAGEBASE_REL21 relocation and symbol reduction for relocations
Richard Earnshaw (lists)
Richard.Earnshaw@arm.com
Mon Sep 30 14:15:34 GMT 2024
On 26/08/2024 11:28, Evgeny Karpov wrote:
> Initially, the change was to fix IMAGE_REL_ARM64_PAGEBASE_REL21,
> which was addressing only 1MB instead of 4GB. This change triggered
> multiple changes in binutils and gcc to support the small code model.
>
> Probably, rel->r_type = IMAGE_REL_ARM64_ABSOLUTE was done on purpose
> to bypass changes in processed relocations in a later call to
> _bfd_coff_generic_relocate_section. However, it was breaking the
> relocation reduction logic, as the initial relocation type was no
> longer known because PE relocations have been replaced with
> IMAGE_REL_ARM64_ABSOLUTE. To avoid extra processing and introducing
> an extra field in the relocation structure, rel->r_type =
> IMAGE_REL_ARM64_ABSOLUTE has been replaced with rel->r_vaddr = -1,
> and a check to skip relocation processing has been added to
> _bfd_coff_generic_relocate_section.
>
> In one of the relocation processing steps, there is logic to reduce
> a symbol name to a section name where it is placed. In the COFF
> format on AArch64, this should not happen for symbols used in
> relocations that cannot be fully handled at the object compiling
> time. This information will be used by the linker to make correct
> relocation. Otherwise, it can lead to a silent relocation overflow
> during the linking step because the linker assumes everything looks
> good, which most likely results in a crash upon execution.
>
> To resolve this issue, S_SHOULD_BE_REDUCED_TO_SECTION_NAME has been
> added.
I'm not exactly sure where we got to with this patch, it might have been dropped. But one thing I noticed is that S_SHOULD_BE_REDUCED_TO_SECTION_NAME seems to be trying to achieve the same effect as the tc_fix_adjustable() hook, which can tell the generic assembler code if it is safe to replace one symbol with another+offset. I note that aarch64 does not define this, which may be the source of some of the issues you are seeing.
R.
>
> bfd/ChangeLog:
>
> * coff-aarch64.c (coff_aarch64_rel21_reloc): Fix relocation
> (coff_pe_aarch64_relocate_section): Likewise.
> * cofflink.c (_bfd_coff_generic_relocate_section): Likewise.
>
> gas/ChangeLog:
>
> * symbols.c (S_SHOULD_BE_REDUCED_TO_SECTION_NAME): New.
> (defined): New.
> * symbols.h (S_SHOULD_BE_REDUCED_TO_SECTION_NAME): New.
> * write.c (adjust_reloc_syms): Use
> S_SHOULD_BE_REDUCED_TO_SECTION_NAME.
> ---
> bfd/coff-aarch64.c | 35 ++++++++++++++---------------------
> bfd/cofflink.c | 3 +++
> gas/symbols.c | 17 +++++++++++++++++
> gas/symbols.h | 2 ++
> gas/write.c | 3 +++
> 5 files changed, 39 insertions(+), 21 deletions(-)
>
> diff --git a/bfd/coff-aarch64.c b/bfd/coff-aarch64.c
> index 3f6a608b728..9369193ddf5 100644
> --- a/bfd/coff-aarch64.c
> +++ b/bfd/coff-aarch64.c
> @@ -78,8 +78,9 @@ coff_aarch64_rel21_reloc (bfd *abfd,
> relocation -= (reloc_entry->address
> + input_section->output_offset
> + input_section->output_section->vma);
> - relocation = (bfd_signed_vma) relocation >> reloc_entry->howto->rightshift;
> }
> +
> + relocation = (bfd_signed_vma) relocation >> reloc_entry->howto->rightshift;
> if (relocation + 0x100000 > 0x1fffff)
> ret = bfd_reloc_overflow;
>
> @@ -579,8 +580,7 @@ coff_pe_aarch64_relocate_section (bfd *output_bfd,
> input_section, rel->r_vaddr - input_section->vma);
>
> bfd_putl32 (val, contents + rel->r_vaddr);
> - rel->r_type = IMAGE_REL_ARM64_ABSOLUTE;
> -
> + rel->r_vaddr = -1;
> break;
> }
>
> @@ -614,7 +614,7 @@ coff_pe_aarch64_relocate_section (bfd *output_bfd,
> opcode |= val & 0x3ffffff;
>
> bfd_putl32 (opcode, contents + rel->r_vaddr);
> - rel->r_type = IMAGE_REL_ARM64_ABSOLUTE;
> + rel->r_vaddr = -1;
>
> break;
> }
> @@ -649,8 +649,7 @@ coff_pe_aarch64_relocate_section (bfd *output_bfd,
> opcode |= (val & 0x7ffff) << 5;
>
> bfd_putl32 (opcode, contents + rel->r_vaddr);
> - rel->r_type = IMAGE_REL_ARM64_ABSOLUTE;
> -
> + rel->r_vaddr = -1;
> break;
> }
>
> @@ -684,8 +683,7 @@ coff_pe_aarch64_relocate_section (bfd *output_bfd,
> opcode |= (val & 0x3fff) << 5;
>
> bfd_putl32 (opcode, contents + rel->r_vaddr);
> - rel->r_type = IMAGE_REL_ARM64_ABSOLUTE;
> -
> + rel->r_vaddr = -1;
> break;
> }
>
> @@ -702,6 +700,7 @@ coff_pe_aarch64_relocate_section (bfd *output_bfd,
>
> if (addend & 0x100000)
> addend |= 0xffffffffffe00000;
> + addend <<= 12;
>
> dest_vma += addend;
> cur_vma = input_section->output_section->vma
> @@ -721,7 +720,7 @@ coff_pe_aarch64_relocate_section (bfd *output_bfd,
> opcode |= (val & 0x1ffffc) << 3;
>
> bfd_putl32 (opcode, contents + rel->r_vaddr);
> - rel->r_type = IMAGE_REL_ARM64_ABSOLUTE;
> + rel->r_vaddr = -1;
>
> break;
> }
> @@ -758,8 +757,7 @@ coff_pe_aarch64_relocate_section (bfd *output_bfd,
> opcode |= (val & 0x1ffffc) << 3;
>
> bfd_putl32 (opcode, contents + rel->r_vaddr);
> - rel->r_type = IMAGE_REL_ARM64_ABSOLUTE;
> -
> + rel->r_vaddr = -1;
> break;
> }
>
> @@ -787,8 +785,7 @@ coff_pe_aarch64_relocate_section (bfd *output_bfd,
> input_section, rel->r_vaddr - input_section->vma);
>
> bfd_putl32 (val, contents + rel->r_vaddr);
> - rel->r_type = IMAGE_REL_ARM64_ABSOLUTE;
> -
> + rel->r_vaddr = -1;
> break;
> }
>
> @@ -832,8 +829,7 @@ coff_pe_aarch64_relocate_section (bfd *output_bfd,
> opcode |= val << 10;
>
> bfd_putl32 (opcode, contents + rel->r_vaddr);
> - rel->r_type = IMAGE_REL_ARM64_ABSOLUTE;
> -
> + rel->r_vaddr = -1;
> break;
> }
>
> @@ -855,8 +851,7 @@ coff_pe_aarch64_relocate_section (bfd *output_bfd,
> opcode |= val << 10;
>
> bfd_putl32 (opcode, contents + rel->r_vaddr);
> - rel->r_type = IMAGE_REL_ARM64_ABSOLUTE;
> -
> + rel->r_vaddr = -1;
> break;
> }
>
> @@ -876,8 +871,7 @@ coff_pe_aarch64_relocate_section (bfd *output_bfd,
> input_section, rel->r_vaddr - input_section->vma);
>
> bfd_putl32 (val, contents + rel->r_vaddr);
> - rel->r_type = IMAGE_REL_ARM64_ABSOLUTE;
> -
> + rel->r_vaddr = -1;
> break;
> }
>
> @@ -901,8 +895,7 @@ coff_pe_aarch64_relocate_section (bfd *output_bfd,
>
>
> bfd_putl16 (idx, contents + rel->r_vaddr);
> - rel->r_type = IMAGE_REL_ARM64_ABSOLUTE;
> -
> + rel->r_vaddr = -1;
> break;
> }
>
> diff --git a/bfd/cofflink.c b/bfd/cofflink.c
> index 4a91d5a1833..6cded78c3bb 100644
> --- a/bfd/cofflink.c
> +++ b/bfd/cofflink.c
> @@ -2934,6 +2934,9 @@ _bfd_coff_generic_relocate_section (bfd *output_bfd,
>
> symndx = rel->r_symndx;
>
> + if (rel->r_vaddr == (bfd_vma) -1)
> + continue;
> +
> if (symndx == -1)
> {
> h = NULL;
> diff --git a/gas/symbols.c b/gas/symbols.c
> index fa3aaa37ed4..cd7f0adb4d3 100644
> --- a/gas/symbols.c
> +++ b/gas/symbols.c
> @@ -2414,6 +2414,23 @@ S_IS_DEBUG (const symbolS *s)
> return 0;
> }
>
> +int
> +S_SHOULD_BE_REDUCED_TO_SECTION_NAME (const symbolS *s)
> +{
> +#if defined (COFFAARCH64)
> + if (S_GET_STORAGE_CLASS ((symbolS *) s) == C_STAT)
> + return 0;
> +
> + if (s->bsym->section->flags & (SEC_DATA | SEC_READONLY))
> + return 0;
> +
> +#else
> + (void) s; /* Avoid unused variable warning. */
> +#endif
> +
> + return 1;
> +}
> +
> int
> S_IS_LOCAL (const symbolS *s)
> {
> diff --git a/gas/symbols.h b/gas/symbols.h
> index ee7ca67ac4c..999d1cb48ce 100644
> --- a/gas/symbols.h
> +++ b/gas/symbols.h
> @@ -124,6 +124,8 @@ extern void S_SET_THREAD_LOCAL (symbolS *);
> extern void S_SET_VOLATILE (symbolS *);
> extern void S_CLEAR_VOLATILE (symbolS *);
> extern void S_SET_FORWARD_REF (symbolS *);
> +extern int S_SHOULD_BE_REDUCED_TO_SECTION_NAME (const symbolS *s);
> +
>
> #ifndef WORKING_DOT_WORD
> struct broken_word
> diff --git a/gas/write.c b/gas/write.c
> index 18cf18fc830..9569bdeb638 100644
> --- a/gas/write.c
> +++ b/gas/write.c
> @@ -891,6 +891,9 @@ adjust_reloc_syms (bfd *abfd ATTRIBUTE_UNUSED,
> if ((symsec->flags & SEC_THREAD_LOCAL) != 0)
> continue;
>
> + if (!S_SHOULD_BE_REDUCED_TO_SECTION_NAME (sym))
> + continue;
> +
> val = S_GET_VALUE (sym);
>
> #if defined(TC_AARCH64) && defined(OBJ_COFF)
More information about the Binutils
mailing list