[PATCH] ARM: Add support for SHF_ARM_NOREAD section flag

[Andre Vieira (lists)]

On 08/01/16 13:33, Richard Earnshaw (lists) wrote:
> On 08/01/16 13:00, Christophe MONAT wrote:
>> 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 ?
>>
>
> Just a quick comment, I don't currently have time to dig into this deeply.
>
> My main concern is that requiring a specific template in the section
> name feels like a cludge.  If gas/ld is not correctly dealing with the
> section flags, then that is a bug that ought to be fixed properly.
> Working around that by inventing new special meta-data is not helpful --
> don't forget that other linkers besides GNU LD have to be able to
> process these sections (and GNU LD has to process object files produced
> by tools other than GCC/GAS).
>
> R.
>
>> Best regards,
>> --C
>

So yes the unnamed, aka '.text', section can not be used as is because 
the assembler has a "default" .text with attributes 'ax' and when it 
merges with the .text from the incoming assembly it drops other attributes.

This however is not a problem for gcc as the plans for an execute-only 
option were to name any unnamed section that was noread and I don't mind 
using .text.noread as a default. However, my opinion is that that should 
not be the denominator for a 'noread' section. Instead it should really 
just be seen as a way to circumvent this property of unnamed sections.

I don't know why unnamed sections have this behavior though, I will try 
to dig that one up and see if maybe we ought to change the merging 
behavior of unnamed sections with different attributes.

I currently see three approaches we could take in the assembler to deal 
with unnamed sections with 'noread' ('y'):
1) merge all '.text' sections and if one of them is 'noread', all should 
be (not a big fan!);
2) rename, at assemble time, all '.text' sections that have the 'noread' 
attribute to '.text.noread';
3) don't merge (current behavior), but warn about dropping the 
attribute, the code for this is already there, it just doesnt check for 
the NOREAD flag.

My favorite approach would be 3. That way the user would be hinted that 
he/she should name their 'noread' section.

Cheers,
Andre