ksh has a little-known field splitting feature that conflicts with
POSIX: if a single-byte whitespace character (cf. isspace(3)) is
repated in $IFS, then field splitting is done as if that character
wasn't a whitespace character. An exmaple with the tab character:
$ (IFS=$'\t'; val=$'\tone\t\ttwo\t'; set -- $val; echo $#)
2
$ (IFS=$'\t\t'; val=$'\tone\t\ttwo\t'; set -- $val; echo $#)
4
The latter being the same as, for example
$ (IFS=':'; val='1️⃣2️⃣'; set -- $val; echo $#)
4
However, this is incompatible with the POSIX spec and with every
other shell except zsh, in which repeating a character in IFS does
not have any effect. So the POSIX mode must disable this.
src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/init.c:
- Add sh_invalidate_ifs() function that invalidates the IFS state
table by setting the ifsnp discipline struct member to NULL,
which will cause the next get_ifs() call to regenerate it.
- get_ifs(): Treat a repeated char as S_DELIM even if whitespace,
unless --posix is on.
src/cmd/ksh93/sh/args.c:
- sh_argopts(): Call sh_invalidate_ifs() when enabling or disabling
the POSIX option. This is needed to make the change in field
splitting behaviour take immediate effect instead of taking
effect at the next assignment to IFS.
b_enum() contains a check that exactly one argument is given:
237: if (error_info.errors || !*argv || *(argv + 1))
But the subsequent argument handling loop will happily deal with
multiple arguments:
246: while(cp = *argv++)
Every other declaration command supports multiple arguments and I
see no reason why enum shouldn't. Simply removing the '*(argv + 1)'
check allows 'enum' to create more than one type per invocation.
src/cmd/ksh93/bltins/enum.c:
- b_enum(): Remove check for >1 args as described above.
- Update documentation to describe the behaviour of enumeration
types in arithmetic expressions and to add an example: a bool
type with two enumeration values 'false' (0) and 'true' (1).
That type is predefined in ksh 93v- and 2020. We're not going
to do that in 93u+m but it's good to document the possibility.
src/cmd/ksh93/sh.1:
- Make changes parallel to the enum.c self-doc update.
Changes:
- Fixed two xtrace test failures introduced in commit cfc8744c.
- The definition of _use_ntfork_tcpgrp in xec.c is now dependent on
SHOPT_SPAWN being defined (re: 8e9ed5be).
- Removed many unnecessary newlines and fixed various typos.
This commit fixes an infinite loop introduced in commit 0863a8eb
that caused ksh to enter an infinite loop if posix_spawn failed
to start a new process after setting the terminal process group.
Reproducer (warning: it will cause ksh to crash Wayland sessions
and drives up CPU usage by a ton):
$ /tmp/this/file/does/not/exist
/usr/bin/ksh: /tmp/this/file/does/not/exist: not found
$ <Press enter>
(ksh now prints $PS1 in a loop until killed with SIGKILL)
The first bug fixed is the infinite loop that occurs when
posix_spawn fails to execute a command. This was fixed by setting
the terminal process group to the main interactive shell.
The second bug fixed is related to the signal handling of the
SIGTTIN, SIGTTOU and SIGTSTP signals. In sh_ntfork() these signals
are set to their default signal handlers (SIG_DFL) before running
a command. The signal handlers were only restored to SIG_IGN
(ignore signal) when sh_ntfork() successfully ran a command.
This could cause a SIGTTOU lockup under strace when a command
failed to execute in an interactive shell, while also being one
cause of the infinite loop.
src/cmd/ksh93/sh/xec.c: sh_ntfork():
- Restore the terminal process group if posix_spawn failed to
launch a new process. This is necessary because posix_spawn will
set the terminal process group before it attempts to run a
command and doesn't restore it on failure.
This change turns off O_NONBLOCK for stdin if a previously ran
program left it on so that interactive programs that expect it
to be off work properly.
src/cmd/ksh93/sh/io.c: slowread():
- Turn off O_NONBLOCK for stdin if it is on.
Fixes: https://github.com/ksh93/ksh/issues/469
The previous fix for the += operator introduced a use-after-free
bug that could result in a variable pointing to random garbage:
$ foo=bar
$ foo+=_foo true
$ typeset -p foo
foo=V V
The use after free issue occurs because when nv_clone creates a
copy of $foo in the true command's invocation-local scope, it does
not duplicate the string $foo points to. As a result, the $foo
variable in the parent scope points to the same string as $foo in
the invocation-local scope, which causes the use after free bug
when cloned $foo variable is freed from memory.
src/cmd/ksh93/sh/nvdisc.c:
- To fix the use after free bug, allow nv_clone to duplicate the
string with memdup or strdup when no flags are passed.
src/cmd/ksh93/tests/variables.sh:
- Add a regression test for using the += operator with regular
commands.
src/cmd/ksh93/tests/leaks.sh:
- Add a regression test to ensure the bugfix doesn't introduce any
memory leaks.
Reproducer (symptoms on at least macOS and FreeBSD):
$ mkfifo f
$ echo foo > f
(press Ctrl+Z)
^Zksh: f: cannot create [Interrupted system call]
Abort
The shell either aborts (dev builds) or crashes with 'Illegal
instruction' (release builds). This is consistent with
UNREACHABLE() being reached.
Backtrace:
0 libsystem_kernel.dylib __kill + 10
1 ksh sh_done + 836 (fault.c:678)
2 ksh sh_fault + 1324
3 libsystem_platform.dylib _sigtramp + 29
4 dyld ImageLoaderMachOCompressed::resolve(ImageLoader::LinkContext const&, char const*, unsigned ch
5 libsystem_c.dylib abort + 127
6 ksh sh_redirect + 3576 (io.c:1356)
7 ksh sh_exec + 7231 (xec.c:1308)
8 ksh exfile + 3247 (main.c:607)
9 ksh sh_main + 3551 (main.c:368)
10 ksh main + 38 (pmain.c:45)
11 libdyld.dylib start + 1
This means that UNREACHABLE() is actually reached here:
ksh/src/cmd/ksh93/sh/io.c
1351: if((fd=sh_open(tname?tname:fname,o_mode,RW_ALL)) <0)
1352: {
1353: errormsg(SH_DICT,ERROR_system(1),((o_mode&O_CREAT)?e_create:e_open),fname);
1354: UNREACHABLE();
1355: }
The cause is that, in the following section of code in sh_fault():
ksh/src/cmd/ksh93/sh/fault.c
183: #ifdef SIGTSTP
184: if(sig==SIGTSTP)
185: {
186: sh.trapnote |= SH_SIGTSTP;
187: if(pp->mode==SH_JMPCMD && sh_isstate(SH_STOPOK))
188: {
189: sigrelease(sig);
190: sh_exit(SH_EXITSIG);
191: return;
192: }
193: }
194: #endif /* SIGTSTP */
...sh_exit() is not getting called and the function will not return
because the SH_STOPOK bit is not set while the shell is blocked
waiting to write to a FIFO.
Even if sh_exit() did get called, that would not fix it, because
that function also checks for the SH_STOPOK bit and returns without
doing a longjmp if the signal is SIGTSTP and the SH_STOPOK bit is
not set. That is direct the reason why UNREACHABLE() was raeched:
errormsg() does call sh_exit() but sh_exit() then does not longjmp.
src/cmd/ksh93/sh/fault.c: sh_fault():
- To avoid the crash, we simply need to return from sh_fault() if
SH_STOPOK is off, so that the code path does not continue, no
error message is given on Ctrl+Z, UNREACHABLE() is not reached,
and the shell resumes waiting on trying to write to the FIFO.
The sh.trapnote flag should not be set if we're not going to
process the signal. This makes ksh behave like all other shells.
Resolves: https://github.com/ksh93/ksh/issues/464
Reproducer:
$ ksh -c 'unset PWD; (cd /); :'
Memory fault
The shell crashes because b_cd() is testing the value of the PWD
variable without checking if there is one.
src/cmd/ksh93/sh/path.c: path_pwd():
- Never return an unfreeable pointer to e_dot; always return a
freeable pointer. This fixes another corner-case crashing bug.
- Make sure the PWD variable gets assigned a value if it doesn't
have one, even if it's the "." fallback. However, if the PWD is
inaccessible but we did inherit a $PWD value that starts with a
/, then use the existing $PWD value as this will help the shell
fail gracefully.
src/cmd/ksh93/bltins/cd_pwd.c:
- b_cd(): When checking if the PWD is valid, use the sh.pwd copy
instead of the PWD variable. This fixes the crash above.
- b_cd(): Since path_pwd() now always returns a freeable value,
free sh.pwd unconditionally before setting the new value.
- b_pwd(): Not only check that path_pwd() returns a value starting
with a slash, but also verify it with test_inode() and error out
if it's wrong. This makes the 'pwd' command useful for checking
that the PWD is currently accessible.
src/cmd/ksh93/data/msg.c:
- Change e_pwd error message for accuracy and clarity.
This backports two minor additions to the 'read' built-in from ksh
93v-: '-a' is now the same as '-A' and '-u p' is the same as '-p'.
This is for compatibility with some 93v- or ksh2020 scripts.
Note that their change to the '-p' option to support both prompts
and reading from the coprocess was *not* backported because we
found it to be broken and unfixable. Discussoin at:
https://github.com/ksh93/ksh/issues/463
src/cmd/ksh93/bltins/read.c: b_read():
- Backport as described above.
- Rename the misleadingly named 'name' variable to 'prompt'.
It points to the prompt string, not to a variable name.
src/cmd/ksh93/data/builtins.c: sh_optpwd[]:
- Add -a as an alterative to -A. All that is needed is adding '|a'
and optget(3) will automatically convert it to 'A'.
- Change -u from a '#' (numeric) to ':' option to support 'p'. Note
that b_read() now needs a corresponding strtol() to convert file
descriptor strings to numbers where applicable.
- Tweaks.
src/cmd/ksh93/sh.1:
- Update accordingly.
- Tidy up the unreadable mess that was the 'read' documentation.
The options are now shown in a list.
From README:
FILESCAN on Experimental option that allows fast reading of files
using while < file;do ...; done and allowing fields in
each line to be accessed as positional parameters.
As SHOPT_FILESCAN has been enabled by default since ksh 93l
2001-06-01, the filescan loop is now documented in the manual page
and the compile-time option is no longer considered experimental.
We must disable this at runtime if --posix is active because it
breaks a portable use case: POSIXly, 'while <file; do stuff; done'
repeatedly excutes 'stuff' while 'file' can successfully be opened
for reading, without actually reading from 'file'.
This also backports a bugfix from the 93v- beta. Reproducer:
$ echo 'one two three' >foo
$ while <foo; do printf '[%s] ' "$@"; echo; done
[one two three]
Expected output:
[one] [two] [three]
The bug is that "$@" acts like "$*", joining all the positional
parameters into one word though it should be generating one word
for each.
src/cmd/ksh93/sh/macro.c: varsub():
- Backport fix for the bug described above. I do not understand the
opaque macro.c code well enough yet to usefully describe the fix.
src/cmd/ksh93/sh/xec.c: sh_exec():
- Improved sanity check for filescan loop: do not recognise it if
the simple command includes variable assignments, more than one
redirection, or an output or append redirection.
- Disable filescan loops if --posix is active.
- Another 93v- fix: handle interrupts (errno==EINTR) when closing
the input file.
In UTF-8 locales, ksh breaks when a KEYBD trap is active, even a
dummy no-op one like 'trap : KEYBD'. Entering multi-byte characters
fails (the input is interrupted and a new prompt is displayed) and
pasting content with multi-byte characters produces corrupted
results.
The cause is that the KEYBD trap code is not multibyte-ready.
Unfortunately nobody yet understands the edit.c code well enough
to implement a proper fix. Pending that, this commit implements
a workaround that at least avoids breaking the shell.
src/cmd/ksh93/edit/edit.c: ed_getchar():
- When a multi-byte locale is active, do not trigger the the KEYBD
trap except for ASCII characters (1-127).
Resolves: https://github.com/ksh93/ksh/issues/307
The -f/--noglob shell option is documented simply as: "Disables
pathname expansion." But after 'set -f' on an interactive shell,
command completion and file name completion also stop working. This
is because they internally use the pathname expansion mechanism.
But it is not documented anywhere that 'set -f' disables
completion; it's just a side effect of an implementation detail.
Though ksh has always acted like this, I think it should change
because it's not useful or expected behaviour. Other shells like
bash, yash or zsh don't act like this.
src/cmd/ksh93/sh/expand.c,
src/cmd/ksh93/sh/macro.c:
- Allow the SH_COMPLETE (command completion) or SH_FCOMPLETE
(file name completion) state bit to override SH_NOGLOB in
path_generate() and in sh_macexpand().
Reproducer:
exec 9>&1
( { exec 9>&1; } 9>&- )
echo "test" >&9 # => 9: cannot open [Bad file descriptor]
The 9>&- incorrectly persists beyond the { } block that it
was attached to *and* beyond the ( ) subshell. This is yet another
bug with non-forking subshells; forking it with something like
'ulimit -t unlimited' works around the bug.
In over a year we have not been able to find a real fix, but I came
up with a workaround that forks a virtual subshell whenever it
executes a code block with a >&- or <&- redirection attached. That
use case is obscure enough that it should not cause any performance
regression except in very rare corner cases.
src/cmd/ksh93/sh/xec.c: sh_exec(): TSETIO:
- This is where redirections attached to code blocks are handled.
Check for a >&- or <&- redirection using bit flaggery from
shnodes.h and fork if we're executing such in a virtual subshell.
Resolves: https://github.com/ksh93/ksh/issues/161
Thanks to @ko1nksm for the bug report.
This commit backports all of the relevant .sh.match bugfixes from
ksh93v-. Most of the .sh.match rewrite is from versions 2012-08-24
and 2012-10-04, with patches from later releases of 93v- and
ksh2020 also applied. Note that there are still some remaining bugs
in .sh.match, although now the total count of .sh.match bugs should
be less that before.
These are the relevant changes in the ksh93v- changelog that were
backported:
12-08-07 .sh.match no longer gets set for patterns in PS4 during
set -x.
12-08-10 Rewrote .sh.match expansions fixing several bugs and
improving performance.
12-08-22 .sh.match now handles subpatterns that had no matches with
${var//pattern} correctly.
12-08-21 A bug in setting .sh.match after ${var//pattern/string}
when string is empty has been fixed.
12-08-21 A bug in setting .sh.match after [[ string == pattern ]]
has been fixed.
12-08-31 A bug that could cause a core dump after
typeset -m var=.sh.match has been fixed.
12-09-10 Fixed a bug in typeset -m the .sh.match is being renamed.
12-09-07 Fixed a bug in .sh.match code that coud cause the shell
to quitely
13-02-21 The 12-01-16 bug fix prevented .sh.match from being used
in the replacement string. The previous code was restored
and a different fix which prevented .sh.match from being
computed for nested replacement has been used instead.
13-05-28 Fixed two bug for typeset -c and typeset -m for variable
.sh.match.
Changes:
- The SHOPT_2DMATCH option has been removed. This was already the
default behavior previously, and now it's documented in the man
page.
- init.c: Backported the sh_setmatch() rewrite from 93v- 2012-08-24
and 2012-10-04.
- Backported the libast 93v- strngrpmatch() function, as the
.sh.match rewrite requires this API.
- Backported the sh_match regression tests from ksh93v-, with many
other sh_match tests backported from ksh2020. Much of the sh_match
script is based on code from Roland Mainz:
https://marc.info/?l=ast-developers&m=134606574109162&w=2https://marc.info/?l=ast-developers&m=134490505607093
- tests/{substring,treemove}.sh: Backported other relevant .sh.match
fixes, with tests added to the substring and treemove test scripts.
- tests/types.sh: One of the (now reverted) memory leak bugfixes
introduced a CI test failure in this script, so for that test the
error message has been improved.
- string/strmatch.c: The original ksh93v- code for the strngrpmatch()
changes introduced a crash that could occur because strlen would
be used on a null pointer. This has been fixed by avoiding strlen
if the string is null.
One nice side effect of these changes is a considerable performance
improvement in the shbench[1] gsub benchmark (results from 20
iterations with CCFLAGS=-Os):
--------------------------------------------------
name /tmp/ksh-current /tmp/ksh-matchfixes
--------------------------------------------------
gsub.ksh 0.883 [0.822-0.959] 0.457 [0.442-0.505]
--------------------------------------------------
Despite all of the many fixes and improvements in the backported
93v- .sh.match code, there are a few remaining bugs:
- .sh.match is printed with a default [0] subscript (see also
https://github.com/ksh93/ksh/issues/308#issuecomment-1025016088):
$ arch/*/bin/ksh -c 'echo ${!.sh.match}'
.sh.match[0]
This bug appears to have been introduced by the changes from
ksh93v- 2012-08-24.
- The wrong variable name is given for 'parameter not set' errors
(from https://marc.info/?l=ast-developers&m=134489094602596):
$ arch/*/bin/ksh -u
$ x=1234
$ true "${x//~(X)([012])|([345])/}"
$ compound co
$ typeset -m co.array=.sh.match
$ printf "%q\n" "${co.array[2][0]}"
arch/linux.i386-64/bin/ksh: co.array[2][(null)]: parameter not set
- .sh.match leaks out of subshells. Further information and a
reproducer can be found here:
https://marc.info/?l=ast-developers&m=136292897330187
[1]: https://github.com/ksh-community/shbench
When executing a script without a hashbang path like #!/bin/ksh,
ksh forks itself, longjmps back to sh_main(), and then (among other
things) calling sh_reinit() which is the function that tries to
reinitialise as much of the shell as it can. This is its way of
ensuring the child script is run in ksh and not some other shell.
However, this appraoch is incredibly buggy. Among other things,
changes in built-in commands and custom type definitions survived
the reinitialisation, "exporting" variables didn't work properly,
and the hash table and ${.sh.stats} weren't reset. As a result,
depending on what the invoking script did, the invoked script could
easily fail or malfunction.
It is not actually possible to reinitialise the shell correctly,
because some of the shell state is in locally scoped static
variables that cannot simply be reinitialised. There are probably
huge memory leaks with this approach as well. At some point, all
this is going to need a total redesign. Clearly, the only reliable
way involves execve(2) and a start from scratch.
For now though, this seems to fix the known bugs at least. I'm sure
there are more to be discovered.
This commit makes another change: instead of the -h/trackall option
(which has been a no-op for decades), the posix option is now
inherited by the child script. Since there is no hashbang path from
which to decide whether the shell should run in POSIX mode nor not,
the best guess is probably the invoking script's setting.
src/cmd/ksh93/sh/init.c: sh_reinit():
- Keep the SH_INIT state on during the entire procedure.
- Delete remaining non-exported, non-default variables.
- Remove attributes from exported variables. In POSIX mode, remove
all attributes; otherwise, only remove readonly.
- Unset discipline function pointers for variables.
- Delete all custom types.
- Delete all functions and built-ins, then reinitialise the built-ins
table from scatch.
- Free the alias values before clearing the alias table.
- Same with hash table entries (tracked aliases).
- Reset statistics.
- Inherit SH_POSIX instead of SH_TRACKALL.
- Call user init function last, not somewhere in the middle.
src/cmd/ksh93/sh/name.c: sh_envnolocal():
- Be sure to preserve the export attribute of kept variables.
Resolves: https://github.com/ksh93/ksh/issues/350
This bugfix was backported from ksh93v- 2012-10-04. The bug fixed
by this change is one that causes 'typeset -p' to omit the -C flag
when listing compound arrays belonging to a type:
$ typeset -T Foo_t=(compound -a bar)
$ Foo_t baz
$ typeset -p baz.bar
typeset -a baz.bar='' # This should be 'typeset -C -a'
src/cmd/ksh93/sh/nvtype.c:
- Backport change from 93v- 2012-10-04 that sets the array nvalue to
a pointer named Null (which is "") in nv_mktype(), then to Empty
in fixnode().
- Change the Null name from the 93v- code to AltEmpty to avoid
misleading code readers into thinking that it's a null pointer.
src/cmd/ksh93/tests/types.sh:
- Backport the relevant 93v- changes to the types regression tests.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
In ksh93v- 2012-10-04 the following bugfix is noted in the changelog
(this fix was most likely part of ksh93v- 2012-09-27, although that
version is not archived anywhere):
12-09-21 A bug in which the output of a two dimensional sparse
indexed array would cause the second subscript be treated
as an associative array when read back in has been fixed.
Elements that are sparse indexed arrays now are prefixed
type "typeset -a".
Below is a before and after of this change:
# Before
$ typeset -a foo[1][2]=bar
$ typeset -p foo
typeset -a foo=([1]=([2]=bar) )
# After
$ typeset -a foo[1][2]=bar
$ typeset -p foo
typeset -a foo=(typeset -a [1]=([2]=bar) )
src/cmd/ksh93/sh/*.c:
- Backport changes from ksh93v- to print 'typeset -a' before sparse
indexed arrays and properly handle 'typeset -a' in reinput
commands from 'typeset -p'.
src/cmd/ksh93/tests:
- Add two regression tests to arrays.sh for this change.
- Update the existing regression tests for compatibility with the
new printed typeset output.
Special builtins are undeleteable for a reason. But 'enum' and
'typeset -T' allow overriding them, causing an inconsistent state.
@JohnoKing writes:
| The behavior is rather buggy, as it appears to successfully
| override normal builtins but fails to delete the special
| builtins, leading to scenarios where both the original builtin
| and type are run:
|
| $ typeset -T eval=(typeset BAD; typeset TYPE) # This should have failed
| $ eval foo=BAD
| /usr/bin/ksh: eval: line 1: foo: not found
| $ enum trap=(BAD TYPE) # This also should have failed
| $ trap foo=BAD
| /usr/bin/ksh: trap: condition(s) required
| $ enum umask=(BAD TYPE)
| $ umask foo=BAD
| $ echo $foo
| BAD
|
| # Examples of general bugginess
| $ trap bar=TYPE
| /usr/bin/ksh: trap: condition(s) required
| $ echo $bar
| TYPE
| $ eval var=TYPE
| /usr/bin/ksh: eval: line 1: var: not found
| $ echo $var
| TYPE
This commit fixes the following:
The 'enum' and 'typeset -T' commands are no longer allowed to
override and replace special built-in commands, except for type
definition commands previously created by these commands; these
are already (dis)allowed elsewhere.
A command like 'typeset -T foo_t' without any assignments no longer
creates an incompletely defined 'foo_t' built-in comamnd. Instead,
it is now silently ignored for backwards compatibility. This did
have a regression test checking for it, but I'm changing it because
that's just not a valid use case. An incomplete type definition
command does nothing useful and only crashes the shell when run.
src/cmd/ksh93/bltins/enum.c: b_enum():
- Do not allow overriding non-type special built-ins.
src/cmd/ksh93/sh/name.c: nv_setlist():
- Do not allow 'typeset -T' to override non-type special built-ins.
To avoid an inconsistent state, this must be checked for while
processing the assignments list before typeset is really invoked.
src/cmd/ksh93/bltins_typeset.c: b_typeset():
- Only create a type command if sh.envlist is set, i.e., if some
shell assignment(s) were passed to the 'typeset -T' command.
Progresses: https://github.com/ksh93/ksh/issues/350
$ unset foo
$ echo ${foo[42]=bar}
(empty line)
Instead of the empty line, 'bar' was expected. As foo[42] was
unset, the conditional assignment should have worked.
$ unset foo
$ : ${foo[42]?error: unset}
(no output)
The expansion should have thrown an error with the given message.
This bug was introduced in ksh 93t 2008-10-01. Thanks to @JohnoKing
for finding the breaking change.
Analysis: The problem was experimenally determined to be in in the
following lines of nv_putsub(). If the array member is unset (i.e.
null), the value is set to the empty string instead:
src/cmd/ksh93/sh/array.c
1250: else
1251: ap->val[size].cp = Empty;
It makes some sense: if there is a value (even an empty one), the
variable is set and these expansions should behave accordingly.
Sure enough, deleting these lines fixes the bug, but at the expense
of introducing a lot of other array-related regressions. So we need
a way to special-case the affected expansions.
Where to do this? If we replace line 1251 with an abort(3) call, we
get this stack trace:
0 libsystem_kernel.dylib __pthread_kill + 10
1 libsystem_pthread.dylib pthread_kill + 284
2 libsystem_c.dylib abort + 127
3 ksh nv_putsub + 1411 (array.c:1255)
4 ksh nv_endsubscript + 940 (array.c:1547)
5 ksh nv_create + 4732 (name.c:1066)
6 ksh nv_open + 1951 (name.c:1425)
7 ksh varsub + 4934 (macro.c:1322)
[rest omitted]
The special-casing needs to be done on line 1250 of array.c, but
flagged in varsub() which processes these expansions. So, varsub()
calls nv_open() calls nv_create() calls nv_endsubscript() calls
nv_putsub(). That's a fairly deep call stack, so passing an extra
flag argument does not seem doable. I did try an approach using a
couple of new bit flags passed via these functions' flags and mode
parameters, but the way this code base uses bit flags is so
intricate, it does not seem to be possible to add or change
anything without unwanted side effects in all sorts of places.
So the only fix I can think of adds yet another global flag
variable for a very special case. It's ugly, but it works.
An elegant fix would probably involve a fairly comprehensive
redesign, which is simply not going to happen.
src/cmd/ksh93/include/shell.h:
- Add global sh.cond_expan flag.
src/cmd/ksh93/sh/array.c: nv_putsub():
- Do not set value to empty string if sh.cond_expan is set.
src/cmd/ksh93/sh/macro.c: varsub():
- Set sh.cond_expan flag while calling nv_open() for one of the
affected expansions.
- Minor refactoring for legibility and to make the fix fit better.
- SSOT: Instead of repeating string "REPLY", use the node's nvname.
- Do not pointlessly add an extra 0 byte when saving id for error
message; sfstruse() already adds this.
Thanks to @oguz-ismail for the bug report.
Resolves: https://github.com/ksh93/ksh/issues/383
Variables with a dot in their name, such as those declared in
namespace { ... } blocks, are usually stored in a separate tree
with their actual names not containing any dots. But under some
circumstances, including at least direct assignment of a
non-preexisting dot variable, dot variables are stored in the main
sh.var_tree with names actually containing dots. With allexport
active, those could end up exported to the environment. This bug
was also present in previous release versions of ksh.
src/cmd/ksh93/sh/name.c: pushnam():
- Check for a dot in the name before pushing a variable to export.
This fix was backported from ksh 93v- 2012-10-04.
src/cmd/ksh93/sh/nvtree.c: nv_outnode():
- If the array is supposed to be empty, do not continue. This
avoids outputting a nonexistent [0]= element for empty arrays.
Resolves: https://github.com/ksh93/ksh/issues/420
Co-authored-by: Martijn Dekker <martijn@inlv.org>
This commit adds a fix for the trap command, backported from a fork
of ksh2020: https://github.com/l0stman/ksh/commit/2033375f
src/cmd/ksh93/sh/jobs.c: job_chldtrap():
- Fixed a use after free bug in the for loop. The string pointed to
by sh.st.trapcom[SIGCHLD] may be freed from memory after
sh_trap(), so it must be reobtained each time sh_trap() is called
from within the for loop.
All variables that are assigned a value should be exported while
the allexport shell option is enabled. This works in most cases,
but variables assigned to with ${var:=foo} or $((var=123)) aren't
exported while allexport is on.
src/cmd/ksh93/sh/name.c:
- nv_putval(): This is the central assignment function; all forms
of variable assignment end up here. So this is the best place
to check for SH_ALLEXPORT and turn on the export attribute.
- nv_setlist(): Remove allexport handling, now redundant.
src/cmd/ksh93/bltins/read.c: sh_readline():
- Remove allexport handling, now redundant.
src/cmd/ksh93/sh/main.c: sh_main():
- nv_putval() is used to initialize PS4 and IFS using nv_putval();
this is after an -a/--allexport specified on the ksh command
line has been processed, so temporarily turn that off.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
If the VISUAL or EDITOR environment variable is not set to a value
matching *[Vv][Ii]* or *macs* at initialisation time, then ksh does
not turn on any line editor.
This is user-hostile. New users on Unix-like systems typically have
a simple editor like nano preconfigured as their default, or may
not have the VISUAL or EDITOR variable set at all. So if they try
ksh, they find themselves without basic functionality such as arrow
keys and probably go straight back to bash.
The emacs line editor is by far the most widely used, especially
among new users, so ksh should default to that. Most other shells
already do this.
src/cmd/ksh93/sh/main.c: sh_main():
- On an interactive shell, if on editor was turned on based on
$VISUAL or $EDITOR, turn on emacs before reading input.
Reproducer:
$ ksh -c 'bash -c '\''kill -s INT $$'\''; echo "$?, continuing"'
Expected result: output "258, continuing"; exit status 0.
Actual result: no output; exit status 258. The child process sent
SIGINT only to itself and not to the process group, so the parent
script was wrongly interrupted.
Every shell except ksh93 produces the expected result. ksh93 also
gave the expected result before version 2008-01-31 93s+, which
introduced the code below.
Analysis: The problem is in these lines of code in xec.c,
sh_exec(), TFORK case, parent branch of fork:
1649: if(!sh_isstate(SH_MONITOR))
1650: {
1651: if(!(sh.sigflag[SIGINT]&(SH_SIGFAULT|SH_SIGOFF)))
1652: sh_sigtrap(SIGINT);
1653: sh.trapnote |= SH_SIGIGNORE;
1654: }
[...pipe and I/O handling, wait for command to finish...]
1667: if(!sh_isstate(SH_MONITOR))
1668: {
1669: sh.trapnote &= ~SH_SIGIGNORE;
1700: if(sh.exitval == (SH_EXITSIG|SIGINT))
1701: kill(sh.current_pid,SIGINT);
1702: }
When a user presses Ctrl+C, SIGINT is sent to the entire process
group. If job control is fully off (i.e., !sh_isstate(SH_MONITOR)),
then the process group includes the parent script. Therefore, in a
script such as
$ ksh -c 'bash -c '\''read x'\''; echo "$?, continuing"'
when the user presses Ctrl+C while bash waits for 'read x' input,
the parent ksh script should be interrupted as well.
Now, the code above ignores SIGINT while bash is running. (This is
done using special-casing in sh_fault() to handle that SH_SIGIGNORE
flag for SIGINT.) So, when Ctrl+C interrupts the process group, the
parent script is not getting interrupted as it should.
To compensate for that, the code then detects, using sh.exitval
(the child process' exit status), whether the child process was
killed by SIGINT. If so, it simply assumes that the signal was
meant for the process group including the parent script, so it
reissues SIGINT to itself after unignoring it.
But, as we can see from the broken reproducer above, that
assumption is not valid. Scripts are perfectly free to send SIGINT
to themselves only, and that must work as expected.
src/cmd/ksh93/sh/xec.c: sh_exec(): TFORK: parent branch:
- Instead of ignoring SIGINT, sigblock() it, which delays handling
the signal until sigrelease(). (Note that these are macros
defined in src/cmd/ksh93/features/sigfeatures according to OS
capabilities.)
- This makes reissuing SIGINT redundant, so delete that, which
fixes the bug.
src/cmd/ksh93/sh/fault.c:
- Nothing now sets the SH_SIGIGNORE flag in sh.trapnote, so remove
special-casing added in 2008-01-31 93s+.
Add extra key bindings to the emacs and vi modes
This patch adds the following key bindings to the emacs and vi
editing modes:
- Support for Home key sequences ^[[1~ and ^[[7~ as well as End key
sequences ^[[4~ and ^[[8~.
- Support for arrow key sequences ^[OA, ^[OB, ^[OC and ^[OD.
- Support for the following keyboard shortcuts (if the platform
supports the expected escape sequence):
- Ctrl-Left Arrow: Go back one word
- Alt-Left Arrow: Go back one word (Not supported on Haiku)
- Ctrl-Right Arrow: Go forward one word
- Alt-Right Arrow: Go forward one word (Not supported on Haiku)
- Ctrl-G: Cancel reverse search
- Ctrl-Delete: Delete next word (Not supported on Haiku)
- Added a key binding for the Insert key, which differs in the
emacs and vi editing modes:
- In emacs mode, Insert escapes the next character.
- In vi mode, Insert will switch the editor to insert mode (like
in vim).
src/cmd/ksh93/edit/{emacs,vi}.c:
- Add support for the <M-Left> and <M-Right> sequences. Like in
bash and mksh, these shortcuts move the cursor one word backward
or forward (like the <Ctrl-Left> and <Ctrl-Right> shortcuts).
- Only attempt to process these shortcuts if the escape sequence
begins with $'\E[1;'.
src/cmd/ksh93/edit/vi.c:
- If the shell isn't doing a reverse search, insert the bell
character when Ctrl+G is input.
- Add the Ctrl-Delete shortcut as an alias of 'dw'. Calling
ed_ungetchar twice does not work for 'dw', so Ctrl-Delete was
implemented by using a vp->del_word variable.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
Currently, running ksh under ASan without the ASAN_OPTIONS variable
set to 'detect_leaks=0' usually ends with ASan complaining about a
memory leak in defpathinit() (this leak doesn't grow in a loop, so
no regression test was added to leaks.sh). Reproducer:
$ ENV=/dev/null arch/*/bin/ksh
$ cp -?
cp: invalid option -- '?'
Try 'cp --help' for more information.
$ exit
=================================================================
==225132==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 85 byte(s) in 1 object(s) allocated from:
#0 0x7f6dab42d459 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
#1 0x5647b77fe144 in sh_calloc /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/init.c:265
#2 0x5647b788fea9 in path_addcomp /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/path.c:1567
#3 0x5647b78911ed in path_addpath /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/path.c:1705
#4 0x5647b7888e82 in defpathinit /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/path.c:442
#5 0x5647b78869f3 in ondefpath /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/sh/path.c:67
--- cut ---
SUMMARY: AddressSanitizer: 174 byte(s) leaked in 2 allocation(s).
Analysis: The previous code was leaking memory because
defpathinit() returns a pointer from path_addpath(), which has
memory allocated to it in path_addcomp(). This is the code ASan
traced as having allocated memory:
442: return(path_addpath((Pathcomp_t*)0,(defpath),PATH_PATH));
In path_addpath():
1705: first = path_addcomp(first,old,cp,type);
[...]
1729: return(first);
In path_addcomp():
1567: pp = sh_newof((Pathcomp_t*)0,Pathcomp_t,1,len+1);
The ondefpath() function doesn't save a reference to the pointer
returned by defpathinit(), which causes the memory leak:
66: if(!defpath)
67: defpathinit();
The changes in this commit avoid this problem by setting the
defpath variable without also calling path_addpath().
src/cmd/ksh93/sh/path.c:
- Move the code for allocating defpath from defpathinit() into its
own dedicated function called std_path(). This function is called
by defpathinit() and ondefpath() to obtain the current string
stored in the defpath variable. This bugfix is adapted from a
fork of ksh2020: https://github.com/l0stman/ksh/commit/db5c83a6
'eval' suffers from the same bug. Reproducer:
$ eval vi
then suspend vi, then try to resume it -- the same as in the
reproducer shown in the previous commit.
src/cmd/ksh93/bltins/misc.c: b_eval():
- Same fix. Do *not* turn off SH_MONITOR.
This fixes yet another whopper of a bug in job control. And it's
been in every version of ksh93 back to 1995, the beginning of
ast-open-archive. <sigh>
This is also bug number 23 that is fixed by simply removing code.
Reproducer:
1. Run vi, or another suspendable program, from a dot script or
POSIX function (ksh handles them the same way). So either:
$ echo vi >v
$ . ./v
or:
$ v() { vi; }
$ v
2. Suspend vi by typing Ctrl+Z.
3. Not one but two jobs are registered:
$ jobs -l
[2] + 85513 Stopped . ./v
[1] - 85512 Stopped . ./v
4. 'fg' does not work on either of them, just printing the job
command name but not resuming the editor. The second job
disappears from the table after 'fg'.
$ fg %2
. ./v
$ fg %2
ksh: fg: no such job
$ fg %1
. ./v
$ fg %1
. ./v
Either way, you're stuck with an unresumable vi that you have to
'kill -9' manually.
src/cmd/ksh93/sh/xec.c: sh_exec(): TFORK:
- Do *not* turn off the SH_MONITOR state flag (which tells ksh to
keep track of jobs) when running an external command from a
profile script or dot script/POSIX function. It's obvious that
this results in an inconsistent job control state as ksh will not
update it when the external command is suspended. The purpose of
this nonsense is surely lost to history as it's been there since
1995 or before. My testing says that removing it doesn't break
anything. If that turns out to be wrong, then the breakage will
have to be fixed in a correct way instead.
Attempting to use array subscript expansion with variables that
aren't set currently causes a spurious syntax error (in ksh93u+ and
older commits the reproducer crashes):
$ ksh -c 'echo ${foo[${bar}..${baz}]}' # Shouldn't print anything
ksh: : arithmetic syntax error
src/cmd/ksh93/sh/macro.c:
- Backport a parser bugfix from ksh93v- 2012-08-24 that avoids
setting mp->dotdot until the copyto() function's loop is
finished.
src/cmd/ksh93/tests/arrays.sh:
- Add regression tests for this bug.
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.
src/cmd/ksh93/sh.1:
- Add a new section on history expansion mostly adapted from the
"History substitution" section from the tcsh(1) man page. This
has the standard BSD license which is already in the COPYRIGHT
file. Inapplicable stuff was removed, some new stuff added.
src/cmd/ksh93/edit/hexpand.c,
src/cmd/ksh93/sh/io.c:
- Set '#' as the default history comment character as on bash;
no longer disable it by default.
- Add the 'a' modifier as a synonym for 'g', as on bash.
- Remove pointless static keyword from np variable; since the
value from previous calls is never used it can just be local.
- Use NV_NOADD flag with nv_open() to avoid pointlessly creating
the node if the variable doesn't exist yet.
- Fix a bug in history expansion where the 'p' modifier had no
effect if the 'histverify' option is on.
Reproducer:
$ set -H -o histv
$ true a b c
$ !!:p
$ true a b c▁ <= instead of printed, the line is re-edited
Expected:
$ set -H -o histv
$ true a b c
$ !!:p
true a b c
$ ▁
This is fixed by making 'p' remove the HIST_EVENT bit from the
returned flag bits in hist_expand(), leaving only the HIST_PRINT
flag, then adding special handling for this case to slowread()
in io.c (print the line, then instead of executing it, continue
and read the next line).
Reproducer:
$ set -o histexpand
$ echo foo !#^:h !#^:&
/usr/local/bin/ksh: :&: no previous substitution
ksh(80822,0x10bc2a5c0) malloc: *** error for object 0x10a13bae3: pointer being freed was not allocated
ksh(80822,0x10bc2a5c0) malloc: *** set a breakpoint in malloc_error_break to debug
Abort
Analysis: In hist_expand(), the 'cc' variable has two functions:
it holds a pointer to a malloc'ed copy of the current line, and is
also used as a temporary pointer with functions like strchr().
After that temporary use, it is set to NULL again, because the
'done:' routine checks if it non-NULL to decide whether to free the
pointer. But if an error occurs, the function may jump straight to
'done' without first setting cc to NULL if it had been used as a
temporary pointer. It then tries to free an unallocated pointer.
src/cmd/ksh93/edit/hexpand.c: hist_expand():
- Eliminate this bad practice by using a separate variable for
temporary pointer purposes.
(I was unable to reproduce the crash in a pty regression test,
though it is consistently reproducible in a real interactive
session. So I haven't added that test.)
So far all ksh versions accept event numbers referring to
nonexistent history events in history expansion (-H/-o histexpand),
e.g. !9999 is accepted even if the history file has no item 9999.
These expansions seem to show random content from the history file,
sometimes including binary data. Of course an "event not found"
error should have been thrown instead.
hist_expand() in hexpand.c calls hist_seek() (from history.c)
without any bounds checking except verifying the history event
number is greater than zero. This commit adds a bounds check
to hist_seek() itself as it's called from three other places
in history.c, so perhaps this fixes a few other bugs as well.
src/cmd/ksh93/edit/history.c: hist_seek():
- Use the hist_min() and hist_max() macros provided in history.h
to check bounds. Note that hist_max() yields the number of the
command line currently being entered, so the maximum for seeking
purposes is actually its result minus 1.
History expansion currently crashes under ASan due to a buffer
overflow. Reproducer:
$ set -H
$ !!:s/old/new/
Explanation from <https://github.com/att/ast/issues/1369>:
> The problem is the code assumes the buffer allocated for a string
> stream is zero initialized. But the SFIO code uses malloc() to
> allocate the buffer and does not explicitly initialize it with
> memset(). That it works at all, even without ASAN enabled, is
> purely accidental. It will fail if that malloc() returns a block
> that had been previously allocated, used, and freed. Under ASAN
> the buffer is initialized (at least on my system) to a sequence
> of 0xBE bytes. So the strdup() happily tries to duplicate a
> string that is the size of that buffer and fails when it reads
> past the end of the buffer looking for the terminating zero byte.
src/cmd/ksh93/edit/hexpand.c:
- Backport ksh2020 bugfix that avoids assuming the string stream
has been initialized to zeros:
https://github.com/att/ast/commit/cf16bcca
(minus the incorrect change to the static wm variable).
This adds two long-form shell options that modify history expansion
(-H/--histexpand). If --histreedit is on and a history expansion
fails, the command line is reloaded into the next prompt's edit
buffer, allowing corrections. If --histverify is on, the results of
a history expansion are not immediately executed but instead loaded
into the next prompt's edit buffer, allowing further changes.
SH_HISTREEDIT and SH_HISTVERIFY were already handled all along in
slowread() in io.c and via 'reedit' arguments to functions called
there, but could not be turned on as they were only ever exposed as
shopt options in the removed bash compatibility mode (in theory
only, as it failed to compile). I had overlooked them until now.
So, since the code is there, let's expose these options through the
normal long options interface. They're working fine, and activating
them actually makes history expansion tolerable to use.
src/cmd/ksh93/data/options.c:
- Make these options available as "histreedit" and "histverify".
src/cmd/ksh93/data/builtins.c,
src/cmd/ksh93/sh.1:
- Document the "new" options.
src/cmd/ksh93/include/defs.h:
- Remove unused SH_HISTAPPEND and SH_HISTORY2 macros which were
part of the removed bash compatibility code. Note that ksh does
not need a histappend option as it never overwrites the history
file (in the bash mode, this shopt option was a no-op).
Attempting to complete file names in vi mode using tab completion can
fail if the last character on the command line is a space. Reproducer
(note that this bug doesn't occur in emacs mode):
$ set -o vi
$ mkdir '/tmp/foo bar'
$ test -d /tmp/foo\ <Tab>
src/cmd/ksh93/edit/vi.c:
- Don't disable tab completion or reset the tab count just because the
last character on the command line is a space. This bugfix was
backported from ksh93v- 2014-06-06.
src/cmd/ksh93/tests/pty.sh:
- Add a regression test for the tab completion bug.
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.
After 'unset CDPATH', CDPATH continued to work as if nothing
happened. Unsetting it should be a valid way to deactivate it.
This bug is in every ksh93 version.
src/cmd/ksh93/bltins/cd_pwd.c: b_cd():
- Fix a manifest logic error: first check if CDPATH (CDPNOD) is
unset before assigning to 'cdpath', not the other way around.
Setting the 'cdpath' pointer is what activates the CDPATH search.
This commit ports performance optimizations from illumos for the libsum
code (used by the cksum and sum builtins):
98bea71f0d
The new codepath in libsum uses prefetching and loop unrolling to
improve performance (prefetching is done with __builtin_prefetch()
or sun_prefetch_read_many() if either is available).
Script for testing (note that cksum must be enabled in
src/cmd/ksh93/data/builtins.c):
#!/bin/ksh
builtin cksum || exit 1
for ((i=0; i!=50000; i++)) do
cksum -x att /etc/hosts
done >/dev/null
Results on Linux x86_64 (using CCFLAGS=-O2):
$ echo 'UNPATCHED:'; time arch/linux.i386-64/bin/ksh /tmp/foo; echo 'PATCHED'; time /tmp/ksh /tmp/foo
UNPATCHED:
real 0m09.989s
user 0m07.582s
sys 0m02.406s
PATCHED:
real 0m06.536s
user 0m04.331s
sys 0m02.204s
src/lib/libsum/{sum-att.c,sum-crc.c,Mamfile}:
- Port the performance optimizations from illumos to 93u+m libsum. To
prevent problems with older versions of GCC, avoid the new codepath
if GCC is older than the 3.1 release series. Additionally, the ast.h
header must be included to handle tcc defining __GNUC__ on FreeBSD.
- Apply some build fixes to allow the new codepath to build with Clang
3.6 and newer (my own testing indicates an even better performance
improvement with Clang than with GCC).
This patch adds a few extra options to the ulimit command (if the OS
supports them). These options are also present in Bash, although in ksh
additional long forms of each option are available:
ulimit -k/--kqueues This is the maximum number of kqueues.
ulimit -P/--npts This is the maximum number of pseudo-terminals.
ulimit -R/--rttime This is the time a real-time process can run
before blocking, in microseconds. When the
limit is exceeded, the process is sent SIGXCPU.
Other changes:
- bltins/ulimit.c: Change the formatting from sfprintf and increase the
size of the tmp buffer to prevent text from being cut off in ulimit
-a (this was required to add ulimit -R).
- data/limits.c: Add support for using microseconds as a unit.
The unused mkservice and eloop builtins are currently not built, and if
an attempt to compile them is made the build ends in failure. This
commit backports a few build fixes from ksh93v- 2012-08-24 that allow
mkservice and eloop to build (plus an additional compiler warning fix
not in ksh93v-). I've also added a new SHOPT_MKSERVICE setting (turned
off by default) so that mkservice and eloop can be built if the user
chooses to include them in their build of ksh.
This commit backports the whence '-t' option from ksh93v-. The '-t'
option is useful when one needs to identify the type of a command.
The '-t' flag was added by ksh93v- for compatibility with Bash.
It should be noted the ksh93v- patch had one bug, which this commit
fixes. Path-bound builtins from /opt/ast/bin were classified as
files if loaded from /opt/ast/bin in the PATH. Reproducer:
$ PATH=/opt/ast/bin whence -t cat
file
src/cmd/ksh93/bltins/whence.c:
- Simplify the bitmask values for the command and whence builtin
flags.
- Add the -t flag to the whence and type builtins. To prevent bugs,
-t will always override -v if both of those flags were passed.
src/cmd/ksh93/data/builtins.c,
src/cmd/ksh93/sh.1:
- Add documentation for the new -t option.
Depending on the OS, the heredoc.sh regression tests, and possibly
others, still crashed with the -x option (xtrace) on.
Analysis: The lexer crashes in lex_advance(). Something has caused
an inconsistent lexer state, and it happened earlier on, so the
backtrace is useless for figuring out where that happened.
But I think I've found it. It's the sh_mactry() call here:
src/cmd/ksh93/sh/xec.c, lines 2800 to 2807 in f7213f03
2800: if(!(cp=nv_getval(sh_scoped(shp,PS4NOD))))
2801: cp = "+ ";
2802: else
2803: {
2804: sh_offoption(SH_XTRACE);
2805: cp = sh_mactry(shp,cp);
2806: sh_onoption(SH_XTRACE);
2807: }
sh_mactry() needs to parse the contents of $PS4 to perform
expansions and command substitutions in it, which involves the
lexer. If that happens in a here-document, the lexer is in the C
function call stack, in the middle of parsing the here-document.
Result: inconsistent lexer state. Solution: save and restore lexer
state in sh_mactry().
After this commit, all regression tests should pass with the
'-x'/'--xtrace' option in use, with no errors or crashes.
Note for backporters: this fix depends both on on d7cada7b and on
the consistency fix for the Lex_t type's size applied in a7ed5d9f.
src/cmd/ksh93/include/shlex.h:
- Cosmetic fix: remove a copied & pasted backslash. (re: a7ed5d9f)
src/cmd/ksh93/sh/macro.c: sh_mactry():
- Save and restore the lexer state before letting sh_mactrim()
indirectly parse and execute code.
src/cmd/ksh93/tests/*.sh:
- Turn off xtrace in various command substitutions that contain
2>&1 redirections, so that the xtrace output is not caught by
the command substitutions, causing tests to fail incorrectly.
- Turn off xtrace for a few code blocks with 2>&1 redirections,
stopping xtrace output from being written to standard output.
Resolves: https://github.com/ksh93/ksh/issues/306 (again)
On some systems (at least Linux and macOS):
1. Run on a command line: t=$(sleep 10|while :; do :; done)
2. Press Ctrl+C in the first 10 seconds.
3. Execute any other command substitution. The shell crashes.
Analysis: Something in the job_wait() call in the sh_subshell()
restore routine may be interrupted by a signal such as SIGINT on
Linux and macOS. Exactly what that interruptible thing is remains
to be determined. In any case, since job_wait() was invoked after
sh_popcontext(), interrupting it caused the sh_subshell() restore
routine to be aborted, resulting in an inconsistent state of the
shell. The fix is to sh_popcontext() at a later stage instead.
src/cmd/ksh93/sh/subshell.c: sh_subshell():
- Rename struct checkpt buff to checkpoint because it's clearer.
- Move the sh_popcontext() call to near the end, just after
decreasing the subshell level counters and restoring the global
subshell data struct to its parent. This seems like a logical
place for it and could allow other things to be interrupted, too.
- Get rid of the if(shp->subshell) because it is known that the
value is > 0 at this point.
- The short exit routine run if the subshell forked now needs a new
sh_popcontext() call, because this is handled before restoring
the virtual subshell state.
- While we're here, do a little more detransitioning from all those
pointless shp pointers.
Fixes: https://github.com/ksh93/ksh/issues/397
This commit was originally intended to fix just one bug with shcomp's
handling of 'alias -p', but while fixing that I found a large number
of related issues in the alias command's -p, -t and -x options. The
current patch provides bugfixes for all of the bugs listed below:
1) Listing aliases in a script with 'alias -p' or 'alias' broke
shcomp's bytecode output:
https://github.com/ksh93/ksh/issues/87#issuecomment-813819122
2) Listing individual aliases with the -p option doesn't work:
$ alias foo=bar bar=foo
$ alias foo
foo=bar
$ alias -p foo # No output
3) Listing specific tracked aliases with -pt does not display them
in a reusable format, but rather adds another tracked alias:
$ hash -r cat vi
$ alias -pt vi # No output
$ alias -pt rm
$ alias -t
cat=/usr/bin/cat
rm=/usr/bin/rm
vi=/usr/bin/vi
4) Listing all tracked aliases with -pt does not output them in a
reusable format (the resulting command printed only creates a
normal alias, which is different from a tracked alias):
$ hash -r cat
$ alias -pt
alias cat=/usr/bin/cat # Expected 'alias -t cat'
5) Listing a non-existent alias with -p doesn't cause an error:
$ unalias -a
$ alias -p notanalias # No output
$ echo $?
0
$ alias notanalias
notanalias: alias not found
$ echo $?
1
$ hash -r
$ alias -pt notacommand # No output
$ echo $?
0
6) Attempting to list 256 non-existent aliases results in exit
status zero:
$ unalias -a
$ alias $(awk -v ORS= 'BEGIN { for(i=0;i<256;i++) print "x "; }')
x: alias not found
--cut error message--
$ echo $?
0
Changes:
- typeset.c: Avoid printing anything while shcomp is compiling a
script. This is needed because the alias command is run by shcomp
to prevent parsing issues.
- b_alias(): To avoid adding tracked aliases with -pt, set
tdata.aflag to '+' so that setall() and other related functions
only list tracked aliases.
- b_alias(): Set tdata.pflag to 1 so that setall() and other
functions recognize -p was passed.
- print_value(): Add support for listing specific aliases with
'alias -p'.
- setall(): To avoid any issues with zombie tracked aliases (see also
the regression tests) ignore tracked alias nodes marked with the
NV_NOALIAS attribute. This bit is set for tracked alias nodes by
the nv_rehash() function.
- setall(): For backward compatibility, continue incrementing the
exit status for each invalid alias and tracked alias passed. This
was already how alias behaved when listing aliases without -p, so
using -p shouldn't cause a change in behavior:
$ unalias -a
$ alias foo bar
foo: alias not found
bar: alias not found
$ echo $?
2
To fix bug 6, the exit status is set to one if an enforced 8-bit
exit status would be zero.
- print_namval(): Set the prefix to 'alias -t' so that listing
tracked aliases with 'alias -pt' works correctly.
- data/msg.c and include/name.h: Add an error message for when
'alias -pt' doesn't find a tracked alias.
- tests/alias.sh: Add a ton of regression tests for the bugs fixed in
this commit.
- sh/args.c: A process substitution run in a profile script may print
its PID as if it was a command spawned with '&'. Reproducer:
$ cat /tmp/env
true >(false)
$ ENV=/tmp/env ksh
[1] 730227
$
This bug is fixed by turning off the SH_PROFILE state while running
a process substitution.
- sh/subshell.c: The SH_INTERACTIVE fix in 3525535e renders the extra
check for SH_PROFILE redundant, so it has been removed.
- tests/io.sh: Update the procsub PIDs test to also check the result
after using process substitution in a profile script.
Reproducer: run vi in a subshell:
$ (vi)
vi opens; now press Ctrl+Z to suspend. The output is as expected:
[2] + Stopped (vi)
…but the exit status is 18 (SIGTSTP's signal number) instead of 0.
Now do:
$ fg
(vi)
$
The exit status is 18 again, vi is not resumed, and the job is
lost. You have to find vi's pid manually using ps and kill it.
Forking all non-command substitution subshells invoked from the
interactive main shell is the only reliable and effective fix I've
found. I've tried to fork the subshell conditionally in every other
remotely plausible place I can think of in fault.c and xec.c, but I
can't get anything to work properly. If anyone can get this to work
without forking as much (or at all), please do submit a patch or PR
that supersedes this fix.
At least subshells of subshells don't need to fork, so the
performance impact can be limited. Plus, it's not as if most people
need maximum speed on the interactive command line. Scripts
(including login/profile scripts) are not affected at all.
Command substitutions can be handled differently. My testing shows
that all shells except ksh93 simply block SIGTSTP (the ^Z signal)
while they run. We should do the same, so they don't need to fork.
NOTE for any backporters: the subshell.c and fault.c changes depend
on commits 35b02626 and 48ba6964 to work correctly.
src/cmd/ksh93/sh/subshell.c: sh_subshell():
- If the interactive shell state bit is on, then before executing
the subshell's code:
- for command substitutions, block SIGTSTP;
- for other subshells, fork.
- For command substitutions, release SIGTSTP if the interactive
shell state bit was on upon invoking the subshell.
src/cmd/ksh93/sh/fault.c:
- Instead of checking for a virtual subshell, check the shell's
interactive state bit to decide whether to handle SIGTSTP, as
that is only turned on in the interactive main shell.
src/cmd/ksh93/sh/main.c: sh_main():
- To avoid bugs, ignore SIGTSTP while running profile scripts.
Blocking it doesn't work because delaying it until after
sigrelease() will cause a crash. Thanks to @JohnoKing for this.
- While we're here, prevent a possible overflow of the 'beenhere'
static char variable by only incrementing it once.
Co-authored-by: Johnothan King <johnothanking@protonmail.com>
Resolves: https://github.com/ksh93/ksh/issues/390
Reproducer:
$ (sleep 1& echo done)
done
$ (eval "echo hi"; sleep 1& echo done)
hi
[1] 30587
done
No job control output should be printed for a background process
invoked from a subshell, not even after 'eval'.
The cause: sh_parse() turns on the shell's interactive state bit
(sh_state(SH_INTERACTIVE)) if the interactive shell option is on.
This is incorrect. The parser should have no involvement with shell
interactivity in principle because that's not its domain.
Not only that, the parser may need to run in a subshell, e.g. when
executing traps or 'eval' commands (as above). By definition, a
subshell can never be interactive.
We already fixed many bugs related to job control and the shell's
interactive state. Even if these two lines previously papered over
some breakage, I can't find any now after simply removing them. If
any is found later, then it'll need to be fixed properly instead.
Related: https://github.com/ksh93/ksh/issues/390
Announcing: KornShell 93u+m 1.0.0-beta.2
https://github.com/ksh93/ksh
In May 2020, when every KornShell (ksh93) development project was
abandoned, development was rebooted in a new fork based on the last
stable AT&T version: ksh 93u+. This new fork is called ksh 93u+m as a
permanent nod to its origin. We're restarting it at version 1.0. Seven
months after the first beta, the second one is ready. Please test this
second beta and report any bugs you find, or help us fix known bugs.
We're now the default ksh93 in some OS distributions, at least Debian
and Slackware! Even though we don't think it's stable release quality
yet, the consensus seems to be that 93u+m is already much better than
the last AT&T release.
Main developers: Martijn Dekker, Johnothan King, hyenias
Contributors: Andy Fiddaman, Anuradha Weeraman, Chase, Gordon Woodhull,
Govind Kamat, Harald van Dijk, Lev Kujawski, Marc Wilson, Ryan Schmidt,
Sterling Jensen
HOW TO GET IT
Please download the source code tarball from our GitHub releases page:
https://github.com/ksh93/ksh/releases
To build, follow the instructions in README.md or src/cmd/ksh93/README.
HOW TO GET INVOLVED
To report a bug, please open an issue at our GitHub page (see above).
Alternatively, email me at martijn@inlv.org with your report.
To get involved in development, read the brief policy information in
README.md and then jump right in with a pull request or email a patch.
See the TODO file in the top-level directory for a to-do list.
*** MAIN CHANGES between 1.0.0-beta.1 and 1.0.0-beta.2 ***
New features in built-in commands:
- 'cd' now supports an -e option that, when combined with -P, verifies
that $PWD is correct after changing directories; this helps detect
access permission problems. See:
https://www.austingroupbugs.net/view.php?id=253
- 'printf' now supports a -v option as in bash. This assigns formatted
output directly to variables, which is very fast and will not strip
final newline (\n) characters.
- The 'return' command, when used to return from a function, can now
return any status value in the 32-bit signed integer range, like on
zsh. However, due to a traditional Unix kernel limitation, $? is
still trimmed to its least significant 8 bits whenever leaving a
(sub)shell environment.
- 'test'/'[' now supports all the same operators as [[ (including =~,
\<, \>) except for the different 'and'/'or' operators. Note that
'test'/'[' remains deprecated due to its unfixable pitfalls;
[[ ... ]] is recommended instead.
Shell language changes:
- Several improvements were made to the --noexec shell code linter.
- Arithmetic expressions in native ksh mode no longer interpret a
number with a leading zero as octal in any context. Use 8#octalnumber
instead (e.g. 8#400 == 256). Arithmetic expressions now also behave
identically within and outside ((...)) and $((...)).
- POSIX compatibility mode fixes (only applicable with the --posix shell
option on):
- A leading zero is now consistently recognised as introducing an octal
number in all arithmetic contexts.
- $((inf)) and $((nan)) are now interpreted as regular variables.
- The '.' built-in no longer runs ksh functions and now only runs
files.
Bugs fixed:
- '.' and '..' are now once again completed by tab completion.
- If SIGINT is set to ignore, the interactive shell no longer exits on
Ctrl+C.
- ksh now builds and runs on Apple's new M1 hardware.
- The 'return' and 'exit' commands no longer risk triggering actual
signals by returning or exiting with a status > 256.
- Ksh no longer behaves badly when parsing a type definition command
('typeset -T' or 'enum') without executing it or when executing it in
a subshell. Types can now safely be defined in subshells and defined
conditionally as in 'if condition; then enum ...; fi'.
- Discipline functions, especially those applied to PS2 or .sh.tilde,
will no longer crash your shell upon being interrupted or throwing an
error.
- Fixed a bug that could corrupt output if standard output is closed
upon initialising the shell.
- Fixed a bug in the [[ ... ]] compound command: the '!' logical
negation operator now correctly negates another '!', e.g.,
[[ ! ! 1 -eq 1 ]] now returns 0/true. Note that this has always been
the case for 'test'/'['.
- Fixed SHLVL so that replacing ksh by itself (exec ksh) will not
increase it.
- Arithmetic expressions are no longer allowed to assign out-of-range
values to variables of types declared with enum.
- The 'time' keyword no longer makes the --errexit shell option
ineffective.
- Various bugs in libcmd built-in commands (those bound to the
/opt/ast/bin path by default) have been fixed.
- Various other crashing bugs have been fixed.
Fixes for the shcomp byte code compiler:
- shcomp is now able to compile scripts that define types using enum.
- shcomp now refuses to mess up your terminal by writing bytecode
to it.
*** MAIN CHANGES between ksh 93u+ 2012-08-01 and 93u+m 1.0.0-beta.1 ***
Hundreds of bugs have been fixed, including many serious/critical bugs.
This includes upstreamed patches from OpenSUSE, Red Hat, and Solaris, fixes
backported from the abandoned 93v- beta and ksh2020 fork, as well as many
new fixes from the community. See the NEWS file for more information, and
the git commit log for complete documentation of every fix. Incompatible
changes have been minimised, but not at the expense of fixing bugs. For a
list of potentially incompatible changes, see src/cmd/ksh93/COMPATIBILITY.
Though there was a "no new features, bugfixes only" policy, some new
features were found necessary, either to fix serious design flaws or to
complete functionality that was evidently intended, but not finished.
Below is a summary of these new features.
New command line editor features:
- The forward-delete and End keys are now handled as expected in the
emacs and vi built-in line editors.
- In the vi and emacs line editors, repeat count parameters can now also
be used for the arrow keys and the forward-delete key. E.g., in emacs
mode, <ESC> 7 <left-arrow> will now move the cursor seven positions to
the left. In vi control mode, this would be entered as: 7 <left-arrow>.
New shell language features:
- The &>file redirection shorthand (for >file 2>&1) is now available for
all scripts and interactive sessions and not only for profile/login
scripts, bringing ksh 93u+m in line with mksh, bash, and zsh.
- File name generation (a.k.a. pathname expansion, a.k.a. globbing) now
never matches the special navigational names '.' (current directory)
and '..' (parent directory). This change makes a pattern like .*
useful; it now matches all hidden files (dotfiles) in the current
directory, without the harmful inclusion of '.' and '..'.
- Tilde expansion can now be extended or modified by defining a
.sh.tilde.get or .sh.tilde.set discipline function. This replaces a
2004 undocumented attempt to add this functionality via a .sh.tilde
command, which never worked and crashed the shell. See the manual for
details on the new method.
New features in built-in commands:
- Usage error messages now show the --help/--man self-documentation options.
- Path-bound built-ins (such as /opt/ast/bin/cat) can now be executed by
invoking the canonical path, so the following will now work as expected:
$ /opt/ast/bin/cat --version
version cat (AT&T Research) 2012-05-31
- 'command -x' now looks for external commands only, skipping built-ins.
In addition, its xargs-like functionality no longer freezes the shell on
Linux and macOS, making it effectively a new feature on these systems.
- 'redirect' now checks if all arguments are valid redirections before
performing them. If an error occurs, it issues an error message instead
of terminating the shell.
- 'suspend' now refuses to suspend a login shell, as there is probably no
parent shell to return to and the login session would freeze.
- 'times' now gives high precision output in a POSIX compliant format.
- 'typeset' now gives an informative error message if an incompatible
combination of options is given.
- 'whence -v/-a' now reports the location of autoloadable functions.
New features in shell options:
- A new --globcasedetect shell option is added on OSs where we can
check for a case-insensitive file system (currently Windows/Cygwin,
macOS, Linux and QNX 7.0+). When this option is turned on, file name
generation (globbing), as well as file name tab completion on
interactive shells, automatically become case-insensitive on file
systems where the difference between upper and lower case is ignored
for file names. This is transparently determined for each directory, so
a path pattern that spans multiple file systems can be part
case-sensitive and part case-insensitive.
- A new --nobackslashctrl shell option disables the special escaping
behaviour of the backslash character in the emacs and vi built-in
editors. Particularly in the emacs editor, this makes it much easier to
go backward, insert a forgotten backslash into a command, and then
continue editing without having your next cursor key replace your
backslash with garbage. Note that Ctrl+V (or whatever other character
was set using 'stty lnext') always escapes all control characters in
either editing mode.
- A new --posix shell option has been added to ksh 93u+m that makes the
ksh language more compatible with other shells by following the POSIX
standard more closely. See the manual page for details. It is enabled by
default if ksh is invoked as sh, otherwise it is disabled by default.
- Enhancement to -G/--globstar: symbolic links to directories are now
followed if they match a normal (non-**) glob pattern. For example, if
'/lnk' is a symlink to a directory, '/lnk/**' and '/l?k/**' now work as
you would expect.
Strings compared in [[ with the > and < operators should be compared
lexically. This does not work when the strings are single digits, as
the parser interprets it as a syntax error:
$ [[ 10<2 ]] # 10 lexically sorts before 2
$ echo $?
0
$ [[ 1<2 ]]
/usr/bin/ksh: syntax error: `<' unexpected
$ echo $?
3
src/cmd/ksh93/sh/lex.c:
- Don't interpret numbers next to > and < as a redirection while
inside of [[. This bugfix was backported from ksh93v- 2014-06-25.
src/cmd/ksh93/tests/bracket.sh:
- Add regression tests for the > and < operators.
New version. I'm pretty sure the problems that forced me to revert
it earlier are fixed.
This commit mitigates the effects of the hack explained in the
referenced commit so that dummy built-in command nodes added by the
parser for declaration/assignment purposes do not leak out into the
execution level, except in a relatively harmless corner case.
Something like
if false; then
typeset -T Foo_t=(integer -i bar)
fi
will no longer leave a broken dummy Foo_t declaration command. The
same applies to declaration commands created with enum.
The corner case remaining is:
$ ksh -c 'false && enum E_t=(a b c); E_t -a x=(b b a c)'
ksh: E_t: not found
Since the 'enum' command is not executed, this should have thrown
a syntax error on the 'E_t -a' declaration:
ksh: syntax error at line 1: `(' unexpected
This is because the -c script is parsed entirely before being
executed, so E_t is recognised as a declaration built-in at parse
time. However, the 'not found' error shows that it was successfully
eliminated at execution time, so the inconsistent state will no
longer persist.
This fix now allows another fix to be effective as well: since
built-ins do not know about virtual subshells, fork a virtual
subshell into a real subshell before adding any built-ins.
src/cmd/ksh93/sh/parse.c:
- Add a pair of functions, dcl_hactivate() and dcl_dehacktivate(),
that (de)activate an internal declaration built-ins tree into
which check_typedef() can pre-add dummy type declaration command
nodes. A viewpath from the main built-ins tree to this internal
tree is added, unifying the two for search purposes and causing
new nodes to be added to the internal tree. When parsing is done,
we close that viewpath. This hides those pre-added nodes at
execution time. Since the parser is sometimes called recursively
(e.g. for command substitutions), keep track of this and only
activate and deactivate at the first level.
(Fixed compared to previous version of this commit: calling
dcl_dehacktivate() when the recursion level is already zero is
now a harmless no-op. Since this only occurs in error handling
conditions, who cares.)
- We also need to catch errors. This is done by setting libast's
error_info.exit variable to a dcl_exit() function that tidies up
and then passes control to the original (usually sh_exit()).
(Fixed compared to previous version of this commit: dcl_exit()
immediately deactivates the hack, no matter the recursion level,
and restores the regular sh_exit(). This is the right thing to
do when we're in the process of erroring out.)
- sh_cmd(): This is the most central function in the parser. You'd
think it was sh_parse(), but $(modern)-form command substitutions
use sh_dolparen() instead. Both call sh_cmd(). So let's simply
add a dcl_hacktivate() call at the beginning and a
dcl_deactivate() call at the end.
- assign(): This function calls path_search(), which among many
other things executes an FPATH search, which may execute
arbitrary code at parse time (!!!). So, regardless of recursion
level, forcibly dehacktivate() to avoid those ugly parser side
effects returning in that context.
src/cmd/ksh93/bltins/enum.c: b_enum():
- Fork a virtual subshell before adding a built-in.
src/cmd/ksh93/sh/xec.c: sh_exec():
- Fork a virtual subshell when detecting typeset's -T option.
Improves fix to https://github.com/ksh93/ksh/issues/256
The killpg(getpgrp(),SIGINT) call added to ed_getchar() in that
commit caused the interactive shell to exit on ^C even if SIGINT is
being ignored. We cannot revert or remove that call without
breaking job control. This commit applies a new fix instead.
Reproducers fixed by this commit:
SIGINT ignored by child:
$ PS1='childshell$ ' ksh
childshell$ trap '' INT
childshell$ (press Ctrl+C)
$
SIGINT ignored by parent:
$ (trap '' INT; ENV=/./dev/null PS1='childshell$ ' ksh)
childshell$ (press Ctrl+C)
$
SIGINT ignored by parent, trapped in child:
$ (trap '' INT; ENV=/./dev/null PS1='childshell$ ' ksh)
childshell$ trap 'echo test' INT
childshell$ (press Ctrl+C)
$
I've experimentally determined that, under these conditions, the
SFIO stream error state is set to 256 == 0400 == SH_EXITSIG.
src/cmd/ksh93/sh/main.c: exfile():
- On EOF or error, do not return (exiting the shell) if the shell
state is interactive and if sferror(iop)==SH_EXITSIG.
- Refactor that block a little to make the new check fit in nicely.
src/cmd/ksh93/tests/pty.sh:
- Test the above three reproducers.
Fixes: https://github.com/ksh93/ksh/issues/343
Note that this is only about the /opt/ast/bin built-in commands,
not about the regular pathless builtins such as printf.
To use these, either add /opt/ast/bin to your $PATH or use a
command like 'builtin cp'. As usual, --man provides info.
Removed as defaults for lack of convincing advantages over the OS's
external commands:
- chmod, cmp, head, logname, mkdir, sync, uname, wc
Remain as useful defaults:
- basename, cat, cut, dirname. These are commonly used in
performance-sensitive code paths in scripts and having them as
built-ins can be good for performance.
- getconf: This is the only interface to some libast internals that
is available to ksh. It's also has better functionality than most
OS-shipped 'getconf' commands, e.g., it can list and query all
the configuration values.
Added as defaults:
- cp, ln, mv: Having these built in can speed up scripts that
manage files. Also the AST versions have extended functionality
(see cp --man, etc.).
- mktemp: External mktemp commands vary too widely and are
incompatible, but it's important that scripts can securely make
temporary files, so it's good to ship a known interface to this
functionality.
As a result, the statically linked ksh binary is very slightly
smaller than before.
Resolves: https://github.com/ksh93/ksh/issues/349
This bug was first reported at <https://www.illumos.org/issues/3782>.
The chown builtin when used on illumos can fail with different error
messages after running the same command twice:
$ touch /tmp/x
$ /opt/ast/bin/chown -h 433:434 /tmp/px
chown: /tmp/x: cannot change owner and group [Not owner]
$ /opt/ast/bin/chown -h 433:434 /tmp/px
chown: /tmp/x: cannot change owner and group [Invalid argument]
The error messages differ because the libast struid and strgid
functions will return -2 if the same nonexistent ID is used twice.
The fix for this bug has been ported from here:
https://github.com/illumos/illumos-gate/commit/4162633a7c5961f388fd
src/lib/libcmd/chgrp.c:
- Remove NOID macro and check for a < 0 error status instead.
This is different from the Illumos fix at
<https://github.com/illumos/illumos-gate/commit/4162633a7c59>
which added another macro.
src/lib/libast/man/{strgid,struid}.3:
- Correct errors in the strgid and struid documentation.
- Document that the strgid and struid functions will return -2 if
the same invalid name is used twice.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
Turns out there is a bona fide, honest-to-goodness use case for
matching '.' and '..' in globbing after all. It's when globbing is
used as the backend mechanism for file name completion in
interactive shell editors. A tab invisibly adds a * at the end of
the word to the left of your cursor and the resulting pattern is
expanded. In 5312a59d, this broke for '.' and '..'.
Typing '.' followed by two tabs should result in a menu that
includes './' and '../'. Typing '..' followed by a tab should
result in '../', (or a menu that includes it if there are files
with names starting with '..'). This is the behaviour in 93u+ and
we should maintain this.
To restore this functionality without reintroducing the harmful
behaviour fixed in the referenced commits, we should special-case
this, allowing '.' and '..' to match only for file name completion.
src/lib/libast/include/glob.h:
- Fix an inaccurate comment: the GLOB_COMPLETE flag is used for
command completion, not file name completion. This is very clear
from reading the path_expand() function in sh/expand.c.
- Add new GLOB_FCOMPLETE flag for file name completion.
src/lib/libast/misc/glob.c:
- Adapt flags mask to fit the new flag.
- glob_dir(): If GLOB_FCOMPLETE is passed, allow '.' and '..' to
match even if expanded from a pattern.
- Clarify the fix from aad74597 with an extended comment based on
<https://github.com/ksh93/ksh/issues/146#issuecomment-790991990>.
src/cmd/ksh93/sh/expand.c: path_expand():
- If we're in the SH_FCOMPLETE (file name completion) state, then
pass the new GLOB_FCOMPLETE flag to AST glob(3).
Fixes: https://github.com/ksh93/ksh/issues/372
Thanks to @fbrau for the bug report.
This commit adds onto <https://github.com/ksh93/ksh/pull/353> by porting
over two additional improvements to the shell linter:
1) The changes in the aforementioned pull request were merged into
illumos-gate with an additional change.[*] The illumos revision of
the patch improved the warning for (( $foo = $? )) to specify '$foo'
causes the warning.[**] Example:
$ ksh -n -c '(( $? != $bar ))'
ksh: warning: line 1: in '(( $? != $bar ))', using '$' as in '$bar' is slower and can introduce rounding errors
While I was porting the illumos patch I did notice one problem. The
string it uses from paramsub() skips over the initial '{' in
'${var}', resulting in the warning printing '$var}' instead:
$ ksh -n -c '(( ${.sh.pid} != $$ ))'
... in '(( ${.sh.pid} != $$ ))', using '$' as in '$.sh.pid}' is slower ...
This was fixed by including the missing '{' in the string returned by
paramsub for ${var} variables.
2) In ksh93v-, parsing x=$((expr)) with the shell linter will cause ksh
to warn the user x=$((expr)) is slower than ((x=expr)). This
improvement has been backported with a modified warning:
# Result from this commit
$ ksh -n -c 'x=$((1 + 2))'
ksh: warning: line 1: x=$((1 + 2)) is slower than ((x=1 + 2))
# Result from ksh93v-
$ ksh93v -n -c 'x=$((1 + 2))'
ksh93v: warning: line 1: ((x=1 + 2)) is more efficient than x=$((1 + 2))
Minor note: the ksh93v- patch had an invalid use of memcmp; this
version of the patch uses strncmp instead.
References:
be548e87bchttps://code.illumos.org/c/illumos-gate/+/1834/comment/65722363_22fdf8e7/
*** Crash 1: ***
ksh crashed if the PS1 prompt contains one or more command
substitutions and you enter a multi-line command substitution
on the command line, then interrupt while on the PS2 prompt.
$ ENV=/./dev/null /usr/local/bin/ksh -o emacs
$ PS1='$(echo foo) $(echo bar) $(echo baz) ! % '
foo bar baz 16999 % echo $(
> true <-- here, press Ctrl+C instead of Return
Memory fault
The crash occurred due to a corrupted lexer state while trying to
display the PS1 prompt.
Analysis: My fix for the crashing bug with Ctrl+C in commit
3023d53b is incorrect and only worked accidentally. sh_fault() is
not the right place to reset the lexer state because, when we press
Ctrl+C on a PS2 prompt, ksh had been waiting for input to finish
lexing a multi-line command, so sh_lex() and other lexer functions
are on the function call stack and will be returned to.
src/cmd/ksh93/sh/fault.c: sh_fault():
- Remove incorrect SIGINT fix.
src/cmd/ksh93/sh/io.c: io_prompt():
- Reset the lexer state immediately before printing every PS1
prompt. Even in situations where this is redundant it should be
perfectly safe, the overhead is negligible, and it resolves this
crash. It may pre-empt other problems as well.
*** Crash 2: ***
If an INT trap is set, and you start entering a multi-line command
substitution, then press Ctrl+C on the PS2 prompt to trigger the
crash, the lexer state is corrupted because the lexer is invoked to
eval the trap action. A crash then occurs on entering the final ')'
of the command substitution.
$ trap 'echo TRAPPED' INT
$ echo $(
> trueTRAPPED <-- press Ctrl+C to output "TRAPPED"
> )
Memory fault
Technically, as SIGINT is trapped, it should not interrupt, so ksh
should execute the trap, then continue with the PS2 prompt to let
the user finish inputting the command. But I have been unsuccessful
in many different attempts to make this work properly. I managed to
get multi-line command substitutions to lex correctly by saving and
restoring the lexer state, but command substitutions were still
corrupted at the parser and/or execution level and I have not
managed to trace the cause of that.
My testing showed that all other shells interrupt the PS2 prompt
and return to PS1 when the user presses Ctrl+C, even if SIGINT is
trapped. I think that is a reasonable alternative, and it is
something I managed to make work.
src/cmd/ksh93/sh/fault.c: sh_chktrap():
- Immediately after invoking sh_trap() to run a trap action, check
if we're in a PS2 prompt (sh.nextprompt == 2). If so, assume the
lexer state is now overwritten. Closing the fcin stream with
fcclose() seems to reliably force the lexer to stop doing
anything else. Then we can just reset the prompt to PS1 and
invoke sh_exit() to start new command line, which will now reset
the lexer state as per above.
ksh crashed if you pressed Ctrl+C or Ctrl+D on a PS2 prompt while
you haven't finished entering a $(command substitution). It
corrupts subsequent command substitutions. Sometimes the situation
recovers, sometimes the shell crashes.
Simple crash reproducer:
$ PS1="\$(echo foo) \$(echo bar) \$(echo baz) > "
foo bar baz > echo $( <-- now press Ctrl+D
> ksh: syntax error: `(' unmatched
Memory fault
The same happens with Ctrl+C, minus the syntax error message.
The problem is that the lexer state becomes inconsistent when the
lexer is interrupted in the middle of reading a command
substitution of the form $( ... ). This is tracked in the
'lexd.dolparen' variable in the lexer state struct.
Resetting that variable is sufficient to fix this issue. However,
in this commit I prefer to just reinitialise the lexer state
completely to pre-empt any other possible issues. Whether there was
a syntax error or the user pressed Ctrl+C, we just interrupted all
lexing and parsing, so the lexer *should* restart from scratch.
src/cmd/ksh93/sh/fault.c: sh_fault():
- If the shell is in an interactive state (e.g. not a subshell) and
SIGINT was received, reinitialise the lexer state. This fixes the
crash with Ctrl+C.
src/cmd/ksh93/sh/lex.c: sh_syntax():
- When handling a syntax error, reset the lexer state. This fixes
the crash with Ctrl+D.
NEWS:
- Also add the forgotten item for the previous fix (re: 2322f939).
This reverts c0334e32, thereby restoring 936a1939.
After the fixes in 0a343244 and a2bc49be, the tilde expansion
disciplines work nicely, so they can come back to the 1.0 branch.
The head and tail builtins don't correctly handle files that lack
newlines[*]:
$ print -n foo > /tmp/bar
$ /opt/ast/bin/head -1 /tmp/bar # No output
$ print -n 'foo\nbar' > /tmp/bar
$ /opt/ast/bin/tail -1 /tmp/bar
foo
bar$
This commit backports the required changes from ksh93v- to handle files
without a newline in the head and tail builtins. (Also note that the
required fix to sfmove was already backported in commit 1bd06207.)
src/lib/libcmd/{head,tail}.c:
- Backport the relevant ksh93v- code for handling files
without newlines.
src/cmd/ksh93/tests/builtins.sh:
- Add a few regression tests for using 'head -1' and 'tail -1' on a file
missing and ending newline.
[*]: https://www.illumos.org/issues/4149
When a global EXIT trap is set, and a ksh-style function exits with
a status > 256 that could have been the result of a signal, then
the shell incorrectly issues that signal to itself. Depending on
the signal, this causes ksh to terminate itself ungracefully:
$ cat /tmp/exit267
trap 'echo OK' EXIT # This trap triggers the crash
function foo { return 267; }
foo
$ bash /tmp/exit267
OK
$ ksh-3aee10d7 /tmp/exit267
OK
$ ksh /tmp/exit267
Memory fault(coredump)
On most systems, status 267 corresponds to SIGSEGV. The reported
memory fault is not real; it results from ksh incorrectly killing
itself with that signal.
The problem is caused by two factors:
1. As of 93u+ 2012-08-01, ksh explicitly allows 'return' to use an
exit status corresponding to a signal (from 257 to end of signal
range). The rest of the integer range is trunctated to 8 bits.
This is contrary to both 'man ksh' and 'return --man' which both
say it's always truncated to 8 bits. Plus, combined with point 2
below, this new behaviour is nonsensical, as 'return' has no
business actually generating signals. However, a couple of
regression tests now depend on this, as may some scripts.
2. When a ksh-style function does not handle a signal, the signal
is passed down to the parent environment and ksh does this by
reissuing the signal to its own process after leaving the
function scope. However, it does this by checking the exit
status, which is very bad practice as there is no guarantee
that an exit status corresponding to a signal was in fact
produced by a signal, particularly after they changed the
behaviour of 'return' per 1 above.
This commit fixes both issues. It also takes a proper decision on
allowable 'return' exit status arguments. Since 93u+ was released
nearly a decade ago and some scripts may now rely on being able to
pass certain exit statuses out of the 8-bit range, we should not
disallow this now. But neither should we be half-hearted in
allowing only some arbitrary selection of 9-bit statuses; 'return'
values categorically should have nothing to do with signals, so
this is no basis for limiting them. We're now allowing the full
unsigned integer range, which is usually 32 bits. This is like zsh,
and may create some interesting possibilities for scripts.
Just don't forget that $? will still lose all but its 8 least
significant bits when leaving the current (sub)shell environment.
src/cmd/ksh93/sh/xec.c: sh_funscope():
- Fix passing down unhandled signals from interrupted ksh functions
(jumpval==SH_JMPFUN) to the parent environment. Do not pay any
attention to the exit status. Instead, use sh.lastsig (a.k.a.
shp->lastsig). It is set by sh_fault() in fault.c for just this
purpose and contains the last signal handled for the current
command. It is reset in sh_exec() before running any new command.
So if it contains a signal, that is the one that interrupted the
ksh function, so it's the correct one to pass down. (Further
evidence: sh_subshell() was already using this in the same way.)
src/cmd/ksh93/bltins/cflow.c: b_return():
- Allow any signed int return value when invoked as and behaving
like 'return'.
- Add warning if a passed value is out of int range. Set the exit
status to 128 in that case; int overflow is undefined behaviour
in C and we want consistent behaviour across platforms. It should
be safe enough to check if the long and int values are equal.
- Refactor for clarity.
src/cmd/ksh93/sh/subshell.c: sh_subshell():
- If a function returns with a status out of the 8 bit range in a
virtual subshell, this status could be passed down to the parent
shell in full. However, if the subshell forks, then the kernel
will enforce an 8-bit exit status. That is inconsistent. Scripts
should not be able to tell the difference between forked and
non-forked subshells, so artificially enforce that limit here.
Other changed files:
- Documentation updates and copy-edits.
- Update an AT&T functions.sh regress test to allow arbitrary
integer return values for functions.
- Add regression tests based in part on @JohnoKing's reproducers.
- Rework some vaguely related regression tests to fail gracefully.
Thanks to Johnothan King for the report and the testing.
Fixes: https://github.com/ksh93/ksh/issues/364
This change adds the -e flag to the cd builtin, as specified in
<https://www.austingroupbugs.net/view.php?id=253>. The -e flag is
used to verify if the the current working directory after 'cd -P'
successfully changes the directory, and returns with exit status 1
if the cwd couldn't be determined. Additionally, it causes all
other errors to return with exit status >1 (i.e., status 2 unless
ENOMEM occurs) if -e and -P are both active.
src/cmd/ksh93/bltins/cd_pwd.c:
- Add -e option to the cd builtin command. It verifies $PWD by
using test_inode() to execute the equivalent of [[ . -ef $PWD ]].
- The check for restricted mode has been moved after optget to
allow 'cd -eP' to return with exit status 2 when in restricted
mode. To avoid changing the previous behavior of cd when -e isn't
passed, extra checks have been added to prevent cd from printing
usage information in restricted mode.
src/cmd/ksh93/tests/builtins.sh:
- Add regression tests for the exit status when using the cd -P
flag with and without -e.
src/cmd/ksh93/data/builtins.c,
src/cmd/ksh93/sh.1:
- Document the addition of -e to the cd builtin.
This commit ports over two of Andy Fiddaman's bugfixes to conf.sh
on illumos:
- The compiler isn't passed on to an invocation of iffe. The bugfix is
from this commit: <https://github.com/citrus-it/ast/commit/63563232>
- The getconf builtin is missing several parameters on illumos.
Reproducer:
$ /opt/ast/bin/getconf ADDRESS_WIDTH
getconf: Invalid argument (ADDRESS_WIDTH) # Should output '64'
This bug occurs because conf.sh expects GNU sed and fails to work
properly with other sed implementations. The bugfix and original bug
report can be found here:
https://www.illumos.org/issues/14044https://github.com/citrus-it/ast/commit/ba443cfd
This patch fixes the crashes experienced when a discipline function
exited because of a signal or an error from a special builtin. The
crashes were caused by ksh entering an inconsistent state after
performing a longjmp away from the assign() and lookup() functions in
nvdisc.c. Fixing the crash requires entering a new context, then setting
a nonlocal goto with sigsetjmp(3). Any longjmps that happen while
running the discipline function will go back to assign/lookup, allowing
ksh to do a proper cleanup afterwards.
Resolves: https://github.com/ksh93/ksh/issues/346
This commit ports over two improvements to the shell linter from
illumos (original patch written by Andy Fiddaman). Links to the
relevant bug reports and the original patch:
https://www.illumos.org/issues/13601https://www.illumos.org/issues/13631c7b656fc71
The first improvement is to the lint warning for arithmetic
operators in [[ ... ]]. The ksh linter now suggests the correct
equivalent operator to use in ((...)). Example:
$ ksh -nc '[[ 30 -gt 25 ]]'
# Original warning
warning: line 1: -gt within [[ ... ]] obsolete, use ((...))
# New warning
warning: line 1: [[ ... -gt ... ]] obsolete, use ((... > ...))
The second improvement pertains to variable expansion in arithmetic
expressions. The ksh linter now suggests referencing variable names
directly:
$ ksh -nc 'integer foo=40; (($foo < 50 ))'
# Old warning
warning: line 1: variable expansion makes arithmetic evaluation less efficient
# New warning
warning: line 1: in '(($foo < 50))', using '$' is slower and can introduce rounding errors
src/cmd/ksh93/{data/lexstates,sh/lex,sh/parse}.c:
- Port the improved shell lint warnings from illumos to ksh93u+m.
- The original checks for arithmetic operators involved a bunch of
if statements with inefficient calls to strcmp(3). These were
replaced with a more efficient switch statement that avoids
strcmp.
This change fixes a crash that can occur after setting a KEYBD trap
then inputting a multi-line command substitution. The crash is
similar to issue #347, but it's easier to reproduce since it
doesn't require you to setup a kshrc file. Reproducer for the
crash:
$ ENV=/./dev/null ksh
$ trap : KEYBD
$ : $(
> true)
Memory fault(coredump)
The bugfix was backported (with considerable changes) from ksh93v-
2013-10-08. The crash was first reported on the old mailing list:
https://www.mail-archive.com/ast-users@lists.research.att.com/msg00313.html
src/cmd/ksh93/{include/shlex.h,sh/lex.c}:
- To fix this properly, we need sizeof(Lex_t) to work as expected
in edit.c, but that is thwarted by the _SHLEX_PRIVATE macro in
lex.c which shlex.h uses to add private structs to the Lex_t type
in lex.c only. So get rid of that _SHLEX_PRIVATE macro and make
those members part of the centrally defined struct, renaming them
to make it clear they're considered private to lex.c.
src/cmd/ksh93/edit/edit.c:
- Now that we can get its size, save and restore the shell lexing
context when a KEYBD trap is present.
src/cmd/ksh93/tests/pty.sh:
- Add a regression test for the KEYBD trap crash.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
Ksh segfaults on M1 Macs, apparently because Apple's compiler
optimizer can't cope with overriding bzero(3) with libast's
implementation (though it's nothing more than "memset(b, 0, n);").
Apple has disabled libast's bzero function since 2018-12-04:
https://opensource.apple.com/source/ksh/ksh-27/patches/src__lib__libast__comp__omitted.c.diff.auto.html
src/lib/libast/comp/omitted.c:
- Only fall back to the libast bzero function if the OS lacks an
implementation of bzero.
- Remove the bzero undef since this fallback is only reached if the
OS doesn't have bzero.
NEWS:
- Add compat info for macOS on ARM64. This notes that macOS
Monterey can now compile and run ksh, although there is one
regression test failure:
builtins.sh[345]: printf %H: invalid UTF-8 characters
(expected %3F%C2%86%3F%3F%3F; got %3F%C2%86%3F%3Fv%3F%3F)
Thanks to @DesantBucie for the report and the testing.
Resolves: https://github.com/ksh93/ksh/issues/329
This bug was first reported in <https://www.illumos.org/issues/7694>.
The time keyword currently overrides the errexit shell option,
allowing failing scripts to continue after an error:
$ cat 1.sh
#!/bin/sh
time false # This should cause the script to exit
echo FAILURE
true
$ ksh -o errexit 1.sh
real 0m0.00s
user 0m0.00s
sys 0m0.00s
FAILURE
src/cmd/ksh93/sh/xec.c:
- When the time keyword runs a command, pass the errexit state flag
to the sh_exec call. This state flag is required for ksh to exit
when a command fails while the errexit option is on.
src/cmd/ksh93/tests/basic.sh:
- Add a regression test based on the reproducer.
This reverts commit 2b9cbbbc8e.
This is not ready for prime time. Crashses when running a $PS2
discipline function. This needs fixing and more testing in
development before making it into the 1.0 branch. In the meantime,
that terrible problem with types is back, sorry about that.
This commit mitigates the effects of the hack explained in the
referenced commit so that dummy built-in command nodes added by the
parser for declaration/assignment purposes do not leak out into the
execution level, except in a relatively harmless corner case.
Something like
if false; then
typeset -T Foo_t=(integer -i bar)
fi
will no longer leave a broken dummy Foo_t declaration command. The
same applies to declaration commands created with enum.
The corner case remaining is:
$ ksh -c 'false && enum E_t=(a b c); E_t -a x=(b b a c)'
ksh: E_t: not found
Since the 'enum' command is not executed, this should have thrown
a syntax error on the 'E_t -a' declaration:
ksh: syntax error at line 1: `(' unexpected
This is because the -c script is parsed entirely before being
executed, so E_t is recognised as a declaration built-in at parse
time. However, the 'not found' error shows that it was successfully
eliminated at execution time, so the inconsistent state will no
longer persist.
This fix now allows another fix to be effective as well: since
built-ins do not know about virtual subshells, fork a virtual
subshell into a real subshell before adding any built-ins.
src/cmd/ksh93/sh/parse.c:
- Add a pair of functions, dcl_hactivate() and dcl_dehacktivate(),
that (de)activate an internal declaration built-ins tree into
which check_typedef() can pre-add dummy type declaration command
nodes. A viewpath from the main built-ins tree to this internal
tree is added, unifying the two for search purposes and causing
new nodes to be added to the internal tree. When parsing is done,
we close that viewpath. This hides those pre-added nodes at
execution time. Since the parser is sometimes called recursively
(e.g. for command substitutions), keep track of this and only
activate and deactivate at the first level.
- We also need to catch errors. This is done by setting libast's
error_info.exit variable to a dcl_exit() function that tidies up
and then passes control to the original (usually sh_exit()).
- sh_cmd(): This is the most central function in the parser. You'd
think it was sh_parse(), but $(modern)-form command substitutions
use sh_dolparen() instead. Both call sh_cmd(). So let's simply
add a dcl_hacktivate() call at the beginning and a
dcl_deactivate() call at the end.
- assign(): This function calls path_search(), which among many
other things executes an FATH search, which may execute arbitrary
code at parse time (!!!). So, regardless of recursion level,
forcibly dehacktivate() to avoid those ugly parser side effects
returning in that context.
src/cmd/ksh93/bltins/enum.c: b_enum():
- Fork a virtual subshell before adding a built-in.
src/cmd/ksh93/sh/xec.c: sh_exec():
- Fork a virtual subshell when detecting typeset's -T option.
Improves fix to https://github.com/ksh93/ksh/issues/256
The -d flag implemented in the rm builtin is completely broken. No
matter what you do it refuses to remove directories, even if -r is
also passed. Reproducer:
$ mkdir /tmp/empty
$ PATH=/opt/ast/bin rm -d /tmp/empty
rm: /tmp/empty: directory
$ PATH=/opt/ast/bin rm -dr /tmp/empty
rm: /tmp/empty: directory not removed [Is a directory]
Additionally, the description of 'rm -d' in the man page contradicts
how it's specified in <https://www.austingroupbugs.net/view.php?id=802>.
The ksh93v- rm builtin fixed nearly all of these issues, so I've
backported it to 93u+m and applied one additional fix for 'rm -rd'.
src/lib/libcmd/rm.c:
- Backported the fixes from the ksh93v- rm builtin's -d flag when
used on empty directories.
- Backported the man page update for rm(1) from ksh93v-.
- The ksh93v- rm builtin had one additional bug that caused the -r
option to fail when combined with -d. This was fixed by
overriding -d if -r is also passed.
src/cmd/ksh93/tests/builtins.sh:
- Add regression tests for the rm builtin's -d option.
POSIXly, '.' loads only files, not functions.
This only applies to '.', not 'source' (which is not in POSIX).
src/cmd/ksh93/bltins/misc.c: b_source():
- For ksh function lookup, add an additional check that we're not
in POSIX mode and running the '.' (SYSDOT) builtin.
Defining a .sh.tilde.get or .sh.tilde.set discipline function to
extend tilde expansion works well as long as the discipline
function doesn't get interrupted (e.g. with Crtl+C) or produce an
error message. Either of those will cause the shell to become
unstable and crash.
This feature is now removed from the 1.0 branch as it is not ready
for prime time. It can return to a release branch if/when we manage
to fix it on the master branch.
Related: https://github.com/ksh93/ksh/issues/346
Within arithmetic expressions, enumeration values of variables of a
type created with the 'enum' command translate to index numbers
from 0 to the number of elements minus 1. However, there was no
range checking on this in the arithmetic subsystem, allowing the
assignment of out-of-range values that did not correspond to any
enumeration value.
Variables of an enum type are internally unsigned short integers
(NV_UINT16), like those created with 'integer -su', except with an
additional discipline function (ENUM_disc).
src/cmd/ksh93/bltins/enum.c,
src/cmd/ksh93/include/builtins.h:
- To implement range checking, the arithmetic system needs access
to the 'nelem' (number of elements) member of 'struct Enum'. This
is only defined locally in enum.c. We could move that to name.h
so arith.c can access it, but enum.c has code that supports
compiling as standalone. So, instead, define a quick extern
function, b_enum_elem(), that does the necessary type conversion
and returns a type's number of elements.
- Add --man documentation for the arithmetic subsystem behaviour
for enum types. Tell the enuminfo() function, which dynamically
inserts values into the documentation, how to process new \f tags
'lastv' (the last-defined value) and 'lastn' (the number of the
last element).
src/cmd/ksh93/sh/arith.c: arith():
- For NV_UINT16 variables with an ENUM_disc discipline, check the
range using b_enum_elem() and error out if necessary.
Resolves: https://github.com/ksh93/ksh/issues/335
This commit adds support for 'stty size' to the stty builtin, as
defined in <https://austingroupbugs.net/view.php?id=1053>. The size
mode is used to display the terminal's number of rows and columns.
Note that stty isn't included in the default list of builtin
commands; testing this addition requires adding CMDLIST(stty) to
the table of builtins in src/cmd/ksh93/data/builtins.c.
src/lib/libcmd/stty.c:
- Add support for the size mode to the stty builtin. This mode is
only used to display the terminal's number of rows and columns,
so error out if any arguments are given that attempt to set the
terminal size.
Parser limitations prevent shcomp or source from handling enum
types correctly:
$ cat /tmp/colors.sh
enum Color_t=(red green blue orange yellow)
Color_t -A Colors=([foo]=red)
$ shcomp /tmp/colors.sh > /dev/null
/tmp/colors.sh: syntax error at line 2: `(' unexpected
$ source /tmp/colors.sh
/bin/ksh: source: syntax error: `(' unexpected
Yet, for types created using 'typeset -T', this works. This is done
via a check_typedef() function that preliminarily adds the special
declaration builtin at parse time, with details to be filled in
later at execution time.
This hack will produce ugly undefined behaviour if the definition
command creating that type built-in is then not actually run at
execution time before the type built-in is accessed.
But the hack is necessary because we're dealing with a fundamental
design flaw in the ksh language. Dynamically addable built-ins that
change the syntactic parsing of the shell language on the fly are
an absurdity that violates the separation between parsing and
execution, which muddies the waters and creates the need for some
kind of ugly hack to keep things like shcomp more or less working.
This commit extends that hack to support enum.
src/cmd/ksh93/sh/parse.c:
- check_typedef():
- Add 'intypeset' parameter that should be set to 1 for typeset
and friends, 2 for enum.
- When processing enum arguments, use AST getopt(3) to skip over
enum's options to find the name of the type to be defined.
(getopt failed if we were running a -c script; deal with this
by zeroing opt_info.index first.)
- item(): Update check_typedef() call, passing lexp->intypeset.
- simple(): Set lexp->intypeset to 2 when processing enum.
The rest of the changes are all to support the above and should be
fairly obvious, except:
src/cmd/ksh93/bltins/enum.c:
- enuminfo(): Return on null pointer, avoiding a crash upon
executing 'Type_t --man' if Type_t has not been fully defined due
to the definition being pre-added at parse time but not executed.
It's all still wrong, but a crash is worse.
Resolves: https://github.com/ksh93/ksh/issues/256
Listing types with 'typeset -T' will list not only types created with
typeset, but also types created with enum. However, the types created
by enum are not displayed correctly in the resulting output:
$ enum Foo_t=(foo bar)
$ typeset -T
typeset -T Foo_t
typeset -T Foo_t=fo)
The fix for this bug was backported from ksh93v- 2013-10-08.
src/cmd/ksh93/sh/nvtype.c:
- sh_outtype(): Skip over enums when listing types with 'typeset -T'.
This commit fixes an issue I found in the subshell $RANDOM
reseeding code.
The main issue is a performance regression in the shbench fibonacci
benchmark, introduced in commit af6a32d1. Performance dropped in
this benchmark because $RANDOM is always reseeded and restored,
even when it's never used in a subshell. Performance results from
before and after this performance fix (results are on Linux with
CC=gcc and CCFLAGS='-O2 -D_std_malloc'):
$ ./shbench -b bench/fibonacci.ksh -l 100 ./ksh-0f06a2e ./ksh-af6a32d ./ksh-f31e368 ./ksh-randfix
benchmarking ./ksh-0f06a2e, ./ksh-af6a32d, ./ksh-f31e368, ./ksh-randfix ...
*** fibonacci.ksh ***
# ./ksh-0f06a2e # Recent version of ksh93u+m
# ./ksh-af6a32d # Commit that introduced the regression
# ./ksh-f31e368 # Commit without the regression
# ./ksh-randfix # Ksh93u+m with this patch applied
-------------------------------------------------------------------------------------------------
name ./ksh-0f06a2e ./ksh-af6a32d ./ksh-f31e368 ./ksh-randfix
-------------------------------------------------------------------------------------------------
fibonacci.ksh 0.481 [0.459-0.515] 0.472 [0.455-0.504] 0.396 [0.380-0.442] 0.407 [0.385-0.439]
-------------------------------------------------------------------------------------------------
src/cmd/ksh93/include/variables.h,
src/cmd/ksh93/sh/{init,subshell}.c:
- Rather than reseed $RANDOM every time a subshell is created, add
a sh_save_rand_seed() function that does this only when the
$RANDOM variable is used in a subshell. This function is called
by the $RANDOM discipline functions nget_rand() and put_rand().
As a minor optimization, sh_save_rand_seed doesn't reseed if it's
called from put_rand().
- Because $RANDOM may have a seed of zero (i.e., RANDOM=0),
sp->rand_seed isn't enough to tell if $RANDOM has been reseeded.
Add sp->rand_state for this purpose.
- sh_subshell(): Only restore the former $RANDOM seed and state if
it is necessary to prevent a subshell leak.
src/cmd/ksh93/tests/variables.sh:
- Add two regression tests for bugs I ran into while making this
patch.
'printf' on bash and zsh has a popular -v option that allows
assigning formatted output directly to variables without using a
command substitution. This is much faster and avoids snags with
stripping final linefeeds. AT&T had replicated this feature in the
abandoned 93v- beta version. This backports it with a few tweaks
and one user-visible improvement.
The 93v- version prohibited specifying a variable name with an
array subscript, such as printf -v var\[3\] foo. This works fine on
bash and zsh, so I see no reason why this should not work on ksh,
as nv_putval() deals with array subscripts just fine.
src/cmd/ksh93/bltins/print.c: b_print():
- While processing the -v option when called as printf, get a
pointer to the variable, creating it if necessary. Pass only the
NV_VARNAME flag to enforce a valid variable name, and not (as
93v- does) the NV_NOARRAY flag to prohibit array subscripts.
- If a variable was given, set the output file to an internal
string buffer and jump straight to processing the format.
- After processing the format, assign the contents to the string
buffer to the variable.
src/cmd/ksh93/data/builtins.c:
- Document the new option, adding a warning that unquoted square
brackets may trigger pathname expansion.
In C/POSIX arithmetic, a leading 0 denotes an octal number, e.g.
010 == 8. But this is not a desirable feature as it can cause
problems with processing things like dates with a leading zero.
In ksh, you should use 8#10 instead ("10" with base 8).
It would be tolerable if ksh at least implemented it consistently.
But AT&T made an incredible mess of it. For anyone who is not
intimately familiar with ksh internals, it is inscrutable where
arithmetic evaluation special-cases a leading 0 and where it
doesn't. Here are just some of the surprises/inconsistencies:
1. The AT&T maintainers tried to honour a leading 0 inside of
((...)) and $((...)) and not for arithmetic contexts outside it,
but even that inconsistency was never quite consistent.
2. Since 2010-12-12, $((x)) and $(($x)) are different:
$ /bin/ksh -c 'x=010; echo $((x)) $(($x))'
10 8
That's a clear violation of both POSIX and the principle of
least astonishment. $((x)) and $(($x)) should be the same in
all cases.
3. 'let' with '-o letoctal' acts in this bizarre way:
$ set -o letoctal; x=010; let "y1=$x" "y2=010"; echo $y1 $y2
10 8
That's right, 'let y=$x' is different from 'let y=010' even
when $x contains the same string value '010'! This violates
established shell grammar on the most basic level.
This commit introduces consistency. By default, ksh now acts like
mksh and zsh: the octal leading zero is disabled in all arithmetic
contexts equally. In POSIX mode, it is enabled equally.
The one exception is the 'let' built-in, where this can still be
controlled independently with the letoctal option as before (but,
because letoctal is synched with posix when switching that on/off,
it's consistent by default).
We're also removing the hackery that causes variable expansions for
the 'let' builtin to be quietly altered, so that 'x=010; let y=$x'
now does the same as 'let y=010' even with letoctal on.
Various files:
- Get rid of now-redundant sh.inarith (shp->inarith) flag, as we're
no longer distinguishing between being inside or outside ((...)).
src/cmd/ksh93/sh/arith.c:
- arith(): Let disabling POSIX octal constants by skipping leading
zeros depend on either the letoctal option being off (if we're
running the "let" built-in") or the posix option being off.
- sh_strnum(): Preset a base of 10 for strtonll(3) depending on the
posix or letoctal option being off, not on the sh.inarith flag.
src/cmd/ksh93/include/argnod.h,
src/cmd/ksh93/sh/args.c,
src/cmd/ksh93/sh/macro.c:
- Remove astonishing hackery that violated shell grammar for 'let'.
src/cmd/ksh93/sh/name.c (nv_getnum()),
src/cmd/ksh93/sh/nvdisc.c (nv_getn()):
- Remove loops for skipping leading zeroes that included a broken
check for justify/zerofill attributes, thereby fixing this bug:
$ typeset -Z x=0x15; echo $((x))
-ksh: x15: parameter not set
Even if this code wasn't redundant before, it is now: sh_arith()
is called immediately after the removed code and it ignores
leading zeroes via sh_strnum() and strtonll(3).
Resolves: https://github.com/ksh93/ksh/issues/334
This commit fixes two file descriptor leaks in the hist built-in.
The bugfix for the first file descriptor leak was backported from
ksh2020. See:
https://github.com/att/ast/issues/872https://github.com/att/ast/commit/73bd61b5
Reproducer:
$ echo no
$ hist -s no=yes
The second file descriptor leak occurs after a substitution error
in the hist built-in (this leak wasn't fixed in ksh2020).
Reproducer:
$ echo no
$ ls /proc/$$/fd
$ hist -s no=yes
$ hist -s no=yes
$ ls /proc/$$/fd
src/cmd/ksh93/bltins/hist.c:
- Close leftover file descriptors when an error occurs and after
'hist -s' runs a command.
src/cmd/ksh93/tests/builtins.sh:
- Add two regression tests for both of the file descriptor leaks.
The --posix compliance option now disables the case-insensitive
special floating point constants Inf and NaN so that all case
variants of $((inf)) and $((nan)) refer to the variables by those
names as the standard requires. (BUG_ARITHNAN)
src/cmd/ksh93/sh/arith.c: arith():
- Only do case-insensitive checks for "Inf" and "NaN" if the POSIX
option is off.
ksh crashed after unsetting .sh.match and then matching a pattern:
$ unset .sh.match
$ [[ bar == ba* ]]
Memory fault
src/cmd/ksh93/sh/init.c: sh_setmatch():
- Do nothing if we cannot get an array pointer to SH_MATCHNOD.
Symptoms:
$ test \( string1 -a string2 \)
/usr/local/bin/ksh: test: argument expected
$ test \( string1 -o string2 \)
/usr/local/bin/ksh: test: argument expected
The parentheses should be irrelevant and this should be a test for
the non-emptiness of string1 and/or string2.
src/cmd/ksh93/bltins/test.c:
- b_test(): There is a block where the case of 'test' with five or
less arguments, the first and last one being parentheses, is
special-cased. The parentheses are removed as a workaround: argv
is increased to skip the opening parenthesis and argc is
decreased by 2. However, there is no corresponding increase of
tdata.av which is a copy of this function's argv. This renders
the workaround ineffective. The fix is to add that increase.
- e3(): Do not handle '!' as a negator if not followed by an
argument. This allows a right-hand expression that is equal to
'!' (i.e. a test for the non-emptiness of the string '!').
In ksh88, the test/[ built-in supported both the '<' and '>'
lexical sorting comparison operators, same as in [[. However, in
every version of ksh93, '<' does not work though '>' still does!
Still, the code for both is present in test_binop():
src/cmd/ksh93/bltins/test.c
548: case TEST_SGT:
549: return(strcoll(left, right)>0);
550: case TEST_SLT:
551: return(strcoll(left, right)<0);
Analysis: The binary operators are looked up in shtab_testops[] in
data/testops.c using a macro called sh_lookup, which expands to a
sh_locate() call. If we examine that function in sh/string.c, it's
easy to see that on systems using ASCII (i.e. all except IBM
mainframes), it assumes the table is sorted in ASCII order.
src/cmd/ksh93/sh/string.c
64: while((c= *tp->sh_name) && (CC_NATIVE!=CC_ASCII || c <= first))
The problem was that the '<' operator was not correctly sorted in
shtab_testops[]; it was sorted immediately before '>', but after
'='. The ASCII order is: < (60), = (61), > (62). This caused '<' to
never be found in the table.
The test_binop() function is also used by [[, yet '<' always worked
in that. This is because the parser has code that directly checks
for '<' and '>' within [[ (in sh/parse.c, lines 1949-1952).
This commit also adds '=~' to 'test', which took three lines of
code and allowed eliminating error handling in test_binop() as
test/[ and [[ now support the same binary ops. (re: fc2d5a60)
src/cmd/ksh93/*/*.[ch]:
- Rename a couple of very misleadingly named macros in test.h:
. For == and !=, the TEST_PATTERN bit is off for pattern compares
and on for literal string compares! Rename to TEST_STRCMP.
. The TEST_BINOP bit does not denote all binary operators, but
only the logical -a/-o ops in test/[. Rename to TEST_ANDOR.
src/cmd/ksh93/bltins/test.c: test_binop():
- Add support for =~. This is only used by test/[. The method is
implemented in two lines that convert the ERE to a shell pattern
by prefixing it with ~(E), then call test_strmatch with that
temporary string to match the ERE and update ${.sh.match}.
- Since all binary ops from shtab_testops[] are now accounted for,
remove unknown op error handling from this function.
src/cmd/ksh93/data/testops.c:
- shtab_testops[]:
. Correctly sort the '<' (TEST_SLT) entry.
. Remove ']]' (TEST_END). It's not an op and doesn't belong here.
- Update sh_opttest[] documentation with =~, \<, \>.
- Remove now-unused e_unsupported_op[] error message.
src/cmd/ksh93/sh/lex.c: sh_lex():
- Check for ']]' directly instead of relying on the removed
TEST_END entry from shtab_testops[].
src/cmd/ksh93/tests/bracket.sh:
- Add relevant tests.
src/cmd/ksh93/tests/builtins.sh:
- Fix an old test that globally deleted the 'test' builtin. Delete
it within the command substitution subshell only.
- Remove the test for non-support of =~ in test/[.
- Update the test for invalid test/[ op to use test directly.
POSIX requires
test "$a" -a "$b"
to return true if both $a and $b are non-empty, and
test "$a" -o "$b"
to return true if either $a or $b is non-empty.
In ksh, this fails if "$a" is '!' or '(' as this causes ksh to
interpret the -a and -o as unary operators (-a being a file
existence test like -e, and -o being a shell option test).
$ test ! -a ""; echo "$?"
0 (expected: 1/false)
$ set -o trackall; test ! -o trackall; echo "$?"
1 (expected: 0/true)
$ test \( -a \); echo "$?"
ksh: test: argument expected
2 (expected: 0/true)
$ test \( -o \)
ksh: test: argument expected
2 (expected: 0/true)
Unfortunately this problem cannot be fixed without risking breakage
in legacy scripts. For instance, a script may well use
test ! -a filename
to check that a filename is nonexistent. POSIX specifies that this
always return true as it is a test for the non-emptiness of both
strings '!' and 'filename'.
So this commit fixes it for POSIX mode only.
src/cmd/ksh93/bltins/test.c: e3():
- If the posix option is active, specially handle the case of
having at least three arguments with the second being -a or -o,
overriding their handling as unary operators.
src/cmd/ksh93/data/testops.c:
- Update 'test --man --' date and say that unary -a is deprecated.
src/cmd/ksh93/sh.1:
- Document the fix under the -o posix option.
- For test/[, explain that binary -a/-o are deprecated.
src/cmd/ksh93/tests/bracket.sh:
- Add tests based on reproducers in bug report.
Resolves: https://github.com/ksh93/ksh/issues/330
Stéphane Chazelas reported:
> As noted in this austin-group-l discussion[*] (relevant to this
> issue):
>
> $ ksh93u+m -c 'pwd; echo "$?" >&2; echo test; echo "$?" >&2' >&-
> 0
> 1
> /home/chazelas
>
> when stdout is closed, pwd does claim it succeeds (by returning a
> 0 exit status), while echo doesn't (not really relevant to the
> problem here, only to show it doesn't affect all builtins), and
> the output that pwd failed to write earlier ends up being written
> on stderr here instead of stdout upon exit (presumably) because
> of that >&2 redirection.
>
> strace shows ksh93 attempting write(1, "/home/chazelas\n", 15) 6
> times (1, the last one, successful).
>
> It gets even weirder when redirecting to a file:
>
> $ ksh93u+m -c 'pwd; echo "$?" >&2; echo test; echo "$?" > file' >&-
> 0
> $ cat file
> 1
> 1
> ome/chazelas
In my testing, the problem does not occur when closing stdout at
the start of the -c script itself (using redirect >&- or exec >&-);
it only occurs if stdout was closed before initialising the shell.
That made me suspect that the problem had to do with an
inconsistent file descriptor state in the shell. ksh uses internal
sh_open() and sh_close() functions, among others, to maintain that
state.
src/cmd/ksh93/sh/main.c: sh_main():
- If the shell is initialised with stdin, stdout or stderr closed,
then make the shell's file descriptor state tables reflect that
fact by calling sh_close() for the closed file descriptors.
This commit also improves the BUG_PUTIOERR fix from 93e15a30. Error
checking after sfsync() is not sufficient. For instance, on
FreeBSD, the following did not produce a non-zero exit status:
ksh -c 'echo hi' >/dev/full
even though this did:
ksh -c 'echo hi >/dev/full'
Reliable error checking requires not only checking the result of
every SFIO command that writes output, but also synching the buffer
at the end of the operation and checking the result of that.
src/cmd/ksh93/bltins/print.c:
- Make exitval variable global to allow functions called by
b_print() to set a nonzero exit status.
- Check the result of all SFIO output commands that write output.
- b_print(): Always sfsync() at the end, except if the s (history)
flag was given. This allows getting rid of the sfsync() call that
required the workaround introduced in 846ad932.
[*] https://www.mail-archive.com/austin-group-l@opengroup.org/msg08056.html
Resolves: https://github.com/ksh93/ksh/issues/314
Bug 1: POSIX requires numbers used as arguments for all the %d,
%u... in printf to be interpreted as in the C language, so
printf '%d\n' 010
should output 8 when the posix option is on. However, it outputs 10.
This bug was introduced as a side effect of a change introduced in
the 2012-02-07 version of ksh 93u+m, which caused the recognition
of leading-zero numbers as octal in arithmetic expressions to be
disabled outside ((...)) and $((...)). However, POSIX requires
leading-zero octal numbers to be recognised for printf, too.
The change in question introduced a sh.arith flag that is set while
we're processing a POSIX arithmetic expression, i.e., one that
recognises leading-zero octal numbers.
Bug 2: Said flag is not reset in a command substitution used within
an arithmetic expression. A command substitution should be a
completely new context, so the following should both output 10:
$ ksh -c 'integer x; x=010; echo $x'
10 # ok; it's outside ((…)) so octals are not recognised
$ ksh -c 'echo $(( $(integer x; x=010; echo $x) ))'
8 # bad; $(comsub) should create new non-((…)) context
src/cmd/ksh93/bltins/print.c: extend():
- For the u, d, i, o, x, and X conversion modifiers, set the POSIX
arithmetic context flag before calling sh_strnum() to convert the
argument. This fixes bug 1.
src/cmd/ksh93/sh/subshell.c: sh_subshell():
- When invoking a command substitution, save and unset the POSIX
arithmetic context flag. Restore it at the end. This fixes bug 2.
Reported-by: @stephane-chazelas
Resolves: https://github.com/ksh93/ksh/issues/326
When invoking a script without an interpreter (#!hashbang) path,
ksh forks, but there is no exec syscall in the child. The existing
command line is overwritten in fixargs() with the name of the new
script and associated arguments. In the generic/fallback version of
fixargs() which is used on Linux and macOS, if the new command line
is longer than the existing one, it is truncated. This works well
when calling a script with a shorter name.
However, it generates a misleading name in the common scenario
where a script is invoked from an interactive shell, which
typically has a short command line. For instance, if "/tmp/script"
is invoked, "ksh" gets replaced with "/tm" in "ps" output.
A solution is found in the fact that, on these systems, the
environment is stored immediately after the command line arguments.
This space can be made available for use by a longer command line
by moving the environment strings out of the way.
src/cmd/ksh93/sh/main.c: fixargs():
- Refactor BSD setproctitle(3) version to be more self-contained.
- In the generic (Linux/macOS) version, on init (i.e. mode==0), if
the command line is smaller than 128 bytes and the environment
strings have not yet been moved (i.e. if they still immediately
follow the command line arguments in memory), then strdup the
environment strings, pointing the *environment[] members to the
new strings and adding the length of the strings to the maximum
command line buffer size.
Reported-by: @gkamat
Resolves: https://github.com/ksh93/ksh/pull/300
ksh93 currently has three command substitution mechanisms:
- type 1: old-style backtick comsubs that use a pipe;
- type 3: $(modern) comsubs that use a temp file, currently with
fallback to a pipe if a temp file cannot be created;
- type 2: ${ shared-state; } comsubs; same as type 3, but shares
state with parent environment.
Type 1 is buggy. There are at least two reproducers that make it
hang. The Red Hat patch applied in 4ce486a7 fixed a hang in
backtick comsubs but reintroduced another hang that was fixed in
ksh 93v-. So far, no one has succeeded in making pipe-based comsubs
work properly.
But, modern (type 3) comsubs use temp files. How does it make any
sense to have two different command substitution mechanisms at the
execution level? The specified functionality between backtick and
modern command substitutions is exactly the same; the difference
*should* be purely syntactic.
So this commit removes the type 1 comsub code at the execution
level, treating them all like type 3 (or 2). As a result, the
related bugs vanish while the regression tests all pass.
The only side effect that I can find is that the behaviour of bug
https://github.com/ksh93/ksh/issues/124 changes for backtick
comsubs. But it's broken either way, so that's neutral.
So this commit can now be added to my growing list of ksh93 issues
fixed by simply removing code.
src/cmd/ksh93/sh/xec.c:
- Remove special code for type 1 comsubs from iousepipe(),
sh_iounpipe(), sh_exec() and _sh_fork().
src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/subshell.c:
- Remove pipe support from sh_subtmpfile(). This also removes the
use of a pipe as a fallback for $(modern) comsubs. Instead, panic
and error out if temp file creation fails. If the shell cannot
create a temporary file, there are fatal system problems anyway
and a script should not continue.
- No longer pass comsub type to sh_subtmpfile().
All other changes:
- Update sh_subtmpfile() calls.
src/cmd/ksh93/tests/subshell.sh:
- Add two regression tests based on reproducers from bug reports.
Resolves: https://github.com/ksh93/ksh/issues/305
Resolves: https://github.com/ksh93/ksh/issues/316
Bug: [[ ! ! 1 -eq 1 ]] returns false, but should return true.
This bug was reported for bash, but ksh has it too:
https://lists.gnu.org/archive/html/bug-bash/2021-06/msg00006.html
Op 24-05-21 om 17:47 schreef Chet Ramey:
> On 5/22/21 2:45 PM, Vincent Menegaux wrote:
>> Previously, these commands:
>>
>> [[ ! 1 -eq 1 ]]; echo $?
>> [[ ! ! 1 -eq 1 ]]; echo $?
>>
>> would both result in `1', since parsing `!' set CMD_INVERT_RETURN
>> instead of toggling it.
>
> Interestingly, ksh93 produces the same result as bash. I agree
> that it's more intuitive to toggle it.
Also interesting is that '!' as an argument to the simple
'test'/'[' command does work as expected (on both bash and ksh93):
'test ! ! 1 -eq 1' and '[ ! ! 1 -eq 1 ]' return 0/true.
Even the man page for [[ is identical for bash and ksh93:
| ! expression
| True if expression is false.
This suggests it's supposed to be a logical negation operator, i.e.
'!' is implicitly documented to negate another '!'. Bolsky & Korn's
1995 ksh book, p. 167, is slightly more explicit about it:
"! test-expression. Logical negation of test-expression."
I also note that multiple '!' negators in '[[' work as expected on
mksh, yash and zsh.
src/cmd/ksh93/sh/parse.c: test_primary():
- Fix bitwise logic for '!': xor the TNEGATE bit into tretyp
instead of or'ing it, which has the effect of toggling it.
This regression also exists on ksh 93v- and ksh2020, from which it
was backported.
Reproducer:
$ (fn() { true; }; fn >/dev/null/ne; true) 2>/dev/null; echo $?
1
Expected output: 0 (as on ksh 93u+).
FreeBSD sh and NetBSD sh are the only other known shells that share
this behaviour. POSIX currently allows both behaviours, but may
require the ksh 93u+ behaviour in future. In any case, this causes
an incompatibility with established ksh behaviour that could easily
break existing ksh scripts.
src/cmd/ksh93/sh/xec.c: sh_exec():
- Commit 23f2e23 introduced a check for jmpval > SH_JMPIO (5).
When a function call pushes context for a redirection, this is
done with the jmpval exit value of SH_JMPCMD (6). Change that to
SH_JMPIO to avoid triggering that check.
src/cmd/ksh93/tests/exit.sh:
- Add regression tests for exit behaviour on various kinds of
shell errors as listed in the POSIX standard, including an error
in a redirection of a function call.
Fixes: https://github.com/ksh93/ksh/issues/310
Problem:
$ exec ksh
$ echo $SHLVL
2
$ exec ksh
$ echo $SHLVL
3
$ exec ksh
$ echo $SHLVL
4
...etc. SHLVL is supposed to acount the number of shell processes
that you need to exit before you get logged out. Since ksh was
replacing itself with a new shell in the same process using 'exec',
SHLVL should not increase.
src/cmd/ksh93/bltins/misc.c: b_exec():
- When about to replace the shell and we're not in a subshell,
decrease SHLVL to cancel out a subsequent increase by the
replacing shell. Bash and zsh also do this.
Since a command substitution no longer forks on non-permanently
redirecting standard output within it for a specific command,
test -t 1, [ -t 1 ], and [[ -t 1 ]] broke as follows:
v=$(test -t 1 >/dev/tty && echo ok) did not assign 'ok' to v.
This is because the assumption in tty_check() that standard output
is never on a terminal in a non-forked command substitution, added
in 55f0f8ce, was made invalid by 090b65e7.
src/cmd/ksh93/edit/edit.c: tty_check():
- Implement a new method. Return false if the file descriptor
stream is of type SF_STRING, which is the case for non-forked
command substitutions -- it means the sfio stream writes directly
into a memory area. This can be checked with the sfset(3)
function (see src/lib/libast/man/sfio.3). To avoid a segfault
when accessing sh.sftable, we need to validate the FD first.
src/cmd/ksh93/tests/pty.sh:
- Add the above reproducer.
In May 2020, when every KornShell (ksh93) development project was
abandoned, development was rebooted in a new fork based on the last
stable AT&T version: ksh 93u+. Now, one year and hundreds of bug
fixes later, the first beta version is ready, and KornShell lives
again. This new fork is called ksh 93u+m as a permanent nod to its
origin; a standard semantic version number is added starting at
1.0.0-beta.1. Please test the beta and report any bugs you find,
or help us fix known bugs.
src/cmd/ksh93/data/math.tab:
- Added exp10().
- Remove int() as being an alias to floor().
- Created entries for local float() and local int() which are
defined in features/math.sh.
src/cmd/ksh93/features/math.sh:
- Backport floor() and int() related code from ksh93v-.
src/cmd/ksh93/sh.1:
- Sync man page to math.tab's potential functions.
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.
