diff mbox

Bump size of stack checking protection area

Message ID 2953624.HgYYlYoG9S@polaris
State New
Headers show

Commit Message

Eric Botcazou Sept. 14, 2015, 8:23 a.m. UTC
Hi,

as documented, STACK_CHECK_PROTECT is supposed to be an "estimate of the 
amount of stack required to propagate an exception".  It's (mainly) for Ada 
and it needs to distinguish the various EH schemes, which might have different 
needs.  While the current setting is OK for the front-end SJLJ scheme used up 
to now in Ada, it's not sufficient for the middle-end SJLJ scheme that we are 
experimenting with; you need 8K on some platforms to pass the ACATS testsuite.

Tested on x86_64-suse-linux, OK for the mainline?


2015-09-14  Eric Botcazou  <ebotcazou@adacore.com>

	* defaults.h (STACK_OLD_CHECK_PROTECT): Adjust for -fno-exceptions.
	Bump to 4K for SJLJ exceptions.
	(STACK_CHECK_PROTECT): Likewise.  Bump to 8K for SJLJ exceptions.

Comments

Bernd Schmidt Sept. 14, 2015, 9:41 a.m. UTC | #1
On 09/14/2015 10:23 AM, Eric Botcazou wrote:

> as documented, STACK_CHECK_PROTECT is supposed to be an "estimate of the
> amount of stack required to propagate an exception".  It's (mainly) for Ada
> and it needs to distinguish the various EH schemes, which might have different
> needs.  While the current setting is OK for the front-end SJLJ scheme used up
> to now in Ada, it's not sufficient for the middle-end SJLJ scheme that we are
> experimenting with; you need 8K on some platforms to pass the ACATS testsuite.

So it looks like some targets are at least optionally still using sjlj 
exceptions and would be affected by this change. AFAICT it only makes a 
difference with -fstack-check and would be a bugfix even for those 
targets - correct? Or is there something Ada-specific that makes it 
require more stack?
If it's not Ada-specific, the patch is ok if you also update the tm.texi 
documentation.


Bernd
Eric Botcazou Sept. 15, 2015, 7:50 a.m. UTC | #2
> So it looks like some targets are at least optionally still using sjlj
> exceptions and would be affected by this change. AFAICT it only makes a
> difference with -fstack-check and would be a bugfix even for those
> targets - correct?

Yes, for example the 32-bit Windows compiler still uses SJLJ exceptions.

> Or is there something Ada-specific that makes it require more stack?

The history is as follows: -fstack-check was implemented for Ada (it's 
required by the language) and the `setjmp'/`longjmp'-based exception handling 
mechanism referred to in the doc is the SJLJ scheme of the Ada compiler.
This scheme is purely driven by the front-end (for example, -fexceptions is 
*not* set when it is in use) and the 75 words are sufficient with it.
This front-end SJLJ scheme has some annoying drawbacks/limitations so we are 
experimenting with the middle SJLJ scheme (-fexceptions set) and for it the 75 
words are definitely not sufficient; 4K/8K are required e.g. on Solaris.

> If it's not Ada-specific, the patch is ok if you also update the tm.texi
> documentation.

It's not Ada-specific in the sense that, if you want to raise an exception on 
a stack overflow in any language with the middle-end SJLJ scheme, the current 
setting is definitely not sufficient and you'll get a crash during unwinding.
Bernd Schmidt Sept. 15, 2015, 9:20 a.m. UTC | #3
On 09/15/2015 09:50 AM, Eric Botcazou wrote:

> It's not Ada-specific in the sense that, if you want to raise an exception on
> a stack overflow in any language with the middle-end SJLJ scheme, the current
> setting is definitely not sufficient and you'll get a crash during unwinding.

For avoidance of doubt, the patch is fine then.


Bernd
Eric Botcazou Sept. 15, 2015, 4:27 p.m. UTC | #4
> For avoidance of doubt, the patch is fine then.

Thanks, installed with the tm.texi bits.
diff mbox

Patch

Index: defaults.h
===================================================================
--- defaults.h	(revision 227729)
+++ defaults.h	(working copy)
@@ -1406,9 +1406,11 @@  see the files COPYING3 and COPYING.RUNTI
 #define STACK_OLD_CHECK_PROTECT STACK_CHECK_PROTECT
 #else
 #define STACK_OLD_CHECK_PROTECT						\
- (targetm_common.except_unwind_info (&global_options) == UI_SJLJ	\
+ (!global_options.x_flag_exceptions					\
   ? 75 * UNITS_PER_WORD							\
-  : 8 * 1024)
+  : targetm_common.except_unwind_info (&global_options) == UI_SJLJ	\
+    ? 4 * 1024								\
+    : 8 * 1024)
 #endif
 
 /* Minimum amount of stack required to recover from an anticipated stack
@@ -1416,9 +1418,11 @@  see the files COPYING3 and COPYING.RUNTI
    of stack required to propagate an exception.  */
 #ifndef STACK_CHECK_PROTECT
 #define STACK_CHECK_PROTECT						\
- (targetm_common.except_unwind_info (&global_options) == UI_SJLJ	\
-  ? 75 * UNITS_PER_WORD							\
-  : 12 * 1024)
+ (!global_options.x_flag_exceptions					\
+  ? 4 * 1024								\
+  : targetm_common.except_unwind_info (&global_options) == UI_SJLJ	\
+    ? 8 * 1024								\
+    : 12 * 1024)
 #endif
 
 /* Make the maximum frame size be the largest we can and still only need