The current fix appears to be only partially successful in
eliminating the intermittent crash, and also breaks '-o notify'
during the 60-second $TMOUT grace period. This replaces it.
The root cause appears to be that the state of job control becomes
somehow inconsistent when running external commands in a command
substitution expanded from the $PS1 prompt. The job_unpost() or
(sometimes) the job_list() function intermittently crash. These are
called if the SH_TTYWAIT state is active:
https://github.com/ksh93/ksh/blob/88e8fa67/src/cmd/ksh93/sh/jobs.c#L463-L469
Temporarily deactivating the SSH_TTYWAIT state while expanding
PS{1..4} prompts appears to fix the problem reliably.
It is quite possible that this fix merely masks a bug in the job
control system, but testing has shown that it stops ksh crashing
without side effects, so I'm calling it good for now.
Thanks to Marc Wilson for many hours of persistent testing.
src/cmd/ksh93/sh/jobs.c:
- Revert changes made in 33858689 and e805c7d9.
src/cmd/ksh93/sh/io.c: io_prompt():
- Save SH_TTYWAIT state and turn it off while expanding prompts.
Resolves: https://github.com/ksh93/ksh/issues/103
Resolves: https://github.com/ksh93/ksh/issues/112
This applies a number of fixes to the printf formatting directives
%H and %#H (as well as their equivalents %(html)q and %(url)q):
1. Both formatters have been made multibyte/UTF-8 aware, and no
longer delete multibyte characters. Invalid UTF-8 byte sequences
are rendered as ASCII question marks.
2. %H no longer wrongly encodes spaces as non-breaking spaces
( ) and instead correctly encodes the UTF-8 non-breaking
space as such.
3. %H now converts the single quote (') to '%#39;' instead of
''' which is not a valid entity in all HTML versions.
4. %#H failed to encode some reserved characters (e.g. '?') while
encoding some unreserved ones (e.g. '~'). It now percent-encodes
all characters except those 'unreserved' as per RFC3986 (ASCII
alphanumeric plus -._~).
Prior discussion:
https://groups.google.com/d/msgid/korn-shell/ce8d1467-4a6d-883b-45ad-fc3c7b90e681%40inlv.org
src/cmd/ksh93/include/defs.h:
src/cmd/ksh93/sh/string.c:
- defs.h: If compiling without SHOPT_MULTIBYTE, redefine the
mbwide() macro (which tests if we're in a multibyte locale) as 0.
This lets the compiler optimiser do the work that would otherwise
require a lot of tedious '#if SHOPT_MULTIBYTE' directives.
- string.c: Remove some now-unneeded '#if SHOPT_MULTIBYTE' stuff.
- defs.h, string.c: Rename is_invisible() to sh_isprint(), invert
the boolean return value, and make it an extern for use in
fmthtml() -- see below. If compiling without SHOPT_MULTIBYTE,
simply #define sh_isprint() as equivalent to isprint(3).
- defs.h: Add URI_RFC3986_UNRESERVED macro for fmthtml() containing
the characters "unreserved" for purposes of URI percent-encoding.
src/cmd/ksh93/bltins/print.c: fmthtml():
- Remove kludge that skipped all multibyte characters (!).
- Complete rewrite to implement fixes described above.
- Don't bother with '#if SHOPT_MULTIBYTE' directives (see above).
src/cmd/ksh93/data/builtins.c:
- sh_optprintf[]: %H: Add single quote to encoded chars doc.
- Edit credits and bump version date.
src/cmd/ksh93/tests/builtins.sh:
- Update and tweak old regression tests.
- Add a number of new tests for UTF-8 HTML and URI encoding, which
are only run when running tests in a UTF-8 locale (shtests -u).
There are convincing arguments why including '.' and '..' in the
result of pathname expansion is actively harmful. See:
https://www.austingroupbugs.net/view.php?id=1228https://github.com/ksh93/ksh/issues/58#issuecomment-653716846
pdksh, mksh and zsh already skip these special traversal names
in all cases. This commit makes ksh act like these shells.
Since passing '.' and especially '..' as arguments to commands like
'chmod -R' and 'cp -r' may cause harm, this change seems likely to
fix more legacy scripts than it breaks. I'm unaware of anyone ever
having come up with a concrete use case for the old behaviour.
This change also fixes the bug that '.' and '..' failed to be
ignored as documented if FIGNORE is set.
src/lib/libast/misc/glob.c: glob_dir():
- Explicitly skip any matching '.' and '..' in all cases.
src/cmd/ksh93/tests/glob.sh:
- Add test_glob() tests for '*' and '.*'.
src/cmd/ksh93/sh.1: File Name Generation:
- Update to match new behaviour.
Resolves: https://github.com/ksh93/ksh/issues/58
The 'redirect' builtin command did not error out before executing
any valid redirections. For example, 'redirect ls >foo.txt' issued
an "incorrect syntax" error, but still created 'foo.txt' and left
standard output permanently redirected to it.
src/cmd/ksh93/sh/xec.c: sh_exec():
- If we have redirections (io != NULL), and the command is
SYSREDIR, then check for arguments and error out if there are
any, before calling sh_redirect() to execute redirections.
(Note, the other check for arguments in b_exec() in bltins/misc.c
must be kept, as that applies if there are no redirections.)
src/cmd/ksh93/sh/io.c: sh_redirect():
- Edit comments to better explain what the flag values do.
src/cmd/ksh93/bltins/misc.c:
- Add a dummy b_redirect() function declaration "for the dictionary
generator" as has historically been done for other builtins that
share one C function. I'm not sure what that dictionary generator
is supposed to be, but this also improves greppability.
src/cmd/ksh93/data/builtins.c,
src/cmd/ksh93/sh.1:
- Fix misleading "I/O redirection arguments" term. I/O redirections
are not arguments at all; no argument parser ever sees them.
src/cmd/ksh93/tests/io.sh:
- Test both conditions that should make 'redirect' produce an
"incorrect syntax" error.
- Test that any redirections are not executed if erroneous
non-redirection arguments exist.
src/cmd/ksh93/tests/builtins.sh:
- "... should show usage info on unrecognized options" test:
Because 'redirect' now refuses to process redirections on error,
the error message was not captured. The fix is to run the builtin
in a braces block and add the redirection to the block.
The crash in job_list() or job_unpost() could still occur after the
previous patch if a signal was being handled after $TMOUT was
exceeded and the 60-second grace period was entered.
It *should* work to add a general check for !sh_isstate(SH_GRACE).
We know that the SH_GRACE state is set immediately after printing
the 60 second grace period warning message:
https://github.com/ksh93/ksh/blob/9de65210/src/cmd/ksh93/sh/io.c#L1869-L1870
(and that the crashes occur upon re-evaluating the $PS1 prompt
after setting the SH_GRACE state). We know that the SH_GRACE state
is not turned off again until either the user enters a line:
https://github.com/ksh93/ksh/blob/9de65210/src/cmd/ksh93/sh/main.c#L474
or the shell times out after the grace period:
https://github.com/ksh93/ksh/blob/9de65210/src/cmd/ksh93/sh/io.c#L1861
The SH_GRACE state flag is not used or changed in any other context
(verified with grep -rn SH_GRACE src/cmd/ksh93). So, logically,
this should suffice to make sure the crash stays gone.
src/cmd/ksh93/sh/jobs.c: job_reap():
- Do not list jobs when the SH_GRACE state (the 60 second timeout
grace period after TMOUT was exceeded) is active.
- Keep the previous check for job control just to be sure, and
because it makes sense.
Fixes: https://github.com/ksh93/ksh/issues/103 (again)
This variable is like Bash's $BASHPID, but in virtual subshells
it will retain its previous value as virtual subshells don't fork.
Both $BASHPID and ${.sh.pid} are different from $$ as the latter
is only set to the parent shell's process ID (i.e. it isn't set
to the process ID of the current subshell).
src/cmd/ksh93/include/defs.h:
- Add 'current_pid' for storing the current process ID at a valid
memory address.
- Change 'ppid' from 'int32_t' to 'pid_t', as the return value from
'getppid' is of the 'pid_t' data type.
src/cmd/ksh93/data/variables.c,
src/cmd/ksh93/include/variables.h,
src/cmd/ksh93/sh/init.c,
src/cmd/ksh93/sh/xec.c:
- Add the ${.sh.pid} variable as an alternative to $BASHPID.
The process ID is stored in a struct before ${.sh.pid} is set
as environment variables are pointers that must point to a
valid memory address. ${.sh.pid} is updated by the _sh_fork()
function, which is called when ksh forks a new process with
sh_fork() or sh_ntfork().
src/cmd/ksh93/tests/variables.sh:
- Add ${.sh.pid} to the list of special variables and add three
regression tests for ${.sh.pid}.
src/cmd/ksh93/tests/subshell.sh:
- Update the PATH forking regression test to use ${.sh.pid} and
remove the TODO note.
This bug caused an undefined state, which sometimes crashed the
shell in job_list() or job_unpost(), if $PS1 contains a command
substitution running an external command and the '-b'/'-o notify'
shell option is active. So far the only known way to trigger the
crash is by letting $TMOUT time out the interactive shell. See
https://github.com/ksh93/ksh/issues/103 for details.
src/cmd/ksh93/sh/jobs.c: job_reap():
- The check for the SH_NOTIFY option and the SH_TTYWAIT state
before listing jobs was insufficient. Job control is disabled in
command substitutions, so also check that job control is active
before listing jobs.
src/cmd/ksh93/sh.1:
- Fix TMOUT documentation. The 'read' command in fact only times
out when reading from a terminal, just like 'select'. Also
document the extra 60 second grace period when an interactive
shell prompt reads from a terminal.
Fixes: https://github.com/ksh93/ksh/issues/103
This commit fixes two bugs in the generation of $'...' shellquoted
strings:
1. A bug introduced in f9d28935. In UTF-8 locales, a byte that is
invalid in UTF-8, e.g. hex byte 86, would be shellquoted as
\u[86], which is not the same as the correct quoting, \x86.
2. A bug inherited from 93u+. Single bytes (e.g. hex 11) were
always quoted as \x11 and not \x[11], even if a subsequent
character was a hexadecimal digit. However, the parser reads
past two hexadecimal digits, so we got:
$ printf '%q\n' $'\x[11]1'
$'\x111'
$ printf $'\x111' | od -t x1
0000000 c4 91
0000002
After the bug fix, this works correctly:
$ printf '%q\n' $'\x[11]1'
$'\x[11]1'
$ printf $'\x[11]1' | od -t x1
0000000 11 31
0000002
src/cmd/ksh93/sh/string.c: sh_fmtq():
- Make the multibyte code for $'...' more readable, eliminating the
'isbyte' flag.
- When in a multibyte locale, make sure to shellquote both invalid
multibyte characters and unprintable ASCII characters as
hexadecimal bytes (\xNN). This reinstates 93u+ behaviour.
- When quoting bytes, use isxdigit(3) to determine if the next
character is a hex digit, and if so, protect the quoted byte with
square brackets.
src/cmd/ksh93/tests/quoting2.sh:
- Move the 'printf %q' shellquoting regression tests here from
builtins.sh; they test the shellquoting algorithm, not so much
the printf builtin itself.
- Add regression tests for these bugs.
A segfault happens when an array with an unset method
is turned into a multidimensional array. Reproducer:
function foo {
typeset -a a
a.unset() {
print unset
}
a[3][6][11][20]=7
}
foo
src/cmd/ksh93/sh/nvdisc:
- Fix the multidimensional array unset method crash by
checking if np->nvenv is an array, since multidimensional
arrays need to be handled as arrays. This bugfix was
backported from ksh93v- 2013-10-10-alpha.
src/cmd/ksh93/tests/arrays2.sh:
- Add the reproducer as a regression test for the crash
with multidimensional arrays.
Bug report on the old mailing list:
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01195.html
The required longjmp used to terminate scripts was not being run
when over-shifting in a POSIX function with a redirection. This
caused scripts to continue after an error in the shift builtin,
which is incorrect since shift is a special builtin. The
interpreter is sent into an indeterminate state that causes
undefined behavior as well:
$ cat reproducer.ksh
some_func() {
shift 10
}
for i in a b c d e f; do
echo "read $i"
[ "$i" != "c" ] && continue
some_func 2>&1
echo "$i = c"
done
$ ksh ./reproducer.ksh
read a
read b
read c
/tmp/k[2]: shift: 10: bad number
c = c
read d
/tmp/k[2]: shift: 10: bad number
d = c
read e
/tmp/k[2]: shift: 10: bad number
e = c
read f
/tmp/k[2]: shift: 10: bad number
f = c
src/cmd/ksh93/sh/xec.c: sh_exec():
- Do the necessary longjmp needed to terminate the script after
over-shifting in a POSIX function when the function call has a
redirection.
src/cmd/ksh93/tests/functions.sh:
- Add the over-shifting regression test from ksh93v- 2013-10-10-alpha.
Bug report and fix on the old mailing list:
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg00732.html
A multibyte character immediately following an expansion of a
single-character name, e.g. $1 through $9, $?, $-, etc. was
corrupted when in a UTF-8 locale, e.g.:
$ set -- foo; echo "$1テスト"
foo?スト
Prior discussion:
https://www.mail-archive.com/ast-users@lists.research.att.com/msg01060.htmlhttps://bugzilla.redhat.com/show_bug.cgi?id=1256495
src/cmd/ksh93/sh/macro.c:
- Apply a Red Hat patch by Paulo Andrade that avoids calling
fcmbget() if backtracking more than one byte might be required.
src/cmd/ksh93/tests/basic.c:
- Test "テスト" following expansion of "$1", "$?" and "$#".
Co-authored-by: Martijn Dekker <martijn@inlv.org>
Multidimensional associative arrays are created with an extra array
member named '0', which is set to no value. Reproducer:
$ typeset -A foo
$ typeset -A foo[bar]
$ typeset -p foo
typeset -A foo=([bar]=([0]='') )
The bugfix prevents nv_setarray from creating the extra '[0]' member
when an associative array is empty. This bug was discussed on the old
mailing list:
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01574.html
src/cmd/ksh93/sh/array.c:
- Do not allow the creation of an extra array member when an array
is empty.
src/cmd/ksh93/tests/arrays.sh:
- Add a regression test for creating multidimensional associative
arrays, but use the output from 'typeset -p' instead of fgrep.
When the classic fork/exec mechanism was used (via sh_fork()) to
run an external command from within a non-forking subshell, SIGINT
was blocked until that subshell was exited. If a subsequent loop
was run in the subshell, it became uninterruptible, e.g.:
$ arch/*/bin/ksh -c '(/usr/bin/true; while :; do :; done); exit'
^C^C^C^C^C
src/cmd/ksh93/sh/xec.c:
- sh_fork() did not reset the savesig variable in the parent part
of the fork when running in a virtual subshell. This had the
effect of delaying signal handling until exiting the subshell.
There is no reason for that subshell check that I can discern, so
this removes it.
I've verified that this causes no regression test failures
even when ksh is compiled with -DSHOPT_SPAWN=0 which means the
classic fork/exec mechanism is always used.
Fixes: https://github.com/ksh93/ksh/issues/86
If an array or upper/lowercase variable was declared with a null
initial value within a virtual/non-forked subshell, like:
( typeset -a foo; ... )
( typeset -A foo; ... )
( typeset -l foo; ... )
( typeset -u foo; ... )
then the type declaration leaked out of the subshell into the
parent shell environment, though without any values that may
subsequently have been assigned.
src/cmd/ksh93/bltins/typeset.c: setall():
- When deciding whether to create a virtual subshell scope for a
variable, use sh_assignok(), which was actually designed for the
purpose, instead of _nv_unset(). This allows getting rid of a
tangled mess of special-casing that never worked quite right.
src/cmd/ksh93/tests/arrays.sh:
- Add regression tests checking that array declarations don't leak
out of virtual subshells.
src/cmd/ksh93/tests/attributes.sh:
- Add regression tests for combining the 'export' and 'readonly'
attributes with every other possible typeset attribute on unset
variables. This also includes a subshell leak test for each one.
Fixes: https://github.com/ksh93/ksh/issues/88
When a builtin is given an unrecognized option, the usage information
for that builtin should be shown as 'Usage: builtin-name options'. The
sleep and suspend builtins were an exception to this. 'suspend' would
not show usage information and sleep wouldn't exit on error:
$ suspend -e
/usr/bin/ksh: suspend: -e: unknown option
$ time sleep -e 1
sleep: -e: unknown option
real 0m1.00s
user 0m0.00s
sys 0m0.00s
src/cmd/ksh93/bltins/sleep.c:
- Show usage information and exit when sleep is given an unknown
option. This bugfix was backported from ksh2020: https://github.com/att/ast/pull/1024
src/cmd/ksh93/bltins/trap.c:
- Use the normal method of parsing options with optget to fix the
suspend builtin's test failure.
src/cmd/ksh93/tests/builtins.sh:
- Add the ksh2020 regression test for getting the usage information
of each builtin. Enable all /opt/ast/bin builtins in a subshell
since those should be tested as well (aside from getconf and uname
because those builtins fallback to the real commands on error).
Add support for multibyte characters to $IFS
This commit fixes BUG_MULTIBIFS, which had two bug reports in the ksh2020 branch.
src/cmd/ksh93/sh/macro.c:
- Backport Eric Scrivner's fix for multibyte IFS characters (slightly modified
for compatibility with C89). Explanation from https://github.com/att/ast/pull/737:
Previously, the varsub method used for the macro expansion of $param, ${param},
and ${param op word} would incorrectly expand the internal field separator (IFS)
if it was a multibyte character. This was due to truncation based on the
incorrect assumption that the IFS would never be larger than a single byte.
This change fixes this issue by carefully tracking the number of bytes that
should be persisted in the IFS case and ensuring that all bytes are written
during expansion and substitution.
Bug report: https://github.com/att/ast/issues/13
- Fixed another bug that caused multibyte characters with the same initial byte
to be treated as the same character by the IFS. This bug was occurring because
the first byte of a multibyte character wasn't being written to the stack when
the IFS delimiter had the same initial byte:
$ IFS=£
$ v='§'
$ set -- $v
$ v="${1-}"
$ echo "$v" | hd # The first byte should be c2, but it isn't due to the bug
00000000 a7 0a |..|
00000002
Bug report: https://github.com/att/ast/issues/1372
src/cmd/ksh93/tests/variables.sh:
- Add (reworked) regression tests from ksh2020 for the multibyte IFS bugs.
- Add a regression test for att/ast#1372 based on the reproducer.
The following explanation is mostly taken from Tomas Klacko's report on
the old mailing list (which also contains a C program reproducer) [*]:
1. When ksh starts a binary, it sets its environment variable "_"
to "*number*/path/to/binary". Where "number" is the pid of the
ksh process.
2. The binary forks and the child executes a suid root shell script
which begins with #!/bin/sh. For this bug to occur, ksh must be /bin/sh.
3. The ksh process interpreting the suid shell script leaves the "_"
variable as not set (nv_getval(L_ARGNOD) returns NULL) because
the "number" from step 1 is not the pid of its parent process.
4-5. Because "_" is not set and the script is suid root, an infinite
loop occurs because when the SHELL environment variable contains
"/bin/sh" pathshell() returns "/bin/sh". This becomes an infinite
loop of /bin/sh /dev/fd/3 executing /bin/sh /dev/fd/3.
src/cmd/ksh93/sh/init.c: get_lastarg():
- Disable the check for if the "number" refers to the process id of
the parent process.
src/cmd/ksh93/sh/main.c: sh_main():
- Prevent an infinite loop when '$_' is not passed in from the environment.
Solaris applies this bugfix to their version of ksh:
https://github.com/oracle/solaris-userland/blob/master/components/ksh93/patches/190-17432413.patch
[*]: https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01680.html
When a command substitution is run on the same line as a here-document,
a syntax error occurs due to a regression introduced in ksh93u+ 2011-04-15:
true << EOF; true $(true)
EOF
syntax error at line 1: `<<EOF' here-document not contained within command substitution
The regression is caused by an error check that was added to make
the following script causes a syntax error (because the here-document
isn't completed inside of the command substitution):
$(true << EOF)
EOF
src/cmd/ksh93/sh/lex.c:
- Only throw an error when a here-document in a command substitution
isn't completed inside of the command substitution.
src/cmd/ksh93/tests/heredoc.sh:
- Add a regression test for running a command substitution on the
same line as a here-document.
- Add a missed regression test for using here-documents in command
substitutions. This is the original bug that was fixed in ksh93u+
2011-04-15 (it is why the error message was added), but a regression
test for here-documents in command substitutions wasn't added in
that version.
This bugfix was backported from ksh93v- 2013-10-10-alpha.
When ksh is compiled with SHOPT_SPAWN (the default), which uses
posix_spawn(3) or vfork(2) (via sh_ntfork()) to launch external
commands, at least two race conditions occur when launching
external commands while job control is active. See:
https://bugs.launchpad.net/ubuntu/+source/ksh/+bug/1887863/comments/3https://www.mail-archive.com/ast-developers@research.att.com/msg00717.html
The basic issue is that this performance optimisation is
incompatible with job control, because it uses a spawning mechanism
that doesn't copy the parent process' memory pages into the child
process, therefore no state that involves memory can be set before
exec-ing the external program. This makes it impossible to
correctly set the terminal's process group ID in the child process,
something that is essential for job control to work.
src/cmd/ksh93/sh/xec.c:
- Use sh_fork() instead of sh_ntfork() if job control is active.
This uses fork(2), which is 30%-ish slower on most sytems, but
allows for correctly setting the terminal process group.
src/cmd/ksh93/tests/basic.sh:
- Add regression test for the race condition reported in #79.
src/cmd/INIT/cc.darwin:
- Remove hardcoded flag to disable SHOPT_SPAWN on the Mac.
It should be safe to use now.
Fixes https://github.com/ksh93/ksh/issues/79
A file descriptor (at least 3, can't reproduce for 4 and up) opened
with 'exec' or 'redirect' in a virtual/non-forked subshell survived
that subshell after exiting it:
$ ksh -c '(redirect 3>&1); echo bug >&3'
bug
src/cmd/ksh93/sh/io.c:
- Apply a patch from OpenSUSE (ksh93-redirectleak.dif). Source:
https://build.opensuse.org/package/show/openSUSE:Leap:42.3:Update/ksh
src/cmd/ksh93/tests/io.sh:
- Add regression test.
Thanks to Marc Wilson for flagging this up.
'whence -a' is documented to list all possible interpretations of a
command, but failed to list a built-in command if a shell function
by the same name exists or is marked undefined using 'autoload'.
src/cmd/ksh93/bltins/whence.c: whence():
- Refactor and separate the code for reporting functions and
built-in commands so that both can be reported for one name.
src/cmd/ksh93/data/builtins.c: sh_optwhence[]:
- Correct 'whence --man' to document that:
* 'type' is equivalent to 'whence -v'
* '-a' output is like '-v'
src/cmd/ksh93/tests/builtins.sh:
- Test 'whence -a' with these combinations:
* a function, built-in and external command
* an undefined/autoload function, built-in and external command
Fixes https://github.com/ksh93/ksh/issues/83
This commit fixes two different crashes related to kshdb:
- When redirect is given an invalid file descriptor, a segfault
no longer occurs. Reproducer:
$ ksh -c 'redirect 9>&200000000000'
- Fix a crash due to free(3) being used on an invalid pointer.
This can be reproduced with kshdb (commands from att/ast#582):
$ git clone https://github.com/rocky/kshdb.git
$ cd kshdb
$ ksh autogen.sh
$ echo "print hi there" > $HOME/.kshdbrc
$ ./kshdb -L . test/example/dbg-test1.sh
src/cmd/ksh93/bltins/misc.c: b_dot_cmd():
- The string pointed to by shp->st.filename must be able to be
freed from memory with free(3), so duplicate the string with
strdup(3).
src/cmd/ksh93/sh/io.c: sh_redirect():
- Show an error message when a file descriptor is invalid to
fix a memory fault.
This commit backports the main changes to sh_delay from ksh93v-
and ksh2020, which fixes the following bugs:
- Microsecond amounts of less than one millisecond are no longer
ignored. The following loop will now take a minimum of one
second to complete:
for ((i = 0; i != 10000; i++)) do
sleep PT100U
done
- 'sleep 30' no longer adds an extra 30 milliseconds to the total
amount of time to sleep. This bug is hard to notice since 30
milliseconds can be considered within the margin of error. The
only reason why longer delays weren't affected is because the old
code masked the bug when the interval is greater than 30 seconds:
else if(n > 30)
{
sleep(n);
t -= n;
}
This caused 'sleep -s' to break with intervals greater than 30
seconds, so an actual fix is used instead of a workaround.
- 'sleep -s' now functions correctly with intervals of more than
30 seconds as the new code doesn't need the old workaround. This
is done by handling '-s' in sh_delay.
src/cmd/ksh93/bltins/sleep.c:
- Remove the replacement for sleep(3) from the sleep builtin.
- Replace the old sh_delay function with the newer one from ksh2020.
The new function uses tvsleep, which uses nanosleep(3) internally.
src/cmd/ksh93/include/shell.h,
src/cmd/ksh93/edit/edit.c,
src/cmd/ksh93/sh/jobs.c,
src/cmd/ksh93/sh/xec.c,
src/cmd/ksh93/shell.3:
- Update sh_delay documentation and usage since the function now
requires two arguments.
src/cmd/ksh93/tests/builtins.sh:
- Add a regression test for 'sleep -s' when the interval is greater
than 30 seconds. The other bugs can't be tested for in a feasible
manner across all systems:
https://github.com/ksh93/ksh/pull/72#issuecomment-657215616
With this change no more preset aliases exist, so the preset alias
tables can be safely removed. All ksh commands can now be used
without 'unalias -a' removing them, even in interactive shells.
Additionally, the history and r commands are no longer limited to
being used in interactive shells.
src/cmd/ksh93/bltins/hist.c:
- Implement the history and r commands as builtins. Also guarantee
lflag is set to one by avoiding 'lflag++'.
src/cmd/ksh93/Makefile,
src/cmd/ksh93/Mamfile,
src/cmd/ksh93/sh/main.c,
src/cmd/ksh93/sh/init.c,
src/cmd/ksh93/data/aliases.c:
- Remove the table of predefined aliases because the last few have
been removed. During init the alias tree is now initialized the
same way as the function tree.
src/cmd/ksh93/bltins/typeset.c:
- Remove the bugfix for unsetting predefined aliases because it is
now a no-op. Aliases are no longer able to have the NV_NOFREE
attribute.
src/cmd/ksh93/tests/alias.sh:
- Remove the regression test for unsetting predefined aliases since
those no longer exist.
src/cmd/ksh93/data/builtins.c:
- Update sh_opthist[] for 'hist --man', etc.
src/cmd/ksh93/sh.1:
- Remove the list of preset aliases since those no longer exist.
- Document history and r as builtins instead of preset aliases.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
These two default aliases are useful on interactive shells. In
scripts, they interfere with possible function or command names.
As of this commit, these final two default aliases are only loaded
for interactive shells, leaving zero default aliases for scripts.
This completes the project to get rid of misguided default aliases.
src/cmd/ksh93/include/shtable.h,
src/cmd/ksh93/data/aliases.c:
src/cmd/ksh93/sh/init.c:
- Add empty alias table shtab_noaliases[] for scripts.
- Rename inittree() to sh_inittree() and make it external.
- nv_init(), sh_reinit(): Initialise empty alias tree for scripts.
src/cmd/ksh93/sh/main.c: sh_main():
- If interactive, reinitialise alias tree for interactive shells.
src/cmd/ksh93/tests/alias.sh:
- To test default alias removal, launch shell with -i.
Many terminals (xterm being one example) give the Home and End keys
the escape sequences '^[[H' and '^[[F'. The first sequence is
handled in both editing modes by moving the cursor to start of
line, but ksh ignored the second sequence.
src/cmd/ksh93/edit/emacs.c,
src/cmd/ksh93/edit/vi.c:
- Add case labels for '^[[F' so that in both editing modes the End
key moves the cursor to the end of the line.
This converts the 'autoload', 'compound', 'float', 'functions',
'integer' and 'nameref' default aliases into regular built-in
commands, so that 'unalias -a' does not remove them. Shell
functions can now use these names, which improves compatibility
with POSIX shell scripts.
src/cmd/ksh93/data/aliases.c:
- Remove default typeset aliases.
src/cmd/ksh93/data/builtins.c,
src/cmd/ksh93/include/builtins.h:
- Add corresponding built-in command declarations. Typeset-style
commands are now defined by a pointer range, SYSTYPESET ..
SYSTYPESET_END. A couple need their own IDs (SYSCOMPOUND,
SYSNAMEREF) for special-casing in sh/xec.c.
- Update 'typeset --man'.
src/cmd/ksh93/bltins/typeset.c: b_typeset():
- Recognise the new builtin commands by argv[0]. Implement them by
inserting the corresponding 'typeset' options into the argument
list before parsing options. This may seem like a bit of a hack,
but it is simpler, shorter, more future-proof and less
error-prone than manually copying and adapting all the complex
flaggery from the option parsing loop.
src/cmd/ksh93/sh/parse.c,
src/cmd/ksh93/sh/xec.c:
- Recognise typeset-style commands by SYSTYPESET .. SYSTYPESET_END
pointer range.
- Special-case 'compound' (SYSCOMPOUND) and 'nameref' (SYSNAMEREF)
along with recognising the corresponding 'typeset' options.
src/cmd/ksh93/sh.1:
- Update to document the new built-ins.
- Since not all declaration commands are special built-ins now,
identify declaration commands using a double-dagger "\(dd"
character (which renders as '=' in ASCII) and disassociate their
definition from that of special built-ins.
src/cmd/ksh93/tests/variables.sh:
- Adapt a regression test as there is no more 'integer' alias.
Now that we have an iffe feature test for getrusage(3), introduced
in 70fc1da7, the millisecond-precision 'times' command from the
last version of ksh2020 can easily be backported.
src/cmd/ksh93/bltins/misc.c:
- Incorporate ksh2020 'times' command, with a couple of tweaks:
* Use locale's radix point instead of '.'.
* Pad seconds with initial zero if < 10.
src/cmd/ksh93/data/builtins.c:
- Update version date for 'times --man'.
src/cmd/ksh93/tests/builtins.sh:
- Update 'times' test for 3 digits after radix point.
This commit backports the required fixes from ksh2020 for using
millisecond precision with the 'time' keyword. The bugfix refactors
a decent amount of code to rely on the BSD 'timeradd' and
'timersub' macros for calculating the total amount of time elapsed
(as these aren't standard, they are selectively implemented in an
iffe feature test for platforms without them). getrusage(3) is now
preferred since it usually has higher precision than times(3) (the
latter is used as a fallback).
There are three other fixes as well:
src/lib/libast/features/time:
- Test for getrusage with an iffe feature test rather than
assume _sys_times == _lib_getrusage.
src/cmd/ksh93/sh/xec.c:
- A single percent at the end of a format specifier is now
treated as a literal '%' (like in Bash).
- Zero-pad seconds if seconds < 10. This was already done for
the times builtin in commit 5c677a4c, although it wasn't
applied to the time keyword.
- Backport the ksh2020 bugfix for the time keyword by using
timeradd and timersub with gettimeofday (which is used with
a timeofday macro). Prefer getrusage when it is available.
- Allow compiling without the 'timeofday' ifdef for better
portability.
This is the order of priority for getting the elapsed time:
1) getrusage (most precise)
2) times + gettimeofday (best fallback)
3) only times (doesn't support millisecond precision)
This was tested by using debug '#undef' statements in xec.c.
src/cmd/ksh93/features/time:
- Implement feature tests for the 'timeradd' and 'timersub'
macros.
- Do a feature test for getrusage like in the libast time test.
src/cmd/ksh93/tests/basic.sh:
- Add test for millisecond precision.
- Add test for handling of '%' at the end of a format specifier.
- Add test for locale-specific radix point.
'set -b' had no effect; it should cause the shell to notify job
state changes immediately instead of waiting for the next prompt.
This fixes a regression that was introduced in ksh93t 2008-07-25.
The bugfix is from: https://github.com/att/ast/pull/1089
src/cmd/ksh93/sh/jobs.c:
- Save the tty wait state and avoid changing it if TTYWAIT was
already on to avoid breaking 'set -b'.
The last 'sh_offstate' is inside of an '#if' directive because it
is only required when ksh is compiled with SHOPT_COSHELL enabled.
src/cmd/ksh93/tests/pty.sh:
- Add a regression test for 'set -b' in interactive shells.
This applies ksh-20100621-fdstatus.patch from Red Hat. Not very
much information is available, so this one is more or less taken
on faith. But it seems to make sense on the face of it: calling
sh_fcntl() instead of fcntl(2) directly makes the shell update its
internal file descriptor state more frequently.
It claims to fix Red Hat bug 924440. The report is currently closed
to the public: https://bugzilla.redhat.com/show_bug.cgi?id=924440
However, Kamil Dudka at Red Hat writes:
https://github.com/ksh93/ksh/issues/67#issuecomment-656379993
| Yes, the summary of RHBZ#924440 is "crash in bestreclaim() after
| traversing a memory block with a very large size". We did not have
| any in house reproducer for the bug. The mentioned patch was
| provided and verified by a customer.
...and Marc Wilson dug up a Red Hat erratum containing this info:
https://download.rhn.redhat.com/errata/RHBA-2013-1599.html
| Previously, the ksh shell did not resize the file descriptor list
| every time it was necessary. This could lead to memory corruption
| when several file descriptors were used. As a consequence, ksh
| terminated unexpectedly. This updated version resizes the file
| descriptor list every time it is needed, and ksh no longer
| crashes in the described scenario. (BZ#924440)
No reproducer means no regression test can be added now.
src/cmd/ksh93/sh/io.c,
src/cmd/ksh93/sh/subshell.c,
src/cmd/ksh93/sh/xec.c:
- Change several fcntl(2) calls to sh_fcntl(). This function calls
fcntl(2) and then updates the shell's file descriptor state.
Type names that start with a lowercase 'a' cause an error when used:
$ typeset -T al=(typeset bar)
$ al foo=(bar=testset)
/usr/bin/ksh: al: : invalid variable name
The error occurs because when the parser checks for the alias
builtin (to set 'assignment' to two instead of one), only the first
letter of 'argp->argval' is checked (rather than the entire
string). This was fixed in ksh93v- by comparing argp->argval
against "alias", but in ksh93u+m the check can simply be removed
because it is only run when a builtin has the BLT_DCL flag. As of
04b9171, the alias builtin does not have that flag.
src/cmd/ksh93/sh/parse.c:
- Remove the bugged check for the alias builtin.
src/cmd/ksh93/tests/types.sh:
- Add a regression test for type names starting with a lowercase 'a'.
This fixes an annoying issue in the shell's quoting algorithm
(used for xtrace (set -x), printf %q, and other things) for UTF-8
locales, that caused it to encode perfectly printable UTF-8
characters unnecessarily and inconsistently. For example:
$ (set -x; : 'aeu aéu')
+ : $'aeu a\u[e9]u'
$ (set -x; : 'aéu aeu')
+ : 'aéu aeu'
$ (set -x; : '正常終了 aeu')
+ : '正常終了 aeu'
$ (set -x; : 'aeu 正常終了')
+ : $'aeu \u[6b63]\u[5e38]\u[7d42]\u[4e86]'
This issue was originally reported by lijo george in May 2017:
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01958.html
src/cmd/ksh93/sh/string.c:
- Add is_invisible() function that returns true if a character is a
Unicode invisible (non-graph) character, excluding ASCII space.
Ref.: https://unicode.org/charts/PDF/U2000.pdf
- Use a fallback in is_invisible() if we cannot use the system's
iswprint(3); this is the case for the ksh C.UTF-8 locale if the
OS doesn't support that. Fall back to a hardcoded blacklist of
invisible and control characters and put up with not encoding
nonexistent characters into \u[xxxx] escapes.
Ref.: https://unicode.org/charts/PDF/U2000.pdf
- When deciding whether to switch to $'...' quoting mode (state=2),
use is_invisible() instead of testing for ASCII 0-127 range.
- In $'...' quoting mode, use is_invisible() to decide whether to
encode wide characters into \u[xxxx] escapes.
src/cmd/ksh93/tests/builtins.sh:
- Add regression tests for shellquoting Arabic, Japanese and Latin
UTF-8 characters, to be run only in a UTF-8 locale. The Arabic
sample text[*] contains a couple of direction markers that are
expected to be encoded into \u[xxxx] escapes.
[*] source: https://r12a.github.io/scripts/tutorial/summaries/arabic
The ksh -R option creates a cross-reference database that can be
parsed with a "C Query Language" (CQL) tool.
See cql-1994.pdf at: http://gsf.cococlyde.org/files
The -R option puts ksh in noexec mode as it parses the script, and
this can produce warnings as the syntax is parsed. The bug is that
these warnings can end up in the database file, corrupting it.
This applies a fix from Paulo Andrade, via Siteshwar Vashisht:
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01952.html
src/cmd/ksh93/sh/parse.c:
- Terminate names with a zero character when writing database
output.
A regression test is not very feasible because the majority of the
database output consists of cryptic IDs/hashes that vary depending
on the session and/or system and possibly other things.
This bugfix was backported from ksh93v- 2013-10-10-alpha.
src/cmd/ksh93/sh/parse: item():
- The done label is placed after the 'inout' call for handling I/O
redirections. This causes the command below to produce a syntax
error because the '>' is not handled as a redirection operator
after 'goto done':
$ ((1+2)) > /dev/null
/usr/bin/ksh: syntax error: `>' unexpected
Moving the done label fixes the syntax error as 'inout' is now
called to handle the redirection operator.
src/cmd/ksh93/tests/arith.sh:
- Add a simple regression test.
There is a bug in path_alias() that may cause a memory leak when
clearing the hash table while setting/restoring PATH.
This applies a fix from Siteshwar Vashist:
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01945.html
Note that, contrary to Siteshwar's analysis linked above, this bug
has nothing directly to do with subshells, forked or otherwise; it
can also be reproduced by temporarily setting PATH for a command,
for example, 'PATH=/dev/null true', and then doing a PATH search.
Modified analysis:
ksh maintains the value of PATH as a linked list. When a local
scope for PATH is created (e.g. in a virtual subshell or when doing
something like PATH=/foo/bar command ...), ksh duplicates PATH by
increasing the refcount for every element in the linked list by
calling the path_dup() and path_alias() functions. However, when
the state of PATH is restored, this refcount is not decreased. Next
time when PATH is reset to a new value, ksh calls the path_delete()
function to delete the linked list that stored the older path. But
the path_delete() function does not free elements whose refcount is
greater than 1, causing a memory leak.
src/cmd/ksh93/sh/path.c: path_alias():
- Decrease refcount and free old item if needed.
(The 'old' variable was already introduced in 99065353, but
its value was never used there; this fixes that as well.)
src/cmd/ksh93/tests/leaks.sh:
- Add regression test. With the bug, setting/restoring PATH
(which clears the hash table) and doing a PATH search 16 times
causes about 1.5 KiB of memory to be leaked.
There is a bug in print_scan() function that may cause ksh to crash
while listing indexed arrays. The crash happens in nv_search() when
called from print_scan().
This applies a fix from Siteshwar Vashist:
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01944.html
src/cmd/ksh93/bltins/typeset.c:
- Call nv_scan() without the NV_IARRAY flag, even for a null scan.
src/cmd/ksh93/tests/arrays.sh:
- Add regression test for 'typeset -a' crash and check output.
There is a bug in sh_eval() that may cause ksh to crash due to a
double free() after sourcing multiple files with '.' or 'source'
if a longjmp is triggered, e.g. by a syntax error.
This applies a fix from Siteshwar Vashist:
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01943.html
src/cmd/ksh93/sh/xec.c: sh_eval():
- Zero file descriptor io_save after closing it. This prevents a
double free() after returning from a longjmp.
src/cmd/ksh93/tests/basic.sh:
- Add reproducer as regression test.
This commit changes the behavior of four date formats accepted
by 'printf %()T' because the old behavior is not compatible with
modern implementations of date(1):
- %k and %l now return a blank-padded hour, the former based on a
24-hour clock and the latter a 12-hour clock (these are common
extensions present on Linux and *BSD).
- %f now returns a date with the format '%Y.%m.%d-%H:%M:%S'
(BusyBox extension).
- %q now returns the quarter of the current year (GNU extension).
src/cmd/ksh93/data/builtins.c:
- Copy the date format documentation from date in libcmd to
the printf man page (for documenting 'printf %T').
src/cmd/ksh93/tests/builtins.sh:
- Add four regression tests for the changed date formats.
src/cmd/ksh93/sh.1:
- Remove inaccurate information about the date formats accepted by
printf %T'. The KornShell uses a custom version of strftime(3)
that isn't guaranteed to accepts the same formats as the native
strftime function.
src/lib/libast/tm/tmxfmt.c:
- Change the behavior of %f, %k, %l and %q to the common behavior.
%k and %l are implemented as aliases to %_H and %_I to avoid
duplicating code.
src/lib/libcmd/date.c:
- Update the documentation for the AST date command since it is
also affected by the changes to 'printf %T'.
Fixes#62
Associative arrays weren't being properly freed from memory, which
was causing a memory leak.
This commit incorporates a patch and reproducer/regress test from:
https://www.mail-archive.com/ast-users@lists.research.att.com/msg01016.html
src/cmd/ksh93/sh/name.c:
- Properly free associative arrays from memory in nv_delete().
src/cmd/ksh93/tests/leaks.sh:
- Add regression test.
The following set of commands can rarely cause a memory fault
when auditing[*] is enabled, although most of the time it will
simply cause ksh to write '(null)' to the auditing file in place
of a tty name:
$ [ -e /etc/ksh_audit ] || echo "/tmp/ksh_auditfile;$(id -u)" | sudo tee /etc/ksh_audit;
$ v=$(ksh 2> /dev/null +o rc -ic $'getopts a:bc: opt --man\nprint $?')
$ cat /tmp/ksh_auditfile
1000;1593599493;(null); getopts a:bc: opt --man
This happens because strdup is used unconditionally on the pointer
returned by 'ttyname', which can be NULL if stderr is closed. This
then causes 'hp->tty' to be set to null, as strdup returns NULL.
See https://github.com/att/ast/issues/1028
src/cmd/ksh93/edit/history.c:
- Make strdup duplicate 'notty' instead of NULL to prevent
crashes.
[*] https://blog.fpmurphy.com/2008/12/ksh93-auditing-and-accounting.html
If the processing of a multibyte character was interrupted in UTF-8
locales, e.g. by reading just one byte of a two-byte character 'ü'
(\303\274) with a command like:
print -nr $'\303\274' | read -n1 g
then the shellquoting algorithm was corrupted in such a way that
the final quote in simple single-quoted string was missing. This
bug may have had other, as yet undiscovered, effects as well. The
problem was with corrupted multibyte character processing and not
with the shell-quoting routine sh_fmtq() itself.
Full trace and discussion at: https://github.com/ksh93/ksh/issues/5
(which is also an attempt to begin to understand the esoteric
workings of the libast mb* macros that process UTF-8 characters).
src/lib/libast/comp/setlocale.c: utf8_mbtowc():
- If called from the mbinit() macro (i.e. if both pointer
parameters are null), reset the global multibyte character
synchronisation state variable. This fixes the problem with
interrupted processing leaving an inconsistent state, provided
that mbinit() is called before processing multibyte characters
(which it is, in most (?) places that do this). Before this fix,
calling mbinit() in UTF-8 locales was a no-op.
src/cmd/ksh93/sh/string.c: sh_fmtq():
- Call mbinit() before potentially processing multibyte characters.
Testing suggests that this could be superfluous, but at worst,
it's harmless; better be sure.
src/cmd/ksh93/tests/builtins.sh:
- Add regression test for shellquoting with 'printf %q' after
interrupting the processing of a multibyte characeter with
'read -n1'. This test only fails in a UTF-8 locale, e.g. when
running: bin/shtests -u builtins SHELL=/buggy/ksh-2012-08-01
Fixes#5.
src/cmd/ksh93/data/signals.c:
- SIGINFO was absent from the table of signals, which caused
commands like 'kill -INFO $$' to fail even on platforms with
SIGINFO (such as macOS and FreeBSD). Fix that by adding
it to the signal table.
src/cmd/ksh93/tests/signal.sh:
- Add a regression tests for using SIGINFO with the kill builtin.
The test will only be run if the external kill command supports
SIGINFO.
This commit fixes the following bugs in the 'vi' editing mode
backslash escape feature. Ref.: Bolsky & Korn (1995), p. 113, which
states for \: "Similar to Control+V [...] except that it escapes
only the next Erase or Kill charactrer".
1. The vi mode now only escapes the next character if the last
character input was a backslash, fixing the bug demonstrated at:
https://asciinema.org/a/E3Rq3et07MMQG5BaF7vkXQTg0
2. Escaping backslashes are now disabled in vi.c if the vi mode is
disabled (note that vi.c handles raw editing mode in UTF-8
locales). This makes the behavior of the raw editing mode
consistent in C/POSIX and UTF-8 locales.
3. An odd interaction with Backspace when the character prior to a
separate buffer entered with Shift-C was a backslash has been
fixed. Demonstration at: https://asciinema.org/a/314833
^? will no longer be output repeatedly when attempting to erase
a separate buffer with a Backspace, although, to be consistent
with vi(1), you still cannot backspace past it before escaping
out of it. Ref.:
https://github.com/ksh93/ksh/issues/56#issuecomment-653586994
src/cmd/ksh93/edit/vi.c:
- Prevent a backslash from escaping the next input if the previous
input wasn't a backslash. This is done by unsetting a variable
named backslash if a backslash escaped a character. backslash is
set to the result of c == '\\' when the user enters a new
character.
- Disable escaping backslashes in the raw editing mode because
it should not be enabled there.
src/cmd/ksh93/tests/pty.sh:
- Add some tests for how ksh handles backslashes in each
editing mode to test for the bugs fixed by this commit.
Fixes#56.
ksh crashed if it encountered a .paths directory in any of the
directories in $PATH.
Ref: https://bugs.launchpad.net/ubuntu/+source/ksh/+bug/1534855
src/cmd/ksh93/sh/path.c: path_chkpaths():
- Refuse to read .paths if it's not a regular file
or a symlink to a regular file.
Regular expressions that combine a repetition expression with
a parenthesized sub-expression throw a garbled syntax error:
$ [[ AATAAT =~ (AAT){2} ]]
ksh: syntax error: `~(E)(AAT){2} ]]
:'%Cred%h%Creseksh: syntax error: `~(E)(AAT){2} ]]
:'%Cred%h%Creseksh: syntax' unexpected
The syntax error occurs because ksh is not fully
accounting for '=~' when it runs into a curly bracket.
This fix disables the syntax error when the operator
is '=~' and adds handling for '(str){x}' (to allow for
more than one sub-expression). This bugfix and the
regression tests for it were backported from ksh93v-
2014-12-24-beta.
src/cmd/ksh93/sh/lex.c:
- Do not trigger a syntax error for '{x}' when the operator
is '=~' and add handling for multiple parentheses when
combined with '{x}'.
src/cmd/ksh93/tests/bracket.sh:
- Add two tests from ksh93v- to test sub-expressions
combined with the '{x}' quantifier.
File descriptors that are too far out of range will cause the
read builtin to crash. The following example will generate
two crashes:
$ ksh -c 'read -u 2000000' || ksh -c 'read -u-2000000'
The fix is to error out when the given file descriptor is out
of range. This bugfix is from Tomas Klacko, although it was
modified to use 'sh_iovalidfd' and reject numbers greater
than 'INT_MAX':
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01912.html
The question about 'shp->fdstatus[-1]' only applies to ksh93v-
(ksh93u+ doesn't have any references to 'shp->fdstatus[-1]').
src/cmd/ksh93/bltins/read.c:
- File descriptors that are out of range should be rejected
with an error message (like invalid file descriptors that
are in range). The seemingly redundant check for negative
numbers is there because out of range negative numbers also
cause memory faults despite the later 'fd<0' check.
src/cmd/ksh93/tests/io.sh:
- Add three tests for attempting 'read -u' on various invalid
file descriptor numbers.
