This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH, ARM 4/7] Add support for linking ARMv8-M object files
- From: "Richard Earnshaw (lists)" <Richard dot Earnshaw at arm dot com>
- To: Thomas Preud'homme <thomas dot preudhomme at foss dot arm dot com>, binutils at sourceware dot org
- Date: Wed, 23 Dec 2015 14:04:05 +0000
- Subject: Re: [PATCH, ARM 4/7] Add support for linking ARMv8-M object files
- Authentication-results: sourceware.org; auth=none
- References: <001d01d13873$10d9aab0$328d0010$ at foss dot arm dot com> <56741D1E dot 50203 at arm dot com> <004301d13d4f$dc651da0$952f58e0$ at foss dot arm dot com>
On 23/12/15 07:02, Thomas Preud'homme wrote:
> Hi Richard,
>
>> From: binutils-owner@sourceware.org [mailto:binutils-
>> owner@sourceware.org] On Behalf Of Richard Earnshaw (lists)
>> Sent: Friday, December 18, 2015 10:50 PM
>>
>>>
>>>
>>> diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
>>> index
>> 49dfc5338e45fd1df518cc8781e918444dc1831e..b10a3430e2d0cd5ad22f13
>> 6aa5f706506a40759e 100644
>>> --- a/bfd/elf32-arm.c
>>> +++ b/bfd/elf32-arm.c
>>> @@ -3445,18 +3445,8 @@ create_ifunc_sections (struct bfd_link_info
>> *info)
>>> static bfd_boolean
>>> using_thumb_only (struct elf32_arm_link_hash_table *globals)
>>> {
>>> - int arch = bfd_elf_get_obj_attr_int (globals->obfd, OBJ_ATTR_PROC,
>>> - Tag_CPU_arch);
>>> - int profile;
>>> -
>>> - if (arch == TAG_CPU_ARCH_V6_M || arch ==
>> TAG_CPU_ARCH_V6S_M)
>>> - return TRUE;
>>> -
>>> - if (arch != TAG_CPU_ARCH_V7 && arch != TAG_CPU_ARCH_V7E_M)
>>> - return FALSE;
>>> -
>>> - profile = bfd_elf_get_obj_attr_int (globals->obfd, OBJ_ATTR_PROC,
>>> - Tag_CPU_arch_profile);
>>> + int profile = bfd_elf_get_obj_attr_int (globals->obfd,
>> OBJ_ATTR_PROC,
>>> + Tag_CPU_arch_profile);
>>>
>>> return profile == 'M';
>>> }
>>
>>
>> I don't think it's completely mandatory that when an Arch tag specifies
>> an M-profile architecture that setting the profile tag is also required
>> (don't forget that we may be processing object files produced by other
>> toolchains so we have to be as liberal as we can be in terms of what we
>> accept). So the following two test files are probably legal, although
>> not standard with the GNU tools.
>
> You're absolutely right. Be liberal in what you accept, and strict in what you produce.
>
>>
>> t1.s:
>> .eabi_attribute Tag_CPU_arch, 11 @ V6-M
>> .thumb
>> .type _start, function
>> .global _start
>> .text
>> _start:
>> bl myfunc
>> b .
>>
>> t2.s:
>> .eabi_attribute Tag_CPU_arch, 11 @ V6-M
>> .thumb
>> .type myfunc, function
>> .global myfunc
>> .text
>> .space 102400000
>> myfunc:
>> bx lr
>>
>> Now, if I link these together, I expect the linker to generate a veneer
>> which is compatible with execution on ARMv6-M devices. With the
>> current
>> linker I get that behaviour:
>>
>> 00008000 <_start>:
>> 8000: f000 f802 bl 8008 <__myfunc_veneer>
>> 8004: e7fe b.n 8004 <_start+0x4>
>> ...
>>
>> 00008008 <__myfunc_veneer>:
>> 8008: b401 push {r0}
>> 800a: 4802 ldr r0, [pc, #8] ; (8014
>> <__myfunc_veneer+0xc>)
>> 800c: 4684 mov ip, r0
>> 800e: bc01 pop {r0}
>> 8010: 4760 bx ip
>> 8012: bf00 nop
>> 8014: 061b0019 .word 0x061b0019
>> ...
>>
>> 00080000 <_stack>:
>> ...
>>
>> 061b0018 <myfunc>:
>> 61b0018: 4770 bx lr
>>
>> However, when I apply your patch I then get the tools creating veneers
>> that use instructions that do not exist on M-profile devices:
>>
>> 00008000 <_start>:
>> 8000: f000 e802 blx 8008 <__myfunc_veneer>
>> 8004: e7fe b.n 8004 <_start+0x4>
>> ...
>>
>> 00008008 <__myfunc_veneer>:
>> 8008: e51ff004 ldr pc, [pc, #-4] ; 800c
>> <__myfunc_veneer+0x4>
>> 800c: 061b0011 .word 0x061b0011
>> ...
>>
>> 00080000 <_stack>:
>> ...
>>
>> 061b0010 <myfunc>:
>> 61b0010: 4770 bx lr
>>
>> which is clearly not what we want.
>>
>> If you're going to continue to examine the build attributes directly
>> rather than internalising them and canonicalizing the set of attributes
>> into a consistent internal description of the machine capabilities, then
>> I think you need to continue to explicitly check the architecture tags
>> as well here.
>
> This is fixed in this updated patch. I reworked the logic a tad to look at profile first and use that if available, otherwise fallback on arch information. Here are the updated ChangeLog entries and patch:
>
> *** bfd/ChangeLog ***
>
> 2015-12-22 Thomas Preud'homme <thomas.preudhomme@arm.com>
>
> * elf32-arm.c (using_thumb_only): Check that profile is 'M' and update
> logic around Tag_CPU_arch values to return TRUE for ARMv8-M
> architectures.
> (tag_cpu_arch_combine): Define v8m_baseline and v8m_mainline and update
> v4t_plus_v6_m and comb to deal with ARMv8-M Tag_CPU_arch merging logic.
> (elf32_arm_merge_eabi_attributes): Add Tag_CPU_name values for
> ARMv8-M.
>
>
> *** bfd/testsuite/ChangeLog ***
>
> 2015-12-14 Thomas Preud'homme <thomas.preudhomme@arm.com>
>
> * ld-arm/arm-elf.exp (armeabitests_common): Run new tests
> "Thumb-Thumb farcall v8-M", "EABI attribute merging 8",
> "EABI attribute merging 9" and "EABI attribute merging 10".
> (Thumb-Thumb farcall v8-M): Renamed to ...
> (Thumb-Thumb farcall v8-M Mainline): This.
> (Thumb-Thumb farcall v8-M Baseline): New test.
> * ld-arm/attr-merge-8a.s: New file.
> * ld-arm/attr-merge-8b.s: Likewise.
> * ld-arm/attr-merge-8.attr: Likewise.
> * ld-arm/attr-merge-9a.s: Likewise.
> * ld-arm/attr-merge-9b.s: Likewise.
> * ld-arm/attr-merge-9.out: Likewise.
> * ld-arm/attr-merge-10a.s: Likewise.
> * ld-arm/attr-merge-10b.s: Likewise.
> * ld-arm/attr-merge-10.attr: Likewise.
>
>
This is OK.
As a follow-up could you please convert the example I gave into a
testcase? I'll pre-approve that :-)
R.