Re: [PATCH, fortran] PR 53379 Backtrace on error termination
On Thu, Sep 17, 2015 at 7:08 AM, David Edelsohn wrote: > On Wed, Sep 9, 2015 at 12:12 AM, Ian Lance Taylor wrote: >> Mike Stump writes: >> >>> Not a big issue, but slightly better if (O_CLOEXEC>>32) != 0 is also >>> true. See, if AIX should ever define this to a sensible value, the >>> above would disappear the feature. However, if they did, then this >>> expression should then be false. >> >> Yes, I think this might be even better in code. How about something >> like >> >> /* On some versions of AIX O_CLOEXEC does not fit in int, so use a >> cast to force it. */ >> descriptor = open (filename, (int) (O_RDONLY | O_BINARY | O_CLOEXEC)); >> >> Does that work on AIX? > > Any decision about this patch? This patch is OK if it works. Ian
Re: [PATCH, fortran] PR 53379 Backtrace on error termination
On Wed, Sep 9, 2015 at 12:12 AM, Ian Lance Taylor wrote: > Mike Stump writes: > >> Not a big issue, but slightly better if (O_CLOEXEC>>32) != 0 is also >> true. See, if AIX should ever define this to a sensible value, the >> above would disappear the feature. However, if they did, then this >> expression should then be false. > > Yes, I think this might be even better in code. How about something > like > > /* On some versions of AIX O_CLOEXEC does not fit in int, so use a > cast to force it. */ > descriptor = open (filename, (int) (O_RDONLY | O_BINARY | O_CLOEXEC)); > > Does that work on AIX? Any decision about this patch? Thanks, David
Re: [PATCH, fortran] PR 53379 Backtrace on error termination
On Sep 8, 2015, at 9:12 PM, Ian Lance Taylor wrote: > Yes, I think this might be even better in code. How about something > like > > /* On some versions of AIX O_CLOEXEC does not fit in int, so use a > cast to force it. */ > descriptor = open (filename, (int) (O_RDONLY | O_BINARY | O_CLOEXEC)); How about: descriptor = open (filename, /* AIX */ (int) (O_RDONLY | O_BINARY | O_CLOEXEC)); ? It makes it removable in the nearer term. :-)
Re: [PATCH, fortran] PR 53379 Backtrace on error termination
On Wed, Sep 9, 2015 at 12:12 AM, Ian Lance Taylor wrote: > Mike Stump writes: > >> Not a big issue, but slightly better if (O_CLOEXEC>>32) != 0 is also >> true. See, if AIX should ever define this to a sensible value, the >> above would disappear the feature. However, if they did, then this >> expression should then be false. > > Yes, I think this might be even better in code. How about something > like > > /* On some versions of AIX O_CLOEXEC does not fit in int, so use a > cast to force it. */ > descriptor = open (filename, (int) (O_RDONLY | O_BINARY | O_CLOEXEC)); > > Does that work on AIX? Yes, that approach also fixes the warning and the build failure. Thanks, David
Re: [PATCH, fortran] PR 53379 Backtrace on error termination
Mike Stump writes: > Not a big issue, but slightly better if (O_CLOEXEC>>32) != 0 is also > true. See, if AIX should ever define this to a sensible value, the > above would disappear the feature. However, if they did, then this > expression should then be false. Yes, I think this might be even better in code. How about something like /* On some versions of AIX O_CLOEXEC does not fit in int, so use a cast to force it. */ descriptor = open (filename, (int) (O_RDONLY | O_BINARY | O_CLOEXEC)); Does that work on AIX? Ian
Re: [PATCH, fortran] PR 53379 Backtrace on error termination
> Not a big issue, but slightly better if (O_CLOEXEC>>32) != 0 is also true. > See, if AIX should ever define this to a sensible value, the above would > disappear the feature. However, if they did, then this expression should > then be false. Sounds good. This being a libbacktrace patch, you need Ian to approve it, it’s his area. FX
Re: [PATCH, fortran] PR 53379 Backtrace on error termination
On Sep 8, 2015, at 6:53 AM, David Edelsohn wrote: > On Tue, Sep 8, 2015 at 9:51 AM, FX wrote: >>> #define _FCLOEXEC 0x0010L >>> #define O_CLOEXEC _FCLOEXEC /* sets FD_CLOEXEC on open */ >> >> That’s weird, and definitely an AIX bug: >> http://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html > > Welcome to AIX :-/ > >> How does that even work? open() takes int as second arg. > > No one else uses it? ;-) > > The following kluge works: > > Index: posix.c > === > --- posix.c (revision 227528) > +++ posix.c (working copy) > @@ -45,6 +45,10 @@ > #define O_BINARY 0 > #endif > > +#ifdef _AIX > +#undef O_CLOEXEC > +#endif > + Not a big issue, but slightly better if (O_CLOEXEC>>32) != 0 is also true. See, if AIX should ever define this to a sensible value, the above would disappear the feature. However, if they did, then this expression should then be false.
Re: [PATCH, fortran] PR 53379 Backtrace on error termination
On Tue, Sep 8, 2015 at 9:51 AM, FX wrote: >> #define _FCLOEXEC 0x0010L >> #define O_CLOEXEC _FCLOEXEC /* sets FD_CLOEXEC on open */ > > That’s weird, and definitely an AIX bug: > http://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html Welcome to AIX :-/ > How does that even work? open() takes int as second arg. No one else uses it? ;-) The following kluge works: Index: posix.c === --- posix.c (revision 227528) +++ posix.c (working copy) @@ -45,6 +45,10 @@ #define O_BINARY 0 #endif +#ifdef _AIX +#undef O_CLOEXEC +#endif + #ifndef O_CLOEXEC #define O_CLOEXEC 0 #endif Thanks, David
Re: [PATCH, fortran] PR 53379 Backtrace on error termination
> #define _FCLOEXEC 0x0010L > #define O_CLOEXEC _FCLOEXEC /* sets FD_CLOEXEC on open */ That’s weird, and definitely an AIX bug: http://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html How does that even work? open() takes int as second arg. FX
Re: [PATCH, fortran] PR 53379 Backtrace on error termination
> /home/dje/src/src/libbacktrace/posix.c: In function 'backtrace_open': > /home/dje/src/src/libbacktrace/posix.c:67:32: error: overflow in > implicit constant conversion [-Werror=overflow] > descriptor = open (filename, O_RDONLY | O_BINARY | O_CLOEXEC); ?? I have a hard time understanding how the non-constant filename can give an overflow error. Or maybe the error is wrong, and it’s O_RDONLY | O_BINARY | O_CLOEXEC that have crazy values? This looks like a valid POSIX construct to me. FX
Re: [PATCH, fortran] PR 53379 Backtrace on error termination
On Tue, Sep 8, 2015 at 9:40 AM, David Edelsohn wrote: > On Tue, Sep 8, 2015 at 9:15 AM, FX wrote: >>> libbacktrace is not supported on AIX. This patch breaks bootstrap on AIX. >>> It's okay if Fortran backtrace does not work on AIX, but not all >>> targets support libbacktrace. >> >> libbacktrace is designed to be compiled on all targets. Some targets offer >> full support, some offer nothing, but libbacktrace is compiled and its >> headers provided in all cases. >> >> Can you please give us something to investigate? Like, the error message >> you’re seeing. > > /home/dje/src/src/libbacktrace/posix.c: In function 'backtrace_open': > /home/dje/src/src/libbacktrace/posix.c:67:32: error: overflow in > implicit constant conversion [-Werror=overflow] >descriptor = open (filename, O_RDONLY | O_BINARY | O_CLOEXEC); > ^ AIX system headers do not define O_BINARY, so posix.c defines it as 0. #define _FCLOEXEC 0x0010L #define O_CLOEXEC _FCLOEXEC /* sets FD_CLOEXEC on open */ Thanks, David
Re: [PATCH, fortran] PR 53379 Backtrace on error termination
On Tue, Sep 8, 2015 at 9:15 AM, FX wrote: >> libbacktrace is not supported on AIX. This patch breaks bootstrap on AIX. >> It's okay if Fortran backtrace does not work on AIX, but not all >> targets support libbacktrace. > > libbacktrace is designed to be compiled on all targets. Some targets offer > full support, some offer nothing, but libbacktrace is compiled and its > headers provided in all cases. > > Can you please give us something to investigate? Like, the error message > you’re seeing. /home/dje/src/src/libbacktrace/posix.c: In function 'backtrace_open': /home/dje/src/src/libbacktrace/posix.c:67:32: error: overflow in implicit constant conversion [-Werror=overflow] descriptor = open (filename, O_RDONLY | O_BINARY | O_CLOEXEC); ^ Thanks, David
Re: [PATCH, fortran] PR 53379 Backtrace on error termination
> libbacktrace is not supported on AIX. This patch breaks bootstrap on AIX. > It's okay if Fortran backtrace does not work on AIX, but not all > targets support libbacktrace. libbacktrace is designed to be compiled on all targets. Some targets offer full support, some offer nothing, but libbacktrace is compiled and its headers provided in all cases. Can you please give us something to investigate? Like, the error message you’re seeing. FX
[PATCH, fortran] PR 53379 Backtrace on error termination
libbacktrace is not supported on AIX. This patch breaks bootstrap on AIX. It's okay if Fortran backtrace does not work on AIX, but not all targets support libbacktrace. Thanks, David
Re: [PATCH, fortran] PR 53379 Backtrace on error termination
> 2015-09-05 Janne Blomqvist > >PR fortran/53579 >* libgfortran.h (exit_error): New prototype. >* runtime/error.c (exit_error): New function. >(os_error): Call exit_error instead of exit. >(runtime_error): Likewise. >(runtime_error_at): Likewise. >(internal_error): Likewise. >(generate_error): Likewise. >(notify_std): Likewise. >* runtime/stop.c (error_stop_string): Likewise. >(error_stop_numeric): Likewise. OK, except that the PR number is wrong in the ChangeLog). (Related: I’ve just closed PR 63930 as WONTFIX.) Thanks for the patch! FX
[PATCH, fortran] PR 53379 Backtrace on error termination
Hi, the consesus in PR 53379 seems to be that a backtrace is desired in case of error termination. The attached patch implements this. Regtested on x86_64-pc-linux-gnu, Ok for trunk? 2015-09-05 Janne Blomqvist PR fortran/53579 * libgfortran.h (exit_error): New prototype. * runtime/error.c (exit_error): New function. (os_error): Call exit_error instead of exit. (runtime_error): Likewise. (runtime_error_at): Likewise. (internal_error): Likewise. (generate_error): Likewise. (notify_std): Likewise. * runtime/stop.c (error_stop_string): Likewise. (error_stop_numeric): Likewise. -- Janne Blomqvist diff --git a/libgfortran/libgfortran.h b/libgfortran/libgfortran.h index 3eb0d85..81240e5 100644 --- a/libgfortran/libgfortran.h +++ b/libgfortran/libgfortran.h @@ -675,6 +675,9 @@ internal_proto(show_backtrace); extern _Noreturn void sys_abort (void); internal_proto(sys_abort); +extern _Noreturn void exit_error (int); +internal_proto(exit_error); + extern ssize_t estr_write (const char *); internal_proto(estr_write); diff --git a/libgfortran/runtime/error.c b/libgfortran/runtime/error.c index 4aabe4a..d357edb 100644 --- a/libgfortran/runtime/error.c +++ b/libgfortran/runtime/error.c @@ -74,15 +74,17 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see 2.3.5 also explains how co-images synchronize during termination. - In libgfortran we have two ways of ending a program. exit(code) is - a normal exit; calling exit() also causes open units to be - closed. No backtrace or core dump is needed here. When something - goes wrong, we have sys_abort() which tries to print the backtrace - if -fbacktrace is enabled, and then dumps core; whether a core file - is generated is system dependent. When aborting, we don't flush and - close open units, as program memory might be corrupted and we'd - rather risk losing dirty data in the buffers rather than corrupting - files on disk. + In libgfortran we have three ways of ending a program. exit(code) + is a normal exit; calling exit() also causes open units to be + closed. No backtrace or core dump is needed here. For error + termination, we have exit_error(status), which prints a backtrace + if backtracing is enabled, then exits. Finally, when something + goes terribly wrong, we have sys_abort() which tries to print the + backtrace if -fbacktrace is enabled, and then dumps core; whether a + core file is generated is system dependent. When aborting, we don't + flush and close open units, as program memory might be corrupted + and we'd rather risk losing dirty data in the buffers rather than + corrupting files on disk. */ @@ -181,6 +183,23 @@ sys_abort (void) } +/* Exit in case of error termination. If backtracing is enabled, print + backtrace, then exit. */ + +void +exit_error (int status) +{ + if (options.backtrace == 1 + || (options.backtrace == -1 && compile_options.backtrace == 1)) +{ + estr_write ("\nError termination. Backtrace:\n"); + show_backtrace (false); +} + exit (status); +} + + + /* gfc_xtoa()-- Integer to hexadecimal conversion. */ const char * @@ -326,7 +345,7 @@ os_error (const char *message) estr_write ("\n"); estr_write (message); estr_write ("\n"); - exit (1); + exit_error (1); } iexport(os_error); @@ -345,7 +364,7 @@ runtime_error (const char *message, ...) st_vprintf (message, ap); va_end (ap); estr_write ("\n"); - exit (2); + exit_error (2); } iexport(runtime_error); @@ -364,7 +383,7 @@ runtime_error_at (const char *where, const char *message, ...) st_vprintf (message, ap); va_end (ap); estr_write ("\n"); - exit (2); + exit_error (2); } iexport(runtime_error_at); @@ -402,7 +421,7 @@ internal_error (st_parameter_common *cmp, const char *message) because hopefully it doesn't happen too often). */ stupid_function_name_for_static_linking(); - exit (3); + exit_error (3); } @@ -574,7 +593,7 @@ generate_error (st_parameter_common *cmp, int family, const char *message) estr_write ("Fortran runtime error: "); estr_write (message); estr_write ("\n"); - exit (2); + exit_error (2); } iexport(generate_error); @@ -636,7 +655,7 @@ notify_std (st_parameter_common *cmp, int std, const char * message) estr_write ("Fortran runtime error: "); estr_write (message); estr_write ("\n"); - exit (2); + exit_error (2); } else { diff --git a/libgfortran/runtime/stop.c b/libgfortran/runtime/stop.c index 5c5483b..8b8a41f 100644 --- a/libgfortran/runtime/stop.c +++ b/libgfortran/runtime/stop.c @@ -145,7 +145,7 @@ error_stop_string (const char *string, GFC_INTEGER_4 len) (void) write (STDERR_FILENO, string, len); estr_write ("\n"); - exit (1); + exit_error (1); } @@ -159,5 +159,5 @@ error_stop_numeric (GFC_INTEGER_4 code) { report_exception (); st_printf ("ERROR STOP %d\n", (int