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

55 commits

Author SHA1 Message Date
Martijn Dekker
fbfd4d3ab8 Fix syntax error detection in associative array assignments
Reproducer:

$ fn=([foo_key]=foo_val [bar_key])
-ksh: [bar_key]: not found

Expected output:
-ksh: syntax error: `[bar_key]' unexpected

As soon as one correct associative array assignment element has
been processed, a subsequent one, starting with '[' but not
containing ']=', is incorrectly seen as a command to execute.
If a command '[bar_key]' existed on $PATH, it would have been run.

src/cmd/ksh93/sh/parse.c: simple():
- In the syntax check for associative array assignments, don't just
  check for an initial '[' but also verify the presence of ']='.

Thanks to @JohnoKing for finding this bug.

Resolves: https://github.com/ksh93/ksh/issues/427
2022-07-05 22:16:55 +02:00
Martijn Dekker
06e56251b9 Fix wrong syntax error upon process substitution after redirection
Grammatically, redirections may occur anywhere within a command
line and are removed after processing them, whereas a process
substitution (<(commandlist) or >(commandlist)) is replaced by a
file name which should be treated as just another simple word.
So the following should not be a syntax error:

$ cat </dev/null <(true)
-ksh: syntax error: `)' unexpected
$ cat </dev/null >(true)
-ksh: syntax error: `)' unexpected
$ cat >/dev/null <(true)
-ksh: syntax error: `)' unexpected
$ cat >/dev/null >(true)
-ksh: syntax error: `)' unexpected

This bug is in every ksh93 version.

The problem is in the parser (parse.c). The process substitution is
misparsed as a redirection due to inout() recursively parsing
multiple redirections without recognising process substitutions.
inout() is mistaking '<(' for '<' and '>(' for '>', which explains
the incorrect syntax error.

This also causes the following to fail to detect a syntax error:
$ cat >&1 <(README.md
[the contents of README.md are shown]

...and other syntax errors detected in the wrong spot, for example:
$ { true; } <(echo wrong)
-ksh: syntax error: `wrong' unexpected
which should be:
-ksh: syntax error: `<(' unexpected

src/cmd/ksh93/sh/parse.c:
- Add global inout_found_procsub flag.
- inout(): On encountering a process substitution, set this flag
  and return, otherwise clear the flag.
- simple(): After calling inout(), check this flag and, if set,
  jump back to where process substitutions are parsed.

Resolves: https://github.com/ksh93/ksh/issues/418
2022-07-05 13:20:28 +02:00
Martijn Dekker
cc1689849e Another round of tweaks
Notable changes:
- Change a bunch of uses of memcmp(3) to compare strings (which can
  cause buffer read overflows) to strncmp(3).
- src/cmd/ksh93/include/name.h: Eliminate redundant Nambfp_f type.
  Replace with Shbltin_f type from libast's shcmd.h. Since that is
  not guaranteed to be included here, repeat the type definition
  here without fully defining the struct (which is valid in C).
2022-06-24 15:34:55 +01:00
Martijn Dekker
cae9af5eec tweak: make better use of funptr() macro and sh.bltinfun pointer
src/cmd/ksh93/include/name.h:
- Include the ususally-wanted (Shbltin_f) typecast in funptr().

Various files:
- Change several direct foo->nvalue.bfp usages to funptr(np).
- Reduce explicit typecasts after the name.h change.
- To determine if we are (or just were) running a certain built-in
  command, instead of comparing sh.bltindata.bnode with a builtin
  table node, use sh.bltinfun to directly compare the builtin's
  function; this is more readable and efficient.
2022-06-20 22:25:41 +01:00
Martijn Dekker
69d37d5eae parse.c: fix use-after-free probs related to funstaks()
When the funstaks() function deletes a stack, other code could
still reference that stack's pointer, at least if a script's DEBUG
trap changed the function context by assigning to ${.sh.level}.
This crashed @ormaaj's funcname.ksh script in certain contexts, at
least when run as a dot script or in a virtual subshell.

This allows that script to run in all contexts by making
funstaks(s) set the stack pointer to NULL after deleting the stack
and making various other points in the code check for a null
pointer before dereferencing it.

This may not be the most elegant fix but (in my testing) it does
work, even when compiling ksh with AddressSanitiser.

Thanks to @JohnoKing for help researching this problem.

Resolves: https://github.com/ksh93/ksh/issues/212
2022-06-14 13:47:00 +01:00
Martijn Dekker
148a8a3f46 Another build system overhaul (re: 35672208, 580ff616, 6cc2f6a0)
So far we've been handling AST release build and git commit flags
and ksh SHOPT_* compile time options in the generic package build
script. That was a hack that was necessary before I had sufficient
understanding of the build system. Some of it did not work very
well, e.g. the correct git commit did not show up in ${.sh.version}
when compiling from a git repo.

As of this commit, this is properly included in the mamake
dependency tree by handling it from the libast and ksh93 Mamfiles,
guaranteeing they are properly up to date.

For a release build, the _AST_ksh_release macro is renamed to
_AST_release, because some aspects of libast also use this.

This commit also adds my first attempt at documenting the (very
simple, six-command) mamake language as it is currently implemented
-- which is significantly different from Glenn Fowler's original
paper. This is mostly based on reading the mamake.c source code.

src/cmd/INIT/README-mamake.md:
- Added.

bin/package, src/cmd/INIT/package.sh:
- Delete the hack.

**/Mamfile:
- Remove KSH_RELFLAGS and KSH_SHOPTFLAGS, which supported the hack.
- Delete 'meta' commands. They were no-ops; mamake.c ignores them.
  They also did not add any informative value.

src/lib/libast/Mamfile:
- Add a 'virtual' target that obtains the current git commit,
  examines the git branch, and decides whether to auto-set an
  _AST_git_commit and/or or _AST_release #define to a new
  releaseflags.h header file. This is overwritten on each run.
- Add code to the install target that copies limit.h to
  include/ast, but only if it doesn't exist or the content of the
  original changed. This allows '#include <releaseflags.h>' from
  any program using libast while avoiding needless recompiles.
- When there are uncommitted changes, add /MOD (modified) to the
  commit hash instead of not defining it at all.

src/cmd/ksh93/**:
- Mamfile: Add a shopt.h target that reads SHOPT.sh and converts it
  into a new shopt.h header file in the object code directory. The
  shopt.h header only contains SHOPT_* directives that have a value
  in SHOPT.sh (not the empty/probe ones). They also do not redefine
  the macros if they already exist, so overriding with something
  like CCFLAGS+=' -DSHOPT_FOO=1' remains possible.
- **.c: Every c file now #includes "shopt.h" first. So SHOPT_*
  macros are no longer passed via environment/MAM variables.
* SHOPT.sh: The AUDITFILE and CMDLIB_DIR macros no longer need an
  extra backslash-escape for the double quotes in their values.
  (The old way required this because mamake inserts MAM variables
  directly into shell scripts as literals without quoting.  :-/ )

src/cmd/INIT/mamake.c:
- Import the two minor changes between from 93u+ and 93v-: bind()
  is renamed to bindfile() and there is a tweak to detecting an
  "improper done statement".
- Allow arbitrary whitespace (isspace()) everywhere, instead of
  spaces only. This obsoletes my earlier indentation workaround
  from 6cc2f6a0; turns out mamake always supported indentation, but
  with spaces only.
- Do not skip line numbers at the beginning of each line. This
  undocumented feature is not and (AFAICT) never has been used.
- Throw an error on unknown command or rule attribute. Quite an
  important feature for manual maintenance: catches typos, etc.
2022-06-12 05:47:02 +01:00
Martijn Dekker
b398f33c49 Another round of accumulated minor fixes and cleanups
Only notable changes listed below.

**/Mamfile:
- Do not bother redirecting standard error for 'cmp -s' to
  /dev/null. Normally, 'cmp -s' on Linux, macOS, *BSD, or Solaris
  do not not print any message. If it does, something unusual is
  going on and I would want to see the message.
- Since we now require a POSIX shell, we can use '!'.

src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/init.c:
- Remove SH_TYPE_PROFILE symbol, unused after the removal of the
  SHOPT_PFSH code. (re: eabd6453)

src/cmd/ksh93/sh/io.c:
- piperead(), slowread(): Replace redundant sffileno() calls by
  the variables already containing their results. (re: 50d342e4)

src/cmd/ksh93/bltins/mkservice.c,
rc/lib/libcmd/vmstate.c:
- If these aren't compiled, define a stub function to silence the
  ranlib(1) warning that the .o file does not contain symbols.
2022-03-11 21:20:32 +01:00
Johnothan King
8fc8c2f51c Fix a few minor issues (#473)
Changes:
- Fixed two xtrace test failures introduced in commit cfc8744c.
- The definition of _use_ntfork_tcpgrp in xec.c is now dependent on
  SHOPT_SPAWN being defined (re: 8e9ed5be).
- Removed many unnecessary newlines and fixed various typos.
2022-03-11 21:18:42 +01:00
Martijn Dekker
f43bb4981f Improve custom type declaration command parsing (re: c5018f7c)
dcl_dehacktivate() actually left a bit of inconsistent state beyond
the current line because it did not clear the dummy builtins tree.
This caused assignments to variables of types whose definitions
were parsed but not executed to throw a "not found" error instead
of a syntax error, even beyond the current line.

There is also an opportunity for an optimisation. We do not need to
initialise and maintain the dummy builtins tree if we're never
going to use it. So move that to the check_typedef() function right
before the sh_addbuiltin() call: check there if the tree exists and
if not create it and open a view. If no 'typeset -T' or 'enum'
type definition command is executed, the tree is never created.

dcl_hacktivate() now basically does nothing until the tree is
needed, but it does still count the recursion level and install the
error_info.exit hook because we need this to dcl_dehacktivate() at
the correct time when the tree does exist.

dcl_dehacktivate() is amended to clear the tree -- except if we're
running shcomp, as shcomp parses the script line by line without
executing anything, so we need the dummies to persist beyond the
sh_parse() invocation for the entire script. (Note that we do not
need this workaround for dot scripts or noexec mode, as the script
is entirely parsed in a single sh_parse() call in those cases.)
2022-02-18 03:44:19 +00:00
Johnothan King
787058bdbf Fix the output of typeset -p for two dimensional indexed arrays (#454)
In ksh93v- 2012-10-04 the following bugfix is noted in the changelog
(this fix was most likely part of ksh93v- 2012-09-27, although that
version is not archived anywhere):
12-09-21  A bug in which the output of a two dimensional sparse
          indexed array would cause the second subscript be treated
          as an associative array when read back in has been fixed.
          Elements that are sparse indexed arrays now are prefixed
          type "typeset -a".

Below is a before and after of this change:

   # Before
   $ typeset -a foo[1][2]=bar
   $ typeset -p foo
   typeset -a foo=([1]=([2]=bar) )

   # After
   $ typeset -a foo[1][2]=bar
   $ typeset -p foo
   typeset -a foo=(typeset -a [1]=([2]=bar) )

src/cmd/ksh93/sh/*.c:
- Backport changes from ksh93v- to print 'typeset -a' before sparse
  indexed arrays and properly handle 'typeset -a' in reinput
  commands from 'typeset -p'.

src/cmd/ksh93/tests:
- Add two regression tests to arrays.sh for this change.
- Update the existing regression tests for compatibility with the
  new printed typeset output.
2022-02-10 21:01:40 +00:00
Martijn Dekker
9c313c7fe3 Update copyright years in files changed since 1st Jan 2022 2022-01-30 20:49:04 +00:00
Martijn Dekker
172becffea Some more accumulated minor tweaks and cleanups
Notable changes:

src/cmd/ksh93/include/fault.h:
- Get rid of the superflous sh pointer argument in the
  sh_pushcontext() and sh_popcontext() macros. (re: 2d3ec8b6)

src/cmd/ksh93/tests/io.sh:
- Tweak a process substitution test to allow up to a second for
  unused process substitution processes to disappear. On the Alpine
  Linux console (at least the musl libc version), this is needed to
  avoid a test failure as long as no GUI is active. As soon as you
  start X11, this phenomenon disappears, even on the console. Very
  strange, but also very probably not ksh's fault.

src/cmd/ksh93/tests/shtests:
- Instead of just SIGCONT and SIGPIPE, set all signals to default,
  just to be sure to avoid spurious test failures due to signals
  that were ignored on entry. (It made no difference to the
  aforementioned Alpine Linux test failure, so ignored signals had
  nothing to do with that -- but still a good idea.)

.github/workflows/ci.yml:
- On the GitHub CI runs, when testing with SHOPTs disabled, disable
  SHOPT_SPAWN as well, which tests that everything still works
  correctly with the regular fork(2) method.

COPYRIGHT:
- Remove duplicate of BSD license.
2022-01-25 16:13:15 +00:00
Martijn Dekker
b590a9f155 [shp cleanup 01..20] all the rest (re: 2d3ec8b6)
This combines 20 cleanup commits from the dev branch.

All changed files:
- Clean up pointer defererences to sh.
- Remove shp arguments from functions.

Other notable changes:

src/cmd/ksh93/include/shell.h,
src/cmd/ksh93/sh/init.c:
- On second thought, get rid of the function version of
  sh_getinterp() as libshell ABI compatibility is moot. We've
  already been breaking that by reordering the sh struct, so there
  is no way it's going to work without recompiling.

src/cmd/ksh93/sh/name.c:
- De-obfuscate the relationship between nv_scan() and scanfilter().
  The former just calls the latter as a static function, there's no
  need to do that via a function pointer and void* type conversions.

src/cmd/ksh93/bltins/typeset.c,
src/cmd/ksh93/sh/name.c,
src/cmd/ksh93/sh/nvdisc.c:
- 'struct adata' and 'struct tdata', defined as local struct types
  in these files, need to have their first three fields in common,
  the first being a pointer to sh. This is because scanfilter() in
  name.c accesses these fields via a type conversion. So the sh
  field needed to be removed in all three at the same time.
  TODO: de-obfuscate: good practice definition via a header file.

src/cmd/ksh93/sh/path.c:
- Naming consistency: reserve the path_ function name prefix for
  externs and rename statics with that prefix.
- The default path was sometimes referred to as the standard path.
  To use one term, rename std_path to defpath and onstdpath() to
  ondefpath().
- De-obfuscate SHOPT_PFSH conditional code by only calling
  pf_execve() (was path_pfexecve()) if that is compiled in.

src/cmd/ksh93/include/streval.h,
src/cmd/ksh93/sh/streval.c:
- Rename extern strval() to arith_strval() for consistency.

src/cmd/ksh93/sh/string.c:
- Remove outdated/incorrect isxdigit() fallback; '#ifnded isxdigit'
  is not a correct test as isxdigit() is specified as a function.
  Plus, it's part of C89/C90 which we now require. (re: ac8991e5)

src/cmd/ksh93/sh/suid_exec.c:
- Replace an incorrect reference to shgd->current_pid with
  getpid(); it cannot work as (contrary to its misleading directory
  placement) suid_exec is an independent libast program with no
  link to ksh or libshell at all. However, no one noticed because
  this was in fallback code for ancient systems without
  setreuid(2). Since that standard function was specified in POSIX
  Issue 4 Version 2 from 1994, we should remove that fallback code
  sometime as part of another obsolete code cleanup operation to
  avoid further bit rot. (re: 843b546c)

src/cmd/ksh93/bltins/print.c: genformat():
- Remove preformat[] which was always empty and had no effect.

src/cmd/ksh93/shell.3:
- Minor copy-edit.
- Remove documentation for nonexistent sh.infile_name. A search
  through ast-open-archive[*] reveals this never existed at all.
- Document sh.savexit (== $?).

src/cmd/ksh93/shell.3,
src/cmd/ksh93/include/shell.h,
src/cmd/ksh93/sh/init.c:
- Remove sh.gd/shgd; this is now unused and was never documented
  or exposed in the shell.h public interface.
- sh_sigcheck() was documented in shell.3 as taking no arguments
  whereas in the actual code it took a shp argument. I decided to
  go with the documentation.
- That leaves sh_parse() as the only documented function that still
  takes an shp argument. I'm just going to go ahead and remove it
  for consistency, reverting sh_parse() to its pre-2003 spec.
- Remove undocumented/unused sh_bltin_tree() function which simply
  returned sh.bltin_tree.
- Bump SH_VERSION to 20220106.
2022-01-07 16:16:31 +00:00
Martijn Dekker
a1f5c99204 INIT: remove proto, ratz (re: 46593a89, 6137b99a); major cleanup
This takes another step towards cleaning up the build system. We
now do not even pretend to be theoretically compatible with
pre-1989 K&R C compilers or with C++ compilers. In practice, this
had already been broken for many years due to bit rot.

Commit 46593a89 already removed the license handling enormity that
depended on proto, so now we can cleanly remove it altogether. But
we do need to leave some backwards compatibility stubs to keep the
build system compatible with older AST code; it should remain
possible to build older ksh versions with the current build system
(the bin/ and src/cmd/INIT/ directories) for testing purposes.

So as of now there is no more __MANGLE__d rubbish in your generated
header files. This is only about a quarter of a century overdue...

This commit also includes a huge amount of code cleanup to remove
thousands of unused K&R C fallbacks and other cruft, particularly
in libast. This code base should now be a little easier to
understand for people who are familiar with a modern(ish) C
standard.

ratz is now also removed; this was a standalone and simplified 2005
version of gunzip. As of 6137b99a, none of our code uses it, even
theoretically. And the real g(un)zip is now everywhere.

src/cmd/INIT/proto.c, src/cmd/INIT/ratz.c:
- Removed.

COPYRIGHT:
- Remove zlib license; this only applied to ratz.

bin/package, src/cmd/INIT/package.sh:
- Related cleanups.
- Unset LC_ALL before invoking a new shell, respecting the user's
  locale again and avoiding multibyte character corruption on the
  command line.

src/cmd/INIT/proto.sh:
- Add stub for backwards compatibility with Mamfiles that depend on
  proto. It does nothing but pass input without modification and is
  now installed as the new arch/*/bin/proto by src/cmd/INIT/Mamfile.

src/cmd/INIT/iffe.sh:
- Ignore the proto-related -e (--package) and -p (--prototyped)
  options; keep parsing them for backwards compatibility.
- Trim the macros passed to every test to their standard C
  versions, removing K&R C and C++ versions. These are now
  considered to be for backwards compatibility only.

src/cmd/INIT/iffe.tst:
- Remove proto(1) mangling code.
  By the way, iffe can be regression-tested as follows:
        $ bin/package use   # set up environment in a child shell
        $ regress src/cmd/INIT/iffe.tst
        $ exit              # leave package environment

src/cmd/INIT/make.probe, src/cmd/INIT/probe.win32:
- Remove code to handle C++.

src/lib/libast/features/common:
- As in iffe.sh above, trim macros designed for compatibility with
  C++ and ancient C compilers to their standard C versions and
  comment that they are for backwards compatibility with AST code.
  This is needed to keep all the old ast and ksh code compiling.

src/cmd/ksh93/sh/init.c,
src/cmd/ksh93/sh/name.c:
- Clarify libshell ABI compatibility function versions of macros.
  A "proto workaround" comment in the original code mislead me into
  thinking this had something to do with the removed proto(1), but
  it's unrelated. Call the workaround macro BYPASS_MACRO instead.

src/cmd/ksh93/include/defs.h:
- sh_sigcheck() macro: allow &sh as an argument: parenthesise shp.

src/cmd/ksh93/sh/nvtype.c:
- Remove unused nv_mkstruct() function. (re: d0a5cab1)

**/features/*:
- Remove obsolete iffe 'set prototyped' option.

**/Mamfile:
- Remove all references to the ast/prototyped.h header.
- Remove all use of the proto command. Simply copy instead.

*** 850-ish source files: ***
- Remove all '#pragma prototyped' directives.
- Remove all C++ compat code conditional upon defined(__cplusplus).
- Remove all use of the _ARG_ macro, which on standard C expands to
  its argument:
        #define _ARG_(x)        x
  (on K&R C, it expanded to nothing)
- Remove all use of _BEGIN_EXTERNS_ and _END_EXTERNS_ macros (empty
  on standard C; this was for C++ compatibility)
- Reduce all #if __STD_C (standard code) #else (K&R code) #endif
  blocks to the standard code only, without use of the macro.
- Same for _STD_ macro which seems to have had the same function.
- Change all instances of 'Void_t' to standard 'void'.
2021-12-24 07:05:22 +00:00
Martijn Dekker
3525535e1f sh_parse(): don't turn on interactive state (re: 48ba6964)
Reproducer:

	$ (sleep 1& echo done)
	done
	$ (eval "echo hi"; sleep 1& echo done)
	hi
	[1]	30587
	done

No job control output should be printed for a background process
invoked from a subshell, not even after 'eval'.

The cause: sh_parse() turns on the shell's interactive state bit
(sh_state(SH_INTERACTIVE)) if the interactive shell option is on.

This is incorrect. The parser should have no involvement with shell
interactivity in principle because that's not its domain.

Not only that, the parser may need to run in a subshell, e.g. when
executing traps or 'eval' commands (as above). By definition, a
subshell can never be interactive.

We already fixed many bugs related to job control and the shell's
interactive state. Even if these two lines previously papered over
some breakage, I can't find any now after simply removing them. If
any is found later, then it'll need to be fixed properly instead.

Related: https://github.com/ksh93/ksh/issues/390
2021-12-22 05:06:12 +00:00
Martijn Dekker
e67df29c07 Re-fix defining types conditionally or in subshells (re: f508660d)
New version. I'm pretty sure the problems that forced me to revert
it earlier are fixed.

This commit mitigates the effects of the hack explained in the
referenced commit so that dummy built-in command nodes added by the
parser for declaration/assignment purposes do not leak out into the
execution level, except in a relatively harmless corner case.

Something like

    if false; then
        typeset -T Foo_t=(integer -i bar)
    fi

will no longer leave a broken dummy Foo_t declaration command. The
same applies to declaration commands created with enum.

The corner case remaining is:

    $ ksh -c 'false && enum E_t=(a b c); E_t -a x=(b b a c)'
    ksh: E_t: not found

Since the 'enum' command is not executed, this should have thrown
a syntax error on the 'E_t -a' declaration:
ksh: syntax error at line 1: `(' unexpected

This is because the -c script is parsed entirely before being
executed, so E_t is recognised as a declaration built-in at parse
time. However, the 'not found' error shows that it was successfully
eliminated at execution time, so the inconsistent state will no
longer persist.

This fix now allows another fix to be effective as well: since
built-ins do not know about virtual subshells, fork a virtual
subshell into a real subshell before adding any built-ins.

src/cmd/ksh93/sh/parse.c:

- Add a pair of functions, dcl_hactivate() and dcl_dehacktivate(),
  that (de)activate an internal declaration built-ins tree into
  which check_typedef() can pre-add dummy type declaration command
  nodes. A viewpath from the main built-ins tree to this internal
  tree is added, unifying the two for search purposes and causing
  new nodes to be added to the internal tree. When parsing is done,
  we close that viewpath. This hides those pre-added nodes at
  execution time. Since the parser is sometimes called recursively
  (e.g. for command substitutions), keep track of this and only
  activate and deactivate at the first level.
     (Fixed compared to previous version of this commit: calling
  dcl_dehacktivate() when the recursion level is already zero is
  now a harmless no-op. Since this only occurs in error handling
  conditions, who cares.)

- We also need to catch errors. This is done by setting libast's
  error_info.exit variable to a dcl_exit() function that tidies up
  and then passes control to the original (usually sh_exit()).
     (Fixed compared to previous version of this commit: dcl_exit()
  immediately deactivates the hack, no matter the recursion level,
  and restores the regular sh_exit(). This is the right thing to
  do when we're in the process of erroring out.)

- sh_cmd(): This is the most central function in the parser. You'd
  think it was sh_parse(), but $(modern)-form command substitutions
  use sh_dolparen() instead. Both call sh_cmd(). So let's simply
  add a dcl_hacktivate() call at the beginning and a
  dcl_deactivate() call at the end.

- assign(): This function calls path_search(), which among many
  other things executes an FPATH search, which may execute
  arbitrary code at parse time (!!!). So, regardless of recursion
  level, forcibly dehacktivate() to avoid those ugly parser side
  effects returning in that context.

src/cmd/ksh93/bltins/enum.c: b_enum():

- Fork a virtual subshell before adding a built-in.

src/cmd/ksh93/sh/xec.c: sh_exec():

- Fork a virtual subshell when detecting typeset's -T option.

Improves fix to https://github.com/ksh93/ksh/issues/256
2021-12-17 01:28:28 +01:00
Johnothan King
cd562b16e2 Port more shell lint improvements from illumos and ksh93v- (#374)
This commit adds onto <https://github.com/ksh93/ksh/pull/353> by porting
over two additional improvements to the shell linter:

1) The changes in the aforementioned pull request were merged into
   illumos-gate with an additional change.[*] The illumos revision of
   the patch improved the warning for (( $foo = $? )) to specify '$foo'
   causes the warning.[**] Example:
      $ ksh -n -c '(( $? != $bar ))'
      ksh: warning: line 1: in '(( $? != $bar ))', using '$' as in '$bar' is slower and can introduce rounding errors
   While I was porting the illumos patch I did notice one problem. The
   string it uses from paramsub() skips over the initial '{' in
   '${var}', resulting in the warning printing '$var}' instead:
      $ ksh -n -c '(( ${.sh.pid} != $$ ))'
      ...  in '(( ${.sh.pid} != $$ ))', using '$' as in '$.sh.pid}' is slower  ...
   This was fixed by including the missing '{' in the string returned by
   paramsub for ${var} variables.

2) In ksh93v-, parsing x=$((expr)) with the shell linter will cause ksh
   to warn the user x=$((expr)) is slower than ((x=expr)). This
   improvement has been backported with a modified warning:
      # Result from this commit
      $ ksh -n -c 'x=$((1 + 2))'
      ksh: warning: line 1: x=$((1 + 2)) is slower than ((x=1 + 2))
      # Result from ksh93v-
      $ ksh93v -n -c 'x=$((1 + 2))'
      ksh93v: warning: line 1: ((x=1 + 2)) is more efficient than x=$((1 + 2))
   Minor note: the ksh93v- patch had an invalid use of memcmp; this
   version of the patch uses strncmp instead.

References:
be548e87bc
65722363_22fdf8e7/
2021-12-13 01:49:37 +01:00
Martijn Dekker
aa3048880b cleanup: get rid of KSHELL and _BLD_shell preprocessor macros
Once upon a time it might have been possible to build certain parts
of ksh, such as the emacs and vi editors and possibly even the
name/value library (nval(3)) as independent libraries. But given
the depressing amount of bit rot in the code that we inherited, I
am certain that disabling either of these macros had been resulting
in a broken build for many years before AT&T abandoned this code
base. These are certainly not going to be useful now.

Meanwhile the KSHELL macro got in the way of me today, because the
Mamfile did not define it for all the .c files, but some headers
declared some functionality conditionally upon that macro. So
including <io.h> in, e.g., nvdisc.c did not declare the same
functions as including that header in files with KSHELL defined.
This inconsistency is now gone as well, for various files.

I'm currently working on making it possible once again to build
libshell as a dynamic library; that should be good enough. And that
never involved disabling either of these macros.
2021-12-09 06:43:22 +01: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
Johnothan King
6904585f49 Port illumos' shell linter improvements (#353)
This commit ports over two improvements to the shell linter from
illumos (original patch written by Andy Fiddaman). Links to the
relevant bug reports and the original patch:
https://www.illumos.org/issues/13601
https://www.illumos.org/issues/13631
c7b656fc71

The first improvement is to the lint warning for arithmetic
operators in [[ ... ]]. The ksh linter now suggests the correct
equivalent operator to use in ((...)). Example:
  $ ksh -nc '[[ 30 -gt 25 ]]'
  # Original warning
  warning: line 1: -gt within [[ ... ]] obsolete, use ((...))
  # New warning
  warning: line 1: [[ ... -gt ... ]] obsolete, use ((... > ...))

The second improvement pertains to variable expansion in arithmetic
expressions. The ksh linter now suggests referencing variable names
directly:
  $ ksh -nc 'integer foo=40; (($foo < 50 ))'
  # Old warning
  warning: line 1: variable expansion makes arithmetic evaluation less efficient
  # New warning
  warning: line 1: in '(($foo < 50))', using '$' is slower and can introduce rounding errors

src/cmd/ksh93/{data/lexstates,sh/lex,sh/parse}.c:
- Port the improved shell lint warnings from illumos to ksh93u+m.
- The original checks for arithmetic operators involved a bunch of
  if statements with inefficient calls to strcmp(3). These were
  replaced with a more efficient switch statement that avoids
  strcmp.
2021-12-05 19:27:16 +01:00
Martijn Dekker
f508660ddf Revert "Fix defining types conditionally and/or in subshells (re: 8ced1daa)"
This reverts commit 2b9cbbbc8e.

This is not ready for prime time. Crashses when running a $PS2
discipline function. This needs fixing and more testing in
development before making it into the 1.0 branch. In the meantime,
that terrible problem with types is back, sorry about that.
2021-11-29 20:08:53 +01:00
Martijn Dekker
2b9cbbbc8e Fix defining types conditionally and/or in subshells (re: 8ced1daa)
This commit mitigates the effects of the hack explained in the
referenced commit so that dummy built-in command nodes added by the
parser for declaration/assignment purposes do not leak out into the
execution level, except in a relatively harmless corner case.

Something like

	if false; then
		typeset -T Foo_t=(integer -i bar)
	fi

will no longer leave a broken dummy Foo_t declaration command. The
same applies to declaration commands created with enum.

The corner case remaining is:

$ ksh -c 'false && enum E_t=(a b c); E_t -a x=(b b a c)'
ksh: E_t: not found

Since the 'enum' command is not executed, this should have thrown
a syntax error on the 'E_t -a' declaration:
ksh: syntax error at line 1: `(' unexpected

This is because the -c script is parsed entirely before being
executed, so E_t is recognised as a declaration built-in at parse
time. However, the 'not found' error shows that it was successfully
eliminated at execution time, so the inconsistent state will no
longer persist.

This fix now allows another fix to be effective as well: since
built-ins do not know about virtual subshells, fork a virtual
subshell into a real subshell before adding any built-ins.

src/cmd/ksh93/sh/parse.c:

- Add a pair of functions, dcl_hactivate() and dcl_dehacktivate(),
  that (de)activate an internal declaration built-ins tree into
  which check_typedef() can pre-add dummy type declaration command
  nodes. A viewpath from the main built-ins tree to this internal
  tree is added, unifying the two for search purposes and causing
  new nodes to be added to the internal tree. When parsing is done,
  we close that viewpath. This hides those pre-added nodes at
  execution time. Since the parser is sometimes called recursively
  (e.g. for command substitutions), keep track of this and only
  activate and deactivate at the first level.

- We also need to catch errors. This is done by setting libast's
  error_info.exit variable to a dcl_exit() function that tidies up
  and then passes control to the original (usually sh_exit()).

- sh_cmd(): This is the most central function in the parser. You'd
  think it was sh_parse(), but $(modern)-form command substitutions
  use sh_dolparen() instead. Both call sh_cmd(). So let's simply
  add a dcl_hacktivate() call at the beginning and a
  dcl_deactivate() call at the end.

- assign(): This function calls path_search(), which among many
  other things executes an FATH search, which may execute arbitrary
  code at parse time (!!!). So, regardless of recursion level,
  forcibly dehacktivate() to avoid those ugly parser side effects
  returning in that context.

src/cmd/ksh93/bltins/enum.c: b_enum():

- Fork a virtual subshell before adding a built-in.

src/cmd/ksh93/sh/xec.c: sh_exec():

- Fork a virtual subshell when detecting typeset's -T option.

Improves fix to https://github.com/ksh93/ksh/issues/256
2021-11-29 09:02:07 +01:00
Martijn Dekker
43cd8da2fe Fix 'command' prefix in enum type def pre-parsing (re: 1dc18346)
Symptom:

$ ksh -c 'command enum -i P_t=(a b); P_t -A v=([f]=b); typeset -p v'
ksh: syntax error at line 1: `(' unexpected

Expected: no syntax error, and output of 'P_t -A v=([f]=b)'.

src/cmd/ksh93/sh/parse.c: check_typedef():
- For enum, skip over any possible 'command' prefixes before
  pre-parsing options with optget (or, technically, skip anything
  else that might come before 'enum', though I don't think anything
  else is possible).
- The sh_addbuiltin() call at the end to pre-add the builtin
  obtained the node pointer to the built-in and the node flags from
  the parser tree. This did not work if a 'command' prefix was
  present. However, we don't actually need this. For parsing
  purposes, the BLT_DCL flag for a declaration built-in is
  sufficient; this is what gets the parser to accept
  assignment-arguments including parentheses. So just apply that.
  In addition, let's point it to an actual dummy built-in, 'true'
  (SYSTRUE), so that if a user does run something like 'if false;
  then enum Foo_t=(...); fi', the leaked Foo_t dummy at least won't
  do anything (not even crash).
2021-11-28 21:16:19 +01:00
Martijn Dekker
8ced1daadf Fix enum type definition pre-parsing for shcomp and dot/source
Parser limitations prevent shcomp or source from handling enum
types correctly:

    $ cat /tmp/colors.sh
    enum Color_t=(red green blue orange yellow)
    Color_t -A Colors=([foo]=red)

    $ shcomp /tmp/colors.sh > /dev/null
    /tmp/colors.sh: syntax error at line 2: `(' unexpected
    $ source /tmp/colors.sh
    /bin/ksh: source: syntax error: `(' unexpected

Yet, for types created using 'typeset -T', this works. This is done
via a check_typedef() function that preliminarily adds the special
declaration builtin at parse time, with details to be filled in
later at execution time.

This hack will produce ugly undefined behaviour if the definition
command creating that type built-in is then not actually run at
execution time before the type built-in is accessed.

But the hack is necessary because we're dealing with a fundamental
design flaw in the ksh language. Dynamically addable built-ins that
change the syntactic parsing of the shell language on the fly are
an absurdity that violates the separation between parsing and
execution, which muddies the waters and creates the need for some
kind of ugly hack to keep things like shcomp more or less working.

This commit extends that hack to support enum.

src/cmd/ksh93/sh/parse.c:
- check_typedef():
  - Add 'intypeset' parameter that should be set to 1 for typeset
    and friends, 2 for enum.
  - When processing enum arguments, use AST getopt(3) to skip over
    enum's options to find the name of the type to be defined.
    (getopt failed if we were running a -c script; deal with this
    by zeroing opt_info.index first.)
- item(): Update check_typedef() call, passing lexp->intypeset.
- simple(): Set lexp->intypeset to 2 when processing enum.

The rest of the changes are all to support the above and should be
fairly obvious, except:

src/cmd/ksh93/bltins/enum.c:
- enuminfo(): Return on null pointer, avoiding a crash upon
  executing 'Type_t --man' if Type_t has not been fully defined due
  to the definition being pre-added at parse time but not executed.
  It's all still wrong, but a crash is worse.

Resolves: https://github.com/ksh93/ksh/issues/256
2021-11-21 17:43:55 +01:00
Martijn Dekker
c81473061a test/[: binary operators: fix '<' and add '=~'; some more cleanups
In ksh88, the test/[ built-in supported both the '<' and '>'
lexical sorting comparison operators, same as in [[. However, in
every version of ksh93, '<' does not work though '>' still does!

Still, the code for both is present in test_binop():

src/cmd/ksh93/bltins/test.c
548:		case TEST_SGT:
549:			return(strcoll(left, right)>0);
550:		case TEST_SLT:
551:			return(strcoll(left, right)<0);

Analysis: The binary operators are looked up in shtab_testops[] in
data/testops.c using a macro called sh_lookup, which expands to a
sh_locate() call. If we examine that function in sh/string.c, it's
easy to see that on systems using ASCII (i.e. all except IBM
mainframes), it assumes the table is sorted in ASCII order.

src/cmd/ksh93/sh/string.c
64:	while((c= *tp->sh_name) && (CC_NATIVE!=CC_ASCII || c <= first))

The problem was that the '<' operator was not correctly sorted in
shtab_testops[]; it was sorted immediately before '>', but after
'='. The ASCII order is: < (60), = (61), > (62). This caused '<' to
never be found in the table.

The test_binop() function is also used by [[, yet '<' always worked
in that. This is because the parser has code that directly checks
for '<' and '>' within [[ (in sh/parse.c, lines 1949-1952).

This commit also adds '=~' to 'test', which took three lines of
code and allowed eliminating error handling in test_binop() as
test/[ and [[ now support the same binary ops. (re: fc2d5a60)

src/cmd/ksh93/*/*.[ch]:
- Rename a couple of very misleadingly named macros in test.h:
  . For == and !=, the TEST_PATTERN bit is off for pattern compares
    and on for literal string compares! Rename to TEST_STRCMP.
  . The TEST_BINOP bit does not denote all binary operators, but
    only the logical -a/-o ops in test/[. Rename to TEST_ANDOR.

src/cmd/ksh93/bltins/test.c: test_binop():
- Add support for =~. This is only used by test/[. The method is
  implemented in two lines that convert the ERE to a shell pattern
  by prefixing it with ~(E), then call test_strmatch with that
  temporary string to match the ERE and update ${.sh.match}.
- Since all binary ops from shtab_testops[] are now accounted for,
  remove unknown op error handling from this function.

src/cmd/ksh93/data/testops.c:
- shtab_testops[]:
  . Correctly sort the '<' (TEST_SLT) entry.
  . Remove ']]' (TEST_END). It's not an op and doesn't belong here.
- Update sh_opttest[] documentation with =~, \<, \>.
- Remove now-unused e_unsupported_op[] error message.

src/cmd/ksh93/sh/lex.c: sh_lex():
- Check for ']]' directly instead of relying on the removed
  TEST_END entry from shtab_testops[].

src/cmd/ksh93/tests/bracket.sh:
- Add relevant tests.

src/cmd/ksh93/tests/builtins.sh:
- Fix an old test that globally deleted the 'test' builtin. Delete
  it within the command substitution subshell only.
- Remove the test for non-support of =~ in test/[.
- Update the test for invalid test/[ op to use test directly.
2021-11-14 02:46:34 +01:00
Martijn Dekker
d25dbcc1ef [[ ... ]]: fix '!' to negate another '!'
Bug: [[ ! ! 1 -eq 1 ]] returns false, but should return true.

This bug was reported for bash, but ksh has it too:
https://lists.gnu.org/archive/html/bug-bash/2021-06/msg00006.html

Op 24-05-21 om 17:47 schreef Chet Ramey:
> On 5/22/21 2:45 PM, Vincent Menegaux wrote:
>> Previously, these commands:
>>
>>    [[ ! 1 -eq 1 ]]; echo $?
>>    [[ ! ! 1 -eq 1 ]]; echo $?
>>
>> would both result in `1', since parsing `!' set CMD_INVERT_RETURN
>> instead of toggling it.
>
> Interestingly, ksh93 produces the same result as bash. I agree
> that it's more intuitive to toggle it.

Also interesting is that '!' as an argument to the simple
'test'/'[' command does work as expected (on both bash and ksh93):
'test ! ! 1 -eq 1' and '[ ! ! 1 -eq 1 ]' return 0/true.

Even the man page for [[ is identical for bash and ksh93:

|               ! expression
|                      True if expression is false.

This suggests it's supposed to be a logical negation operator, i.e.
'!' is implicitly documented to negate another '!'. Bolsky & Korn's
1995 ksh book, p. 167, is slightly more explicit about it:
"! test-expression. Logical negation of test-expression."

I also note that multiple '!' negators in '[[' work as expected on
mksh, yash and zsh.

src/cmd/ksh93/sh/parse.c: test_primary():
- Fix bitwise logic for '!': xor the TNEGATE bit into tretyp
  instead of or'ing it, which has the effect of toggling it.
2021-06-03 15:57:16 +02:00
Martijn Dekker
d309d604e7 POSIX: 'command': don't disable declaration proprts (re: b9d10c5a)
Following the resolution of Austin Group bug 1393[*] that is set to
be included in the next version of the POSIX standard, the
'command' prefix in POSIX mode (set -o posix) no longer disables
the declaration properties of declaration built-ins.
[*] https://austingroupbugs.net/view.php?id=1393

src/cmd/ksh93/sh/parse.c: lex():
- Skip the 'command' prefix even in POSIX mode so that any
  declaration commands prefixed by it are treated as such in xec.c
  (sh_exec()).

src/cmd/ksh93/sh/xec.c: sh_exec():
- The foregoing change reintroduced a variant of BUG_CMDSPEXIT: the
  shell exits on something like 'command export readonlyvar=foo'.
  This now fixes that bug for both POSIX and non-POSIX mode. When
  calling nv_setlist() to process true shell assignments, and there
  is a 'command' prefix, push a shell context and use sigsetjmp to
  intercept any errors in assignments and stop the shell exiting.

src/cmd/ksh93/tests/builtins.sh:
- Borrow the BUG_CMDSPEXIT regression test from modernish and adapt
  it for ksh. (I'm the author so yes, I can do this.) Original:
  ae8fe9c3/lib/modernish/tst/builtin.t (L80-L109)
2021-05-04 00:52:10 +01:00
Martijn Dekker
2aad3cab06 Add ksh 93u+m contributors notice to 964 copyright headers 2021-04-26 00:19:31 +01:00
Martijn Dekker
519bb08265
Allow invoking path-bound built-in commands by direct path or preceding PATH assignment (#275)
Path-bound builtins on ksh (such as /opt/ast/bin/cat) break some
basic assumptions about paths in the shell that should hold true,
e.g., that a path output by whence -p or command -v should actually
point to an executable command. This commit should fix the
following:

1. Path-bound built-ins (such as /opt/ast/bin/cat) can now be
   executed by invoking the canonical path (independently of the
   value of $PATH), so the following will now work as expected:

        $ /opt/ast/bin/cat --version
          version         cat (AT&T Research) 2012-05-31
        $ (PATH=/opt/ast/bin:$PATH; "$(whence -p cat)" --version)
          version         cat (AT&T Research) 2012-05-31

   In the event an external command by that path exists, the
   path-bound builtin will now override it when invoked using the
   canonical path. To invoke a possible external command at that
   path, you can still use a non-canonical path, e.g.:
   /opt//ast/bin/cat or /opt/ast/./bin/cat

2. Path-bound built-ins will now also be found on a PATH set
   locally using an assignment preceding the command, so something
   like the following will now work as expected:

        $ PATH=/opt/ast/bin cat --version
          version         cat (AT&T Research) 2012-05-31

   The builtin is not found by sh_exec() because the search for
   builtins happens long before invocation-local preceding
   assignments are processsed. This only happens in sh_ntfork(),
   before forking, or in sh_fork(), after forking. Both sh_ntfork()
   and sh_fork() call path_spawn() to do the actual path search, so
   a check there will cover both cases.

   This does mean the builtin will be run in the forked child if
   sh_fork() is used (which is the case on interactive shells with
   job.jobcontrol set, or always after compiling with SHOPT_SPAWN
   disabled). Searching for it before forking would mean
   fundamentally redesigning that function to be basically like
   sh_ntfork(), so this is hard to avoid.

src/cmd/ksh93/sh/path.c: path_spawn():
- Before doing anything else, check if the passed path appears in
  the builtins tree as a pathbound builtin. If so, run it. Since a
  builtin will only be found if a preceding PATH assignment
  temporarily changed the PATH, and that assignment is currently in
  effect, we can just sh_run() the builtin so a nested sh_exec()
  invocation will find and run it.
- If 'spawn' is not set (i.e. we must return), set errno to 0 and
  return -2. See the change to sh_ntfork() below.

src/cmd/ksh93/sh/xec.c:
- sh_exec(): When searching for built-ins and the restricted option
  isn't active, also search bltin_tree for names beginning with a
  slash.
- sh_ntfork(): Only throw an error if the PID value returned is
  exactly -1. This allows path_spawn() to return -2 after running a
  built-in to tell sh_ntfork() to do the right things to restore
  state.

src/cmd/ksh93/sh/parse.c: simple():
- When searching for built-ins at parse time, only exclude names
  containing a slash if the restricted option is active. This
  allows finding pointers to built-ins invoked by literal path like
  /opt/ast/bin/cat, as long as that does not result from an
  expansion. This is not actually necessary as sh_exec() will also
  cover this case, but it is an optimisation.

src/lib/libcmd/getconf.c:
- Replace convoluted deferral to external command by a simple
  invocation of the path to the native getconf command determined
  at compile time (by src/lib/libast/comp/conf.sh). Based on:
  https://github.com/ksh93/ksh/issues/138#issuecomment-816384871
  If there is ever a system that has /opt/ast/bin/getconf as its
  default native external 'getconf', then there would still be an
  infinite recursion crash, but this seems extremely unlikely.

Resolves: https://github.com/ksh93/ksh/issues/138
2021-04-15 04:08:12 +01:00
Johnothan King
a065558291
Fix more compiler warnings, typos and other minor issues (#260)
Many of these changes are minor typo fixes. The other changes
(which are mostly compiler warning fixes) are:

NEWS:
- The --globcasedetect shell option works on older Linux kernels
  when used with FAT32/VFAT file systems, so remove the note about
  it only working with 5.2+ kernels.

src/cmd/ksh93/COMPATIBILITY:
- Update the documentation on function scoping with an addition
  from ksh93v- (this does apply to ksh93u+).

src/cmd/ksh93/edit/emacs.c:
- Check for '_AST_ksh_release', not 'AST_ksh_release'.

src/cmd/INIT/mamake.c,
src/cmd/INIT/ratz.c,
src/cmd/INIT/release.c,
src/cmd/builtin/pty.c:
- Add more uses of UNREACHABLE() and noreturn, this time for the
  build system and pty.

src/cmd/builtin/pty.c,
src/cmd/builtin/array.c,
src/cmd/ksh93/sh/name.c,
src/cmd/ksh93/sh/nvtype.c,
src/cmd/ksh93/sh/suid_exec.c:
- Fix six -Wunused-variable warnings (the name.c nv_arrayptr()
  fixes are also in ksh93v-).
- Remove the unused 'tableval' function to fix a -Wunused-function
  warning.

src/cmd/ksh93/sh/lex.c:
- Remove unused 'SHOPT_DOS' code, which isn't enabled anywhere.
  https://github.com/att/ast/issues/272#issuecomment-354363112

src/cmd/ksh93/bltins/misc.c,
src/cmd/ksh93/bltins/trap.c,
src/cmd/ksh93/bltins/typeset.c:
- Add dictionary generator function declarations for former
  aliases that are now builtins (re: 1fbbeaa1, ef1621c1, 3ba4900e).
- For consistency with the rest of the codebase, use '(void)'
  instead of '()' for print_cpu_times.

src/cmd/ksh93/sh/init.c,
src/lib/libast/path/pathshell.c:
- Move the otherwise unused EXE macro to pathshell() and only
  search for 'sh.exe' on Windows.

src/cmd/ksh93/sh/xec.c,
src/lib/libast/include/ast.h:
- Add an empty definition for inline when compiling with C89.
  This allows the timeval_to_double() function to be inlined.

src/cmd/ksh93/include/shlex.h:
- Remove the unused 'PIPESYM2' macro.

src/cmd/ksh93/tests/pty.sh:
- Add '# err_exit #' to count the regression test added in
  commit 113a9392.

src/lib/libast/disc/sfdcdio.c:
- Move diordwr, dioread, diowrite and dioexcept behind
  '#ifdef F_DIOINFO' to fix one -Wunused-variable warning and
  multiple -Wunused-function warnings (sfdcdio() only uses these
  functions when F_DIOINFO is defined).

src/lib/libast/string/fmtdev.c:
- Fix two -Wimplicit-function-declaration warnings on Linux by
  including sys/sysmacros.h in fmtdev().
2021-04-08 19:58:07 +01:00
Johnothan King
c4f980eb29
Introduce usage of __builtin_unreachable() and noreturn (#248)
This commit adds an UNREACHABLE() macro that expands to either the
__builtin_unreachable() compiler builtin (for release builds) or
abort(3) (for development builds). This is used to mark code paths
that are never to be reached.

It also adds the 'noreturn' attribute to functions that never
return: path_exec(), sh_done() and sh_syntax(). The UNREACHABLE()
macro is not added after calling these.

The purpose of these is:
* to slightly improve GCC/Clang compiler optimizations;
* to fix a few compiler warnings;
* to add code clarity.

Changes of note:

src/cmd/ksh93/sh/io.c: outexcept():
- Avoid using __builtin_unreachable() here since errormsg can
  return despite using ERROR_system(1), as shp->jmplist->mode is
  temporarily set to 0. See: https://github.com/att/ast/issues/1336

src/cmd/ksh93/tests/io.sh:
- Add a regression test for the ksh2020 bug referenced above.

src/lib/libast/features/common:
- Detect the existence of either the C11 stdnoreturn.h header or
  the GCC noreturn attribute, preferring the former when available.
- Test for the existence of __builtin_unreachable(). Use it for
  release builds. On development builds, use abort() instead, which
  crahses reliably for debugging when unreachable code is reached.

Co-authored-by: Martijn Dekker <martijn@inlv.org>
2021-04-05 00:28:24 +01:00
Johnothan King
ed478ab7e3
Fix many GCC -Wimplicit-fallthrough warnings (#243)
This commit adds '/* FALLTHROUGH */' comments to fix many
GCC warnings when compiling with -Wimplicit-fallthrough.
Additionally, the existing fallthrough comments have been
changed for consistency.
2021-03-30 21:49:20 +01:00
Martijn Dekker
21d591dbd8 parse.c: rm overlooked SHOPT_BASH stuff (re: 921bbcae)
That bit of code supported bash's redundant 'function foo()'
function declaration syntax (with both the 'function' keyword
and the '()') which is a syntax error on ksh, as it should be.
2021-03-23 20:03:18 +00: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
0b814b53bd Remove more legacy libast code (re: f9c127e3, 651bbd56)
This removes #ifdefs checking for the existence of
SH_PLUGIN_VERSION (version check for dynamically loaded builtins)
and the SFIO identifiers SF_BUFCONST, SF_CLOSING, SF_APPENDWR,
SF_ATEXIT, all of which are defined by the bundled libast.
2021-03-21 06:39:32 +00:00
Johnothan King
7ad274f8b6
Add more out of memory checks (re: 18529b88) (#192)
The referenced commit neglected to add checks for strdup() calls.
That calls malloc() as well, and is used a lot.

This commit switches to another strategy: it adds wrapper functions
for all the allocation macros that check if the allocation
succeeded, so those checks don't need to be done manually.

src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/init.c:
- Add sh_malloc(), sh_realloc(), sh_calloc(), sh_strdup(),
  sh_memdup() wrapper functions with success checks. Call nospace()
  to error out if allocation fails.
- Update new_of() macro to use sh_malloc().
- Define new sh_newof() macro to replace newof(); it uses
  sh_realloc().

All other changed files:
- Replace the relevant calls with the wrappers.
- Remove now-redundant success checks from 18529b88.
- The ERROR_PANIC error message calls are updated to inclusive-or
  ERROR_SYSTEM into the exit code argument, so libast's error()
  appends the human-readable version of errno in square brackets.
  See src/lib/libast/man/error.3

src/cmd/ksh93/edit/history.c:
- Include "defs.h" to get access to the wrappers even if KSHELL is
  not defined.
- Since we're here, fix a compile error that occurred with KSHELL
  undefined by updating the type definition of hist_fname[] to
  match that of history.h.

src/cmd/ksh93/bltins/enum.c:
- To get access to sh_newof(), include "defs.h" instead of
  <shell.h> (note that "defs.h" includes <shell.h> itself).

src/cmd/ksh93/Mamfile:
- enum.c: depend on defs.h instead of shell.h.
- enum.o: add an -I. flag in the compiler invocation so that defs.h
  can find its subsequent includes.

src/cmd/builtin/pty.c:
- Define one outofmemory() function and call that instead of
  repeating the error message call.
- outofmemory() never returns, so remove superfluous exit handling.

Co-authored-by: Martijn Dekker <martijn@inlv.org>
2021-02-27 21:21:58 +00:00
Martijn Dekker
18529b88c6 Add lots of checks for out of memory (re: 0ce0b671)
Huge typeset -L/-R adjustment length values were still causing
crashses on sytems with not enough memory. They should error out
gracefully instead of crashing.

This commit adds out of memory checks to all malloc/calloc/realloc
calls that didn't have them (which is all but two or three).

The stkalloc/stakalloc calls don't need the checks; it has
automatic checking, which is done by passing a pointer to the
outofspace() function to the stakinstall() call in init.c.

src/lib/libast/include/error.h:
- Change the ERROR_PANIC exit status value from ERROR_LEVEL (255)
  to 77, which is what it is supposed to be according to the libast
  error.3 manual page. Exit statuses > 128 for anything else than
  signals are not POSIX compliant and may cause misbehaviour.

src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/init.c:
- To facilitate consistency, add a simple extern sh_outofmemory()
  function that throws an ERROR_PANIC "out of memory".

src/cmd/ksh93/include/shell.h,
src/cmd/ksh93/data/builtins.c:
- Remove now-redundant e_nospace[] extern message; it is now only
  used in one place so it might as well be a string literal in
  sh_outofmemory().

All other changed files:
- Verify the result of all malloc/calloc/realloc calls and call
  sh_outofmemory() if they fail.
2021-02-21 22:27:28 +00:00
Martijn Dekker
403e864ac8 Disallow >;word and <>;word for 'redirect' (re: 7b59fb80, 7b82c338)
The >;word and <>;word redirection operators cannot be used with
the 'exec' builtin, but the 'redirect' builtin (which used to be
an alias of 'command exec') permitted them. However, they do not
have the documented effect of the added ';'. So this commit blocks
those operators for 'redirect' as they are blocked for 'exec'.

It also tweaks redirect's error message if a non-redirection
argument is encountered.

src/cmd/ksh93/sh/parse.c: simple():
- Set the lexp->inexec flag for SYSREDIR (redirect) as well as
  SYSEXEC (exec). This flag is checked for in sh_lex() (lex.c) to
  throw a syntax error if one of these two operators is used.

src/cmd/ksh93/sh.1, src/cmd/ksh93/data/builtins.c:
- Documentation tweaks.

src/cmd/ksh93/sh/xec.c, src/cmd/ksh93/bltins/misc.c:
- When 'redirect' gives an 'incorrect syntax' (e_badsyntax) error
  message, include the first word that was found not to be a valid
  redirection. This is simply the first argument, as redirections
  are removed from the arguments list.

src/cmd/ksh93/tests/io.sh:
- Update test to reflect new error message format.
2021-02-07 03:23:56 +00:00
Martijn Dekker
288b6c6517 Fix various possible uses of uninitialised variables
Patch from OpenSUSE, slightly adapted for 93u+m. Source:
https://build.opensuse.org/package/view_file/shells/ksh/ksh93-uninitialized.dif
2021-01-28 04:54:41 +00:00
Martijn Dekker
d2cc520883 Disable SHOPT_KIA (ksh -R) by default
SHOPT_KIA enables the -R option that generates a cross-reference
database from a script. However, no tool to analyse this database
is shipped or seems to be available anywhere (in spite of multiple
people looking for one), and the format is very opaque. No usage
examples are known or findable on the internet. This seems like it
should not be compiled in by default, although we'll keep the code
in case some way to use it is found.

src/cmd/ksh93/SHOPT.sh:
- Disable SHOPT_KIA by default by removing the default 1 value.

src/cmd/ksh93/sh/args.c, src/cmd/ksh93/sh/parse.c:
- Fix a couple of preprocessor logic bugs that made it impossible
  to compile ksh without SHOPT_KIA.

src/cmd/ksh93/data/builtins.c:
- Fix typo in -R doc in ksh --man (in case SHOPT_KIA is enabled).

src/cmd/ksh93/sh.1:
- Since sh.1 is not generated dynamically, remove the -R doc.
2021-01-23 18:26:38 +00:00
Martijn Dekker
7fdeadd4f1 Increase shcomp bytecode header version; doc updates
src/cmd/ksh93/include/version.h:
- Centrally define the 93u+m copyright (SH_RELEASE_CPYR) for adding
  to the original AT&T copyright in 'ksh --man' and 'shcomp --man'.
- Centrally define the binary header version number for bytecode
  generated by shcomp: SHCOMP_HDR_VERSION.
- Bump SHCOMP_HDR_VERSION from 3 to 4. Converting all the preset
  aliases to builtin commands has caused new bytecode to be
  incompatible with old ksh. (However, old bytecode runs fine on
  93u+m, because shcomp pre-expands the preset aliases.)

src/cmd/ksh93/sh/shcomp.c:
- Instead of keeping its own version date (not changed since 2003),
  use the same version string as ksh itself (SH_RELEASE).
- Use SH_RELEASE_CPYR for the extra 93u+m copyright string.
- Use SHCOMP_HDR_VERSION for the bytecode header.

src/cmd/ksh93/sh/parse.c: sh_parse():
- Use SHCOMP_HDR_VERSION for the bytecode version check.

src/cmd/ksh93/data/builtins.c: opt_ksh[]:
- Use SH_RELEASE_CPYR for the extra 93u+m copyright string.

src/cmd/ksh93/COMPATIBILITY:
- Mention that 93u+m shcomp bytecode won't run on older ksh.
- Document changes in printf %T (re: 9526b3fa).

src/cmd/ksh93/README:
- Mention that we run on UnixWare (with major regressions).
  https://github.com/ksh93/ksh/pull/159#issuecomment-764667929
2021-01-21 17:04:14 +00: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
55f0f8ce52 -o posix: disable '[ -t ]' == '[ -t 1 ]' hack
On ksh93, 'test -t' is equivalent to 'test -t 1' (and of course
"[ -t ]" is equivalent to "[ -t 1 ]").

This is purely for compatibility with ancient Bourne shell
breakage. No other shell supports this. ksh93 should probably keep
it for backwards compatibility, but it should definitely be
disabled in POSIX mode as it is a violation of the standard; 'test
-t' is an instance of 'test "$string"', which tests if the string
is empty, so it should test if the string '-t' is empty (quod non).

This also replaces the fix for 'test -t 1' in a command
substitution with a better one that avoids forking (re: cafe33f0).

src/cmd/ksh93/sh/parse.c:
- qscan(): If the posix option is active, disable the parser-based
  hack that converts a simple "[ -t ]" to "[ -t 1 ]".

src/cmd/ksh93/bltins/test.c:
- e3(): If the posix option is active, disable the part of the
  compatibility hack that was used for compound expressions
  that end in '-t', e.g. "[ -t 2 -o -t ]".
- test_unop(): Remove the forking fix for "[ -t 1 ]".

src/cmd/ksh93/edit/edit.c:
- tty_check(): This function is used by "[ -t 1 ]" and in other
  contexts as well, so a fix here is more comprehensive. Forking
  here would cause a segfault, but we don't actually need to. This
  adds a fix that simply returns false if we're in a virtual
  subshell that is also a command substitution. Since command
  substitutions always fork upon redirecting standard output within
  them (making them no longer virtual), it is safe to do this.

src/cmd/ksh93/tests/bracket.sh
- Add comprehensive regression tests for test/[/[[ -t variants in
  command substitutions, in simple and compound expressions, with
  and without redirecting stdout to /dev/tty within the comsub.
- Add tests verifying that -o posix disables the old hack.
- Tweak other tests, including one that globally disabled xtrace.
2020-09-01 20:24:44 +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
3613da4240 Remove unused libcoshell
The coshell(1) command, which is required for libcoshell to be
useful, is not known to be shipped by any distribution. It was
removed by the ksh-community fork and hence also by 93u+m (in
2940b3f5). The coshell facility as a whole is obsolete and
insecure. For a long time now, the statically linked libcoshell
library has been 40+ kilobytes of dead weight in the ksh binary.

Prior discussion (ksh2020): https://github.com/att/ast/issues/619

src/lib/libcoshell/*:
- Removed.

src/cmd/ksh93/*:
- Remove the SHOPT_COSHELL compiler option (which was enabled) and
  a lot of code that was conditional upon #ifdef SHOPT_COSHELL.

- init.c: e_version[]: Removing SHOPT_COSHELL changed the "J"
  feature identifier in ${.sh.version} to a lowercase "j", which
  was conditional upon SHOPT_BGX (background job extensions).
  But src/cmd/ksh93/RELEASE documents (at 08-12-04, on line 1188):
    | +SHOPT_BGX enables background job extensions. Noted by "J" in
    |  the version string when enabled. [...]
  That is the only available documentation. So change that "j" back
  to a "J", leaving the version string unchanged after this commit.

- jobs.c: job_walk(): We need to keep one 'job_waitsafe(SIGCHLD);'
  call that was conditional upon SHOPT_COSHELL; removing it caused
  a regression test failure in tests/sigchld.sh, 'SIGCHLD blocked
  for script at end of pipeline' (which means that until now, a ksh
  compiled without libcoshell had broken SIGCHLD handling.)

bin/package, src/cmd/INIT/package.sh:
- Don't export COSHELL variable.
2020-07-17 19:28:52 +01:00
Martijn Dekker
1fbbeaa19d Convert default typeset aliases to regular builtins
This converts the 'autoload', 'compound', 'float', 'functions',
'integer' and 'nameref' default aliases into regular built-in
commands, so that 'unalias -a' does not remove them. Shell
functions can now use these names, which improves compatibility
with POSIX shell scripts.

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

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

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

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

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

src/cmd/ksh93/tests/variables.sh:
- Adapt a regression test as there is no more 'integer' alias.
2020-07-15 20:54:06 +01:00
Johnothan King
c4236cc295 Fix type names starting with lowercase 'a' (#69)
Type names that start with a lowercase 'a' cause an error when used:

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

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

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

src/cmd/ksh93/tests/types.sh:
- Add a regression test for type names starting with a lowercase 'a'.
2020-07-10 17:54:51 +01:00
Martijn Dekker
588a1ff7ca Fix spurious warning output in KIA (-R) database file
The ksh -R option creates a cross-reference database that can be
parsed with a "C Query Language" (CQL) tool.
See cql-1994.pdf at: http://gsf.cococlyde.org/files

The -R option puts ksh in noexec mode as it parses the script, and
this can produce warnings as the syntax is parsed. The bug is that
these warnings can end up in the database file, corrupting it.

This applies a fix from Paulo Andrade, via Siteshwar Vashisht:
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01952.html

src/cmd/ksh93/sh/parse.c:
- Terminate names with a zero character when writing database
  output.

A regression test is not very feasible because the majority of the
database output consists of cryptic IDs/hashes that vary depending
on the session and/or system and possibly other things.
2020-07-09 23:18:41 +01:00
Johnothan King
6930666234
Fix a syntax error when ((...)) is combined with redirections (#68)
This bugfix was backported from ksh93v- 2013-10-10-alpha.

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

src/cmd/ksh93/tests/arith.sh:
- Add a simple regression test.
2020-07-09 22:12:04 +01:00
Martijn Dekker
ef1621c18f Make 'source' a regular built-in
The 'source' alias is now converted into a regular built-in command
so that 'unalias -a' does not remove it, and something like
	cmd=source; $cmd name args
will now work.

This is part of the project to replace default aliases that define
essential commands by proper builtins that act identically (except
you now get the actual command's name in any error/usage messages).

src/cmd/ksh93/data/aliases.c:
- Remove 'source' default alias.

src/cmd/ksh93/data/builtins.c,
src/cmd/ksh93/include/builtins.h:
- Define 'source' regular builtin with extra parser ID "SYSSOURCE".
  Same definition as '.', minus the BLT_SPC flag indicating a
  special builtin. This preserves the behaviour of 'command .'.
- Update sh_optdot[] to include info for 'source --man'.
  (Note that \f?\f expands to the current command name.
  This allows several commands to share a single --man page.)

src/cmd/ksh93/sh/parse.c:
- In the two places that SYSDOT is checked for, also check for
  SYSSOURCE, making sure the two commands are parsed identically.

src/cmd/ksh93/sh.1:
- Remove 'source' default alias.
- Document 'source' regular builtin.
2020-06-15 11:33:44 +02:00