Bug 34118

It would certainly be in the Ada spirit to have stack-checking enabled by default.

If GCC offers a selection of stack-checking methods, I think the default method should be the most reliable and general one, even if this is the slower method. If the "guard pages" method is less reliable than explicit stack checking code (comparing the stack pointer to a limit) I think the explicit checks should be the default; a specific command-line switch should be required to use the less reliable method instead.

Furthermore, if stack checks depend on the value of an environment variable (eg. GNAT_STACK_LIMIT) I think that execution of the program should fail immediately if the variable is not defined in the program's environment.

> GCC supports two ways to implement stack overflow checks: using guard pages
> called "probes", and inserting stack checking code into every subprogram.

That's confused.  Probes are not "guard pages" and you always need to insert
stack checking code into every subprogram to do stack checking.

> The probes require support from the both OS and hardware, so are not
> suitable for all targets.

Correct, but it's nevertheless the preferred method.

> Moreover, they can miss stack overflows if a subprogram writes to a page
> *after* the guard page (rather that into it).

That's not true of probes in general, only of the generic implementation of
the probing method in GCC.  The implementation on Alpha/Tru64 doesn't suffer
from this defect for example.

> GCC has a warning about this situation.  Therefore, I suggest a new 
> command-line switch to force insertion of stack checking code (rather than 
> guard pages) for those programs that trigger the warning.

That's even more confused.  The warning is independent of the checking method,
it is issued with the other method (stack checking routine) too, for example
on x86/Linux.  You cannot get rid of it without changing the program, with
the current implementation of stack checking.  That's one of the reasons why
stack checking is not enabled by default.

Subject: Re:  Please enable stack checking (-fstack-check) by default

On 16 Nov 2007 18:35:15 -0000, ebotcazou at gcc dot gnu dot org
<gcc-bugzilla@gcc.gnu.org> wrote:
> That's not true of probes in general, only of the generic implementation of
> the probing method in GCC.  The implementation on Alpha/Tru64 doesn't suffer
> from this defect for example.

Or even the spu-elf implementation.

-- Pinski

Questions:

1) If Targparm.Stack_Check_Probes_On_Target is False (i.e. the "GNAT Stack-limit checking" is in effect), what circumstances would cause the stack check to not detect an overflow (i.e. "unreliable stack checking")?

2) If Targparm.Stack_Check_Probes_On_Target is True (i.e. the "GCC Probing Mechanism" is in effect, using the guard pages), would it take much effort to make it "reliable" on all targets, not just Alpha/Tru64, spu-elf and maybe some others?

> 1) If Targparm.Stack_Check_Probes_On_Target is False (i.e. the "GNAT
> Stack-limit checking" is in effect), what circumstances would cause the
> stack check to not detect an overflow (i.e. "unreliable stack checking")?

If the static frame size of a function is too large (typically > 4KB).

> 2) If Targparm.Stack_Check_Probes_On_Target is True (i.e. the "GCC Probing
> Mechanism" is in effect, using the guard pages), would it take much effort
> to make it "reliable" on all targets, not just Alpha/Tru64, spu-elf and
> maybe some others?

It's not difficult, but you have to do it on a per-architecture basis.

AdaCore has done it on 7 architectures and is ready to contribute this code,
see http://gcc.gnu.org/ml/gcc-patches/2006-11/msg01846.html

I note that this (impressive) patch is not in mainline yet; it seems nobody has reviewed it yet.  In the patch, you say: "-fstack-check is broken in the 4.x series of compilers in the sense that you cannot recover from a stack overflow condition (for example in Ada). It's a regression from the 3.x series although there were bugs in that series too."  Therefore, marking this PR as depending on middle-end/20548.

(In reply to comment #5)
> 
> AdaCore has done it on 7 architectures and is ready to contribute this code,
> see http://gcc.gnu.org/ml/gcc-patches/2006-11/msg01846.html

Why don't you ping directly the relevant maintainers? Otherwise, the patch is going to keep rotting in the patch queue.

> Why don't you ping directly the relevant maintainers?

Probably because I have other things more interesting to do. :-)

(In reply to comment #8)
> > Why don't you ping directly the relevant maintainers?
> 
> Probably because I have other things more interesting to do. :-)
> 

Then I humbly think you should have clearly stated that in your email and asked people interested in the patch to ping the relevant maintainers on your behalf. It is evident that there is a sense of ownership and responsibility with respect to patches and if you don't openly relinquish them, people seem to be reluctant to take the responsibility in their own hands.

In short, I think people interested in the patch (and there seem to be a few) would push it if you send it again and prominently manifest that you hope someone will take care of the patch on your behalf.

> Then I humbly think you should have clearly stated that in your email and
> asked people interested in the patch to ping the relevant maintainers on
> your behalf.

That's what I've sort of done in the second sentence.

> In short, I think people interested in the patch (and there seem to be a few)
> would push it if you send it again and prominently manifest that you hope
> someone will take care of the patch on your behalf.

Sorry, I won't have time to do that before 4.3.0 is released.

(In reply to comment #10)
> > Then I humbly think you should have clearly stated that in your email and
> > asked people interested in the patch to ping the relevant maintainers on
> > your behalf.
> 
> That's what I've sort of done in the second sentence.
> 

I feel "Hopefully it will now spur a little more interest than last time" perhaps wasn't understood as "***NOTE: I am NOT going to pursue this patch further, so if you are interested in this, please, feel free to ping the relevant maintainers, update the patch as appropriate and commit it on my behalf."

;-)

> Sorry, I won't have time to do that before 4.3.0 is released.

I think that is ok, it seems a big change for stage3 anyway. I added it to the list of 4.4 pending patches.

Any update on this?

Let's suspend it for now.