Re: [PATCH] Fix stack red zone bug (PR38644)

[Richard Guenther <richard dot guenther at gmail dot com>]

On Fri, Sep 30, 2011 at 2:46 PM, Richard Sandiford
<richard.sandiford@linaro.org> wrote:
> "Jiangning Liu" <jiangning.liu@arm.com> writes:
>>> You seem to feel strongly about this because it's a wrong-code bug that
>>> is very easy to introduce and often very hard to detect. ?And I
>>> defintely
>>> sympathise with that. ?If we were going to to do it in a target-
>>> independent
>>> way, though, I think it would be better to scan patterns like epilogue
>>> and
>>> automatically introduce barriers before assignments to
>>> stack_pointer_rtx
>>> (subject to the kind of hook in your patch). ?But I still don't think
>>> that's better than leaving the onus on the backend. ?The backend is
>>> still responsible for much more complicated things like determning
>>> the correct deallocation and register-restore sequence, and for
>>> determining the correct CFI sequence.
>>>
>>
>> I think middle-end in GCC is actually shared code rather than the part
>> exactly in the middle. A pass working on RTL can be a middle end just
>> because the code can be shared for all targets, and some passes can even
>> work for both GIMPLE and RTL.
>>
>> Actually some optimizations need to work through "shared part" (middle-end)
>> plus "target specific part" (back-end). You are thinking the interface
>> between this "shared part" and "target specific part" should be using
>> "barrier" as a properly model. To some extension I agree with this. However,
>> it doesn't mean the fix should be in back-end rather than middle end,
>> because obviously this problem is a common ABI issue for all targets. If we
>> can abstract this issue to be a shared part, why shouldn't we do it in
>> middle end to reduce the onus of back-end? Back-end should handle the target
>> specific things rather than only the complicated things.
>
> And for avoidance of doubt, the automatic barrier insertion that I
> described would be one way of doing it in target-independent code.
> But...
>
>> If a complicated problem can be implemented in a "shared code" manner, we
>> still want to put it into middle end rather than back-end. I believe those
>> optimizations based on SSA form are complicated enough, but they are all in
>> middle end. This is the logic I'm seeing in GCC.
>
> The situation here is different. ?The target-independent rtl code is
> being given a blob of instructions that the backend has generated for
> the epilogue. ?There's no fine-tuning beyond that. ?E.g. we don't have
> separate patterns for "restore registers", "deallocate stack", "return":
> we just have one monolithic "epilogue" pattern. ?The target-independent
> code has very little control.
>
> In contrast, after the tree optimisers have handed off the initial IL,
> the tree optimisers are more or less in full control. ?There are very
> few cases where we generate further trees outside the middle-end. ?The only
> case I know off-hand is the innards of va_start and va_arg, which can be
> generated by the backend.
>
> So let's suppose we had a similar situation there, where we wanted
> va_arg do something special in a certain situation. ?If we had the
> same three choices of:
>
> ?1. use an on-the-side hook to represent the special something
> ?2. scan the code generated by the backend and automatically
> ? ? inject the special something at an appropriate place
> ?3. require each backend to do it properly from the start
>
> (OK, slightly prejudiced wording :-)) I think we'd still choose 3.
>
>> For this particular issue, I don't think that hook interface I'm
>> proposing is more complicated than the barrier. Instead, it is easier
>> for back-end implementer to be aware of the potential issue before
>> really solving stack red zone problem, because it is very clearly
>> listed in target hook list.
>
> The point for "model it in the IL" supporters like myself is that we
> have both many backends and many rtl passes. ?Putting it in a hook keeps
> things simple for the backends, but it means that every rtl pass must be
> aware of this on-the-side dependency. ?Perhaps sched2 really is the only
> pass that needs to look at the hook at present. ?But perhaps not.
> E.g. dbr_schedule (not a problem on ARM, I realise) also reorders
> instructions, so maybe it would need to be audited to see whether any
> calls to this hook are needed. ?And perhaps we'd add more rtl passes
> later.
>
> The point behind using a barrier is that the rtl passes do not then need
> to treat the stack-deallocation dependency as a special case. ?They can
> just use the normal analysis and get it right.
>
> In other words, we're both arguing for safety here.

Indeed.  It's certainly not only scheduling that can move instructions,
but RTL PRE, combine, ifcvt all can effectively cause instruction motion
(just to name a few).

Richard.

> Richard
>