src/cmd/ksh93/sh/xec.c: sh_funct():
- The np->nvalue.rp pointer was dereferenced before the check that
it is non-null. Do this check before dereferencing it.
Since at least 1999, whence -v on pdksh (and its successor mksh)
reports the path where an autoloadable function may be found:
$ mkdir ~/fun; FPATH=~/fun
$ echo 'myfn() { echo hi; }' >~/fun/myfn
$ whence -v myfn
myfn is a undefined (autoload from /home/user/fun/myfn) function
Whereas ksh93 only reports, rather uselessly:
myfn is an undefined function
As of this commit, whence -v/-a on ksh 93u+m does the same as
pdksh, but with correct grammar:
myfn is an undefined function (autoload from /home/user/fun/myfn)
This may be a small violation of my own "no new features" policy
for 93u+m, but I couldn't resist. This omission has been annoying
me, and it's just embarrassing to lack a pdksh feature :)
src/cmd/ksh93/include/path.h,
src/cmd/ksh93/data/msg.c:
- Add e_autoloadfrom[] = " (autoload from %s)" message.
src/cmd/ksh93/bltins/whence.c: whence():
- Report the path (if any) when reporting an undefined function.
This needs to be done in two places:
1. When a function has been explicitly marked undefined with
'autoload', we need to do a quick path_search() loop to find
the path. (These undefined functions take precedence over
regular commands, so are reported first.)
2. When a function is not explicitly autoloaded but merely
available in $FPATH, that path search was already done, so all
we need to do is report it. (These are reported last.)
Note that the output remains as on 93u+ if no function definition
file is found on $FPATH. This is also like pdksh/mksh.
src/cmd/ksh93/data/builtins.c:
- Bump 'whence' version date. The inline docs never detailed very
exactly what 'whence -v' reports, so no need for further edits.
src/cmd/ksh93/tests/path.sh:
- Regress-test the new whence behaviour plus actual autoloading,
including the command override behaviour of autoloaded functions.
The fixargs() function is invoked when ksh needs to run a script
without a #!/hashbang/path. Instead of letting the kernel invoke a
shell, ksh exfile()s the script itself from sh_main(). In the
forked child, it calls fixargs() to set the argument list in the
environment to the args of the new script, so that 'ps' and
/proc/PID/cmdline show the expected output.
But fixargs() is broken because, on systems other than HP-UX (on
which ksh uses pstat(2)), ksh simply inserts a terminating zero.
The arguments list is not a zero-terminated C string. Unix systems
expect the entire arguments buffer to be zeroed out, otherwise 'ps'
and /proc/*/cmdline will have fragments of previous command lines
in the output.
The Red Hat patch for this bug is:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-argvfix.patch
However, that fix is incomplete because 'command_len' was also
hardcoded to be limited to 64 characters (!), which still gave
invalid 'ps' output if the erased command line was longer.
src/cmd/ksh93/sh/main.c: fixargs():
- Remove CMD_LENGTH macro which was defined as 64.
- Remove code that limited the erasure of the arguments buffer to
CMD_LENGTH characters. That code also had quite a dodgy strdup()
call -- it copies arguments to the heap, but they are never freed
(or even used), so it's a memory leak. Also, none of this is
ever done if the length is calculated using pstat(2) on HP-UX,
which is a clear indication that it's unnecessary.
(I think this code block must have been some experiment they
forgot to remove. One reason why I think so is that a 64 byte
arguments limit never made sense, even in the 1980s when they
wrote ksh on 80-column CRT displays. Another indication of this
is that fixing it didn't require adding anything; the code to do
the right thing was already there, it was just being overridden.)
- Zero out the full arguments length as in the Red Hat patch.
src/cmd/ksh93/tests/basic.sh:
- Add test. It's sort of involved because 'ps' is one of the least
portable commands in practice, in spite of standardisation.
This fixes a regression introduced in commit f9c127e3.
When the legacy code for older versions of libast was
removed, the fmtident wrapper wasn't removed. As a result,
the version string output by Ctrl+Alt+V is garbled because
the fmtident wrapper doesn't do any formatting:
$ <Ctrl+Alt+V>
^J@(#)$Id: Version AJM 93u+m 2020-09-14
src/cmd/ksh93/sh/string.c:
- Remove the old version of fmtident that was overriding
the current version of fmtident provided by libast
(in src/lib/libast/string/fmtident.c).
There was no check for the -B/braceexpand option before calling
path_expand() to process brace expansion, making it impossible to
turn off brace expansion within command substitutions. Normally the
lexer flags brace expansion so that this code is not reached, but
shell code within command substitutions is handled differently.
Red Hat patches this by adding this check to the function itself:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20140301-fikspand.patch
But I think it's more logical to patch it at the point of decision.
src/cmd/ksh93/sh/macro.c: endfield():
- Decide to call either path_generate() or path_expand() based on
the state of the SH_BRACEEXPAND shell option.
- Fix '#if SHOPT_BRACEPAT' preprocessor check that previously
hardcoded this decision at compile time.
src/cmd/ksh93/tests/options.sh:
- Add tests.
The new command substitution mechanism imported in 970069a6 from
Red Hat patches introduced this bug: backtick-style command
substitutions hang when processing about 117KiB of data or more.
It is fixed by another Red Hat patch:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20140415-hokaido.patch
It saves the value of the shp->comsub flag so that it is set to 2
(usually meaning new-style $(comsubs)) in two specific cases even
when processing backtick comsubs. This stops the sh_subtmpfile()
function in subshell.c from creating a /tmp file. However, I think
that approach is quite ugly, so I'm taking a slightly different one
that has the same effect.
src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/subshell.c:
- Redefine sh_subtmpfile() to pass the comsub flag as an argument.
(Remove the shp pointer argument, which is redundant; a pointer
to the shell state can easily be obtained in the function.)
src/cmd/ksh93/sh/xec.c: sh_exec():
- Apply the Red Hat fix by passing flag 2 to sh_subtmpfile().
src/cmd/ksh93/tests/subshell.sh:
- Move regress test from ce68e1be from basic.sh to here; this is
the place for command substitution tests as they are subshells.
- Add regress test for this bug.
All other changed files:
- Update sh_subtmpfile() calls to pass on the shp->comsub flag.
When using typeset -l or -u on a variable that cannot be changed
when the shell is in restricted mode, ksh crashed.
This fixed is inspired by this Red Hat fix, which is incomplete:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-tpstl.patch
The crash was caused by the nv_shell() function. It walks though a
discipline function tree to get the pointer to the interpreter
associated with it. Evidently, the problem is that some pointer in
that walk is not set correctly for all special variables.
Thing is, ksh only has one shell language interpreter, and only one
global data structure (called 'sh') to keep its main state[*]. Yet,
the code is full of 'shp' pointers to that structure. Most (not
all) functions pass that pointer around to each other, accessing
that struct indirectly, ostensibly to account for the non-existent
possibility that there might be more than one interpreter state.
The "why" of that is an interesting cause for speculation that I
may get to sometime. For now, it is enough to know that, in the
code as it is, it matters not one iota what pointer to the shell
interpreter state is used; they all point to the same thing (unless
it's broken, as in this bug).
So, rather than fixing nv_shell() and/or associated pointer
assignments, this commit simply removes it, and replaces it with
calls to sh_getinterp(), which always returns a pointer to sh (see
init.c, where that function is defined as literally 'return &sh').
[*] Defined in shell.h, with the _SH_PRIVATE part in defs.h
src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/name.c:
- Remove nv_shell().
src/cmd/ksh93/sh/init.c:
- In all the discipline functions for special variables, initialise
shp using sh_getinterp() instead of nv_shell().
src/cmd/ksh93/tests/variables.sh:
- Add regression test for typeset -l/-u on all special variables.
Now that we have ${.sh.pid} a.k.a. shgd->current_pid, which is
updated using getpid() whenever forking a new process, there is no
need for anything else to ever call getpid(); we can use the stored
value instead. There were a lot of these syscalls kicking around,
some of them in performance-sensitive places.
The following lists only changes *other* than changing getpid() to
shgd->currentpid.
src/cmd/ksh93/include/defs.h:
- Comments: clarify what shgd->{pid,ppid,current_pid} are for.
src/cmd/ksh93/sh/main.c,
src/cmd/ksh93/sh/init.c:
- On reinit for a new script, update shgd->{pid,ppid,current_pid}
in the sh_reinit() function itself instead of calling sh_reinit()
from sh_main() and then updating those immediately after that
call. It just makes more sense this way. Nothing else ever calls
sh_reinit() so there are no side effects.
src/cmd/ksh93/sh/xec.c: _sh_fork():
- Update shgd->current_pid in the child early, so that the rest of
the function can use it instead of calling getpid() again.
- Remove reassignment of SH_PIDNOD->nvalue.lp value pointer to
shgd->current_pid (which makes ${.sh.pid} work in the shell).
It's constant and was already set on init.
This imports another fix from Red Hat/Fedora. Original patch:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-crash.patch
src/cmd/ksh93/include/jobs.h,
src/cmd/ksh93/sh/jobs.c,
src/cmd/ksh93/sh/subshell.c,
src/cmd/ksh93/sh/xec.c:
- Import the Red Hat fix with these differences:
- Rename the 'hack1_waitall' variable to 'bktick_waitall' and add
a comment describing what it's for.
- Remove unused 'pipefail' variable.
src/cmd/ksh93/tests/basic.sh:
- Regression test from reproducer given in the Red Hat bug report.
- Add special handling to SIGKILL it, as it might freeze hard.
var=$(< file) now reads the file even if the standard inout,
standard output and/or standard error file descriptors are closed.
Original patch:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-filecomsubst.patch
src/cmd/ksh93/sh/io.c: sh_redirect():
- When processing the '<' redirector as part of $(< ...), i.e. if
flag==3, make sure the FD of the file to read is > 2 by calling
sh_iomovefd(). Unlike the RedHat patch, this checks for flag==3
to avoid unnecessary sh_iomovefd() calls for normal redirections,
as there was no bug with those.
src/cmd/ksh93/tests/io.sh:
- Add test.
When ksh was compiled with SHOPT_SPAWN (the default), any command
substitution embedded in a here-document returned an empty string.
The bug was also present in 93u+ 2012-08-01 (although not in every
case as some systems compile it without SHOPT_SPAWN).
This fixes it by applying a slightly edited combination of two Red
Hat patches (the second containing a fix for the first), which
backport a new command substitution mechanism from the abandoned
ksh 93v- beta version. The originals are:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-macro.patchhttps://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-fd2lost.patch
src/cmd/ksh93/include/io.h:
- The iopipe() function from xec.c is now needed in sh_subshell()
(subshell.c), so rename it to sh_iounpipe() and declare it as an
extern here. The 93v- beta did it as well. (The Red Hat patch did
this without renaming it.)
src/cmd/ksh93/sh/xec.c:
- Backport new versions of iousepipe() and sh_iounpipe() from ksh
93v-. New 'type' flaggery is introduced to distinguish between
different command substitution conditions. What all that means
remains to be determined.
- sh_exec(): I made one change to the Red Hat patch myself: if in a
subshell and the type flags FAMP (for "ampersand" as in '&' as in
background job) and TFORK are set, continue to call sh_subfork()
to fork the subshell unconditionally, instead of only if we're in
a command substitution connected to an unseekable file. Maybe the
latter works for the 93v- code, but on 93u+(m) it causes a couple
of regressions, which are fixed by my change:
signal.sh[273]: subshell ignoring signal does not send signal to parent
signal.sh[276]: subshell catching signal does not send signal to parent
Details: https://github.com/ksh93/ksh/issues/104#issuecomment-696341902
src/cmd/ksh93/sh/macro.c,
src/cmd/ksh93/sh/subshell.c:
- Updates that go with those new functions.
Fixes: https://github.com/ksh93/ksh/issues/104
Affects: https://github.com/ksh93/ksh/issues/124
This fixes two memory leaks in old-style command substitutions
(one when invoking an alias, one when invoking an autoloaded
function), as well as a possible third leak with an unknown
reproducer, by applying this Red Hat patch:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-mlikfiks.patch
src/cmd/ksh93/sh/macro.c: comsubst():
- For as-yet unknown reasons, the alias leak did not occur when
adding a space at the end of the command substitution, as in
a=`some_alias `. This fix is a workaround that simply writes
an extra space to the stack. TODO: a real fix.
src/cmd/ksh93/sh/path.c: funload():
- Add missing free() before return. This fixes the leak with
autoloaded functions.
src/cmd/ksh93/sh/lex.c: alias_exceptf():
- This function is called "whenever an end of string is found with
alias". This adds a check for an SF_FINAL stream status flag when
deciding whether to call free(). In sfio.h this is commented as:
#define SF_FINAL 11 /* closing is done except stream free */
When I revert this change, none of the regression tests fail, so
I don't know how to trigger this supposed leak. But it makes some
sense given the sfio.h comment, so I'll keep it.
src/cmd/ksh93/tests/leaks.sh:
- Add the reproducers from rhbz#982142 as regression tests
(including an extra one for nested command substitutions that was
already fixed as of 93u+, but testing is good).
I replaced the external 'expr' and 'ls' commands by uses of
the 'true' builtin, otherwise the tests take far too long to run
with 16384 iterations. At least the alias leak was still behaving
identically after replacing 'ls' by 'true'.
Hopefully this doesn't introduce new bugs, but it does fix at
least the following:
1. When whence -v/-a found an "undefined" (i.e. autoloadable)
function in $FPATH, it actually loaded the function as a side
effect of reporting on its existence (!). Now it only reports.
2. 'whence' will now canonicalise paths properly. Examples:
$ whence ///usr/lib/../bin//./env
/usr/bin/env
$ (cd /; whence -v dev/../usr/bin//./env)
dev/../usr/bin//./env is /usr/bin/env
3. 'whence' no longer prefixes a spurious double slash when doing
something like 'cd / && whence bin/echo'. On Cygwin, an initial
double slash denotes a network server, so this was not just a
cosmetic problem.
4. 'whence -a' now reports a "tracked alias" (a.k.a. hash table
entry, i.e. cached $PATH search) even if an actual alias by the
same name exists. This needed fixing because in fact the hash
table entry continues to be used when bypassing the alias.
Aliases and "tracked aliases" are not remotely the same thing;
confusing nomenclature is not a reason to report wrong results.
5. When using 'hash' or 'alias -t' on a command that is also a
builtin to force caching a $PATH search for the external
command, 'whence -a' double-reported the path:
$ hash printf; whence -a printf
printf is a shell builtin
printf is /usr/bin/printf
printf is a tracked alias for /usr/bin/printf
This is now fixed so that the second output line is gone.
Plus, if there were multiple versions of the command on $PATH,
the tracked alias was reported at the end, which is the wrong
order. This is also fixed.
src/cmd/ksh93/bltins/whence.c: whence():
- Refactor the do...while loop that handles whence -v/-a for path
searches in such a way that the code actually makes sense and
stops looking like higher esotericism. Just doing this fixed#2,
#4 and #5 above (the latter two before I even noticed them). For
instance, the path_fullname() call to canonicalise paths was
already there; it was just never used.
- Remove broken 'notrack' flaggery for deciding whether to report a
hash table entry a.k.a. "tracked alias"; instead, check the hash
table (shp->track_tree).
src/cmd/ksh93/sh/path.c:
- path_search(): Re #3: When prefixing the PWD, first check if
we're in '/' and if so, don't prefix it; otherwise, adding the
next slash causes an initial double slash. (Since '/' is the only
valid single-character absolute path, all we need to do is check
if the second character pwd[1] is non-null.)
- path_search(): Re #1: Stop autoloading when called by 'whence':
* The 'flag==2' check to avoid autoloading a function was
broken. The flag value is 2 on the first whence() loop
iteration, but 3 on subsequent ones. Change to 'flag >= 2'.
* However, this only fixes it if the function file does not have
the x permission bit, as executable files are handled by
path_absolute() which unconditionally autoloads functions!
So, pass on our flag parameter when callling path_absolute().
- path_absolute(): Re #1: Add flag parameter. Do not autoload
functions if flag >= 2.
src/cmd/ksh93/include/path.h,
src/cmd/ksh93/bltins/typeset.c,
src/cmd/ksh93/sh/main.c,
src/cmd/ksh93/sh/xec.c:
- Re #1: Update path_absolute() calls, adding a 0 flag parameter.
src/cmd/ksh93/include/name.h:
- Remove now-unused pathcomp member from union Value. It was
introduced in 99065353 to allow examining the value of a tracked
alias. This commit uses nv_getval() instead.
src/cmd/ksh93/tests/builtins.sh,
src/cmd/ksh93/tests/path.sh:
- Add and tweak various related tests.
Fixes: https://github.com/ksh93/ksh/issues/84
{Brace,expansion} is potentially incompatible with POSIX scripts,
because in POSIX those are simple literal strings with no special
meaning. So the POSIX option should really turn that off.
As of b301d417, the 'posix' option was also forcing 'letoctal'
behaviour on, without actually setting that option. I've since
found that to be a botch; 'let' may recognise octals without that
option being set, and that looks like a bug.
So as of this commit, the '-o posix' option actually toggles both
of these options off/on and on/of, respectively. 'set +o posix'
toggles them inversely. However, it is now possible to control both
options (and their associated behaviour) independently in between
'set -o posix' and 'set +o posix'. Much better.
src/cmd/ksh93/sh/main.c: sh_main():
- If SH_POSIX was set on init, turn on SH_LETOCTAL by default
instead of SH_BRACEEXPAND.
src/cmd/ksh93/sh/args.c: sh_applyopts():
- Turn off SH_BRACEEXPAND and turn on SH_LETOCTAL when SH_POSIX is
turned on (but not if it was already on).
- Turn on SH_BRACEEXPAND and turn off SH_LETOCTAL when SH_POSIX is
turned off (but not if it was already off).
src/cmd/ksh93/sh/arith.c: arith():
- Revert to pre-b301d417 and only check SH_LETOCTAL option when
deciding whether 'let' should skip initial zeros.
src/cmd/ksh93/tests/options.sh:
- Update $- test to allow '-o posix' to switch B = braceexpand.
src/cmd/ksh93/sh.1:
- Update.
- Edit for clarity.
This allows running 'bin/shtests leaks' on a ksh without the
vmstate builtin and/or that is not compiled with AST vmalloc.
It falls back to 'ps -o rss= -p $$' to get the memory state.
This is in preparation for the beta and release versions, which
will not use vmalloc due to its defects[*]. Unfortunately,
abandoning vmalloc means abandoning the vmstate builtin which makes
it extremely efficient to test for memory leaks.
Because 'ps' only has a KiB granularity and also shows larger
artefacts/variations than vmalloc on most systems, we need many
more iterations (16384) and also tolerate a higher number of bytes
per iterations (8). So the run takes much longer. To tolerate only
2 bytes per iteration, we would need four times as many iterations,
which would make it take too long to run. Since a single word (e.g.
one pointer) on a 64-bit system is 8 bytes, it still seems very
unlikely for a real memory leak to be that small.
This is coded to make it easy to detect and add iteration and
tolerance parameters for a new method to get the memory state,
if some efficient or precise system-specific way is discovered.
I've also managed to trigger a false leak with shcomp in a UTF-8
locale on CentOS on a ksh with vmalloc/vmstate. So this increases
the tolerance for vmalloc from 2 to 4 bytes/iteration.
[*] Discussion: https://github.com/ksh93/ksh/issues/95
When compiling ksh with '-O0 -g -D_std_malloc' on my Mac, the
paths.sh regress test set crashed. This is the test that crashed:
print 'FPATH=../fun' > bin/.paths
cat <<- \EOF > fun/myfun
function myfun
{
print myfun
}
EOF
x=$(FPATH= PATH=$PWD/bin $SHELL -c ': $(whence less);myfun') 2> /dev/null
[[ $x == myfun ]] || err_exit 'function myfun not found'
The crash occurred on the second-to-last line. The backtrace
suggests an invalid use of strcpy() with overlapping memory:
0 libsystem_kernel.dylib __pthread_kill + 10
1 libsystem_pthread.dylib pthread_kill + 284
2 libsystem_c.dylib abort + 127
3 libsystem_c.dylib abort_report_np + 177
4 libsystem_c.dylib __chk_fail + 48
5 libsystem_c.dylib __chk_fail_overlap + 16
6 libsystem_c.dylib __chk_overlap + 34
7 libsystem_c.dylib __strcpy_chk + 64
8 ksh path_chkpaths + 1038 (path.c:1534)
9 ksh path_addcomp + 1032 (path.c:1481)
10 ksh path_addpath + 395 (path.c:1598)
11 ksh put_restricted + 626 (init.c:329)
[...]
src/cmd/ksh93/sh/path.c: path_chkpaths():
- When reading the '.paths' file, use memmove(3) instead of
strcpy(3) as the former does a non-destructive copy with
tolerance for overlap.
The following set of commands caused ksh to crash:
$ unalias history; unalias r
Memory fault
When ksh is compiled with -D_std_malloc, the crash always
occurs when the 'r' alias is removed with 'unalias r',
although with vmalloc 'unalias history' must be run first
for the crash to occur. With the native malloc, the crash
message is also different:
$ unalias history; unalias r
free(): invalid pointer
Abort
This crash happens because when an alias is unset, _nv_unset
removes the NV_NOFREE flag which results in an invalid use
of free(3) as nv_isattr no longer detects NV_NOFREE afterward.
The history and r aliases shouldn't be freed from memory by
nv_delete because those aliases are given the NV_NOFREE attribute.
src/cmd/ksh93/bltins/typeset.c:
- Save the state of NV_NOFREE for aliases to fix the crash
caused by 'unalias r'.
src/cmd/ksh93/tests/alias.sh:
- Use unalias on both history and r to check for the crash.
'unalias -a' can't be used to replicate the crash.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
This patch from Red Hat fixes the following:
1. ksh was ignoring the -m (-o monitor) option when specified on
the invocation command line.
2. Scripts did not properly terminate their background processes
on Ctrl+C if the -m option was turned off. Reproducer:
xterm &
read junk
When run as a script without turning on -m, pressing Ctrl+C
should terminate the xterm, and now does.
3. Scripts no longer attempt to set the terminal foreground process
group ID, as only interactive shells should be doing that.
This makes some progress on https://github.com/ksh93/ksh/issues/119
but we're a long way from fixing all of that.
src/cmd/ksh93/sh/main.c: exfile():
- On non-interactive shells, do not turn off the monitor option.
Instead, if it was turned on, turn on the SH_MONITOR state flag.
src/cmd/ksh93/edit/edit.c: ed_getchar():
- On Ctrl+C, issue SIGINT to the current process group using
killpg(2) instead of going via sh_fault(), which handles a
signal only for the current shell process.
src/cmd/ksh93/sh/jobs.c: job_reap(), job_reset(),
src/cmd/ksh93/sh/xec.c: sh_exec():
- Only attempt to set the terminal foreground process group ID
using tcsetpgrp(3) if the shell is interactive.
Original patch: https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-kshmfix.patch
This was applied to Red Hat's ksh 93u+ on 8 July 2013.
src/cmd/ksh93/data/signals.c includes two checks for the JOBS
identifier; if it is not defined then the interactive shell's
background job signal messages for SIGINT and SIGPIPE are empty.
The cause was that the "jobs.h" header, which defines that ID, was
not #included in signals.c. This commit adds that #include.
(ksh 93u+, ksh 93v- and ksh2020 all have this bug as well.)
Before:
$ sleep 30 &
[1] 86430
$ kill -s INT "$!"
[1] + sleep 30 &
$
After:
$ sleep 30 &
[1] 86445
$ kill -s INT "$!"
[1] + Interrupt sleep 30 &
$
In the vi and emacs line editors, repeat count parameters can now
also be used for the arrow keys and the forward-delete key. E.g.,
in emacs mode, <ESC> 7 <left-arrow> will now move the cursor seven
positions to the left. In vi control mode, this would be entered
as: 7 <left-arrow>.
src/cmd/ksh93/edit/emacs.c:
- ed_emacsread(): Upon getting ^[ (ESC), save current repeat count
in a new variable; restore and reset it upon the next character.
- escape(): Minor bugfix: when processing a ^[[x sequence where 'x'
is a character other than '~' (which would be DEL), also reinsert
the final character into the buffer so scripts can detect them.
src/cmd/ksh93/edit/vi.c:
- cntlmode(): Do not reset the repeat count if the command is '[',
the character following ESC in VT220 escape sequences.
- mvcursor():
* Do not use getcount() to get the character following '[', as
that was parsing repetition parameters in the wrong place.
There wouldn't be any, so this would reset the repeat count.
* After that, no more need for the special-casing of ^[[3~ (DEL)
introduced in f2a3f4e3. Move it to within the 'switch' block.
* When handling left and right arrows and Home and End keys, do
not modify cursor directly but ed_ungetchar() the corresponding
traditional command keys as with the rest. Otherwise a repeat
count parameter would now wrongly survive those keys.
src/cmd/ksh93/sh.1:
- Document control character notation used for vi mode docs.
- Since vi control mode beeps and aborts on ESC except if a
subsequent [ is already in the input buffer upon receiving ESC,
document that VT220 escape sequences only preserve repeat counts
when entered into the input buffer all at once.
- Don't skip the initial ESC in the documentation of the VT220
escape sequences. In control mode, skipping the initial ESC still
works as before, but that is now undocumented, as it's really
nothing more than an artefact of VT220 escape processing.
- Move the two long paragraphs on '-o viraw' and canonical (i.e.
line-based) input processing from the vi editor introduction to
the options section under 'viraw'. It is much too arcane for the
intro, and besides, ksh 93u+ (and hence also 93u+m) has
SHOPT_VIRAW enabled by default, so the shell is compiled to force
this option on at all times, making it even less relevant for
most users.
A memory leak occurred when typeset was used in a function called
from within a command substitution. This fix was backported from
the 93v- beta by Red Hat on 22 Jan 2014. Source:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-memlik3.patch
src/cmd/ksh93/include/name.h,
src/cmd/ksh93/sh/subshell.c:
- Replace the nv_subsaved() function by the version from ksh 93v-.
This version frees a table from memory if the NV_TABLE flag is
passed in the new second parameter, a bitmask for flags (which
was oddly named 'table'; I've renamed it to 'flags').
src/cmd/ksh93/sh/name.c:
- nv_delete(): When calling nv_subsaved(), pass on the NV_TABLE
flag if given.
- table_unset(): Call nv_delete() with the NV_TABLE flag.
src/cmd/ksh93/tests/leaks.sh:
- Add test based on the reproducer provided in Red Hat bug 1036470.
I now have access to some of the private bugs on the Red Hat bug
tracker. This one doesn't have a lot of information on the patch,
but it contains a good reproducer, so we can at least verify that
it works.
src/cmd/ksh93/sh/array.c,
src/cmd/ksh93/sh/name.c:
- Apply the patch associated with Red Hat bug #921455. Source:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-memlik.patch
This was applied to Red Hat's ksh on 04 Jul 2013.
src/cmd/ksh93/tests/leaks.sh:
- Add leak tests for associative and indexed arrays in functions
based on the reproducer from rhbz#921455.
- Both tests still leak (though much less) when run in a locale
other than C. For now, temporarily set the locale to C and add
a TODO note. Perhaps another Red Hat patch is yet to fix this.
One of the few AT&T fixes applied in early 2020 was a one-line
change to emacs.c tab handling, with only this info in the commit
message:
| - fix to emacs.c (I think from dgk)
So, it's unknown what that was meant to accomplish, but I did just
find that it breaks menu-driven pathname completion:
$ ls arch/darwin.i386-64/
1) bin/
2) fun/
3) include/
4) lib/
5) man/
6) src/
$ ls arch/darwin.i386-64/3 _
Typing 3+TAB should have inserted 'include/' but inserted a literal
tab instead. Reverting the vague "fix" fixes this bug.
On every modern system, the forward-delete key on PC/Mac keyboards
generates the VT220 sequence ESC [ 3 ~. Every other shell with an
editor handles this now, ksh93 seems to be the last not to.
src/cmd/ksh93/edit/emacs.c: escape():
- Handle the ^[[3 as part of normal escape processing, then read an
extra character to check for the final '~'. If detected, insert
an ERASECHAR key event.
src/cmd/ksh93/edit/vi.c: mvcursor():
- Replace the ^[[3~ sequence by an 'x' command. We have to
special-case its processing, because vi mode parses numbers as
repetition operators. The escape sequence contains a number,
making it incompatible with normal command handling. This means
number repetitions don't work with the forward-delete key. If
that annoys anyone enough to fix it, a patch would be welcome.
For now, it will do to make the forward-delete key stop
exhibiting bizarre behaviour (beep + change case + move forward).
src/cmd/ksh93/sh.1
- Copy-edit emacs documentation for VT220-style sequences; map them
to their actual key, otherwise it's meaningless to the reader.
- Document the new forward-delete key behaviour for emacs mode.
- Leave the forward-delete key for vi mode undocumented for now, as
repetitions don't work, so it doesn't really match the vi canon.
(OTOH, it doesn't work in vim, either...)
_sfcvt(), "convert a floating point value to ASCII", did not adjust
for negative decimal place movement as what happens with leading
zeroes. This caused ksh's 'printf %f' formatter to fail to round
floating point values correctly.
src/lib/libast/sfio/sfcvt.c:
- Removed constraint of <1e-8 for doubles by matching what was done
for long doubles having <.1.
- Corrected a condition when the next power of 10 occurred and that
new 1 digit was being overwritten by a 0.
src/cmd/ksh93/tests/math.sh:
- Validate that typeset -E/F formatting matches that of their
equivalent printf formatting options as well as checking for
correct float scaling of the fractional parts.
The fix was incomplete: expansions using '?' (${var?w(ord},
${var:?wo)rd}) still did not tolerate parentheses in the word
as regular characters.
It was also possible to simplify the fix by making use of the
ST_BRACE (sh_lexstate7[]) state table. See data/lexstates.c and
include/lexstates.h.
src/cmd/ksh93/sh/lex.c: sh_lex(): case S_MOD1:
- The previous fix tested for modifier operator characters : - + =
as part of the S_MOD2 case, though they are defined as S_MOD1 in
the ST_BRACE state table. It only worked because of the
fallthrough. And it turns out the S_MOD1 case already had a
similar fix, though incomplete. The new fix effectively cancelled
the old one out as any S_MOD1 character eventually led to
'continue'. So it can be simplified by removing most of that
code, without causing any change in behaviour. Only the mode
change to the ST_QUOTE state table followed by 'continue' is
necessary. This also fixes it for the '?' operator as that is
also defined as S_MOD1 in the ST_BRACE state table.
src/cmd/ksh93/sh/macro.c:
- When skipping a ${...} expansion using sh_lexskip(), use the
ST_QUOTE state table if the character c is an S_MOD1 modifier
operator character. This makes it consistent with the S_MOD1
handling in sh_lex().
src/cmd/ksh93/tests/variables.sh:
- Update regression tests to include ? and :? operators.
File descriptors are not properly closed, causing a leak, when
using a process substitution as an argument to a shell function.
See: https://github.com/ksh93/ksh/issues/67
Process substitution uses /dev/fd/NN pseudofiles if the kernel
provides them. This is tested in src/cmd/ksh93/features/options
which causes SHOPT_DEVFD to be defined if /dev/fd/9 can be used.
If not, ksh uses a fallback mechanism involving a temporary FIFO,
which works on all Unix variants.
As it happens, the leak only occurs when using the /dev/fd
mechanism. So, until a fix is found, we can work around the bug by
disabling it. The FIFO mechanism might be slightly less robust,
but it's an improvement over leaking file descriptors. Plus, there
is room for improving it.
src/cmd/ksh93/include/defs.h:
- Unconditionally redefine SHOPT_DEVFD as 0 for now.
src/cmd/ksh93/sh/args.c: sh_argprocsub():
- pathtemp() does appropriate access checks using access(2), but
there is an inherent race condition between calling it and
mkfifo(). Make the FIFO mechanism more robust by handling errors,
trying again if an error occurs that must have resulted from
losing that race, e.g. file name conflict or temp dir
permission/location change.
- Initially create the FIFO without any permissions, then chmod()
the appropriate user read/write permissions. Since mkfifo()
honours the umask and chmod() does not, this ensures that process
substitution continues to work if a shell script sets a umask
that disallows user read or write. (The /dev/fd/ mechanism does
not care about the umask, so neither should the fallback.)
To find the temporary files directory to use, the pathtemp()
function (generate a unique path to a temporary file) first checks
$TMPDIR and $TMPPATH, then falls back to /tmp, then to /usr/tmp as
a last resort. But all systems replaced /usr/tmp by /var/tmp
decades ago to allow mounting /usr as read-only, and a /usr/tmp
compatibility symlink is no longer commonly provided.
src/lib/libast/path/pathtemp.c:
- Change TMP2 definition from "/usr/tmp" to "/var/tmp".
src/lib/libast/features/mmap,
src/lib/libast/features/stdio:
- Change "/usr/tmp" to "/var/tmp" in feature tests.
Following a community discussion, it became clear that 'r' is
particularly problematic as a regular builtin, as the name can and
does conflict with at least one legit external command by that
name. There was a consensus against removing it altogether and
letting users set the alias in their login scripts. However,
aliases are easier to bypass, remove or rename than builtins are.
My compromise is to reinstate 'r' as a preset alias on interactive
shells only, along with 'history', as was done in 17f81ebe before
they were converted to builtins in 03224ae3. So this reintroduces
the notion of predefined aliases to ksh 93u+m, but only for
interactive shells that are not initialised in POSIX mode.
src/cmd/ksh93/Makefile,
src/cmd/ksh93/Mamfile,
src/cmd/ksh93/include/shtable.h,
src/cmd/ksh93/data/aliases.c:
- Restore aliases.c containing shtab_aliases[], a table specifying
the preset aliases.
src/cmd/ksh93/include/shtable.h,
src/cmd/ksh93/sh/init.c:
- Rename inittree() to sh_inittree() and make it extern, because we
need to use it in main.c (sh_main()).
src/cmd/ksh93/sh/main.c: sh_main():
- Init preset aliases from shtab_aliases[] only if the shell is
interactive and not in POSIX mode.
src/cmd/ksh93/bltins/typeset.c,
src/cmd/ksh93/tests/alias.sh:
- unall(): When unsetting an alias, pass on the NV_NOFREE attribute
to nv_delete() to avoid an erroneous attempt to free a preset
alias from read-only memory. See: 5d50f825
src/cmd/ksh93/data/builtins.c:
- Remove "history" and "r" entries from shtab_builtins[].
- Revert changes to inline fc/hist docs in sh_opthist[].
src/cmd/ksh93/bltins/hist.c: b_hist():
- Remove handling for 'history' and 'r' as builtins.
src/cmd/ksh93/sh.1:
- Update accordingly.
Resolves: https://github.com/ksh93/ksh/issues/125
The 'command' name can now result from an expansion, e.g.:
c=command; "$c" ls
set -- command ls; "$@"
both work now. This fixes BUG_CMDEXPAN.
If -o posix is on, 'command' now disables not only the "special"
but also the "declaration" properties of builtin commands that it
invokes. This is because POSIX specifies 'command' as a simple
regular builtin, and any command name following 'command' is just
an argument to the 'command' command, so there is nothing that
allows any further arguments (such as assignment-arguments) to be
treated specially by the parser. So, if and only if -o posix is on:
a. Arguments that start with a variable name followed by '=' are
always treated as regular words subject to normal shell syntax.
b. Since assignment-arguments are not processed as assignments
before the command itself, 'command' can now stop the shell from
exiting (as required by the standard) if a command that it
invokes (such as 'export') tries to modify a readonly variable.
This fixes BUG_CMDSPEXIT.
Most of 'command' is integrated in the parser and parse tree
executer, so that is where it needed fixing.
src/cmd/ksh93/sh/parse.c: simple():
- If the posix option is on, do not skip past SYSCOMMAND so that
any declaration builtin commands that are arguments to 'command'
are not detected and thus not treated specially at parsetime.
src/cmd/ksh93/sh/xec.c: sh_exec():
- When detecting SYSCOMMAND in order to skip past it, not only
compare the Namval_t pointer 'np' to SYSCOMMAND, but also handle
the case where that pointer is NULL, as when the command name
results from an expansion. In that case, search the function tree
shp->fun_tree for the name and see if that yields the SYSCOMMAND
pointer. fun_tree is initialised with a dtview to bltin_tree, so
searching fun_tree instead allows for overriding 'command' with a
shell function (which the POSIX standard requires us to allow).
src/cmd/ksh93/sh.1,
src/cmd/ksh93/data/builtins.c:
- Update documentation to match these changes.
- Various related edits and improvements.
src/cmd/ksh93/tests/builtins.sh:
- Check that 'command' works if resulting from an expansion.
- Check that 'command' can be overridden by a shell function.
The 'exit' and 'return' commands without an argument failed to pass
down the exit status of the last-run command when incorporated in a
block with redirection, &&/|| list, 'case' statement, or 'while',
'until' or 'for' loop.
src/cmd/ksh93/bltins/cflow.c:
- Use $?, which is sh.savexit a.k.a. shp->savexit, as the default
exit status value if there is no argument, instead of
shp->oldexit. This fixes the default exit status behaviour to
match POSIX and other shells.
src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/include/shell.h:
- Remove now-unused sh.oldexit (a.k.a. shp->oldexit) private struct
member. It appeared to fulfill the same function as sh.savexit,
but in a slightly broken way.
- Move the savexit/$? declaration from the _SH_PRIVATE part of the
struct definition to the public API part. Since $? uses this,
it's clearly a publicly exposed value already, and this is
generally the one to use. (If anything, it's exitval that should
have been private.) This declares savexit right next to exitval,
rewriting the comments to clarify the difference between them.
src/cmd/ksh93/sh/fault.c,
src/cmd/ksh93/sh/subshell.c,
src/cmd/ksh93/sh/xec.c:
- Remove assignments to shp->oldexit.
src/cmd/ksh93/tests/basic.sh:
- Add thorough regression tests for the default exit status
behaviour of 'return' and 'exit' in various lexical contexts.
- Verify that 'for' and 'case' without any command, as well as a
lone redirection, still correctly reset the exit status to 0.
Fixes: #117
src/cmd/ksh93/include/jobs.h:
- The commit that removed legacy code mistakenly removed the
definition of vmbusy() required for ksh to compile with
-D_std_malloc. Ksh assumes vmbusy is always a macro, even
when _std_malloc is defined. This commit reintroduces the
_std_malloc definition of vmbusy to fix undefined
reference errors.
A coprocess cleanup test could fail on rare occasions because I had
lowered the 'sleep 1' between two test coprocesses to 'sleep .1'.
This increases the sleep to prevent future spurious fails.
Fixes: https://github.com/ksh93/ksh/issues/129
Using a process of elimination I've identified ${.sh.level}
(SH_LEVELNOD) as the cause of the crash. This node apparently
cannot be copied or moved without destabilising the shell. It
contains the current depth of function calls and it cannot be
changed by assignment, so this is not actually a problem.
Meanwhile, this commit re-fixes it for the other three.
src/cmd/ksh93/sh/subshell.c:
- Simplify sh_assignok() by removing special-casing for L_ARGNOD,
SH_SUBSCRNOD and SH_NAMENOD. 'add' now has 3 modes (0, 1, 2).
- The test for a ${ subshare; } was actually wrong. sp->subshare is
a saved backup value. We must test shp->subshare. (re: a9de50bf)
src/cmd/ksh93/bltins/typeset.c:
- setall(): Update the mode 3 sh_assignok() call.
src/cmd/ksh93/tests/variables.sh:
- Regress-test subshell leaks for all special variables except
${.sh.level}.
This reverts commit b3d37b00b0.
While ksh's own regression test suite passed just fine, when
running the modernish[*] regression tests uite, ksh either froze
hard (needing SIGKILL) or threw a spurious syntax error.
Cause unknown, but I'm certainly reverting until I find out.
This reintroduces a subshell leak for four special variables.
[*] https://github.com/modernish/modernish
${var:-wor)d} or ${var+w(ord}. The parentheses now correctly lose
their normal grammatical meaning within the braces. Fix by Eric
Scrivner (@etscrivner) from July 2018 backported from ksh2020.
This fix complies with POSIX:
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_02
src/cmd/ksh93/sh/lex.c: sh_lex():
- Set the ST_QUOTE state when analysing a modifier with parameter
expansions using operators ':', '-', '+', '='. This state causes
subsequent characters (including parentheses) to be considered
quoted, suppressing their normal grammatical meaning.
src/cmd/ksh93/sh/macro.c: varsub():
- Same for skipping the expansion.
Fixes: https://github.com/ksh93/ksh/issues/126
Prior discussion: https://github.com/att/ast/issues/475
The following special variables leaked out of a subshell:
$_, ${.sh.name}, ${.sh.level}, ${.sh.subscript}.
This was due to a faulty optimisation in sh_assignok().
bd3e2a80 fixed that in part, this fixes the rest.
src/cmd/ksh93/sh/subshell.c:
- Simplify sh_assignok() by removing special-casing for these four
special variables. The 'add' param reverts to a simple boolean.
- The test for a ${ subshare; } was actually wrong. sp->subshare is
a saved backup value. We must test shp->subshare. (re: a9de50bf)
src/cmd/ksh93/bltins/typeset.c:
- setall(), unall(): Update sh_assignok() calls.
src/cmd/ksh93/tests/variables.sh:
- Regress-test subshell leaks for all special variables.
Closes: #122
When exporting variables, ksh exports their attributes (such as
'integer' or 'readonly') in a magic environment variable called
"A__z" (string defined in e_envmarker[] in data/msg.c). Child
shells recognise that variable and restore the attributes.
This little-known feature is risky; the environment cannot
necessarily be trusted and that A__z variable is easy to manipulate
before or between ksh invocations, so you can cause a script's
variables to be of the wrong type, or readonly. Backwards
compatibility requires keeping it, at least for now. But it should
be disabled in the posix mode, as it violates POSIX.
To do this, we have to solve a catch-22 in init.c. We must parse
options to know whether to turn on posix mode; it may be specified
as '-o posix' on the command line. The option parsing loop depends
on an initialised environment[*], while environment initialisation
(i.e., importing attributes) should depend on the posix option.
The catch-22 can be solved because initialising just the values
before option parsing is enough to avoid regressions. Importing the
attributes can be delayed until after option parsing. That involves
basically splitting env_init() into two parts while keeping a local
static state variable between them.
src/cmd/ksh93/sh/init.c:
- env_init():
* Split the function in two stages based on a new
'import_attributes' parameter. Import values in the first
stage; import attributes from A__z in the second (if ever).
Make the 'next' variable static as it keeps a state needed for
the attributes import stage.
* Single point of truth, greppability: don't hardcode "A__z" in
separate character comparisons, but use e_envmarker[].
* Fix an indentation error.
- sh_init(): When initialising the environment (env_init), don't
import the attributes from A__z yet; parse options first, then
import attributes only if posix option is not set.
src/cmd/ksh93/sh/name.c:
- sh_envgen(): Don't export variable attributes to A__z if the
posix option is set.
src/cmd/ksh93/tests/attributes.sh:
- Check that variable attributes aren't imported or exported
if the POSIX option is set.
src/cmd/ksh93/sh.1:
- Update.
This was the last item on the TODO list for -o posix for now.
Closes: #20
[*] If environment initialisation is delayed until after option
parsing, bin/shtests shows various regressions, including:
restricted mode breaks; the locale is not initialised properly
so that multibyte variable names break; $SHLVL breaks.
This commit removes the following standards check on init:
strcmp(astconf("CONFORMANCE",0,0),"standard")==0
This also checks for the POSIXLY_CORRECT variable; the libast
configuration system uses it to set "CONFORMANCE" to "standard",
*but*, only if that parameter wasn't already initialised from the
_AST_FEATURES environment variable (see 'getconf --man').
Problem is, there is a harmful interaction between POSIXLY_CORRECT
and _AST_FEATURES. If the latter exists, it overrides the former.
Not only that, merely querying CONFORMANCE makes astconf create and
export the _AST_FEATURES variable, propagating the current setting
to child ksh processes, which will then ignore POSIXLY_CORRECT.
We could get around this by simply using getenv("POSIXLY_CORRECT").
But then the results may be inconsistent with the AST config state.
The whole thing may not be the best idea anyway. Honouring
POSIXLY_CORRECT at startup introduces a backwards compatibility
issue. Existing scripts or setups may export POSIXLY_CORRECT=y to
put external GNU utilities in standards mode, while still expecting
traditional ksh behaviour from newly initialised shells.
So it's probably better to just get rid of the check. This is not
bash, after all. If ksh is invoked as sh (the POSIX standard
command name), or with '-o posix' on the command line, you get the
standards mode; that ought to be good enough.
src/cmd/ksh93/sh/init.c: sh_init():
- Remove astconf call as per above.
In the SHOPT_BASH code, the -o posix option was given a '\374'
(0xFC, 252) single-letter option character. Reasons unclear. The
'set' builtin doesn't accept it. It can be omitted and the option
still works. And it caused the "$-" expansion (listing active
short-form options) to include that invalid high-bit character if
the -o posix option is active, which is clearly wrong.
src/cmd/ksh93/sh/args.c: optksh[], flagval[]:
- Remove '\374' one-letter option equivalent for SH_POSIX.
src/cmd/ksh93/tests/options.sh:
- Add test verifying that '-o posix' does not affect "$-".
The inclusion of the special parameter expansions ${!} and ${$}
(including the braces) in a here-document caused a syntax error.
Bug reported by @Saikiran-m on Github.
src/cmd/ksh93/data/lexstates.c: sh_lexstate7[]:
- Change the state for ! (33) and $ (36) from S_ERR to 0. State
table 7 is for skipping over ${...}, so this avoids the S_ERR
state being invoked in sh_lex() (lex.c) for these characters
while skipping ${...} in a here-doc.
src/cmd/ksh93/tests/heredoc.sh:
- Test evaluating the braces expansion form for all special
parameters (@ * # ! $ - ? 0) in a here-document.
Fixes: https://github.com/ksh93/ksh/issues/127
Since ksh 93u+m comes bundled with libast 20111111, there's no need
to support older versions, so this is another cleanup opportunity.
src/cmd/ksh93/include/defs.h:
- Throw an #error if AST_VERSION is undefined or < 20111111.
(Note that _AST_VERSION is the same as AST_VERSION, but the
latter is newer and preferred; see src/lib/libast/features/api)
All other changed files:
- Remove legacy code for versions older than the currently used
versions, which are:
_AST_VERSION 20111111
ERROR_VERSION 20100309
GLOB_VERSION 20060717
OPT_VERSION 20070319
SFIO_VERSION 20090915
VMALLOC_VERSION 20110808
SHOPT_ENV is an undocumented compile-time option implementing an
experimental method for handling environment variables, which is
implemented in env.h and env.c. There is no mention in the docs or
Makefile, and no mention in the mailing list archives. It adds no
new functionality, but at first glance it's a clean-looking
interface.
However, unfortunately, it's broken. Compiling with -DSHOPT_ENV
added to CCFLAGS causes bin/shtests to show these regressions:
functions.sh[341]: export not restored name=value function call -- expected 'base', got ''
functions.sh[1274]: Environment variable is not passed to a function
substring.sh[236]: export not restored name=value function call
variables.sh[782]: SHLVL should be 3 not 2
In addition, 'export' stops working on unset variables.
In the 93v- beta this code is still present, unchanged, though 93v-
made lots of incompatible changes. By the time ksh2020 noticed it,
it was no longer compiling, so it probably wasn't compiling in the
93v- beta either. Discussion: https://github.com/att/ast/issues/504
So the experiment was already abandoned by D. Korn and his team.
Meanwhile it was leaving sh/name.c with two versions of several
enviornment-related functions, and it's not clear which one is
actually compiled without doing detective work tracing header files
(most of the code was made conditional on _ENV_H, which is defined
in env.h, which is included by defs.h if SHOPT_ENV is defined).
This actively hinders understanding of the codebase. And any
changes to these functions would need to be implemented twice.
src/cmd/ksh93/include/env.h,
src/cmd/ksh93/sh/env.c:
- Removed.
src/cmd/ksh93/DESIGN,
src/cmd/ksh93/Makefile,
src/cmd/ksh93/Mamfile:
- Update accordingly.
All other changed files:
- Remove deactivated code behind SHOPT_ENV and _ENV_H.
An oops in tests/io.sh (re: c607c48c) wrote temporary files outside
$tmp and into src/cmd/ksh93/tests. Let's fix this properly so it
doesn't happen again.
src/cmd/ksh93/tests/shtests:
- Start each test set in its own temporary directory by default.
src/cmd/ksh93/tests/*.sh:
- Refuse to run if $tmp != $PWD.
- Related cleanups.
On ksh93, 'test -t' is equivalent to 'test -t 1' (and of course
"[ -t ]" is equivalent to "[ -t 1 ]").
This is purely for compatibility with ancient Bourne shell
breakage. No other shell supports this. ksh93 should probably keep
it for backwards compatibility, but it should definitely be
disabled in POSIX mode as it is a violation of the standard; 'test
-t' is an instance of 'test "$string"', which tests if the string
is empty, so it should test if the string '-t' is empty (quod non).
This also replaces the fix for 'test -t 1' in a command
substitution with a better one that avoids forking (re: cafe33f0).
src/cmd/ksh93/sh/parse.c:
- qscan(): If the posix option is active, disable the parser-based
hack that converts a simple "[ -t ]" to "[ -t 1 ]".
src/cmd/ksh93/bltins/test.c:
- e3(): If the posix option is active, disable the part of the
compatibility hack that was used for compound expressions
that end in '-t', e.g. "[ -t 2 -o -t ]".
- test_unop(): Remove the forking fix for "[ -t 1 ]".
src/cmd/ksh93/edit/edit.c:
- tty_check(): This function is used by "[ -t 1 ]" and in other
contexts as well, so a fix here is more comprehensive. Forking
here would cause a segfault, but we don't actually need to. This
adds a fix that simply returns false if we're in a virtual
subshell that is also a command substitution. Since command
substitutions always fork upon redirecting standard output within
them (making them no longer virtual), it is safe to do this.
src/cmd/ksh93/tests/bracket.sh
- Add comprehensive regression tests for test/[/[[ -t variants in
command substitutions, in simple and compound expressions, with
and without redirecting stdout to /dev/tty within the comsub.
- Add tests verifying that -o posix disables the old hack.
- Tweak other tests, including one that globally disabled xtrace.
ksh was enabling POSIX mode on init if it was invoked as any name
that merely started with 'sh' (after parsing initial 'r'). This
included shcomp, which was bad news.
src/cmd/ksh93/sh/init.c: sh_type():
- Check that the 'sh' is at the end of the string by checking
for a final zero byte.
- On Windows (_WINIX, see src/lib/libast/features/common), allow
for a file name extension (sh.exe) by checking for a dot as well.
eeee77ed implemented a POSIX compliance fix that caused a potential
incompatibility with existing ksh scripts; it made the (rarely
used) read/write redirection operator, <>, default to file
descriptor 0 (standard input) as POSIX specified, instead of 1
(standard output) which is traditional ksh93 behaviour. So ksh
scripts needed to change all <> to 1<> to override the new default.
This commit reverts that change, except in the new posix mode.
src/cmd/ksh93/sh/lex.c:
- Make FD for <> default to 0 in POSIX mode, 1 otherwise.
src/cmd/ksh93/tests/io.sh:
- Revert <> regression test changes from 60516872; we no longer
need 1<> instead of <> in ksh code.
If there are file descriptors > 2 opened with 'exec' or 'redirect',
ksh93 has always closed them when invoking another pogram. This is
contrary to POSIX which states:
Utilities other than the special built-ins […] shall be invoked
in a separate environment that consists of the following. The
initial value of these objects shall be the same as that for
the parent shell, except as noted below.
* Open files inherited on invocation of the shell, open files
controlled by the exec special built-in plus any
modifications, and additions specified by any redirections to
the utility
* […]
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_12
src/cmd/ksh93/sh/io.c: sh_redirect():
- When flag==2, do not close FDs > 2 if POSIX mode is active.
src/cmd/ksh93/tests/io.sh:
- Regress-test inheriting FD 7 with and without POSIX mode.
src/cmd/ksh93/sh.1:
- Update.
Though the "let" builtin is not itself a POSIX standard command, it
processes standard shell arithmetic, so it should recognise octals
by leading zeros as POSIX requires if the 'posix' option is on.
This overrides the setting of the 'letoctal' option.
Note that none of this applies to the ((...)) arithmetic command,
which has always recognised leading-octal zeros and does not listen
to 'letoctal'. So setting the posix mode makes this consistent.
src/cmd/ksh93/sh/arith.c:
- When running the 'let' builtin, test that both SH_LETOCTAL and
SH_POSIX are off before stripping leading zeros to disable octal
number recognition.
- Cosmetic: fix spurious newline.
src/cmd/ksh93/sh.1:
- Document the change.
src/cmd/ksh93/tests/shtests:
- Make sure to disable posix mode by default for regression tests.
On 16 June there was a call for volunteers to fix the bash
compatibility mode; it has never successfully compiled in 93u+.
Since no one showed up, it is now removed due to lack of interest.
A couple of things are kept, which are now globally enabled:
1. The &>file redirection shorthand (for >file 2>&1). As a matter
of fact, ksh93 already supported this natively, but only while
running rc/profile/login scripts, and it issued a warning. This
makse it globally available and removes the warning, bringing
ksh93 in line with mksh, bash and zsh.
2. The '-o posix' standard compliance option. It is now enabled on
startup if ksh is invoked as 'sh' or if the POSIXLY_CORRECT
variable exists in the environment. To begin with, it disables
the aforementioned &> redirection shorthand. Further compliance
tweaks will be added in subsequent commits. The differences will
be fairly minimal as ksh93 is mostly compliant already.
In all changed files, code was removed that was compiled (more
precisely, failed to compile/link) if the SHOPT_BASH preprocessor
identifier was defined. Below are other changes worth mentioning:
src/cmd/ksh93/sh/bash.c,
src/cmd/ksh93/data/bash_pre_rc.sh:
- Removed.
src/cmd/ksh93/data/lexstates.c,
src/cmd/ksh93/include/shlex.h,
src/cmd/ksh93/sh/lex.c:
- Globally enable &> redirection operator if SH_POSIX not active.
- Remove warning that was issued when &> was used in rc scripts.
src/cmd/ksh93/data/options.c,
src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/args.c:
- Keep SH_POSIX option (-o posix).
- Replace SH_TYPE_BASH shell type by SH_TYPE_POSIX.
src/cmd/ksh93/sh/init.c:
- sh_type(): Return SH_TYPE_POSIX shell type if ksh was invoked
as sh (or rsh, restricted sh).
- sh_init(): Enable posix option if the SH_TYPE_POSIX shell type
was detected, or if the CONFORMANCE ast config variable was set
to "standard" (which libast sets on init if POSIXLY_CORRECT
exists in the environment).
src/cmd/ksh93/tests/options.sh,
src/cmd/ksh93/tests/io.sh:
- Replace regression tests for &> and move to io.sh. Since &> is
now for general use, no longer test in an rc script, and don't
check that a warning is issued.
Closes: #9
Progresses: #20
Following a community objection to its removal, the inline 'test'
manual page along with its strange method of invocation is
restored. I've taken the opportunity to correct several mistakes,
add some missing info, do some copy-editing, and document the way
to get these docs in the main (k)sh.1 manual.
Discussion:
https://github.com/ksh93/ksh/commit/fa6a180f#commitcomment-41897553
Did you know that you could get a manual page for the 'test'/'['
builtin command using one of these strange command lines?
test --man --
[ --man -- ]
Neither did I. It's not documented or mentioned anywhere (and this
syntax violates POSIX). So nobody knows about it, which makes that
documentation useless. (The regular --man option doesn't work
because that would break 'test'.) I only found out how to invoke it
when I understood what the uncommented C code handling this does.
The test/[ command's self-documentation is unmaintained since 2003
and somewhat incomplete. It's also mostly redundant with the
documentation on Conditional Expressions in the main (k)sh.1 manual
page. But unlike the latter, this is resident in RAM, wasting
working memory in every shell process.
src/cmd/ksh93/sh.1:
- Add documentation for 'test'/'[' commands (yes, they were not
mentioned in the main manual page until now), describing them
in terms of differences from '[[' and recommending the latter.
src/cmd/ksh93/include/test.h,
src/cmd/ksh93/bltins/test.c,
src/cmd/ksh93/data/testops.c:
- Remove RAM-resident --man doc for test/[ command.
- Remove the bizarre option parsing that allowed invoking it.
Instead of a global 'sh_in_test_builtin' integer flag, it is nicer
to use the mechanism for shell state bits, which was designed for
this sort of thing.
src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/defs.c:
- Remove global sh_in_test_builtin integer.
- Define new SH_INTESTCMD state bit.
src/cmd/ksh93/bltins/test.c: _ERROR_exit_b_test(), b_test():
- Use the new state bit.
This removes various blocks of uncommented experimental code that
was disabled using '#if 0' or '#if 1 ... #else' directives. It's
hard or impossible to figure out what the thoughts behind them
might have been, and we can really do without those distractions.
As of 8477d2ce, the mbwide() macro (which tests if we're in a
multibyte locale, i.e. UTF-8) is redefined as a constant 0 if we're
compiling without SHOPT_MULTIBYTE. See src/cmd/ksh93/include/defs.h
The other multibyte macros use mbwide() as well, so they all revert
to the single-byte fallbacks in that case, and the multibyte code
in them is never compiled. See src/lib/libast/include/ast.h
Consequently we can now do a bit of cleanup and get rid of many of
the '#if SHOPT_MULTIBYTE' directives, as the compiler optimiser
will happily remove the multibyte-specific code. This increases the
legibility of the ksh code.
I'm taking the opportunity to fix a few typos and whitespace
formatting glitches as well.
The first of the two multibyte fixes from 8b5f11dc (which was for
using the first character of IFS as an output field separator when
expanding "$*" and similar) had a minor backwards compatibility
problem: if $IFS started with a byte sequence that is not a valid
UTF-8 character, then it treated IFS as empty in UTF-8 locales, so
the fields would be joined without any separator. The expected
behaviour would be for it to fall back to using the first byte of
IFS as it used to (and as bash and zsh do).
The new code handling this was also a bit kludgy and inefficient,
repeating the mbsize() calculation for every byte of the separator
character and for every field joined by the expansion.
src/cmd/ksh93/sh/macro.c: varsub():
- Rewrite code for joining fields for $* in a quoted or scalar
context and $@ in a scalar context, eliminating a confusing 'd'
variable and concentrating the routine in one block.
- When expanding $* with a multibyte separator (first character
of $IFS), only calculate the size in bytes once per expansion.
- If $IFS starts with a byte sequence that represents an invalid
multibyte character, fall back to using the first byte.
src/cmd/ksh93/tests/variables.sh:
- Tweak some regression tests, including one that overwrote $LANG.
- Add test for invalid multibyte character behaviour as per above.
src/cmd/ksh93/sh/path.c: canexecute():
- Close file descriptors inside of the err label. This fixes
a file descriptor leak that occurs when open succeeds but
fstat fails with EIO. The previous code only returned -1
after 'goto err', leaving the opened file descriptor
inaccessible. This bugfix was backported from ksh2020:
https://github.com/att/ast/commit/55cad1d
The package script searches for a good shell to run the build
scripts, preferring a ksh. But it also finds any recently compiled
development version of ksh in arch/*/bin that may be broken, or
have debug code, etc. -- and uses that in preference to anything
else. This is quite capable of breaking the build process.
The way to get around it is to do something like
bin/package make SHELL=/bin/ksh
which is annoying to have to keep doing.
bin/package,
src/cmd/INIT/package.sh:
- When finding a good shell, use the saved user path ($path), not
the current $PATH which includes arch/$HOSTTYPE/bin. Prefix this
temporary path with `getconf PATH`, the system's default path,
so that known-good system shells are found first.
All changed files:
- Put the shell in POSIX mode if it has an '-o posix' option.
- Remove nonsense disabling 'set -x' on bash. It's not broken.
bin/package, src/cmd/INIT/package.sh:
- Add check blocking native zsh mode (e.g., "$path" conflicts).
Using a 'sh -> zsh' symlink works, so recommend that.
- Remove old ksh93 version check for a supposed conflict with
libcmd. It was broken; it would revert to /bin/sh, but on illumos
distributions, /bin/sh is a ksh93 of a version that is supposedly
affected. It builds fine anyway.
- Rewrite checksh() to incorporate the shell compatibility checks
that were previously in two different places in 'package'.
bin/ignore, src/cmd/INIT/ignore.sh,
bin/silent, src/cmd/INIT/silent.sh:
- Change bad check for a full POSIX 'export' command (no, $RANDOM
has nothing to do with that) with a proper feature test.
The shell detection test assumed that any shell that has $RANDOM
and isn't bash must be ksh and have the 'print' built-in command.
This broke iffe on NetBSD 9.0 sh, which has $RANDOM but not 'print'.
src/cmd/INIT/iffe.sh:
- Rewrite shell detection test to test for features actually used
by iffe. "$shell" can now have the values 'bsh' for an ancient
pre-POSIX Bourne shell, 'bash' for bash, 'ksh' for a shell with
'let', 'typeset -u' and 'print' (ksh88, ksh93, mksh, pdksh, zsh),
and 'posix' for another POSIX or POSIX-ish shell with modern
command substitutions and parameter substitutions. (The 'posix'
value currently is not actively checked for anywhere, but avoids
unnecessary use of bsh fallbacks.)
If ksh was compiled with -DSHOPT_REGRESS=1, it would immediately
segfault on init. After fixing that, another segfault remained that
occurred when using the --regress= command line option with an
invalid option-argument.
The __regress__ builtin allows tracing a few things (see
'__regress__ --man' after compiling with -DSHOPT_REGRESS=1, or
usage[] in src/cmd/ksh93/bltins/regress.c). It seems of limited
use, but at least it can be used/tested now.
src/cmd/ksh93/sh/init.c: sh_init():
- Move the call to sh_regress_init() up. The crash on init was
caused by geteuid() being intercepted by regress.c before the
shp->regress (== sh.regress) pointer was initialised.
- The builtin can also be called using a --regress= option-argument
on the ksh command line. Before calling b___regress__() to parse
that, temporarily change error_info.exit so any usage error calls
exit(3) instead of sh_exit(), as the latter assumes a fully
defined shell state and this call is done before the shell is
fully initialised.
The INIT scripts are supposed to be Bourne shell scripts, not
POSIX, so we can't use $( ... ) command substitutions.
bin/package,
src/cmd/INIT/package.sh:
- Change a POSIX command substitution to old-style backticks.
This fixes a hanging bug that could occur on macOS when using the
'read' command to read from a FIFO and encountering end-of-file
without a final newline character. It also makes the 'read' command
perform 15-25% faster on macOS and Linux.
The previous version (ff385e5a) failed on SunOS/Solaris/Illumos
because those systems apparently don't (fully) support the POSIX
standard recv(2) syscall with MSG_PEEK[*], which is the feature
that iffe detects under the 'socket_peek' identifier. On Illumos,
using that methods causes a compilation failure (unknown identifier
MSG_PEEK); on Solaris 11.4, that method causes multiple regressions
in tests/io.sh, suggesting the method compiles but doesn't work at
all. Instead, SunOS/Solaris/Illumos requires the method using
ioctl(2)+I_PEEK and select(2). No other system that ksh currently
builds on requires this method, so it is now only used on
SunOS/Solaris/Illumos.
So far, this version of sfpkrd() has been tested to work correctly
on Linux, macOS, FreeBSD, NetBSD, OpenBSD, HP-UX, Solaris, and
OmniOS (an Illumos distribution).
It still fails to peek on Cygwin, but in the exact same way it
failed before, so that's no loss.
To test, run the 'io' test set: bin/shtests -p io
src/lib/libast/sfio/sfpkrd.c: sfpkrd():
- Remove long-obsolete Mac OS X and Solaris bug workarounds.
- Remove methods that are no longer needed.
On systems with a POSIX compliant recv(2), the only thing that
is required to avoid regressions is the code that was conditional
upon the socket_peek feature test, which tests for the correct
functioning of the recv(2) syscall. This has now been made
mandatory for non-SunOS/Solaris/Illumos systems (using an #error
directive if it is not detected), with the other methods removed.
The result performs 15-25% faster on macOS and Linux while
passing all the regression tests.
On macOS, avoiding the select(2) method fixes the hanging bug.
On SunOS/Solaris/Illumos (the '__sun' identifier), the method
using ioctl(2)+I_PEEK and select(2) (iffe feature IDs:
stream_peek and lib_select) is preserved.
Resolves: https://github.com/ksh93/ksh/issues/118 (again)
[*] https://pubs.opengroup.org/onlinepubs/9699919799/functions/recv.html
This fixes a hanging bug that could occur on macOS when using the
'read' command to read from a FIFO and encountering end-of-file
without a final newline character. It also makes the 'read' command
perform 15-25% faster on macOS and Linux and maybe other systems.
src/lib/libast/sfio/sfpkrd.c: sfpkrd():
- Get rid of the optional stuff that uses the poll(2) or select(2)
syscalls. The only thing that is required to avoid regressions is
the code that was conditional upon the socket_peek feature test,
which tests for the correct functioning of the recv(2) syscall.
This has now been made mandatory. The rest now uses what was
previously a fallback in plain C, resulting in a function that is
not only more readable, but actually faster than the syscalls.
Resolves: https://github.com/ksh93/ksh/issues/118
CDE <https://cdesktopenv.sf.net/> developer Chase writes, re dtksh:
| Everything is now completely working, and we are almost ready to
| add ksh93 as a submodule, but I have one last commit to get rid
| of some warnings we are facing. nval.h has some of these
| "compatiblity redefines" that are causing issues whenever we
| include it (warnings about redefining values) [...].
src/cmd/ksh93/include/nval.h:
- Replace ancient compatibility redefines by an unconditional
'#include <hash.h>'; ksh works fine with the "new" hash library.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
If a command's path was previously added to the hash table as a
'tracked alias', then the hash table entry was used, bypassing
the default utility path search activated by 'command -p'.
'command -p' activates a SH_DEFPATH shell state. The bug was caused
by a failure to check for this state before using the hash table.
This check needs to be added in four places.
src/cmd/ksh93/sh/path.c,
src/cmd/ksh93/sh/xec.c:
- path_search(), path_spawn(), sh_exec(), sh_ntfork(): Only consult
the hash table, which is shp->track_tree, if the SH_DEFPATH shell
state is not active.
src/cmd/ksh93/tests/path.sh:
- Add regress tests checking that 'command -p' and 'command -p -v'
still search in the default path if a hash table entry exists for
the command searched.
'command -x' (basically builtin xargs for 'command') worked for
long argument lists on *BSD and HP-UX, but not on macOS and Linux,
where it reliably entered into an infinite loop.
The problem was that it assumed that every byte of the environment
space can be used for arguments, without accounting for alignment
that some OSs do. MacOS seems to be the most wasteful one: it
aligns on 16-byte boundaries and requires some extra bytes per
argument as well.
src/cmd/ksh93/sh/path.c:
- path_xargs(): When calculating how much space to subtract per
argument, add 16 extra bytes to the length of each argument, then
align the result on 16-byte boundaries. The extra 16 bytes is
more than even macOS needs, but hopefully it is future-proof.
- path_spawn(): If path_xargs() does fail, do not enter a retry
loop (which always becomes an infinite loop if the argument list
exceeds OS limitations), but abort with an error message.
Four libast hash functions/macros (which ksh93 doesn't actually use)
were overridden with the following comment:
/*
* These following are for binary compatibility with the old hash library
* They will be removed someday
*/
This has been there for decades, and I just received word that they
cause problems for the dtksh (CDE) developers as dtksh does call
hashlook().
src/cmd/ksh93/sh/name.c:
- Remove 'hashscope', 'hashfree', 'hashname' and 'hashlook'
compatibility overrides.
A memory leak occurred upon leaving a virtual subshell if a
function was defined within it. If this was done more than 32766
(= 2^15-2 = the 'short' max value - 1) times, the shell crashed.
Discussion and reproducer: https://github.com/ksh93/ksh/issues/114
src/cmd/ksh93/sh/subshell.c: table_unset():
- A subshell-defined function was never freed because a broken
check for autoloaded functions (which must not be freed[*]). It
looked for an initial '/' in the canonical path of the script
file that defined the function, but that path is also stored for
regular functions. Now use a check that executes nv_search() in
fpathdict, the same method used in _nv_unset() in name.c for a
regular function unset.
src/cmd/ksh93/bltins/misc.c: b_dot_cmd():
- Fix an additional memory leak introduced in bd88cc7f, that caused
POSIX functions (which are run with b_dot_cmd() like dot scripts)
to leak extra. This fix avoids both the crash fixed there and the
memory leak by introducing a 'tofree' variable remembering the
filename to free. Thanks to Johnothan King for the patch.
src/lib/libast/include/stk.h,
src/lib/libast/misc/stk.c,
src/lib/libast/man/stk.3,
src/lib/libast/man/stak.3:
- Make the stack more resilient by extending the stack reference
counter 'stkref' from (signed) short to unsigned int. On modern
systems with 32-bit ints, this extends the maximum number of
elements on a stack from 2^15-1==32767 to 2^32-1==4294967295.
The ref counter can never be negative, so there is no reason for
signedness. sizeof(int) is defined as the size of a single CPU
word, so this should not affect performance at all.
On a 16-bit system (not that ksh still compiles there), this
doubles the max number of entries to 2^16-1=65535.
src/cmd/ksh93/tests/leaks.sh:
- Add leak regression tests for ksh functions, POSIX functions, dot
scripts run with '.', and dot scripts run with 'source'.
src/cmd/ksh93/tests/path.sh:
- Add an output builtin with a redirect to an autoloaded function
so that a crash[*] is triggered if the check for an autoloaded
function is ever removed from table_unset(), as was done in ksh
93v- (which crashed).
[*] Freeing autoloaded functions after leaving a virtual subshell
causes a crashing bug: https://github.com/att/ast/issues/803
Co-authored-by: Johnothan King <johnothanking@protonmail.com>
Fixes: https://github.com/ksh93/ksh/issues/114
