Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 25 Feb 2012 04:27:35 +1100 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Jilles Tjoelker <jilles@freebsd.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r232108 - head/usr.bin/xargs
Message-ID:  <20120225024051.C1150@besplex.bde.org>
In-Reply-To: <201202241235.q1OCZH2U059017@svn.freebsd.org>
References:  <201202241235.q1OCZH2U059017@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 24 Feb 2012, Jilles Tjoelker wrote:

> Log:
>  xargs: If a utility exits with 255 or a signal, write an error message.
>
>  If a utility called by xargs exits with status 255 or because of a signal,
>  POSIX requires writing an error message.

Is an exit status of 255 really possible?  I thought that there was
magic for the top bit.  But testing reminded me that at the level of
a single exit(), the magic in the top bit is for signals, and showed
that WIFSIGNALED() is broken for signal 127:

% #include <sys/wait.h>
% 
% #include <signal.h>
% #include <stdio.h>
% #include <stdlib.h>
% #include <unistd.h>
% 
% int
% main(void)
% {
% 	int status;
% 
% 	switch (fork()) {
% 	case -1:
% 	default:
% 		wait(&status);
% 		printf("CONTINUED = %d\n", WIFCONTINUED(status));
% 		printf("EXITED = %d\n", WIFEXITED(status));
% 		printf("SIGNALED = %d\n", WIFSIGNALED(status));
% 		printf("STOPPED = %d\n", WIFSTOPPED(status));
% 		printf("EXITSTATUS = %d\n", WEXITSTATUS(status));
% 		printf("TERMSIG = %d\n", WTERMSIG(status));
% 		printf("COREDUMP = %d\n", WCOREDUMP(status));
% 		printf("STOPSIG = %d\n", WSTOPSIG(status));
% 		printf("status = %#x\n", status);
% 		exit(0);
% 	case 0:
% 		kill(getpid(), 127);
% 	}
% }

This prints SIGNALED = 0 and STOPPED = 1, when it should print
SIGNALED = 1 and STOPPED = 0.  This is because the special _WSTATUS()
of 127 is _WSTOPPED and this cannot be distinguished from an actual
signal 127.  Probably this signal should be disallowed.  (The magic
in the top bit is just the core dump flag.  Signals 128-255 are
already disallowed, so they don't give ambiguity.)

Utilities are quite broken near here too:
- under FreeBSD-~5.2:
   - bash-1(1) exit status messages, bash-1 builtin kill, kill(1), killall(1),
     and time(1) don't support signals >= 32, and give cryptic error messages
   - kill(2) with signal 127 works, but after ktracing processes killed by it,
     kdump(1) dumps core.
   - kill(2) with signal 126 works, but bash-1 exits when a sub-process
     does this.  (Bash-1 also exits for kill on a nonexisting process.)
     After ktrace is run under /bin/sh, kdump dumps core.
   - kill(2) with signal 32 works, but bash-1 is broken as for signal 126.
     kdump now doesn't dump core, but prints PSIG(null).

- under -current:
   - utilities still don't support signals >= 32, but give better error
     messages.
   - kill(2) with signal 127 works, but bash prints the bogus termination
     message "Stopped ..." for it.  Apparently it us confused by the above
     bug in WIFSTOPPED()/WIFSIGNALED().  Job control is broken by this,
     but the breakage is not very noticeable -- the jobs command shows
     many jobs for the dead processes, and you can clean it out by trying
     to resume these jobs.
   - /bin/sh is similarly confused, but prints "Suspended ..." instead of
     "Stopped ...".
   - kdump now works.

Support for 128 signals was added to the kernel in 1999 in time for
FreeBSD-4, so even FreeBSD-5 userland shouldn't be so broken.  However,
ISTR a PR about problems with specifically signal 127, and thought
that is was already disallowed.

> Modified: head/usr.bin/xargs/xargs.c
> ==============================================================================
> --- head/usr.bin/xargs/xargs.c	Fri Feb 24 12:32:50 2012	(r232107)
> +++ head/usr.bin/xargs/xargs.c	Fri Feb 24 12:35:17 2012	(r232108)
> @@ -608,8 +608,11 @@ waitchildren(const char *name, int waita
> 		 * If utility signaled or exited with a value of 255,
> 		 * exit 1-125.
> 		 */

This comment didn't match the code before (the code always exited with
status 1), and is now further from matching the code.

This comment is hard to parse.

> -		if (WIFSIGNALED(status) || WEXITSTATUS(status) == 255)
> -			exit(1);
> +		if (WIFSIGNALED(status))
> +			errx(1, "%s: terminated with signal %d, aborting",
> +			    name, WTERMSIG(status));

This misclassifies signal 127 due to the above bug.

> +		if (WEXITSTATUS(status) == 255)
> +			errx(1, "%s: exited with status 255, aborting", name);
> 		if (WEXITSTATUS(status))
> 			rval = 1;

Neither WIFSTOPPED() nor WIFEXITED() are tested, so the result of the
misclassification is turning signal 127 into a normal exit with status
0.  A normal WIFSTOPPED() should not get here, else job control would
just break xargs, so the bug can probably be worked around by turning
WIFSTOPPED() into WIFSIGNALED() here, or just blindly using WTERMSIG()
!= 0 to classify signals.

The messages have a grammar error (comma splice).

> 	}
>

The magic for the top bit is when exit statuses are combined with
signal statuses and bits are lost by forcing them through an 8-bit
filter (perhaps another exit()).  Shells report signal n as $? =
(0x80 | n).  When n is 127, shells are not quite confused enough to
turn the status into 0 -- they just turn n into 0, giving status 0x80.

Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120225024051.C1150>