On Sat, Apr 28, 2018 at 11:20 AM, Eric Botcazou <ebotca...@adacore.com> wrote:
>> This looks not a generic enough fix to me - consider
>>
>> void foo(void) { int a[100000]; a[0] = 1; a[99999] = 1; }
>> int main() { try { foo (); } catch (...) {} }
>>
>> with -fnon-call-exceptions.  If we'd like to catch the SEGV from stack
>> overflows then your fix doesn't handle the non-recursive case nor the case
>> where -fstack-check is not supplied.
>
> But -fstack-check is required to detect stack overflows...  Moreover the above
> testcase will be entirely optimized at -O so there is no stack overflow at -O.
>
>> So to me your attempt in fixing this isn't complete but a complete fix would
>> be quite pessimizing :/ (for -fnon-call-exceptions)
>
> Then let's at least fix the recursive case since it's simple and cheap.  But I
> can indeed key this on -fnon-call-exceptions explicitly.
>
>> At least all this should be documented somewhere, that is, what to expect
>> when trying to catch stack faults in general with -fnon-call-exceptions
>> [-fstack-check].
>
> It's already documented that you need -fstack-check to detect stack overflows.
> But I can add a blurb to the -fnon-call-exceptions entry about it.
>
> Revised patch attached.

Is the propagate_nothrow hunk really necessary since you set ->can_throw = true
in local analysis?  Again this not only handles only recursion but also only
direct recursion.  I wonder if there's a way to prove (or at least estimate with
good confidence) that a function does _not_ need local stack space?  Thus
for -fstack-check -fnon-call-exceptions consider all functions not
marked explicitely
as can_throw unless we "prove" the opposite?  Or consider all non-leaf functions
as possibly throwing that way?

I'm not against your patch but I think that this kind of limitations
need to be documented.

Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi     (revision 259642)
+++ doc/invoke.texi     (working copy)
@@ -12812,6 +12812,9 @@ not exist everywhere.  Moreover, it only
 instructions to throw exceptions, i.e.@: memory references or floating-point
 instructions.  It does not allow exceptions to be thrown from
 arbitrary signal handlers such as @code{SIGALRM}.
+This option must be specified if you enable @option{-fstack-check} and
+want stack overflows to throw exceptions.  Note that this again requires
+platform-specific runtime support.


I'd phrase it as "If you want to handle stack overflows as exceptions
you need to
enable @option{-fstack-check} in addition to this option.  Support for this is
experimental, some stack overflows might not be catchable."

Honza, do you have any suggestions here for the issue of detecting possible
recursion as opposted to just direct recursion?

Richard.

> --
> Eric Botcazou

Reply via email to