load of in-place relocation addend in AArch64

load of in-place relocation addend in AArch64

[Frédéric Rivière]

Dear binutils group,

I'm actually working on an in-house compiler that generates AArch64 ELF 
object files.

I'm actually fighting with the following issue:

My compiler generates REL relocations with an in-place addend but the 
addend is not taken into account for computing the final relocated 
symbol address (S+A).

The addend is initialized to 0 
(https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=bfd/elfnn-aarch64.c;h=4faf642b422ffdff5c7879bff6bd08991b38eab8;hb=HEAD#l6931)

Then in elf32_arm_final_link_relocate, the addend is added from the 
relocation field "r_addend" (always 0 in case of REL)

https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=bfd/elfnn-aarch64.c;h=4faf642b422ffdff5c7879bff6bd08991b38eab8;hb=HEAD#l5674

So it seems the that I'm the first one that generates REL relocations 
for AArch64 target.

However, I don't see any limitation on using REL relocations in the 
AARCH64 ABI.

https://github.com/ARM-software/abi-aa/blob/2982a9f3b512a5bfdc9e3fea5d3b298f9165c36b/aaelf64/aaelf64.rst#572addends-and-pc-bias

I would appreciate any comment/feedback that would help me understanding 
the reasons behind that.

Thanks for your help.

Best Regards,

--Frédéric RIVIERE


	

Re: load of in-place relocation addend in AArch64

[Nick Clifton]

Hi Frédéric,

> My compiler generates REL relocations with an in-place addend but the addend is not taken into account for computing the final relocated symbol address (S+A).

Is there any particular reason for this ?

Ie, if your compiler generated RELA relocations then all of your problems
with the binutils should go away...

> So it seems the that I'm the first one that generates REL relocations for AArch64 target.

True.

> However, I don't see any limitation on using REL relocations in the AARCH64 ABI.

Correct.  There are - in theory - allowed.

> I would appreciate any comment/feedback that would help me understanding the reasons behind that.

As a general rule RELA relocations are preferable in that they
are simpler - it is easier to extract the addend - and in theory
they have the possibility to express a wider range of addends
than would be available to REL relocations.

Have you tried modifying the AArch64 code in the BFD library to
support REL relocations ?  In theory all that you need to do is
to change a few lines in bfd/elfnn-aarch64.c:

   #define elf_backend_may_use_rel_p      0

and maybe:

   #define elf_backend_may_use_rela_p     1
   #define elf_backend_default_use_rela_p 1
   #define elf_backend_rela_normal	 1

In practice I am pretty sure that there will be assumptions of RELA
relocs elsewhere in the AArch64 specific code, so I doubt it will be
as simple as I have suggested.

Cheers
   Nick

Re: load of in-place relocation addend in AArch64

[Frédéric Rivière]

Hi Nick,

Thanks for the quick reply.

My compiler generates REL relocations because I was just naively 
following the official AArch64 ABI.

I understand that RELA is more easy to manage since there is no need to 
retrieve the source section for getting the addend.

However, I believe that ld AArch64 should not support REL entries at all 
(with a clear error message) instead of silently generating a wrong 
object file.

Finally, I just updated my compiler to generate RELA and now it works fine.

Best regards,

--Frédéric

On 12/6/2023 12:14 PM, Nick Clifton wrote:
> Hi Frédéric,
>
>> My compiler generates REL relocations with an in-place addend but the 
>> addend is not taken into account for computing the final relocated 
>> symbol address (S+A).
>
> Is there any particular reason for this ?
>
> Ie, if your compiler generated RELA relocations then all of your problems
> with the binutils should go away...
>
>> So it seems the that I'm the first one that generates REL relocations 
>> for AArch64 target.
>
> True.
>
>> However, I don't see any limitation on using REL relocations in the 
>> AARCH64 ABI.
>
> Correct.  There are - in theory - allowed.
>
>> I would appreciate any comment/feedback that would help me 
>> understanding the reasons behind that.
>
> As a general rule RELA relocations are preferable in that they
> are simpler - it is easier to extract the addend - and in theory
> they have the possibility to express a wider range of addends
> than would be available to REL relocations.
>
> Have you tried modifying the AArch64 code in the BFD library to
> support REL relocations ?  In theory all that you need to do is
> to change a few lines in bfd/elfnn-aarch64.c:
>
>   #define elf_backend_may_use_rel_p      0
>
> and maybe:
>
>   #define elf_backend_may_use_rela_p     1
>   #define elf_backend_default_use_rela_p 1
>   #define elf_backend_rela_normal     1
>
> In practice I am pretty sure that there will be assumptions of RELA
> relocs elsewhere in the AArch64 specific code, so I doubt it will be
> as simple as I have suggested.
>
> Cheers
>   Nick

	

Re: load of in-place relocation addend in AArch64

[Nick Clifton]

Hi Frédéric,

> However, I believe that ld AArch64 should not support REL entries at all (with a clear error message) instead of silently generating a wrong object file.

That is a good point.  Would you happen to have a small binary
file containing one or more REL relocs which I can use for testing ?

I would like to add the error message to the linker and I would like
a way to test to make sure that it works properly.

Cheers
   Nick

Re: load of in-place relocation addend in AArch64

[Frédéric Rivière]

[-- Attachment #1.1: Type: text/plain, Size: 601 bytes --]

Sure with pleasure !

Here's a small test file attached.

Thanks,

--Frédéric

On 12/6/2023 4:31 PM, Nick Clifton wrote:
> Hi Frédéric,
>
>> However, I believe that ld AArch64 should not support REL entries at 
>> all (with a clear error message) instead of silently generating a 
>> wrong object file.
>
> That is a good point.  Would you happen to have a small binary
> file containing one or more REL relocs which I can use for testing ?
>
> I would like to add the error message to the linker and I would like
> a way to test to make sure that it works properly.
>
> Cheers
>   Nick

	



=

[-- Attachment #2: test_AArch64_with_REL.o --]
[-- Type: application/octet-stream, Size: 2982 bytes --]

Re: load of in-place relocation addend in AArch64

[Nick Clifton]

[-- Attachment #1: Type: text/plain, Size: 1247 bytes --]

Hi Frédéric,

   OK, I have produced a small patch (attached) which adds this warning
   message to the linker's output:

     $ ld test_AArch64_with_REL.o
     ld: test_AArch64_with_REL.o: Warning: REL type relocations are not expected for architecture 'aarch64' - support may be incomplete

   This warning does not actually stop the link from completing however.
   The reason for this is that the code where the relocations are loaded
   is shared by other tools, which do still work with REL relocations.
   For example the objdump tool is able to display the relocations without
   any problems:

     $ objdump -r test_AArch64_with_REL.o
     BFD: test_AArch64_with_REL.o: Warning: REL type relocations are not expected for architecture 'aarch64' - support may be incomplete

     test_AArch64_with_REL.o:     file format elf64-littleaarch64

     RELOCATION RECORDS FOR [.text.__icetea__virtual__com_is2t_icetea_support_test_BImpl___bar__I]:
     OFFSET           TYPE              VALUE
     0000000000000008 R_AARCH64_ABS32   com_is2t_icetea_support_test_AImpl___bar
     [...]

   This strikes me as a compromise solution at best, but it is better
   than the current situation nonetheless.

   What do you think ?

Cheers
   Nick




[-- Attachment #2: rel.warning.patch --]
[-- Type: text/x-patch, Size: 1589 bytes --]

diff --git a/bfd/elf.c b/bfd/elf.c
index db45d1a074b..02246b1360c 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -2928,6 +2928,44 @@ bfd_section_from_shdr (bfd *abfd, unsigned int shindex)
 	    goto success;
 	  }
 
+	/* Check to make sure that the target supports this type of relocation.
+	   Note - we do not treat lack of support as a failure, since we may
+	   just be displaying information from an experimental file that is
+	   using an unexpected relocation type.  Plus if we 'goto fail' here
+	   our error message will be suppressed by the format matching code.
+	   Instead we tell the users that they might not be getting the
+	   results that they expect.  */
+	static bool already_warned = false;  /* Only warn once.  */
+	if (! already_warned)
+	  {
+	    bool supported;
+	    const char * rel_name;
+
+	    switch (hdr->sh_type)
+	      {
+	      case SHT_REL:
+		supported = get_elf_backend_data (abfd)->may_use_rel_p;
+		rel_name = "REL";
+		break;
+
+	      case SHT_RELA:
+		supported = get_elf_backend_data (abfd)->may_use_rela_p;
+		rel_name = "RELA";
+		break;
+
+	      default:
+		supported = true;
+	      }
+
+	    if (! supported)
+	      {
+		/* xgettext:c-format */
+		_bfd_error_handler (_("%pB: Warning: %s type relocations are not expected for architecture '%s' - supprt may be incomplete"),
+				    abfd, rel_name, bfd_printable_name (abfd));
+		already_warned = true;
+	      }
+	  }
+
 	/* Get the symbol table.  */
 	if ((elf_elfsections (abfd)[hdr->sh_link]->sh_type == SHT_SYMTAB
 	     || elf_elfsections (abfd)[hdr->sh_link]->sh_type == SHT_DYNSYM)

Re: load of in-place relocation addend in AArch64

[Frédéric Rivière]

Hi Nick,

I understand the reason and I agree with a warning, because in the 
meantime I realized that REL relocations are correctly supported when 
there is no in-place addend.

So we could improve the message as following:

"Warning: REL type relocations are not expected for architecture 
'aarch64' - in-place addends with non-zero values are not supported."

Also I believe it would be perfect if this message could be printed only 
during the relocation process, to avoid impacting the other tools.

I'm not familiar with LD code base but is there any chance to be able to 
retrieve the original relocation kind when we are in the 
"elfNN_aarch64_relocate_section" function ?

(for example by being able to retrieve the original relocation section 
type or the relocation entry size for example)

Best Regards,

--Frédéric

On 12/7/2023 6:16 PM, Nick Clifton wrote:
> Hi Frédéric,
>
>   OK, I have produced a small patch (attached) which adds this warning
>   message to the linker's output:
>
>     $ ld test_AArch64_with_REL.o
>     ld: test_AArch64_with_REL.o: Warning: REL type relocations are not 
> expected for architecture 'aarch64' - support may be incomplete
>
>   This warning does not actually stop the link from completing however.
>   The reason for this is that the code where the relocations are loaded
>   is shared by other tools, which do still work with REL relocations.
>   For example the objdump tool is able to display the relocations without
>   any problems:
>
>     $ objdump -r test_AArch64_with_REL.o
>     BFD: test_AArch64_with_REL.o: Warning: REL type relocations are 
> not expected for architecture 'aarch64' - support may be incomplete
>
>     test_AArch64_with_REL.o:     file format elf64-littleaarch64
>
>     RELOCATION RECORDS FOR 
> [.text.__icetea__virtual__com_is2t_icetea_support_test_BImpl___bar__I]:
>     OFFSET           TYPE              VALUE
>     0000000000000008 R_AARCH64_ABS32 
> com_is2t_icetea_support_test_AImpl___bar
>     [...]
>
>   This strikes me as a compromise solution at best, but it is better
>   than the current situation nonetheless.
>
>   What do you think ?
>
> Cheers
>   Nick
>
>
>
-- 
MicroEJ <http://www.microej.com> 	

Frédéric RIVIERE
/CTO Office Member & VP Product/

11 rue du Chemin Rouge - Bat. D 44373 Nantes Cedex 3, France 
https://www.linkedin.com/in/fredericriviere/

Twitter <https://twitter.com/microej> LinkedIn 
<https://www.linkedin.com/company/microej> Youtube 
<https://www.youtube.com/@MicroEJ_News>

event <https://www.microej.com/event/>

This email (and all attachments) may contain information that is 
proprietary privileged, and/or confidential. If you received this e-mail 
in error or if it was improperly forwarded to you, the information 
contained in the e-mail should, at all times, remain confidential. 
Please notify the sender immediately by e-mail, and delete or destroy 
the original and any copies. Any unauthorized use, disclosure, 
reproduction, retention or distribution by any person other than the 
intended recipient is strictly prohibited.

Re: load of in-place relocation addend in AArch64

[Fangrui Song]

On Sun, Dec 10, 2023 at 11:51 PM Frédéric Rivière
<frederic.riviere@microej.com> wrote:
>
> Hi Nick,
>
> I understand the reason and I agree with a warning, because in the
> meantime I realized that REL relocations are correctly supported when
> there is no in-place addend.
>
> So we could improve the message as following:
>
> "Warning: REL type relocations are not expected for architecture
> 'aarch64' - in-place addends with non-zero values are not supported."
>
> Also I believe it would be perfect if this message could be printed only
> during the relocation process, to avoid impacting the other tools.
>
> I'm not familiar with LD code base but is there any chance to be able to
> retrieve the original relocation kind when we are in the
> "elfNN_aarch64_relocate_section" function ?
>
> (for example by being able to retrieve the original relocation section
> type or the relocation entry size for example)
>
> Best Regards,
>
> --Frédéric

I'd love to see better REL support :), at least for static data
relocations (generated from .long/.quad).

While REL is often inadequate for static code relocations because the
instruction to be modified (commonly 4 bytes on RISC architectures)
limits the available space for the addend,
using RELA can be unnecessarily bulky for data and debug sections
where the constraints on addend size don't apply.

I have made a quick analysis using a modified clang.
In a -O0 -g -gpubnames build, using REL for non-code sections
(sections without the SHF_EXECINSTR flag) decreased relocation size by
27.1% and the .o file size by 6.4%.

```
         |reloc size | .o size
---------+-----------+------------
RELA     | 550519056 | 2339938120
REL data | 401209104 | 2190607000  -Wa,--implicit-addends-for-data
CREL     |  44865612 | 1834284744  -Wa,--crel,--implicit-addends-for-data
```

I have filed https://sourceware.org/bugzilla/show_bug.cgi?id=31567
("gas/ld: Implicit addends for non-code sections").
LLVM RFC: https://discourse.llvm.org/t/rfc-implicit-addends-for-non-code-sections/78000

> On 12/7/2023 6:16 PM, Nick Clifton wrote:
> > Hi Frédéric,
> >
> >   OK, I have produced a small patch (attached) which adds this warning
> >   message to the linker's output:
> >
> >     $ ld test_AArch64_with_REL.o
> >     ld: test_AArch64_with_REL.o: Warning: REL type relocations are not
> > expected for architecture 'aarch64' - support may be incomplete
> >
> >   This warning does not actually stop the link from completing however.
> >   The reason for this is that the code where the relocations are loaded
> >   is shared by other tools, which do still work with REL relocations.
> >   For example the objdump tool is able to display the relocations without
> >   any problems:
> >
> >     $ objdump -r test_AArch64_with_REL.o
> >     BFD: test_AArch64_with_REL.o: Warning: REL type relocations are
> > not expected for architecture 'aarch64' - support may be incomplete
> >
> >     test_AArch64_with_REL.o:     file format elf64-littleaarch64
> >
> >     RELOCATION RECORDS FOR
> > [.text.__icetea__virtual__com_is2t_icetea_support_test_BImpl___bar__I]:
> >     OFFSET           TYPE              VALUE
> >     0000000000000008 R_AARCH64_ABS32
> > com_is2t_icetea_support_test_AImpl___bar
> >     [...]
> >
> >   This strikes me as a compromise solution at best, but it is better
> >   than the current situation nonetheless.
> >
> >   What do you think ?
> >
> > Cheers
> >   Nick
> >
> >
> >
> --
> MicroEJ <http://www.microej.com>
>
> Frédéric RIVIERE
> /CTO Office Member & VP Product/
>
> 11 rue du Chemin Rouge - Bat. D 44373 Nantes Cedex 3, France
> https://www.linkedin.com/in/fredericriviere/
>
> Twitter <https://twitter.com/microej> LinkedIn
> <https://www.linkedin.com/company/microej> Youtube
> <https://www.youtube.com/@MicroEJ_News>
>
> event <https://www.microej.com/event/>
>
> This email (and all attachments) may contain information that is
> proprietary privileged, and/or confidential. If you received this e-mail
> in error or if it was improperly forwarded to you, the information
> contained in the e-mail should, at all times, remain confidential.
> Please notify the sender immediately by e-mail, and delete or destroy
> the original and any copies. Any unauthorized use, disclosure,
> reproduction, retention or distribution by any person other than the
> intended recipient is strictly prohibited.
>