Re: [PATCH, fortran] PR 53379 Backtrace on error termination

2015-09-17 Thread David Edelsohn 
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

2015-09-17 Thread Ian Lance Taylor 
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

2015-09-09 Thread David Edelsohn 
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

2015-09-09 Thread Mike Stump 
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.  :-)

[PATCH, fortran] PR 53379 Backtrace on error termination

2015-09-08 Thread David Edelsohn 
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-08 Thread David Edelsohn 
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

2015-09-08 Thread David Edelsohn 
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

2015-09-08 Thread FX 
> /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

2015-09-08 Thread FX 
> 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

Re: [PATCH, fortran] PR 53379 Backtrace on error termination

2015-09-08 Thread David Edelsohn 
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

2015-09-08 Thread FX 
> 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

2015-09-08 Thread Ian Lance Taylor 
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

2015-09-08 Thread Mike Stump 
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

2015-09-08 Thread FX 
> #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

[PATCH, fortran] PR 53379 Backtrace on error termination

2015-09-04 Thread Janne Blomqvist 
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

Re: [PATCH, fortran] PR 53379 Backtrace on error termination

2015-09-04 Thread FX 
> 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