Re: [RFA][PATCH] Stack clash protection 07/08 -- V4 (aarch64 bits)

[Jeff Law <law@redhat.com>]

On 11/27/2017 08:48 AM, Szabolcs Nagy wrote:
> On 28/10/17 05:08, Jeff Law wrote:
>> On 10/13/2017 02:26 PM, Wilco Dijkstra wrote:
>>> For larger frames the first oddity is that there are now 2 separate params
>>> controlling how probes are generated:
>>>
>>> stack-clash-protection-guard-size (default 12, but set to 16 on AArch64)
>>> stack-clash-protection-probe-interval (default 12)
>>>
>>> I don't see how this makes sense. These values are closely related, so if
>>> one is different from the other, probing becomes ineffective/incorrect. 
>>> For example we generate code that trivially bypasses the guard despite
>>> all the probing:
>> My hope would be that we simply don't ever use the params.  They were
>> done as much for *you* to experiment with as anything.  I'd happy just
>> delete them as there's essentially no guard rails to ensure their values
>> are sane.
> 
> so is there a consensus now that 64k guard size is what
> gcc stack probing will assume?
Only aarch64 currently assumes a guard that large.  Other targets are
still assuming a single 4k page guard.

I'd certainly like to see a larger guard, at least on the 64bit targets.
 Address space is at a much higher premium on the 32bit targets so it's
less clear if we can really bump the guard size on them.


> 
>>> Also on AArch64 --param=stack-clash-protection-probe-interval=16 causes
>>> crashes due to the offsets used in the probes - we don't need large offsets
>>> as we want to probe close to the bottom of the stack.
>> Not a surprise.  While I tried to handle larger intervals, I certainly
>> didn't test them.  Given the ISA I wouldn't expect an interval > 12 to
>> be useful or necessarily even work correctly.
> 
> it's not clear what makes probing at every 64k hard,
> i think this should be clarified before we stick to
> this design. (..or before backporting such patches)
You just need to be able to find additional scratch registers then the
probe interval is > 12 on aarch64.  It also impacts the dwarf2 unwinding
notes.

jeff