Bug 34118 - enable stack checking (-fstack-check) by default
Summary: enable stack checking (-fstack-check) by default
Status: SUSPENDED
Alias: None
Product: gcc
Classification: Unclassified
Component: ada (show other bugs)
Version: 4.2.2
: P3 enhancement
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on: 20548
Blocks:
  Show dependency treegraph
 
Reported: 2007-11-16 13:00 UTC by Ludovic Brenta
Modified: 2018-03-12 22:30 UTC (History)
8 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2008-11-30 09:46:46


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ludovic Brenta 2007-11-16 13:00:04 UTC
By default, GCC is not an Ada compiler and has not been one for years, because stack overflow checks are disabled by default and require an explicit switch (-fstack-check).  The rationale, as I understand it, was that stack overflow checks were expensive in terms of CPU usage.

However, the rationale is much less valid now that CPUs are more powerful and, more importantly, thanks to the new static stack analyzer in GCC which should allow elimination of some stack overflow checks.

The drawback of the current situation is that almost every new user of Ada, at some point, is surprised because they don't get the expected Storage_Error.  In other words, GCC fails the Law of Least Astonishment.

Stack overflow checks can always be disabled explicitly with -fno-stack-check or pragma Suppress.

Therefore: please enable stack overflow checks by default.

GCC supports two ways to implement stack overflow checks: using guard pages called "probes", and inserting stack checking code into every subprogram.  The probes require support from the both OS and hardware, so are not suitable for all targets.  Moreover, they can miss stack overflows if a subprogram writes to a page *after* the guard page (rather that into it).  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.
Comment 1 Niklas Holsti 2007-11-16 18:19:36 UTC
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.
Comment 2 Eric Botcazou 2007-11-16 18:35:15 UTC
> 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.
Comment 3 pinskia@gmail.com 2007-11-16 20:26:10 UTC
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
Comment 4 Ludovic Brenta 2007-11-20 12:31:58 UTC
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?
Comment 5 Eric Botcazou 2007-11-20 15:37:13 UTC
> 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
Comment 6 Ludovic Brenta 2007-11-21 10:44:22 UTC
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.
Comment 7 Manuel López-Ibáñez 2007-11-21 12:34:27 UTC
(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.
Comment 8 Eric Botcazou 2007-11-21 12:40:54 UTC
> Why don't you ping directly the relevant maintainers?

Probably because I have other things more interesting to do. :-)
Comment 9 Manuel López-Ibáñez 2007-11-21 13:17:41 UTC
(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.
Comment 10 Eric Botcazou 2007-11-21 13:34:57 UTC
> 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.
Comment 11 Manuel López-Ibáñez 2007-11-21 13:45:45 UTC
(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.
Comment 12 Eric Gallager 2018-03-12 22:20:21 UTC
Any update on this?
Comment 13 Eric Botcazou 2018-03-12 22:30:32 UTC
Let's suspend it for now.