- sh/args.c: A process substitution run in a profile script may print
its PID as if it was a command spawned with '&'. Reproducer:
$ cat /tmp/env
true >(false)
$ ENV=/tmp/env ksh
[1] 730227
$
This bug is fixed by turning off the SH_PROFILE state while running
a process substitution.
- sh/subshell.c: The SH_INTERACTIVE fix in 3525535e renders the extra
check for SH_PROFILE redundant, so it has been removed.
- tests/io.sh: Update the procsub PIDs test to also check the result
after using process substitution in a profile script.
I ended up committing versions of the fix to the master and 1.0
branches that differed only in whitespace in a few lines (no code
differences). This commit makes the whitespace identical so this
does not keep annoying me when I look at 'git diff 1.0 master'.
Reproducer: run vi in a subshell:
$ (vi)
vi opens; now press Ctrl+Z to suspend. The output is as expected:
[2] + Stopped (vi)
…but the exit status is 18 (SIGTSTP's signal number) instead of 0.
Now do:
$ fg
(vi)
$
The exit status is 18 again, vi is not resumed, and the job is
lost. You have to find vi's pid manually using ps and kill it.
Forking all non-command substitution subshells invoked from the
interactive main shell is the only reliable and effective fix I've
found. I've tried to fork the subshell conditionally in every other
remotely plausible place I can think of in fault.c and xec.c, but I
can't get anything to work properly. If anyone can get this to work
without forking as much (or at all), please do submit a patch or PR
that supersedes this fix.
At least subshells of subshells don't need to fork, so the
performance impact can be limited. Plus, it's not as if most people
need maximum speed on the interactive command line. Scripts
(including login/profile scripts) are not affected at all.
Command substitutions can be handled differently. My testing shows
that all shells except ksh93 simply block SIGTSTP (the ^Z signal)
while they run. We should do the same, so they don't need to fork.
NOTE for any backporters: the subshell.c and fault.c changes depend
on commits 35b02626 and 48ba6964 to work correctly.
src/cmd/ksh93/sh/subshell.c: sh_subshell():
- If the interactive shell state bit is on, then before executing
the subshell's code:
- for command substitutions, block SIGTSTP;
- for other subshells, fork.
- For command substitutions, release SIGTSTP if the interactive
shell state bit was on upon invoking the subshell.
src/cmd/ksh93/sh/fault.c:
- Instead of checking for a virtual subshell, check the shell's
interactive state bit to decide whether to handle SIGTSTP, as
that is only turned on in the interactive main shell.
src/cmd/ksh93/sh/main.c: sh_main():
- To avoid bugs, ignore SIGTSTP while running profile scripts.
Blocking it doesn't work because delaying it until after
sigrelease() will cause a crash. Thanks to @JohnoKing for this.
- While we're here, prevent a possible overflow of the 'beenhere'
static char variable by only incrementing it once.
Co-authored-by: Johnothan King <johnothanking@protonmail.com>
Resolves: https://github.com/ksh93/ksh/issues/390
Reproducer:
$ (sleep 1& echo done)
done
$ (eval "echo hi"; sleep 1& echo done)
hi
[1] 30587
done
No job control output should be printed for a background process
invoked from a subshell, not even after 'eval'.
The cause: sh_parse() turns on the shell's interactive state bit
(sh_state(SH_INTERACTIVE)) if the interactive shell option is on.
This is incorrect. The parser should have no involvement with shell
interactivity in principle because that's not its domain.
Not only that, the parser may need to run in a subshell, e.g. when
executing traps or 'eval' commands (as above). By definition, a
subshell can never be interactive.
We already fixed many bugs related to job control and the shell's
interactive state. Even if these two lines previously papered over
some breakage, I can't find any now after simply removing them. If
any is found later, then it'll need to be fixed properly instead.
Related: https://github.com/ksh93/ksh/issues/390
- Redirect error output from the ulimit builtin (re: 3e58851f).
- Fix the test failure for 'cd -eP' on illumos by making a directory
symlink first, then removing the symlink after cd.
- Fix the test failure for 'getconf -l' on illumos by quoting
strings with the -q option.
- astconf.c: Only quote strings if the -q option was passed.
- Improve error messages from intermittently failing types.sh tests
If the compiler is called gcc but not cc, the 64-bit detection
didn't work and $HOSTTYPE (and the arch/ subdirectory) did not get
the -64 suffix.
bin/package, src/cmd/INIT/package.sh:
- Run checkcc() before attempting to compile the program. This will
set $cc to the path to gcc if there is no 'cc' command.
- trap: use 'rm -rf' to also delete .dSYM directories (macOS).
- checkcc(): Since we're here, find clang as well.
The goal is to get rid of all compiler/linker wrapper scripts as
they are overridden by passing CC/LD and it should be possible to
select your compiler or linker without breaking the build. The
probing and feature testing system should set the appropriate flags
and macros. This makes some progress towards that.
src/lib/libast/features/standards:
- Eliminate the shotgun approach to standards macros on popular
systems where the macros we we need to set are known and
documented. The following will enable standards compliance plus
all the available extensions:
- Set no macros at all for any BSD system (excluding macOS).
- Set _DARWIN_C_SOURCE on Darwin/macOS.
- Set everything and the kitchen sink for Solaris/illumos in
a way that enables backwards compatibility with older Solaris.
This is unofficial, but following the standards(5) manual
disables a lot of basic functionality that we depend on.
- Set _GNU_SOURCE for GNU (glibc).
- Remove the covered macros from the shotgun approach fallback.
- Add a new heuristic. _POSIX_PATH_MAX and _SC_PAGESIZE are among
the basic macros disabled when you pass recommended standards
macros, killing the build, so it's good to check if they compile.
src/cmd/INIT/ar.freebsd12.amd64,
src/cmd/INIT/ar.linux.i386-64:
- Removed. May cause build failures on some systems as not all 'ar'
implementations support the U option. Plus, I can think of no
good reason to disable deterministic mode (which always creates
identical output) on 'ar' implementations that support it. See:
https://groups.google.com/g/comp.unix.shell/c/LdOD1Ya0C9E/m/U6DhgHVICwAJ
src/cmd/INIT/cc.linux.*-icc,
Removed icc wrappers. These manually source /etc/profile.d/icc.sh
but I don't think that is the build system's job. Profile scripts
should be run at login time and export variables we inherit
through the environment.
src/cmd/INIT.cc.{freebsd,linux,openbsd}*:
- Removed. Should be entirely superfluous now that the standards
feature test sets the appropriate macros.
src/cmd/INIT.cc.sol11.*:
- Removed as the standards feature test now sets the approopriate
macros. Note the Solaris build system should now simply pass CC
as normal instead of passing CC_EXPLICIT.
Inserting the _common script instead of sourcing it caused all test
failures in shcomp runs to be reported with the number of lines in
_common added.
src/cmd/ksh93/shtests:
- Only incorporate the aliases from _common; dot/source the rest of
the code as normal. Replace the first few lines with the aliases
to avoid affecting $LINENO; they are comments anyway.
With a better understanding of the code 1.5 years later, the
special-casing for IFS introduced in that commit seems like a hack.
The problem was not that the IFS node always exists but that it is
always considered to have a 'get' discipline function. Variables
with a 'get' discipline are considered set. This makes sense for
all variables except IFS.
The nv_isnull() macro is used to check if a variable is set. It
calls nv_hasget() to determine if the variable has a 'get'
discipline. So a better fix is for nv_hasget() always to return
false for IFS.
src/cmd/ksh93/bltins/test.c, src/cmd/ksh93/sh/macro.c:
- Remove special-casing for IFS.
src/cmd/ksh93/sh/nvdisc.c: nv_hasget():
- Always return false for IFS, taking local scope into account.
src/cmd/INIT/package.sh, bin/package:
- Derive the command name from $0 instead of hardcoding it.
- Remove NPROC and related code to support parallel building. This
is not supported with mamake, is unlikely to be reintroduced any
time soon, and if it ever is it will need to be done in a
different way anwyay.
- Invoke 'sed' and 'tr' directly instead of via $SED and $TR
variables. We're not building our own dynamically linked 'sed'
and 'tr' in this distribution so LD_LIBRARY_PATH is irrelevant.
If we ever do again, there are better ways to make sure the OS
standard 'sed' and 'tr' are invoked than this kludge.
- Use note() consistently to print warnings to standard error.
note() is changed to print each argument on a new line prefixed
by the command name, so arguments need to be quoted now if they
are to be shown on a single line.
- Use a new err_out() function to error out, avoiding code
repetition.
Announcing: KornShell 93u+m 1.0.0-beta.2
https://github.com/ksh93/ksh
In May 2020, when every KornShell (ksh93) development project was
abandoned, development was rebooted in a new fork based on the last
stable AT&T version: ksh 93u+. This new fork is called ksh 93u+m as a
permanent nod to its origin. We're restarting it at version 1.0. Seven
months after the first beta, the second one is ready. Please test this
second beta and report any bugs you find, or help us fix known bugs.
We're now the default ksh93 in some OS distributions, at least Debian
and Slackware! Even though we don't think it's stable release quality
yet, the consensus seems to be that 93u+m is already much better than
the last AT&T release.
Main developers: Martijn Dekker, Johnothan King, hyenias
Contributors: Andy Fiddaman, Anuradha Weeraman, Chase, Gordon Woodhull,
Govind Kamat, Harald van Dijk, Lev Kujawski, Marc Wilson, Ryan Schmidt,
Sterling Jensen
HOW TO GET IT
Please download the source code tarball from our GitHub releases page:
https://github.com/ksh93/ksh/releases
To build, follow the instructions in README.md or src/cmd/ksh93/README.
HOW TO GET INVOLVED
To report a bug, please open an issue at our GitHub page (see above).
Alternatively, email me at martijn@inlv.org with your report.
To get involved in development, read the brief policy information in
README.md and then jump right in with a pull request or email a patch.
See the TODO file in the top-level directory for a to-do list.
*** MAIN CHANGES between 1.0.0-beta.1 and 1.0.0-beta.2 ***
New features in built-in commands:
- 'cd' now supports an -e option that, when combined with -P, verifies
that $PWD is correct after changing directories; this helps detect
access permission problems. See:
https://www.austingroupbugs.net/view.php?id=253
- 'printf' now supports a -v option as in bash. This assigns formatted
output directly to variables, which is very fast and will not strip
final newline (\n) characters.
- The 'return' command, when used to return from a function, can now
return any status value in the 32-bit signed integer range, like on
zsh. However, due to a traditional Unix kernel limitation, $? is
still trimmed to its least significant 8 bits whenever leaving a
(sub)shell environment.
- 'test'/'[' now supports all the same operators as [[ (including =~,
\<, \>) except for the different 'and'/'or' operators. Note that
'test'/'[' remains deprecated due to its unfixable pitfalls;
[[ ... ]] is recommended instead.
Shell language changes:
- Several improvements were made to the --noexec shell code linter.
- Arithmetic expressions in native ksh mode no longer interpret a
number with a leading zero as octal in any context. Use 8#octalnumber
instead (e.g. 8#400 == 256). Arithmetic expressions now also behave
identically within and outside ((...)) and $((...)).
- POSIX compatibility mode fixes (only applicable with the --posix shell
option on):
- A leading zero is now consistently recognised as introducing an octal
number in all arithmetic contexts.
- $((inf)) and $((nan)) are now interpreted as regular variables.
- The '.' built-in no longer runs ksh functions and now only runs
files.
Bugs fixed:
- '.' and '..' are now once again completed by tab completion.
- If SIGINT is set to ignore, the interactive shell no longer exits on
Ctrl+C.
- ksh now builds and runs on Apple's new M1 hardware.
- The 'return' and 'exit' commands no longer risk triggering actual
signals by returning or exiting with a status > 256.
- Ksh no longer behaves badly when parsing a type definition command
('typeset -T' or 'enum') without executing it or when executing it in
a subshell. Types can now safely be defined in subshells and defined
conditionally as in 'if condition; then enum ...; fi'.
- Discipline functions, especially those applied to PS2 or .sh.tilde,
will no longer crash your shell upon being interrupted or throwing an
error.
- Fixed a bug that could corrupt output if standard output is closed
upon initialising the shell.
- Fixed a bug in the [[ ... ]] compound command: the '!' logical
negation operator now correctly negates another '!', e.g.,
[[ ! ! 1 -eq 1 ]] now returns 0/true. Note that this has always been
the case for 'test'/'['.
- Fixed SHLVL so that replacing ksh by itself (exec ksh) will not
increase it.
- Arithmetic expressions are no longer allowed to assign out-of-range
values to variables of types declared with enum.
- The 'time' keyword no longer makes the --errexit shell option
ineffective.
- Various bugs in libcmd built-in commands (those bound to the
/opt/ast/bin path by default) have been fixed.
- Various other crashing bugs have been fixed.
Fixes for the shcomp byte code compiler:
- shcomp is now able to compile scripts that define types using enum.
- shcomp now refuses to mess up your terminal by writing bytecode
to it.
*** MAIN CHANGES between ksh 93u+ 2012-08-01 and 93u+m 1.0.0-beta.1 ***
Hundreds of bugs have been fixed, including many serious/critical bugs.
This includes upstreamed patches from OpenSUSE, Red Hat, and Solaris, fixes
backported from the abandoned 93v- beta and ksh2020 fork, as well as many
new fixes from the community. See the NEWS file for more information, and
the git commit log for complete documentation of every fix. Incompatible
changes have been minimised, but not at the expense of fixing bugs. For a
list of potentially incompatible changes, see src/cmd/ksh93/COMPATIBILITY.
Though there was a "no new features, bugfixes only" policy, some new
features were found necessary, either to fix serious design flaws or to
complete functionality that was evidently intended, but not finished.
Below is a summary of these new features.
New command line editor features:
- The forward-delete and End keys are now handled as expected in the
emacs and vi built-in line editors.
- In the vi and emacs line editors, repeat count parameters can now also
be used for the arrow keys and the forward-delete key. E.g., in emacs
mode, <ESC> 7 <left-arrow> will now move the cursor seven positions to
the left. In vi control mode, this would be entered as: 7 <left-arrow>.
New shell language features:
- The &>file redirection shorthand (for >file 2>&1) is now available for
all scripts and interactive sessions and not only for profile/login
scripts, bringing ksh 93u+m in line with mksh, bash, and zsh.
- File name generation (a.k.a. pathname expansion, a.k.a. globbing) now
never matches the special navigational names '.' (current directory)
and '..' (parent directory). This change makes a pattern like .*
useful; it now matches all hidden files (dotfiles) in the current
directory, without the harmful inclusion of '.' and '..'.
- Tilde expansion can now be extended or modified by defining a
.sh.tilde.get or .sh.tilde.set discipline function. This replaces a
2004 undocumented attempt to add this functionality via a .sh.tilde
command, which never worked and crashed the shell. See the manual for
details on the new method.
New features in built-in commands:
- Usage error messages now show the --help/--man self-documentation options.
- Path-bound built-ins (such as /opt/ast/bin/cat) can now be executed by
invoking the canonical path, so the following will now work as expected:
$ /opt/ast/bin/cat --version
version cat (AT&T Research) 2012-05-31
- 'command -x' now looks for external commands only, skipping built-ins.
In addition, its xargs-like functionality no longer freezes the shell on
Linux and macOS, making it effectively a new feature on these systems.
- 'redirect' now checks if all arguments are valid redirections before
performing them. If an error occurs, it issues an error message instead
of terminating the shell.
- 'suspend' now refuses to suspend a login shell, as there is probably no
parent shell to return to and the login session would freeze.
- 'times' now gives high precision output in a POSIX compliant format.
- 'typeset' now gives an informative error message if an incompatible
combination of options is given.
- 'whence -v/-a' now reports the location of autoloadable functions.
New features in shell options:
- A new --globcasedetect shell option is added on OSs where we can
check for a case-insensitive file system (currently Windows/Cygwin,
macOS, Linux and QNX 7.0+). When this option is turned on, file name
generation (globbing), as well as file name tab completion on
interactive shells, automatically become case-insensitive on file
systems where the difference between upper and lower case is ignored
for file names. This is transparently determined for each directory, so
a path pattern that spans multiple file systems can be part
case-sensitive and part case-insensitive.
- A new --nobackslashctrl shell option disables the special escaping
behaviour of the backslash character in the emacs and vi built-in
editors. Particularly in the emacs editor, this makes it much easier to
go backward, insert a forgotten backslash into a command, and then
continue editing without having your next cursor key replace your
backslash with garbage. Note that Ctrl+V (or whatever other character
was set using 'stty lnext') always escapes all control characters in
either editing mode.
- A new --posix shell option has been added to ksh 93u+m that makes the
ksh language more compatible with other shells by following the POSIX
standard more closely. See the manual page for details. It is enabled by
default if ksh is invoked as sh, otherwise it is disabled by default.
- Enhancement to -G/--globstar: symbolic links to directories are now
followed if they match a normal (non-**) glob pattern. For example, if
'/lnk' is a symlink to a directory, '/lnk/**' and '/l?k/**' now work as
you would expect.
Strings compared in [[ with the > and < operators should be compared
lexically. This does not work when the strings are single digits, as
the parser interprets it as a syntax error:
$ [[ 10<2 ]] # 10 lexically sorts before 2
$ echo $?
0
$ [[ 1<2 ]]
/usr/bin/ksh: syntax error: `<' unexpected
$ echo $?
3
src/cmd/ksh93/sh/lex.c:
- Don't interpret numbers next to > and < as a redirection while
inside of [[. This bugfix was backported from ksh93v- 2014-06-25.
src/cmd/ksh93/tests/bracket.sh:
- Add regression tests for the > and < operators.
New version. I'm pretty sure the problems that forced me to revert
it earlier are fixed.
This commit mitigates the effects of the hack explained in the
referenced commit so that dummy built-in command nodes added by the
parser for declaration/assignment purposes do not leak out into the
execution level, except in a relatively harmless corner case.
Something like
if false; then
typeset -T Foo_t=(integer -i bar)
fi
will no longer leave a broken dummy Foo_t declaration command. The
same applies to declaration commands created with enum.
The corner case remaining is:
$ ksh -c 'false && enum E_t=(a b c); E_t -a x=(b b a c)'
ksh: E_t: not found
Since the 'enum' command is not executed, this should have thrown
a syntax error on the 'E_t -a' declaration:
ksh: syntax error at line 1: `(' unexpected
This is because the -c script is parsed entirely before being
executed, so E_t is recognised as a declaration built-in at parse
time. However, the 'not found' error shows that it was successfully
eliminated at execution time, so the inconsistent state will no
longer persist.
This fix now allows another fix to be effective as well: since
built-ins do not know about virtual subshells, fork a virtual
subshell into a real subshell before adding any built-ins.
src/cmd/ksh93/sh/parse.c:
- Add a pair of functions, dcl_hactivate() and dcl_dehacktivate(),
that (de)activate an internal declaration built-ins tree into
which check_typedef() can pre-add dummy type declaration command
nodes. A viewpath from the main built-ins tree to this internal
tree is added, unifying the two for search purposes and causing
new nodes to be added to the internal tree. When parsing is done,
we close that viewpath. This hides those pre-added nodes at
execution time. Since the parser is sometimes called recursively
(e.g. for command substitutions), keep track of this and only
activate and deactivate at the first level.
(Fixed compared to previous version of this commit: calling
dcl_dehacktivate() when the recursion level is already zero is
now a harmless no-op. Since this only occurs in error handling
conditions, who cares.)
- We also need to catch errors. This is done by setting libast's
error_info.exit variable to a dcl_exit() function that tidies up
and then passes control to the original (usually sh_exit()).
(Fixed compared to previous version of this commit: dcl_exit()
immediately deactivates the hack, no matter the recursion level,
and restores the regular sh_exit(). This is the right thing to
do when we're in the process of erroring out.)
- sh_cmd(): This is the most central function in the parser. You'd
think it was sh_parse(), but $(modern)-form command substitutions
use sh_dolparen() instead. Both call sh_cmd(). So let's simply
add a dcl_hacktivate() call at the beginning and a
dcl_deactivate() call at the end.
- assign(): This function calls path_search(), which among many
other things executes an FPATH search, which may execute
arbitrary code at parse time (!!!). So, regardless of recursion
level, forcibly dehacktivate() to avoid those ugly parser side
effects returning in that context.
src/cmd/ksh93/bltins/enum.c: b_enum():
- Fork a virtual subshell before adding a built-in.
src/cmd/ksh93/sh/xec.c: sh_exec():
- Fork a virtual subshell when detecting typeset's -T option.
Improves fix to https://github.com/ksh93/ksh/issues/256
The killpg(getpgrp(),SIGINT) call added to ed_getchar() in that
commit caused the interactive shell to exit on ^C even if SIGINT is
being ignored. We cannot revert or remove that call without
breaking job control. This commit applies a new fix instead.
Reproducers fixed by this commit:
SIGINT ignored by child:
$ PS1='childshell$ ' ksh
childshell$ trap '' INT
childshell$ (press Ctrl+C)
$
SIGINT ignored by parent:
$ (trap '' INT; ENV=/./dev/null PS1='childshell$ ' ksh)
childshell$ (press Ctrl+C)
$
SIGINT ignored by parent, trapped in child:
$ (trap '' INT; ENV=/./dev/null PS1='childshell$ ' ksh)
childshell$ trap 'echo test' INT
childshell$ (press Ctrl+C)
$
I've experimentally determined that, under these conditions, the
SFIO stream error state is set to 256 == 0400 == SH_EXITSIG.
src/cmd/ksh93/sh/main.c: exfile():
- On EOF or error, do not return (exiting the shell) if the shell
state is interactive and if sferror(iop)==SH_EXITSIG.
- Refactor that block a little to make the new check fit in nicely.
src/cmd/ksh93/tests/pty.sh:
- Test the above three reproducers.
Fixes: https://github.com/ksh93/ksh/issues/343
Note that this is only about the /opt/ast/bin built-in commands,
not about the regular pathless builtins such as printf.
To use these, either add /opt/ast/bin to your $PATH or use a
command like 'builtin cp'. As usual, --man provides info.
Removed as defaults for lack of convincing advantages over the OS's
external commands:
- chmod, cmp, head, logname, mkdir, sync, uname, wc
Remain as useful defaults:
- basename, cat, cut, dirname. These are commonly used in
performance-sensitive code paths in scripts and having them as
built-ins can be good for performance.
- getconf: This is the only interface to some libast internals that
is available to ksh. It's also has better functionality than most
OS-shipped 'getconf' commands, e.g., it can list and query all
the configuration values.
Added as defaults:
- cp, ln, mv: Having these built in can speed up scripts that
manage files. Also the AST versions have extended functionality
(see cp --man, etc.).
- mktemp: External mktemp commands vary too widely and are
incompatible, but it's important that scripts can securely make
temporary files, so it's good to ship a known interface to this
functionality.
As a result, the statically linked ksh binary is very slightly
smaller than before.
Resolves: https://github.com/ksh93/ksh/issues/349
The pty tests tests the interactive shell. Therefore, running them
through the script compiler is a waste of time.
Not only that, it is reported that the pty tests intermittently
fail with shcomp on some systems. This is not worth trying to fix.
src/cmd/ksh93/tests/shtests:
- Only run pty.sh with shcomp if -c/--compile was explicitly
specified.
- Document the change.
Welcome to AT&T engineering practices in action: a fix in one thing
breaks a completely unreleated thing, but only in very specific
and inscrutable circumstances.
Commit ffe84ee7 introduced a regression test failure in types.sh:
test types begins at 2021-12-14+23:57:35
types.sh[130]: z.r.s should be z.r.x
test types failed at 2021-12-14+23:57:35 with exit code 1 [ 86 tests 1 error ]
test types(C.UTF-8) begins at 2021-12-14+23:57:35
test types(C.UTF-8) passed at 2021-12-14+23:57:35 [ 86 tests 0 errors ]
test types(shcomp) begins at 2021-12-14+23:57:35
test types(shcomp) passed at 2021-12-14+23:57:35 [ 86 tests 0 errors ]
Only enough, I've *only* found this regression on the GitHub CI
runner. I've tried it on three different regular Linux systems and
it occurs on none of them, nor on macOS.
Another odd thing: it only fails on the first of those three test
runs. But my experiments show it fails very consistently.
Through a process of systematic elimination in a test branch, I've
found that the failure is triggered by the change to using a
separate stack in the regex code. All the other changes are fine.
Using a separate stack improves the robustness of the regex code,
but it apparently exposes some breakage in how the very dodgy
'typeset -T' code is handling the stack, which was being masked by
sharing a stack with it. Or at least that seems like the most
plausible explanation to me right now.
So, until that breakage can be traced and fixed, the regex code now
shares the main stack with everything else again for the time being.
_____
Just to record this: by adding a couple of debug lines:
typeset -p z | sed 's/^/[DEBUG] /'
printf '[DEBUG] %s\n' "${z.r.s}" "${z.r.x}"
the symptom reveals itself more clearly on the GitHub runner:
test types begins at 2021-12-15+17:25:57
[DEBUG] Y_t z=(X_t r=(x=foo;y=bam;s=''))
[DEBUG]
[DEBUG] foo
types.sh[132]: z.r.s should be z.r.x
test types failed at 2021-12-15+17:25:57 with exit code 1 [ 86 tests 1 error ]
test types(C.UTF-8) begins at 2021-12-15+17:25:57
[DEBUG] Y_t z=(X_t r=(x=foo;y=bam;s=foo))
[DEBUG] foo
[DEBUG] foo
test types(C.UTF-8) passed at 2021-12-15+17:25:57 [ 86 tests 0 errors ]
test types(shcomp) begins at 2021-12-15+17:25:57
[DEBUG] Y_t z=(X_t r=(x=foo;y=bam;s=foo))
[DEBUG] foo
[DEBUG] foo
test types(shcomp) passed at 2021-12-15+17:25:57 [ 86 tests 0 errors ]
Yes, we're finally abandoning the old Bourne shell so we can
use sane $(command substitutions) and the like. POSIX sh (with
tolerance for shell bugs) is very highly portable these days.
Even Solaris 10 has a POSIX shell, though not as /bin/sh.
bin/package, src/cmd/INIT/package.sh:
- Be nice: if we're on an obsolete or broken shell, try hard to
escape to a good one. This should preserve the possibility to
just run 'bin/package make' via ancient /bin/sh on Solaris 10.
- Note: zsh without sh emulation is considered broken because
$path, which we use, is linked to $PATH. You have to run it via
a symlink named sh or via 'zsh --emulate sh' to disable this.
Enabling emulation mode after initialisation will not work.
- More self-documentation cleanups and updates.
- Regenerate the text-only fallback version of the self-doc.
- Remove flat view functionality (no arch directory); it may have
been broken for some time, but quite frankly I could not care
less. It's yet more featuritis. Building in arch/ is fine.
There are two main changes:
1. The regex code now creates and uses its own stack (env->mst)
instead of using the shared standard stack (stkstd). That seems
likely to be a good thing.
2. Missing mbinit() calls were inserted. The 93v- code uses a
completely different multibyte characters API, so these needed
to be translated back to the older API. But, as mbinit() is no
longer a no-op as of 300cd199, these calls do stop things from
breaking if a previous operation is interrupted mid-character.
I think there might be a couple of off-by-one errors fixed as well,
as there are two instances of this change:
- while ((index += skip[buf[index]]) < mid);
+ while (index < mid)
+ index += skip[buf[index]];
In the referenced commit message I neglected to mention that, when
doing bin/package make, we're now running 'tee' in the background
again and the building job in the foreground, as opposed to the
other way around. Foreground jobs are more reliably interruptable.
But that reintroduced the problem fixed in 5b8d29d3. Now I don't
know what I was thinking then -- the obvious fix is to add a 'wait'
command, allowing 'tee' to catch up before returning to the prompt.
This reduces the bin/package script by more than half!
bin/package, src/cmd/INIT/package.sh:
- Remove obsolete and unused package commands: admin, contents,
license, list, remote, regress, setup, update, verify, write.
- Remove associated documentation.
- Replace install command with a dummy. It'll come back when we
reintroduce the building of dynamic libaries.
- Update the test command to run the regression tests properly
and capture the output in arch/*/lib/package/gen/tests.out, as
documented. Arguments are simply passed to bin/shtests.
src/cmd/INIT/{ditto.sh,hurl.sh,release.c}:
- Removed. These were support scripts for some of the removed
package commands.
src/cmd/ksh93/tests/pty.sh:
- Avoid failure when capturing output via 'bin/package test' by
redirecting standard error to /dev/tty when running the tests.
This bug was first reported at <https://www.illumos.org/issues/3782>.
The chown builtin when used on illumos can fail with different error
messages after running the same command twice:
$ touch /tmp/x
$ /opt/ast/bin/chown -h 433:434 /tmp/px
chown: /tmp/x: cannot change owner and group [Not owner]
$ /opt/ast/bin/chown -h 433:434 /tmp/px
chown: /tmp/x: cannot change owner and group [Invalid argument]
The error messages differ because the libast struid and strgid
functions will return -2 if the same nonexistent ID is used twice.
The fix for this bug has been ported from here:
https://github.com/illumos/illumos-gate/commit/4162633a7c5961f388fd
src/lib/libcmd/chgrp.c:
- Remove NOID macro and check for a < 0 error status instead.
This is different from the Illumos fix at
<https://github.com/illumos/illumos-gate/commit/4162633a7c59>
which added another macro.
src/lib/libast/man/{strgid,struid}.3:
- Correct errors in the strgid and struid documentation.
- Document that the strgid and struid functions will return -2 if
the same invalid name is used twice.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
The standards macros consistency fix for iffe exposed breakage on
illumos: the standards flags aren't set properly. Back in 580ff616,
I set _XPG6 from features/common, which is the wrong place; the
correct place is features/standards -- especially now that iffe
uses its results.
In addition, to get header declarations that aren't somehow in
conflict with themselves on illumos, don't result in "implicit
function declaration" warnings, and expose all the functionality,
we need to define *all* the _XPG[4-7] macros *and* __EXTENSIONS__
*and* _XOPEN_SOURCE. Welp. Thankfully, that's just fine with
Solaris, too.
Thanks to @JohnoKing for the heads-up.
Running 'bin/package environment' should only show what will happen
during the build process, but in it's current state the feature has
some bugs:
1. Errors can occur relating to the failed creation of files and a
failed attempt to change the directory. Various errors from
'bin/package environment make' and 'bin/package environment make
CC=tcc':
bin/package[5632]: cd: /home/johno/GitRepos/KornShell/ksh/arch/linux.i386-64/lib/package/gen: [No such file or directory]
bin/package: line 5869: /home/johno/GitRepos/KornShell/ksh/arch/linux.i386-64/lib/package/gen/CC: cannot create [No such file or directory]
bin/package: line 5869: /home/johno/GitRepos/KornShell/ksh/arch/linux.i386-64/lib/package/gen/CCFLAGS: cannot create [No such file or directory]
bin/package: line 5869: /home/johno/GitRepos/KornShell/ksh/arch/linux.i386-64/lib/package/gen/CCLDFLAGS: cannot create [No such file or directory]
bin/package: line 5869: /home/johno/GitRepos/KornShell/ksh/arch/linux.i386-64/lib/package/gen/LDFLAGS: cannot create [No such file or directory]
bin/package: line 5869: /home/johno/GitRepos/KornShell/ksh/arch/linux.i386-64/lib/package/gen/KSH_RELFLAGS: cannot create [No such file or directory]
bin/package[5888]: /home/johno/GitRepos/KornShell/ksh/arch/linux.i386-64/lib/package/gen/host: cannot create [No such file or directory]
2. The package script may in some scenarios create a temporary file
at the root of the repository, such as 'pkg77213.c'.
bin/package, src/cmd/INIT/package.sh:
- Avoid creating files or changing the directory while the
environment qualifier is on (this also affects the debug
qualifier). Part of this fix is based on a patch from Marcin
Cieślak[*], with other fixes applied for similar problems the
environment qualifier had.
I'm now taking another small step towards extricating this build
system from the long-dead AT&T AST universe.
This commit modifies/reduces the tool called proto. AT&T used proto
for two purposes:
1. To convert ANSI C code to a form compatible with ancient
(pre-ANSI) K&R C compilers using extremely complex macro
voodo. It was similarly capable of translating to C++.
Theoretically, this entire code base should compile on
anything from a 1980s K&R C compiler to a modern C++ compiler.
In practice, given the massive amount of bit rot we inherited,
I am 99.9% sure that this has been broken for many years.
2. To automagically insert license comments into source files
based on an extremely complicated license database system.
(In all-too-typical AT&T fashion, this second function of
proto is completely unrelated to the first.)
Function 2 has now been removed because, unlike the AT&T legal
department, I don't think it's worth going to unspeakably extreme
lengths to avoid maintaining license information in source code
files by hand.
In the process, proto.c was cleaned up to look halfway like actual
C code, but it's still processed code: most macros have been
expanded to their numeric value, all comments were stripped, etc.
So don't expect to understand this code. The actual source code is
in these two directories in the ast-open-history repo:
https://github.com/ksh93/ast-open-history/tree/master/src/cmd/protohttps://github.com/ksh93/ast-open-history/tree/master/src/lib/libpp
Meanwhile, nobody wants to compile ksh with a pre-ANSI K&R C
compiler in 2021 -- and there's no good reason to be compatible
with C++ because standard C compilers are universally available.
So, proto will go away when I manage to figure out how to pry it
loose from the innards of this build system.
src/lib/libast/port/astlicense.c:
- Removed. This is al the license handling code that was
incorporated in proto.c in stripped form. It was not used
anywhere else, and the environment where it was useful is gone.
src/cmd/INIT/proto.c:
- Cleanup to make this halfway maintainable: indentation, huge
blocks of empty lines, #line directives, etc.
- Delete all the code corresponding to astlicense.c. This was
actually easy as it was in a discrete block.
- proto(), pppopen(): Remove 'license'/'notice' and 'options'
arguments.
- main(): Remove processing of -l (license) and -o (license
options) flags.
**/Mamfile:
- Update all the proto invocations to remove the -l and -o flags.
bin/package, src/cmd/INIT/package.sh:
- Delete the 'copyright' command, which used the -l and -o
options to tell proto to extract copyright information from
*.lic/*.def files in lib/package.
COPYRIGHT:
- Added. This has the information from 'bin/package copyright', with
the copyright years corrected to plausible values as the AST code
used the current year (2021) for all of them. It adds ksh 93u+m
copyright and contributor information at the top as well.
(Yes, some of the lines in the old non-AT&T copyright notices
are clipped. This is the actual output of the 'bin/package
copyright' command as generated by 'proto' in the AST
distribution. For all that extreme complexity, they couldn't even
reproduce the notices correctly. But it's officially sanctioned
by AT&T in exactly this form, so there you have it.)
lib/package/**:
- Removed. All these files are now obsolete and redundant.
This commit fixes an issue with how ksh was obtaining the value of
NGROUPS_MAX. On some systems this setting can be changed (e.g., on
illumos adding 'set ngroups_max=32' to /etc/system then rebooting
changes NGROUPS_MAX from 16 to 32). Ksh was using NGROUPS_MAX with
the assumption it's a static value, which could cause issues on
systems where it isn't static. This bugfix is inspired by the one
from <https://github.com/lkujaw/ast/commit/b1362c3a5>, although it
has been expanded a bit to account for OPEN_MAX as well.
src/cmd/ksh93/sh/init.c, src/lib/libcmd/fds.c:
- Rename the getconf() macro to astconf_long() and move it to ast.h
to prevent redundancy. Other sections of the code have been
modified to use this macro for astconf() to account for
dynamic settings.
- An equivalent macro for unsigned long values (astconf_ulong) has
been added.
- Prefer sysconf(3) where available. It has better performance as it
returns a numeric value directly instead of via string
conversion.
- The astconf_long and astconf_ulong macros have been documented in
the ast(3) man page.
Turns out there is a bona fide, honest-to-goodness use case for
matching '.' and '..' in globbing after all. It's when globbing is
used as the backend mechanism for file name completion in
interactive shell editors. A tab invisibly adds a * at the end of
the word to the left of your cursor and the resulting pattern is
expanded. In 5312a59d, this broke for '.' and '..'.
Typing '.' followed by two tabs should result in a menu that
includes './' and '../'. Typing '..' followed by a tab should
result in '../', (or a menu that includes it if there are files
with names starting with '..'). This is the behaviour in 93u+ and
we should maintain this.
To restore this functionality without reintroducing the harmful
behaviour fixed in the referenced commits, we should special-case
this, allowing '.' and '..' to match only for file name completion.
src/lib/libast/include/glob.h:
- Fix an inaccurate comment: the GLOB_COMPLETE flag is used for
command completion, not file name completion. This is very clear
from reading the path_expand() function in sh/expand.c.
- Add new GLOB_FCOMPLETE flag for file name completion.
src/lib/libast/misc/glob.c:
- Adapt flags mask to fit the new flag.
- glob_dir(): If GLOB_FCOMPLETE is passed, allow '.' and '..' to
match even if expanded from a pattern.
- Clarify the fix from aad74597 with an extended comment based on
<https://github.com/ksh93/ksh/issues/146#issuecomment-790991990>.
src/cmd/ksh93/sh/expand.c: path_expand():
- If we're in the SH_FCOMPLETE (file name completion) state, then
pass the new GLOB_FCOMPLETE flag to AST glob(3).
Fixes: https://github.com/ksh93/ksh/issues/372
Thanks to @fbrau for the bug report.
This commit adds onto <https://github.com/ksh93/ksh/pull/353> by porting
over two additional improvements to the shell linter:
1) The changes in the aforementioned pull request were merged into
illumos-gate with an additional change.[*] The illumos revision of
the patch improved the warning for (( $foo = $? )) to specify '$foo'
causes the warning.[**] Example:
$ ksh -n -c '(( $? != $bar ))'
ksh: warning: line 1: in '(( $? != $bar ))', using '$' as in '$bar' is slower and can introduce rounding errors
While I was porting the illumos patch I did notice one problem. The
string it uses from paramsub() skips over the initial '{' in
'${var}', resulting in the warning printing '$var}' instead:
$ ksh -n -c '(( ${.sh.pid} != $$ ))'
... in '(( ${.sh.pid} != $$ ))', using '$' as in '$.sh.pid}' is slower ...
This was fixed by including the missing '{' in the string returned by
paramsub for ${var} variables.
2) In ksh93v-, parsing x=$((expr)) with the shell linter will cause ksh
to warn the user x=$((expr)) is slower than ((x=expr)). This
improvement has been backported with a modified warning:
# Result from this commit
$ ksh -n -c 'x=$((1 + 2))'
ksh: warning: line 1: x=$((1 + 2)) is slower than ((x=1 + 2))
# Result from ksh93v-
$ ksh93v -n -c 'x=$((1 + 2))'
ksh93v: warning: line 1: ((x=1 + 2)) is more efficient than x=$((1 + 2))
Minor note: the ksh93v- patch had an invalid use of memcmp; this
version of the patch uses strncmp instead.
References:
be548e87bchttps://code.illumos.org/c/illumos-gate/+/1834/comment/65722363_22fdf8e7/
The "fd is first arg to poll()" and "fd is second arg to poll()" tests
use write() but don't include the system header in which that function
is declared, leading to "error: implicit declaration of function 'write'
is invalid in C99'" when trying to compile the test. By including the
header, the test can now compile and run as intended.
The "mmap is worth using" test uses memcpy but doesn't include the
system header in which that function is declared, leading to
"error: implicitly declaring library function 'memcpy'" when trying
to compile the test. By including the header, the test can now
compile and run as intended.
On macOS, the result is still negative. The test seems to time
using mmap and another method and only picks mmap if it's faster.
Editing the test to print out timing information on a few runs, I
see that mmap is 2–3✕ slower than the other method:
$ for i in $(seq 1 5); do ./mmap_worth_using; done
/* mmtm=11 rdtm=3 */
/* 4*mmtm=44 3*rdtm=9 */
/* 4*mmtm=44 5*rdtm=15 */
/* mmtm=9 rdtm=5 */
/* 4*mmtm=36 3*rdtm=15 */
/* 4*mmtm=36 5*rdtm=25 */
/* mmtm=10 rdtm=4 */
/* 4*mmtm=40 3*rdtm=12 */
/* 4*mmtm=40 5*rdtm=20 */
/* mmtm=12 rdtm=4 */
/* 4*mmtm=48 3*rdtm=12 */
/* 4*mmtm=48 5*rdtm=20 */
/* mmtm=12 rdtm=4 */
/* 4*mmtm=48 3*rdtm=12 */
/* 4*mmtm=48 5*rdtm=20 */
If you passed CC=/some/compiler, the build broke on macOS because the
cc.darwin compiler wrapper wasn't used. Among other things, this
wrapper adds a -D_lib_memccpy flag, defining _lib_memccpy as 1
during the build. That was used to override a false negative result
of the lib_memccpy feature test. This commit fixes that feature
test instead, so it correctly returns positive on macOS.
Thanks to Ryan Smith (@ryandesign) for the bug report and for the
fix to the lib_memccpy test.
src/lib/libast/features/lib:
- Fix the lib_memccpy feature test. It was checking the result of
mmap(2) incorrectly, resulting in the test crashing on macOS.
Failure does not return NULL, it returns MAP_FAILED which is
usually -1.
src/cmd/INIT/cc.darwin*:
- Removed. Any other flags in these wrappers are either related to
building dynamic libraries, which is not currently supported, or
were determined to be unnecessary. See the GitHub issue for
discussion. This now makes it possible to pass `CC` to use any
compiler you like on the Mac. Notes:
- Apple's -D_ast_int8_t=int64_t is a no-op; another AST feature
test already defines _ast_int8_t a 64-bit integer type, even on
32-bit systems (on which it is defined as 'long long').
- The -search_paths_first linker flag is the default since 2010.
But even on my museum-grade Power Mac G5 with Mac OS X 10.3
(from 2004), it builds and runs just fine without.
- DCLK_TCK=100 is a no-op as even that ancient Mac system already
defines it as 100. Plus, it's not even actually used.
If a need is found for any of these, please report this in a new
issue so I can special-case it elsewhere in the code.
Resolves: https://github.com/ksh93/ksh/issues/373
*** Crash 1: ***
ksh crashed if the PS1 prompt contains one or more command
substitutions and you enter a multi-line command substitution
on the command line, then interrupt while on the PS2 prompt.
$ ENV=/./dev/null /usr/local/bin/ksh -o emacs
$ PS1='$(echo foo) $(echo bar) $(echo baz) ! % '
foo bar baz 16999 % echo $(
> true <-- here, press Ctrl+C instead of Return
Memory fault
The crash occurred due to a corrupted lexer state while trying to
display the PS1 prompt.
Analysis: My fix for the crashing bug with Ctrl+C in commit
3023d53b is incorrect and only worked accidentally. sh_fault() is
not the right place to reset the lexer state because, when we press
Ctrl+C on a PS2 prompt, ksh had been waiting for input to finish
lexing a multi-line command, so sh_lex() and other lexer functions
are on the function call stack and will be returned to.
src/cmd/ksh93/sh/fault.c: sh_fault():
- Remove incorrect SIGINT fix.
src/cmd/ksh93/sh/io.c: io_prompt():
- Reset the lexer state immediately before printing every PS1
prompt. Even in situations where this is redundant it should be
perfectly safe, the overhead is negligible, and it resolves this
crash. It may pre-empt other problems as well.
*** Crash 2: ***
If an INT trap is set, and you start entering a multi-line command
substitution, then press Ctrl+C on the PS2 prompt to trigger the
crash, the lexer state is corrupted because the lexer is invoked to
eval the trap action. A crash then occurs on entering the final ')'
of the command substitution.
$ trap 'echo TRAPPED' INT
$ echo $(
> trueTRAPPED <-- press Ctrl+C to output "TRAPPED"
> )
Memory fault
Technically, as SIGINT is trapped, it should not interrupt, so ksh
should execute the trap, then continue with the PS2 prompt to let
the user finish inputting the command. But I have been unsuccessful
in many different attempts to make this work properly. I managed to
get multi-line command substitutions to lex correctly by saving and
restoring the lexer state, but command substitutions were still
corrupted at the parser and/or execution level and I have not
managed to trace the cause of that.
My testing showed that all other shells interrupt the PS2 prompt
and return to PS1 when the user presses Ctrl+C, even if SIGINT is
trapped. I think that is a reasonable alternative, and it is
something I managed to make work.
src/cmd/ksh93/sh/fault.c: sh_chktrap():
- Immediately after invoking sh_trap() to run a trap action, check
if we're in a PS2 prompt (sh.nextprompt == 2). If so, assume the
lexer state is now overwritten. Closing the fcin stream with
fcclose() seems to reliably force the lexer to stop doing
anything else. Then we can just reset the prompt to PS1 and
invoke sh_exit() to start new command line, which will now reset
the lexer state as per above.
ksh crashed if you pressed Ctrl+C or Ctrl+D on a PS2 prompt while
you haven't finished entering a $(command substitution). It
corrupts subsequent command substitutions. Sometimes the situation
recovers, sometimes the shell crashes.
Simple crash reproducer:
$ PS1="\$(echo foo) \$(echo bar) \$(echo baz) > "
foo bar baz > echo $( <-- now press Ctrl+D
> ksh: syntax error: `(' unmatched
Memory fault
The same happens with Ctrl+C, minus the syntax error message.
The problem is that the lexer state becomes inconsistent when the
lexer is interrupted in the middle of reading a command
substitution of the form $( ... ). This is tracked in the
'lexd.dolparen' variable in the lexer state struct.
Resetting that variable is sufficient to fix this issue. However,
in this commit I prefer to just reinitialise the lexer state
completely to pre-empt any other possible issues. Whether there was
a syntax error or the user pressed Ctrl+C, we just interrupted all
lexing and parsing, so the lexer *should* restart from scratch.
src/cmd/ksh93/sh/fault.c: sh_fault():
- If the shell is in an interactive state (e.g. not a subshell) and
SIGINT was received, reinitialise the lexer state. This fixes the
crash with Ctrl+C.
src/cmd/ksh93/sh/lex.c: sh_syntax():
- When handling a syntax error, reset the lexer state. This fixes
the crash with Ctrl+D.
NEWS:
- Also add the forgotten item for the previous fix (re: 2322f939).
This reverts c0334e32, thereby restoring 936a1939.
After the fixes in 0a343244 and a2bc49be, the tilde expansion
disciplines work nicely, so they can come back to the 1.0 branch.
This should fix various crashes that remain, at least:
* when running a PS2 discipline at parse time
* when pressing Ctrl+C on a PS2 prompt
* when a special builtin within a discipline throws an error within
a virtual subshell
src/cmd/ksh93/sh/nvdisc.c:
- In both assign() which handles .set disciplines and lookup()
which handles .get disciplines, to stop errors in discipline
functions from wreaking havoc:
- Save, reinitialise and restore the lexer state in case the
discipline is run at parse time. This happens with PS2; I'm not
currently aware of other contexts but that doesn't mean there
aren't any or that there won't be any. Plus, I determined by
experimenting that doing this here seems to be the only way to
make it work reliably. Thankfully the overhead is low.
- Check the topfd redirection state and run sh_iorestore() if
needed. Without this, if a special builtin with a redirection
throws an error in a discipline function, its redirection(s)
remain permanent. For example, 'trap --bad-option 2>/dev/null'
in a PS2.get() discipline would kill standard error, including
all your prompts.
src/cmd/ksh93/sh/io.c: io_prompt():
- Before getting the value of the PS2 prompt, save the stack state
and restore it after. This stops a PS2.get discipline function
from corrupting a command substitution that the user is typing.
Doing this in assign()/lookup() is ineffective, so do it here.
Fixes: https://github.com/ksh93/ksh/issues/347
- tests/basic.sh: Fix a regression test that was failing under dtksh by
allowing the error message to name ksh 'lt-dtksh'. Additionally, fix
the test's inaccurate failure message (a version string is not what
the regression test expects).
- test/builtins.sh: Exclude the expr builtin from the unrecognized
options test because it's incompatible. Additionally, put the
unrecognized options test inside of a function to ensure that it works
with the future local builtin (https://github.com/ksh93/ksh/issues/123).
- tests/io.sh: The long seek test may fail to seek past the 2 GiB
boundary on 32-bit systems, so only allow it to run on 64-bit.
References:
3222ac2b59/f/ksh-1.0.0-beta.1-regre-tests.patcha5c692e1bd
- tests/substring.sh: Add a regression test for the ${.sh.match}
crashing bug fixed in commit 1bf1d2f8.
src/cmd/ksh93/sh/Mamfile:
- For edit/edit.c, add include/shlex.h (re: f99ce517).
- For sh/shcomp.c, add include/{path,terminal}.h (re: 141fa68e).
Thanks to @JohnoKing for flagging these up.
Once upon a time it might have been possible to build certain parts
of ksh, such as the emacs and vi editors and possibly even the
name/value library (nval(3)) as independent libraries. But given
the depressing amount of bit rot in the code that we inherited, I
am certain that disabling either of these macros had been resulting
in a broken build for many years before AT&T abandoned this code
base. These are certainly not going to be useful now.
Meanwhile the KSHELL macro got in the way of me today, because the
Mamfile did not define it for all the .c files, but some headers
declared some functionality conditionally upon that macro. So
including <io.h> in, e.g., nvdisc.c did not declare the same
functions as including that header in files with KSHELL defined.
This inconsistency is now gone as well, for various files.
I'm currently working on making it possible once again to build
libshell as a dynamic library; that should be good enough. And that
never involved disabling either of these macros.
The head and tail builtins don't correctly handle files that lack
newlines[*]:
$ print -n foo > /tmp/bar
$ /opt/ast/bin/head -1 /tmp/bar # No output
$ print -n 'foo\nbar' > /tmp/bar
$ /opt/ast/bin/tail -1 /tmp/bar
foo
bar$
This commit backports the required changes from ksh93v- to handle files
without a newline in the head and tail builtins. (Also note that the
required fix to sfmove was already backported in commit 1bd06207.)
src/lib/libcmd/{head,tail}.c:
- Backport the relevant ksh93v- code for handling files
without newlines.
src/cmd/ksh93/tests/builtins.sh:
- Add a few regression tests for using 'head -1' and 'tail -1' on a file
missing and ending newline.
[*]: https://www.illumos.org/issues/4149
List of changes:
- Fixed some -Wuninitialized warnings and removed some unused variables.
- Removed the unused extern for B_login (re: d8eba9d1).
- The libcmd builtins and the vmalloc memfatal function now handle
memory errors with 'ERROR_SYSTEM|ERROR_PANIC' for consistency with how
ksh itself handles out of memory errors.
- Added usage of UNREACHABLE() where it was missing from error handling.
- Extend many variables from short to int to prevent overflows (most
variables involve file descriptors).
- Backported a ksh2020 patch to fix unused value Coverity issues
(https://github.com/att/ast/pull/740).
- Note in src/cmd/ksh93/README that ksh compiles with Cygwin on
Windows 10 and Windows 11, albeit with many test failures.
- Add comments to detail some sections of code. Extensive list of
commits related to this change:
ca2443b5, 7e7f1372, 2db9953a, 7003aba4, 6f50ff64, b1a41311,
222515bf, a0dcdeea, 0aa9e03f, 61437b27, 352e68da, 88e8fa67,
bc8b36fa, 6e515f1d, 017d088c, 035a4cb3, 588a1ff7, 6d63b57d,
a2f13c19, 794d1c86, ab98ec65, 1026006d
- Removed a lot of dead ifdef code.
- edit/emacs.c: Hide an assignment to avoid a -Wunused warning. (See
also https://github.com/att/ast/pull/753, which removed the assignment
because ksh2020 removed the !SHOPT_MULTIBYTE code.)
- sh/nvdisc.c: The sh_newof macro cannot return a null pointer because
it will instead cause the shell to exit if memory cannot be allocated.
That makes the if statement here a no-op, so remove it.
- sh/xec.c: Fixed one unused variable warning in sh_funscope().
- sh/xec.c: Remove a fallthrough comment added in commit ed478ab7
because the TFORK code doesn't fall through (GCC also produces no
-Wimplicit-fallthrough warning here).
- data/builtins.c: The cd and pwd man pages state that these builtins
default to -P if PATH_RESOLVE is 'physical', which isn't accurate:
$ /opt/ast/bin/getconf PATH_RESOLVE
physical
$ mkdir /tmp/dir; ln -s /tmp/dir /tmp/sym
$ cd /tmp/sym
$ pwd
/tmp/sym
$ cd -P /tmp/sym
$ pwd
/tmp/dir
The behavior described by these man pages isn't specified in the ksh
man page or by POSIX, so to avoid changing these builtin's behavior
the inaccurate PATH_RESOLVE information has been removed.
- Mamfiles: Preserve multi-line errors by quoting the $x variable.
This fix was backported from 93v-.
(See also <https://github.com/lkujaw/ast/commit/a7e9cc82>.)
- sh/subshell.c: Remove set but not used sp->errcontext variable.
When a global EXIT trap is set, and a ksh-style function exits with
a status > 256 that could have been the result of a signal, then
the shell incorrectly issues that signal to itself. Depending on
the signal, this causes ksh to terminate itself ungracefully:
$ cat /tmp/exit267
trap 'echo OK' EXIT # This trap triggers the crash
function foo { return 267; }
foo
$ bash /tmp/exit267
OK
$ ksh-3aee10d7 /tmp/exit267
OK
$ ksh /tmp/exit267
Memory fault(coredump)
On most systems, status 267 corresponds to SIGSEGV. The reported
memory fault is not real; it results from ksh incorrectly killing
itself with that signal.
The problem is caused by two factors:
1. As of 93u+ 2012-08-01, ksh explicitly allows 'return' to use an
exit status corresponding to a signal (from 257 to end of signal
range). The rest of the integer range is trunctated to 8 bits.
This is contrary to both 'man ksh' and 'return --man' which both
say it's always truncated to 8 bits. Plus, combined with point 2
below, this new behaviour is nonsensical, as 'return' has no
business actually generating signals. However, a couple of
regression tests now depend on this, as may some scripts.
2. When a ksh-style function does not handle a signal, the signal
is passed down to the parent environment and ksh does this by
reissuing the signal to its own process after leaving the
function scope. However, it does this by checking the exit
status, which is very bad practice as there is no guarantee
that an exit status corresponding to a signal was in fact
produced by a signal, particularly after they changed the
behaviour of 'return' per 1 above.
This commit fixes both issues. It also takes a proper decision on
allowable 'return' exit status arguments. Since 93u+ was released
nearly a decade ago and some scripts may now rely on being able to
pass certain exit statuses out of the 8-bit range, we should not
disallow this now. But neither should we be half-hearted in
allowing only some arbitrary selection of 9-bit statuses; 'return'
values categorically should have nothing to do with signals, so
this is no basis for limiting them. We're now allowing the full
unsigned integer range, which is usually 32 bits. This is like zsh,
and may create some interesting possibilities for scripts.
Just don't forget that $? will still lose all but its 8 least
significant bits when leaving the current (sub)shell environment.
src/cmd/ksh93/sh/xec.c: sh_funscope():
- Fix passing down unhandled signals from interrupted ksh functions
(jumpval==SH_JMPFUN) to the parent environment. Do not pay any
attention to the exit status. Instead, use sh.lastsig (a.k.a.
shp->lastsig). It is set by sh_fault() in fault.c for just this
purpose and contains the last signal handled for the current
command. It is reset in sh_exec() before running any new command.
So if it contains a signal, that is the one that interrupted the
ksh function, so it's the correct one to pass down. (Further
evidence: sh_subshell() was already using this in the same way.)
src/cmd/ksh93/bltins/cflow.c: b_return():
- Allow any signed int return value when invoked as and behaving
like 'return'.
- Add warning if a passed value is out of int range. Set the exit
status to 128 in that case; int overflow is undefined behaviour
in C and we want consistent behaviour across platforms. It should
be safe enough to check if the long and int values are equal.
- Refactor for clarity.
src/cmd/ksh93/sh/subshell.c: sh_subshell():
- If a function returns with a status out of the 8 bit range in a
virtual subshell, this status could be passed down to the parent
shell in full. However, if the subshell forks, then the kernel
will enforce an 8-bit exit status. That is inconsistent. Scripts
should not be able to tell the difference between forked and
non-forked subshells, so artificially enforce that limit here.
Other changed files:
- Documentation updates and copy-edits.
- Update an AT&T functions.sh regress test to allow arbitrary
integer return values for functions.
- Add regression tests based in part on @JohnoKing's reproducers.
- Rework some vaguely related regression tests to fail gracefully.
Thanks to Johnothan King for the report and the testing.
Fixes: https://github.com/ksh93/ksh/issues/364
In iffe tests, some C functions are found in system libraries, but
then are not declared by the system headers on some systems because
the expected standards macros aren't defined, causing the system
headers to hide the function declarations. This may cause warnings
about invalid implicit function declarations in some tests (which
only show up if you export IFFEFLAGS=-d1), but may also cause false
negative test results. The iffe tests should be given the same
environment that their test results are going to be used in.
libast's first-run and most central feature test, 'standards',
figures out what standards macros need to be used on the current
system to get the system headers to declare the expected
functionality. All code that links to libast depends on the header
generated by this feature test. So iffe should use this result for
the tests as well, as soon as it's available (which is early in
libast's compilation cycle).
Concrete example: on Cygwin, in src/cmd/builtin/features/pty, the
'lib ptsname' test detects ptsname(3) in the system library, but
the output{...}end block that uses the _lib_ptsname feature test
result throws an 'implicit function definition' warning because
Cygwin's stdlib.h does not define this function without the
appropriate standards macros being defined first.
src/cmd/INIT/iffe.sh:
- If ${INSTALLROOT}/src/lib/libast/FEATURE/standards is available,
incorporate it directly into iffe's own block of compiler
massaging macros. Do not use #include as the necessary -I flags
are not added for every test.
- Centrally define the iffe version once, not twice.
- Update and tweak getopts self-documentation (iffe --man).
- While we're here, add macOS/Darwin's DYLD_LIBRARY_PATH to the
supported dynamic library search variables. On macOS, this is
normally disabled by System Integrity Protection, plus this
distribution uses static libraries at build time, so this is for
completeness' sake and not much else. This was ported from
@lkujaw's fork: https://github.com/lkujaw/ast/commit/48ff05442994
(thanks to @JohnoKing for pointing it out).
src/lib/libast/features/standards:
- Add a comment. This is to update the file's timestamp, ensuring
that everything will be recompiled after this commit.
Another day, another attempt to quash regress test fails caused by
implausibly small memory "leaks".
Apparently, on later macOS versions, 'ps' got more unreliable, so
increase tolerance from 8 to 12 bytes. Maximum reported leaks.sh
failure was 188 KiB after 16384 iterations (11 bytes/iteration).
And on some Linux versions, /proc sometimes acts weird. The
following is apparently consistent on Arch Linux:
shcomp-leaks.ksh[177]: memory leak with read -C when using
<<< (leaked approx 65536 bytes after 4096 iterations)
...which is 16 bytes per iteration, still not large enough to make
a real leak plausible.
Resolves: https://github.com/ksh93/ksh/issues/363
src/lib/libast/features/standards:
- Add heuristic (u_long availability) for systems that hide rather
than reveal functionality in the presence of _POSIX_SOURCE, etc.
- Define _DARWIN_C_SOURCE, like _GNU_SOURCE, to enable the full
range of definitions on macOS systems.
- Due to the above, remove MACH (macOS)-specific hack.
- These changes ported from https://github.com/att/ast/pull/1492 -
thanks to Lev Kujawski (@lkujaw). His PR indicates that this
fixes the standards macros on UnixWare, too. Therefore, no longer
exclude UnixWare from standards macros (re: ff70c27f).
src/lib/libast/comp/conf.sh:
- Promote the 'op' member in Conf_t (struct Conf_s) from short to
int. This allows some Darwin/macOS values, now exposed, to fit
that would otherwise be truncated, namely:
_CS_DARWIN_USER_CACHE_DIR 65538
_CS_DARWIN_USER_DIR 65536
_CS_DARWIN_USER_TEMP_DIR 65537
Thus, the following AST getconf values are now correct on macOS:
$ /opt/ast/bin/getconf | grep ^DARWIN
DARWIN_USER_CACHE_DIR=/var/folders/nx/(REDACTED)/C/
DARWIN_USER_DIR=/var/folders/nx/(REDACTED)/0/
DARWIN_USER_TEMP_DIR=/var/folders/nx/(REDACTED)/T/
src/lib/libast/features/tty:
- Include <sys/ioctl.h> if available. This silences a compiler
warning in src/lib/libast/misc/procopen.c about an invalid
implicit declaration of ioctl(2).
The information about an out of memory error does not apply
to the version of the PR that was eventually committed since
it does not use getcwd() which might cause such an error.
See https://github.com/ksh93/ksh/pull/358#issuecomment-986294934
This change adds the -e flag to the cd builtin, as specified in
<https://www.austingroupbugs.net/view.php?id=253>. The -e flag is
used to verify if the the current working directory after 'cd -P'
successfully changes the directory, and returns with exit status 1
if the cwd couldn't be determined. Additionally, it causes all
other errors to return with exit status >1 (i.e., status 2 unless
ENOMEM occurs) if -e and -P are both active.
src/cmd/ksh93/bltins/cd_pwd.c:
- Add -e option to the cd builtin command. It verifies $PWD by
using test_inode() to execute the equivalent of [[ . -ef $PWD ]].
- The check for restricted mode has been moved after optget to
allow 'cd -eP' to return with exit status 2 when in restricted
mode. To avoid changing the previous behavior of cd when -e isn't
passed, extra checks have been added to prevent cd from printing
usage information in restricted mode.
src/cmd/ksh93/tests/builtins.sh:
- Add regression tests for the exit status when using the cd -P
flag with and without -e.
src/cmd/ksh93/data/builtins.c,
src/cmd/ksh93/sh.1:
- Document the addition of -e to the cd builtin.
This commit ports over two of Andy Fiddaman's bugfixes to conf.sh
on illumos:
- The compiler isn't passed on to an invocation of iffe. The bugfix is
from this commit: <https://github.com/citrus-it/ast/commit/63563232>
- The getconf builtin is missing several parameters on illumos.
Reproducer:
$ /opt/ast/bin/getconf ADDRESS_WIDTH
getconf: Invalid argument (ADDRESS_WIDTH) # Should output '64'
This bug occurs because conf.sh expects GNU sed and fails to work
properly with other sed implementations. The bugfix and original bug
report can be found here:
https://www.illumos.org/issues/14044https://github.com/citrus-it/ast/commit/ba443cfd
Vmalloc is incompatible with Cygwin, but the code to disable it on
Cygwin did not work properly, somehow causing the build to freeze
at a seemingly unrelated point (i.e., when iffe feature tests
attempt to write to sfstdout).
Vmalloc has wasted my time for the last time, so now it's getting
disabled by default even on development builds and you'll have to
pass -D_AST_vmalloc in CCFLAGS to enable it for testing purposes.
This commit has a few other build tweaks as well.
src/lib/libast/features/vmalloc:
- tst map_malloc: Remove no-op #if __CYGWIN__ block which was in
the #else clause of another #if __CYGWIN__ block.
- Output block ('cat{'):
- Instead of disabling vmalloc for certain systems, disable it
unless _AST_vmalloc is defined.
- To disable it, set _AST_std_malloc as well as _std_malloc, just
to be sure.
src/lib/libast/vmalloc/malloc.c:
- Remove ineffective Cygwin special-casing.
src/lib/libcmd/vmstate.c:
- This is only useful for vmalloc, so do not pointlessly compile it
if vmalloc is disabled.
src/lib/libast/man/vmalloc.3:
- Add deprecation notice.
Resolves: https://github.com/ksh93/ksh/issues/360
This patch fixes the crashes experienced when a discipline function
exited because of a signal or an error from a special builtin. The
crashes were caused by ksh entering an inconsistent state after
performing a longjmp away from the assign() and lookup() functions in
nvdisc.c. Fixing the crash requires entering a new context, then setting
a nonlocal goto with sigsetjmp(3). Any longjmps that happen while
running the discipline function will go back to assign/lookup, allowing
ksh to do a proper cleanup afterwards.
Resolves: https://github.com/ksh93/ksh/issues/346
If a bug is ever introduced that causes a [[ ... ]] operator to be
unhandled by the linter, we should at least avoid writing random
memory contents to standard error. In non-release builds, let's
abort() so the problem can be more easily backtraced.
This commit ports over two improvements to the shell linter from
illumos (original patch written by Andy Fiddaman). Links to the
relevant bug reports and the original patch:
https://www.illumos.org/issues/13601https://www.illumos.org/issues/13631c7b656fc71
The first improvement is to the lint warning for arithmetic
operators in [[ ... ]]. The ksh linter now suggests the correct
equivalent operator to use in ((...)). Example:
$ ksh -nc '[[ 30 -gt 25 ]]'
# Original warning
warning: line 1: -gt within [[ ... ]] obsolete, use ((...))
# New warning
warning: line 1: [[ ... -gt ... ]] obsolete, use ((... > ...))
The second improvement pertains to variable expansion in arithmetic
expressions. The ksh linter now suggests referencing variable names
directly:
$ ksh -nc 'integer foo=40; (($foo < 50 ))'
# Old warning
warning: line 1: variable expansion makes arithmetic evaluation less efficient
# New warning
warning: line 1: in '(($foo < 50))', using '$' is slower and can introduce rounding errors
src/cmd/ksh93/{data/lexstates,sh/lex,sh/parse}.c:
- Port the improved shell lint warnings from illumos to ksh93u+m.
- The original checks for arithmetic operators involved a bunch of
if statements with inefficient calls to strcmp(3). These were
replaced with a more efficient switch statement that avoids
strcmp.
This change fixes a crash that can occur after setting a KEYBD trap
then inputting a multi-line command substitution. The crash is
similar to issue #347, but it's easier to reproduce since it
doesn't require you to setup a kshrc file. Reproducer for the
crash:
$ ENV=/./dev/null ksh
$ trap : KEYBD
$ : $(
> true)
Memory fault(coredump)
The bugfix was backported (with considerable changes) from ksh93v-
2013-10-08. The crash was first reported on the old mailing list:
https://www.mail-archive.com/ast-users@lists.research.att.com/msg00313.html
src/cmd/ksh93/{include/shlex.h,sh/lex.c}:
- To fix this properly, we need sizeof(Lex_t) to work as expected
in edit.c, but that is thwarted by the _SHLEX_PRIVATE macro in
lex.c which shlex.h uses to add private structs to the Lex_t type
in lex.c only. So get rid of that _SHLEX_PRIVATE macro and make
those members part of the centrally defined struct, renaming them
to make it clear they're considered private to lex.c.
src/cmd/ksh93/edit/edit.c:
- Now that we can get its size, save and restore the shell lexing
context when a KEYBD trap is present.
src/cmd/ksh93/tests/pty.sh:
- Add a regression test for the KEYBD trap crash.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
Ksh segfaults on M1 Macs, apparently because Apple's compiler
optimizer can't cope with overriding bzero(3) with libast's
implementation (though it's nothing more than "memset(b, 0, n);").
Apple has disabled libast's bzero function since 2018-12-04:
https://opensource.apple.com/source/ksh/ksh-27/patches/src__lib__libast__comp__omitted.c.diff.auto.html
src/lib/libast/comp/omitted.c:
- Only fall back to the libast bzero function if the OS lacks an
implementation of bzero.
- Remove the bzero undef since this fallback is only reached if the
OS doesn't have bzero.
NEWS:
- Add compat info for macOS on ARM64. This notes that macOS
Monterey can now compile and run ksh, although there is one
regression test failure:
builtins.sh[345]: printf %H: invalid UTF-8 characters
(expected %3F%C2%86%3F%3F%3F; got %3F%C2%86%3F%3Fv%3F%3F)
Thanks to @DesantBucie for the report and the testing.
Resolves: https://github.com/ksh93/ksh/issues/329
This bug was first reported in <https://www.illumos.org/issues/7694>.
The time keyword currently overrides the errexit shell option,
allowing failing scripts to continue after an error:
$ cat 1.sh
#!/bin/sh
time false # This should cause the script to exit
echo FAILURE
true
$ ksh -o errexit 1.sh
real 0m0.00s
user 0m0.00s
sys 0m0.00s
FAILURE
src/cmd/ksh93/sh/xec.c:
- When the time keyword runs a command, pass the errexit state flag
to the sh_exec call. This state flag is required for ksh to exit
when a command fails while the errexit option is on.
src/cmd/ksh93/tests/basic.sh:
- Add a regression test based on the reproducer.
This reverts commit 2b9cbbbc8e.
This is not ready for prime time. Crashses when running a $PS2
discipline function. This needs fixing and more testing in
development before making it into the 1.0 branch. In the meantime,
that terrible problem with types is back, sorry about that.
This commit mitigates the effects of the hack explained in the
referenced commit so that dummy built-in command nodes added by the
parser for declaration/assignment purposes do not leak out into the
execution level, except in a relatively harmless corner case.
Something like
if false; then
typeset -T Foo_t=(integer -i bar)
fi
will no longer leave a broken dummy Foo_t declaration command. The
same applies to declaration commands created with enum.
The corner case remaining is:
$ ksh -c 'false && enum E_t=(a b c); E_t -a x=(b b a c)'
ksh: E_t: not found
Since the 'enum' command is not executed, this should have thrown
a syntax error on the 'E_t -a' declaration:
ksh: syntax error at line 1: `(' unexpected
This is because the -c script is parsed entirely before being
executed, so E_t is recognised as a declaration built-in at parse
time. However, the 'not found' error shows that it was successfully
eliminated at execution time, so the inconsistent state will no
longer persist.
This fix now allows another fix to be effective as well: since
built-ins do not know about virtual subshells, fork a virtual
subshell into a real subshell before adding any built-ins.
src/cmd/ksh93/sh/parse.c:
- Add a pair of functions, dcl_hactivate() and dcl_dehacktivate(),
that (de)activate an internal declaration built-ins tree into
which check_typedef() can pre-add dummy type declaration command
nodes. A viewpath from the main built-ins tree to this internal
tree is added, unifying the two for search purposes and causing
new nodes to be added to the internal tree. When parsing is done,
we close that viewpath. This hides those pre-added nodes at
execution time. Since the parser is sometimes called recursively
(e.g. for command substitutions), keep track of this and only
activate and deactivate at the first level.
- We also need to catch errors. This is done by setting libast's
error_info.exit variable to a dcl_exit() function that tidies up
and then passes control to the original (usually sh_exit()).
- sh_cmd(): This is the most central function in the parser. You'd
think it was sh_parse(), but $(modern)-form command substitutions
use sh_dolparen() instead. Both call sh_cmd(). So let's simply
add a dcl_hacktivate() call at the beginning and a
dcl_deactivate() call at the end.
- assign(): This function calls path_search(), which among many
other things executes an FATH search, which may execute arbitrary
code at parse time (!!!). So, regardless of recursion level,
forcibly dehacktivate() to avoid those ugly parser side effects
returning in that context.
src/cmd/ksh93/bltins/enum.c: b_enum():
- Fork a virtual subshell before adding a built-in.
src/cmd/ksh93/sh/xec.c: sh_exec():
- Fork a virtual subshell when detecting typeset's -T option.
Improves fix to https://github.com/ksh93/ksh/issues/256
Symptom:
$ ksh -c 'command enum -i P_t=(a b); P_t -A v=([f]=b); typeset -p v'
ksh: syntax error at line 1: `(' unexpected
Expected: no syntax error, and output of 'P_t -A v=([f]=b)'.
src/cmd/ksh93/sh/parse.c: check_typedef():
- For enum, skip over any possible 'command' prefixes before
pre-parsing options with optget (or, technically, skip anything
else that might come before 'enum', though I don't think anything
else is possible).
- The sh_addbuiltin() call at the end to pre-add the builtin
obtained the node pointer to the built-in and the node flags from
the parser tree. This did not work if a 'command' prefix was
present. However, we don't actually need this. For parsing
purposes, the BLT_DCL flag for a declaration built-in is
sufficient; this is what gets the parser to accept
assignment-arguments including parentheses. So just apply that.
In addition, let's point it to an actual dummy built-in, 'true'
(SYSTRUE), so that if a user does run something like 'if false;
then enum Foo_t=(...); fi', the leaked Foo_t dummy at least won't
do anything (not even crash).
When giving an invalid or incompatible option to a typeset option
equivalent command (former default alias) such as 'compound' or
'integer', the resulting usage messages are incorrect. Example:
$ ksh -c 'compound -T foo=(typeset -a bar[1]=23)'
ksh: compound: -T cannot be used with other options
Usage: compound [-bflmnprstuxACHS] [-a[[type]]] [-i[base]] [-E[n]]
[-F[n]] [-L[n]] [-M[mapping]] [-R[n]] [-X[n]]
[-h string] [-T[tname]] [-Z[n]] [name[=value]...]
Or: compound -f [name...]
Or: compound -m [name=name...]
Or: compound -n [name=name...]
Or: compound -T [tname[=(type definition)]...]
Help: compound [ --help | --man ] 2>&1
The error message is wrong (there were no other options) and some
of the listed usages are invalid, like 'compound -f'.
Typeset option equivalent commands should just use 'typeset' in all
their error messages to avoid confusion. This is done by setting
error_info.id to the name of the typeset builtin.
There is quite a bit of no-op code in the job_hup() function due
to conditions that always test false. This commit removes that code
and clarifies the rest, making the purpose of this function clear.
job_hup() (before 62cf88d0: job_terminate()) is called via
job_walk() by sh_done() in fault.c to issue SIGHUP, the "hang up"
signal, to every background job's process group when the current
session is ungracefully disconnected. (One way to trigger such a
disconnection is to forcibly terminate a ssh session by typing '~.'
on a new prompt.)
The bug that Solaris patch 260-22964338 fixed is that ksh then
killed all non-disowned jobs' process groups without considering
that ksh still remembers a job even when all its processes are
finished (have the P_DONE flag). In that condition, the process
group ID may well be reused by another process by now, so it is
dangerous to killpg() it; we risk killing unrelated processes!
This is *not* a hypothetical problem; the Solaris patch exists
because this happened to a Solaris customer. However, the bug
exists on all operating systems. It's rarely triggered but serious,
and it's more likely to occur on heavy workloads that re-use
process/group IDs a lot. And it's on every currently released
non-Solaris version of ksh93. Eesh.
src/cmd/ksh93/sh/jobs.c:
src/cmd/ksh93/include/jobs.h:
- Remove job_terminate() which was unused as of 62cf88d0.
It could have been fixed instead of replaced. Oh well.
- Refactor job_hup():
- Remove code that will never be executed because, at
those points, it is known that pw->p_pgrp != 0.
- Simplify the loop that checks that there is at least
one non-P_DONE process so it doesn't need a flag.
For documentation purposes, below is a reproducer for the bug
before the Solaris patch. It is rather involved.
1. Compile the C program below (cpid).
2. In one terminal, 'ssh localhost'.
3. Within the ssh session:
- 'exec -a-ksh /path/to/buggy/ksh' to get a ksh login shell.
- 'sleep 1 &' and let it finish. Note down the reported PID.
That is the one we will reuse. Let's say 26650.
4. In another terminal, run: ./cpid 26650 (the PID from the
previous step). Now wait until it says "PID 26650 is ready"; it
has now succeeded at re-using that PID, and will just sit there.
This process will never voluntarily terminate. If we have the
bug, the termination of this process will be the symptom.
5. In the first terminal, forcibly terminate the ssh session by
typing, on a new prompt: ~. (tilde, dot). This triggers the
buggy routine to issue SIGHUP to all of ksh's background jobs.
6. In the second terminal, the bug is reproduced if cpid has been
terminated, reporting 'waitpid return 26650, status 0x0001', so
ksh just killed this process that it had nothing to do with.
(Note that status 0x0001 refers to being killed by signal 1
which is SIGHUP.)
cpid.c follows (written by George Lijo, tweaked by me):
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <signal.h>
#include <sys/wait.h>
int main(int argc, char *argv[])
{
pid_t pid, rpid, opid;
int i, status, npid;
if (argc != 2)
{
fprintf(stderr, "Usage: cpid <PID to re-use>\n");
exit(1);
}
rpid = atoi(argv[1]);
opid = getpid();
for (;;)
{
if ((pid = fork()) == 0)
{
setpgrp();
pause();
_exit(0);
}
if (pid == rpid)
break;
kill(pid, SIGKILL);
waitpid(pid, NULL, 0);
if (opid < rpid && pid > rpid)
printf("Cannot create PID %d\n", rpid);
opid = pid;
}
printf("PID %d is ready\n", pid);
i = waitpid(pid, &status, 0);
printf("waitpid return %d, status 0x%4.4x\n", i, status);
return status;
}
Since the arithmetic recursion level only becomes incorrect when
an error interrupts the arithmetic subsystem, and all such error
messages call sh_exit(), it should be good enough to reset it
there, so we don't need to do that for nearly every sh_exec() run.
The -d flag implemented in the rm builtin is completely broken. No
matter what you do it refuses to remove directories, even if -r is
also passed. Reproducer:
$ mkdir /tmp/empty
$ PATH=/opt/ast/bin rm -d /tmp/empty
rm: /tmp/empty: directory
$ PATH=/opt/ast/bin rm -dr /tmp/empty
rm: /tmp/empty: directory not removed [Is a directory]
Additionally, the description of 'rm -d' in the man page contradicts
how it's specified in <https://www.austingroupbugs.net/view.php?id=802>.
The ksh93v- rm builtin fixed nearly all of these issues, so I've
backported it to 93u+m and applied one additional fix for 'rm -rd'.
src/lib/libcmd/rm.c:
- Backported the fixes from the ksh93v- rm builtin's -d flag when
used on empty directories.
- Backported the man page update for rm(1) from ksh93v-.
- The ksh93v- rm builtin had one additional bug that caused the -r
option to fail when combined with -d. This was fixed by
overriding -d if -r is also passed.
src/cmd/ksh93/tests/builtins.sh:
- Add regression tests for the rm builtin's -d option.
This function adds the NV_ADD flag to its 'flags' variable for
nv_serach() calls subject to some checks. However, every call that
uses that variable explicitly turns off the NV_ADD bit again.
A search in the ast-open-history repo reveals that this check
briefly made a difference between versions 2010-06-25 and
2010-08-11, but it's been a complete no-op ever since.
src/cmd/ksh93/sh/arith.c: scope():
- Remove no-op code.
- Resolve the constant expressions involving the 'flags' variable,
get rid of the variable, and just indicate the flag bitmasks
directly in the nv_search() calls.
- Detangle and split up the excessively long 'if' construct.
No change in behaviour.
Previously noticed by Kurtis Rader for ksh2020:
https://github.com/att/ast/commit/d5ce3b05
POSIXly, '.' loads only files, not functions.
This only applies to '.', not 'source' (which is not in POSIX).
src/cmd/ksh93/bltins/misc.c: b_source():
- For ksh function lookup, add an additional check that we're not
in POSIX mode and running the '.' (SYSDOT) builtin.
Defining a .sh.tilde.get or .sh.tilde.set discipline function to
extend tilde expansion works well as long as the discipline
function doesn't get interrupted (e.g. with Crtl+C) or produce an
error message. Either of those will cause the shell to become
unstable and crash.
This feature is now removed from the 1.0 branch as it is not ready
for prime time. It can return to a release branch if/when we manage
to fix it on the master branch.
Related: https://github.com/ksh93/ksh/issues/346
In 93v-/ksh2020, namespace defs in any function are a syntax error.
This commit blocks namespace defs for ksh functions only, at the
execution level. This follows some of AT&T original intention while
working around some of the known bugs with namespaces.
Related: https://github.com/ksh93/ksh/issues/325
As of the previous commit, I finally know how to properly check for
a variable of a type created by 'enum'. We need to check for both
the NV_UINT16 attribute and the ENUM_disc discipline.
Also:
- regression test tweaks
- add missing tests for previous commit (f600a5ea)
Within arithmetic expressions, enumeration values of variables of a
type created with the 'enum' command translate to index numbers
from 0 to the number of elements minus 1. However, there was no
range checking on this in the arithmetic subsystem, allowing the
assignment of out-of-range values that did not correspond to any
enumeration value.
Variables of an enum type are internally unsigned short integers
(NV_UINT16), like those created with 'integer -su', except with an
additional discipline function (ENUM_disc).
src/cmd/ksh93/bltins/enum.c,
src/cmd/ksh93/include/builtins.h:
- To implement range checking, the arithmetic system needs access
to the 'nelem' (number of elements) member of 'struct Enum'. This
is only defined locally in enum.c. We could move that to name.h
so arith.c can access it, but enum.c has code that supports
compiling as standalone. So, instead, define a quick extern
function, b_enum_elem(), that does the necessary type conversion
and returns a type's number of elements.
- Add --man documentation for the arithmetic subsystem behaviour
for enum types. Tell the enuminfo() function, which dynamically
inserts values into the documentation, how to process new \f tags
'lastv' (the last-defined value) and 'lastn' (the number of the
last element).
src/cmd/ksh93/sh/arith.c: arith():
- For NV_UINT16 variables with an ENUM_disc discipline, check the
range using b_enum_elem() and error out if necessary.
Resolves: https://github.com/ksh93/ksh/issues/335
This commit adds support for 'stty size' to the stty builtin, as
defined in <https://austingroupbugs.net/view.php?id=1053>. The size
mode is used to display the terminal's number of rows and columns.
Note that stty isn't included in the default list of builtin
commands; testing this addition requires adding CMDLIST(stty) to
the table of builtins in src/cmd/ksh93/data/builtins.c.
src/lib/libcmd/stty.c:
- Add support for the size mode to the stty builtin. This mode is
only used to display the terminal's number of rows and columns,
so error out if any arguments are given that attempt to set the
terminal size.
For some reason, Void Linux (with musl libc) sets SIGCONT to
ignored on the Linux console, causing the 'sleep -s' test in
builtins.sh to fail spuriously as it relies on SIGCONT to work.
src/cmd/ksh93/tests/shtests:
- Reset SIGCONT using the unadvertised 'trap + SIGCONT' feature.
Resolves: https://github.com/ksh93/ksh/issues/301
As I got to know the code better, it now seems painfully obvious
that getting test/[ to issue an exit status >= 2 on error only
requires a simple check in sh_exit() in fault.c, which is called
whenever the shell issues an error message.
Parser limitations prevent shcomp or source from handling enum
types correctly:
$ cat /tmp/colors.sh
enum Color_t=(red green blue orange yellow)
Color_t -A Colors=([foo]=red)
$ shcomp /tmp/colors.sh > /dev/null
/tmp/colors.sh: syntax error at line 2: `(' unexpected
$ source /tmp/colors.sh
/bin/ksh: source: syntax error: `(' unexpected
Yet, for types created using 'typeset -T', this works. This is done
via a check_typedef() function that preliminarily adds the special
declaration builtin at parse time, with details to be filled in
later at execution time.
This hack will produce ugly undefined behaviour if the definition
command creating that type built-in is then not actually run at
execution time before the type built-in is accessed.
But the hack is necessary because we're dealing with a fundamental
design flaw in the ksh language. Dynamically addable built-ins that
change the syntactic parsing of the shell language on the fly are
an absurdity that violates the separation between parsing and
execution, which muddies the waters and creates the need for some
kind of ugly hack to keep things like shcomp more or less working.
This commit extends that hack to support enum.
src/cmd/ksh93/sh/parse.c:
- check_typedef():
- Add 'intypeset' parameter that should be set to 1 for typeset
and friends, 2 for enum.
- When processing enum arguments, use AST getopt(3) to skip over
enum's options to find the name of the type to be defined.
(getopt failed if we were running a -c script; deal with this
by zeroing opt_info.index first.)
- item(): Update check_typedef() call, passing lexp->intypeset.
- simple(): Set lexp->intypeset to 2 when processing enum.
The rest of the changes are all to support the above and should be
fairly obvious, except:
src/cmd/ksh93/bltins/enum.c:
- enuminfo(): Return on null pointer, avoiding a crash upon
executing 'Type_t --man' if Type_t has not been fully defined due
to the definition being pre-added at parse time but not executed.
It's all still wrong, but a crash is worse.
Resolves: https://github.com/ksh93/ksh/issues/256
Listing types with 'typeset -T' will list not only types created with
typeset, but also types created with enum. However, the types created
by enum are not displayed correctly in the resulting output:
$ enum Foo_t=(foo bar)
$ typeset -T
typeset -T Foo_t
typeset -T Foo_t=fo)
The fix for this bug was backported from ksh93v- 2013-10-08.
src/cmd/ksh93/sh/nvtype.c:
- sh_outtype(): Skip over enums when listing types with 'typeset -T'.
The referenced commit did not fix the symptoms on the 1.0 branch
(no vmalloc) on the GitHub CI runners.
The failures are intermittent and are not reproduced with vmalloc
or on other operating systems.
Though the failures occur on a different test each time, the total
amount of "leaked" bytes is always 36864, e.g.:
leaks.sh[388]: run command with preceding PATH assignment in
main shell (leaked approx 36864 bytes after 4096 iterations)
36864/4096 equals exactly 9. An odd number, literally and
figuratively, but I suppose that's the tolerance Linux needs.
src/cmd/ksh93/tests/leaks.sh
- Increase tolerance of bytes per iteration from 8 to 9.
This commit fixes an issue I found in the subshell $RANDOM
reseeding code.
The main issue is a performance regression in the shbench fibonacci
benchmark, introduced in commit af6a32d1. Performance dropped in
this benchmark because $RANDOM is always reseeded and restored,
even when it's never used in a subshell. Performance results from
before and after this performance fix (results are on Linux with
CC=gcc and CCFLAGS='-O2 -D_std_malloc'):
$ ./shbench -b bench/fibonacci.ksh -l 100 ./ksh-0f06a2e ./ksh-af6a32d ./ksh-f31e368 ./ksh-randfix
benchmarking ./ksh-0f06a2e, ./ksh-af6a32d, ./ksh-f31e368, ./ksh-randfix ...
*** fibonacci.ksh ***
# ./ksh-0f06a2e # Recent version of ksh93u+m
# ./ksh-af6a32d # Commit that introduced the regression
# ./ksh-f31e368 # Commit without the regression
# ./ksh-randfix # Ksh93u+m with this patch applied
-------------------------------------------------------------------------------------------------
name ./ksh-0f06a2e ./ksh-af6a32d ./ksh-f31e368 ./ksh-randfix
-------------------------------------------------------------------------------------------------
fibonacci.ksh 0.481 [0.459-0.515] 0.472 [0.455-0.504] 0.396 [0.380-0.442] 0.407 [0.385-0.439]
-------------------------------------------------------------------------------------------------
src/cmd/ksh93/include/variables.h,
src/cmd/ksh93/sh/{init,subshell}.c:
- Rather than reseed $RANDOM every time a subshell is created, add
a sh_save_rand_seed() function that does this only when the
$RANDOM variable is used in a subshell. This function is called
by the $RANDOM discipline functions nget_rand() and put_rand().
As a minor optimization, sh_save_rand_seed doesn't reseed if it's
called from put_rand().
- Because $RANDOM may have a seed of zero (i.e., RANDOM=0),
sp->rand_seed isn't enough to tell if $RANDOM has been reseeded.
Add sp->rand_state for this purpose.
- sh_subshell(): Only restore the former $RANDOM seed and state if
it is necessary to prevent a subshell leak.
src/cmd/ksh93/tests/variables.sh:
- Add two regression tests for bugs I ran into while making this
patch.
The ksh manual page is one of the few places that calls globbing
"file name generation". The mksh and zsh manuals use the same term.
But every other shell's manual calls it "pathname expansion": bash,
dash, yash, FreeBSD sh. So does ksh's built-in documentation (alias
--man, export --man, readonly --man, set --man, typeset --man).
What's more, the authoritative ksh reference, Bolsky & Korn's 1995
"The New Kornshell" book, also calls it "pathname expansion", and
so does the POSIX standard.
Similarly, "arithmetic substitution" should be called "arithmetic
expansion" per Bolsky & Korn as well as POSIX.
This commit has several other miscellaneous documentation tweaks as
well.
'printf' on bash and zsh has a popular -v option that allows
assigning formatted output directly to variables without using a
command substitution. This is much faster and avoids snags with
stripping final linefeeds. AT&T had replicated this feature in the
abandoned 93v- beta version. This backports it with a few tweaks
and one user-visible improvement.
The 93v- version prohibited specifying a variable name with an
array subscript, such as printf -v var\[3\] foo. This works fine on
bash and zsh, so I see no reason why this should not work on ksh,
as nv_putval() deals with array subscripts just fine.
src/cmd/ksh93/bltins/print.c: b_print():
- While processing the -v option when called as printf, get a
pointer to the variable, creating it if necessary. Pass only the
NV_VARNAME flag to enforce a valid variable name, and not (as
93v- does) the NV_NOARRAY flag to prohibit array subscripts.
- If a variable was given, set the output file to an internal
string buffer and jump straight to processing the format.
- After processing the format, assign the contents to the string
buffer to the variable.
src/cmd/ksh93/data/builtins.c:
- Document the new option, adding a warning that unquoted square
brackets may trigger pathname expansion.
%(pattern)q is equivalent to %P. It's also equivalent to %#P, but
since the alternative format specifier '#' does nothing for %P,
%P and %#P are the same and documenting #%P is just confusing.
Thanks to @stephane-chazelas for the report.
src/cmd/ksh93/bltins/print.c:
- In the printmap struct, document %P as equivalent of %(pattern)q.
- Sort it alphabetically.
- Do not pointlessly repeat the string "Equivalent to". Instead,
let the discipline function infof() insert it for each entry.
(This is the function used to dynamically insert the equivalents
documentation into the --man output at the \fextra\f tag in
sh_optprintf[] in data/builtins.c.)
Resolves: https://github.com/ksh93/ksh/issues/338
An obsolete struct was left that passed some variables on between
b_exec() and the deleted B_login(). We can simply make those local
variables now. Let's get rid of the redundant sh pointer, too.
As the (original AT&T) comment at the top says, "the trickiest part
of the tests is avoiding typeahead in the pty dialogue".
Two tests failed to [p]eek at the prompt before they started
'typing'. This causes unpredictable results. On Debian Bullseye
this triggers typeahead, which produces unwanted echo to the
terminal, killing the tests.
src/cmd/ksh93/tests/pty.sh:
- Add missing 'p' commands for the first prompt to the tests
'nobackslashctrl in emacs' and 'emacs backslash escaping'.
Resolves: https://github.com/ksh93/ksh/issues/332
Tried to compile on Solaris 10.1 for the first time in a while.
Turns out the obsolete Bourne /bin/sh does not support 'test -e'.
bin/package, src/cmd/INIT/package.sh:
- Use 'test -f' instead.
In C/POSIX arithmetic, a leading 0 denotes an octal number, e.g.
010 == 8. But this is not a desirable feature as it can cause
problems with processing things like dates with a leading zero.
In ksh, you should use 8#10 instead ("10" with base 8).
It would be tolerable if ksh at least implemented it consistently.
But AT&T made an incredible mess of it. For anyone who is not
intimately familiar with ksh internals, it is inscrutable where
arithmetic evaluation special-cases a leading 0 and where it
doesn't. Here are just some of the surprises/inconsistencies:
1. The AT&T maintainers tried to honour a leading 0 inside of
((...)) and $((...)) and not for arithmetic contexts outside it,
but even that inconsistency was never quite consistent.
2. Since 2010-12-12, $((x)) and $(($x)) are different:
$ /bin/ksh -c 'x=010; echo $((x)) $(($x))'
10 8
That's a clear violation of both POSIX and the principle of
least astonishment. $((x)) and $(($x)) should be the same in
all cases.
3. 'let' with '-o letoctal' acts in this bizarre way:
$ set -o letoctal; x=010; let "y1=$x" "y2=010"; echo $y1 $y2
10 8
That's right, 'let y=$x' is different from 'let y=010' even
when $x contains the same string value '010'! This violates
established shell grammar on the most basic level.
This commit introduces consistency. By default, ksh now acts like
mksh and zsh: the octal leading zero is disabled in all arithmetic
contexts equally. In POSIX mode, it is enabled equally.
The one exception is the 'let' built-in, where this can still be
controlled independently with the letoctal option as before (but,
because letoctal is synched with posix when switching that on/off,
it's consistent by default).
We're also removing the hackery that causes variable expansions for
the 'let' builtin to be quietly altered, so that 'x=010; let y=$x'
now does the same as 'let y=010' even with letoctal on.
Various files:
- Get rid of now-redundant sh.inarith (shp->inarith) flag, as we're
no longer distinguishing between being inside or outside ((...)).
src/cmd/ksh93/sh/arith.c:
- arith(): Let disabling POSIX octal constants by skipping leading
zeros depend on either the letoctal option being off (if we're
running the "let" built-in") or the posix option being off.
- sh_strnum(): Preset a base of 10 for strtonll(3) depending on the
posix or letoctal option being off, not on the sh.inarith flag.
src/cmd/ksh93/include/argnod.h,
src/cmd/ksh93/sh/args.c,
src/cmd/ksh93/sh/macro.c:
- Remove astonishing hackery that violated shell grammar for 'let'.
src/cmd/ksh93/sh/name.c (nv_getnum()),
src/cmd/ksh93/sh/nvdisc.c (nv_getn()):
- Remove loops for skipping leading zeroes that included a broken
check for justify/zerofill attributes, thereby fixing this bug:
$ typeset -Z x=0x15; echo $((x))
-ksh: x15: parameter not set
Even if this code wasn't redundant before, it is now: sh_arith()
is called immediately after the removed code and it ignores
leading zeroes via sh_strnum() and strtonll(3).
Resolves: https://github.com/ksh93/ksh/issues/334
When starting a new interactive ksh with the -v or -x option, an
annoying symptom occurs: the 'tput' command that ed_setup() issues
to get the escape sequence for cursor-up is xtraced or echoed,
corrupting prompt display, for example ('▂' is the cursor):
$ ksh -x
$ + /usr/bin/tput cuu1
+ 2> /dev/null
+ .sh.subscript=$'\E[A'
▂
or
$ ksh -v
$ .sh.subscript=$(/usr/bin/tput cuu1 2>/dev/null)▂
src/cmd/ksh93/edit/edit.c: ed_setup():
- Turn off xtrace and verbose while sh_trap()ing tput.
So, shcomp has messed up my terminal once too often by writing
compiled binary data to it. While fixing that I've done some other
tweaks as well.
src/cmd/ksh93/sh/shcomp.c: main():
- Fix error/warning message id (the "name:" prefix before messages)
so it makes sense to the user. Save shcomp's argv[0] id for error
messages that are directly from shcomp's main(), and use the
argv[1] script id (set by sh_init()) for warnings produced by the
compilation process. If there is no script id because we're
reading from stdin, set it to "(stdin)".
- If no arguments are given, refuse to read from standard input if
it's on a tty. Instead, write a brief usage message (with pointer
to --help and --man, see e21a053e) and exit. This is far more
helpful; people will rarely want to compile a script by manually
typing it in. If you really want to do that, use /dev/stdin as
the input filename. :)
- Error out if we're about to write binary data to a tty (even if
/dev/stdout was given as the output filename).
- Turn off SH_MULTILINE to avoid some pointless editor init in case
we're reading from stdin on a terminal.
- Do not attempt to copy remaining data if we're already at EOF.
This fixes a bug that required the user to press Ctrl+D twice
when manually entering a script on the terminal. Pressing Ctrl+D
once and then entering more data would corrupt the bytecode.
This commit fixes two file descriptor leaks in the hist built-in.
The bugfix for the first file descriptor leak was backported from
ksh2020. See:
https://github.com/att/ast/issues/872https://github.com/att/ast/commit/73bd61b5
Reproducer:
$ echo no
$ hist -s no=yes
The second file descriptor leak occurs after a substitution error
in the hist built-in (this leak wasn't fixed in ksh2020).
Reproducer:
$ echo no
$ ls /proc/$$/fd
$ hist -s no=yes
$ hist -s no=yes
$ ls /proc/$$/fd
src/cmd/ksh93/bltins/hist.c:
- Close leftover file descriptors when an error occurs and after
'hist -s' runs a command.
src/cmd/ksh93/tests/builtins.sh:
- Add two regression tests for both of the file descriptor leaks.
When testing whether subshell $RANDOM reseeding worked, checking
for non-identical numbers is not sufficient. There is no check for
randomly occurring duplicate numbers, nor can there be, because
subshells cannot (or, in the case of virtual subshells, should not)
influence each other or the parent shell.
src/cmd/ksh93/tests/variables.sh:
- Try up to three times, tolerating identical numbers twice.
The --posix compliance option now disables the case-insensitive
special floating point constants Inf and NaN so that all case
variants of $((inf)) and $((nan)) refer to the variables by those
names as the standard requires. (BUG_ARITHNAN)
src/cmd/ksh93/sh/arith.c: arith():
- Only do case-insensitive checks for "Inf" and "NaN" if the POSIX
option is off.
e_badnum from streval.h and e_number from shell.h are both defined
as "%s: bad number". We only need one. Remove the one that is used
only once: e_badnum.
Turns out there is a way to check what built-in we're running at
any time. It is done for 'let' in arith.c:
sh.bltindata.bnode==SYSLET
For test/[, that would be (see include/builtins.h):
sh.bltindata.bnode==SYSTEST || sh.bltindata.bnode==SYSBRACKET
ksh crashed after unsetting .sh.match and then matching a pattern:
$ unset .sh.match
$ [[ bar == ba* ]]
Memory fault
src/cmd/ksh93/sh/init.c: sh_setmatch():
- Do nothing if we cannot get an array pointer to SH_MATCHNOD.
There are one or two leaks that show up intermittently on the
Github runners for the 1.0 branch (which is compiled as a release,
i.e. no vmalloc). If they're intermittent, they must be false
positives due to malloc artefacts. Let's double the number of
iterations for the /proc/$$/stat method and see what happens.
Symptoms:
$ test \( string1 -a string2 \)
/usr/local/bin/ksh: test: argument expected
$ test \( string1 -o string2 \)
/usr/local/bin/ksh: test: argument expected
The parentheses should be irrelevant and this should be a test for
the non-emptiness of string1 and/or string2.
src/cmd/ksh93/bltins/test.c:
- b_test(): There is a block where the case of 'test' with five or
less arguments, the first and last one being parentheses, is
special-cased. The parentheses are removed as a workaround: argv
is increased to skip the opening parenthesis and argc is
decreased by 2. However, there is no corresponding increase of
tdata.av which is a copy of this function's argv. This renders
the workaround ineffective. The fix is to add that increase.
- e3(): Do not handle '!' as a negator if not followed by an
argument. This allows a right-hand expression that is equal to
'!' (i.e. a test for the non-emptiness of the string '!').
In ksh88, the test/[ built-in supported both the '<' and '>'
lexical sorting comparison operators, same as in [[. However, in
every version of ksh93, '<' does not work though '>' still does!
Still, the code for both is present in test_binop():
src/cmd/ksh93/bltins/test.c
548: case TEST_SGT:
549: return(strcoll(left, right)>0);
550: case TEST_SLT:
551: return(strcoll(left, right)<0);
Analysis: The binary operators are looked up in shtab_testops[] in
data/testops.c using a macro called sh_lookup, which expands to a
sh_locate() call. If we examine that function in sh/string.c, it's
easy to see that on systems using ASCII (i.e. all except IBM
mainframes), it assumes the table is sorted in ASCII order.
src/cmd/ksh93/sh/string.c
64: while((c= *tp->sh_name) && (CC_NATIVE!=CC_ASCII || c <= first))
The problem was that the '<' operator was not correctly sorted in
shtab_testops[]; it was sorted immediately before '>', but after
'='. The ASCII order is: < (60), = (61), > (62). This caused '<' to
never be found in the table.
The test_binop() function is also used by [[, yet '<' always worked
in that. This is because the parser has code that directly checks
for '<' and '>' within [[ (in sh/parse.c, lines 1949-1952).
This commit also adds '=~' to 'test', which took three lines of
code and allowed eliminating error handling in test_binop() as
test/[ and [[ now support the same binary ops. (re: fc2d5a60)
src/cmd/ksh93/*/*.[ch]:
- Rename a couple of very misleadingly named macros in test.h:
. For == and !=, the TEST_PATTERN bit is off for pattern compares
and on for literal string compares! Rename to TEST_STRCMP.
. The TEST_BINOP bit does not denote all binary operators, but
only the logical -a/-o ops in test/[. Rename to TEST_ANDOR.
src/cmd/ksh93/bltins/test.c: test_binop():
- Add support for =~. This is only used by test/[. The method is
implemented in two lines that convert the ERE to a shell pattern
by prefixing it with ~(E), then call test_strmatch with that
temporary string to match the ERE and update ${.sh.match}.
- Since all binary ops from shtab_testops[] are now accounted for,
remove unknown op error handling from this function.
src/cmd/ksh93/data/testops.c:
- shtab_testops[]:
. Correctly sort the '<' (TEST_SLT) entry.
. Remove ']]' (TEST_END). It's not an op and doesn't belong here.
- Update sh_opttest[] documentation with =~, \<, \>.
- Remove now-unused e_unsupported_op[] error message.
src/cmd/ksh93/sh/lex.c: sh_lex():
- Check for ']]' directly instead of relying on the removed
TEST_END entry from shtab_testops[].
src/cmd/ksh93/tests/bracket.sh:
- Add relevant tests.
src/cmd/ksh93/tests/builtins.sh:
- Fix an old test that globally deleted the 'test' builtin. Delete
it within the command substitution subshell only.
- Remove the test for non-support of =~ in test/[.
- Update the test for invalid test/[ op to use test directly.
POSIX requires
test "$a" -a "$b"
to return true if both $a and $b are non-empty, and
test "$a" -o "$b"
to return true if either $a or $b is non-empty.
In ksh, this fails if "$a" is '!' or '(' as this causes ksh to
interpret the -a and -o as unary operators (-a being a file
existence test like -e, and -o being a shell option test).
$ test ! -a ""; echo "$?"
0 (expected: 1/false)
$ set -o trackall; test ! -o trackall; echo "$?"
1 (expected: 0/true)
$ test \( -a \); echo "$?"
ksh: test: argument expected
2 (expected: 0/true)
$ test \( -o \)
ksh: test: argument expected
2 (expected: 0/true)
Unfortunately this problem cannot be fixed without risking breakage
in legacy scripts. For instance, a script may well use
test ! -a filename
to check that a filename is nonexistent. POSIX specifies that this
always return true as it is a test for the non-emptiness of both
strings '!' and 'filename'.
So this commit fixes it for POSIX mode only.
src/cmd/ksh93/bltins/test.c: e3():
- If the posix option is active, specially handle the case of
having at least three arguments with the second being -a or -o,
overriding their handling as unary operators.
src/cmd/ksh93/data/testops.c:
- Update 'test --man --' date and say that unary -a is deprecated.
src/cmd/ksh93/sh.1:
- Document the fix under the -o posix option.
- For test/[, explain that binary -a/-o are deprecated.
src/cmd/ksh93/tests/bracket.sh:
- Add tests based on reproducers in bug report.
Resolves: https://github.com/ksh93/ksh/issues/330
On Linux, the 'su' program sets $0 to '-su' when doing 'su -' or
'su - username'. When ksh is the target account's default shell,
this caused ksh to consider itself to be launched as a standard
POSIX sh, which (among other things) disables the default aliases
on interactive shells. This caused confusion for at least one user
as they lost their 'history' alias after 'su -':
https://www.linuxquestions.org/questions/slackware-14/in-current-with-downgrade-to-ksh93-lost-the-alias-history-4175703408/
bash does not consider itself to be sh when invoked as su, so ksh
probably shouldn't, either. The behaviour was also undocumented,
making it even more surprising.
src/cmd/ksh93/sh/init.c: sh_type():
- Only set the SH_TYPE_POSIX bit if we're invoked as 'sh' (or, on
windows, as 'sh.exe').
A change in FreeBSD 13 now causes extremely long command names to
exit with errno set to E2BIG if the name can't fit in the list of
arguments. This was causing the regression tests for ENAMETOOLONG
to fail on FreeBSD 13 because the exit status for these errors
differ (ENAMETOOLONG uses status 127 while E2BIG uses status 126).
src/cmd/ksh93/tests/path.sh:
- To fix the failing regression tests, the command name has been
shortened to twice the length of NAME_MAX. This length is still
long enough to trigger an ENAMETOOLONG error without causing an
E2BIG failure on FreeBSD 13.
Fixes https://github.com/ksh93/ksh/issues/331
The sh_trace() function, which prints an xtrace line to standard
error, clears the SF_SHARE and SF_PUBLIC flags from the sfstderr
stream during the xtrace in order to guarantee an atomic trace
write. But it only restored those flags if the passed argv pointer
is non-NULL. Redirections are traced with a NULL argv parameter, so
the stderr state was not restored for them.
This somehow caused unpredictable behaviour, including (on some
systems) a crash in sfwrite(3) when running the heredoc.sh tests
with xtrace on.
src/cmd/ksh93/sh/xec.c: sh_xtrace():
- Move the sfset() invocation that restores the SF_SHARE|SF_PUBLIC
flags to sfstderr out of the if(argv) block.
- Since we're here, don't bother wasting cycles initialising local
variable values if xtrace is not on. Move that inside the
if(sh_isoption(SH_XTRACE)) block.
Resolves: https://github.com/ksh93/ksh/issues/306
This was:
/*
* -lcmd specific workaround to handle
* fts_namelen
* fts_pathlen
* fts_level
* changing from [unsigned] short bit to [s]size_t
*
* ksh (or any other main application) that pulls in -lcmd
* at runtime may result in old -last running with new -lcmd
* which is not a good situation (tm)
*
* probably safe to drop after 20150101
*/
According to the version check in fts_fix.c, this change occurred
in the libast API version 2010-01-02, which is also the API version
of the bundled libast (see src/lib/libast/misc/state.c).
src/lib/libcmd/fts_fix.{c,h}:
- Removed.
src/lib/libcmd/{chgrp,chmod,cksum,cp,rm}.c:
- Change uses of fts_fix.h to fts.h from libast.
src/lib/libcmd/Mamfile:
- Update accordingly.
Stéphane Chazelas reported:
> As noted in this austin-group-l discussion[*] (relevant to this
> issue):
>
> $ ksh93u+m -c 'pwd; echo "$?" >&2; echo test; echo "$?" >&2' >&-
> 0
> 1
> /home/chazelas
>
> when stdout is closed, pwd does claim it succeeds (by returning a
> 0 exit status), while echo doesn't (not really relevant to the
> problem here, only to show it doesn't affect all builtins), and
> the output that pwd failed to write earlier ends up being written
> on stderr here instead of stdout upon exit (presumably) because
> of that >&2 redirection.
>
> strace shows ksh93 attempting write(1, "/home/chazelas\n", 15) 6
> times (1, the last one, successful).
>
> It gets even weirder when redirecting to a file:
>
> $ ksh93u+m -c 'pwd; echo "$?" >&2; echo test; echo "$?" > file' >&-
> 0
> $ cat file
> 1
> 1
> ome/chazelas
In my testing, the problem does not occur when closing stdout at
the start of the -c script itself (using redirect >&- or exec >&-);
it only occurs if stdout was closed before initialising the shell.
That made me suspect that the problem had to do with an
inconsistent file descriptor state in the shell. ksh uses internal
sh_open() and sh_close() functions, among others, to maintain that
state.
src/cmd/ksh93/sh/main.c: sh_main():
- If the shell is initialised with stdin, stdout or stderr closed,
then make the shell's file descriptor state tables reflect that
fact by calling sh_close() for the closed file descriptors.
This commit also improves the BUG_PUTIOERR fix from 93e15a30. Error
checking after sfsync() is not sufficient. For instance, on
FreeBSD, the following did not produce a non-zero exit status:
ksh -c 'echo hi' >/dev/full
even though this did:
ksh -c 'echo hi >/dev/full'
Reliable error checking requires not only checking the result of
every SFIO command that writes output, but also synching the buffer
at the end of the operation and checking the result of that.
src/cmd/ksh93/bltins/print.c:
- Make exitval variable global to allow functions called by
b_print() to set a nonzero exit status.
- Check the result of all SFIO output commands that write output.
- b_print(): Always sfsync() at the end, except if the s (history)
flag was given. This allows getting rid of the sfsync() call that
required the workaround introduced in 846ad932.
[*] https://www.mail-archive.com/austin-group-l@opengroup.org/msg08056.html
Resolves: https://github.com/ksh93/ksh/issues/314
The documentation for the supported unit suffixes for options
accepting numeric arguments was woefully outdated in 'head --man'
and 'tail --man'.
A quick look at the very short head(1) code shows that it does not
know or care about unit suffixes at all – it leaves that to libast
optget(3) which in turn calls strtoll() which is implemented in
strtoi.h where the multiplier suffixes are handled.
Note that on GNU head/tail, single-letter suffixes use power-of-2
units, e.g. k == KiB, etc. Libast used to do the same but this is
not standards compliant and AT&T changed/fixed this in 2011. From
libast/RELEASE:
10-04-11 string/strtoi.h: k (1000) and ki (1024) now differentiated
(They didn't mention the same change applies to all handled units.)
Note that the tail(1) builtin is currently not compiled in by
default. This can be changed in src/cmd/ksh93/data/builtins.c.
src/lib/libcmd/head.c, src/lib/libcmd/tail.c:
- Update the internal head/tail man pages to reflect what is
handled in strtoi.h.
Resolves: https://github.com/ksh93/ksh/issues/319
The functions of the three flags controlling job control are
crucial to understand in order to maintain the code, so they should
be documented in the comments and not just in the git log.
This commit does not change any code.
Bug 1: POSIX requires numbers used as arguments for all the %d,
%u... in printf to be interpreted as in the C language, so
printf '%d\n' 010
should output 8 when the posix option is on. However, it outputs 10.
This bug was introduced as a side effect of a change introduced in
the 2012-02-07 version of ksh 93u+m, which caused the recognition
of leading-zero numbers as octal in arithmetic expressions to be
disabled outside ((...)) and $((...)). However, POSIX requires
leading-zero octal numbers to be recognised for printf, too.
The change in question introduced a sh.arith flag that is set while
we're processing a POSIX arithmetic expression, i.e., one that
recognises leading-zero octal numbers.
Bug 2: Said flag is not reset in a command substitution used within
an arithmetic expression. A command substitution should be a
completely new context, so the following should both output 10:
$ ksh -c 'integer x; x=010; echo $x'
10 # ok; it's outside ((…)) so octals are not recognised
$ ksh -c 'echo $(( $(integer x; x=010; echo $x) ))'
8 # bad; $(comsub) should create new non-((…)) context
src/cmd/ksh93/bltins/print.c: extend():
- For the u, d, i, o, x, and X conversion modifiers, set the POSIX
arithmetic context flag before calling sh_strnum() to convert the
argument. This fixes bug 1.
src/cmd/ksh93/sh/subshell.c: sh_subshell():
- When invoking a command substitution, save and unset the POSIX
arithmetic context flag. Restore it at the end. This fixes bug 2.
Reported-by: @stephane-chazelas
Resolves: https://github.com/ksh93/ksh/issues/326
When invoking a script without an interpreter (#!hashbang) path,
ksh forks, but there is no exec syscall in the child. The existing
command line is overwritten in fixargs() with the name of the new
script and associated arguments. In the generic/fallback version of
fixargs() which is used on Linux and macOS, if the new command line
is longer than the existing one, it is truncated. This works well
when calling a script with a shorter name.
However, it generates a misleading name in the common scenario
where a script is invoked from an interactive shell, which
typically has a short command line. For instance, if "/tmp/script"
is invoked, "ksh" gets replaced with "/tm" in "ps" output.
A solution is found in the fact that, on these systems, the
environment is stored immediately after the command line arguments.
This space can be made available for use by a longer command line
by moving the environment strings out of the way.
src/cmd/ksh93/sh/main.c: fixargs():
- Refactor BSD setproctitle(3) version to be more self-contained.
- In the generic (Linux/macOS) version, on init (i.e. mode==0), if
the command line is smaller than 128 bytes and the environment
strings have not yet been moved (i.e. if they still immediately
follow the command line arguments in memory), then strdup the
environment strings, pointing the *environment[] members to the
new strings and adding the length of the strings to the maximum
command line buffer size.
Reported-by: @gkamat
Resolves: https://github.com/ksh93/ksh/pull/300
ksh93 currently has three command substitution mechanisms:
- type 1: old-style backtick comsubs that use a pipe;
- type 3: $(modern) comsubs that use a temp file, currently with
fallback to a pipe if a temp file cannot be created;
- type 2: ${ shared-state; } comsubs; same as type 3, but shares
state with parent environment.
Type 1 is buggy. There are at least two reproducers that make it
hang. The Red Hat patch applied in 4ce486a7 fixed a hang in
backtick comsubs but reintroduced another hang that was fixed in
ksh 93v-. So far, no one has succeeded in making pipe-based comsubs
work properly.
But, modern (type 3) comsubs use temp files. How does it make any
sense to have two different command substitution mechanisms at the
execution level? The specified functionality between backtick and
modern command substitutions is exactly the same; the difference
*should* be purely syntactic.
So this commit removes the type 1 comsub code at the execution
level, treating them all like type 3 (or 2). As a result, the
related bugs vanish while the regression tests all pass.
The only side effect that I can find is that the behaviour of bug
https://github.com/ksh93/ksh/issues/124 changes for backtick
comsubs. But it's broken either way, so that's neutral.
So this commit can now be added to my growing list of ksh93 issues
fixed by simply removing code.
src/cmd/ksh93/sh/xec.c:
- Remove special code for type 1 comsubs from iousepipe(),
sh_iounpipe(), sh_exec() and _sh_fork().
src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/subshell.c:
- Remove pipe support from sh_subtmpfile(). This also removes the
use of a pipe as a fallback for $(modern) comsubs. Instead, panic
and error out if temp file creation fails. If the shell cannot
create a temporary file, there are fatal system problems anyway
and a script should not continue.
- No longer pass comsub type to sh_subtmpfile().
All other changes:
- Update sh_subtmpfile() calls.
src/cmd/ksh93/tests/subshell.sh:
- Add two regression tests based on reproducers from bug reports.
Resolves: https://github.com/ksh93/ksh/issues/305
Resolves: https://github.com/ksh93/ksh/issues/316
This upstreams the patch 'src__cmd__ksh93__sh__array.c.diff' from
Apple's ksh 93u+ distribution in ksh-28.tar.gz:
https://opensource.apple.com/tarballs/ksh/
src/cmd/ksh93/sh/array.c: array_putval(), nv_associative():
- Zero two table pointers after closing/freeing the tables with
libast's dtclose(). No information is available from Apple as to
what specific problems this fixes, but at worst this is harmless.
I don't expect anyone else to actually use ksh93 on a museum-grade
Power Mac G5 running Mac OS X 10.3.7, but ancient platforms are
great bug and compatibility testing tools. These tweaks restore the
ability to build on that platform.
Also, to avoid a strange path search bug on that platform and
possibly other ancient ones, set SHOPT_DYNAMIC to 0 in SHOPT.sh.
Bug: [[ ! ! 1 -eq 1 ]] returns false, but should return true.
This bug was reported for bash, but ksh has it too:
https://lists.gnu.org/archive/html/bug-bash/2021-06/msg00006.html
Op 24-05-21 om 17:47 schreef Chet Ramey:
> On 5/22/21 2:45 PM, Vincent Menegaux wrote:
>> Previously, these commands:
>>
>> [[ ! 1 -eq 1 ]]; echo $?
>> [[ ! ! 1 -eq 1 ]]; echo $?
>>
>> would both result in `1', since parsing `!' set CMD_INVERT_RETURN
>> instead of toggling it.
>
> Interestingly, ksh93 produces the same result as bash. I agree
> that it's more intuitive to toggle it.
Also interesting is that '!' as an argument to the simple
'test'/'[' command does work as expected (on both bash and ksh93):
'test ! ! 1 -eq 1' and '[ ! ! 1 -eq 1 ]' return 0/true.
Even the man page for [[ is identical for bash and ksh93:
| ! expression
| True if expression is false.
This suggests it's supposed to be a logical negation operator, i.e.
'!' is implicitly documented to negate another '!'. Bolsky & Korn's
1995 ksh book, p. 167, is slightly more explicit about it:
"! test-expression. Logical negation of test-expression."
I also note that multiple '!' negators in '[[' work as expected on
mksh, yash and zsh.
src/cmd/ksh93/sh/parse.c: test_primary():
- Fix bitwise logic for '!': xor the TNEGATE bit into tretyp
instead of or'ing it, which has the effect of toggling it.
The memory leak regression tests added in commit 05683ec7 only leak memory
in the C.UTF-8 locale if ksh is compiled with vmalloc. I've ran these
regression tests against ksh93v- and neither fail in that version of
ksh, which indicates the bug causing these tests to fail may be similar to
the one that causes <https://github.com/ksh93/ksh/issues/95>.
Since the memory leak tests work with -D_std_malloc, only set $LANG to
'C' if ksh is compiled with vmalloc enabled.
This regression also exists on ksh 93v- and ksh2020, from which it
was backported.
Reproducer:
$ (fn() { true; }; fn >/dev/null/ne; true) 2>/dev/null; echo $?
1
Expected output: 0 (as on ksh 93u+).
FreeBSD sh and NetBSD sh are the only other known shells that share
this behaviour. POSIX currently allows both behaviours, but may
require the ksh 93u+ behaviour in future. In any case, this causes
an incompatibility with established ksh behaviour that could easily
break existing ksh scripts.
src/cmd/ksh93/sh/xec.c: sh_exec():
- Commit 23f2e23 introduced a check for jmpval > SH_JMPIO (5).
When a function call pushes context for a redirection, this is
done with the jmpval exit value of SH_JMPCMD (6). Change that to
SH_JMPIO to avoid triggering that check.
src/cmd/ksh93/tests/exit.sh:
- Add regression tests for exit behaviour on various kinds of
shell errors as listed in the POSIX standard, including an error
in a redirection of a function call.
Fixes: https://github.com/ksh93/ksh/issues/310
Problem:
$ exec ksh
$ echo $SHLVL
2
$ exec ksh
$ echo $SHLVL
3
$ exec ksh
$ echo $SHLVL
4
...etc. SHLVL is supposed to acount the number of shell processes
that you need to exit before you get logged out. Since ksh was
replacing itself with a new shell in the same process using 'exec',
SHLVL should not increase.
src/cmd/ksh93/bltins/misc.c: b_exec():
- When about to replace the shell and we're not in a subshell,
decrease SHLVL to cancel out a subsequent increase by the
replacing shell. Bash and zsh also do this.
On NetBSD, for some reason, the wctrans(3) and towctrans(3) C
library functions exist, but have no effect; the "toupper" and
"tolower" maps don't even translate case for ASCII, never mind wide
characters. This kills 'typeset -u' and 'typeset -l' on ksh, which
was the cause of most of the regression test failures on NetBSD.
Fallback versions for these functions are provided in init.c, but
were not being used on NetBSD because the feature test detected the
presence of these functions in the C library.
src/cmd/ksh93/features/locale:
- Replace the simple test for the presence of wctrans(3),
towctrans(3), and the wctrans_t type by an actual feature test
that checks that these functions not only compile, but are also
capable of changing an ASCII 'q' to upper case and back again.
src/cmd/ksh93/sh/init.c: towctrans():
- Add wide character support to the fallback function, for whatever
good that may do; on NetBSD, the wide-character towupper(3) and
towlower(3) functions only change case for ASCII.
After the last increase from 4 to 6 bytes, there are still
intermittent false leaks.sh failures (different ones on each run)
on the GitHub CI runner on the 1.0 branch, which is compiled with
the OS's malloc (as opposed to ast vmalloc). Increase the byte
tolerance for the leaks test from 6 to 8 bytes on Linux when
compiling with standard malloc.
The last commit still failed to build on macOS M1. That va_listval
macro keeps causing trouble. It's an AST thing that is defined in
src/lib/libast/features/common. That looks like some incredibly
opaque attempt to make it compatible with everything, and clearly
it no longer works properly on all systems. I don't dare touch it,
though. That code looks like any minimal change will probably break
the build on some system or other.
src/lib/libast/features/hack:
- Add feature test to check if that macro needs (0) no workaround,
or (1) the workaround from the 93v- beta, or (2) the FreeBSD one.
Whichever version compiles first, it will use. If none does, this
test will not output a value, which will be treated as 0.
src/lib/libast/hash/hashalloc.c,
src/lib/libast/string/tokscan.c:
- Update to use the result of the hack feature test.
src/lib/libast/Mamfile:
- Update for new #include dependencies.
src/cmd/ksh93/tests/{shtests,_common}:
- When xtrace is active, set SECONDS to the float type so that
the $SECONDS expansion in $PS4 shows fractional seconds.
src/cmd/ksh93/tests/*.sh:
- Various fixes to avoid command substitutions incorporating xtrace
output into their results. Sometimes this is done by avoiding a
preceding assignment on a command that redirects 2>&1 (as that
will also redirect the preceding assignment and its xtrace,
causing the command substitution to capture the xtrace); other
times it was easiest to just turn off xtrace outright within the
command substitution.
src/cmd/ksh93/tests/math.sh:
- Remove an obsolete 'fixme' note.
There are intermittent false failures on the GitHub CI runners on
the 1.0 branch, which is compiled with the OS's malloc (as opposed
to ast vmalloc). Increase the byte tolerance for the leaks test
from 4 to 6 bytes on Linux when compiling with standard malloc.
hyenias writes, re the referenced commit:
> This has caused my Ubuntu 18.04 ARMv7 to fail to compile.
>
> /dev/shm/ksh/src/lib/libast/hash/hashalloc.c: In function 'hashalloc':
> /dev/shm/ksh/src/lib/libast/hash/hashalloc.c:156:11: error:
> incompatible types when assigning to type 'va_list * {aka
> __va_list *}' from type 'va_list {aka __va_list}'
> tmpval = va_listval(va_arg(ap, va_listarg));
> ^
> In file included from ./ast_common.h:192:0,
> from /dev/shm/ksh/src/lib/libast/include/ast_std.h:37,
> from /dev/shm/ksh/src/lib/libast/include/ast.h:36,
> from /dev/shm/ksh/src/lib/libast/hash/hashlib.h:34,
> from /dev/shm/ksh/src/lib/libast/hash/hashalloc.c:33:
> /dev/shm/ksh/src/lib/libast/hash/hashalloc.c:157:16: error:
> incompatible type for argument 2 of '__builtin_va_copy'
> va_copy(ap, tmpval);
> ^
> /dev/shm/ksh/src/lib/libast/hash/hashalloc.c:157:16: note: expected
> '__va_list' but argument is of type 'va_list * {aka __va_list *}'
> mamake [lib/libast]: *** exit code 1 making hashalloc.o
> mamake: *** exit code 1 making lib/libast
> mamake: *** exit code 1 making all
> package: make done at Fri May 14 06:10:16 EDT 2021 in
> /dev/shm/ksh/arch/linux.arm
src/lib/libast/hash/hashalloc.c,
src/lib/libast/string/tokscan.c:
- Revert the FreeBSD fix.
- Backport a conditional workaround for clang from ksh 93v- beta.
On some systems, the following won't compile because of the way the
macros are defined in the system headers:
va_copy(ap, va_listval(va_arg(ap, va_listarg)));
The error from clang is something like:
.../hashalloc.c:155:16: error: non-const lvalue reference to type
'__builtin_va_list' cannot bind to a temporary of type 'va_list'
(aka 'char *')
va_copy(ap, va_listval(va_arg(ap, va_listarg)));
~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./ast_common.h:200:23: note: expanded from macro 'va_listval'
#define va_listval(p) (p)
^
.../include/stdarg.h:27:53: note: expanded from macro 'va_copy'
#define va_copy(dest, src) __builtin_va_copy(dest, src)
^~~
1 error generated.
mamake [lib/libast]: *** exit code 1 making hashalloc.o
This commit backports a FreeBSD build fix from:
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=255308
Thanks to Chase <nicetrynsa@protonmail.ch> for the bug report.
src/lib/libast/hash/hashalloc.c,
src/lib/libast/string/tokscan.c:
- Store va_listval() result in variable and pass that to va_copy().
Since a command substitution no longer forks on non-permanently
redirecting standard output within it for a specific command,
test -t 1, [ -t 1 ], and [[ -t 1 ]] broke as follows:
v=$(test -t 1 >/dev/tty && echo ok) did not assign 'ok' to v.
This is because the assumption in tty_check() that standard output
is never on a terminal in a non-forked command substitution, added
in 55f0f8ce, was made invalid by 090b65e7.
src/cmd/ksh93/edit/edit.c: tty_check():
- Implement a new method. Return false if the file descriptor
stream is of type SF_STRING, which is the case for non-forked
command substitutions -- it means the sfio stream writes directly
into a memory area. This can be checked with the sfset(3)
function (see src/lib/libast/man/sfio.3). To avoid a segfault
when accessing sh.sftable, we need to validate the FD first.
src/cmd/ksh93/tests/pty.sh:
- Add the above reproducer.
The bitmask of attributes to export was repeatedly defined in three
different places, and that fix changed only one of them.
src/cmd/ksh93/sh/name.c:
- Single point of truth: define ATTR_TO_EXPORT macro with the
bitmask of all the attributes to export (excluding NV_RDONLY).
- attstore(), pushnam(), sh_envgen(): Use the ATTR_TO_EXPORT macro,
removing superflous NV_RDONLY handling from the former two.
Commit 92f7ca54 broke compilation with tcc on Linux. The following
error would occur while compiling ksh with tcc:
In file included from /home/johno/GitRepos/KornShell/ksh/src/cmd/ksh93/data/strdata.c:105:
./FEATURE/math:91: error: too many basic types
mamake [cmd/ksh93]: *** exit code 1 making strdata.o
The build failure is fixed by backporting the relevant bugfix from
the 93v- version of iffe.
src/cmd/INIT/iffe.sh:
- Backport the 2013 iffe bugfix for the intrinsic function test to
rule out type names (dated 2013-08-11 in the 93v- changelog).
The build started failing on Solaris Studio cc when 'noreturn' was
introduced, because the wrappers pass the -xc99 flag which sets the
compiler to C99 mode. 'noreturn' is a C11 feature. The
stdnoreturn.h header was correctly included but the compiler still
threw a syntax error (long path abbreviated below):
".../stk.c", line 124: warning: _Noreturn is a keyword in ISO C11
".../stk.c", line 124: warning: old-style declaration or incorrect
type for: _Noreturn
".../stk.c", line 124: syntax error before or at: static
src/cmd/INIT/cc.sol11.*:
- Pass -std=c11 to cc instead of -xc99. At least on i386-64, this
is sufficient to fix the build.
README.md, src/cmd/ksh93/README.md:
- Remove -xc99 from the Solaris build flags example as that is
incompatible with -std=c11 (and was already redundant with the
-xc99 in the wrappers).
src/cmd/ksh93/tests/basic.sh:
- Don't run a newly backported 93v- regression test on Solaris
because it uses the 'join' command with process subsitutions;
Solaris 11.4's join(1) hangs when trying to read from /dev/fd.
This is not ksh's fault. (re: 59bacfd4)
On Fedora, this regression test failure occurs:
locale.sh[84]: 'read' doesn't skip multibyte input
correctly (ja_JP.ujis, \x95\x5c)
This is a problem with the test; this Shift-JIS specific test
should not be run in a non-Shift-JIS locale. So this commit skips
it unless the locale string ends in '.SJIS' (case insensitive).
It also adds cleanup for the 'chr' variable's special attributes
in case that name is ever going to be used in another test.
nmake was removed long ago (2940b3f5) and so were the outdated
Makefiles (6cc2f6a0). However, the build system still looked for an
AT&T nmake in $PATH. If a user had it installed, the build would
fail as the system tried to use it.
https://groups.google.com/g/korn-shell/c/2VK8kS0_VhA/m/-Rlnv7PRAgAJ
bin/package, src/cmd/INIT/package.sh:
- Remove all the code supporting nmake.
- Make 'bin/package test' work by simply exec'ing bin/shtests.
src/cmd/INIT/Mamfile:
- Do not install *.mk nmake support files.
lib/package/*.mk, src/cmd/INIT/*.mk:
- nmake support files removed.
src/cmd/ksh93/sh.1:
- The POSIX option description still said that attributes "such as
integer and readonly" aren't imported from the environment. But
as of 7954855f, the readonly attribute is never imported or
exported. So change that to another example (left/right justify).
- Tweak idiosyncratic use of hyphens.
- be inputted => be input.
In May 2020, when every KornShell (ksh93) development project was
abandoned, development was rebooted in a new fork based on the last
stable AT&T version: ksh 93u+. Now, one year and hundreds of bug
fixes later, the first beta version is ready, and KornShell lives
again. This new fork is called ksh 93u+m as a permanent nod to its
origin; a standard semantic version number is added starting at
1.0.0-beta.1. Please test the beta and report any bugs you find,
or help us fix known bugs.
src/cmd/ksh93/data/math.tab:
- Added exp10().
- Remove int() as being an alias to floor().
- Created entries for local float() and local int() which are
defined in features/math.sh.
src/cmd/ksh93/features/math.sh:
- Backport floor() and int() related code from ksh93v-.
src/cmd/ksh93/sh.1:
- Sync man page to math.tab's potential functions.
src/cmd/ksh93/sh/xec.c: sh_exec(): TCOM:
- In the referenced commit I'd accidentally deleted this line:
shgd->current_pid = getpid();
from the routine to optimise the ( simple_command & ) case.
This resulted in the following regression test failure on
ARM boxes:
variables.sh[71]: Test 4: $RANDOM seed in ( simple_command & )
The cause was that the current PID shgd->current_pid, which is
factored into the seed, was not updated before reseeding.
Apparently the system clock on ARM systems is not fine-grained
enough to compensate.
This adds a #pragma to disable -Wdeprecated-register* on newer
versions of clang. We could remove all use of the register keyword
instead, as modern compilers ignore it. But it's not harmful, and
for the time being I prefer not to do doing any reformatting or
changing the historic character of this code base.
The #pragmas are removed from src/lib/libast/include/ast.h, because
they're better placed in src/lib/libast/features/common which
generates ast_common.h which is included by everything.
* https://clang.llvm.org/docs/DiagnosticsReference.html#wdeprecated-register
src/cmd/ksh93/tests/builtins.sh:
- An original AT&T test for 'read -s' was disabled and marked
FIXME. Fix the invalid invocation and check that 'read -s'
actually writes to the history file.
- Remove a temporary 'command -p ls' debug test that I accidentally
committed (re: a197b042).
I did not realize that lvalue->nosub and lvalue->sub variables are
not reset when another assignment occurs later down the line.
Example: (( arr[0][1]+=1, arr[2]=7 ))
src/cmd/ksh93/sh/arith.c: arith():
- For assignment operations, reset lvalue's nosub and sub variables
so the target for the next assignment is not redirected.
src/cmd/ksh93/tests/arrays2.sh:
- Add in a few regression tests that utilize compound arithmetic
expressions having at least an assignment operation (+=) followed
by a normal assignment (=).
BUG 1: Though 'command' is specified/documented as a regular
builtin, preceding assignments survive the invocation (as with
special or declaration builtins) if 'command' has no command
arguments in these cases:
$ foo=wrong1 command; echo $foo
wrong1
$ foo=wrong2 command -p; echo $foo
wrong2
$ foo=wrong3 command -x; echo $foo
wrong3
Analysis: sh_exec(), case TCOM (simple command), contains the
following loop that skips over 'command' prefixes, preparsing any
options and remembering the offset in the 'command' variable:
src/cmd/ksh93/sh/xec.c
1059 while(np==SYSCOMMAND || !np && com0
&& nv_search(com0,shp->fun_tree,0)==SYSCOMMAND)
1060 {
1061 register int n = b_command(0,com,&shp->bltindata);
1062 if(n==0)
1063 break;
1064 command += n;
1065 np = 0;
1066 if(!(com0= *(com+=n)))
1067 break;
1068 np = nv_bfsearch(com0, shp->bltin_tree, &nq, &cp);
1069 }
This skipping is not done if the preliminary b_command() call on
line 1061 (with argc==0) returns zero. This is currently the case
for command -v/-V, so that 'command' is treated as a plain and
regular builtin for those options.
The cause of the bug is that this skipping is even done if
'command' has no arguments. So something like 'foo=bar command' is
treated as simply 'foo=bar', which of course survives.
So the fix is for b_command() to return zero if there are no
arguments. Then b_command() itself needs changing to not error out
on the second/main b_command() call if there are no arguments.
src/cmd/ksh93/bltins/whence.c: b_command():
- When called with argc==0, return a zero offset not just for -v
(X_FLAG) or -V (V_FLAG), but also if there are no arguments left
(!*argv) after parsing options.
- When called with argc>0, do not issue a usage error if there are
no arguments, but instead return status 0 (or, if -v/-V was given,
status 2 which was the status of the previous usage message).
This way, 'command -v $emptyvar' now also works as you'd expect.
BUG 2: 'command -p' sometimes failed after executing certain loops.
src/cmd/ksh93/sh/path.c: defpath_init():
- astconf() returns a pointer to memory that may be overwritten
later, so duplicate the string returned. Backported from ksh2020.
(re: f485fe0f, aa4669ad, <https://github.com/att/ast/issues/959>)
src/cmd/ksh93/tests/builtins.sh:
- Update the test for BUG_CMDSPASGN to check every variant of
'command' (all options and none; invoking/querying all kinds of
command and none) with a preceding assignment. (re: fae8862c)
This also covers bug 2 as 'command -p' was failing on macOS prior
to the fix due to a loop executed earlier in another test.
@JohnoKing writes:
> In emacs mode, using Alt+D or Alt+H with a repeat parameter
> results in the deletion of extra characters. Reproducer:
>
> $ set -o emacs
> $ foo bar delete add # <Ctrl+A> <ESC+3+Alt+D>
> $ d # Should be ' add'
>
> $ foo bar delete add # <ESC+3+Alt+H>
> $ f # Should be 'foo '
>
> [...] this bug also affects the Delete and Arrow keys [...].
> Reproducer:
>
> $ test_string <Ctrl+A> <ESC+3+Delete>
> # This will delete all of 'test', which is four characters
> $ test_string <Ctrl+A> <ESC+4+Right Arrow>
> # This should move the cursor to '_', not 's'
src/cmd/ksh93/edit/emacs.c: ed_emacsread():
- Revert part of 29b11bba: once again set 'count' to
'vt220_save_repeat' instead of adding the value.
- do_escape: If the escape() function (which handles both ESC
repeat counts and commands like ESC d and ESC h) returns a repeat
count, do not use the saved repeat count for v220 sequences.
src/cmd/ksh93/tests/pty.sh:
- Test the four reproducers above.
Fixes: https://github.com/ksh93/ksh/issues/292
This PR corrects #168 for indexed arrays having more than one
level. Turns out ksh was only keeping track of the subscript number
for assignment in lvalue's nosub variable. By saving the actual
subscript reference, the result can be assigned to its proper
destination instead of putting the result into the last looked
value or subscript location.
src/cmd/ksh93/include/streval.h: struct lval:
- Create a new pointer named sub to hold the reference that nosub
describes.
src/cmd/ksh93/sh/arith.c: arith():
- Adjust LOOKUP: for lvalue ARITH_ASSIGNOP operations on indexed
arrays to save the np of the destination subscript for later use.
- Adjust ASSIGN: to act when lvalue's nosub > 0 which happens as
the last step in the arithmetic parsing loop for assignment
operations. Only indexed arrays will have a nosub value > 0. All
others have a nosub of 0 unless they are involved in a unary
operation (++, --) which sets nosub to -1. All said in the
context of assignment operations like (( arr[0][1] += 1 )).
src/cmd/ksh93/sh/streval.c:
- Initialize the new sub pointer to 0.
src/cmd/ksh93/tests/arrays2.sh:
- Created a few multidimensional indexed array tests for assignment
operations like += as an example.
Resolves: https://github.com/ksh93/ksh/issues/168
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.
