diff mbox

[fortran] PR 53379 Backtrace on error termination

Message ID CAGWvnynCSzSfJensW4g2yoipF9U4Vxmwrh9Wd84713=Qt180kA@mail.gmail.com
State New
Headers show

Commit Message

David Edelsohn Sept. 8, 2015, 1:53 p.m. UTC 
On Tue, Sep 8, 2015 at 9:51 AM, FX <fxcoudert@gmail.com> wrote:
>> #define _FCLOEXEC       0x0000001000000000L
>> #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:


Thanks, David

Comments

Mike Stump Sept. 8, 2015, 3:25 p.m. UTC | #1 
On Sep 8, 2015, at 6:53 AM, David Edelsohn <dje.gcc@gmail.com> wrote:
> On Tue, Sep 8, 2015 at 9:51 AM, FX <fxcoudert@gmail.com> wrote:
>>> #define _FCLOEXEC       0x0000001000000000L
>>> #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.
FX Coudert Sept. 8, 2015, 3:53 p.m. UTC | #2 
> 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
Ian Lance Taylor Sept. 9, 2015, 4:12 a.m. UTC | #3 
Mike Stump <mikestump@comcast.net> 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
David Edelsohn Sept. 9, 2015, 12:29 p.m. UTC | #4 
On Wed, Sep 9, 2015 at 12:12 AM, Ian Lance Taylor <ian@airs.com> wrote:
> Mike Stump <mikestump@comcast.net> 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
Mike Stump Sept. 9, 2015, 2:52 p.m. UTC | #5 
On Sep 8, 2015, at 9:12 PM, Ian Lance Taylor <ian@airs.com> 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.  :-)
David Edelsohn Sept. 17, 2015, 2:08 p.m. UTC | #6 
On Wed, Sep 9, 2015 at 12:12 AM, Ian Lance Taylor <ian@airs.com> wrote:
> Mike Stump <mikestump@comcast.net> 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
Ian Lance Taylor Sept. 17, 2015, 3:06 p.m. UTC | #7 
On Thu, Sep 17, 2015 at 7:08 AM, David Edelsohn <dje.gcc@gmail.com> wrote:
> On Wed, Sep 9, 2015 at 12:12 AM, Ian Lance Taylor <ian@airs.com> wrote:
>> Mike Stump <mikestump@comcast.net> 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
diff mbox

Patch

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