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

56 commits

Author SHA1 Message Date
Martijn Dekker
cda661d34c Various cleanups, mostly in regression tests
src/cmd/ksh93/data/variables.sh: shtab_variables[]:
- Remove unused "CSWIDTH" entry. All use of it (including the
  matching CSWIDTHNOD macro) was removed in version 2003-04-22.

src/cmd/ksh93/tests/variables.sh:
- For the tests on the shtab_variables[] variables, read the
  variable names straight from variables.c instead of synching
  the list in the test script, which would surely be forgotten.

src/cmd/ksh93/tests/*.sh:
- Fix a number of mistaken tries to count errors from a subshell.
- Fix miscellaneous minor breakage and typos.
2022-01-24 02:58:25 +00:00
Martijn Dekker
e569f23ef9 bump internal libast version; various minor cleanups
These are minor things I accumulated over the last month or so.

Notable changes:

src/lib/libast/features/api,
src/lib/libast/misc/state.c,
src/lib/libast/comp/conf.tab,
src/cmd/ksh93/include/defs.h:
- Bump internal libast version to 20220101L. We've made a few
  additions to the API, at least pathicase (see 71934570, ca3ec200)
  and astconf_long (see c2ac69b2), so this should have been done
  already. This also updates '/opt/ast/bin/getconf _AST_VERSION'.
- Use AST_VERSION instead of outdated _AST_VERSION.
- In state.c, use AST_VERSION instead of hardcoding the version.

src/cmd/ksh93/sh/xec.c:
- Remove 'restorefd' variable, unused as of 42becab6.
- Remove 'cmdrecurse' function and SH_RUNPROG macro; this was once
  used by a few libcmd commands, but ast-open-archive reveals it's
  unused as of ast 1999-12-25.

src/cmd/ksh93/sh/*.c:
- Where available, use e_dot instead of "." for consistency; it is
  defined as an extern so we might as well use it.

src/cmd/ksh93/tests/*.sh:
- When reporting signal names in fails, include the SIG prefix.
- Fix a broken process hang test in subshell.sh.

src/lib/libast/man/sfdisc.3:
- Removed. The interfaces described here never made it out of AT&T;
  they do not exist in any libast version in ast-open-archive.
  Resolves: https://github.com/ksh93/ksh/issues/426
2022-01-14 19:55:35 +00:00
Johnothan King
de795e1f9d Allow regression tests to pass without any /opt/ast/bin builtins (#403)
This commit primarily makes changes that allow the regression tests to
run without any of the /opt/ast/bin builtins compiled into ksh. It also
makes a minor improvement to one of the tests in locale.sh by
shellquoting an error message.

Co-authored-by: Martijn Dekker <martijn@inlv.org>
2021-12-28 17:52:57 +00:00
Martijn Dekker
db3a3d8fc0 tests/leaks.sh: redesign with a more robust testing algorithm
On modern operating systems, memory management is non-deterministic
(i.e. random, unpredictable) to varying degrees. This makes testing
for memory leaks a nightmare as the OS may decide to randomly grow
a process's memory allocation at any time for no apparent reason,
causing intermittent test failures that do not represent real
memory leaks. So far, the leaks test tried to cope with this by
using a large number of iterations plus a certain amount of bytes
of tolerance per iteration. This was inefficient and on some
systems still did not fully eliminate intermittent test failures.

This commit introduces a new testing algorithm that is designed to
cope with a large degree of unpredictability. Instead of a fixed
number of test iterations, it defines a maximum (16384), dividing
them in blocks of 128 iterations. It also defines a minimum number
of sequential "good" iteration blocks, counted if memory usage did
not increase from one block to the next. That minimum number is set
to 16. The theory is that if we can get 16 "good" iteration blocks
in a row, we can safely assume it's not a real memory leak, break
the loop, and consider the test succeeded. That "good" sequence is
allowed to occur at any point in the loop, creating a high built-in
tolerance for non-deterministic shenanigans. It also speeds up the
tests, as successful tests can bow out at 16 * 128 == 2048
iterations if they're lucky. If the OS decides to randomly grow the
memory heap, it may take more tries, but almost (?) certainly not
more than the maximum 16384 (128 blocks). If the counter reaches
that, then we assume a memory leak and throw a test failure.

We're also no longer testing with byte granularity in any case; the
randomness of memory management makes that pointless. All getmem()
function versions now return kibibytes (1024 bytes).

This should eliminate the need for workarounds such as initial
iterations to "steady the state" or a tolerance of a certain number
of bytes. I've experimentally determined the exact values
(max_iter, block_iter, min_good_blocks) that seem to work reliably
on all systems I've tested. They are easy to tweak if necessary.

To make all this manageable, this commit hides all the supporting
code in a triplet of aliases (TEST, DO, DONE) that, when used
correctly, create a grammatically robust shell code block: you can
add redirections, pipe into it, etc. as expected. This makes the
actual tests a great deal easier to read as well.

src/cmd/ksh93/tests/pty.sh:
- Implement new leaks testing framework as described and convert
  all the tests to it.
- Mark known leaks with a 'known' variable. Print non-fail warnings
  for all known leaks, but skip the tests by default. Test them
  only if DEBUG is exported. This is better than commenting them
  out as we will no longer be tempted to forget about these.
- Move the test for large command substitutions to subshell.sh --
  it's not in fact a leak test; instead, it checks that command
  substitutions don't lose data.

src/cmd/ksh93/tests/_common: err_exit():
- Since we're printing more warnings, clearly mark all test
  failures with 'FAIL:' to make them stand out.

src/cmd/ksh93/tests/shtests.
src/cmd/ksh93/tests/pty.sh:
- Special-case leaks.sh for counting tests; grep ^TEST.
- Special-case pty.sh as well while we're at it by grepping tst()
  calls. Remove all the dummy '# err_exit #' comments from pty.sh
  as they are now no longer used for counting the tests.
2021-12-28 17:47:29 +00:00
Johnothan King
beccb93fd4 Fix various compiler warnings and minor issues (#362)
List of changes:
- Fixed some -Wuninitialized warnings and removed some unused variables.

- Removed the unused extern for B_login (re: d8eba9d1).

- The libcmd builtins and the vmalloc memfatal function now handle
  memory errors with 'ERROR_SYSTEM|ERROR_PANIC' for consistency with how
  ksh itself handles out of memory errors.

- Added usage of UNREACHABLE() where it was missing from error handling.

- Extend many variables from short to int to prevent overflows (most
  variables involve file descriptors).

- Backported a ksh2020 patch to fix unused value Coverity issues
  (https://github.com/att/ast/pull/740).

- Note in src/cmd/ksh93/README that ksh compiles with Cygwin on
  Windows 10 and Windows 11, albeit with many test failures.

- Add comments to detail some sections of code. Extensive list of
  commits related to this change:
  ca2443b5, 7e7f1372, 2db9953a, 7003aba4, 6f50ff64, b1a41311,
  222515bf, a0dcdeea, 0aa9e03f, 61437b27, 352e68da, 88e8fa67,
  bc8b36fa, 6e515f1d, 017d088c, 035a4cb3, 588a1ff7, 6d63b57d,
  a2f13c19, 794d1c86, ab98ec65, 1026006d

- Removed a lot of dead ifdef code.

- edit/emacs.c: Hide an assignment to avoid a -Wunused warning. (See
  also https://github.com/att/ast/pull/753, which removed the assignment
  because ksh2020 removed the !SHOPT_MULTIBYTE code.)

- sh/nvdisc.c: The sh_newof macro cannot return a null pointer because
  it will instead cause the shell to exit if memory cannot be allocated.
  That makes the if statement here a no-op, so remove it.

- sh/xec.c: Fixed one unused variable warning in sh_funscope().

- sh/xec.c: Remove a fallthrough comment added in commit ed478ab7
  because the TFORK code doesn't fall through (GCC also produces no
  -Wimplicit-fallthrough warning here).

- data/builtins.c: The cd and pwd man pages state that these builtins
  default to -P if PATH_RESOLVE is 'physical', which isn't accurate:
     $ /opt/ast/bin/getconf PATH_RESOLVE
     physical
     $ mkdir /tmp/dir; ln -s /tmp/dir /tmp/sym
     $ cd /tmp/sym
     $ pwd
     /tmp/sym
     $ cd -P /tmp/sym
     $ pwd
     /tmp/dir
  The behavior described by these man pages isn't specified in the ksh
  man page or by POSIX, so to avoid changing these builtin's behavior
  the inaccurate PATH_RESOLVE information has been removed.

- Mamfiles: Preserve multi-line errors by quoting the $x variable.
  This fix was backported from 93v-.
  (See also <a7e9cc82>.)

- sh/subshell.c: Remove set but not used sp->errcontext variable.
2021-12-09 06:42:59 +01:00
Martijn Dekker
b3050769ea Fix 'return' emitting signals; allow arbitrary return values
When a global EXIT trap is set, and a ksh-style function exits with
a status > 256 that could have been the result of a signal, then
the shell incorrectly issues that signal to itself. Depending on
the signal, this causes ksh to terminate itself ungracefully:

    $ cat /tmp/exit267
    trap 'echo OK' EXIT  # This trap triggers the crash
    function foo { return 267; }
    foo
    $ bash /tmp/exit267
    OK
    $ ksh-3aee10d7 /tmp/exit267
    OK
    $ ksh /tmp/exit267
    Memory fault(coredump)

On most systems, status 267 corresponds to SIGSEGV. The reported
memory fault is not real; it results from ksh incorrectly killing
itself with that signal.

The problem is caused by two factors:

1. As of 93u+ 2012-08-01, ksh explicitly allows 'return' to use an
   exit status corresponding to a signal (from 257 to end of signal
   range). The rest of the integer range is trunctated to 8 bits.
   This is contrary to both 'man ksh' and 'return --man' which both
   say it's always truncated to 8 bits. Plus, combined with point 2
   below, this new behaviour is nonsensical, as 'return' has no
   business actually generating signals. However, a couple of
   regression tests now depend on this, as may some scripts.

2. When a ksh-style function does not handle a signal, the signal
   is passed down to the parent environment and ksh does this by
   reissuing the signal to its own process after leaving the
   function scope. However, it does this by checking the exit
   status, which is very bad practice as there is no guarantee
   that an exit status corresponding to a signal was in fact
   produced by a signal, particularly after they changed the
   behaviour of 'return' per 1 above.

This commit fixes both issues. It also takes a proper decision on
allowable 'return' exit status arguments. Since 93u+ was released
nearly a decade ago and some scripts may now rely on being able to
pass certain exit statuses out of the 8-bit range, we should not
disallow this now. But neither should we be half-hearted in
allowing only some arbitrary selection of 9-bit statuses; 'return'
values categorically should have nothing to do with signals, so
this is no basis for limiting them. We're now allowing the full
unsigned integer range, which is usually 32 bits. This is like zsh,
and may create some interesting possibilities for scripts.
Just don't forget that $? will still lose all but its 8 least
significant bits when leaving the current (sub)shell environment.

src/cmd/ksh93/sh/xec.c: sh_funscope():
- Fix passing down unhandled signals from interrupted ksh functions
  (jumpval==SH_JMPFUN) to the parent environment. Do not pay any
  attention to the exit status. Instead, use sh.lastsig (a.k.a.
  shp->lastsig). It is set by sh_fault() in fault.c for just this
  purpose and contains the last signal handled for the current
  command. It is reset in sh_exec() before running any new command.
  So if it contains a signal, that is the one that interrupted the
  ksh function, so it's the correct one to pass down. (Further
  evidence: sh_subshell() was already using this in the same way.)

src/cmd/ksh93/bltins/cflow.c: b_return():
- Allow any signed int return value when invoked as and behaving
  like 'return'.
- Add warning if a passed value is out of int range. Set the exit
  status to 128 in that case; int overflow is undefined behaviour
  in C and we want consistent behaviour across platforms. It should
  be safe enough to check if the long and int values are equal.
- Refactor for clarity.

src/cmd/ksh93/sh/subshell.c: sh_subshell():
- If a function returns with a status out of the 8 bit range in a
  virtual subshell, this status could be passed down to the parent
  shell in full. However, if the subshell forks, then the kernel
  will enforce an 8-bit exit status. That is inconsistent. Scripts
  should not be able to tell the difference between forked and
  non-forked subshells, so artificially enforce that limit here.

Other changed files:
- Documentation updates and copy-edits.
- Update an AT&T functions.sh regress test to allow arbitrary
  integer return values for functions.
- Add regression tests based in part on @JohnoKing's reproducers.
- Rework some vaguely related regression tests to fail gracefully.

Thanks to Johnothan King for the report and the testing.

Fixes: https://github.com/ksh93/ksh/issues/364
2021-12-09 06:41:39 +01:00
Martijn Dekker
a2196f9434 Fix backtick comsubs by making them act like $(modern) ones
ksh93 currently has three command substitution mechanisms:
- type 1: old-style backtick comsubs that use a pipe;
- type 3: $(modern) comsubs that use a temp file, currently with
  fallback to a pipe if a temp file cannot be created;
- type 2: ${ shared-state; } comsubs; same as type 3, but shares
  state with parent environment.

Type 1 is buggy. There are at least two reproducers that make it
hang. The Red Hat patch applied in 4ce486a7 fixed a hang in
backtick comsubs but reintroduced another hang that was fixed in
ksh 93v-. So far, no one has succeeded in making pipe-based comsubs
work properly.

But, modern (type 3) comsubs use temp files. How does it make any
sense to have two different command substitution mechanisms at the
execution level? The specified functionality between backtick and
modern command substitutions is exactly the same; the difference
*should* be purely syntactic.

So this commit removes the type 1 comsub code at the execution
level, treating them all like type 3 (or 2). As a result, the
related bugs vanish while the regression tests all pass.

The only side effect that I can find is that the behaviour of bug
https://github.com/ksh93/ksh/issues/124 changes for backtick
comsubs. But it's broken either way, so that's neutral.

So this commit can now be added to my growing list of ksh93 issues
fixed by simply removing code.

src/cmd/ksh93/sh/xec.c:
- Remove special code for type 1 comsubs from iousepipe(),
  sh_iounpipe(), sh_exec() and _sh_fork().

src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/subshell.c:
- Remove pipe support from sh_subtmpfile(). This also removes the
  use of a pipe as a fallback for $(modern) comsubs. Instead, panic
  and error out if temp file creation fails. If the shell cannot
  create a temporary file, there are fatal system problems anyway
  and a script should not continue.
- No longer pass comsub type to sh_subtmpfile().

All other changes:
- Update sh_subtmpfile() calls.

src/cmd/ksh93/tests/subshell.sh:
- Add two regression tests based on reproducers from bug reports.

Resolves: https://github.com/ksh93/ksh/issues/305
Resolves: https://github.com/ksh93/ksh/issues/316
2021-08-13 09:14:11 +02:00
Martijn Dekker
c59d888394 Fix xtrace (shtests -x) for the regression tests
src/cmd/ksh93/tests/{shtests,_common}:
- When xtrace is active, set SECONDS to the float type so that
  the $SECONDS expansion in $PS4 shows fractional seconds.

src/cmd/ksh93/tests/*.sh:
- Various fixes to avoid command substitutions incorporating xtrace
  output into their results. Sometimes this is done by avoiding a
  preceding assignment on a command that redirects 2>&1 (as that
  will also redirect the preceding assignment and its xtrace,
  causing the command substitution to capture the xtrace); other
  times it was easiest to just turn off xtrace outright within the
  command substitution.

src/cmd/ksh93/tests/math.sh:
- Remove an obsolete 'fixme' note.
2021-05-14 19:56:11 +02:00
Martijn Dekker
1a1e3709c2 tests/subshell.sh: replace use of unportable seq command 2021-05-03 21:51:03 +01:00
Martijn Dekker
af6a32d14f
Fix $RANDOM to act consistently in subshells (#294)
This fixes the following:
1. Using $RANDOM in a virtual/non-forked subshell no longer
   influences the reproducible $RANDOM sequence in the parent
   environment.
2. When invoking a subshell $RANDOM is now re-seeded (as mksh and
   bash do) so that invocations in repeated subshells (including
   forked subshells) longer produce identical sequences by default.
3. Program flow corruption that occurred in scripts on executing
   ( ( simple_command & ) ).

src/cmd/ksh93/include/variables.h:
- Move 'struct rand' here as it will be needed in subshell.c. Add
  rand_seed member to save the pseudorandom generator seed. Remove
  the pointer to the shell state as it's redundant.

src/cmd/ksh93/sh/init.c:
- put_rand(): Store given seed in rand_seed while calling srand().
  No longer pointlessly limit the number of possible seeds with the
  RANDMASK bitmask (that mask is to limit the values to 0-32767,
  it should not limit the number of possible sequences to 32768).
- nget_rand(): Instead of using rand(), use rand_r() to update the
  random_seed value. This makes it possible to save/restore the
  current seed of the pseudorandom generator.
- Add sh_reseed_rand() function that reseeds the pseudorandom
  generator by calling srand() with a bitwise-xor combination of
  the current PID, the current time with a granularity of 1/10000
  seconds, and a sequence number that is increased on each
  invocation.
- nv_init(): Set the initial seed using sh_reseed_rand() here
  instead of in sh_main(), as this is where the other struct rand
  members are initialised.

src/cmd/ksh93/sh/main.c: sh_main():
- Remove the srand() call that was replaced by the sh_reseed_rand()
  call in init.c.

src/cmd/ksh93/sh/subshell.c: sh_subshell():
- Upon entering a virtual subshell, save the current $RANDOM seed
  and state, then reseed $RANDOM for the subshell.
- Upon exiting a virtual subshell, restore $RANDOM seed and state
  and reseed the generator using srand() with the restored seed.

src/cmd/ksh93/sh/xec.c: sh_exec():
- When optimizing out a subshell that is the last command, still
  act like a subshell: reseed $RANDOM and increase ${.sh.subshell}.
- Fix a separate bug discovered while implementing this. Do not
  optimize '( simple_command & )' when in a virtual subshell; doing
  this causes program flow corruption.
- When optimizing '( simple_command & )', also reseed $RANDOM and
  increment ${.sh.subshell}.

src/cmd/ksh93/tests/subshell.sh,
src/cmd/ksh93/tests/variables.sh:
- Add various tests for all of the above.

Co-authored-by: Johnothan King <johnothanking@protonmail.com>
Resolves: https://github.com/ksh93/ksh/issues/285
2021-05-03 04:03:46 +01:00
Martijn Dekker
88a1f3d661 Fork before entering shared-state command substitution
The code contains various checks to see if a subshell needs to
fork, like this one in the ulimit builtin:

	if(shp->subshell && !shp->subshare)
		sh_subfork();

All checks of this form are fatally broken, as each one of them
causes shared-state command substitutions to ignore parent virtual
subshells.

Currently the only feasible way to fix this is to fork a virtual
subshell before executing a shared-state command substitution in
it. In the long term I think shared-state command substitutions
should probably be redesigned to disassociate them completely from
the virtual subshell mechanism.

src/cmd/ksh93/sh/macro.c: comsubst():
- If we're in a non-subshare virtual subshell, fork it before
  entering a type 2 (subshare) command substitution.

src/cmd/ksh93/sh/subshell.c:
- sh_assignok(): Remove subshare fix from 911d6b06 as it's
  redundant now that the parent of a subshare is never a virtual
  subshell. Go back to not doing anything if the current "subshell"
  is a subshare.
- sh_subtracktree(), sh_subfuntree(): Similarly, remove the
  now-redundant subshare fixes from 13c57e4b.

src/cmd/ksh93/sh/xec.c: sh_exec():
- Fix a separate bug: only fork a virtual subshell before running a
  background job if that "subshell" is not a subshare.

src/cmd/ksh93/tests/subshell.sh:
- Add test for bug fixed in xec.c.
- Add tests for 'ulimit', 'builtin' and 'exec' run in subshare
  within subshell -- all commands that use checks of the form
  'if(sh.subshell && !sh.subshare) sh_subfork();'.

Resolves: https://github.com/ksh93/ksh/issues/289
2021-05-01 00:47:39 +01:00
Martijn Dekker
090b65e79b Fix fork after redirecting stdout in subshare (re: 500757d7)
Previously, command substitutions executed as virtual subshells
were always forked if any command was run within them that
redireceted standard output, even if the redirection was local to
that command.

Commit 500757d7 removed the check for a shared-state command
substitution (subshare), so introduced a bug where even that would
fork, causing it to stop sharing its state.

We can further improve on that fix by only forking if the
redirection is permanent as with `exec` or `redirect`. There should
be no need to do that if the redirection is local to a command run
within the command substitution, as the file descriptor is restored
when that command finishes, which is still within the command
substitution.

src/cmd/ksh93/sh/io.c: sh_redirect():
- Only fork upon redirecting stdout if the virtual subshell is a
  command substitution, and if the redirection is permanent
  (flag==1 or flag==2).
2021-04-26 18:22:17 +01:00
Martijn Dekker
2aad3cab06 Add ksh 93u+m contributors notice to 964 copyright headers 2021-04-26 00:19:31 +01:00
Johnothan King
24b7fcb771
Don't detect unset functions with whence (re: 13c57e4b) (#287)
src/cmd/ksh93/bltins/whence.c:
- The previous commit that fixed 'unset -f' in virtual subshells left
  one bug. The type builtin (or 'whence -v') could still find the unset
  function in virtual subshells:
    $ foo() { echo foo; }
    $ (unset -f foo; type foo)
    foo is an undefined function
  To fix this bug, avoid detecting functions in the whence builtin
  unless they have the NV_FUNCTION flag.

src/cmd/ksh93/tests/subshell.sh:
- Add a regression test for using 'type' on a function unset inside of
  a virtual subshell.
2021-04-24 21:25:45 +01:00
Martijn Dekker
07faf38425 Fix 'unset -f' in sub-subshell (re: 13c57e4b)
A bug introduced in the previous commit caused 'unset -f' in a
subshell of a subshell to fail to unset a function created in a
parent subshell. Reproducer:

$ ( f2() { echo WRONG; }; ( unset -f f2; f2 ) )
WRONG

src/cmd/ksh93/bltins/typeset.c: unall():
- Do not nv_search() in sh.fun_base before setting the dummy node
  that marks the function as unset in this subshell. That search
  only reaches the base tree and not any of its subtrees. Setting
  the dummy unconditionally is not harmful; the cost is negligible.

src/cmd/ksh93/tests/subshell.sh:
- Add test for the bug.
2021-04-24 12:41:40 +01:00
Martijn Dekker
13c57e4b58 Fix 'unset -f' to work in subshells without forking (re: 047cb330)
This commit implements unsetting functions in virtual subshells,
removing the need for the forking workaround. This is done by
either invalidating the function found in the current subshell
function tree by unsetting its NV_FUNCTION attribute bits (which
will cause sh_exec() to skip it) or, if the function exists in a
parent shell, by creating an empty dummy subshell node in the
current function tree without that attribute.

As a beneficial side effect, it seems that bug 228 (unset -f fails
in forked subshells if a function is defined before forking) is now
also fixed.

src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/init.c:
- Add sh.fun_base for a saved pointer to the main shell's function
  tree for checking when in a subshell, analogous to sh.var_base.

src/cmd/ksh93/bltins/typeset.c: unall():
- Remove the fork workaround.
- When unsetting a function found in the current function tree
  (troot) and that tree is not sh.var_base (which checks if we're
  in a virtual subshell in a way that handles shared-state command
  substitutions correctly), then do not delete the function but
  invalidate it by unsetting its NV_FUNCTION attribute bits.
- When unsetting a function not found in the current function tree,
  search for it in sh.fun_base and if found, add an empty dummy
  node to mask the parent shell environment's function. The dummy
  node will not have NV_FUNCTION set, so sh_exec() will skip it.

src/cmd/ksh93/sh/subshell.c:
- sh_subfuntree(): For 'unset -f' to work correctly with
  shared-state command substitutions (subshares), this function
  needs a fix similar to the one applied to sh_assignok() for
  variables in commit 911d6b06. Walk up on the subshells tree until
  we find a non-subshare.
- sh_subtracktree(): Apply the same fix for the hash table.
- Remove table_unset() and incorporate an updated version of its
  code in sh_subshell(). As of ec888867, this function was only
  used to clean up the subshell function table as the alias table
  no longer exists.
- sh_subshell():
  * Simplify the loop to free the subshell hash table.
  * Add table_unset() code, slightly refactored for readability.
    Treat dummy nodes now created by unall() separately to avoid a
    memory leak; they must be nv_delete()d without passing the
    NV_FUNCTION bits. For non-dummy nodes, turn on the NV_FUNCTION
    attribute in case they were invalidated by unall(); this is
    needed for _nv_unset() to free the function definition.

src/cmd/ksh93/tests/subshell.sh:
- Update the test for multiple levels of subshell functions to test
  a subshare as well. While we're add it, add a very similar test
  for multiple levels of subshell variables that was missing.
- Add @JohnoKing's reproducer from #228.

src/cmd/ksh93/tests/leaks.sh:
- Add leak tests for unsetting functions in a virtual subshell.
  Test both the simple unset case (unall() creates a dummy node)
  and the define/unset case (unall() invalidates existing node).

Resolves: https://github.com/ksh93/ksh/issues/228
2021-04-24 06:57:49 +01:00
Martijn Dekker
feaf718f16 More fixes for subshell directory handling (re: 7bab9508, 5ee290c7)
This commit fixes what are hopefully the two final aspects of #153:

1. If the present working directory does not exist (was moved or
   deleted) upon entering a virtual subshell, no PWD directory path
   is saved. Since restoring the state after exiting a virtual
   subshell is contingent on a previous PWD path existing, this
   resulted in entire aspects of the virtual subshell, such as the
   subshell function tree, not being cleaned up.
2. A separate problem is that 'cd ..' does not update PWD or OLDPWD
   when run from a nonexistent directory.

A reproducer exposing both problems is:

$ mkdir test
$ cd test
$ ksh -c '(subfn() { BAD; }; cd ..; echo subPWD==$PWD);
			typeset -f subfn; echo mainPWD==$PWD'
subPWD==/usr/local/src/ksh93/ksh/test
subfn() { BAD; };mainPWD==/usr/local/src/ksh93/ksh/test

Expected output:
subPWD==/usr/local/src/ksh93/ksh
mainPWD==/usr/local/src/ksh93/ksh/test

src/cmd/ksh93/bltins/cd_pwd.c:
- If path_pwd() fails to get the PWD (usually it no longer exists),
  don't set $OLDPWD to '.' as that is pointless; use $PWD instead.
  After cd'ing from a nonexistent directory, 'cd -' *should* fail
  and should not be equivalent to 'cd .'.
- Remove a redundant check for (!oldpwd) where it is always set.
- Do not prematurely return without setting PWD or OLDPWD if
  pathcanon() fails to canonicalise a nonexistent directory.
  Instead, fall back to setting PWD to the result of getcwd(3).

src/cmd/ksh93/sh/subshell.c:
- Minor stylistic adjustment. Some NULL macros sneaked in. This
  historic code base does not use them (yet); change to NIL(type*).
- sh_subshell(): Fix logic for determining whether to save/restore
  subshell state.
  1. When saving, 'if(!comsub || !shp->subshare)' is redundant;
     'if(!shp->subshare)' should be enough. If we're not in a
     subshare, state should be saved.
  2. When restoring, 'if(sp->shpwd)' is just nonsense as there is
     no guarantee that the PWD exists upon entering a subshell.
     Simply use the same 'if(!shp->subshare)'. Add an extra check
     for sp->pwd to avoid a possible segfault. Always restore the
     PWD on subshell exit and not only if shp->pwd is set.
- sh_subshell(): Issue fatal errors in libast's "panic" format.

src/cmd/ksh93/tests/builtins.sh:
- Adjust a relevant test to run err_exit() outside of the subshell
  so that any error is counted in the main shell.
- Add test for problem 2 described at the top.

src/cmd/ksh93/tests/subshell.sh:
- Add test for problems 1 and 2 based on reproducer above.

Resolves: https://github.com/ksh93/ksh/issues/153
2021-04-19 00:06:43 +01:00
Johnothan King
814b5c6890
Fix various minor problems and update the documentation (#237)
These are minor fixes I've accumulated over time. The following
changes are somewhat notable:

- Added a missing entry for 'typeset -s' to the man page.
- Add strftime(3) to the 'see also' section. This and the date(1)
  addition are meant to add onto the documentation for 'printf %T'.
- Removed the man page the entry for ksh reading $PWD/.profile on
  login. That feature was removed in commit aa7713c2.
- Added date(1) to the 'see also' section of the man page.
- Note that the 'hash' command can be used instead of 'alias -t' to
  workaround one of the caveats listed in the man page.
- Use an 'out of memory' error message rather than 'out of space'
  when memory allocation fails.
- Replaced backticks with quotes in some places for consistency.
- Added missing documentation for the %P date format.
- Added missing documentation for the printf %Q and %p formats
  (backported from ksh2020: https://github.com/att/ast/pull/1032).
- The comments that show each builtin's options have been updated.
2021-03-21 14:39:03 +00:00
Martijn Dekker
844e6b2410 ...and now make it work with shcomp (re: aed5c6d7) 2021-03-13 19:27:15 +00:00
Martijn Dekker
aed5c6d70a Regress tests: keep common code in one place
src/cmd/ksh93/tests/_common:
- Added. This keeps one common version of 'err_exit', 'warning',
  and other init code.

src/cmd/ksh93/tests/*.sh:
- Source _common as a dot script.
- Remove 50-odd, occasionally slightly different, versions of the
  common code.
- Some minor tweaks.
2021-03-13 18:39:20 +00:00
Martijn Dekker
350b52ea4e Update comsub-with-alias anti-leak hack (re: fe20311f)
In the 93v- beta, they add a newline instead of a space.
This has fewer side effects as final newlines get stripped.
It's still a hack and it would still be nice to have a real fix,
but it seems even the AT&T guys couldn't come up with one.

src/cmd/ksh93/sh/macro.c:
- To somehow avoid a memory leak involving alias substitution,
  append a linefeed instead of a space to the comsub buffer.

src/cmd/ksh93/tests/subshell.sh:
- Add test for minor regression caused by the RedHat version.
2021-02-18 23:35:20 +00:00
Martijn Dekker
911d6b066f Fix subshell scoping of changes in shared command substitution
A ${ shared-state command substitution; } (internally called
subshare) is documented to share its state with the parent shell
environment, so all changes made within the command substitution
survive outside of it. However, when it is run within a
virtual/non-forked subshell, variables that are not already local
to that subshell will leak out of it into the grandparent state.
Reproducer:

	$ ksh -c '( v=${ bug=BAD; } ); echo "$bug"'
	BAD

If the variable pre-exists in the subshell, the bug does not occur:

	$ ksh -c '( bug=BAD1; v=${ bug=BAD2; } ); echo "$bug"'
	(empty line, as expected)

The problem is that the sh_assignok() function, which is
responsible for variable scoping in virtual subshells, does not
ever bother to create a virtual subshell scope for a subshare.
That is an error if a subshare's parent (or higher-up ancestor)
environment is a virtual subshell, because a scope needs to be
created in that parent environment if none exists.

To make this bugfix possible, first we need to get something out of
the way. nv_restore() temporarily sets the subshell's pointer to
the preesnt working directory, shpwd, to null. This causes
sh_assignok() to assume that the subshell is a subshare (because
subshares don't store their own PWD) and refuse to create a scope.
However, nv_restore() sets it to null for a different purpose: to
temporarily disable scoping for *all* virtual subshells, making
restoring possible. This is a good illustration of why it's often
not a good idea to use the same variable for unrelated purposes.

src/cmd/ksh93/sh/subshell.c:
- Add a global static subshell_noscope flag variable to replace the
  misuse of sh.shpwd described above.
- sh_assignok():
  . Check subshell_noscope instead of shpwd to see if scope
    creation is disabled. This makes it possible to distinguish
    between restoring scope and handling subshares.
  . If the current environment is a subshare that is in a virtual
    subshell, create a scope in the parent subshell. This is done
    by temporarily making the parent virtual subshell the current
    subshell (by setting the global subshell_data pointer to it)
    and calling sh_assignok() again, recursively.
- nv_restore(): To disable subshell scope creation while restoring,
  set subshell_noscope instead of saving and unsetting sh.shpwd.

src/cmd/ksh93/tests/subshell.sh:
- Add tests. I like tests. Tests are good.

Fixes: https://github.com/ksh93/ksh/issues/143
2021-02-17 15:33:48 +00:00
Martijn Dekker
5ee290c7a8 cd: Fork if $PWD exists but is not actually the PWD (re: d1483150)
Commit d1483150 did not fully fix #153.
Test case from Harald van Dijk that was still failing:

$ mkdir test
$ cd test
$ rmdir $PWD
$ mkdir $PWD
$ ksh -c "(cd /); pwd"
/

Forking a virtual subshell in that case is needed to avoid ending
up in a directory that replaced the PWD, because it will not be
possible for a process to change back to the original directory.

src/cmd/ksh93/bltins/cd_pwd.c:
- When deciding whether to fork, instead of attempting to opendir
  the PWD, compare the inodes $PWD and "." to determine if $PWD
  still actually refers to the current directory. This uses the
  test_inode() function which is also used by 'test foo -ef bar'.

src/cmd/ksh93/tests/subshell.sh:
- Add test based on the above.

Progresses: https://github.com/ksh93/ksh/issues/153
2021-01-20 05:56:38 +00:00
Martijn Dekker
7bab9508aa Fix crash on subshell exit if PWD is inaccessible (re: dd9bc229)
This commit also further mitigates the problems with restoring an
inaccessible or nonexistent PWD on exiting a virtual subshell.

Harald van Dijk writes:
> On a build of ksh with -fsanitize=undefined to help diagnose
> problems:
>
> $ mkdir deleted
> $ cd deleted
> $ rmdir ../deleted
> $ ksh -c '(cd /; (cd /)); :'
> /home/harald/ksh/src/cmd/ksh93/sh/subshell.c:561:22: runtime
> error: null pointer passed as argument 1, which is declared to
> never be null
> Segmentation fault (core dumped)
>
> Note that it segfaults the same with default compilation flags,
> but it does not print out the useful extra message. The code
> assumes that pwd is non-null and passes it to strcmp without
> checking, but it will be null if the current directory cannot be
> determined, for instance because it has been deleted.

src/cmd/ksh93/sh/subshell.c: sh_subshell():
- Avoid the null pointer dereference reported above.

src/cmd/ksh93/bltins/cd_pwd.c: b_cd():
- Fork a virtual subshell even on systems with fchdir(2) if the
  present working directory tests as inaccessible on invoking 'cd';
  it may no longer exist and fchdir would fail to get a handle.
  (For the test we have to opendir(3) the full path to the PWD and
  not ".", as the latter may succeed even if the PWD is gone.)

src/cmd/ksh93/data/builtins.c:
- Update 'cd' version string.

Fixes:   https://github.com/ksh93/ksh/issues/153
Related: https://github.com/ksh93/ksh/issues/141
2021-01-19 18:47:41 +00:00
Martijn Dekker
ab98ec65e4 Replace safe FD fix with Solaris/ksh2020 version (re: 045fe6a1)
This pulls a new version of sh_iosafefd() from:
https://github.com/oracle/solaris-userland/blob/master/components/ksh93/patches/285-30771135.patch

It was written by Kurtis Rader for ksh2020:
https://github.com/att/ast/issues/198
https://github.com/att/ast/pull/199
It is presumably better than the Red Hat version and also comes
with more regression test cases (although it still doesn't fix
modernish BUG_CSUBSTDO, which remains in the TODO file).

This commit does not go along with other peripheral changes from
that patch, i.e. a different name and location of this function.

src/cmd/ksh93/sh/io.c:
- Replace sh_iosafefd() as above.

src/cmd/ksh93/tests/subshell.sh:
- Add and tweak tests from the patch.
2021-01-08 16:35:26 +00:00
Martijn Dekker
222515bf08 Implement hash tables for virtual subshells (re: 102868f8, 9d428f8f)
The forking fix implemented in 102868f8 and 9d428f8f, which stops
the main shell's hash table from being cleared if PATH is changed
in a subshell, can cause a significant performance penalty for
certain scripts that do something like

    ( PATH=... command foo )

in a subshell, especially if done repeatedly. This is because the
hash table is cleared (and hence a subshell forks) even for
temporary PATH assignments preceding commands.

It also just plain doesn't work. For instance:

    $ hash -r; (ls) >/dev/null; hash
    ls=/bin/ls

Simply running an external command in a subshell caches the path in
the hash table that is shared with a main shell. To remedy this, we
would have to fork the subshell before forking any external
command. And that would be an unacceptable performance regression.

Virtual subshells do not need to fork when changing PATH if they
get their own hash tables. This commit adds these. The code for
alias subshell trees (which was removed in ec888867 because they
were broken and unneeded) provided the beginning of a template for
their implementation.

src/cmd/ksh93/sh/subshell.c:
- struct subshell: Add strack pointer to subshell hash table.
- Add sh_subtracktree(): return pointer to subshell hash table.
- sh_subfuntree(): Refactor a bit for legibility.
- sh_subshell(): Add code for cleaning up subshell hash table.

src/cmd/ksh93/sh/name.c:
- nv_putval(): Remove code to fork a subshell upon resetting PATH.
- nv_rehash(): When in a subshell, invalidate a hash table entry
  for a subshell by creating the subshell scope if needed, then
  giving that entry the NV_NOALIAS attribute to invalidate it.

src/cmd/ksh93/sh/path.c: path_search():
- To set a tracked alias/hash table entry, use sh_subtracktree()
  and pass the HASH_NOSCOPE flag to nv_search() so that any new
  entries are added to the current subshell table (if any) and do
  not influence any parent scopes.

src/cmd/ksh93/bltins/typeset.c: b_alias():
- b_alias(): For hash table entries, use sh_subtracktree() instead
  of forking a subshell. Keep forking for normal aliases.
- setall(): To set a tracked alias/hash table entry, pass the
  HASH_NOSCOPE flag to nv_search() so that any new entries are
  added to the current subshell table (if any) and do not influence
  any parent scopes.

src/cmd/ksh93/sh/init.c: put_restricted():
- Update code for clearing the hash table (when changing $PATH) to
  use sh_subtracktree().

src/cmd/ksh93/bltins/cd_pwd.c:
- When invalidating path name bindings to relative paths, use the
  subshell hash tree if applicable by calling sh_subtracktree().
- rehash(): Call nv_rehash() instead of _nv_unset()ting the hash
  table entry; this is needed to work correctly in subshells.

src/cmd/ksh93/tests/leaks.sh:
- Add leak tests for various PATH-related operations in the main
  shell and in a virtual subshell.
- Several pre-existing memory leaks are exposed by the new tests
  (I've confirmed these in 93u+). The tests are disabled and marked
  TODO for now, as these bugs have not yet been fixed.

src/cmd/ksh93/tests/subshell.sh:
- Update.

Resolves: https://github.com/ksh93/ksh/issues/66
2021-01-07 22:18:25 +00:00
Martijn Dekker
a5d38b1de7 add missing function crash test (re: 6193c6a3)
Of course I was wrong to say the bug had nothing to do with
functions; traps in ksh functions are local, are handled the same
way as traps that are local to virtual subshells, and had the same
crashing bug. So this adds a test for that as well.
2020-09-27 06:46:52 +02:00
Martijn Dekker
6193c6a3c5 Fix crash while handling subshell trap (rhbz#1117404)
Contrary to the RH bug report, this is yet another bug with
virtual/non-forked subshells and has nothing to do with functions.
If a signal is ignored (empty trap) in the main shell while any
trap (empty or not) is set on the same signal in a subshell, a
crash eventually occurred upon restoring state when leaving the
subshell.

Original patch:
642af4d6/f/ksh-20120801-trapcom.patch

Prior discussion:
https://bugzilla.redhat.com/1117404

Paulo Andrade wrote there:
> The problem is that the sh_subshell function was saving pointers
> that could change, and when restoring, bad things would happen.
[...]
> The only comment I added:
> /* contents of shp->st.trapcom may change */
> may be a bit misleading, the "bad" save/restore already knows it,
> probably I should have added a better description telling that the
> data is, usually, modified in code like:
>
> tmp = buf[i]; buf[i] = strdup(tmp); free(tmp);
>
> so the shp->st.trapcom needs a "deep copy", as done in the
> patch, to properly save/restore pointers.

src/cmd/ksh93/sh/subshell.c,
src/cmd/ksh93/sh/xec.c:
- sh_subshell(), sh_funscope(): Make *savsig/*savstak into a
  **savsig array. Use strdup(3) to save the data and get known
  pointers that will not change. Free these upon restore.
- Change the comment from the patch as Paulo wished he had done.

src/cmd/ksh93/tests/subshell.sh:
- Test 2500 times. This should trigger the crash most of the time.
2020-09-27 06:17:54 +02:00
Martijn Dekker
045fe6a110 Fix: Closing a FD within a comsub broke output (rhbz#1116072)
Another Red Hat patch. "Prior to this update, the result of a
command substitution was lost if a file descriptor used for the
substitution was previously explicitly closed. With this update,
ksh no longer reuses file descriptors that were closed during the
execution of a command substitution. Now, command substitutions
work as expected in the described situation."

Prior discussion:
https://bugzilla.redhat.com/1116072

Original patch:
642af4d6/f/ksh-20140929-safefd.patch

src/cmd/ksh93/include/io.h,
src/cmd/ksh93/sh/io.c:
- Add sh_iosafefd() function to get a file descriptor that is not
  in use or otherwise occupied (including marked as closed).

src/cmd/ksh93/sh/subshell.c: sh_subshell():
- Use that function to obtain a safe FD upon restoring state when
  exiting a command substitution. I don't really know the how and
  why -- all that I/O magic is still beyond me and the code is
  uncommented as usual.

src/cmd/ksh93/tests/subshell.sh:
- Add regression test from the reproducer in the bug, reduced to
  the minimum necessary.
2020-09-27 04:46:24 +02:00
Martijn Dekker
95225e1e01 tests/subshell.sh: test from rhbz#1138751 reproducer (re: 4ce486a7) 2020-09-26 01:17:19 +02:00
Martijn Dekker
4ce486a7a4 Fix hang in comsubs (rhbz#1062296) (re: 970069a6)
The new command substitution mechanism imported in 970069a6 from
Red Hat patches introduced this bug: backtick-style command
substitutions hang when processing about 117KiB of data or more.

It is fixed by another Red Hat patch:
642af4d6/f/ksh-20140415-hokaido.patch

It saves the value of the shp->comsub flag so that it is set to 2
(usually meaning new-style $(comsubs)) in two specific cases even
when processing backtick comsubs. This stops the sh_subtmpfile()
function in subshell.c from creating a /tmp file. However, I think
that approach is quite ugly, so I'm taking a slightly different one
that has the same effect.

src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/subshell.c:
- Redefine sh_subtmpfile() to pass the comsub flag as an argument.
  (Remove the shp pointer argument, which is redundant; a pointer
  to the shell state can easily be obtained in the function.)

src/cmd/ksh93/sh/xec.c: sh_exec():
- Apply the Red Hat fix by passing flag 2 to sh_subtmpfile().

src/cmd/ksh93/tests/subshell.sh:
- Move regress test from ce68e1be from basic.sh to here; this is
  the place for command substitution tests as they are subshells.
- Add regress test for this bug.

All other changed files:
- Update sh_subtmpfile() calls to pass on the shp->comsub flag.
2020-09-24 06:07:12 +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:

642af4d6/f/ksh-20120801-macro.patch
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
5395641036 shtests: cd to each test set's temp dir before running
An oops in tests/io.sh (re: c607c48c) wrote temporary files outside
$tmp and into src/cmd/ksh93/tests. Let's fix this properly so it
doesn't happen again.

src/cmd/ksh93/tests/shtests:
- Start each test set in its own temporary directory by default.

src/cmd/ksh93/tests/*.sh:
- Refuse to run if $tmp != $PWD.
- Related cleanups.
2020-09-02 06:02:40 +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:
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
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
d3cd4cf906 Fixes to compile on Solaris variants, NetBSD, and NixOS
Solaris, Illumos distributions, and NetBSD need LDFLAGS set to link
explicitly to libm, otherwise, due to as-yet unknown reasons, the
src/lib/libdll/features/dll fails to write a valid header file and
compilation fails due to unknown identifiers such as Dllscan_t.
This commit adds the flag on those systems.

NixOS is a Linux distro that uses very different paths from the
usual Unix conventions (though it's POSIX compliant), and the
regression tests still needed a lot of tweaks to be compatible.

src/cmd/INIT/package.sh, bin/package:
- On SunOS (Solaris and illumos distros) and NetBSD, add '-lm' to
  LDFLAGS before compiling.

src/cmd/INIT/mamprobe.sh, bin/mamprobe,
src/cmd/INIT/execrate.sh, bin/execrate:
- Instead of only in /bin, /usr/bin, /sbin and /usr/sbin, search
  utilities in the path given by the OS 'getconf PATH', and use the
  user's original $PATH as a fallback.

src/cmd/ksh93/tests/*.sh:
- Miscellaneous portability fixes, mainly elimination of unportable
  hardcoded paths to commands.
- basic.sh: Remove test for 'time' keyword millisecond precision.
  It was racy and could fail depending on system and system load.
2020-08-03 09:24:16 +01:00
Martijn Dekker
cba895ed5f tests/subshell.sh: fix backticks test failure report (re: 7f2c8110) 2020-08-02 19:24:27 +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
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
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
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
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
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
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
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
b6b8b522a7 regress tests: remove use of unportable direct paths to commands
Many tests used direct paths to some commands, mostly /bin/echo and
/bin/cat. This is unportable (breaks on e.g. NixOS).
The correct way is to obtain the direct path using 'whence -p'.

There was also one use of '/usr/bin/pstack' in tests/comvario.sh
that seemed bogus. Apparently this was supposed to analyse a core
file after a crash. Even on Solaris and Linux, where that command
exists, the argument is documented to be a PID, not a core file. If
this ever worked anywhere, then it was system-specific enough to be
useless here, so I've removed it.

(cherry picked from commit 4563b8bc651cd9cb18dc73f56a041f7ac5534395)
2020-06-12 01:45:15 +02:00