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

109 commits

Author SHA1 Message Date
Martijn Dekker
3654ee73c0 Fix typeset -l/-u crash on special vars (rhbz#1083713)
When using typeset -l or -u on a variable that cannot be changed
when the shell is in restricted mode, ksh crashed.

This fixed is inspired by this Red Hat fix, which is incomplete:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-tpstl.patch

The crash was caused by the nv_shell() function. It walks though a
discipline function tree to get the pointer to the interpreter
associated with it. Evidently, the problem is that some pointer in
that walk is not set correctly for all special variables.

Thing is, ksh only has one shell language interpreter, and only one
global data structure (called 'sh') to keep its main state[*]. Yet,
the code is full of 'shp' pointers to that structure. Most (not
all) functions pass that pointer around to each other, accessing
that struct indirectly, ostensibly to account for the non-existent
possibility that there might be more than one interpreter state.
The "why" of that is an interesting cause for speculation that I
may get to sometime. For now, it is enough to know that, in the
code as it is, it matters not one iota what pointer to the shell
interpreter state is used; they all point to the same thing (unless
it's broken, as in this bug).

So, rather than fixing nv_shell() and/or associated pointer
assignments, this commit simply removes it, and replaces it with
calls to sh_getinterp(), which always returns a pointer to sh (see
init.c, where that function is defined as literally 'return &sh').

[*] Defined in shell.h, with the _SH_PRIVATE part in defs.h

src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/name.c:
- Remove nv_shell().

src/cmd/ksh93/sh/init.c:
- In all the discipline functions for special variables, initialise
  shp using sh_getinterp() instead of nv_shell().

src/cmd/ksh93/tests/variables.sh:
- Add regression test for typeset -l/-u on all special variables.
2020-09-24 03:03:29 +02:00
Martijn Dekker
843b546c1a rm redundant getpid(2) syscalls (re: 9de65210)
Now that we have ${.sh.pid} a.k.a. shgd->current_pid, which is
updated using getpid() whenever forking a new process, there is no
need for anything else to ever call getpid(); we can use the stored
value instead. There were a lot of these syscalls kicking around,
some of them in performance-sensitive places.

The following lists only changes *other* than changing getpid() to
shgd->currentpid.

src/cmd/ksh93/include/defs.h:
- Comments: clarify what shgd->{pid,ppid,current_pid} are for.

src/cmd/ksh93/sh/main.c,
src/cmd/ksh93/sh/init.c:
- On reinit for a new script, update shgd->{pid,ppid,current_pid}
  in the sh_reinit() function itself instead of calling sh_reinit()
  from sh_main() and then updating those immediately after that
  call. It just makes more sense this way. Nothing else ever calls
  sh_reinit() so there are no side effects.

src/cmd/ksh93/sh/xec.c: _sh_fork():
- Update shgd->current_pid in the child early, so that the rest of
  the function can use it instead of calling getpid() again.
- Remove reassignment of SH_PIDNOD->nvalue.lp value pointer to
  shgd->current_pid (which makes ${.sh.pid} work in the shell).
  It's constant and was already set on init.
2020-09-23 04:19:02 +02:00
Martijn Dekker
ce68e1be37 Fix crash in backtick comsubs with job control on (rhbz#825520)
This imports another fix from Red Hat/Fedora. Original patch:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-crash.patch

src/cmd/ksh93/include/jobs.h,
src/cmd/ksh93/sh/jobs.c,
src/cmd/ksh93/sh/subshell.c,
src/cmd/ksh93/sh/xec.c:
- Import the Red Hat fix with these differences:
  - Rename the 'hack1_waitall' variable to 'bktick_waitall' and add
    a comment describing what it's for.
  - Remove unused 'pipefail' variable.

src/cmd/ksh93/tests/basic.sh:
- Regression test from reproducer given in the Red Hat bug report.
- Add special handling to SIGKILL it, as it might freeze hard.
2020-09-23 01:56:09 +02:00
Martijn Dekker
5683155cb5 update NEWS, SH_RELEASE (re: 970069a6) 2020-09-22 01:45:01 +02:00
Martijn Dekker
970069a6fe Fix command substitutions in here-docs (rhbz#994241, rhbz#1036802)
When ksh was compiled with SHOPT_SPAWN (the default), any command
substitution embedded in a here-document returned an empty string.
The bug was also present in 93u+ 2012-08-01 (although not in every
case as some systems compile it without SHOPT_SPAWN).

This fixes it by applying a slightly edited combination of two Red
Hat patches (the second containing a fix for the first), which
backport a new command substitution mechanism from the abandoned
ksh 93v- beta version. The originals are:

https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-macro.patch
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-fd2lost.patch

src/cmd/ksh93/include/io.h:
- The iopipe() function from xec.c is now needed in sh_subshell()
  (subshell.c), so rename it to sh_iounpipe() and declare it as an
  extern here. The 93v- beta did it as well. (The Red Hat patch did
  this without renaming it.)

src/cmd/ksh93/sh/xec.c:
- Backport new versions of iousepipe() and sh_iounpipe() from ksh
  93v-. New 'type' flaggery is introduced to distinguish between
  different command substitution conditions. What all that means
  remains to be determined.
- sh_exec(): I made one change to the Red Hat patch myself: if in a
  subshell and the type flags FAMP (for "ampersand" as in '&' as in
  background job) and TFORK are set, continue to call sh_subfork()
  to fork the subshell unconditionally, instead of only if we're in
  a command substitution connected to an unseekable file. Maybe the
  latter works for the 93v- code, but on 93u+(m) it causes a couple
  of regressions, which are fixed by my change:
  signal.sh[273]: subshell ignoring signal does not send signal to parent
  signal.sh[276]: subshell catching signal does not send signal to parent
  Details: https://github.com/ksh93/ksh/issues/104#issuecomment-696341902

src/cmd/ksh93/sh/macro.c,
src/cmd/ksh93/sh/subshell.c:
- Updates that go with those new functions.

Fixes:   https://github.com/ksh93/ksh/issues/104
Affects: https://github.com/ksh93/ksh/issues/124
2020-09-21 23:02:08 +02:00
Martijn Dekker
a329c22dba Multiple 'whence' and path search fixes
Hopefully this doesn't introduce new bugs, but it does fix at
least the following:

1. When whence -v/-a found an "undefined" (i.e. autoloadable)
   function in $FPATH, it actually loaded the function as a side
   effect of reporting on its existence (!). Now it only reports.

2. 'whence' will now canonicalise paths properly. Examples:
	$ whence ///usr/lib/../bin//./env
	/usr/bin/env
	$ (cd /; whence -v dev/../usr/bin//./env)
	dev/../usr/bin//./env is /usr/bin/env

3. 'whence' no longer prefixes a spurious double slash when doing
   something like 'cd / && whence bin/echo'. On Cygwin, an initial
   double slash denotes a network server, so this was not just a
   cosmetic problem.

4. 'whence -a' now reports a "tracked alias" (a.k.a. hash table
   entry, i.e. cached $PATH search) even if an actual alias by the
   same name exists. This needed fixing because in fact the hash
   table entry continues to be used when bypassing the alias.
   Aliases and "tracked aliases" are not remotely the same thing;
   confusing nomenclature is not a reason to report wrong results.

5. When using 'hash' or 'alias -t' on a command that is also a
   builtin to force caching a $PATH search for the external
   command, 'whence -a' double-reported the path:
	$ hash printf; whence -a printf
	printf is a shell builtin
	printf is /usr/bin/printf
	printf is a tracked alias for /usr/bin/printf
   This is now fixed so that the second output line is gone.
   Plus, if there were multiple versions of the command on $PATH,
   the tracked alias was reported at the end, which is the wrong
   order. This is also fixed.

src/cmd/ksh93/bltins/whence.c: whence():
- Refactor the do...while loop that handles whence -v/-a for path
  searches in such a way that the code actually makes sense and
  stops looking like higher esotericism. Just doing this fixed #2,
  #4 and #5 above (the latter two before I even noticed them). For
  instance, the path_fullname() call to canonicalise paths was
  already there; it was just never used.
- Remove broken 'notrack' flaggery for deciding whether to report a
  hash table entry a.k.a. "tracked alias"; instead, check the hash
  table (shp->track_tree).

src/cmd/ksh93/sh/path.c:
- path_search(): Re #3: When prefixing the PWD, first check if
  we're in '/' and if so, don't prefix it; otherwise, adding the
  next slash causes an initial double slash. (Since '/' is the only
  valid single-character absolute path, all we need to do is check
  if the second character pwd[1] is non-null.)
- path_search(): Re #1: Stop autoloading when called by 'whence':
  * The 'flag==2' check to avoid autoloading a function was
    broken. The flag value is 2 on the first whence() loop
    iteration, but 3 on subsequent ones. Change to 'flag >= 2'.
  * However, this only fixes it if the function file does not have
    the x permission bit, as executable files are handled by
    path_absolute() which unconditionally autoloads functions!
    So, pass on our flag parameter when callling path_absolute().
- path_absolute(): Re #1: Add flag parameter. Do not autoload
  functions if flag >= 2.

src/cmd/ksh93/include/path.h,
src/cmd/ksh93/bltins/typeset.c,
src/cmd/ksh93/sh/main.c,
src/cmd/ksh93/sh/xec.c:
- Re #1: Update path_absolute() calls, adding a 0 flag parameter.

src/cmd/ksh93/include/name.h:
- Remove now-unused pathcomp member from union Value. It was
  introduced in 99065353 to allow examining the value of a tracked
  alias. This commit uses nv_getval() instead.

src/cmd/ksh93/tests/builtins.sh,
src/cmd/ksh93/tests/path.sh:
- Add and tweak various related tests.

Fixes: https://github.com/ksh93/ksh/issues/84
2020-09-20 07:56:09 +02:00
Martijn Dekker
f45a0f1650 -o posix: inverse-sync braceexpand; properly sync letoctal
{Brace,expansion} is potentially incompatible with POSIX scripts,
because in POSIX those are simple literal strings with no special
meaning. So the POSIX option should really turn that off.

As of b301d417, the 'posix' option was also forcing 'letoctal'
behaviour on, without actually setting that option. I've since
found that to be a botch; 'let' may recognise octals without that
option being set, and that looks like a bug.

So as of this commit, the '-o posix' option actually toggles both
of these options off/on and on/of, respectively. 'set +o posix'
toggles them inversely. However, it is now possible to control both
options (and their associated behaviour) independently in between
'set -o posix' and 'set +o posix'. Much better.

src/cmd/ksh93/sh/main.c: sh_main():
- If SH_POSIX was set on init, turn on SH_LETOCTAL by default
  instead of SH_BRACEEXPAND.

src/cmd/ksh93/sh/args.c: sh_applyopts():
- Turn off SH_BRACEEXPAND and turn on SH_LETOCTAL when SH_POSIX is
  turned on (but not if it was already on).
- Turn on SH_BRACEEXPAND and turn off SH_LETOCTAL when SH_POSIX is
  turned off (but not if it was already off).

src/cmd/ksh93/sh/arith.c: arith():
- Revert to pre-b301d417 and only check SH_LETOCTAL option when
  deciding whether 'let' should skip initial zeros.

src/cmd/ksh93/tests/options.sh:
- Update $- test to allow '-o posix' to switch B = braceexpand.

src/cmd/ksh93/sh.1:
- Update.
- Edit for clarity.
2020-09-18 22:07:44 +02:00
Martijn Dekker
13c3fb21e9 emacs, vi: Support repeat parameters to VT220 keys (re: f2a3f4e3)
In the vi and emacs line editors, repeat count parameters can now
also be used for the arrow keys and the forward-delete key. E.g.,
in emacs mode, <ESC> 7 <left-arrow> will now move the cursor seven
positions to the left. In vi control mode, this would be entered
as: 7 <left-arrow>.

src/cmd/ksh93/edit/emacs.c:
- ed_emacsread(): Upon getting ^[ (ESC), save current repeat count
  in a new variable; restore and reset it upon the next character.
- escape(): Minor bugfix: when processing a ^[[x sequence where 'x'
  is a character other than '~' (which would be DEL), also reinsert
  the final character into the buffer so scripts can detect them.

src/cmd/ksh93/edit/vi.c:
- cntlmode(): Do not reset the repeat count if the command is '[',
  the character following ESC in VT220 escape sequences.
- mvcursor():
  * Do not use getcount() to get the character following '[', as
    that was parsing repetition parameters in the wrong place.
    There wouldn't be any, so this would reset the repeat count.
  * After that, no more need for the special-casing of ^[[3~ (DEL)
    introduced in f2a3f4e3. Move it to within the 'switch' block.
  * When handling left and right arrows and Home and End keys, do
    not modify cursor directly but ed_ungetchar() the corresponding
    traditional command keys as with the rest. Otherwise a repeat
    count parameter would now wrongly survive those keys.

src/cmd/ksh93/sh.1:
- Document control character notation used for vi mode docs.
- Since vi control mode beeps and aborts on ESC except if a
  subsequent [ is already in the input buffer upon receiving ESC,
  document that VT220 escape sequences only preserve repeat counts
  when entered into the input buffer all at once.
- Don't skip the initial ESC in the documentation of the VT220
  escape sequences. In control mode, skipping the initial ESC still
  works as before, but that is now undocumented, as it's really
  nothing more than an artefact of VT220 escape processing.
- Move the two long paragraphs on '-o viraw' and canonical (i.e.
  line-based) input processing from the vi editor introduction to
  the options section under 'viraw'. It is much too arcane for the
  intro, and besides, ksh 93u+ (and hence also 93u+m) has
  SHOPT_VIRAW enabled by default, so the shell is compiled to force
  this option on at all times, making it even less relevant for
  most users.
2020-09-17 19:14:39 +02:00
Martijn Dekker
c24c3dc0af include/edit.h: cleanup: rm unused Edit_t struct members 2020-09-16 06:38:53 +02:00
Martijn Dekker
461a1aebc1 Fix memory leak in typeset (rhbz#1036470)
A memory leak occurred when typeset was used in a function called
from within a command substitution. This fix was backported from
the 93v- beta by Red Hat on 22 Jan 2014. Source:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-memlik3.patch

src/cmd/ksh93/include/name.h,
src/cmd/ksh93/sh/subshell.c:
- Replace the nv_subsaved() function by the version from ksh 93v-.
  This version frees a table from memory if the NV_TABLE flag is
  passed in the new second parameter, a bitmask for flags (which
  was oddly named 'table'; I've renamed it to 'flags').

src/cmd/ksh93/sh/name.c:
- nv_delete(): When calling nv_subsaved(), pass on the NV_TABLE
  flag if given.
- table_unset(): Call nv_delete() with the NV_TABLE flag.

src/cmd/ksh93/tests/leaks.sh:
- Add test based on the reproducer provided in Red Hat bug 1036470.
2020-09-15 23:52:32 +02:00
hyenias
d7c90eadc3 sfio: correct floating decimal point scaling of fractions (#131)
_sfcvt(), "convert a floating point value to ASCII", did not adjust
for negative decimal place movement as what happens with leading
zeroes. This caused ksh's 'printf %f' formatter to fail to round
floating point values correctly.

src/lib/libast/sfio/sfcvt.c:
- Removed constraint of <1e-8 for doubles by matching what was done
  for long doubles having <.1.
- Corrected a condition when the next power of 10 occurred and that
  new 1 digit was being overwritten by a 0.

 src/cmd/ksh93/tests/math.sh:
- Validate that typeset -E/F formatting matches that of their
  equivalent printf formatting options as well as checking for
  correct float scaling of the fractional parts.
2020-09-14 13:46:40 +02:00
Martijn Dekker
ab5dedded7 Work around process substitution file descriptor leak
File descriptors are not properly closed, causing a leak, when
using a process substitution as an argument to a shell function.
See: https://github.com/ksh93/ksh/issues/67

Process substitution uses /dev/fd/NN pseudofiles if the kernel
provides them. This is tested in src/cmd/ksh93/features/options
which causes SHOPT_DEVFD to be defined if /dev/fd/9 can be used.
If not, ksh uses a fallback mechanism involving a temporary FIFO,
which works on all Unix variants.

As it happens, the leak only occurs when using the /dev/fd
mechanism. So, until a fix is found, we can work around the bug by
disabling it. The FIFO mechanism might be slightly less robust,
but it's an improvement over leaking file descriptors. Plus, there
is room for improving it.

src/cmd/ksh93/include/defs.h:
- Unconditionally redefine SHOPT_DEVFD as 0 for now.

src/cmd/ksh93/sh/args.c: sh_argprocsub():
- pathtemp() does appropriate access checks using access(2), but
  there is an inherent race condition between calling it and
  mkfifo(). Make the FIFO mechanism more robust by handling errors,
  trying again if an error occurs that must have resulted from
  losing that race, e.g. file name conflict or temp dir
  permission/location change.
- Initially create the FIFO without any permissions, then chmod()
  the appropriate user read/write permissions. Since mkfifo()
  honours the umask and chmod() does not, this ensures that process
  substitution continues to work if a shell script sets a umask
  that disallows user read or write. (The /dev/fd/ mechanism does
  not care about the umask, so neither should the fallback.)
2020-09-12 20:22:19 +02:00
Martijn Dekker
ddaa145b3d Reinstate 'r' and 'history' as preset aliases for interactive ksh
Following a community discussion, it became clear that 'r' is
particularly problematic as a regular builtin, as the name can and
does conflict with at least one legit external command by that
name. There was a consensus against removing it altogether and
letting users set the alias in their login scripts. However,
aliases are easier to bypass, remove or rename than builtins are.
My compromise is to reinstate 'r' as a preset alias on interactive
shells only, along with 'history', as was done in 17f81ebe before
they were converted to builtins in 03224ae3. So this reintroduces
the notion of predefined aliases to ksh 93u+m, but only for
interactive shells that are not initialised in POSIX mode.

src/cmd/ksh93/Makefile,
src/cmd/ksh93/Mamfile,
src/cmd/ksh93/include/shtable.h,
src/cmd/ksh93/data/aliases.c:
- Restore aliases.c containing shtab_aliases[], a table specifying
  the preset aliases.

src/cmd/ksh93/include/shtable.h,
src/cmd/ksh93/sh/init.c:
- Rename inittree() to sh_inittree() and make it extern, because we
  need to use it in main.c (sh_main()).

src/cmd/ksh93/sh/main.c: sh_main():
- Init preset aliases from shtab_aliases[] only if the shell is
  interactive and not in POSIX mode.

src/cmd/ksh93/bltins/typeset.c,
src/cmd/ksh93/tests/alias.sh:
- unall(): When unsetting an alias, pass on the NV_NOFREE attribute
  to nv_delete() to avoid an erroneous attempt to free a preset
  alias from read-only memory. See: 5d50f825

src/cmd/ksh93/data/builtins.c:
- Remove "history" and "r" entries from shtab_builtins[].
- Revert changes to inline fc/hist docs in sh_opthist[].

src/cmd/ksh93/bltins/hist.c: b_hist():
- Remove handling for 'history' and 'r' as builtins.

src/cmd/ksh93/sh.1:
- Update accordingly.

Resolves: https://github.com/ksh93/ksh/issues/125
2020-09-11 21:35:45 +02:00
Martijn Dekker
b9d10c5a9c Fix 'command' expansion bug and POSIX compliance
The 'command' name can now result from an expansion, e.g.:
	c=command; "$c" ls
	set -- command ls; "$@"
both work now. This fixes BUG_CMDEXPAN.

If -o posix is on, 'command' now disables not only the "special"
but also the "declaration" properties of builtin commands that it
invokes. This is because POSIX specifies 'command' as a simple
regular builtin, and any command name following 'command' is just
an argument to the 'command' command, so there is nothing that
allows any further arguments (such as assignment-arguments) to be
treated specially by the parser. So, if and only if -o posix is on:
a. Arguments that start with a variable name followed by '=' are
   always treated as regular words subject to normal shell syntax.
b. Since assignment-arguments are not processed as assignments
   before the command itself, 'command' can now stop the shell from
   exiting (as required by the standard) if a command that it
   invokes (such as 'export') tries to modify a readonly variable.
   This fixes BUG_CMDSPEXIT.

Most of 'command' is integrated in the parser and parse tree
executer, so that is where it needed fixing.

src/cmd/ksh93/sh/parse.c: simple():
- If the posix option is on, do not skip past SYSCOMMAND so that
  any declaration builtin commands that are arguments to 'command'
  are not detected and thus not treated specially at parsetime.

src/cmd/ksh93/sh/xec.c: sh_exec():
- When detecting SYSCOMMAND in order to skip past it, not only
  compare the Namval_t pointer 'np' to SYSCOMMAND, but also handle
  the case where that pointer is NULL, as when the command name
  results from an expansion. In that case, search the function tree
  shp->fun_tree for the name and see if that yields the SYSCOMMAND
  pointer. fun_tree is initialised with a dtview to bltin_tree, so
  searching fun_tree instead allows for overriding 'command' with a
  shell function (which the POSIX standard requires us to allow).

src/cmd/ksh93/sh.1,
src/cmd/ksh93/data/builtins.c:
- Update documentation to match these changes.
- Various related edits and improvements.

src/cmd/ksh93/tests/builtins.sh:
- Check that 'command' works if resulting from an expansion.
- Check that 'command' can be overridden by a shell function.
2020-09-11 10:06:43 +02:00
Martijn Dekker
092b90da81 Fix BUG_LOOPRET2 and related return/exit misbehaviour
The 'exit' and 'return' commands without an argument failed to pass
down the exit status of the last-run command when incorporated in a
block with redirection, &&/|| list, 'case' statement, or 'while',
'until' or 'for' loop.

src/cmd/ksh93/bltins/cflow.c:
- Use $?, which is sh.savexit a.k.a. shp->savexit, as the default
  exit status value if there is no argument, instead of
  shp->oldexit. This fixes the default exit status behaviour to
  match POSIX and other shells.

src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/include/shell.h:
- Remove now-unused sh.oldexit (a.k.a. shp->oldexit) private struct
  member. It appeared to fulfill the same function as sh.savexit,
  but in a slightly broken way.
- Move the savexit/$? declaration from the _SH_PRIVATE part of the
  struct definition to the public API part. Since $? uses this,
  it's clearly a publicly exposed value already, and this is
  generally the one to use. (If anything, it's exitval that should
  have been private.) This declares savexit right next to exitval,
  rewriting the comments to clarify the difference between them.

src/cmd/ksh93/sh/fault.c,
src/cmd/ksh93/sh/subshell.c,
src/cmd/ksh93/sh/xec.c:
- Remove assignments to shp->oldexit.

src/cmd/ksh93/tests/basic.sh:
- Add thorough regression tests for the default exit status
  behaviour of 'return' and 'exit' in various lexical contexts.
- Verify that 'for' and 'case' without any command, as well as a
  lone redirection, still correctly reset the exit status to 0.

Fixes: #117
2020-09-09 20:02:20 +02:00
Johnothan King
400c107773
Fix compile with -D_std_malloc (re: f9c127e) (#130)
src/cmd/ksh93/include/jobs.h:
- The commit that removed legacy code mistakenly removed the
  definition of vmbusy() required for ksh to compile with
  -D_std_malloc. Ksh assumes vmbusy is always a macro, even
  when _std_malloc is defined. This commit reintroduces the
  _std_malloc definition of vmbusy to fix undefined
  reference errors.
2020-09-08 03:20:18 +01:00
Martijn Dekker
5ed9ffd6c4 This fixes erroneous syntax errors in parameter expansions such as
${var:-wor)d} or ${var+w(ord}. The parentheses now correctly lose
their normal grammatical meaning within the braces. Fix by Eric
Scrivner (@etscrivner) from July 2018 backported from ksh2020.

This fix complies with POSIX:
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_02

src/cmd/ksh93/sh/lex.c: sh_lex():
- Set the ST_QUOTE state when analysing a modifier with parameter
  expansions using operators ':', '-', '+', '='. This state causes
  subsequent characters (including parentheses) to be considered
  quoted, suppressing their normal grammatical meaning.

src/cmd/ksh93/sh/macro.c: varsub():
- Same for skipping the expansion.

Fixes: https://github.com/ksh93/ksh/issues/126
Prior discussion: https://github.com/att/ast/issues/475
2020-09-05 16:20:22 +02:00
Martijn Dekker
bec6556236 update NEWS, SH_RELEASE (re: 6575903d) 2020-09-04 05:29:52 +02:00
Martijn Dekker
f9c127e39e Remove legacy code for older libast versions
Since ksh 93u+m comes bundled with libast 20111111, there's no need
to support older versions, so this is another cleanup opportunity.

src/cmd/ksh93/include/defs.h:
- Throw an #error if AST_VERSION is undefined or < 20111111.
  (Note that _AST_VERSION is the same as AST_VERSION, but the
  latter is newer and preferred; see src/lib/libast/features/api)

All other changed files:
- Remove legacy code for versions older than the currently used
  versions, which are:
  _AST_VERSION    20111111
  ERROR_VERSION   20100309
  GLOB_VERSION    20060717
  OPT_VERSION     20070319
  SFIO_VERSION    20090915
  VMALLOC_VERSION 20110808
2020-09-04 02:31:39 +02:00
Martijn Dekker
8d7f616e75 Remove abandoned SHOPT_ENV experiment
SHOPT_ENV is an undocumented compile-time option implementing an
experimental method for handling environment variables, which is
implemented in env.h and env.c. There is no mention in the docs or
Makefile, and no mention in the mailing list archives. It adds no
new functionality, but at first glance it's a clean-looking
interface.

However, unfortunately, it's broken. Compiling with -DSHOPT_ENV
added to CCFLAGS causes bin/shtests to show these regressions:

functions.sh[341]: export not restored name=value function call -- expected 'base', got ''
functions.sh[1274]: Environment variable is not passed to a function
substring.sh[236]: export not restored name=value function call
variables.sh[782]: SHLVL should be 3 not 2

In addition, 'export' stops working on unset variables.

In the 93v- beta this code is still present, unchanged, though 93v-
made lots of incompatible changes. By the time ksh2020 noticed it,
it was no longer compiling, so it probably wasn't compiling in the
93v- beta either. Discussion: https://github.com/att/ast/issues/504
So the experiment was already abandoned by D. Korn and his team.

Meanwhile it was leaving sh/name.c with two versions of several
enviornment-related functions, and it's not clear which one is
actually compiled without doing detective work tracing header files
(most of the code was made conditional on _ENV_H, which is defined
in env.h, which is included by defs.h if SHOPT_ENV is defined).
This actively hinders understanding of the codebase. And any
changes to these functions would need to be implemented twice.

src/cmd/ksh93/include/env.h,
src/cmd/ksh93/sh/env.c:
- Removed.

src/cmd/ksh93/DESIGN,
src/cmd/ksh93/Makefile,
src/cmd/ksh93/Mamfile:
- Update accordingly.

All other changed files:
- Remove deactivated code behind SHOPT_ENV and _ENV_H.
2020-09-02 16:09:57 +01:00
Martijn Dekker
921bbcaeb7 Remove SHOPT_BASH; keep &> redir operator, '-o posix' option
On 16 June there was a call for volunteers to fix the bash
compatibility mode; it has never successfully compiled in 93u+.
Since no one showed up, it is now removed due to lack of interest.

A couple of things are kept, which are now globally enabled:

1. The &>file redirection shorthand (for >file 2>&1). As a matter
   of fact, ksh93 already supported this natively, but only while
   running rc/profile/login scripts, and it issued a warning. This
   makse it globally available and removes the warning, bringing
   ksh93 in line with mksh, bash and zsh.

2. The '-o posix' standard compliance option. It is now enabled on
   startup if ksh is invoked as 'sh' or if the POSIXLY_CORRECT
   variable exists in the environment. To begin with, it disables
   the aforementioned &> redirection shorthand. Further compliance
   tweaks will be added in subsequent commits. The differences will
   be fairly minimal as ksh93 is mostly compliant already.

In all changed files, code was removed that was compiled (more
precisely, failed to compile/link) if the SHOPT_BASH preprocessor
identifier was defined. Below are other changes worth mentioning:

src/cmd/ksh93/sh/bash.c,
src/cmd/ksh93/data/bash_pre_rc.sh:
- Removed.

src/cmd/ksh93/data/lexstates.c,
src/cmd/ksh93/include/shlex.h,
src/cmd/ksh93/sh/lex.c:
- Globally enable &> redirection operator if SH_POSIX not active.
- Remove warning that was issued when &> was used in rc scripts.

src/cmd/ksh93/data/options.c,
src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/args.c:
- Keep SH_POSIX option (-o posix).
- Replace SH_TYPE_BASH shell type by SH_TYPE_POSIX.

src/cmd/ksh93/sh/init.c:
- sh_type(): Return SH_TYPE_POSIX shell type if ksh was invoked
  as sh (or rsh, restricted sh).
- sh_init(): Enable posix option if the SH_TYPE_POSIX shell type
  was detected, or if the CONFORMANCE ast config variable was set
  to "standard" (which libast sets on init if POSIXLY_CORRECT
  exists in the environment).

src/cmd/ksh93/tests/options.sh,
src/cmd/ksh93/tests/io.sh:
- Replace regression tests for &> and move to io.sh. Since &> is
  now for general use, no longer test in an rc script, and don't
  check that a warning is issued.

Closes: #9
Progresses: #20
2020-09-01 06:19:19 +01:00
Martijn Dekker
77beec1e0d restore 'test --man --' oddness (re: fa6a180f)
Following a community objection to its removal, the inline 'test'
manual page along with its strange method of invocation is
restored. I've taken the opportunity to correct several mistakes,
add some missing info, do some copy-editing, and document the way
to get these docs in the main (k)sh.1 manual.

Discussion:
https://github.com/ksh93/ksh/commit/fa6a180f#commitcomment-41897553
2020-08-31 23:43:22 +01:00
Martijn Dekker
fa6a180fdd test/[: remove effectively inaccessible self-doc
Did you know that you could get a manual page for the 'test'/'['
builtin command using one of these strange command lines?

	test --man --

	[ --man -- ]

Neither did I. It's not documented or mentioned anywhere (and this
syntax violates POSIX). So nobody knows about it, which makes that
documentation useless. (The regular --man option doesn't work
because that would break 'test'.) I only found out how to invoke it
when I understood what the uncommented C code handling this does.

The test/[ command's self-documentation is unmaintained since 2003
and somewhat incomplete. It's also mostly redundant with the
documentation on Conditional Expressions in the main (k)sh.1 manual
page. But unlike the latter, this is resident in RAM, wasting
working memory in every shell process.

src/cmd/ksh93/sh.1:
- Add documentation for 'test'/'[' commands (yes, they were not
  mentioned in the main manual page until now), describing them
  in terms of differences from '[[' and recommending the latter.

src/cmd/ksh93/include/test.h,
src/cmd/ksh93/bltins/test.c,
src/cmd/ksh93/data/testops.c:
- Remove RAM-resident --man doc for test/[ command.
- Remove the bizarre option parsing that allowed invoking it.
2020-08-30 08:08:01 +01:00
Martijn Dekker
cd2cf236c2 test/[: use a shell state bit (re: 7003aba4)
Instead of a global 'sh_in_test_builtin' integer flag, it is nicer
to use the mechanism for shell state bits, which was designed for
this sort of thing.

src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/defs.c:
- Remove global sh_in_test_builtin integer.
- Define new SH_INTESTCMD state bit.

src/cmd/ksh93/bltins/test.c: _ERROR_exit_b_test(), b_test():
- Use the new state bit.
2020-08-30 05:33:59 +01:00
Martijn Dekker
42301639d6 '#if 0' cleanup
This removes various blocks of uncommented experimental code that
was disabled using '#if 0' or '#if 1 ... #else' directives. It's
hard or impossible to figure out what the thoughts behind them
might have been, and we can really do without those distractions.
2020-08-30 04:51:20 +01:00
Martijn Dekker
f8feed1bd2 SHOPT_MULTIBYTE-related cleanup (re: 8477d2ce)
As of 8477d2ce, the mbwide() macro (which tests if we're in a
multibyte locale, i.e. UTF-8) is redefined as a constant 0 if we're
compiling without SHOPT_MULTIBYTE. See src/cmd/ksh93/include/defs.h

The other multibyte macros use mbwide() as well, so they all revert
to the single-byte fallbacks in that case, and the multibyte code
in them is never compiled. See src/lib/libast/include/ast.h

Consequently we can now do a bit of cleanup and get rid of many of
the '#if SHOPT_MULTIBYTE' directives, as the compiler optimiser
will happily remove the multibyte-specific code. This increases the
legibility of the ksh code.

I'm taking the opportunity to fix a few typos and whitespace
formatting glitches as well.
2020-08-30 04:50:57 +01:00
Martijn Dekker
9ba2c2e0df Speed up 'read', fixing macOS hang (take 2)
This fixes a hanging bug that could occur on macOS when using the
'read' command to read from a FIFO and encountering end-of-file
without a final newline character. It also makes the 'read' command
perform 15-25% faster on macOS and Linux.

The previous version (ff385e5a) failed on SunOS/Solaris/Illumos
because those systems apparently don't (fully) support the POSIX
standard recv(2) syscall with MSG_PEEK[*], which is the feature
that iffe detects under the 'socket_peek' identifier. On Illumos,
using that methods causes a compilation failure (unknown identifier
MSG_PEEK); on Solaris 11.4, that method causes multiple regressions
in tests/io.sh, suggesting the method compiles but doesn't work at
all. Instead, SunOS/Solaris/Illumos requires the method using
ioctl(2)+I_PEEK and select(2). No other system that ksh currently
builds on requires this method, so it is now only used on
SunOS/Solaris/Illumos.

So far, this version of sfpkrd() has been tested to work correctly
on Linux, macOS, FreeBSD, NetBSD, OpenBSD, HP-UX, Solaris, and
OmniOS (an Illumos distribution).

It still fails to peek on Cygwin, but in the exact same way it
failed before, so that's no loss.

To test, run the 'io' test set:  bin/shtests -p io

src/lib/libast/sfio/sfpkrd.c: sfpkrd():
- Remove long-obsolete Mac OS X and Solaris bug workarounds.
- Remove methods that are no longer needed.
     On systems with a POSIX compliant recv(2), the only thing that
  is required to avoid regressions is the code that was conditional
  upon the socket_peek feature test, which tests for the correct
  functioning of the recv(2) syscall. This has now been made
  mandatory for non-SunOS/Solaris/Illumos systems (using an #error
  directive if it is not detected), with the other methods removed.
  The result performs 15-25% faster on macOS and Linux while
  passing all the regression tests.
     On macOS, avoiding the select(2) method fixes the hanging bug.
     On SunOS/Solaris/Illumos (the '__sun' identifier), the method
  using ioctl(2)+I_PEEK and select(2) (iffe feature IDs:
  stream_peek and lib_select) is preserved.

Resolves: https://github.com/ksh93/ksh/issues/118 (again)

[*] https://pubs.opengroup.org/onlinepubs/9699919799/functions/recv.html
2020-08-19 23:54:55 +01:00
Martijn Dekker
569c1bb9c1 Revert "Speed up 'read', fixing macOS hang"
This reverts commit ff385e5a89.
It broke Solaris and illumos. More testing is needed.
2020-08-19 04:10:55 +01:00
Martijn Dekker
ff385e5a89 Speed up 'read', fixing macOS hang
This fixes a hanging bug that could occur on macOS when using the
'read' command to read from a FIFO and encountering end-of-file
without a final newline character. It also makes the 'read' command
perform 15-25% faster on macOS and Linux and maybe other systems.

src/lib/libast/sfio/sfpkrd.c: sfpkrd():
- Get rid of the optional stuff that uses the poll(2) or select(2)
  syscalls. The only thing that is required to avoid regressions is
  the code that was conditional upon the socket_peek feature test,
  which tests for the correct functioning of the recv(2) syscall.
  This has now been made mandatory. The rest now uses what was
  previously a fallback in plain C, resulting in a function that is
  not only more readable, but actually faster than the syscalls.

Resolves: https://github.com/ksh93/ksh/issues/118
2020-08-19 01:36:01 +01:00
Chase
c3388ffd85 nval.h: remove dtksh additions & old compat redefs (re: e2d1b593)
CDE <https://cdesktopenv.sf.net/> developer Chase writes, re dtksh:
| Everything is now completely working, and we are almost ready to
| add ksh93 as a submodule, but I have one last commit to get rid
| of some warnings we are facing. nval.h has some of these
| "compatiblity redefines" that are causing issues whenever we
| include it (warnings about redefining values) [...].

src/cmd/ksh93/include/nval.h:
- Replace ancient compatibility redefines by an unconditional
  '#include <hash.h>'; ksh works fine with the "new" hash library.

Co-authored-by: Martijn Dekker <martijn@inlv.org>
2020-08-17 23:11:51 +01:00
Martijn Dekker
d03e948bcd Fix 'command -p' lookup if hash table entry exists (re: c9ccee86)
If a command's path was previously added to the hash table as a
'tracked alias', then the hash table entry was used, bypassing
the default utility path search activated by 'command -p'.

'command -p' activates a SH_DEFPATH shell state. The bug was caused
by a failure to check for this state before using the hash table.
This check needs to be added in four places.

src/cmd/ksh93/sh/path.c,
src/cmd/ksh93/sh/xec.c:
- path_search(), path_spawn(), sh_exec(), sh_ntfork(): Only consult
  the hash table, which is shp->track_tree, if the SH_DEFPATH shell
  state is not active.

src/cmd/ksh93/tests/path.sh:
- Add regress tests checking that 'command -p' and 'command -p -v'
  still search in the default path if a hash table entry exists for
  the command searched.
2020-08-17 20:23:39 +01:00
Martijn Dekker
56805b25af Fix leak and crash upon defining functions in subshells
A memory leak occurred upon leaving a virtual subshell if a
function was defined within it. If this was done more than 32766
(= 2^15-2 = the 'short' max value - 1) times, the shell crashed.
Discussion and reproducer: https://github.com/ksh93/ksh/issues/114

src/cmd/ksh93/sh/subshell.c: table_unset():
- A subshell-defined function was never freed because a broken
  check for autoloaded functions (which must not be freed[*]). It
  looked for an initial '/' in the canonical path of the script
  file that defined the function, but that path is also stored for
  regular functions. Now use a check that executes nv_search() in
  fpathdict, the same method used in _nv_unset() in name.c for a
  regular function unset.

src/cmd/ksh93/bltins/misc.c: b_dot_cmd():
- Fix an additional memory leak introduced in bd88cc7f, that caused
  POSIX functions (which are run with b_dot_cmd() like dot scripts)
  to leak extra. This fix avoids both the crash fixed there and the
  memory leak by introducing a 'tofree' variable remembering the
  filename to free. Thanks to Johnothan King for the patch.

src/lib/libast/include/stk.h,
src/lib/libast/misc/stk.c,
src/lib/libast/man/stk.3,
src/lib/libast/man/stak.3:
- Make the stack more resilient by extending the stack reference
  counter 'stkref' from (signed) short to unsigned int. On modern
  systems with 32-bit ints, this extends the maximum number of
  elements on a stack from 2^15-1==32767 to 2^32-1==4294967295.
  The ref counter can never be negative, so there is no reason for
  signedness. sizeof(int) is defined as the size of a single CPU
  word, so this should not affect performance at all.
     On a 16-bit system (not that ksh still compiles there), this
  doubles the max number of entries to 2^16-1=65535.

src/cmd/ksh93/tests/leaks.sh:
- Add leak regression tests for ksh functions, POSIX functions, dot
  scripts run with '.', and dot scripts run with 'source'.

src/cmd/ksh93/tests/path.sh:
- Add an output builtin with a redirect to an autoloaded function
  so that a crash[*] is triggered if the check for an autoloaded
  function is ever removed from table_unset(), as was done in ksh
  93v- (which crashed).

[*] Freeing autoloaded functions after leaving a virtual subshell
    causes a crashing bug: https://github.com/att/ast/issues/803

Co-authored-by: Johnothan King <johnothanking@protonmail.com>
Fixes: https://github.com/ksh93/ksh/issues/114
2020-08-14 00:25:31 +01:00
Johnothan King
05ac1dbb41
Fix crash upon running many subshells (#113)
Co-authored-by: Martijn Dekker <martijn@inlv.org>

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

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

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

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

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

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

src/cmd/ksh93/tests/subshell.sh:
- Show wrong exit status in message on failure of 'wait' builtin.
2020-08-12 18:50:59 +01:00
Martijn Dekker
f485fe0f8d rm redundant hardcoded default paths (re: aa4669ad)
As of aa4669ad, astconf("PATH") is implemented as a hardcoded AST
configuration variable that always has a value, instead of one that
falls back on the OS. Its value is now obtained from the OS (with a
fallback) at configure time and not at runtime. This means that any
fallback for astconf("PATH") is now never used.

src/cmd/ksh93/data/msg.c,
src/cmd/ksh93/include/shell.h:
- Remove e_defpath[]. (The path "/bin:/usr/bin:" made no sense as a
  default path anyway, as the final empty element is wrong: default
  utilities should never be sought in the current working dir.)

src/cmd/ksh93/sh/path.c,
src/lib/libast/path/pathbin.c:
- abort() if astconf("PATH") returns null.

src/lib/libast/comp/conf.tab: PATH:
- If no 'getconf' utility can be found, use a fallback path that
  finds more utilities by also searching in 'sbin' directories.
  On some systems, this is needed to find chown(1).

src/cmd/ksh93/sh.1:
- Update doc re default path.
2020-08-11 15:20:10 +01:00
Martijn Dekker
e01801572d printf %H: fix/reduce encoding into entities (re: 8477d2ce)
The &nbsp; entity is not valid in XML, only in HTML. Since we must
be compatible with both, it can't be used. Thanks to Andras Farkas
for the bug report.

In addition, the generation of numeric entities for unprintable
characters was only valid while processing UTF-8 text while in a
UTF-8 locale. In all other conditions it produced invalid results.
This is not worth trying to fix.

Discussion:
https://groups.google.com/d/msgid/korn-shell/CAA0nTRta%3DPbOYduyBv%3DXCzumTcUCU8Lki%3DQQf2O8Erk2BFvO1g%40mail.gmail.com

src/cmd/ksh93/bltins/print.c:
- Remove conversion to &nbsp; entity.
- Remove conversion of non-graph characters to numeric entities.
  Convert only the 5 semantically meaningful characters: < > & " '

src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/string.c:
- We don't need sh_isprint() in print.c anymore, so turn it back
  into a static function.

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

Prior discussion:
https://groups.google.com/d/msgid/korn-shell/ce8d1467-4a6d-883b-45ad-fc3c7b90e681%40inlv.org

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Fixes: https://github.com/ksh93/ksh/issues/103 (again)
2020-08-07 21:09:01 +01:00
Johnothan King
9de65210c6
Add ${.sh.pid} as an alternative to $BASHPID (#109)
This variable is like Bash's $BASHPID, but in virtual subshells
it will retain its previous value as virtual subshells don't fork.
Both $BASHPID and ${.sh.pid} are different from $$ as the latter
is only set to the parent shell's process ID (i.e. it isn't set
to the process ID of the current subshell).

src/cmd/ksh93/include/defs.h:
- Add 'current_pid' for storing the current process ID at a valid
  memory address.
- Change 'ppid' from 'int32_t' to 'pid_t', as the return value from
  'getppid' is of the 'pid_t' data type.

src/cmd/ksh93/data/variables.c,
src/cmd/ksh93/include/variables.h,
src/cmd/ksh93/sh/init.c,
src/cmd/ksh93/sh/xec.c:
 - Add the ${.sh.pid} variable as an alternative to $BASHPID.
   The process ID is stored in a struct before ${.sh.pid} is set
   as environment variables are pointers that must point to a
   valid memory address. ${.sh.pid} is updated by the _sh_fork()
   function, which is called when ksh forks a new process with
   sh_fork() or sh_ntfork().

src/cmd/ksh93/tests/variables.sh:
- Add ${.sh.pid} to the list of special variables and add three
  regression tests for ${.sh.pid}.

src/cmd/ksh93/tests/subshell.sh:
- Update the PATH forking regression test to use ${.sh.pid} and
  remove the TODO note.
2020-08-07 02:53:25 +01:00
Martijn Dekker
338586896d Fix crash: do not list jobs if there is no job control
This bug caused an undefined state, which sometimes crashed the
shell in job_list() or job_unpost(), if $PS1 contains a command
substitution running an external command and the '-b'/'-o notify'
shell option is active. So far the only known way to trigger the
crash is by letting $TMOUT time out the interactive shell. See
https://github.com/ksh93/ksh/issues/103 for details.

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

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

Fixes: https://github.com/ksh93/ksh/issues/103
2020-08-06 22:46:02 +01:00
Johnothan King
49ae483574
Make liblist an extern to fix dtksh compile (#108)
The liblist variable needs to be an extern for dtksh to build.
Quote from CDE developer Chase:
we use an old function that no longer appears in kornshell,
sh_getliblist, it seems to be replaced by the function sh_getlib,
which is fine, but it seems to return a "Shbltin_f" type, which I
can't seem to find any information on what it is. We need the void
pointer dlsym provides for some widget init stuff, I tried making
liblist an extern, but it kept giving me an error about libcomp_t
being undefined.

src/cmd/ksh93/bltins/typeset.c,
src/cmd/ksh93/include/shell.h:
- Fix the compiler error reported above by moving the type definition
  for Libcomp_t to shell.h.
- Make liblist an extern since findsym.c in dtksh needs it to build.
  The old sh_getliblist function doesn't need to be reintroduced
  since the only purpose it served was to workaround the problem
  of liblist being a static variable. Now that liblist is an extern,
  dtksh fsym can use liblist directly to avoid sh_getliblist.

dtksh findsym.c:
https://sourceforge.net/p/cdesktopenv/code/ci/2.3.2/tree/cde/programs/dtksh/findsym.c
2020-08-05 22:18:22 +01:00
Johnothan King
23f2e23385
Over-shifting in a POSIX function should cause scripts to exit (#106)
The required longjmp used to terminate scripts was not being run
when over-shifting in a POSIX function with a redirection. This
caused scripts to continue after an error in the shift builtin,
which is incorrect since shift is a special builtin. The
interpreter is sent into an indeterminate state that causes
undefined behavior as well:
$ cat reproducer.ksh
some_func() {
   shift 10
}

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

$(true << EOF)
EOF

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

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

This bugfix was backported from ksh93v- 2013-10-10-alpha.
2020-07-24 00:03:57 +01:00
Martijn Dekker
f207cd5787 Fix race conditions running external commands with job control on
When ksh is compiled with SHOPT_SPAWN (the default), which uses
posix_spawn(3) or vfork(2) (via sh_ntfork()) to launch external
commands, at least two race conditions occur when launching
external commands while job control is active. See:
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
Johnothan King
e2d1b593ac
Merge dtksh patches from one of the CDE developers (#85)
This merges some fixes to support building dtksh with -DBUILD_DTKSH.
These patches were sent through private email from the CDE developer
Chase. The reason these patches were submitted is because Chase wishes
to include ksh in CDE as an up-to-date git submodule. Quote from Chase:
"... my priority is to get your new version into our code as a git
 submodule, and do it quickly before our code bases differ too widely."

Link to CDE project for anyone interested:
https://sourceforge.net/projects/cdesktopenv/

Although the patches were privately discussed, there are some public
emails on the CDE mailing list (links shortened due to long URLs):
ksh-chaos thread:   https://bit.ly/3hjJ83b
dtksh alias thread: https://bit.ly/3hkzKfJ

The main fix is for suid_exec, which is now told that /usr/dt is a
valid directory to run from via preprocessor flags. A patch for
Shift-JIS was also submitted, but it isn't in this commit because it
isn't an effective fix for the existing Shift-JIS bugs. I will be
giving that patch some more testing.

From: Chase <nicetrynsa@protonmail.ch>
Co-authored by: Johnothan King <johnothanking@protonmail.com>
2020-07-22 06:44:24 +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