GNU bug report logs - #15970
[Bug-tar] Crash in gettext() after fork() on Mac OS X

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 15970 in the body.
You can then email your comments to 15970 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: bug-tar <at> gnu.org, bug-coreutils <at> gnu.org
Subject: Re: [Bug-tar] Crash in gettext() after fork() on Mac OS X
Date: Sun, 24 Nov 2013 16:10:16 -0800

Eric Blake wrote:
> I bet coreutils' sort has a similar bug

[Adding bug-coreutils to the CC:.]

Coreutils 'sort' runs into this problem only
if dup2 fails when called from move_fd_or_die
or if execlp fails.

dup2 should never fail when it's called
the way that 'sort' calls it.  So perhaps
'sort' should be simplified to call dup2
and discard its return value.

And when execlp fails, 'sort' could _exit
with a special value, so that the parent
could report the failure.

Message #8 received at 15970 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 15970 <at> debbugs.gnu.org, bug-tar <at> gnu.org
Subject: Re: bug#15970: [Bug-tar] Crash in gettext() after fork() on Mac OS X
Date: Tue, 26 Nov 2013 03:06:00 +0000

[Message part 1 (text/plain, inline)]

On 11/25/2013 12:10 AM, Paul Eggert wrote:
> Eric Blake wrote:
>> I bet coreutils' sort has a similar bug
> 
> [Adding bug-coreutils to the CC:.]

Referencing the original coreutils thread from the new bug:
http://lists.gnu.org/archive/html/coreutils/2013-11/msg00083.html

> Coreutils 'sort' runs into this problem only
> if dup2 fails when called from move_fd_or_die
> or if execlp fails.

Right.

> dup2 should never fail when it's called
> the way that 'sort' calls it.  So perhaps
> 'sort' should be simplified to call dup2
> and discard its return value.
> 
> And when execlp fails, 'sort' could _exit
> with a special value, so that the parent
> could report the failure.

For both cases we could _exit(errno),
which the parent would get with waitpid().

Interestingly, currently the parent only
notices an error after stdio eventually issues a write(),
at which point the parent receives SIGPIPE and
exits with status 141 (128 + SIGPIPE).

If instead the parent caught sigchld it
could waitpid() to get the status more directly.
Though there are various gotchas with sigchld handling,
and we'd have to handle the reap() error() calls
outside of the signal handler.  This would also be
the case if we wanted to reap on SIGPIPE.
So at first glance this seems to be a bit overcomplicated
for what it's solving.

How about the attached instead that just uses
a somewhat degraded but simpler error() equivalent.

thanks,
Pádraig.

[sort-async-safe.patch (text/x-patch, attachment)]

Message #11 received at 15970 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Pádraig Brady <P <at> draigBrady.com>
Cc: 15970 <at> debbugs.gnu.org, bug-tar <at> gnu.org
Subject: Re: bug#15970: [Bug-tar] Crash in gettext() after fork() on Mac OS X
Date: Mon, 25 Nov 2013 20:55:29 -0800

Pádraig Brady wrote:

> How about the attached instead that just uses
> a somewhat degraded but simpler error() equivalent.

That looks safe, though it could be simplified:
use inttostr instead of repeating its body,
and since !!p == !!errnum there's no need
to have those duplicate conditions.
Plus, just use STDERR_FILENO rather than
fiddling with fileno (stderr), and async_safe_error
should be _Noreturn and should unconditionally call
_exit.  And, I wouldn't bother looking at dup2's
return value, any more than we look at close's.

Message #14 received at 15970 <at> debbugs.gnu.org (full text, mbox):

From: Joerg Schilling <Joerg.Schilling <at> fokus.fraunhofer.de>
To: <P <at> draigBrady.com>, <eggert <at> cs.ucla.edu>
Cc: 15970 <at> debbugs.gnu.org, bug-tar <at> gnu.org
Subject: Re: [Bug-tar] bug#15970: Crash in gettext() after fork() on Mac OS X
Date: Tue, 26 Nov 2013 11:53:50 +0100

Are you using "ignore_value" because of the gcc bug that ignores the official
method to tell that the return code is ignored intentionally:

	(void) write(fd, buf, len);

Jörg

-- 
 EMail:joerg <at> schily.isdn.cs.tu-berlin.de (home) Jörg Schilling D-13353 Berlin
       js <at> cs.tu-berlin.de                (uni)  
       joerg.schilling <at> fokus.fraunhofer.de (work) Blog: http://schily.blogspot.com/
 URL:  http://cdrecord.berlios.de/private/ ftp://ftp.berlios.de/pub/schily

Message #19 received at 15970-done <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: bug-tar <at> gnu.org, 15970-done <at> debbugs.gnu.org
Subject: Re: bug#15970: [Bug-tar] Crash in gettext() after fork() on Mac OS X
Date: Tue, 26 Nov 2013 12:23:15 +0000

[Message part 1 (text/plain, inline)]

On 11/26/2013 04:55 AM, Paul Eggert wrote:
> Pádraig Brady wrote:
> 
>> How about the attached instead that just uses
>> a somewhat degraded but simpler error() equivalent.
> 
> That looks safe, though it could be simplified:
> use inttostr instead of repeating its body,

Hah, I was thinking this must be done somewhere else :)

> and since !!p == !!errnum there's no need
> to have those duplicate conditions.

I meant to use (p || errstr), but let's assume
errstr is present, so yes we can then remove condition.

> Plus, just use STDERR_FILENO rather than
> fiddling with fileno (stderr),

done

> and async_safe_error
> should be _Noreturn and should unconditionally call
> _exit.

fair enough. I changed the name so to async_safe_die()

> And, I wouldn't bother looking at dup2's
> return value, any more than we look at close's.

Updated patch attached.

thanks!
Pádraig.

[sort-async-safe.patch (text/x-patch, attachment)]

Message #22 received at 15970 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Joerg Schilling <Joerg.Schilling <at> fokus.fraunhofer.de>
Cc: eggert <at> cs.ucla.edu, 15970 <at> debbugs.gnu.org, bug-tar <at> gnu.org
Subject: Re: bug#15970: [Bug-tar] bug#15970: Crash in gettext() after fork()
 on Mac OS X
Date: Tue, 26 Nov 2013 12:34:55 +0000

On 11/26/2013 10:53 AM, Joerg Schilling wrote:
> 
> Are you using "ignore_value" because of the gcc bug that ignores the official
> method to tell that the return code is ignored intentionally:
> 
> 	(void) write(fd, buf, len);

Right: http://git.sv.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/ignore-value.h;hb=HEAD

I thought it marginally cleaner to use that,
than to have a goto to just _exit() on write error.

thanks,
Pádraig.

Message #25 received at 15970 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: 15970 <at> debbugs.gnu.org, P <at> draigBrady.com
Subject: Re: bug#15970: [Bug-tar] Crash in gettext() after fork() on Mac OS X
Date: Tue, 26 Nov 2013 12:33:55 -0800

On 11/26/2013 04:23 AM, Pádraig Brady wrote:
> Updated patch attached.

Thanks, looks good.  Hmm, I don't see the patch committed
but the bug is marked as closed?

Message #28 received at 15970 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Pádraig Brady <P <at> draigbrady.com>
Cc: Paul Eggert <eggert <at> cs.ucla.edu>, 15970 <at> debbugs.gnu.org, bug-tar <at> gnu.org
Subject: Re: bug#15970: [Bug-tar] Crash in gettext() after fork() on Mac OS X
Date: Tue, 26 Nov 2013 14:09:34 -0800

Hi Pádraig,

Thanks for dealing with that. Your patch looks correct.
Did you consider using inttostr in place of that first part of async_safe_error?

Message #31 received at 15970 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Jim Meyering <jim <at> meyering.net>
Cc: Paul Eggert <eggert <at> cs.ucla.edu>, 15970 <at> debbugs.gnu.org
Subject: Re: bug#15970: [Bug-tar] Crash in gettext() after fork() on Mac OS X
Date: Wed, 27 Nov 2013 01:25:52 +0000

On 11/26/2013 10:09 PM, Jim Meyering wrote:
> Hi Pádraig,
> 
> Thanks for dealing with that. Your patch looks correct.
> Did you consider using inttostr in place of that first part of async_safe_error?

Paul suggested that too and I've pushed an updated patch now.
I'd searched for INTTOSTR without luck, and in my tired state
just open coded it meaning to come back and search for the routine :)

thanks,
Pádraig.

Message #34 received at 15970 <at> debbugs.gnu.org (full text, mbox):

From: Jack Howarth <howarth <at> bromo.med.uc.edu>
To: Pádraig Brady <P <at> draigBrady.com>
Cc: Paul Eggert <eggert <at> cs.ucla.edu>, Jim Meyering <jim <at> meyering.net>,
 15970 <at> debbugs.gnu.org
Subject: Re: bug#15970: [Bug-tar] Crash in gettext() after fork() on Mac OS X
Date: Wed, 27 Nov 2013 09:38:57 -0500

On Wed, Nov 27, 2013 at 01:25:52AM +0000, Pádraig Brady wrote:
> On 11/26/2013 10:09 PM, Jim Meyering wrote:
> > Hi Pádraig,
> > 
> > Thanks for dealing with that. Your patch looks correct.
> > Did you consider using inttostr in place of that first part of async_safe_error?
> 
> Paul suggested that too and I've pushed an updated patch now.
> I'd searched for INTTOSTR without luck, and in my tired state
> just open coded it meaning to come back and search for the routine :)
> 
> thanks,
> Pádraig.

Pádraig,
   What revision was the commit? Also was the commit to the tar or gnulib git? I ask
because neither's web interface is showing a recent commit for this fix. Thanks in 
advance so we can test this fix on darwin.
          Jack
> 
> 

Message #37 received at 15970 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Jack Howarth <howarth <at> bromo.med.uc.edu>
Cc: Paul Eggert <eggert <at> cs.ucla.edu>, Jim Meyering <jim <at> meyering.net>,
 15970 <at> debbugs.gnu.org
Subject: Re: bug#15970: [Bug-tar] Crash in gettext() after fork() on Mac OS X
Date: Wed, 27 Nov 2013 14:57:35 +0000

On 11/27/2013 02:38 PM, Jack Howarth wrote:
> On Wed, Nov 27, 2013 at 01:25:52AM +0000, Pádraig Brady wrote:
>> On 11/26/2013 10:09 PM, Jim Meyering wrote:
>>> Hi Pádraig,
>>>
>>> Thanks for dealing with that. Your patch looks correct.
>>> Did you consider using inttostr in place of that first part of async_safe_error?
>>
>> Paul suggested that too and I've pushed an updated patch now.
>> I'd searched for INTTOSTR without luck, and in my tired state
>> just open coded it meaning to come back and search for the routine :)
>>
>> thanks,
>> Pádraig.
> 
> Pádraig,
>    What revision was the commit? Also was the commit to the tar or gnulib git? I ask
> because neither's web interface is showing a recent commit for this fix. Thanks in 
> advance so we can test this fix on darwin.

Sorry there was some crossposting going on.
My commit was only precautionary and for coreutils sort:
http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commit;h=39e0c9ba

thanks,
Pádraig.

Message #40 received at 15970 <at> debbugs.gnu.org (full text, mbox):

From: Joerg Schilling <Joerg.Schilling <at> fokus.fraunhofer.de>
To: <P <at> draigBrady.com>
Cc: eggert <at> cs.ucla.edu, 15970 <at> debbugs.gnu.org, bug-tar <at> gnu.org
Subject: Re: [Bug-tar] bug#15970: bug#15970: Crash in gettext() after fork()
 on Mac OS X
Date: Wed, 27 Nov 2013 17:30:49 +0100

Pádraig Brady <P <at> draigBrady.com> wrote:

> On 11/26/2013 10:53 AM, Joerg Schilling wrote:
> > 
> > Are you using "ignore_value" because of the gcc bug that ignores the official
> > method to tell that the return code is ignored intentionally:
> > 
> > 	(void) write(fd, buf, len);
>
> Right: http://git.sv.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/ignore-value.h;hb=HEAD
>
> I thought it marginally cleaner to use that,
> than to have a goto to just _exit() on write error.

Do you believe this is the right method to deal with the problem?
It will most likely prevent to create enough pressure on the GCC people to fix 
the current warning problem.

Jörg

-- 
 EMail:joerg <at> schily.isdn.cs.tu-berlin.de (home) Jörg Schilling D-13353 Berlin
       js <at> cs.tu-berlin.de                (uni)  
       joerg.schilling <at> fokus.fraunhofer.de (work) Blog: http://schily.blogspot.com/
 URL:  http://cdrecord.berlios.de/private/ ftp://ftp.berlios.de/pub/schily

Message #43 received at 15970 <at> debbugs.gnu.org (full text, mbox):

From: Eric Blake <eblake <at> redhat.com>
To: Joerg Schilling <Joerg.Schilling <at> fokus.fraunhofer.de>, P <at> draigBrady.com
Cc: eggert <at> cs.ucla.edu, 15970 <at> debbugs.gnu.org, bug-tar <at> gnu.org
Subject: ignore_value vs (void) [was: [Bug-tar] bug#15970: bug#15970: Crash
 in gettext() after fork() on Mac OS X]
Date: Wed, 27 Nov 2013 09:44:06 -0700

[Message part 1 (text/plain, inline)]

On 11/27/2013 09:30 AM, Joerg Schilling wrote:
>>> Are you using "ignore_value" because of the gcc bug that ignores the official
>>> method to tell that the return code is ignored intentionally:
>>>
>>> 	(void) write(fd, buf, len);

The C standard does not document that '(void) write(...)' is required to
avoid diagnostics, so I don't see the gcc behavior as a bug (yes, it's
annoying behavior, but that doesn't make it incorrect behavior).

> Do you believe this is the right method to deal with the problem?

We're already stuck with compilers in the wild where casting to void is
insufficient to shut up the compiler warnings, so at this point in time,
any GNU code that wants to avoid warnings is better off using gnulib's
ignore_value() than a cast to void.  At which point, it should not
matter what ignore_value() uses under the hood (personally, I PREFER use
of ignore_value() - function syntax is easier to read as declaring the
intent of what I'm doing than cast syntax, and a coding style that
minimizes casts is preferred).

> It will most likely prevent to create enough pressure on the GCC people to fix 
> the current warning problem.

No, I don't think that gnulib's use of ignore_value() is enough pressure
to make gcc change their warning behavior.  Nor do I care - even if gcc
changes behavior, I still plan on using ignore_value() (and let gnulib
figure out how to map ignore_value to the most efficient semantics for
each compiler it is tested with).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

[signature.asc (application/pgp-signature, attachment)]

Message #46 received at 15970 <at> debbugs.gnu.org (full text, mbox):

From: Joerg Schilling <Joerg.Schilling <at> fokus.fraunhofer.de>
To: <P <at> draigBrady.com>, <eblake <at> redhat.com>
Cc: eggert <at> cs.ucla.edu, 15970 <at> debbugs.gnu.org, bug-tar <at> gnu.org
Subject: Re: ignore_value vs (void) [was: [Bug-tar] bug#15970: bug#15970:
 Crash in gettext() after fork() on Mac OS X]
Date: Wed, 27 Nov 2013 17:53:42 +0100

Eric Blake <eblake <at> redhat.com> wrote:

> On 11/27/2013 09:30 AM, Joerg Schilling wrote:
> >>> Are you using "ignore_value" because of the gcc bug that ignores the official
> >>> method to tell that the return code is ignored intentionally:
> >>>
> >>> 	(void) write(fd, buf, len);
>
> The C standard does not document that '(void) write(...)' is required to
> avoid diagnostics, so I don't see the gcc behavior as a bug (yes, it's
> annoying behavior, but that doesn't make it incorrect behavior).

Well, there is a more than 30 year history of lint that causes 
'(void) write(...)' to be accepted without a warning.

Jörg

-- 
 EMail:joerg <at> schily.isdn.cs.tu-berlin.de (home) Jörg Schilling D-13353 Berlin
       js <at> cs.tu-berlin.de                (uni)  
       joerg.schilling <at> fokus.fraunhofer.de (work) Blog: http://schily.blogspot.com/
 URL:  http://cdrecord.berlios.de/private/ ftp://ftp.berlios.de/pub/schily

Message #49 received at 15970 <at> debbugs.gnu.org (full text, mbox):

From: Eric Blake <eblake <at> redhat.com>
To: Joerg Schilling <Joerg.Schilling <at> fokus.fraunhofer.de>, P <at> draigBrady.com
Cc: eggert <at> cs.ucla.edu, 15970 <at> debbugs.gnu.org, bug-tar <at> gnu.org
Subject: Re: ignore_value vs (void) [was: [Bug-tar] bug#15970: bug#15970:
 Crash in gettext() after fork() on Mac OS X]
Date: Wed, 27 Nov 2013 09:57:12 -0700

[Message part 1 (text/plain, inline)]

On 11/27/2013 09:53 AM, Joerg Schilling wrote:
>> The C standard does not document that '(void) write(...)' is required to
>> avoid diagnostics, so I don't see the gcc behavior as a bug (yes, it's
>> annoying behavior, but that doesn't make it incorrect behavior).
> 
> Well, there is a more than 30 year history of lint that causes 
> '(void) write(...)' to be accepted without a warning.

So? Lint is not gcc, and gcc is not lint.  Different compilers have
different capabilities at producing warnings, and you cannot expect that
something that works warning-free on one compiler will remain
warning-free on other compilers (even later versions of the same
compiler).  Warnings are an art form, and not an exact science,
precisely because the C standard does not (rather, cannot) specify them.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

[signature.asc (application/pgp-signature, attachment)]

Message #52 received at 15970 <at> debbugs.gnu.org (full text, mbox):

From: Joerg Schilling <Joerg.Schilling <at> fokus.fraunhofer.de>
To: <P <at> draigBrady.com>, <eblake <at> redhat.com>
Cc: eggert <at> cs.ucla.edu, 15970 <at> debbugs.gnu.org, bug-tar <at> gnu.org
Subject: Re: ignore_value vs (void) [was: [Bug-tar] bug#15970: bug#15970:
 Crash in gettext() after fork() on Mac OS X]
Date: Wed, 27 Nov 2013 18:05:39 +0100

Eric Blake <eblake <at> redhat.com> wrote:

> On 11/27/2013 09:53 AM, Joerg Schilling wrote:
> >> The C standard does not document that '(void) write(...)' is required to
> >> avoid diagnostics, so I don't see the gcc behavior as a bug (yes, it's
> >> annoying behavior, but that doesn't make it incorrect behavior).
> > 
> > Well, there is a more than 30 year history of lint that causes 
> > '(void) write(...)' to be accepted without a warning.
>
> So? Lint is not gcc, and gcc is not lint.  Different compilers have
> different capabilities at producing warnings, and you cannot expect that
> something that works warning-free on one compiler will remain
> warning-free on other compilers (even later versions of the same
> compiler).  Warnings are an art form, and not an exact science,
> precisely because the C standard does not (rather, cannot) specify them.

If you like to stay in an environment that is based on GCC only, you may use 
ignore_value(). If you however like to write portable code, you cannot expect 
something like ignore_value() to be available. Given the fact that other 
compilers honor the (void) cast, it seems that gcc is trying to be different in 
a way that just causes pain but that does not give extra value.

Jörg

-- 
 EMail:joerg <at> schily.isdn.cs.tu-berlin.de (home) Jörg Schilling D-13353 Berlin
       js <at> cs.tu-berlin.de                (uni)  
       joerg.schilling <at> fokus.fraunhofer.de (work) Blog: http://schily.blogspot.com/
 URL:  http://cdrecord.berlios.de/private/ ftp://ftp.berlios.de/pub/schily

Message #55 received at 15970 <at> debbugs.gnu.org (full text, mbox):

From: Eric Blake <eblake <at> redhat.com>
To: Joerg Schilling <Joerg.Schilling <at> fokus.fraunhofer.de>, P <at> draigBrady.com
Cc: eggert <at> cs.ucla.edu, 15970 <at> debbugs.gnu.org, bug-tar <at> gnu.org
Subject: Re: ignore_value vs (void) [was: [Bug-tar] bug#15970: bug#15970:
 Crash in gettext() after fork() on Mac OS X]
Date: Wed, 27 Nov 2013 10:11:42 -0700

[Message part 1 (text/plain, inline)]

On 11/27/2013 10:05 AM, Joerg Schilling wrote:
> If you like to stay in an environment that is based on GCC only, you may use 
> ignore_value(). If you however like to write portable code, you cannot expect 
> something like ignore_value() to be available.

False.  Gnulib already takes care of that; it #defines ignore_value(e)
to ((void)(e)) on all non-gcc compilers.  In other words, ignore_value()
is portable to ANY standards-compliant compiler, and ALSO has the
benefit of shutting up warnings on as many compilers as gnulib knows how
to test.

> Given the fact that other 
> compilers honor the (void) cast,

GCC honors (void) casting - but that doesn't shut up the warning.  Just
because the standards require (void)expression to compile (and gcc
compiles it just fine) doesn't mean that the compiler can't warn about
it being suspicious.

> it seems that gcc is trying to be different in 
> a way that just causes pain but that does not give extra value.

Maybe, but that cat is already out of the bag, and cross-posting to
these lists is not the effective forum to do anything about it.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

[signature.asc (application/pgp-signature, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.