Re: [PATCH] ARM: Add support for thumb1 PCROP relocations.

[Ramana Radhakrishnan <ramana dot gcc at googlemail dot com>]

Hi Mickael,

On Thu, Dec 3, 2015 at 9:27 AM, mickael guene <mickael.guene@st.com> wrote:
> Hi Ramana,
>
>  Thanks for your feedback. I agree with you about gas
> directives names. Your names proposal are better than
> mines, but I'm still not happy with them since for instance
> 'lower0_8' seems to imply  bit  numbers (but then should be lower0_7)
> whereas pre-existing 'lower16' seems to imply a size.

I do not think it's perfect and if anyone has a better name I'm happy
to hear of it. My reasoning for this suggestion was that the ranges
would usually imply bit ranges.  (Agreed on 7 rather than 8). Please
also make sure you update the documentation in c-arm.texi for these
new directives.


regards
Ramana

>
> Regards
> Mickael
>
>
> On 12/01/2015 04:31 PM, Ramana Radhakrishnan wrote:
>>
>> On Mon, Nov 23, 2015 at 8:43 AM, mickael guene <mickael.guene@st.com>
>> wrote:
>>>
>>> Hi all,
>>>
>>> This is a proposal for a patch that implements the new ARM relocations
>>> needed to support execute-only code segment for thumb1 code sequences.
>>>
>>> Some STM32 MCUs implement a security feature called 'Proprietary Code
>>> Read-Out Protection' aka PCROP that forbids data read access to some
>>> code areas (only fetch access is allowed).
>>> This protection prevents usage of literal pools (since one cannot load
>>> data from code sections), so compilers have to use specific code
>>> sequence to generate constants. This is particularly a problem to
>>> generate symbol addresses.
>>>
>>> For armv7-m architecture compilers can generate movt/movw pair
>>> instructions relying on R_ARM_THM_MOVW_ABS_NC and R_ARM_THM_MOVT_ABS
>>> relocations support.
>>> For armv6-m there is currently no possibility to generate such symbol
>>> addresses.
>>> This is why the ARM ABI defines four new relocations that allow
>>> symbol address generation in armv6-m architecture using a sequence of
>>> movs/adds/lsls instructions.
>>>
>>> This patch implements this feature in gas and bfd.
>>>
>>> Unit tests have be written to check correct implementation for both
>>> gas and bfd.
>>> No regressions have been observed for arm-linux-gnueabi,
>>> arm-linux-gnueabihf, arm-none-eabi, arm-none-nacl and
>>> armeb-linux-gnueabihf targets on 64 bits linux host.
>>>
>>
>>
>> I am not happy about directives like low_low, low_high, high_low and
>> high_high - without thinking much about it, I would much rather see
>> something like lower0_8 , lower8_16 , upper0_8 and upper8_16 in line
>> with the pre-existing lower16 and upper16 directives.
>>
>> regards
>> Ramana
>>
>>
>>
>>
>>> Changelogs:
>>>
>>> include/ChangeLog:
>>>
>>> 2015-11-17  Mickael Guene <mickael.guene@st.com>
>>>
>>>          * elf/arm.h: Add new arm relocations.
>>>
>>> bfd/ChangeLog:
>>>
>>> 2015-11-17  Mickael Guene <mickael.guene@st.com>
>>>
>>>          * bfd-in2.h: Regenerate.
>>>          * reloc.c: Add new relocations.
>>>          * libbfd.h (bfd_reloc_code_real_names): Add new relocations
>>> display
>>>          names.
>>>          * elf32-arm.c (elf32_arm_howto_table_1): Add HOWTO for new
>>> relocations.
>>>          (elf32_arm_reloc_map): Add bfd/arm mapping for new relocations.
>>>          (elf32_arm_final_link_relocate): Implement new relocations
>>> resolution.
>>>
>>> gas/ChangeLog:
>>>
>>> 2015-11-17  Mickael Guene <mickael.guene@st.com>
>>>
>>>          * config/tc-arm.c (group_reloc_table): Add mapping between gas
>>> syntax
>>>          and new relocations.
>>>          (do_t_add_sub): Keep new relocations for add operand.
>>>          (do_t_mov_cmp): Keep new relocations for mov operand.
>>>          (insns): Use 'shifter operand with possible group relocation'
>>> operand
>>>          parse code for movs operand.
>>>          (md_apply_fix): Implement mov and add encoding when new
>>> relocations
>>>          on them.
>>>          (tc_gen_reloc): Add new relocations.
>>>          (arm_fix_adjustable): Since offset has a limited range ([0:255])
>>> we
>>>          disable adjust_reloc_syms() for new relocations.
>>>
>>> gas/testsuite/ChangeLog:
>>>
>>> 2015-11-17  Mickael Guene <mickael.guene@st.com>
>>>
>>>          * gas/arm/adds-thumb1-reloc-local.d: New
>>>          * gas/arm/adds-thumb1-reloc-local.s: New
>>>          * gas/arm/movs-thumb1-reloc-local.d: New
>>>          * gas/arm/movs-thumb1-reloc-local.s: New
>>>
>>> ld/testsuite/ChangeLog:
>>>
>>> 2015-11-17  Mickael Guene <mickael.guene@st.com>
>>>
>>>          * ld-arm/arm-elf.exp (armelftests_common): Add new relocations
>>> tests.
>>>          * ld-arm/thumb1-adds.d: New
>>>          * ld-arm/thumb1-adds.s: New
>>>          * ld-arm/thumb1-movs.d: New
>>>          * ld-arm/thumb1-movs.s: New