The liblist variable needs to be an extern for dtksh to build.
Quote from CDE developer Chase:
we use an old function that no longer appears in kornshell,
sh_getliblist, it seems to be replaced by the function sh_getlib,
which is fine, but it seems to return a "Shbltin_f" type, which I
can't seem to find any information on what it is. We need the void
pointer dlsym provides for some widget init stuff, I tried making
liblist an extern, but it kept giving me an error about libcomp_t
being undefined.
src/cmd/ksh93/bltins/typeset.c,
src/cmd/ksh93/include/shell.h:
- Fix the compiler error reported above by moving the type definition
for Libcomp_t to shell.h.
- Make liblist an extern since findsym.c in dtksh needs it to build.
The old sh_getliblist function doesn't need to be reintroduced
since the only purpose it served was to workaround the problem
of liblist being a static variable. Now that liblist is an extern,
dtksh fsym can use liblist directly to avoid sh_getliblist.
dtksh findsym.c:
https://sourceforge.net/p/cdesktopenv/code/ci/2.3.2/tree/cde/programs/dtksh/findsym.c
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
src/lib/libast/tm/tmxfmt.c:
- Making %l and %k aliases to %_I and %_H caused zero padding with
%0l and %0k to fail. Fix that by fully implementing %l and %k
without 'goto push'. This duplicates code from %I and %H, but it
is necessary for these formats to work correctly when zero padded.
src/cmd/ksh93/tests/builtins.sh:
- Add a regression test for manually specifying blank and zero
padding with sixteen different formats.
Some systems disallow executing files in /tmp and there is nothing
regular users can do about it. The build would fail with a
misleading error message about cc being a cross-compiler.
This commit makes the build system consistently use $TMPDIR with
/tmp as a fallback if that variable is not defined. This allows the
user to use another temporary directory with execute permission.
The error message in bin/package is also extended to signal the
possibility of a noexec temp dir.
It was working on Solaris 11.3, but there were still problems
building on Solaris 11.4 with GCC (as on the evaluation VM
downloaded directly from Oracle):
1. ksh immediately segfaulted. Experimenting with the compiler
flags Oracle uses revealed that we need to define _XPG6 for ksh
not to segfault. Why is a mystery.
2. The default path logic used by 'command -p' and the 'getconf
PATH' builtin command was still broken: the result did not
include any of the /usr/xpg?/bin directories where the standard
POSIX utilities actually live. Testing shows that the result of
the C language probe 'confstr(_CS_PATH,name,length)' is broken
on Solaris (it only yields the paths to the historic
non-standard utilities, defeating the purpose) unless _XPG7 is
defined; but the latter makes ksh segfault again. So another
solution is needed.
src/cmd/INIT/package.sh, bin/package:
- Add another hack to add the -D_XPG6 flag to CCFLAGS if we're
running SunOS aka Solaris. (I've tried to add a 'cc.sol11' script
to src/cmd/INIT/ instead, but for some reason that I just don't
have time to figure out, the INIT system ignores that on Solaris
with gcc, so this is the only way I could come up with. Any
patches for less hacky alternatives would be welcome.)
src/lib/libast/comp/conf.sh:
- Sanitise the code for finding the best 'getconf' utility.
src/lib/libast/comp/conf.tab: PATH:
- Since the C-languge getconf(_CS_PATH,...) is broken on Solaris
11.4, replace the C language probe with a shell script probe that
uses the external 'getconf' utility.
- To avoid ksh overriding the result of this probe with the result
of its own getconf(_CS_PATH,...) call, which would make Solaris
use the wrong value again, specify this as an AST configuration
entry instead of a POSIX entry. This should be good enough for
all systems; the OS 'getconf' utility should be reliable and the
default path value is constant for each OS, so can be hardcoded.
src/cmd/ksh93/tests/builtins.sh:
- Add another 'sleep .1' to the 'sleep -s 31' test as it was still
intermittently failing on Solaris and possibly other systems.
Solaris, Illumos distributions, and NetBSD need LDFLAGS set to link
explicitly to libm, otherwise, due to as-yet unknown reasons, the
src/lib/libdll/features/dll fails to write a valid header file and
compilation fails due to unknown identifiers such as Dllscan_t.
This commit adds the flag on those systems.
NixOS is a Linux distro that uses very different paths from the
usual Unix conventions (though it's POSIX compliant), and the
regression tests still needed a lot of tweaks to be compatible.
src/cmd/INIT/package.sh, bin/package:
- On SunOS (Solaris and illumos distros) and NetBSD, add '-lm' to
LDFLAGS before compiling.
src/cmd/INIT/mamprobe.sh, bin/mamprobe,
src/cmd/INIT/execrate.sh, bin/execrate:
- Instead of only in /bin, /usr/bin, /sbin and /usr/sbin, search
utilities in the path given by the OS 'getconf PATH', and use the
user's original $PATH as a fallback.
src/cmd/ksh93/tests/*.sh:
- Miscellaneous portability fixes, mainly elimination of unportable
hardcoded paths to commands.
- basic.sh: Remove test for 'time' keyword millisecond precision.
It was racy and could fail depending on system and system load.
This fixes 'command -p' for systems where getconf(1) lives
somewhere other than in /bin or /usr/bin, i.e. NixOS.
src/lib/libast/comp/conf.tab:
- To determine the default path value for AST 'getconf PATH' and
'command -p', compile a small C program to get the correct local
default path value (_CS_PATH) from the operating system so it
gets hardcoded in the ksh binary. This eliminates the need to to
invoke 'getconf PATH' to get this value, which fixes a catch-22
problem on systems where getconf(1) exists somewhere other than
/bin or /usr/bin.
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
src/cmd/ksh93/tests/builtins.sh:
- Sleep longer after forking a background job to give the OS more
time to launch it; this will hopefully avoid an intermittent
regression test failure on the Github CI runners.
Due to the mysterious workings of vmalloc(3), occasionally a
spurious leak result still showed up. The leak is always smaller
in bytes than the number of test iterations, so it can't be a leak
in the thing tested.
src/cmd/ksh93/tests/leaks.sh:
- Run each test N=512 times.
- Use a 'err_exit_if_leak' function to add a tolerance of N/4 (128)
bytes to each test result check.
Resolves: https://github.com/ksh93/ksh/issues/100
The following is quoted from Marcin Cieślak [*]:
When running under FreeBSD /bin/sh (and not ksh) we get spurious
file named '=' created in the root. This is because the "checksh"
function runs /bin/sh -c '(( .sh.version >= 20111111 ))' which
produces a "=" file with /bin/sh as a side effect.
Fixes https://github.com/ksh93/ksh/issues/13
bin/package,
src/cmd/INIT/package.sh:
- Fix the creation of a spurious '=' file by making sure the shell
has support for (( ... )) expressions.
.gitignore:
- Remove the '=' file entry since it no longer has a purpose.
[*]: https://bsd.network/@saper/103196289917156347
This bugfix is from Marcin Cieślak's fork of the INIT build
system. Before this bugfix, running 'bin/package host cpu'
on FreeBSD would always report one CPU core, even if the CPU
is multi-core:
$ ./bin/package host cpu
1
bin/package,
src/cmd/INIT/package.sh:
- Correctly report the number of CPUs on FreeBSD by using
'sysctl -n hw.ncpu'.
src/cmd/INIT/mamake.c:
- Fix a rare build error by applying Oracle's patch to increase
mamake's buffer size[*]. Description from the original patch:
The build of KornShell might spuriously fail
with the following error.
...
/usr/bin/ksh: line 40: syntax error at line 44: `else unmatched
mamake [lib/libast]: *** exit code 3 making ast.req
mamake: *** exit code 139 making lib/libast
The patch increases the buffer size of mamake to avoid
spurious build failures.
I can't reproduce build error, but this patch should be merged
anyway because OpenSUSE also increases mamake's buffer size
in a patch titled 'workaround-stupid-build-system.diff'[**].
This indicates that the build failure is a heisenbug that can
occur on at least Linux and Solaris.
[*]: 7cad9dae78
[**]: https://build.opensuse.org/package/view_file/shells/ksh/workaround-stupid-build-system.diff?expand=1
src/*/*/Mamfile,
src/lib/libast/Makefile:
- There were a few instances where the CCFLAGS and LDFLAGS were missing
in the Mamfiles and a Makefile. This commit fixes the problem by merging
the changes from Debian's blhc.diff patch:
f8fea737c9/debian/patches/blhc.diff
Related discussion:
https://github.com/ksh93/ksh/issues/95#issuecomment-664010969
src/cmd/ksh93/tests/leaks.sh:
- When ksh is compiled to use the system's malloc(3) instead of AST
vmalloc(3), the vmstate builtin returns either nothing or zero.
Detect this as a regression test failure and refuse to run tests.
- Tweak iterations. Tests don't need 500 or 1000 runs for vmstate.
src/cmd/ksh93/data/builtins.c:
- Do not compile in vmstate builtin when using system's malloc(3).
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
This code has always been completely undocumented since it was
added sometime between 2002 and 2004[*]. No one (including Google)
knows what it's for and no one is likely to find out.
Not only that, it doesn't compile. If SHOPT_AMP is defined, then it
errors out on an undefined function `print_fun` and an undefined
member `shpath` of 'struct Shell_s'. So it's clear that the code
had been abandoned by its authors for some time as of 2012.
src/cmd/ksh93/sh/xec.c:
- Remove vestigial SHOPT_AMP stuff, whatever that was.
[*] Found out by searching multishell ksh93 repo:
https://github.com/multishell/ksh93/
This merges some fixes to support building dtksh with -DBUILD_DTKSH.
These patches were sent through private email from the CDE developer
Chase. The reason these patches were submitted is because Chase wishes
to include ksh in CDE as an up-to-date git submodule. Quote from Chase:
"... my priority is to get your new version into our code as a git
submodule, and do it quickly before our code bases differ too widely."
Link to CDE project for anyone interested:
https://sourceforge.net/projects/cdesktopenv/
Although the patches were privately discussed, there are some public
emails on the CDE mailing list (links shortened due to long URLs):
ksh-chaos thread: https://bit.ly/3hjJ83b
dtksh alias thread: https://bit.ly/3hkzKfJ
The main fix is for suid_exec, which is now told that /usr/dt is a
valid directory to run from via preprocessor flags. A patch for
Shift-JIS was also submitted, but it isn't in this commit because it
isn't an effective fix for the existing Shift-JIS bugs. I will be
giving that patch some more testing.
From: Chase <nicetrynsa@protonmail.ch>
Co-authored by: Johnothan King <johnothanking@protonmail.com>
This applies ksh93-jobs.dif from OpenSUSE. Source:
https://build.opensuse.org/package/show/openSUSE:Leap:42.3:Update/ksh
src/cmd/ksh93/sh/jobs.c:
- jog_init(): Save errno in case close(JOBTTY) fails. If cause of
failure was interruption by a signal (EINTR), repeat close.
- job_kill(): Replace Red Hat fix for #35 with nicer OpenSUSE fix
that doesn't add a goto before declaring variables. Re: ff358f34
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.
ksh's built-in test, [ and [[ commands treat /dev/fd/* specially:
e.g. 'test /dev/fd/0' returns true even if it doesn't physically
exist, as on e.g. HP-UX. However, external commands need it to
exist physically.
src/cmd/ksh93/tests/subshell.sh:
- To decide whether to run a test with 'tee', use external 'test'
command to check if /dev/stdout and /dev/fd/1 actually exist.
'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
$ ksh -c 'whence -a printf'
printf is a shell builtin
printf is /usr/bin/printf
printf is an undefined function
The third line should not appear.
src/cmd/ksh93/bltins/whence.c:
- Remove faulty extra check for undefined (= autoload) functions.
This was already handled earlier, on lines 192-193.
src/cmd/ksh93/tests/builtins.sh:
- Add regression test.
- For previous 'whence -a' test, don't bother with shell function.
Fixes https://github.com/ksh93/ksh/issues/26
Some regression tests have to be run with the -i option, making the
shell behave (mostly) as if it is interactive. This causes ksh to
print a final newline upon EOF (Ctrl+D). This is functional if the
shell is really interactive, i.e. if standard input is on a
terminal and we're not running a shell script: it ensures that a
parent shell's prompt appears on a new line. But for tests like
ksh -i -c 'testcommands'
or
ksh -i <<EOF
testcommands
EOF
it's a minor annoyance. Adding an explicit 'exit' is an effective
workaround, but we might as well fix it.
src/cmd/ksh93/sh/main.c: exfile(): done:
- If shell is "interactive", only print final newline if standard
input is on a terminal and we're not running a -c script.
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.
Commit 80d9ae2b removed the line that set the NV_EXPORT flag on an
alias when the obsolete ksh88 'alias -x' option was used. But it
turns out that flag actually did something: it caused 'whence -v'
to report the alias as an exported alias -- misleadingly, because
exported aliases have never actually exised in ksh93. Since '-x' no
longer sets that flag, that message is never printed.
src/cmd/ksh93/data/msg.c,
src/cmd/ksh93/include/path.h:
- Remove is_xalias[] = "%s is an exported alias for " message.
src/cmd/ksh93/bltins/whence.c:
- Remove dead code to check for (formerly) exported alias.
My tests with running shbench[*] on ksh binaries compiled by clang
and gcc yielded no performance difference between compiling with
'-O2' and '-Os'. So we might as well reduce ksh's size and memory
footprint by default.
[*] http://fossil.0branch.com/csb/https://github.com/ksh-community/shbench
src/cmd/INIT/make.probe:
- Change default gcc optimisation level from -O2 to -Os.
- Change default non-gcc optimisation level from -O to -Os.
The coshell(1) command, which is required for libcoshell to be
useful, is not known to be shipped by any distribution. It was
removed by the ksh-community fork and hence also by 93u+m (in
2940b3f5). The coshell facility as a whole is obsolete and
insecure. For a long time now, the statically linked libcoshell
library has been 40+ kilobytes of dead weight in the ksh binary.
Prior discussion (ksh2020): https://github.com/att/ast/issues/619
src/lib/libcoshell/*:
- Removed.
src/cmd/ksh93/*:
- Remove the SHOPT_COSHELL compiler option (which was enabled) and
a lot of code that was conditional upon #ifdef SHOPT_COSHELL.
- init.c: e_version[]: Removing SHOPT_COSHELL changed the "J"
feature identifier in ${.sh.version} to a lowercase "j", which
was conditional upon SHOPT_BGX (background job extensions).
But src/cmd/ksh93/RELEASE documents (at 08-12-04, on line 1188):
| +SHOPT_BGX enables background job extensions. Noted by "J" in
| the version string when enabled. [...]
That is the only available documentation. So change that "j" back
to a "J", leaving the version string unchanged after this commit.
- jobs.c: job_walk(): We need to keep one 'job_waitsafe(SIGCHLD);'
call that was conditional upon SHOPT_COSHELL; removing it caused
a regression test failure in tests/sigchld.sh, 'SIGCHLD blocked
for script at end of pipeline' (which means that until now, a ksh
compiled without libcoshell had broken SIGCHLD handling.)
bin/package, src/cmd/INIT/package.sh:
- Don't export COSHELL variable.
Support for the long-dead 3DFS userland versioning file system was
already removed from ksh93 (although I'd overlooked some minor
things), but libast still supported it. This removes that too.
src/lib/libast/include/fs3d.h,
src/lib/libast/man/fs3d.3,
src/lib/libast/misc/fs3d.c:
- Removed.
bin/package,
src/cmd/INIT/package.sh:
- Remove attempted use of removed vpath builtin.
src/cmd/ksh93/*:
- Remove minor 3dfs vestiges.
src/lib/lib{ast,cmd,coshell}/*:
- Remove code supporting 3dfs.
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.
In a locale other than C/POSIX, ksh produces corrupted usage
messages for alternatives, e.g. this output of 'typeset -\?':
| Usage: typeset [-bflmnprstuxACHS] [-a[type]] [-i[base]] <..CUT..>
| [-T[tname]] [-Z[n]] [name[=value]...]
| Or:[name[=value]...]
| typeset[name[=value]...]
| [[name[=value]...]
| options[name[=value]...]
| ] -f [name...]
Correct output is:
| Usage: typeset [-bflmnprstuxACHS] [-a[type]] [-i[base]] <..CUT..>
| [-T[tname]] [-Z[n]] [name[=value]...]
| Or: typeset [ options ] -f [name...]
Similar corruption occurs in --help and --man output.
This bug is ancient: it's already in ksh 1993-12-28 s+.
ksh2020 has this fixed. A 'git bisect' run pinpointed the fix
to this commit, which fixes the ERROR_translating macro after
removing the AST-specific locale subsystem:
https://github.com/att/ast/commit/4abc061e
But making the same change in ksh 93u+m produced no results
(probably because we have not removed that subsystem).
However, disabling the use of translation macros in optget.sh
altogether (replacing them with dummies that were already coded in
a preprocessor directive fallback for a reduced standalone libast)
turns out to work. It's not as if there is actually any translation
anyway, so this effectively fixes this bug.
The actual cause of this bug remains mysterious, but should be
somewhere in the AST translation and/or locale subsystem.
src/lib/libast/misc/optget.c:
- Use fallback translation macros.
src/cmd/ksh93/tests/builtins.sh:
- Add regression tests for output of -?, --?x, --help and --man
for a usage string with an alternative ("Or:") usage message.
Before the fix, these failed when running the tests in the
C.UTF-8 locale (as in 'bin/shtests -u builtins').
A regression test failure was occurring on FreeBSD for
bin/shtests -u builtins
because UTF-8 characters were wrongly encoded as bytes in the
C.UTF-8 locale. The cause is that iswprint() always returns false
on FreeBSD if the ksh-specific C.UTF-8 locale is active, as the OS
doesn't support it.
That iswprint() call is redundant anyway; the new is_invisible()
function now handles this.
src/cmd/ksh93/sh/string.c: sh_fmtq():
- Remove redundant iswprint() test.
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.
The backported 'time' keyword code introduced a bug (shared by
ksh2020): the $TIMEFORMAT format sequences %0R, %0U and %0S output
a decimal fraction, acting as %1R, %1U and %1S.
A minor ksh2020 behaviour change that was also backported was that
the $TIMEFORMAT formatting no longer errored out on encountering an
invalid identifier, but continued. That behaviour is now reverted.
Neither of these two regressions occurred on older systems that
have to use times(3) instead of getrusage(2) or gettimeofday(2).
This commit also tweaks a regression test so that it doesn't fail
if the old times(3) interface is used.
src/cmd/ksh93/sh/xec.c: p_time():
- (Fix indentation of a for loop.)
- On modern systems, when outputting the result of $TIMEFORMAT
format sequences, only print fraction if precision is nonzero.
- On modern systems, when encountering an invalid format sequence,
abort formatting in the same way as done for old systems.
- On old systems, initialise 'n' in a more readable way when used
as the index for tm[].
src/cmd/ksh93/tests/basic.sh:
- Don't fail, but issue warning on old systems that use times(3).
Otherwise, check milliseconds: with the ksh 'sleep' builtin,
'TIMEFORMAT=%3R; time sleep .002' should always output '0.002'.
- Change regression test for TIMEFORMAT='%0S%' to check for the
correct output, '0%', instead of checking for an error message.
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.
src/cmd/ksh93/tests/pty.sh:
- init: Remove superfluous lineno=$LINENO assignments. They aren't
needed if we avoid alias expansion on the err_exit function call.
- In the test "vi mode file name completion", append the main
shell's PID to /tmp/fakehome to make a slightly less insecure
temporary directory name. Unfortunately we cannot use $tmp as
that uses $TMPDIR which may cause a false pass. (re: 4cecde1d)
Apparently, on FreeBSD, the stty command does not work correctly
for setting 'erase' or 'kill' on a pty pseudoterminal. I've no
idea whether the bug is in FreeBSD stty or in AST pty, but in any
case, a workaround is needed for the time being.
src/cmd/ksh93/tests/pty.sh:
- Save terminal state on init; set a trap to restore it on exit.
- Issue 'stty erase ^H kill ^X' on the real terminal before
entering pty pseudoterminals.
Resolves#44.
For unknown reasons, the test for a memory leak in 'read -C stat
<<< "$data"' can show an intermittent minor variation in memory
usage when run with shcomp on certain versions of macOS.
The reported variations are 48 bytes or 80 bytes. This is too small
to be the result of an actual memory leak in the tested command;
it is repeated 500 times so that any real leak should show a
difference of at least 500 bytes.
src/cmd/ksh93/tests/leaks.sh:
- Add a tolerance of 128 bytes to get rid of the false failure.
Fixes#70 (hopefully).
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 shows a better layout in '--man' or '--nroff' output.
src/lib/libast/misc/optget.c:
- Incorporate the '--help | --man' addition in the printf format
instead of hardcoding it in the default options string.
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.
'ps' does not always give reliable results; on macOS, 'ps' appears
to produce nondeterministic (i.e. randomly varying) results for
'vsz' and 'rss', making it unusable for memory leak tests. See:
https://github.com/ksh93/ksh/pull/64#issuecomment-655094931
and further comments.
So let's compile in the vmstate builtin so that we can make sure to
measure things properly. It also reports bytes instead of 1024-byte
blocks, so smaller leaks can be detected.
To be decided: whether or not to disable the vmstate builtin for
release builds in order to save about 12K in the ksh binary.
src/cmd/ksh93/data/builtins.c:
- Add vmstate to the list of builtins that are compiled in.
src/cmd/ksh93/tests/leaks.sh:
- getmem(): get size using: vmstate --format='%(busy_size)u'
(Using busy_size instead of size seems to make more sense as it
excludes freed blocks. See vmstate --man)
- Introduce a $unit variable for reporting leaks and set it to
'bytes'; this makes it easier to change the unit in future.
- Since the tests are now more sensitive, initialise all variables
before use to avoid false leak detections.
- The last test seemed to need a few more 'read' invocations in
order to get memory usage to a steady state before the 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 gets rid of repetitive code in test scripts to create their
own temporary directories. Instead, shtests exports a $tmp to each
test script that is a subdirectory of its own temporary directory.
This has the advantage of having all test script temporary
directories in one hierarchy. Along with a new option to keep
temporary files, this makes it easy to inspect them if wanted.
This does make the test scripts less self-contained as they now
depend on a temporary directory being exported as $tmp. But they
already depended on $SHELL being the shell to test, so they already
were not quite self-contained.
src/cmd/ksh93/tests/shtests:
- Add -k/--keep option to keep temporary directory. Make the EXIT
trap report its location instead of deleting it.
- For each test, create a subdirectory of $tmp (named after the
test script plus the tested locale or 'shcomp') and export that
subdirectory to the test script as its own $tmp.
- If -k is not given, delete each script's temporary files
immediately after running it to minimise disk usage.
src/cmd/ksh93/tests/*.sh:
- Don't make own temp directory.
- Refuse to run if $tmp is not set.
- Miscellaneous tweaks.
src/cmd/ksh93/tests/pty.sh:
- Fix race condition in the test "raw Bourne mode literal tab
characters with wide characters enabled" by adding 'd 10' to add
a 10-millisecond delay before every write. Thanks to @JohnoKing:
https://github.com/ksh93/ksh/pull/57#issuecomment-653617531
- Fix locale for test "raw Bourne mode backslash handling" (should
be UTF-8, not UTF8) (re: a0dcdeea).
- Add a few more dummy # err_exit # comments to allow shtests to
count the number of tests.
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.
In <https://github.com/att/ast/commit/d2771913>, GCC version 10 was
specifically special-cased for skipping the -nostartfiles flag
along with versions 7, 8, and 9. It seems more future-proof to
specifically include it for versions up to 6 and remove it for any
version 7 and up.
src/cmd/INIT/make.probe:
- Remove the -nostartfiles for all version of gcc > 7.
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.
src/cmd/ksh93/tests/leaks.sh:
- This script was never actually running the regression
tests because 'vmstate' isn't available as a builtin.
While this can be fixed by adding vmstate to the builtin
table, that has the downside of increasing the binary
size of ksh. This commit replaces all usage of 'vmstate'
with 'ps' and 'awk' as a different way to measure
memory usage. The memory leaks regression tests are now
always run.
- Rename old $n to $N due to new $n interfering with the old
regression test.
- Add before and after results for the number of 1024-byte
blocks leaked in each test.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
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.
Not cleaning up the FIFO broke 'grep -r' in the arch directory,
making it hang forever. So we need a better way of cleaning it up.
bin/package,
src/cmd/INIT/package.sh:
- Unlink the FIFO early after sleeping a second in the background.
This works because the named directory entry is only needed to
establish the pipe, not to keep it going.
To get ksh to prefer UTC over GMT in 'printf %T' output, only the
change in format[] was needed. The corresponding change in zone[]
made it prefer UTC for London time, even in summer time, which is
wrong -- e.g.:
$ LANG=C TZ=Europe/London arch/*/bin/ksh -c 'date; printf %T\\n now'
Tue Jun 30 01:39:09 BST 2020
Tue Jun 30 00:39:09 UTC 2020
src/lib/libast/tm/tmdata.c:
- Revert change in zone[].
The regression test failed on systems where 'chmod' exists at more
than one location, e.g. Slackware where it's at both /bin/chmod and
/usr/bin/chmod.
src/cmd/ksh93/tests/builtins.sh: 'whence -a'/tracked aliases test:
- In the expected value, use modified 'whence -a -p chmod' output
to get all of the paths to chmod.
- On failure, report both expected and actual values.
* Fix the readonly builtin's scope in functions
This bug was first reported at https://github.com/att/ast/issues/881
'tdata.sh->prefix' is only set to the correct value when
'b_readonly' is called as 'export', which breaks 'readonly' in
functions because the correct scope isn't set. As a result, the
following example will only print a newline:
$ function show_bar { readonly foo=bar; echo $foo; }; show_bar
The fix is to move the required code out of the if statement for
'export', as it needs to be run for 'readonly' as well. This bugfix
is from https://github.com/att/ast/pull/906
src/cmd/ksh93/bltins/typeset.c:
- Set 'tdata.sh->prefix' to the correct value, otherwise 'readonly'
uses the wrong scope.
src/cmd/ksh93/tests/builtins.sh:
- Add the regression test from ksh2020, modified to run in a
subshell.
src/cmd/ksh93/data/builtins.c,
src/cmd/ksh93/sh.1:
- Add documentation of 'readonly' vs. 'typeset -r' difference:
'readonly' does not create a function-local scope.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
The following set of commands ends with a memory fault under
certain circumstances because ksh attempts to free memory
twice, causing memory corruption:
$ testarray=(1 2)
$ compound testarray
$ unset testarray
$ eval testarray=
The fix is to make sure 'np->nvfun' is a valid pointer before
attempting to free memory in 'put_tree'. This patch is from
OpenSUSE: https://build.opensuse.org/package/view_file/shells/ksh/ksh93-nvtree-free.dif?expand=1
src/cmd/ksh93/sh/nvtree.c:
- Do not try to free memory when 'np->nvfun' and 'val'
are false.
src/cmd/ksh93/tests/comvar.sh:
- Add a regression test for the double free problem. The
reproducer must be run from an executable script
with 'ksh -c'.
Variables created with 'typeset -RF' were being treated as
short integers, even though they are actually floating point
values. As a result the following example will cause a crash:
$ typeset -RF foo=1
$ test "$foo"
This is fixed by checking for 'NV_DOUBLE' with 'nv_isattr',
which prevents ksh from treating floating point values as
short integers due to '== NV_INT16P' excluding 'NV_DOUBLE'.
This bugfix was backported from ksh93v- 2013-10-10-alpha.
src/cmd/ksh93/sh/array.c,
src/cmd/ksh93/sh/name.c,
src/cmd/ksh93/sh/nvdisc:
- Avoid treating floating point values as short integers by
checking for 'NV_DOUBLE' with 'nv_isattr'.
src/cmd/ksh93/tests/types.sh:
- Add a regression test for the 'typeset -RF' crash. The
crash cannot be replicated if 'typeset -RF' sets 'foo'
to zero.
ksh, even non-interactive, loads /etc/ksh.kshrc by default. On
some systems this can be a problem, e.g. OpenBSD, which installs a
default /etc/ksh.kshrc which is designed for its version of pdksh.
Quoth sh.1:
On systems that support a system wide /etc/ksh.kshrc
initialization file, if the filename generated by the expansion
of ENV begins with /./ or ././ the system wide initialization
file will not be executed.
src/cmd/ksh93/tests/shtests,
src/cmd/ksh93/tests/builtins.sh,
src/cmd/ksh93/tests/options.sh,
src/cmd/ksh93/tests/pty.sh:
- Instead of emptying or unsetting ENV, ensure it is exported with
a default value of /./dev/null so we skip loading the system-wide
profile and load an empty user profile.
- Where a specific ENV path was required for the tests, prefix it
with '/.' so it starts with '/./'.
The cd builtin was removing '.' from directory names when combined
with a preceding '../', which caused commands like 'cd ../.local'
to become 'cd ../local'. This patch fixes the problem by limiting
the extra handling to leading '..'. The bugfix comes from ksh93v-
2013-10-10-alpha, although this version is a shortened patch from
Solaris (as ksh93v- refactored a decent amount of the code for the
cd builtin).
src/cmd/ksh93/bltins/cd_pwd.c:
- cd should only check for leading '..', as trying to handle a lone
'.' only causes problems.
src/cmd/ksh93/tests/builtins.sh:
- Add a regression test for this problem based on the test present in
ksh93v- 2013-10-10-alpha.
Patch from Solaris:
https://github.com/oracle/solaris-userland/blob/860d27f/components/ksh93/patches/270-23319761.patch
Apparently some systems are still configured to use GMT instead of
UTC after all. This included our own GitHub CI runner config.
Oops. This made the previous commit fail to pass the CI test run.
We can't win this one, it's got to be either one or the other.
UTC is the international standard on which civil time is based.
GMT is often taken as synonymous for UTC, but in navigation,
it can differ from UTC by up to 0.9 seconds. Ref.:
https://en.wikipedia.org/w/index.php?title=Greenwich_Mean_Time&oldid=963422787
The more ambiguous term should not be the first preference.
src/cmd/ksh93/tests/builtins.sh:
- Before checking 'printf %T now' output against 'date' output,
change any ' GMT ' in the latter to ' UTC '.
.github/workflows/ci.yml:
- Set time zone to UTC, not GMT.
"UTC" is the modern name for what used to be "GMT", but ksh still
preferred GMT. On systems configured to use the UTC time zone, this
caused a 'printf %T' regression test failure in tests/builtins.sh
as the external 'data' utility will prefer UTC these days.
src/lib/libast/tm/tmdata.c:
- Reorder the name alternatives for UTC/GMT so that UTC is
the first preference.
src/cmd/ksh93/tests/builtins.sh:
- Report expected and actual values on 'printf %T' failure.
Related: #6
When neither '-o emacs' nor '-o vi' is active, there were a couple
of bugs with entering tab characters:
1. Tab completion was erroneously left active. The cause of this
was that raw Bourne edit mode is handled by ed_viread() in vi.c
on shells with wide character support, instead of the default
ed_read() in edit.c, and the former failed to check if vi mode
is active when processing tab characters.
2. When entering literal tab characters, the cursor was moved to
the right only one character, instead of the amount of
characters corresponding to the tab.
src/cmd/ksh93/edit/vi.c: getline():
- Before processing '\t' (tab) for command completion, check that
the 'vi' shell option (SH_VI) is active.
src/cmd/ksh93/edit/edit.c: ed_virt_to_phys():
- When translating literal tabs to on-terminal spaces and when
recalculating the cursor position, remove erroneous checks for
SH_VI; this is also needed in raw Bourne mode. According to my
own testing, this has no effect on emacs mode (knock on wood).
src/cmd/ksh93/tests/pty.sh:
- Add two regression tests. An odd race condition reveals itself in
either pty or in ksh's raw/Bourne edit mode; see comment in test.
Effect is we have to expect either literal tabs or tabs expanded
to spaces, until that is tracked down and fixed.
Fixes#43.
This commit fixes the bug reported in:
https://github.com/att/ast/issues/682
The following sequence fails in vi mode because ksh looks in the
wrong part of the 'virtual' buffer:
$ touch ~/testfile
$ ls ~/test<tab>
The fix is to change 'virtual[i]' to 'virtual[last_virt]' in the
bugged section of code. The other changes are to make sure listing
files in a directory with something like 'ls /etc/<tab>' calls the
code for Ctrl+L to preserve 'ls /etc/' rather than try (and fail)
to complete the directory name, producing 'ls /etc\n/'. This bugfix
was backported from ksh93v- 2013-10-10-alpha.
src/cmd/ksh93/edit/vi.c
- Backport the bugfix from ksh93v- 2013-10-10-alpha for this
problem.
src/cmd/ksh93/tests/pty.sh
- Add a regression test for this issue using pty, adjusted slightly
for a fake home directory in /tmp.
Somewhat notable changes in this commit:
- The 'set +r' bugfix (re: 74b41621) is now documented in the
changelog.
- Missing options have been added to the synopsis section of the
ksh man page.
- The minor formatting fix from https://github.com/ksh-community/ksh/pull/5
has been applied to the ksh man page.
- A few fixes from https://github.com/att/ast/commit/5e747cfb
have been applied to the ksh man page.
- The man page fixes from https://github.com/att/ast/pull/353
have been applied, being:
- An addition to document the behavior of 'set -H'.
- A fix for the cd section appending rksh93.
- A fix for some options being indented too far.
- Removal of a duplicate section documenting '-D'.
- Reordering the options for 'set' in alphabetical order.
- A minor fix for the documentation of 'ksh -i'.
src/cmd/ksh93/tests/bracket.sh:
- Disable tests for [[ -N ... ]] (test -N ...), because it is
expected to break on systems where $TMPDIR (or even the entire
root file system) is mounted with noatime for better performance.
Ref.: https://opensource.com/article/20/6/linux-noatime
(It also needs annoyingly long sleep times on older systems with
a 1-second timestamp granularity.)
Testing the behaviour of an external editor, even the standard one,
is outside the scope of the ksh regression tests.
src/cmd/ksh93/tests/pty.sh:
- Disable a test that invoked vi(1) and that failed, either
intermittently or consistently, on too many systems because
whatever vi(1) is installed locally doesn't write the string
"/tmp/" exactly as and/or when expected.
The output format is now identical to mksh's except for
the locale-dependent radix point ('.' or ',').
src/cmd/ksh93/bltins/misc.c:
- Output format tweak: pad seconds with initial zero if < 10.
- Use "too many operands" (e_toomanyops) error msg from 3ba4900e
if there are operands, instead of "bad syntax" (e_badsyntax).
- Consolidate repetitive calculating and printing code
into print_times().
- Get rid of some excessive variables.
src/cmd/ksh93/tests/builtins.sh:
- Update regression tests to match the above.
src/cmd/ksh93/data/builtins.c:
- Update sh_opttimes[] version string.
Well, that's what I get for backporting code without properly
checking it over. There was an elementary math error in how the
times builtin calculated seconds:
utime_sec = utime - utime_min;
which could cause output such as "1m98.38s" or "3m234.77s".
src/cmd/ksh93/bltins/misc.c: b_times():
- Use fmod(), i.e. floating point modulus, to calculate seconds.
When a nonexistent test script was given as an argument to
shtests, this was not counted as an error and shtests exited
successfully (with status 0).
src/cmd/ksh93/tests/shtests:
- Increase total error count if a test script is not found.
The code for handling process substitution with redirection was
never being run because IORAW is usually set when IOPROCSUB is
set. This commit fixes the problem by moving the required code
out of the !IORAW if statement. The following command now prints
'good' instead of writing 'ok' to a bizzare file:
$ ksh -c 'echo ok > >(sed s/ok/good/); wait'
good
This commit also fixes a bug that caused the process ID of the
asynchronous process to print when the shell was in interactive
mode. The following command no longer prints a process ID,
behaving like in Bash and zsh:
$ echo >(true)
/dev/fd/5
src/cmd/ksh93/sh/args.c:
- Temporarily turn off the interactive state while in a process
substitution to prevent the shell from printing the PID of
the asynchronous process.
src/cmd/ksh93/sh/io.c:
- Move the code for process substitution with redirection into
a separate if statement.
src/cmd/ksh93/tests/io.sh:
- Add two tests for both process substitution bugs fixed by this
commit.
src/cmd/ksh93/tests/shtests:
- Update shtests with a patch from Martijn Dekker to use
pretty-printing for the output from the times builtin (if it
is available).
Fixes#2
This commit fixes the bug described in att/ast#32. The fix and
following explanation is from att/ast#467:
While copying variables from function's local scope to a new scope,
variable attributes were not copied. Such variables were not marked
to be exported in the new function. For e.g.
function f2 { env | grep -i "^foo"; }
function f1 { env | grep -i "^foo"; f2; }
foo=bar f1
prints 'foo=bar' only once, but it should print be twice.
src/cmd/ksh93/sh/xec.c:
- When variables from the local scope of a function are copied into
the scope of a nested function, the attributes of the variables
need to be copied as well.
src/cmd/ksh93/tests/functions.sh:
- Add regression tests from ksh2020 to check environment variables
passed to functions.
src/cmd/ksh93/tests/builtins.sh:
- The output of 'printf %T now' and the external 'date'
command aren't guaranteed to be the same unless $LC_ALL
is set to 'C'. Set LC_ALL in these command substitutions
to fix a spurious test failure on Linux.
Ksh was trying to use the 'pw' variable as a valid pointer even
when it was NULL. This is fixed by doing the error check for
'pw' before doing anything else in 'job_kill'.
This bugfix is from Red Hat:
44e0a643a9/f/SOURCES/ksh-20130214-fixkill.patchFixes#34
The 'stop' and 'suspend' default aliases are now converted into
regular built-in commands so that 'unalias -a' does not remove
them, 'suspend' can do some sanity checks, and something like
cmd=stop; $cmd $!
will now work.
src/cmd/ksh93/bltins/trap.c:
- b_kill(): Incorporate 'stop' functionality, which is simply
setting the same flag and variable as '-s STOP' would have done.
- b_suspend(): Add simple builtin function that sends SIGSTOP to
the main shell. Check for no operands, and refuse to suspend a
login shell (which would leave the user stuck with no way out).
Also check that 'kill' succeeds; if we're in an asynchronous
subshell, it is possible the main shell no longer exists.
src/cmd/ksh93/data/aliases.c:
- Remove "stop" and "suspend" default aliases. (Why were these
conditional upon SIGTSTP when they actually issued SIGSTOP?)
src/cmd/ksh93/include/builtins.h,
src/cmd/ksh93/data/builtins.c,
src/cmd/ksh93/data/msg.c:
- Add declarations of "stop" and "suspend" regular built-ins.
- Add option strings (AST manual/--man pages) for them.
- Add e_toomanyops ("too many operands") reusable error message for
b_suspend(). Other new commands may want this at some point.
src/cmd/ksh93/sh.1:
- Remove "stop" and "suspend" default aliases.
- Document "stop" and "suspend" regular built-in commands.
This appears to be originating from:
2755 *) if test ! -d $INSTALLROOT
2756 then INSTALLROOT=$PACKAGEROOT;
where INSTALLROOT=PACKAGEROOT and 'clean' deletes everything under
INSTALLROOT thus deleting the entire git repo. This only applies when
there's no arch/$HOSTTYPE directory due to the condition above.
bin/package,
src/cmd/INIT/package.sh:
- Delete arch/$HOSTTYPE as stated in the documentation
for the clean action instead of $INSTALLROOT.
This fixes compiler warnings for implicit-ints:
warning: return type defaults to 'int' [-Wimplicit-int]
1854: sh_tcgetattr(int fd, struct termios *tty)
1863: sh_tcsetattr(int fd, int cmd, struct termios *tty)
cmd/ksh93/edit/edit.c:
- Set the return type explicitly to int and align with the prototype
declared in include/terminal.h.
After making PATH readonly in a virtual subshell (without otherwise
changing it, so the subshell is never forked), then the main shell
would erroneously fork into a background process immediately after
leaving the virtual subshell. This was caused by a bug in the
forking workaround that prevents changes in PATH in a virtual
subshell from clearing the parent shell's hash table.
src/cmd/ksh93/sh/name.c: nv_putval():
- If we're either setting or restoring PATH, do an additional check
for the NV_RDONLY flag, which means the function was told to
ignore the variable's readonly state. It is told to ignore that
when restoring the parent shell state after exiting a virtual
subshell. If we don't fork then, we don't fork the parent shell.
src/cmd/ksh93/tests/subshell.sh:
- Add regression test verifying that no forking happens when making
PATH readonly in a subshell.
Fixes#30.
src/cmd/ksh93/data/variables.c:
- Running 'unset .sh.lineno' creates a memory fault, so fix that
by giving it the NV_NOFREE attribute. This crash was happening
because ${.sh.lineno} is an integer that cannot be freed from
memory with free(3).
src/cmd/ksh93/sh/init.c:
- Tell _nv_unset to ignore NV_RDONLY when $RANDOM and $LINENO are
restored from the subshell scope. This is required to fully
restore the original state of these variables after a virtual
subshell finishes.
src/cmd/ksh93/bltins/typeset.c,
src/cmd/ksh93/sh/subshell.c:
- Disabled some optimizations for two instances of 'sh_assignok' to
fix 'readonly' in virtual subshells and '(unset .sh.level)' in
nested functions. This fixes the following variables when
'(readonly $varname); enum varname=' is run:
$_
${.sh.name}
${.sh.subscript}
${.sh.level}
The optimization in question prevents sh_assignok from saving the
original state of these variables by making the sh_assignok call
a no-op. Ksh needs the original state of a variable for it to be
properly restored after a virtual subshell has run, otherwise ksh
will simply carry over any new flags (being NV_RDONLY in this case)
from the subshell into the main shell.
src/cmd/ksh93/tests/variables.sh:
- Add regression tests from Martijn Dekker for setting special
variables as readonly in virtual subshells and for unsetting
special variables in general.
Fixes#4
src/cmd/INIT/ratz.c:
- Fix build warning:
src/cmd/INIT/ratz.c:4741:2: warning: case label value exceeds maximum value for
type
4741 | case 0241:
| ^~~~
The character literal in the switch expression was being treated
as a signed char while the case label 0241 is greater than 127,
resulting in this warning.
'whence -a' bases the path for tracked aliases on the user's
current working directory if an enabled ksh builtin of the same
name is also available. The following example will claim 'cat'
is in the user's current working directory:
$ whence -a cat
cat is a tracked alias for /usr/bin/cat
$ builtin cat
$ whence -a cat
cat is a shell builtin
cat is /usr/bin/cat
cat is a tracked alias for /current/working/directory/cat
This patch from ksh2020 fixes this problem by properly saving the
path of the tracked alias for use with 'whence -a', since
'path_pwd' (as implied by the function's name) only gets the users
current working directory, not the location of tracked aliases.
Ref.: https://github.com/att/ast/issues/1049
This bug was originally reported by David Morano about two decades
ago to the AST team: https://github.com/att/ast/issues/954
src/cmd/ksh93/bltins/whence.c:
- Print the actual path of a tracked alias, path_pwd doesn't
have this functionality.
src/cmd/ksh93/include/name.h:
- Add 'pathcomp' for saving the value of tracked aliases.
src/cmd/ksh93/sh/path.c:
- Save the value of tracked aliases for use by whence.
src/cmd/ksh93/tests/builtins.sh:
- Add a regression test for using 'whence -a' on tracked
aliases with a builtin equivalent.
src/cmd/ksh93/data/builtins.c:
- sh_optexec[], sh_optdirect[]: Move paragraphs on exit behaviour
to the EXIT STATUS section, where people would look for them.
- sh_optexec[]: Since we modified b_exec() to support 'redirect',
add "ksh93" to the credits to avoid blaming AT&T for our changes.
Applying the fix for 'unset -f' exposed a crashing bug in lookup()
in sh/nvdisc.c, which is the function for looking up discipline
functions. This is what caused tests/variables.sh to crash.
Ref.: https://github.com/ksh93/ksh/issues/23#issuecomment-645699614
src/cmd/ksh93/sh/nvdisc.c: lookup():
- To avoid segfault, check that the function pointer nq->nvalue.rp
is actually set before checking if nq->nvalue.rp->running==1.
src/cmd/ksh93/sh/xec.c,
src/cmd/ksh93/tests/functions.sh:
- Uncomment the 'unset -f' fix from b7932e87.
Resolves#21 (again).
The fix in sh/xec.c, which was backported from the ksh 93v- beta to
delay the actual removal of a running function that unsets itself,
caused a segfault in the variables.sh regression tests (see #23).
src/cmd/ksh93/sh/xec.c:
- Comment out the backported code pending a correct fix for #21.
Now both types of functions silently fail to unset themselves
(unless they're discipline functions).
src/cmd/ksh93/tests/functions.sh:
- Disable regression tests checking that the function was actually
unset, pending a correct fix for #21.
Resolves: #23Reopens: #21
src/cmd/ksh93/sh/name.c:
- Correct the check for when a function is currently running
to fix a segmentation fault that occurred when a POSIX
function tries to unset itself while it is running.
This bug fix was backported from ksh93v-.
src/cmd/ksh93/sh/xec.c:
- If a function tries to unset itself, unset the function
with '_nv_unset(np, NV_RDONLY)' to fix a silent failure.
This fix was also backported from ksh93v-.
src/cmd/ksh93/tests/functions.sh:
- Add four regression tests for when a function unsets itself.
Resolves#21
Note that shtests simply does a 'grep -c err_exit' and substracts 1
to count the number of regression tests in a test script. Not all
test scripts make temp dirs, so subtracting 2 instead won't do.
src/cmd/ksh93/tests/*.sh:
- Escape the err_exit call in the routine to create a temporary
directory so that it is not counted as a regression test.
That bypasses the alias, so we have to pass $LINENO manually.
Four added tests did not correctly report their line numbers
upon failure and were counted as one, because the err_exit
alias/function pair was called from a shell function.
Note that shtests simply does a 'grep -c err_exit' to count the
number of regression tests in a test script.
src/cmd/ksh93/tests/subshell.sh:
- check_hash_table():
- Take line number as 1st argument.
- Quote a character in err_exit to bypass the alias when calling
it, so we can pass on the argument for the line number. This
also stops this helper function from being counted as a test.
- When calling check_hash_table(), pass $LINENO.
- Add dummy err_exit comments to have the tests counted.
Ksh was not checking for `command` when running a special builtin,
which caused preceding invocation-local variable assignments to
become global. This is the reproducer from the att/ast#72:
$ foo=BUG command eval ':'
$ echo "$foo"
This no longer prints 'BUG', as ksh now makes sure the command builtin
is not running a special builtin before making invocation-local
variable assignments global.
src/cmd/ksh93/sh/xec.c:
- Backport the bugfix for BUG_CMDSPASGN from ksh93v- 2013-10-10-alpha.
src/cmd/ksh93/tests/builtins.sh:
- Add a regression test based on the reproducer in att/ast#72.
As previously reported in rhbz#1112306 (https://bugzilla.redhat.com/show_bug.cgi?id=1112306),
ksh may crash when receiving SIGCHLD because GCC's optimizer will fail to generate
`addl` and `sub` instructions to increment and decrement `job.in_critical` in the
`job_subsave` function. This bug *does* occur in GCC 10 with `-O2`, but not `-O1`;
it doesn't appear this bug has been fixed. As a reference, here is the relevant
debug assembly output of `job_subsave` when KSH is compiled with `CCFLAGS` set to
`-g -O1`:
0000000000034c97 <job_subsave>:
void *job_subsave(void)
{
34c97: 53 push %rbx
struct back_save *bp = new_of(struct back_save,0);
34c98: bf 18 00 00 00 mov $0x18,%edi
34c9d: e8 34 4a 0a 00 callq d96d6 <_ast_malloc>
34ca2: 48 89 c3 mov %rax,%rbx
job_lock();
34ca5: 83 05 3c 50 13 00 01 addl $0x1,0x13503c(%rip) # 169ce8 <job+0x28>
*bp = bck;
34cac: 66 0f 6f 05 4c 5a 13 movdqa 0x135a4c(%rip),%xmm0 # 16a700 <bck>
34cb3: 00
34cb4: 0f 11 00 movups %xmm0,(%rax)
34cb7: 48 8b 05 52 5a 13 00 mov 0x135a52(%rip),%rax # 16a710 <bck+0x10>
34cbe: 48 89 43 10 mov %rax,0x10(%rbx)
bp->prev = bck.prev;
34cc2: 48 8b 05 47 5a 13 00 mov 0x135a47(%rip),%rax # 16a710 <bck+0x10>
34cc9: 48 89 43 10 mov %rax,0x10(%rbx)
bck.count = 0;
34ccd: c7 05 29 5a 13 00 00 movl $0x0,0x135a29(%rip) # 16a700 <bck>
34cd4: 00 00 00
bck.list = 0;
34cd7: 48 c7 05 26 5a 13 00 movq $0x0,0x135a26(%rip) # 16a708 <bck+0x8>
34cde: 00 00 00 00
bck.prev = bp;
34ce2: 48 89 1d 27 5a 13 00 mov %rbx,0x135a27(%rip) # 16a710 <bck+0x10>
job_unlock();
34ce9: 8b 05 f9 4f 13 00 mov 0x134ff9(%rip),%eax # 169ce8 <job+0x28>
34cef: 83 e8 01 sub $0x1,%eax
34cf2: 89 05 f0 4f 13 00 mov %eax,0x134ff0(%rip) # 169ce8 <job+0x28>
34cf8: 75 2b jne 34d25 <job_subsave+0x8e>
34cfa: 8b 3d ec 4f 13 00 mov 0x134fec(%rip),%edi # 169cec <job+0x2c>
34d00: 85 ff test %edi,%edi
34d02: 74 21 je 34d25 <job_subsave+0x8e>
34d04: c7 05 da 4f 13 00 01 movl $0x1,0x134fda(%rip) # 169ce8 <job+0x28>
When `-O2` is used instead of `-O1`, the `addl` and `sub` instructions for
incrementing and decrementing the lock are removed. GCC instead generates a
broken `mov` instruction for `job_lock` and removes the initial `sub` instruction
in job_unlock (this is also seen in Red Hat's bug report):
job_lock();
*bp = bck;
37d7c: 66 0f 6f 05 7c 79 14 movdqa 0x14797c(%rip),%xmm0 # 17f700 <bck>
37d83: 00
struct back_save *bp = new_of(struct back_save,0);
37d84: 49 89 c4 mov %rax,%r12
job_lock();
37d87: 8b 05 5b 6f 14 00 mov 0x146f5b(%rip),%eax # 17ece8 <job+0x28>
...
job_unlock();
37dc6: 89 05 1c 6f 14 00 mov %eax,0x146f1c(%rip) # 17ece8 <job+0x28>
37dcc: 85 c0 test %eax,%eax
37dce: 75 2b jne 37dfb <job_subsave+0x8b>
The original patch works around this bug by using the legacy `__sync_fetch_and_add/sub`
GCC builtins. This forces GCC to generate instructions that change the lock with
`lock addl`, `lock xadd` and `lock subl`:
job_lock();
37d9f: f0 83 05 41 6f 14 00 lock addl $0x1,0x146f41(%rip) # 17ece8 <job+0x28>
37da6: 01
...
job_unlock();
37deb: f0 0f c1 05 f5 6e 14 lock xadd %eax,0x146ef5(%rip) # 17ece8 <job+0x28>
37df2: 00
37df3: 83 f8 01 cmp $0x1,%eax
37df6: 74 08 je 37e00 <job_subsave+0x70>
...
37e25: 74 11 je 37e38 <job_subsave+0xa8>
37e27: f0 83 2d b9 6e 14 00 lock subl $0x1,0x146eb9(%rip) # 17ece8 <job+0x28>
While this does work, it isn't portable. This patch implements a different
workaround for this compiler bug. If `job_lock` is put at the beginning of
`job_subsave`, GCC will generate the required `addl` and `sub` instructions:
job_lock();
37d67: 83 05 7a 5f 14 00 01 addl $0x1,0x145f7a(%rip) # 17dce8 <job+0x28>
...
job_unlock();
37dbb: 83 e8 01 sub $0x1,%eax
37dbe: 89 05 24 5f 14 00 mov %eax,0x145f24(%rip) # 17dce8 <job+0x28>
It is odd that moving a single line of code fixes this problem, although
GCC _should_ have generated these instructions from the original code anyway.
I'll note that this isn't the only way to get these instructions to generate.
The problem also seems to go away when inserting almost anything else inside
of the code for `job_subsave`. This is just a simple workaround for a strange
compiler bug.
This bug was previously reported in att/ast#37.
Ksh ignores `-r` when `read -r -d` is run because when
the bit for `D_FLAG` is set, the bit for `R_FLAG` is unset
as a side effect of setting `D_FLAG`. The following set
of commands fails to print a backslash:
$ printf '\\\000' | read -r -d ''
$ echo $REPLY
The fix for this bug is to set `D_FLAG` with `D_FLAG + 1`,
which prevents `R_FLAG` from being unset. This bugfix
has been backported from ksh93v- 2013-10-10-alpha.
src/cmd/ksh93/bltins/read.c:
- Set `D_FLAG` with `D_FLAG + 1` to prevent the bit for
`R_FLAG` from being unset.
src/cmd/ksh93/tests/builtins.sh:
- Add the regression test for `read -r -d` from ksh93v-.
Running shbench after undoing the incorrect subshell optimisation
showed that the performance of ${ subshare; }-type command
substitutions went down very slightly, but consistently.
The main purpose of using this ksh-specific type of command
substitution vs. a normal one is performance. Thus, it *is*
appropriate to eke every last bit of performance out of it that
we can, provided correctness is completely preserved.
It is also a type of command substitution where every change is
supposed to be shared with the main shell environment; only command
output is captured in a subshell-like fashion.
Thus, on the face of it, it would be a logical optimisation for
sh_assignok() to avoid bothering with saving a subshell context for
variables if we're in a subshare.
Lo and behold, applying it does not introduce any regress fails.
Here are my average results of the braces.ksh benchmark from
shbench <http://fossil.0branch.com/csb/tktnew> against stock
/bin/ksh 93u+ vs. current 93u+m (same compiler flags),
100 runs pre-optimisation and 100 runs post-optimisation:
Stock /bin/ksh: Pre-optimisation (at 3d38270b):
93u+: 0.743 secs 93u+m: 0.739 secs
Stock /bin/ksh: Post-optimisation (now):
93u+: 0.744 secs 93u+m: 0.726 secs
The left column shows only a small margin of error with 100 runs;
the right one shows a very small, but not insignificant difference.
However, these tests were not very rigorous with 100 runs each.
If anyone wants to do it properly, please report results to
korn-shell@googlegroups.com. I'm happy enough with this, though.
Thanks to Joerg van den Hoff for providing shbench, without
which it would not have occurred to me to try this.
src/cmd/ksh93/sh/subshell.c: sh_assignok():
- Don't bother if we're in a ${ subshare; }.
The fact that every ksh builtin command self-documents with the
options --help and --man (and others, see 'getopts --man'; but
these are the essential ones) is poorly known; the information is
buried somewhere deep in the sh.1 manual page, and is incomplete at
that. None of the terse usage messages displayed on error point the
user to these options. So let's fix that.
src/lib/libast/misc/optget.c:
- Change generic 'options' placeholder, used in all terse usage
messages, to 'options | --help | --man'.
src/cmd/ksh93/sh.1:
- Edit documentation of --man/-?, adding documentation on --help
which was completely undocumented. Refer to 'getopts --man' for
more advanced info.
- Separate these from the (important) documentation on special
builtins using a paragraph break.
ksh 93v- beta introduced a regression with nested command
substitutions: backticks nested in $( ) result in misdirected
output. This has never been in 93u+, but since we're often
backporting things, let's avoid backporting this bug. It is also
useful if this shows up when running our bin/shtests against the
actual beta by adding a SHELL=... argument.
Ref.: https://github.com/att/ast/issues/478
src/cmd/ksh93/tests/subshell.sh:
- Add reproducer submitted by the reporter as a regression test.
This bug was originally reported by @lijog in att/ast#7 and has been
reported again in #15. KSH does not save the state of a variable if it
is in a newer scope. This is because of an optimization in sh_assignok
first introduced in ksh93t+ 2010-05-24. Here is the code change in that
version:
return(np);
/* don't bother to save if in newer scope */
- if(!(rp=shp->st.real_fun) || !(dp=rp->sdict))
- dp = sp->var;
- if(np->nvenv && !nv_isattr(np,NV_MINIMAL|NV_EXPORT) && shp->last_root)
- dp = shp->last_root;
- if((mp=nv_search((char*)np,dp,HASH_BUCKET))!=np)
- {
- if(mp || !np->nvfun || np->nvfun->subshell>=sh.subshell)
- return(np);
- }
+ if(sp->var!=shp->var_tree && shp->last_root==shp->var_tree)
+ return(np);
if((ap=nv_arrayptr(np)) && (mp=nv_opensub(np)))
{
This change was originally made to replace a buggier optimization.
However, the current optimization causes variables set in subshells
to wrongly affect the environment outside of the subshell, as the
variable does not get set back to its original value. This patch
simply removes the buggy optimization to fix this problem.
src/cmd/ksh93/sh/subshell.c:
- Remove a buggy optimization that caused variables set in subshells
to affect the environment outside of the subshell.
src/cmd/ksh93/tests/subshell.sh:
- Add a regression test for setting variables in subshells. This
test has to be run from the disk after being created with a here
document because it always returns the expected result when run
directly in the regression test script.
The 'source' alias is now converted into a regular built-in command
so that 'unalias -a' does not remove it, and something like
cmd=source; $cmd name args
will now work.
This is part of the project to replace default aliases that define
essential commands by proper builtins that act identically (except
you now get the actual command's name in any error/usage messages).
src/cmd/ksh93/data/aliases.c:
- Remove 'source' default alias.
src/cmd/ksh93/data/builtins.c,
src/cmd/ksh93/include/builtins.h:
- Define 'source' regular builtin with extra parser ID "SYSSOURCE".
Same definition as '.', minus the BLT_SPC flag indicating a
special builtin. This preserves the behaviour of 'command .'.
- Update sh_optdot[] to include info for 'source --man'.
(Note that \f?\f expands to the current command name.
This allows several commands to share a single --man page.)
src/cmd/ksh93/sh/parse.c:
- In the two places that SYSDOT is checked for, also check for
SYSSOURCE, making sure the two commands are parsed identically.
src/cmd/ksh93/sh.1:
- Remove 'source' default alias.
- Document 'source' regular builtin.
This reverts the patch for the job_lock and job_unlock macros.
As I said in 58560db7, this is very probably a workaround for an
optimiser bug in certain versions of GCC, at least 2017 versions.
In the version I committed, that workaround version never gets
used, because you cannot use #if defined(...) to check for the
presence of a compiler builtin. Thanks to Johnothan King for
keeping an eye on my code and pointing that out to me.
What is needed to test for the presence of a compiler builtin is a
builtin macro called __has_builtin (and it *can* be tested for
using #if defined...()).. This is a clang invention. It was not
added to gcc until version 10, which was only released in a first
stable version just over a month ago.
Ref.: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66970#c14https://gcc.gnu.org/gcc-10/
However, for gcc 10, it seems unlikely the patch is still needed.
(Although it would certainly be a good idea to test that.)
And for the older gcc versions that do need it, we cannot use
__has_builtin, which means we need to define a dummy that always
returns false, so the workaround version is never used.
Ref.: https://github.com/ksh93/ksh/commit/58560db7#commitcomment-39900259
And we cannot use the workaround version unconditionally either,
because it would cause build failures on compilers without the
__sync_fetch_and_add() and __sync_fetch_and_sub() builtins.
Which means the only sensible thing left to do is to revert the
patch for now.
As far as I can tell at this point, for the patch to return to this
upstream in a sensible manner, someone would need to:
1. Write a small C program that tests these macros and somehow
checks for the presence of that optimiser bug. (This is not
going to come from me; my C-fu is simply not good enough.)
2. Incorporate that into the distribution as a test for iffe.
(Few know how iffe works, but it's probably not that hard as
there are plenty of existing tests to use as a template.)
3. Reinsert the workaround version using the macro defined (or not)
by that iffe test, so that it is only compiled in when using a
compiler that actually has the bug in question.
Until then, this can just continue to be an OS-specific patch for
systems with GCC versions that might have that bug.
The more I think about it, the more it seems obvious that commit
07cc71b8 (PR #14) is quite simply a workaround for a GCC optimiser
bug, and (who knows?) possibly an old, long-fixed one, as the bug
report is years old.
The commit also caused ksh to fail to build on HP-UX B.11.11 with
GCC 4.2.3 (hosted at polarhome.com), because it doesn't have
__sync_fetch_and_add() and __sync_fetch_and_sub(). It may fail on
other systems. The GCC documentation says these are legacy:
https://gcc.gnu.org/onlinedocs/gcc/_005f_005fsync-Builtins.html
HELP WANTED: what I would like best is if someone could come up
with some way of detecting this optimiser bug and then error out
with a message along the lines of "please upgrade your broken
compiler". It would probably need to be a new iffe test.
Meanwhile, let's try it this way for a while and see what happens:
src/cmd/ksh93/include/jobs.h:
- Restore original ksh version of job_lock()/job_unlock() macros.
- Use the workaround version only if the compiler has the builtins
__sync_fetch_and_add() and __sync_fetch_and_sub().
ksh segfaults in job_chksave after receiving SIGCHLD
https://bugs.launchpad.net/ubuntu/+source/ksh/+bug/1697501
Eric Desrochers wrote on 2017-06-12:
[Impact]
* The compiler optimization dropped parts from the ksh job
locking mechanism from the binary code. As a consequence, ksh
could terminate unexpectedly with a segmentation fault after
it received the SIGCHLD signal.
[Test Case]
Unfortunately, there is no clear and easy way to reproduce the
segfault.
* But the original reporter of this bug can randomly reproduce
the problem using an in-house ksh script that only works
inside his infrastructure as follow : "ksh
<in-house-script.ksh>" and then once in a while ksh will
segfault as follow :
(gdb) bt
#0 job_chksave (pid=pid@entry=19003) at /build/ksh-6IEHIC/ksh-93u+20120801/src/cmd/ksh93/sh/jobs.c:1948
#1 0x00000000004282ab in job_reap (sig=17) at /build/ksh-6IEHIC/ksh-93u+20120801/src/cmd/ksh93/sh/jobs.c:428
#2 <signal handler called>
...
[Regression Potential]
* Regression risk : low/none expected, the package has been
highly/intensively tested by a user who run over 18M ksh
scripts a day on each of their clusters.
[...]
* The fix has been written by RH and has been proven to work for
them for the last 3 years.
* A test package including the RH fix has been intensively tested
and verified (pre-SRU) by an affected user with positive
feedbacks using a reproducer that segfault without the RH
patch.
* Test package (pre-SRU) feedbacks :
https://bugs.launchpad.net/ubuntu/xenial/+source/ksh/+bug/1697501/comments/7
[Other Info]
* Details about the RH bug :
- https://bugzilla.redhat.com/show_bug.cgi?id=1123467
- https://bugzilla.redhat.com/show_bug.cgi?id=1112306
- https://access.redhat.com/solutions/1253243
- http://rhn.redhat.com/errata/RHBA-2014-1015.html
- ksh.spec
* Fri Jul 25 2014 Michal Hlavinka <email address hidden> - 20120801-10.8
* job locking mechanism did not survive compiler optimization (#1123467)
- patch
* ksh-20120801-locking.patch
Debian bug:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=867181
[Original Description]
# gdb
[New LWP 3882]
Core was generated by `/bin/ksh <KSH_SCRIPT>.ksh'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0 job_chksave (pid=pid@entry=19385) at /build/ksh-6IEHIC/ksh-93u+20120801/src/cmd/ksh93/sh/jobs.c:1948
1948 if(jp->pid==pid)
(gdb) p *jp
Cannot access memory at address 0xb
(gdb) p *jp->pid
Cannot access memory at address 0x13
(gdb) p pid
$2 = 19385
(gdb) p *jpold
$1 = {next = 0xb, pid = -604008960, exitval = 11124}
The struct is corrupted at some point looking at the next,pid and
exitval struct members values which isn't valid data.
# assembly code
=> 0x0000000000427159 <+41>: cmp %edi,0x8(%rdx)
(gdb) p $edi ## pid variable
$1 = 19385
(gdb) p *($rdx + 8) ## jp->pid struct
Cannot access memory at address 0x13
--
ksh is segfaulting because it can't access struct "jp" ($rdx)
thus cannot de-reference the struct member "jp>pid" ($rdx + 8) at
line : src/cmd/ksh93/sh/jobs.c:1948 when looking if jp->pid is
equal to pid ($edi) variable.
Prior to this bugfix, the following set of commands would
fail to print two double quotes:
IFS=',' read -S a b c <<<'foo,"""title"" data",bar'
echo $b
This fix is from ksh93v- 2013-10-10-alpha, although it has
been revised to use stakputc to put the required double quote
into the buffer for consistency with the ksh93u+ codebase.
src/cmd/ksh93/bltins/read.c:
- When handling nested double quotes, put the required double
quote in read's buffer with stakputc.
src/cmd/ksh93/tests/builtins.sh:
- Add the regression test for `read -S` from ksh93v-.
src/cmd/ksh93/sh.1:
- Fix a minor formatting error to highlight '-S' in the ksh(1)
man page.
Fixes for implicit declaration warnings
This is to upstream some minor fixes for implicit declaration warnings currently in Debian.
Author: Anuradha Weeraman aweeraman@gmail.com
Reviewed-By: Thorsten Glaser t.glaser@tarent.de
Last-Update: 2020-02-09
src/cmd/ksh93/edit/history.c:
- The `time_t` data type is usually 64-bit, so cast `t` to an
unsigned long and use the correct format specifier with sfprintf.
src/lib/libast/tm/tmdata.c:
- Update the leap second array by adding the current leap
seconds that were absent, being:
2016-12-31+23:59:60 1483228826
2015-06-30+23:59:60 1435708825
2012-06-30+23:59:60 1341100824
I couldn't find a proper list of leap seconds in Unix Epoch time,
so I used an Epoch Converter:
https://www.epochconverter.com/
The converter doesn't support leap seconds, so each conversion
had to be corrected by adding the number of leap seconds required
(e.g. 1230767999 + 24 = 1230768023, for 2008-12-31+23:59:60 in GMT).
'cd' with no argument is supposed to go to your home directory, but
in this override function it produced an error due to an incorrect
use of $@ within another parameter substitution, preventing $@ from
expanding to zero words.
Ref.: https://groups.google.com/d/msgid/korn-shell/781ed34b-6860-5d47-dfe8-be21280a6b31%40inlv.org
src/cmd/ksh93/fun/dirs: _cd():
- Fix the bug by setting a positional parameter if needed,
then using a normal and correct "$@" expansion for \cd.
- While we're here, restore a historic comment about using
'command' to bypass aliases that exists in the listing of this
function in the official 1995 KornShell book (pp. 274-276).
Presumably, that comment was deleted when they decided to add the
obnoxious and broken default alias command='command ', whenever
that was. This branch got rid of that default alias in 61d9bca5.
This fix for `printf '%T' now` on FreeBSD was written by
@krader1961. This is from https://github.com/att/ast/pull/591:
On FreeBSD calling tzset() does not guarantee the tzname array will
be correctly populated. On most systems that works but on FreeBSD you
have to call localtime() or a related function (e.g., ctime()).
This change also eliminates a potential, very small, memory leak due to
the strdup()'ed tznames not being freed.
src/lib/libast/tm/tminit.c:
- Fix timezone name determination on FreeBSD and a memory leak.
The regression this commit fixes was first introduced in ksh93t
2008-07-25. It was previously worked around in 6f0e008c by forking
subshells if any special environment variable is unset.
The reason why this problem doesn't occur in ksh93s+ is because in
that version of ksh sh_assignok never moves nodes, it only clones
them. The second argument doesn't set NV_MOVE, which makes
`sh_assignok(np,0)` is similar to `sh_assignok(np,1)`. In ksh93t and
higher, setting the second argument to zero causes the node to be moved
with NV_MOVE, which causes the discipline function associated with
the variable node to be removed when `np->nvfun` is set to zero (i.e.
NULL). This is why a command like `(unset LC_NUMERIC; LC_NUMERIC=invalid)`
doesn't print a diagnostic, as it looses its discipline function.
This patch fixes the problem by cloning the node with sh_assignok
if it is a special variable with a discipline function. This allows
special variables to work as expected in virtual subshells. The
original workaround has been kept for the $PATH variable only, as
hash tables are still broken in virtual subshells. It has been updated
accordingly to only fork subshells if it detects the variable node
for PATH. I have added two more regression tests for changing the
PATH in subshells to make sure hash tables continue working as
expected with this fix.
src/cmd/ksh93/bltins/typeset.c:
- Only fork virtual subshells if the PATH will be changed. If a
variable is a special variable with a discipline function, it
should be just be cloned, not moved.
src/cmd/ksh93/sh/nvdisc.c:
- Add a comment to clarify that NV_MOVE will delete the discipline
function associated with the node.
src/cmd/ksh93/tests/subshells.sh:
- Add two more regression tests for unsetting the PATH in subshells,
one for if PATH is being pointed to by a nameref. Condense the
hash table tests by moving the main test into a single function.
Test 137(C) was failing on some systems because $TMPDIR was set and
the local vi(1) honours it, so that the expected '/tmp/' string was
never output by vi. For compatibility with vi programs that honour
$TMPDIR and those that always use /tmp, we must export TMPDIR=/tmp.
src/cmd/ksh93/tests/pty.sh:
- Export TMPDIR=/tmp for test 137(C).
Note that this exports TMPDIR to the environment for the
duration of the 'tst' function run because the function was
defined using the ksh 'function tst { ...; }' syntax.
We cannot identify the new builtins as coming from "AT&T Research"
as they don't. 'ksh community' is accurate but a bit long. Since
we're now at a GitHub org called ksh93, let's just use that.
