This fixes the following:
1. 'set --posix' now works as an equivalent of 'set -o posix'.
2. The posix option turns off braceexpand and turns on letoctal.
Any attempt to override that in a single command such as 'set -o
posix +o letoctal' was quietly ignored. This now works as long
as the overriding option follows the posix option in the command.
3. The --default option to 'set' now stops the 'posix' option, if
set or unset in the same 'set' command, from changing other
options. This allows the command output by 'set +o' to correctly
restore the current options.
src/cmd/ksh93/data/builtins.c:
- To make 'set --posix' work, we must explicitly list it in
sh_set[] as a supported option so that AST optget(3) recognises
it and won't override it with its own default --posix option,
which converts the optget(3) string to at POSIX getopt(3) string.
This means it will appear as a separate entry in --man output,
whether we want it to or not. So we might as well use it as an
example to document how --optionname == -o optionname, replacing
the original documentation that was part of the '-o' description.
src/cmd/ksh93/sh/args.c: sh_argopts():
- Add handling for explitit --posix option in data/builtins.c.
- Move SH_POSIX syncing SH_BRACEEXPAND and SH_LETOCTAL from
sh_applyopts() into the option parsing loop here. This fixes
the bug that letoctal was ignored in 'set -o posix +o letoctal'.
- Remember if --default was used in a flag, and do not sync options
with SH_POSIX if the flag is set. This makes 'set +o' work.
src/cmd/ksh93/include/argnod.h,
src/cmd/ksh93/data/msg.c,
src/cmd/ksh93/sh/args.c: sh_printopts():
- Do not potentially translate the 'on' and 'off' labels in 'set
-o' output. No other shell does, and some scripts parse these.
src/cmd/ksh93/sh/init.c: sh_init():
- Turn on SH_LETOCTAL early along with SH_POSIX if the shell was
invoked as sh; this makes 'sh -o' and 'sh +o' show expected
options (not that anyone does this, but correctness is good).
src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/include/shell.h:
- The state flags were in defs.h and most (but not all) of the
shell options were in shell.h. Gather all the shell state and
option flag definitions into one place in shell.h for clarity.
- Remove unused SH_NOPROFILE and SH_XARGS option flags.
src/cmd/ksh93/tests/options.sh:
- Add tests for these bugs.
src/lib/libast/misc/optget.c: styles[]:
- Edit default optget(3) option self-documentation for clarity.
Several changed files:
- Some SHOPT_PFSH fixes to avoid compiling dead code.
If I haven't missed anything, this should make the non-interactive
aspects of job control in scripts work as expected, except for the
"<command unknown>" issue in the output of 'bg', 'fg' and 'jobs'
(which is not such a high priority as those commands are really
designed for interactive use).
Plus, I believe I now finally understand what these three are for:
* The job.jobcontrol variable is set to nonzero by job_init() in
jobs.c if, and only if, the shell is interactive *and* managed to
get control of the terminal. Therefore, any changing of terminal
settings (tcsetpgrp(3), tty_set()) should only be done if
job.jobcontrol is nonzero. This commit changes several checks for
sh_isoption(SH_INTERACTIVE) to checks for job.jobcontrol for
better consistency with this.
* The state flag, sh_isstate(SH_MONITOR), determines whether the
bits of job control that are relevant for both scripts and
interactive shells are active, which is mostly making sure that a
background job gets its own process group (setpgid(3)).
* The shell option, sh_isoption(SH_MONITOR), is just that. When the
user turns it on or off, the state flag is synched with it. It
should usually not be directly checked for, as the state may be
temporarily turned off without turning off the option.
Prior discussion:
https://www.mail-archive.com/austin-group-l@opengroup.org/msg06456.html
src/cmd/ksh93/bltins/typeset.c, src/cmd/ksh93/sh/args.c:
- Move synching the SH_MONITOR state flag with the SH_MONITOR
shell option from b_set() (the 'set' builtin) to sh_applyopts()
which is indirectly called from b_set() and is also used when
parsing the shell invocation command line. This ensures -m is
properly enabled in both scenarios.
src/cmd/ksh93/sh/jobs.c:
- job_init(): Do not refuse to initialise job control on
non-interactive shells. Instead, skip everything that should only
be done on interactive shells (i.e., everything to do with the
terminal). This function is now even more of a mess than it was
before, so refactoring may be desirabe at some point.
- job_close(), job_set(), job_reset(), job_wait(): Do not reset the
terminal process group (tcsetpgrp()) if job.jobcontrol isn't on.
src/cmd/ksh93/sh/xec.c:
- sh_exec(): TFORK: For SIGINT handling, check the SH_MONITOR
state flag, not the shell option.
- sh_exec(): TFORK: Do not turn off the SH_MONITOR state flag in
forked children. The non-interactive part of job control should
stay active. Instead, turn off the SH_INTERACTIVE state flag so
we don't get interactive shell behaviour (i.e. job control noise
on the terminal) in forked subshells.
- _sh_fork(), sh_ntfork(): Do not reset the terminal process group
(tcsetpgrp()) if job.jobcontrol isn't on. Do not turn off the
SH_MONITOR state flag in forked children.
src/cmd/ksh93/sh/subshell.c: sh_subfork():
- Do not turn off the monitor option and state in forked subshells.
The non-interactive part of job control should stay active.
src/cmd/ksh93/bltins/misc.c: b_bg():
- Check isstate(SH_MONITOR) instead of sh_isoption(SH_MONITOR) &&
job.jobcontrol before throwing a 'no job control' error.
This fixes a minor bug: fg, bg and disown could quietly fail.
src/cmd/ksh93/tests/jobs.sh:
- Add tests for 'fg' with job control IDs (%%, %1) in scripts.
- Add test checking that a background job launched from a subsell
with job control enabled correctly becomes the leader of its own
process group.
Makes progress on: https://github.com/ksh93/ksh/issues/119
Another longstanding whopper of a bug in basic ksh93 functionality:
run a ${ shared-state; } command substitution twice and job control
promptly loses track of all your running jobs. New jobs are tracked
again until you run another two shared-state command substitutions.
This is in at least 93t+, 93u-, 93u+, 93v- and ksh2020.
$ sleep 300 &
[1] 56883
$ jobs # OK
[1] + Running sleep 300 &
$ v=${ echo hi1; }
$ jobs # OK
[1] + Running sleep 300 &
$ v=${ echo hi2; }
$ jobs # Nothing!
$ fg
ksh: fg: no such job
src/cmd/ksh93/sh/subshell.c: sh_subshell():
- The current environment number shp->curenv (a.k.a. sh.curenv) was
not being restored if the virtual subshell we're leaving is of
the shared-state command substitution variety as it was wrongly
considered to be part of the environment that didn't need
restoring. This caused it to be out of sync with shp->jobenv
(a.k.a. sh.jobenv) which did get restored from savedcurenv.
Restore both from savedcurenv at the same time for any subshell.
(How these numbers are used exactly remains to be discovered.)
src/cmd/ksh93/tests/jobs.sh:
- Added, with a test for this bug to start it off. There is no
other test script where job control fits, and a lot more related
fixes are anticipated: https://github.com/ksh93/ksh/issues/119
This is some nonsense: redirections that store a file descriptor
greater than 9 in a variable, like {var}<&2 and the like, stopped
working if brace expansion was turned off. '{var}' is not a brace
expansion as it doesn't contain ',' or '..'; something like 'echo
{var}' is always output unexpanded. And redirections and brace
expansion are two completely unrelated things. It wasn't documented
that these redirections require the -B/braceexpand option, either.
src/cmd/ksh93/sh/lex.c: sh_lex():
- Remove incorrect check for braceexpand option before processing
redirections of this form.
src/cmd/ksh93/COMPATIBILITY:
- Insert a brief item mentioning this.
src/cmd/ksh93/sh.1:
- Correction: these redirections do not yield a file descriptor >
10, but > 9, a.k.a. >= 10.
- Add a brief example showing how these redirections can be used.
src/cmd/ksh93/tests/io.sh:
- Add a quick regression test.
This commit corrects how shortint was being applied to various
possible typeset variables in error. The short integer option
modifier 'typeset -s' should only be able to be applied if the
the variable is also an integer. Several issues were resolved
with this fix:
- 'typeset -s': created a short integer having an invalid base
of zero. 'typeset -s foo' created 'typeset -s -i 0 foo=0' and
now will result in an empty string.
- 'typeset -sL': previously resulted in a segmentation fault.
The following are the various incorrect 'typeset' instances
that have been fixed:
$ 'export foo; typeset -s foo; readonly foo; typeset -p foo'
(before) typeset -x -r -s -i 0 foo=0
( after) typeset -x -r foo
$ 'typeset -sL foo=1*2; typeset -p foo'
(before) Segmentation fault (core dumped)
( after) typeset -L 3 foo='1*2'
$ 'typeset -sR foo=1*2; typeset -p foo'
(before) typeset -s -i foo=2
( after) typeset -R 3 foo='1*2'
$ 'typeset -sZ foo=1*2; typeset -p foo'
(before) typeset -F 0 foo=2
( after) typeset -Z 3 -R 3 foo='1*2'
src/cmd/ksh93/bltins/typeset.c: b_typeset():
- Add conditional check within the 's' option to only
apply NV_SHORT as well as remove any NV_LONG flag
if NV_INTEGER flag was set.
- Relocate shortint conditional logic to the 'i' option.
src/cmd/ksh93/tests/attributes.sh:
- Adjust regression tests for '-s' and add '-si' check.
This fixes part of https://github.com/ksh93/ksh/issues/87:
Scalar arrays (-a) and associative arrays (-A) of a type created by
'enum' did not consistently block values not specified by the enum
type, yielding corrupted results.
An expansion of type "${array[@]}" yielded random numbers instead
of values for associative arrays of a type created by 'enum'.
This does not yet fix another problem: ${array[@]} does not yield
all values for associative enum arrays.
src/cmd/ksh93/bltins/enum.c: put_enum():
- Always throw an error if the value is not in the list of possible
values for an enum type. Remove incorrect check for the NV_NOFREE
flag. Whatever that was meant to accomplish, I've no idea.
src/cmd/ksh93/sh/array.c: nv_arraysettype():
- Instead of sh_eval()ing a shell assignment, use nv_putval()
directly. Also use the stack (see src/lib/libast/man/stk.3)
instead of malloc to save the value; it's faster and will be
auto-freed at some point. This shortens the function and makes it
faster by not entering into a whole new shell context -- which
also fixes another problem: the error message from put_enum()
didn't cause the shell to exit for indexed enum arrays.
src/cmd/ksh93/sh/name.c: nv_setlist():
- Apply a patch from David Korn that correctly sets the data type
for associative arrays, fixing the ${array[@]} expansion yielding
random numbers. Thanks to @JohnoKing for the pointer.
https://github.com/ksh93/ksh/issues/87#issuecomment-662613887https://www.mail-archive.com/ast-developers@lists.research.att.com/msg00697.html
src/cmd/ksh93/tests/enum.sh:
- Add tests checking that invalid values are correctly blocked for
indexed and associative arrays of an enum type.
Makes progress on: https://github.com/ksh93/ksh/issues/87
This commit fixes 'command -x' to adapt to OS limitations with
regards to data alignment in the arguments list. A feature test is
added that detects if the OS aligns the argument on 32-bit or
64-bit boundaries or not at all, allowing 'command -x' to avoid
E2BIG errors while maximising efficiency.
Also, as of now, 'command -x' is a way to bypass built-ins and
run/query an external command. Built-ins do not limit the length of
their argument list, so '-x' never made sense to use for them. And
because '-x' hangs on Linux and macOS on every ksh93 release
version to date (see acf84e96), few use it, so there is little
reason not to make this change.
Finally, this fixes a longstanding bug that caused the minimum exit
status of 'command -x' to be 1 if a command with many arguments was
divided into several command invocations. This is done by replacing
broken flaggery with a new SH_XARG state flag bit.
src/cmd/ksh93/features/externs:
- Add new C feature test detecting byte alignment in args list.
The test writes a #define ARG_ALIGN_BYTES with the amount of
bytes the OS aligns arguments to, or zero for no alignment.
src/cmd/ksh93/include/defs.h:
- Add new SH_XARG state bit indicating 'command -x' is active.
src/cmd/ksh93/sh/path.c: path_xargs():
- Leave extra 2k in the args buffer instead of 1k, just to be sure;
some commands add large environment variables these days.
- Fix a bug in subtracting the length of existing arguments and
environment variables. 'size -= strlen(cp)-1;' subtracts one less
than the size of cp, which makes no sense; what is necessary is
to substract the length plus one to account for the terminating
zero byte, i.e.: 'size -= strlen(cp)+1'.
- Use the ARG_ALIGN_BYTES feature test result to match the OS's
data alignment requirements.
- path_spawn(): E2BIG: Change to checking SH_XARG state bit.
src/cmd/ksh93/bltins/whence.c: b_command():
- Allow combining -x with -p, -v and -V with the expected results
by setting P_FLAG to act like 'whence -p'. E.g., as of now,
command -xv printf
is equivalent to
whence -p printf
but note that 'whence' has no equivalent of 'command -pvx printf'
which searches $(getconf PATH) for a command.
- When -x will run a command, now set the new SH_XARG state flag.
src/cmd/ksh93/sh/xec.c: sh_exec():
- Change to using the new SH_XARG state bit.
- Skip the check for built-ins if SH_XARG is active, so that
'command -x' now always runs an external command.
src/lib/libcmd/date.c, src/lib/libcmd/uname.c:
- These path-bound builtins sometimes need to run the external
system command by the same name, but they did that by hardcoding
an unportable direct path. Now that 'command -x' runs an external
command, change this to using 'command -px' to guarantee using
the known-good external system utility in the default PATH.
- In date.c, fix the format string passed to 'command -px date'
when setting the date; it was only compatible with BSD systems.
Use the POSIX variant on non-BSD systems.
This commit resolves the following incorrect variable assignments:
$ unset a; typeset -uF a=2; typeset -p a
typeset -X a=0x1.0000000000p+1
$ unset a; typeset -Fu a=2; typeset -p a
typeset -X a=0x1.0000000000p+1
$ unset a; typeset -ulF a=2; typeset -p a
typeset -l -X a=0x1.0000000000p+1
$ unset a; typeset -Ful a=2; typeset -p a
typeset -l -X a=0x1.0000000000p+1
$ unset a; typeset -Eu a=2; typeset -p a
typeset -E -X a=2
$ unset a; typeset -Eul a=2; typeset -p a
typeset -l -E -X a=2
src/cmd/ksh93/bltins/typeset.c:
- If the unsigned option (-u) was provided in conjunction with a
floating point (-F) then due to a flag collision with NV_UNSIGN
and NV_HEXFLOAT both having the value of NV_LTOU caused the
floating point to become a hexadecimal floating point (-X) in
error. Also, if a -E option flag was followed with a -u option
then the resulting variable would be both a scientific notation
and a hexadecimal floating point at the same time.
src/cmd/ksh93/tests/attributes.sh:
- Add regression tests.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
An unquoted variable expansion evaluated in a DEBUG trap action
caused IFS field splitting to be deactivated in code executed after
the trap action. Thanks to Koichi Nakashima for the reproducer:
| v=''
| trap ': $v' DEBUG
| A="a b c"
| set -- $A
| printf '%s\n' "$@"
|
| Expected
|
| a
| b
| c
|
| Actual
|
| a b c
src/cmd/ksh93/sh/fault.c: sh_trap():
- Remove incorrect save/restore of sh.ifstable, the internal state
table for field splitting. This reverts three lines added in ksh
93t+ 2009-11-30. Analysis: As an expansion is split into fields
(macro.c, lines 2367-2471), sh.ifstable is modified. If that
happens within a DEBUG trap, any modifications in ifstable are
undone by the restoring memccpy, leaving an inconsistent state.
src/cmd/ksh93/COMPATIBILITY:
- Document the DEBUG trap fixes, particularly the incorrect
inheritance by subshells and functions that some scripts may now
rely on because this bug is so longstanding. (re: 2a835a2d)
src/cmd/ksh93/tests/basic.sh:
- Add relevant tests.
Resolves: https://github.com/ksh93/ksh/issues/155
TODO: add a -T (-o functrace) option as in bash, which should allow
subshells and ksh-style functions to inherit DEBUG traps.
P.S.: The very handy multishell repo allows us to use 'git blame'
to trace the origin of the recently fixed DEBUG trap bugs.
The off-by-one error causing various bugs, reverted in 2a835a2d,
was introduced in ksh 93t 2008-07-25:
https://github.com/multishell/ksh93/commit/8e947ccf
(fault.c, line 321)
The incorrect check causing the exit status bug, reverted in
d00b4b39, was introduced in ksh 93t 2008-11-04:
https://github.com/multishell/ksh93/commit/b1ade268
(fault.c, line 459)
The ifstable save/restore causing the field splitting bug, reverted
in this commit, was introduced in ksh 93t+ 2009-11-30:
https://github.com/multishell/ksh93/commit/53d9f009
(fault.c, lines 440, 444, 482)
So all the bugs reported in #155 were fixed by simply reverting
these specific changes. I think that they are some experiments that
the developers simply forgot to remove. I've suspected such a thing
multiple times before. ksh93 was developed by researchers who were
genius innovators, but incredibly sloppy maintainers.
Turns out the previous commit also fixed the bug that disables the
DEBUG trap if a redirection is used in a DEBUG trap action -- in
other words, that's the same bug.
src/cmd/ksh93/tests/basic.sh:
- Add test from the reproducer in the bug report.
Makes progress on: https://github.com/ksh93/ksh/issues/155
This trap failed to be restored correctly when being trapped in
a subshell, causing corruption or a crash when restoring the
parent shell environment's trap upon leaving the subshell.
Thanks to Koichi Nakashima for the report and reproducer.
src/cmd/ksh93/sh/fault.c: sh_sigreset():
- Fix an off-by-one error in the loop that restores the
pseudosignal traps.
src/cmd/ksh93/tests/basic.sh:
- Test overwriting the main shell trap in a subshell for all
pseudosignals.
Makes progress on: https://github.com/ksh93/ksh/issues/155
The build system is adapted to make SHOPT_* compile-time options
editable without nmake. We can now easily change ksh's compile-time
options by editing src/cmd/ksh93/SHOPT.sh. The bin/package script
is adapted to turn these into compile flags. This resolves the most
important drawback of not using nmake.
Also, mamake now has support for indented Mam (Make Abstract
Machine) code. Only one type of block (make...done) is supported in
Mamfiles, so they are easy to indent automatically. A script to
(re)do this is included.
Since nmake is not going to be restored (it has too many problems
that no one is interested in fixing), this at least makes mamake
significantly easier to work with.
The Makefiles are deleted. They may still be handy for reference to
understand the Mamfiles, but they haven't actually matched the
Mamfiles for a while -- and you can still look in the git history.
Deleting them requires some adaptations to bin/package and mamake.c
because, even though they do not use those files, they still looked
for them to decide whether to build code in a directory.
Finally, this commit incorporates some #pragmas for clang to
suppress annoying warnings about the coding style used in this
historic code base. (gcc does not complain so much.)
src/cmd/ksh93/SHOPT.sh:
- Added.
bin/package, src/cmd/INIT/package.sh:
- cd into our own directory in case we were run from another dir.
- $makefiles: only look for Mamfiles.
- Add ksh compile-options via KSH_SHOPTFLAGS. Include SHOPT.sh.
- make_recurse(): Do not write a missing Makefile.
- finalize environment: Look for Mamfiles instead of Makefiles.
src/cmd/INIT/mamake.c:
- Tell clang to suppress annoying warnings about coding style.
- Update version string and self-documentation.
- input(): Add support for indented Mam code by skipping initial
whitespace on each input line.
- files[]: Instead of looking for various of Makefiles to decide
where to build, only look for Mamfiles.
src/Makefile, src/cmd/INIT/Makefile, src/cmd/Makefile,
src/cmd/builtin/Makefile, src/cmd/ksh93/Makefile, src/lib/Makefile,
src/lib/libast/Makefile, src/lib/libcmd/Makefile,
src/lib/libdll/Makefile, src/lib/libsum/Makefile:
- Removed.
src/Mamfile, src/cmd/INIT/Mamfile, src/cmd/Mamfile,
src/cmd/builtin/Mamfile, src/cmd/ksh93/Mamfile, src/lib/Mamfile,
src/lib/libast/Mamfile, src/lib/libcmd/Mamfile,
src/lib/libdll/Mamfile, src/lib/libsum/Mamfile:
- Indent the code with tabs.
- In ksh93/Mamfile, add ${KSH_SHOPT_FLAGS} to every $CC command.
- In ksh93/Mamfile, add "prev SHOPT.sh" for every *.o file
so they are rebuilt whenever SHOPT.sh changes.
bin/Mamfile_indent:
- Added, in case someone wants to re-indent a Mamfile.
src/cmd/INIT/proto.c, src/cmd/INIT/ratz.c, src/cmd/INIT/release.c,
src/lib/libast/features/common, src/lib/libast/include/ast.h:
- Tell clang to suppress annoying warnings about coding style that
it disapproves of (mainly concerning the use of parentheses).
src/cmd/INIT/cc.darwin, src/cmd/INIT/cc.freebsd,
src/cmd/INIT/cc.openbsd:
- Remove now-redundant clang warning suppression flags.
Resolves: https://github.com/ksh93/ksh/issues/60
This adds informative error messages if incompatible options are
given. It also documents the exclusive -m, -n and -T options on
separate usage lines, as was already done with -f. The usage
message for incompatible options now looks something like this:
| $ ksh -c 'typeset -L10 -F -f -i foo'
| ksh: typeset: -i/-F/-E/-X cannot be used with -L/-R/-Z
| ksh: typeset: -f cannot be used with other options
| Usage: typeset [-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: typeset -f [name...]
| Or: typeset -m [name=name...]
| Or: typeset -n [name=name...]
| Or: typeset -T [tname[=(type definition)]...]
| Help: typeset [ --help | --man ] 2>&1
(see also the previous commit, e21a053e)
Unfortunately the first "Usage" line has some redundancies with the
"Or:" lines showing separate usages. It doesn't seem to be possible
to avoid this; it's a flaw in how libast generates everything
(usage, help, manual) from one huge getopt(3) string. I still think
the three added "Or:" lines are an improvement as it wasn't
previously shown that these options need to be used on their own.
src/cmd/ksh93/bltins/typeset.c: b_typeset():
- Instead of only showing a generic usage message, add an
informative error message if incompatible options were given.
- Conflicting options detection was failing because NV_LJUST and
NV_EXPNOTE have the same bitmask value. Use a new 'isadjust'
flag for -L/-R/-Z to remember if one of these was set.
- Detect conflict between -L/-R/-Z and a float option, not just -i.
src/cmd/ksh93/include/name.h, src/cmd/ksh93/data/msg.c:
- Add the two new error messages for incompatible options.
src/cmd/ksh93/data/builtins.c: sh_opttypeset[]:
- Add a space after 'float' in in "[+float?\btypeset -lE\b]" as
this makes 'float' appear on its own line, improving formatting.
- Show -m, -n, -T on separate usage lines like -f, as none of these
can be combined with other options.
- Remove "cannot be combined with other options" from -m and -n
descriptions, as that should now be clear from the separate usage
lines -- and even if not, the error message is now informative.
src/cmd/ksh93/sh.1, src/cmd/ksh93/COMPATIBILITY:
- Update.
src/cmd/ksh93/tests/types.sh:
- Remove obsolete test: 'typeset -RF' is no longer accepted.
(It crashed in 93u+, so this is not an incompatibility...)
Resolves: https://github.com/ksh93/ksh/issues/48
This fixes the following:
trap ':' DEBUG
r=$(exit 123)
echo $? # Expected 123, but actually 0.
Thanks to Koichi Nakashima for the report and reproducer.
src/cmd/ksh93/sh/fault.c: sh_trap():
- Restore the saved current exit status (exitval) for all traps.
Do not except the DEBUG trap from doing that. I've no idea why
this exception was made, but it's not correct.
src/cmd/ksh93/tests/basic.sh:
- Add tests.
Makes progress on: https://github.com/ksh93/ksh/issues/155
This commit also further mitigates the problems with restoring an
inaccessible or nonexistent PWD on exiting a virtual subshell.
Harald van Dijk writes:
> On a build of ksh with -fsanitize=undefined to help diagnose
> problems:
>
> $ mkdir deleted
> $ cd deleted
> $ rmdir ../deleted
> $ ksh -c '(cd /; (cd /)); :'
> /home/harald/ksh/src/cmd/ksh93/sh/subshell.c:561:22: runtime
> error: null pointer passed as argument 1, which is declared to
> never be null
> Segmentation fault (core dumped)
>
> Note that it segfaults the same with default compilation flags,
> but it does not print out the useful extra message. The code
> assumes that pwd is non-null and passes it to strcmp without
> checking, but it will be null if the current directory cannot be
> determined, for instance because it has been deleted.
src/cmd/ksh93/sh/subshell.c: sh_subshell():
- Avoid the null pointer dereference reported above.
src/cmd/ksh93/bltins/cd_pwd.c: b_cd():
- Fork a virtual subshell even on systems with fchdir(2) if the
present working directory tests as inaccessible on invoking 'cd';
it may no longer exist and fchdir would fail to get a handle.
(For the test we have to opendir(3) the full path to the PWD and
not ".", as the latter may succeed even if the PWD is gone.)
src/cmd/ksh93/data/builtins.c:
- Update 'cd' version string.
Fixes: https://github.com/ksh93/ksh/issues/153
Related: https://github.com/ksh93/ksh/issues/141
The forking fix implemented in 102868f8 and 9d428f8f, which stops
the main shell's hash table from being cleared if PATH is changed
in a subshell, can cause a significant performance penalty for
certain scripts that do something like
( PATH=... command foo )
in a subshell, especially if done repeatedly. This is because the
hash table is cleared (and hence a subshell forks) even for
temporary PATH assignments preceding commands.
It also just plain doesn't work. For instance:
$ hash -r; (ls) >/dev/null; hash
ls=/bin/ls
Simply running an external command in a subshell caches the path in
the hash table that is shared with a main shell. To remedy this, we
would have to fork the subshell before forking any external
command. And that would be an unacceptable performance regression.
Virtual subshells do not need to fork when changing PATH if they
get their own hash tables. This commit adds these. The code for
alias subshell trees (which was removed in ec888867 because they
were broken and unneeded) provided the beginning of a template for
their implementation.
src/cmd/ksh93/sh/subshell.c:
- struct subshell: Add strack pointer to subshell hash table.
- Add sh_subtracktree(): return pointer to subshell hash table.
- sh_subfuntree(): Refactor a bit for legibility.
- sh_subshell(): Add code for cleaning up subshell hash table.
src/cmd/ksh93/sh/name.c:
- nv_putval(): Remove code to fork a subshell upon resetting PATH.
- nv_rehash(): When in a subshell, invalidate a hash table entry
for a subshell by creating the subshell scope if needed, then
giving that entry the NV_NOALIAS attribute to invalidate it.
src/cmd/ksh93/sh/path.c: path_search():
- To set a tracked alias/hash table entry, use sh_subtracktree()
and pass the HASH_NOSCOPE flag to nv_search() so that any new
entries are added to the current subshell table (if any) and do
not influence any parent scopes.
src/cmd/ksh93/bltins/typeset.c: b_alias():
- b_alias(): For hash table entries, use sh_subtracktree() instead
of forking a subshell. Keep forking for normal aliases.
- setall(): To set a tracked alias/hash table entry, pass the
HASH_NOSCOPE flag to nv_search() so that any new entries are
added to the current subshell table (if any) and do not influence
any parent scopes.
src/cmd/ksh93/sh/init.c: put_restricted():
- Update code for clearing the hash table (when changing $PATH) to
use sh_subtracktree().
src/cmd/ksh93/bltins/cd_pwd.c:
- When invalidating path name bindings to relative paths, use the
subshell hash tree if applicable by calling sh_subtracktree().
- rehash(): Call nv_rehash() instead of _nv_unset()ting the hash
table entry; this is needed to work correctly in subshells.
src/cmd/ksh93/tests/leaks.sh:
- Add leak tests for various PATH-related operations in the main
shell and in a virtual subshell.
- Several pre-existing memory leaks are exposed by the new tests
(I've confirmed these in 93u+). The tests are disabled and marked
TODO for now, as these bugs have not yet been fixed.
src/cmd/ksh93/tests/subshell.sh:
- Update.
Resolves: https://github.com/ksh93/ksh/issues/66
The SHOPT_2DMATCH code block in sh_setmatch() modifies the 'ap'
pointer, which is initialised as nv_arrayptr(SH_MATCHNOD). This
caused a (rarely occurring) segfault in the following line near the
end of the function:
ap->nelem -= x;
as this line assumed that 'ap' still had the initial value.
src/cmd/ksh93/sh/init.c: sh_setmatch():
- On init, save ap in ap_save and use ap_save instead of ap where
it should be pointing to SH_MATCHNOD. This also allows removing
two redundant nv_arrayptr(SH_MATCHNOD) calls, slightly increasing
the efficiency of this function.
@stephane-chazelas writes:
> Per POSIX[*], cd should skip the $CDPATH processing if the first
> component of the directory given to cd is . or ...
>
> Yet, with ksh93u+m 2021-01-03 at least, while that's OK with ..,
> it's not with . with or without the posix option:
>
> $ CDPATH=/ ./ksh -o posix -c 'cd -P ./etc && pwd'
> /etc
> /etc
>
> It seems to be a regression introduced with ksh93u+ as I can't
> reproduce it with ksh93u or any version prior to that. I can also
> reproduce in u+, v- and the ksh2020 from the Ubuntu 20.04
> package.
src/cmd/ksh93/bltins/cd_pwd.c: b_cd():
- Skip $CDPATH processing not only if the path is absolute, but
also if the initial path component is '.' or '..' (in the latter
case the $CDPATH processing was done but appeared to be a no-op).
src/cmd/ksh93/tests/builtins.sh:
- Add regression test.
[*] https://pubs.opengroup.org/onlinepubs/9699919799.2018edition/utilities/cd.html
Fixes: https://github.com/ksh93/ksh/issues/151
When starting ksh +s, it gets stuck in an infinite loop continually
trying to parse its own binary as a shell script and rejecting it:
$ arch/linux.i386-64/bin/ksh +s
arch/linux.i386-64/bin/ksh: arch/linux.i386-64/bin/ksh: cannot execute [Exec format error]
arch/linux.i386-64/bin/ksh: arch/linux.i386-64/bin/ksh: cannot execute [Exec format error]
arch/linux.i386-64/bin/ksh: arch/linux.i386-64/bin/ksh: cannot execute [Exec format error]
arch/linux.i386-64/bin/ksh: arch/linux.i386-64/bin/ksh: cannot execute [Exec format error]
arch/linux.i386-64/bin/ksh: arch/linux.i386-64/bin/ksh: cannot execute [Exec format error]
[...]
$ echo 'echo "this is stdin"' | arch/linux.i386-64/bin/ksh +s
arch/linux.i386-64/bin/ksh: arch/linux.i386-64/bin/ksh: cannot execute [Exec format error]
(no loop, but still ksh trying to parse itself)
src/cmd/ksh93/sh/init.c: sh_init():
- When forcing on the '-s' option upon finding no command
arguments, also update sh.offoptions, a.k.a. shp->offoptions.
This avoids the inconsistent state causing this problem.
In main.c, there is:
if(sh_isoption(SH_SFLAG))
fdin = 0;
else
(code to open $0 as a file)
This was entering the else block because sh_isoption(SH_SFLAG)
was returning 0, and $0 is set to the ksh binary as it is
supposed to when no other script is provided. When I looked for
why sh_isoption was returning 0, I found main.c's
for(i=0; i<elementsof(shp->offoptions.v); i++)
shp->options.v[i] &= ~shp->offoptions.v[i];
Before this loop, shp->offoptions tracks which options were
explicitly disabled by the user on the command line. The effect
of this loop is to make "explicitly disabled" take precedence
over "implicitly enabled". My patch removes the registration of
the +s option.
Fixes: https://github.com/ksh93/ksh/issues/150
Co-authored-by: Martijn Dekker <martijn@inlv.org>
Issuing typeset floating point numerics having a precision of 0
failed as the precision/size was being overwritten with the string
length of the value, e.g. 'typeset -F0 x=5.67' would result in
'typeset -F 4 x=5.6700' as len('5.67') is 4.
src/cmd/ksh93/include/nval.h:
- Created a symbolic name of NV_FLTSIZEZERO to respresent a float
having a precision/size of 0. NV_FLTSIZEZERO needs to be a
negative value.
src/cmd/ksh93/bltins/typeset.c:
- In b_typeset(), added code to set tdata.argnum to NV_FLTSIZEZERO
for E, F, X options.
- In setall(), adjusted code to allow for tp->argnum to be negative.
src/cmd/ksh93/sh/name.c: nv_newattr():
- Adjusted option value only change code to handle NV_FLTSIZEZERO as
well as changed to directly setting np->nvsize instead of using
nv_setsize(np,size) as nv_setsize might contain conflicting and/or
redundant code.
- Added missing conditional check of '!(newatts&NV_INTEGER)' to
constrain the size==0 code block to justified strings as
NV_LJUST, NV_RJUST, or NV_ZFILL are only valid for strings if
NV_INTEGER is not set. This code block was mistakenly setting
the precision/size value to the length of the value of an
assignment for floats whereas it should only be performing
auto assignment length for justified strings.
'typeset -xu' and 'typeset -xl' would export the variable but fail
to change case in the value as the check between old and new
attributes did not provide the necesssary insight for lower or
upper case transcoding due to the lower or upper case attribute
being set within typeset.c prior to calling name.c nv_newattr
function.
Previous rhbz#1188377 patch added a conditional check for size==-1
which in effect caused the nv_newattr export code block return
optimization to never be executed as one cannot set any attributes
using the readonly builtin. By altering the size==-1 check to !trans
the export only optimization can run.
Also, the rhbz#1188377 patch altered new_attr function by setting
the new size to oldsize if run by the readonly builtin. The result
of setting size==oldsize allowed the succeeding if statement to
run more frequently and if size was a non-zero value resulted in
nv_setsize resetting the value to what it already was. Investigation
yielded that size was always 0 coming from the readonly builtin.
src/cmd/ksh93/bltins/typeset.c:
- Remove the setting of tdata.argnum to -1 as it is not needed due to
existing name.c nv_newattr() logic.
src/cmd/ksh93/sh/name.c: nv_newattr():
- Corrected the export only check optimization by using !trans instead
of using size==-1.
- Removed previous condition check to set size=oldsize if coming from
the readonly builtin. nv_newattr already had existing logic to
prevent changing the size via nv_setsize as size is always 0 when
coming from readonly builtin.
Virtual/non-forking subshells that change the present working
directory (PWD) with 'cd' suffer from a serious race condition. The
PWD is changed within the same process. This means it may not be
possible to change back to the original PWD when exiting the
subshell, as some other process may destroy the PWD or modify its
permissions in the meantime. ksh did not handle this error
condition at all, so, after exiting a subshell that invoked 'cd',
it could silently end up running the script's following command(s)
in the wrong directory. Which might be 'rm -rf *'. So, ouch.
The proper and obvious fix is never to allow a virtual subshell to
change the PWD, as it can never be guaranteed you can return to a
previous directory. If the PWD is changed in a child process, there
is no need to restore it in the parent process, and this whole
problem is avoided. So subshells really should always fork on
encountering a 'cd' command.
But forking is slow. It is not uncommon for scripts to 'cd' in a
subshell that is run repeatedly in a loop.
There is also the issue of custom builtins that can be added to ksh
via shared libraries. In the standard shell language, 'cd' is the
only command that changes the PWD, so we could just make that
command fork the subshell it is run from. But there's no telling
what a custom builtin might do.
So this commit implements a compromise that will not affect
performance unless there is the pathological condition of a PWD
that has been rendered inaccessible in some way:
1. When entering a virtual subshell, if the parent shell's PWD
proves inaccessible upon saving it, the subshell will now fork into
a separate process, avoiding the unrestorable PWD problem.
2. If some attack renders the parent shell's PWD unrestorable
*after* ksh enters a virtual subshell, ksh will now error out when
exiting it. There is nothing else left to do then. Continuing would
mean running arbitrary commands in the wrong PWD.
src/cmd/ksh93/sh/subshell.c:
- Put all the code/variables only needed for fchdir() behind '#if
_lib_fchdir'. This makes it clearer what's what.
(I don't know if there is still any system out there without
fchdir(3); I haven't found any. The chdir(3) fallback version may
be removed later as there is no way to make it remotely secure.)
- Fix the attempt to use the O_PATH mode for open(2) as a fallback
for nonexistent O_SEARCH on Linux. Define _GNU_SOURCE on Linux,
or <fcntl.h> (which is included indirectly) won't define O_PATH.
- Fix use of O_SEARCH. The code was simply wrong, repeating an
open(".",O_RDONLY) instead. Since a nonexistent O_SEARCH is now
redefined as either O_PATH or O_RDONLY, we can simply
open(".",O_SEARCH) and be done with it.
- Fix fatal error handling. Introduce fatal error condition for
failure to fchdir(3) back to the parent's PWD; rename 'duped' to
'fatalerror' and use it for error numbers; save and restore errno
on fatal error so the message will report the cause. (We must
call errormsg() near the end of sh_subshell() to avoid crashes.)
- If open(".",O_SEARCH) was not able get a file descriptor to our
PWD on entry, then call sh_subfork() immediately before running
the subshell commands. (Forking earlier causes a crash.)
- When restoring the PWD, if fchdir(3) fails, do *not* fall back to
chdir(3). We already know the PWD is inaccessible, so if chdir(3)
"succeeds" then, it's very likely to be a substitute injected by
an attacker.
src/cmd/ksh93/bltins/cd_pwd.c:
- If we don't have fchdir(3), then sh_subshell() must fall back to
chdir(2) to restore the PWD. That is highly vulnerable, as a
well-timed rename would allow an attacker to usurp the PWD. We
can't do anything about that if some custom builtin changes the
PWD, but we can at least make 'cd' always fork a subshell, which
slows down ksh but removes the need for the parent shell ever to
restore the PWD. (There is certainly no popular system where this
is relevant and there might not be any such current system.)
This commit adds no regression test because a portable regression
test is not really doable. Different kernels, external /bin/pwd
utilities, etc. all have quite different behaviour under the
pathological condition of an inaccessible PWD, so both the
before-fix and the after-fix behaviour differs. See link below.
Resolves: https://github.com/ksh93/ksh/issues/141
Thanks to Stéphane Chazelas for the bug report.
Autoloading a function caused the calling script's $LINENO to be
off by the number of lines in the function definition file. In
addition, while running autoloaded functions, errors/warnings were
reported with wrong line numbers.
src/cmd/ksh93/sh/path.c:
- Save $LINENO (shp->inlineno) before autoloading a function, reset
it to 1 so that the correct line number offset is remembered for
the function definition, and restore it after.
src/cmd/ksh93/tests/variables.sh:
- Add regression test for $LINENO, directly and in error messages,
within and outside a non-autoloaded and an autoloaded function.
Fixes: https://github.com/ksh93/ksh/issues/116
ksh 93u+ introduced a regression in the combination of the
'set -o pipefail' and 'set -e'/'set -o errexit' options:
$ ksh93 -o errexit -o pipefail -c \
'(exit 3) | true; echo "still here despite $? status"'
still here despite 3 status
The bug is in how the the huge sh_exec() function in xec.c handles
the 'echeck' flag. Near the end of sh_exec(), this flag triggers a
sh_chktrap() call to check whether to trigger any traps, including
the ERR trap -- and that same function also handles the errexit
option, which is basically the same as 'trap "exit" ERR'.
We can learn more easily how sh_exec() works by inserting debug
warnings in all its 'switch(type&COMMSK)' cases, like:
case TCOM:
errormsg(SH_DICT,ERROR_warn(0),"[DEBUG] TCOM");
... and same for all the others. With that done, the output
of a very simple dummy pipeline looks as follows:
$ arch/*/bin/ksh -c 'true | true | true'
arch/darwin.i386-64/bin/ksh: warning: [DEBUG] TFIL
arch/darwin.i386-64/bin/ksh: warning: [DEBUG] TFORK
arch/darwin.i386-64/bin/ksh: warning: [DEBUG] TFORK
arch/darwin.i386-64/bin/ksh: warning: [DEBUG] TSETIO
arch/darwin.i386-64/bin/ksh: warning: [DEBUG] TCOM
arch/darwin.i386-64/bin/ksh: warning: [DEBUG] TCOM
arch/darwin.i386-64/bin/ksh: warning: [DEBUG] TCOM
So, it looks like sh_exec() handles this pipeline as follows:
TFIL
|_____TFORK
| |_____TCOM
|_____TFORK
| |_____TCOM
|_____TSETIO
|_____TCOM
Each time a pipeline like command1 | command2 | ... is executed,
sh_exec() is invoked with type TFIL; this then recursively invokes
sh_exec() to handle the individual elements. The last element of
the pipe triggers a sh_exec() run with type TSETIO; since it is run
in the current shell environment, it is effectively treated as a
command with an input redirection. All the previous elements are of
type TFORK instead, because they are executed asynchronously in
separate, forked subshell processes. Finally, the TFORK or TSETIO
code then recursively calls sh_exec() again with type TCOM to
actually execute the commands.
When reading the code, we find that the 'echeck' flag is set as
part of the TSETIO code. This makes sense of why only an error in
the last element of the pipe triggers the errexit/ERR trap action.
So that's the bug: the flag is set in the wrong place.
This can be fixed by setting that flag in the TFIL handling code
instead, as this is what calls everything else and collects all the
exit statuses. So the sh_chktrap() call is now executed after
handling the entire pipeline, at the TFIL recursion level.
This also allows getting rid of the special-casing in the buggy
TSETIO version. The SH_ERREXIT state is restored at the end of each
sh_exec() call, so since we're now doing this at a lower recursion
level, it will already have been restored.
src/cmd/ksh93/sh/xec.c: sh_exec():
- Fix the bug as per the above.
src/cmd/ksh93/tests/options.sh:
- Add tests for errexit and ERR trap combined with pipefail.
src/cmd/ksh93/tests/basic.sh:
- Tweak a couple of tests that reported a trap wasn't triggered
even if it was actually triggered more than once.
Fixes: https://github.com/ksh93/ksh/issues/121
Thanks to Stéphane Chazelas for the bug report.
'typeset -xu' and 'typeset -xl' would export the variable but fail
to change case in the value under certain conditions.
Original patch:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-xufix.patch
This applies the patch essentially without change and adds a
regression test based on the reproducer provided in the RH bug.
Unfortunately there is no description of how the patch works and
it's a little obscure to me. As far as I can figure out, the cause
of the problem was that nv_newattr() erroneously processed a
nonexistent size option-argument such as what can be given to
options like typeset -F, e.g. typeset -F3 for 3 digits after the
dot. A nonexistent size argument is represented by the value of -1.
Prior discussion:
https://bugzilla.redhat.com/1454804
On 2017-05-23 13:33:25 UTC, Paulo Andrade wrote:
> In previous ksh versions, when exiting the scope of a ksh
> (not posix) function, it would restore the trap table of
> the "calling context" and if the reason the function exited
> was a signal, it would call sh_fault() passing as argument
> the signal value.
> Newer ksh checks it, but calls kill(getpid(), signal_number)
> after restoring the trap table, but only calls for SIGINT and
> SIGQUIT.
[...]
> The old way appears to have been more appropriate, but there
> must be a reason to only pass SIGINT and SIGQUIT as it is an
> explicit patch.
The last paragraph is where I differ. This would not be the first
example of outright breakage that appeared to be added deliberately
and that 93u+m has fixed or removed, see e.g. 8477d2ce ('printf %H'
had code that deleted all multibyte characters), cefe087d, or
781f0a39. Sometimes it seems the developers added a little
experiment and then forgot all about it, so it became a misfeature.
In this instance, the correct pre-2012 ksh behaviour is still
explicitly documented in (k)sh.1: "A trap condition that is not
caught or ignored by the function causes the function to terminate
and the condition to be passed on to the caller". Meaning, if there
is no function-local trap, the signal defaults to the parent scope.
There is no language that limits this to SIGINT and SIGQUIT only.
It also makes no sense at all to do so -- signals such as SIGPIPE,
SIGTERM, or SIGSEGV need to be caught by default and to do
otherwise results in misbehaviour by default.
src/cmd/ksh93/sh/xec.c: sh_funscope():
- When resending a signal after restoring the global traps state,
remove the spurious check that limits this to SIGINT and SIGQUIT.
- Replace it with a check for nsig!=0, as that means there were
parent trap states to restore. Otherwise 'kill' may be called
with an invalid signal argument, causing a crash on macOS.
src/cmd/ksh93/tests/signal.sh:
- Update a test to check that a function-local SIGTERM trap is
triggered correctly when signalled from another process.
- Complete the tests for 3aee10d7; this bug needed fixing before
we could test that previous fix in a ksh function scope.
- Add a test for triggering global traps from ksh functions,
testing multiple POSIX-standard signals.
This imports a new version of the code to import environment
variable values that was sent to Red Hat from upstream in 2014.
It avoids importing environment variables whose names are not valid
in the shell language, as it would be impossible to change or unset
them. However, they stay in the environment to be passed to child
processes.
Prior discussion: https://bugzilla.redhat.com/1147645
Original patch: https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-oldenvinit.patch
src/cmd/ksh93/sh/init.c:
- env_init(): Import new, simplified code to import environment
variable name/value pairs. Instead of doing the heavy lifting
itself, this version uses nv_open(), passing the NV_IDENT flag to
reject and skip invalid names.
- Get rid of gotos and a static var by splitting off the code to
import attributes into a new env_import_attributes() function.
This is a better way to avoid importing attributes when
initialising the shell in POSIX mode (re: 00d43960
- Remove an nv_mapchar() call that was based on some unclear
flaggery which was also removed by upstream as sent to Red Hat.
I don't know what that did, if anything; looks like it might have
had something to do with typeset -u/-l, but those particular
attributes have never been successfully inherited through the
environment.
(Maybe that's another bug, or maybe I just don't care as
inheriting attributes is a misfeature anyway; we have to put up
with it because legacy scripts might use it. Maybe someone can
prove it's an unacceptable security risk to import attributes
like readonly from an environment variable that is inherently
vulnerable to manipulation. That would be nice, as a CVE ID
would give us a solid reason to get rid of this nonsense.)
- Remove an 'else cp += 2;' that was very clearly a no-op; 'cp' is
immediately overwritten on the next loop iteration and not used
past the loop.
src/cmd/ksh93/tests/variables.sh:
- Test.
According to 'whence --man', 'whence -f' should ignore functions:
-f Do not check for functions.
Right now this is only accomplished partially. As of commit
a329c22d 'whence -f' avoids any output when encountering a
function (in ksh93u+ 'whence -f' has incorrect output). The
return value is still wrong though:
$ foo() { true; }
$ whence -f foo; echo $?
0
This commit fixes the return value and makes 'type -f' error out
when given a function (like in Bash).
src/cmd/ksh93/bltins/whence.c:
- If -f was passed, set 'cp' to NULL since functions should be
ignored (as documented).
- Simplify return value by avoiding bitwise logic.
src/cmd/ksh93/tests/builtins.sh:
- Add regression tests for 'whence -f' and 'type -f'.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
Since at least 1999, whence -v on pdksh (and its successor mksh)
reports the path where an autoloadable function may be found:
$ mkdir ~/fun; FPATH=~/fun
$ echo 'myfn() { echo hi; }' >~/fun/myfn
$ whence -v myfn
myfn is a undefined (autoload from /home/user/fun/myfn) function
Whereas ksh93 only reports, rather uselessly:
myfn is an undefined function
As of this commit, whence -v/-a on ksh 93u+m does the same as
pdksh, but with correct grammar:
myfn is an undefined function (autoload from /home/user/fun/myfn)
This may be a small violation of my own "no new features" policy
for 93u+m, but I couldn't resist. This omission has been annoying
me, and it's just embarrassing to lack a pdksh feature :)
src/cmd/ksh93/include/path.h,
src/cmd/ksh93/data/msg.c:
- Add e_autoloadfrom[] = " (autoload from %s)" message.
src/cmd/ksh93/bltins/whence.c: whence():
- Report the path (if any) when reporting an undefined function.
This needs to be done in two places:
1. When a function has been explicitly marked undefined with
'autoload', we need to do a quick path_search() loop to find
the path. (These undefined functions take precedence over
regular commands, so are reported first.)
2. When a function is not explicitly autoloaded but merely
available in $FPATH, that path search was already done, so all
we need to do is report it. (These are reported last.)
Note that the output remains as on 93u+ if no function definition
file is found on $FPATH. This is also like pdksh/mksh.
src/cmd/ksh93/data/builtins.c:
- Bump 'whence' version date. The inline docs never detailed very
exactly what 'whence -v' reports, so no need for further edits.
src/cmd/ksh93/tests/path.sh:
- Regress-test the new whence behaviour plus actual autoloading,
including the command override behaviour of autoloaded functions.
The fixargs() function is invoked when ksh needs to run a script
without a #!/hashbang/path. Instead of letting the kernel invoke a
shell, ksh exfile()s the script itself from sh_main(). In the
forked child, it calls fixargs() to set the argument list in the
environment to the args of the new script, so that 'ps' and
/proc/PID/cmdline show the expected output.
But fixargs() is broken because, on systems other than HP-UX (on
which ksh uses pstat(2)), ksh simply inserts a terminating zero.
The arguments list is not a zero-terminated C string. Unix systems
expect the entire arguments buffer to be zeroed out, otherwise 'ps'
and /proc/*/cmdline will have fragments of previous command lines
in the output.
The Red Hat patch for this bug is:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-argvfix.patch
However, that fix is incomplete because 'command_len' was also
hardcoded to be limited to 64 characters (!), which still gave
invalid 'ps' output if the erased command line was longer.
src/cmd/ksh93/sh/main.c: fixargs():
- Remove CMD_LENGTH macro which was defined as 64.
- Remove code that limited the erasure of the arguments buffer to
CMD_LENGTH characters. That code also had quite a dodgy strdup()
call -- it copies arguments to the heap, but they are never freed
(or even used), so it's a memory leak. Also, none of this is
ever done if the length is calculated using pstat(2) on HP-UX,
which is a clear indication that it's unnecessary.
(I think this code block must have been some experiment they
forgot to remove. One reason why I think so is that a 64 byte
arguments limit never made sense, even in the 1980s when they
wrote ksh on 80-column CRT displays. Another indication of this
is that fixing it didn't require adding anything; the code to do
the right thing was already there, it was just being overridden.)
- Zero out the full arguments length as in the Red Hat patch.
src/cmd/ksh93/tests/basic.sh:
- Add test. It's sort of involved because 'ps' is one of the least
portable commands in practice, in spite of standardisation.
There was no check for the -B/braceexpand option before calling
path_expand() to process brace expansion, making it impossible to
turn off brace expansion within command substitutions. Normally the
lexer flags brace expansion so that this code is not reached, but
shell code within command substitutions is handled differently.
Red Hat patches this by adding this check to the function itself:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20140301-fikspand.patch
But I think it's more logical to patch it at the point of decision.
src/cmd/ksh93/sh/macro.c: endfield():
- Decide to call either path_generate() or path_expand() based on
the state of the SH_BRACEEXPAND shell option.
- Fix '#if SHOPT_BRACEPAT' preprocessor check that previously
hardcoded this decision at compile time.
src/cmd/ksh93/tests/options.sh:
- Add tests.
When using typeset -l or -u on a variable that cannot be changed
when the shell is in restricted mode, ksh crashed.
This fixed is inspired by this Red Hat fix, which is incomplete:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-tpstl.patch
The crash was caused by the nv_shell() function. It walks though a
discipline function tree to get the pointer to the interpreter
associated with it. Evidently, the problem is that some pointer in
that walk is not set correctly for all special variables.
Thing is, ksh only has one shell language interpreter, and only one
global data structure (called 'sh') to keep its main state[*]. Yet,
the code is full of 'shp' pointers to that structure. Most (not
all) functions pass that pointer around to each other, accessing
that struct indirectly, ostensibly to account for the non-existent
possibility that there might be more than one interpreter state.
The "why" of that is an interesting cause for speculation that I
may get to sometime. For now, it is enough to know that, in the
code as it is, it matters not one iota what pointer to the shell
interpreter state is used; they all point to the same thing (unless
it's broken, as in this bug).
So, rather than fixing nv_shell() and/or associated pointer
assignments, this commit simply removes it, and replaces it with
calls to sh_getinterp(), which always returns a pointer to sh (see
init.c, where that function is defined as literally 'return &sh').
[*] Defined in shell.h, with the _SH_PRIVATE part in defs.h
src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/name.c:
- Remove nv_shell().
src/cmd/ksh93/sh/init.c:
- In all the discipline functions for special variables, initialise
shp using sh_getinterp() instead of nv_shell().
src/cmd/ksh93/tests/variables.sh:
- Add regression test for typeset -l/-u on all special variables.
This imports another fix from Red Hat/Fedora. Original patch:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-crash.patch
src/cmd/ksh93/include/jobs.h,
src/cmd/ksh93/sh/jobs.c,
src/cmd/ksh93/sh/subshell.c,
src/cmd/ksh93/sh/xec.c:
- Import the Red Hat fix with these differences:
- Rename the 'hack1_waitall' variable to 'bktick_waitall' and add
a comment describing what it's for.
- Remove unused 'pipefail' variable.
src/cmd/ksh93/tests/basic.sh:
- Regression test from reproducer given in the Red Hat bug report.
- Add special handling to SIGKILL it, as it might freeze hard.
var=$(< file) now reads the file even if the standard inout,
standard output and/or standard error file descriptors are closed.
Original patch:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-filecomsubst.patch
src/cmd/ksh93/sh/io.c: sh_redirect():
- When processing the '<' redirector as part of $(< ...), i.e. if
flag==3, make sure the FD of the file to read is > 2 by calling
sh_iomovefd(). Unlike the RedHat patch, this checks for flag==3
to avoid unnecessary sh_iomovefd() calls for normal redirections,
as there was no bug with those.
src/cmd/ksh93/tests/io.sh:
- Add test.
Hopefully this doesn't introduce new bugs, but it does fix at
least the following:
1. When whence -v/-a found an "undefined" (i.e. autoloadable)
function in $FPATH, it actually loaded the function as a side
effect of reporting on its existence (!). Now it only reports.
2. 'whence' will now canonicalise paths properly. Examples:
$ whence ///usr/lib/../bin//./env
/usr/bin/env
$ (cd /; whence -v dev/../usr/bin//./env)
dev/../usr/bin//./env is /usr/bin/env
3. 'whence' no longer prefixes a spurious double slash when doing
something like 'cd / && whence bin/echo'. On Cygwin, an initial
double slash denotes a network server, so this was not just a
cosmetic problem.
4. 'whence -a' now reports a "tracked alias" (a.k.a. hash table
entry, i.e. cached $PATH search) even if an actual alias by the
same name exists. This needed fixing because in fact the hash
table entry continues to be used when bypassing the alias.
Aliases and "tracked aliases" are not remotely the same thing;
confusing nomenclature is not a reason to report wrong results.
5. When using 'hash' or 'alias -t' on a command that is also a
builtin to force caching a $PATH search for the external
command, 'whence -a' double-reported the path:
$ hash printf; whence -a printf
printf is a shell builtin
printf is /usr/bin/printf
printf is a tracked alias for /usr/bin/printf
This is now fixed so that the second output line is gone.
Plus, if there were multiple versions of the command on $PATH,
the tracked alias was reported at the end, which is the wrong
order. This is also fixed.
src/cmd/ksh93/bltins/whence.c: whence():
- Refactor the do...while loop that handles whence -v/-a for path
searches in such a way that the code actually makes sense and
stops looking like higher esotericism. Just doing this fixed#2,
#4 and #5 above (the latter two before I even noticed them). For
instance, the path_fullname() call to canonicalise paths was
already there; it was just never used.
- Remove broken 'notrack' flaggery for deciding whether to report a
hash table entry a.k.a. "tracked alias"; instead, check the hash
table (shp->track_tree).
src/cmd/ksh93/sh/path.c:
- path_search(): Re #3: When prefixing the PWD, first check if
we're in '/' and if so, don't prefix it; otherwise, adding the
next slash causes an initial double slash. (Since '/' is the only
valid single-character absolute path, all we need to do is check
if the second character pwd[1] is non-null.)
- path_search(): Re #1: Stop autoloading when called by 'whence':
* The 'flag==2' check to avoid autoloading a function was
broken. The flag value is 2 on the first whence() loop
iteration, but 3 on subsequent ones. Change to 'flag >= 2'.
* However, this only fixes it if the function file does not have
the x permission bit, as executable files are handled by
path_absolute() which unconditionally autoloads functions!
So, pass on our flag parameter when callling path_absolute().
- path_absolute(): Re #1: Add flag parameter. Do not autoload
functions if flag >= 2.
src/cmd/ksh93/include/path.h,
src/cmd/ksh93/bltins/typeset.c,
src/cmd/ksh93/sh/main.c,
src/cmd/ksh93/sh/xec.c:
- Re #1: Update path_absolute() calls, adding a 0 flag parameter.
src/cmd/ksh93/include/name.h:
- Remove now-unused pathcomp member from union Value. It was
introduced in 99065353 to allow examining the value of a tracked
alias. This commit uses nv_getval() instead.
src/cmd/ksh93/tests/builtins.sh,
src/cmd/ksh93/tests/path.sh:
- Add and tweak various related tests.
Fixes: https://github.com/ksh93/ksh/issues/84
{Brace,expansion} is potentially incompatible with POSIX scripts,
because in POSIX those are simple literal strings with no special
meaning. So the POSIX option should really turn that off.
As of b301d417, the 'posix' option was also forcing 'letoctal'
behaviour on, without actually setting that option. I've since
found that to be a botch; 'let' may recognise octals without that
option being set, and that looks like a bug.
So as of this commit, the '-o posix' option actually toggles both
of these options off/on and on/of, respectively. 'set +o posix'
toggles them inversely. However, it is now possible to control both
options (and their associated behaviour) independently in between
'set -o posix' and 'set +o posix'. Much better.
src/cmd/ksh93/sh/main.c: sh_main():
- If SH_POSIX was set on init, turn on SH_LETOCTAL by default
instead of SH_BRACEEXPAND.
src/cmd/ksh93/sh/args.c: sh_applyopts():
- Turn off SH_BRACEEXPAND and turn on SH_LETOCTAL when SH_POSIX is
turned on (but not if it was already on).
- Turn on SH_BRACEEXPAND and turn off SH_LETOCTAL when SH_POSIX is
turned off (but not if it was already off).
src/cmd/ksh93/sh/arith.c: arith():
- Revert to pre-b301d417 and only check SH_LETOCTAL option when
deciding whether 'let' should skip initial zeros.
src/cmd/ksh93/tests/options.sh:
- Update $- test to allow '-o posix' to switch B = braceexpand.
src/cmd/ksh93/sh.1:
- Update.
- Edit for clarity.
This patch from Red Hat fixes the following:
1. ksh was ignoring the -m (-o monitor) option when specified on
the invocation command line.
2. Scripts did not properly terminate their background processes
on Ctrl+C if the -m option was turned off. Reproducer:
xterm &
read junk
When run as a script without turning on -m, pressing Ctrl+C
should terminate the xterm, and now does.
3. Scripts no longer attempt to set the terminal foreground process
group ID, as only interactive shells should be doing that.
This makes some progress on https://github.com/ksh93/ksh/issues/119
but we're a long way from fixing all of that.
src/cmd/ksh93/sh/main.c: exfile():
- On non-interactive shells, do not turn off the monitor option.
Instead, if it was turned on, turn on the SH_MONITOR state flag.
src/cmd/ksh93/edit/edit.c: ed_getchar():
- On Ctrl+C, issue SIGINT to the current process group using
killpg(2) instead of going via sh_fault(), which handles a
signal only for the current shell process.
src/cmd/ksh93/sh/jobs.c: job_reap(), job_reset(),
src/cmd/ksh93/sh/xec.c: sh_exec():
- Only attempt to set the terminal foreground process group ID
using tcsetpgrp(3) if the shell is interactive.
Original patch: https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-kshmfix.patch
This was applied to Red Hat's ksh 93u+ on 8 July 2013.
src/cmd/ksh93/data/signals.c includes two checks for the JOBS
identifier; if it is not defined then the interactive shell's
background job signal messages for SIGINT and SIGPIPE are empty.
The cause was that the "jobs.h" header, which defines that ID, was
not #included in signals.c. This commit adds that #include.
(ksh 93u+, ksh 93v- and ksh2020 all have this bug as well.)
Before:
$ sleep 30 &
[1] 86430
$ kill -s INT "$!"
[1] + sleep 30 &
$
After:
$ sleep 30 &
[1] 86445
$ kill -s INT "$!"
[1] + Interrupt sleep 30 &
$
In the vi and emacs line editors, repeat count parameters can now
also be used for the arrow keys and the forward-delete key. E.g.,
in emacs mode, <ESC> 7 <left-arrow> will now move the cursor seven
positions to the left. In vi control mode, this would be entered
as: 7 <left-arrow>.
src/cmd/ksh93/edit/emacs.c:
- ed_emacsread(): Upon getting ^[ (ESC), save current repeat count
in a new variable; restore and reset it upon the next character.
- escape(): Minor bugfix: when processing a ^[[x sequence where 'x'
is a character other than '~' (which would be DEL), also reinsert
the final character into the buffer so scripts can detect them.
src/cmd/ksh93/edit/vi.c:
- cntlmode(): Do not reset the repeat count if the command is '[',
the character following ESC in VT220 escape sequences.
- mvcursor():
* Do not use getcount() to get the character following '[', as
that was parsing repetition parameters in the wrong place.
There wouldn't be any, so this would reset the repeat count.
* After that, no more need for the special-casing of ^[[3~ (DEL)
introduced in f2a3f4e3. Move it to within the 'switch' block.
* When handling left and right arrows and Home and End keys, do
not modify cursor directly but ed_ungetchar() the corresponding
traditional command keys as with the rest. Otherwise a repeat
count parameter would now wrongly survive those keys.
src/cmd/ksh93/sh.1:
- Document control character notation used for vi mode docs.
- Since vi control mode beeps and aborts on ESC except if a
subsequent [ is already in the input buffer upon receiving ESC,
document that VT220 escape sequences only preserve repeat counts
when entered into the input buffer all at once.
- Don't skip the initial ESC in the documentation of the VT220
escape sequences. In control mode, skipping the initial ESC still
works as before, but that is now undocumented, as it's really
nothing more than an artefact of VT220 escape processing.
- Move the two long paragraphs on '-o viraw' and canonical (i.e.
line-based) input processing from the vi editor introduction to
the options section under 'viraw'. It is much too arcane for the
intro, and besides, ksh 93u+ (and hence also 93u+m) has
SHOPT_VIRAW enabled by default, so the shell is compiled to force
this option on at all times, making it even less relevant for
most users.
On every modern system, the forward-delete key on PC/Mac keyboards
generates the VT220 sequence ESC [ 3 ~. Every other shell with an
editor handles this now, ksh93 seems to be the last not to.
src/cmd/ksh93/edit/emacs.c: escape():
- Handle the ^[[3 as part of normal escape processing, then read an
extra character to check for the final '~'. If detected, insert
an ERASECHAR key event.
src/cmd/ksh93/edit/vi.c: mvcursor():
- Replace the ^[[3~ sequence by an 'x' command. We have to
special-case its processing, because vi mode parses numbers as
repetition operators. The escape sequence contains a number,
making it incompatible with normal command handling. This means
number repetitions don't work with the forward-delete key. If
that annoys anyone enough to fix it, a patch would be welcome.
For now, it will do to make the forward-delete key stop
exhibiting bizarre behaviour (beep + change case + move forward).
src/cmd/ksh93/sh.1
- Copy-edit emacs documentation for VT220-style sequences; map them
to their actual key, otherwise it's meaningless to the reader.
- Document the new forward-delete key behaviour for emacs mode.
- Leave the forward-delete key for vi mode undocumented for now, as
repetitions don't work, so it doesn't really match the vi canon.
(OTOH, it doesn't work in vim, either...)
_sfcvt(), "convert a floating point value to ASCII", did not adjust
for negative decimal place movement as what happens with leading
zeroes. This caused ksh's 'printf %f' formatter to fail to round
floating point values correctly.
src/lib/libast/sfio/sfcvt.c:
- Removed constraint of <1e-8 for doubles by matching what was done
for long doubles having <.1.
- Corrected a condition when the next power of 10 occurred and that
new 1 digit was being overwritten by a 0.
src/cmd/ksh93/tests/math.sh:
- Validate that typeset -E/F formatting matches that of their
equivalent printf formatting options as well as checking for
correct float scaling of the fractional parts.
Following a community discussion, it became clear that 'r' is
particularly problematic as a regular builtin, as the name can and
does conflict with at least one legit external command by that
name. There was a consensus against removing it altogether and
letting users set the alias in their login scripts. However,
aliases are easier to bypass, remove or rename than builtins are.
My compromise is to reinstate 'r' as a preset alias on interactive
shells only, along with 'history', as was done in 17f81ebe before
they were converted to builtins in 03224ae3. So this reintroduces
the notion of predefined aliases to ksh 93u+m, but only for
interactive shells that are not initialised in POSIX mode.
src/cmd/ksh93/Makefile,
src/cmd/ksh93/Mamfile,
src/cmd/ksh93/include/shtable.h,
src/cmd/ksh93/data/aliases.c:
- Restore aliases.c containing shtab_aliases[], a table specifying
the preset aliases.
src/cmd/ksh93/include/shtable.h,
src/cmd/ksh93/sh/init.c:
- Rename inittree() to sh_inittree() and make it extern, because we
need to use it in main.c (sh_main()).
src/cmd/ksh93/sh/main.c: sh_main():
- Init preset aliases from shtab_aliases[] only if the shell is
interactive and not in POSIX mode.
src/cmd/ksh93/bltins/typeset.c,
src/cmd/ksh93/tests/alias.sh:
- unall(): When unsetting an alias, pass on the NV_NOFREE attribute
to nv_delete() to avoid an erroneous attempt to free a preset
alias from read-only memory. See: 5d50f825
src/cmd/ksh93/data/builtins.c:
- Remove "history" and "r" entries from shtab_builtins[].
- Revert changes to inline fc/hist docs in sh_opthist[].
src/cmd/ksh93/bltins/hist.c: b_hist():
- Remove handling for 'history' and 'r' as builtins.
src/cmd/ksh93/sh.1:
- Update accordingly.
Resolves: https://github.com/ksh93/ksh/issues/125
The 'command' name can now result from an expansion, e.g.:
c=command; "$c" ls
set -- command ls; "$@"
both work now. This fixes BUG_CMDEXPAN.
If -o posix is on, 'command' now disables not only the "special"
but also the "declaration" properties of builtin commands that it
invokes. This is because POSIX specifies 'command' as a simple
regular builtin, and any command name following 'command' is just
an argument to the 'command' command, so there is nothing that
allows any further arguments (such as assignment-arguments) to be
treated specially by the parser. So, if and only if -o posix is on:
a. Arguments that start with a variable name followed by '=' are
always treated as regular words subject to normal shell syntax.
b. Since assignment-arguments are not processed as assignments
before the command itself, 'command' can now stop the shell from
exiting (as required by the standard) if a command that it
invokes (such as 'export') tries to modify a readonly variable.
This fixes BUG_CMDSPEXIT.
Most of 'command' is integrated in the parser and parse tree
executer, so that is where it needed fixing.
src/cmd/ksh93/sh/parse.c: simple():
- If the posix option is on, do not skip past SYSCOMMAND so that
any declaration builtin commands that are arguments to 'command'
are not detected and thus not treated specially at parsetime.
src/cmd/ksh93/sh/xec.c: sh_exec():
- When detecting SYSCOMMAND in order to skip past it, not only
compare the Namval_t pointer 'np' to SYSCOMMAND, but also handle
the case where that pointer is NULL, as when the command name
results from an expansion. In that case, search the function tree
shp->fun_tree for the name and see if that yields the SYSCOMMAND
pointer. fun_tree is initialised with a dtview to bltin_tree, so
searching fun_tree instead allows for overriding 'command' with a
shell function (which the POSIX standard requires us to allow).
src/cmd/ksh93/sh.1,
src/cmd/ksh93/data/builtins.c:
- Update documentation to match these changes.
- Various related edits and improvements.
src/cmd/ksh93/tests/builtins.sh:
- Check that 'command' works if resulting from an expansion.
- Check that 'command' can be overridden by a shell function.
The 'exit' and 'return' commands without an argument failed to pass
down the exit status of the last-run command when incorporated in a
block with redirection, &&/|| list, 'case' statement, or 'while',
'until' or 'for' loop.
src/cmd/ksh93/bltins/cflow.c:
- Use $?, which is sh.savexit a.k.a. shp->savexit, as the default
exit status value if there is no argument, instead of
shp->oldexit. This fixes the default exit status behaviour to
match POSIX and other shells.
src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/include/shell.h:
- Remove now-unused sh.oldexit (a.k.a. shp->oldexit) private struct
member. It appeared to fulfill the same function as sh.savexit,
but in a slightly broken way.
- Move the savexit/$? declaration from the _SH_PRIVATE part of the
struct definition to the public API part. Since $? uses this,
it's clearly a publicly exposed value already, and this is
generally the one to use. (If anything, it's exitval that should
have been private.) This declares savexit right next to exitval,
rewriting the comments to clarify the difference between them.
src/cmd/ksh93/sh/fault.c,
src/cmd/ksh93/sh/subshell.c,
src/cmd/ksh93/sh/xec.c:
- Remove assignments to shp->oldexit.
src/cmd/ksh93/tests/basic.sh:
- Add thorough regression tests for the default exit status
behaviour of 'return' and 'exit' in various lexical contexts.
- Verify that 'for' and 'case' without any command, as well as a
lone redirection, still correctly reset the exit status to 0.
Fixes: #117
