It's amazing what can happen when you compile ksh using standard
malloc (i.e. with AST vmalloc disabled) on OpenBSD. Its security
hardening provokes crashes that reveal decades-old unsolved bugs.
This one is an attempt to access one byte before the beginning of
the command line buffer when the cursor is at the beginning of it.
On this system configuration, it provoked an instant crash whenever
you moved the cursor back to the beginning of the command line,
e.g. with ^A or the cursor keys.
src/cmd/ksh93/edit/emacs.c: draw():
- Check that the cursor is actually past the first position of the
command line buffer before trying to read the position
immediately before it. If not, zero the value.
The following caused an infinite loop:
v=${ exec >/dev/tty; }
v=${ redirect >/dev/tty; }
Even the original authors didn't figure out how to 'exec >foo' or
'redirect >foo' inside a non-forking command substitution, so they
fork it by calling sh_subfork(). If we delete that call, even
normal command substitutions enter into that infinite loop. But of
course a shared-state comsub can never fork as it would no longer
share its state. Without a solution to make this work without
forking, an error message is the only sensible thing left to do.
src/cmd/ksh93/sh/io.c: sh_redirect():
- If we're redirecting standard output (1), the redirection is
permanent as in 'exec'/'redirect' (flag==2), and we're in a
subshare, then error out.
Resolves: https://github.com/ksh93/ksh/issues/128
POSIX warns about "unset PWD" leading to unspecified behavior from
the pwd util, which we then use.
bin/package, src/cmd/INIT/package.sh:
- Determine if the shell has $PWD with a feature test. Only unset
PWD if it does not (the old Bourne shell).
- Clean up 'case' flow.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
ksh crashed in various different and operating system-dependent
ways when attempting to create or apply justification strings
using typeset -L/-R/-Z, especially if large sizes are used.
The crashes had two immediate causes:
- In nv_newattr(), when applying justification attributes, a buffer
was allocated for the justified string that was exactly 8 bytes
longer than the original string. Any larger justification string
caused a buffer overflow (!!!).
- In nv_putval(), when applying existing attributes to a new value,
the corresponding memmove() either did not zero-terminate the
justified string (if the original string was longer than the
justified string) or could read memory past the original string
(if the original string was shorter than the justified string).
Both scenarios can cause a crash.
This commit fixes other minor issues as well, such as a mysterious
8 extra bytes allocated by several malloc/realloc calls. This may
have been some naive attempt to paper over the above bugs. It seems
no one can make any other kind of sense of it.
A readjustment bug with zero-filling was also fixed.
src/cmd/ksh93/sh/name.c:
- nv_putval():
. Get rid of the magical +8 bytes for malloc and realloc. Just
allocate one extra byte for the terminating zero.
. Fix the memmove operation to use strncpy instead, so that
buffer overflows are avoided in both scenarios described above.
Also make it conditional upon a size adjustment actually
happening (i.e. if 'dot' is nonzero).
. Mild refactoring: combine two 'if(sp)' blocks into one;
declare variables only used there locally for legibility.
- nv_newattr():
* Replace the fatally broken "let's allocate string length + 8
bytes no matter the size of the adjustment" routine with a new
one based on work by @hyenias (see comments in #142). It is
efficient with memory, taking into account numeric types,
growing strings, and shrinking strings.
* Fix zero-filling in readjustment after changing the initial
size of a -Z attribute. If the number was zero, all zeros were
still skipped, leaving an empty string.
Thanks to @hyenias for originally identifying this breakage and
laying the groundwork for fixing nv_newattr(), and to @lijog for
the crash analysis that revealed the key to the nv_putval() fix.
Resolves: https://github.com/ksh93/ksh/issues/142
Resolves: https://github.com/ksh93/ksh/issues/181
So now we know what that faulty check for shp->indebug in sh_trap()
was meant to do: it was meant to pass down the trap handler's exit
status, via sh_debug(), down to sh_exec() (xec.c) so that it could
then skip the execution of the next command if the trap's exit
status is 2, as documented in the manual page. As of d00b4b39, exit
status 2 was not passed down, so this stopped working.
This commit reinstates that functionality, but without the exit
status bug in command substitutions caused by the old way.
src/cmd/ksh93/sh/fault.c: sh_trap():
- Save the trap's exit status before restoring the parent
envionment's exit status. Make this saved exit status the return
value of the function. (This does not break anything, AFAICT; the
majority of sh_trap() calls ignore the return value, and the few
that don't ignore it seem to expect it to return exactly this.)
src/cmd/ksh93/sh/xec.c: sh_exec():
- The sh_trap() fix has one side effect: whereas the exit status of
a skipped command was always 2 (as per the trap handler), now it
is always 0, because it gets reset in sh_exec() but no command is
executed. That is probably not a desirable change in behaviour,
so let's fix that here instead: set sh.exitval to 2 when skipping
commands.
src/cmd/ksh93/sh.1:
- Document that ${.sh.command} shell-quotes its arguments for use
by 'eval' and such. This fact was not documented anywhere, AFAIK.
src/cmd/ksh93/shell.3:
- Document that $? (exit status) is made local to trap handlers.
- Document that sh_trap() returns the trap handler's exit status.
src/cmd/ksh93/tests/basic.sh:
- Add test for this bug.
- Add a missing test for the exit status 255 functionality (if a
DEBUG trap handler yields this exit status and we're executing a
function or dot script, a return is triggered).
Fixes: https://github.com/ksh93/ksh/issues/187
Many of the errors fixed in this commit are word repetitions
such as 'the the' and minor spelling errors. One formatting
error in the ksh man page has also been fixed.
On SVR4 platforms, select is a sometimes erroneous wrapper around
the poll system call, so call poll directly for efficiency purposes if
it implies no loss in precision.
See, e.g., http://bugs.motifzone.net/long_list.cgi?buglist=129 .
src/lib/libast/features/tvlib:
- For systems lacking nanosleep, test whether select is truly more
precise than poll.
src/lib/libast/tm/tvsleep.c:
- Verify that tv argument is not null.
- Immediately return if asked to sleep for a duration of zero.
- Immediately initialize timespec in the nanosleep case.
- Revise variable names to use Apps Hungarian.
- Use poll instead of select when there is no loss in precision.
- Check for overflow in the poll case.
- Improve comments.
- Revise arithmetic to work with unsigned types, rather than
casting to long.
- Do not return non-zero if we have slept for a sufficient
time.
In the 93v- beta, they add a newline instead of a space.
This has fewer side effects as final newlines get stripped.
It's still a hack and it would still be nice to have a real fix,
but it seems even the AT&T guys couldn't come up with one.
src/cmd/ksh93/sh/macro.c:
- To somehow avoid a memory leak involving alias substitution,
append a linefeed instead of a space to the comsub buffer.
src/cmd/ksh93/tests/subshell.sh:
- Add test for minor regression caused by the RedHat version.
Compilers like GCC are capable of optimizing away calls like
pow(1,inf), which caused the IEEE compliance feature test within
libast to incorrectly succeed on platforms with non-IEEE behavior.
This is arguably a bug within GCC, as floating point optimizations
should never alter the behavior of code unless IEEE compliance is
explicitly disabled via a flag like -ffast-math. Programs in which
only some calls to pow are optimized away are liable to severely
malfunction.
Thanks to Martijn Dekker for pointing this issue out and the kind
operators of polarhome.com for permitting me gratis use of their
Unix systems.
src/lib/libast/comp/omitted.c:
- Add IEEE compliant functions that wrap powf, pow, and powl.
src/lib/libast/features/float:
- Look for powf, pow, and powl in the C library.
- For compilers that do the right thing, like the native toolchains
of Solaris and UnixWare, use lightweight macros to wrap the pow
functions.
- Use a volatile function pointer through which to access the C
library's pow function in an attempt to defeat code optimization.
- For these overzealous compilers, define pow to _ast_pow so that
the same technique can be used within the above functions.
This commit fixes a bug in the 'read' built-in: it did not properly
skip over multibyte characters. The bug never affects UTF-8 locales
because all UTF-8 bytes have the high-order bit set. But Shift-JIS
characters may include a byte corresponding to the ASCII backslash
character, which cauased buggy behaviour when using 'read' without
the '-r' option that disables backslash escape processing.
It also makes the regression tests compatible with Shift-JIS
locales. They failed with syntax errors.
src/cmd/ksh93/bltins/read.c:
- Use the multibyte macros when skipping over word characters.
Based on a patch from the old ast-developers mailing list:
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01848.html
src/cmd/ksh93/include/defs.h:
- Be a bit smarter about causing the compiler to optimise out
multibyte code when SHOPT_MULTIBYTE is disabled. See the updated
comment for details.
src/cmd/ksh93/tests/locale.sh:
- Put all the supported locales in an array for future tests.
- Add test for the 'read' bug. Include it in a loop that tests
64 SHIFT-JIS character combinations. Only one fails on old ksh:
the one where the final byte corresponds to the ASCII backslash.
It doesn't hurt to test all the others anyway.
src/cmd/ksh93/tests/basic.sh,
src/cmd/ksh93/tests/builtins.sh,
src/cmd/ksh93/tests/quoting2.sh:
- Fix syntax errors that occurred in SHIFT-JIS locales as the
parser was processing literal UTF-8 characters. Not executing
that code is not enough; we need to make sure it never gets
parsed as well. This is done by wrapping the commands containing
literal UTF-8 strings in an 'eval' command as a single-quoted
operand.
.github/workflows/ci.yml:
- Run the tests in the ja_JP.SJIS locale instead of ja_JP.UTF-8.
UTF-8 is already covered by the nl_NL.UTF-8 test run; that should
be good enough.
This commit introduced the following bug, which is worse than the
one that commit fixed: it became impossible to alter the size of an
existing justified string attribute.
Thanks to @hyenias for catching this bug:
https://github.com/ksh93/ksh/issues/142#issuecomment-780931533
$ unset s; typeset -L 100 s=h; typeset +p s; typeset -L 5 s; typeset +p s
typeset -L 100 s
typeset -L 100 s
Expected output:
typeset -L 100 s
typeset -L 5 s
src/cmd/ksh93/sh/name.c:
- Revert.
src/cmd/ksh93/tests/attributes.sh:
- Revert: re-disable tests for minor attribute output regressions.
- Add a test for this bug and potential similar bugs.
A ${ shared-state command substitution; } (internally called
subshare) is documented to share its state with the parent shell
environment, so all changes made within the command substitution
survive outside of it. However, when it is run within a
virtual/non-forked subshell, variables that are not already local
to that subshell will leak out of it into the grandparent state.
Reproducer:
$ ksh -c '( v=${ bug=BAD; } ); echo "$bug"'
BAD
If the variable pre-exists in the subshell, the bug does not occur:
$ ksh -c '( bug=BAD1; v=${ bug=BAD2; } ); echo "$bug"'
(empty line, as expected)
The problem is that the sh_assignok() function, which is
responsible for variable scoping in virtual subshells, does not
ever bother to create a virtual subshell scope for a subshare.
That is an error if a subshare's parent (or higher-up ancestor)
environment is a virtual subshell, because a scope needs to be
created in that parent environment if none exists.
To make this bugfix possible, first we need to get something out of
the way. nv_restore() temporarily sets the subshell's pointer to
the preesnt working directory, shpwd, to null. This causes
sh_assignok() to assume that the subshell is a subshare (because
subshares don't store their own PWD) and refuse to create a scope.
However, nv_restore() sets it to null for a different purpose: to
temporarily disable scoping for *all* virtual subshells, making
restoring possible. This is a good illustration of why it's often
not a good idea to use the same variable for unrelated purposes.
src/cmd/ksh93/sh/subshell.c:
- Add a global static subshell_noscope flag variable to replace the
misuse of sh.shpwd described above.
- sh_assignok():
. Check subshell_noscope instead of shpwd to see if scope
creation is disabled. This makes it possible to distinguish
between restoring scope and handling subshares.
. If the current environment is a subshare that is in a virtual
subshell, create a scope in the parent subshell. This is done
by temporarily making the parent virtual subshell the current
subshell (by setting the global subshell_data pointer to it)
and calling sh_assignok() again, recursively.
- nv_restore(): To disable subshell scope creation while restoring,
set subshell_noscope instead of saving and unsetting sh.shpwd.
src/cmd/ksh93/tests/subshell.sh:
- Add tests. I like tests. Tests are good.
Fixes: https://github.com/ksh93/ksh/issues/143
This commit fixes the following:
1. Emacs mode ignores --nobackslashctrl (re: 24598fed) when in
reverse search.
2. When entering more than one backslash, emacs reverse search mode
deletes multiple backslashes after pressing backspace once.
Reproducer:
$ set --emacs --nobackslashctrl
$ <Ctrl+R> \\\\<Backspace>
3. Except when in reverse search, the backslash fails to escape a
subsequent interrupt character (^C). Reproducer:
$ set --emacs --backslashctrl
$ teststring \<Ctrl+C>
src/cmd/ksh93/edit/emacs.c:
- Disable escaping backslashes in emacs reverse search if
'nobackslashctrl' is enabled.
- Fix the buggy behavior of backslashes in emacs reverse
search by processing backslashes in a loop.
src/cmd/ksh93/tests/pty.sh:
- Add regression tests.
src/cmd/ksh93/sh.1:
- Fix a minor documentation error (^C is the usual interrupt
character, not ^?).
Co-authored-by: Martijn Dekker <martijn@inlv.org>
'case x in esac' should be syntactically correct, but was an error:
$ ksh -c 'case x in esac'
ksh: syntax error at line 1: `case' unmatched
Inserting a newline was a workaround:
$ ksh -c $'case x in\nesac'
(no output)
The problem was that the 'esac' reserved word was not being
recognised if it immediately followed the 'in' reserved word.
src/cmd/ksh93/sh/lex.c: sh_lex():
- Do not turn off recognition of reserved words after 'in' if we're
in a 'case' construct; only do this for 'for' and 'select'.
src/cmd/ksh93/tests/case.sh:
- Add seven regression test for correct recognition of 'esac'.
Only two failed on ksh93. The rest is to catch future bugs.
Fixes: https://github.com/ksh93/ksh/issues/177
This commit fixes the functionality of Alt+D and Alt+H in emacs mode.
These keyboard shortcuts are intended to work on whole words, but
after commit 13c3fb21 their functionality was reduced to deleting only
singular letters:
$ Test word <Alt+H> # This should delete 'word', not just 'd'.
$ Foo <Alt+B> <Alt+D> # This should delete 'Foo', not just 'F'.
Man page entries for reference:
M-d Delete current word.
M-^H (Meta-backspace) Delete previous word.
M-h Delete previous word.
src/cmd/ksh93/edit/emacs.c:
- 'count' cannot be overridden when handling Alt+D or Alt+H,
so add the total number of repetitions to count (the number of
repetitions can't be negative).
- If 'count' is a negative number, set it to one before adding the
number of repetitions.
The following emacs editor 'feature' kept making me want to go back
to bash. I forget a backslash escape in a command somewhere. So I
go back to insert it. I type the \, then want to go forward. My
right arrow key, instead of moving the cursor, then replaces my
backslash with garbage. Why? The backslash escapes the following
control character at the editor level and inserts it literally.
The vi editor has a variant of this which is much less harmful. It
only works in insert mode and the backslash only escapes the next
kill or erase character.
In both editors, this feature is completely redundant with the
'stty lnext' character which is ^V by default -- and works better
as well because it also escapes ^C, ^J (linefeed) and ^M (Return).
[In fact, you could even issue 'stty lnext \\' and get a much more
consistent version of this feature on any shell. You have to type
two backslashes to enter one, but it won't kill your cursor keys.]
If it were up to me alone, I'd simply remove this misfeature from
both editors. However, it is long-standing documented behaviour.
It's in the 1995 book. Plus, POSIX specifies the vi variant of it.
So, this adds a shell option instead. It was quite trivial to do.
Now I can 'set --nobackslashctrl' in my ~/.kshrc. What a relief!
Note: To keep .kshrc compatibile with older ksh versions, use:
command set --nobackslashctrl 2>/dev/null
src/cmd/ksh93/include/shell.h,
src/cmd/ksh93/data/options.c:
- Add new SH_NOBACKSLCTRL/"nobackslashctrl" long-form option. The
"no" prefix shows it to the user as "backslashctrl" which is on
by default. This avoids unexpectedly changing historic behaviour.
src/cmd/ksh93/edit/emacs.c: ed_emacsread(),
src/cmd/ksh93/edit/vi.c: getline():
- Only set the flag for special backslash handling if
SH_NOBACKSLCTRL is off.
src/cmd/ksh93/sh.1,
src/cmd/ksh93/data/builtins.c:
- Document the new option (as "backslashctrl", on by default).
Other minor tweaks:
src/cmd/ksh93/edit/edit.c:
- ed_setup(): Add fallback #error if no tput method is set. This
should never be triggered; it's to catch future editing mistakes.
- escape(): cntl('\t') is nonsense as '\t' is already a control
character, so change this to just '\t'.
- xcommands(): Let's enable the ^X^D command for debugging
information on non-release builds.
src/cmd/ksh93/features/cmds:
- The tput feature tests assumed a functioning terminal in $TERM.
However, for all we know we might be compiling with no tty and
TERM=dumb. The tput commands won't work then. So set TERM=ansi
to use a standard default.
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.
With this patch, the Korn shell can now guarantee that calls to
sleep on systems using the select or poll method always result in
the system clock advancing by that much time, assuming no
interruptions. This compensates for deficiencies in certain
systems, including SCO UnixWare.
Discussion: https://github.com/ksh93/ksh/pull/174
src/lib/libast/tm/tvsleep.c:
- Ensure that at least the time requested to sleep has elapsed
for the select and poll methods.
- Simplify the logic of calculating the time remaining to
sleep and handle the case of an argument of greater than
10e9 nanoseconds being passed to tvsleep.
src/cmd/ksh93/bltins/sleep.c:
- Eliminate the check for EINTR to handle other cases wherein
we have not slept enough.
src/cmd/ksh93/tests/variables.sh:
- Improve the diagnostic message when the sleep test fails.
- Revise the SECONDS function test to expect that we always
sleep for at least the time specified.
src/cmd/ksh93/tests/functions.h:
- Redirect ps stderr to /dev/null. UnixWare ps prints an error
message about not being able to find the controlling terminal
when shtests output is piped, but we are only using ps to find
the PID.
This makes ksh 93u+m build on the following system:
$ uname -a
QNX qnx 6.5.0 2010/07/09-14:44:03EDT x86pc x86
Thanks to polarhome.com for providing the QNX shell account.
There are a number of regressions left to work out:
arrays.sh[636]: copying a large array fails
bracket.sh[129]: /tmp/ksh93.shtests.1753215026.6923/bracket.C/original should be older than /tmp/ksh93.shtests.1753215026.6923/bracket.C/newer
bracket.sh[132]: /tmp/ksh93.shtests.1753215026.6923/bracket.C/newer should be newer than /tmp/ksh93.shtests.1753215026.6923/bracket.C/original
builtins.sh[683]: real_t1 not found after parent directory renamed in subshell
functions.sh[1023]: cannot handle comsub depth > 256 in function
io.sh[252]: <# not working for pipes
io.sh[337]: read -n3 from pipe not working
io.sh[346]: read -n3 from fifo failed -- expected 'a', got 'abc'
io.sh[349]: read -n1 from fifo failed -- expected 'b', got 'd'
io.sh[379]: should have timed out
io.sh[380]: line1 should be 'prompt1: '
io.sh[381]: line2 should be line2
io.sh[382]: line3 should be 'prompt2: '
io.sh[406]: LC_ALL=C read -n2 from pipe 'a bcd' failed -- expected 'a bcd', got 'ab cd'
io.sh[406]: LC_ALL=C.UTF-8 read -n2 from pipe 'a bcd' failed -- expected 'a bcd', got 'ab cd'
jobs.sh[86]: warning: skipping subshell job control test due to non-compliant 'ps'
pty.sh[105]: POSIX sh 026(C): line 120: expected "(Stopped|Suspended)", got EOF
pty.sh[128]: POSIX sh 028(C): line 143: expected "(Stopped|Suspended) \(SIGTTIN\)", got EOF
pty.sh[151]: POSIX sh 029(C): line 166: expected "(Stopped|Suspended) \(SIGTTOU\)", got EOF
signal.sh[310]: kill -TERM $$ failed, required termination by signal 'EXIT'
signal.sh[310]: kill -VTALRM $$ failed, required termination by signal 'EXIT'
signal.sh[310]: kill -PIPE $$ failed, required termination by signal 'EXIT'
(The io.sh failures mean libast sfpkrd() is not working.)
src/lib/libast/obsolete/spawn.c:
- Removed. Didn't compile due to wrong number of arguments to
spawnve(2), but is obsolete and unused.
src/lib/libast/comp/localeconv.c:
- The initialisation of two static 'struct lconv' variables was
done in a way that depended on OS headers declaring the struct
members in a certain order. This holds on most systems, but not
on QNX, and POSIX does not actually specify the order at all:
https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/locale.h.html
So each member must be initialised by name. But C89 does not
support initialising struct members by name, so we have to do it
using an initialiser function that simply assigns the values.
src/lib/libast/comp/spawnveg.c:
- Fix for systems without either P_DETACH or _P_DETACH.
src/lib/libast/features/vmalloc,
src/lib/libast/vmalloc/vmmopen.c,
src/lib/libast/Mamfile:
- Add test for sys/shm.h header. If it doesn't exist, as it doesn't
on QNX, use the stub vmmapopen() as the real one won't compile.
(Mamfile: Add dependency on FEATURE/vmalloc to vmmopen.c.)
src/lib/libast/vmalloc/malloc.c:
- Remove superfluous externs that are already provided by either
AST or system headers. The 'void cfree' extern caused a build
failure on QNX because cfree() is of type int on QNX.
src/lib/libast/comp/conf.tab:
- Remove check for _map_spawnve; src/lib/libast/RELEASE says it was
removed.
That OpenSUSE patch introduced a bug: file descriptors other than 1
that were globally redirected using 'exec' or 'redirect' no longer
survived a ${ shared-state; } command substitution.
Related: https://github.com/ksh93/ksh/issues/128
src/cmd/ksh93/sh/io.c:
- Add check for shp->subshare to the OpenSUSE patch.
src/cmd/ksh93/tests/io.sh:
- Add test.
.github/workflows/ci.yml:
- Go back to wrapping the regression tests in script(1).
src/cmd/ksh93/data/builtins.c:
- Never mind about the stty builtin.
src/cmd/ksh93/tests/pty.sh:
- Refuse to run if there isn't a functioning tty.
- Make sure stty(1) works on /dev/tty by redirecting stdin.
So, the pty regression tests on the Linux GitHub runner all failed.
Let's test an assumption: the reason is that we need the stty
builtin to properly set the pty state, because the OS-provided stty
command does not work if there is no real tty.
src/cmd/ksh93/data/builtins.c:
- Compile in the stty built-in. This adds about 20k to the binary
for a command that most users rarely need and even more rarely
need to be built in, so only compile it in on non-release builds.
src/cmd/ksh93/tests/pty.sh:
- Skip the tests if we cannot either use the stty builtin or change
the state of the real terminal to be compatible with the tests.
The Mac runner is still broken: intermittent pipe- and
signal-related regressions that do not occur on any real Mac.
https://github.com/ksh93/ksh/runs/1892358749
.github/workflows/ci.yml:
- Remove the macOS runner.
src/cmd/ksh93/tests/pty.sh:
- Do not skip pty tests if there is no tty. (On FreeBSD with no
tty, the tty builtin would need to be enabled in builtins.c.)
src/cmd/ksh93/tests/bracket.sh:
- Don't be noisy when skipping unavailable locales.
It is desirable to be able to run the tests on a system without
a functioning tty. Since this distribution comes with its own
pseudo-tty facility, pty, it should be possible to run the few
tests that require a tty on the pseudo-tty instead. I've verified
that they fail as expected on older ksh93.
Discussion: https://github.com/ksh93/ksh/pull/171
src/cmd/ksh93/tests/basic.sh,
src/cmd/ksh93/tests/bracket.sh:
- Remove tests that require a tty.
src/cmd/ksh93/tests/pty.sh:
- Put them here, adapted to work as interactive pty scripts.
src/cmd/ksh93/tests/shtests:
- No longer refuse to run if there is no functioning tty.
.github/workflows/ci.yml:
- Since the tests no longer require a tty, no longer use script(1)
to get a pseudo-tty. Let's see if this works...
- Re-enable the Mac runner (re: 14632361). Maybe it has improved.
src/cmd/ksh93/tests/leaks.sh: Read vsz from UnixWare's ps
UnixWare's ps reports an accurate virtual size, so collecting that is
preferable to trying to parse the real resident size.
The GitHub runners apparently provide a non-working /dev/tty. To
avoid failures and confusion, shtests shold refuse to run the tests
and tell people to use script(1) to simulate a tty. On Linux, it
goes like this:
script -q -e -c 'bin/shtests --your-options-here'
On macOS and FreeBSD, the invocation is:
script -q /dev/null bin/shtests --your-options-here
The NetBSD and OpenBSD variants of script(1) need different
invocations again. They also don't pass down the command's exit
status, so would need a workaround for that.
It would be nice if we could use pty for this as this comes with
the distribution, so would work the same on every OS, but it seems
to be broken for this use case.
src/cmd/ksh93/tests/shtest:
- Use 'test -t 1' with stdout (fd 1) redirected to /dev/tty to
ensure the tty is actually on a terminal.
src/cmd/ksh93/tests/basic.sh:
- Remove superflous check for tty. All tests run through shtests.
Resolves: https://github.com/ksh93/ksh/pull/171
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.
src/cmd/ksh93/tests/bracket.sh:
- Read the list of installed locales to ensure the locale to be tested
actually exists on the system under test.
- Produce a warning diagnostic for skipped locales.
- Additionally test the en_US.ISO8859-1 and en_US.UTF-8 locales.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
src/cmd/ksh93/features/math.sh:
- Specify ast_float.h within iffehdrs instead of math.h, so that iffe
will pick up on macro substitutions within libast. This should make
any future efforts to remedy floating point behavior easier as well.
- Always include ast_float.h within the generated math header file,
not just on IA64 platforms.
src/cmd/ksh93/tests/arith.sh:
- Test pow(1.0,-Inf) and pow(1.0,NaN) for IEEE compliance as well.
- Test the exponentiation operator (**) in addition, as streval.c,
which processes the same, calls pow() separately.
src/lib/libast/features/float:
- Test the IEEE compliance of the underlying math library's pow()
function and substitute macros producing compliant behavior if
necessary.
If I haven't missed anything, this should make the non-interactive
aspects of job control in scripts work as expected, except for the
"<command unknown>" issue in the output of 'bg', 'fg' and 'jobs'
(which is not such a high priority as those commands are really
designed for interactive use).
Plus, I believe I now finally understand what these three are for:
* The job.jobcontrol variable is set to nonzero by job_init() in
jobs.c if, and only if, the shell is interactive *and* managed to
get control of the terminal. Therefore, any changing of terminal
settings (tcsetpgrp(3), tty_set()) should only be done if
job.jobcontrol is nonzero. This commit changes several checks for
sh_isoption(SH_INTERACTIVE) to checks for job.jobcontrol for
better consistency with this.
* The state flag, sh_isstate(SH_MONITOR), determines whether the
bits of job control that are relevant for both scripts and
interactive shells are active, which is mostly making sure that a
background job gets its own process group (setpgid(3)).
* The shell option, sh_isoption(SH_MONITOR), is just that. When the
user turns it on or off, the state flag is synched with it. It
should usually not be directly checked for, as the state may be
temporarily turned off without turning off the option.
Prior discussion:
https://www.mail-archive.com/austin-group-l@opengroup.org/msg06456.html
src/cmd/ksh93/bltins/typeset.c, src/cmd/ksh93/sh/args.c:
- Move synching the SH_MONITOR state flag with the SH_MONITOR
shell option from b_set() (the 'set' builtin) to sh_applyopts()
which is indirectly called from b_set() and is also used when
parsing the shell invocation command line. This ensures -m is
properly enabled in both scenarios.
src/cmd/ksh93/sh/jobs.c:
- job_init(): Do not refuse to initialise job control on
non-interactive shells. Instead, skip everything that should only
be done on interactive shells (i.e., everything to do with the
terminal). This function is now even more of a mess than it was
before, so refactoring may be desirabe at some point.
- job_close(), job_set(), job_reset(), job_wait(): Do not reset the
terminal process group (tcsetpgrp()) if job.jobcontrol isn't on.
src/cmd/ksh93/sh/xec.c:
- sh_exec(): TFORK: For SIGINT handling, check the SH_MONITOR
state flag, not the shell option.
- sh_exec(): TFORK: Do not turn off the SH_MONITOR state flag in
forked children. The non-interactive part of job control should
stay active. Instead, turn off the SH_INTERACTIVE state flag so
we don't get interactive shell behaviour (i.e. job control noise
on the terminal) in forked subshells.
- _sh_fork(), sh_ntfork(): Do not reset the terminal process group
(tcsetpgrp()) if job.jobcontrol isn't on. Do not turn off the
SH_MONITOR state flag in forked children.
src/cmd/ksh93/sh/subshell.c: sh_subfork():
- Do not turn off the monitor option and state in forked subshells.
The non-interactive part of job control should stay active.
src/cmd/ksh93/bltins/misc.c: b_bg():
- Check isstate(SH_MONITOR) instead of sh_isoption(SH_MONITOR) &&
job.jobcontrol before throwing a 'no job control' error.
This fixes a minor bug: fg, bg and disown could quietly fail.
src/cmd/ksh93/tests/jobs.sh:
- Add tests for 'fg' with job control IDs (%%, %1) in scripts.
- Add test checking that a background job launched from a subsell
with job control enabled correctly becomes the leader of its own
process group.
Makes progress on: https://github.com/ksh93/ksh/issues/119
Another longstanding whopper of a bug in basic ksh93 functionality:
run a ${ shared-state; } command substitution twice and job control
promptly loses track of all your running jobs. New jobs are tracked
again until you run another two shared-state command substitutions.
This is in at least 93t+, 93u-, 93u+, 93v- and ksh2020.
$ sleep 300 &
[1] 56883
$ jobs # OK
[1] + Running sleep 300 &
$ v=${ echo hi1; }
$ jobs # OK
[1] + Running sleep 300 &
$ v=${ echo hi2; }
$ jobs # Nothing!
$ fg
ksh: fg: no such job
src/cmd/ksh93/sh/subshell.c: sh_subshell():
- The current environment number shp->curenv (a.k.a. sh.curenv) was
not being restored if the virtual subshell we're leaving is of
the shared-state command substitution variety as it was wrongly
considered to be part of the environment that didn't need
restoring. This caused it to be out of sync with shp->jobenv
(a.k.a. sh.jobenv) which did get restored from savedcurenv.
Restore both from savedcurenv at the same time for any subshell.
(How these numbers are used exactly remains to be discovered.)
src/cmd/ksh93/tests/jobs.sh:
- Added, with a test for this bug to start it off. There is no
other test script where job control fits, and a lot more related
fixes are anticipated: https://github.com/ksh93/ksh/issues/119
src/cmd/ksh93/bltins/enum.c:
- enum_type[]: Fix typos; minor edit for style.
- enum_type[], enuminfo(): Make the list of supported values
comma-separated, instead of using a comma at the start of each.
src/cmd/ksh93/sh/nvtype.c:
- sh_opttype[]: Fix typos.
It was easier than expected to fix this one. The many regression
test failures caused by disabling it were all due to one bug:
'typeset -p' output broke when building without this option.
src/cmd/ksh93/sh/nvtree.c: nv_attribute():
- In this function to print the attributes of a name-value pair,
move four lines of code out of #if SHOPT_FIXEDARRAY...#endif that
were inadvertently moved into the #if block in ksh93 2012-05-18.
See the changes to nvtree.c in this multishell repo commit:
https://github.com/multishell/ksh93/commit/aabab56a
src/cmd/ksh93/data/builtins.c:
- Update/rewrite 'typeset -a' documentation.
- Make it adapt to SHOPT_FIXEDARRAY.
- Fix a few typos.
src/cmd/ksh93/tests/arrays2.sh:
- Only one regression test needs a SHOPT_FIXEDARRAY check.
.github/workflows/ci.yml:
- Disable SHOPT_FIXEDARRAY when regression-testing without SHOPTs.
- Enable xtrace, add ':' commands for traced comments. This should
make the CI runner output logs a little more readable.
Many compile-time options were broken so that they could not be
turned off without causing compile errors and/or regression test
failures. This commit now allows the following to be disabled:
SHOPT_2DMATCH # two dimensional ${.sh.match} for ${var//pat/str}
SHOPT_BGX # one SIGCHLD trap per completed job
SHOPT_BRACEPAT # C-shell {...,...} expansions (, required)
SHOPT_ESH # emacs/gmacs edit mode
SHOPT_HISTEXPAND # csh-style history file expansions
SHOPT_MULTIBYTE # multibyte character handling
SHOPT_NAMESPACE # allow namespaces
SHOPT_STATS # add .sh.stats variable
SHOPT_VSH # vi edit mode
The following still break ksh when disabled:
SHOPT_FIXEDARRAY # fixed dimension indexed array
SHOPT_RAWONLY # make viraw the only vi mode
SHOPT_TYPEDEF # enable typeset type definitions
Compiling without SHOPT_RAWONLY just gives four regression test
failures in pty.sh, but turning off SHOPT_FIXEDARRAY and
SHOPT_TYPEDEF causes compilation to fail. I've managed to tweak the
code to make it compile without those two options, but then dozens
of regression test failures occur, often in things nothing directly
to do with those options. It looks like the separation between the
code for these options and the rest was never properly maintained.
Making it possible to disable SHOPT_FIXEDARRAY and SHOPT_TYPEDEF
may involve major refactoring and testing and may not be worth it.
This commit has far too many tweaks to list. Notables fixes are:
src/cmd/ksh93/data/builtins.c,
src/cmd/ksh93/data/options.c:
- Do not compile in the shell options and documentation for
disabled features (braceexpand, emacs/gmacs, vi/viraw), so the
shell is not left with no-op options and inaccurate self-doc.
src/cmd/ksh93/data/lexstates.c:
- Comment the state tables to associte them with their IDs.
- In the ST_MACRO table (sh_lexstate9[]), do not make the S_BRACE
state for position 123 (ASCII for '{') conditional upon
SHOPT_BRACEPAT (brace expansion), otherwise disabling this causes
glob patterns of the form {3}(x) (matching 3 x'es) to stop
working as well -- and that is ksh globbing, not brace expansion.
src/cmd/ksh93/edit/edit.c: ed_read():
- Fixed a bug: SIGWINCH was not handled by the gmacs edit mode.
src/cmd/ksh93/sh/name.c: nv_putval():
- The -L/-R left/right adjustment options to typeset do not count
zero-width characters. This is the behaviour with SHOPT_MULTIBYTE
enabled, regardless of locale. Of course, what a zero-width
character is depends on the locale, but control characters are
always considered zero-width. So, to avoid a regression, add some
fallback code for non-SHOPT_MULTIBYTE builds that skips ASCII
control characters (as per iscntrl(3)) so they are still
considered to have zero width.
src/cmd/ksh93/tests/shtests:
- Export the SHOPT_* macros from SHOPT.sh to the tests as
environment variables, so the tests can check for them and decide
whether or how to run tests based on the compile-time options
that the tested binary was presumably compiled with.
- Do not run the C.UTF-8 tests if SHOPT_MULTIBYTE is not enabled.
src/cmd/ksh93/tests/*.sh:
- Add a bunch of checks for SHOPT_* env vars. Since most should
have a value 0 (off) or 1 (on), the form ((SHOPT_FOO)) is a
convenient way to use them as arithmetic booleans.
.github/workflows/ci.yml:
- Make GitHub do more testing: run two locale tests (Dutch and
Japanese UTF-8 locales), then disable all the SHOPTs that we can
currently disable, recompile ksh, and run the tests again.
src/lib/libast/misc/optget.c:
- Add screen* (which includes tmux) and dtterm* (CDE terminal) to
the glob pattern deciding whether to use ANSI boldface sequences.
- Don't bother parsing the env var if stderr is not on a terminal.
src/cmd/ksh93/sh.1:
- Extend self-documentation documentation; document how optget(3)
uses the ERROR_OPTIONS env var to control boldface output.
- Tweaks and minor edits.
This fixes a bug in libast optget()'s use of emphasis in the
display of --man(uals) via standard error on a terminal.
Symptom:
$ printf --man 2>&1 | more
(ok; emphasis disabled, no escape codes shown)
$ printf --man
(ok; emphasis correctly displayed)
$ printf --man 2>&1 | more
(whoops; emphasis not disabled; escape codes garble 'more' output)
The problem was that the state.emphasis variable was not
initialised and, when set to one, was never reset again
(except through the use of the --api, --html or --nroff option).
The source code also reveals an undocumented feature: if the
environment variable $ERROR_OPTIONS contains 'noemphasi', emphasis
is forced off, else if it contains 'emphasi', it's forced on.
Other characters (such as the final 's' of emphasis) are ignored.
This was also broken (forcing off didn't work) and is now fixed.
src/lib/libast/misc/optget.c:
- Do not assume that enabling emphasis is forever; re-initialise
the state on every relevant getopts invocation.
- Increase the number of terminals on which emphasis is displayed
using ANSI escape codes. (This is a hack and we should ask the OS
for the correct codes, but never mind -- ANSI is now universal.)
