1
0
Fork 0
mirror of git://git.code.sf.net/p/cdesktopenv/code synced 2025-02-24 15:04:13 +00:00
Commit graph

186 commits

Author SHA1 Message Date
Martijn Dekker
6f50ff6497 disable 'vmstate' builtin when using system's malloc(3)
Related discussion:
https://github.com/ksh93/ksh/issues/95#issuecomment-664010969

src/cmd/ksh93/tests/leaks.sh:
- When ksh is compiled to use the system's malloc(3) instead of AST
  vmalloc(3), the vmstate builtin returns either nothing or zero.
  Detect this as a regression test failure and refuse to run tests.
- Tweak iterations. Tests don't need 500 or 1000 runs for vmstate.

src/cmd/ksh93/data/builtins.c:
- Do not compile in vmstate builtin when using system's malloc(3).
2020-07-26 20:39:22 +01:00
Martijn Dekker
a2f13c19f2 Fix typeset attributes -a, -A, -l, -u leaking out of subshells
If an array or upper/lowercase variable was declared with a null
initial value within a virtual/non-forked subshell, like:
	( typeset -a foo; ... )
	( typeset -A foo; ... )
	( typeset -l foo; ... )
	( typeset -u foo; ... )
then the type declaration leaked out of the subshell into the
parent shell environment, though without any values that may
subsequently have been assigned.

src/cmd/ksh93/bltins/typeset.c: setall():
- When deciding whether to create a virtual subshell scope for a
  variable, use sh_assignok(), which was actually designed for the
  purpose, instead of _nv_unset(). This allows getting rid of a
  tangled mess of special-casing that never worked quite right.

src/cmd/ksh93/tests/arrays.sh:
- Add regression tests checking that array declarations don't leak
  out of virtual subshells.

src/cmd/ksh93/tests/attributes.sh:
- Add regression tests for combining the 'export' and 'readonly'
  attributes with every other possible typeset attribute on unset
  variables. This also includes a subshell leak test for each one.

Fixes: https://github.com/ksh93/ksh/issues/88
2020-07-26 02:41:12 +01:00
Johnothan King
1bc2c74c74
Fix how unrecognized options are handled in 'sleep' and 'suspend' (#93)
When a builtin is given an unrecognized option, the usage information
for that builtin should be shown as 'Usage: builtin-name options'. The
sleep and suspend builtins were an exception to this. 'suspend' would
not show usage information and sleep wouldn't exit on error:

$ suspend -e
/usr/bin/ksh: suspend: -e: unknown option
$ time sleep -e 1
sleep: -e: unknown option

real	0m1.00s
user	0m0.00s
sys	0m0.00s

src/cmd/ksh93/bltins/sleep.c:
- Show usage information and exit when sleep is given an unknown
  option. This bugfix was backported from ksh2020: https://github.com/att/ast/pull/1024

src/cmd/ksh93/bltins/trap.c:
- Use the normal method of parsing options with optget to fix the
  suspend builtin's test failure.

src/cmd/ksh93/tests/builtins.sh:
- Add the ksh2020 regression test for getting the usage information
  of each builtin. Enable all /opt/ast/bin builtins in a subshell
  since those should be tested as well (aside from getconf and uname
  because those builtins fallback to the real commands on error).
2020-07-26 02:18:49 +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:
https://bugs.launchpad.net/ubuntu/+source/ksh/+bug/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
db72f41f4b Fix subshell file descriptor leak
A file descriptor (at least 3, can't reproduce for 4 and up) opened
with 'exec' or 'redirect' in a virtual/non-forked subshell survived
that subshell after exiting it:

    $ ksh -c '(redirect 3>&1); echo bug >&3'
    bug

src/cmd/ksh93/sh/io.c:
- Apply a patch from OpenSUSE (ksh93-redirectleak.dif). Source:
  https://build.opensuse.org/package/show/openSUSE:Leap:42.3:Update/ksh

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

Thanks to Marc Wilson for flagging this up.
2020-07-21 04:12:40 +01:00
Martijn Dekker
0c96f9749b tests/subshell.sh: fix a test for systems without /dev/fd/*
ksh's built-in test, [ and [[ commands treat /dev/fd/* specially:
e.g. 'test /dev/fd/0' returns true even if it doesn't physically
exist, as on e.g. HP-UX. However, external commands need it to
exist physically.

src/cmd/ksh93/tests/subshell.sh:
- To decide whether to run a test with 'tee', use external 'test'
  command to check if /dev/stdout and /dev/fd/1 actually exist.
2020-07-21 01:12:15 +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
Martijn Dekker
01c25cb14b whence -a: fix spurious 'undefined function' message
$ ksh -c 'whence -a printf'
	printf is a shell builtin
	printf is /usr/bin/printf
	printf is an undefined function

The third line should not appear.

src/cmd/ksh93/bltins/whence.c:
- Remove faulty extra check for undefined (= autoload) functions.
  This was already handled earlier, on lines 192-193.

src/cmd/ksh93/tests/builtins.sh:
- Add regression test.
- For previous 'whence -a' test, don't bother with shell function.

Fixes https://github.com/ksh93/ksh/issues/26
2020-07-20 17:03:04 +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
2db9953ae0
Fix three bugs in the sleep builtin (#77)
This commit backports the main changes to sh_delay from ksh93v-
and ksh2020, which fixes the following bugs:

- Microsecond amounts of less than one millisecond are no longer
  ignored. The following loop will now take a minimum of one
  second to complete:
  for ((i = 0; i != 10000; i++)) do
    sleep PT100U
  done

- 'sleep 30' no longer adds an extra 30 milliseconds to the total
  amount of time to sleep. This bug is hard to notice since 30
  milliseconds can be considered within the margin of error. The
  only reason why longer delays weren't affected is because the old
  code masked the bug when the interval is greater than 30 seconds:
  else if(n > 30)
  {
      sleep(n);
      t -= n;
  }
  This caused 'sleep -s' to break with intervals greater than 30
  seconds, so an actual fix is used instead of a workaround.

- 'sleep -s' now functions correctly with intervals of more than
  30 seconds as the new code doesn't need the old workaround. This
  is done by handling '-s' in sh_delay.

src/cmd/ksh93/bltins/sleep.c:
- Remove the replacement for sleep(3) from the sleep builtin.
- Replace the old sh_delay function with the newer one from ksh2020.
  The new function uses tvsleep, which uses nanosleep(3) internally.

src/cmd/ksh93/include/shell.h,
src/cmd/ksh93/edit/edit.c,
src/cmd/ksh93/sh/jobs.c,
src/cmd/ksh93/sh/xec.c,
src/cmd/ksh93/shell.3:
- Update sh_delay documentation and usage since the function now
  requires two arguments.

src/cmd/ksh93/tests/builtins.sh:
- Add a regression test for 'sleep -s' when the interval is greater
  than 30 seconds. The other bugs can't be tested for in a feasible
  manner across all systems:
  https://github.com/ksh93/ksh/pull/72#issuecomment-657215616
2020-07-17 05:00:28 +01:00
Johnothan King
ea5b25b93a
Fix some formatting errors, typos and other problems (#78)
Some notes:
- Removed a TODO note that was fixed in commit 43d9fbac.
- Removed a duplicate note about the '%l' time format in the changelog.
- Applied the following documentation fixes from Terrence J. Doyle:
  - https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01852.html
  - https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01856.html
- Fixed strange grammar in one of the error messages.
- Added missing options for rksh to the synopsis section.
- Applied a formatting fix from ksh93v- to the man page.
- Replaced a C99 line comment in src/lib/libast/comp/realpath.c with a
  proper comment that is valid in C89.
- Prioritize UTC over GMT in the documentation (missed by commit c9634e90).
- Add some extra information for 'ksh -R file' to the man page. This patch
  is from Red Hat: https://git.centos.org/rpms/ksh/blob/c8/f/SOURCES/ksh-20080202-manfix.patch
2020-07-16 22:27:00 +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
17f81ebedb Load 'r' and 'history' default aliases on interactive only
These two default aliases are useful on interactive shells. In
scripts, they interfere with possible function or command names.

As of this commit, these final two default aliases are only loaded
for interactive shells, leaving zero default aliases for scripts.
This completes the project to get rid of misguided default aliases.

src/cmd/ksh93/include/shtable.h,
src/cmd/ksh93/data/aliases.c:
src/cmd/ksh93/sh/init.c:
- Add empty alias table shtab_noaliases[] for scripts.
- Rename inittree() to sh_inittree() and make it external.
- nv_init(), sh_reinit(): Initialise empty alias tree for scripts.

src/cmd/ksh93/sh/main.c: sh_main():
- If interactive, reinitialise alias tree for interactive shells.

src/cmd/ksh93/tests/alias.sh:
- To test default alias removal, launch shell with -i.
2020-07-16 06:44:05 +01:00
Martijn Dekker
a42ac7e77a Fix annoying usage/--help/--man message corruption
In a locale other than C/POSIX, ksh produces corrupted usage
messages for alternatives, e.g. this output of 'typeset -\?':
| Usage: typeset [-bflmnprstuxACHS] [-a[type]] [-i[base]] <..CUT..>
|                [-T[tname]] [-Z[n]] [name[=value]...]
|    Or:[name[=value]...]
| typeset[name[=value]...]
| [[name[=value]...]
| options[name[=value]...]
| ] -f [name...]

Correct output is:
| Usage: typeset [-bflmnprstuxACHS] [-a[type]] [-i[base]] <..CUT..>
|                [-T[tname]] [-Z[n]] [name[=value]...]
|    Or: typeset [ options ] -f [name...]

Similar corruption occurs in --help and --man output.
This bug is ancient: it's already in ksh 1993-12-28 s+.

ksh2020 has this fixed. A 'git bisect' run pinpointed the fix
to this commit, which fixes the ERROR_translating macro after
removing the AST-specific locale subsystem:
https://github.com/att/ast/commit/4abc061e
But making the same change in ksh 93u+m produced no results
(probably because we have not removed that subsystem).

However, disabling the use of translation macros in optget.sh
altogether (replacing them with dummies that were already coded in
a preprocessor directive fallback for a reduced standalone libast)
turns out to work. It's not as if there is actually any translation
anyway, so this effectively fixes this bug.

The actual cause of this bug remains mysterious, but should be
somewhere in the AST translation and/or locale subsystem.

src/lib/libast/misc/optget.c:
- Use fallback translation macros.

src/cmd/ksh93/tests/builtins.sh:
- Add regression tests for output of -?, --?x, --help and --man
  for a usage string with an alternative ("Or:") usage message.
  Before the fix, these failed when running the tests in the
  C.UTF-8 locale (as in 'bin/shtests -u builtins').
2020-07-16 05:13:53 +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
Martijn Dekker
45cfecfc1e tests/basic.sh: fix tests to work with xtrace (re: c5820aab) 2020-07-15 05:02:29 +01:00
Martijn Dekker
b1a4131123 Millisecond precision for 'times' builtin (re: 65d363fd, 5c677a4c)
Now that we have an iffe feature test for getrusage(3), introduced
in 70fc1da7, the millisecond-precision 'times' command from the
last version of ksh2020 can easily be backported.

src/cmd/ksh93/bltins/misc.c:
- Incorporate ksh2020 'times' command, with a couple of tweaks:
  * Use locale's radix point instead of '.'.
  * Pad seconds with initial zero if < 10.

src/cmd/ksh93/data/builtins.c:
- Update version date for 'times --man'.

src/cmd/ksh93/tests/builtins.sh:
- Update 'times' test for 3 digits after radix point.
2020-07-15 04:22:45 +01:00
Martijn Dekker
c5820aabc9 Fix $TIMEFORMAT zero-decimal and error behaviour (re: 70fc1da7)
The backported 'time' keyword code introduced a bug (shared by
ksh2020): the $TIMEFORMAT format sequences %0R, %0U and %0S output
a decimal fraction, acting as %1R, %1U and %1S.

A minor ksh2020 behaviour change that was also backported was that
the $TIMEFORMAT formatting no longer errored out on encountering an
invalid identifier, but continued. That behaviour is now reverted.

Neither of these two regressions occurred on older systems that
have to use times(3) instead of getrusage(2) or gettimeofday(2).

This commit also tweaks a regression test so that it doesn't fail
if the old times(3) interface is used.

src/cmd/ksh93/sh/xec.c: p_time():
- (Fix indentation of a for loop.)
- On modern systems, when outputting the result of $TIMEFORMAT
  format sequences, only print fraction if precision is nonzero.
- On modern systems, when encountering an invalid format sequence,
  abort formatting in the same way as done for old systems.
- On old systems, initialise 'n' in a more readable way when used
  as the index for tm[].

src/cmd/ksh93/tests/basic.sh:
- Don't fail, but issue warning on old systems that use times(3).
  Otherwise, check milliseconds: with the ksh 'sleep' builtin,
  'TIMEFORMAT=%3R; time sleep .002' should always output '0.002'.
- Change regression test for TIMEFORMAT='%0S%' to check for the
  correct output, '0%', instead of checking for an error message.
2020-07-15 02:43:35 +01:00
Johnothan King
70fc1da73e
Fix the max precision of the 'time' keyword (#72)
This commit backports the required fixes from ksh2020 for using
millisecond precision with the 'time' keyword. The bugfix refactors
a decent amount of code to rely on the BSD 'timeradd' and
'timersub' macros for calculating the total amount of time elapsed
(as these aren't standard, they are selectively implemented in an
iffe feature test for platforms without them). getrusage(3) is now
preferred since it usually has higher precision than times(3) (the
latter is used as a fallback).

There are three other fixes as well:

src/lib/libast/features/time:
- Test for getrusage with an iffe feature test rather than
  assume _sys_times == _lib_getrusage.

src/cmd/ksh93/sh/xec.c:
- A single percent at the end of a format specifier is now
  treated as a literal '%' (like in Bash).
- Zero-pad seconds if seconds < 10. This was already done for
  the times builtin in commit 5c677a4c, although it wasn't
  applied to the time keyword.
- Backport the ksh2020 bugfix for the time keyword by using
  timeradd and timersub with gettimeofday (which is used with
  a timeofday macro). Prefer getrusage when it is available.
- Allow compiling without the 'timeofday' ifdef for better
  portability.
  This is the order of priority for getting the elapsed time:
  1) getrusage (most precise)
  2) times + gettimeofday (best fallback)
  3) only times (doesn't support millisecond precision)
  This was tested by using debug '#undef' statements in xec.c.

src/cmd/ksh93/features/time:
- Implement feature tests for the 'timeradd' and 'timersub'
  macros.
- Do a feature test for getrusage like in the libast time test.

src/cmd/ksh93/tests/basic.sh:
- Add test for millisecond precision.
- Add test for handling of '%' at the end of a format specifier.
- Add test for locale-specific radix point.
2020-07-14 22:48:04 +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
Martijn Dekker
39692fc3f6 tests/pty.sh: a couple of minor tweaks
src/cmd/ksh93/tests/pty.sh:
- init: Remove superfluous lineno=$LINENO assignments. They aren't
  needed if we avoid alias expansion on the err_exit function call.
- In the test "vi mode file name completion", append the main
  shell's PID to /tmp/fakehome to make a slightly less insecure
  temporary directory name. Unfortunately we cannot use $tmp as
  that uses $TMPDIR which may cause a false pass. (re: 4cecde1d)
2020-07-13 21:02:04 +01:00
Martijn Dekker
8ad56f90ab Add FreeBSD stty workaround for pty regression tests
Apparently, on FreeBSD, the stty command does not work correctly
for setting 'erase' or 'kill' on a pty pseudoterminal. I've no
idea whether the bug is in FreeBSD stty or in AST pty, but in any
case, a workaround is needed for the time being.

src/cmd/ksh93/tests/pty.sh:
- Save terminal state on init; set a trap to restore it on exit.
- Issue 'stty erase ^H kill ^X' on the real terminal before
  entering pty pseudoterminals.

Resolves #44.
2020-07-13 21:28:21 +02:00
Martijn Dekker
84e2f6d92f tests/leaks.sh: workaround minor variation when run with shcomp
For unknown reasons, the test for a memory leak in 'read -C stat
<<< "$data"' can show an intermittent minor variation in memory
usage when run with shcomp on certain versions of macOS.

The reported variations are 48 bytes or 80 bytes. This is too small
to be the result of an actual memory leak in the tested command;
it is repeated 500 times so that any real leak should show a
difference of at least 500 bytes.

src/cmd/ksh93/tests/leaks.sh:
- Add a tolerance of 128 bytes to get rid of the false failure.

Fixes #70 (hopefully).
2020-07-10 23:01:22 +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
f9d28935bb Fix UTF-8 shellquoting for xtrace, printf %q, etc.
This fixes an annoying issue in the shell's quoting algorithm
(used for xtrace (set -x), printf %q, and other things) for UTF-8
locales, that caused it to encode perfectly printable UTF-8
characters unnecessarily and inconsistently. For example:

$ (set -x; : 'aeu aéu')
+ : $'aeu a\u[e9]u'
$ (set -x; : 'aéu aeu')
+ : 'aéu aeu'
$ (set -x; : '正常終了 aeu')
+ : '正常終了 aeu'
$ (set -x; : 'aeu 正常終了')
+ : $'aeu \u[6b63]\u[5e38]\u[7d42]\u[4e86]'

This issue was originally reported by lijo george in May 2017:
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01958.html

src/cmd/ksh93/sh/string.c:
- Add is_invisible() function that returns true if a character is a
  Unicode invisible (non-graph) character, excluding ASCII space.
  Ref.: https://unicode.org/charts/PDF/U2000.pdf
- Use a fallback in is_invisible() if we cannot use the system's
  iswprint(3); this is the case for the ksh C.UTF-8 locale if the
  OS doesn't support that. Fall back to a hardcoded blacklist of
  invisible and control characters and put up with not encoding
  nonexistent characters into \u[xxxx] escapes.
  Ref.: https://unicode.org/charts/PDF/U2000.pdf
- When deciding whether to switch to $'...' quoting mode (state=2),
  use is_invisible() instead of testing for ASCII 0-127 range.
- In $'...' quoting mode, use is_invisible() to decide whether to
  encode wide characters into \u[xxxx] escapes.

src/cmd/ksh93/tests/builtins.sh:
- Add regression tests for shellquoting Arabic, Japanese and Latin
  UTF-8 characters, to be run only in a UTF-8 locale. The Arabic
  sample text[*] contains a couple of direction markers that are
  expected to be encoded into \u[xxxx] escapes.

[*] source: https://r12a.github.io/scripts/tutorial/summaries/arabic
2020-07-10 05:55:11 +01:00
Johnothan King
6930666234
Fix a syntax error when ((...)) is combined with redirections (#68)
This bugfix was backported from ksh93v- 2013-10-10-alpha.

src/cmd/ksh93/sh/parse: item():
- The done label is placed after the 'inout' call for handling I/O
  redirections. This causes the command below to produce a syntax
  error because the '>' is not handled as a redirection operator
  after 'goto done':
  $ ((1+2)) > /dev/null
  /usr/bin/ksh: syntax error: `>' unexpected
  Moving the done label fixes the syntax error as 'inout' is now
  called to handle the redirection operator.

src/cmd/ksh93/tests/arith.sh:
- Add a simple regression test.
2020-07-09 22:12:04 +01:00
Martijn Dekker
361fe1fcc3 Fix hash table memory leak when restoring PATH
There is a bug in path_alias() that may cause a memory leak when
clearing the hash table while setting/restoring PATH.

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

Note that, contrary to Siteshwar's analysis linked above, this bug
has nothing directly to do with subshells, forked or otherwise; it
can also be reproduced by temporarily setting PATH for a command,
for example, 'PATH=/dev/null true', and then doing a PATH search.

Modified analysis:
ksh maintains the value of PATH as a linked list. When a local
scope for PATH is created (e.g. in a virtual subshell or when doing
something like PATH=/foo/bar command ...), ksh duplicates PATH by
increasing the refcount for every element in the linked list by
calling the path_dup() and path_alias() functions. However, when
the state of PATH is restored, this refcount is not decreased. Next
time when PATH is reset to a new value, ksh calls the path_delete()
function to delete the linked list that stored the older path. But
the path_delete() function does not free elements whose refcount is
greater than 1, causing a memory leak.

src/cmd/ksh93/sh/path.c: path_alias():
- Decrease refcount and free old item if needed.
  (The 'old' variable was already introduced in 99065353, but
  its value was never used there; this fixes that as well.)

src/cmd/ksh93/tests/leaks.sh:
- Add regression test. With the bug, setting/restoring PATH
  (which clears the hash table) and doing a PATH search 16 times
  causes about 1.5 KiB of memory to be leaked.
2020-07-09 18:34:15 +01:00
Martijn Dekker
5e7d335f2f Fix crash when listing indexed arrays with 'typeset -a'
There is a bug in print_scan() function that may cause ksh to crash
while listing indexed arrays. The crash happens in nv_search() when
called from print_scan().

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

src/cmd/ksh93/bltins/typeset.c:
- Call nv_scan() without the NV_IARRAY flag, even for a null scan.

src/cmd/ksh93/tests/arrays.sh:
- Add regression test for 'typeset -a' crash and check output.
2020-07-09 16:42:16 +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
e70925ce10
Fix memory leak on unset of associative array (#64)
Associative arrays weren't being properly freed from memory, which
was causing a memory leak.

This commit incorporates a patch and reproducer/regress test from:
https://www.mail-archive.com/ast-users@lists.research.att.com/msg01016.html

src/cmd/ksh93/sh/name.c:
- Properly free associative arrays from memory in nv_delete().

src/cmd/ksh93/tests/leaks.sh:
- Add regression test.
2020-07-09 01:09:40 +01:00
Martijn Dekker
bf79131f40 Use vmstate for memory leak regress tests (re: ad9a9219)
'ps' does not always give reliable results; on macOS, 'ps' appears
to produce nondeterministic (i.e. randomly varying) results for
'vsz' and 'rss', making it unusable for memory leak tests. See:
https://github.com/ksh93/ksh/pull/64#issuecomment-655094931
and further comments.

So let's compile in the vmstate builtin so that we can make sure to
measure things properly. It also reports bytes instead of 1024-byte
blocks, so smaller leaks can be detected.

To be decided: whether or not to disable the vmstate builtin for
release builds in order to save about 12K in the ksh binary.

src/cmd/ksh93/data/builtins.c:
- Add vmstate to the list of builtins that are compiled in.

src/cmd/ksh93/tests/leaks.sh:
- getmem(): get size using: vmstate --format='%(busy_size)u'
  (Using busy_size instead of size seems to make more sense as it
  excludes freed blocks. See vmstate --man)
- Introduce a $unit variable for reporting leaks and set it to
  'bytes'; this makes it easier to change the unit in future.
- Since the tests are now more sensitive, initialise all variables
  before use to avoid false leak detections.
- The last test seemed to need a few more 'read' invocations in
  order to get memory usage to a steady state before the test.
2020-07-08 23:23:19 +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
Martijn Dekker
1ca9286ab8 tests/pty.sh: disable 'process/terminal group exercise' for now
Apparently, pty doens't handle SIGTSTP correctly:
https://github.com/att/ast/issues/375
https://github.com/ksh93/ksh/issues/45#issuecomment-653789092

src/cmd/ksh93/tests/pty.sh:
- Disable the 'process/terminal group exercise' regression test,
  which depends on correct SIGTSTP handling, until pty can be
  fixed.

Closes #45.
2020-07-04 23:31:40 +02:00
Martijn Dekker
b2382efd8b tests/shtests: rm debug command that snuck through (re: d7afb57c) 2020-07-04 17:40:07 +02:00
Johnothan King
658bba748e
Fix 'kill -INFO' on systems that support SIGINFO (#59)
src/cmd/ksh93/data/signals.c:
- SIGINFO was absent from the table of signals, which caused
  commands like 'kill -INFO $$' to fail even on platforms with
  SIGINFO (such as macOS and FreeBSD). Fix that by adding
  it to the signal table.

src/cmd/ksh93/tests/signal.sh:
- Add a regression tests for using SIGINFO with the kill builtin.
  The test will only be run if the external kill command supports
  SIGINFO.
2020-07-04 15:57:47 +01:00
Martijn Dekker
d7afb57c49 shtests: use central temporary directory; add --keep option
This gets rid of repetitive code in test scripts to create their
own temporary directories. Instead, shtests exports a $tmp to each
test script that is a subdirectory of its own temporary directory.
This has the advantage of having all test script temporary
directories in one hierarchy. Along with a new option to keep
temporary files, this makes it easy to inspect them if wanted.

This does make the test scripts less self-contained as they now
depend on a temporary directory being exported as $tmp. But they
already depended on $SHELL being the shell to test, so they already
were not quite self-contained.

src/cmd/ksh93/tests/shtests:
- Add -k/--keep option to keep temporary directory. Make the EXIT
  trap report its location instead of deleting it.
- For each test, create a subdirectory of $tmp (named after the
  test script plus the tested locale or 'shcomp') and export that
  subdirectory to the test script as its own $tmp.
- If -k is not given, delete each script's temporary files
  immediately after running it to minimise disk usage.

src/cmd/ksh93/tests/*.sh:
- Don't make own temp directory.
- Refuse to run if $tmp is not set.
- Miscellaneous tweaks.
2020-07-04 01:28:08 +02:00
Martijn Dekker
fa70fc3f77 tests/pty.sh: misc tweaks
src/cmd/ksh93/tests/pty.sh:
- Fix race condition in the test "raw Bourne mode literal tab
  characters with wide characters enabled" by adding 'd 10' to add
  a 10-millisecond delay before every write. Thanks to @JohnoKing:
  https://github.com/ksh93/ksh/pull/57#issuecomment-653617531
- Fix locale for test "raw Bourne mode backslash handling" (should
  be UTF-8, not UTF8) (re: a0dcdeea).
- Add a few more dummy # err_exit # comments to allow shtests to
  count the number of tests.
2020-07-03 21:59:58 +02:00
Johnothan King
a0dcdeeade Fix bugs with backslash escaping in interactive vi mode (#57)
This commit fixes the following bugs in the 'vi' editing mode
backslash escape feature. Ref.: Bolsky & Korn (1995), p. 113, which
states for \: "Similar to Control+V [...] except that it escapes
only the next Erase or Kill charactrer".

1. The vi mode now only escapes the next character if the last
   character input was a backslash, fixing the bug demonstrated at:
   https://asciinema.org/a/E3Rq3et07MMQG5BaF7vkXQTg0
2. Escaping backslashes are now disabled in vi.c if the vi mode is
   disabled (note that vi.c handles raw editing mode in UTF-8
   locales). This makes the behavior of the raw editing mode
   consistent in C/POSIX and UTF-8 locales.
3. An odd interaction with Backspace when the character prior to a
   separate buffer entered with Shift-C was a backslash has been
   fixed. Demonstration at: https://asciinema.org/a/314833
   ^? will no longer be output repeatedly when attempting to erase
   a separate buffer with a Backspace, although, to be consistent
   with vi(1), you still cannot backspace past it before escaping
   out of it. Ref.:
   https://github.com/ksh93/ksh/issues/56#issuecomment-653586994

src/cmd/ksh93/edit/vi.c:
- Prevent a backslash from escaping the next input if the previous
  input wasn't a backslash. This is done by unsetting a variable
  named backslash if a backslash escaped a character. backslash is
  set to the result of c == '\\' when the user enters a new
  character.
- Disable escaping backslashes in the raw editing mode because
  it should not be enabled there.

src/cmd/ksh93/tests/pty.sh:
- Add some tests for how ksh handles backslashes in each
  editing mode to test for the bugs fixed by this commit.

Fixes #56.
2020-07-03 21:15:21 +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: 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
ad9a9219f0
Fix memory leak regression tests by using ps(1) (#50)
src/cmd/ksh93/tests/leaks.sh:
- This script was never actually running the regression
  tests because 'vmstate' isn't available as a builtin.
  While this can be fixed by adding vmstate to the builtin
  table, that has the downside of increasing the binary
  size of ksh. This commit replaces all usage of 'vmstate'
  with 'ps' and 'awk' as a different way to measure
  memory usage. The memory leaks regression tests are now
  always run.
- Rename old $n to $N due to new $n interfering with the old
  regression test.
- Add before and after results for the number of 1024-byte
  blocks leaked in each test.

Co-authored-by: Martijn Dekker <martijn@inlv.org>
2020-07-01 20:00:58 +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
Martijn Dekker
fa51a5ce3b Fix 'whence -a' regression test (re: 99065353)
The regression test failed on systems where 'chmod' exists at more
than one location, e.g. Slackware where it's at both /bin/chmod and
/usr/bin/chmod.

src/cmd/ksh93/tests/builtins.sh: 'whence -a'/tracked aliases test:
- In the expected value, use modified 'whence -a -p chmod' output
  to get all of the paths to chmod.
- On failure, report both expected and actual values.
2020-06-30 01:43:25 +02:00
Johnothan King
1b5bc1802a
Fix the readonly builtin's scope in functions (#51)
* Fix the readonly builtin's scope in functions

This bug was first reported at https://github.com/att/ast/issues/881

'tdata.sh->prefix' is only set to the correct value when
'b_readonly' is called as 'export', which breaks 'readonly' in
functions because the correct scope isn't set. As a result, the
following example will only print a newline:

$ function show_bar { readonly foo=bar; echo $foo; }; show_bar

The fix is to move the required code out of the if statement for
'export', as it needs to be run for 'readonly' as well. This bugfix
is from https://github.com/att/ast/pull/906

src/cmd/ksh93/bltins/typeset.c:
- Set 'tdata.sh->prefix' to the correct value, otherwise 'readonly'
  uses the wrong scope.

src/cmd/ksh93/tests/builtins.sh:
- Add the regression test from ksh2020, modified to run in a
  subshell.

src/cmd/ksh93/data/builtins.c,
src/cmd/ksh93/sh.1:
- Add documentation of 'readonly' vs. 'typeset -r' difference:
  'readonly' does not create a function-local scope.

Co-authored-by: Martijn Dekker <martijn@inlv.org>
2020-06-29 19:09:20 +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
Martijn Dekker
c870be9fea regress: avoid interference from systemwide /etc/ksh.kshrc
ksh, even non-interactive, loads /etc/ksh.kshrc by default. On
some systems this can be a problem, e.g. OpenBSD, which installs a
default /etc/ksh.kshrc which is designed for its version of pdksh.

Quoth sh.1:

    On systems that support a system wide /etc/ksh.kshrc
    initialization file, if the filename generated by the expansion
    of ENV begins with /./ or ././ the system wide initialization
    file will not be executed.

src/cmd/ksh93/tests/shtests,
src/cmd/ksh93/tests/builtins.sh,
src/cmd/ksh93/tests/options.sh,
src/cmd/ksh93/tests/pty.sh:
- Instead of emptying or unsetting ENV, ensure it is exported with
  a default value of /./dev/null so we skip loading the system-wide
  profile and load an empty user profile.
- Where a specific ENV path was required for the tests, prefix it
  with '/.' so it starts with '/./'.
2020-06-27 00:52:17 +02: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:
https://github.com/oracle/solaris-userland/blob/860d27f/components/ksh93/patches/270-23319761.patch
2020-06-26 23:36:29 +01:00
Martijn Dekker
eaaa0de74d tests/builtins.sh: change GMT to UTC before testing (re: c9634e90)
Apparently some systems are still configured to use GMT instead of
UTC after all. This included our own GitHub CI runner config.
Oops. This made the previous commit fail to pass the CI test run.

We can't win this one, it's got to be either one or the other.
UTC is the international standard on which civil time is based.
GMT is often taken as synonymous for UTC, but in navigation,
it can differ from UTC by up to 0.9 seconds. Ref.:
https://en.wikipedia.org/w/index.php?title=Greenwich_Mean_Time&oldid=963422787
The more ambiguous term should not be the first preference.

src/cmd/ksh93/tests/builtins.sh:
- Before checking 'printf %T now' output against 'date' output,
  change any ' GMT ' in the latter to ' UTC '.

.github/workflows/ci.yml:
- Set time zone to UTC, not GMT.
2020-06-26 13:42:06 +02:00
Martijn Dekker
c9634e908d tmdata: prioritise "UTC" over "GMT"
"UTC" is the modern name for what used to be "GMT", but ksh still
preferred GMT. On systems configured to use the UTC time zone, this
caused a 'printf %T' regression test failure in tests/builtins.sh
as the external 'data' utility will prefer UTC these days.

src/lib/libast/tm/tmdata.c:
- Reorder the name alternatives for UTC/GMT so that UTC is
  the first preference.

src/cmd/ksh93/tests/builtins.sh:
- Report expected and actual values on 'printf %T' failure.

Related: #6
2020-06-26 13:25:40 +02:00
Martijn Dekker
8c705bf3b7 Fix behaviour of tabs in raw Bourne Shell-like editing mode
When neither '-o emacs' nor '-o vi' is active, there were a couple
of bugs with entering tab characters:
1. Tab completion was erroneously left active. The cause of this
   was that raw Bourne edit mode is handled by ed_viread() in vi.c
   on shells with wide character support, instead of the default
   ed_read() in edit.c, and the former failed to check if vi mode
   is active when processing tab characters.
2. When entering literal tab characters, the cursor was moved to
   the right only one character, instead of the amount of
   characters corresponding to the tab.

src/cmd/ksh93/edit/vi.c: getline():
- Before processing '\t' (tab) for command completion, check that
  the 'vi' shell option (SH_VI) is active.

src/cmd/ksh93/edit/edit.c: ed_virt_to_phys():
- When translating literal tabs to on-terminal spaces and when
  recalculating the cursor position, remove erroneous checks for
  SH_VI; this is also needed in raw Bourne mode. According to my
  own testing, this has no effect on emacs mode (knock on wood).

src/cmd/ksh93/tests/pty.sh:
- Add two regression tests. An odd race condition reveals itself in
  either pty or in ksh's raw/Bourne edit mode; see comment in test.
  Effect is we have to expect either literal tabs or tabs expanded
  to spaces, until that is tracked down and fixed.

Fixes #43.
2020-06-26 11:34:02 +02: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
d41ec674c7 Fix some errors in the documentation and other minor issues (#42)
Somewhat notable changes in this commit:
- The 'set +r' bugfix (re: 74b41621) is now documented in the
  changelog.
- Missing options have been added to the synopsis section of the
  ksh man page.
- The minor formatting fix from https://github.com/ksh-community/ksh/pull/5
  has been applied to the ksh man page.
- A few fixes from https://github.com/att/ast/commit/5e747cfb
  have been applied to the ksh man page.
- The man page fixes from https://github.com/att/ast/pull/353
  have been applied, being:
  - An addition to document the behavior of 'set -H'.
  - A fix for the cd section appending rksh93.
  - A fix for some options being indented too far.
  - Removal of a duplicate section documenting '-D'.
  - Reordering the options for 'set' in alphabetical order.
  - A minor fix for the documentation of 'ksh -i'.
2020-06-25 19:31:51 +02:00
Martijn Dekker
43d9fbac1f tests/bracket.sh: disable 'test -N' tests due to noatime mounts
src/cmd/ksh93/tests/bracket.sh:
- Disable tests for [[ -N ... ]] (test -N ...), because it is
  expected to break on systems where $TMPDIR (or even the entire
  root file system) is mounted with noatime for better performance.
  Ref.: https://opensource.com/article/20/6/linux-noatime
  (It also needs annoyingly long sleep times on older systems with
  a 1-second timestamp granularity.)
2020-06-25 14:28:37 +02:00
Martijn Dekker
2315f6687a Add regress test for fixed BUG_KUNSETIFS (re: 6f0e008c, 7b994b6a)
Modernish is no longer detecting BUG_KUNSETIFS, as I've just
discovered. Always nice when bugs spontaneously vanish...

A 'git reset HEAD~1'/recompile/retest loop reveals this bug was
fixed by 6f0e008c, as later modified by 7b994b6a.

So, let's make sure it stays fixed.

src/cmd/ksh93/tests/variables.sh:
- Add a couple of regression tests for BUG_KUNSETIFS presence,
  detection and known workaround, based on the same in modernish.
  Ref.: https://github.com/modernish/modernish/blob/3ddcbd13/lib/modernish/cap/BUG_KUNSETIFS.t
	https://github.com/modernish/modernish/blob/3ddcbd13/lib/modernish/tst/isset.t#L204-L222
2020-06-24 20:00:01 +02:00
Martijn Dekker
43c09c2d6f tests/pty.sh: disable 137(C) because it actually tests vi(1)
Testing the behaviour of an external editor, even the standard one,
is outside the scope of the ksh regression tests.

src/cmd/ksh93/tests/pty.sh:
- Disable a test that invoked vi(1) and that failed, either
  intermittently or consistently, on too many systems because
  whatever vi(1) is installed locally doesn't write the string
  "/tmp/" exactly as and/or when expected.
2020-06-24 16:40:28 +02:00
Martijn Dekker
5c677a4c6c Refactor the new 'times' builtin; zero-pad seconds (re: 65d363fd)
The output format is now identical to mksh's except for
the locale-dependent radix point ('.' or ',').

src/cmd/ksh93/bltins/misc.c:
- Output format tweak: pad seconds with initial zero if < 10.
- Use "too many operands" (e_toomanyops) error msg from 3ba4900e
  if there are operands, instead of "bad syntax" (e_badsyntax).
- Consolidate repetitive calculating and printing code
  into print_times().
- Get rid of some excessive variables.

src/cmd/ksh93/tests/builtins.sh:
- Update regression tests to match the above.

src/cmd/ksh93/data/builtins.c:
- Update sh_opttimes[] version string.
2020-06-24 14:32:20 +02:00
Martijn Dekker
d8fe061f4c shtests: count nonexistent tests as errors (re: c2eabc57)
When a nonexistent test script was given as an argument to
shtests, this was not counted as an error and shtests exited
successfully (with status 0).

src/cmd/ksh93/tests/shtests:
- Increase total error count if a test script is not found.
2020-06-24 00:53:59 +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
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
e0b326ae15
Fix a test failure for printf %T now on Linux (#38)
src/cmd/ksh93/tests/builtins.sh:
 - The output of 'printf %T now' and the external 'date'
   command aren't guaranteed to be the same unless $LC_ALL
   is set to 'C'. Set LC_ALL in these command substitutions
   to fix a spurious test failure on Linux.
2020-06-22 23:55:51 +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
Martijn Dekker
746ce73671 regress: don't count temp dir creation as test (re: 2318de32)
Note that shtests simply does a 'grep -c err_exit' and substracts 1
to count the number of regression tests in a test script. Not all
test scripts make temp dirs, so subtracting 2 instead won't do.

src/cmd/ksh93/tests/*.sh:
- Escape the err_exit call in the routine to create a temporary
  directory so that it is not counted as a regression test.
  That bypasses the alias, so we have to pass $LINENO manually.
2020-06-17 17:14:03 +02:00
Martijn Dekker
9ff692c2bb regress: count tests and report line numbers (re: 7b994b6a)
Four added tests did not correctly report their line numbers
upon failure and were counted as one, because the err_exit
alias/function pair was called from a shell function.

Note that shtests simply does a 'grep -c err_exit' to count the
number of regression tests in a test script.

src/cmd/ksh93/tests/subshell.sh:
- check_hash_table():
  - Take line number as 1st argument.
  - Quote a character in err_exit to bypass the alias when calling
    it, so we can pass on the argument for the line number. This
    also stops this helper function from being counted as a test.
- When calling check_hash_table(), pass $LINENO.
- Add dummy err_exit comments to have the tests counted.
2020-06-17 16:07:09 +02: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
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
7f2c81103b regress: avoid backporting a cmd subst bug from beta
ksh 93v- beta introduced a regression with nested command
substitutions: backticks nested in $( ) result in misdirected
output. This has never been in 93u+, but since we're often
backporting things, let's avoid backporting this bug. It is also
useful if this shows up when running our bin/shtests against the
actual beta by adding a SHELL=... argument.
Ref.: https://github.com/att/ast/issues/478

src/cmd/ksh93/tests/subshell.sh:
- Add reproducer submitted by the reporter as a regression test.
2020-06-15 16:52:12 +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
ae25b7f886 move read -S regress test to readcsv.sh (re: af0bd6ad) 2020-06-14 20:10:22 +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
Johnothan King
7b994b6a7e Implement a better fix for unsetting special env vars
The regression this commit fixes was first introduced in ksh93t
2008-07-25. It was previously worked around in 6f0e008c by forking
subshells if any special environment variable is unset.

The reason why this problem doesn't occur in ksh93s+ is because in
that version of ksh sh_assignok never moves nodes, it only clones
them. The second argument doesn't set NV_MOVE, which makes
`sh_assignok(np,0)` is similar to `sh_assignok(np,1)`. In ksh93t and
higher, setting the second argument to zero causes the node to be moved
with NV_MOVE, which causes the discipline function associated with
the variable node to be removed when `np->nvfun` is set to zero (i.e.
NULL). This is why a command like `(unset LC_NUMERIC; LC_NUMERIC=invalid)`
doesn't print a diagnostic, as it looses its discipline function.

This patch fixes the problem by cloning the node with sh_assignok
if it is a special variable with a discipline function. This allows
special variables to work as expected in virtual subshells. The
original workaround has been kept for the $PATH variable only, as
hash tables are still broken in virtual subshells. It has been updated
accordingly to only fork subshells if it detects the variable node
for PATH. I have added two more regression tests for changing the
PATH in subshells to make sure hash tables continue working as
expected with this fix.

src/cmd/ksh93/bltins/typeset.c:
 - Only fork virtual subshells if the PATH will be changed. If a
   variable is a special variable with a discipline function, it
   should be just be cloned, not moved.

src/cmd/ksh93/sh/nvdisc.c:
 - Add a comment to clarify that NV_MOVE will delete the discipline
   function associated with the node.

src/cmd/ksh93/tests/subshells.sh:
 - Add two more regression tests for unsetting the PATH in subshells,
   one for if PATH is being pointed to by a nameref. Condense the
   hash table tests by moving the main test into a single function.
2020-06-13 12:55:48 -07:00
Martijn Dekker
289f56cd4c tests/pty.sh: fix regress fail due to $TMPDIR
Test 137(C) was failing on some systems because $TMPDIR was set and
the local vi(1) honours it, so that the expected '/tmp/' string was
never output by vi. For compatibility with vi programs that honour
$TMPDIR and those that always use /tmp, we must export TMPDIR=/tmp.

src/cmd/ksh93/tests/pty.sh:

- Export TMPDIR=/tmp for test 137(C).
     Note that this exports TMPDIR to the environment for the
  duration of the 'tst' function run because the function was
  defined using the ksh 'function tst { ...; }' syntax.
2020-06-13 01:48:13 +02:00
Martijn Dekker
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
802ea67afb tests/io.sh: don't abort entire suite on failure
src/cmd/ksh93/tests/io.sh:
- 16 tests for the 'redirect' builtin used the ((arithmetic
  command)) to check the result. This does not tolerate the result
  being a non-number, such as the empty string, which may occur on
  test failure. So use [[ ... -eq ArithExpression ]] to check these
  results instead; it simply returns false, failing gracefully.
2020-06-12 11:34:10 +02:00
Martijn Dekker
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
Johnothan King
74b4162178 Fix set +r so that it cannot unset the restricted option
The ksh man page documents that the restricted option cannot be
unset once it is set, which means `set +r` should be invalid.
While this was true for `set +o restricted`, `set +r` was causing
the restricted option to be unset. The fix for this problem comes
from one of Solaris' patches, which adds an error check to prevent
this behavior.

Solaris' patch:
https://github.com/oracle/solaris-userland/blob/master/components/ksh93/patches/020-CR6919590.patch

src/cmd/ksh93/sh/args.c:
 - Add an error check to stop `set +r` from unsetting the
   restricted option.

src/cmd/ksh93/tests/restricted.sh:
 - Add two regression tests to make sure the restricted option
   cannot be unset.

(cherry picked from commit bef4fee404d8e24b38fce66420c14a39ac4a123e)
2020-06-12 01:45:18 +02:00
Martijn Dekker
9cef2d534a shtests: make tests more interruptable with Ctrl+C
Sometimes you have to put ^C on rapid repeat to get out of running
the tests. This is because each test scripts is launch as a child
shell (not a subshell) in the foreground, which thus handle the ^C.
It then exits with a status indicating SIGINT, but the shtest script
wasn't handling this and just kept going.

src/cmd/ksh93/tests/shtests:
- For reasons I don't understand yet, interrupting tests with ^C
  tends to make them exit with status 130 (128+2) instead of what
  I would expect for ksh93, 258 (256+2). Not a big deal: POSIX
  specifies that any exit > 128 signifies a signal in any case,
  and 'kill -l' works even on those values. But the checks need
  changing to '> 128'.
- Check each result for SIGINT, and issue SIGINT to self if found.

(cherry picked from commit d0dfb37c6c71ac7b157060249125e0959130927d)
2020-06-12 01:45:18 +02:00
Johnothan King
102868f850 Replace the hash alias with a proper builtin
This commit replaces the old hash alias with a proper builtin.
I based this builtin off of the code alias uses for handling
`alias -t --`, but with the hack for `--` removed as it has
no use in the new builtin. `alias -t --` will no longer work,
that hack is now gone.

While I was testing this builtin, I found a bug with hash tables
in non-forking subshells. If the hash table of a non-forking
subshell is changed, the parent shell's hash table is also changed.
As an example, running `(hash -r)` was resetting the parent shell's
hash table. The workaround is to force the subshell to fork if the
hash table will be changed.

src/cmd/ksh93/bltins/typeset.c:
 - Move the code for hash out of the alias builtin into a dedicated
   hash builtin. `alias -t --` is no longer supported.

src/cmd/ksh93/data/aliases.c:
 - Remove the old alias for hash from the table of predefined aliases.

src/cmd/ksh93/data/builtins.c:
 - Fix the broken entry for the hash builtin and add a man page for
   the new builtin.

src/cmd/ksh93/sh.1:
 - Replace the entry for the hash alias with a more detailed entry
   for the hash builtin.

src/cmd/ksh93/sh/name.c:
 - Force non-forking subshells to fork if the PATH is being reset
   to workaround a bug with the hash tree.

src/cmd/ksh93/tests/alias.sh:
 - Add a regression test for resetting a hash table, then adding
   a utility to the refreshed hash table.

src/cmd/ksh93/tests/subshell.sh:
 - Add regression tests for changing the hash table in subshells.

(cherry picked from commit d8428a833afe9270b61745ba3d6df355fe1d5499)
2020-06-12 01:45:18 +02:00
Martijn Dekker
d1bd8f89b7 shtests: print CPU times used at end (re: ebf71e61)
Since we now have a shiny new POSIX compliant 'times' builtin,
we might as well use it.

src/cmd/ksh93/tests/shtests:
- Run 'times' at end of test run.
- Skip the pretty-printing until #7 is fixed.

(cherry picked from commit 2c27d9fbc239583004ec70377db98627eea5e294)
2020-06-12 01:45:18 +02:00
Johnothan King
e92faddbf9 Fix 39 spelling errors and a formatting issue
A column of whitespace in the NEWS file was removed for consistent
formatting. Most of the spelling errors were found with this
codespell dictionary:
https://github.com/orbitcowboy/codespell_dictionary

(cherry picked from commit 0e36b17abe5609c461a3e4da7041eb0fdf9991b7)
2020-06-12 01:45:18 +02:00
Martijn Dekker
a1f46d785f rm "I/O error" error msg; just keep >0 exit status (re: 9011fa93)
The bug was really that I/O errors in output builtins were
undetectable by any means. Having a >0 exit status is sufficient.
Adding an error message risks making existing ksh scripts noisier,
or even breaking them if they redirect stderr to stdout.

Note to self: in future, implement the minimum change necessary to
fix bugs, nothing more. The fact that I needed to add four extra
2>/dev/null to the regression tests should have been a hint.

src/cmd/ksh93/bltins/print.c,
src/cmd/ksh93/data/msg.c,
src/cmd/ksh93/include/io.h:
- Remove "I/O error" message.

src/cmd/ksh93/tests/builtins.sh:
- Update to check for exit status only.

src/cmd/ksh93/tests/basic.sh,
src/cmd/ksh93/tests/coprocess.sh:
- Revert four new '2>/dev/null' to suppress the error message.

(cherry picked from commit 5e17be24d18455b575b6e98bc631c6935ffc795a)
2020-06-12 01:45:18 +02:00
Johnothan King
5d50f825e4 The unalias builtin should return an error for non-existent aliases
This commit fixes a bug that caused unalias to return a zero status
when it tries to remove an alias twice. The following set of commands
will no longer end with an error:

$ alias foo=bar
$ unalias foo
$ unalias foo && echo 'Error'

This commit is based on the fix present in ksh2020, but it has been
extended with another bugfix. The initial fix for this problem tried to
remove aliases from the alias tree without accounting for NV_NOFREE. This
caused any attempt to remove a predefined aliases (e.g. `unalias float`)
to trigger an error with free, as all predefined aliases are in read-only
memory. The fix for this problem is to set NV_NOFREE when removing aliases
from the alias tree, but only if the alias is in read-only memory. All
other aliases must be freed from memory to prevent memory leaks.

I'll also note that I am using an `isalias` variable rather than the `type`
enum from ksh2020, as the `VARIABLE` value is never used and was replaced
with a bool called `aliases` in the ksh2020 release. The `isalias` variable
is an int as the ksh93u+ codebase does not use C99 bools.

Previous discussion: https://github.com/att/ast/issues/909

- src/cmd/ksh93/bltins/typeset.c:
  Remove aliases from the alias tree by using nv_delete. NV_NOFREE
  is only used when it is necessary.

- src/cmd/ksh93/tests/alias.sh:
  Add two regression tests for the bugs fixed by this commit.

(cherry picked from commit 16d5ea9b52ba51f9d1bca115ce8f4f18e97abbc4)
2020-06-12 01:45:18 +02:00
Martijn Dekker
c2eabc57e0 shtests: report total errors on terminal and in exit status
Setting the exit status allows build scripts and the like
to actually detect regression test failures.

src/cmd/ksh93/tests/shtests:
- Add counter for total number of errors.
- Report total to stdout at the end.
- Set the exit status to the total number of errors, up to 125
  (higher statuses have special meanings).

(cherry picked from commit c0dd80b14d4d316b4e9eff4b1b67d4e47f23a6ba)
2020-06-12 01:45:17 +02:00
Martijn Dekker
fb652a7e50 shtests: fix loose ends
src/cmd/ksh93/tests/variables.sh:
- Tolerate a bit more time for the SECONDS verification test.

src/cmd/ksh93/tests/subshell.sh:
- Replace unportable 'head -c 1' by 'dd bs=1 count=1'
- Remove unnecessary uses of 'whence'.

src/cmd/ksh93/tests/builtins.sh:
- Add a regression test for a weirdly specific 'whence' bug exposed
  by the aforementioned unneccessary uses of 'whence', which only
  shows up on my old Power Mac G5 running Mac OS X 10.3. For all I
  know it's a compiler bug, but let's add a more clear failure for
  it here, in case that happens anywhere else.

(cherry picked from commit c3898bd1e6e40874845771d33a5b37220ef0b06e)
2020-06-12 01:45:17 +02:00
Martijn Dekker
712261c89b shtests: More speedups; also fix xtrace (re: 734e5953)
This reduces a bunch more unnecessarily long sleeps to give
asynchronous processes time to run, etc. (No, we don't need to be
compatible anymore with your cool 1985 Intel 80386DX 16 MHz
battlestation...) Running the test suite is almost tolerable now,
making me much more likely to actually run the regression test
suite and catch my own regressions.

In addition, there are various fixes to make the test suite
compatible with 'set -x' ('set -o xtrace') so that you can now
actually *use* the documented 'bin/shtests -x' option. Recommend
combining with '-p' to avoid tracing everything three times.

I've also added a really useful $PS4 trace prompt to shtests that
traces pretty much everything there is to trace. (It does use
expansions that modify ${.sh.match}, which affected several tests,
particularly in tests/substring.sh; for those we need to set a
temporary simpler $PS4.)

(cherry picked from commit c3a5d47cfe880b526cabb5370ddaced0e8626acd)
2020-06-12 01:45:17 +02:00
Martijn Dekker
8f90d98280 tests/signal.sh: slash sleeps by 10 to reduce a long wait
The signal.sh regression tests took over a minute and a half to
complete, so with the default three runs (for different locales)
they took about five minutes. Annoying and unnecessary. As ksh
supports floating point shell arithmetic plus a 'sleep' builtin
that supports fractional seconds, this is easy to fix.

src/cmd/ksh93/tests/signal.sh:
- Slash all the 'sleep' arguments and associated factors by a
  factor of 10. Since computers are now hundreds or thousands of
  times faster than they were in the early 1990s, this should be
  plenty safe enough and all the tests should still be valid.
- Change one unportable direct invocation of /bin/sleep to use the
  builtin.

(cherry picked from commit 734e5953650e9a62048c45eb9a3b827bb9efcfb5)
2020-06-12 01:45:17 +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
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
e2a648b41f tests/options.sh: fix child shell test by exporting variable
src/cmd/ksh93/tests/options.sh:
- Pipe hang bugfix test: The child shell has no idea how long to
  sleep for if we don't export the variable telling it so.

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