This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH][AArch64] PR92424: Fix -fpatchable-function-entry=N,M with BTI


On 21/01/2020 11:34, Mark Rutland wrote:
> Hi Szabolcs,
> 
> Answers from a linux dev perspective below.

thanks.

> On Mon, Jan 20, 2020 at 10:53:33AM +0000, Szabolcs Nagy wrote:
>> i have to ask some linux developers which way they prefer:
>>
>> e.g. -fpatchable-function-entry=3,1 is
>>
>>  .section __patchable_function_entries
>>  .8byte .Lpatch
>>  .text
>> .Lpatch:
>>   nop
>> func:
>>   nop
>>   nop
>>   ...
>>
>> with bti the code will be emitted as:
>>
>> .Lpatch:
>>   nop
>> func:
>>   bti c
>>   nop
>>   nop
>>   ...
> 
> That looks good to me.
> 
>> but e.g. -fpatchable-function-entry=2,0 has two reasonable
>> approaches with bti:
>>
>> (a)
>>
>> func:
>> .Lpatch:
>>   bti c
>>   nop
>>   nop
>>   ...
>>
>> (b)
>>
>> func:
>>   bti c
>> .Lpatch:
>>   nop
>>   nop
>>   ...
> 
> I had assumed (b); that means that .Lpatch consistently points to the
> first NOP. To my mental model, that seems more consistent than (a).
> 
> However, I can work with either so long as it's consistent.
...
> As above, my weak preference is (b), but I can work with either. I just
> need the behaviour to be consistent.
> 
> Was there a rationale for LLVM choosing (a) rather than (b), e.g. was
> that also ease of implementation? If there isn't a rationale otherwise,
> and if LLVM could also do (b), that would be nice from my PoV.
> 
> How big is "a bit more changes" for GCC?

".Lpatch: nop; nop" is generated by generic code now which
a backend can override, but then i have to copy that logic
(~30 lines) to add something after the .Lpatch, or i have
to change generic code to have a new hook after the .Lpatch:.

to provide more argument for (b) and expand on consistency:

right now bti is not emitted if a function is not called
indirectly (compiler knows this for some local functions)
or bti can be turned off by a target attribute per function.

so a mix of bti and non-bti functions are possible, so
the user patching code has to deal with this (e.g. read
the instructions before patching etc)

in the M>0, M!=N case bti is emitted in the middle of a
patch area (e.g. 3,1 case above), but some functions will
not have bti. i can make it so that if patchable function
entry is requested then bti is always added, irrespective
of any other option or attribute, but that seems to be a
big hammer, without such big hammer solution users will
have a difficult time to deal with the additional bti.

if the patch area is entirely before the function label,
then no changes are needed (M=N), if patch area is after
the function label (M=0) then using fix (b) would allow
simple user code. in other cases user code will be
complicated no matter what. so if linux uses M=0 then i
think (b) is preferable.

hope this helps.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]