On Cygwin, ksh does not execute scripts without a #! path in a fork
of the ksh process as it does on other systems. Reproducer (run
from ksh):
$ cat test.sh
echo "${BASH_VERSION:-not bash}"
echo "${.sh.version}"
$ chmod +x test.sh
$ ./test.sh
4.4.12(3)-release
./test.sh: line 2: ${.sh.version}: bad substitution
The script was executed in bash instead of ksh.
After this fix, the output on Cygwin is like ksh on other systems:
not bash
Version AJM 93u+m/1.1.0-alpha+dev 2022-01-26
This also fixes a number of regression test failures, as quite a
few tests create and execute temp scripts without a hashbang path.
Analysis: On Cygwin, execve(2) happily executes shell scripts
without a #! path with /bin/sh (which is bash --posix). However,
ksh relies on execve(2) executing binaries or #! only, as it uses
an ENOEXEC failure to decide whether to fork and execute a #!-less
shell script with a reinitialized copy of itself via exscript().
src/cmd/ksh93/sh/path.c: path_spawn():
- Look at the magic first two bytes of the file; if it is "MZ"
(Mark Zbikowski, originator of the .exe format) or "#!", continue
as normal, otherwise simulate an ENOEXEC failure from execve(2)
which will cause ksh to fall back on #!-less script execution.
These are minor things I accumulated over the last month or so.
Notable changes:
src/lib/libast/features/api,
src/lib/libast/misc/state.c,
src/lib/libast/comp/conf.tab,
src/cmd/ksh93/include/defs.h:
- Bump internal libast version to 20220101L. We've made a few
additions to the API, at least pathicase (see 71934570, ca3ec200)
and astconf_long (see c2ac69b2), so this should have been done
already. This also updates '/opt/ast/bin/getconf _AST_VERSION'.
- Use AST_VERSION instead of outdated _AST_VERSION.
- In state.c, use AST_VERSION instead of hardcoding the version.
src/cmd/ksh93/sh/xec.c:
- Remove 'restorefd' variable, unused as of 42becab6.
- Remove 'cmdrecurse' function and SH_RUNPROG macro; this was once
used by a few libcmd commands, but ast-open-archive reveals it's
unused as of ast 1999-12-25.
src/cmd/ksh93/sh/*.c:
- Where available, use e_dot instead of "." for consistency; it is
defined as an extern so we might as well use it.
src/cmd/ksh93/tests/*.sh:
- When reporting signal names in fails, include the SIG prefix.
- Fix a broken process hang test in subshell.sh.
src/lib/libast/man/sfdisc.3:
- Removed. The interfaces described here never made it out of AT&T;
they do not exist in any libast version in ast-open-archive.
Resolves: https://github.com/ksh93/ksh/issues/426
This commit makes various different improvements to the documentation:
- sh.1: Backported (with changes) mandoc warning fixes from ksh2020
for the ksh93(1) man page: <https://github.com/att/ast/pull/1406>
- Removed unnecessary spaces at the end of lines to fix a few other
mandoc warnings.
- Fixed various typos and capitalization errors in the documentation.
- ANNOUNCE: Document the addition of the ${.sh.pid} variable
(re: 9de65210).
- libast/man/str*: Update the man pages for the libast str* functions
to improve how accurately each function is described.
- ksh93/README: Update regression test/compatibility notes to include
OpenBSD 7.0, FreeBSD 13.0 and WSL running Ubuntu 20.04.
- Change a few places to store the return value from strlen in a
size_t variable rather than signed int.
- comp/setlocale.c: To avoid confusion of two separate variables named
lang, the function local variable has been renamed to langidx.
This commit implements the build fixes required to get ksh running on
Haiku. Note that while ksh does compile, it has a ton of regression test
failures on Haiku.
src/cmd/ksh93/data/signals.c,
src/lib/libast/features/signal.c:
- Add support for the SIGKILLTHR signal, which is supported by BeOS and
Haiku.
- SIGINFO was missing an entry in the libast feature test, so add one
(re: 658bba74).
src/cmd/ksh93/RELEASE:
- Add an entry noting that ksh now compiles on Haiku, albeit with many
regression test failures.
src/cmd/ksh93/{include/terminal.h,sh/path.c}:
- Silence compiler warnings on Haiku.
src/lib/libast/features/mmap:
- The mmap feature test freezes on Haiku, so modify the test to fail
immediately on that OS.
src/lib/libast/misc/signal.c:
- Avoid redefining the signal definition on Haiku to fix a compiler
error.
src/lib/libast/features/nl_types:
- For some reason the nl_item typedef on Haiku doesn't work correctly.
Work around that by creating the nl_item type in the libast nl_types
feature test.
This combines 20 cleanup commits from the dev branch.
All changed files:
- Clean up pointer defererences to sh.
- Remove shp arguments from functions.
Other notable changes:
src/cmd/ksh93/include/shell.h,
src/cmd/ksh93/sh/init.c:
- On second thought, get rid of the function version of
sh_getinterp() as libshell ABI compatibility is moot. We've
already been breaking that by reordering the sh struct, so there
is no way it's going to work without recompiling.
src/cmd/ksh93/sh/name.c:
- De-obfuscate the relationship between nv_scan() and scanfilter().
The former just calls the latter as a static function, there's no
need to do that via a function pointer and void* type conversions.
src/cmd/ksh93/bltins/typeset.c,
src/cmd/ksh93/sh/name.c,
src/cmd/ksh93/sh/nvdisc.c:
- 'struct adata' and 'struct tdata', defined as local struct types
in these files, need to have their first three fields in common,
the first being a pointer to sh. This is because scanfilter() in
name.c accesses these fields via a type conversion. So the sh
field needed to be removed in all three at the same time.
TODO: de-obfuscate: good practice definition via a header file.
src/cmd/ksh93/sh/path.c:
- Naming consistency: reserve the path_ function name prefix for
externs and rename statics with that prefix.
- The default path was sometimes referred to as the standard path.
To use one term, rename std_path to defpath and onstdpath() to
ondefpath().
- De-obfuscate SHOPT_PFSH conditional code by only calling
pf_execve() (was path_pfexecve()) if that is compiled in.
src/cmd/ksh93/include/streval.h,
src/cmd/ksh93/sh/streval.c:
- Rename extern strval() to arith_strval() for consistency.
src/cmd/ksh93/sh/string.c:
- Remove outdated/incorrect isxdigit() fallback; '#ifnded isxdigit'
is not a correct test as isxdigit() is specified as a function.
Plus, it's part of C89/C90 which we now require. (re: ac8991e5)
src/cmd/ksh93/sh/suid_exec.c:
- Replace an incorrect reference to shgd->current_pid with
getpid(); it cannot work as (contrary to its misleading directory
placement) suid_exec is an independent libast program with no
link to ksh or libshell at all. However, no one noticed because
this was in fallback code for ancient systems without
setreuid(2). Since that standard function was specified in POSIX
Issue 4 Version 2 from 1994, we should remove that fallback code
sometime as part of another obsolete code cleanup operation to
avoid further bit rot. (re: 843b546c)
src/cmd/ksh93/bltins/print.c: genformat():
- Remove preformat[] which was always empty and had no effect.
src/cmd/ksh93/shell.3:
- Minor copy-edit.
- Remove documentation for nonexistent sh.infile_name. A search
through ast-open-archive[*] reveals this never existed at all.
- Document sh.savexit (== $?).
src/cmd/ksh93/shell.3,
src/cmd/ksh93/include/shell.h,
src/cmd/ksh93/sh/init.c:
- Remove sh.gd/shgd; this is now unused and was never documented
or exposed in the shell.h public interface.
- sh_sigcheck() was documented in shell.3 as taking no arguments
whereas in the actual code it took a shp argument. I decided to
go with the documentation.
- That leaves sh_parse() as the only documented function that still
takes an shp argument. I'm just going to go ahead and remove it
for consistency, reverting sh_parse() to its pre-2003 spec.
- Remove undocumented/unused sh_bltin_tree() function which simply
returned sh.bltin_tree.
- Bump SH_VERSION to 20220106.
This takes another step towards cleaning up the build system. We
now do not even pretend to be theoretically compatible with
pre-1989 K&R C compilers or with C++ compilers. In practice, this
had already been broken for many years due to bit rot.
Commit 46593a89 already removed the license handling enormity that
depended on proto, so now we can cleanly remove it altogether. But
we do need to leave some backwards compatibility stubs to keep the
build system compatible with older AST code; it should remain
possible to build older ksh versions with the current build system
(the bin/ and src/cmd/INIT/ directories) for testing purposes.
So as of now there is no more __MANGLE__d rubbish in your generated
header files. This is only about a quarter of a century overdue...
This commit also includes a huge amount of code cleanup to remove
thousands of unused K&R C fallbacks and other cruft, particularly
in libast. This code base should now be a little easier to
understand for people who are familiar with a modern(ish) C
standard.
ratz is now also removed; this was a standalone and simplified 2005
version of gunzip. As of 6137b99a, none of our code uses it, even
theoretically. And the real g(un)zip is now everywhere.
src/cmd/INIT/proto.c, src/cmd/INIT/ratz.c:
- Removed.
COPYRIGHT:
- Remove zlib license; this only applied to ratz.
bin/package, src/cmd/INIT/package.sh:
- Related cleanups.
- Unset LC_ALL before invoking a new shell, respecting the user's
locale again and avoiding multibyte character corruption on the
command line.
src/cmd/INIT/proto.sh:
- Add stub for backwards compatibility with Mamfiles that depend on
proto. It does nothing but pass input without modification and is
now installed as the new arch/*/bin/proto by src/cmd/INIT/Mamfile.
src/cmd/INIT/iffe.sh:
- Ignore the proto-related -e (--package) and -p (--prototyped)
options; keep parsing them for backwards compatibility.
- Trim the macros passed to every test to their standard C
versions, removing K&R C and C++ versions. These are now
considered to be for backwards compatibility only.
src/cmd/INIT/iffe.tst:
- Remove proto(1) mangling code.
By the way, iffe can be regression-tested as follows:
$ bin/package use # set up environment in a child shell
$ regress src/cmd/INIT/iffe.tst
$ exit # leave package environment
src/cmd/INIT/make.probe, src/cmd/INIT/probe.win32:
- Remove code to handle C++.
src/lib/libast/features/common:
- As in iffe.sh above, trim macros designed for compatibility with
C++ and ancient C compilers to their standard C versions and
comment that they are for backwards compatibility with AST code.
This is needed to keep all the old ast and ksh code compiling.
src/cmd/ksh93/sh/init.c,
src/cmd/ksh93/sh/name.c:
- Clarify libshell ABI compatibility function versions of macros.
A "proto workaround" comment in the original code mislead me into
thinking this had something to do with the removed proto(1), but
it's unrelated. Call the workaround macro BYPASS_MACRO instead.
src/cmd/ksh93/include/defs.h:
- sh_sigcheck() macro: allow &sh as an argument: parenthesise shp.
src/cmd/ksh93/sh/nvtype.c:
- Remove unused nv_mkstruct() function. (re: d0a5cab1)
**/features/*:
- Remove obsolete iffe 'set prototyped' option.
**/Mamfile:
- Remove all references to the ast/prototyped.h header.
- Remove all use of the proto command. Simply copy instead.
*** 850-ish source files: ***
- Remove all '#pragma prototyped' directives.
- Remove all C++ compat code conditional upon defined(__cplusplus).
- Remove all use of the _ARG_ macro, which on standard C expands to
its argument:
#define _ARG_(x) x
(on K&R C, it expanded to nothing)
- Remove all use of _BEGIN_EXTERNS_ and _END_EXTERNS_ macros (empty
on standard C; this was for C++ compatibility)
- Reduce all #if __STD_C (standard code) #else (K&R code) #endif
blocks to the standard code only, without use of the macro.
- Same for _STD_ macro which seems to have had the same function.
- Change all instances of 'Void_t' to standard 'void'.
List of changes:
- Fixed some -Wuninitialized warnings and removed some unused variables.
- Removed the unused extern for B_login (re: d8eba9d1).
- The libcmd builtins and the vmalloc memfatal function now handle
memory errors with 'ERROR_SYSTEM|ERROR_PANIC' for consistency with how
ksh itself handles out of memory errors.
- Added usage of UNREACHABLE() where it was missing from error handling.
- Extend many variables from short to int to prevent overflows (most
variables involve file descriptors).
- Backported a ksh2020 patch to fix unused value Coverity issues
(https://github.com/att/ast/pull/740).
- Note in src/cmd/ksh93/README that ksh compiles with Cygwin on
Windows 10 and Windows 11, albeit with many test failures.
- Add comments to detail some sections of code. Extensive list of
commits related to this change:
ca2443b5, 7e7f1372, 2db9953a, 7003aba4, 6f50ff64, b1a41311,
222515bf, a0dcdeea, 0aa9e03f, 61437b27, 352e68da, 88e8fa67,
bc8b36fa, 6e515f1d, 017d088c, 035a4cb3, 588a1ff7, 6d63b57d,
a2f13c19, 794d1c86, ab98ec65, 1026006d
- Removed a lot of dead ifdef code.
- edit/emacs.c: Hide an assignment to avoid a -Wunused warning. (See
also https://github.com/att/ast/pull/753, which removed the assignment
because ksh2020 removed the !SHOPT_MULTIBYTE code.)
- sh/nvdisc.c: The sh_newof macro cannot return a null pointer because
it will instead cause the shell to exit if memory cannot be allocated.
That makes the if statement here a no-op, so remove it.
- sh/xec.c: Fixed one unused variable warning in sh_funscope().
- sh/xec.c: Remove a fallthrough comment added in commit ed478ab7
because the TFORK code doesn't fall through (GCC also produces no
-Wimplicit-fallthrough warning here).
- data/builtins.c: The cd and pwd man pages state that these builtins
default to -P if PATH_RESOLVE is 'physical', which isn't accurate:
$ /opt/ast/bin/getconf PATH_RESOLVE
physical
$ mkdir /tmp/dir; ln -s /tmp/dir /tmp/sym
$ cd /tmp/sym
$ pwd
/tmp/sym
$ cd -P /tmp/sym
$ pwd
/tmp/dir
The behavior described by these man pages isn't specified in the ksh
man page or by POSIX, so to avoid changing these builtin's behavior
the inaccurate PATH_RESOLVE information has been removed.
- Mamfiles: Preserve multi-line errors by quoting the $x variable.
This fix was backported from 93v-.
(See also <a7e9cc82>.)
- sh/subshell.c: Remove set but not used sp->errcontext variable.
BUG 1: Though 'command' is specified/documented as a regular
builtin, preceding assignments survive the invocation (as with
special or declaration builtins) if 'command' has no command
arguments in these cases:
$ foo=wrong1 command; echo $foo
wrong1
$ foo=wrong2 command -p; echo $foo
wrong2
$ foo=wrong3 command -x; echo $foo
wrong3
Analysis: sh_exec(), case TCOM (simple command), contains the
following loop that skips over 'command' prefixes, preparsing any
options and remembering the offset in the 'command' variable:
src/cmd/ksh93/sh/xec.c
1059 while(np==SYSCOMMAND || !np && com0
&& nv_search(com0,shp->fun_tree,0)==SYSCOMMAND)
1060 {
1061 register int n = b_command(0,com,&shp->bltindata);
1062 if(n==0)
1063 break;
1064 command += n;
1065 np = 0;
1066 if(!(com0= *(com+=n)))
1067 break;
1068 np = nv_bfsearch(com0, shp->bltin_tree, &nq, &cp);
1069 }
This skipping is not done if the preliminary b_command() call on
line 1061 (with argc==0) returns zero. This is currently the case
for command -v/-V, so that 'command' is treated as a plain and
regular builtin for those options.
The cause of the bug is that this skipping is even done if
'command' has no arguments. So something like 'foo=bar command' is
treated as simply 'foo=bar', which of course survives.
So the fix is for b_command() to return zero if there are no
arguments. Then b_command() itself needs changing to not error out
on the second/main b_command() call if there are no arguments.
src/cmd/ksh93/bltins/whence.c: b_command():
- When called with argc==0, return a zero offset not just for -v
(X_FLAG) or -V (V_FLAG), but also if there are no arguments left
(!*argv) after parsing options.
- When called with argc>0, do not issue a usage error if there are
no arguments, but instead return status 0 (or, if -v/-V was given,
status 2 which was the status of the previous usage message).
This way, 'command -v $emptyvar' now also works as you'd expect.
BUG 2: 'command -p' sometimes failed after executing certain loops.
src/cmd/ksh93/sh/path.c: defpath_init():
- astconf() returns a pointer to memory that may be overwritten
later, so duplicate the string returned. Backported from ksh2020.
(re: f485fe0f, aa4669ad, <https://github.com/att/ast/issues/959>)
src/cmd/ksh93/tests/builtins.sh:
- Update the test for BUG_CMDSPASGN to check every variant of
'command' (all options and none; invoking/querying all kinds of
command and none) with a preceding assignment. (re: fae8862c)
This also covers bug 2 as 'command -p' was failing on macOS prior
to the fix due to a loop executed earlier in another test.
Noteworthy changes:
- The man pages have been updated to fix a ton of instances of
runaway underlining (this was done with `sed -i 's/\\f5/\\f3/g'`
commands). This commit dramatically increased in size because
of this change.
- The documentation for spawnveg(3) has been extended with
information about its usage of posix_spawn(3) and vfork(2).
- The documentation for tmfmt(3) has been updated with the changes
previously made to the man pages for the printf and date builtins
(though the latter builtin is disabled by default).
- The shell's tracked alias tree (hash table) is now documented in
the shell(3) man page.
- Removed the commented out regression test for an ERRNO variable
as the COMPATIBILITY file states it was removed in ksh93.
There were still problems left after the previous commit. On at
least one system (QNX i386), the following regression test crashed:
src/cmd/ksh93/test/subshell.c
900 got=$( { "$SHELL" -c '(cd /; (cd /)); print -r -- "PWD=$PWD"'; } 2>&1 )
A backtrace done on the core dunp pointed to the free() call here:
src/cmd/ksh93/bltins/cd_pwd.c
90 if(oldpwd && oldpwd!=shp->pwd && oldpwd!=e_dot)
91 free(oldpwd);
Analysis: The interaction between $PWD, sh.pwd aka shp->pwd, and
the path_pwd() function is a mess. path_pwd() usually returns a
freeable value, but not always. sh.pwd is sometimes a pointer to
the value of $PWD, but not always (e.g. when you unset PWD or
assign to it). Instead of debugging the exact cause of the crash, I
think it is better to make this work in a more consistent way.
As of this commit:
1. sh.pwd keeps its own copy of the PWD, independently of the PWD
variable. The old value must always be freed immediately before
assigning a new one. This is simple and consistent, reducing the
chance of bugs at negligible cost.
2. The PWD variable is no longer given the NV_NOFREE attribute
because its value no longer points to sh.pwd. It is now a
variable like any other.
src/cmd/ksh93/sh/path.c: path_pwd():
- Do not give PWDNOD the NV_NOFREE attribute.
- Give sh.pwd its own copy of the PWD by strdup'ing PWDNOD's value.
src/cmd/ksh93/bltins/cd_pwd.c: b_cd():
- Since sh.pwd is now consistently freed before giving it a new
value and at no other time, oldpwd must not be freed any longer
and can become a regular non-static variable.
- If the PWD needs reinitialising, call path_pwd() to do it.
src/cmd/ksh93/sh/subshell.c: sh_subshell():
- Systems with fchdir(2): Always restore the PWD upon exiting a
non-subshare subshell. The check to decide whether or not to
restore it was unsafe: it was not restored if the current PWD
pointer and value was identical to the saved one, but a directory
can be deleted and recreated under the same name.
- Systems without fchdir(2) (if any exist):
. Entry: Fork if the PWD is nonexistent or has no x permission.
. Restore: Only chdir back if the subshell PWD was changed.
That's probably the best we can do. It remains inherently unsafe.
We should probably just require fchdir(2) at some point.
Path-bound builtins on ksh (such as /opt/ast/bin/cat) break some
basic assumptions about paths in the shell that should hold true,
e.g., that a path output by whence -p or command -v should actually
point to an executable command. This commit should fix the
following:
1. Path-bound built-ins (such as /opt/ast/bin/cat) can now be
executed by invoking the canonical path (independently of the
value of $PATH), so the following will now work as expected:
$ /opt/ast/bin/cat --version
version cat (AT&T Research) 2012-05-31
$ (PATH=/opt/ast/bin:$PATH; "$(whence -p cat)" --version)
version cat (AT&T Research) 2012-05-31
In the event an external command by that path exists, the
path-bound builtin will now override it when invoked using the
canonical path. To invoke a possible external command at that
path, you can still use a non-canonical path, e.g.:
/opt//ast/bin/cat or /opt/ast/./bin/cat
2. Path-bound built-ins will now also be found on a PATH set
locally using an assignment preceding the command, so something
like the following will now work as expected:
$ PATH=/opt/ast/bin cat --version
version cat (AT&T Research) 2012-05-31
The builtin is not found by sh_exec() because the search for
builtins happens long before invocation-local preceding
assignments are processsed. This only happens in sh_ntfork(),
before forking, or in sh_fork(), after forking. Both sh_ntfork()
and sh_fork() call path_spawn() to do the actual path search, so
a check there will cover both cases.
This does mean the builtin will be run in the forked child if
sh_fork() is used (which is the case on interactive shells with
job.jobcontrol set, or always after compiling with SHOPT_SPAWN
disabled). Searching for it before forking would mean
fundamentally redesigning that function to be basically like
sh_ntfork(), so this is hard to avoid.
src/cmd/ksh93/sh/path.c: path_spawn():
- Before doing anything else, check if the passed path appears in
the builtins tree as a pathbound builtin. If so, run it. Since a
builtin will only be found if a preceding PATH assignment
temporarily changed the PATH, and that assignment is currently in
effect, we can just sh_run() the builtin so a nested sh_exec()
invocation will find and run it.
- If 'spawn' is not set (i.e. we must return), set errno to 0 and
return -2. See the change to sh_ntfork() below.
src/cmd/ksh93/sh/xec.c:
- sh_exec(): When searching for built-ins and the restricted option
isn't active, also search bltin_tree for names beginning with a
slash.
- sh_ntfork(): Only throw an error if the PID value returned is
exactly -1. This allows path_spawn() to return -2 after running a
built-in to tell sh_ntfork() to do the right things to restore
state.
src/cmd/ksh93/sh/parse.c: simple():
- When searching for built-ins at parse time, only exclude names
containing a slash if the restricted option is active. This
allows finding pointers to built-ins invoked by literal path like
/opt/ast/bin/cat, as long as that does not result from an
expansion. This is not actually necessary as sh_exec() will also
cover this case, but it is an optimisation.
src/lib/libcmd/getconf.c:
- Replace convoluted deferral to external command by a simple
invocation of the path to the native getconf command determined
at compile time (by src/lib/libast/comp/conf.sh). Based on:
https://github.com/ksh93/ksh/issues/138#issuecomment-816384871
If there is ever a system that has /opt/ast/bin/getconf as its
default native external 'getconf', then there would still be an
infinite recursion crash, but this seems extremely unlikely.
Resolves: https://github.com/ksh93/ksh/issues/138
Previous discussion: https://github.com/att/ast/issues/485
If ksh attempts to execute a non-executable command found in the
PATH, in some instances the error message and return status are
incorrect. In the example below, ksh returns with exit status 126
when using the -c execve(2) optimization or when using fork(2) in
an interactive shell. However, using posix_spawn(3) causes the exit
status to change:
$ echo 'print cannot execute' > /tmp/x
# Runs command with spawnveg (i.e., posix_spawn or vfork)
$ ksh -c 'PATH=/tmp; x; echo $?'
ksh: x: not found
127
# Runs command with execve
$ ksh -c 'PATH=/tmp; x'; echo $?
ksh: x: cannot execute [Permission denied]
126
# Runs command with fork
$ ksh -ic 'PATH=/tmp; x; echo $?'
ksh: x: cannot execute [Permission denied]
126
Since 'x' is in the PATH but can't be executed, the correct exit
status is 126, not 127. It's worth noting this bug doesn't cause
the regression tests to fail with ksh93u+m, but it does cause one
test to fail when run under dtksh:
path.sh[706]: Long nonexistent command name: got status 126, ''
This commit backports various fixes for this bug from ksh2020, with
additional fixes applied (since there were still some additional
issues the ksh2020 patch didn't fix). The lacking regression test
for exit status 126 in path.sh has been rewritten to test for more
scenarios where ksh failed to return the correct error message
and/or exit status. I can also confirm with this patch applied the
path.sh regression tests now pass when run under dtksh.
src/cmd/ksh93/sh/path.c:
- Add a comment to path_absolute() describing 'oldpp' is the
current pointer in the while loop and 'pp' is the next pointer.
Backported from:
a6cad450
- The patch from ksh2020 didn't fix this bug in the SHOPT_SPAWN
code (because ksh2020 prefers fork(2)), so issues with the exit
status could still occur when using spawnveg. To fix this, always
set 'noexec' to the value of errno if can_execute fails. Before
this fix, errno was discarded if 'pp' was a null pointer and
can_execute failed.
- If a command couldn't be executed and the error wasn't ENOENT,
save errno in a 'not_executable' variable. If an executable
command couldn't be found in the PATH, exit with status 126 and
set errno to the saved value. This was based on a ksh2020 bugfix,
but it has been reworked a little bit to fix a bug that caused a
mismatch between the error message shown and errno. Example with
a non-executable file in PATH:
$ nonexec
ksh2020: nonexec: cannot execute [No such file or directory]
The ksh2020 patch: <https://github.com/att/ast/pull/493>
- Backport a ksh2020 bugfix for directories in the PATH when
running one of the added regression tests on OpenBSD:
https://github.com/att/ast/pull/767
src/cmd/ksh93/data/msg.c,
src/cmd/ksh93/include/shell.h,
src/cmd/ksh93/sh/{path,xec}.c:
- If a command name is too long (ENAMETOOLONG), then it wasn't
found in the PATH. For that case return exit status 127, like
for ENOENT.
src/cmd/ksh93/tests/path.sh:
- Replace the old test with a new set of more extensive tests.
These tests check the error message and exit status when ksh
attempts to run a command using any of the following:
- execve(2), used with the last command run with -c (*A tests).
- posix_spawn(3)/vfork(2), used in noninteractive scripts (*B tests).
- fork(2), used in interactive shells with job control (*C tests).
- command -x (*D tests).
- exec(1) (*E tests).
- Add a regression test from ksh2020 for attempting to execute a
directory:
https://github.com/att/ast/pull/758
src/lib/libast/include/ast.h,
src/lib/libast/include/wait.h:
- Avoid bitshifts in macros for static error codes. The return
values of command not found and exec related errors are static
values and should not require any macro magic for calculation.
Backported from: c073b102
- Simplify EXIT_* and W* macros to use 8 bits.
Many of these changes are minor typo fixes. The other changes
(which are mostly compiler warning fixes) are:
NEWS:
- The --globcasedetect shell option works on older Linux kernels
when used with FAT32/VFAT file systems, so remove the note about
it only working with 5.2+ kernels.
src/cmd/ksh93/COMPATIBILITY:
- Update the documentation on function scoping with an addition
from ksh93v- (this does apply to ksh93u+).
src/cmd/ksh93/edit/emacs.c:
- Check for '_AST_ksh_release', not 'AST_ksh_release'.
src/cmd/INIT/mamake.c,
src/cmd/INIT/ratz.c,
src/cmd/INIT/release.c,
src/cmd/builtin/pty.c:
- Add more uses of UNREACHABLE() and noreturn, this time for the
build system and pty.
src/cmd/builtin/pty.c,
src/cmd/builtin/array.c,
src/cmd/ksh93/sh/name.c,
src/cmd/ksh93/sh/nvtype.c,
src/cmd/ksh93/sh/suid_exec.c:
- Fix six -Wunused-variable warnings (the name.c nv_arrayptr()
fixes are also in ksh93v-).
- Remove the unused 'tableval' function to fix a -Wunused-function
warning.
src/cmd/ksh93/sh/lex.c:
- Remove unused 'SHOPT_DOS' code, which isn't enabled anywhere.
https://github.com/att/ast/issues/272#issuecomment-354363112
src/cmd/ksh93/bltins/misc.c,
src/cmd/ksh93/bltins/trap.c,
src/cmd/ksh93/bltins/typeset.c:
- Add dictionary generator function declarations for former
aliases that are now builtins (re: 1fbbeaa1, ef1621c1, 3ba4900e).
- For consistency with the rest of the codebase, use '(void)'
instead of '()' for print_cpu_times.
src/cmd/ksh93/sh/init.c,
src/lib/libast/path/pathshell.c:
- Move the otherwise unused EXE macro to pathshell() and only
search for 'sh.exe' on Windows.
src/cmd/ksh93/sh/xec.c,
src/lib/libast/include/ast.h:
- Add an empty definition for inline when compiling with C89.
This allows the timeval_to_double() function to be inlined.
src/cmd/ksh93/include/shlex.h:
- Remove the unused 'PIPESYM2' macro.
src/cmd/ksh93/tests/pty.sh:
- Add '# err_exit #' to count the regression test added in
commit 113a9392.
src/lib/libast/disc/sfdcdio.c:
- Move diordwr, dioread, diowrite and dioexcept behind
'#ifdef F_DIOINFO' to fix one -Wunused-variable warning and
multiple -Wunused-function warnings (sfdcdio() only uses these
functions when F_DIOINFO is defined).
src/lib/libast/string/fmtdev.c:
- Fix two -Wimplicit-function-declaration warnings on Linux by
including sys/sysmacros.h in fmtdev().
If a system administrator prefixes /opt/ast/bin to the path and
then invokes the shell in restricted mode, they clearly intend for
the user to run those AST utilities.
Similarly, if a system administrator sets a PATH for a restricted
shell that includes libraries listed in the .paths file, they must
have intended for the user to use those loadable built-ins, as they
will be associated with the pathnames of their respective
libraries. Since the user cannot change PATH or use the builtin
command, they still cannot load just any built-in they choose.
src/cmd/ksh93/sh/path.c:
- Remove SH_RESTRICTED check when handling path-bound builtins
or dynamic libaries containining builtins in $PATH.
src/cmd/ksh93/tests/builtins.sh:
- Add test verifying a restricted user can use /opt/ast/bin/cat
via a PATH search.
Progresses: https://github.com/ksh93/ksh/issues/138
This commit adds an UNREACHABLE() macro that expands to either the
__builtin_unreachable() compiler builtin (for release builds) or
abort(3) (for development builds). This is used to mark code paths
that are never to be reached.
It also adds the 'noreturn' attribute to functions that never
return: path_exec(), sh_done() and sh_syntax(). The UNREACHABLE()
macro is not added after calling these.
The purpose of these is:
* to slightly improve GCC/Clang compiler optimizations;
* to fix a few compiler warnings;
* to add code clarity.
Changes of note:
src/cmd/ksh93/sh/io.c: outexcept():
- Avoid using __builtin_unreachable() here since errormsg can
return despite using ERROR_system(1), as shp->jmplist->mode is
temporarily set to 0. See: https://github.com/att/ast/issues/1336
src/cmd/ksh93/tests/io.sh:
- Add a regression test for the ksh2020 bug referenced above.
src/lib/libast/features/common:
- Detect the existence of either the C11 stdnoreturn.h header or
the GCC noreturn attribute, preferring the former when available.
- Test for the existence of __builtin_unreachable(). Use it for
release builds. On development builds, use abort() instead, which
crahses reliably for debugging when unreachable code is reached.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
This bug was first reported at <https://github.com/att/ast/issues/8>.
The 'cd' command currently takes the value of $OLDPWD from the
wrong scope. In the following example 'cd -' will change the
directory to /bin instead of /tmp:
$ OLDPWD=/bin ksh93 -c 'OLDPWD=/tmp cd -'
/bin
src/cmd/ksh93/bltins/cd_pwd.c:
- Use sh_scoped() to obtain the correct value of $OLDPWD.
- Fix a use-after-free bug. Make the 'oldpwd' variable a static
char that points to freeable memory. Each time cd is used, this
variable is freed if it points to a freeable memory address and
isn't also a pointer to shp->pwd.
src/cmd/ksh93/sh/path.c: path_pwd():
- Simplify and add comments.
- Scope $PWD properly.
src/cmd/ksh93/tests/builtins.sh,
src/cmd/ksh93/tests/leaks.sh:
- Backport the ksh2020 regression tests for 'cd -' when $OLDPWD is
set.
- Add test for $OLDPWD and $PWD after subshare.
- Add test for $PWD after 'cd'.
- Add test for possible memory leak.
- Add testing for 'unset' on OLDPWD and PWD.
src/cmd/ksh93/COMPATIBILITY:
- Add compatibility note about changes to $PWD and $OLDPWD.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
This commit adds '/* FALLTHROUGH */' comments to fix many
GCC warnings when compiling with -Wimplicit-fallthrough.
Additionally, the existing fallthrough comments have been
changed for consistency.
These are minor fixes I've accumulated over time. The following
changes are somewhat notable:
- Added a missing entry for 'typeset -s' to the man page.
- Add strftime(3) to the 'see also' section. This and the date(1)
addition are meant to add onto the documentation for 'printf %T'.
- Removed the man page the entry for ksh reading $PWD/.profile on
login. That feature was removed in commit aa7713c2.
- Added date(1) to the 'see also' section of the man page.
- Note that the 'hash' command can be used instead of 'alias -t' to
workaround one of the caveats listed in the man page.
- Use an 'out of memory' error message rather than 'out of space'
when memory allocation fails.
- Replaced backticks with quotes in some places for consistency.
- Added missing documentation for the %P date format.
- Added missing documentation for the printf %Q and %p formats
(backported from ksh2020: https://github.com/att/ast/pull/1032).
- The comments that show each builtin's options have been updated.
In 2021, it seems like it's about time to join the 21st century
and officially require fork(2). In practice this was already the
case as the legacy code was unmaintained and didn't compile.
This removes #ifdefs checking for the existence of
SH_PLUGIN_VERSION (version check for dynamically loaded builtins)
and the SFIO identifiers SF_BUFCONST, SF_CLOSING, SF_APPENDWR,
SF_ATEXIT, all of which are defined by the bundled libast.
The referenced commit neglected to add checks for strdup() calls.
That calls malloc() as well, and is used a lot.
This commit switches to another strategy: it adds wrapper functions
for all the allocation macros that check if the allocation
succeeded, so those checks don't need to be done manually.
src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/init.c:
- Add sh_malloc(), sh_realloc(), sh_calloc(), sh_strdup(),
sh_memdup() wrapper functions with success checks. Call nospace()
to error out if allocation fails.
- Update new_of() macro to use sh_malloc().
- Define new sh_newof() macro to replace newof(); it uses
sh_realloc().
All other changed files:
- Replace the relevant calls with the wrappers.
- Remove now-redundant success checks from 18529b88.
- The ERROR_PANIC error message calls are updated to inclusive-or
ERROR_SYSTEM into the exit code argument, so libast's error()
appends the human-readable version of errno in square brackets.
See src/lib/libast/man/error.3
src/cmd/ksh93/edit/history.c:
- Include "defs.h" to get access to the wrappers even if KSHELL is
not defined.
- Since we're here, fix a compile error that occurred with KSHELL
undefined by updating the type definition of hist_fname[] to
match that of history.h.
src/cmd/ksh93/bltins/enum.c:
- To get access to sh_newof(), include "defs.h" instead of
<shell.h> (note that "defs.h" includes <shell.h> itself).
src/cmd/ksh93/Mamfile:
- enum.c: depend on defs.h instead of shell.h.
- enum.o: add an -I. flag in the compiler invocation so that defs.h
can find its subsequent includes.
src/cmd/builtin/pty.c:
- Define one outofmemory() function and call that instead of
repeating the error message call.
- outofmemory() never returns, so remove superfluous exit handling.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
Huge typeset -L/-R adjustment length values were still causing
crashses on sytems with not enough memory. They should error out
gracefully instead of crashing.
This commit adds out of memory checks to all malloc/calloc/realloc
calls that didn't have them (which is all but two or three).
The stkalloc/stakalloc calls don't need the checks; it has
automatic checking, which is done by passing a pointer to the
outofspace() function to the stakinstall() call in init.c.
src/lib/libast/include/error.h:
- Change the ERROR_PANIC exit status value from ERROR_LEVEL (255)
to 77, which is what it is supposed to be according to the libast
error.3 manual page. Exit statuses > 128 for anything else than
signals are not POSIX compliant and may cause misbehaviour.
src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/init.c:
- To facilitate consistency, add a simple extern sh_outofmemory()
function that throws an ERROR_PANIC "out of memory".
src/cmd/ksh93/include/shell.h,
src/cmd/ksh93/data/builtins.c:
- Remove now-redundant e_nospace[] extern message; it is now only
used in one place so it might as well be a string literal in
sh_outofmemory().
All other changed files:
- Verify the result of all malloc/calloc/realloc calls and call
sh_outofmemory() if they fail.
This fixes the following:
1. 'set --posix' now works as an equivalent of 'set -o posix'.
2. The posix option turns off braceexpand and turns on letoctal.
Any attempt to override that in a single command such as 'set -o
posix +o letoctal' was quietly ignored. This now works as long
as the overriding option follows the posix option in the command.
3. The --default option to 'set' now stops the 'posix' option, if
set or unset in the same 'set' command, from changing other
options. This allows the command output by 'set +o' to correctly
restore the current options.
src/cmd/ksh93/data/builtins.c:
- To make 'set --posix' work, we must explicitly list it in
sh_set[] as a supported option so that AST optget(3) recognises
it and won't override it with its own default --posix option,
which converts the optget(3) string to at POSIX getopt(3) string.
This means it will appear as a separate entry in --man output,
whether we want it to or not. So we might as well use it as an
example to document how --optionname == -o optionname, replacing
the original documentation that was part of the '-o' description.
src/cmd/ksh93/sh/args.c: sh_argopts():
- Add handling for explitit --posix option in data/builtins.c.
- Move SH_POSIX syncing SH_BRACEEXPAND and SH_LETOCTAL from
sh_applyopts() into the option parsing loop here. This fixes
the bug that letoctal was ignored in 'set -o posix +o letoctal'.
- Remember if --default was used in a flag, and do not sync options
with SH_POSIX if the flag is set. This makes 'set +o' work.
src/cmd/ksh93/include/argnod.h,
src/cmd/ksh93/data/msg.c,
src/cmd/ksh93/sh/args.c: sh_printopts():
- Do not potentially translate the 'on' and 'off' labels in 'set
-o' output. No other shell does, and some scripts parse these.
src/cmd/ksh93/sh/init.c: sh_init():
- Turn on SH_LETOCTAL early along with SH_POSIX if the shell was
invoked as sh; this makes 'sh -o' and 'sh +o' show expected
options (not that anyone does this, but correctness is good).
src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/include/shell.h:
- The state flags were in defs.h and most (but not all) of the
shell options were in shell.h. Gather all the shell state and
option flag definitions into one place in shell.h for clarity.
- Remove unused SH_NOPROFILE and SH_XARGS option flags.
src/cmd/ksh93/tests/options.sh:
- Add tests for these bugs.
src/lib/libast/misc/optget.c: styles[]:
- Edit default optget(3) option self-documentation for clarity.
Several changed files:
- Some SHOPT_PFSH fixes to avoid compiling dead code.
src/cmd/ksh93/features/externs: ARG_EXTRA_BYTES detection:
- Improve detection of extra bytes per argument: on every loop
iteration, recalculate the size of the environment while taking
the amount extra bytes we're currently trying into account. Also
count arguments (argv[]) as they are stored in the same buffer.
On 64-bit Linux with glibc, this now detects 9 extra bytes per
argument instead of 8. An odd number (literally and figuratively)
but apparently it needs it; I do think my method is correct now.
On 64-bit Solaris and macOS, this still detects 8 extra bytes.
(On 64-bit Linux with musl C library, it detects 0 bytes. Nice.)
src/cmd/ksh93/sh/path.c: path_xargs():
- Remove the kludge subtracting twice the size of the environment.
With the feature test fixed, this should no longer fail on Linux.
- Take into account the size of the final null element in the
argument and environment lists.
src/cmd/ksh93/tests/path.sh:
- Do not use awk for the test due to breakage in the system awks
on Solaris/Illumos (hangs) and AIX & UnixWare (drops arguments).
Instead, use (wait for it...) ksh. It's a bit slower, but works.
It was trivial to crash ksh by making an autoloaded function
definition file autoload itself, causing a stack overflow due to
infinite recursion. This commit adds loop detection that stops a
function that is being autoloaded from autoloading itself either
directly or indirectly, without removing the ability of autoloaded
function definition files to autoload other functions.
src/cmd/ksh93/sh/path.c: funload():
- Detect loops by checking if the path of a function to be
autoloaded was already added to a new internal static tree,
and if not, adding it while the function is being loaded.
src/cmd/ksh93/tests/path.sh:
- Add regression test.
- Tweak a couple of others to be freeze- and crash-proof.
NEWS:
- Add this fix + a forgotten entry for the previous fix (6f3b23e6).
Fixes: https://github.com/ksh93/ksh/issues/136
Reproducer from @Saikiran-m:
| ~# sh -c `perl -e 'print "a"x100000'`
| genunix: NOTICE: core_log: sh[1221] core dumped: /var/cores/core.sh.0.1602153496
| Memory fault(coredump)
The crash was in trying to decide whether the name was suitable for
autoloading as a function on $FPATH. This calls strmatch() to check
the name against a regex for valid function name. But the libast
regex code is not designed optimally and uses too much recursion,
limiting the length of the strings it's able to cope with.
src/cmd/ksh93/sh/path.c: path_search():
- Before calling strmatch(), check that the name is shorter than
256 bytes. The maximum length of file names on Linux and macOS is
255 bytes, so an autoload function can't have a name longer than
that anyway.
src/cmd/ksh93/tests/path.sh:
- Add test for this bug.
- Tweak 'command -x' test to not leave a hanging process on Ctrl+C.
Fixes: https://github.com/ksh93/ksh/issues/144
This time it was failing on a 64-bit Debian Linux system with very
few and short environment variables. Sigh.
src/cmd/ksh93/sh/path.c:
- Combine the strategy from 63979488 with that of 8f5235a5.
That fix turned out to be insufficient as NixOS has huge
environment variable lists because (due to each software package
being installed in its own directory tree) it has to keep dozens
of directories in variables like XDG_CONFIG_DIRS and others.
The 'command -x' regression test was failing on NixOS.
src/cmd/ksh93/sh/path.c:
- Different strategy. Leave twice the size of the existing
environment free.
I got one intermittent regression test failure due to 'argument
list too long' on a Debian x86_64 system.
src/cmd/ksh93/sh/path.c: path_xargs():
- Leave extra argument space for systems that need extra bytes:
1KiB per extra byte, with a minimum of 2KiB (the old value).
Turns out the assumption I was operating on, that Linux and macOS
align arguments on 32 or 64 bit boundaries, is incorrect -- they
just need some extra bytes per argument. So we can use a bit more
of the arguments buffer on these systems than I thought.
src/cmd/ksh93/features/externs:
- Change the feature test to simply detect the # of extra bytes per
argument needed. On *BSD and commercial Unices, ARG_EXTRA_BYTES
shows as zero; on Linux and macOS (64-bit), this yields 8. On
Linux (32-bit), this yields 4.
src/cmd/ksh93/sh/path.c: path_xargs():
- Do not try to calculate alignment, just add ARG_EXTRA_BYTES to
each argument.
- Also add this when substracting the length of environment
variables and leading and trailing static command arguments.
src/cmd/ksh93/tests/path.sh:
- Test command -v/-V with -x.
- Add a robust regression test for command -x.
src/cmd/ksh93/data/builtins.c, src/cmd/ksh93/sh.1:
- Tweak docs. Glob patterns also expand to multiple words.
This commit fixes 'command -x' to adapt to OS limitations with
regards to data alignment in the arguments list. A feature test is
added that detects if the OS aligns the argument on 32-bit or
64-bit boundaries or not at all, allowing 'command -x' to avoid
E2BIG errors while maximising efficiency.
Also, as of now, 'command -x' is a way to bypass built-ins and
run/query an external command. Built-ins do not limit the length of
their argument list, so '-x' never made sense to use for them. And
because '-x' hangs on Linux and macOS on every ksh93 release
version to date (see acf84e96), few use it, so there is little
reason not to make this change.
Finally, this fixes a longstanding bug that caused the minimum exit
status of 'command -x' to be 1 if a command with many arguments was
divided into several command invocations. This is done by replacing
broken flaggery with a new SH_XARG state flag bit.
src/cmd/ksh93/features/externs:
- Add new C feature test detecting byte alignment in args list.
The test writes a #define ARG_ALIGN_BYTES with the amount of
bytes the OS aligns arguments to, or zero for no alignment.
src/cmd/ksh93/include/defs.h:
- Add new SH_XARG state bit indicating 'command -x' is active.
src/cmd/ksh93/sh/path.c: path_xargs():
- Leave extra 2k in the args buffer instead of 1k, just to be sure;
some commands add large environment variables these days.
- Fix a bug in subtracting the length of existing arguments and
environment variables. 'size -= strlen(cp)-1;' subtracts one less
than the size of cp, which makes no sense; what is necessary is
to substract the length plus one to account for the terminating
zero byte, i.e.: 'size -= strlen(cp)+1'.
- Use the ARG_ALIGN_BYTES feature test result to match the OS's
data alignment requirements.
- path_spawn(): E2BIG: Change to checking SH_XARG state bit.
src/cmd/ksh93/bltins/whence.c: b_command():
- Allow combining -x with -p, -v and -V with the expected results
by setting P_FLAG to act like 'whence -p'. E.g., as of now,
command -xv printf
is equivalent to
whence -p printf
but note that 'whence' has no equivalent of 'command -pvx printf'
which searches $(getconf PATH) for a command.
- When -x will run a command, now set the new SH_XARG state flag.
src/cmd/ksh93/sh/xec.c: sh_exec():
- Change to using the new SH_XARG state bit.
- Skip the check for built-ins if SH_XARG is active, so that
'command -x' now always runs an external command.
src/lib/libcmd/date.c, src/lib/libcmd/uname.c:
- These path-bound builtins sometimes need to run the external
system command by the same name, but they did that by hardcoding
an unportable direct path. Now that 'command -x' runs an external
command, change this to using 'command -px' to guarantee using
the known-good external system utility in the default PATH.
- In date.c, fix the format string passed to 'command -px date'
when setting the date; it was only compatible with BSD systems.
Use the POSIX variant on non-BSD systems.
The forking fix implemented in 102868f8 and 9d428f8f, which stops
the main shell's hash table from being cleared if PATH is changed
in a subshell, can cause a significant performance penalty for
certain scripts that do something like
( PATH=... command foo )
in a subshell, especially if done repeatedly. This is because the
hash table is cleared (and hence a subshell forks) even for
temporary PATH assignments preceding commands.
It also just plain doesn't work. For instance:
$ hash -r; (ls) >/dev/null; hash
ls=/bin/ls
Simply running an external command in a subshell caches the path in
the hash table that is shared with a main shell. To remedy this, we
would have to fork the subshell before forking any external
command. And that would be an unacceptable performance regression.
Virtual subshells do not need to fork when changing PATH if they
get their own hash tables. This commit adds these. The code for
alias subshell trees (which was removed in ec888867 because they
were broken and unneeded) provided the beginning of a template for
their implementation.
src/cmd/ksh93/sh/subshell.c:
- struct subshell: Add strack pointer to subshell hash table.
- Add sh_subtracktree(): return pointer to subshell hash table.
- sh_subfuntree(): Refactor a bit for legibility.
- sh_subshell(): Add code for cleaning up subshell hash table.
src/cmd/ksh93/sh/name.c:
- nv_putval(): Remove code to fork a subshell upon resetting PATH.
- nv_rehash(): When in a subshell, invalidate a hash table entry
for a subshell by creating the subshell scope if needed, then
giving that entry the NV_NOALIAS attribute to invalidate it.
src/cmd/ksh93/sh/path.c: path_search():
- To set a tracked alias/hash table entry, use sh_subtracktree()
and pass the HASH_NOSCOPE flag to nv_search() so that any new
entries are added to the current subshell table (if any) and do
not influence any parent scopes.
src/cmd/ksh93/bltins/typeset.c: b_alias():
- b_alias(): For hash table entries, use sh_subtracktree() instead
of forking a subshell. Keep forking for normal aliases.
- setall(): To set a tracked alias/hash table entry, pass the
HASH_NOSCOPE flag to nv_search() so that any new entries are
added to the current subshell table (if any) and do not influence
any parent scopes.
src/cmd/ksh93/sh/init.c: put_restricted():
- Update code for clearing the hash table (when changing $PATH) to
use sh_subtracktree().
src/cmd/ksh93/bltins/cd_pwd.c:
- When invalidating path name bindings to relative paths, use the
subshell hash tree if applicable by calling sh_subtracktree().
- rehash(): Call nv_rehash() instead of _nv_unset()ting the hash
table entry; this is needed to work correctly in subshells.
src/cmd/ksh93/tests/leaks.sh:
- Add leak tests for various PATH-related operations in the main
shell and in a virtual subshell.
- Several pre-existing memory leaks are exposed by the new tests
(I've confirmed these in 93u+). The tests are disabled and marked
TODO for now, as these bugs have not yet been fixed.
src/cmd/ksh93/tests/subshell.sh:
- Update.
Resolves: https://github.com/ksh93/ksh/issues/66
The '-o nolog' option (which prevented function definitions from being
recorded in the history file) was removed a long time ago, leaving
only a stub for backwards compatibility to stop 'set' from erroring
out if the option is set. But some other vestiges remained.
src/cmd/ksh93/sh/path.c,
src/cmd/ksh93/sh/subshell.c,
src/cmd/ksh93/sh/xec.c:
- Remove a few pointless 'sh_onstate(SH_NOLOG)' statements. As of
93u+ or earlier, this is never checked for anywhere.
src/cmd/ksh93/sh.1:
- They forgot to remove the 'nolog' option documentation here.
Specify that it's obsolete and has no effect.
src/cmd/ksh93/data/builtins.c: sh_set[]:
- Be more concise.
Autoloading a function caused the calling script's $LINENO to be
off by the number of lines in the function definition file. In
addition, while running autoloaded functions, errors/warnings were
reported with wrong line numbers.
src/cmd/ksh93/sh/path.c:
- Save $LINENO (shp->inlineno) before autoloading a function, reset
it to 1 so that the correct line number offset is remembered for
the function definition, and restore it after.
src/cmd/ksh93/tests/variables.sh:
- Add regression test for $LINENO, directly and in error messages,
within and outside a non-autoloaded and an autoloaded function.
Fixes: https://github.com/ksh93/ksh/issues/116
This applies the following Red Hat patch:
642af4d6/f/ksh-20120801-cdfork.patch
The associated bug report is public, but nearly all info (such as
a reproducer) has been wiped: https://bugzilla.redhat.com/1168611
However, the errata blurb is mildly informative:
"Previously, ksh sometimes incorrectly initialized a variable
holding the path of the working directory. If a program changed the
working directory between forking and ksh execution, then ksh could
contain an incorrect value in the working directory variable. With
this update, initialization of the working directory variable has
been corrected, and ksh now contains the correct value in the
aforementioned situation."
Also, the patch makes a lot of sense on the face of it. It removes
an optimisation in path_pwd() that checks for the directory defined
by e_crondir[] in data/msg.c, which is:
const char e_crondir[] = "/usr/spool/cron/atjobs";
Of /usr/spool not existed on any system for decades as it is common
to mount usr as read-only, so all the writable stuff was moved to
/var. So that would never check out. And if 'flag' is nonzero, the
optimizing 'count++' is executed regardless of whether that
directory exists, ensuring that it never gets the real PWD and
defaults to returning ".".
src/cmd/ksh93/sh/path.c:
- Apply patch as described.
- Mark 'flag' variable as NOT_USED to suppress compiler warning.
Keep it for backwards compat, as some programs that link with
libshell might use this function (though it's undocumented).
src/cmd/ksh93/include/path.h,
src/cmd/ksh93/data/msg.c:
- Remove now-unused e_crondir[].
Now that we have ${.sh.pid} a.k.a. shgd->current_pid, which is
updated using getpid() whenever forking a new process, there is no
need for anything else to ever call getpid(); we can use the stored
value instead. There were a lot of these syscalls kicking around,
some of them in performance-sensitive places.
The following lists only changes *other* than changing getpid() to
shgd->currentpid.
src/cmd/ksh93/include/defs.h:
- Comments: clarify what shgd->{pid,ppid,current_pid} are for.
src/cmd/ksh93/sh/main.c,
src/cmd/ksh93/sh/init.c:
- On reinit for a new script, update shgd->{pid,ppid,current_pid}
in the sh_reinit() function itself instead of calling sh_reinit()
from sh_main() and then updating those immediately after that
call. It just makes more sense this way. Nothing else ever calls
sh_reinit() so there are no side effects.
src/cmd/ksh93/sh/xec.c: _sh_fork():
- Update shgd->current_pid in the child early, so that the rest of
the function can use it instead of calling getpid() again.
- Remove reassignment of SH_PIDNOD->nvalue.lp value pointer to
shgd->current_pid (which makes ${.sh.pid} work in the shell).
It's constant and was already set on init.
This fixes two memory leaks in old-style command substitutions
(one when invoking an alias, one when invoking an autoloaded
function), as well as a possible third leak with an unknown
reproducer, by applying this Red Hat patch:
642af4d6/f/ksh-20120801-mlikfiks.patch
src/cmd/ksh93/sh/macro.c: comsubst():
- For as-yet unknown reasons, the alias leak did not occur when
adding a space at the end of the command substitution, as in
a=`some_alias `. This fix is a workaround that simply writes
an extra space to the stack. TODO: a real fix.
src/cmd/ksh93/sh/path.c: funload():
- Add missing free() before return. This fixes the leak with
autoloaded functions.
src/cmd/ksh93/sh/lex.c: alias_exceptf():
- This function is called "whenever an end of string is found with
alias". This adds a check for an SF_FINAL stream status flag when
deciding whether to call free(). In sfio.h this is commented as:
#define SF_FINAL 11 /* closing is done except stream free */
When I revert this change, none of the regression tests fail, so
I don't know how to trigger this supposed leak. But it makes some
sense given the sfio.h comment, so I'll keep it.
src/cmd/ksh93/tests/leaks.sh:
- Add the reproducers from rhbz#982142 as regression tests
(including an extra one for nested command substitutions that was
already fixed as of 93u+, but testing is good).
I replaced the external 'expr' and 'ls' commands by uses of
the 'true' builtin, otherwise the tests take far too long to run
with 16384 iterations. At least the alias leak was still behaving
identically after replacing 'ls' by 'true'.
Hopefully this doesn't introduce new bugs, but it does fix at
least the following:
1. When whence -v/-a found an "undefined" (i.e. autoloadable)
function in $FPATH, it actually loaded the function as a side
effect of reporting on its existence (!). Now it only reports.
2. 'whence' will now canonicalise paths properly. Examples:
$ whence ///usr/lib/../bin//./env
/usr/bin/env
$ (cd /; whence -v dev/../usr/bin//./env)
dev/../usr/bin//./env is /usr/bin/env
3. 'whence' no longer prefixes a spurious double slash when doing
something like 'cd / && whence bin/echo'. On Cygwin, an initial
double slash denotes a network server, so this was not just a
cosmetic problem.
4. 'whence -a' now reports a "tracked alias" (a.k.a. hash table
entry, i.e. cached $PATH search) even if an actual alias by the
same name exists. This needed fixing because in fact the hash
table entry continues to be used when bypassing the alias.
Aliases and "tracked aliases" are not remotely the same thing;
confusing nomenclature is not a reason to report wrong results.
5. When using 'hash' or 'alias -t' on a command that is also a
builtin to force caching a $PATH search for the external
command, 'whence -a' double-reported the path:
$ hash printf; whence -a printf
printf is a shell builtin
printf is /usr/bin/printf
printf is a tracked alias for /usr/bin/printf
This is now fixed so that the second output line is gone.
Plus, if there were multiple versions of the command on $PATH,
the tracked alias was reported at the end, which is the wrong
order. This is also fixed.
src/cmd/ksh93/bltins/whence.c: whence():
- Refactor the do...while loop that handles whence -v/-a for path
searches in such a way that the code actually makes sense and
stops looking like higher esotericism. Just doing this fixed#2,
#4 and #5 above (the latter two before I even noticed them). For
instance, the path_fullname() call to canonicalise paths was
already there; it was just never used.
- Remove broken 'notrack' flaggery for deciding whether to report a
hash table entry a.k.a. "tracked alias"; instead, check the hash
table (shp->track_tree).
src/cmd/ksh93/sh/path.c:
- path_search(): Re #3: When prefixing the PWD, first check if
we're in '/' and if so, don't prefix it; otherwise, adding the
next slash causes an initial double slash. (Since '/' is the only
valid single-character absolute path, all we need to do is check
if the second character pwd[1] is non-null.)
- path_search(): Re #1: Stop autoloading when called by 'whence':
* The 'flag==2' check to avoid autoloading a function was
broken. The flag value is 2 on the first whence() loop
iteration, but 3 on subsequent ones. Change to 'flag >= 2'.
* However, this only fixes it if the function file does not have
the x permission bit, as executable files are handled by
path_absolute() which unconditionally autoloads functions!
So, pass on our flag parameter when callling path_absolute().
- path_absolute(): Re #1: Add flag parameter. Do not autoload
functions if flag >= 2.
src/cmd/ksh93/include/path.h,
src/cmd/ksh93/bltins/typeset.c,
src/cmd/ksh93/sh/main.c,
src/cmd/ksh93/sh/xec.c:
- Re #1: Update path_absolute() calls, adding a 0 flag parameter.
src/cmd/ksh93/include/name.h:
- Remove now-unused pathcomp member from union Value. It was
introduced in 99065353 to allow examining the value of a tracked
alias. This commit uses nv_getval() instead.
src/cmd/ksh93/tests/builtins.sh,
src/cmd/ksh93/tests/path.sh:
- Add and tweak various related tests.
Fixes: https://github.com/ksh93/ksh/issues/84
When compiling ksh with '-O0 -g -D_std_malloc' on my Mac, the
paths.sh regress test set crashed. This is the test that crashed:
print 'FPATH=../fun' > bin/.paths
cat <<- \EOF > fun/myfun
function myfun
{
print myfun
}
EOF
x=$(FPATH= PATH=$PWD/bin $SHELL -c ': $(whence less);myfun') 2> /dev/null
[[ $x == myfun ]] || err_exit 'function myfun not found'
The crash occurred on the second-to-last line. The backtrace
suggests an invalid use of strcpy() with overlapping memory:
0 libsystem_kernel.dylib __pthread_kill + 10
1 libsystem_pthread.dylib pthread_kill + 284
2 libsystem_c.dylib abort + 127
3 libsystem_c.dylib abort_report_np + 177
4 libsystem_c.dylib __chk_fail + 48
5 libsystem_c.dylib __chk_fail_overlap + 16
6 libsystem_c.dylib __chk_overlap + 34
7 libsystem_c.dylib __strcpy_chk + 64
8 ksh path_chkpaths + 1038 (path.c:1534)
9 ksh path_addcomp + 1032 (path.c:1481)
10 ksh path_addpath + 395 (path.c:1598)
11 ksh put_restricted + 626 (init.c:329)
[...]
src/cmd/ksh93/sh/path.c: path_chkpaths():
- When reading the '.paths' file, use memmove(3) instead of
strcpy(3) as the former does a non-destructive copy with
tolerance for overlap.
Since ksh 93u+m comes bundled with libast 20111111, there's no need
to support older versions, so this is another cleanup opportunity.
src/cmd/ksh93/include/defs.h:
- Throw an #error if AST_VERSION is undefined or < 20111111.
(Note that _AST_VERSION is the same as AST_VERSION, but the
latter is newer and preferred; see src/lib/libast/features/api)
All other changed files:
- Remove legacy code for versions older than the currently used
versions, which are:
_AST_VERSION 20111111
ERROR_VERSION 20100309
GLOB_VERSION 20060717
OPT_VERSION 20070319
SFIO_VERSION 20090915
VMALLOC_VERSION 20110808
This removes various blocks of uncommented experimental code that
was disabled using '#if 0' or '#if 1 ... #else' directives. It's
hard or impossible to figure out what the thoughts behind them
might have been, and we can really do without those distractions.
src/cmd/ksh93/sh/path.c: canexecute():
- Close file descriptors inside of the err label. This fixes
a file descriptor leak that occurs when open succeeds but
fstat fails with EIO. The previous code only returned -1
after 'goto err', leaving the opened file descriptor
inaccessible. This bugfix was backported from ksh2020:
55cad1d
If a command's path was previously added to the hash table as a
'tracked alias', then the hash table entry was used, bypassing
the default utility path search activated by 'command -p'.
'command -p' activates a SH_DEFPATH shell state. The bug was caused
by a failure to check for this state before using the hash table.
This check needs to be added in four places.
src/cmd/ksh93/sh/path.c,
src/cmd/ksh93/sh/xec.c:
- path_search(), path_spawn(), sh_exec(), sh_ntfork(): Only consult
the hash table, which is shp->track_tree, if the SH_DEFPATH shell
state is not active.
src/cmd/ksh93/tests/path.sh:
- Add regress tests checking that 'command -p' and 'command -p -v'
still search in the default path if a hash table entry exists for
the command searched.
'command -x' (basically builtin xargs for 'command') worked for
long argument lists on *BSD and HP-UX, but not on macOS and Linux,
where it reliably entered into an infinite loop.
The problem was that it assumed that every byte of the environment
space can be used for arguments, without accounting for alignment
that some OSs do. MacOS seems to be the most wasteful one: it
aligns on 16-byte boundaries and requires some extra bytes per
argument as well.
src/cmd/ksh93/sh/path.c:
- path_xargs(): When calculating how much space to subtract per
argument, add 16 extra bytes to the length of each argument, then
align the result on 16-byte boundaries. The extra 16 bytes is
more than even macOS needs, but hopefully it is future-proof.
- path_spawn(): If path_xargs() does fail, do not enter a retry
loop (which always becomes an infinite loop if the argument list
exceeds OS limitations), but abort with an error message.
As of aa4669ad, astconf("PATH") is implemented as a hardcoded AST
configuration variable that always has a value, instead of one that
falls back on the OS. Its value is now obtained from the OS (with a
fallback) at configure time and not at runtime. This means that any
fallback for astconf("PATH") is now never used.
src/cmd/ksh93/data/msg.c,
src/cmd/ksh93/include/shell.h:
- Remove e_defpath[]. (The path "/bin:/usr/bin:" made no sense as a
default path anyway, as the final empty element is wrong: default
utilities should never be sought in the current working dir.)
src/cmd/ksh93/sh/path.c,
src/lib/libast/path/pathbin.c:
- abort() if astconf("PATH") returns null.
src/lib/libast/comp/conf.tab: PATH:
- If no 'getconf' utility can be found, use a fallback path that
finds more utilities by also searching in 'sbin' directories.
On some systems, this is needed to find chown(1).
src/cmd/ksh93/sh.1:
- Update doc re default path.
The coshell(1) command, which is required for libcoshell to be
useful, is not known to be shipped by any distribution. It was
removed by the ksh-community fork and hence also by 93u+m (in
2940b3f5). The coshell facility as a whole is obsolete and
insecure. For a long time now, the statically linked libcoshell
library has been 40+ kilobytes of dead weight in the ksh binary.
Prior discussion (ksh2020): https://github.com/att/ast/issues/619
src/lib/libcoshell/*:
- Removed.
src/cmd/ksh93/*:
- Remove the SHOPT_COSHELL compiler option (which was enabled) and
a lot of code that was conditional upon #ifdef SHOPT_COSHELL.
- init.c: e_version[]: Removing SHOPT_COSHELL changed the "J"
feature identifier in ${.sh.version} to a lowercase "j", which
was conditional upon SHOPT_BGX (background job extensions).
But src/cmd/ksh93/RELEASE documents (at 08-12-04, on line 1188):
| +SHOPT_BGX enables background job extensions. Noted by "J" in
| the version string when enabled. [...]
That is the only available documentation. So change that "j" back
to a "J", leaving the version string unchanged after this commit.
- jobs.c: job_walk(): We need to keep one 'job_waitsafe(SIGCHLD);'
call that was conditional upon SHOPT_COSHELL; removing it caused
a regression test failure in tests/sigchld.sh, 'SIGCHLD blocked
for script at end of pipeline' (which means that until now, a ksh
compiled without libcoshell had broken SIGCHLD handling.)
bin/package, src/cmd/INIT/package.sh:
- Don't export COSHELL variable.
