Following the resolution of Austin Group bug 1393[*] that is set to
be included in the next version of the POSIX standard, the
'command' prefix in POSIX mode (set -o posix) no longer disables
the declaration properties of declaration built-ins.
[*] https://austingroupbugs.net/view.php?id=1393
src/cmd/ksh93/sh/parse.c: lex():
- Skip the 'command' prefix even in POSIX mode so that any
declaration commands prefixed by it are treated as such in xec.c
(sh_exec()).
src/cmd/ksh93/sh/xec.c: sh_exec():
- The foregoing change reintroduced a variant of BUG_CMDSPEXIT: the
shell exits on something like 'command export readonlyvar=foo'.
This now fixes that bug for both POSIX and non-POSIX mode. When
calling nv_setlist() to process true shell assignments, and there
is a 'command' prefix, push a shell context and use sigsetjmp to
intercept any errors in assignments and stop the shell exiting.
src/cmd/ksh93/tests/builtins.sh:
- Borrow the BUG_CMDSPEXIT regression test from modernish and adapt
it for ksh. (I'm the author so yes, I can do this.) Original:
https://github.com/modernish/modernish/blob/ae8fe9c3/lib/modernish/tst/builtin.t#L80-L109
Tab completion in emacs and vi wrongly parses and executes command
substitutions. Example reproducers:
$ $(~)<Tab> # Result:
$ $(~)ksh[1]: /home/johno: cannot execute [Is a directory]
$ $(~ksh)<Tab> # Result:
$ $(~ksh)ksh: /home/johno/GitRepos/KornShell/ksh: cannot execute [Is a directory]
$ $(echo true)<Tab> # Result:
$ /usr/bin/true # or just 'true' -- it's unpredictable
In addition, backtick command substitutions had the following bug:
$ `echo hi`<Tab> # Result:
$ `echo hi`ksh: line 1: BUG_BRACQUOT_test.sh: not found
(where BUG_BRACQUOT_test.sh happens to be lexically the
first-listed file in my ksh development working directory).
There's also a crash associated with this due to an access beyond
buffer boundaries, which is only triggered on some systems (macOS
included).
src/cmd/ksh93/edit/completion.c:
- find_begin():
* When finding the beginning of a command substitution and the
last character is ')', do not increase the character pointer
cp. Increasing it caused the condition 'if(c && c==endchar)' in
the 'default:' block to be true, causing 'return(xp);' to be
executed, which returns a pointer the beginning of the command
substitution to ed_expand() on line 290, so that ed_expand()
eventually executes the command substitution with the
sh_argbuild() call on line 349. After deleting this 'else
cp++', that statement 'if(c && c==endchar) return(xp);' is not
executed and `find_begin()` returns the null pointer, which
avoids anything being executed. Thanks to @JohnoKing:
https://github.com/ksh93/ksh/issues/268#issuecomment-817249164
* Add code for properly skipping over backtick-style command
substitutions, based on the $( ) code.
- ed_expand(): Avoid out[-1] reading one byte to the left of
outbuff by first checking that out>outbuff. Thanks to @JohnoKing
for using ASan to find the location of the crash:
https://github.com/ksh93/ksh/issues/268#issuecomment-825574885
src/cmd/ksh93/tests/pty.sh:
- Test for the bugs detailed above.
Resolves: https://github.com/ksh93/ksh/issues/268
On slower systems it could fail with an arithmetic syntax error
because the output was verified before it had been written.
Also make another test xtrace-proof.
This applies when ksh is compiled with standard malloc.
Apparently, 1024 iterations is not enough on Gentoo Linux i386, at
least not when running the full test suite. The leak tests fail
intermittently and different tests fail each time, but always with
a leak of exactly 36864 bytes for each failing test. So those
failures are clearly spurious. Doubling the number of iterations
seems to make them go away.
src/cmd/ksh93/tests/{basic.sh,builtins.sh,shtests}:
- Redirect error output from the ulimit builtin to silence irrelevant
errors in the regression tests (these errors may occur when a
command such as 'ulimit -t 4' is run before the regression tests).
- Shellquote the error messages from the getconf regression tests.
src/cmd/ksh93/tests/{arrays,io,variables}.sh:
- Backport the ksh2020 regression tests for the following bugs:
https://github.com/att/ast/issues/23https://github.com/att/ast/issues/203https://github.com/att/ast/issues/472https://github.com/att/ast/issues/492
- Minor fix to POSIX mode regression tests in ksh93v-. In ksh93v-,
[[ -o ?posix ]] doesn't return an error (because it's implemented
in the bash mode). However, 'set -o posix' will fail in ksh93v-
if it's not in bash compatibility mode, which causes this test
script to exit prematurely.
src/cmd/ksh93/tests/{basic,pty}.sh:
- Add test for https://github.com/att/ast/issues/1461
- The ksh2020 fix for [ -t 1 ] in non-forking command substitutions
caused the following bug in interactive shells:
$ ( [ -t 1 ]; echo $? )
1 # Always fails
To avoid introducing this bug, this commit adds a regression
test for it.
src/cmd/ksh93/tests/functions.sh:
- Add test for https://github.com/att/ast/issues/1160
Put the test to the start of functions.sh (if it's at the end
of the script, it refuses to fail under ksh2020). Output from
this regression test when run against ksh2020:
functions.sh[46]: eval'ing function dumps function body to
stdout (got $' { eval "bar() { FAILURE; }"; }\n { FAILURE; }')
The regression is:
quoting.sh[189]: expansion of "{q:+'}" not correct when q unset
The failure was that, for unset q, "${q:+'}q${q:+'}" yielded empty
and not 'q'. This is because the single quotes within the double
quotes were erroneously parsed as meaningful.
The originally used ST_QUOTE state table (see data/lexstates.c),
where no quote character has any special meaning, was for avoiding
this problem.
The newly introduced ST_MOD1 state table is a copy of ST_QUOTE
except the ' has been given its special meaning back. We need this
to fix#290, but only for unquoted expansions.
So we need to go back to using ST_QUOTE if the string is quoted
(mp->quote) and we're not parsing a substitution that uses patterns
where quotes are significant (newops, ST_MOD2), i.e., only for
old-style ST_MOD1 operators.
src/cmd/ksh93/sh/macro.c: varsub():
- When the ${var<OP>string} expansion is quoted, and of an old
(S_MOD1) type, then use the ST_QUOTE state table to skip over it
instead of the new ST_MOD1 one.
This fixes the following:
1. Using $RANDOM in a virtual/non-forked subshell no longer
influences the reproducible $RANDOM sequence in the parent
environment.
2. When invoking a subshell $RANDOM is now re-seeded (as mksh and
bash do) so that invocations in repeated subshells (including
forked subshells) longer produce identical sequences by default.
3. Program flow corruption that occurred in scripts on executing
( ( simple_command & ) ).
src/cmd/ksh93/include/variables.h:
- Move 'struct rand' here as it will be needed in subshell.c. Add
rand_seed member to save the pseudorandom generator seed. Remove
the pointer to the shell state as it's redundant.
src/cmd/ksh93/sh/init.c:
- put_rand(): Store given seed in rand_seed while calling srand().
No longer pointlessly limit the number of possible seeds with the
RANDMASK bitmask (that mask is to limit the values to 0-32767,
it should not limit the number of possible sequences to 32768).
- nget_rand(): Instead of using rand(), use rand_r() to update the
random_seed value. This makes it possible to save/restore the
current seed of the pseudorandom generator.
- Add sh_reseed_rand() function that reseeds the pseudorandom
generator by calling srand() with a bitwise-xor combination of
the current PID, the current time with a granularity of 1/10000
seconds, and a sequence number that is increased on each
invocation.
- nv_init(): Set the initial seed using sh_reseed_rand() here
instead of in sh_main(), as this is where the other struct rand
members are initialised.
src/cmd/ksh93/sh/main.c: sh_main():
- Remove the srand() call that was replaced by the sh_reseed_rand()
call in init.c.
src/cmd/ksh93/sh/subshell.c: sh_subshell():
- Upon entering a virtual subshell, save the current $RANDOM seed
and state, then reseed $RANDOM for the subshell.
- Upon exiting a virtual subshell, restore $RANDOM seed and state
and reseed the generator using srand() with the restored seed.
src/cmd/ksh93/sh/xec.c: sh_exec():
- When optimizing out a subshell that is the last command, still
act like a subshell: reseed $RANDOM and increase ${.sh.subshell}.
- Fix a separate bug discovered while implementing this. Do not
optimize '( simple_command & )' when in a virtual subshell; doing
this causes program flow corruption.
- When optimizing '( simple_command & )', also reseed $RANDOM and
increment ${.sh.subshell}.
src/cmd/ksh93/tests/subshell.sh,
src/cmd/ksh93/tests/variables.sh:
- Add various tests for all of the above.
Co-authored-by: Johnothan King <johnothanking@protonmail.com>
Resolves: https://github.com/ksh93/ksh/issues/285
The following problems remained:
$ var=x; echo ${var:-'{}'}
x}
$ var=; echo ${var:+'{}'}
}
src/cmd/ksh93/sh/macro.c: varsub():
- Use the new ST_MOD1 state table to skip over ${var-'foo'}, etc.
instead of ST_QUOTE. In ST_MOD1 the ' is categorised as S_LIT
which causes the single quotes to be skipped over correctly.
See d087b031 for more info.
src/cmd/ksh93/tests/quoting2.sh:
- Add tests for this remaining bug.
- Make the new test xtrace-proof.
Resolves: https://github.com/ksh93/ksh/issues/290 (again)
src/cmd/ksh93/{bltins/typeset,sh/name,sh/nvtree,sh/nvtype}.c:
- Replace more instances of memcmp with strncmp to fix
heap-buffer-overflow errors when running the regression tests
with ASan enabled.
src/cmd/ksh93/edit/vi.c:
- Fix an invalid dereference of the 'p' pointer to fix a crash in
vi mode when entering a comment in the command history. This
bugfix was backported from ksh2020:
https://github.com/att/ast/issues/798
src/cmd/ksh93/tests/pty.sh:
- Add a regression test for the vi mode crash.
The code contains various checks to see if a subshell needs to
fork, like this one in the ulimit builtin:
if(shp->subshell && !shp->subshare)
sh_subfork();
All checks of this form are fatally broken, as each one of them
causes shared-state command substitutions to ignore parent virtual
subshells.
Currently the only feasible way to fix this is to fork a virtual
subshell before executing a shared-state command substitution in
it. In the long term I think shared-state command substitutions
should probably be redesigned to disassociate them completely from
the virtual subshell mechanism.
src/cmd/ksh93/sh/macro.c: comsubst():
- If we're in a non-subshare virtual subshell, fork it before
entering a type 2 (subshare) command substitution.
src/cmd/ksh93/sh/subshell.c:
- sh_assignok(): Remove subshare fix from 911d6b06 as it's
redundant now that the parent of a subshare is never a virtual
subshell. Go back to not doing anything if the current "subshell"
is a subshare.
- sh_subtracktree(), sh_subfuntree(): Similarly, remove the
now-redundant subshare fixes from 13c57e4b.
src/cmd/ksh93/sh/xec.c: sh_exec():
- Fix a separate bug: only fork a virtual subshell before running a
background job if that "subshell" is not a subshare.
src/cmd/ksh93/tests/subshell.sh:
- Add test for bug fixed in xec.c.
- Add tests for 'ulimit', 'builtin' and 'exec' run in subshare
within subshell -- all commands that use checks of the form
'if(sh.subshell && !sh.subshare) sh_subfork();'.
Resolves: https://github.com/ksh93/ksh/issues/289
src/cmd/ksh93/bltins/typeset.c:
- setall(): Only run sh_assignok() if troot points to the variable
tree. For instance, it's pointless to run it for an alias.
- Remove vestigial SHOPT_BSH code. The ast-open-history repo shows
that earlier SHOPT_BSH code was removed on 2008-06-02 and
2005-05-22. This may have been experimental code for increased
compatibility with the ancient Bourne shell. There was never any
documentation.
This avoids splitting on quoted whitespace when extracting words
from the command history using the emacs M-. or vi _ command.
Example: if the prior command is
$ ls Stairway\ To\ Heaven.mp3
then, M-. in Emacs editing mode (and _ in vi mode) now inserts
Stairway\ To\ Heaven.mp3 instead of Heaven.mp3. The behavior is
similar for 'Stairway To Heaven.mp3' and "Stairway To Heaven.mp3".
src/cmd/ksh93/edit/history.c: hist_word():
- Skip over single-quoted and double-quoted strings and
backslash-escaped characters.
src/cmd/ksh93/tests/pty.sh:
- Add regression test for this feature in vi mode. Since emacs and
vi both use the same code for this, that should be good enough.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
The referenced commit introduced the following bug:
> The closing quote does not appear to be registering during the
> parse of the following:
>
> echo ${var:+'{}'}
>
> Within a script, this will result in:
>
> syntax error at line 1: `'' unmatched
src/cmd/ksh93/data/lexstates.c,
src/cmd/ksh93/include/lexstates.h:
- Add new ST_MOD1 state table that is a copy of ST_QUOTE, but adds
a special meaning (ST_LIT) for the single quote (position 39).
src/cmd/ksh93/sh/lex.c: sh_lex():
- For parameter expansion operators with old-style quoting
(S_MOD1), use the new ST_MOD1 state table instead of ST_QUOTE.
This causes single quotes within them to be processed properly.
src/cmd/ksh93/tests/quoting2.sh:
- Add tests.
Thanks to @gkamat for the bug report.
Resolves: https://github.com/ksh93/ksh/issues/290
Previously, command substitutions executed as virtual subshells
were always forked if any command was run within them that
redireceted standard output, even if the redirection was local to
that command.
Commit 500757d7 removed the check for a shared-state command
substitution (subshare), so introduced a bug where even that would
fork, causing it to stop sharing its state.
We can further improve on that fix by only forking if the
redirection is permanent as with `exec` or `redirect`. There should
be no need to do that if the redirection is local to a command run
within the command substitution, as the file descriptor is restored
when that command finishes, which is still within the command
substitution.
src/cmd/ksh93/sh/io.c: sh_redirect():
- Only fork upon redirecting stdout if the virtual subshell is a
command substitution, and if the redirection is permanent
(flag==1 or flag==2).
Like tdump() and trestore() before commit 32d1abb1, sh_deparse() fails
to handle process substitutions correctly. This limitation of the shell
deparser is rather minor since it's unused. However, seeing as the
deparser was left in the code base intentionally it should at least
function properly.
src/cmd/ksh93/sh/deparse.c:
- Add a PROCSUBST flag for handling process substitutions in
sh_deparse().
- If we're handling a process substitution, add an ending ')'
without an extra newline.
- Avoid adding an extra ' &' to commands inside of a process
substitution. An extra ' &' is only added if the FAMP and FINT
flags are set, which indicates the command was spawned as a separate
job with '&'.
- Add process substitution handling to 'p_redirect' by calling p_tree()
when encountering a process substitution.
src/cmd/ksh93/bltins/typeset.c:
- Removing the nv_search() call altogether was actually not
neccessary, I was just searching the wrong tree: instead of
sh.fun_base, simply search the current sh.fun_tree which has a
view to all the layered parent subshell copes. It is not going to
find it in the current subshell tree but will find it in one of
the parent trees if it exists. The cost of an unnecessary dummy
is negligible, but so is the cost of this search, and doing it is
more correct.
src/cmd/ksh93/bltins/whence.c:
- The previous commit that fixed 'unset -f' in virtual subshells left
one bug. The type builtin (or 'whence -v') could still find the unset
function in virtual subshells:
$ foo() { echo foo; }
$ (unset -f foo; type foo)
foo is an undefined function
To fix this bug, avoid detecting functions in the whence builtin
unless they have the NV_FUNCTION flag.
src/cmd/ksh93/tests/subshell.sh:
- Add a regression test for using 'type' on a function unset inside of
a virtual subshell.
A bug introduced in the previous commit caused 'unset -f' in a
subshell of a subshell to fail to unset a function created in a
parent subshell. Reproducer:
$ ( f2() { echo WRONG; }; ( unset -f f2; f2 ) )
WRONG
src/cmd/ksh93/bltins/typeset.c: unall():
- Do not nv_search() in sh.fun_base before setting the dummy node
that marks the function as unset in this subshell. That search
only reaches the base tree and not any of its subtrees. Setting
the dummy unconditionally is not harmful; the cost is negligible.
src/cmd/ksh93/tests/subshell.sh:
- Add test for the bug.
This commit implements unsetting functions in virtual subshells,
removing the need for the forking workaround. This is done by
either invalidating the function found in the current subshell
function tree by unsetting its NV_FUNCTION attribute bits (which
will cause sh_exec() to skip it) or, if the function exists in a
parent shell, by creating an empty dummy subshell node in the
current function tree without that attribute.
As a beneficial side effect, it seems that bug 228 (unset -f fails
in forked subshells if a function is defined before forking) is now
also fixed.
src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/init.c:
- Add sh.fun_base for a saved pointer to the main shell's function
tree for checking when in a subshell, analogous to sh.var_base.
src/cmd/ksh93/bltins/typeset.c: unall():
- Remove the fork workaround.
- When unsetting a function found in the current function tree
(troot) and that tree is not sh.var_base (which checks if we're
in a virtual subshell in a way that handles shared-state command
substitutions correctly), then do not delete the function but
invalidate it by unsetting its NV_FUNCTION attribute bits.
- When unsetting a function not found in the current function tree,
search for it in sh.fun_base and if found, add an empty dummy
node to mask the parent shell environment's function. The dummy
node will not have NV_FUNCTION set, so sh_exec() will skip it.
src/cmd/ksh93/sh/subshell.c:
- sh_subfuntree(): For 'unset -f' to work correctly with
shared-state command substitutions (subshares), this function
needs a fix similar to the one applied to sh_assignok() for
variables in commit 911d6b06. Walk up on the subshells tree until
we find a non-subshare.
- sh_subtracktree(): Apply the same fix for the hash table.
- Remove table_unset() and incorporate an updated version of its
code in sh_subshell(). As of ec888867, this function was only
used to clean up the subshell function table as the alias table
no longer exists.
- sh_subshell():
* Simplify the loop to free the subshell hash table.
* Add table_unset() code, slightly refactored for readability.
Treat dummy nodes now created by unall() separately to avoid a
memory leak; they must be nv_delete()d without passing the
NV_FUNCTION bits. For non-dummy nodes, turn on the NV_FUNCTION
attribute in case they were invalidated by unall(); this is
needed for _nv_unset() to free the function definition.
src/cmd/ksh93/tests/subshell.sh:
- Update the test for multiple levels of subshell functions to test
a subshare as well. While we're add it, add a very similar test
for multiple levels of subshell variables that was missing.
- Add @JohnoKing's reproducer from #228.
src/cmd/ksh93/tests/leaks.sh:
- Add leak tests for unsetting functions in a virtual subshell.
Test both the simple unset case (unall() creates a dummy node)
and the define/unset case (unall() invalidates existing node).
Resolves: https://github.com/ksh93/ksh/issues/228
Noteworthy changes:
- The man pages have been updated to fix a ton of instances of
runaway underlining (this was done with `sed -i 's/\\f5/\\f3/g'`
commands). This commit dramatically increased in size because
of this change.
- The documentation for spawnveg(3) has been extended with
information about its usage of posix_spawn(3) and vfork(2).
- The documentation for tmfmt(3) has been updated with the changes
previously made to the man pages for the printf and date builtins
(though the latter builtin is disabled by default).
- The shell's tracked alias tree (hash table) is now documented in
the shell(3) man page.
- Removed the commented out regression test for an ERRNO variable
as the COMPATIBILITY file states it was removed in ksh93.
There is a TODO note in variables.sh that notes the value of LINENO
is wrong after a virtual subshell. The following script should
print '6', but the bug causes it to print '1' instead:
$ cat /tmp/lineno
#!/bin/ksh
(
unset LINENO
:
)
echo $LINENO
This bug started to occur after the bugfix applied in 7b994b6a.
However, that commit is not where the cause of bug was (when that
bugfix is applied to ksh versions 2008-07-25 through 2012-01-01,
$LINENO works fine). Rather, the cause of this bug was introduced
in 93u+ 2012-02-29. In that version, the mp->nvfun pointer was only
copied from np->nvfun if the variable can be freed from memory.
This is what caused 7b994b6a to break $LINENO in subshells, so to
fix this bug the mp->nvfun and np->nvfun must point to the same
object, even when the variable isn't freed from memory.
src/cmd/ksh93/sh/subshell.c: nv_restore():
- Always copy the np->nvfun pointer to mp->nvfun. To prevent
crashes, the value of np->nvfun->nofree is set to the value given
by the nofree variable, which is set before _nv_unset. See also
commit 7e7f1372, which fixed a crash that happened because
_nv_unset discards the NV_NOFREE flag.
src/cmd/ksh93/tests/variables.sh:
- Remove the workaround for LINENO after a virtual subshell.
- Add a regression test for the value of LINENO when unset in a
virtual subshell, then used after the subshell. Note that before
commit 997ad43b LINENO's value was corrupted after being unset in
a subshell, so the test checks for corruption of the LINENO
variable (in prior commits LINENO was set to '49' because of the
previous bug).
The changes in this commit allow ksh to be built and run with
ASan[*], although for now it only works under vmalloc. Example
command to build ksh with ASan:
$ bin/package make CCFLAGS='-O0 -g -fsanitize=address'
[*] https://en.wikipedia.org/wiki/AddressSanitizer
src/cmd/INIT/mamake.c:
- Fix a few memory leaks in mamake. This doesn't fix all of the
memory leaks ASan complains about (there is one remaining in the
view() function), but it's enough to get ksh to build under ASan.
src/lib/libast/features/map.c,
src/lib/libast/misc/glob.c:
- Rename the ast globbing functions to _ast_glob() and
_ast_globfree(). Without this change the globbing tests fail
under ASan. See: https://github.com/att/ast/commit/2c49eb6e
src/cmd/ksh93/sh/{init,io,nvtree,subshell}.c:
- Fix buffer overflows by using strncmp(3) instead of memcmp(3).
src/cmd/ksh93/sh/name.c:
- Fix another invalid usage of memcmp by using strncmp instead.
This change is also in one of Red Hat's patches:
https://git.centos.org/rpms/ksh/blob/c8s/f/SOURCES/ksh-20120801-nv_open-memcmp.patch
Resolves: https://github.com/ksh93/ksh/issues/230
The commands within a process substitution used as an argument to a
redirection (e.g. < <(...) or > >(...)) are simply not included in
parse trees dumped by shcomp. This can be verified with a command
like hexdump -C. As a result, these process substitutions do not
work when running a bytecode-compiled shell script.
The fix is surprisingly simple. A process substitution is encoded
as a complete parse tree. When used with a redirection, that parse
tree is used as the file name for the redirection. All we need to
do is treat the "file name" as a parse tree instead of a string if
flags indicate a process substitution.
A process substitution is detected by the struct ionod field
'iofile'. Checking the IOPROCSUB bit flag is not enough. We also
need to exclude the IOLSEEK flag as that form of redirection may
use the IOARITH flag which has the same bit value as IOPROCSUB (see
include/shnodes.h).
src/cmd/ksh93/sh/tdump.c: p_redirect():
- Call p_tree() instead of p_string() for a process substitution.
src/cmd/ksh93/sh/trestore.c: r_redirect():
- Call r_tree() instead of r_string() for a process substitution.
src/cmd/ksh93/include/version.h:
- Bump the shcomp binary header version as this change is not
backwards compatible; previous trestore.c versions don't know how
to read the newly compiled process substitutions and would crash.
src/cmd/ksh93/tests/io.sh:
- Add test.
src/cmd/ksh93/tests/builtins.sh,
src/cmd/ksh93/tests/options.sh:
- Revert shcomp workarounds. (re: 6701bb30)
Resolves: https://github.com/ksh93/ksh/issues/165
Johnothan King writes:
> There are two regressions related to how ksh handles syntax
> errors in the .kshrc file. If ~/.kshrc or the file pointed to by
> $ENV have a syntax error, ksh exits during startup. Additionally,
> the error message printed is incorrect:
>
> $ cat /tmp/synerror
> ((
> echo foo
>
> # ksh93u+m
> $ ENV=/tmp/synerror arch/*/bin/ksh -ic 'echo ${.sh.version}'
> /tmp/synerror: syntax error: `/t/tmp/synerror' unmatched
>
> # ksh93u+
> $ ENV=/tmp/synerror ksh93u -ic 'echo ${.sh.version}'
> /tmp/synerror: syntax error: `(' unmatched
> Version AJM 93u+ 2012-08-01
>
> The regression that causes the incorrect error message was
> introduced by commit cb67a01. The other bug that causes ksh to
> exit on startup was introduced by commit ceb77b1.
src/cmd/ksh93/sh/lex.c: fmttoken():
- Call stakfreeze(0) to terminate a possible unterminated previous
stack item before writing the token string onto the stack. This
fixes the bug with garbage in a syntax error message.
src/cmd/ksh93/sh/main.c: exfile():
- Revert Red Hat's ksh-20140801-diskfull.patch applied in ceb77b13.
This fixes the bug with interactive ksh exiting on syntax error
in a profile script. Testing by @JohnoKing showed the patch is no
longer necessary to fix a login crash on disk full, as commit
970069a6 (which applied Red Hat patches ksh-20120801-macro.patch
and ksh-20120801-fd2lost.patch) also fixes that crash.
src/cmd/ksh93/README:
- Fix typos. (re: fdc08b23)
Co-authored-by: Johnothan King <johnothanking@protonmail.com>
Resolves: https://github.com/ksh93/ksh/issues/281
src/cmd/ksh93/README:
- Update compile-time options docuemntation.
- Update build instructions.
- Remove obsolete stuff.
src/cmd/ksh93/SHOPT.sh:
- Remove unused SHOPT_SEVENBIT option. A search in ast-open-history
shows it was removed from include/edit.h on 2001-10-31. You can
still get its effect by changing STRIP from 0377 to 0177 there.
While automagically importing/exporting ksh variable attributes via
the environment is probably a misfeature in general (now disabled
for POSIX standard mode), doing so with the readonly attribute is
particularly problematic. Scripts can take into account the
possibility of importing unwanted attributes by unsetting or
typesetting variables before using them. But there is no way for a
script to get rid of an unwanted imported readonly variable. This
is a possible attack vector with no possible mitigation.
This commit blocks both the import and the export of the readonly
attribute through the environment. I consider it a security fix.
src/cmd/ksh93/sh/init.c: env_import_attributes():
- Clear NV_RDONLY from imported attributes before applying them.
src/cmd/ksh93/sh/name.c: sh_envgen():
- Remove NV_RDONLY from bitmask defining attributes to export.
This commit fixes three problems with getconf pathbound builtin:
1. The -l/--lowercase option did not change all variable names to
lower case.
2. The -q/--quote option now quotes all string values. Previously,
it only quoted string values that had a space or other
non-shellsafe character.
3. The -c/--call, -n/--name and -s/--standard options matched all
variable names provided by 'getconf -a', even if none were
actual matches.
Additionally, references to the confstr and sysconf functions have
been updated to reference section 3 of the man pages instead of
section 2.
src/lib/libast/port/astconf.c:
- Previously, only values that had spaces in them were quoted. Change
that behavior to quote all string values by using the FMT_ALWAYS
flag. Bug report: https://github.com/att/ast/issues/1173
- Not all variable names were printed in lowercase by 'getconf -l'.
Fix it by adding a few missing instances of fmtlower.
Bug report: https://github.com/att/ast/issues/1171
- Add the missing code to the '#if _pth_getconf_a' block to handle
-c/-n/-s while parsing the OS's native 'getconf -a' output. This
approach reuses code for name matching from other parts of
astconflist(). Resolves: https://github.com/ksh93/ksh/issues/279
src/lib/libcmd/getconf.c:
- Update the documentation to note the -q flag only quotes strings.
src/cmd/ksh93/tests/bulitins.sh:
- Add regression tests for the getconf bugs fixed in this commit.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
There were still problems left after the previous commit. On at
least one system (QNX i386), the following regression test crashed:
src/cmd/ksh93/test/subshell.c
900 got=$( { "$SHELL" -c '(cd /; (cd /)); print -r -- "PWD=$PWD"'; } 2>&1 )
A backtrace done on the core dunp pointed to the free() call here:
src/cmd/ksh93/bltins/cd_pwd.c
90 if(oldpwd && oldpwd!=shp->pwd && oldpwd!=e_dot)
91 free(oldpwd);
Analysis: The interaction between $PWD, sh.pwd aka shp->pwd, and
the path_pwd() function is a mess. path_pwd() usually returns a
freeable value, but not always. sh.pwd is sometimes a pointer to
the value of $PWD, but not always (e.g. when you unset PWD or
assign to it). Instead of debugging the exact cause of the crash, I
think it is better to make this work in a more consistent way.
As of this commit:
1. sh.pwd keeps its own copy of the PWD, independently of the PWD
variable. The old value must always be freed immediately before
assigning a new one. This is simple and consistent, reducing the
chance of bugs at negligible cost.
2. The PWD variable is no longer given the NV_NOFREE attribute
because its value no longer points to sh.pwd. It is now a
variable like any other.
src/cmd/ksh93/sh/path.c: path_pwd():
- Do not give PWDNOD the NV_NOFREE attribute.
- Give sh.pwd its own copy of the PWD by strdup'ing PWDNOD's value.
src/cmd/ksh93/bltins/cd_pwd.c: b_cd():
- Since sh.pwd is now consistently freed before giving it a new
value and at no other time, oldpwd must not be freed any longer
and can become a regular non-static variable.
- If the PWD needs reinitialising, call path_pwd() to do it.
src/cmd/ksh93/sh/subshell.c: sh_subshell():
- Systems with fchdir(2): Always restore the PWD upon exiting a
non-subshare subshell. The check to decide whether or not to
restore it was unsafe: it was not restored if the current PWD
pointer and value was identical to the saved one, but a directory
can be deleted and recreated under the same name.
- Systems without fchdir(2) (if any exist):
. Entry: Fork if the PWD is nonexistent or has no x permission.
. Restore: Only chdir back if the subshell PWD was changed.
That's probably the best we can do. It remains inherently unsafe.
We should probably just require fchdir(2) at some point.
This commit fixes what are hopefully the two final aspects of #153:
1. If the present working directory does not exist (was moved or
deleted) upon entering a virtual subshell, no PWD directory path
is saved. Since restoring the state after exiting a virtual
subshell is contingent on a previous PWD path existing, this
resulted in entire aspects of the virtual subshell, such as the
subshell function tree, not being cleaned up.
2. A separate problem is that 'cd ..' does not update PWD or OLDPWD
when run from a nonexistent directory.
A reproducer exposing both problems is:
$ mkdir test
$ cd test
$ ksh -c '(subfn() { BAD; }; cd ..; echo subPWD==$PWD);
typeset -f subfn; echo mainPWD==$PWD'
subPWD==/usr/local/src/ksh93/ksh/test
subfn() { BAD; };mainPWD==/usr/local/src/ksh93/ksh/test
Expected output:
subPWD==/usr/local/src/ksh93/ksh
mainPWD==/usr/local/src/ksh93/ksh/test
src/cmd/ksh93/bltins/cd_pwd.c:
- If path_pwd() fails to get the PWD (usually it no longer exists),
don't set $OLDPWD to '.' as that is pointless; use $PWD instead.
After cd'ing from a nonexistent directory, 'cd -' *should* fail
and should not be equivalent to 'cd .'.
- Remove a redundant check for (!oldpwd) where it is always set.
- Do not prematurely return without setting PWD or OLDPWD if
pathcanon() fails to canonicalise a nonexistent directory.
Instead, fall back to setting PWD to the result of getcwd(3).
src/cmd/ksh93/sh/subshell.c:
- Minor stylistic adjustment. Some NULL macros sneaked in. This
historic code base does not use them (yet); change to NIL(type*).
- sh_subshell(): Fix logic for determining whether to save/restore
subshell state.
1. When saving, 'if(!comsub || !shp->subshare)' is redundant;
'if(!shp->subshare)' should be enough. If we're not in a
subshare, state should be saved.
2. When restoring, 'if(sp->shpwd)' is just nonsense as there is
no guarantee that the PWD exists upon entering a subshell.
Simply use the same 'if(!shp->subshare)'. Add an extra check
for sp->pwd to avoid a possible segfault. Always restore the
PWD on subshell exit and not only if shp->pwd is set.
- sh_subshell(): Issue fatal errors in libast's "panic" format.
src/cmd/ksh93/tests/builtins.sh:
- Adjust a relevant test to run err_exit() outside of the subshell
so that any error is counted in the main shell.
- Add test for problem 2 described at the top.
src/cmd/ksh93/tests/subshell.sh:
- Add test for problems 1 and 2 based on reproducer above.
Resolves: https://github.com/ksh93/ksh/issues/153
Accessing t->tre.treio for every sh_exec() run is invalid because
't' is of type Shnode_t, which is a union that can contain many
different kinds of structs. As all members of a union occupy the
same address space, only one can be used at a time. Which member is
valid to access depends on the node type sh_exec() was called with.
The invalid access triggered a crash on 32-bit systems when
executing an arithmetic command like ((x=1)).
The t->tre.treio union member should be accessed for a simple
command (case TCOM in sh_exec()). The fix is also needed for
redirections attached to blocks (case TSETIO) in which case the
union member to use is t->fork.forkio.
src/cmd/ksh93/sh/xec.c:
- Add check_exec_optimization() function that checks for all the
conditions where the exec optimisation should not be done. For
redirections we need to loop through the whole list to check for
an IOREWRITE (<>;) one.
- sh_exec(): case TCOM (simple command): Only bother to call
check_exec_optimization() if there are either command arguments
or redirections (IOW: don't bother for bare variable
assignments), so move it to within the if(io||argn) block.
- sh_exec(): case TSETIO: This needs a similar fix. To avoid the
optimization breaking again if the last command is a subshell
with a <>; redirection attached, we need to not only set execflg
to 0 but also clear the SH_NOFORK state bit from the 'flags'
variable which is passed on to the recursive sh_exec() call.
src/cmd/ksh93/tests/io.sh:
- Update and expand tests. Add tests for redirections attached to
simple commands (TCOM) and various kinds of code block (TSETIO).
Co-authored-by: Johnothan King <johnothanking@protonmail.com>
Resolves: https://github.com/ksh93/ksh/issues/278
Immediately after tab-completing the name of a directory, it is
not possible to type digits after the slash; ksh eats them as it
parses them as a menu selection for a nonexistent menu.
Reproducer:
$ mkdir -p emacstest/123abc
$ cd emacste[tab]123abc
Actual results:
$ cd emacstest/abc
Expected results:
$ cd emacstest/123abc
Workarounds are to press a non-numeric key followed by backspace,
or hit [tab] again to get a list of options.
Originally reported by Arnon Weinberg, 2012-12-23 07:15:19 UTC, at:
https://bugzilla.redhat.com/889745
The fix had been partially backported from ksh 93v- by AT&T
(16e4824c), which made things worse, so it was reverted (e8b3274a).
This commit backports a slightly edited version of the complete
fix. Thanks to @JohnoKing for finding the correct code. Discussion:
https://github.com/ksh93/ksh/issues/198#issuecomment-820178514
src/cmd/ksh93/edit/emacs.c: escape():
- Backport the fix for this bug that was implemented in ksh 93v-
alpha 2013-10-10. Immediately after a slash, do not stay in "\"
mode (file name completion) and reset the tab count.
src/cmd/ksh93/tests/pty.sh:
- Test the fix.
Resolves: https://github.com/ksh93/ksh/issues/198
The <>; operator doesn't work correctly if it's used as the last
command of a -c script. Reproducer:
$ echo test > a; ksh -c 'echo x 1<>; a'; cat a
x
st
This bug is caused by ksh running the last command of -c scripts
with execve(2) instead of posix_spawn(3) or fork(2). The <>;
operator is noted by the man page as being incompatible with the
exec builtin (see also the ksh93u+ man page), so it's not
surprising this bug occurs when ksh runs a command using execve:
> <>;word cannot be used with the exec and redirect built-ins.
The ksh2020 fix simply removed the code required for ksh to use
this optimization at all. It's not a performance friendly fix and
only papers over the bug, so this commit provides a better fix.
This bug was first reported at:
https://github.com/att/ast/issues/9
In addition, this commit re-enables the execve(2) optimization for
the last command for scripts loaded from a file. It was enabled in
in older ksh versions, and was only disabled in interactive shells:
https://github.com/ksh93/ast-open-history/blob/2011-06-30/src/cmd/ksh93/sh/main.c#L593-L599
It was changed on 2011-12-24 to only be used for -c scripts:
https://github.com/ksh93/ast-open-history/blob/2011-12-24/src/cmd/ksh93/sh/main.c#L593-L599
We think there is no good reason why scripts loaded from a file
should be optimised less than scripts loaded from a -c argument.
They're both scripts; there's no essential difference between them.
So this commit reverts that change. If there is a bug left in the
optimization after this fix, this revert increases the chance of
exposing it so that it can be fixed.
src/cmd/ksh93/sh/xec.c:
- The IOREWRITE flag is set when handling the <>; operator, so to
fix this bug, avoid exec'ing the last command if it uses <>;. See
also commit 17ebfbf6, which fixed another issue related to the
execve optimization.
src/cmd/ksh93/tests/io.sh:
- Enable a regression test that was failing because of this bug.
- Add the reproducer from https://github.com/att/ast/issues/9 as a
regression test.
src/cmd/ksh93/sh/main.c:
- Only avoid the non-forking optimization in interactive shells.
src/cmd/ksh93/tests/signal.sh:
- Add an extra comment to avoid the non-forking optimization in the
regression test for rhbz#1469624.
- If the regression test for rhbz#1469624 fails, show the incorrect
exit status in the error message.
src/cmd/ksh93/tests/builtins.sh,
src/cmd/ksh93/tests/options.sh:
- This bugfix was causing the options regression test to segfault
when run under shcomp. The cause is the same as
<https://github.com/ksh93/ksh/issues/165>, so as a workaround,
avoid parsing process substitutions with shcomp until that is
fixed. This workaround should also avoid the other problem
detailed in <https://github.com/ksh93/ksh/issues/274>.
Resolves: https://github.com/ksh93/ksh/issues/274
This is the underlying cause for the issue worked around in
3654ee73.
The following explanation refers to the current illumos version of
ksh93 and shows output from illumos' modular debugger:
https://illumos.org/books/dev/debugging.html
Each environment variable (name/value pair) has a linked list of
disciplines attached to it, and at the end of that list there is
optionally a shell context pointer. For example, for the EDITOR
variable:
> ::bp libshell.so.1`put_ed
> ::run
$
$ EDITOR=vim
> ::stack ! head -1
libshell.so.1`put_ed+0x14(e06208, e01c58, 0, dced90)
> e06208::print Namval_t
{
nvname = 0xfffffbffeec40a0e "EDITOR"
nvfun = 0xdced90
nvalue = 0
}
> e06208::print Namval_t nvfun | ::print Namfun_t
{
disc = libshell.so.1`EDITOR_disc
next = libshell.so.1`sh+0x710
}
Here, the EDITOR Namval_t has a discipline stack containing
EDITOR_disc and &Shell_t.nvfun.
The problem arises when a new discipline is pushed onto the stack,
such as when using typeset -u to add an upper-case translation
discipline.
$ typeset -u EDITOR
> e06208::print Namval_t
{
nvname = 0xfffffbffeec40a0e "EDITOR"
nvfun = 0xdced90
nvalue = 0xe0fdb0 "vim"
}
> e06208::print Namval_t nvfun | ::print Namfun_t
{
disc = libshell.so.1`EDITOR_disc
next = 0xdc27a0
}
> e06208::print Namval_t nvfun | ::print Namfun_t next | ::print Namfun_t
{
disc = libshell.so.1`TRANS_disc
next = 0
}
TRANS_disc has been pushed onto the end of the discipline stack,
but the shell handle has been lost.
With this change, the attributes and variables tests pass (this is
on illumos where this change originates).
Path-bound builtins on ksh (such as /opt/ast/bin/cat) break some
basic assumptions about paths in the shell that should hold true,
e.g., that a path output by whence -p or command -v should actually
point to an executable command. This commit should fix the
following:
1. Path-bound built-ins (such as /opt/ast/bin/cat) can now be
executed by invoking the canonical path (independently of the
value of $PATH), so the following will now work as expected:
$ /opt/ast/bin/cat --version
version cat (AT&T Research) 2012-05-31
$ (PATH=/opt/ast/bin:$PATH; "$(whence -p cat)" --version)
version cat (AT&T Research) 2012-05-31
In the event an external command by that path exists, the
path-bound builtin will now override it when invoked using the
canonical path. To invoke a possible external command at that
path, you can still use a non-canonical path, e.g.:
/opt//ast/bin/cat or /opt/ast/./bin/cat
2. Path-bound built-ins will now also be found on a PATH set
locally using an assignment preceding the command, so something
like the following will now work as expected:
$ PATH=/opt/ast/bin cat --version
version cat (AT&T Research) 2012-05-31
The builtin is not found by sh_exec() because the search for
builtins happens long before invocation-local preceding
assignments are processsed. This only happens in sh_ntfork(),
before forking, or in sh_fork(), after forking. Both sh_ntfork()
and sh_fork() call path_spawn() to do the actual path search, so
a check there will cover both cases.
This does mean the builtin will be run in the forked child if
sh_fork() is used (which is the case on interactive shells with
job.jobcontrol set, or always after compiling with SHOPT_SPAWN
disabled). Searching for it before forking would mean
fundamentally redesigning that function to be basically like
sh_ntfork(), so this is hard to avoid.
src/cmd/ksh93/sh/path.c: path_spawn():
- Before doing anything else, check if the passed path appears in
the builtins tree as a pathbound builtin. If so, run it. Since a
builtin will only be found if a preceding PATH assignment
temporarily changed the PATH, and that assignment is currently in
effect, we can just sh_run() the builtin so a nested sh_exec()
invocation will find and run it.
- If 'spawn' is not set (i.e. we must return), set errno to 0 and
return -2. See the change to sh_ntfork() below.
src/cmd/ksh93/sh/xec.c:
- sh_exec(): When searching for built-ins and the restricted option
isn't active, also search bltin_tree for names beginning with a
slash.
- sh_ntfork(): Only throw an error if the PID value returned is
exactly -1. This allows path_spawn() to return -2 after running a
built-in to tell sh_ntfork() to do the right things to restore
state.
src/cmd/ksh93/sh/parse.c: simple():
- When searching for built-ins at parse time, only exclude names
containing a slash if the restricted option is active. This
allows finding pointers to built-ins invoked by literal path like
/opt/ast/bin/cat, as long as that does not result from an
expansion. This is not actually necessary as sh_exec() will also
cover this case, but it is an optimisation.
src/lib/libcmd/getconf.c:
- Replace convoluted deferral to external command by a simple
invocation of the path to the native getconf command determined
at compile time (by src/lib/libast/comp/conf.sh). Based on:
https://github.com/ksh93/ksh/issues/138#issuecomment-816384871
If there is ever a system that has /opt/ast/bin/getconf as its
default native external 'getconf', then there would still be an
infinite recursion crash, but this seems extremely unlikely.
Resolves: https://github.com/ksh93/ksh/issues/138
Previous discussion: https://github.com/att/ast/issues/485
If ksh attempts to execute a non-executable command found in the
PATH, in some instances the error message and return status are
incorrect. In the example below, ksh returns with exit status 126
when using the -c execve(2) optimization or when using fork(2) in
an interactive shell. However, using posix_spawn(3) causes the exit
status to change:
$ echo 'print cannot execute' > /tmp/x
# Runs command with spawnveg (i.e., posix_spawn or vfork)
$ ksh -c 'PATH=/tmp; x; echo $?'
ksh: x: not found
127
# Runs command with execve
$ ksh -c 'PATH=/tmp; x'; echo $?
ksh: x: cannot execute [Permission denied]
126
# Runs command with fork
$ ksh -ic 'PATH=/tmp; x; echo $?'
ksh: x: cannot execute [Permission denied]
126
Since 'x' is in the PATH but can't be executed, the correct exit
status is 126, not 127. It's worth noting this bug doesn't cause
the regression tests to fail with ksh93u+m, but it does cause one
test to fail when run under dtksh:
path.sh[706]: Long nonexistent command name: got status 126, ''
This commit backports various fixes for this bug from ksh2020, with
additional fixes applied (since there were still some additional
issues the ksh2020 patch didn't fix). The lacking regression test
for exit status 126 in path.sh has been rewritten to test for more
scenarios where ksh failed to return the correct error message
and/or exit status. I can also confirm with this patch applied the
path.sh regression tests now pass when run under dtksh.
src/cmd/ksh93/sh/path.c:
- Add a comment to path_absolute() describing 'oldpp' is the
current pointer in the while loop and 'pp' is the next pointer.
Backported from:
https://github.com/att/ast/commit/a6cad450
- The patch from ksh2020 didn't fix this bug in the SHOPT_SPAWN
code (because ksh2020 prefers fork(2)), so issues with the exit
status could still occur when using spawnveg. To fix this, always
set 'noexec' to the value of errno if can_execute fails. Before
this fix, errno was discarded if 'pp' was a null pointer and
can_execute failed.
- If a command couldn't be executed and the error wasn't ENOENT,
save errno in a 'not_executable' variable. If an executable
command couldn't be found in the PATH, exit with status 126 and
set errno to the saved value. This was based on a ksh2020 bugfix,
but it has been reworked a little bit to fix a bug that caused a
mismatch between the error message shown and errno. Example with
a non-executable file in PATH:
$ nonexec
ksh2020: nonexec: cannot execute [No such file or directory]
The ksh2020 patch: <https://github.com/att/ast/pull/493>
- Backport a ksh2020 bugfix for directories in the PATH when
running one of the added regression tests on OpenBSD:
https://github.com/att/ast/pull/767
src/cmd/ksh93/data/msg.c,
src/cmd/ksh93/include/shell.h,
src/cmd/ksh93/sh/{path,xec}.c:
- If a command name is too long (ENAMETOOLONG), then it wasn't
found in the PATH. For that case return exit status 127, like
for ENOENT.
src/cmd/ksh93/tests/path.sh:
- Replace the old test with a new set of more extensive tests.
These tests check the error message and exit status when ksh
attempts to run a command using any of the following:
- execve(2), used with the last command run with -c (*A tests).
- posix_spawn(3)/vfork(2), used in noninteractive scripts (*B tests).
- fork(2), used in interactive shells with job control (*C tests).
- command -x (*D tests).
- exec(1) (*E tests).
- Add a regression test from ksh2020 for attempting to execute a
directory:
https://github.com/att/ast/pull/758
src/lib/libast/include/ast.h,
src/lib/libast/include/wait.h:
- Avoid bitshifts in macros for static error codes. The return
values of command not found and exec related errors are static
values and should not require any macro magic for calculation.
Backported from: https://github.com/att/ast/commit/c073b102
- Simplify EXIT_* and W* macros to use 8 bits.
The usage options test wasn't properly excluding all dtksh builtins,
which was causing the regression tests to fail under dtksh. This commit
adds exclusions for the builtins missed in commit ef4fe41.
This commit fixes a segmentation fault when an attempt was made to
unset the default KSH_VERSION variable prior any other nameref
activity such as creating another nameref or even reassigning the
nameref KSH_VERSION to something else.
(new shell without prior nameref activity)
$ nameref
KSH_VERSION=.sh.version
$ unset -n KSH_VERSION
Memory fault
src/cmd/ksh93/sh/name.c: _nv_unset():
- Add a 'Refdict' check before attempting to remove a value from it
as apparently one does not exist until some sort of nameref
activity occurs after shell startup as the default nameref of
'KSH_VERSION=.sh.version' does not create one.
The bugfix for BUG_CMDSPASGN backported in commit fae8862c caused
two regressions with the += operator:
1. The += operator did not append to variables. Reproducer:
$ integer foo=3
$ foo+=2 command eval 'echo $foo'
2
2. The += operator ignored the readonly attribute, modifying readonly
variables in the same manner as above. Reproducer
$ readonly bar=str
$ bar+=ing command eval 'echo $bar'
ing
Both of the regressions above were caused by nv_putval() failing to
clone the variable from the previous scope into the invocation-local
scope. As a result, 'foo+=2' was effectively 0 + 2 (since ksh didn't
clone 3). The first regression was noticed during the development of
ksh93v-, so to fix both bugs I've backported the bugfix for the
regression from the ksh93v- 2013-10-10 alpha version:
https://www.mail-archive.com/ast-users@lists.research.att.com/msg00369.html
src/cmd/ksh93/sh/name.c:
- To fix both of the bugs above, find the variable to modify with
nv_search(), then clone it into the invocation local scope. To
fix the readonly bug as well, this is done before the NV_RDONLY
check (otherwise np will be missing that attribute and be
incorrectly modified in the invocation-local scope).
- Update a nearby comment describing what sh_assignok() does (per this
comment: https://github.com/ksh93/ksh/pull/249#issuecomment-811381759)
src/cmd/ksh93/tests/builtins.sh:
- Add regression tests for both of the now fixed regressions,
loosely based on the regression tests in ksh93v-.
src/cmd/ksh93/tests/readonly.sh:
- Use a 'ulimit --cpu' as a workaround to close down hung processes
that might be caused due to a couple of known bugs (recursion and
type variable function)
Discussion: https://github.com/ksh93/ksh/issues/264
- Adjust tests so xtrace can be used
- Use integer n within for loop
The recursion level for arithmetic expressions is kept track of in
a static 'level' variable in streval.c. It is reset when arithmetic
expressions throw an error.
But an error for an arithmetic expression may also occur elsewhere
-- at least in one case: when an arithmetic expression attempts to
change a read-only variable. In that case, the recursion level is
never reset because that code does not have access to the static
'level' variable.
If many such conditions occur (as in the new readonly.sh regression
tests), an arithmetic command like 'i++' may eventually fail with a
'recursion too deep' error.
To mitigate the problem, MAXLEVEL in streval.c was changed from 9
to 1024 in 264ba48b (as in the ksh 93v- beta). This commit leaves
that increase, but adds a proper fix.
src/cmd/ksh93/include/defs.h:
- Add global sh.arithrecursion (a.k.a. shp->arithrecursion)
variable to keep track of the arithmetic recursion level,
replacing the static 'level' variable in streval.c.
src/cmd/ksh93/sh/xec.c: sh_exec():
- Reset sh.arithrecursion before starting a new simple command
(TCOM), a new subshell with parentheses (TPAR), a new pipe
(TFIL), or a new [[ ... ]] command (TTST). These are the same
places where 'echeck' is set to 1 for --errexit and ERR trap
checks, so it should cover everything.
src/cmd/ksh93/sh/streval.c:
- Change all uses of 'level' to sh.arithrecursion.
- _seterror, aritherror(): No longer bother to reset the level
to zero here; xec.c should have this covered for all cases now.
src/cmd/ksh93/tests/arith.sh:
- Add tests for main shell and subshell.
One area where readonly is still ineffective is the local
environment list for a command (preceding assignments) if that
command is not executed using exec(3) after fork(2). Builtin
commands are one example. The following succeeds but should fail:
(readonly v=1; v=2 true) # succeeds, but should fail
If the shell is compiled with SHOPT_SPAWN (the default) then this
also applies to external commands invoked with sh_ntfork():
(readonly v=1; v=2 env) # succeeds if SHOPT_SPAWN
This presents to the user as inconsitent behaviour because external
commands may be fork()ed under certain circumstances but not
others, depending on complex optimisations. One example is:
$ ksh -c 'readonly v=1; v=2 env'
ksh: v: is read only
$ ksh -c 'readonly v=1; v=2 env; :'
(bad: environment list is output, including 'v=2')
In the first command above, where 'v2=env' is the last command in
the -c script, the optimisation skips creating a scope and assigns
the environment list in the current scope.
src/cmd/ksh93/sh/name.c: nv_setlist():
- Add check for readonly. This requires searching for the variable
in the main tree using nv_search() before a locally scoped one is
added using nv_open(). Since nv_search() only works with plain
variable names, temporarily end the string at '='.
src/cmd/ksh93/tests/readonly.sh:
- Add version check and fork the test command substitution subshell
on older versions that would otherwise abort the tests due to the
combination of an excessively low arithmetic recursion tolerance
and a bug that sometimes fails to restore the shell's arithmetic
recursion level.
Since f207cd57, sh_ntfork() is never called if job.jobcontrol is
set (i.e. if job control is active on an interactive shell), so the
code that is only run if job.jobcontrol is set should be removed.
src/cmd/ksh93/sh/xec.c:
- Remove spawnveg() define that is unused as of 7b0e0776.
- sh_exec(): Simplify SHOPT_SPAWN preprocessor logic. As sh_fork()
never returns a negative value, only run the parent<0 check after
running sh_ntfork() -- that check already didn't happen when
compiling ksh with SHOPT_SPAWN disabled.
- sh_ntfork(): Remove signal and terminal handling (with race
condition) that was only run with job.jobcontrol set.
src/cmd/ksh93/sh/args.c: sh_argopts():
- Remove special-casing for --posix (see also data/builtins.c) and
move the case -5: to the case ':' instead, so this option is
handled like all other long options. This change fixes two bugs:
1. 'set --posix' had no effect on the letoctal or braceexpand
options. Reproducer:
$ set --posix
$ [[ -o braceexpand ]]; echo $?
0
$ [[ -o letoctal ]]; echo $?
1
2. 'ksh --posix' could not run scripts correctly because it
wrongly enabled '-c'. Reproducer:
$ ksh --posix < <(echo 'exit 0')
ksh: -c requires argument
Usage: ksh [--posix] [arg ...]
Help: ksh [ --help | --man ] 2>&1
- Don't allow 'set --default' to unset the restricted option.
src/cmd/ksh93/tests/options.sh:
- Add regression tests for the bugs described above, using -o posix
and --posix.
src/cmd/ksh93/tests/restricted.sh:
- Add a regression test for 'set --default' in rksh.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
Using the stack makes it impossible for future buffer overflows to
occur. It also simplifies fmttoken() by eliminating the need to
declare a local buffer and pass a pointer to that as an argument.
For info: man src/lib/libast/man/stak.3
src/lib/libast/tm/tmlocale.c:
- Load the locale set by LC_TIME or LC_ALL if it hasn't been loaded
before or if it was loaded previously but isn't the current locale.
src/cmd/ksh93/tests/locale.sh:
- Add a regression test using the nl_NL.UTF-8 and ja_JP.UTF-8 locales.
Fixes: https://github.com/ksh93/ksh/issues/261
fmttoken() needs a minimal char[4] token buffer passed to it.
Originally reported by: Jakub Wilk <jwilk@jwilk.net>
Original bug report: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=879464
The following code lines from fmttoken() yield a n=3 for SYMSEMI as
n=1 from the start, e.g. 'for <>;'.
case SYMSEMI:
if(tok[0]=='<')
tok[n++] = '>';
sym = ';';
break;
default:
sym = 0;
}
tok[n++] = sym;
}
tok[n] = 0;
n[0]='<'
n[1]='>'
n[2]=';'
n[3]=0 # <-- BUFFER overflow as the passed character buffers have a size of 3
src/cmd/ksh93/sh/lex.c:
- DBUG: sh_lex(): Adjust char tokstr[3] to char tokstr[4]
- sh_syntax(): Adjust char tokbuf[3] to char tokbuf[4]
Many of these changes are minor typo fixes. The other changes
(which are mostly compiler warning fixes) are:
NEWS:
- The --globcasedetect shell option works on older Linux kernels
when used with FAT32/VFAT file systems, so remove the note about
it only working with 5.2+ kernels.
src/cmd/ksh93/COMPATIBILITY:
- Update the documentation on function scoping with an addition
from ksh93v- (this does apply to ksh93u+).
src/cmd/ksh93/edit/emacs.c:
- Check for '_AST_ksh_release', not 'AST_ksh_release'.
src/cmd/INIT/mamake.c,
src/cmd/INIT/ratz.c,
src/cmd/INIT/release.c,
src/cmd/builtin/pty.c:
- Add more uses of UNREACHABLE() and noreturn, this time for the
build system and pty.
src/cmd/builtin/pty.c,
src/cmd/builtin/array.c,
src/cmd/ksh93/sh/name.c,
src/cmd/ksh93/sh/nvtype.c,
src/cmd/ksh93/sh/suid_exec.c:
- Fix six -Wunused-variable warnings (the name.c nv_arrayptr()
fixes are also in ksh93v-).
- Remove the unused 'tableval' function to fix a -Wunused-function
warning.
src/cmd/ksh93/sh/lex.c:
- Remove unused 'SHOPT_DOS' code, which isn't enabled anywhere.
https://github.com/att/ast/issues/272#issuecomment-354363112
src/cmd/ksh93/bltins/misc.c,
src/cmd/ksh93/bltins/trap.c,
src/cmd/ksh93/bltins/typeset.c:
- Add dictionary generator function declarations for former
aliases that are now builtins (re: 1fbbeaa1, ef1621c1, 3ba4900e).
- For consistency with the rest of the codebase, use '(void)'
instead of '()' for print_cpu_times.
src/cmd/ksh93/sh/init.c,
src/lib/libast/path/pathshell.c:
- Move the otherwise unused EXE macro to pathshell() and only
search for 'sh.exe' on Windows.
src/cmd/ksh93/sh/xec.c,
src/lib/libast/include/ast.h:
- Add an empty definition for inline when compiling with C89.
This allows the timeval_to_double() function to be inlined.
src/cmd/ksh93/include/shlex.h:
- Remove the unused 'PIPESYM2' macro.
src/cmd/ksh93/tests/pty.sh:
- Add '# err_exit #' to count the regression test added in
commit 113a9392.
src/lib/libast/disc/sfdcdio.c:
- Move diordwr, dioread, diowrite and dioexcept behind
'#ifdef F_DIOINFO' to fix one -Wunused-variable warning and
multiple -Wunused-function warnings (sfdcdio() only uses these
functions when F_DIOINFO is defined).
src/lib/libast/string/fmtdev.c:
- Fix two -Wimplicit-function-declaration warnings on Linux by
including sys/sysmacros.h in fmtdev().
There's an annoying inconsistency in error messages if ksh is
compiled with SHOPT_SPAWN. One way to trigger it:
$ /usr/local/bin/ksh -c '/tmp/nonexistent'
/usr/local/bin/ksh: /tmp/nonexistent: not found
$ /usr/local/bin/ksh -c '/tmp/nonexistent; :'
/usr/local/bin/ksh: /tmp/nonexistent: not found [No such file or directory]
In the first variant, as an optimisation, ksh went straight to
exec'ing the command without forking first. In the second variant,
sh_ntfork() was used.
The first variant is done in path_exec(), path.c, line 1049:
errormsg(SH_DICT,ERROR_exit(ERROR_NOENT),e_found,arg0);
The second one is in sh_ntfork(), xec.c, line 3654:
errormsg(SH_DICT,ERROR_system(ERROR_NOENT),e_found+4);
In both cases, the e_found message is only used if errno==ENOENT,
so the extra '[No such file or directory]' message generated by
ERROR_system() is pointless as that will never change for that
message.
src/cmd/ksh93/sh/xec.c: sh_ntfork():
- Use ERROR_exit() instead of ERROR_system() for the e_found
message to avoid the superfluous addition.
If a system administrator prefixes /opt/ast/bin to the path and
then invokes the shell in restricted mode, they clearly intend for
the user to run those AST utilities.
Similarly, if a system administrator sets a PATH for a restricted
shell that includes libraries listed in the .paths file, they must
have intended for the user to use those loadable built-ins, as they
will be associated with the pathnames of their respective
libraries. Since the user cannot change PATH or use the builtin
command, they still cannot load just any built-in they choose.
src/cmd/ksh93/sh/path.c:
- Remove SH_RESTRICTED check when handling path-bound builtins
or dynamic libaries containining builtins in $PATH.
src/cmd/ksh93/tests/builtins.sh:
- Add test verifying a restricted user can use /opt/ast/bin/cat
via a PATH search.
Progresses: https://github.com/ksh93/ksh/issues/138
This commit fixes BUG_CSUBSTDO, which could break stdout inside of
non-forking command substitutions. The breakage only occurred when
stdout was closed outside of the command substitution and a file
descriptor other than stdout was redirected in the command substitution
(such as stderr). Thanks to the ast-open-history repo, I was able to
identify and backport the bugfix from ksh93v- 2012-08-24.
This backport may fix other bugs as well. On 93v- 2012-08-24 it
fixed the regression below, though it was not triggered on 93u+(m).
src/cmd/ksh93/tests/heredoc.sh
487 print foo > $tmp/foofile
488 x=$( $SHELL 2> /dev/null 'read <<< $(<'"$tmp"'/foofile) 2> /dev/null;print -r "$REPLY"')
489 [[ $x == foo ]] || err_exit '<<< $(<file) not working'
src/cmd/ksh93/sh/io.c: sh_open():
- If the just-opened file descriptor exists in sftable and is
flagged with SF_STRING (as in non-forking command substitutions,
among other situations), then move the file descriptor to a
number >= 10.
src/cmd/ksh93/tests/io.sh:
- Add a regression test for BUG_CSUBSTDO, adapted from the one in
modernish.
The current version of 93u+m does not have proper support for the
LC_TIME variable. Setting LC_TIME has no effect on printf %T, and
if the locale is invalid no error message is shown:
$ LC_TIME=ja_JP.UTF-8
$ printf '%T\n' now
Wed Apr 7 15:18:13 PDT 2021
$ LC_TIME=invalid.locale
$ # No error message
src/cmd/ksh93/data/variables.c,
src/cmd/ksh93/include/variables.h,
src/cmd/ksh93/sh/init.c:
- Add support for the $LC_TIME variable. ksh93v- attempted to add
support for LC_TIME, but the patch from that version was extended
because the variable still didn't function correctly.
src/cmd/ksh93/tests/variables.sh:
- Add LC_TIME to the regression tests for LC_* variables.
$ /usr/local/bin/ksh -c 'readonly v=1; export v'
/usr/local/bin/ksh: export: v: is read only
Every POSIX shell (even zsh, as of 5.8) allows this. So did ksh,
until the referenced commit.
src/cmd/ksh93/bltins/typeset.c: setall():
- Allow setting attributes on a readonly variable if any of
NV_ASSIGN (== NV_NOFREE), NV_EXPORT or NV_RDONLY are the only
flag bits that are set. This allows readonly, export, typeset -r,
typeset -x, and typeset -rx on variable arguments without an
assignment. Note that NV_ASSIGN is set for the first variable
argument even though it is not an assignment, so we must allow
it. The logic (or lack thereof) of that is yet to be worked out.
src/cmd/ksh93/tests/readonly.sh:
- Tests.
Resolves: https://github.com/ksh93/ksh/issues/258
This experiment, the initialisation of which was disabled with '#if
0', defines a bunch of integer type commands as special builtins.
Most are boring as they define variables just like normal integers:
pid_t, size_t, etc.
One is interesting: mode_t is a type that automatically converts
from a octal permission bits (e.g. 755) to a mode string like
u+rwx,g+rw,o+rw. That's not a compelling enough use case to
permanently define a special and immutable builtin though.
stat_t is odd: it takes a file name as an argument and fills the
variable with stat information, but it is base64 encoded binary
data and there doesn't seem to be anything that can parse it.
Anyway, none of this is going to be enabled, so we should get rid.
The typecast fix was insufficient, avoiding the crash only when
compiling with optimisation disabled. The real problem is that
put_lineno() was passed a misaligned pointer, and that the value
didn't actually contain a double but a string. The bug occurred
when restoring the LINENO value upon exiting a virtual subshell.
Thanks to Harald van Dijk for figuring out the fix.
src/cmd/ksh93/sh/subshell.c: nv_restore():
- When restoring a special variable as defined by nv_cover(),
do not pass either the np->nvflag bits or NV_NOFREE. Why?
* The np->nvflag bits are not needed. They are also harmful
because they may include the NV_INTEGER bit. This is set
when the value is numeric. However, nv_getval() always
returns the value in string form, converting it if it is
numeric. So the NV_INTEGER flag should never be passed
to nv_putval() when it uses the result of nv_getval().
* According to nval.3, the NV_NOFREE flag stops nv_putval() from
creating a copy of the value. But this should be unnecessary
because the earlier _nv_unset(mp,NV_RDONLY|NV_CLONE) should
ensure there is no previous value. In addition, the NV_NOFREE
flag triggered another bug that caused the value of SECONDS to
be corrupted upon restoring it when exiting a virtual subshell.
- When restoring a regular variable, copy the entire nvalue union
and not just the 'cp' member. In practice this worked because
no current member of the nvalue union is larger than a pointer.
However, there is no guarantee it will stay that way.
src/cmd/ksh93/tests/leaks.sh:
- Add disabled test for a memory leak that was discovered in the
course of dealing with this bug. The fix doesn't introduce or
influence it. It will have to be dealt with later.
src/cmd/ksh93/tests/locale.sh:
- Add test for restoring locale on leaving virtual subshell.
https://github.com/ksh93/ksh/issues/253#issuecomment-815290154
src/cmd/ksh93/tests/variables.sh:
- Test against corruption of SECONDS on leaving virtual subshell.
https://github.com/ksh93/ksh/issues/253#issuecomment-815191052
Co-authored-by: Harald van Dijk <harald@gigawatt.nl>
Progresses: https://github.com/ksh93/ksh/issues/253
On Ubuntu arm7, two variables.sh regression tests crashed with a
bus error (SIGBUS) in init.c on line 720 while testing $LINENO:
707 static void put_lineno(Namval_t* np,const char *val,int flags,Namfun_t *fp)
708 {
709 register long n;
710 Shell_t *shp = sh_getinterp();
711 if(!val)
712 {
713 fp = nv_stack(np, NIL(Namfun_t*));
714 if(fp && !fp->nofree)
715 free((void*)fp);
716 _nv_unset(np,NV_RDONLY);
717 return;
718 }
719 if(flags&NV_INTEGER)
720 n = *(double*)val;
721 else
722 n = sh_arith(shp,val);
723 shp->st.firstline += nget_lineno(np,fp)+1-n;
724 }
Apparently, gcc on arm7 doesn't like the implicit typecast from
double to long.
Those three $LINENO discipline functions are generally a mess of
implicit typecasts between Sfdouble_t, double, long and int.
Line numbers are internally stored as int. The discipline functions
need to use Sfdouble_t for API compatibility.
src/cmd/ksh93/sh/init.c: nget_lineno(), put_lineno(), get_lineno():
- Get rid of unnecessary implicit typecasts by adjusting the types
of local variables.
- Make the typecasts that are done explicit.
Progresses: https://github.com/ksh93/ksh/issues/253
These fixes are applied rather blindly as no one has yet managed to
understand the almost entirely uncommented arrays and variables
handling code (arrays.c, name.c, nvdisc.c, nvtree.c, nvtype.c).
Hopefully we'll figure all that out at some point. In the meantime
these backported fixes appear to work fine, and these bugs impact
the usability of 'enum', so I'm just going to have to violate my
own policy and backport these fixes without understanding them.
Thanks to @JohnoKing for putting in a lot of work tracing these.
Further discussion at: https://github.com/ksh93/ksh/issues/87
src/cmd/ksh93/sh/array.c:
- nv_arraysettype():
* Further simplify the function. After my initial simplification
of it (re: 5491fe97), I don't believe there's actually a need
to save a duplicate copy of the value. Use the pointer returned
by nv_getval() directly to restore the value.
* Cope with a null value (nv_getval() returning a NULL pointer).
This is needed for compatibility with the backported fix in
nvtype.c (below).
- array_putval(): If the array's value pointer (up->cp) is a
pointer to the empty string, it is set to NULL before calling
nv_putv() to prevent an empty string from being deleted. Backport
a fix from 93v- that restores the pointer to the empty string if
the NV_NOFREE attribute is set. Removing it somehow causes these
regressions:
enum.sh[86]: ${array[@]} doesn't yield all values for
associative enum arrays (expected 'green blue blue red
yellow green red orange'; got 'green blue blue yellow
green orange')
enum.sh[94]: unsetting associative enum array does not work
(got 'Color_t -A Colors=([foo]=red [rood]=red)')
enum.sh[116]: assigning first enum element to indexed array
failed (expected 'red red'; got 'BUG BUG')
- nv_associative(): Do not increase the 'nelem' (number of
elements) value of the array's 'header' struct if the array is
associative and of an enum type. The original 93v- fix only
checked for the NV_INTEGER attribute, but backporting that caused
several regressions. Using a debug output command I've determined
that the exact value of 'type' is somehow consistently set to
0x26 if the array is associative and of an enum type, which is
NV_INTEGER | NV_LTOU | NV_RJUST as defined in include/nval.h. I
cannot find where/how that value is determined. In any case this
fix, based on but more specific than the 93v- one, appears to
work fine. Removing it somehow causes this regression:
enum.sh[94]: unsetting associative enum array does not work
(got 'Color_t -A Colors=()')
src/cmd/ksh93/sh/nvtype.c: nv_settype():
- Another fix backported from 93v-. If the variable is an array,
also set the type of element 0 of that array using a call to
nv_arraysettype(). The value may be null. Removing this somehow
causes this regression:
enum.sh[94]: unsetting associative enum array does not work
(got 'Color_t -A Colors=()')
src/cmd/ksh93/tests/enum.sh:
- Add tests for all the bugs fixed here, plus some hypothetical
bugs (e.g., do the same tests for indexed enum type arrays as for
associative enum type arrays, even though indexed enum type
arrays didn't have all the same problems).
Co-authored-by: Johnothan King <johnothanking@protonmail.com>
Resolves: https://github.com/ksh93/ksh/issues/87
Simple reproducer:
set -A arr a b c d; : ${arr[1..2]}; unset arr[1]; echo ${arr[@]}
Output:
a
Expected output:
a c d
The ${arr[1..2]} expansion broke the subsequent 'unset' command
so that it unsets element 1 and on, instead of only 1.
This regression was introduced in nv_endsubscript() on 2009-07-31:
https://github.com/ksh93/ast-open-history/commit/c47896b4/src/cmd/ksh93/sh/array.c
That change checks for the ARRAY_SCAN attribute which enables
processing ranges of array elements instead of single array
elements, and restores it after. That restore is evidently not
correct as it causes the subsequent unset command to malfunction.
If we revert that change, the bug disappears and the regression
tests show no failures. However, I don't know what this was meant
to accomplish and what other bug we might introduce by reverting
this. However, no corresponding regression test was added along
with the 2009-07-31 change, nor is there any corresponding message
in the changelog. So this looks to be one of those mystery changes
that we'll never know the reason for.
Since we currently have proof that this change causes breakage and
no evidence that it fixes anything, I'll go ahead and revert it
(and add a regression test, of course). If that causes another
regression, hopefully someone will find it at some point.
src/cmd/ksh93/sh/array.c: nv_endsubscript():
- Revert the 2009-07-31 change that saves/restores the ARRAY_SCAN
attribute.
- Keep the 'ap' pointer as it is now used by newer code. Move the
declaration up to the beginning of the block, as is customary.
src/cmd/ksh93/sh/init.c:
- Cosmetic change: remove an unused array_scan() macro that I found
when grepping the code for ARRAY_SCAN. The macro was introduced
in version 2001-06-01 but the code that used it was replaced in
version 2001-07-04, without removing the macro itself.
Resolves: https://github.com/ksh93/ksh/issues/254
To set a window title in bash and zsh, the $PS1 prompt can be set
with the title placed between $'\E]0;' and $'\a':
set -o emacs # Or vi mode
typeset -A fmt=(
[start_title]=$'\E]0;'
[end_title]=$'\a'
)
PS1="${fmt[start_title]}$(hostname): $(uname)${fmt[end_title]}\$ "
This also works in ksh unless the shell receives SIGWINCH. With a
$PS1 that sets a window title, the prompt breaks until two
interrupts are received. This is caused by ed_setup() skipping
$'\a' (the bell character) when setting up the e_prompt buffer
which is an edited version of the final line of the PS1 prompt for
use when redrawing the command line.
One fix would be to avoid cutting out the bell character. But if
the prompt contains a bell, we only want the terminal to beep when
a new prompt is printed, and not upon refreshing the command line,
e.g. when receiving SIGWINCH or pressing Ctrl+L.
To avoid the problem, this commit adds code that cuts out sequences
of the form ESC ] <number> ; <text> BELL from the prompt redraw
buffer altogether. They are not needed there because these
sequences will already have taken effect when the full prompt was
printed by io_prompt().
This commit also adds a tweak that should improve the recognition
of other escape sequences to count their length.
src/cmd/ksh93/edit/edit.c: ed_setup():
- When preparing the e_prompt buffer, cut out dtterm/xterm
Operating System Commands that set window/icon title, etc.
See: https://invisible-island.net/xterm/ctlseqs/ctlseqs.html
- When counting the length of escape sequences in that part of PS1,
try to recognize some more types of sequences. These changes are
part of a ksh2020 patch: https://github.com/att/ast/issues/399
src/cmd/ksh93/sh.1:
- Document that any '!' in escape sequences in the PS1 prompt needs
to be changed to '!!'. To avoid breaking compatibility, this
requirement is documented instead of backporting the changes to
io_prompt() from https://github.com/att/ast/issues/399 which try
to remove that requirement for specific escape sequences.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
Ksh currently restricts readonly scalar variables from having their
values directly changed via a value assignment. However, since ksh
allows variable attributes to be altered, the variable's value can
be indirectly altered. For instance, if TMOUT=900 (for a 15 minute
idle timeout) was set to readonly, all that is needed to alter the
value of TMOUT from 900 to 0 is to issue 'typeset -R1 TMOUT',
perhaps followed by a 'typeset -i TMOUT' to turn off the shell's
timeout value.
In addition, there are problems with arrays. The following is
incorrectly allowed:
typeset -a arr=((a b c) 1)
readonly arr
arr[0][1]=d
arr=(alphas=(a b c);name=x)
readonly arr.alphas
arr.alphas[1]=([b]=5)
arr=(alphas=(a b c);name=x)
readonly arr.alphas
arr.alphas[1]=(b)
typeset -C arr=(typeset -r -a alphas=(a b c);name=x)
arr.alphas[1]=()
src/cmd/ksh93/bltins/typeset.c: setall():
- Relocate readonly attribute check higher up the code and widen
its application to issue an error message if the pre-existing
name-pair has the readonly bit flag set.
- To avoid compatibility problems, don't check for readonly if
NV_RDONLY is the only attribute set (ignoring NV_NOFREE). This
allows 'readonly foo; readonly foo' to keep working.
src/cmd/ksh93/sh/array.c: nv_endsubscript():
- Apply a readonly flag check when an array subscript or append
assignment occurs, but allow type variables (typeset -T) as they
utilize '-r' for 'required' sub-variables.
src/cmd/ksh93/tests/readonly.sh:
- New file. Create readonly tests that validate the warning message
and validate that the readonly variable did not change.
src/cmd/ksh93/sh/streval.c:
- Bump MAXLEVEL from 9 to 1024 as a workaround for arithmetic
expansion, avoiding a spurious error about too much recursion
when the readonly.sh tests are run. This change is backported
from ksh 93v-.
TODO: debug a spurious increase in arithmetic recursion level
variable when readonly.sh tests with 'typeset -i' are run.
That is a different bug for a different commit.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
This commit adds an UNREACHABLE() macro that expands to either the
__builtin_unreachable() compiler builtin (for release builds) or
abort(3) (for development builds). This is used to mark code paths
that are never to be reached.
It also adds the 'noreturn' attribute to functions that never
return: path_exec(), sh_done() and sh_syntax(). The UNREACHABLE()
macro is not added after calling these.
The purpose of these is:
* to slightly improve GCC/Clang compiler optimizations;
* to fix a few compiler warnings;
* to add code clarity.
Changes of note:
src/cmd/ksh93/sh/io.c: outexcept():
- Avoid using __builtin_unreachable() here since errormsg can
return despite using ERROR_system(1), as shp->jmplist->mode is
temporarily set to 0. See: https://github.com/att/ast/issues/1336
src/cmd/ksh93/tests/io.sh:
- Add a regression test for the ksh2020 bug referenced above.
src/lib/libast/features/common:
- Detect the existence of either the C11 stdnoreturn.h header or
the GCC noreturn attribute, preferring the former when available.
- Test for the existence of __builtin_unreachable(). Use it for
release builds. On development builds, use abort() instead, which
crahses reliably for debugging when unreachable code is reached.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
This commit fixes a bug in the ksh uname builtin's -d option that could
change the output of -o (I was only able to reproduce this on Linux):
$ builtin uname
$ uname -o
GNU/Linux
$ uname -d
(none)
$ uname -o
(none)
I identified this patch from ksh2020 as a fix for this bug:
<https://github.com/att/ast/pull/1187>
The linked patch was meant to fix a crash in 'uname -d', although I've
had no luck reproducing it: <https://github.com/att/ast/issues/1184>
src/lib/libcmd/uname.c:
- Pass correct buffer to getdomainname() while executing uname -d.
src/cmd/ksh93/tests/builtins.sh:
- Add a regression test for the reported 'uname -d' crash.
- Add a regression test for the output of 'uname -o' after 'uname -d'.
- To handle potential crashes when running the regression tests in older
versions of ksh, fork the command substitutions that run 'uname -d'.
This bug was first reported at <https://github.com/att/ast/issues/8>.
The 'cd' command currently takes the value of $OLDPWD from the
wrong scope. In the following example 'cd -' will change the
directory to /bin instead of /tmp:
$ OLDPWD=/bin ksh93 -c 'OLDPWD=/tmp cd -'
/bin
src/cmd/ksh93/bltins/cd_pwd.c:
- Use sh_scoped() to obtain the correct value of $OLDPWD.
- Fix a use-after-free bug. Make the 'oldpwd' variable a static
char that points to freeable memory. Each time cd is used, this
variable is freed if it points to a freeable memory address and
isn't also a pointer to shp->pwd.
src/cmd/ksh93/sh/path.c: path_pwd():
- Simplify and add comments.
- Scope $PWD properly.
src/cmd/ksh93/tests/builtins.sh,
src/cmd/ksh93/tests/leaks.sh:
- Backport the ksh2020 regression tests for 'cd -' when $OLDPWD is
set.
- Add test for $OLDPWD and $PWD after subshare.
- Add test for $PWD after 'cd'.
- Add test for possible memory leak.
- Add testing for 'unset' on OLDPWD and PWD.
src/cmd/ksh93/COMPATIBILITY:
- Add compatibility note about changes to $PWD and $OLDPWD.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
This commit adds '/* FALLTHROUGH */' comments to fix many
GCC warnings when compiling with -Wimplicit-fallthrough.
Additionally, the existing fallthrough comments have been
changed for consistency.
src/cmd/ksh93/tests/variables.sh: LC_* error tests:
- Since operating systems validate locale strings differently,
try a few different bad locale strings to find one that makes
setlocale(2) fail, fixing test failures on OpenBSD and Debian.
- Restore warning removed in aed5c6d7, issuing it if none of the
bad locale strings produce a diagnostic.
- Reenable test for diagnostic message disabled in aed5c6d7.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
This bug was originally reported at <https://github.com/att/ast/issues/1467>.
A crash can occur when using the 'b' or 'B' vi mode commands to go back
one word. I was able to reproduce these crashes with 100% consistency on
an OpenBSD virtual machine when ksh is compiled with -D_std_malloc.
Reproducer:
$ set -o vi
$ asdf <ESC> <b or B>
The fix is based on Matthew DeVore's analysis:
> I suspect this is caused by this line:
>> while (vi_isalph(tcur_virt) && tcur_virt >= first_virt) --tcur_virt;
> which is in the b codepath. It checks vi_isalph(tcur_virt) before checking
> if tcur_virt is in range. These two clauses should be reversed. Note that
> line 316 is a similar check for pressing B, and there the tcur_virt value
> is checked first.
src/cmd/ksh93/edit/vi.c:
- Check tcur_virt before using isalph() or isblank() to fix both crashes.
At the start of the backword() while loop this check was performed
twice, so the redundant check has been removed.
src/cmd/ksh93/tests/pty.sh:
- Add a regression test for the b, B, w and W editor commands.
src/cmd/ksh93/bltins/test.c:
- Fix the following compiler warnings from clang:
test.c:554:11: warning: assigning to 'char *' from 'const char []'
discards qualifiers
[-Wincompatible-pointer-types-discards-qualifiers]
e_msg = e_badop;
^ ~~~~~~~
test.c:556:11: warning: assigning to 'char *' from 'const char []'
discards qualifiers
[-Wincompatible-pointer-types-discards-qualifiers]
e_msg = e_unsupported_op;
^ ~~~~~~~~~~~~~~~~
test.c:560:1: warning: control may reach end of non-void function
[-Wreturn-type]
src/cmd/ksh93/tests/builtins.sh:
- Fix regression test by updating error message text.
When test is passed the '=~' operator, it will silently fail with
exit status 1:
$ test foo =~ foo; echo $?
1
This bug is caused by test_binop reaching the 'NOTREACHED' area of
code. The bugfix was adapted from ksh2020:
https://github.com/att/ast/issues/1152
src/cmd/ksh93/bltins/test.c: test_binop():
- Error out with a message suggesting usage of '[[ ... ]]' if '=~'
is passed to the test builtin.
- Special-case TEST_END (']]') as that is not really an operator.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
src/lib/libast/tm/tminit.c:
- Commit 9f43f8d1, in addition to backporting fixes from ksh93v-, also
backported this bug:
$ printf '%(%Z)T' now
PPT # Should be PDT
Reapply the ksh2020 bugfix to fix the %Z time
format again.
src/cmd/ksh93/tests/builtins.sh:
- Add a regression test so this bug (hopefully) isn't backported from
ksh93v- again).
Every so often, a commit's GitHub CI run throws the following
regression test failure:
sigchld.sh[57]: expected '2 background' -- got '3' (DELAY=0.02)
When I re-run the job, the failure usually goes away.
In 712261c8 the DELAY variable was changed from 0.2 to 0.02 to
speed up the first SIGCHLD test. It's possible the GitHub CI
runners are just too slow or too heavily loaded for that.
src/cmd/ksh93/tests/sigchld.sh:
- Restore 0.2 value for 'float DELAY'.
I grepped for #include changes in all the commits and compared
that to the changes in the Mamfiles. I found 7 commits that don't
update the Mamfiles with the appropriate dependencies while
adding #includes, as I only learned how this works after having
worked with this code for some time.
This commit adds the missing Mamfile updates for the
corresponding #include changes in the following commits:
06e721c3, 65d363fd, 70fc1da7, 79d19458, b1a41311, bb4d6a2e,
db71b3ad, and this commit.
Additionally:
src/lib/libast/comp/setlocale.c:
- Change include errno.h to error.h to use EILSEQ fallback if
needed; remove corresponding #ifdef (re: 4dcf5c50, 71bfe028).
src/cmd/ksh93/Mamfile:
- Fix a broken dependency on libast FEATURE/float (re: 72968eae).
We can't use 'prev' for a file that was not mentioned before in
the same Mamfile, we have to use a 'make'...'done' on the first
mention. Add subdependencies matching those in libast/Mamfile.
src/cmd/ksh93/bltins/print.c:
- Rename the unlisted and misleadingly named SHOPT_ECHOE option
(which disables, not enables, 'echo -e') to SHOPT_NOECHOE.
src/cmd/ksh93/SHOPT.sh:
- Add the SHOPT_NOECHOE and SHOPT_TEST_L compile time options to
the list of SHOPT options. Since there is a probe for TEST_L,
set it to probe (empty) by default. NOECHE is off by default.
src/cmd/ksh93/features/options:
- Small bugfix: Allow SHOPT_TEST_L to be manually enabled on
systems that don't support '$(whence -p test) -l /foo'.
- Add a comment describing the SHOPT_MULTIBYTE feature test and
separate it from the SHOPT_DEVFD test.
This bugfix comes from <https://github.com/att/ast/pull/711>.
Eric Scrivner provided the following explanation for the fix:
> Coverity identified an issue with integer truncation in
> `put_enum`. The function was truncating the return values of
> `strcasecmp` and `strcmp` from an `int` to an `unsigned short`
> when assigning them to the local variable `n`. Since either of
> these methods can return a value that is not in the set `{0, 1,
> -1}` the later check if `n == 0` could spuriously evaluate to
> true. For example, in the case where either function returned
> `-65536`.
> The fix is simply to change `n` from an `unsigned short` to an
> `int` to avoid the possibility of truncation. Since the only
> purpose of `n` is the store the return values of these checks,
> this does not have any side effects.
That bit of code supported bash's redundant 'function foo()'
function declaration syntax (with both the 'function' keyword
and the '()') which is a syntax error on ksh, as it should be.
Moving the 'err_exit' and 'warning' alias definitions in the
regression tests to one _common file introduced a bug: they are no
longer expanded at compile time when the tests are run with shcomp,
resulting in a 'command not found' (at best) on trying to execute
one. shcomp requires that the alias definitions need to be present
in the file itself. But that means maintaining 50-odd copies again.
I'd rather add a hack to shtests to avoid this.
src/cmd/ksh93/tests/shtests:
- Before running a test with shcomp, physically concatenate _common
and the test script together into a temporary file, minus the '.'
command that includes _common, and compile that with shcomp.
The NOT_USED() macro is already defined in ast.h (which is included
by shell.h) as an alias of NoP(). So it's better to apply the fix
to NoP() so it takes effect for both verrsions, for libast and ksh.
One of the best-kept secrets of libast/ksh93 is that the code
includes support for case-insensitive file name generation (a.k.a.
pathname expansion, a.k.a. globbing) as well as case-insensitive
file name completion on interactive shells, depending on whether
the file system is case-insensitive or not. This is transparently
determined for each directory, so a path pattern that spans
multiple file systems can be part case-sensitive and part case-
insensitive. In more precise terms, each slash-separated path name
component pattern P is treated as ~(i:P) if its parent directory
exists on a case-insensitive file system. I recently discovered
this while dealing with <https://github.com/ksh93/ksh/issues/223>.
However, that support is dead code on almost all current systems.
It depends on pathconf(2) having a _PC_PATH_ATTRIBUTES selector.
The 'c' attribute is supposedly returned if the given directory is
on a case insensitive file system. There are other attributes as
well (at least 'l', see src/lib/libcmd/rm.c). However, I have been
unable to find any system, current or otherwise, that has
_PC_PATH_ATTRIBUTES. Google and mailing list searches yield no
relevant results at all. If anyone knows of such a system, please
add a comment to this commit on GitHub, or email me.
An exception is Cygwin/Windows, on which the "c" attribute was
simply hardcoded, so globbing/completion is always case-
insensitive. As of Windows 10, that is wrong, as it added the
possibility to mount case-sensitive file systems.
On the other hand, this was never activated on the Mac, even
though macOS has always used a case-insensitive file like Windows.
But, being UNIX, it can also mount case-sensitive file systems.
Finally, Linux added the possibility to create individual case-
insensitive ext4 directories fairly recently, in version 5.2.
https://www.collabora.com/news-and-blog/blog/2020/08/27/using-the-linux-kernel-case-insensitive-feature-in-ext4/
So, since this functionality latently exists in the code base, and
three popular OSs now have relevant file system support, we might
as well make it usable on those systems. It's a nice idea, as it
intuitively makes sense for globbing and completion behaviour to
auto-adapt to file system case insensitivity on a per-directory
basis. No other shell does this, so it's a nice selling point, too.
However, the way it is coded, this is activated unconditionally on
supported systems. That is not a good idea. It will surprise users.
Since globbing is used with commands like 'rm', we do not want
surprises. So this commit makes it conditional upon a new shell
option called 'globcasedetect'. This option is only compiled into
ksh on systems where we can actually detect FS case insensitivity.
To implement this, libast needs some public API additions first.
*** libast changes ***
src/lib/libast/features/lib:
- Add probes for the linux/fs.h and sys/ioctl.h headers.
Linux needs these to use ioctl(2) in pathicase(3) (see below).
src/lib/libast/path/pathicase.c,
src/lib/libast/include/ast.h,
src/lib/libast/man/path.3,
src/lib/libast/Mamfile:
- Add new pathicase(3) public API function. This uses whatever
OS-specific method it can detect at compile time to determine if
a particular path is on a case-insensitive file system. If no
method is available, it only sets errno to ENOSYS and returns -1.
Currently known to work on: macOS, Cygwin, Linux 5.2+, QNX 7.0+.
- On systems (if any) that have the mysterious _PC_PATH_ATTRIBUTES
selector for pathconf(2), call astconf(3) and check for the 'c'
attribute to determine case insensitivity. This should preserve
compatibility with any such system.
src/lib/libast/port/astconf.c:
- dynamic[]: As case-insensitive globbing is now optional on all
systems, do not set the 'c' attribute by default on _WINIX
(Cygwin/Windows) systems.
- format(): On systems that do not have _PC_PATH_ATTRIBUTES, call
pathicase(3) to determine the value for the "c" (case
insensitive) attribute only. This is for compatibility as it is
more efficient to call pathicase(3) directly.
src/lib/libast/misc/glob.c,
src/lib/libast/include/glob.h:
- Add new GLOB_DCASE public API flag to glob(3). This is like
GLOB_ICASE (case-insensitive matching) except it only makes the
match case-insensitive if the file system for the current
pathname component is determined to be case-insensitive.
- gl_attr(): For efficiency, call pathicase(3) directly instead of
via astconf(3).
- glob_dir(): Only call gl_attr() to determine file system case
insensitivity if the GLOB_DCASE flag was passed. This makes case
insensitive globbing optional on all systems.
- glob(): The options bitmask needs to be widened to fit the new
GLOB_DCASE option. Define this centrally in a new GLOB_FLAGMASK
macro so it is easy to change it along with GLOB_MAGIC (which
uses the remaining bits for a sanity check bit pattern).
src/lib/libast/path/pathexists.c:
- For efficiency, call pathicase(3) directly instead of via
astconf(3).
*** ksh changes ***
src/cmd/ksh93/features/options,
src/cmd/ksh93/SHOPT.sh:
- Add new SHOPT_GLOBCASEDET compile-time option. Set it to probe
(empty) by default so that the shell option is compiled in on
supported systems only, which is determined by new iffe feature
test that checks if pathicase(3) returns an ENOSYS error.
src/cmd/ksh93/data/options.c,
src/cmd/ksh93/include/shell.h:
- Add -o globcasedetect shell option if compiling with
SHOPT_GLOBCASEDET.
src/cmd/ksh93/sh/expand.c: path_expand():
- Pass the new GLOB_DCASE flag to glob(3) if the
globcasedetect/SH_GLOBCASEDET shell option is set.
src/cmd/ksh93/edit/completion.c:
- While file listing/completion is based on globbing and
automatically becomes case-insensitive when globbing does, it
needs some additional handling to make a string comparison
case-insensitive in corresponding cases. Otherwise, partial
completions may be deleted from the command line upon pressing
tab. This code was already in ksh 93u+ and just needs to be
made conditional upon SHOPT_GLOBCASEDET and globcasedetect.
- For efficiency, call pathicase(3) directly instead of via
astconf(3).
src/cmd/ksh93/sh.1:
- Document the new globcasedetect shell option.
These are minor fixes I've accumulated over time. The following
changes are somewhat notable:
- Added a missing entry for 'typeset -s' to the man page.
- Add strftime(3) to the 'see also' section. This and the date(1)
addition are meant to add onto the documentation for 'printf %T'.
- Removed the man page the entry for ksh reading $PWD/.profile on
login. That feature was removed in commit aa7713c2.
- Added date(1) to the 'see also' section of the man page.
- Note that the 'hash' command can be used instead of 'alias -t' to
workaround one of the caveats listed in the man page.
- Use an 'out of memory' error message rather than 'out of space'
when memory allocation fails.
- Replaced backticks with quotes in some places for consistency.
- Added missing documentation for the %P date format.
- Added missing documentation for the printf %Q and %p formats
(backported from ksh2020: https://github.com/att/ast/pull/1032).
- The comments that show each builtin's options have been updated.
In 2021, it seems like it's about time to join the 21st century
and officially require fork(2). In practice this was already the
case as the legacy code was unmaintained and didn't compile.
src/cmd/ksh93/edit/edit.c,
src/cmd/ksh93/edit/history.c,
src/cmd/ksh93/sh/deparse.c:
- Remove experimental code protected by '#ifdef future'.
No one is going to do anything with this, it's just clutter.
src/lib/libast/sfio/sfcvt.c:
- In 2021, it might be time to actually start using some C99
features were available. Change two checks for a _c99_in_the_wild
macro to actual checks for C99, enabling the use of fpclassify().
Resolves: https://github.com/ksh93/ksh/issues/219
This removes #ifdefs checking for the existence of
SH_PLUGIN_VERSION (version check for dynamically loaded builtins)
and the SFIO identifiers SF_BUFCONST, SF_CLOSING, SF_APPENDWR,
SF_ATEXIT, all of which are defined by the bundled libast.
While experimenting with #233, a memory segmentation fault occurred.
A search of other emacs issues found a potential matching issue as
described in https://github.com/att/ast/pull/791. Also, a duplicate
PR of https://github.com/att/ast/pull/1489 was submitted. This
commit backports that fix.
src/cmd/ksh93/edit/history.c: hist_word():
- Switch from using strcpy to memmove as the two strings could overlap.
This was failing again on FreeBSD. Replicating the test in a real
session worked as expected.
Apparently, we just cannot rely on external 'vi' utilities playing
well with pty. This test has caused enough trouble. Removed.
Upon encountering two filenames with multibyte characters starting
with the same byte, a partial multibyte character was completed.
Reproducer (to run in UTF-8 locale):
$ touch XXXá XXXë
$ : XX <== pres tab
$ : XXX^? <== partial multibyte character appears
Note: á is $'\xc3\xa1' and ë is $'\xc3\xab' (same initial byte).
src/cmd/ksh93/edit/completion.c:
- Add multibyte support to the charcmp() and overlaid() functions.
Thanks to Harald van Dijk for useful code and suggestions.
- Add a few missing mbinit() calls. The state of multibyte
processing must be reset before starting a new loop in case a
previous processing run was interrupted mid-character.
src/cmd/ksh93/tests/pty.sh:
- Add test based on Harald's reproducer.
Resolves: https://github.com/ksh93/ksh/issues/223
Until now, when performing any tilde expansion like ~/foo or
~user/foo, ksh added a placeholder built-in command called
'.sh.tilde', ostensibly with the intention to allow users to
override it with a shell function or custom builtin. The multishell
ksh93 repo <https://github.com/multishell/ksh93/> shows this was
added sometime between 2002-06-28 and 2004-02-29. However, it has
never worked and crashed the shell.
This commit replaces that with something that works. Specific tilde
expansions can now be overridden using .set or .get discipline
functions associated with the .sh.tilde variable (see manual,
Discipline Functions).
For example, you can use either of:
.sh.tilde.set()
{
case ${.sh.value} in
'~tmp') .sh.value=${XDG_RUNTIME_DIR:-${TMPDIR:-/tmp}} ;;
'~doc') .sh.value=~/Documents ;;
'~ksh') .sh.value=/usr/local/src/ksh93/ksh ;;
esac
}
.sh.tilde.get()
{
case ${.sh.tilde} in
'~tmp') .sh.value=${XDG_RUNTIME_DIR:-${TMPDIR:-/tmp}} ;;
'~doc') .sh.value=~/Documents ;;
'~ksh') .sh.value=/usr/local/src/ksh93/ksh ;;
esac
}
src/cmd/ksh93/include/variables.h,
src/cmd/ksh93/data/variables.c:
- Add SH_TILDENOD for a new ${.sh.tilde} predefined variable.
It is initially unset.
src/cmd/ksh93/sh/macro.c:
- sh_btilde(): Removed.
- tilde_expand2(): Rewritten. I started out with the tiny version
of this function from the 2002-06-28 version of ksh. It uses the
stack instead of sfio, which is more efficient. A bugfix for
$HOME == '/' was retrofitted so that ~/foo does not become
//foo instead of /foo. The rest is entirely new code.
To implement the override functionality, it now checks if
${.sh.tilde} has any discipline function associated with it.
If it does, it assigns the tilde expression to ${.sh.tilde} using
nv_putval(), triggering the .set discipline, and then reads it
back using nv_getval(), triggering the .get discipline. The
resulting value is used if it is nonempty and does not still
start with a tilde.
src/cmd/ksh93/bltins/typeset.c,
src/cmd/ksh93/tests/builtins.sh:
- Since ksh no longer adds a dummy '.sh.tilde' builtin, remove the
ad-hoc hack that suppressed it from the output of 'builtin'.
src/cmd/ksh93/tests/tilde.sh:
- Add tests verifying everything I can think of, as well as tests
for bugs found and fixed during this rewrite.
src/cmd/ksh93/tests/pty.sh:
- Add test verifying that the .sh.tilde.set() discipline does not
modify the exit status value ($?) when performing tilde expansion
as part of tab completion.
src/cmd/ksh93/sh.1:
- Instead of "tilde substitution", call the basic mechanism "tilde
expansion", which is the term used everywhere else (including the
1995 Bolsky/Korn ksh book).
- Document the new override feature.
Resolves: https://github.com/ksh93/ksh/issues/217
That patch broke the build on Cygwin, where gcc apparently doesn't
have the required atomic addition/subtraction compiler builtins.
The build fails at link time with those functions not found.
As far as I know, ksh was actually working fine (after @JohnoKing's
gcc workaround in c258a04f), so I'll just revert this for now. If a
need for it is demonstrated later, we'll have to add a feature test
or find some other way to get it working on Cygwin.
"savxit -= SH_EXITSIG + 128;" may have worked accidentally due to
subsequent bitmasking, but is blatantly wrong . It subtracts 256 +
128 = 384 from the exit status.
Use bitwise logic instead, with an octal literal 0200 instead of
128. This makes more sense in this context.
