This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] ARM: Add support for SHF_ARM_NOREAD section flag
- From: Christophe MONAT <christophe dot monat at st dot com>
- To: Andre Vieira <Andre dot SimoesDiasVieira at foss dot arm dot com>, Nick Clifton <nickc at redhat dot com>
- Cc: "binutils at sourceware dot org" <binutils at sourceware dot org>, Mickael GUENE <mickael dot guene at st dot com>
- Date: Fri, 8 Jan 2016 14:00:38 +0100
- Subject: Re: [PATCH] ARM: Add support for SHF_ARM_NOREAD section flag
- Authentication-results: sourceware.org; auth=none
- References: <5671884D dot 70702 at st dot com> <alpine dot DEB dot 2 dot 10 dot 1512161815480 dot 15602 at digraph dot polyomino dot org dot uk> <5673F93D dot 5080501 at st dot com> <56795B20 dot 8070802 at redhat dot com> <568EAF73 dot 40002 at foss dot arm dot com>
Hi Andre, Nick,
[Sorry that Mickael does not answer by himself, he was not subscribed to
the list so far -- now corrected].
On 01/07/16 19:33, Andre Vieira wrote:
Terry sent a patch upstream to handle the noread attribute in 2014:
https://sourceware.org/ml/binutils/2014-04/msg00181.html
Having seen this patch I believe the approach taken here to use section
names to represent the noread attribute in assembly is inferior to
Terry's approach.
For the GCC implementation of either an attribute or compile option for
execute-only we should not use section names to represent the noread
attribute, since for instance that means it can not be combined with
-ffunction-sections, or any other option that sets section names for
functions.
We disagree with that specific point : the section names that we emit
when gcc is using -ffunction-sections is in the form of:
.text.noread.*
which are perfectly caught and handled (it matters to us - we just
checked this).
I would like to rebase Terry's patch and make the necessary changes to
it, slightly different attribute name and so on, and use that instead of
this patch.
Would there be any objections to this?
The binutils patch that we contributed was in the perspective of
up-streaming the so-called PCROP support also in gcc also - the gcc
proposal is completed on our side, but still not public.
For the matter of marking the .text sections read-only, we tried the two
following strategies (and choose 2)):
1) keep the .text sections' names and emit the noread attribute in the
assembly (with the very same 'y' key) : we failed doing that because of
the specific treatment done by gas, that in the end *ignores* at some
final point the custom attributes on the pure .text name.
This implies that the .text sections in noread mode cannot be called
.text but must be called .text.something (which in your patch's tests
appears as .text.foo, otherwise the noread attribute would not been have
accounted for).
In addition from the gcc standpoint, this emission forces to duplicate
emission code since the particular place we need to touch is hook-able,
but requires a complete duplication of the section attribute emission
(default_elf_asm_named_section) just to add two lines dealing with the
'y' case.
2) emit some .text.noread sections (that work nicely with
-ffunction-sections), without even requiring a section attribute change,
and delegating the treatment to gas that handles specifically those
sections.
This to write that your patch is certainly very good (just regretting
that it was not contributed at that time) but leads to uglier code in
gcc, which you may consider a problem.
Would it be possible to simply add the 'y' support from your patch, this
would not break our changes and fulfill your purposes ?
Best regards,
--C