Re: [PATCH] ARM: Replace catch of noread sections by section name by a section attribute letter.

["Andre Vieira (lists)" <Andre dot SimoesDiasVieira at arm dot com>]

Hi Mickael,
On 12/01/16 13:40, mickael guene wrote:
> Hi all,
>
>     This is a proposal for a patch that replaces the catch of noread
> sections by section name by a section attribute letter.
>     I have taken changes from Terry Guo's patch[1] for documentation and
> for section-execute-only.s test.
>
>     No regressions have been observed for arm-linux-gnueabi,
> arm-linux-gnueabihf, arm-none-eabi, arm-none-nacl,
> armeb-linux-gnueabihf, arm-netbsdelf and arm-vxworks targets
> on 64-bit Linux host.
>
> [1]: https://sourceware.org/ml/binutils/2014-04/msg00181.html
>
> Changelogs:
>
> bfd/ChangeLog:
>
> 2016-01-12 Mickael Guene <mickael.guene@st.com>
>
>      * elf32-arm.c ((elf32_arm_special_sections): Remove catch of noread
>      section using '.text.noread' pattern.
>
> gas/ChangeLog:
>
> 2016-01-12 Mickael Guene <mickael.guene@st.com>
>
>      * config/obj-elf.c (obj_elf_change_section) : Allow arm section with
>      SHF_ARM_NOREAD section flag.
>      * config/tc-arm.h (md_elf_section_letter) : Implement this hook to
>      handle letter 'y'.
>      (arm_elf_section_letter) : Declare it.
>      * config/tc-arm.c (arm_elf_section_letter): Handle letter 'y' to set
>      SHF_ARM_NOREAD section flag.
>      * doc/c-arm.texi (ARM section attribute 'y'): Document it.
>
> gas/testsuite/ChangeLog:
>
> 2016-01-12  Terry Guo  <terry.guo@arm.com>
>
>      * gas/arm/section-execute-only.s: New test case.
>      * gas/arm/section-execute-only.d: Expected output.
>
> ld/testsuite/ChangeLog:
>
> 2016-01-12 Mickael Guene <mickael.guene@st.com>
>
>      * ld-arm/thumb1-noread-not-present-mixing-two-section.s: Add 'y'
>      attribute usage.
>      * ld-arm/thumb1-noread-present-one-section.s: Likewise.
>      * ld-arm/thumb1-noread-present-two-section.s: Likewise.
>      * ld-arm/thumb1-input-section-flag-match.s: Likewise.

Thanks for this!

I have a small comment though. I see you left out the part of Terry's 
patch for readelf, see his changes to binutils/readelf.c, where it 
checks for the target being ARM to actually name the section attribute 
'y' for SHF_ARM_NOREAD instead of using the "target specific section 
attribute" flag 'p'.

I think it would be better to have readelf print out the 'y' for this 
case instead of 'p'.

Cheers,
Andre