1
0
Fork 0
mirror of git://git.code.sf.net/p/cdesktopenv/code synced 2025-03-09 15:50:02 +00:00
Commit graph

52 commits

Author SHA1 Message Date
Johnothan King
05ac1dbb41
Fix crash upon running many subshells (#113)
Co-authored-by: Martijn Dekker <martijn@inlv.org>

An intermittent crash occurred after running many thousands of
virtual/non-forked subshells. One reproducer is a crash in the
shbench fibonacci.ksh test, as documented here:
f3d9e134/bench/fibonacci.ksh (L4-L10)

The apparent cause was the signed and insufficiently large 'short'
data type of 'curenv' and related variables which wrapped around to
a negative number when overflowing. These IDs are necessary for the
'wait' builtin to obtain the exit status from a background job.

This fix is inspired by a patch based on ksh 93v-:
https://build.opensuse.org/package/view_file/shells/ksh/ksh93-longenv.dif?expand=1
https://src.fedoraproject.org/rpms/ksh/blob/f24/f/ksh-20130628-longer.patch

However, we change the type to 'unsigned int' instead of 'long'. On
all remotely modern systems, ints are 32-bit values, and using this
type avoids a performance degradation on 32-bit sytems. Making them
unsigned prevents an overflow to negative values.

src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/include/jobs.h,
src/cmd/ksh93/include/nval.h,
src/cmd/ksh93/include/shell.h:
- Change the types of the static global 'subenv' and the subshell
  structure members 'curenv', 'jobenv', 'subenv', 'p_env' and
  'subshell' to one consistent type, unsigned int.

src/cmd/ksh93/sh/jobs.c,
src/cmd/ksh93/sh/macro.c:
src/cmd/ksh93/sh/name.c:
src/cmd/ksh93/sh/nvtype.c,
src/cmd/ksh93/sh/subshell.c:
- Updates to match new variable types.

src/cmd/ksh93/tests/subshell.sh:
- Show wrong exit status in message on failure of 'wait' builtin.
2020-08-12 18:50:59 +01:00
Martijn Dekker
8477d2ce22 printf: Fix HTML and URI encoding (%H, %#H)
This applies a number of fixes to the printf formatting directives
%H and %#H (as well as their equivalents %(html)q and %(url)q):
1. Both formatters have been made multibyte/UTF-8 aware, and no
   longer delete multibyte characters. Invalid UTF-8 byte sequences
   are rendered as ASCII question marks.
2. %H no longer wrongly encodes spaces as non-breaking spaces
   (&nbsp;) and instead correctly encodes the UTF-8 non-breaking
   space as such.
3. %H now converts the single quote (') to '%#39;' instead of
   '&apos;' which is not a valid entity in all HTML versions.
4. %#H failed to encode some reserved characters (e.g. '?') while
   encoding some unreserved ones (e.g. '~'). It now percent-encodes
   all characters except those 'unreserved' as per RFC3986 (ASCII
   alphanumeric plus -._~).

Prior discussion:
ce8d1467-4a6d-883b-45ad-fc3c7b90e681%40inlv.org

src/cmd/ksh93/include/defs.h:
src/cmd/ksh93/sh/string.c:
- defs.h: If compiling without SHOPT_MULTIBYTE, redefine the
  mbwide() macro (which tests if we're in a multibyte locale) as 0.
  This lets the compiler optimiser do the work that would otherwise
  require a lot of tedious '#if SHOPT_MULTIBYTE' directives.
- string.c: Remove some now-unneeded '#if SHOPT_MULTIBYTE' stuff.
- defs.h, string.c: Rename is_invisible() to sh_isprint(), invert
  the boolean return value, and make it an extern for use in
  fmthtml() -- see below. If compiling without SHOPT_MULTIBYTE,
  simply #define sh_isprint() as equivalent to isprint(3).
- defs.h: Add URI_RFC3986_UNRESERVED macro for fmthtml() containing
  the characters "unreserved" for purposes of URI percent-encoding.

src/cmd/ksh93/bltins/print.c: fmthtml():
- Remove kludge that skipped all multibyte characters (!).
- Complete rewrite to implement fixes described above.
- Don't bother with '#if SHOPT_MULTIBYTE' directives (see above).

src/cmd/ksh93/data/builtins.c:
- sh_optprintf[]: %H: Add single quote to encoded chars doc.
- Edit credits and bump version date.

src/cmd/ksh93/tests/builtins.sh:
- Update and tweak old regression tests.
- Add a number of new tests for UTF-8 HTML and URI encoding, which
  are only run when running tests in a UTF-8 locale (shtests -u).
2020-08-10 22:51:55 +01:00
Martijn Dekker
5312a59d5a Skip '.' and '..' when globbing patterns like .*
There are convincing arguments why including '.' and '..' in the
result of pathname expansion is actively harmful. See:
https://www.austingroupbugs.net/view.php?id=1228
https://github.com/ksh93/ksh/issues/58#issuecomment-653716846

pdksh, mksh and zsh already skip these special traversal names
in all cases. This commit makes ksh act like these shells.

Since passing '.' and especially '..' as arguments to commands like
'chmod -R' and 'cp -r' may cause harm, this change seems likely to
fix more legacy scripts than it breaks. I'm unaware of anyone ever
having come up with a concrete use case for the old behaviour.

This change also fixes the bug that '.' and '..' failed to be
ignored as documented if FIGNORE is set.

src/lib/libast/misc/glob.c: glob_dir():
- Explicitly skip any matching '.' and '..' in all cases.

src/cmd/ksh93/tests/glob.sh:
- Add test_glob() tests for '*' and '.*'.

src/cmd/ksh93/sh.1: File Name Generation:
- Update to match new behaviour.

Resolves: https://github.com/ksh93/ksh/issues/58
2020-08-10 00:35:53 +01:00
Martijn Dekker
be5ea8bbb2 redirect: check args before executing redirections (re: 7b82c338)
The 'redirect' builtin command did not error out before executing
any valid redirections. For example, 'redirect ls >foo.txt' issued
an "incorrect syntax" error, but still created 'foo.txt' and left
standard output permanently redirected to it.

src/cmd/ksh93/sh/xec.c: sh_exec():
- If we have redirections (io != NULL), and the command is
  SYSREDIR, then check for arguments and error out if there are
  any, before calling sh_redirect() to execute redirections.
  (Note, the other check for arguments in b_exec() in bltins/misc.c
  must be kept, as that applies if there are no redirections.)

src/cmd/ksh93/sh/io.c: sh_redirect():
- Edit comments to better explain what the flag values do.

src/cmd/ksh93/bltins/misc.c:
- Add a dummy b_redirect() function declaration "for the dictionary
  generator" as has historically been done for other builtins that
  share one C function. I'm not sure what that dictionary generator
  is supposed to be, but this also improves greppability.

src/cmd/ksh93/data/builtins.c,
src/cmd/ksh93/sh.1:
- Fix misleading "I/O redirection arguments" term. I/O redirections
  are not arguments at all; no argument parser ever sees them.

src/cmd/ksh93/tests/io.sh:
- Test both conditions that should make 'redirect' produce an
  "incorrect syntax" error.
- Test that any redirections are not executed if erroneous
  non-redirection arguments exist.

src/cmd/ksh93/tests/builtins.sh:
- "... should show usage info on unrecognized options" test:
  Because 'redirect' now refuses to process redirections on error,
  the error message was not captured. The fix is to run the builtin
  in a braces block and add the redirection to the block.
2020-08-09 00:47:22 +01:00
Martijn Dekker
e805c7d9b1 Fix crash: do not list job if in 60 sec grace period (re: 33858689)
The crash in job_list() or job_unpost() could still occur after the
previous patch if a signal was being handled after $TMOUT was
exceeded and the 60-second grace period was entered.

It *should* work to add a general check for !sh_isstate(SH_GRACE).
We know that the SH_GRACE state is set immediately after printing
the 60 second grace period warning message:
9de65210/src/cmd/ksh93/sh/io.c (L1869-L1870)
(and that the crashes occur upon re-evaluating the $PS1 prompt
after setting the SH_GRACE state). We know that the SH_GRACE state
is not turned off again until either the user enters a line:
9de65210/src/cmd/ksh93/sh/main.c (L474)
or the shell times out after the grace period:
9de65210/src/cmd/ksh93/sh/io.c (L1861)
The SH_GRACE state flag is not used or changed in any other context
(verified with grep -rn SH_GRACE src/cmd/ksh93). So, logically,
this should suffice to make sure the crash stays gone.

src/cmd/ksh93/sh/jobs.c: job_reap():
- Do not list jobs when the SH_GRACE state (the 60 second timeout
  grace period after TMOUT was exceeded) is active.
- Keep the previous check for job control just to be sure, and
  because it makes sense.

Fixes: https://github.com/ksh93/ksh/issues/103 (again)
2020-08-07 21:09:01 +01:00
Martijn Dekker
338586896d Fix crash: do not list jobs if there is no job control
This bug caused an undefined state, which sometimes crashed the
shell in job_list() or job_unpost(), if $PS1 contains a command
substitution running an external command and the '-b'/'-o notify'
shell option is active. So far the only known way to trigger the
crash is by letting $TMOUT time out the interactive shell. See
https://github.com/ksh93/ksh/issues/103 for details.

src/cmd/ksh93/sh/jobs.c: job_reap():
- The check for the SH_NOTIFY option and the SH_TTYWAIT state
  before listing jobs was insufficient. Job control is disabled in
  command substitutions, so also check that job control is active
  before listing jobs.

src/cmd/ksh93/sh.1:
- Fix TMOUT documentation. The 'read' command in fact only times
  out when reading from a terminal, just like 'select'. Also
  document the extra 60 second grace period when an interactive
  shell prompt reads from a terminal.

Fixes: https://github.com/ksh93/ksh/issues/103
2020-08-06 22:46:02 +01:00
Johnothan King
23f2e23385
Over-shifting in a POSIX function should cause scripts to exit (#106)
The required longjmp used to terminate scripts was not being run
when over-shifting in a POSIX function with a redirection. This
caused scripts to continue after an error in the shift builtin,
which is incorrect since shift is a special builtin. The
interpreter is sent into an indeterminate state that causes
undefined behavior as well:
$ cat reproducer.ksh
some_func() {
   shift 10
}

for i in a b c d e f; do
  echo "read $i"
  [ "$i" != "c" ] && continue
  some_func 2>&1
  echo "$i = c"
done
$ ksh ./reproducer.ksh
read a
read b
read c
/tmp/k[2]: shift: 10: bad number
c = c
read d
/tmp/k[2]: shift: 10: bad number
d = c
read e
/tmp/k[2]: shift: 10: bad number
e = c
read f
/tmp/k[2]: shift: 10: bad number
f = c

src/cmd/ksh93/sh/xec.c: sh_exec():
- Do the necessary longjmp needed to terminate the script after
  over-shifting in a POSIX function when the function call has a
  redirection.

src/cmd/ksh93/tests/functions.sh:
- Add the over-shifting regression test from ksh93v- 2013-10-10-alpha.

Bug report and fix on the old mailing list:
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg00732.html
2020-08-05 18:06:16 +01:00
Johnothan King
02a14ff9b7
Fix creation of extra associative array element '0' (#101)
Multidimensional associative arrays are created with an extra array
member named '0', which is set to no value. Reproducer:

$ typeset -A foo
$ typeset -A foo[bar]
$ typeset -p foo
typeset -A foo=([bar]=([0]='') )

The bugfix prevents nv_setarray from creating the extra '[0]' member
when an associative array is empty. This bug was discussed on the old
mailing list:
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01574.html

src/cmd/ksh93/sh/array.c:
- Do not allow the creation of an extra array member when an array
  is empty.

src/cmd/ksh93/tests/arrays.sh:
- Add a regression test for creating multidimensional associative
  arrays, but use the output from 'typeset -p' instead of fgrep.
2020-07-31 17:32:09 +01:00
Martijn Dekker
70f6d758c0 Fix blocked signals after fork(2)ing external command in subshell
When the classic fork/exec mechanism was used (via sh_fork()) to
run an external command from within a non-forking subshell, SIGINT
was blocked until that subshell was exited. If a subsequent loop
was run in the subshell, it became uninterruptible, e.g.:

   $ arch/*/bin/ksh -c '(/usr/bin/true; while :; do :; done); exit'
   ^C^C^C^C^C

src/cmd/ksh93/sh/xec.c:
- sh_fork() did not reset the savesig variable in the parent part
  of the fork when running in a virtual subshell. This had the
  effect of delaying signal handling until exiting the subshell.
  There is no reason for that subshell check that I can discern, so
  this removes it.
      I've verified that this causes no regression test failures
  even when ksh is compiled with -DSHOPT_SPAWN=0 which means the
  classic fork/exec mechanism is always used.

Fixes: https://github.com/ksh93/ksh/issues/86
2020-07-30 01:46:00 +01:00
Johnothan King
8b5f11dcd7
Add support for multibyte characters to $IFS (#92)
Add support for multibyte characters to $IFS

This commit fixes BUG_MULTIBIFS, which had two bug reports in the ksh2020 branch.

src/cmd/ksh93/sh/macro.c:
- Backport Eric Scrivner's fix for multibyte IFS characters (slightly modified
  for compatibility with C89). Explanation from https://github.com/att/ast/pull/737:

  Previously, the varsub method used for the macro expansion of $param, ${param},
  and ${param op word} would incorrectly expand the internal field separator (IFS)
  if it was a multibyte character. This was due to truncation based on the
  incorrect assumption that the IFS would never be larger than a single byte.

  This change fixes this issue by carefully tracking the number of bytes that
  should be persisted in the IFS case and ensuring that all bytes are written
  during expansion and substitution.

  Bug report: https://github.com/att/ast/issues/13

- Fixed another bug that caused multibyte characters with the same initial byte
  to be treated as the same character by the IFS. This bug was occurring because
  the first byte of a multibyte character wasn't being written to the stack when
  the IFS delimiter had the same initial byte:

  $ IFS=£
  $ v='§'
  $ set -- $v
  $ v="${1-}"
  $ echo "$v" | hd # The first byte should be c2, but it isn't due to the bug
  00000000  a7 0a                                             |..|
  00000002

  Bug report: https://github.com/att/ast/issues/1372

src/cmd/ksh93/tests/variables.sh:
- Add (reworked) regression tests from ksh2020 for the multibyte IFS bugs.
- Add a regression test for att/ast#1372 based on the reproducer.
2020-07-25 19:46:11 +01:00
Johnothan King
6e515f1d45
Fix command substitutions run on the same line as a here-doc (#91)
When a command substitution is run on the same line as a here-document,
a syntax error occurs due to a regression introduced in ksh93u+ 2011-04-15:

true << EOF; true $(true)
EOF
syntax error at line 1: `<<EOF' here-document not contained within command substitution

The regression is caused by an error check that was added to make
the following script causes a syntax error (because the here-document
isn't completed inside of the command substitution):

$(true << EOF)
EOF

src/cmd/ksh93/sh/lex.c:
- Only throw an error when a here-document in a command substitution
  isn't completed inside of the command substitution.

src/cmd/ksh93/tests/heredoc.sh:
- Add a regression test for running a command substitution on the
  same line as a here-document.
- Add a missed regression test for using here-documents in command
  substitutions. This is the original bug that was fixed in ksh93u+
  2011-04-15 (it is why the error message was added), but a regression
  test for here-documents in command substitutions wasn't added in
  that version.

This bugfix was backported from ksh93v- 2013-10-10-alpha.
2020-07-24 00:03:57 +01:00
Martijn Dekker
f207cd5787 Fix race conditions running external commands with job control on
When ksh is compiled with SHOPT_SPAWN (the default), which uses
posix_spawn(3) or vfork(2) (via sh_ntfork()) to launch external
commands, at least two race conditions occur when launching
external commands while job control is active. See:
1887863/comments/3
https://www.mail-archive.com/ast-developers@research.att.com/msg00717.html

The basic issue is that this performance optimisation is
incompatible with job control, because it uses a spawning mechanism
that doesn't copy the parent process' memory pages into the child
process, therefore no state that involves memory can be set before
exec-ing the external program. This makes it impossible to
correctly set the terminal's process group ID in the child process,
something that is essential for job control to work.

src/cmd/ksh93/sh/xec.c:
- Use sh_fork() instead of sh_ntfork() if job control is active.
  This uses fork(2), which is 30%-ish slower on most sytems, but
  allows for correctly setting the terminal process group.

src/cmd/ksh93/tests/basic.sh:
- Add regression test for the race condition reported in #79.

src/cmd/INIT/cc.darwin:
- Remove hardcoded flag to disable SHOPT_SPAWN on the Mac.
  It should be safe to use now.

Fixes https://github.com/ksh93/ksh/issues/79
2020-07-22 13:45:33 +01:00
Martijn Dekker
bc8b36faba whence -a/type -a: report both function and built-in by same name
'whence -a' is documented to list all possible interpretations of a
command, but failed to list a built-in command if a shell function
by the same name exists or is marked undefined using 'autoload'.

src/cmd/ksh93/bltins/whence.c: whence():
- Refactor and separate the code for reporting functions and
  built-in commands so that both can be reported for one name.

src/cmd/ksh93/data/builtins.c: sh_optwhence[]:
- Correct 'whence --man' to document that:
  * 'type' is equivalent to 'whence -v'
  * '-a' output is like '-v'

src/cmd/ksh93/tests/builtins.sh:
- Test 'whence -a' with these combinations:
  * a function, built-in and external command
  * an undefined/autoload function, built-in and external command

Fixes https://github.com/ksh93/ksh/issues/83
2020-07-20 21:16:24 +01:00
Johnothan King
bd88cc7f4f
Fix two crashes related to kshdb (#82)
This commit fixes two different crashes related to kshdb:
- When redirect is given an invalid file descriptor, a segfault
  no longer occurs. Reproducer:
  $ ksh -c 'redirect 9>&200000000000'

- Fix a crash due to free(3) being used on an invalid pointer.
  This can be reproduced with kshdb (commands from att/ast#582):
  $ git clone https://github.com/rocky/kshdb.git
  $ cd kshdb
  $ ksh autogen.sh
  $ echo "print hi there" > $HOME/.kshdbrc
  $ ./kshdb -L . test/example/dbg-test1.sh

src/cmd/ksh93/bltins/misc.c: b_dot_cmd():
- The string pointed to by shp->st.filename must be able to be
  freed from memory with free(3), so duplicate the string with
  strdup(3).

src/cmd/ksh93/sh/io.c: sh_redirect():
- Show an error message when a file descriptor is invalid to
  fix a memory fault.
2020-07-19 23:42:12 +01:00
Johnothan King
03224ae3af
Make the 'history' and 'r' commands builtins (#76)
With this change no more preset aliases exist, so the preset alias
tables can be safely removed. All ksh commands can now be used
without 'unalias -a' removing them, even in interactive shells.
Additionally, the history and r commands are no longer limited to
being used in interactive shells.

src/cmd/ksh93/bltins/hist.c:
- Implement the history and r commands as builtins. Also guarantee
  lflag is set to one by avoiding 'lflag++'.

src/cmd/ksh93/Makefile,
src/cmd/ksh93/Mamfile,
src/cmd/ksh93/sh/main.c,
src/cmd/ksh93/sh/init.c,
src/cmd/ksh93/data/aliases.c:
- Remove the table of predefined aliases because the last few have
  been removed. During init the alias tree is now initialized the
  same way as the function tree.

src/cmd/ksh93/bltins/typeset.c:
- Remove the bugfix for unsetting predefined aliases because it is
  now a no-op. Aliases are no longer able to have the NV_NOFREE
  attribute.

src/cmd/ksh93/tests/alias.sh:
- Remove the regression test for unsetting predefined aliases since
  those no longer exist.

src/cmd/ksh93/data/builtins.c:
- Update sh_opthist[] for 'hist --man', etc.

src/cmd/ksh93/sh.1:
- Remove the list of preset aliases since those no longer exist.
- Document history and r as builtins instead of preset aliases.

Co-authored-by: Martijn Dekker <martijn@inlv.org>
2020-07-16 18:56:49 +01:00
Martijn Dekker
1fbbeaa19d Convert default typeset aliases to regular builtins
This converts the 'autoload', 'compound', 'float', 'functions',
'integer' and 'nameref' default aliases into regular built-in
commands, so that 'unalias -a' does not remove them. Shell
functions can now use these names, which improves compatibility
with POSIX shell scripts.

src/cmd/ksh93/data/aliases.c:
- Remove default typeset aliases.

src/cmd/ksh93/data/builtins.c,
src/cmd/ksh93/include/builtins.h:
- Add corresponding built-in command declarations. Typeset-style
  commands are now defined by a pointer range, SYSTYPESET ..
  SYSTYPESET_END. A couple need their own IDs (SYSCOMPOUND,
  SYSNAMEREF) for special-casing in sh/xec.c.
- Update 'typeset --man'.

src/cmd/ksh93/bltins/typeset.c: b_typeset():
- Recognise the new builtin commands by argv[0]. Implement them by
  inserting the corresponding 'typeset' options into the argument
  list before parsing options. This may seem like a bit of a hack,
  but it is simpler, shorter, more future-proof and less
  error-prone than manually copying and adapting all the complex
  flaggery from the option parsing loop.

src/cmd/ksh93/sh/parse.c,
src/cmd/ksh93/sh/xec.c:
- Recognise typeset-style commands by SYSTYPESET .. SYSTYPESET_END
  pointer range.
- Special-case 'compound' (SYSCOMPOUND) and 'nameref' (SYSNAMEREF)
  along with recognising the corresponding 'typeset' options.

src/cmd/ksh93/sh.1:
- Update to document the new built-ins.
- Since not all declaration commands are special built-ins now,
  identify declaration commands using a double-dagger "\(dd"
  character (which renders as '=' in ASCII) and disassociate their
  definition from that of special built-ins.

src/cmd/ksh93/tests/variables.sh:
- Adapt a regression test as there is no more 'integer' alias.
2020-07-15 20:54:06 +01:00
Johnothan King
fc655f1a26
Restore 'set -b'/'set -o notify' functionality (#74)
'set -b' had no effect; it should cause the shell to notify job
state changes immediately instead of waiting for the next prompt.

This fixes a regression that was introduced in ksh93t 2008-07-25.
The bugfix is from: https://github.com/att/ast/pull/1089

src/cmd/ksh93/sh/jobs.c:
- Save the tty wait state and avoid changing it if TTYWAIT was
  already on to avoid breaking 'set -b'.
  The last 'sh_offstate' is inside of an '#if' directive because it
  is only required when ksh is compiled with SHOPT_COSHELL enabled.

src/cmd/ksh93/tests/pty.sh:
- Add a regression test for 'set -b' in interactive shells.
2020-07-14 22:00:28 +01:00
Johnothan King
66c955bc8f
Fix a fork bomb when vi is run from a script and sent Ctrl-Z (#73)
This bug was reported on the old mailing list:
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg00207.html

A fork bomb can occur when SIGTSTP is sent to the vi editor. Vi
must be launched from a script run with exec (tested with
BusyBox vi, nvi and vim):
$ cat /tmp/foo
vi /tmp/bar
echo end
$ ksh
$ chmod +x /tmp/foo
$ exec /tmp/foo
While in vi, send SIGTSTP using Ctrl-Z

src/cmd/ksh93/sh/fault.c:
- Only fork after Ctrl-Z if job control is available. The patch
  used checks 'job.jobcontrol' instead of 'SH_MONITOR':
  https://git.centos.org/rpms/ksh/blob/c8/f/SOURCES/ksh-20120801-forkbomb.patch
2020-07-13 19:10:23 +01:00
Johnothan King
c4236cc295 Fix type names starting with lowercase 'a' (#69)
Type names that start with a lowercase 'a' cause an error when used:

$ typeset -T al=(typeset bar)
$ al foo=(bar=testset)
/usr/bin/ksh: al: : invalid variable name

The error occurs because when the parser checks for the alias
builtin (to set 'assignment' to two instead of one), only the first
letter of 'argp->argval' is checked (rather than the entire
string). This was fixed in ksh93v- by comparing argp->argval
against "alias", but in ksh93u+m the check can simply be removed
because it is only run when a builtin has the BLT_DCL flag. As of
04b9171, the alias builtin does not have that flag.

src/cmd/ksh93/sh/parse.c:
- Remove the bugged check for the alias builtin.

src/cmd/ksh93/tests/types.sh:
- Add a regression test for type names starting with a lowercase 'a'.
2020-07-10 17:54:51 +01:00
Martijn Dekker
a8f6d6b842 Fix crash due to double free() when sourcing multiple files
There is a bug in sh_eval() that may cause ksh to crash due to a
double free() after sourcing multiple files with '.' or 'source'
if a longjmp is triggered, e.g. by a syntax error.

This applies a fix from Siteshwar Vashist:
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01943.html

src/cmd/ksh93/sh/xec.c: sh_eval():
- Zero file descriptor io_save after closing it. This prevents a
  double free() after returning from a longjmp.

src/cmd/ksh93/tests/basic.sh:
- Add reproducer as regression test.
2020-07-09 15:35:07 +01:00
Johnothan King
9526b3fa08
Fix unexpected output from 'printf %T' with certain formats (#65)
This commit changes the behavior of four date formats accepted
by 'printf %()T' because the old behavior is not compatible with
modern implementations of date(1):
- %k and %l now return a blank-padded hour, the former based on a
  24-hour clock and the latter a 12-hour clock (these are common
  extensions present on Linux and *BSD).
- %f now returns a date with the format '%Y.%m.%d-%H:%M:%S'
  (BusyBox extension).
- %q now returns the quarter of the current year (GNU extension).

src/cmd/ksh93/data/builtins.c:
- Copy the date format documentation from date in libcmd to
  the printf man page (for documenting 'printf %T').

src/cmd/ksh93/tests/builtins.sh:
- Add four regression tests for the changed date formats.

src/cmd/ksh93/sh.1:
- Remove inaccurate information about the date formats accepted by
  printf %T'. The KornShell uses a custom version of strftime(3)
  that isn't guaranteed to accepts the same formats as the native
  strftime function.

src/lib/libast/tm/tmxfmt.c:
- Change the behavior of %f, %k, %l and %q to the common behavior.
  %k and %l are implemented as aliases to %_H and %_I to avoid
  duplicating code.

src/lib/libcmd/date.c:
- Update the documentation for the AST date command since it is
  also affected by the changes to 'printf %T'.

Fixes #62
2020-07-09 05:08:28 +01:00
Johnothan King
9a9da2c299
Fix use of strdup on a NULL pointer (#63)
The following set of commands can rarely cause a memory fault
when auditing[*] is enabled, although most of the time it will
simply cause ksh to write '(null)' to the auditing file in place
of a tty name:

$ [ -e /etc/ksh_audit ] || echo "/tmp/ksh_auditfile;$(id -u)" | sudo tee /etc/ksh_audit;
$ v=$(ksh  2> /dev/null +o rc -ic $'getopts a:bc: opt --man\nprint $?')
$ cat /tmp/ksh_auditfile
1000;1593599493;(null); getopts a:bc: opt --man

This happens because strdup is used unconditionally on the pointer
returned by 'ttyname', which can be NULL if stderr is closed. This
then causes 'hp->tty' to be set to null, as strdup returns NULL.
See https://github.com/att/ast/issues/1028

src/cmd/ksh93/edit/history.c:
- Make strdup duplicate 'notty' instead of NULL to prevent
  crashes.

[*] https://blog.fpmurphy.com/2008/12/ksh93-auditing-and-accounting.html
2020-07-06 21:51:44 +01:00
Martijn Dekker
300cd19987 Fix corrupt UTF-8 char processing & shellquoting after aborted read
If the processing of a multibyte character was interrupted in UTF-8
locales, e.g. by reading just one byte of a two-byte character 'ü'
(\303\274) with a command like:
	print -nr $'\303\274' | read -n1 g
then the shellquoting algorithm was corrupted in such a way that
the final quote in simple single-quoted string was missing. This
bug may have had other, as yet undiscovered, effects as well. The
problem was with corrupted multibyte character processing and not
with the shell-quoting routine sh_fmtq() itself.

Full trace and discussion at: https://github.com/ksh93/ksh/issues/5
(which is also an attempt to begin to understand the esoteric
workings of the libast mb* macros that process UTF-8 characters).

src/lib/libast/comp/setlocale.c: utf8_mbtowc():
- If called from the mbinit() macro (i.e. if both pointer
  parameters are null), reset the global multibyte character
  synchronisation state variable. This fixes the problem with
  interrupted processing leaving an inconsistent state, provided
  that mbinit() is called before processing multibyte characters
  (which it is, in most (?) places that do this). Before this fix,
  calling mbinit() in UTF-8 locales was a no-op.

src/cmd/ksh93/sh/string.c: sh_fmtq():
- Call mbinit() before potentially processing multibyte characters.
  Testing suggests that this could be superfluous, but at worst,
  it's harmless; better be sure.

src/cmd/ksh93/tests/builtins.sh:
- Add regression test for shellquoting with 'printf %q' after
  interrupting the processing of a multibyte characeter with
  'read -n1'. This test only fails in a UTF-8 locale, e.g. when
  running: bin/shtests -u builtins SHELL=/buggy/ksh-2012-08-01

Fixes #5.
2020-07-05 19:24:41 +02:00
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: 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
120aec25ba
Fix a crash when 'read -u' is given an invalid fd (#53)
File descriptors that are too far out of range will cause the
read builtin to crash. The following example will generate
two crashes:

$ ksh -c 'read -u 2000000' || ksh -c 'read -u-2000000'

The fix is to error out when the given file descriptor is out
of range. This bugfix is from Tomas Klacko, although it was
modified to use 'sh_iovalidfd' and reject numbers greater
than 'INT_MAX':
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01912.html
The question about 'shp->fdstatus[-1]' only applies to ksh93v-
(ksh93u+ doesn't have any references to 'shp->fdstatus[-1]').

src/cmd/ksh93/bltins/read.c:
- File descriptors that are out of range should be rejected
  with an error message (like invalid file descriptors that
  are in range). The seemingly redundant check for negative
  numbers is there because out of range negative numbers also
  cause memory faults despite the later 'fd<0' check.

src/cmd/ksh93/tests/io.sh:
- Add three tests for attempting 'read -u' on various invalid
  file descriptor numbers.
2020-07-01 18:14:10 +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
bb4745e897
Fix incorrect behavior of 'cd ../.foo' (#46)
The cd builtin was removing '.' from directory names when combined
with a preceding '../', which caused commands like 'cd ../.local'
to become 'cd ../local'. This patch fixes the problem by limiting
the extra handling to leading '..'. The bugfix comes from ksh93v-
2013-10-10-alpha, although this version is a shortened patch from
Solaris (as ksh93v- refactored a decent amount of the code for the
cd builtin).

src/cmd/ksh93/bltins/cd_pwd.c:
- cd should only check for leading '..', as trying to handle a lone
  '.' only causes problems.

src/cmd/ksh93/tests/builtins.sh:
- Add a regression test for this problem based on the test present in
  ksh93v- 2013-10-10-alpha.

Patch from Solaris:
860d27f/components/ksh93/patches/270-23319761.patch
2020-06-26 23:36:29 +01:00
Johnothan King
4cecde1dd3 Fix buggy completion of ~/some in vi mode (#41)
This commit fixes the bug reported in:
https://github.com/att/ast/issues/682
The following sequence fails in vi mode because ksh looks in the
wrong part of the 'virtual' buffer:

$ touch ~/testfile
$ ls ~/test<tab>

The fix is to change 'virtual[i]' to 'virtual[last_virt]' in the
bugged section of code. The other changes are to make sure listing
files in a directory with something like 'ls /etc/<tab>' calls the
code for Ctrl+L to preserve 'ls /etc/' rather than try (and fail)
to complete the directory name, producing 'ls /etc\n/'. This bugfix
was backported from ksh93v- 2013-10-10-alpha.

src/cmd/ksh93/edit/vi.c
 - Backport the bugfix from ksh93v- 2013-10-10-alpha for this
   problem.

src/cmd/ksh93/tests/pty.sh
 - Add a regression test for this issue using pty, adjusted slightly
   for a fake home directory in /tmp.
2020-06-25 23:13:45 +02: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
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
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
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
764acefaf1 read -r -d should not ignore -r
This bug was previously reported in att/ast#37.
Ksh ignores `-r` when `read -r -d` is run because when
the bit for `D_FLAG` is set, the bit for `R_FLAG` is unset
as a side effect of setting `D_FLAG`. The following set
of commands fails to print a backslash:

$ printf '\\\000' | read -r -d ''
$ echo $REPLY

The fix for this bug is to set `D_FLAG` with `D_FLAG + 1`,
which prevents `R_FLAG` from being unset. This bugfix
has been backported from ksh93v- 2013-10-10-alpha.

src/cmd/ksh93/bltins/read.c:
 - Set `D_FLAG` with `D_FLAG + 1` to prevent the bit for
   `R_FLAG` from being unset.

src/cmd/ksh93/tests/builtins.sh:
 - Add the regression test for `read -r -d` from ksh93v-.
2020-06-16 13:49:23 +01: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
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
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
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
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
65d363fd34 POSIX compliance fix: make 'times' a proper builtin
As of this commit, the 'times' command is a POSIX-compliant special
builtin command instead of an alias that doesn't produce the
required output. It displays the accumulated user and system CPU
times, one line with the times used by the shell and another with
those used by all of the shell's child processes.
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_27

This was originally written by Kurtis Rader and is now backported
and tweaked from the abandoned ksh2020 branch. I chose an earlier
and simpler version[*1] that uses times(3), with a precision of
hundredths of seconds, so it outputs the same precision as mksh and
zsh. Rader later wrote another version[*2] that uses getrusage(2),
giving it the same millisecond precision as bash. But that required
adding a feature test and a fallback to the old version, which is
non-trivial in the old INIT/iffe system. This simpler version is
enough to gain POSIX compliance and I think it will do very nicely
in this stable bugfix branch.

[*1] https://github.com/att/ast/pull/1332
[*2] 038045f6

src/cmd/ksh93/bltins/misc.c
- Add b_times() function for 'times' builtin.
- Note we include <times.h>, not <sys/times.h>, so that we use the
  AST feature-tested version with fallback on systems that need it.

src/cmd/ksh93/data/aliases.c:
- Remove times='{ { time;} 2>&1;}' builtin alias.

src/cmd/ksh93/data/builtins.c,
src/cmd/ksh93/include/builtins.h:
- Add entry for 'times' special builtin.
- Add --help/--man info for same.

src/cmd/ksh93/sh.1:
- Update manual page.

src/cmd/ksh93/tests/builtins.sh:
- Add a couple of simple regression tests.

(cherry picked from commit ebf71e619eb298ec1cf6b81d1828fa7cdf6e9203)
2020-06-12 01:45:16 +02:00
Martijn Dekker
6f0e008cf7 Fix unsetting special vars in subshells (re: efa31503, 8b7f8f9b)
This fixes (or at least works around) a bug that caused special
variables such as PATH, LANG, LC_ALL, LINENO, etc. to lose their
effect after being unset in a subshell.

For example:
(unset PATH; PATH=/dev/null; ls); : wrongly ran 'ls'
(unset LC_ALL; LC_ALL=badlocale); : failed to print a diagnostic

This is yet another problem with non-forking/virtual subshells. If
you forced the subshell to fork (one way of doing this is using the
'ulimit' builtin, e.g. ulimit -t unlimited) before unsetting the
special variable, the problem vanished.

I've tried to localise the problem. I suspect the sh_assignok()
function, which is called from unall(), is to blame. This function
is supposed to make a copy of a variable node in the virtual
subshell's variable tree. Apparently, it fails to copy the
associated permanent discipline function settings (stored in the
np->nvfun->disc pointer) that gave these variables their special
effect, and which survive unset. However, my attempts to fix that
have been unsuccessful. If anyone can figure out a fix, please send
a patch/pull request!
Data point: This bug existed in 93u 2011-02-08, but did not yet
exist in M-1993-12-28-s+. So it is a regression.

Meanwhile, pending a proper fix, this commit adds a safe
workaround: it forces a non-forked subshell to fork before
unsetting such a special variable.

src/cmd/ksh93/bltins/typeset.c: unall():
- If we're in a non-forked, non-${ ...; } subshell, then before
  unsetting any variable, check for variables with internal
  trap/discipline functions, and call sh_subfork() if any are
  found. To avoid crashing, this must be done before calling
  sh_pushcontext(), so we need to loop through the args separately.

src/cmd/ksh93/tests/variables.sh:
- Remove the 'ulimit' that forced the fork; we do this in C now.

(cherry picked from commit 21b1a67156582e3cbd36936f4af908bb45211a4b)
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:
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
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
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
cafe33f048 Fix 'test -t 1' in $(command substitutions)
Standard output (FD 1) tested as being on a terminal within a
command substitution, which makes no sense as the command
substitution is supposed to be catching standard output.
    ksh -c 'v=$(echo begincomsub
                [ -t 1 ] && echo oops
                echo endcomsub)
            echo "$v"'
This should not output "oops".

This is one of the many bugs with ksh93 virtual (non-forked)
subshells. On the abandoned Vashist/Rader ksh2020 branch, this bug
was fixed by changing quite a lot of code, which introduced and/or
exposed another bug:
	https://github.com/att/ast/issues/1079
	8e1e405e
	https://github.com/att/ast/issues/1088
That issue was unresolved when the ksh2020 branch was abandoned.

The safer and more conservative fix is simply forcing the subshell
to fork if we're in a non-forked command substitution and testing
'-t 1'. It is hard to imagine a situation where this would cause a
noticable performance hit.

Note that this fix does not affect ksh93-specific "shared"
non-subshell ${ command substitutions; } which are executed in the
main shell environment, so that variables survive, etcetera.
'test -t 1' continues to wrongly return true there, but command
substitutions of that form cannot be forked because that would
defeat their purpose.

src/cmd/ksh93/bltins/test.c:
- Fix 'test -t 1', '[ -t 1 ]' and '[[ -t 1 ]]' by forking the
  current subshell if it is a virtual/non-forked subshell
  (shp->subshell), and a command substitution (shp->comsub), but
  NOT a "shared" ${ command substitution; } (!shp->subshare).

src/cmd/ksh93/tests/bracket.sh:
- Add two regression tests for this issue, which were adapted from
  the Vashist/Rader ksh2020 branch.

NEWS, src/cmd/ksh93/include/version.h:
- Update.

(cherry picked from commit b8ef05e457ead65b83417699b8dd8632f855e2fa)
2020-06-12 01:45:13 +02:00
Martijn Dekker
846ad93272 Fix 'print -s -f'
This fix is backported from the Vashisht/Rader ksh2020 branch.

src/cmd/ksh93/bltins/print.c:
- Fix syncing history when print -s -f is used. For example, the
  following now correctly adds a 'cd' command to the history:
	print -s -f 'cd -- %q\n' "$PWD"
  Ref.:	https://github.com/att/ast/issues/425
	https://github.com/att/ast/pull/442

src/cmd/ksh93/include/version.h:
- Version date bump.

(cherry picked from commit 46ba7ecdc5c14cd73f6cb24b16c50bdc331a000e)
2020-06-12 01:45:13 +02:00