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.
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 <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
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: <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/14044ba443cfd
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
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>
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
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;
}
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
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
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'.
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.
'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.
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
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
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
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 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
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.
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.
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.
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.
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.
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
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)
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).
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
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
