ksh 93v- beta introduced a regression with nested command
substitutions: backticks nested in $( ) result in misdirected
output. This has never been in 93u+, but since we're often
backporting things, let's avoid backporting this bug. It is also
useful if this shows up when running our bin/shtests against the
actual beta by adding a SHELL=... argument.
Ref.: https://github.com/att/ast/issues/478
src/cmd/ksh93/tests/subshell.sh:
- Add reproducer submitted by the reporter as a regression test.
This bug was originally reported by @lijog in att/ast#7 and has been
reported again in #15. KSH does not save the state of a variable if it
is in a newer scope. This is because of an optimization in sh_assignok
first introduced in ksh93t+ 2010-05-24. Here is the code change in that
version:
return(np);
/* don't bother to save if in newer scope */
- if(!(rp=shp->st.real_fun) || !(dp=rp->sdict))
- dp = sp->var;
- if(np->nvenv && !nv_isattr(np,NV_MINIMAL|NV_EXPORT) && shp->last_root)
- dp = shp->last_root;
- if((mp=nv_search((char*)np,dp,HASH_BUCKET))!=np)
- {
- if(mp || !np->nvfun || np->nvfun->subshell>=sh.subshell)
- return(np);
- }
+ if(sp->var!=shp->var_tree && shp->last_root==shp->var_tree)
+ return(np);
if((ap=nv_arrayptr(np)) && (mp=nv_opensub(np)))
{
This change was originally made to replace a buggier optimization.
However, the current optimization causes variables set in subshells
to wrongly affect the environment outside of the subshell, as the
variable does not get set back to its original value. This patch
simply removes the buggy optimization to fix this problem.
src/cmd/ksh93/sh/subshell.c:
- Remove a buggy optimization that caused variables set in subshells
to affect the environment outside of the subshell.
src/cmd/ksh93/tests/subshell.sh:
- Add a regression test for setting variables in subshells. This
test has to be run from the disk after being created with a here
document because it always returns the expected result when run
directly in the regression test script.
The 'source' alias is now converted into a regular built-in command
so that 'unalias -a' does not remove it, and something like
cmd=source; $cmd name args
will now work.
This is part of the project to replace default aliases that define
essential commands by proper builtins that act identically (except
you now get the actual command's name in any error/usage messages).
src/cmd/ksh93/data/aliases.c:
- Remove 'source' default alias.
src/cmd/ksh93/data/builtins.c,
src/cmd/ksh93/include/builtins.h:
- Define 'source' regular builtin with extra parser ID "SYSSOURCE".
Same definition as '.', minus the BLT_SPC flag indicating a
special builtin. This preserves the behaviour of 'command .'.
- Update sh_optdot[] to include info for 'source --man'.
(Note that \f?\f expands to the current command name.
This allows several commands to share a single --man page.)
src/cmd/ksh93/sh/parse.c:
- In the two places that SYSDOT is checked for, also check for
SYSSOURCE, making sure the two commands are parsed identically.
src/cmd/ksh93/sh.1:
- Remove 'source' default alias.
- Document 'source' regular builtin.
This reverts the patch for the job_lock and job_unlock macros.
As I said in 58560db7, this is very probably a workaround for an
optimiser bug in certain versions of GCC, at least 2017 versions.
In the version I committed, that workaround version never gets
used, because you cannot use #if defined(...) to check for the
presence of a compiler builtin. Thanks to Johnothan King for
keeping an eye on my code and pointing that out to me.
What is needed to test for the presence of a compiler builtin is a
builtin macro called __has_builtin (and it *can* be tested for
using #if defined...()).. This is a clang invention. It was not
added to gcc until version 10, which was only released in a first
stable version just over a month ago.
Ref.: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66970#c14https://gcc.gnu.org/gcc-10/
However, for gcc 10, it seems unlikely the patch is still needed.
(Although it would certainly be a good idea to test that.)
And for the older gcc versions that do need it, we cannot use
__has_builtin, which means we need to define a dummy that always
returns false, so the workaround version is never used.
Ref.: https://github.com/ksh93/ksh/commit/58560db7#commitcomment-39900259
And we cannot use the workaround version unconditionally either,
because it would cause build failures on compilers without the
__sync_fetch_and_add() and __sync_fetch_and_sub() builtins.
Which means the only sensible thing left to do is to revert the
patch for now.
As far as I can tell at this point, for the patch to return to this
upstream in a sensible manner, someone would need to:
1. Write a small C program that tests these macros and somehow
checks for the presence of that optimiser bug. (This is not
going to come from me; my C-fu is simply not good enough.)
2. Incorporate that into the distribution as a test for iffe.
(Few know how iffe works, but it's probably not that hard as
there are plenty of existing tests to use as a template.)
3. Reinsert the workaround version using the macro defined (or not)
by that iffe test, so that it is only compiled in when using a
compiler that actually has the bug in question.
Until then, this can just continue to be an OS-specific patch for
systems with GCC versions that might have that bug.
The more I think about it, the more it seems obvious that commit
07cc71b8 (PR #14) is quite simply a workaround for a GCC optimiser
bug, and (who knows?) possibly an old, long-fixed one, as the bug
report is years old.
The commit also caused ksh to fail to build on HP-UX B.11.11 with
GCC 4.2.3 (hosted at polarhome.com), because it doesn't have
__sync_fetch_and_add() and __sync_fetch_and_sub(). It may fail on
other systems. The GCC documentation says these are legacy:
https://gcc.gnu.org/onlinedocs/gcc/_005f_005fsync-Builtins.html
HELP WANTED: what I would like best is if someone could come up
with some way of detecting this optimiser bug and then error out
with a message along the lines of "please upgrade your broken
compiler". It would probably need to be a new iffe test.
Meanwhile, let's try it this way for a while and see what happens:
src/cmd/ksh93/include/jobs.h:
- Restore original ksh version of job_lock()/job_unlock() macros.
- Use the workaround version only if the compiler has the builtins
__sync_fetch_and_add() and __sync_fetch_and_sub().
ksh segfaults in job_chksave after receiving SIGCHLD
https://bugs.launchpad.net/ubuntu/+source/ksh/+bug/1697501
Eric Desrochers wrote on 2017-06-12:
[Impact]
* The compiler optimization dropped parts from the ksh job
locking mechanism from the binary code. As a consequence, ksh
could terminate unexpectedly with a segmentation fault after
it received the SIGCHLD signal.
[Test Case]
Unfortunately, there is no clear and easy way to reproduce the
segfault.
* But the original reporter of this bug can randomly reproduce
the problem using an in-house ksh script that only works
inside his infrastructure as follow : "ksh
<in-house-script.ksh>" and then once in a while ksh will
segfault as follow :
(gdb) bt
#0 job_chksave (pid=pid@entry=19003) at /build/ksh-6IEHIC/ksh-93u+20120801/src/cmd/ksh93/sh/jobs.c:1948
#1 0x00000000004282ab in job_reap (sig=17) at /build/ksh-6IEHIC/ksh-93u+20120801/src/cmd/ksh93/sh/jobs.c:428
#2 <signal handler called>
...
[Regression Potential]
* Regression risk : low/none expected, the package has been
highly/intensively tested by a user who run over 18M ksh
scripts a day on each of their clusters.
[...]
* The fix has been written by RH and has been proven to work for
them for the last 3 years.
* A test package including the RH fix has been intensively tested
and verified (pre-SRU) by an affected user with positive
feedbacks using a reproducer that segfault without the RH
patch.
* Test package (pre-SRU) feedbacks :
https://bugs.launchpad.net/ubuntu/xenial/+source/ksh/+bug/1697501/comments/7
[Other Info]
* Details about the RH bug :
- https://bugzilla.redhat.com/show_bug.cgi?id=1123467
- https://bugzilla.redhat.com/show_bug.cgi?id=1112306
- https://access.redhat.com/solutions/1253243
- http://rhn.redhat.com/errata/RHBA-2014-1015.html
- ksh.spec
* Fri Jul 25 2014 Michal Hlavinka <email address hidden> - 20120801-10.8
* job locking mechanism did not survive compiler optimization (#1123467)
- patch
* ksh-20120801-locking.patch
Debian bug:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=867181
[Original Description]
# gdb
[New LWP 3882]
Core was generated by `/bin/ksh <KSH_SCRIPT>.ksh'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0 job_chksave (pid=pid@entry=19385) at /build/ksh-6IEHIC/ksh-93u+20120801/src/cmd/ksh93/sh/jobs.c:1948
1948 if(jp->pid==pid)
(gdb) p *jp
Cannot access memory at address 0xb
(gdb) p *jp->pid
Cannot access memory at address 0x13
(gdb) p pid
$2 = 19385
(gdb) p *jpold
$1 = {next = 0xb, pid = -604008960, exitval = 11124}
The struct is corrupted at some point looking at the next,pid and
exitval struct members values which isn't valid data.
# assembly code
=> 0x0000000000427159 <+41>: cmp %edi,0x8(%rdx)
(gdb) p $edi ## pid variable
$1 = 19385
(gdb) p *($rdx + 8) ## jp->pid struct
Cannot access memory at address 0x13
--
ksh is segfaulting because it can't access struct "jp" ($rdx)
thus cannot de-reference the struct member "jp>pid" ($rdx + 8) at
line : src/cmd/ksh93/sh/jobs.c:1948 when looking if jp->pid is
equal to pid ($edi) variable.
Prior to this bugfix, the following set of commands would
fail to print two double quotes:
IFS=',' read -S a b c <<<'foo,"""title"" data",bar'
echo $b
This fix is from ksh93v- 2013-10-10-alpha, although it has
been revised to use stakputc to put the required double quote
into the buffer for consistency with the ksh93u+ codebase.
src/cmd/ksh93/bltins/read.c:
- When handling nested double quotes, put the required double
quote in read's buffer with stakputc.
src/cmd/ksh93/tests/builtins.sh:
- Add the regression test for `read -S` from ksh93v-.
src/cmd/ksh93/sh.1:
- Fix a minor formatting error to highlight '-S' in the ksh(1)
man page.
Fixes for implicit declaration warnings
This is to upstream some minor fixes for implicit declaration warnings currently in Debian.
Author: Anuradha Weeraman aweeraman@gmail.com
Reviewed-By: Thorsten Glaser t.glaser@tarent.de
Last-Update: 2020-02-09
src/cmd/ksh93/edit/history.c:
- The `time_t` data type is usually 64-bit, so cast `t` to an
unsigned long and use the correct format specifier with sfprintf.
src/lib/libast/tm/tmdata.c:
- Update the leap second array by adding the current leap
seconds that were absent, being:
2016-12-31+23:59:60 1483228826
2015-06-30+23:59:60 1435708825
2012-06-30+23:59:60 1341100824
I couldn't find a proper list of leap seconds in Unix Epoch time,
so I used an Epoch Converter:
https://www.epochconverter.com/
The converter doesn't support leap seconds, so each conversion
had to be corrected by adding the number of leap seconds required
(e.g. 1230767999 + 24 = 1230768023, for 2008-12-31+23:59:60 in GMT).
'cd' with no argument is supposed to go to your home directory, but
in this override function it produced an error due to an incorrect
use of $@ within another parameter substitution, preventing $@ from
expanding to zero words.
Ref.: https://groups.google.com/d/msgid/korn-shell/781ed34b-6860-5d47-dfe8-be21280a6b31%40inlv.org
src/cmd/ksh93/fun/dirs: _cd():
- Fix the bug by setting a positional parameter if needed,
then using a normal and correct "$@" expansion for \cd.
- While we're here, restore a historic comment about using
'command' to bypass aliases that exists in the listing of this
function in the official 1995 KornShell book (pp. 274-276).
Presumably, that comment was deleted when they decided to add the
obnoxious and broken default alias command='command ', whenever
that was. This branch got rid of that default alias in 61d9bca5.
The regression this commit fixes was first introduced in ksh93t
2008-07-25. It was previously worked around in 6f0e008c by forking
subshells if any special environment variable is unset.
The reason why this problem doesn't occur in ksh93s+ is because in
that version of ksh sh_assignok never moves nodes, it only clones
them. The second argument doesn't set NV_MOVE, which makes
`sh_assignok(np,0)` is similar to `sh_assignok(np,1)`. In ksh93t and
higher, setting the second argument to zero causes the node to be moved
with NV_MOVE, which causes the discipline function associated with
the variable node to be removed when `np->nvfun` is set to zero (i.e.
NULL). This is why a command like `(unset LC_NUMERIC; LC_NUMERIC=invalid)`
doesn't print a diagnostic, as it looses its discipline function.
This patch fixes the problem by cloning the node with sh_assignok
if it is a special variable with a discipline function. This allows
special variables to work as expected in virtual subshells. The
original workaround has been kept for the $PATH variable only, as
hash tables are still broken in virtual subshells. It has been updated
accordingly to only fork subshells if it detects the variable node
for PATH. I have added two more regression tests for changing the
PATH in subshells to make sure hash tables continue working as
expected with this fix.
src/cmd/ksh93/bltins/typeset.c:
- Only fork virtual subshells if the PATH will be changed. If a
variable is a special variable with a discipline function, it
should be just be cloned, not moved.
src/cmd/ksh93/sh/nvdisc.c:
- Add a comment to clarify that NV_MOVE will delete the discipline
function associated with the node.
src/cmd/ksh93/tests/subshells.sh:
- Add two more regression tests for unsetting the PATH in subshells,
one for if PATH is being pointed to by a nameref. Condense the
hash table tests by moving the main test into a single function.
Test 137(C) was failing on some systems because $TMPDIR was set and
the local vi(1) honours it, so that the expected '/tmp/' string was
never output by vi. For compatibility with vi programs that honour
$TMPDIR and those that always use /tmp, we must export TMPDIR=/tmp.
src/cmd/ksh93/tests/pty.sh:
- Export TMPDIR=/tmp for test 137(C).
Note that this exports TMPDIR to the environment for the
duration of the 'tst' function run because the function was
defined using the ksh 'function tst { ...; }' syntax.
We cannot identify the new builtins as coming from "AT&T Research"
as they don't. 'ksh community' is accurate but a bit long. Since
we're now at a GitHub org called ksh93, let's just use that.
The man page for the builtin command says special builtins cannot
be deleted. This wasn't the case though, running `builtin -d` on
a special builtin was deleting it. As an example, the following
set of commands was ending with 'export: not found':
$ builtin -d export
$ export foo=bar
This commit backports the bugfix from ksh93v- (2014-12-24-beta),
which added an error check to prevent special builtins from being
deleted.
src/cmd/ksh93/sh/nvdisc.c:
- Add an error check to prevent special builtins from being deleted.
src/cmd/ksh93/tests/builtins.sh
- Add a regression test for using `builtin -d` on special builtins.
src/cmd/ksh93/tests/io.sh:
- 16 tests for the 'redirect' builtin used the ((arithmetic
command)) to check the result. This does not tolerate the result
being a non-number, such as the empty string, which may occur on
test failure. So use [[ ... -eq ArithExpression ]] to check these
results instead; it simply returns false, failing gracefully.
This commit removes the undocumented 'login' and 'newgrp' builtin
commands. They already stopped blocking shell functions by that
name by changing from special to regular builtins in 04b91718 (a
change I forgot to mention in that commit message), but there is
another obnoxious aspect to these: being glorified hooks into
'exec', they replaced your shell session with the external commands
by the same name. This makes argument and error checking
impossible, so if you made so much as a typo, you would be
immediately logged out.
Even if that behaviour is wanted by a few, having it as the default
is user-hostile enough to be called a bug. It also violates the
POSIX definition of the 'newgrp' utility which explicitly says that
it "shall create a new shell execution environment", not replace
the existing one.
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/newgrp.html
Users who do want this behaviour can easily restore it by setting:
alias login='exec login'
alias newgrp='exec newgrp'
src/cmd/ksh93/bltins/misc.c:
- As there is no more 'login' builtin, combine b_exec() and
B_login() functions, which allows eliminating a few variables.
Note that most of 'exec' was actually implemented in B_login()!
src/cmd/ksh93/data/builtins.c:
- Remove "login" and "newgrp" table entries.
src/cmd/ksh93/include/builtins.h:
- Remove SYSLOGIN parser ID. As this was the first, all the others
needed renumbering.
src/cmd/ksh93/sh/xec.c:
- Remove SYSLOGIN parser check that made 'login' and 'newgrp' act
like 'exec' and replace the shell.
This commit converts the redirect='command exec' alias to a regular
'redirect' builtin command that only accepts I/O redirections, which
persist as in 'exec'. This means that:
* 'unlias -a' no longer removes the 'redirect' command;
* users no longer accidentally get logged out of their shells if
they type something intuitive but wrong, like 'redirect ls >file'.
This should not introduce any legitimate change in behaviour. If
someone did accidentally pass non-redirection arguments to
'redirect', unexpected behaviour would occur; this now produces
an 'incorrect syntax' error.
src/cmd/ksh93/bltins/misc.c: b_exec():
- Recognise 'redirect' when parsing options.
- If invoked as 'redirect', produce error if there are arguments.
src/cmd/ksh93/data/aliases.c:
- Remove redirect='command exec' alias.
src/cmd/ksh93/data/builtins.c:
- Update/improve comments re ordering.
- Add 'redirect' builtin entry.
- sh_optexec[]: Abbreviate redirection-related documentation;
refer to redirect(1) instead.
- sh_optredirect[]: Add documentation.
src/cmd/ksh93/include/builtins.h:
- Add SYSREDIR parser ID, renumbering those following it.
- Improve comments.
- Add extern sh_optredirect[].
src/cmd/ksh93/sh.1:
- exec: Abbreviate redirection-related documentation; refer to
'redirect' instead.
- redirect: Add documentation.
src/cmd/ksh93/sh/xec.c:
- Recognise SYSREDIR parser ID in addition to SYSEXEC when
determining whether to make redirections persistent.
src/cmd/ksh93/tests/io.sh:
- To regress-test the new builtin, change most 'command exec' uses
to 'redirect'.
- Add tests verifying the exit behaviour of 'exec', 'command exec',
'redirect' on redirections.
Both 'alias' and 'unalias' are specified as regular builtins. Among
a few other things, that means it ought to be portable to use these
names for shell functions. But ksh93 disallowed that until now.
src/cmd/ksh93/data/builtins.c:
- Make 'unalias' a regular builtin by removing the BLT_SPC flag.
- (same fix for 'alias' was already done in afa68dca)
- Add the BLT_ENV flag to the 'alias' and 'hash' commands. In
include/name.h, this flag is commented: "non-stoppable, can
modify environment". The "non-stoppable" bit seems like a good
idea: these operations should not be interruptable as that would
cause an inconsistent state.
src/cmd/ksh93/sh.1:
- Remove the '-', indicating special builtin, from 'alias' entry.
- Minor cosmetic fix: space after the '-' for 'unset'.
(cherry picked from commit a4315d7672204acb543010b4d4916b22dcb9cb08)
The b_hash() function duplicated much of its code from b_alias(),
while b_alias() retained some code to support being called as
'hash'. There is no reason why 'hash' and 'alias' can't be handled
with a single function, as is the case several other builtins. Note
that option parsing can easily be made dependent on the name the
command was invoked with (in this case, argv[0]=='h').
The new hash builtin's -r option cleared the hash table by
assigning to PATH its existing value, triggering its associated
discipline function (put_restricted() in init.c) which then
actually cleared the hash table. That's a bit of a hack. It's nicer
if we can just do that directly. This requires taking a static
handler function rehash() from init.c, which invalidates one hash
table entry, and making it available to the builtin.
src/cmd/ksh93/bltins/typeset.c,
src/cmd/ksh93/include/builtins.h,
src/cmd/ksh93/include/nval.h,
src/cmd/ksh93/sh/init.c,
src/cmd/ksh93/sh/name.c:
- Merge b_hash() into b_alias().
- The -x option was still uselessly setting the NV_EXPORT flag.
Exported aliases were in ksh88 but were removed in ksh93.
- Rename rehash() handler function from init.c to nv_rehash
(avoiding a possible conflict with another rehash() in cd_pwd.c)
and move it to name.c just above nv_scan(), which it's meant to
be used with. Make it an extern so typeset.c can use it.
- b_alias(): Replace the PATH assignment by an nv_scan() call to
clear the hash table directly using the nv_rehash() handler.
src/cmd/ksh93/data/builtins.c:
- POSIX compliance fix: Remove BLT_SPC (special builtin) flag from
"alias" definition. 'alias' is specified as a regular builtin.
- sh_optalias[]: Fix uninformative -t option documentation.
- sh_opthash[]: Edit for conciseness and clarity.
src/cmd/ksh93/sh.1:
- Edit the 'alias -t' and 'hash' documentation.
- Remove the -- prefix from the 'alias' entry, which indicated that
it was supposed to be a declaration builtin like 'typeset', with
assignment-arguments expanding tildes and not being subject to
field splitting. However, my testing shows that 'alias' has never
actually behaved that way on ksh93. Even adding the BLT_DCL flag
in data/builtins.c doesn't seem to change that.
(cherry picked from commit afa68dca5c786daa13213973e8b0f9bf3a1dadf6)
The ksh man page documents that the restricted option cannot be
unset once it is set, which means `set +r` should be invalid.
While this was true for `set +o restricted`, `set +r` was causing
the restricted option to be unset. The fix for this problem comes
from one of Solaris' patches, which adds an error check to prevent
this behavior.
Solaris' patch:
https://github.com/oracle/solaris-userland/blob/master/components/ksh93/patches/020-CR6919590.patch
src/cmd/ksh93/sh/args.c:
- Add an error check to stop `set +r` from unsetting the
restricted option.
src/cmd/ksh93/tests/restricted.sh:
- Add two regression tests to make sure the restricted option
cannot be unset.
(cherry picked from commit bef4fee404d8e24b38fce66420c14a39ac4a123e)
Sometimes you have to put ^C on rapid repeat to get out of running
the tests. This is because each test scripts is launch as a child
shell (not a subshell) in the foreground, which thus handle the ^C.
It then exits with a status indicating SIGINT, but the shtest script
wasn't handling this and just kept going.
src/cmd/ksh93/tests/shtests:
- For reasons I don't understand yet, interrupting tests with ^C
tends to make them exit with status 130 (128+2) instead of what
I would expect for ksh93, 258 (256+2). Not a big deal: POSIX
specifies that any exit > 128 signifies a signal in any case,
and 'kill -l' works even on those values. But the checks need
changing to '> 128'.
- Check each result for SIGINT, and issue SIGINT to self if found.
(cherry picked from commit d0dfb37c6c71ac7b157060249125e0959130927d)
This commit replaces the old hash alias with a proper builtin.
I based this builtin off of the code alias uses for handling
`alias -t --`, but with the hack for `--` removed as it has
no use in the new builtin. `alias -t --` will no longer work,
that hack is now gone.
While I was testing this builtin, I found a bug with hash tables
in non-forking subshells. If the hash table of a non-forking
subshell is changed, the parent shell's hash table is also changed.
As an example, running `(hash -r)` was resetting the parent shell's
hash table. The workaround is to force the subshell to fork if the
hash table will be changed.
src/cmd/ksh93/bltins/typeset.c:
- Move the code for hash out of the alias builtin into a dedicated
hash builtin. `alias -t --` is no longer supported.
src/cmd/ksh93/data/aliases.c:
- Remove the old alias for hash from the table of predefined aliases.
src/cmd/ksh93/data/builtins.c:
- Fix the broken entry for the hash builtin and add a man page for
the new builtin.
src/cmd/ksh93/sh.1:
- Replace the entry for the hash alias with a more detailed entry
for the hash builtin.
src/cmd/ksh93/sh/name.c:
- Force non-forking subshells to fork if the PATH is being reset
to workaround a bug with the hash tree.
src/cmd/ksh93/tests/alias.sh:
- Add a regression test for resetting a hash table, then adding
a utility to the refreshed hash table.
src/cmd/ksh93/tests/subshell.sh:
- Add regression tests for changing the hash table in subshells.
(cherry picked from commit d8428a833afe9270b61745ba3d6df355fe1d5499)
Since we now have a shiny new POSIX compliant 'times' builtin,
we might as well use it.
src/cmd/ksh93/tests/shtests:
- Run 'times' at end of test run.
- Skip the pretty-printing until #7 is fixed.
(cherry picked from commit 2c27d9fbc239583004ec70377db98627eea5e294)
A column of whitespace in the NEWS file was removed for consistent
formatting. Most of the spelling errors were found with this
codespell dictionary:
https://github.com/orbitcowboy/codespell_dictionary
(cherry picked from commit 0e36b17abe5609c461a3e4da7041eb0fdf9991b7)
An oversight...
src/cmd/ksh93/sh.1:
- Undocument the fixed ${.sh.subshell} breakage.
(cherry picked from commit 42be8a3c6dd5a76d03e0456d7f53e38536757aa5)
The bug was really that I/O errors in output builtins were
undetectable by any means. Having a >0 exit status is sufficient.
Adding an error message risks making existing ksh scripts noisier,
or even breaking them if they redirect stderr to stdout.
Note to self: in future, implement the minimum change necessary to
fix bugs, nothing more. The fact that I needed to add four extra
2>/dev/null to the regression tests should have been a hint.
src/cmd/ksh93/bltins/print.c,
src/cmd/ksh93/data/msg.c,
src/cmd/ksh93/include/io.h:
- Remove "I/O error" message.
src/cmd/ksh93/tests/builtins.sh:
- Update to check for exit status only.
src/cmd/ksh93/tests/basic.sh,
src/cmd/ksh93/tests/coprocess.sh:
- Revert four new '2>/dev/null' to suppress the error message.
(cherry picked from commit 5e17be24d18455b575b6e98bc631c6935ffc795a)
src/cmd/ksh93/bltins/typeset.c: unall():
- There's no need to keep track of whether we're unsetting an alias
using a new 'isalias' vaariable, as we can just repeat the check
for 'troot==shp->alias_tree'.
(cherry picked from commit 68f3ae3acd0d19dd80805049a1c9afc1ae1e75c3)
This commit fixes a bug that caused unalias to return a zero status
when it tries to remove an alias twice. The following set of commands
will no longer end with an error:
$ alias foo=bar
$ unalias foo
$ unalias foo && echo 'Error'
This commit is based on the fix present in ksh2020, but it has been
extended with another bugfix. The initial fix for this problem tried to
remove aliases from the alias tree without accounting for NV_NOFREE. This
caused any attempt to remove a predefined aliases (e.g. `unalias float`)
to trigger an error with free, as all predefined aliases are in read-only
memory. The fix for this problem is to set NV_NOFREE when removing aliases
from the alias tree, but only if the alias is in read-only memory. All
other aliases must be freed from memory to prevent memory leaks.
I'll also note that I am using an `isalias` variable rather than the `type`
enum from ksh2020, as the `VARIABLE` value is never used and was replaced
with a bool called `aliases` in the ksh2020 release. The `isalias` variable
is an int as the ksh93u+ codebase does not use C99 bools.
Previous discussion: https://github.com/att/ast/issues/909
- src/cmd/ksh93/bltins/typeset.c:
Remove aliases from the alias tree by using nv_delete. NV_NOFREE
is only used when it is necessary.
- src/cmd/ksh93/tests/alias.sh:
Add two regression tests for the bugs fixed by this commit.
(cherry picked from commit 16d5ea9b52ba51f9d1bca115ce8f4f18e97abbc4)
Setting the exit status allows build scripts and the like
to actually detect regression test failures.
src/cmd/ksh93/tests/shtests:
- Add counter for total number of errors.
- Report total to stdout at the end.
- Set the exit status to the total number of errors, up to 125
(higher statuses have special meanings).
(cherry picked from commit c0dd80b14d4d316b4e9eff4b1b67d4e47f23a6ba)
src/cmd/ksh93/tests/variables.sh:
- Tolerate a bit more time for the SECONDS verification test.
src/cmd/ksh93/tests/subshell.sh:
- Replace unportable 'head -c 1' by 'dd bs=1 count=1'
- Remove unnecessary uses of 'whence'.
src/cmd/ksh93/tests/builtins.sh:
- Add a regression test for a weirdly specific 'whence' bug exposed
by the aforementioned unneccessary uses of 'whence', which only
shows up on my old Power Mac G5 running Mac OS X 10.3. For all I
know it's a compiler bug, but let's add a more clear failure for
it here, in case that happens anywhere else.
(cherry picked from commit c3898bd1e6e40874845771d33a5b37220ef0b06e)
This reduces a bunch more unnecessarily long sleeps to give
asynchronous processes time to run, etc. (No, we don't need to be
compatible anymore with your cool 1985 Intel 80386DX 16 MHz
battlestation...) Running the test suite is almost tolerable now,
making me much more likely to actually run the regression test
suite and catch my own regressions.
In addition, there are various fixes to make the test suite
compatible with 'set -x' ('set -o xtrace') so that you can now
actually *use* the documented 'bin/shtests -x' option. Recommend
combining with '-p' to avoid tracing everything three times.
I've also added a really useful $PS4 trace prompt to shtests that
traces pretty much everything there is to trace. (It does use
expansions that modify ${.sh.match}, which affected several tests,
particularly in tests/substring.sh; for those we need to set a
temporary simpler $PS4.)
(cherry picked from commit c3a5d47cfe880b526cabb5370ddaced0e8626acd)
This regression was caused by the commit that introduced
the times builtin. The ordering of the #include directives
was causing the build to fail on Ubuntu 20.04 and Arch Linux.
src/cmd/ksh93/bltins/misc.c:
- Move the includes for standard headers to the bottom
of the includes to fix build failures on Linux.
(cherry picked from commit 3a42b206a0cc71bedc4773ce545435570e862e61)
This fixes two regression test failures in tests/functions.sh.
src/cmd/ksh93/bltins/cflow.c: b_exit():
- The exit status should of course only be cropped to 8 bits if
b_exit() will actually exit, not if it returns from a function.
(cherry picked from commit 2d1e7f87551159c942b16de2a98dc72697988d26)
Emacs editing mode is bugged in ksh93u+ and ksh2020. Let's
say you were to run the following commands after starting
a fresh instance of ksh:
$ alias foo='true'
$ unalias foo
If you type 'a' and then press the up arrow on your keyboard,
ksh will complete 'a' to `alias foo='true'` by doing a reverse
search for the last command that starts with 'a'.
Run the alias command again, then type 'u' and press the up
arrow key again. If ksh is in Vi mode, you will get `unalias foo`,
but in Emacs mode you will get `alias foo='true'` again.
This bug was occurring because ksh was only doing a reverse search
based on the first command that was completed using the up arrow.
All subsequent commands were ignored as ksh was saving the first
command and only based later searches off of it.
NEWS:
- Add instructions for reproducing this bug with the up arrow
key and information about why this bug was happening in the
first place.
src/cmd/ksh93/edit/emacs.c:
- Remove a bad check that was preventing ksh from using the
latest input for reverse search.
(cherry picked from commit 745b065487ad6bac693ec6f44752f96e87f9a63b)
The signal.sh regression tests took over a minute and a half to
complete, so with the default three runs (for different locales)
they took about five minutes. Annoying and unnecessary. As ksh
supports floating point shell arithmetic plus a 'sleep' builtin
that supports fractional seconds, this is easy to fix.
src/cmd/ksh93/tests/signal.sh:
- Slash all the 'sleep' arguments and associated factors by a
factor of 10. Since computers are now hundreds or thousands of
times faster than they were in the early 1990s, this should be
plenty safe enough and all the tests should still be valid.
- Change one unportable direct invocation of /bin/sleep to use the
builtin.
(cherry picked from commit 734e5953650e9a62048c45eb9a3b827bb9efcfb5)
This fixes two bugs: issuing the 'exit' command with a value > 256
would cause ksh 93u+ to kill itself with the corresponding signal
(try 'exit 265' to SIGKILL your interactive shell), and, if the
last command of a script exits due to a signal, the shell would
repeat that signal to itself, causing any parent ksh to also be
killed.
Discussion:
https://bugzilla.redhat.com/show_bug.cgi?id=1469624https://rainbow.chard.org/2017/03/21/ksh-deliberately-segfaults-if-the-last-command-in-a-script-crashes/
This commit is loosely based on a patch applied to the 93v- beta
and the abandoned ksh2020, but that patch was incomplete & broken:
$ ksh-2020.0.0 -c 'exit 265'; echo $?
137
Expected: 9. Since the exit was *not* due to a signal, the value
should simply be cropped to the 8 bits supported by the OS.
src/cmd/ksh93/bltins/cflow.c: b_exit():
- For the 'exit' builtin command, bitwise-AND the argument to
'exit' with SH_EXITMASK (8 bits, crop to 0-255) before passing it
on to sh_exit(). This restores the behaviour of <=2011 ksh93
versions and is in line with all other POSIX shells.
It also fixes this bogosity:
$ (exit 265); echo $? # non-forked subshell
265
$ (ulimit -t unlimited; exit 265); echo $? # forked subshell
9
Forked or non-forked should make no difference at all
(see commit message a0e0e29e for why).
src/cmd/ksh93/sh/fault.c: sh_done():
- If the current exit status is equal to the status for the last
signal that was received from a child process, remove the
SH_EXITSIG (9th) bit, so that the shell doesn't kill itself.
- If the shell's last child process exits due to a signal, exit
with a portable 8-bit exit status (128 + signal number). This
avoids the exit status being < 128 by being cropped to 8 bits.
src/cmd/ksh93/tests/signal.sh:
- Add regression test for exit with status > 256.
- Add regression test verifying the shell no longer kills itself.
(cherry picked from commit 98e0fc94393e175ce6adfee390327c320795bf12)
This commit gets rid of dead weight related to an obscure early
1990s Bell Labs versioning file system research project called
3DFS, which has not existed for decades and for which I have not
managed to find any evidence that it was ever used outside the lab.
This removes:
- the SHOPT_FS_3D compile option (which was forced on even when 0)
- the obnoxious default alias 2d='set -f;_2d' that turned off your
globbing and then tried to run a nonexistent _2d command
- undocumented builtins 'vmap' and 'vpath' that only errored out
- a non-functional -V unary operator for the test and [[ commands
- some specific code for Apollo workstations (last made in 1997),
which was inseparably intertwined with the 3DFS code
(cherry picked from commit 20cdf3709f4fb4e468057b534dcee819b1961fb6)
POSIX requires[*] that expanding any unset parameter other than $@
and $* is an error when 'set -u'/'set -o nounset' is active.
However, on ksh93, $! was exempt as well. That is a bug.
[*] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_25
src/cmd/ksh93/sh/macro.c:
- special(): Handle 'set -u' for special parameters if/when it is
about to return NIL. That code path is currently only possible to
reach for "$!", but this is future-proof and will do the right
thing if any other special parameter can ever have no value.
src/cmd/ksh93/tests/options.sh:
- Add and tweak 'set -u' regression tests.
(cherry picked from commit 75cc7a38cafe3a9929e1ed17d8b952babda22a09)
POSIX requires[*] that expanding any unset parameter other than $@
and $* is an error when 'set -u'/'set -o nounset' is active.
However, on ksh93, $1, $2, ... were exempt as well. That is a bug.
[*] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_25
src/cmd/ksh93/sh/macro.c:
- varsub(): Backport code for handling 'set -u' for positional
parameters from the ast 2016-10-01-beta branch.
src/cmd/ksh93/tests/options.sh:
- Add relevant regression tests.
src/cmd/ksh93/sh.1:
- Document that $@ and $* are exempt from 'set -u'.
(cherry picked from commit f954c6be0748c4c38a680a75f27564965fbd328e)
src/cmd/ksh93/tests/options.sh:
- Pipe hang bugfix test: The child shell has no idea how long to
sleep for if we don't export the variable telling it so.
(cherry picked from commit ede0960c4ec84f0f934b17072a892a4e40798e97)
According to Red Hat, this fixes "a bug on input buffer boundary
and/or temporary composing buffer of multibyte characters".
The patch was credited to Paulo Andrade <pandrade@redhat.com>.
To be honest, I don't know how to trigger this bug or what the code
removed by this fix really does, but this patch is in production
use at Red Hat, removes some smelly stuff, and is not triggering
any regression test failures, so I'll just take this one on faith.
https://bugzilla.redhat.com/show_bug.cgi?id=1417886https://github.com/att/ast/commit/4fa2020b
src/cmd/ksh93/sh/fcin.c:
- _fcmbget(): Remove some dodgy-looking buffer-fiddling code that
is marked as "for testing purposes with small buffers".
(cherry picked from commit 407760fdbddcb7f8ac92b5d1da29d3e09dac0369)
