Skip to content

Commit

Permalink
spawnveg (libast): remove vfork support (re: b810e2d)
Browse files Browse the repository at this point in the history
spawnveg(3) too engaged in undefined behaviour according to POSIX
if its vfork fallback version was compiled in. It may have worked
well enough on most systems at the time it was written, but no one
has time to keep verifying that. Plus, all modern systems now have
posix_spawn(3), which is the preferred option for spawnveg. So for
us, this is another opportunity for code cleanup.

This commit does not affect ksh's ability to work on old systems,
it's only that running external commands might now be slightly
slower (and quite possibly more reliable) on old systems without
posix_spawn as they will now use the standard fork(2).
  • Loading branch information
McDutchie committed Apr 5, 2023
1 parent b810e2d commit 0ea7d1a
Show file tree
Hide file tree
Showing 10 changed files with 8 additions and 174 deletions.
3 changes: 0 additions & 3 deletions src/cmd/ksh93/Mamfile
Original file line number Diff line number Diff line change
Expand Up @@ -1006,8 +1006,6 @@ make install
make sh/path.c
prev FEATURE/time implicit
prev ${PACKAGE_ast_INCLUDE}/endian.h implicit
make ${PACKAGE_ast_INCLUDE}/ast_vfork.h implicit
done ${PACKAGE_ast_INCLUDE}/ast_vfork.h dontcare
make exec_attr.h implicit
done exec_attr.h dontcare virtual
prev FEATURE/dynamic implicit
Expand Down Expand Up @@ -1117,7 +1115,6 @@ make install
done waitevent.o generated
make xec.o
make sh/xec.c
prev ${PACKAGE_ast_INCLUDE}/ast_vfork.h implicit
prev ${PACKAGE_ast_INCLUDE}/vmalloc.h implicit
prev include/streval.h implicit
prev FEATURE/locale implicit
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/tests/path.sh
Original file line number Diff line number Diff line change
Expand Up @@ -738,7 +738,7 @@ PATH=$savePATH
# POSIX: If a command is found but isn't executable, the exit status should be 126.
# The tests are arranged as follows:
# Test *A runs commands with the -c execve(2) optimization.
# Test *B runs commands with spawnveg (i.e., with posix_spawn(3) or vfork(2)).
# Test *B runs commands with spawnveg (i.e., with posix_spawn(3) where available).
# Test *C runs commands with fork(2) in an interactive shell.
# Test *D runs commands with 'command -x'.
# Test *E runs commands with 'exec'.
Expand Down
26 changes: 1 addition & 25 deletions src/lib/libast/Mamfile
Original file line number Diff line number Diff line change
Expand Up @@ -1463,8 +1463,6 @@ make install
prev port/lclib.h implicit
make sfio/sfhdr.h implicit
prev port/lclib.h implicit
make vfork.h implicit
done vfork.h dontcare virtual
make stropts.h implicit
done stropts.h dontcare virtual
prev std/wchar.h implicit
Expand Down Expand Up @@ -2706,14 +2704,6 @@ make install
done execvpe.o generated
make spawnveg.o
make comp/spawnveg.c
make ast_vfork.h implicit
make FEATURE/vfork
make features/vfork
done features/vfork
exec - iffe ${IFFEFLAGS} -v -X ast -X std -c "${CC} ${mam_cc_FLAGS} ${CCFLAGS} ${LDFLAGS}" run features/vfork
done FEATURE/vfork generated
exec - cmp -s FEATURE/vfork ast_vfork.h || { rm -f ast_vfork.h; ${STDCP} FEATURE/vfork ast_vfork.h; }
done ast_vfork.h dontcare generated
prev ast_tty.h implicit
prev sig.h implicit
make process.h implicit
Expand All @@ -2725,14 +2715,6 @@ make install
prev comp/spawnveg.c
exec - ${CC} ${mam_cc_FLAGS} ${CCFLAGS} ${-debug-symbols?1?${mam_cc_DEBUG} -D_BLD_DEBUG?${CCFLAGS.FORCE}?} -I. -Icomp -Iinclude -Istd -c comp/spawnveg.c
done spawnveg.o generated
make vfork.o
make comp/vfork.c
prev include/error.h implicit
prev include/ast.h implicit
done comp/vfork.c
prev comp/vfork.c
exec - ${CC} ${mam_cc_FLAGS} ${CCFLAGS} -I. -Icomp -Iinclude -Istd -c comp/vfork.c
done vfork.o generated
make killpg.o
make comp/killpg.c
prev sig.h implicit
Expand Down Expand Up @@ -5094,7 +5076,7 @@ make install
exec - ${AR} rc libast.a state.o opendir.o readdir.o rewinddir.o seekdir.o telldir.o getcwd.o fastfind.o hashalloc.o hashdump.o hashfree.o hashlast.o hashlook.o hashscan.o hashsize.o hashview.o hashwalk.o memhash.o memsum.o strhash.o strkey.o strsum.o stracmp.o strnacmp.o ccmap.o ccmapid.o ccnative.o chresc.o chrtoi.o
exec - ${AR} rc libast.a streval.o strexpr.o strmatch.o strcopy.o modei.o modex.o strmode.o strlcat.o strlcpy.o strlook.o strncopy.o strsearch.o strpsearch.o stresc.o stropt.o strtape.o strpcmp.o strnpcmp.o strvcmp.o strnvcmp.o tok.o tokline.o tokscan.o pathaccess.o pathcat.o pathcanon.o pathcheck.o pathpath.o pathexists.o pathfind.o pathicase.o pathkey.o pathprobe.o pathrepl.o pathnative.o pathposix.o pathtemp.o pathtmp.o pathstat.o pathgetlink.o pathsetlink.o pathbin.o pathshell.o pathcd.o pathprog.o ftwalk.o ftwflags.o fts.o astintercept.o conformance.o getenv.o setenviron.o optget.o optjoin.o optesc.o optctx.o strsort.o struniq.o magic.o mime.o mimetype.o signal.o sigflag.o systrace.o error.o errorf.o errormsg.o errorx.o localeconv.o setlocale.o translate.o catopen.o iconv.o lc.o lctab.o mc.o base64.o recfmt.o recstr.o reclen.o fmtrec.o fmtbase.o fmtbuf.o fmtclock.o fmtdev.o fmtelapsed.o fmterror.o fmtesc.o fmtfmt.o fmtfs.o fmtident.o fmtint.o fmtip4.o fmtip6.o fmtls.o fmtmatch.o fmtmode.o fmtnum.o fmtperm.o fmtre.o fmttime.o
exec - ${AR} rc libast.a fmtuid.o fmtgid.o fmtsignal.o fmtscale.o fmttmx.o fmttv.o fmtversion.o strelapsed.o strperm.o struid.o strgid.o strtoip4.o strtoip6.o stack.o stk.o swapget.o swapmem.o swapop.o swapput.o sigdata.o sigcrit.o sigunblock.o procopen.o procclose.o procrun.o procfree.o tmdate.o tmequiv.o tmfix.o tmfmt.o tmform.o tmgoff.o tminit.o tmleap.o tmlex.o tmlocale.o tmmake.o tmpoff.o tmscan.o tmsleep.o tmtime.o tmtype.o tmweek.o tmword.o tmzone.o tmxdate.o tmxduration.o tmxfmt.o tmxgettime.o tmxleap.o tmxmake.o tmxscan.o tmxsettime.o tmxsleep.o tmxtime.o tmxtouch.o tvcmp.o tvgettime.o tvsettime.o tvsleep.o tvtouch.o cmdarg.o vecargs.o vecfile.o vecfree.o vecload.o vecstring.o univdata.o touch.o mnt.o debug.o memccpy.o memchr.o memcmp.o memcpy.o memdup.o memmove.o memset.o mkdir.o mkfifo.o mknod.o rmdir.o remove.o rename.o link.o unlink.o strdup.o strchr.o strrchr.o strstr.o strtod.o strtold.o strtol.o strtoll.o strtoul.o strtoull.o strton.o strtonll.o strntod.o strntold.o strnton.o
exec - ${AR} rc libast.a strntonll.o strntol.o strntoll.o strntoul.o strntoull.o strcasecmp.o strncasecmp.o strerror.o mktemp.o tmpnam.o fsync.o execlp.o execve.o execvp.o execvpe.o spawnveg.o vfork.o killpg.o getlogin.o putenv.o setenv.o unsetenv.o lstat.o statvfs.o eaccess.o gross.o omitted.o readlink.o symlink.o getpgrp.o setpgid.o setsid.o fcntl.o open.o atexit.o getdents.o getwd.o dup2.o errno.o getgroups.o mount.o system.o iblocks.o modedata.o tmdata.o memfatal.o sfkeyprintf.o sfdcdio.o sfdcdos.o sfdcfilter.o sfdcseekable.o sfdcslow.o sfdcsubstr.o sfdctee.o sfdcunion.o sfdcmore.o sfdcprefix.o wc.o wc2utf8.o basename.o closelog.o dirname.o fmtmsglib.o fnmatch.o ftw.o getdate.o getsubopt.o glob.o nftw.o openlog.o re_comp.o resolvepath.o realpath.o regcmp.o regexp.o setlogmask.o strftime.o strptime.o swab.o syslog.o tempnam.o wordexp.o mktime.o regalloc.o regclass.o regcoll.o regcomp.o regcache.o regdecomp.o regerror.o regexec.o regfatal.o reginit.o
exec - ${AR} rc libast.a strntonll.o strntol.o strntoll.o strntoul.o strntoull.o strcasecmp.o strncasecmp.o strerror.o mktemp.o tmpnam.o fsync.o execlp.o execve.o execvp.o execvpe.o spawnveg.o killpg.o getlogin.o putenv.o setenv.o unsetenv.o lstat.o statvfs.o eaccess.o gross.o omitted.o readlink.o symlink.o getpgrp.o setpgid.o setsid.o fcntl.o open.o atexit.o getdents.o getwd.o dup2.o errno.o getgroups.o mount.o system.o iblocks.o modedata.o tmdata.o memfatal.o sfkeyprintf.o sfdcdio.o sfdcdos.o sfdcfilter.o sfdcseekable.o sfdcslow.o sfdcsubstr.o sfdctee.o sfdcunion.o sfdcmore.o sfdcprefix.o wc.o wc2utf8.o basename.o closelog.o dirname.o fmtmsglib.o fnmatch.o ftw.o getdate.o getsubopt.o glob.o nftw.o openlog.o re_comp.o resolvepath.o realpath.o regcmp.o regexp.o setlogmask.o strftime.o strptime.o swab.o syslog.o tempnam.o wordexp.o mktime.o regalloc.o regclass.o regcoll.o regcomp.o regcache.o regdecomp.o regerror.o regexec.o regfatal.o reginit.o
exec - ${AR} rc libast.a regnexec.o regsubcomp.o regsubexec.o regsub.o regrecord.o regrexec.o regstat.o dtclose.o dtdisc.o dthash.o dtlist.o dtmethod.o dtopen.o dtstat.o dtstrhash.o dttree.o dtuser.o dtview.o dtwalk.o dtnew.o dtcomp.o sfclose.o sfclrlock.o sfdisc.o sfdlen.o sfexcept.o sfgetl.o sfgetu.o sfcvt.o sfecvt.o sffcvt.o sfextern.o sffilbuf.o sfflsbuf.o sfprints.o sfgetd.o sfgetr.o sfllen.o sfmode.o sfmove.o sfnew.o sfpkrd.o sfnotify.o sfnputc.o sfopen.o sfpeek.o sfpoll.o sfpool.o sfpopen.o sfprintf.o sfputd.o sfputl.o sfputr.o sfputu.o sfrd.o sfread.o sfreserve.o sfscanf.o sfseek.o sfset.o sfsetbuf.o sfsetfd.o sfsize.o sfsk.o sfstack.o sfstrtod.o sfsync.o sfswap.o sftable.o sftell.o sftmp.o sfungetc.o sfvprintf.o sfvscanf.o sfwr.o sfwrite.o sfpurge.o sfraise.o sfwalk.o sfgetm.o sfputm.o sfresize.o _sfclrerr.o _sfeof.o _sferror.o _sffileno.o _sfopen.o _sfstacked.o _sfvalue.o _sfgetc.o _sfgetl.o _sfgetl2.o _sfgetu.o _sfgetu2.o _sfdlen.o _sfllen.o _sfslen.o _sfulen.o _sfputc.o _sfputd.o _sfputl.o _sfputm.o
exec - ${AR} rc libast.a _sfputu.o clearerr.o fclose.o fdopen.o fflush.o fgetc.o fgetpos.o fgets.o fopen.o fprintf.o fpurge.o fputs.o fread.o freopen.o fscanf.o fseek.o fseeko.o fsetpos.o ftell.o ftello.o fwrite.o getw.o pclose.o popen.o printf.o putchar.o puts.o putw.o rewind.o scanf.o setbuf.o setbuffer.o setlinebuf.o setvbuf.o snprintf.o sprintf.o sscanf.o asprintf.o vasprintf.o tmpfile.o ungetc.o vfprintf.o vfscanf.o vprintf.o vscanf.o vsnprintf.o vsprintf.o vsscanf.o _doprnt.o _doscan.o _filbuf.o _flsbuf.o _stdopen.o _stdprintf.o _stdscanf.o _stdsprnt.o _stdvbuf.o _stdvsnprnt.o _stdvsprnt.o _stdvsscn.o fgetwc.o fwprintf.o putwchar.o vfwscanf.o wprintf.o fgetws.o fwscanf.o swprintf.o vswprintf.o wscanf.o fputwc.o getwc.o swscanf.o vswscanf.o fputws.o getwchar.o ungetwc.o vwprintf.o fwide.o putwc.o vfwprintf.o vwscanf.o stdio_c99.o fcloseall.o fmemopen.o getdelim.o getline.o frexp.o frexpl.o astcopy.o
exec - ${AR} rc libast.a astconf.o astdynamic.o astquery.o astwinsize.o conftab.o aststatic.o getopt.o getoptl.o aso.o asolock.o asometh.o asorelax.o aso-sem.o aso-fcntl.o vmbest.o vmclear.o vmclose.o vmdcheap.o vmdebug.o vmdisc.o vmlast.o vmopen.o vmpool.o vmprivate.o vmprofile.o vmregion.o vmsegment.o vmset.o vmstat.o vmstrdup.o vmtrace.o vmwalk.o vmmopen.o malloc.o vmgetmem.o
Expand Down Expand Up @@ -5961,12 +5943,6 @@ make install
exec - then ${STDCP} ast_tty.h ${INSTALLROOT}/include/ast/ast_tty.h
exec - fi
done ${INSTALLROOT}/include/ast/ast_tty.h generated
make ${INSTALLROOT}/include/ast/ast_vfork.h
prev ast_vfork.h
exec - if ! cmp -s ast_vfork.h ${INSTALLROOT}/include/ast/ast_vfork.h
exec - then ${STDCP} ast_vfork.h ${INSTALLROOT}/include/ast/ast_vfork.h
exec - fi
done ${INSTALLROOT}/include/ast/ast_vfork.h generated
make ${INSTALLROOT}/include/ast/ast_wait.h
prev ast_wait.h
exec - if ! cmp -s ast_wait.h ${INSTALLROOT}/include/ast/ast_wait.h
Expand Down
32 changes: 0 additions & 32 deletions src/lib/libast/comp/spawnveg.c
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,6 @@ spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid, i
#include <wait.h>
#include <sig.h>
#include <ast_tty.h>
#include <ast_vfork.h>

#if _lib_spawnve && _hdr_process
#include <process.h>
Expand All @@ -162,10 +161,6 @@ spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid, i
#endif
#endif

#if !_lib_vfork
#undef _real_vfork
#endif

/*
* fork+exec+(setsid|setpgid)
*/
Expand All @@ -177,12 +172,7 @@ spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid, i
int m;
pid_t pid;
pid_t rid;
#if _real_vfork
volatile int exec_errno;
volatile int* volatile exec_errno_ptr;
#else
int err[2];
#endif /* _real_vfork */

NOT_USED(tcfd);
if (!envv)
Expand All @@ -192,24 +182,15 @@ spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid, i
return spawnve(path, argv, envv);
#endif /* _lib_spawnve */
n = errno;
#if _real_vfork
exec_errno = 0;
exec_errno_ptr = &exec_errno;
#else
if (pipe(err) < 0)
err[0] = -1;
else
{
fcntl(err[0], F_SETFD, FD_CLOEXEC);
fcntl(err[1], F_SETFD, FD_CLOEXEC);
}
#endif /* _real_vfork */
sigcritical(SIG_REG_EXEC|SIG_REG_PROC);
#if _lib_vfork
pid = vfork();
#else
pid = fork();
#endif /* _lib_vfork */
if (pid == -1)
n = errno;
else if (!pid)
Expand All @@ -233,26 +214,14 @@ spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid, i
#endif /* _lib_tcgetpgrp */
}
execve(path, argv, envv);
#if _real_vfork
*exec_errno_ptr = errno;
#else
if (err[0] != -1)
{
m = errno;
write(err[1], &m, sizeof(m));
}
#endif /* _real_vfork */
_exit(errno == ENOENT ? EXIT_NOTFOUND : EXIT_NOEXEC);
}
rid = pid;
#if _real_vfork
if (pid != -1 && (m = *exec_errno_ptr))
{
while (waitpid(pid, NULL, 0) == -1 && errno == EINTR);
rid = pid = -1;
n = m;
}
#else
if (err[0] != -1)
{
close(err[1]);
Expand All @@ -274,7 +243,6 @@ spawnveg(const char* path, char* const argv[], char* const envv[], pid_t pgid, i
}
close(err[0]);
}
#endif /* _real_vfork */
sigcritical(0);
if (pid != -1 && pgid > 0)
{
Expand Down
38 changes: 0 additions & 38 deletions src/lib/libast/comp/vfork.c

This file was deleted.

49 changes: 3 additions & 46 deletions src/lib/libast/features/lib
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ ref -D_def_map_ast=1
cmd universe

hdr dirent,direntry,filio,fmtmsg,fnmatch,jioctl,libgen,limits
hdr locale,ndir,nl_types,process,spawn,syslog,utime,vfork
hdr locale,ndir,nl_types,process,spawn,syslog,utime
hdr linux/fs,linux/msdos_fs,sys/ioctl
hdr wctype
hdr wchar note{ <wchar.h> and isw*() really work }end execute{
Expand Down Expand Up @@ -52,7 +52,7 @@ mem DIR.dd_fd sys/types.h - dirent.h - sys/dir.h
mem inheritance.pgroup spawn.h

sys dir,filio,jioctl,localedef,ptem,resource
sys socket,stream,systeminfo,universe,vfork
sys socket,stream,systeminfo,universe

tst tst_errno note{ errno can be assigned }end link{
#define error ______error
Expand Down Expand Up @@ -187,49 +187,6 @@ tst pipe_rw note{ full duplex pipes }end execute{
}
}end

tst lib_vfork unistd.h stdlib.h vfork.h note{ vfork exists and it works }end execute{
#include <signal.h>
#include <sys/wait.h>
int
main(argc, argv)
int argc;
char** argv;
{
int status;
char* cmd[3];
if (argv[1])
_exit(signal(SIGHUP, SIG_DFL) != SIG_IGN);
signal(SIGHUP, SIG_IGN);
switch (vfork())
{
case -1:
_exit(1);
case 0:
cmd[0] = argv[0];
cmd[1] = "test";
cmd[2] = 0;
execv(cmd[0], cmd);
_exit(2);
}
status = 1;
_exit(wait(&status) < 0 || status != 0);
}
}end

tst real_vfork note{ vfork child shares data with parent }end execute{
extern int _exit(int);
extern int vfork(void);
int code;
int
main()
{
code = 1;
if (!vfork())
code = 0;
_exit(code);
}
}end

tst lib_posix_spawn unistd.h stdlib.h spawn.h -Dfork=______fork note{ posix_spawn exists, it works and it's worth using }end status{
#include <sys/types.h>
#include <sys/stat.h>
Expand Down Expand Up @@ -636,7 +593,7 @@ tst - output{
int
main()
{
#if _lib_posix_spawn || _lib_spawn_mode || _lib_spawn && _hdr_spawn && _mem_pgroup_inheritance || _lib_vfork && _real_vfork
#if _lib_posix_spawn || _lib_spawn_mode || _lib_spawn && _hdr_spawn && _mem_pgroup_inheritance
printf("#if !_AST_no_spawnveg\n");
printf("#define _use_spawnveg 1\n");
printf("#endif\n");
Expand Down
12 changes: 0 additions & 12 deletions src/lib/libast/features/vfork

This file was deleted.

1 change: 0 additions & 1 deletion src/lib/libast/man/compat.3
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ long sysconf(int);
int system(const char*);
char* tmpnam(char*);
int unlink(const char*);
int vfork(void);
int waitpid(pid_t, int*, int);
.EE
.SH DESCRIPTION
Expand Down
8 changes: 3 additions & 5 deletions src/lib/libast/man/spawnveg.3
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,9 @@ combines
and
.IR setsid (2)
into a single call.
If one of either
If
.IR posix_spawn (3)
or
.IR vfork (2)
is available, those functions are preferred over
is available, that function is preferred over
.IR fork .
.PP
.LR command ,
Expand Down Expand Up @@ -109,4 +107,4 @@ cannot make the new process a session leader when using the
.I posix_spawn
API.
.SH "SEE ALSO"
fork(2), vfork(2), posix_spawn(3), exec(2), setpgid(2), setsid(2), spawnve(2)
fork(2), posix_spawn(3), exec(2), setpgid(2), setsid(2), spawnve(2)
11 changes: 0 additions & 11 deletions src/lib/libast/sfio/sfhdr.h
Original file line number Diff line number Diff line change
Expand Up @@ -138,17 +138,6 @@
#define X_OK 01
#endif

/* alternative process forking */
#if _lib_vfork && !defined(fork) && !defined(__sparc) && !defined(__sparc__)
#if _hdr_vfork
#include <vfork.h>
#endif
#if _sys_vfork
#include <sys/vfork.h>
#endif
#define fork vfork
#endif

/* Private flags in the "bits" field */
#define SF_MMAP 00000001 /* in memory mapping mode */
#define SF_BOTH 00000002 /* both read/write */
Expand Down

0 comments on commit 0ea7d1a

Please sign in to comment.