1
0
Fork 0
mirror of git://git.code.sf.net/p/cdesktopenv/code synced 2025-02-25 08:34:36 +00:00
Commit graph

53 commits

Author SHA1 Message Date
Anuradha Weeraman
035a4cb3f4
Fix segfault if $PATH contains a .paths directory (#55)
ksh crashed if it encountered a .paths directory in any of the
directories in $PATH.

Ref: https://bugs.launchpad.net/ubuntu/+source/ksh/+bug/1534855

src/cmd/ksh93/sh/path.c: path_chkpaths():
- Refuse to read .paths if it's not a regular file
  or a symlink to a regular file.
2020-07-02 23:29:07 +01:00
Johnothan King
db1d539d49
Fix ERE repetition expressions in [[ ... =~ ERE{x,y} ]] (#54)
Regular expressions that combine a repetition expression with
a parenthesized sub-expression throw a garbled syntax error:

$ [[ AATAAT =~ (AAT){2} ]]
ksh: syntax error: `~(E)(AAT){2} ]]
:'%Cred%h%Creseksh: syntax error: `~(E)(AAT){2} ]]
:'%Cred%h%Creseksh: syntax' unexpected

The syntax error occurs because ksh is not fully
accounting for '=~' when it runs into a curly bracket.
This fix disables the syntax error when the operator
is '=~' and adds handling for '(str){x}' (to allow for
more than one sub-expression). This bugfix and the
regression tests for it were backported from ksh93v-
2014-12-24-beta.

src/cmd/ksh93/sh/lex.c:
- Do not trigger a syntax error for '{x}' when the operator
  is '=~' and add handling for multiple parentheses when
  combined with '{x}'.

src/cmd/ksh93/tests/bracket.sh:
- Add two tests from ksh93v- to test sub-expressions
  combined with the '{x}' quantifier.
2020-07-02 18:40:15 +01:00
Johnothan King
10b6ba801d
Fix memory corruption when a compound variable is unset (#49)
The following set of commands ends with a memory fault under
certain circumstances because ksh attempts to free memory
twice, causing memory corruption:

$ testarray=(1 2)
$ compound testarray
$ unset testarray
$ eval testarray=

The fix is to make sure 'np->nvfun' is a valid pointer before
attempting to free memory in 'put_tree'. This patch is from
OpenSUSE: https://build.opensuse.org/package/view_file/shells/ksh/ksh93-nvtree-free.dif?expand=1

src/cmd/ksh93/sh/nvtree.c:
- Do not try to free memory when 'np->nvfun' and 'val'
  are false.

src/cmd/ksh93/tests/comvar.sh:
- Add a regression test for the double free problem. The
  reproducer must be run from an executable script
  with 'ksh -c'.
2020-06-29 18:08:28 +01:00
Johnothan King
5135cf651c
Fix crashes caused by 'typeset -RF' (#47)
Variables created with 'typeset -RF' were being treated as
short integers, even though they are actually floating point
values. As a result the following example will cause a crash:

$ typeset -RF foo=1
$ test "$foo"

This is fixed by checking for 'NV_DOUBLE' with 'nv_isattr',
which prevents ksh from treating floating point values as
short integers due to '== NV_INT16P' excluding 'NV_DOUBLE'.
This bugfix was backported from ksh93v- 2013-10-10-alpha.

src/cmd/ksh93/sh/array.c,
src/cmd/ksh93/sh/name.c,
src/cmd/ksh93/sh/nvdisc:
- Avoid treating floating point values as short integers by
  checking for 'NV_DOUBLE' with 'nv_isattr'.

src/cmd/ksh93/tests/types.sh:
- Add a regression test for the 'typeset -RF' crash. The
  crash cannot be replicated if 'typeset -RF' sets 'foo'
  to zero.
2020-06-28 23:30:27 +01:00
Johnothan King
0aa9e03f55
Fix process substitution combined with redirection (#40)
The code for handling process substitution with redirection was
never being run because IORAW is usually set when IOPROCSUB is
set. This commit fixes the problem by moving the required code
out of the !IORAW if statement. The following command now prints
'good' instead of writing 'ok' to a bizzare file:

$ ksh -c 'echo ok > >(sed s/ok/good/); wait'
good

This commit also fixes a bug that caused the process ID of the
asynchronous process to print when the shell was in interactive
mode. The following command no longer prints a process ID,
behaving like in Bash and zsh:

$ echo >(true)
/dev/fd/5

src/cmd/ksh93/sh/args.c:
 - Temporarily turn off the interactive state while in a process
   substitution to prevent the shell from printing the PID of
   the asynchronous process.

src/cmd/ksh93/sh/io.c:
 - Move the code for process substitution with redirection into
   a separate if statement.

src/cmd/ksh93/tests/io.sh:
 - Add two tests for both process substitution bugs fixed by this
   commit.

src/cmd/ksh93/tests/shtests:
 - Update shtests with a patch from Martijn Dekker to use
   pretty-printing for the output from the times builtin (if it
   is available).

Fixes #2
2020-06-23 23:02:16 +01:00
Johnothan King
c1994b87f1
Fix nested functions ignoring prefixed variable assignments (#37)
This commit fixes the bug described in att/ast#32. The fix and
following explanation is from att/ast#467:

While copying variables from function's local scope to a new scope,
variable attributes were not copied. Such variables were not marked
to be exported in the new function. For e.g.

function f2 { env | grep -i "^foo"; }
function f1 { env | grep -i "^foo"; f2; }
foo=bar f1

prints 'foo=bar' only once, but it should print be twice.

src/cmd/ksh93/sh/xec.c:
 - When variables from the local scope of a function are copied into
   the scope of a nested function, the attributes of the variables
   need to be copied as well.

src/cmd/ksh93/tests/functions.sh:
 - Add regression tests from ksh2020 to check environment variables
   passed to functions.
2020-06-23 00:27:05 +01:00
Johnothan King
ff358f3464 Fix a crash when 'kill %%' and 'kill %+' are run (#35)
Ksh was trying to use the 'pw' variable as a valid pointer even
when it was NULL. This is fixed by doing the error check for
'pw' before doing anything else in 'job_kill'.

This bugfix is from Red Hat:
44e0a643a9/f/SOURCES/ksh-20130214-fixkill.patch

Fixes #34
2020-06-22 19:11:49 +02:00
Martijn Dekker
9d428f8f5e Fix erroneous fork after 'readonly PATH' in subshell (re: 102868f8)
After making PATH readonly in a virtual subshell (without otherwise
changing it, so the subshell is never forked), then the main shell
would erroneously fork into a background process immediately after
leaving the virtual subshell. This was caused by a bug in the
forking workaround that prevents changes in PATH in a virtual
subshell from clearing the parent shell's hash table.

src/cmd/ksh93/sh/name.c: nv_putval():
- If we're either setting or restoring PATH, do an additional check
  for the NV_RDONLY flag, which means the function was told to
  ignore the variable's readonly state. It is told to ignore that
  when restoring the parent shell state after exiting a virtual
  subshell. If we don't fork then, we don't fork the parent shell.

src/cmd/ksh93/tests/subshell.sh:
- Add regression test verifying that no forking happens when making
  PATH readonly in a subshell.

Fixes #30.
2020-06-20 23:47:42 +02:00
Johnothan King
bd3e2a8001
Fix unreliable behavior when special vars are readonly or unset (#27)
src/cmd/ksh93/data/variables.c:
 - Running 'unset .sh.lineno' creates a memory fault, so fix that
   by giving it the NV_NOFREE attribute. This crash was happening
   because ${.sh.lineno} is an integer that cannot be freed from
   memory with free(3).

src/cmd/ksh93/sh/init.c:
 - Tell _nv_unset to ignore NV_RDONLY when $RANDOM and $LINENO are
   restored from the subshell scope. This is required to fully
   restore the original state of these variables after a virtual
   subshell finishes.

src/cmd/ksh93/bltins/typeset.c,
src/cmd/ksh93/sh/subshell.c:
 - Disabled some optimizations for two instances of 'sh_assignok' to
   fix 'readonly' in virtual subshells and '(unset .sh.level)' in
   nested functions. This fixes the following variables when
   '(readonly $varname); enum varname=' is run:

   $_
   ${.sh.name}
   ${.sh.subscript}
   ${.sh.level}

   The optimization in question prevents sh_assignok from saving the
   original state of these variables by making the sh_assignok call
   a no-op. Ksh needs the original state of a variable for it to be
   properly restored after a virtual subshell has run, otherwise ksh
   will simply carry over any new flags (being NV_RDONLY in this case)
   from the subshell into the main shell.

src/cmd/ksh93/tests/variables.sh:
 - Add regression tests from Martijn Dekker for setting special
   variables as readonly in virtual subshells and for unsetting
   special variables in general.

Fixes #4
2020-06-20 18:08:41 +01:00
Johnothan King
99065353b3 Fix 'whence -a' to print correct path for tracked alias (#25)
'whence -a' bases the path for tracked aliases on the user's
current working directory if an enabled ksh builtin of the same
name is also available. The following example will claim 'cat'
is in the user's current working directory:

$ whence -a cat
cat is a tracked alias for /usr/bin/cat
$ builtin cat
$ whence -a cat
cat is a shell builtin
cat is /usr/bin/cat
cat is a tracked alias for /current/working/directory/cat

This patch from ksh2020 fixes this problem by properly saving the
path of the tracked alias for use with 'whence -a', since
'path_pwd' (as implied by the function's name) only gets the users
current working directory, not the location of tracked aliases.
Ref.: https://github.com/att/ast/issues/1049

This bug was originally reported by David Morano about two decades
ago to the AST team: https://github.com/att/ast/issues/954

src/cmd/ksh93/bltins/whence.c:
 - Print the actual path of a tracked alias, path_pwd doesn't
   have this functionality.

src/cmd/ksh93/include/name.h:
 - Add 'pathcomp' for saving the value of tracked aliases.

src/cmd/ksh93/sh/path.c:
 - Save the value of tracked aliases for use by whence.

src/cmd/ksh93/tests/builtins.sh:
 - Add a regression test for using 'whence -a' on tracked
   aliases with a builtin equivalent.
2020-06-19 14:03:58 +02:00
Martijn Dekker
3e3f6b0f12 Restore #22 'unset -f' fix minus segfault (re: b7932e87, 97511748)
Applying the fix for 'unset -f' exposed a crashing bug in lookup()
in sh/nvdisc.c, which is the function for looking up discipline
functions. This is what caused tests/variables.sh to crash.
Ref.: https://github.com/ksh93/ksh/issues/23#issuecomment-645699614

src/cmd/ksh93/sh/nvdisc.c: lookup():
- To avoid segfault, check that the function pointer nq->nvalue.rp
  is actually set before checking if nq->nvalue.rp->running==1.

src/cmd/ksh93/sh/xec.c,
src/cmd/ksh93/tests/functions.sh:
- Uncomment the 'unset -f' fix from b7932e87.

Resolves #21 (again).
2020-06-18 02:48:51 +02:00
Martijn Dekker
975117485c Part revert #22 to undo memory fault (re: b7932e87)
The fix in sh/xec.c, which was backported from the ksh 93v- beta to
delay the actual removal of a running function that unsets itself,
caused a segfault in the variables.sh regression tests (see #23).

src/cmd/ksh93/sh/xec.c:
- Comment out the backported code pending a correct fix for #21.
  Now both types of functions silently fail to unset themselves
  (unless they're discipline functions).

src/cmd/ksh93/tests/functions.sh:
- Disable regression tests checking that the function was actually
  unset, pending a correct fix for #21.

Resolves: #23
Reopens: #21
2020-06-17 21:01:55 +02:00
Johnothan King
b7932e87b6
Fix two problems with 'unset -f' behavior (#22)
src/cmd/ksh93/sh/name.c:
 - Correct the check for when a function is currently running
   to fix a segmentation fault that occurred when a POSIX
   function tries to unset itself while it is running.
   This bug fix was backported from ksh93v-.

src/cmd/ksh93/sh/xec.c:
 - If a function tries to unset itself, unset the function
   with '_nv_unset(np, NV_RDONLY)' to fix a silent failure.
   This fix was also backported from ksh93v-.

src/cmd/ksh93/tests/functions.sh:
 - Add four regression tests for when a function unsets itself.

Resolves #21
2020-06-17 18:26:43 +01:00
Johnothan King
fae8862c53
Fix assignments preceding 'command <special builtin>' (#19)
Ksh was not checking for `command` when running a special builtin,
which caused preceding invocation-local variable assignments to
become global. This is the reproducer from the att/ast#72:

$ foo=BUG command eval ':'
$ echo "$foo"

This no longer prints 'BUG', as ksh now makes sure the command builtin
is not running a special builtin before making invocation-local
variable assignments global.

src/cmd/ksh93/sh/xec.c:
 - Backport the bugfix for BUG_CMDSPASGN from ksh93v- 2013-10-10-alpha.

src/cmd/ksh93/tests/builtins.sh:
 - Add a regression test based on the reproducer in att/ast#72.
2020-06-16 22:58:05 +01:00
Johnothan King
c258a04f7a
Implement a portable fix for SIGCHLD crashes (#18)
As previously reported in rhbz#1112306 (https://bugzilla.redhat.com/show_bug.cgi?id=1112306),
ksh may crash when receiving SIGCHLD because GCC's optimizer will fail to generate
`addl` and `sub` instructions to increment and decrement `job.in_critical` in the
`job_subsave` function. This bug *does* occur in GCC 10 with `-O2`, but not `-O1`;
it doesn't appear this bug has been fixed. As a reference, here is the relevant
debug assembly output of `job_subsave` when KSH is compiled with `CCFLAGS` set to
`-g -O1`:

0000000000034c97 <job_subsave>:

void *job_subsave(void)
{
   34c97:       53                      push   %rbx
        struct back_save *bp = new_of(struct back_save,0);
   34c98:       bf 18 00 00 00          mov    $0x18,%edi
   34c9d:       e8 34 4a 0a 00          callq  d96d6 <_ast_malloc>
   34ca2:       48 89 c3                mov    %rax,%rbx
        job_lock();
   34ca5:       83 05 3c 50 13 00 01    addl   $0x1,0x13503c(%rip)        # 169ce8 <job+0x28>
        *bp = bck;
   34cac:       66 0f 6f 05 4c 5a 13    movdqa 0x135a4c(%rip),%xmm0        # 16a700 <bck>
   34cb3:       00
   34cb4:       0f 11 00                movups %xmm0,(%rax)
   34cb7:       48 8b 05 52 5a 13 00    mov    0x135a52(%rip),%rax        # 16a710 <bck+0x10>
   34cbe:       48 89 43 10             mov    %rax,0x10(%rbx)
        bp->prev = bck.prev;
   34cc2:       48 8b 05 47 5a 13 00    mov    0x135a47(%rip),%rax        # 16a710 <bck+0x10>
   34cc9:       48 89 43 10             mov    %rax,0x10(%rbx)
        bck.count = 0;
   34ccd:       c7 05 29 5a 13 00 00    movl   $0x0,0x135a29(%rip)        # 16a700 <bck>
   34cd4:       00 00 00
        bck.list = 0;
   34cd7:       48 c7 05 26 5a 13 00    movq   $0x0,0x135a26(%rip)        # 16a708 <bck+0x8>
   34cde:       00 00 00 00
        bck.prev = bp;
   34ce2:       48 89 1d 27 5a 13 00    mov    %rbx,0x135a27(%rip)        # 16a710 <bck+0x10>
        job_unlock();
   34ce9:       8b 05 f9 4f 13 00       mov    0x134ff9(%rip),%eax        # 169ce8 <job+0x28>
   34cef:       83 e8 01                sub    $0x1,%eax
   34cf2:       89 05 f0 4f 13 00       mov    %eax,0x134ff0(%rip)        # 169ce8 <job+0x28>
   34cf8:       75 2b                   jne    34d25 <job_subsave+0x8e>
   34cfa:       8b 3d ec 4f 13 00       mov    0x134fec(%rip),%edi        # 169cec <job+0x2c>
   34d00:       85 ff                   test   %edi,%edi
   34d02:       74 21                   je     34d25 <job_subsave+0x8e>
   34d04:       c7 05 da 4f 13 00 01    movl   $0x1,0x134fda(%rip)        # 169ce8 <job+0x28>

When `-O2` is used instead of `-O1`, the `addl` and `sub` instructions for
incrementing and decrementing the lock are removed. GCC instead generates a
broken `mov` instruction for `job_lock` and removes the initial `sub` instruction
in job_unlock (this is also seen in Red Hat's bug report):

       job_lock();
       *bp = bck;
  37d7c:       66 0f 6f 05 7c 79 14    movdqa 0x14797c(%rip),%xmm0        # 17f700 <bck>
  37d83:       00
       struct back_save *bp = new_of(struct back_save,0);
  37d84:       49 89 c4                mov    %rax,%r12
       job_lock();
  37d87:       8b 05 5b 6f 14 00       mov    0x146f5b(%rip),%eax        # 17ece8 <job+0x28>
...
        job_unlock();
  37dc6:       89 05 1c 6f 14 00       mov    %eax,0x146f1c(%rip)        # 17ece8 <job+0x28>
  37dcc:       85 c0                   test   %eax,%eax
  37dce:       75 2b                   jne    37dfb <job_subsave+0x8b>

The original patch works around this bug by using the legacy `__sync_fetch_and_add/sub`
GCC builtins. This forces GCC to generate instructions that change the lock with
`lock addl`, `lock xadd` and `lock subl`:

       job_lock();
  37d9f:       f0 83 05 41 6f 14 00    lock addl $0x1,0x146f41(%rip)        # 17ece8 <job+0x28>
  37da6:       01
...
       job_unlock();
  37deb:       f0 0f c1 05 f5 6e 14    lock xadd %eax,0x146ef5(%rip)        # 17ece8 <job+0x28>
  37df2:       00
  37df3:       83 f8 01                cmp    $0x1,%eax
  37df6:       74 08                   je     37e00 <job_subsave+0x70>
...
  37e25:       74 11                   je     37e38 <job_subsave+0xa8>
  37e27:       f0 83 2d b9 6e 14 00    lock subl $0x1,0x146eb9(%rip)        # 17ece8 <job+0x28>

While this does work, it isn't portable. This patch implements a different
workaround for this compiler bug. If `job_lock` is put at the beginning of
`job_subsave`, GCC will generate the required `addl` and `sub` instructions:

       job_lock();
  37d67:       83 05 7a 5f 14 00 01    addl   $0x1,0x145f7a(%rip)        # 17dce8 <job+0x28>
...
        job_unlock();
  37dbb:       83 e8 01                sub    $0x1,%eax
  37dbe:       89 05 24 5f 14 00       mov    %eax,0x145f24(%rip)        # 17dce8 <job+0x28>

It is odd that moving a single line of code fixes this problem, although
GCC _should_ have generated these instructions from the original code anyway.
I'll note that this isn't the only way to get these instructions to generate.
The problem also seems to go away when inserting almost anything else inside
of the code for `job_subsave`. This is just a simple workaround for a strange
compiler bug.
2020-06-16 22:44:02 +01:00
Martijn Dekker
ad349c7668 silence macro redefinition warnings (re: 7003aba4)
src/cmd/ksh93/bltins/test.c,
src/cmd/ksh93/sh/arith.c,
src/cmd/ksh93/sh/streval.c:
- #undef ERROR_exit before redefining it, so clang stops nagging.
2020-06-16 04:51:21 +02:00
Martijn Dekker
a9de50bf79 Apply simple optimisation for ${ subshare; } (re: 3d38270b)
Running shbench after undoing the incorrect subshell optimisation
showed that the performance of ${ subshare; }-type command
substitutions went down very slightly, but consistently.

The main purpose of using this ksh-specific type of command
substitution vs. a normal one is performance. Thus, it *is*
appropriate to eke every last bit of performance out of it that
we can, provided correctness is completely preserved.

It is also a type of command substitution where every change is
supposed to be shared with the main shell environment; only command
output is captured in a subshell-like fashion.

Thus, on the face of it, it would be a logical optimisation for
sh_assignok() to avoid bothering with saving a subshell context for
variables if we're in a subshare.

Lo and behold, applying it does not introduce any regress fails.

Here are my average results of the braces.ksh benchmark from
shbench <http://fossil.0branch.com/csb/tktnew> against stock
/bin/ksh 93u+ vs. current 93u+m (same compiler flags),
100 runs pre-optimisation and 100 runs post-optimisation:

Stock /bin/ksh:		Pre-optimisation (at 3d38270b):
93u+: 0.743 secs	93u+m: 0.739 secs

Stock /bin/ksh:		Post-optimisation (now):
93u+: 0.744 secs	93u+m: 0.726 secs

The left column shows only a small margin of error with 100 runs;
the right one shows a very small, but not insignificant difference.

However, these tests were not very rigorous with 100 runs each.
If anyone wants to do it properly, please report results to
korn-shell@googlegroups.com. I'm happy enough with this, though.

Thanks to Joerg van den Hoff for providing shbench, without
which it would not have occurred to me to try this.

src/cmd/ksh93/sh/subshell.c: sh_assignok():
- Don't bother if we're in a ${ subshare; }.
2020-06-15 20:27:32 +02:00
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
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
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
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
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
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
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
d024d4c895 Fix signal handling due to exit status > 256
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=1469624
https://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)
2020-06-12 01:45:17 +02:00
Martijn Dekker
f88f302c38 Remove code related to long-dead 3DFS research project
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)
2020-06-12 01:45:17 +02:00
Martijn Dekker
5f8b0512f0 POSIX compliance fix: apply 'set -u' to $!
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)
2020-06-12 01:45:17 +02:00
Martijn Dekker
36da314c9e POSIX compliance fix: apply 'set -u' to $1, $2, ...
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)
2020-06-12 01:45:17 +02:00
Martijn Dekker
781f0a39aa Fix parsing of multibyte characters
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=1417886
https://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)
2020-06-12 01:45:17 +02:00
Martijn Dekker
7559f83eff Fix ksh hanging in pipes
Under certain conditions, ksh scripts may hang when piped through
a program that terminates before EOF.

Discussion and reproducer:
https://bugzilla.redhat.com/show_bug.cgi?id=1457990
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01961.html

src/cmd/ksh93/sh/jobs.c:
- In job_wait(), correctly set the terminal foreground process
  group when resuming a process.

(cherry picked from commit 99ef2ca5309da91fdead74bea00743960d470c6a)
2020-06-12 01:45:16 +02:00
Martijn Dekker
98c1e37d86 Fix memory corruption while parsing functions
Discussion: https://bugzilla.redhat.com/show_bug.cgi?id=1451057

src/cmd/ksh93/sh/parse.c: funct():
- Make the savstak variable volatile and always initialise it to
  avoid undefined behaviour.

(cherry picked from commit 5e56b28cd63ec2120c5f70a6e0abf2f8dbb7e7dc)
2020-06-12 01:45:16 +02:00
Martijn Dekker
8e97419b0b Fix ${.sh.subshell} counter to actually count level of subshells
This counter is documented as follows:
"The current depth for subshells and command substitution."

But before this commit, the actual behaviour was that the counter
was reset to zero whenever a subshell forked for any reason: a
pipe, background job, running 'ulimit', redirecting stdout in a
command substitution, and more. This behaviour was:

1. Not consistent with the documentation. Non-forked (a.k.a.
   virtual) subshells are an internal implementation detail which
   scripts should not have to be concerned with. The manual page
   doesn't mention them at all.

2. Inherently broken. Since a subshell may fork for any number of
   reasons, even mid-run, and those reasons may change with
   bugfixes and further development, scripts have never actually
   been able to rely on the value of ${.sh.subshell}.

So, this commit fixes the counter to count the levels of all
subshells, both virtual and forked.

src/cmd/ksh93/sh/xec.c: _sh_fork():
- Increase ${.sh.subshell} whenever we fork.

src/cmd/ksh93/sh/subshell.c:
- sh_subfork():
  * Fix comment to properly explain what it does. It doesn't
    "create" a subshell, it forks off an existing virtual subshell.
  * Don't zero ${.sh.subshell}. Instead, since sh_fork() increases
    it but we're forking an existing subshell, undo the increase.
- sh_subshell():
  * Remove 'int16_t subshell' variable. It was unnecessary and
    mostly unused. It was also the wrong type: it was assigned the
    value from shp->subshell which is of type short.
  * Increase and decrease the level of virtual subshells and
    ${.sh.subshell} independently.

src/cmd/ksh93/tests/variables.sh:
- Add regression tests for ${.sh.subshell} in virtual and forked
  subshells of several kinds: comsub, parentheses, pipe, bg job.
- Undo wrong error test count fix from 04b4aef0.

(cherry picked from commit a0e0e29e7e0dbf21e4b3958ae02bde6665fb2696)
2020-06-12 01:45:16 +02:00
Martijn Dekker
6c68b53170 For consistency & greppability: global replace "sub-shell" => "subshell"
(cherry picked from commit 25eeb9bcdc0a23a66765fd3e5d7e36337da65051)
2020-06-12 01:45:16 +02:00
Martijn Dekker
1026006db3 Fix BUG_KBGPID: $! was not updated under certain conditions
The $! special parameter was not set if a background job
(somecommand &) or co-process (somecommand |&) was launched as the
only command within a braces block with an attached redirection,
for example:
	{
		somecommand &
	} >&2
With the bug, $! was unchanged; now it contains the PID of
somecommand.

Ref.: https://github.com/att/ast/issues/1357

src/cmd/ksh93/sh/parse.c: item():
- When processing redirections following a compound command, always
  create a parent node with the TSETIO (I/O redirection) token.
     Before this commit, if the last command was of type TFORK (and
  the last command only tested as TFORK if the bg job or coprocess
  was the only command in a braces block, because the ksh parser
  optimises away the braces in that case), then the parent node was
  created with the TFORK token instead.
     I have no idea what David Korn's intention was with that, but
  this is clearly very wrong. Creating another TFORK node when
  parsing the redirection caused sh_exec() in sh/xec.c to execute
  the redirection in an extra forked, non-background subshell.
  Since redirections are executed before anything else, this
  subshell is what then launched the background job between the
  braces, so $! (a.k.a. shp->bckpid) was updated in that subshell
  only, and never in the main shell. The extra subshell also
  prevented the background job from being noticed by job control
  on interactive shells.
     So, the fix is simply to remove the broken test for TFORK.

src/cmd/ksh93/tests/variables.sh:
- Add regression tests for a bg job and a co-process as the only
  command within a braces block with attached redirection.

(cherry picked from commit ffe5df30e69f7b596941a98498014d8e838861f2)
2020-06-12 01:45:15 +02:00
Martijn Dekker
eee47df423 Fix handling of skipped directories when autoloading functions
Fix a bug in autoloading functions. Directories in the path search
list which should be skipped (e.g. because they don't exist) did
not interact correctly with autoloaded functions, so that a
function to autoload was not always found.

Details:
https://github.com/att/ast/issues/1454

Fix backported (and cleaned up) from:
https://github.com/att/ast/commit/3bc58164

src/cmd/ksh93/sh/path.c:
- path_opentype(): Fix the path search loop so that entries marked
  with PATH_SKIP are handled correctly.

src/cmd/ksh93/tests/functions.sh:
- Add regression test verifying an autoloaded function with a PATH
  that triggered the bug.
  The bug in path_opentype() fixed by this commit may affect other
  scenarios but we know it affects autoloaded functions. Hence the
  test for that scenario.

(cherry picked from commit a27903165775309f4f032de5d42ec1785f14cfbc)
2020-06-12 01:45:15 +02:00
Martijn Dekker
7003aba487 Fix 'test'/'[' exit status >1 on error in arithmetic expression
Fix BUG_TESTERR1A: POSIX non-compliance of 'test'/'[' exit status
on error. The command now returns status 2 instead of 1 when given
an invalid number or arithmetic expression, e.g.: [ 123 -eq 123x ]

The problem was that the test builtin (b_test()) calls the generic
arithmetic evaluation subsystem (sh/arith.c, sh/streval.c) which
has no awareness of the test builtin. A simple solution would be to
always make the arithmetic subsystem use an exit status > 1 for
arithmetic errors, but globally changing this may cause backwards
compatibility issues. So it's best to change the behaviour of the
'test' builtin only. This requires the arithmetic subsystem to be
aware of whether it was called from the 'test' builtin or not. To
that end, this commit adds a global flag and overrides the
ERROR_exit macro where needed.

src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/defs.c:
- Declare and initialise a global sh_in_test_builtin flag.
- Declare internal function for ERROR_exit override in test.c.

src/cmd/ksh93/bltins/test.c:
- Add override for ERROR_exit macro using a function that checks if
  the exit status is at least 2 if the error occurred while running
  the test builtin.
- b_test(): Set sh_in_test_builtin flag while running test builtin.

src/cmd/ksh93/sh/arith.c,
src/cmd/ksh93/sh/streval.c:
- Override ERROR_exit macro using function from test.c.

src/cmd/ksh93/tests/bracket.sh:
- Add regression test verifying status > 1 on arith error in test.

(cherry picked from commit 5eeae5eb9fd5ed961a5096764ad11ab870a223a9)
2020-06-12 01:45:15 +02:00
Martijn Dekker
ec888867fd Fix unsetting aliases in subshells
Aliases can now be correctly unset within subshell environments
(such as ( ... ), $(command substitutions), etc), as well as
non-subshell "shared" command substitutions (${ ...; }). Before,
attempts to unset aliases within these were silently ignored.

Prior discussion: https://github.com/att/ast/issues/108

Subshell alias trees are only referenced in a few places in the
code, *and* have always been broken, so this commit gets rid of the
whole notion of a subshell alias tree. Instead, there is now just
one flat alias tree, and subshells fork into a separate process
when aliases are set or unset within them. It is not really
conceivable that this could be a performance-sensitive operation,
or even a common one, so this is a clean fix with no downside.

src/cmd/ksh93/include/defs.h:
- Remove sh_subaliastree() definition.

src/cmd/ksh93/sh/subshell.c:
- Remove salias element (pointer to subshell alias tree) from
  subshell struct.
- Remove sh_subaliastree() function.
- sh_subshell(): Remove alias subshell tree cleanup.

src/cmd/ksh93/bltins/typeset.c:
- b_alias(): If in subshell, fork before setting alias.
- b_unalias(): If in subshell, fork before unsetting alias.
- unall(): Remove sh_subaliastree() call.

src/cmd/ksh93/sh/name.c:
- nv_open(): Remove sh_subaliastree() call.

src/cmd/ksh93/tests/subshell.sh:
- Add regression tests for unsetting or redefining aliases within
  subshells.

(cherry picked from commit 12a15605b9521a2564a6e657905705a060e89095)
2020-06-12 01:45:14 +02:00
Martijn Dekker
047cb3303c Fix redefining & unsetting functions in subshells (BUG_FNSUBSH)
Functions can now be correctly redefined and unset in subshell
environments (such as ( ... ), $(command substitutions), etc).
Before this fix, attempts to do this were silently ignored (!!!),
causing the wrong code (i.e.: the function by the same name from
the parent shell environment) to be executed.

Redefining and unsetting functions within "shared" command
substitutions of the form '${ ...; }' is also fixed.

Prior discussion: https://github.com/att/ast/issues/73

src/cmd/ksh93/sh/parse.c:
- A fix from George Koelher (URL above). He writes:
  | The parser can set t->comnamp to the wrong function.
  | Suppose that the shell has executed
  |     foo() { echo WRONG; }
  | and is now parsing
  |     (foo() { echo ok; } && foo)
  | The parser was setting t->comnamp to the wrong foo. [This
  | fix] doesn't set t->comnamp unless it was a builtin. Now the
  | subshell can't call t->comnamp, so it looks for foo and finds
  | the ok foo in the subshell's function tree.

src/cmd/ksh93/bltins/typeset.c:
- Unsetting functions in a virtual/non-forked subshell still
  doesn't work: nv_open() fails to find the function. To work
  around this problem, make 'unset -f' fork the subshell into its
  own process with sh_subfork().
- The workaround exposed another bug: if we unset a function in a
  subshell tree that overrode a function by the same name in the
  main shell, then nv_delete() exposes the function from the main
  shell scope. Since 'unset -f' now always forks a subshell, the
  fix is to simply walk though troot's parent views and delete any
  such zombie functions as well. (Without this, the 4 'more fun'
  tests in tests/subshell.sh fail.)

src/cmd/ksh93/sh/subshell.c: sh_subfuntree():
- Fix function (re)definitions and unsetting in "shared" command
  substitutions of the form '${ commandlist; }' (i.e.: if
  sp->shp->subshare is true). Though internally this is a weird
  form of virtual subshell, the manual page says it does not
  execute in a subshell (meaning, all changes must survive it), so
  a subshell function tree must not be created for these.

src/cmd/ksh93/tests/subshell.sh:
- Add regression tests related to these bugfixes. Test unsetting
  and redefining a function in all three forms of virtual subshell.

(cherry picked from commit dde387825ab1bbd9f2eafc5dc38d5fd0bf9c3652)
2020-06-12 01:45:14 +02:00
Martijn Dekker
593a5a8b7f Patch vulnerability CVE-2019-14868
Certain environment variables were interpreted as arithmetic
expressions on startup, leading to code injection.

Ref.:
https://bugzilla.redhat.com/show_bug.cgi?id=1757324
c7de8b6412

(cherry picked from commit ee6b001d0611ad2e00b6da2c2b42051995c0a678)
2020-06-12 01:45:14 +02:00
Martijn Dekker
e999f6b169 Fix truncating of files with <>;file combined with <#pattern
The issue with truncating files was caused by out-of-sync streams.
Details and discussion: https://github.com/att/ast/issues/61

src/cmd/ksh93/sh/io.c: sh_iorestore():
- To be safe, sync all streams before restoring file descriptors.

src/cmd/ksh93/tests/io.sh:
- Add two regression tests for truncating files with this
  combination of redirections.
- The second test, which invokes a -c script, is disabled for now
  as this triggers another corner case bug involving the SH_NOFORK
  optimisaton for -c scripts. That fix is for another commit.

(cherry picked from commit 18fb64840365c2ff4608188e5487bd79d08f67d1)
2020-06-12 01:45:14 +02:00
Martijn Dekker
952944197f Fix bugs in testing if a parameter is set
This fixes three related bugs:
1. Expansions like ${var+set} remained static when used within a
   'for', 'while' or 'until' loop; the expansions din't change
   along with the state of the variable, so they could not be used
   to check whether a variable is set within a loop if the state of
   that variable changed in the course of the loop. (BUG_ISSETLOOP)
2. ${IFS+s} always yielded 's', and [[ -v IFS ]] always yielded
   true, even if IFS is unset. (BUG_IFSISSET)
3. IFS was incorrectly exempt from '-u' ('-o nounset') checks.

src/cmd/ksh93/sh/macro.c: varsub():
- When getting a node pointer (np) to the parameter to test,
  special-case IFS by checking if it has a value and not setting
  the pointer if not. The node to IFS always exists, even after
  'unset -v IFS', so before this fix it always followed the code
  path for a parameter that is set. This fixes BUG_IFSISSET for
  ${IFS+s} and also fixes set -u (-o nounset) with IFS.
- Before using the 'nv_isnull' macro to check if a regular variable
  is set, call nv_optimize() if needed. This fixes BUG_ISSETLOOP.
  Idea from Kurtis Rader: https://github.com/att/ast/issues/1090
  Of course this only works if SHOPT_OPTIMIZE==1 (the default),
  but if not, then this bug is not triggered in the first place.
- Add some comments for future reference.

src/cmd/ksh93/bltins/test.c: test_unop():
- Fix BUG_IFSISSET for [[ -v IFS ]]. The nv_optimize() method
  doesn't seem to have any effect here, so the only way that I can
  figure out is to special-case IFS, nv_getval()'ing it to check if
  IFS has a value in the current scope.

src/cmd/ksh93/tests/variables.sh:
- Add regression tests for checking if a varariable is set within a
  loop, within and outside a function with that variable made local
  (to check if the scope is honoured). Repeat these tests for a
  regular variable and for IFS, for ${foo+set} and [[ -v foo ]].

(cherry picked from commit a2cf79cb98fa3e47eca85d9049d1d831636c9b16)
2020-06-12 01:45:14 +02:00
Martijn Dekker
c9ccee86bb Fix 'command -p' by fixing initialisation of default PATH variable
'command -p' was broken for non-interactive shells as the variable
used to store the default system PATH, std_path, was not
initialised correctly. For instance:
	$ ksh -c 'command -p ls'
	ksh: ls: not found
This fix by Siteshwar Vashisht is backported from ksh2020.
Ref.:
https://github.com/att/ast/issues/426
https://github.com/att/ast/pull/448

src/cmd/ksh93/sh/path.c:
- Correctly initialise std_path (the default PATH) when ksh is
  started as a non-interactive shell.

src/cmd/ksh93/sh.1:
- Fix vague explanation of 'command -p'.

src/cmd/ksh93/tests/path.sh:
- Add regression test.

(cherry picked from commit a76439d60b70c18cf44d84c1962fcd8df84c947c)
2020-06-12 01:45:14 +02:00
Martijn Dekker
75fc493e7d sh/suid_exec.c: fix 2 confusing typos in comments
(cherry picked from commit 6cccaedd91e4309b542b8a17c8318f3bff9511b0)
2020-06-12 01:45:14 +02:00
Martijn Dekker
412cd96dc1 fix 5 more typos: incorrect "than"
(cherry picked from commit 7e2e4651d2dd5884dd3ea06fc8df4dc45ec9bfb5)
2020-06-12 01:45:13 +02:00
Martijn Dekker
eeee77edd1 Fix BUG_REDIRIO
ksh used to redirect standard output by default when no file
descriptor was specified with the rarely used '<>' reading/writing
redirection operator. It now redirects standard input by default,
as POSIX specifies and as all other POSIX shells do. To redirect
standard output for reading and writing, you now need '1<>'.

Ref.: https://github.com/att/ast/issues/75
      http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_07_07

(cherry picked from commit 29afc16c47824fc79ed092ae7704c525b1db6a0a)
2020-06-12 01:45:13 +02:00
Martijn Dekker
6a4972069f Fix BUG_CASELIT: pattern matching as literal string in 'case'
This fixes an undocumented 'case' pattern matching misbehaviour
(labelled BUG_CASELIT in modernish) that goes back to the original
Bourne shell, but wasn't discovered until 2018.

If a pattern doesn't match as a pattern, it's tried again as a
literal string. This breaks common validation use cases, such as:

n='[0-9]'
case $n in
( [0-9] )  echo "$n is a number" ;;
esac

would output "[0-9] is a number" as the literal string fallback
matches the pattern. As this misbehaviour was never documented
anywhere (not for Bourne, ksh88, or ksh93), and it was never
replicated in other shells (not even in ksh88 clones pdksh and
mksh), it is unlikely any scripts rely on it.

Of course, a literal string fallback, should it be needed, is
trivial to implement correctly without this breakage:

case $n in
( [0-9] | "[0-9]")  echo "$n is a number or the number pattern" ;;
esac

src/cmd/ksh93/sh/xec.c:
- Remove trim_eq() function responsible for implementing the
  misbehaviour described above.

NEWS:
- Added. Document this bugfix.

Ref.:
- The problem: thread starting at
  https://www.mail-archive.com/austin-group-l@opengroup.org/msg02127.html
- The solution, thanks to George Koehler: comments/commits in
  https://github.com/att/ast/issues/476
- Modernish BUG_CASELIT bug test & documentation:
  https://github.com/modernish/modernish/commit/b2024ae3

(cherry picked from commit 8d6c8ce69884767a160c1e20049e77bdd849c248
with some extra edits to NEWS to upate the info for this reboot)
2020-06-12 01:45:13 +02:00