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