This commit adds an UNREACHABLE() macro that expands to either the
__builtin_unreachable() compiler builtin (for release builds) or
abort(3) (for development builds). This is used to mark code paths
that are never to be reached.
It also adds the 'noreturn' attribute to functions that never
return: path_exec(), sh_done() and sh_syntax(). The UNREACHABLE()
macro is not added after calling these.
The purpose of these is:
* to slightly improve GCC/Clang compiler optimizations;
* to fix a few compiler warnings;
* to add code clarity.
Changes of note:
src/cmd/ksh93/sh/io.c: outexcept():
- Avoid using __builtin_unreachable() here since errormsg can
return despite using ERROR_system(1), as shp->jmplist->mode is
temporarily set to 0. See: https://github.com/att/ast/issues/1336
src/cmd/ksh93/tests/io.sh:
- Add a regression test for the ksh2020 bug referenced above.
src/lib/libast/features/common:
- Detect the existence of either the C11 stdnoreturn.h header or
the GCC noreturn attribute, preferring the former when available.
- Test for the existence of __builtin_unreachable(). Use it for
release builds. On development builds, use abort() instead, which
crahses reliably for debugging when unreachable code is reached.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
This commit fixes a bug in the ksh uname builtin's -d option that could
change the output of -o (I was only able to reproduce this on Linux):
$ builtin uname
$ uname -o
GNU/Linux
$ uname -d
(none)
$ uname -o
(none)
I identified this patch from ksh2020 as a fix for this bug:
<https://github.com/att/ast/pull/1187>
The linked patch was meant to fix a crash in 'uname -d', although I've
had no luck reproducing it: <https://github.com/att/ast/issues/1184>
src/lib/libcmd/uname.c:
- Pass correct buffer to getdomainname() while executing uname -d.
src/cmd/ksh93/tests/builtins.sh:
- Add a regression test for the reported 'uname -d' crash.
- Add a regression test for the output of 'uname -o' after 'uname -d'.
- To handle potential crashes when running the regression tests in older
versions of ksh, fork the command substitutions that run 'uname -d'.
This bug was first reported at <https://github.com/att/ast/issues/8>.
The 'cd' command currently takes the value of $OLDPWD from the
wrong scope. In the following example 'cd -' will change the
directory to /bin instead of /tmp:
$ OLDPWD=/bin ksh93 -c 'OLDPWD=/tmp cd -'
/bin
src/cmd/ksh93/bltins/cd_pwd.c:
- Use sh_scoped() to obtain the correct value of $OLDPWD.
- Fix a use-after-free bug. Make the 'oldpwd' variable a static
char that points to freeable memory. Each time cd is used, this
variable is freed if it points to a freeable memory address and
isn't also a pointer to shp->pwd.
src/cmd/ksh93/sh/path.c: path_pwd():
- Simplify and add comments.
- Scope $PWD properly.
src/cmd/ksh93/tests/builtins.sh,
src/cmd/ksh93/tests/leaks.sh:
- Backport the ksh2020 regression tests for 'cd -' when $OLDPWD is
set.
- Add test for $OLDPWD and $PWD after subshare.
- Add test for $PWD after 'cd'.
- Add test for possible memory leak.
- Add testing for 'unset' on OLDPWD and PWD.
src/cmd/ksh93/COMPATIBILITY:
- Add compatibility note about changes to $PWD and $OLDPWD.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
This commit adds '/* FALLTHROUGH */' comments to fix many
GCC warnings when compiling with -Wimplicit-fallthrough.
Additionally, the existing fallthrough comments have been
changed for consistency.
src/cmd/ksh93/tests/variables.sh: LC_* error tests:
- Since operating systems validate locale strings differently,
try a few different bad locale strings to find one that makes
setlocale(2) fail, fixing test failures on OpenBSD and Debian.
- Restore warning removed in aed5c6d7, issuing it if none of the
bad locale strings produce a diagnostic.
- Reenable test for diagnostic message disabled in aed5c6d7.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
This bug was originally reported at <https://github.com/att/ast/issues/1467>.
A crash can occur when using the 'b' or 'B' vi mode commands to go back
one word. I was able to reproduce these crashes with 100% consistency on
an OpenBSD virtual machine when ksh is compiled with -D_std_malloc.
Reproducer:
$ set -o vi
$ asdf <ESC> <b or B>
The fix is based on Matthew DeVore's analysis:
> I suspect this is caused by this line:
>> while (vi_isalph(tcur_virt) && tcur_virt >= first_virt) --tcur_virt;
> which is in the b codepath. It checks vi_isalph(tcur_virt) before checking
> if tcur_virt is in range. These two clauses should be reversed. Note that
> line 316 is a similar check for pressing B, and there the tcur_virt value
> is checked first.
src/cmd/ksh93/edit/vi.c:
- Check tcur_virt before using isalph() or isblank() to fix both crashes.
At the start of the backword() while loop this check was performed
twice, so the redundant check has been removed.
src/cmd/ksh93/tests/pty.sh:
- Add a regression test for the b, B, w and W editor commands.
src/cmd/ksh93/bltins/test.c:
- Fix the following compiler warnings from clang:
test.c:554:11: warning: assigning to 'char *' from 'const char []'
discards qualifiers
[-Wincompatible-pointer-types-discards-qualifiers]
e_msg = e_badop;
^ ~~~~~~~
test.c:556:11: warning: assigning to 'char *' from 'const char []'
discards qualifiers
[-Wincompatible-pointer-types-discards-qualifiers]
e_msg = e_unsupported_op;
^ ~~~~~~~~~~~~~~~~
test.c:560:1: warning: control may reach end of non-void function
[-Wreturn-type]
src/cmd/ksh93/tests/builtins.sh:
- Fix regression test by updating error message text.
When test is passed the '=~' operator, it will silently fail with
exit status 1:
$ test foo =~ foo; echo $?
1
This bug is caused by test_binop reaching the 'NOTREACHED' area of
code. The bugfix was adapted from ksh2020:
https://github.com/att/ast/issues/1152
src/cmd/ksh93/bltins/test.c: test_binop():
- Error out with a message suggesting usage of '[[ ... ]]' if '=~'
is passed to the test builtin.
- Special-case TEST_END (']]') as that is not really an operator.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
src/lib/libast/tm/tminit.c:
- Commit 9f43f8d1, in addition to backporting fixes from ksh93v-, also
backported this bug:
$ printf '%(%Z)T' now
PPT # Should be PDT
Reapply the ksh2020 bugfix to fix the %Z time
format again.
src/cmd/ksh93/tests/builtins.sh:
- Add a regression test so this bug (hopefully) isn't backported from
ksh93v- again).
Every so often, a commit's GitHub CI run throws the following
regression test failure:
sigchld.sh[57]: expected '2 background' -- got '3' (DELAY=0.02)
When I re-run the job, the failure usually goes away.
In 712261c8 the DELAY variable was changed from 0.2 to 0.02 to
speed up the first SIGCHLD test. It's possible the GitHub CI
runners are just too slow or too heavily loaded for that.
src/cmd/ksh93/tests/sigchld.sh:
- Restore 0.2 value for 'float DELAY'.
I grepped for #include changes in all the commits and compared
that to the changes in the Mamfiles. I found 7 commits that don't
update the Mamfiles with the appropriate dependencies while
adding #includes, as I only learned how this works after having
worked with this code for some time.
This commit adds the missing Mamfile updates for the
corresponding #include changes in the following commits:
06e721c3, 65d363fd, 70fc1da7, 79d19458, b1a41311, bb4d6a2e,
db71b3ad, and this commit.
Additionally:
src/lib/libast/comp/setlocale.c:
- Change include errno.h to error.h to use EILSEQ fallback if
needed; remove corresponding #ifdef (re: 4dcf5c50, 71bfe028).
src/cmd/ksh93/Mamfile:
- Fix a broken dependency on libast FEATURE/float (re: 72968eae).
We can't use 'prev' for a file that was not mentioned before in
the same Mamfile, we have to use a 'make'...'done' on the first
mention. Add subdependencies matching those in libast/Mamfile.
src/cmd/ksh93/bltins/print.c:
- Rename the unlisted and misleadingly named SHOPT_ECHOE option
(which disables, not enables, 'echo -e') to SHOPT_NOECHOE.
src/cmd/ksh93/SHOPT.sh:
- Add the SHOPT_NOECHOE and SHOPT_TEST_L compile time options to
the list of SHOPT options. Since there is a probe for TEST_L,
set it to probe (empty) by default. NOECHE is off by default.
src/cmd/ksh93/features/options:
- Small bugfix: Allow SHOPT_TEST_L to be manually enabled on
systems that don't support '$(whence -p test) -l /foo'.
- Add a comment describing the SHOPT_MULTIBYTE feature test and
separate it from the SHOPT_DEVFD test.
This bugfix comes from <https://github.com/att/ast/pull/711>.
Eric Scrivner provided the following explanation for the fix:
> Coverity identified an issue with integer truncation in
> `put_enum`. The function was truncating the return values of
> `strcasecmp` and `strcmp` from an `int` to an `unsigned short`
> when assigning them to the local variable `n`. Since either of
> these methods can return a value that is not in the set `{0, 1,
> -1}` the later check if `n == 0` could spuriously evaluate to
> true. For example, in the case where either function returned
> `-65536`.
> The fix is simply to change `n` from an `unsigned short` to an
> `int` to avoid the possibility of truncation. Since the only
> purpose of `n` is the store the return values of these checks,
> this does not have any side effects.
That bit of code supported bash's redundant 'function foo()'
function declaration syntax (with both the 'function' keyword
and the '()') which is a syntax error on ksh, as it should be.
This allows ksh to be compiled with versions of tcc that define
__dso_handle in libtcc1.a, i.e., versions as of this commit:
https://repo.or.cz/tinycc.git/commit/dd60b20c
Older versions of tcc still fail to compile ksh, although now they
fail after reaching the libdll feature test. I'm not sure if fixing
that is feasible since even if I hack out the failing libdll
feature test, ksh fails to link with a '__dso_handle' error.
src/lib/libast/comp/atexit.c,
src/lib/libast/features/lib,
src/lib/libast/vmalloc/vmexit.c:
- From what I've been able to gather the only OSes with support
for on_exit are Linux and SunOS 4. However, on_exit takes two
arguments, so the macro that defines it as taking one argument
is incorrect. Since Solaris (SunOS 5) no longer has this call
and the macro breaks on Linux, the clean fix is to remove it
(atexit(3) is used instead).
src/lib/libast/include/ast.h:
- When compiling with tcc on FreeBSD, pretend to be gcc 2.95.3
instead of gcc 9.3.0. This stops /usr/include/math.h from
activating gcc 3.0+ math compiler builtins that don't exist on
tcc, while still identifying as gcc which is needed to avoid
other FreeBSD system header breakage.
src/cmd/builtin/Mamfile,
src/cmd/builtin/features/pty,
src/lib/libdll/Mamfile,
src/lib/libdll/features/dll:
- tcc forbids combining the -c compiler flag with -l* linker flags.
Use the -lm flag in the iffe feature tests instead of the
Mamfiles. This avoids iffe combining -lm with the -c flag.
src/lib/libast/vmalloc/malloc.c:
- Fix failure to compile with -D_std_malloc.
This patch is from OpenSUSE:
https://build.opensuse.org/package/view_file/shells/ksh/ksh93-malloc-hook.dif
As it turns out tcc needs this change to build ksh with
-D_std_malloc, so it has been applied.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
Resolves: https://github.com/ksh93/ksh/issues/232
src/lib/libast/features/lib,
src/lib/libast/path/pathicase.c:
- FAT32 file systems on Linux don't support FS_CASEFOLD_FL, which
caused globbing to break. Reproducer using a UEFI boot partition:
$ echo /boot/eF*
/boot/eF*
This is fixed by checking for FAT attributes with ioctl, then
checking for FS_CASEFOLD_FL if that fails.
- The check for FS_CASEFOLD_FL didn't work correctly; I still wasn't
able to get --globcasedetect to work on a case-insensitive ext4
folder. Fix that by adding missing parentheses.
Moving the 'err_exit' and 'warning' alias definitions in the
regression tests to one _common file introduced a bug: they are no
longer expanded at compile time when the tests are run with shcomp,
resulting in a 'command not found' (at best) on trying to execute
one. shcomp requires that the alias definitions need to be present
in the file itself. But that means maintaining 50-odd copies again.
I'd rather add a hack to shtests to avoid this.
src/cmd/ksh93/tests/shtests:
- Before running a test with shcomp, physically concatenate _common
and the test script together into a temporary file, minus the '.'
command that includes _common, and compile that with shcomp.
The NOT_USED() macro is already defined in ast.h (which is included
by shell.h) as an alias of NoP(). So it's better to apply the fix
to NoP() so it takes effect for both verrsions, for libast and ksh.
One of the best-kept secrets of libast/ksh93 is that the code
includes support for case-insensitive file name generation (a.k.a.
pathname expansion, a.k.a. globbing) as well as case-insensitive
file name completion on interactive shells, depending on whether
the file system is case-insensitive or not. This is transparently
determined for each directory, so a path pattern that spans
multiple file systems can be part case-sensitive and part case-
insensitive. In more precise terms, each slash-separated path name
component pattern P is treated as ~(i:P) if its parent directory
exists on a case-insensitive file system. I recently discovered
this while dealing with <https://github.com/ksh93/ksh/issues/223>.
However, that support is dead code on almost all current systems.
It depends on pathconf(2) having a _PC_PATH_ATTRIBUTES selector.
The 'c' attribute is supposedly returned if the given directory is
on a case insensitive file system. There are other attributes as
well (at least 'l', see src/lib/libcmd/rm.c). However, I have been
unable to find any system, current or otherwise, that has
_PC_PATH_ATTRIBUTES. Google and mailing list searches yield no
relevant results at all. If anyone knows of such a system, please
add a comment to this commit on GitHub, or email me.
An exception is Cygwin/Windows, on which the "c" attribute was
simply hardcoded, so globbing/completion is always case-
insensitive. As of Windows 10, that is wrong, as it added the
possibility to mount case-sensitive file systems.
On the other hand, this was never activated on the Mac, even
though macOS has always used a case-insensitive file like Windows.
But, being UNIX, it can also mount case-sensitive file systems.
Finally, Linux added the possibility to create individual case-
insensitive ext4 directories fairly recently, in version 5.2.
https://www.collabora.com/news-and-blog/blog/2020/08/27/using-the-linux-kernel-case-insensitive-feature-in-ext4/
So, since this functionality latently exists in the code base, and
three popular OSs now have relevant file system support, we might
as well make it usable on those systems. It's a nice idea, as it
intuitively makes sense for globbing and completion behaviour to
auto-adapt to file system case insensitivity on a per-directory
basis. No other shell does this, so it's a nice selling point, too.
However, the way it is coded, this is activated unconditionally on
supported systems. That is not a good idea. It will surprise users.
Since globbing is used with commands like 'rm', we do not want
surprises. So this commit makes it conditional upon a new shell
option called 'globcasedetect'. This option is only compiled into
ksh on systems where we can actually detect FS case insensitivity.
To implement this, libast needs some public API additions first.
*** libast changes ***
src/lib/libast/features/lib:
- Add probes for the linux/fs.h and sys/ioctl.h headers.
Linux needs these to use ioctl(2) in pathicase(3) (see below).
src/lib/libast/path/pathicase.c,
src/lib/libast/include/ast.h,
src/lib/libast/man/path.3,
src/lib/libast/Mamfile:
- Add new pathicase(3) public API function. This uses whatever
OS-specific method it can detect at compile time to determine if
a particular path is on a case-insensitive file system. If no
method is available, it only sets errno to ENOSYS and returns -1.
Currently known to work on: macOS, Cygwin, Linux 5.2+, QNX 7.0+.
- On systems (if any) that have the mysterious _PC_PATH_ATTRIBUTES
selector for pathconf(2), call astconf(3) and check for the 'c'
attribute to determine case insensitivity. This should preserve
compatibility with any such system.
src/lib/libast/port/astconf.c:
- dynamic[]: As case-insensitive globbing is now optional on all
systems, do not set the 'c' attribute by default on _WINIX
(Cygwin/Windows) systems.
- format(): On systems that do not have _PC_PATH_ATTRIBUTES, call
pathicase(3) to determine the value for the "c" (case
insensitive) attribute only. This is for compatibility as it is
more efficient to call pathicase(3) directly.
src/lib/libast/misc/glob.c,
src/lib/libast/include/glob.h:
- Add new GLOB_DCASE public API flag to glob(3). This is like
GLOB_ICASE (case-insensitive matching) except it only makes the
match case-insensitive if the file system for the current
pathname component is determined to be case-insensitive.
- gl_attr(): For efficiency, call pathicase(3) directly instead of
via astconf(3).
- glob_dir(): Only call gl_attr() to determine file system case
insensitivity if the GLOB_DCASE flag was passed. This makes case
insensitive globbing optional on all systems.
- glob(): The options bitmask needs to be widened to fit the new
GLOB_DCASE option. Define this centrally in a new GLOB_FLAGMASK
macro so it is easy to change it along with GLOB_MAGIC (which
uses the remaining bits for a sanity check bit pattern).
src/lib/libast/path/pathexists.c:
- For efficiency, call pathicase(3) directly instead of via
astconf(3).
*** ksh changes ***
src/cmd/ksh93/features/options,
src/cmd/ksh93/SHOPT.sh:
- Add new SHOPT_GLOBCASEDET compile-time option. Set it to probe
(empty) by default so that the shell option is compiled in on
supported systems only, which is determined by new iffe feature
test that checks if pathicase(3) returns an ENOSYS error.
src/cmd/ksh93/data/options.c,
src/cmd/ksh93/include/shell.h:
- Add -o globcasedetect shell option if compiling with
SHOPT_GLOBCASEDET.
src/cmd/ksh93/sh/expand.c: path_expand():
- Pass the new GLOB_DCASE flag to glob(3) if the
globcasedetect/SH_GLOBCASEDET shell option is set.
src/cmd/ksh93/edit/completion.c:
- While file listing/completion is based on globbing and
automatically becomes case-insensitive when globbing does, it
needs some additional handling to make a string comparison
case-insensitive in corresponding cases. Otherwise, partial
completions may be deleted from the command line upon pressing
tab. This code was already in ksh 93u+ and just needs to be
made conditional upon SHOPT_GLOBCASEDET and globcasedetect.
- For efficiency, call pathicase(3) directly instead of via
astconf(3).
src/cmd/ksh93/sh.1:
- Document the new globcasedetect shell option.
In various places in libast and libcmd there are preprocessor
fallbacks like this, for systems that don't define all the commonly
used errno value IDs:
#ifndef ENOSYS
#define ENOSYS EINVAL
#endif
and many others. It is better to have these all in one place so
they are not duplicated and we don't risk inconsistencies when
adding new code.
src/lib/libast/include/error.h includes the OS's <errno.h>, so it
is the logical file to move all these fallbacks into.
Quite possibly there is no remotely current system that needs any
of these, but they won't do any harm either.
Most files already use <error.h> directly or indirectly. Four
needed new #include <error.h> directives to use the fallbacks if
needed. The libast Mamfile is updated to make those files depend on
that header.
These are minor fixes I've accumulated over time. The following
changes are somewhat notable:
- Added a missing entry for 'typeset -s' to the man page.
- Add strftime(3) to the 'see also' section. This and the date(1)
addition are meant to add onto the documentation for 'printf %T'.
- Removed the man page the entry for ksh reading $PWD/.profile on
login. That feature was removed in commit aa7713c2.
- Added date(1) to the 'see also' section of the man page.
- Note that the 'hash' command can be used instead of 'alias -t' to
workaround one of the caveats listed in the man page.
- Use an 'out of memory' error message rather than 'out of space'
when memory allocation fails.
- Replaced backticks with quotes in some places for consistency.
- Added missing documentation for the %P date format.
- Added missing documentation for the printf %Q and %p formats
(backported from ksh2020: https://github.com/att/ast/pull/1032).
- The comments that show each builtin's options have been updated.
In 2021, it seems like it's about time to join the 21st century
and officially require fork(2). In practice this was already the
case as the legacy code was unmaintained and didn't compile.
src/cmd/ksh93/edit/edit.c,
src/cmd/ksh93/edit/history.c,
src/cmd/ksh93/sh/deparse.c:
- Remove experimental code protected by '#ifdef future'.
No one is going to do anything with this, it's just clutter.
src/lib/libast/sfio/sfcvt.c:
- In 2021, it might be time to actually start using some C99
features were available. Change two checks for a _c99_in_the_wild
macro to actual checks for C99, enabling the use of fpclassify().
Resolves: https://github.com/ksh93/ksh/issues/219
This removes #ifdefs checking for the existence of
SH_PLUGIN_VERSION (version check for dynamically loaded builtins)
and the SFIO identifiers SF_BUFCONST, SF_CLOSING, SF_APPENDWR,
SF_ATEXIT, all of which are defined by the bundled libast.
While experimenting with #233, a memory segmentation fault occurred.
A search of other emacs issues found a potential matching issue as
described in https://github.com/att/ast/pull/791. Also, a duplicate
PR of https://github.com/att/ast/pull/1489 was submitted. This
commit backports that fix.
src/cmd/ksh93/edit/history.c: hist_word():
- Switch from using strcpy to memmove as the two strings could overlap.
This was failing again on FreeBSD. Replicating the test in a real
session worked as expected.
Apparently, we just cannot rely on external 'vi' utilities playing
well with pty. This test has caused enough trouble. Removed.
I've had ksh crash one too many times when returning to a previous
build directory as I forgot to restore the previously-used CCFLAGS.
bin/package, src/cmd/INIT/package.sh:
- Save each of CC, CCFLAGS, CCLDFLAGS, LDFLAGS, KSH_RELFLAGS on the
first build run. On subsequent runs, compare and refuse to run if
they changed, issuing an informative error message.
- Allow override by exporting FORCE_FLAGS. Don't tell anyone :)
Upon encountering two filenames with multibyte characters starting
with the same byte, a partial multibyte character was completed.
Reproducer (to run in UTF-8 locale):
$ touch XXXá XXXë
$ : XX <== pres tab
$ : XXX^? <== partial multibyte character appears
Note: á is $'\xc3\xa1' and ë is $'\xc3\xab' (same initial byte).
src/cmd/ksh93/edit/completion.c:
- Add multibyte support to the charcmp() and overlaid() functions.
Thanks to Harald van Dijk for useful code and suggestions.
- Add a few missing mbinit() calls. The state of multibyte
processing must be reset before starting a new loop in case a
previous processing run was interrupted mid-character.
src/cmd/ksh93/tests/pty.sh:
- Add test based on Harald's reproducer.
Resolves: https://github.com/ksh93/ksh/issues/223
Until now, when performing any tilde expansion like ~/foo or
~user/foo, ksh added a placeholder built-in command called
'.sh.tilde', ostensibly with the intention to allow users to
override it with a shell function or custom builtin. The multishell
ksh93 repo <https://github.com/multishell/ksh93/> shows this was
added sometime between 2002-06-28 and 2004-02-29. However, it has
never worked and crashed the shell.
This commit replaces that with something that works. Specific tilde
expansions can now be overridden using .set or .get discipline
functions associated with the .sh.tilde variable (see manual,
Discipline Functions).
For example, you can use either of:
.sh.tilde.set()
{
case ${.sh.value} in
'~tmp') .sh.value=${XDG_RUNTIME_DIR:-${TMPDIR:-/tmp}} ;;
'~doc') .sh.value=~/Documents ;;
'~ksh') .sh.value=/usr/local/src/ksh93/ksh ;;
esac
}
.sh.tilde.get()
{
case ${.sh.tilde} in
'~tmp') .sh.value=${XDG_RUNTIME_DIR:-${TMPDIR:-/tmp}} ;;
'~doc') .sh.value=~/Documents ;;
'~ksh') .sh.value=/usr/local/src/ksh93/ksh ;;
esac
}
src/cmd/ksh93/include/variables.h,
src/cmd/ksh93/data/variables.c:
- Add SH_TILDENOD for a new ${.sh.tilde} predefined variable.
It is initially unset.
src/cmd/ksh93/sh/macro.c:
- sh_btilde(): Removed.
- tilde_expand2(): Rewritten. I started out with the tiny version
of this function from the 2002-06-28 version of ksh. It uses the
stack instead of sfio, which is more efficient. A bugfix for
$HOME == '/' was retrofitted so that ~/foo does not become
//foo instead of /foo. The rest is entirely new code.
To implement the override functionality, it now checks if
${.sh.tilde} has any discipline function associated with it.
If it does, it assigns the tilde expression to ${.sh.tilde} using
nv_putval(), triggering the .set discipline, and then reads it
back using nv_getval(), triggering the .get discipline. The
resulting value is used if it is nonempty and does not still
start with a tilde.
src/cmd/ksh93/bltins/typeset.c,
src/cmd/ksh93/tests/builtins.sh:
- Since ksh no longer adds a dummy '.sh.tilde' builtin, remove the
ad-hoc hack that suppressed it from the output of 'builtin'.
src/cmd/ksh93/tests/tilde.sh:
- Add tests verifying everything I can think of, as well as tests
for bugs found and fixed during this rewrite.
src/cmd/ksh93/tests/pty.sh:
- Add test verifying that the .sh.tilde.set() discipline does not
modify the exit status value ($?) when performing tilde expansion
as part of tab completion.
src/cmd/ksh93/sh.1:
- Instead of "tilde substitution", call the basic mechanism "tilde
expansion", which is the term used everywhere else (including the
1995 Bolsky/Korn ksh book).
- Document the new override feature.
Resolves: https://github.com/ksh93/ksh/issues/217
That patch broke the build on Cygwin, where gcc apparently doesn't
have the required atomic addition/subtraction compiler builtins.
The build fails at link time with those functions not found.
As far as I know, ksh was actually working fine (after @JohnoKing's
gcc workaround in c258a04f), so I'll just revert this for now. If a
need for it is demonstrated later, we'll have to add a feature test
or find some other way to get it working on Cygwin.
The package script was not well behaved with these. When you
pressed Ctrl+C, on some shells (including ksh) both the SIGINT (2)
and EXIT (0) traps are activated, showing a double 'make done'
message. The exit status also wasn't > 128 to indicate a signal.
bin/package, src/cmd/INIT/package.sh:
- Be UNIXly well-behaved. Signals should be passed on after
handling, so when one is caught, make the trap handlers print
their message and then unset both itself and EXIT/0 before
resending the signal to self.
"savxit -= SH_EXITSIG + 128;" may have worked accidentally due to
subsequent bitmasking, but is blatantly wrong . It subtracts 256 +
128 = 384 from the exit status.
Use bitwise logic instead, with an octal literal 0200 instead of
128. This makes more sense in this context.
In ksh93r a crash can occur after switching from emacs mode to vi
mode[*]:
$ ENV=/./dev/null ksh2006 -o emacs
$ echo ${.sh.version}
Version M 1993-12-28 r
$ set -o vi
$ <Esc> <r> <r> # This triggers the memory fault
Commit 129614b9 added the OpenSUSE patch for this crash. This commit
adds the regression test for it.
[*]: https://bugzilla.opensuse.org/show_bug.cgi?id=179917
A discipline function could incorrectly influence the value of $?
(exit status of last command) outside its context if it was
triggered without another command being run, e.g. when a prompt
variable is read, or COLUMNS or LINES is set.
Reproducers include:
PS1 prompt:
$ PS1.get() { true; }
$ false
$ echo $?
0
PS2 prompt:
$ PS2.get() { return 13; }
$ \
>
$ echo $?
13
The set discipline is affected too, e.g. COLUMNS and LINES:
$ COLUMNS.set() { return 13; }
$ true
$ (press return)
$ echo $?
13
There are probably other contexts where the shell reads or changes
variables without running commands, allowing their get or set
disciplines to influence $?. So this commit makes ksh save $? for
all .get, .set, .append, and .unset discipline calls.
src/cmd/ksh93/sh/nvdisc.c:
- assign(): Save/restore $? when running a .set/.append/.unset
discipline function.
- lookup(): Save/restore $? when running a .get discipline.
src/cmd/ksh93/tests/pty.sh:
- Add a regression test for $? after displaying a prompt
and when setting a LINES.set discipline function.
src/cmd/ksh93/tests/return.sh:
- The above test fails in script form on ksh93u+ and ksh2020, as
it exposes another form of #117 that occurs after running a
subshell. Add the above regression test here as well
(re: 092b90da).
Co-authored-by: Martijn Dekker <martijn@inlv.org>
Sometimes the shell returned to the prompt before bin/package was
finished writing all of its output. The problem was that 'tee',
which is used to write the output to both the terminal and the log
in arch/*lib/package/gen/make.out, hadn't caught up yet.
bin/package, src/cmd/INIT/package.sh:
- Run the build itself in the background and 'tee' in the
foreground. This way, the script will not terminate until 'tee'
is finished. The build's exit status is obtained with 'wait'.
For most numeric types the last provided one wins out. This commit
closes the gap for -F and -i numerics to not be covered up by other
preceding float types. Note: -u for requesting an unsigned float or
integer was considered and decided to be left alone as it stands,
so as to not allow the variable to become an uppercased string if
the requested options ended with a -u. As it stands for a case when
multiple numeric types are requested, a -u option may be applied
after the last numeric type is processed.
Examples:
-EF becomes -F
-Fi becomes -i
-Fu becomes -F
-uF becomes -F
-Fui becomes -i (because isfloat==1, unsigned is not applied)
-Fiu becomes -iu (isfloat is reset and allows unsigned to be set)
src/cmd/ksh93/bltins/typeset.c: b_typeset():
- Reset attribute bit flags for -E and -X when -F is requested by
adding in NV_EXPNOTE to be removed.
- For -i option if a float precedes it, reset isfloat and -E/-F
attribute bit flags.
- Take into account the impact of the shortint flag on floats.
src/cmd/ksh93/tests/attributes.sh:
- Add some validation tests to confirm that, when a -F follows
either -E or -X, -F is used.
- Add some validation tests to confirm that, when -F/E/X precede
a -i, the variable becomes an integer and not a float.
- Add in various tests when -s followed a float.
There was an issue with tilde expansion if the HOME var is unset.
$ unset HOME
$ echo ~
martijn
Only the username is returned. Users are more likely to expect the
current user's home directory as configured in the OS.
POSIXly, the expansion of ~ is based on the value of HOME. If HOME
is unset, the results are unspecified. After unsetting HOME, in
bash, ~ returns the user's home directory as specified by the OS,
whereas in all other shells, ~ expands to the empty string. Only
ksh93 returns the username. The behaviour of bash is more useful.
Discussion:
https://github.com/ksh93/ksh/pull/225#issuecomment-799074107
src/cmd/ksh93/sh/macro.c,
src/cmd/ksh93/tests/tilde.sh:
- sh_tilde(): Backport fix by Mike Gilbert from ksh2020.
See: https://github.com/att/ast/issues/1391https://github.com/att/ast/pull/1396https://github.com/att/ast/commit/070d365d
- Add test.
src/cmd/ksh93/COMPATIBILITY:
- Note this change.
src/cmd/ksh93/tests/_common:
- Commit aed5c6d7 renamed the err_exit function,
breaking a few tests in glob.sh that call the function
directly instead of using the alias. Restore the function.
src/cmd/ksh93/tests/builtins.sh:
- The dtksh builtins don't have optget option parsing, so
skip the unrecognized options test for those (this of
course only has relevance when running dtksh against the
regression tests).
src/cmd/ksh93/tests/pty.sh:
- If the vi editor couldn't be found on the $PATH, skip the
regression test that involves it.
src/cmd/ksh93/features/options:
- SHOPT_TEST_L: Use 'env test' instead of '/bin/test' to run
external 'test', as the direct path is unportable. Create a test
symlink and verify the positive case as well as the negative.
- SHOPT_SYSRC: Use if...then..fi instead of ... && ... for the last
test to avoid a non-zero exit status of the script, which outputs
a spurious 'no' result like this:
iffe: test: cross{ ... }end ... no
- Add comments for clarity and to make the SHOPT_* names greppable.
Related: https://github.com/ksh93/ksh/issues/219
src/cmd/ksh93/tests/_common:
- Added. This keeps one common version of 'err_exit', 'warning',
and other init code.
src/cmd/ksh93/tests/*.sh:
- Source _common as a dot script.
- Remove 50-odd, occasionally slightly different, versions of the
common code.
- Some minor tweaks.
This commit fixes a long-standing bug (present since at least
ksh93r) that caused a file descriptor leak when passing a process
substitution to a function, or (if compiled with SHOPT_SPAWN) to a
nonexistent command.
The leaks only occurred when ksh was compiled with SHOPT_DEVFD; the
FIFO method was unaffected.
src/cmd/ksh93/sh/xec.c: sh_exec():
- When a process substitution is passed to a built-in, the
remaining file descriptor is closed with sh_iorestore. Do the
same thing when passing a process substitution to a function.
This is done by delaying the sh_iorestore() call to 'setexit:'
where both built-ins and functions terminate and set the exit
status ($?).
This means that call now will not be executed if a longjmp is
done, e.g. due to an error in a special built-in. However, there
is already another sh_iorestore() call in main.c, exfile(), line
418, that handles that scenario.
- sh_ntfork() can fail, so rather than assume it will succeed,
handle a failure by closing extra file descriptors with
sh_iorestore(). This fixes the leak on command not found with
SHOPT_SPAWN.
src/cmd/ksh93/include/defs.h:
- Since the file descriptor leaks are now fixed, remove the
workaround that forced ksh to use the FIFO method.
src/cmd/ksh93/SHOPT.sh:
- Add SHOPT_DEVFD as a configurable option (default: probe).
src/cmd/ksh93/tests/io.sh:
- Add a regression test for the 'not found' file descriptor leak.
- Add a test to ensure it keeps working with 'command'.
Fixes: https://github.com/ksh93/ksh/issues/67
src/cmd/ksh93/edit/history.c:
- Call sh_close() and sh_fcntl() instead of close(2) and fcntl(2),
updating the shell's file descriptor state.
- Mark files close-on-exec on opening them. The history file should
not remain open if ksh execs another process.
- Another fix for an FD check: < 10 instead of < 2.
src/cmd/ksh93/tests/arrays.sh,
src/cmd/ksh93/tests/arrays2.sh:
- Backport some regression tests from ksh93v- for associative
arrays.
src/cmd/ksh93/tests/basic.sh:
- Add ksh93v- regression tests for background process output in
backtick and shared-state command substitutions as well as
functions used in command substitutions.
- Add regression tests for using EXIT traps in subshells. In
ksh93v- and ksh2020 EXIT traps don't work in forked subshells:
https://github.com/att/ast/issues/1452
- The trap builtin shouldn't segfault after receiving an invalid
signal name. ksh2020 regression:
https://github.com/att/ast/issues/1403
- Add a test to make sure invalid flags don't crash ksh.
ksh2020 regression: https://github.com/att/ast/issues/1284
- Test for an illegal seek error when using the 'join' command with
process substitutions. ksh93v- regression:
https://www.mail-archive.com/ast-users@lists.research.att.com/msg00816.html
src/cmd/ksh93/tests/bracket.sh:
- Add some regression tests from ksh93v- for the -eq test operator.
src/cmd/ksh93/tests/builtins.sh:
- Move the regression test for 'exit' in an interactive shell to
the exit.sh script.
- Test for assignments preceding the command builtin persisting
after an error. ksh2020 regression:
https://github.com/att/ast/issues/1402
- The chmod builtin should modify the permissions of all files
passed to it. ksh2020 regression:
https://github.com/att/ast/issues/949
- Add regression tests for the cd builtin. In ksh93v- 2013-10-10
alpha, using cd on a directory without an execute bit doesn't
cause an error. The test for using cd on a normal file was
backported from ksh93v-.
- Backport a ksh93v- regression test for the exit status
from 'kill %'.
src/cmd/ksh93/tests/case.sh:
- Test for a segfault when ksh handles an invalid character class
in a pattern. ksh2020 regression:
https://github.com/att/ast/issues/1409
src/cmd/ksh93/tests/exit.sh:
- Add regression tests from ksh2020 for the 'exit' builtin:
https://github.com/att/ast/commit/d9491d46
src/cmd/ksh93/tests/io.sh:
- Add a regression test from ksh93v- for a process substitution
hang. This test fails in the 93v- 2013 alpha but succeeds in
the 2014 beta.
src/cmd/ksh93/tests/math.sh:
- 'typeset -s foo=30000' adds garbage to $foo in ksh93u+, ksh93v-
and ksh2020:
$ typeset -s foo=30000
$ echo $foo
5#1430000
This bug was fixed in commit 88a6baa1, but that commit didn't
add a regression test for it.
src/cmd/ksh93/tests/variables.sh:
- Add a regression test for $PS4 incorrectly unsetting
${.sh.subshell}: https://github.com/att/ast/issues/1092
This is an attempt to avoid fairly rare intermittent failures
on the GitHub CI runners. Apparently, they are sometimes so
slow that typeahead can still interfere with a test.
On systems where ksh needs to use the older and less secure FIFO
method for process substitutions (which is currently all of them as
the more modern and solid /dev/fd method is still broken, see #67),
process substitutions could leave background processes hanging in
these two scenarios:
1. If the parent process exits without opening a pipe to the child
process forked by the process substitution. The fifo_check()
function in xec.c, which is periodically called to check if the
parent process still exists while waiting for it to open the
FIFO, verified the parent process's existence by checking if the
PPID had reverted to 1, the traditional PID of init. However,
POSIX specifies that the PPID can revert to any implementation-
defined system process in that case. So this breaks on certain
systems, causing unused process substitutions to hang around
forever as they never detect that the parent disappeared.
The fix is to save the current PID before forking and having the
child check if the PPID has changed from that saved PID.
2. If command invoked from the main shell is passed a process
substitution, but terminates without opening the pipe to the
process substitution. In that case, the parent process never
disappears in the first place, because the parent process is the
main shell. So the same infinite wait occurs in unused process
substitutions, even after correcting problem 1.
The fix is to remember all FIFOs created for any number of
process substitutions passed to a single command, and unlink any
remaining FIFOs as they represent unused command substitutions.
Unlinking them FIFOs causes sh_open() in the child to fail with
ENOENT on the next periodic check, which can easily be handled.
Fixing these problems causes the FIFO method to act identically to
the /dev/fd method, which is good for compatibility. Even when #67
is fixed this will still be important, as ksh also runs on systems
that do not have /dev/fd (such as AIX, HP-UX, and QNX), so will
fall back to using FIFOs.
--- Fix problem 1 ---
src/cmd/ksh93/sh/xec.c:
- Add new static fifo_save_ppid variable.
- sh_exec(): If a FIFO is defined, save the current PID in
fifo_save_ppid for the forked child to use.
- fifo_check(): Compare PPID against the saved value instead of 1.
--- Fix problem 2 ---
To keep things simple I'm abusing the name-value pair routines used
for variables for this purpose. The overhead is negligible. A more
elegant solution is possible but would involve adding more code.
src/cmd/ksh93/include/defs.h: _SH_PRIVATE:
- Define new sh.fifo_tree pointer to a new FIFO cleanup tree.
src/cmd/ksh93/sh/args.c: sh_argprocsubs():
- After launching a process substitution in the background,
add the FIFO to the cleanup list before freeing it.
src/cmd/ksh93/sh/xec.c:
- Add fifo_cleanup() that unlinks all FIFOs in the cleanup list and
clears/closes the list. They should only still exist if the
command never used them, however, just run 'unlink' and don't
check for existence first as that would only add overhead.
- sh_exec():
* Call fifo_cleanup() on finishing all simple commands (when
setting $?) or when a special builtin fails.
* When forking, clear/close the cleanup list; we do not want
children doing duplicate cleanup, particularly as this can
interfere when using multiple process substitutions in one
command.
* Process substitution handling:
> Change FIFO check frequency from 500ms to 50ms.
Note that each check sends a signal that interrupts open(2),
causing sh_open() to reinvoke it. This causes sh_open() to
fail with ENOENT on the next check when the FIFO no longer
exists, so we do not need to add an additional check for
existence to fifo_check(). Unused process substitutions now
linger for a maximum of 50ms.
> Do not issue an error message if errno == ENOENT.
- sh_funct(): Process substitutions can be passed to functions as
well, and we do not want commands within the function to clean up
the FIFOs for the process substitutions passed to it from the
outside. The problem is solved by simply saving fifo_tree in a
local variable, setting it to null before running the function,
and cleaning it up before restoring the parent one at the end.
Since sh_funct() is called recursively for multiple-level
function calls, this correctly gives each function a locally
scoped fifo_tree.
--- Tests ---
src/cmd/ksh93/tests/io.sh:
- Add tests covering the failing scenarios.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
This commit fixes two interrelated problems.
1. The -v unary test/[/[[ operator is documented to test if a
variable is set. However, it always returns true for variable
names with a numeric attribute, even if the variable has not
been given a value. Reproducer:
$ ksh -o nounset -c 'typeset -i n; [[ -v n ]] && echo $n'
ksh: n: parameter not set
That is clearly wrong; 'echo $n' should never be reached and the
error should not occur, and does not occur on mksh or bash.
2. Fixing the previous problem revealed serious breakage in short
integer type variables that was being masked. After applying
that fix and then executing 'typeset -si var=0':
- The conditional assignment expansions ${var=123} and
${var:=123} assigned 123 to var, even though it was set to 0.
- The expansions ${var+s} and ${var:+n} incorrectly acted as if
the variable was unset and empty, respectively.
- '[[ -v var ]]' and 'test -v var' incorrectly returned false.
The problems were caused by a different storage method for short
ints. Their values were stored directly in the 'union Value'
member of the Namval_t struct, instead of allocated on the stack
and referred to by a pointer, as regular integers and all other
types do. This inherently broke nv_isnull() as this leaves no
way to distinguish between a zero value and no value at all.
(I'm also pretty sure it's undefined behaviour in C to check for
a null pointer at the address where a short int is stored.)
The fix is to store short ints like other variables and refer
to them by pointers. The NV_INT16P combined bit mask already
existed for this, but nv_putval() did not yet support it.
src/cmd/ksh93/bltins/test.c: test_unop():
- Fix problem 1. For -v, only check nv_isnull() and do not check
for the NV_INTEGER attribute (which, by the way, is also used
for float variables by combining it with other bits).
See also 5aba0c72 where we recently fixed nv_isnull() to
work properly for all variable types including short ints.
src/cmd/ksh93/sh/name.c: nv_putval():
- Fix problem 2, part 1. Add support for NV_INT16P. The code is
simply copied and adapted from the code for regular integers, a
few lines further on. The regular NV_SHORT code is kept as this
is still used for some special variables like ${.sh.level}.
src/cmd/ksh93/bltins/typeset.c: b_typeset():
- Fix problem 2, part 2. Use NV_INT16P instead of NV_SHORT.
src/cmd/ksh93/tests/attributes.sh:
- Add set/unset/empty/nonempty tests for all numeric types.
src/cmd/ksh93/tests/bracket.sh,
src/cmd/ksh93/tests/comvar.sh:
- Update a couple of existing tests.
- Add test for [[ -v var ]] and [[ -n ${var+s} ]] on unset
and empty variables with many attributes.
src/cmd/ksh93/COMPATIBILITY:
- Add a note detailing the change to test -v.
src/cmd/ksh93/data/builtins.c,
src/cmd/ksh93/sh.1:
- Correct 'typeset -C' documentation. Variables declared as
compound are *not* initially unset, but initially have the empty
compound value. 'typeset' outputs them as:
typeset -C foo=()
and not:
typeset -C foo
and nv_isnull() is never true for them. This may or may not
technically be a bug. I don't think it's worth changing, but
it should at least be documented correctly.
These expansions are supposed to yield all variable names beginning
with the indicated prefix. This should include the variable name
that is identical to the prefix (as 'prefix' begins with 'prefix').
This bugfix is backported from the abandoned ksh 93v- beta, so AT&T
intended this change. It also makes ksh work like bash in this.
src/cmd/ksh93/sh/macro.c: varsub(): M_NAMESCAN:
- Check if the prefix itself exists. If so, start with that.
src/cmd/ksh93/tests/variables.sh:
- Add tests for these expansions.
src/cmd/ksh93/sh.1:
- Fix the incomplete documentation of these expansions.
src/cmd/ksh93/COMPATIBILITY:
- Note the change as it's potentially incompatible in corner cases.
Resolves: https://github.com/ksh93/ksh/issues/183
Removing the nv_putval() calls also stopped making sure the
NV_NOFREE attribute was set for those variables, causing an invalid
free later on. This caused the funcname.ksh script:
https://gist.github.com/ormaaj/12874b68acd06ee98b59
to crash even more readily than it did before.
Even after this commit there are various crashing bugs left for
that script, all intermittent and with different backtraces and
dependent on the operating system and malloc variant used.
Investigation ongoing at: https://github.com/ksh93/ksh/issues/212
This commit fixes at least three bugs:
1. When issuing 'typeset -p' for unset variables typeset as short
integer, a value of 0 was incorrectly diplayed.
2. ${x=y} and ${x:=y} were still broken for short integer types
(re: 9f2389ed). ${x+set} and ${x:+nonempty} were also broken.
3. A memory fault could occur if typeset -l followed a -s option
with integers. Additonally, now the last -s/-l wins out as the
option to utilize instead of it always being short.
src/cmd/ksh93/include/name.h:
- Fix the nv_isnull() macro by removing the direct exclusion of
short integers from this set/unset test. This breaks few things
(only ${.sh.subshell} and ${.sh.level}, as far as we can tell)
while potentially correcting many aspects of short integer use
(at least bugs 1 and 2 above), as this macro is widely used.
- union Value: add new pid_t *pidp pointer member for PID values
(see further below).
src/cmd/ksh93/bltins/typeset.c: b_typeset():
- To fix bug 3 above, unset the 'shortint' flag and NV_SHORT
attribute bit upon encountering the -l optiobn.
*** To fix ${.sh.subshell} to work with the new nv_isnull():
src/cmd/ksh93/sh/defs.h:
- Add new 'realsubshell' member to the shgd (aka shp->gd) struct
which will be the integer value for ${.sh.subshell}.
src/cmd/ksh93/sh/init.c,
src/cmd/ksh93/data/variables.c:
- Initialize SH_SUBSHELLNOD as a pointer to shgd->realsubshell
instead of using a short value (.s) directly. Using a pointer
allows nv_isnull() to return a positive for ${.sh.subshell} as
a non-null pointer is what it checks for.
- While we're at it, initialize PPIDNOD ($PPID) and SH_PIDNOD
(${.sh.pid}) using the new pdip union member, which is more
correct as they are values of type pid_t.
src/cmd/ksh93/sh/subshell.c,
src/cmd/ksh93/sh/xec.c:
- Update the ${.sh.subshell} increases/decreases to refer to
shgd->realsubshell (a.k.a. shp->gd->realsubshell).
*** To fix ${.sh.level} after changing nv_isnull():
src/cmd/ksh93/sh/macro.c: varsub():
- Add a specific exception for SH_LEVLNOD to the nv_isnull() test,
so that ${.sh.level} is always considered to be set. Its handling
throughout the code is too complex/special for a simple fix, so
we have to special-case it, at least for now.
*** Regression test additions:
src/cmd/ksh93/tests/attributes.sh:
- Add in missing short integer tests and correct the one that
existed. The -si test now yields 'typeset -x -r -s -i foo'
instead of 'typeset -x -r -s -i foo=0' which brings it in line
with all the others.
- Add in some other -l attribute tests for floats. Note, -lX test
was not added as the size of long double is platform dependent.
src/cmd/ksh93/tests/variables.sh:
- Add tests for ${x=y} and ${x:=y} used on short int variables.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
Symptoms of this bug below. These only seem to occur on Linux and
only if you replace your initial login shell by ksh using 'exec'.
1. An erroneous 'Interrupt' message is printed after stopping the
read builtin in a script. Reproducer:
$ exec arch/*/bin/ksh
$ cat ./reproducer.sh
#!/bin/sh
read foo
$ ./reproducer.sh
^C$ <Enter>
[1] + Interrupt ../reproducer.sh
2. Ctrl+C fails to stop /bin/package make. Reproducer:
$ exec arch/*/bin/ksh
$ mv arch arch.old
$ bin/package make
# Press Ctrl+C multiple times
Analysis: In 41ebb55a, I made an error in changing job_init() to
work correctly on non-interactive shells. This line from before:
552| if(possible = (setpgid(0,job.mypgid)>=0) || errno==EPERM)
was changed to:
555| possible = (setpgid(0,job.mypgid) >= 0);
556| if(sh_isoption(SH_INTERACTIVE) && (possible || errno==EPERM))
That is wrong. Before, 'possible' was set to 1 (true) if setpgid()
either succeeded or failed with EPERM. After, it is only set to 1
if setpgid() succeeds. As a result, job control initialisation is
aborted later on upon a test for non-zero 'possible'.
src/cmd/ksh93/sh/jobs.c: job_init():
- Once again set possible to 1 even if setpgid() fails with EPERM.
Thanks to @JohnoKing for the bug report and reproducers.
Resolves: https://github.com/ksh93/ksh/issues/210
The fix for '.' and '..' in regular globbing broke '.' and '..' in
globstar. No globstar pattern that contains '.' or '..' as any
pathname component still matched. This commit fixes that.
This commit also makes symlink/** mostly work, which it never has
done in any ksh93 version. It is correct and expected that symlinks
found by patterns are not resolved, but symlinks were not resolved
even when specified as explicit non-pattern pathname components.
For example, /tmp/** breaks if /tmp is a symlink (e.g. on macOS),
which looks like a bug.
src/lib/libast/include/glob.h,
src/lib/libast/misc/glob.c: glob_dir():
- Make symlink/** work. we can check if the string pointed to by
pat is exactly equal to *. If so, we are doing regular globbing
for that particular pathname element, and it's okay to resolve
symlinks. If not (if it's **), we're doing globstar and we should
not be matching symlinks.
- Let's also introduce proper identification of symlinks (GLOB_SYM)
and not lump them in with other special files (GLOB_DEV).
- Fix the bug with literal '.' and '..' components in globstar
patterns. In preceding code, the matchdir pointer gets set to the
complete glob pattern if we're doing globstar for the current
pathname element, null if not. The pat pointer gets set to the
elements of the pattern that are still left to be processed;
already-done elements are trimmed from it by increasing the
pointer. So, to do the right thing, we need to make sure that '.'
or '..' is skipped if, and only if, it is the final element in
the pattern (i.e., if pat does not contain a slash) and is not
specified literally as '.' or '..', i.e., only if '.' or '..' was
actually resolved from a glob pattern. After this change,
'**/.*', '**/../.*', etc. do the right thing, showing all your
hidden files and directories without undesirable '.' and '..'
results; '.' and '..' are skipped as final elements, unless you
literally specify '**/.', '**/..', '**/foo/bar/..', etc.
src/cmd/ksh93/COMPATIBILITY:
- Note the symlink/** globstar change.
src/cmd/ksh93/sh.1:
- Try to document the current globstar behaviour more exhausively.
src/cmd/ksh93/tests/glob.sh:
- Add tests. Try to cover all the corner cases.
src/cmd/ksh93/tests/shtests:
- Since tests in glob.sh do not use err_exit, they were not
counted. Special-case glob.sh for counting the tests: count the
lines starting with a test_* function call.
Resolves: https://github.com/ksh93/ksh/issues/146
Analysis: When a syntax error occurs, the shell performs a
longjmp(3) back to exfile() in main.c on line 417:
415| if(jmpval)
416| {
417| Sfio_t *top;
418| sh_iorestore((void*)shp,0,jmpval);
419| hist_flush(shp->gd->hist_ptr);
420| sfsync(shp->outpool);
The first thing it does is restore the file descriptor state
(sh_iorestore), then it flushes the history file (hist_flush), then
it synchronises sfio's logical stream state with the physical
stream state using (sfsync).
However, the fix applied in e999f6b1 caused sh_iorestore() to sync
all sfio streams unconditionally. So this was done before
hist_flush(), which caused unpredictable behaviour, including
temporary and/or permanent history corruption, as this also synched
shp->outpool before hist_flush() had a chance to do its thing.
The fix is to only call sfsync() in sh_iorestore() if we're
actually about to call ftruncate(2), and not otherwise.
Moral of the story: bug fixes should be as specific as possible to
minimise the risk of side effects.
src/cmd/ksh93/sh/io.c: sh_iorestore():
- Only call sfsync() if we're about to truncate a file.
src/cmd/ksh93/tests/pty.sh:
- Add test.
Thanks to Marc Wilson for reporting the bug and to Johnothan King
for finding the commit that introduced it.
Resolves: https://github.com/ksh93/ksh/issues/209
Relevant: https://github.com/att/ast/issues/61
src/cmd/ksh93/edit/edit.c: ed_read():
- The loop that handles SIGWINCH assumes sfpkrd will return and
set errno to EINTR if ksh is sent SIGWINCH. This only occurs
when select(2) is used to wait for input, so tell sfpkrd to
use select if possible. This is only done if the last argument
given to sfpkrd is '2', which should avoid regressions.
src/lib/libast/sfio/sfpkrd.c: sfpkrd():
- Always use select if the last argument is 2. This allows
sfpkrd() to intercept SIGWINCH when necessary.
Fixes: https://github.com/ksh93/ksh/issues/202
These POSIX expansions first assign y to x if x is unset or empty,
respectively, and then they yield the value of x. This was not
working on any ksh93 version if x was typeset as numeric (integer
or float) but still unset, as in not assigned a value.
$ unset a; typeset -i a; printf '%q\n' "${a:=42}" "$a"
0
''
Expected output:
42
42
src/cmd/ksh93/sh/macro.c:
- Fix the test for set/unset variable. It was broken because it
only checked for the existence of the node, which exists after
'typeset', but did not check if a value had been assigned. This
additional check needs to be done with the nv_isnull() macro, but
only for expansions of the regular M_BRACE type. Special
expansions cannot have an unset state.
- As of commit 95294419, we know that an nv_optimize() call may be
needed before using nv_isnull() if the shell is compiled with
SHOPT_OPTIMIZE. Move the nv_optimize() call from that commit
forward to before the new check that calls nv_isnull(), and only
bother with it if the type is M_BRACE.
src/cmd/ksh93/tests/variables.sh:
- Add tests for this bug. Test float and integer, and also check
that ${a=b} and ${a:=b} correctly treat the value of 'b' as an
arithmetic expression of which the result is assigned to 'a' if
'a' was typeset as numeric.
src/cmd/ksh93/tests/attributes.sh,
src/cmd/ksh93/tests/comvar.sh,
src/cmd/ksh93/tests/nameref.sh,
src/cmd/ksh93/tests/types.sh:
- Fix a number of tests to report failures correctly.
Resolves: https://github.com/ksh93/ksh/issues/157
The old Bourne shell failed to check for closing quotes and command
substitution backticks when encountering end-of-file in a parser
context (such as a script). ksh93 implemented a hack for partial
compatibility with this bug, tolerating unbalanced quotes and
backticks in backtick command subsitutions, 'eval', and command
line invocation '-c' scripts only.
This hack became broken for backtick command substitutions in
fe20311f/350b52ea as a memory leak was fixed by adding a newline to
the stack at the end of the command substitution. That extra
newline becomes part of any string whose quotes are not properly
terminated, causing problems such as the one detailed here:
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01889.html
$ touch abc
$ echo `ls "abc`
ls: abc
: not found
No other fix for the memory leak is known that doesn't cause other
problems. (The alternative fix detailed in the referenced mailing
list post causes a different corner-case regression.)
Besides, the hack has always caused other corner case bugs as well:
$ ksh -c '((i++'
Actual: ksh: i++(: not found
(If an external command 'i++(' existed, it would be run)
Expect: ksh: syntax error at line 1: `(' unmatched
$ ksh -c 'i=0; echo $((++i'
Actual: (empty line; the arithmetic expansion is ignored)
Expect: ksh: syntax error at line 1: `(' unmatched
$ ksh -c 'echo $(echo "hi)'
Actual: ksh: syntax error at line 1: `(' unmatched
Expect: ksh: syntax error at line 1: `"' unmatched
So, it's time to get rid of this hack. The old Bourne shell is
dead and buried. No other shell tries to support this breakage.
Tolerating syntax errors is just asking for strange side effects,
inconsistent states, and corner case bugs. We should not want to do
that. Old scripts that rely on this will just need to be fixed.
src/cmd/ksh93/sh/lex.c:
- struct lexdata: Remove 'char balance' member for remembering an
unbalanced quote or backtick.
- sh_lex(): Remove the back to remember and compensate for
unbalanced quotes/backticks that was executed only if we were
executing a script from a string, as opposed to a file.
src/cmd/ksh93/COMPATIBILITY:
- Note the change.
Resolves: https://github.com/ksh93/ksh/issues/199
That Mac OS X bug workaround is now 23 days shy of the age of
majority, and that bug (symlinks testing as regular files) is
pretty basic, so I'm betting it's fixed by now.
src/lib/libast/include/ast_dir.h:
- Do not disable D_TYPE on macOS.
This commit fixes an arbitrary command execution vulnerability in
array subscripts used within the arithmetic subsystem.
One of the possible reproducers is:
var='1$(echo INJECTION >&2)' ksh -c \
'typeset -A a; ((a[$var]++)); typeset -p a'
Output before this commit:
INJECTION
typeset -A a=([1]=1)
The 'echo' command has been surreptitiously executed from an
external environment variable.
Output after this commit:
typeset -A a=(['1$(echo INJECTION >&2)']=1)
The value is correctly used as an array subscript and nothing in it
is parsed or executed. This is as it should be, as ksh93 supports
arbitrary subscripts for associative arrays.
If we think about it logically, the C-style arithmetic subsystem
simply has no business messing around with shell expansions or
quoting at all, because those don't belong to it. Shell expansions
and quotes are properly resolved by the main shell language before
the arithmetic subsystem is even invoked. It is particularly
important to maintain that separation because the shell expansion
mechanism also executes command substitutions.
Yet, the arithmetic subsystem subjected array subscripts that
contain `$` (and only array subscripts -- how oddly specific) to
an additional level of expansion and quote resolution. For some
unfathomable reason, there are two lines of code doing specifically
this. The vulnerability is fixed by simply removing those.
Incredibly, variants of this vulnerability are shared by bash, mksh
and zsh. Instead of fixing it, it got listed in Bash Pitfalls!
http://mywiki.wooledge.org/BashPitfalls#y.3D.24.28.28_array.5B.24x.5D_.29.29
src/cmd/ksh93/sh/arith.c:
- scope(): Remove these two lines that implement the vulnerability.
if(strchr(sub,'$'))
sub = sh_mactrim(shp,sub,0);
- scope(), arith(): Remove the NV_SUBQUOTE flag from two
nv_endsubscript() calls. That flag causes the array subscript to
retain the current level of shell quoting. The shell quotes
everything as in "double quotes" before invoking the arithmetic
subsystem, and the bad sh_mactrim() call removed one level of
quoting. Since we're no longer doing that, this flag should no
longer be passed, or subscripts may get extra backslash escapes.
src/cmd/ksh93/include/name.h,
src/cmd/ksh93/sh/array.c:
- nv_endsubscript(): The NV_SUBQUOTE flag was only passed from
arith.c. Since it is now unused, remove it.
src/cmd/ksh93/tests/arith.sh:
- Tweak some tests: fix typos, report wrong values.
- Add 21 tests. Most are based on reproducers contributed by
@stephane-chazelas and @hyenias. They verify that this
vulnerability is gone and that no quoting bugs were introduced.
Resolves: https://github.com/ksh93/ksh/issues/152
Corrected the size of attribute(s) being overwritten with 0 when
'readonly' or 'typeset -r' was applied to an existing variable. Since
one cannot set any attributes with the 'readonly' command, its function
call to setall() needs to be adjusted to acquire the current size from
the old size or existing size of the variable. A plain 'typeset -r' is
the same as 'readonly' in that it needs to load the old size as its
current size for use in the subsequent to call to nv_newattr().
src/cmd/ksh93/bltins/typeset.c: setall():
- Both 'readonly' and 'typeset -r' end up calling setall(). setall()
has full visibility into all user supplied values and existing
values that are needed to differentiate whereas name.c newattr()
acquires combined state flags.
- Added a conditional check if the readonly flag was requested by
user then meets the criteria of having present size of 0, cannot
be a numeric nor binary string, and is void of presence of any of
the justified string attributes.
- -L/R/Z justified string attributes if not given a value default
to a size of 0 which means to autosize. A binary string can have
a fixed field size, e.g. -bZ. The present of any of the -L/R/Z
attribules means that current size is valid and should be used
even if it is zero.
src/cmd/ksh93/tests/attributes.sh:
- Added various tests to capture and reiterate that 'readonly' should
be equivalent to 'typeset -r' and applying them should not alter the
previous existing size unless additional attributes are set along
with typeset command.
src/cmd/ksh93/Mamfile:
- regress.c: add missing SH_DICT define for getopt self-doc string,
needed after USAGE_LICENSE macros were removed. (re: ede47996)
src/cmd/ksh93/init.c: sh_init():
- Do not set error_info.exit early in init. This is the function
that is called when an error exits the shell. It defaults to
exit(3). Setting it to sh_exit() early on can cause a crash if an
error is thrown before shell initialisation is fully finished.
So set it at the end of sh_init() instead.
- __regress__: Remove error_info.exit workaround. (re: 506bd2b2)
- Fix SHOPT_P_SUID directive. This is not actually a 0/1 value, so
we should use #ifdef and not #if. If SHOPT_REGRESS is on, it it
set to a function call. (re: 2182ecfa)
src/cmd/ksh93/SHOPT.sh:
- Document that SHOPT_P_SUID cannot be set to 0 to be turned off.
src/cmd/ksh93/tests/basic.sh:
- Fix syntax error (unbalanced single quote) in two -c script
invocations. It only failed to throw a syntax error due to a
problematic hack in ksh that may be removed soon.
See: https://github.com/ksh93/ksh/issues/199
src/cmd/ksh93/tests/builtins.sh,
src/cmd/ksh93/tests/io.sh:
- Redirect standard error on two ksh -i invocations to /dev/null
to work around the test hanging on AIX.
src/cmd/ksh93/tests/comvario.sh:
- Remove duplicate copyright header.
- Fix warning format.
src/cmd/ksh93/tests/functions.sh:
- Fix the 'TERM signal sent to last process of function kills the
script' test so that it works on AIX. We cannot rely on grepping
'ps' output as the external 'sleep' command does not show the
command name on AIX. Instead, find it by its parent PID.
src/cmd/ksh93/tests/locale.sh,
src/cmd/ksh93/tests/substring.sh:
- Rewrite the very broken multibyte locale tests (two outright
syntax errors due to unbalanced quotes, and none of the tests
actually worked).
- Since they set LC_ALL, move them to locale.sh.
src/cmd/ksh93/tests/variables.sh:
- Redirect stderr on some 'ulimit -t unlimited' invocations (which
fork subshells as the intended side effect) to /dev/null in case
that exceeds a system-defined limit.
The referenced commit neglected to add checks for strdup() calls.
That calls malloc() as well, and is used a lot.
This commit switches to another strategy: it adds wrapper functions
for all the allocation macros that check if the allocation
succeeded, so those checks don't need to be done manually.
src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/init.c:
- Add sh_malloc(), sh_realloc(), sh_calloc(), sh_strdup(),
sh_memdup() wrapper functions with success checks. Call nospace()
to error out if allocation fails.
- Update new_of() macro to use sh_malloc().
- Define new sh_newof() macro to replace newof(); it uses
sh_realloc().
All other changed files:
- Replace the relevant calls with the wrappers.
- Remove now-redundant success checks from 18529b88.
- The ERROR_PANIC error message calls are updated to inclusive-or
ERROR_SYSTEM into the exit code argument, so libast's error()
appends the human-readable version of errno in square brackets.
See src/lib/libast/man/error.3
src/cmd/ksh93/edit/history.c:
- Include "defs.h" to get access to the wrappers even if KSHELL is
not defined.
- Since we're here, fix a compile error that occurred with KSHELL
undefined by updating the type definition of hist_fname[] to
match that of history.h.
src/cmd/ksh93/bltins/enum.c:
- To get access to sh_newof(), include "defs.h" instead of
<shell.h> (note that "defs.h" includes <shell.h> itself).
src/cmd/ksh93/Mamfile:
- enum.c: depend on defs.h instead of shell.h.
- enum.o: add an -I. flag in the compiler invocation so that defs.h
can find its subsequent includes.
src/cmd/builtin/pty.c:
- Define one outofmemory() function and call that instead of
repeating the error message call.
- outofmemory() never returns, so remove superfluous exit handling.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
The value of the ${.sh.fun} variable, which is supposed to contain
the name of the function currently being executed, leaks out of the
DEBUG trap if it executes a function. Reproducer:
$ fn() { echo "executing the function"; }
$ trap fn DEBUG
$ trap - DEBUG
executing the function
$ echo ${.sh.fun}
fn
${.sh.fun} should be empty outside the function.
Annalysis:
The sh_debug() function in xec.c, which executes the DEBUG trap
action, contains these lines, which are part of restoring the state
after running the trap action with sh_trap():
nv_putval(SH_PATHNAMENOD,shp->st.filename,NV_NOFREE);
nv_putval(SH_FUNNAMENOD,shp->st.funname,NV_NOFREE);
shp->st = savst;
First the SH_PATHNAMENOD (${.sh.file}) and SH_FUNNAMENOD
(${.sh.fun}) variables get restored from the values in the shell's
scoped information struct (shp->st), but that is done *before*
restoring the parent scope with 'shp->st = savst;'. It should be
done after. Fixing the order is sufficient to fix the bug.
However, I am not convinced that these nv_putval() calls are good
for anything at all. Setting, unsetting, restoring, etc. the
${.sh.fun} and ${.sh.file} variables is already being handled
perfectly well elsewhere in the code for executing functions and
sourcing dot scripts. The DEBUG trap is neither here nor there.
There's no reason for it to get involved with these variables.
I was unable to break anything after simply removing those two
lines. So I strongly suspect this is another case, out of many now,
where a bug in ksh93 is properly fixed by removing some code.
I couldn't get ${.sh.file} to leak similarly -- I think this is
because SH_PATHNAMENOD (and not SH_FUNNOD) is set explicitly in
exfile() in main.c, masking this incorrect restore. It is the only
place where SH_PATHNAMENOD and SH_FUNNOD are not both set.
src/cmd/ksh93/sh/xec.c:
- Remove these two spurious nv_putval() calls.
src/cmd/ksh93/tests/variables.sh:
- Add regression test for leaking ${.sh.fun}.
At init, and then whenever the TERM variable changes, ed_setup()
uses sh_trap() to run the external 'tput' command to get the
current terminal escape sequence for moving up the cursor one line.
A sh_trap() call executes a shell command as if a shell script's
trap action had executed it, so is subject to modes like the
restricted mode. As of 7ff6b73b, we execute tput using its absolute
path (found and hardcoded at compile time) for better
robustness/security. This fails in restricted mode as it does not
allow executing commands by absolute path. But in C, nothing stops
us from turning that off.
src/cmd/ksh93/edit/edit.c: ed_setup():
- Block SIGINT while doing all of the following, so the user can't
interrupt it and escape from restricted mode. Even without that,
it's probably a good idea to do this, so an interrupt doesn't
cause an inconsistent state.
Note that sigblock() and sigrelease() are macros defined in
features/sigfeatures. To get those, we need to include <fault.h>.
- Temporarily turn off SH_RESTRICTED before sh_trap()ping tput to
get the terminal command to move the cursor up one position.
- Avoid potentially using a sequence that was cut off. Only use the
resulting string if its length does not exceed the space reserved
for CURSOR_UP. Otherwise, empty it.
src/cmd/ksh93/Mamfile:
- Add fault.h dependency to edit.c.
src/cmd/ksh93/edit/history.c:
- Fix typos in introductory comment.
1. The editor accepted literal tabs without escaping in certain
cases, causing buggy and inconsistent completion behaviour.
https://github.com/ksh93/ksh/issues/71#issuecomment-656970959https://github.com/ksh93/ksh/issues/71#issuecomment-657216472
2. After completing a filename by choosing from a file completion
menu, the terminal cursor was placed one position too far to the
right, corrupting command line display. This happened with
multiline active.
https://github.com/ksh93/ksh/issues/71#issue-655093805
3. A completion menu was displayed if the file name to be completed
was at the point where the rest of it started with a number,
even if that part uniquely identified it so the menu had 1 item.
https://www.mail-archive.com/ast-users@lists.research.att.com/msg00436.html
src/cmd/ksh93/edit/emacs.c:
- Cosmetic consistency: change two instances of cntl('[') to ESC.
- ed_emacsread(): Fix number 1 by refusing to continue into default
processing if a tab character was not used for tab completion.
Instead, beep and continue to the next read loop iteration. This
behaviour is consistent with most other shells, so I doubt there
will be objections. To enter a literal tab it's simple enough to
escape it with ^V (the 'stty lnext' character) or \.
- draw(): Fix number 2 by correcting an off-by-one error in the
ed_setcursor() call that updates the terminal's cursor display
in multiline mode. The 'old' and 'new' parameters need to have
identical values in this particular call to avoid the cursor
position being off by one to the right. This change makes it
match the corresponding ed_setcursor() call in vi.c. See below*
for details. Thanks to Lev Kujawski for the help in analysing.
src/cmd/ksh93/edit/completion.c: ed_expand():
- Fix number 3 by changing from '=' mode (menu-based completion) to
'\' mode (ordinary filename completion) if the menu would only
show one option, which was pointless and annoying. This never
happened in vi mode, so possibly the ed_expand() call in emacs.c
could have been improved instead. But I'm comfortable with fixing
it here and not in emacs.c, because this fixes it at a more
fundamental level, plus it's straightforward and obvious here.
Resolves: https://github.com/ksh93/ksh/issues/71
____
* Further details on bug number 2:
At https://github.com/ksh93/ksh/issues/71#issuecomment-786391565
Martijn Dekker wrote:
> I'm back to my original hypothesis that there is somehow an
> off-by-one error related to the ed_setcursor() call that gets
> executed when in multiline mode. I cannot confirm whether that
> off-by-one error is actually in the call itself, or occurs
> sometime earlier on one of the many possible occasions where
> ep->cursor is changed. But everything else appears to work
> correctly, so it's not unlikely that the problem is in the call
> itself.
>
> For reference, this is the original version of that call in
> emacs.c:
>
> ksh/src/cmd/ksh93/edit/emacs.c
> Lines 1556 to 1557 in df2b9bf
> if(ep->ed->e_multiline && option == REFRESH)
> ed_setcursor(ep->ed, ep->screen, ep->cursor-ep->screen, ep->ed->e_peol, -1);
>
> There is a corresponding call in the vi.c refresh() function
> (which does the same thing as draw() in emacs.c), where the third
> (old) and fourth (new) arguments are actually identical:
>
> ksh/src/cmd/ksh93/edit/vi.c
>
> Lines 2086 to 2087 in df2b9bf
> if(vp->ed->e_multiline && vp->ofirst_wind==INVALID)
> ed_setcursor(vp->ed, physical, last_phys+1, last_phys+1, -1);
>
> The expectation for this particular call is in fact that they
> should be identical, so that a delta of zero is calculated in
> that function. Delta not being zero is what causes the cursor to
> be positioned wrong.
>
> In vi.c, last_phys is a macro that is defined as editb.e_peol,
> and editb is a macro that is defined as (*vp->ed). Which means
> last_phys means vp->ed->e_peol, which is the same as
> ep->ed->e_peol in emacs.c. (These editors were originally
> separate programs by different authors, and I suppose this is how
> it shows. Korn didn't want to change all the variable names to
> integrate them, so made macros instead.)
>
> That leaves the question of why vi.c adds 1 to both last_phys
> a.k.a. e_peol arguments, and emacs.c uses e_peol for new without
> adding anything. Analysing the ed_setcursor() code could answer
> that question.
>
> So, this patch makes emacs.c do it the same way vi.c does. Let's
> make the third argument identical to the fourth. My brief testing
> shows the bug is fixed, and the regression tests yield no
> failures. This fix is also the most specific change possible, so
> there are few opportunities for side effects (I hope).
At https://github.com/ksh93/ksh/issues/71#issuecomment-786466652
Lev Kujawski wrote:
> I did a bit of research on this, and I think the fix to have the
> Emacs editing mode do the same as Vi is correct.
>
> From RELEASE:
> 08-05-01 In multiline edit mode, the refresh operation will now clear
> the remaining portion of the last line.
>
> Here's a fragment from the completion.c of the venerable but
> dated CDE DtKsh:
>
> else
> while (*com)
> {
> *out++ = ' ';
> out = strcopy(out,*com++);
> }
> *cur = (out-outbuff);
> /* restore rest of buffer */
> out = strcopy(out,stakptr(0));
> *eol = (out-outbuff);
>
> Noticeably missing is the code to add a space after file name
> completions. So, it seems plausible that if multiline editing
> mode was added beforehand,the ep->ed->p_eol !=
> ep->cursor-ep->screen case might never have occurred during
> testing.
>
> Setting the 'first' parameter to -1 seems to be a pretty explicit
> indicator that the author(s) intended the line clearing code to
> run, hence the entry in RELASE.
>
> The real issue is that if we update the cursor by calling
> ed_setcursor on line 1554 with old != new, the later call to
> setcursor on line 1583, here:
>
> I = (ncursor-nscreen) - ep->offset;
> setcursor(ep,i,0);
>
> will use outdated screen information to call setcursor, which,
> coincidentally, calls ed_setcursor.
This bug was backported along with a fix from 93v-. An inconsistent
state occurred if you caused a file name completion menu to appear
with two TABs (which also puts you in command mode) but then
re-enter insert mode (e.g. with 'a') instead of entering a number.
$ set -o vi
$ cd /
$ bin/p [press TAB twice]
1) pax
2) ps
3) pwd [now type 'a', 'wd', return]
$ bin/pwd
> [PS2 prompt wrongly appears; press return]
/
$
Here's another reproducer, suggesting the problem is a write past
the end of the screen buffer:
$ set -o vi
$ cd /
$ bin/p [press TAB twice]
1) pax
2) ps
3) pwd [press '0', then '$']
$ bin/p [cursor is one too far to the right, past the 'p'!]
[Further operations show random evidence of memory corruption]
Harald van Dijk found the cause (thanks!):
> In vi.c's textmod there is
>
> case '=': /** list file name expansions **/
> ...
> ++last_virt;
> ...
> if(ed_expand(vp->ed,(char*)virtual, &cur_virt, &last_virt, ch, vp->repeat_set?vp->repeat:-1)<0)
> {
> ...
> last_virt = i;
> ...
> }
> else if((c=='=' || (c=='\\'&&virtual[last_virt]=='/')) && !vp->repeat_set)
> {
> ...
> }
> else
> {
> ...
> --last_virt;
> ...
> }
> break;
>
> That middle block does not restore last_virt, and everything goes
> wrong after that. That function used to restore last_virt until
> commit 4cecde1 (#41). The commit message says it was taken from
> ksh93v- and indeed this bug is also present in that version too.
> If I restore the last_virt = i; that was there originally, like
> below, then this bug seems to be fixed. I do not know why it was
> taken out, taking it out does not seem to be necessary to fix the
> original bug.
src/cmd/ksh93/edit/vi.c: textmod():
- Restore the missing restore of last_virt.
src/cmd/ksh93/tests/pty.sh:
- Add test that checks basic completion menu functionality works
and runs modified versions of the two reproducers above.
Resolves: https://github.com/ksh93/ksh/issues/195
src/cmd/ksh93/data/builtins.c:
- sh_optksh[]: Edit descriptions of -c and -s options for clarity.
- sh_set[]: The --rc long name equivalent for -E was documented
wrong, but in any case it does not belong in sh_set[], because
that also shows up in 'set --man' and this invocation-only option
cannot be used with 'set'. Remove it. (Note that all other
invocation options already don't have inline documentation of
their long equivalents. This may or may not be fixed at some
point. It is problematic because they should not be documented in
sh_set[] but there is no other good place for them.)
src/cmd/ksh93/sh.1:
- Generally edit the Invocation section for clarity.
- Document the long invocation option equivalents.
- Remove some nonsense from the -s description: "Shell output,
except for the output of the Special Commands listed above, is
written to file descriptor 2" (which is standard error).
In fact, this option has no influence at all on what is written
to standard error or standard output.
Reproducer:
$ ksh -c 'v=${ PATH=/dev/null; }; echo $PATH; whence ls'
/dev/null
/bin/ls
The PATH=/dev/null assignment should survive the shared-state
command substitution, and does, yet 'ls' is still found.
The variable became inconsistent with the internal pathlist.
This bugfix is from the 93v- beta.
src/cmd/ksh93/sh/subshell.c: sh_subshell():
- Do not save and restore pathlist for a subshare.
- A few other subshell tweaks from 93v- that made sense:
. reset shp->subdup (bitmask for dups of 1) after saving it
. use e_dot instead of "." for consistency
. retry close(1) if it was interrupted
src/cmd/ksh93/tests/path.sh:
- Add test for this bug.
On some systems (AIX, HP-UX, OpenBSD) the pty tests may hang.
On all systems except Darwin/macOS, FreeBSD and Linux, the pty
tests show one or more regressions. But when I try out the failing
tests manually in a real session, it seems to work fine. So I
suspect pty is broken and not ksh.
src/cmd/ksh93/tests/pty.sh:
- For now, only run the pty tests on Darwin, FreeBSD and Linux.
src/lib/libast/Mamfile:
- tvsleep.c: Add missing error.h dependency (re: 2f7918de).
(unrelated, but just wasn't worth its own commit)
Most of these changes remove unused variables, functions and labels
to fix -Wunused compiler warnings. Somewhat notable changes:
src/cmd/ksh93/bltins/print.c:
- Removed the unused 'neg' variable.
Patch from ksh2020: https://github.com/att/ast/pull/725
src/cmd/ksh93/bltins/sleep.c:
- Initialized ns to fix three -Wsometimes-uninitialized warnings.
src/cmd/ksh93/edit/{emacs,vi}.c:
- Adjust strncpy size to fix two -Wstringop-truncation warnings.
src/cmd/ksh93/include/shell.h:
- The NOT_USED macro caused many -Wunused-value warnings,
so it has been replaced with ksh2020's macro:
https://github.com/att/ast/commit/19d0620a
src/cmd/ksh93/sh/expand.c:
- Removed an unnecessary 'ap = ' since 'ap' is never read
between stakseek and stakfreeze.
src/cmd/ksh93/edit/vi.c: refresh():
- Undef this function's 'w' macro at the end of it to stop it
potentially interfering with future code changes.
src/cmd/ksh93/sh/nvdisc.c,
src/lib/libast/misc/magic.c,
src/lib/libast/regex/regsubexec.c,
src/lib/libast/sfio/sfpool.c,
src/lib/libast/vmalloc/vmbest.c:
- Fixed some indentation to silence -Wmisleading-indentation
warnings.
src/lib/libast/include/ast.h:
- For clang, now only suppress hundreds of -Wparentheses warnings
as well as a few -Wstring-plus-int warnings.
Clang's -Wparentheses warns about things like
if(foo = bar())
which assigns to foo and checks the assigned value.
Clang wants us to change this into
if((foo = bar()))
Clang's -Wstring-plus-int warns about things like
"string"+x
where x is an integer, e.g. "string"+3 represents the string
"ing". Clang wants us to change that to
"string"[3]
The original versions represent a perfectly valid coding style
that was common in the 1980s and 1990s and is not going to change
in this historic code base. (gcc does not complain about these.)
Co-authored-by: Martijn Dekker <martijn@inlv.org>
In the emacs editor:
1. press ESC
2. change the size of your terminal window
and your screen is mysteriously cleared. (Until recent fixes, the
shell probably also crashed somewhere in the job control code.)
The cause is the way SIGWINCH is handled by ed_read() in edit.c.
For the emacs editor, it sends a Ctrl+L character to the input
buffer. The Ctrl+L command refreshes the command line. And it so
happens that ESC plus Ctrl+L is a command to clear the screen in
the emacs editor.
With the exeption of vi insert/command mode for which it uses a
shared flag, edit.c does not know the state of the editor, because
their data are internal to emacs.c and vi.c. So it doesn't know
whether you're in some mode that treats keyboard input specially.
Which means this way of dealing with SIGWINCH is fundamentally
misdesigned and is not worth fixing.
It gets sillier: in addition to sending keyboard commands, edit.c
was also communicating directly with emacs.c and vi.c via a flag,
e_nocrnl, which means 'please don't make Ctrl+L emit a linefeed'
(it normally refreshes on a new line but that is undesirable for
SIGWINCH). So there is already a hack that breaks the barrier
between edit.c and emacs.c/vi.c. Let's do that properly instead.
As of this commit, ed_read() does not send any fake keystrokes.
Instead, two extern functions, emacs_redraw() and vi_redraw(), are
defined for redrawing the command line. These are put in emacs.c
and vi.c so they have access to relevant static data and functions.
Then, instead of sending keyboard commands to the editor and
returning, ed_read() simply calls the redraw function for the
active editor, then continues and waits for input. Much cleaner.
src/cmd/ksh93/include/edit.h:
- Remove e_nocrnl flag from Edit_t struct.
- Define externs emacs_redraw() and vi_redraw(). Since Emacs_t and
Vi_t types are not known here, we have to declare void* pointers
and the functions will have to use typecasts.
src/cmd/ksh93/edit/edit.c:
- ed_read(): Call emacs_redraw() or vi_redraw() as per above.
- ed_getchar(): Remove comment about a nonexistent while loop.
src/cmd/ksh93/edit/emacs.c:
- Updates corresponding to removal of e_nocrnl flag.
- Add emacs_redraw(). This one is pretty simple. Refresh the
command line, then ed_flush() to update the cursor display.
src/cmd/ksh93/edit/vi.c:
- Updates corresponding to removal of e_nocrnl flag. Also remove a
similar internal 'nonewline' flag which is now equally redundant.
- Move the Ctrl+L handling code (minus writing the newline) into
the vi_redraw() function.
- Change two cases where vi set nonewline and sent Ctrl+L to itself
into simple vi_redraw() calls.
- Add vi_redraw(). This is more complicated as it incorporates the
previous Ctrl+L code. It needs an added refresh() call with a
check whether we're currently in command or insert mode, as those
use different refresh methods. Luckily edit.c already maintains
an *e_vi_insert flag in ed_getchar() that we can use. Since vi's
refresh() already calls ed_flush(), we don't need to add that.
Huge typeset -L/-R adjustment length values were still causing
crashses on sytems with not enough memory. They should error out
gracefully instead of crashing.
This commit adds out of memory checks to all malloc/calloc/realloc
calls that didn't have them (which is all but two or three).
The stkalloc/stakalloc calls don't need the checks; it has
automatic checking, which is done by passing a pointer to the
outofspace() function to the stakinstall() call in init.c.
src/lib/libast/include/error.h:
- Change the ERROR_PANIC exit status value from ERROR_LEVEL (255)
to 77, which is what it is supposed to be according to the libast
error.3 manual page. Exit statuses > 128 for anything else than
signals are not POSIX compliant and may cause misbehaviour.
src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/init.c:
- To facilitate consistency, add a simple extern sh_outofmemory()
function that throws an ERROR_PANIC "out of memory".
src/cmd/ksh93/include/shell.h,
src/cmd/ksh93/data/builtins.c:
- Remove now-redundant e_nospace[] extern message; it is now only
used in one place so it might as well be a string literal in
sh_outofmemory().
All other changed files:
- Verify the result of all malloc/calloc/realloc calls and call
sh_outofmemory() if they fail.
Additional adjustments to previous commit bdb9974 to correct
crashes when the max size of a justified string is requested.
This commit corrects the following:
Before (Ubuntu 64bit):
$ typeset -L $(((1<<31)-1)) s=h; typeset +p s
Segmentation fault (core dumped)
After:
$ typeset -L $(((1<<31)-1)) s=h; typeset +p s
typeset -L 2147483647 s
src/cmd/ksh93/sh/name.c: nv_putval():
- Alter the variables size, dot, and append from int to unsigned
int to prevent unwanted negative values from being expressed.
- By creating size, dot, and append as unsigned ints; (unsigned)
type casting is avoided.
This solves another intermittent crash that happened upon
processing SIGWINCH in the emacs editor. See also: 7ff6b73b
I found this bug while testing ksh 93u+m on OpenBSD. Due to its
pervasive security hardening, this system crashes a program
reliably where others crash it intermittently, which is invaluable.
src/cmd/ksh93/sh/jobs.c: job_reap():
- The pw pointer is not ever given a value if the loop breaks on
line 318-319, but it is used unconditionally on lines 464-470,
Initialise the pointer to null on function entry and do not call
job_list() and job_unpost() if the pointer is still null.
It's amazing what can happen when you compile ksh using standard
malloc (i.e. with AST vmalloc disabled) on OpenBSD. Its security
hardening provokes crashes that reveal decades-old unsolved bugs.
This one is an attempt to access one byte before the beginning of
the command line buffer when the cursor is at the beginning of it.
On this system configuration, it provoked an instant crash whenever
you moved the cursor back to the beginning of the command line,
e.g. with ^A or the cursor keys.
src/cmd/ksh93/edit/emacs.c: draw():
- Check that the cursor is actually past the first position of the
command line buffer before trying to read the position
immediately before it. If not, zero the value.
The following caused an infinite loop:
v=${ exec >/dev/tty; }
v=${ redirect >/dev/tty; }
Even the original authors didn't figure out how to 'exec >foo' or
'redirect >foo' inside a non-forking command substitution, so they
fork it by calling sh_subfork(). If we delete that call, even
normal command substitutions enter into that infinite loop. But of
course a shared-state comsub can never fork as it would no longer
share its state. Without a solution to make this work without
forking, an error message is the only sensible thing left to do.
src/cmd/ksh93/sh/io.c: sh_redirect():
- If we're redirecting standard output (1), the redirection is
permanent as in 'exec'/'redirect' (flag==2), and we're in a
subshare, then error out.
Resolves: https://github.com/ksh93/ksh/issues/128
POSIX warns about "unset PWD" leading to unspecified behavior from
the pwd util, which we then use.
bin/package, src/cmd/INIT/package.sh:
- Determine if the shell has $PWD with a feature test. Only unset
PWD if it does not (the old Bourne shell).
- Clean up 'case' flow.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
ksh crashed in various different and operating system-dependent
ways when attempting to create or apply justification strings
using typeset -L/-R/-Z, especially if large sizes are used.
The crashes had two immediate causes:
- In nv_newattr(), when applying justification attributes, a buffer
was allocated for the justified string that was exactly 8 bytes
longer than the original string. Any larger justification string
caused a buffer overflow (!!!).
- In nv_putval(), when applying existing attributes to a new value,
the corresponding memmove() either did not zero-terminate the
justified string (if the original string was longer than the
justified string) or could read memory past the original string
(if the original string was shorter than the justified string).
Both scenarios can cause a crash.
This commit fixes other minor issues as well, such as a mysterious
8 extra bytes allocated by several malloc/realloc calls. This may
have been some naive attempt to paper over the above bugs. It seems
no one can make any other kind of sense of it.
A readjustment bug with zero-filling was also fixed.
src/cmd/ksh93/sh/name.c:
- nv_putval():
. Get rid of the magical +8 bytes for malloc and realloc. Just
allocate one extra byte for the terminating zero.
. Fix the memmove operation to use strncpy instead, so that
buffer overflows are avoided in both scenarios described above.
Also make it conditional upon a size adjustment actually
happening (i.e. if 'dot' is nonzero).
. Mild refactoring: combine two 'if(sp)' blocks into one;
declare variables only used there locally for legibility.
- nv_newattr():
* Replace the fatally broken "let's allocate string length + 8
bytes no matter the size of the adjustment" routine with a new
one based on work by @hyenias (see comments in #142). It is
efficient with memory, taking into account numeric types,
growing strings, and shrinking strings.
* Fix zero-filling in readjustment after changing the initial
size of a -Z attribute. If the number was zero, all zeros were
still skipped, leaving an empty string.
Thanks to @hyenias for originally identifying this breakage and
laying the groundwork for fixing nv_newattr(), and to @lijog for
the crash analysis that revealed the key to the nv_putval() fix.
Resolves: https://github.com/ksh93/ksh/issues/142
Resolves: https://github.com/ksh93/ksh/issues/181
So now we know what that faulty check for shp->indebug in sh_trap()
was meant to do: it was meant to pass down the trap handler's exit
status, via sh_debug(), down to sh_exec() (xec.c) so that it could
then skip the execution of the next command if the trap's exit
status is 2, as documented in the manual page. As of d00b4b39, exit
status 2 was not passed down, so this stopped working.
This commit reinstates that functionality, but without the exit
status bug in command substitutions caused by the old way.
src/cmd/ksh93/sh/fault.c: sh_trap():
- Save the trap's exit status before restoring the parent
envionment's exit status. Make this saved exit status the return
value of the function. (This does not break anything, AFAICT; the
majority of sh_trap() calls ignore the return value, and the few
that don't ignore it seem to expect it to return exactly this.)
src/cmd/ksh93/sh/xec.c: sh_exec():
- The sh_trap() fix has one side effect: whereas the exit status of
a skipped command was always 2 (as per the trap handler), now it
is always 0, because it gets reset in sh_exec() but no command is
executed. That is probably not a desirable change in behaviour,
so let's fix that here instead: set sh.exitval to 2 when skipping
commands.
src/cmd/ksh93/sh.1:
- Document that ${.sh.command} shell-quotes its arguments for use
by 'eval' and such. This fact was not documented anywhere, AFAIK.
src/cmd/ksh93/shell.3:
- Document that $? (exit status) is made local to trap handlers.
- Document that sh_trap() returns the trap handler's exit status.
src/cmd/ksh93/tests/basic.sh:
- Add test for this bug.
- Add a missing test for the exit status 255 functionality (if a
DEBUG trap handler yields this exit status and we're executing a
function or dot script, a return is triggered).
Fixes: https://github.com/ksh93/ksh/issues/187
Many of the errors fixed in this commit are word repetitions
such as 'the the' and minor spelling errors. One formatting
error in the ksh man page has also been fixed.
On SVR4 platforms, select is a sometimes erroneous wrapper around
the poll system call, so call poll directly for efficiency purposes if
it implies no loss in precision.
See, e.g., http://bugs.motifzone.net/long_list.cgi?buglist=129 .
src/lib/libast/features/tvlib:
- For systems lacking nanosleep, test whether select is truly more
precise than poll.
src/lib/libast/tm/tvsleep.c:
- Verify that tv argument is not null.
- Immediately return if asked to sleep for a duration of zero.
- Immediately initialize timespec in the nanosleep case.
- Revise variable names to use Apps Hungarian.
- Use poll instead of select when there is no loss in precision.
- Check for overflow in the poll case.
- Improve comments.
- Revise arithmetic to work with unsigned types, rather than
casting to long.
- Do not return non-zero if we have slept for a sufficient
time.
In the 93v- beta, they add a newline instead of a space.
This has fewer side effects as final newlines get stripped.
It's still a hack and it would still be nice to have a real fix,
but it seems even the AT&T guys couldn't come up with one.
src/cmd/ksh93/sh/macro.c:
- To somehow avoid a memory leak involving alias substitution,
append a linefeed instead of a space to the comsub buffer.
src/cmd/ksh93/tests/subshell.sh:
- Add test for minor regression caused by the RedHat version.
Compilers like GCC are capable of optimizing away calls like
pow(1,inf), which caused the IEEE compliance feature test within
libast to incorrectly succeed on platforms with non-IEEE behavior.
This is arguably a bug within GCC, as floating point optimizations
should never alter the behavior of code unless IEEE compliance is
explicitly disabled via a flag like -ffast-math. Programs in which
only some calls to pow are optimized away are liable to severely
malfunction.
Thanks to Martijn Dekker for pointing this issue out and the kind
operators of polarhome.com for permitting me gratis use of their
Unix systems.
src/lib/libast/comp/omitted.c:
- Add IEEE compliant functions that wrap powf, pow, and powl.
src/lib/libast/features/float:
- Look for powf, pow, and powl in the C library.
- For compilers that do the right thing, like the native toolchains
of Solaris and UnixWare, use lightweight macros to wrap the pow
functions.
- Use a volatile function pointer through which to access the C
library's pow function in an attempt to defeat code optimization.
- For these overzealous compilers, define pow to _ast_pow so that
the same technique can be used within the above functions.
This commit fixes a bug in the 'read' built-in: it did not properly
skip over multibyte characters. The bug never affects UTF-8 locales
because all UTF-8 bytes have the high-order bit set. But Shift-JIS
characters may include a byte corresponding to the ASCII backslash
character, which cauased buggy behaviour when using 'read' without
the '-r' option that disables backslash escape processing.
It also makes the regression tests compatible with Shift-JIS
locales. They failed with syntax errors.
src/cmd/ksh93/bltins/read.c:
- Use the multibyte macros when skipping over word characters.
Based on a patch from the old ast-developers mailing list:
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01848.html
src/cmd/ksh93/include/defs.h:
- Be a bit smarter about causing the compiler to optimise out
multibyte code when SHOPT_MULTIBYTE is disabled. See the updated
comment for details.
src/cmd/ksh93/tests/locale.sh:
- Put all the supported locales in an array for future tests.
- Add test for the 'read' bug. Include it in a loop that tests
64 SHIFT-JIS character combinations. Only one fails on old ksh:
the one where the final byte corresponds to the ASCII backslash.
It doesn't hurt to test all the others anyway.
src/cmd/ksh93/tests/basic.sh,
src/cmd/ksh93/tests/builtins.sh,
src/cmd/ksh93/tests/quoting2.sh:
- Fix syntax errors that occurred in SHIFT-JIS locales as the
parser was processing literal UTF-8 characters. Not executing
that code is not enough; we need to make sure it never gets
parsed as well. This is done by wrapping the commands containing
literal UTF-8 strings in an 'eval' command as a single-quoted
operand.
.github/workflows/ci.yml:
- Run the tests in the ja_JP.SJIS locale instead of ja_JP.UTF-8.
UTF-8 is already covered by the nl_NL.UTF-8 test run; that should
be good enough.
This commit introduced the following bug, which is worse than the
one that commit fixed: it became impossible to alter the size of an
existing justified string attribute.
Thanks to @hyenias for catching this bug:
https://github.com/ksh93/ksh/issues/142#issuecomment-780931533
$ unset s; typeset -L 100 s=h; typeset +p s; typeset -L 5 s; typeset +p s
typeset -L 100 s
typeset -L 100 s
Expected output:
typeset -L 100 s
typeset -L 5 s
src/cmd/ksh93/sh/name.c:
- Revert.
src/cmd/ksh93/tests/attributes.sh:
- Revert: re-disable tests for minor attribute output regressions.
- Add a test for this bug and potential similar bugs.
A ${ shared-state command substitution; } (internally called
subshare) is documented to share its state with the parent shell
environment, so all changes made within the command substitution
survive outside of it. However, when it is run within a
virtual/non-forked subshell, variables that are not already local
to that subshell will leak out of it into the grandparent state.
Reproducer:
$ ksh -c '( v=${ bug=BAD; } ); echo "$bug"'
BAD
If the variable pre-exists in the subshell, the bug does not occur:
$ ksh -c '( bug=BAD1; v=${ bug=BAD2; } ); echo "$bug"'
(empty line, as expected)
The problem is that the sh_assignok() function, which is
responsible for variable scoping in virtual subshells, does not
ever bother to create a virtual subshell scope for a subshare.
That is an error if a subshare's parent (or higher-up ancestor)
environment is a virtual subshell, because a scope needs to be
created in that parent environment if none exists.
To make this bugfix possible, first we need to get something out of
the way. nv_restore() temporarily sets the subshell's pointer to
the preesnt working directory, shpwd, to null. This causes
sh_assignok() to assume that the subshell is a subshare (because
subshares don't store their own PWD) and refuse to create a scope.
However, nv_restore() sets it to null for a different purpose: to
temporarily disable scoping for *all* virtual subshells, making
restoring possible. This is a good illustration of why it's often
not a good idea to use the same variable for unrelated purposes.
src/cmd/ksh93/sh/subshell.c:
- Add a global static subshell_noscope flag variable to replace the
misuse of sh.shpwd described above.
- sh_assignok():
. Check subshell_noscope instead of shpwd to see if scope
creation is disabled. This makes it possible to distinguish
between restoring scope and handling subshares.
. If the current environment is a subshare that is in a virtual
subshell, create a scope in the parent subshell. This is done
by temporarily making the parent virtual subshell the current
subshell (by setting the global subshell_data pointer to it)
and calling sh_assignok() again, recursively.
- nv_restore(): To disable subshell scope creation while restoring,
set subshell_noscope instead of saving and unsetting sh.shpwd.
src/cmd/ksh93/tests/subshell.sh:
- Add tests. I like tests. Tests are good.
Fixes: https://github.com/ksh93/ksh/issues/143
This commit fixes the following:
1. Emacs mode ignores --nobackslashctrl (re: 24598fed) when in
reverse search.
2. When entering more than one backslash, emacs reverse search mode
deletes multiple backslashes after pressing backspace once.
Reproducer:
$ set --emacs --nobackslashctrl
$ <Ctrl+R> \\\\<Backspace>
3. Except when in reverse search, the backslash fails to escape a
subsequent interrupt character (^C). Reproducer:
$ set --emacs --backslashctrl
$ teststring \<Ctrl+C>
src/cmd/ksh93/edit/emacs.c:
- Disable escaping backslashes in emacs reverse search if
'nobackslashctrl' is enabled.
- Fix the buggy behavior of backslashes in emacs reverse
search by processing backslashes in a loop.
src/cmd/ksh93/tests/pty.sh:
- Add regression tests.
src/cmd/ksh93/sh.1:
- Fix a minor documentation error (^C is the usual interrupt
character, not ^?).
Co-authored-by: Martijn Dekker <martijn@inlv.org>
'case x in esac' should be syntactically correct, but was an error:
$ ksh -c 'case x in esac'
ksh: syntax error at line 1: `case' unmatched
Inserting a newline was a workaround:
$ ksh -c $'case x in\nesac'
(no output)
The problem was that the 'esac' reserved word was not being
recognised if it immediately followed the 'in' reserved word.
src/cmd/ksh93/sh/lex.c: sh_lex():
- Do not turn off recognition of reserved words after 'in' if we're
in a 'case' construct; only do this for 'for' and 'select'.
src/cmd/ksh93/tests/case.sh:
- Add seven regression test for correct recognition of 'esac'.
Only two failed on ksh93. The rest is to catch future bugs.
Fixes: https://github.com/ksh93/ksh/issues/177
This commit fixes the functionality of Alt+D and Alt+H in emacs mode.
These keyboard shortcuts are intended to work on whole words, but
after commit 13c3fb21 their functionality was reduced to deleting only
singular letters:
$ Test word <Alt+H> # This should delete 'word', not just 'd'.
$ Foo <Alt+B> <Alt+D> # This should delete 'Foo', not just 'F'.
Man page entries for reference:
M-d Delete current word.
M-^H (Meta-backspace) Delete previous word.
M-h Delete previous word.
src/cmd/ksh93/edit/emacs.c:
- 'count' cannot be overridden when handling Alt+D or Alt+H,
so add the total number of repetitions to count (the number of
repetitions can't be negative).
- If 'count' is a negative number, set it to one before adding the
number of repetitions.
The following emacs editor 'feature' kept making me want to go back
to bash. I forget a backslash escape in a command somewhere. So I
go back to insert it. I type the \, then want to go forward. My
right arrow key, instead of moving the cursor, then replaces my
backslash with garbage. Why? The backslash escapes the following
control character at the editor level and inserts it literally.
The vi editor has a variant of this which is much less harmful. It
only works in insert mode and the backslash only escapes the next
kill or erase character.
In both editors, this feature is completely redundant with the
'stty lnext' character which is ^V by default -- and works better
as well because it also escapes ^C, ^J (linefeed) and ^M (Return).
[In fact, you could even issue 'stty lnext \\' and get a much more
consistent version of this feature on any shell. You have to type
two backslashes to enter one, but it won't kill your cursor keys.]
If it were up to me alone, I'd simply remove this misfeature from
both editors. However, it is long-standing documented behaviour.
It's in the 1995 book. Plus, POSIX specifies the vi variant of it.
So, this adds a shell option instead. It was quite trivial to do.
Now I can 'set --nobackslashctrl' in my ~/.kshrc. What a relief!
Note: To keep .kshrc compatibile with older ksh versions, use:
command set --nobackslashctrl 2>/dev/null
src/cmd/ksh93/include/shell.h,
src/cmd/ksh93/data/options.c:
- Add new SH_NOBACKSLCTRL/"nobackslashctrl" long-form option. The
"no" prefix shows it to the user as "backslashctrl" which is on
by default. This avoids unexpectedly changing historic behaviour.
src/cmd/ksh93/edit/emacs.c: ed_emacsread(),
src/cmd/ksh93/edit/vi.c: getline():
- Only set the flag for special backslash handling if
SH_NOBACKSLCTRL is off.
src/cmd/ksh93/sh.1,
src/cmd/ksh93/data/builtins.c:
- Document the new option (as "backslashctrl", on by default).
Other minor tweaks:
src/cmd/ksh93/edit/edit.c:
- ed_setup(): Add fallback #error if no tput method is set. This
should never be triggered; it's to catch future editing mistakes.
- escape(): cntl('\t') is nonsense as '\t' is already a control
character, so change this to just '\t'.
- xcommands(): Let's enable the ^X^D command for debugging
information on non-release builds.
src/cmd/ksh93/features/cmds:
- The tput feature tests assumed a functioning terminal in $TERM.
However, for all we know we might be compiling with no tty and
TERM=dumb. The tput commands won't work then. So set TERM=ansi
to use a standard default.
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.
With this patch, the Korn shell can now guarantee that calls to
sleep on systems using the select or poll method always result in
the system clock advancing by that much time, assuming no
interruptions. This compensates for deficiencies in certain
systems, including SCO UnixWare.
Discussion: https://github.com/ksh93/ksh/pull/174
src/lib/libast/tm/tvsleep.c:
- Ensure that at least the time requested to sleep has elapsed
for the select and poll methods.
- Simplify the logic of calculating the time remaining to
sleep and handle the case of an argument of greater than
10e9 nanoseconds being passed to tvsleep.
src/cmd/ksh93/bltins/sleep.c:
- Eliminate the check for EINTR to handle other cases wherein
we have not slept enough.
src/cmd/ksh93/tests/variables.sh:
- Improve the diagnostic message when the sleep test fails.
- Revise the SECONDS function test to expect that we always
sleep for at least the time specified.
src/cmd/ksh93/tests/functions.h:
- Redirect ps stderr to /dev/null. UnixWare ps prints an error
message about not being able to find the controlling terminal
when shtests output is piped, but we are only using ps to find
the PID.
This makes ksh 93u+m build on the following system:
$ uname -a
QNX qnx 6.5.0 2010/07/09-14:44:03EDT x86pc x86
Thanks to polarhome.com for providing the QNX shell account.
There are a number of regressions left to work out:
arrays.sh[636]: copying a large array fails
bracket.sh[129]: /tmp/ksh93.shtests.1753215026.6923/bracket.C/original should be older than /tmp/ksh93.shtests.1753215026.6923/bracket.C/newer
bracket.sh[132]: /tmp/ksh93.shtests.1753215026.6923/bracket.C/newer should be newer than /tmp/ksh93.shtests.1753215026.6923/bracket.C/original
builtins.sh[683]: real_t1 not found after parent directory renamed in subshell
functions.sh[1023]: cannot handle comsub depth > 256 in function
io.sh[252]: <# not working for pipes
io.sh[337]: read -n3 from pipe not working
io.sh[346]: read -n3 from fifo failed -- expected 'a', got 'abc'
io.sh[349]: read -n1 from fifo failed -- expected 'b', got 'd'
io.sh[379]: should have timed out
io.sh[380]: line1 should be 'prompt1: '
io.sh[381]: line2 should be line2
io.sh[382]: line3 should be 'prompt2: '
io.sh[406]: LC_ALL=C read -n2 from pipe 'a bcd' failed -- expected 'a bcd', got 'ab cd'
io.sh[406]: LC_ALL=C.UTF-8 read -n2 from pipe 'a bcd' failed -- expected 'a bcd', got 'ab cd'
jobs.sh[86]: warning: skipping subshell job control test due to non-compliant 'ps'
pty.sh[105]: POSIX sh 026(C): line 120: expected "(Stopped|Suspended)", got EOF
pty.sh[128]: POSIX sh 028(C): line 143: expected "(Stopped|Suspended) \(SIGTTIN\)", got EOF
pty.sh[151]: POSIX sh 029(C): line 166: expected "(Stopped|Suspended) \(SIGTTOU\)", got EOF
signal.sh[310]: kill -TERM $$ failed, required termination by signal 'EXIT'
signal.sh[310]: kill -VTALRM $$ failed, required termination by signal 'EXIT'
signal.sh[310]: kill -PIPE $$ failed, required termination by signal 'EXIT'
(The io.sh failures mean libast sfpkrd() is not working.)
src/lib/libast/obsolete/spawn.c:
- Removed. Didn't compile due to wrong number of arguments to
spawnve(2), but is obsolete and unused.
src/lib/libast/comp/localeconv.c:
- The initialisation of two static 'struct lconv' variables was
done in a way that depended on OS headers declaring the struct
members in a certain order. This holds on most systems, but not
on QNX, and POSIX does not actually specify the order at all:
https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/locale.h.html
So each member must be initialised by name. But C89 does not
support initialising struct members by name, so we have to do it
using an initialiser function that simply assigns the values.
src/lib/libast/comp/spawnveg.c:
- Fix for systems without either P_DETACH or _P_DETACH.
src/lib/libast/features/vmalloc,
src/lib/libast/vmalloc/vmmopen.c,
src/lib/libast/Mamfile:
- Add test for sys/shm.h header. If it doesn't exist, as it doesn't
on QNX, use the stub vmmapopen() as the real one won't compile.
(Mamfile: Add dependency on FEATURE/vmalloc to vmmopen.c.)
src/lib/libast/vmalloc/malloc.c:
- Remove superfluous externs that are already provided by either
AST or system headers. The 'void cfree' extern caused a build
failure on QNX because cfree() is of type int on QNX.
src/lib/libast/comp/conf.tab:
- Remove check for _map_spawnve; src/lib/libast/RELEASE says it was
removed.
That OpenSUSE patch introduced a bug: file descriptors other than 1
that were globally redirected using 'exec' or 'redirect' no longer
survived a ${ shared-state; } command substitution.
Related: https://github.com/ksh93/ksh/issues/128
src/cmd/ksh93/sh/io.c:
- Add check for shp->subshare to the OpenSUSE patch.
src/cmd/ksh93/tests/io.sh:
- Add test.
.github/workflows/ci.yml:
- Go back to wrapping the regression tests in script(1).
src/cmd/ksh93/data/builtins.c:
- Never mind about the stty builtin.
src/cmd/ksh93/tests/pty.sh:
- Refuse to run if there isn't a functioning tty.
- Make sure stty(1) works on /dev/tty by redirecting stdin.
So, the pty regression tests on the Linux GitHub runner all failed.
Let's test an assumption: the reason is that we need the stty
builtin to properly set the pty state, because the OS-provided stty
command does not work if there is no real tty.
src/cmd/ksh93/data/builtins.c:
- Compile in the stty built-in. This adds about 20k to the binary
for a command that most users rarely need and even more rarely
need to be built in, so only compile it in on non-release builds.
src/cmd/ksh93/tests/pty.sh:
- Skip the tests if we cannot either use the stty builtin or change
the state of the real terminal to be compatible with the tests.
The Mac runner is still broken: intermittent pipe- and
signal-related regressions that do not occur on any real Mac.
https://github.com/ksh93/ksh/runs/1892358749
.github/workflows/ci.yml:
- Remove the macOS runner.
src/cmd/ksh93/tests/pty.sh:
- Do not skip pty tests if there is no tty. (On FreeBSD with no
tty, the tty builtin would need to be enabled in builtins.c.)
src/cmd/ksh93/tests/bracket.sh:
- Don't be noisy when skipping unavailable locales.
It is desirable to be able to run the tests on a system without
a functioning tty. Since this distribution comes with its own
pseudo-tty facility, pty, it should be possible to run the few
tests that require a tty on the pseudo-tty instead. I've verified
that they fail as expected on older ksh93.
Discussion: https://github.com/ksh93/ksh/pull/171
src/cmd/ksh93/tests/basic.sh,
src/cmd/ksh93/tests/bracket.sh:
- Remove tests that require a tty.
src/cmd/ksh93/tests/pty.sh:
- Put them here, adapted to work as interactive pty scripts.
src/cmd/ksh93/tests/shtests:
- No longer refuse to run if there is no functioning tty.
.github/workflows/ci.yml:
- Since the tests no longer require a tty, no longer use script(1)
to get a pseudo-tty. Let's see if this works...
- Re-enable the Mac runner (re: 14632361). Maybe it has improved.
src/cmd/ksh93/tests/leaks.sh: Read vsz from UnixWare's ps
UnixWare's ps reports an accurate virtual size, so collecting that is
preferable to trying to parse the real resident size.
The GitHub runners apparently provide a non-working /dev/tty. To
avoid failures and confusion, shtests shold refuse to run the tests
and tell people to use script(1) to simulate a tty. On Linux, it
goes like this:
script -q -e -c 'bin/shtests --your-options-here'
On macOS and FreeBSD, the invocation is:
script -q /dev/null bin/shtests --your-options-here
The NetBSD and OpenBSD variants of script(1) need different
invocations again. They also don't pass down the command's exit
status, so would need a workaround for that.
It would be nice if we could use pty for this as this comes with
the distribution, so would work the same on every OS, but it seems
to be broken for this use case.
src/cmd/ksh93/tests/shtest:
- Use 'test -t 1' with stdout (fd 1) redirected to /dev/tty to
ensure the tty is actually on a terminal.
src/cmd/ksh93/tests/basic.sh:
- Remove superflous check for tty. All tests run through shtests.
Resolves: https://github.com/ksh93/ksh/pull/171
src/cmd/ksh93/features/externs: ARG_EXTRA_BYTES detection:
- Improve detection of extra bytes per argument: on every loop
iteration, recalculate the size of the environment while taking
the amount extra bytes we're currently trying into account. Also
count arguments (argv[]) as they are stored in the same buffer.
On 64-bit Linux with glibc, this now detects 9 extra bytes per
argument instead of 8. An odd number (literally and figuratively)
but apparently it needs it; I do think my method is correct now.
On 64-bit Solaris and macOS, this still detects 8 extra bytes.
(On 64-bit Linux with musl C library, it detects 0 bytes. Nice.)
src/cmd/ksh93/sh/path.c: path_xargs():
- Remove the kludge subtracting twice the size of the environment.
With the feature test fixed, this should no longer fail on Linux.
- Take into account the size of the final null element in the
argument and environment lists.
src/cmd/ksh93/tests/path.sh:
- Do not use awk for the test due to breakage in the system awks
on Solaris/Illumos (hangs) and AIX & UnixWare (drops arguments).
Instead, use (wait for it...) ksh. It's a bit slower, but works.
src/cmd/ksh93/tests/bracket.sh:
- Read the list of installed locales to ensure the locale to be tested
actually exists on the system under test.
- Produce a warning diagnostic for skipped locales.
- Additionally test the en_US.ISO8859-1 and en_US.UTF-8 locales.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
src/cmd/ksh93/features/math.sh:
- Specify ast_float.h within iffehdrs instead of math.h, so that iffe
will pick up on macro substitutions within libast. This should make
any future efforts to remedy floating point behavior easier as well.
- Always include ast_float.h within the generated math header file,
not just on IA64 platforms.
src/cmd/ksh93/tests/arith.sh:
- Test pow(1.0,-Inf) and pow(1.0,NaN) for IEEE compliance as well.
- Test the exponentiation operator (**) in addition, as streval.c,
which processes the same, calls pow() separately.
src/lib/libast/features/float:
- Test the IEEE compliance of the underlying math library's pow()
function and substitute macros producing compliant behavior if
necessary.
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
src/cmd/ksh93/bltins/enum.c:
- enum_type[]: Fix typos; minor edit for style.
- enum_type[], enuminfo(): Make the list of supported values
comma-separated, instead of using a comma at the start of each.
src/cmd/ksh93/sh/nvtype.c:
- sh_opttype[]: Fix typos.
It was easier than expected to fix this one. The many regression
test failures caused by disabling it were all due to one bug:
'typeset -p' output broke when building without this option.
src/cmd/ksh93/sh/nvtree.c: nv_attribute():
- In this function to print the attributes of a name-value pair,
move four lines of code out of #if SHOPT_FIXEDARRAY...#endif that
were inadvertently moved into the #if block in ksh93 2012-05-18.
See the changes to nvtree.c in this multishell repo commit:
https://github.com/multishell/ksh93/commit/aabab56a
src/cmd/ksh93/data/builtins.c:
- Update/rewrite 'typeset -a' documentation.
- Make it adapt to SHOPT_FIXEDARRAY.
- Fix a few typos.
src/cmd/ksh93/tests/arrays2.sh:
- Only one regression test needs a SHOPT_FIXEDARRAY check.
.github/workflows/ci.yml:
- Disable SHOPT_FIXEDARRAY when regression-testing without SHOPTs.
- Enable xtrace, add ':' commands for traced comments. This should
make the CI runner output logs a little more readable.
Many compile-time options were broken so that they could not be
turned off without causing compile errors and/or regression test
failures. This commit now allows the following to be disabled:
SHOPT_2DMATCH # two dimensional ${.sh.match} for ${var//pat/str}
SHOPT_BGX # one SIGCHLD trap per completed job
SHOPT_BRACEPAT # C-shell {...,...} expansions (, required)
SHOPT_ESH # emacs/gmacs edit mode
SHOPT_HISTEXPAND # csh-style history file expansions
SHOPT_MULTIBYTE # multibyte character handling
SHOPT_NAMESPACE # allow namespaces
SHOPT_STATS # add .sh.stats variable
SHOPT_VSH # vi edit mode
The following still break ksh when disabled:
SHOPT_FIXEDARRAY # fixed dimension indexed array
SHOPT_RAWONLY # make viraw the only vi mode
SHOPT_TYPEDEF # enable typeset type definitions
Compiling without SHOPT_RAWONLY just gives four regression test
failures in pty.sh, but turning off SHOPT_FIXEDARRAY and
SHOPT_TYPEDEF causes compilation to fail. I've managed to tweak the
code to make it compile without those two options, but then dozens
of regression test failures occur, often in things nothing directly
to do with those options. It looks like the separation between the
code for these options and the rest was never properly maintained.
Making it possible to disable SHOPT_FIXEDARRAY and SHOPT_TYPEDEF
may involve major refactoring and testing and may not be worth it.
This commit has far too many tweaks to list. Notables fixes are:
src/cmd/ksh93/data/builtins.c,
src/cmd/ksh93/data/options.c:
- Do not compile in the shell options and documentation for
disabled features (braceexpand, emacs/gmacs, vi/viraw), so the
shell is not left with no-op options and inaccurate self-doc.
src/cmd/ksh93/data/lexstates.c:
- Comment the state tables to associte them with their IDs.
- In the ST_MACRO table (sh_lexstate9[]), do not make the S_BRACE
state for position 123 (ASCII for '{') conditional upon
SHOPT_BRACEPAT (brace expansion), otherwise disabling this causes
glob patterns of the form {3}(x) (matching 3 x'es) to stop
working as well -- and that is ksh globbing, not brace expansion.
src/cmd/ksh93/edit/edit.c: ed_read():
- Fixed a bug: SIGWINCH was not handled by the gmacs edit mode.
src/cmd/ksh93/sh/name.c: nv_putval():
- The -L/-R left/right adjustment options to typeset do not count
zero-width characters. This is the behaviour with SHOPT_MULTIBYTE
enabled, regardless of locale. Of course, what a zero-width
character is depends on the locale, but control characters are
always considered zero-width. So, to avoid a regression, add some
fallback code for non-SHOPT_MULTIBYTE builds that skips ASCII
control characters (as per iscntrl(3)) so they are still
considered to have zero width.
src/cmd/ksh93/tests/shtests:
- Export the SHOPT_* macros from SHOPT.sh to the tests as
environment variables, so the tests can check for them and decide
whether or how to run tests based on the compile-time options
that the tested binary was presumably compiled with.
- Do not run the C.UTF-8 tests if SHOPT_MULTIBYTE is not enabled.
src/cmd/ksh93/tests/*.sh:
- Add a bunch of checks for SHOPT_* env vars. Since most should
have a value 0 (off) or 1 (on), the form ((SHOPT_FOO)) is a
convenient way to use them as arithmetic booleans.
.github/workflows/ci.yml:
- Make GitHub do more testing: run two locale tests (Dutch and
Japanese UTF-8 locales), then disable all the SHOPTs that we can
currently disable, recompile ksh, and run the tests again.
src/lib/libast/misc/optget.c:
- Add screen* (which includes tmux) and dtterm* (CDE terminal) to
the glob pattern deciding whether to use ANSI boldface sequences.
- Don't bother parsing the env var if stderr is not on a terminal.
src/cmd/ksh93/sh.1:
- Extend self-documentation documentation; document how optget(3)
uses the ERROR_OPTIONS env var to control boldface output.
- Tweaks and minor edits.
This fixes a bug in libast optget()'s use of emphasis in the
display of --man(uals) via standard error on a terminal.
Symptom:
$ printf --man 2>&1 | more
(ok; emphasis disabled, no escape codes shown)
$ printf --man
(ok; emphasis correctly displayed)
$ printf --man 2>&1 | more
(whoops; emphasis not disabled; escape codes garble 'more' output)
The problem was that the state.emphasis variable was not
initialised and, when set to one, was never reset again
(except through the use of the --api, --html or --nroff option).
The source code also reveals an undocumented feature: if the
environment variable $ERROR_OPTIONS contains 'noemphasi', emphasis
is forced off, else if it contains 'emphasi', it's forced on.
Other characters (such as the final 's' of emphasis) are ignored.
This was also broken (forcing off didn't work) and is now fixed.
src/lib/libast/misc/optget.c:
- Do not assume that enabling emphasis is forever; re-initialise
the state on every relevant getopts invocation.
- Increase the number of terminals on which emphasis is displayed
using ANSI escape codes. (This is a hack and we should ask the OS
for the correct codes, but never mind -- ANSI is now universal.)
The >;word and <>;word redirection operators cannot be used with
the 'exec' builtin, but the 'redirect' builtin (which used to be
an alias of 'command exec') permitted them. However, they do not
have the documented effect of the added ';'. So this commit blocks
those operators for 'redirect' as they are blocked for 'exec'.
It also tweaks redirect's error message if a non-redirection
argument is encountered.
src/cmd/ksh93/sh/parse.c: simple():
- Set the lexp->inexec flag for SYSREDIR (redirect) as well as
SYSEXEC (exec). This flag is checked for in sh_lex() (lex.c) to
throw a syntax error if one of these two operators is used.
src/cmd/ksh93/sh.1, src/cmd/ksh93/data/builtins.c:
- Documentation tweaks.
src/cmd/ksh93/sh/xec.c, src/cmd/ksh93/bltins/misc.c:
- When 'redirect' gives an 'incorrect syntax' (e_badsyntax) error
message, include the first word that was found not to be a valid
redirection. This is simply the first argument, as redirections
are removed from the arguments list.
src/cmd/ksh93/tests/io.sh:
- Update test to reflect new error message format.
Now that the Make Abstract Machine files are maintained manually
and not generated automatically, unused variables are an annoying
distraction -- and there are many.
But the language/format is very simple and very parseable using
shell, awk, etc. -- so this was easy to automate. All variables are
declared with 'setv' and they are used if an expansion of the form
${varname} exists (the braces are mandatory in Mamfiles).
bin/Mamfile_rm_unused_vars:
- Added for reference and future use.
src/*/*/Mamfile:
- Remove all unused 'setv' variable declarations.
Permanent redirections of that form broke in subshells when used
with the 'redirect' command, because I had overlooked one instance
where the new 'redirect' builtin needs to match the behaviour of
the 'exec' builtin.
src/cmd/ksh93/tests/io.sh: sh_exec():
- Do not restore file descriptors in (virtual) subshells for
'redirect' just as this isn't done for 'exec'.
src/cmd/ksh93/tests/io.sh:
- Add regression test for this bug.
- Complete the test for f9427909 which I committed prematurely.
Fixes: https://github.com/ksh93/ksh/issues/167
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.
src/cmd/ksh93/sh/args.c: sh_argprocsub():
- Fix compiler warnings with SHOPT_DEVFD on by including "io.h".
- Without SHOPT_DEVFD, the FIFO code didn't consider that libast's
pathtemp(3) may also fail and return null. Add a check for this.
It was trivial to crash ksh by making an autoloaded function
definition file autoload itself, causing a stack overflow due to
infinite recursion. This commit adds loop detection that stops a
function that is being autoloaded from autoloading itself either
directly or indirectly, without removing the ability of autoloaded
function definition files to autoload other functions.
src/cmd/ksh93/sh/path.c: funload():
- Detect loops by checking if the path of a function to be
autoloaded was already added to a new internal static tree,
and if not, adding it while the function is being loaded.
src/cmd/ksh93/tests/path.sh:
- Add regression test.
- Tweak a couple of others to be freeze- and crash-proof.
NEWS:
- Add this fix + a forgotten entry for the previous fix (6f3b23e6).
Fixes: https://github.com/ksh93/ksh/issues/136
Reproducer from @Saikiran-m:
| ~# sh -c `perl -e 'print "a"x100000'`
| genunix: NOTICE: core_log: sh[1221] core dumped: /var/cores/core.sh.0.1602153496
| Memory fault(coredump)
The crash was in trying to decide whether the name was suitable for
autoloading as a function on $FPATH. This calls strmatch() to check
the name against a regex for valid function name. But the libast
regex code is not designed optimally and uses too much recursion,
limiting the length of the strings it's able to cope with.
src/cmd/ksh93/sh/path.c: path_search():
- Before calling strmatch(), check that the name is shorter than
256 bytes. The maximum length of file names on Linux and macOS is
255 bytes, so an autoload function can't have a name longer than
that anyway.
src/cmd/ksh93/tests/path.sh:
- Add test for this bug.
- Tweak 'command -x' test to not leave a hanging process on Ctrl+C.
Fixes: https://github.com/ksh93/ksh/issues/144
Well, that commit was based on a silly oversight: of course it's
necessary to pass ${KSH_RELFLAGS} to the feature tests too as they
use this flag to determine whether to enable or disable vmalloc.
On further analysis I think the annoying warnings can be solved in
a different way. Quotes (single or double) in 'exec -' commands
don't seem to be special to mamake at all; it looks like they are
passed on to the shell as is. So Mamfile variables are expanded and
the expansions backslash-escaped the same way regardless of quotes.
Which means we can make the shell remove the unwanted level of
backslashes by using double instead of single quotes.
src/*/*/Mamfile:
- On iffe commands, restore ${KSH_RELFLAGS}, using double quotes to
group the compiler command as one argument to iffe.
This reverts an OpenSUSE patch ("libast/comp/conf.sh: apply limits
detection fixes for Linux"). It broke the build on Alpine Linux
with the musl C library (see also e245856f).
This time it was failing on a 64-bit Debian Linux system with very
few and short environment variables. Sigh.
src/cmd/ksh93/sh/path.c:
- Combine the strategy from 63979488 with that of 8f5235a5.
That fix turned out to be insufficient as NixOS has huge
environment variable lists because (due to each software package
being installed in its own directory tree) it has to keep dozens
of directories in variables like XDG_CONFIG_DIRS and others.
The 'command -x' regression test was failing on NixOS.
src/cmd/ksh93/sh/path.c:
- Different strategy. Leave twice the size of the existing
environment free.
Hopefully this will deal with ksh crashing in macOS Terminal.app
once and for all. Trigger: press Command-F to open the find bar,
then press Esc to close it, then press Esc again. Result: crash
somewhere random in the job control code.
Turns out macOS Terminal.app apparently (and wrongly) sends <Esc>
followed by <Ctrl+L> to the terminal, which ksh takes as a sequence
for clearing the screen. The related crash ultimately traced back
to the code for that in emacs.c. The other crash was in the code
for double-ESC file name completion.
This commit also fixes a non-robust invocation of the 'tput'
command by using the direct path found in $(getconf PATH).
src/cmd/ksh93/features/cmds:
- Remove unused tests for the presence of commands
(newgrp,test,id,wc,cut,logname,pfexec).
- Replace 'cmd tput' test by 'pth tput' which will find its path
in $(getconf PATH) and store that path as the macro value.
- Add two tests to determine if 'tput' supports terminfo and/or
termcap codes. (FreeBSD still requires old termcap codes.)
src/cmd/ksh93/edit/emacs.c: escape():
- Fix a crash in the code for double-ESC completion. Check if the
cursor is on a non-zero position; this caused a bus error
(invalid address access) in the subsequent ed_expand call.
- For <Esc><Ctrl+L> (clear screen), fix the strange crash in macOS
Terminal by not using sh_trap() to invoke "tput clear", which
causes ksh itself to invoke that command. ksh apparently doesn't
cope with doing this while SIGWINCH (window size change signal)
is sent by Terminal. The fix is to just use the C standard
system(3) function to invoke tput. This invokes tput via /bin/sh,
but what the hey. (Note that ksh also ran any function or alias
called 'tput' instead of the real command, and that is now also
fixed.)
- Use the new _pth_tput test result to invoke tput with the
hardcoded default system path, increasing robustness.
src/cmd/ksh93/edit/edit.c: ed_setup():
- Use the new _pth_tput test result to invoke tput with the
hardcoded default system path, increasing robustness.
- When getting the escape code for "cursor up", use the new
_tput_terminfo and _tput_termcap test results to determine which
kind of command code to send. This fixes it on FreeBSD.
src/cmd/INIT/iffe.sh:
- Fix "standard system directories" for the cmd test, which were
hardcoded as bin, /etc, /usr/bin, /usr/etc, /usr/ucb. That's both
unportable and antiquated. Replace this with the path output by
'getconf PATH'.
- Add fixes from modernish for 'getconf PATH' output to compensate
for bugs/shortcomigns in NixOS and AIX. Source:
https://github.com/modernish/modernish/blob/9e4bf5eb/lib/modernish/aux/defpath.sh
Ref.: https://github.com/NixOS/nixpkgs/issues/65512
src/lib/libast/comp/conf.tab: PATH:
- Add the NixOS and AIX default path fixes here too; this fixes
'command -p' and the builtin 'getconf PATH' on these systems.
bin/package, src/cmd/INIT/package.sh:
- Re-support being launched with just the command name 'package' in
the command line (if the 'package' command is in $PATH). At least
one other script in the build system does this. (re: 6cc2f6a0)
- Go back three levels (../../..) if we were invoked from
arch/*/bin/package, otherwise we won't find src/cmd/ksh93/SHOPT.sh.
Something similar was previously done in 07cc71b8 from a Debian
patch, and eventually reverted; it redefined the ast atomic
functions asoincint() and asodecint() to be gcc-specific. This
imports the upstream version from the ksh 93v- beta instead.
This commit is based on an OpenSUSE patch:
https://build.opensuse.org/package/view_file/shells/ksh/ksh93-joblock.dif
src/cmd/ksh93/include/jobs.h:
- Replace job locking mechanism with the 93v- version which uses
the atomic libast functions asoincint(), asogetint() and
asodecint(). See: src/lib/libast/man/aso.3
src/cmd/ksh93/sh/jobs.c: job_subsave():
- Revert gcc optimiser bug workaround from c258a04f.
It should now be unnecessary.
I got one intermittent regression test failure due to 'argument
list too long' on a Debian x86_64 system.
src/cmd/ksh93/sh/path.c: path_xargs():
- Leave extra argument space for systems that need extra bytes:
1KiB per extra byte, with a minimum of 2KiB (the old value).
From an OpenSUSE patch:
https://build.opensuse.org/package/view_file/shells/ksh/ksh93-pathtemp.dif
See src/lib/libast/man/path.3 for pathtemp()
and src/lib/libast/man/sfio.3 for sftmp()
src/lib/libast/path/pathtemp.c:
- Error check fix: add an access check wrapper function that checks
if a path was given and if there is enough free space on the
device, setting errno appropriately in case of trouble.
src/lib/libast/sfio/sftmp.c:
- On Linux, use the /dev/shm shared memory objects for the new
temporary file descriptor -- that is, do not access HD or SSD but
only the memory based tmpfs of the POSIX SHM.
This fixes the function that sets ${.sh.match}. Patch from OpenSUSE:
https://build.opensuse.org/package/view_file/shells/ksh/ksh93-limit-name-len.dif
src/cmd/ksh93/sh/init.c: sh_setmatch():
- Fix node size calculation, possibly preventing data corruption.
src/cmd/ksh93/include/ulimit.h: Limit_t:
- Defining the 'name' struct member as 'char name[16]' makes
no sense as the name is being initialised statically in
data/limits.c; just make it a 'char *name' pointer.
This fixes the following regressions marked TODO in attributes.sh:
$ typeset -L 13 bar; readonly bar; typeset -p bar
typeset -r -L 0 foo # exp.: typeset -r -L 13 foo
$ typeset -R 13 bar; readonly bar; typeset -p bar
typeset -r -R 0 bar # exp.: typeset -r -R 13 bar
$ typeset -Z 13 baz; readonly baz; typeset -p baz
typeset -r -Z 0 -R 0 baz # exp.: typeset -r Z 13 -R 13 baz
I've discovered that these were briefly fixed between fdb9781e (Red
Hat patch for typeset -xu/-xl) and 95fe07d8 (reversal of patch,
different -xu/-xl fix, but reintroduced these regressions).
src/cmd/ksh93/sh/name.c: nv_newattr():
- Replace check from 95fe07d8 with a new one that combines its
approach with that of fdb9781e: do not change size (and hence
return early) if NV_RDONLY and/or NV_EXPORT are the only
attributes that are changing.
src/cmd/ksh93/tests/attributes.sh:
- Enable the TODO regression tests.
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
Turns out the assumption I was operating on, that Linux and macOS
align arguments on 32 or 64 bit boundaries, is incorrect -- they
just need some extra bytes per argument. So we can use a bit more
of the arguments buffer on these systems than I thought.
src/cmd/ksh93/features/externs:
- Change the feature test to simply detect the # of extra bytes per
argument needed. On *BSD and commercial Unices, ARG_EXTRA_BYTES
shows as zero; on Linux and macOS (64-bit), this yields 8. On
Linux (32-bit), this yields 4.
src/cmd/ksh93/sh/path.c: path_xargs():
- Do not try to calculate alignment, just add ARG_EXTRA_BYTES to
each argument.
- Also add this when substracting the length of environment
variables and leading and trailing static command arguments.
src/cmd/ksh93/tests/path.sh:
- Test command -v/-V with -x.
- Add a robust regression test for command -x.
src/cmd/ksh93/data/builtins.c, src/cmd/ksh93/sh.1:
- Tweak docs. Glob patterns also expand to multiple words.
iffe feature test that add a -D_LARGEFILE64_SOURCE compiler flag to
detect the presence of 64-bit types like off64_t are very
incorrect; they always find the type even if the rest of the source
is not compiled with that flag, causing an inconsistent compilation
environment. This was the cause of mysterious failures to compile
some feature tests on Linux i386 -- it tried to use an off64_t type
that was wrongly detected.
A flag like -D_LARGEFILE64_SOURCE needs to be added to the compiler
flags consistently so it is used for compiling all files and tests.
src/lib/libast/features/dirent,
src/lib/libast/features/fs,
src/lib/libast/features/lib,
src/lib/libast/features/mmap,
src/cmd/ksh93/features/rlimits:
- Remove the -D_LARGEFILE64_SOURCE flag from all the tests that
used it.
- Fix some preprocessor directives for compiling without
_LARGEFILE64_SOURCE. We cannot rely on the result of the _lib_*64
tests because those functions are still found in glibc even if
_LARGEFILE64_SOURCE is not defined; we have to check for the
existence of the type definitions before using them.
src/cmd/INIT/cc.linux.i386,
src/cmd/INIT/cc.linux.i386-icc:
- Add/update compiler wrappers to hardcode -D_LARGEFILE64_SOURCE
in the flags for the default compiler. If it is overriden with
$CC, then it needs to be added manually if desired.
This test depends on the correctness of the locale data provided
by the OS, and some installations are broken. Failures of this test
most likely do not represent a bug in ksh or libast.
This takes another small step towards disentangling the build
system from the old AT&T environment. The USAGE_LICENSE macros with
author and copyright information, which was formerly generated
dynamically for each file from a database, are eliminated and the
copyright/author information is instead inserted into the AST
getopt usage strings directly.
Repetitive license/copyright information is also removed from the
getopt strings in the builtin commands (src/lib/libcmd/*.c and
src/cmd/ksh93/data/builtins.c). There's no need to include 55
identical license/copyright strings in the ksh binary; one (in the
main ksh getopt string, shown by ksh --man) ought to be enough!
This makes the ksh binary about 10k smaller.
It does mean that something like 'enum --author', 'typeset
--license' or 'shift --copyright' will now not show those notices
for those builtins, but I doubt anyone will care.
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.
Three OpenSUSE patches from:
https://build.opensuse.org/package/show/shells/ksh
As usual, the relevant bug is not currently public:
https://bugzilla.opensuse.org/show_bug.cgi?id=844071
src/cmd/ksh93/sh/xec.c: sh_debug()/sh_exec():
- Fix stk restoration. [bnc#844071]
src/lib/libast/misc/stk.c:
- Fix stk aliasing code. [bnc#844071]
(ksh93-stkalias.dif)
- Make a unknown location fatal in stkset() so that we get a core
dump right away instead of later in an unrelated part of code.
(ksh93-stkset-abort.dif)
src/lib/libast/man/stk.3,
src/lib/libast/man/stak.3:
- Update manual with new stkset() behaviour. (93u+m addition)
(Note that stak is implemented as macros that translate to stk)
This backports most of the Cdt (container data types) mechanism
from the ksh 93v- beta, based on ground work done by OpenSUSE:
https://build.opensuse.org/package/view_file/shells/ksh/ksh93-dttree-crash.dif
plus adaptations to match ksh 93u+m and an updated manual page
(src/lib/libast/man/cdt.3) added directly from the 93v- sources.
| Thu Dec 20 12:48:02 UTC 2012 - werner@suse.de
|
| - Add ksh93-dttree-crash.dif - Allow empty strings in (dt)trees
| (bnc#795324)
|
| Fri Oct 25 14:07:57 UTC 2013 - werner@suse.de
|
| - Rework patch ksh93-dttree-crash.dif
As usual, precious little information is available because the
OpenSUSE bug report is currently closed to the public:
https://bugzilla.opensuse.org/show_bug.cgi?id=795324
However, a cursory inspection suggests that this code contains
improvements to do with concurrent processing and related
robustness. The new cdt.3 manual page adds a lot about that.
This has been in production use on OpenSUSE for a long time,
so hopefully this will make ksh a little more stable again.
Only one way to find out: let's commit and test this...
BTW, to get a nice manual, use groff and ghostscript's ps2pdf:
$ groff -tman src/lib/libast/man/cdt.3 | ps2pdf - cdt.3.pdf
Commit 308696ec caused the build to fail on macOS Catalina.
src/cmd/INIT/iffe.sh:
- Fix a blatantly unportable practice of passing multiple
"|"-separated 'case' patterns through a variable. This was a way
of grepping for some headers including stdio.h, but it only works
this way on ksh93 and possibly the original Bourne shell, and not
on *any* other shell (not even pdksh or mksh) -- and the fact
that it works on ksh93 is arguably a bug. Fix by eliminating the
"noext" variable (which is init'ed once and never changes) and
using the pattern in the relevant 'case' statement directly.
src/cmd/builtin/features/pty:
- No matter what I try, including <stdio.h> causes the build to
fail on Gentoo Linux (i386) with mysterious "invalid identifier:
off64_t" errors -- this is probably some AST preprocessor hackery
gone awry, but I've no idea where to even begin with that. This
works around the problem by using AST sfio instead, which is
built and functional by the time this feature test is run.
- Remove explicit extern declaration for ptsname(2) that was never
used because it depended on an npt_ptsname feature test that
doesn't exist (or no longer exists).
- Add missing <fcntl.h>, <stdlib.h>, and <unistd.h> for open(2),
ptsname(2) and close(2), respectively.
src/lib/libast/features/float,
src/lib/libast/features/sfio,
src/lib/libast/features/stdio:
- Re-include <stdio.h>.
Fixes: https://github.com/ksh93/ksh/issues/164 (I hope)
This fixes annoying warnings from feature tests that show up when
building with IFFEFLAGS=-d1 (show compiler output from iffe), e.g.:
| In file included from <built-in>:367:
| <command line>:3:26: warning: missing terminating '"' character [-Winvalid-pp-token]
| #define _AST_git_commit \"a5c53a59\"
| ^
| 1 warning generated.
This means the double quotes were incorrectly escaped, which is
probably a bug in mamake -- but they're done correctly for the .c
files that actually need these flags. I may or may not trace the
mamake bug sometime.
src/*/*/Mamfile:
- Remove ${KSH_SHOPTFLAGS} en ${KSH_RELFLAGS} from the iffe
invocations; they are not relevant for feature tests, only when
actually compiling .c files (the $CC commands).
src/cmd/builtin/pty.c:
- Add missing #include <signal.h>.
- No need to limit SIGTTOU handling to Linux only -- it is POSIX
compliant. Change #ifdef __linux__ to #ifdef SIGTTOU.
- The ECHOKE flag is not POSIX, so protect it with an #ifdef.
- s/slave/minion/g because minions are way more fun.
The OpenSUSE patch uses cfmakeraw(3) which is on Linux, BSD and
macOS, but not portable. The build failed on Solaris and variants.
src/cmd/builtin/features/pty:
- Add simple test for the presence of cfmakeraw(3). I love iffe.
src/cmd/builtin/pty.c:
- Add POSIX compliant fallback flaggery for systems without it.
This makes ksh build at least on AIX 7.1 on RISC (PowerPC).
There are 4 regression test failures:
leaks.sh[159]: memory leak on PATH reset before PATH search
(leaked approx 220 KiB after 16384 iterations)
pty.sh[351]: POSIX sh 104(C): line 364: expected
"^done\r?\n$", got EOF
signal.sh[280]: subshell ignoring signal does not send
signal to parent (expected 'SIGUSR1', got 'done')
signal.sh[282]: parent does not wait for child to complete
before handling signal
src/cmd/INIT/iffe.sh:
- Unset LIBPATH on AIX. The features/pty output{ ... }end will fail
to link to libiconv otherwise, causing a build failure. See:
https://www.ibm.com/support/pages/member-libiconvso2-not-found-archive
src/cmd/builtin/pty.c:
- CMIN is not defined on AIX, so set it to 1 if it's not defined.
src/cmd/ksh93/README:
- Update list of tested OSs.
iffe --man documents that stdio.h is automatically pre-included for
all feature tests. Including it in the test code is not needed.
You'd think it shouldn't do any harm, but on a Gentoo i386 system,
this include turned out to be the cause of a mysterious 'unknown
type: off64_t' error while compiling the output{ ... }end block in
features/pty. I'm not going to bother with further tracing the
cause of that -- there is some hackery with off64_t defines in the
AST headers that probably has something to do with it.
src/cmd/builtin/features/pty,
src/lib/libast/features/float,
src/lib/libast/features/sfio,
src/lib/libast/features/stdio:
- Remove '#include <stdio.h>' from output{ ... }end blocks.
It was unreasonably hard to debug problems with iffe tests that
fail to compile where they should (particularly output{ ... }end
blocks that write esserntial headers).
In e72543a9 the problem was already somewhat mitigated by making
some of the failing output{ ... }end blocks emit #error directives
so that invalid/incomplete headers would cause an error at a
sensible point, and not a much harder to track error later.
This commit further mitigates the problem by making the Mamfiles
respect an IFFEFLAGS environmenet variable that is prefixed to
every iffe command's arguments. The typical use would be to export
IFFEFLAGS=-d1 to enable debug level 1: show compiler output for all
iffe tests. This now makes it reasonably feasible to detect
problems in the feature tests themselves.
src/**/Mamfile:
- Import IFFEFLAGS environment variable using setv.
- Prefix ${IFFEFLAGS} to every iffe command.
src/**/features/*:
- Amend the new fail error messages to recommend exporting
IFFEFLAGS=-d1 to show the cause of the failure.
README.md, TODO:
- Updates.
It is not correct to save sh.ifstable (a.k.a. shp->ifstable) before
calling a function and then restore it after; this can cause field
splitting to malfunction. See 70368c57.
The change to init.c in the Red Hat patch applied in 18b3f4aa
(shp->ifstable[0] = S_EOF) appears to be sufficient.
src/cmd/ksh93/bltins/alarm.c:
- Revert save/restore of sh.ifstable.
src/cmd/ksh93/tests/builtins.sh:
- Tweak the regression test to work correctly on a slower machine,
i.e. a Raspberry Pi running FreeBSD 12.2 arm64 (thanks to hyenias
for providing testing access).
