This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix stack red zone bug (PR38644)
"Jiangning Liu" <jiangning.liu@arm.com> writes:
>> -----Original Message-----
>> From: Jakub Jelinek [mailto:jakub@redhat.com]
>> Sent: Thursday, September 29, 2011 6:14 PM
>> To: Jiangning Liu
>> Cc: 'Richard Guenther'; Andrew Pinski; gcc-patches@gcc.gnu.org
>> Subject: Re: [PATCH] Fix stack red zone bug (PR38644)
>>
>> On Thu, Sep 29, 2011 at 06:08:50PM +0800, Jiangning Liu wrote:
>> > As far as I know different back-ends are implementing different
>> > prologue/epilogue in GCC. If one day this part can be refined and
>> abstracted
>> > as well, I would say solving this stack-red-zone problem in shared
>> > prologue/epilogue code would be a perfect solution, and barrier can
>> be
>> > inserted there.
>> >
>> > I'm not saying you are wrong on keeping scheduler using a pure
>> barrier
>> > interface. From engineering point of view, I only feel my proposal is
>> so far
>> > so good, because this patch at least solve the problem for all
>> targets in a
>> > quite simple way. Maybe it can be improved in future based on this.
>>
>> But you don't want to listen about any other alternative, other
>> backends are
>> happy with being able to put the best kind of barrier at the best spot
>> in the epilogue and don't need a "generic" solution which won't model
>> very
>> well the target diversity anyway.
>
> Jakub,
>
> Appreciate for your attention on this issue,
>
> 1) Can you clarify who are the "others back-ends"? Does it cover most of the
> back-ends being supported by GCC right now?
Not answering for Jakub of course, but as a maintainer of a backend, I know
MIPS doesn't have the required barrier at the moment. But that's a bug.
Like others in this thread, I'm strongly of the opinion that this should
be modelled directly in the IL. And it's already supposed to be modelled
in the IL. Target-independent code emits the required barriers in cases
where it rather than the backend patterns are responsible. E.g.:
emit_clobber (gen_rtx_MEM (BLKmode, gen_rtx_SCRATCH (VOIDmode)));
emit_clobber (gen_rtx_MEM (BLKmode, hard_frame_pointer_rtx));
emit_move_insn (hard_frame_pointer_rtx, fp);
emit_stack_restore (SAVE_NONLOCAL, stack);
from expand_builtin_longjmp() and:
if (sa != 0)
{
sa = validize_mem (sa);
/* These clobbers prevent the scheduler from moving
references to variable arrays below the code
that deletes (pops) the arrays. */
emit_clobber (gen_rtx_MEM (BLKmode, gen_rtx_SCRATCH (VOIDmode)));
emit_clobber (gen_rtx_MEM (BLKmode, stack_pointer_rtx));
}
from emit_stack_restore(). Backends that fail to follow suit are IMO
just buggy.
FWIW, I intend to fix MIPS this weekend.
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.
Richard