EPL 1.0 says, in section 7: "The Program (including Contributions)
may always be distributed subject to the version of the Agreement
under which it was received. In addition, after a new version of
the Agreement is published, Contributor may elect to distribute the
Program (including its Contributions) under the new version."
The Eclipse Foundation also encourage everyone to upgrade:
https://www.eclipse.org/legal/epl-2.0/faq.php#h.60mjudroo8e5https://www.eclipse.org/legal/epl-2.0/faq.php#h.tci84nlsqpgw
Unfortunately the new Secondary License option is not available to
us as we're not the original copyright holders and don't have the
legal power to add one. So, no GPL compatibility. Sorry.
Attempting to tab-complete the only member of a directory while in
vi mode switches to "control" under the following conditions:
1. pwd is outside of the directory
2. No part of the basename has been typed in (e.g., starting with
vim TEST/t will autocomplete to vim TEST/test, but starting
with vim TEST/ will not).
Also, trying to type with "input" or "replace" fails afterwards
until a new line has been generated (either with ctrl + c or the
j/k keys in control mode).
src/cmd/ksh93/edit/vi.c: textmod():
- The strange completion behavior in vi mode for single-member
directories appears to be at least partially due to completion
failing to use \ or * mode from outside of pwd. Checking for
e_nlist directly from vi.c — 0 unless in = mode — allows for
entering the else loop at line 2575 to reach APPEND mode.
Resolves: https://github.com/ksh93/ksh/issues/485
Since we now have sh.current_ppid, we might as well point a shell
variable to it, as the cost is nil.
Together, ${.sh.pid} and ${.sh.ppid} (updated when a virtual
subshell forks) form a logical counterpart pair to $$ and $PPID
(never updated in subshells).
This commit also adds a section to the manual page that hopefully
does away with the depressingly widespread subshell/child shell
confusion once and for all... :P
The undocumented .sh.dollar variable was a hack used to change the
value of $$ (main shell's PID); any value assigned to .sh.dollar
overrides the value of $$. This was used by libcoshell and by the
pre-fork(2) code to update the value of $$ in child processes
initialised by internally generated shell scripts. Both were
removed long ago, so we don't need this.
The Sfio code uses its own _has_multibyte macro and that was not
being disabled by AST_NOMULTIBYTE, so some of its multibyte code
was still getting compiled in. This is easily fixed in sfhdr.h.
But that exposed another issue: the Sfio stdio wrappers didn't
know about the _has_multibyte macro. So this adds the necessary
directives to keep their multibyte functions from compiling.
For $PPID and ${.sh.pid}, ksh has simply been assuming that pid_t
is a 32-bit integer. In POSIX, the size of pid_t is not specified
and operating systems are free to vary it. On any system where
sizeof(pid_t) != 4, these variables produce undefined results.
(Note that this is not a problem with $$ as it is converted to a
string value directly from pid_t in special() in macro.c.)
src/cmd/ksh93/features/options:
- Add feature test that produces NV_PID, a name-value attribute
macro that is set to NV_INTEGER|NV_SHORT (for int16_t),
NV_INTEGER (for int32_t) or NV_INTEGER|NV_LONG (for Sflong_t
a.k.a. intmax_t), depending on the size of pid_t. These are the
three integer size attributes supported by nv_getval(). The build
will abort on any (hypothetical?) system where pid_t has a size
other than these three. In that case, the test and nv_getval()
will need to have a new integer size attribute added.
src/cmd/ksh93/data/variables.c:
- Use NV_PID instead of NV_INTEGER for $PPID and ${.sh.pid}. This
fixes the issue.
Other changed files:
- Fix the typecasts to various fmtbase() calls; the first argument
is of type intmax_t (a.k.a. Sflong_t), not long.
- When outputting PIDs with sfprintf(), typecast to Sflong_t and
use %lld to account for possible systems with very large PIDs.
(Though %lld is not in C89/C90, Sfio supports it anyway.)
I made a mistake in sh.reinit() which caused $PPID to be set to the
new process ID, not the parent process ID. This commit fixes it by
introducing, updating and using sh.current_ppid, so we continue to
minimise context switches due to getpid(2)/getppid(2) system calls.
Thanks to Geoff Clare for the report.
The arguments to the binary numerical comparison operators (-eq,
-gt, etc.) in the [[ and test/[ commands are treated as arithmetic
expressions, even if $((...)) is not used. But there is some
seriously incorrect behaviour:
Reproducers (all should output 0/true):
$ [[ 0x0A -eq 10 ]]; echo $?
1
$ [[ 1+0x0A -eq 11 ]]; echo $?
0
$ (set --posix; [[ 010 -eq 8 ]]); echo $?
1
$ (set --posix; [[ +010 -eq 8 ]]); echo $?
0
$ [[ 0xA -eq 10 ]]; echo $?
1
$ xA=10; [[ 0xA -eq 10 ]]; echo $?
0
$ xA=WTF; [[ 0xA -eq 10 ]]; echo $?
ksh: WTF: parameter not set
(POSIX mode enables the recognition of the initial 0 as a prefix
indicating an octal number in arithmetic expressions.)
The cause is the two 'while' loops in this section in test_binop()
in src/cmd/ksh93/bltins/test.c:
502: if(op&TEST_ARITH)
503: {
504: while(*left=='0')
505: left++;
506: while(*right=='0')
507: right++;
508: lnum = sh_arith(left);
509: rnum = sh_arith(right);
510: }
So initial zeros are unconditionally skipped. Ostensibly this is to
disable the recognition of the initial zero as an octal prefix as
well as 0x as a hexadecimal prefix. This would be okay for
enforcing a decimal-only limitation for simple numbers, but to do
this for arithmetic expressions is flagrantly incorrect, to say the
least. (insert standard rant about AT&T quality standards here)
The fix for '[[' is simply to delete the two 'while' loops. But
that creates a problem for the deprecated-but-standard 'test'/'['
command, which also uses the test_binop() function. According to
POSIX, test/[ only parses simple decimal numbers, so octal, etc. is
not a thing. But, after that fix, 'test 08 -eq 10' in POSIX mode
yields true, which is unlike every other shell. (Note that this is
unlike [[ 08 -eq 10 ]], which yields true on bash because '[['
parses operands as arithmetic expressions.)
For test/[ in non-POSIX mode, we don't need to change anything. For
POSIX mode, we should only parse literal decimal numbers for these
operators in test/[, disallowing unexpanded arithmetic expressions.
This makes ksh's POSIX-mode test/[ act like every other shell and
like external .../bin/test commands shipped by OSs.
src/cmd/ksh93/bltins/text.c: test_binop():
- Correct a type mismatch. The left and right hand numbers should
be Sfdouble_t, the maximum double length on the current system,
and the type that sh_arith() returns. Instead, long double
(typeset -lF) values were reduced to double (typeset -F) before
comparing!
- For test/[ in POSIX, only accept simple decimal numbers: use
strtold() instead of sh_arith(). Do skip initial zeros here as
this disables the recognition of the hexadecimal prefix. Throw an
error on an invalid decimal number. Floating point constants are
still parsed, but that's fine as this does not cause any
incompatibility with POSIX.
- For code legibility, move handling of TEST_EQ, etc. to within the
if(op&TEST_ARITH) block. This also allows lnum and rnum to be
local to that block.
We're nearly there!
I intend to release ksh 93u+m/1.0.0 on 2022-08-01, precisely ten
years after the last canonical 93u+ release.
We have a week until then, so here's a release candidate. Please
try as hard as you can to break it, and to help fix known bugs.
As of this commit, the 1.0 branch is feature-frozen and will only
get bugfixes.
src/cmd/ksh93/fun/man:
- Last-minute fix: .man.try_os_man(): do not look for arguments
with / in section 1 and 8; this can cause false positives.
Not only is it C++-specific (and we no longer pretend to be
compatible with C++), it's also z/OS-specific. It seems that
access to z/OS for testing is not available to mere mortals.
The Solaris 10.1 man command happily exits with status 0 on error
and even prints error messages to standard output. This was fixed
in later versions of Solaris. but we should still fix the function
for portability as it should work almost anywhere.
.man.try_os_man() now checks that the standard output of a man
command includes at least three newlines; that's probably a
reliable-enough indicator that it's a man page and not an error
message. Thankfully, all man commands do seem to deactivate the
pager when standard output is not on a terminal (e.g. when catching
standard output with a command substitution).
Somewhat notable changes:
- Remove pointless test commands from Mamfiles.
- Consistent use of NoN macro instead of manual void _STUB_foo(){}
(this is to silence "foo.o has no symbols" warnings from ld).
- Remove src/lib/libast/comp/transition.c; obsolete, does nothing.
- xec.c: Fix off-by-one error in sigreset() used by sh_ntfork():
it tried to (re)set signal 0, which is harmless, but wrong.
(note that sh.st.trapmax is the current max trapped sig + one!)
This autoloadable function activates a feature similar to 'shopt -s
autocd' in bash: type only a directory name to change directory.
It uses the DEBUG trap to work. See the file for details.
This adds a new 'man' function in src/cmd/ksh93/fun/man, meant for
autoload. This integrates the --man self-documentation of ksh
built-in and external AST commands with your system's 'man' command
so you can conveniently use 'man' for all commands, whether
built-in or external. See the file for details.
Since 'command -x' provides xargs-like functionality to repeatedly
run external commands if the argument list is too long for one
invocation, it makes little sense to use with built-ins. So I
decided that 'command -x' should double as a way to guarantee
running an external command. However, it was only bypassing plain
built-ins and not path-bound builtins (the ones that show up with a
virtual directory path name in the output of 'builtin').
src/cmd/ksh93/sh/path.c:
- Before looking for a path-bound built-in, check that the SH_XARG
state bit is not on. This needs to be done in path_absolute() as
well as in path_spawn().
Even tough sh_subtmpfile() should only be relevant to command
substitutions, checking the sh.comsub flag instead of sh.subshell
before calling is not valid in all cases; subshells of command
substitutions run into problems in some oddly specific cases, e.g.,
both 'eval' and an external command must be involved. The ksh
regression tests didn't detect a problem, but both modernish[*] and
shellspec[*] have one regression test failure after that change.
Minimal reproducer, assuming a cat at /bin/cat:
v=$(eval 'print output | /bin/cat')
print -r "v=[$v]"
Actual output:
output
v=[]
Expected output:
v=[output]
This commit makes three interrelated changes.
First, the code for erasing the command line before redrawing it
upon a window size change is simplified and modernised. Instead of
erasing the line with lots of spaces, it now uses the sequence
obtained from 'tput ed' (usually ESC, '[', 'J') to "erase to the
end of screen". This avoids messing up the detection and automatic
redrawing of wrapped lines on terminals such as Apple Terminal.app.
Second, the -b/--notify option is made more usable. When it is on
and either the vi or emacs/gmacs line editor is in use, 'Done' and
similar notifications are now buffered and trigger a command line
redraw as if the window size changed, and the redraw routine prints
that notify buffer between erasing and redrawing the commmnd line.
The effect is that the notification appears to magically insert
itself directly above the line you're typing. (The notification
behaviour when not in the shell line editor, e.g. while running
commands such as external editors, is unchanged.)
Third, a bug is fixed that caused -b/--notify to only report on one
job when more than one terminated at the same time. The rest was
reported on the next command line as if -b were not on. Reproducer:
$ set -b; sleep 1 & sleep 1 & sleep 1 &
This commit also includes a fair number of other window size and
$COLUMNS/$LINES handling tweaks that made all this easier, not all
of which are mentioned below.
src/cmd/ksh93/include/fault.h,
src/cmd/ksh93/sh/fault.c:
- Replace sh_update_columns_lines with a new sh_winsize() function.
It calls astwinsize() and is to be used instead of it, and
instead of nv_getval(LINES) and nv_getval(COLUMNS) calls. It:
- Allows passing one or neither of lines or cols pointers.
- Updates COLUMNS and LINES, but only if they actually changed
from the last values. This makes .set discipline functions
defined for these variables more useful.
- Sets the sh.winch flag, but only if COLUMNS changes. If only
the height changes, the command line does not need redrawing.
src/cmd/ksh93/include/io.h:
- Add sh_editor_active() that allows checking whether one of vi,
emacs or gmacs is active without onerous #if SHOPT_* directives.
src/cmd/ksh93/sh/jobs.c: job_reap():
- Remove the fix backported in fc655f1a, which was really just a
workaround that papered over the real bug.
- Move a check for errno==ECHILD so it is only done when waitpid()
returns an error (pid < 0); the check was not correct because C
library functions that do not error out also do not change errno.
- Move the SH_NOTIFY && SH_TTYWAIT code section to within the
while(1) loop so it is run for each job, not only the
last-processed one. This fixes the bug where only one job was
notified when more than one ended at the same time.
- In that section, check if an editor is active; if so, set the
output file for job_list() to sh.notifybuffer instead of standard
error, list the jobs without the initial newline (JOB_NLFLAG),
and trigger a command line redraw by setting sh.winch.
src/cmd/ksh93/edit/edit.c:
- Obtain not just CURSOR_UP but also ERASE_EOS (renamed from
KILL_LINE) using 'tput'. The latter had the ANSI sequence
hardcoded. Define a couple of TPUT_* macros to make it easier to
deal with terminfo/termcap codes.
- Add get_tput() to make it easier to get several tput values
robustly (with SIGINT blocked, trace disabled, etc.)
- ed_crlf(): Removed. Going by those ancient #ifdefs, nothing that
93u+m will ever run on requires a '\r' before a '\n' to start a
new line on the terminal. Plus, as of 93u+, there were already
several spots in emacs.c and vi.c where it printed a sole '\n'.
- ed_read():
- Simplify/modernise command line erase using ERASE_EOS.
- Between erasing and redrawing, print the contents of the notify
buffer. This has the effect of inserting notifications above
the command line while the user is typing.
src/cmd/ksh93/features/cmds:
- To detect terminfo vs termcap codes, use all three codes we're
currently using. This matters on at least on my system (macOS
10.14.6) in which /usr/bin/tput has incomplete terminfo support
(no 'ed') but complete termcap support!
Reproducer:
$ /bin/sleep 100
^Z[1] + Stopped /bin/sleep 100
$ kill %% <--- no notification shown
$ jobs <--- nothing: it was in fact killed
Expected behaviour:
$ /bin/sleep 100
^Z[1] + Stopped /bin/sleep 100
$ kill %%
[1] + Terminated /bin/sleep 100
In the reproducer, the job in fact gets killed, but no notification
is printed of this fact. This is because notifications now require
the P_BG (background job) flag, and it is not set if a foreground
job is stopped. It should be: stopping moves it to the background.
When the 'sleep' builtin is used instead of the external command,
the notice says 'Done' instead of 'Terminated' because SIGTERM is
being handled via sh_fault() instead of set to SIG_DFL. It remains
to be considered if anything needs to change about that.
src/cmd/ksh93/sh/jobs.c: job_reap():
- Also set a job's P_BG bit when it was stopped.
After that commit, iousepipe(), sh_iounpipe(), and supporting flags
were all broken and dead code. Since all command substitutions use
temp files now, they're unused; remove them.
I'm experimenting with reintroducing pipes to command substitutions
in a consistent way, as this is needed for them to wait for
grandchildren. If and when I ever manage to figure out how to do
that in a way that doesn't cause severe hanging and crashing bugs,
these functions may return in some form.
Related: https://github.com/ksh93/ksh/issues/124
Notable changes:
- Tie up some loose ends re: 3de4da5a and 7ba2c685.
- comp/omitted.c: Header include fix for Cygwin.
- misc/optget.c:
- args(): When printing options for the uage line, use a local
pointer for the 'if' block instead of reusing the 'b' pointer.
That variable is used to output blanks later.
- The above fix allows re-enabling the AST translation-aware
macros and deleting the astsa fallback without causing usage
message corruption in multibyte locales. Maybe someday we'll
make ksh actually translatable.
- Remove code to reinitialise _error_infop_ and _opt_info_
'because these are not initialised by all DLLs'. In 2022,
hopefully the buggy dynamic linkers are fixed. If not, we're
not going to find out by keeping the workaround. I suspect that
those bugs may have been triggered by the Microsoft/Cygwin
import/export obfuscation removed in 3de4da5a.
- ksh93:
- Remove unused sh.st.var_local variable. This was a leftover of
a 93v- attempt to implement the bash 'local' command. It used
static scoping, so it's not actually compatible.
- Add a few regression tests for miscellaneous breakage that I
caused in experiments (the breakage never made it to git; the
tests are just to keep it that way).
Windows/Cygwin requires onerous special handling and the definition
of additional _imp__* symbols to import/export symbols between
dynamically linked binaries. Its support in AST used a lot of
macros and code obfuscation. In the features/common test for this,
AT&T called this the "Microsoft import/export nonsense".
They're right, it's nonsense. Somehow, Microsoft's POSIX layer,
SFU/Interix, always managed without it. No one has time to maintain
this (especially considering how incredibly sluggish Cygwin is).
And in fact, it had already fallen victim to bit rot; I confirmed
this in my early experiments with reintroducing dynamic library
support. No one has time to fix it, either.
So, my apologies to any Cygwin fans; ksh 93u+m will never support
dynamically loadable built-ins on Cygwin, even when I do manage to
reintroduce dynamic linking properly.
This removes support for the (AFAIK, completely unavailable) UWIN
preprocessor, which used directives that nothing else understands,
hidden behind the following macros:
__STDPP__
__STDPP__directive
__STDPP__hide
This mailing list post from 2001 says that __STDPP__ is UWIN:
https://lists.gnu.org/archive/html/bug-groff/2001-04/msg00030.html
This also removes code hidden by the __EMX__ symbol. EMX (Eberhard
Mattes eXtender) was a programming environment and POSIX interface
for MS-DOS and OS/2. The last release was in 1998.
https://en.wikipedia.org/wiki/EMX_%28programming_environment%29
UWIN was David Korn's UNIX emulation layer for Microsoft Windows.
It was never very well known, certainly not like Cygwin or
Microsoft SFU/Interix. It was a very interesting system that
exposed the Windows registry to the file system, making it
UNIX-like, and that natively used ksh and all the AST utilities.
Regrettably, it appears to be dead and buried. Only 32-bit binaries
can still be found in the wild, as well as the source code at:
https://github.com/att/uwin
The latter does not seem to be usable since (as far as I can tell)
it requires a UWIN environment with a compiler to build, and UWIN
binaries with a compiler are simply nowhere to be found.
The activity level on that repo (which is zero) also shows how much
interest there still is in this project. And of course the
supporting code in this repo is almost certainly broken by now as
we've never been able to test it on a UWIN system.
The AST team clearly cared about it since roughly 8k lines of code
are dedicated to its support, disabled (directly or indirectly) on
non-UWIN systems via the _UWIN macro. This removes all that.
astsa(3) is a small stand-alone version of the ast library that
only requires cdt(3) and sfio(3). We're not using it. Nothing in
the original AT&T code base was using it, either. It wasn't being
compiled. Bit rot is inevitable, so remove.
An old annoyance of mine: when an error occurs in a ksh function,
the function name is reported in the error message, but the same is
not done for POSIX functions.
Since POSIX functions are treated as glorified dot scripts,
b_dot_cmd() needs an errorpush() call and an assignment to
error_info.id to make this happen for both POSIX functions and dot
scripts. This is the same thing that sh_funscope() does for ksh
functions. There is now no difference from ksh functions in how
these report errors.
Note that the sh_popcontext() macro includes an errorpop() call, so
that does not need to be added. See fault.h and error.h.
In main.c:
158: if(sh.ppid==1)
159: sh.login_sh++;
If that was ever valid, it certainly is not now. As far as I know,
there is no currently existing system where PID 1 (init or systemd
or whatever) is the parent shell of the login shell, even straight
after bootup; login shells are invoked via a program like login(1).
Plus, there is no guarantee the init process actually has PID 1.
This invalidates all use of login_sh that couldn't be replaced by
checks for the login_shell option, so this commit does just that.
src/cmd/ksh93/include/shell.h:
- Remove login_sh flag.
src/cmd/ksh93/sh/init.c:
- If a login shell was detected, just set the login_shell option.
- Remove obsolete check for #! setuid scripts. This was meant to
guard against a symlink called '-i' to a setuid script with a
hashbang path, which used to give users a root shell. All modern
Unixes ignore the setuid bit when they detect a hashbang path.
src/cmd/ksh93/SHOPT.sh:
- By default, let's require the -p/--privileged invocation option
for the setuid/setgid bit on the shell binary to be respected,
for all user IDs (>= 0). This is what bash and mksh do, and
it seems sensible. (See init.c 1475-1483)
The pseudorandom generator generates a reproducible sequnece of
values after seeding it with a known value. But:
$ (RANDOM=1; echo $RANDOM; echo $RANDOM)
2100
18270
$ (RANDOM=1; echo $RANDOM; ls >/dev/null; echo $RANDOM)
2100
30107
Since RANDOM was seeded with a specific value, the two command
lines should output the same pair of numbers. Running 'ls' in the
middle should make no difference.
The cause is a nv_getval(RANDNOD) call in xec.c, case TFORK, that
is run for all TFORK cases, in the parent process -- including
background jobs and external commands. What should happen instead
is that $RANDOM is reseeded in the child process.
This bug is in every version of ksh93 since before 1995.
There is also an opportunity for optimisation. As of 396b388e, the
RANDOM seed may be invalidated by setting rand_last to -2,
triggering a reseed the next time a $RANDOM value is obtained. This
was done to optimise the virtual subshell mechanism. But that can
also be used to eliminate unconditional reseeding elsewhere. So as
of this commit, RANDOM is never (re)seeded until it's used.
src/cmd/ksh93/include/variables.h,
src/cmd/ksh93/sh/subshell.c:
- Add RAND_SEED_INVALIDATED macro, a single source of truth for the
value that triggers a reseeding in sh_save_rand_seed().
- Add convenient sh_invalidate_rand_seed() function macro.
src/cmd/ksh93/sh/init.c,
src/cmd/ksh93/sh/xec.c:
- Optimisation: invalidate seed instead of reseeding directly.
- sh_exec(): case TFORK: Delete the nv_getval(RANDNOD) call. Add a
sh_invalidate_rand_seed() to the child part. This fixes the bug.
Before the fix, a file descriptor leak could occur on command not
found because sh_ntfork() saves sh.topfd on function entry (as part
of the sh_pushcontext() macro expansion) and uses that value to
sh_iorestore(). Process substitution arguments will already have
been processed by then and their file descriptors opened, so those
were not closed on restore. To compensate, the committed fix added
a second sh_iorestore() call using the topfd variable, in which
sh_exec) saves the value of sh.topfd on its function entry. A more
elegant fix is to pass that value to sh_ntfork() as an argument so
its own sh_iorestore() call does the right thing.
In the olden days, ksh used the hash(3) library to store variables,
aliases, functions, etc. For many years, it's been using the cdt(3)
library instead. But the low-level nv_search() name-value tree
lookup function was still repurposing some bit flags from the old
hash API for its options, though that API is otherwise unused.
So we were still including the entire obsolete <hash.h> API just
to use two repurposed HASH_* bit flags for nv_search(). This commit
makes nv_search() repurpose some flags from <nval.h> instead.
This commit should not change ksh's behaviour.
src/cmd/ksh93/sh/nvdisc.c:
- Make nv_search() use NV_NOSCOPE instead of HASH_NOSCOPE and
NV_REF instead of HASH_BUCKET.
- The HASH_SCOPE flag was also passed to some nv_search() calls,
but nv_search() ignores it, so that flag is deleted from those.
- Document nv_search() in a comment.
src/cmd/ksh93/include/name.h:
- Move NV_UNATTR to nval.h to join the other nv_open() options
there. (re: 1184b2ad)
src/cmd/ksh93/include/nval.h:
- Since we no longer use HASH_* macros, do not include <hash.h>.
- Remove unused NV_NOASSIGN macro, defined to 0. This was there
for "backward compatibility" since before 1995; long enough.
src/cmd/ksh93/include/shell.h:
- Bump SH_VERSION due to the nv_search() API change (even though no
changes were made to the APIs documented in nval.3 or shell.3).
All other changed files:
- Update to match the flaggery changes.
*.getn discipline functions cause .sh.value to have a float type
for arithmetic expressions that get the value of foo, avoiding the
problem of having to convert between floats and strings (e.g.
rounding errors). There is no corresponding .setn discipline.
A search in the ast-open-archive repo reveals that the getn
discipline was quietly added in version 2009-08-21 93t+, with not
even a mention in the RELEASE file.
The one available mention on the internet is this old thread:
https://www.mail-archive.com/ast-users@research.att.com/msg00601.html
Apparently a setn discipline *was* planned, but never implemented.
getn discipline functions may also crash in several ways. I've been
unsuccessful at solving all the crashes, particularly as one of
them is intermittent. This should not be in the 1.0 release.
Further discussion: https://github.com/ksh93/ksh/issues/435
This now allows our little make utility to run when compiled with
AddressSanitizer on gcc on Linux; it no longer aborts the build on
exit with a complaint about (very small) memory leaks.
The change in .github/workflows/ci.yml (use 'bin/package test' to
run the iffe regression tests as well) caused the GitHub workflow
to fail immediately with a syntax error. This is because iffe.tst
is a ksh93 script, but the runner does not have a ksh93 installed
as a system package.
As of e08ca80d, package prefers a known-good system shell over the
newly compiled shell to stop builds from failing when I break ksh,
making it convenient to fix it. But that change should not apply to
the regression tests; they should use the newly compiled shell.
src/cmd/INIT/package.sh, bin/package:
- Set KEEP_SHELL to 2 when given a SHELL=* argument.
- Before running the regression tests, override the known-good
shell with the compiled one if $KEEP_SHELL < 2, ensuring that all
tests, including the iffe ones, are run with the compiled shell.
This allows 'bin/package test' to run if the known-good shell is
not a ksh93, while testing that the compiled shell successfully
runs iffe. (re: e08ca80d)
- Standardise 'test' command use: -a and -o are deprecated.
- Remove some more unused cruft. (re: 6137b99a)
src/cmd/ksh93/Mamfile:
- Do not override SHELL when running shtests; this is now set
correctly in 'bin/package test'.
src/cmd/INIT/rt.sh:
- Removed. This regression test output filter was only used with
nmake, which we deleted. (re: 2940b3f5, 6cc2f6a0, aa601a39)
When varsub(), which "handles $param, ${param}, and ${param op
word}", handles arrays, it obtains an 'ap' array pointer once using
nv_arrayptr(np). In several locations it calls nv_putsub() which
may call array_grow() which invalidates that pointer. But the
pointer is never updated. So whenver an array grows, there is a
use-after-free problem, easily caught by AddresSanitizer.
This commit makes sure the 'ap' pointer, if non-null, is refreshed
whenever nv_putsub() may previously have been called.
Notable changes:
.github/workflows/ci.yml:
- Run 'bin/package test' on the github runner so we test iffe too.
src/cmd/ksh93/sh/subshell.c:
- sh_assignok was usually called like 'np = sh_assignok(np,0)'. But
the function never changes np, it just returns the np value
passed to it, so the assignment is pointless and that function
can be changed to a void.
src/cmd/ksh93/sh/fault.c: sh_fault():
- Remove check for sh.subshell after sh_isstate(SH_INTERACTIVE). As
of 48ba6964, it is never set in subshells.
When running an external command while trapping Ctrl+C via SIGINT,
and set -b is on, then a spurious Done job control message is
printed. No background job was executed.
$ trap 'ls' INT
$ set -b
$ <Ctrl+C>[file listing follows]
[1] + Done set -b
In jobs.c (487-493), job_reap() calls job_list() to list a running
or completed background job, passing the JOB_NFLAG bit to only
print jobs with the P_NOTIFY flag. But the 'ls' in the trap is not
a background job. So it is getting the P_NOTIFY flag by mistake.
In fact all processes get the P_NOTIFY flag by default when they
terminate. Somehow the shell normally does not follow a code path
that calls job_list() for foreground processes, but does when
running one from a trap. I have not yet figured out how that works.
What I do know is that there is no reason why non-background
processes should ever have the P_NOTIFY flag set on termination,
because those should never print any 'Done' messages. And we seem
to have a handy P_BG flag that is set for background processes; we
can check for this before setting P_NOTIFY. The only thing is that
flag is only compiled in if SHOPT_BGX is enabled, as it was added
to support that functionality.
For some reason I am unable to reproduce the bug in a pty session,
so there is no pty.sh regression test.
src/cmd/ksh93/sh/jobs.c:
- Rename misleadingly named P_FG flag to P_MOVED2FG; this flag is
not set for all foreground processes but only for processes moved
to the foreground by job_switch(), called by the fg command.
- Compile in the P_BG flag even when SHOPT_BGX is not enabled. We
need to set this flag to check for a background job.
- job_reap(): Do not set the P_NOTIFY flag for all terminated
processes, but only for those with P_BG set.
src/cmd/ksh93/sh/xec.c: sh_fork():
- Also pass special argument 1 for background job to job_post() if
SHOPT_BGX is not enabled. This is what gets it to set P_BG.
- Simplify 5 lines of convoluted code into 1.
Resolves: https://github.com/ksh93/ksh/issues/481
Switching the function scope to a parent scope by assigning to
.sh.level (SH_LEVELNOD) leaves the shell in an inconsistent state,
causing invalid-free and/or use-after-free bugs. The intention of
.sh.level was always to temporarily switch scopes inside a DEBUG
trap, so this commit minimises the pitfalls and instability by
imposing some sensible limitations:
1. .sh.level is now a read-only variable except while executing a
DEBUG trap;
2. while it's writeable, attempts to unset .sh.level or to change
its attributes are ignored;
3. attempts to set a discipline function for .sh.level are ignored;
4. it is an error to set a level < 0 or > the current scope.
Even more crashing bugs are fixed by simplifiying the handling and
initialisation of .sh.level and by exempting it completely from
virtual subshell scoping (to which it's irrelevant).
TODO: one thing remains: scope corruption and use-after-free happen
when using the '.' command inside a DEBUG trap with ${.sh.level}
changed. Behaviour same as before this commit. To be investigated.
All changed files:
- Consistently use the int16_t type for level values as that is the
type of its non-pointer storage in SH_LEVELNOD.
- Update .sh.level by using an update_sh_level() macro that assigns
directly to the node value, then restores the scope if needed.
- To eliminate implicit typecasts, use the same int16_t type (the
type used by short ints such as SH_LEVELNOD) for all variables
containing a function and/or dot script level.
src/cmd/ksh93/include/variables.h:
- Add update_sh_level() macro.
src/cmd/ksh93/include/name.h,
src/cmd/ksh93/sh/macro.c:
- Add a nv_nonptr() macro that checks attributes for a non-pointer
value -- currently only signed or unsigned short integer value,
accessed via the 's' member of 'union Value' (e.g. np->nvalue.s).
- nv_isnull(): To avoid undefined behaviour, check for attributes
indicating a non-pointer value before accessing the nvalue.cp
pointer (re: 5aba0c72).
- varsub(): In the set/unset check, remove the now-redundant
exception for SH_LEVELNOD.
src/cmd/ksh93/data/variables.c,
src/cmd/ksh93/sh/init.c:
- shtab_variables[]: Make .sh.level a read-only short integer.
- sh_inittree(): To avoid undefined behaviour, do not assign to the
'union Value' char pointer if the attribute indicates a non-
pointer short integer value. Instead, the table value is ignored.
src/cmd/ksh93/sh/subshell.c: sh_assignok():
- Never create a subshell scope for SH_LEVELNOD.
src/cmd/ksh93/sh/xec.c:
- Get rid of 'struct Level' and its maxlevel member. This was only
used in put_level() to check for an out of range assignment, but
this can be trivially done by checking sh.fn_depth+sh.dot_depth.
- This in turn allows further simplification that reduces init for
.sh.level to a single nv_disc() call in sh_debug(), so get rid of
init_level().
- put_level(): Throw a "level out of range" error if assigned a
wrong level.
- sh_debug():
- Turn off the NV_RDONLY (read-only) attribute for SH_LEVELNOD
while executing the DEBUG trap.
- Restore the current scope when trap execution is finished.
- sh_funct(): Remove all .sh.level handling. POSIX functions (and
dot scripts) already handle it in b_dot_cmd(), so sh_funct(),
which is used by both, is the wrong place to do it.
- sh_funscope(): Update .sh.level for ksh syntax functions here
instead. Also, do not bother to initialise its discipline here,
as it can now only be changed in a DEBUG trap.
src/cmd/ksh93/bltins/typeset.c: setall():
- When it's not read-only, ignore all attribute changes for
.sh.level, as changing the attributes would crash the shell.
src/cmd/ksh93/sh/nvdisc.c: nv_setdisc():
- Ignore all attempts to set a discipline function for .sh.level,
as doing this would crash the shell.
src/cmd/ksh93/bltins/misc.c: b_dot_cmd():
- Bug fix: also update .sh.level when quitting a dot script.
src/cmd/ksh93/sh/name.c:
- _nv_unset():
- To avoid an inconsistent state, ignore all attempts to unset
.sh.level.
- To avoid undefined behaviour, do not zero np->nvalue.cp if
attributes for np indicate a non-pointer value (the actual bit
value of a null pointer is not defined by the standard, so
there is no guarantee that zeroing .cp will zero .s).
- sh_setscope(): For consistency, always set error_info.id (the
command name for error messages) to the new scope's cmdname.
Previously this was only done for two calls of this function.
- nv_name(): Fix a crashing bug by checking that np->nvname is a
non-null pointer before dereferencing it.
src/cmd/ksh93/include/nval.h:
- The NV_UINT16P macro (which is unsigned NV_INT16P) had a typo in
it, which went unnoticed for many years because it's not directly
used (though its bit flags are set and used indirectly). Let's
fix it anyway and keep it for completeness' sake.
The lexer use 256-byte state tables (see data/lexstates.c), one
byte per possible value for the (unsigned) char type. But the sp
variable used as an index to a state table in loops like this...
while((n = state[*sp++]) == 0)
;
...is a char*, a pointer to a char. The C standard does not define
if the char type is signed or not (!). On clang and gcc, it is
signed. That means that, whenever a single-byte, high-bit (> 127)
character is encountered, the value wraps around to negative, and a
read occurs outside of the actual state table, causing potentially
incorrect behaviour or a crash.
src/cmd/ksh93/sh/lex.c:
- endword(): Make sp and three related variables explicitly
unsigned char pointers. This requires a bunch of annoying
typecasts to stop compilers complaining; so be it.
- To avoid even more typecasts, make stack_shift() follow suit.
- Reorder variable declarations for legibility.
num_clone handles simple numeric values of all types but not array
nodes. Calling it for an array node caused the arith.sh regression
test below to crash in num_clone() with a buffer overflow when ksh
is compiled with AddressSanitizer. The array has the NV_INTEGER
attribute because it is an array of numeric values, but that
doesn't mean the array node itself holds a number.
After this, all the arith.sh tests pass with AddressSanitizer.
The failing test in arith.c was this one, introduced in d50d3d7c:
got=$(
typeset -r -A -i ro_arr=([a]=10 [b]=20 [c]=30)
set +x
for ((i=0; i<loopcount; i++)); do
( ((ro_arr[i+1] += 5)) )
done 2>&1
)
[[ $got == *recursion* ]] && err_exit "recursion level not reset on readonly error (subshell)"
Reproducer: Compile a ksh with AddressSanitizer. In that ksh, edit
the last command line with 'fc', insert an empty line at the start,
and save. Now use the up-arrow to retrieve the empty line. Ksh
aborts on history.c line 1011 as hist_copy() tries to read before
the beginning of the buffer pointed to by s1.
src/cmd/ksh93/edit/history.c: hist_copy():
- Verify that the s1 pointer was increased from the original s1
before trying to read the character *(s1-1).
The race is between '$cat |&' and 'kill $pid'. In between, there
are only a variable assignment and two buffered writes, so there
is nothing that waits for the external 'cat' to finish forking,
execve'ing and initialising -- meaning there is no guarantee it is
ready to catch SIGTERM. This explains the hang; 'cat' misses the
signal, continues to initialise, and simply waits for more input.
src/cmd/ksh93/tests/coprocess.sh:
- Actually read from the /bin/cat coprocess and verify that it
works. This has the beneficial side effect of ensuring it is
fully loaded and initialised before SIGTERMing it.
Resolves: https://github.com/ksh93/ksh/issues/132
Reproducer:
$ x=([x]=1 [y)
-ksh: syntax error: `)' unexpected
$ [[ -z $x ]]
-ksh: [[ -z ]]: not found
Any '[[' command following that syntax error will fail similarly;
the whole of it (after variable expansion) is incorrectly looked up
as a command name. The syntax error must be generated by an
associative array assignment (with or without an explicit typeset
-A) with at least one valid assignment element followed by an
invalid assignment element starting with '[' but not containing
']='.
This seems to be another bug that is in every ksh93 version ever.
I've confirmed that ksh 1993-12-28 s+ and ksh2020 fail identically.
Presumably, so does everything in between.
Analysis:
The syntax error function, sh_syntax(), calls lexopen() in mode 0
to reset the lexer state. There is a variable that isn't getting
reset there though it should be. Using systematic elimination I
found that the variable that needs to be reset is lp->assignok (set
"when name=value is legal"). If it is set, '[[' is not processed.
src/cmd/ksh93/sh/lex.c: lexopen():
- Reset 'assignok' in the lexer state (regardless of mode).
- In the mode 0 total lexer state reinit, several members of lexd
(struct _shlex_pvt_lexdata_) were not getting reset; just memset
the whole thing to zero.
Note for backporters: this change requires commit da97587e to
be correct. That commit took the stack size and pointer (lex_max
and *lex_match) out of this struct; those should not be reset!
Resolves: https://github.com/ksh93/ksh/issues/486
Dynamically loadable built-ins do not work well with a statically
linked ksh; they cannot use ksh's statically linked copies of
libast and libshell, so they would need to bring their own, but
multiple copies of those don't play well together. So dynamically
loaded built-ins cannot interface with the shell. Only non-AST,
non-SFIO built-ins are possible. Which is something that perhaps
five people in the world know how to do as this is not documented
anywhere (hint: your built-in needs the BLT_NOSFIO attribute to use
stdio without problems). And those five people are also able to
compile their own ksh with SHOPT_DYNAMIC reenabled.
Plus, the SHOPT_DYNAMIC code causes strange $PATH search
regressions on a few systems. The cause of that bug has eluded me
so far, but disabling this is effectively a fix on those systems.
src/cmd/ksh93/SHOPT.sh:
- Turn off SHOPT_DYNAMIC by default.
src/cmd/ksh93/data/builtins.c:
- Do not compile in irrelevant sh_optbuiltin[] (builtin --man)
documentation if SHOPT_DYNAMIC is disabled.
The SHOPT_MULTIBYTE compile-time option did not make much sense as
disabling it only disabled multibyte support for ksh/libshell, not
libast or libcmd built-in commands. This commit allows disabling
multibyte support for the entire codebase by defining the macro
AST_NOMULTIBYTE (e.g. via CCFLAGS). This slightly speeds up the
code and makes an optimised binary about 5% smaller.
src/lib/libast/include/ast.h:
- Add non-multibyte fallback versions of the multibyte macros that
are used if AST_NOMULTIBYTE is defined. This should cause most
multibyte handling to be automatically optimised out everywhere.
- Reformat the multibyte macros for legibility.
- Similify mbchar() and and mbsize() macros by defining them in
terms of mbnchar() and mbnsize(), eliminating code duplication.
- Correct non-multibyte fallback of mbwidth(). For consistent
behaviour, control characters and out-of-range values should
return -1 as they do for UTF-8. The fallback is now the same as
default_wcwidth() in src/lib/libast/comp/setlocale.c.
src/lib/libast/comp/setlocale.c:
- If AST_NOMULTIBYTE is defined, do not compile in the debug and
UTF-8 locale conversion functions, including several large
conversion tables. Define their fallback macros as 0 as these are
used as function pointers.
src/cmd/ksh93/SHOPT.sh,
src/cmd/ksh93/Mamfile:
- Change the SHOPT_MULTIBYTE default to empty, indicating "probe".
- Synchronise SHOPT_MULTIBYTE with !AST_NOMULTIBYTE by default.
src/cmd/ksh93/include/defs.h:
- When SHOPT_MULTIBYTE is zero but AST_NOMULTIBYTE is not non-zero,
then enable AST_NOMULTIBYTE here to use the ast.h non-multibyte
fallbacks for ksh. When this is done, the effect is that
multibyte is optimized out for ksh only, as before.
- Remove previous fallback for disabling multibyte (re: c2cb0eae).
src/cmd/ksh93/include/lexstates.h,
src/cmd/ksh93/sh/lex.c:
- Define SETLEN() macro to assign to LEN (i.e. _Fcin.fclen) for
multibyte only and do not assign to it directly. With no
SHOPT_MULTIBYTE, define that macro as empty. This allows removing
multiple '#if SHOPT_MULTIBYTE' directives from lex.c, as that
code will all be optimised out automatically if it's disabled.
src/cmd/ksh93/include/national.h,
src/cmd/ksh93/sh/string.c:
- Fix flagrantly incorrect non-multibyte fallback for sh_strchr().
The latter returns an integer offset (-1 if not found), whereas
strchr(3) returns a char pointer (NULL if not found). Incorporate
the fallback into the function for correct handling instead of
falling back to strchr(3) directly.
src/cmd/ksh93/sh/macro.c:
- lastchar() optimisation: avoid function call if SHOPT_MULTIBYTE
is enabled but we're not actually in a multibyte locale.
src/cmd/ksh93/sh/name.c:
- Use ja_size() even with SHOPT_MULTIBYTE disabled (re: 2182ecfa).
Though no regression tests failed, the non-multibyte fallback for
typeset -L/-R/-Z length calculation was probably not quite
correct as ja_size() does more. The ast.h change to mbwidth()
ensures correct behaviour for non-multibyte locales.
src/cmd/ksh93/tests/shtests:
- Since its value in SHOPT.sh is now empty by default, add a quick
feature test (for the length of the UTF-8 character 'é') to check
if SHOPT_MULTIBYTE needs to be enabled for the regression tests.
inout() initialises its 'token' variable to the value of
lexp->token. So when inout() returns upon finding a process
subtitution, lexp->token is known to contain either IPROCSYM or
OPROCSYM; simple() can check for that instead, making a global flag
unnecessary.
The fact that inout() calls itself recursively to process multiple
redirections does not influence this because the recursive call is
done right before returning from the current call.
Reproducer:
$ fn=([foo_key]=foo_val [bar_key])
-ksh: [bar_key]: not found
Expected output:
-ksh: syntax error: `[bar_key]' unexpected
As soon as one correct associative array assignment element has
been processed, a subsequent one, starting with '[' but not
containing ']=', is incorrectly seen as a command to execute.
If a command '[bar_key]' existed on $PATH, it would have been run.
src/cmd/ksh93/sh/parse.c: simple():
- In the syntax check for associative array assignments, don't just
check for an initial '[' but also verify the presence of ']='.
Thanks to @JohnoKing for finding this bug.
Resolves: https://github.com/ksh93/ksh/issues/427
Grammatically, redirections may occur anywhere within a command
line and are removed after processing them, whereas a process
substitution (<(commandlist) or >(commandlist)) is replaced by a
file name which should be treated as just another simple word.
So the following should not be a syntax error:
$ cat </dev/null <(true)
-ksh: syntax error: `)' unexpected
$ cat </dev/null >(true)
-ksh: syntax error: `)' unexpected
$ cat >/dev/null <(true)
-ksh: syntax error: `)' unexpected
$ cat >/dev/null >(true)
-ksh: syntax error: `)' unexpected
This bug is in every ksh93 version.
The problem is in the parser (parse.c). The process substitution is
misparsed as a redirection due to inout() recursively parsing
multiple redirections without recognising process substitutions.
inout() is mistaking '<(' for '<' and '>(' for '>', which explains
the incorrect syntax error.
This also causes the following to fail to detect a syntax error:
$ cat >&1 <(README.md
[the contents of README.md are shown]
...and other syntax errors detected in the wrong spot, for example:
$ { true; } <(echo wrong)
-ksh: syntax error: `wrong' unexpected
which should be:
-ksh: syntax error: `<(' unexpected
src/cmd/ksh93/sh/parse.c:
- Add global inout_found_procsub flag.
- inout(): On encountering a process substitution, set this flag
and return, otherwise clear the flag.
- simple(): After calling inout(), check this flag and, if set,
jump back to where process substitutions are parsed.
Resolves: https://github.com/ksh93/ksh/issues/418
