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

["Jiangning Liu" <jiangning dot liu at arm dot com>]

> -----Original Message-----
> From: Richard Guenther [mailto:richard.guenther@gmail.com]
> Sent: Friday, September 30, 2011 8:57 PM
> To: Jiangning Liu; Jakub Jelinek; Richard Guenther; Andrew Pinski; gcc-
> patches@gcc.gnu.org; richard.sandiford@linaro.org
> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
> 
> 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 G.,

Yeah, this sounds like a good justification.

Could you please explain more about how RTL PRE (gcse.c) can avoid hoisting
ld/st over intrinsic alloca?

If RTL PRE doesn't handle it at all, then I would say it is a potential bug.

If RTL PRE does handle it, is the current interface in this pass checking
the memory barrier? And how about other passes you mentioned?

Thanks,
-Jiangning

> Richard.
> 
> > Richard
> >