On Mon, Mar 19, 2018 at 1:50 AM, Sascha Hauer <s.hauer@xxxxxxxxxxxxxx> wrote:
> Hi Andrey,
>
> On Fri, Mar 16, 2018 at 02:51:19PM -0700, Andrey Smirnov wrote:
>> On Fri, Mar 16, 2018 at 5:53 AM, Sascha Hauer <s.hauer@xxxxxxxxxxxxxx> wrote:
>> > This adds aarch64 support for relocating binaries linked with -pie.
>> >
>> > Support is integrated into the already exisiting
>> > relocate_to_current_adr() function which is now used for both arm32
>> > and aarch64.
>> >
>> > Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
>>
>>
>> Sascha:
>>
>> Two small suggestions w.r.t. this patch:
>>
>> - I'd consider changing the code of relocate_to_current_adr() such
>> that AARCH64 specific codepaths are not taken on ARM32 (via IS_ENABLED
>> check or something similar)
>
> Why? Do you want to make the code more clear or do you fear that we
> apply fixups for a foreign architecture?
>
You'd be able to get the value of *_R_TYPE(type) once and then use
switch to structure code handling various types, which might be a bit
more readable. But that 50% personal preference, so feel free to keep
the code as is.
>>
>> - I've always wanted to fix the original code to use Elf32_rel type
>> instead of magic hard-coded offsets, so depending on your
>> willingness/time-budget, maybe now would be a good time to do that as
>> well as use Elf64_rela for AARCH64?
>
> You mean using
>
>> struct elf32_rel {
>> Elf32_Addr r_offset;
>> Elf32_Word r_info;
>> } Elf32_Rel;
>
> and:
>
>> struct elf64_rela {
>> Elf64_Addr r_offset; /* Location at which to apply the action */
>> Elf64_Xword r_info; /* index and type of relocation */
>> Elf64_Sxword r_addend; /* Constant addend used to compute value */
>> } Elf64_Rela;
>
> ?
Yep, those are the two types.
>
> Yes, I can change that. I wonder though which type is behind R_ARM_ABS32
>
I think its still would be Elf32_Rel. From what I can tell both
R_ARM_ABS32 and R_ARM_RELATIVE calculate "*fixup = *fixup + offset"
using same pointer offsets it's just former also adds "r" that is
derived from "r_info". With enough #ifdefing it might even be possible
to convert that while() to a for() loop.
Thanks,
Andrey Smirnov
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
