1
0
Fork 0
mirror of git://git.code.sf.net/p/cdesktopenv/code synced 2025-02-13 11:42:21 +00:00
Commit graph

141 commits

Author SHA1 Message Date
Johnothan King
3d38270b32 Remove a buggy optimization for variables in subshells
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.
2020-06-15 07:13:38 -07:00
Martijn Dekker
ef1621c18f Make 'source' a regular built-in
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.
2020-06-15 11:33:44 +02:00
Martijn Dekker
b7f48e8a10 Revert job locking patch (re: 07cc71b8, 58560db7)
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#c14
      https://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.
2020-06-15 03:20:45 +02:00
Martijn Dekker
58560db768 Restore build without __sync_fetch_and_{add,sub} (re: 07cc71b8)
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().
2020-06-14 23:49:07 +02:00
Martijn Dekker
f95d3105ef
Merge pull request #14 from aweeraman/debian-patches-2
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.
2020-06-14 21:46:32 +01:00
Anuradha Weeraman
07cc71b880 ksh segfaults in job_chksave after receiving SIGCHLD
Upstreaming Debian patch:

Prior to this update, 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.
This update implements a fix to ensure the compiler does not drop parts of the
ksh mechanism and the crash no longer occurs.

Author: Michal Hlavinka mhlavink@redhat.com
Origin: Red Hat fix, ksh-20120801-locking.patch
Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1123467
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1697501
Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=867181
2020-06-14 16:19:33 -04:00
Martijn Dekker
ae25b7f886 move read -S regress test to readcsv.sh (re: af0bd6ad) 2020-06-14 20:10:22 +02:00
Martijn Dekker
242d3f768b
Merge pull request #12 from JohnoKing/fix-read-s
`read -S` now correctly handles nested double quotes
2020-06-14 18:50:58 +01:00
Johnothan King
af0bd6ad70 read -S now correctly handles nested double quotes
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.
2020-06-14 10:40:30 -07:00
Martijn Dekker
5498d9ec25
Merge pull request #11 from JohnoKing/leap-seconds
Fix two more problems with time zones and epoch handling
2020-06-14 16:54:23 +01:00
Martijn Dekker
ff01ecdaba
Merge pull request #10 from aweeraman/debian-patches-1
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
2020-06-14 16:43:37 +01:00
Martijn Dekker
a81a64060c sh.1: forgot update after d8eba9d1 2020-06-14 16:49:41 +02:00
Anuradha Weeraman
49cac9e604 Fixes for implicit declaration warnings
Signed-off-by: Anuradha Weeraman <aweeraman@gmail.com>
2020-06-14 09:55:08 -04:00
Johnothan King
51f97cf82f Fix two more problems with time zones and epoch handling
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).
2020-06-14 06:53:46 -07:00
Martijn Dekker
6109f54a5e Fix for no-argument use of 'cd' in .../fun/dirs
'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.
2020-06-14 10:39:48 +02:00
Martijn Dekker
d66f2a80d8 sh.1: default PS1 is "$ ", not "$" 2020-06-14 08:42:07 +02:00
Martijn Dekker
1a6f37dc86 NEWS: update repo URI 2020-06-14 06:28:38 +02:00
Martijn Dekker
76f7d15a93
Merge pull request #8 from JohnoKing/fix-freebsd-date
Backport the ksh2020 fix for timezone name determination

Partial fix for #6.
2020-06-14 00:07:51 +01:00
Martijn Dekker
a739115ffe
Merge pull request #7 from JohnoKing/fix-special-vars
Implement a better fix for unsetting special env vars.

See #7 for more explanations and ksh source code history :)
2020-06-14 00:05:40 +01:00
Johnothan King
d7c9470704 Backport the ksh2020 fix for timezone name determination
This fix for `printf '%T' now` on FreeBSD was written by
@krader1961. This is from https://github.com/att/ast/pull/591:

On FreeBSD calling tzset() does not guarantee the tzname array will
be correctly populated. On most systems that works but on FreeBSD you
have to call localtime() or a related function (e.g., ctime()).

This change also eliminates a potential, very small, memory leak due to
the strdup()'ed tznames not being freed.

src/lib/libast/tm/tminit.c:
 - Fix timezone name determination on FreeBSD and a memory leak.
2020-06-13 14:28:10 -07:00
Johnothan King
7b994b6a7e Implement a better fix for unsetting special env vars
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.
2020-06-13 12:55:48 -07:00
Martijn Dekker
289f56cd4c tests/pty.sh: fix regress fail due to $TMPDIR
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.
2020-06-13 01:48:13 +02:00
Martijn Dekker
881a5be07c Remove obsolete docs, keeping official description
The lib/package/ast-open.* are almost entirely obsolete after
most of the AST utilities were removed to concentrate on ksh93
development. The only part that is both relevant and not repeated
elsewhere is AT&T's introductory blurb on ksh93.

README.md:
- Add the official AT&T ksh93 description from ast-open.html,
  reformatted to Markdown with a couple of added hyperlinks.

lib/package/ast-open.README,
lib/package/ast-open.html:
- Removed. (Note that the historic ksh93 changelog in these files
  is also included in src/cmd/ksh93/RELEASE).
2020-06-12 20:15:20 +02:00
Martijn Dekker
d600cf61fa use 'ksh93' org identifier for new/changed builtins
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.
2020-06-12 18:54:08 +02:00
Martijn Dekker
a5e2fa8db7
README.md: tweak policy 2020-06-12 14:42:23 +01:00
Martijn Dekker
5aea9bbe08
README.md: update NEWS link after transfer to ksh93 org 2020-06-12 13:52:51 +01:00
Martijn Dekker
e500479ede
Merge pull request #1 from JohnoKing/fix-builtin-delete
`builtin -d` should not delete special builtins
2020-06-12 12:36:42 +01:00
Johnothan King
017d088c39 builtin -d should not delete special builtins
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.
2020-06-12 04:26:40 -07:00
Martijn Dekker
cd5c181a39 Restore AST 'pty' pseudoterminal command (re: reboot)
This enables the tests/pty.sh regression tests for the interactive
shell to work. I've managed to restore these without restoring
nmake (which would cause unresolved build failures). It involved
hand-editing the Mamfile originally generated by nmake.

src/cmd/builtin/Makefile:
- Restored. Even though nmake is not installed, apparently mamake
  still needs the Makefile for the 'install' target to work. This
  will probably need editing if nmake is ever restored.

src/cmd/builtin/Mamfile:
- Restored and edited by hand to remove the commands to build all
  the utilities except pty, as well as all test targets (pty has no
  test). Most of this was systematically deleting blocks delimited
  by 'make FOO' and 'done FOO generated' until the build stopped
  failing -- a task made so much easier by the build now failing
  gracefully on error (see d0dfb37c). To see the diff, do:
	git diff cc1f2bf8 src/cmd/builtin/Mamfile

src/cmd/builtin/RELEASE:
- Restored for completeness' sake with all non-pty entries removed.

src/cmd/builtin/features/pty,
src/cmd/builtin/pty.c:
- Restored.
2020-06-12 12:35:58 +02:00
Martijn Dekker
802ea67afb tests/io.sh: don't abort entire suite on failure
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.
2020-06-12 11:34:10 +02:00
Martijn Dekker
d8eba9d112 Remove 'login' and 'newgrp' builtins: not sane default behaviour
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.
2020-06-12 06:57:57 +02:00
Martijn Dekker
7b82c338da Make 'redirect' a regular builtin instead of an alias of 'exec'
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.
2020-06-12 04:54:33 +02:00
Martijn Dekker
936802f92a Simplify exit status fix, restore interruptability
The exit status fix introduced in cherry-picked commit 22c3a6e1
introduced a problem: interrupting the build only interrupted the
main process, and the background job would happily continue.

bin/package, src/cmd/INIT/package.sh:
- Give up on the idea of a background job and put up with not
  cleaning up the FIFO. It's in a build directory that is going to
  get cleaned up anyway.
2020-06-12 02:11:46 +02:00
Martijn Dekker
7a8a46700c Build on more darwin/macOS versions
src/cmd/INIT/cc.darwin*:
- Build on ancient Mac OS X gcc versions (darwin 7+, Mac OS X
  10.3+), more recent gcc versions (darwin 11+, Mac OS X 10.7+), as
  well as current clang versions. So there are now three cc.darwin*
  wrapper scripts for these different darwin versions.

bin/package, src/cmd/INIT/package.sh:
- Differentiate the host ID and pick the correct Darwin script based on
  the detected OS release version.
2020-06-12 01:45:19 +02:00
Martijn Dekker
04b9171858 POSIX compliance fix: make 'unalias' a regular builtin
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)
2020-06-12 01:45:18 +02:00
Martijn Dekker
2da22f4023 data/builtins.c: minor cosmetic 'alias --man' fix (re: d8428a833)
(cherry picked from commit 98be5130d3a46a565d0ad601aac838531765fc43)
2020-06-12 01:45:18 +02:00
Martijn Dekker
80d9ae2b1c Refactor new b_hash(); better hash table clear (re: d8428a83)
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)
2020-06-12 01:45:18 +02:00
Johnothan King
74b4162178 Fix set +r so that it cannot unset the restricted option
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)
2020-06-12 01:45:18 +02:00
Martijn Dekker
9cef2d534a shtests: make tests more interruptable with Ctrl+C
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)
2020-06-12 01:45:18 +02:00
Johnothan King
102868f850 Replace the hash alias with a proper builtin
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)
2020-06-12 01:45:18 +02:00
Martijn Dekker
d1bd8f89b7 shtests: print CPU times used at end (re: ebf71e61)
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)
2020-06-12 01:45:18 +02:00
Johnothan King
e92faddbf9 Fix 39 spelling errors and a formatting issue
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)
2020-06-12 01:45:18 +02:00
Martijn Dekker
90fd867171 Update manual after a0e0e29e
An oversight...

src/cmd/ksh93/sh.1:
- Undocument the fixed ${.sh.subshell} breakage.

(cherry picked from commit 42be8a3c6dd5a76d03e0456d7f53e38536757aa5)
2020-06-12 01:45:18 +02:00
Martijn Dekker
30b111d51a TODO: moar update
(cherry picked from commit 3bce00165113ed22e22c99d6347cc8211bea4db1)
2020-06-12 01:45:18 +02:00
Martijn Dekker
12a079ea2b TODO: update
(cherry picked from commit 1e6bf87c2c435799034c9d87236939ab8bd61fb9)
2020-06-12 01:45:18 +02:00
Martijn Dekker
a1f46d785f rm "I/O error" error msg; just keep >0 exit status (re: 9011fa93)
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)
2020-06-12 01:45:18 +02:00
Martijn Dekker
b883c7a7f8 remove extra 'isalias' variable (re: 16d5ea9b)
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)
2020-06-12 01:45:18 +02:00
Johnothan King
5d50f825e4 The unalias builtin should return an error for non-existent aliases
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)
2020-06-12 01:45:18 +02:00
Martijn Dekker
d18469d608 bin/package: exit with nonzero status on build failure
Build scripts and the like ought to be able to check the results of
the build using the exit status of the build command.

bin/package,
src/cmd/INIT/package.sh:
- Add global error_status variable to keep track of the highest
  exit status of all commands using capture(). Exit with that.
- capture():
  * When capturing build output using 'tee', change the regular
    pipe construct (which doesn't preserve the exit status of pipe
    elements except the last one) to a couple of background jobs
    connected with a FIFO (named pipe). This allows getting the
    build's exit status using the "wait" command.
  * Update error_status on every invocation of capture().

(cherry picked from commit 22c3a6e1b32f3ab7eeca45adb76863982dc3e334)
2020-06-12 01:45:17 +02:00
Johnothan King
22d6b4527d Avoid brute-forcing during the build process to prevent failed builds
The default behavior of the package script is to ignore
errors. This has disastrous effects if any part of ksh
or libast fails to build due to something like a syntax
error, as the build will stop long after the first error
occurred.

This commit removes the -k flag from the $makeflags variable
in both copies of the package script. This forces the build
to fail if a compiler error occurs.

bin/package and src/cmd/INIT/package.sh:
 - Remove the -k assignment to $makeflags so that compiler
   errors cause the build to fail.

(cherry picked from commit 87bfaa19f3d336feb7b7bd4f5d4caf8d553f2547)
2020-06-12 01:45:17 +02:00