1
0
Fork 0
mirror of git://git.code.sf.net/p/cdesktopenv/code synced 2025-02-13 19:52:20 +00:00
Commit graph

1112 commits

Author SHA1 Message Date
Anuradha Weeraman
de2b4a6f97
edit/edit.c: fix compiler warnings (#31)
This fixes compiler warnings for implicit-ints:
warning: return type defaults to 'int' [-Wimplicit-int]
 1854:     sh_tcgetattr(int fd, struct termios *tty)
 1863:     sh_tcsetattr(int fd, int cmd, struct termios *tty)

cmd/ksh93/edit/edit.c:
- Set the return type explicitly to int and align with the prototype
  declared in include/terminal.h.
2020-06-21 01:47:11 +01:00
Martijn Dekker
9d428f8f5e Fix erroneous fork after 'readonly PATH' in subshell (re: 102868f8)
After making PATH readonly in a virtual subshell (without otherwise
changing it, so the subshell is never forked), then the main shell
would erroneously fork into a background process immediately after
leaving the virtual subshell. This was caused by a bug in the
forking workaround that prevents changes in PATH in a virtual
subshell from clearing the parent shell's hash table.

src/cmd/ksh93/sh/name.c: nv_putval():
- If we're either setting or restoring PATH, do an additional check
  for the NV_RDONLY flag, which means the function was told to
  ignore the variable's readonly state. It is told to ignore that
  when restoring the parent shell state after exiting a virtual
  subshell. If we don't fork then, we don't fork the parent shell.

src/cmd/ksh93/tests/subshell.sh:
- Add regression test verifying that no forking happens when making
  PATH readonly in a subshell.

Fixes #30.
2020-06-20 23:47:42 +02:00
Johnothan King
bd3e2a8001
Fix unreliable behavior when special vars are readonly or unset (#27)
src/cmd/ksh93/data/variables.c:
 - Running 'unset .sh.lineno' creates a memory fault, so fix that
   by giving it the NV_NOFREE attribute. This crash was happening
   because ${.sh.lineno} is an integer that cannot be freed from
   memory with free(3).

src/cmd/ksh93/sh/init.c:
 - Tell _nv_unset to ignore NV_RDONLY when $RANDOM and $LINENO are
   restored from the subshell scope. This is required to fully
   restore the original state of these variables after a virtual
   subshell finishes.

src/cmd/ksh93/bltins/typeset.c,
src/cmd/ksh93/sh/subshell.c:
 - Disabled some optimizations for two instances of 'sh_assignok' to
   fix 'readonly' in virtual subshells and '(unset .sh.level)' in
   nested functions. This fixes the following variables when
   '(readonly $varname); enum varname=' is run:

   $_
   ${.sh.name}
   ${.sh.subscript}
   ${.sh.level}

   The optimization in question prevents sh_assignok from saving the
   original state of these variables by making the sh_assignok call
   a no-op. Ksh needs the original state of a variable for it to be
   properly restored after a virtual subshell has run, otherwise ksh
   will simply carry over any new flags (being NV_RDONLY in this case)
   from the subshell into the main shell.

src/cmd/ksh93/tests/variables.sh:
 - Add regression tests from Martijn Dekker for setting special
   variables as readonly in virtual subshells and for unsetting
   special variables in general.

Fixes #4
2020-06-20 18:08:41 +01:00
Anuradha Weeraman
ee698e89d5
Fix compiler warning in INIT/ratz.c (#28)
src/cmd/INIT/ratz.c:
- Fix build warning:

  src/cmd/INIT/ratz.c:4741:2: warning: case label value exceeds maximum value for
   type
   4741 |  case 0241:
        |  ^~~~

  The character literal in the switch expression was being treated
  as a signed char while the case label 0241 is greater than 127,
  resulting in this warning.
2020-06-20 12:42:53 +01:00
Johnothan King
99065353b3 Fix 'whence -a' to print correct path for tracked alias (#25)
'whence -a' bases the path for tracked aliases on the user's
current working directory if an enabled ksh builtin of the same
name is also available. The following example will claim 'cat'
is in the user's current working directory:

$ whence -a cat
cat is a tracked alias for /usr/bin/cat
$ builtin cat
$ whence -a cat
cat is a shell builtin
cat is /usr/bin/cat
cat is a tracked alias for /current/working/directory/cat

This patch from ksh2020 fixes this problem by properly saving the
path of the tracked alias for use with 'whence -a', since
'path_pwd' (as implied by the function's name) only gets the users
current working directory, not the location of tracked aliases.
Ref.: https://github.com/att/ast/issues/1049

This bug was originally reported by David Morano about two decades
ago to the AST team: https://github.com/att/ast/issues/954

src/cmd/ksh93/bltins/whence.c:
 - Print the actual path of a tracked alias, path_pwd doesn't
   have this functionality.

src/cmd/ksh93/include/name.h:
 - Add 'pathcomp' for saving the value of tracked aliases.

src/cmd/ksh93/sh/path.c:
 - Save the value of tracked aliases for use by whence.

src/cmd/ksh93/tests/builtins.sh:
 - Add a regression test for using 'whence -a' on tracked
   aliases with a builtin equivalent.
2020-06-19 14:03:58 +02:00
Martijn Dekker
876da711c0 data/builtins.c: fix typo in 'getopts' usage message 2020-06-18 12:12:15 +02:00
Martijn Dekker
57af42d968 data/builtins.c: tweaks for {exec,redirect} --man (re: 7b82c338)
src/cmd/ksh93/data/builtins.c:
- sh_optexec[], sh_optdirect[]: Move paragraphs on exit behaviour
  to the EXIT STATUS section, where people would look for them.
- sh_optexec[]: Since we modified b_exec() to support 'redirect',
  add "ksh93" to the credits to avoid blaming AT&T for our changes.
2020-06-18 11:52:32 +02:00
Martijn Dekker
8b51d12f4b data/builtins.c: cosmetic fix in comment 2020-06-18 03:40:29 +02:00
Martijn Dekker
3e3f6b0f12 Restore #22 'unset -f' fix minus segfault (re: b7932e87, 97511748)
Applying the fix for 'unset -f' exposed a crashing bug in lookup()
in sh/nvdisc.c, which is the function for looking up discipline
functions. This is what caused tests/variables.sh to crash.
Ref.: https://github.com/ksh93/ksh/issues/23#issuecomment-645699614

src/cmd/ksh93/sh/nvdisc.c: lookup():
- To avoid segfault, check that the function pointer nq->nvalue.rp
  is actually set before checking if nq->nvalue.rp->running==1.

src/cmd/ksh93/sh/xec.c,
src/cmd/ksh93/tests/functions.sh:
- Uncomment the 'unset -f' fix from b7932e87.

Resolves #21 (again).
2020-06-18 02:48:51 +02:00
Martijn Dekker
975117485c Part revert #22 to undo memory fault (re: b7932e87)
The fix in sh/xec.c, which was backported from the ksh 93v- beta to
delay the actual removal of a running function that unsets itself,
caused a segfault in the variables.sh regression tests (see #23).

src/cmd/ksh93/sh/xec.c:
- Comment out the backported code pending a correct fix for #21.
  Now both types of functions silently fail to unset themselves
  (unless they're discipline functions).

src/cmd/ksh93/tests/functions.sh:
- Disable regression tests checking that the function was actually
  unset, pending a correct fix for #21.

Resolves: #23
Reopens: #21
2020-06-17 21:01:55 +02:00
Johnothan King
b7932e87b6
Fix two problems with 'unset -f' behavior (#22)
src/cmd/ksh93/sh/name.c:
 - Correct the check for when a function is currently running
   to fix a segmentation fault that occurred when a POSIX
   function tries to unset itself while it is running.
   This bug fix was backported from ksh93v-.

src/cmd/ksh93/sh/xec.c:
 - If a function tries to unset itself, unset the function
   with '_nv_unset(np, NV_RDONLY)' to fix a silent failure.
   This fix was also backported from ksh93v-.

src/cmd/ksh93/tests/functions.sh:
 - Add four regression tests for when a function unsets itself.

Resolves #21
2020-06-17 18:26:43 +01:00
Martijn Dekker
746ce73671 regress: don't count temp dir creation as test (re: 2318de32)
Note that shtests simply does a 'grep -c err_exit' and substracts 1
to count the number of regression tests in a test script. Not all
test scripts make temp dirs, so subtracting 2 instead won't do.

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

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

src/cmd/ksh93/tests/subshell.sh:
- check_hash_table():
  - Take line number as 1st argument.
  - Quote a character in err_exit to bypass the alias when calling
    it, so we can pass on the argument for the line number. This
    also stops this helper function from being counted as a test.
- When calling check_hash_table(), pass $LINENO.
- Add dummy err_exit comments to have the tests counted.
2020-06-17 16:07:09 +02:00
Johnothan King
fae8862c53
Fix assignments preceding 'command <special builtin>' (#19)
Ksh was not checking for `command` when running a special builtin,
which caused preceding invocation-local variable assignments to
become global. This is the reproducer from the att/ast#72:

$ foo=BUG command eval ':'
$ echo "$foo"

This no longer prints 'BUG', as ksh now makes sure the command builtin
is not running a special builtin before making invocation-local
variable assignments global.

src/cmd/ksh93/sh/xec.c:
 - Backport the bugfix for BUG_CMDSPASGN from ksh93v- 2013-10-10-alpha.

src/cmd/ksh93/tests/builtins.sh:
 - Add a regression test based on the reproducer in att/ast#72.
2020-06-16 22:58:05 +01:00
Johnothan King
c258a04f7a
Implement a portable fix for SIGCHLD crashes (#18)
As previously reported in rhbz#1112306 (https://bugzilla.redhat.com/show_bug.cgi?id=1112306),
ksh may crash when receiving SIGCHLD because GCC's optimizer will fail to generate
`addl` and `sub` instructions to increment and decrement `job.in_critical` in the
`job_subsave` function. This bug *does* occur in GCC 10 with `-O2`, but not `-O1`;
it doesn't appear this bug has been fixed. As a reference, here is the relevant
debug assembly output of `job_subsave` when KSH is compiled with `CCFLAGS` set to
`-g -O1`:

0000000000034c97 <job_subsave>:

void *job_subsave(void)
{
   34c97:       53                      push   %rbx
        struct back_save *bp = new_of(struct back_save,0);
   34c98:       bf 18 00 00 00          mov    $0x18,%edi
   34c9d:       e8 34 4a 0a 00          callq  d96d6 <_ast_malloc>
   34ca2:       48 89 c3                mov    %rax,%rbx
        job_lock();
   34ca5:       83 05 3c 50 13 00 01    addl   $0x1,0x13503c(%rip)        # 169ce8 <job+0x28>
        *bp = bck;
   34cac:       66 0f 6f 05 4c 5a 13    movdqa 0x135a4c(%rip),%xmm0        # 16a700 <bck>
   34cb3:       00
   34cb4:       0f 11 00                movups %xmm0,(%rax)
   34cb7:       48 8b 05 52 5a 13 00    mov    0x135a52(%rip),%rax        # 16a710 <bck+0x10>
   34cbe:       48 89 43 10             mov    %rax,0x10(%rbx)
        bp->prev = bck.prev;
   34cc2:       48 8b 05 47 5a 13 00    mov    0x135a47(%rip),%rax        # 16a710 <bck+0x10>
   34cc9:       48 89 43 10             mov    %rax,0x10(%rbx)
        bck.count = 0;
   34ccd:       c7 05 29 5a 13 00 00    movl   $0x0,0x135a29(%rip)        # 16a700 <bck>
   34cd4:       00 00 00
        bck.list = 0;
   34cd7:       48 c7 05 26 5a 13 00    movq   $0x0,0x135a26(%rip)        # 16a708 <bck+0x8>
   34cde:       00 00 00 00
        bck.prev = bp;
   34ce2:       48 89 1d 27 5a 13 00    mov    %rbx,0x135a27(%rip)        # 16a710 <bck+0x10>
        job_unlock();
   34ce9:       8b 05 f9 4f 13 00       mov    0x134ff9(%rip),%eax        # 169ce8 <job+0x28>
   34cef:       83 e8 01                sub    $0x1,%eax
   34cf2:       89 05 f0 4f 13 00       mov    %eax,0x134ff0(%rip)        # 169ce8 <job+0x28>
   34cf8:       75 2b                   jne    34d25 <job_subsave+0x8e>
   34cfa:       8b 3d ec 4f 13 00       mov    0x134fec(%rip),%edi        # 169cec <job+0x2c>
   34d00:       85 ff                   test   %edi,%edi
   34d02:       74 21                   je     34d25 <job_subsave+0x8e>
   34d04:       c7 05 da 4f 13 00 01    movl   $0x1,0x134fda(%rip)        # 169ce8 <job+0x28>

When `-O2` is used instead of `-O1`, the `addl` and `sub` instructions for
incrementing and decrementing the lock are removed. GCC instead generates a
broken `mov` instruction for `job_lock` and removes the initial `sub` instruction
in job_unlock (this is also seen in Red Hat's bug report):

       job_lock();
       *bp = bck;
  37d7c:       66 0f 6f 05 7c 79 14    movdqa 0x14797c(%rip),%xmm0        # 17f700 <bck>
  37d83:       00
       struct back_save *bp = new_of(struct back_save,0);
  37d84:       49 89 c4                mov    %rax,%r12
       job_lock();
  37d87:       8b 05 5b 6f 14 00       mov    0x146f5b(%rip),%eax        # 17ece8 <job+0x28>
...
        job_unlock();
  37dc6:       89 05 1c 6f 14 00       mov    %eax,0x146f1c(%rip)        # 17ece8 <job+0x28>
  37dcc:       85 c0                   test   %eax,%eax
  37dce:       75 2b                   jne    37dfb <job_subsave+0x8b>

The original patch works around this bug by using the legacy `__sync_fetch_and_add/sub`
GCC builtins. This forces GCC to generate instructions that change the lock with
`lock addl`, `lock xadd` and `lock subl`:

       job_lock();
  37d9f:       f0 83 05 41 6f 14 00    lock addl $0x1,0x146f41(%rip)        # 17ece8 <job+0x28>
  37da6:       01
...
       job_unlock();
  37deb:       f0 0f c1 05 f5 6e 14    lock xadd %eax,0x146ef5(%rip)        # 17ece8 <job+0x28>
  37df2:       00
  37df3:       83 f8 01                cmp    $0x1,%eax
  37df6:       74 08                   je     37e00 <job_subsave+0x70>
...
  37e25:       74 11                   je     37e38 <job_subsave+0xa8>
  37e27:       f0 83 2d b9 6e 14 00    lock subl $0x1,0x146eb9(%rip)        # 17ece8 <job+0x28>

While this does work, it isn't portable. This patch implements a different
workaround for this compiler bug. If `job_lock` is put at the beginning of
`job_subsave`, GCC will generate the required `addl` and `sub` instructions:

       job_lock();
  37d67:       83 05 7a 5f 14 00 01    addl   $0x1,0x145f7a(%rip)        # 17dce8 <job+0x28>
...
        job_unlock();
  37dbb:       83 e8 01                sub    $0x1,%eax
  37dbe:       89 05 24 5f 14 00       mov    %eax,0x145f24(%rip)        # 17dce8 <job+0x28>

It is odd that moving a single line of code fixes this problem, although
GCC _should_ have generated these instructions from the original code anyway.
I'll note that this isn't the only way to get these instructions to generate.
The problem also seems to go away when inserting almost anything else inside
of the code for `job_subsave`. This is just a simple workaround for a strange
compiler bug.
2020-06-16 22:44:02 +01:00
Johnothan King
764acefaf1 read -r -d should not ignore -r
This bug was previously reported in att/ast#37.
Ksh ignores `-r` when `read -r -d` is run because when
the bit for `D_FLAG` is set, the bit for `R_FLAG` is unset
as a side effect of setting `D_FLAG`. The following set
of commands fails to print a backslash:

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

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

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

src/cmd/ksh93/tests/builtins.sh:
 - Add the regression test for `read -r -d` from ksh93v-.
2020-06-16 13:49:23 +01:00
Martijn Dekker
ad349c7668 silence macro redefinition warnings (re: 7003aba4)
src/cmd/ksh93/bltins/test.c,
src/cmd/ksh93/sh/arith.c,
src/cmd/ksh93/sh/streval.c:
- #undef ERROR_exit before redefining it, so clang stops nagging.
2020-06-16 04:51:21 +02:00
Martijn Dekker
a9de50bf79 Apply simple optimisation for ${ subshare; } (re: 3d38270b)
Running shbench after undoing the incorrect subshell optimisation
showed that the performance of ${ subshare; }-type command
substitutions went down very slightly, but consistently.

The main purpose of using this ksh-specific type of command
substitution vs. a normal one is performance. Thus, it *is*
appropriate to eke every last bit of performance out of it that
we can, provided correctness is completely preserved.

It is also a type of command substitution where every change is
supposed to be shared with the main shell environment; only command
output is captured in a subshell-like fashion.

Thus, on the face of it, it would be a logical optimisation for
sh_assignok() to avoid bothering with saving a subshell context for
variables if we're in a subshare.

Lo and behold, applying it does not introduce any regress fails.

Here are my average results of the braces.ksh benchmark from
shbench <http://fossil.0branch.com/csb/tktnew> against stock
/bin/ksh 93u+ vs. current 93u+m (same compiler flags),
100 runs pre-optimisation and 100 runs post-optimisation:

Stock /bin/ksh:		Pre-optimisation (at 3d38270b):
93u+: 0.743 secs	93u+m: 0.739 secs

Stock /bin/ksh:		Post-optimisation (now):
93u+: 0.744 secs	93u+m: 0.726 secs

The left column shows only a small margin of error with 100 runs;
the right one shows a very small, but not insignificant difference.

However, these tests were not very rigorous with 100 runs each.
If anyone wants to do it properly, please report results to
korn-shell@googlegroups.com. I'm happy enough with this, though.

Thanks to Joerg van den Hoff for providing shbench, without
which it would not have occurred to me to try this.

src/cmd/ksh93/sh/subshell.c: sh_assignok():
- Don't bother if we're in a ${ subshare; }.
2020-06-15 20:27:32 +02:00
Martijn Dekker
6916a873c2 optget: display --help and --man in terse usage messages
The fact that every ksh builtin command self-documents with the
options --help and --man (and others, see 'getopts --man'; but
these are the essential ones) is poorly known; the information is
buried somewhere deep in the sh.1 manual page, and is incomplete at
that. None of the terse usage messages displayed on error point the
user to these options. So let's fix that.

src/lib/libast/misc/optget.c:
- Change generic 'options' placeholder, used in all terse usage
  messages, to 'options | --help | --man'.

src/cmd/ksh93/sh.1:
- Edit documentation of --man/-?, adding documentation on --help
  which was completely undocumented. Refer to 'getopts --man' for
  more advanced info.
- Separate these from the (important) documentation on special
  builtins using a paragraph break.
2020-06-15 16:56:11 +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
Martijn Dekker
503a596a3e
Merge pull request #16 from JohnoKing/fix-optimized-variables
Remove a buggy optimization for variables in subshells

Fixes #15
2020-06-15 15:44:50 +01:00
Johnothan King
3d38270b32 Remove a buggy optimization for variables in subshells
This bug was originally reported by @lijog in att/ast#7 and has been
reported again in #15. KSH does not save the state of a variable if it
is in a newer scope. This is because of an optimization in sh_assignok
first introduced in ksh93t+ 2010-05-24. Here is the code change in that
version:

                return(np);
        /* don't bother to save if in newer scope */
-       if(!(rp=shp->st.real_fun)  || !(dp=rp->sdict))
-               dp = sp->var;
-       if(np->nvenv && !nv_isattr(np,NV_MINIMAL|NV_EXPORT) && shp->last_root)
-               dp = shp->last_root;
-       if((mp=nv_search((char*)np,dp,HASH_BUCKET))!=np)
-       {
-               if(mp || !np->nvfun || np->nvfun->subshell>=sh.subshell)
-                       return(np);
-       }
+       if(sp->var!=shp->var_tree && shp->last_root==shp->var_tree)
+               return(np);
        if((ap=nv_arrayptr(np)) && (mp=nv_opensub(np)))
        {

This change was originally made to replace a buggier optimization.
However, the current optimization causes variables set in subshells
to wrongly affect the environment outside of the subshell, as the
variable does not get set back to its original value. This patch
simply removes the buggy optimization to fix this problem.

src/cmd/ksh93/sh/subshell.c:
 - Remove a buggy optimization that caused variables set in subshells
   to affect the environment outside of the subshell.

src/cmd/ksh93/tests/subshell.sh:
 - Add a regression test for setting variables in subshells. This
   test has to be run from the disk after being created with a here
   document because it always returns the expected result when run
   directly in the regression test script.
2020-06-15 07:13:38 -07:00
Martijn Dekker
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
Martijn Dekker
b7f48e8a10 Revert job locking patch (re: 07cc71b8, 58560db7)
This reverts the patch for the job_lock and job_unlock macros.

As I said in 58560db7, this is very probably a workaround for an
optimiser bug in certain versions of GCC, at least 2017 versions.

In the version I committed, that workaround version never gets
used, because you cannot use #if defined(...) to check for the
presence of a compiler builtin. Thanks to Johnothan King for
keeping an eye on my code and pointing that out to me.

What is needed to test for the presence of a compiler builtin is a
builtin macro called __has_builtin (and it *can* be tested for
using #if defined...()).. This is a clang invention. It was not
added to gcc until version 10, which was only released in a first
stable version just over a month ago.
Ref.: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66970#c14
      https://gcc.gnu.org/gcc-10/

However, for gcc 10, it seems unlikely the patch is still needed.
(Although it would certainly be a good idea to test that.)

And for the older gcc versions that do need it, we cannot use
__has_builtin, which means we need to define a dummy that always
returns false, so the workaround version is never used.
Ref.: https://github.com/ksh93/ksh/commit/58560db7#commitcomment-39900259

And we cannot use the workaround version unconditionally either,
because it would cause build failures on compilers without the
__sync_fetch_and_add() and __sync_fetch_and_sub() builtins.

Which means the only sensible thing left to do is to revert the
patch for now.

As far as I can tell at this point, for the patch to return to this
upstream in a sensible manner, someone would need to:

1. Write a small C program that tests these macros and somehow
   checks for the presence of that optimiser bug. (This is not
   going to come from me; my C-fu is simply not good enough.)

2. Incorporate that into the distribution as a test for iffe.
   (Few know how iffe works, but it's probably not that hard as
   there are plenty of existing tests to use as a template.)

3. Reinsert the workaround version using the macro defined (or not)
   by that iffe test, so that it is only compiled in when using a
   compiler that actually has the bug in question.

Until then, this can just continue to be an OS-specific patch for
systems with GCC versions that might have that bug.
2020-06-15 03:20:45 +02:00
Martijn Dekker
58560db768 Restore build without __sync_fetch_and_{add,sub} (re: 07cc71b8)
The more I think about it, the more it seems obvious that commit
07cc71b8 (PR #14) is quite simply a workaround for a GCC optimiser
bug, and (who knows?) possibly an old, long-fixed one, as the bug
report is years old.

The commit also caused ksh to fail to build on HP-UX B.11.11 with
GCC 4.2.3 (hosted at polarhome.com), because it doesn't have
__sync_fetch_and_add() and __sync_fetch_and_sub(). It may fail on
other systems. The GCC documentation says these are legacy:
https://gcc.gnu.org/onlinedocs/gcc/_005f_005fsync-Builtins.html

HELP WANTED: what I would like best is if someone could come up
with some way of detecting this optimiser bug and then error out
with a message along the lines of "please upgrade your broken
compiler". It would probably need to be a new iffe test.

Meanwhile, let's try it this way for a while and see what happens:

src/cmd/ksh93/include/jobs.h:
- Restore original ksh version of job_lock()/job_unlock() macros.
- Use the workaround version only if the compiler has the builtins
  __sync_fetch_and_add() and __sync_fetch_and_sub().
2020-06-14 23:49:07 +02:00
Martijn Dekker
f95d3105ef
Merge pull request #14 from aweeraman/debian-patches-2
ksh segfaults in job_chksave after receiving SIGCHLD

https://bugs.launchpad.net/ubuntu/+source/ksh/+bug/1697501
Eric Desrochers wrote on 2017-06-12:

[Impact]
* The compiler optimization dropped parts from the ksh job
  locking mechanism from the binary code. As a consequence, ksh
  could terminate unexpectedly with a segmentation fault after
  it received the SIGCHLD signal.

[Test Case]

Unfortunately, there is no clear and easy way to reproduce the
segfault.
* But the original reporter of this bug can randomly reproduce
  the problem using an in-house ksh script that only works
  inside his infrastructure as follow : "ksh
  <in-house-script.ksh>" and then once in a while ksh will
  segfault as follow :

(gdb) bt
#0 job_chksave (pid=pid@entry=19003) at /build/ksh-6IEHIC/ksh-93u+20120801/src/cmd/ksh93/sh/jobs.c:1948
#1 0x00000000004282ab in job_reap (sig=17) at /build/ksh-6IEHIC/ksh-93u+20120801/src/cmd/ksh93/sh/jobs.c:428
#2 <signal handler called>
...

[Regression Potential]
* Regression risk : low/none expected, the package has been
  highly/intensively tested by a user who run over 18M ksh
  scripts a day on each of their clusters.
[...]
* The fix has been written by RH and has been proven to work for
  them for the last 3 years.
* A test package including the RH fix has been intensively tested
  and verified (pre-SRU) by an affected user with positive
  feedbacks using a reproducer that segfault without the RH
  patch.
* Test package (pre-SRU) feedbacks :
  https://bugs.launchpad.net/ubuntu/xenial/+source/ksh/+bug/1697501/comments/7

[Other Info]
* Details about the RH bug :
  - https://bugzilla.redhat.com/show_bug.cgi?id=1123467
  - https://bugzilla.redhat.com/show_bug.cgi?id=1112306
  - https://access.redhat.com/solutions/1253243
  - http://rhn.redhat.com/errata/RHBA-2014-1015.html
  - ksh.spec
    * Fri Jul 25 2014 Michal Hlavinka <email address hidden> - 20120801-10.8
    * job locking mechanism did not survive compiler optimization (#1123467)
  - patch
    * ksh-20120801-locking.patch

Debian bug:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=867181

[Original Description]

# gdb
[New LWP 3882]
Core was generated by `/bin/ksh <KSH_SCRIPT>.ksh'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0 job_chksave (pid=pid@entry=19385) at /build/ksh-6IEHIC/ksh-93u+20120801/src/cmd/ksh93/sh/jobs.c:1948
1948 if(jp->pid==pid)

(gdb) p *jp
Cannot access memory at address 0xb

(gdb) p *jp->pid
Cannot access memory at address 0x13

(gdb) p pid
$2 = 19385

(gdb) p *jpold
$1 = {next = 0xb, pid = -604008960, exitval = 11124}

The struct is corrupted at some point looking at the next,pid and
exitval struct members values which isn't valid data.

# assembly code
=> 0x0000000000427159 <+41>: cmp %edi,0x8(%rdx)

(gdb) p $edi ## pid variable
$1 = 19385

(gdb) p *($rdx + 8) ## jp->pid struct
Cannot access memory at address 0x13
--

ksh is segfaulting because it can't access struct "jp" ($rdx)
thus cannot de-reference the struct member "jp>pid" ($rdx + 8) at
line : src/cmd/ksh93/sh/jobs.c:1948 when looking if jp->pid is
equal to pid ($edi) variable.
2020-06-14 21:46:32 +01:00
Anuradha Weeraman
07cc71b880 ksh segfaults in job_chksave after receiving SIGCHLD
Upstreaming Debian patch:

Prior to this update, the compiler optimization dropped parts from the ksh job
locking mechanism from the binary code. As a consequence, ksh could terminate
unexpectedly with a segmentation fault after it received the SIGCHLD signal.
This update implements a fix to ensure the compiler does not drop parts of the
ksh mechanism and the crash no longer occurs.

Author: Michal Hlavinka mhlavink@redhat.com
Origin: Red Hat fix, ksh-20120801-locking.patch
Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1123467
Bug-Ubuntu: https://bugs.launchpad.net/bugs/1697501
Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=867181
2020-06-14 16:19:33 -04:00
Martijn Dekker
ae25b7f886 move read -S regress test to readcsv.sh (re: af0bd6ad) 2020-06-14 20:10:22 +02:00
Martijn Dekker
242d3f768b
Merge pull request #12 from JohnoKing/fix-read-s
`read -S` now correctly handles nested double quotes
2020-06-14 18:50:58 +01:00
Johnothan King
af0bd6ad70 read -S now correctly handles nested double quotes
Prior to this bugfix, the following set of commands would
fail to print two double quotes:

IFS=',' read -S a b c <<<'foo,"""title"" data",bar'
echo $b

This fix is from ksh93v- 2013-10-10-alpha, although it has
been revised to use stakputc to put the required double quote
into the buffer for consistency with the ksh93u+ codebase.

src/cmd/ksh93/bltins/read.c:
 - When handling nested double quotes, put the required double
   quote in read's buffer with stakputc.

src/cmd/ksh93/tests/builtins.sh:
 - Add the regression test for `read -S` from ksh93v-.

src/cmd/ksh93/sh.1:
 - Fix a minor formatting error to highlight '-S' in the ksh(1)
   man page.
2020-06-14 10:40:30 -07:00
Martijn Dekker
5498d9ec25
Merge pull request #11 from JohnoKing/leap-seconds
Fix two more problems with time zones and epoch handling
2020-06-14 16:54:23 +01:00
Martijn Dekker
ff01ecdaba
Merge pull request #10 from aweeraman/debian-patches-1
Fixes for implicit declaration warnings

This is to upstream some minor fixes for implicit declaration warnings currently in Debian.

Author: Anuradha Weeraman aweeraman@gmail.com
Reviewed-By: Thorsten Glaser t.glaser@tarent.de
Last-Update: 2020-02-09
2020-06-14 16:43:37 +01:00
Martijn Dekker
a81a64060c sh.1: forgot update after d8eba9d1 2020-06-14 16:49:41 +02:00
Anuradha Weeraman
49cac9e604 Fixes for implicit declaration warnings
Signed-off-by: Anuradha Weeraman <aweeraman@gmail.com>
2020-06-14 09:55:08 -04:00
Johnothan King
51f97cf82f Fix two more problems with time zones and epoch handling
src/cmd/ksh93/edit/history.c:
 - The `time_t` data type is usually 64-bit, so cast `t` to an
   unsigned long and use the correct format specifier with sfprintf.

src/lib/libast/tm/tmdata.c:
 - Update the leap second array by adding the current leap
   seconds that were absent, being:

   2016-12-31+23:59:60  1483228826
   2015-06-30+23:59:60  1435708825
   2012-06-30+23:59:60  1341100824

   I couldn't find a proper list of leap seconds in Unix Epoch time,
   so I used an Epoch Converter:
   https://www.epochconverter.com/
   The converter doesn't support leap seconds, so each conversion
   had to be corrected by adding the number of leap seconds required
   (e.g. 1230767999 + 24 = 1230768023, for 2008-12-31+23:59:60 in GMT).
2020-06-14 06:53:46 -07:00
Martijn Dekker
6109f54a5e Fix for no-argument use of 'cd' in .../fun/dirs
'cd' with no argument is supposed to go to your home directory, but
in this override function it produced an error due to an incorrect
use of $@ within another parameter substitution, preventing $@ from
expanding to zero words.

Ref.: https://groups.google.com/d/msgid/korn-shell/781ed34b-6860-5d47-dfe8-be21280a6b31%40inlv.org

src/cmd/ksh93/fun/dirs: _cd():
- Fix the bug by setting a positional parameter if needed,
  then using a normal and correct "$@" expansion for \cd.
- While we're here, restore a historic comment about using
  'command' to bypass aliases that exists in the listing of this
  function in the official 1995 KornShell book (pp. 274-276).
  Presumably, that comment was deleted when they decided to add the
  obnoxious and broken default alias command='command ', whenever
  that was. This branch got rid of that default alias in 61d9bca5.
2020-06-14 10:39:48 +02:00
Martijn Dekker
d66f2a80d8 sh.1: default PS1 is "$ ", not "$" 2020-06-14 08:42:07 +02:00
Martijn Dekker
1a6f37dc86 NEWS: update repo URI 2020-06-14 06:28:38 +02:00
Martijn Dekker
76f7d15a93
Merge pull request #8 from JohnoKing/fix-freebsd-date
Backport the ksh2020 fix for timezone name determination

Partial fix for #6.
2020-06-14 00:07:51 +01:00
Martijn Dekker
a739115ffe
Merge pull request #7 from JohnoKing/fix-special-vars
Implement a better fix for unsetting special env vars.

See #7 for more explanations and ksh source code history :)
2020-06-14 00:05:40 +01:00
Johnothan King
d7c9470704 Backport the ksh2020 fix for timezone name determination
This fix for `printf '%T' now` on FreeBSD was written by
@krader1961. This is from https://github.com/att/ast/pull/591:

On FreeBSD calling tzset() does not guarantee the tzname array will
be correctly populated. On most systems that works but on FreeBSD you
have to call localtime() or a related function (e.g., ctime()).

This change also eliminates a potential, very small, memory leak due to
the strdup()'ed tznames not being freed.

src/lib/libast/tm/tminit.c:
 - Fix timezone name determination on FreeBSD and a memory leak.
2020-06-13 14:28:10 -07:00
Johnothan King
7b994b6a7e Implement a better fix for unsetting special env vars
The regression this commit fixes was first introduced in ksh93t
2008-07-25. It was previously worked around in 6f0e008c by forking
subshells if any special environment variable is unset.

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

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

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

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

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

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

- Export TMPDIR=/tmp for test 137(C).
     Note that this exports TMPDIR to the environment for the
  duration of the 'tst' function run because the function was
  defined using the ksh 'function tst { ...; }' syntax.
2020-06-13 01:48:13 +02:00
Martijn Dekker
881a5be07c Remove obsolete docs, keeping official description
The lib/package/ast-open.* are almost entirely obsolete after
most of the AST utilities were removed to concentrate on ksh93
development. The only part that is both relevant and not repeated
elsewhere is AT&T's introductory blurb on ksh93.

README.md:
- Add the official AT&T ksh93 description from ast-open.html,
  reformatted to Markdown with a couple of added hyperlinks.

lib/package/ast-open.README,
lib/package/ast-open.html:
- Removed. (Note that the historic ksh93 changelog in these files
  is also included in src/cmd/ksh93/RELEASE).
2020-06-12 20:15:20 +02:00
Martijn Dekker
d600cf61fa use 'ksh93' org identifier for new/changed builtins
We cannot identify the new builtins as coming from "AT&T Research"
as they don't. 'ksh community' is accurate but a bit long. Since
we're now at a GitHub org called ksh93, let's just use that.
2020-06-12 18:54:08 +02:00
Martijn Dekker
a5e2fa8db7
README.md: tweak policy 2020-06-12 14:42:23 +01:00
Martijn Dekker
5aea9bbe08
README.md: update NEWS link after transfer to ksh93 org 2020-06-12 13:52:51 +01:00
Martijn Dekker
e500479ede
Merge pull request #1 from JohnoKing/fix-builtin-delete
`builtin -d` should not delete special builtins
2020-06-12 12:36:42 +01:00
Johnothan King
017d088c39 builtin -d should not delete special builtins
The man page for the builtin command says special builtins cannot
be deleted. This wasn't the case though, running `builtin -d` on
a special builtin was deleting it. As an example, the following
set of commands was ending with 'export: not found':

$ builtin -d export
$ export foo=bar

This commit backports the bugfix from ksh93v- (2014-12-24-beta),
which added an error check to prevent special builtins from being
deleted.

src/cmd/ksh93/sh/nvdisc.c:
 - Add an error check to prevent special builtins from being deleted.

src/cmd/ksh93/tests/builtins.sh
 - Add a regression test for using `builtin -d` on special builtins.
2020-06-12 04:26:40 -07:00
Martijn Dekker
cd5c181a39 Restore AST 'pty' pseudoterminal command (re: reboot)
This enables the tests/pty.sh regression tests for the interactive
shell to work. I've managed to restore these without restoring
nmake (which would cause unresolved build failures). It involved
hand-editing the Mamfile originally generated by nmake.

src/cmd/builtin/Makefile:
- Restored. Even though nmake is not installed, apparently mamake
  still needs the Makefile for the 'install' target to work. This
  will probably need editing if nmake is ever restored.

src/cmd/builtin/Mamfile:
- Restored and edited by hand to remove the commands to build all
  the utilities except pty, as well as all test targets (pty has no
  test). Most of this was systematically deleting blocks delimited
  by 'make FOO' and 'done FOO generated' until the build stopped
  failing -- a task made so much easier by the build now failing
  gracefully on error (see d0dfb37c). To see the diff, do:
	git diff cc1f2bf8 src/cmd/builtin/Mamfile

src/cmd/builtin/RELEASE:
- Restored for completeness' sake with all non-pty entries removed.

src/cmd/builtin/features/pty,
src/cmd/builtin/pty.c:
- Restored.
2020-06-12 12:35:58 +02:00