Red Hat erratum: https://bugzilla.redhat.com/1221766
"Previously, the gcc utility optimized out a non-NULL test in the
ksh implementation of the strdup() function. This caused an
unexpected termination when ksh was executed in a clean chroot
environment. With this update, ksh compilation parameters have been
updated to prevent optimizing out a non-NULL test, and ksh no
longer crashes in clean chroot environments."
The optimizer bug occurs in that function's single-line body:
return (s && (t = oldof(0, char, n = strlen(s) + 1, 0))) ? (char*)memcpy(t, s, n) : (char*)0;
So it must be the test for non-NULL 's' that fails. And 's' is
declared in the function definition, as follows:
extern char*
strdup(register const char* s)
So that makes me wonder if we can work around the bug by simply
removing the 'const' (and the 'register' while we're at it).
However, I have no easy way to verify that at the moment. The Red
Hat patch instead tells gcc to disable optimization for this
function using a #pragma directive.
I have no idea if that gcc optimiser bug has been fixed in the
meantime, but experience from c258a04f has shown that we cannot
trust that it has been fixed (that other optimizer bug is at least
a decade old and still not fixed). So, in it goes, until someone
shows evidence that we no longer need it.
Original patch:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-badgcc.patch
src/lib/libast/string/strdup.c:
- Tell GCC to disable all optimisations for strdup().
Discussion/analysis: https://bugzilla.redhat.com/1506344
iousepipe() might write out of bounds, causing a crash, if
subpipe[2] is set to a value >= sh.gd.lim.open_max.
src/cmd/ksh93/sh/xec.c: iousepipe():
- Validate the FD using sh_iovalidfd() before the write.
Prior discussion:
https://bugzilla.redhat.com/1454804
On 2017-05-23 13:33:25 UTC, Paulo Andrade wrote:
> In previous ksh versions, when exiting the scope of a ksh
> (not posix) function, it would restore the trap table of
> the "calling context" and if the reason the function exited
> was a signal, it would call sh_fault() passing as argument
> the signal value.
> Newer ksh checks it, but calls kill(getpid(), signal_number)
> after restoring the trap table, but only calls for SIGINT and
> SIGQUIT.
[...]
> The old way appears to have been more appropriate, but there
> must be a reason to only pass SIGINT and SIGQUIT as it is an
> explicit patch.
The last paragraph is where I differ. This would not be the first
example of outright breakage that appeared to be added deliberately
and that 93u+m has fixed or removed, see e.g. 8477d2ce ('printf %H'
had code that deleted all multibyte characters), cefe087d, or
781f0a39. Sometimes it seems the developers added a little
experiment and then forgot all about it, so it became a misfeature.
In this instance, the correct pre-2012 ksh behaviour is still
explicitly documented in (k)sh.1: "A trap condition that is not
caught or ignored by the function causes the function to terminate
and the condition to be passed on to the caller". Meaning, if there
is no function-local trap, the signal defaults to the parent scope.
There is no language that limits this to SIGINT and SIGQUIT only.
It also makes no sense at all to do so -- signals such as SIGPIPE,
SIGTERM, or SIGSEGV need to be caught by default and to do
otherwise results in misbehaviour by default.
src/cmd/ksh93/sh/xec.c: sh_funscope():
- When resending a signal after restoring the global traps state,
remove the spurious check that limits this to SIGINT and SIGQUIT.
- Replace it with a check for nsig!=0, as that means there were
parent trap states to restore. Otherwise 'kill' may be called
with an invalid signal argument, causing a crash on macOS.
src/cmd/ksh93/tests/signal.sh:
- Update a test to check that a function-local SIGTERM trap is
triggered correctly when signalled from another process.
- Complete the tests for 3aee10d7; this bug needed fixing before
we could test that previous fix in a ksh function scope.
- Add a test for triggering global traps from ksh functions,
testing multiple POSIX-standard signals.
The ksh-20120801-trapcom.patch patch contains an off-by-one error,
which was also imported into 93u+m. When saving signals:
ceb77b136f/src/cmd/ksh93/sh/subshell.c (L572-L592)
572 if((nsig=shp->st.trapmax)>0 || shp->st.trapcom[0])
573 {
574 ++nsig;
575 savsig = malloc(nsig * sizeof(char*));
576 /*
577 * the data is, usually, modified in code like:
578 * tmp = buf[i]; buf[i] = strdup(tmp); free(tmp);
579 * so shp->st.trapcom needs a "deep copy" to properly save/restore pointers.
580 */
581 for (isig = 0; isig < nsig; ++isig)
582 {
583 if(shp->st.trapcom[isig] == Empty)
584 savsig[isig] = Empty;
585 else if(shp->st.trapcom[isig])
586 savsig[isig] = strdup(shp->st.trapcom[isig]);
587 else
588 savsig[isig] = NULL;
589 }
On line 574, the number of signals 'nsig' is increased by one. That
increase is permanent, so the 'for' loop on line 581 tries to save
one signal state too many.
The increase was a holdout from the ksh93 code from before the
patch. After the patch, it is not required; it is fine to malloc as
many records as there are trapcom elements to save. So it should
simply be removed. xec.c has the same code to save trap states for
ksh functions, and the same applies.
src/cmd/ksh93/sh/subshell.c: sh_subshell():
- Don't increase nsig.
src/cmd/ksh93/sh/xec.c: sh_funscope():
- Same.
src/cmd/ksh93/tests/signal.sh:
- Add test.
Original patch:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20140801-diskfull.patch
Prior discussion:
https://www.mail-archive.com/ast-users@lists.research.att.com/msg01037.htmlhttps://www.mail-archive.com/ast-users@lists.research.att.com/msg01038.htmlhttps://www.mail-archive.com/ast-users@lists.research.att.com/msg01042.htmlhttps://bugzilla.redhat.com/1212992
On Fri, 08 May 2015 14:37:45 -0700, Paulo Andrade wrote:
> I have a user with a ksh crashing problem, and that has
> some "Write error: No space left on device" messages
> in /var/log/messages.
>
> After some debugging, and creating a chroot on a file
> disk image, and a test user, and slowly filling the
> "on file" filesystem, e.g.
>
> dd if=/dev/zero of=/mnt/tmp/zerosN bs=1M count=1024
> dd if=/dev/zero of=/mnt/tmp/zerosN bs=1K count=2
>
> until leaving just around 12K, I managed to reproduce the
> problem, and be able to debug it with valgrind and vgdb;
> debugging on these conditions is tricky, as cannot tell
> valgrind to spawn gdb, because then gdb itself would fail
> to start.
>
> So, after following the code enough, I learned that at places
> it handles SH_JMPEXIT, there was almost non existing
> handling of SH_JMPERREXIT.
>
> ksh would evently cause a crash due to the struct
> subshell allocated on stack, in sh/subshell.c:sh_subshell
> kept set to the global subshell_data, after it siglongjmp
> back the stack due to, not fully handling the out of disk
> space errors. It would print a few messages, everytime
> a pipe was created, e.g.:
>
> /etc/profile: line 28: write to 3 failed [No space left on device]
>
> until eventually crashing due to corrupted memory; e.g. the
> references to stack data from sh_subsell in the global
> subshell_data. One strange thing to me in coredump analysis
> was that subshell_data prev field was pointing to itself when
> it eventually crashed, what later was understood and expected...
>
> The attached patch handles SH_JMPERREXIT in the code
> paths SH_JMPEXIT is handled, and the failed login, on
> full disk, ends in a pause() call:
>
> ---terminal 1---
> $ valgrind -q --leak-check=full --free-fill=0x5a --vgdb=full
> --vgdb-error=0 /bin/ksh -l
> ==17730== (action at startup) vgdb me ...
> ==17730==
> ==17730== TO DEBUG THIS PROCESS USING GDB: start GDB like this
> ==17730== /path/to/gdb /bin/ksh
> ==17730== and then give GDB the following command
> ==17730== target remote | /usr/lib64/valgrind/../../bin/vgdb --pid=17730
> ==17730== --pid is optional if only one valgrind process is running
> ==17730==
> ==17730== Syscall param mount(type) points to unaddressable byte(s)
> ==17730== at 0x563377A: mount (in /usr/lib64/libc-2.17.so)
> ==17730== by 0x493E58: fs3d_mount (fs3d.c:115)
> ==17730== by 0x493C8B: fs3d (fs3d.c:57)
> ==17730== by 0x423E41: sh_init (init.c:1302)
> ==17730== by 0x405CD3: sh_main (main.c:141)
> ==17730== by 0x405B84: main (pmain.c:45)
> ==17730== Address 0x0 is not stack'd, malloc'd or (recently) free'd
> ==17730==
> ==17730== (action on error) vgdb me ...
> ==17730== Continuing ...
> /etc/profile: line 28: write to 3 failed [No space left on device]
> ---8<---
>
> ---terminal 2---
> (gdb) c
> Continuing.
> ^C
> Program received signal SIGTRAP, Trace/breakpoint trap.
> 0x00000000055fa470 in __pause_nocancel () from /lib64/libc.so.6
> (gdb) bt
> #0 0x00000000055fa470 in __pause_nocancel () from /lib64/libc.so.6
> #1 0x000000000041e73d in sh_done (ptr=0x793360 <sh>, sig=255) at
> /home/pcpa/rhel/ksh/ksh-20120801/src/cmd/ksh93/sh/fault.c:665
> #2 0x0000000000407407 in exfile (shp=0x4542, iop=0xff, fno=0) at
> /home/pcpa/rhel/ksh/ksh-20120801/src/cmd/ksh93/sh/main.c:604
> #3 0x0000000000405c43 in sh_source (shp=0x793360 <sh>, iop=0x0,
> file=0x524804 <e_sysprofile> "/etc/profile")
> at /home/pcpa/rhel/ksh/ksh-20120801/src/cmd/ksh93/sh/main.c:109
> #4 0x00000000004060e4 in sh_main (ac=2, av=0xfff000498, userinit=0x0)
> at /home/pcpa/rhel/ksh/ksh-20120801/src/cmd/ksh93/sh/main.c:202
> #5 0x0000000000405b85 in main (argc=2, argv=0xfff000498) at
> /home/pcpa/rhel/ksh/ksh-20120801/src/cmd/ksh93/sh/pmain.c:45
> (gdb)
> ---8<---
Prior discussion:
https://bugzilla.redhat.com/1217236
Summary: doing
( some_simple_command & )
i.e., launching a background job from within a subshell, on a ksh
interactive login shell, caused misbehaviour (command not run).
For me on 93u+m, the misbehaviour was different -- an outright
crash in the job handling code following SIGCHLD, backtracing to:
0 ksh job_unpost + 49 (jobs.c:1655)
1 ksh job_reap + 1632 (jobs.c:468)
2 libsystem_platform.dylib _sigtramp + 29
3 ??? 0 + 4355528544
4 ksh ed_getchar + 102 (edit.c:1048)
5 ksh ed_emacsread + 659 (emacs.c:280)
[...]
Original patch:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-nohupfork.patch
Lines 1874-1886 in sh_exec() optimise the highly specific case of
'( simple_command & )' by avoiding a sh_subshell() call that sets
up a virtual subshell before forking:
https://github.com/ksh93/ksh/blob/bd283959/src/cmd/ksh93/sh/xec.c#L1874-L1886
1874 else if(((type=t->par.partre->tre.tretyp)&FAMP) && ((type&COMMSK)==TFORK))
1875 {
1876 pid_t pid;
1877 sfsync(NIL(Sfio_t*));
1878 while((pid=fork())< 0)
1879 _sh_fork(shp,pid,0,0);
1880 if(pid==0)
1881 {
1882 sh_exec(t->par.partre,flags);
1883 shp->st.trapcom[0]=0;
1884 sh_done(shp,0);
1885 }
1886 }
1887 else
1888 sh_subshell(shp,t->par.partre,flags,0);
The original patch inserts the following before the sh_done call on
line 1884:
sh_offoption(SH_INTERACTIVE);
sh_done() checks for SH_INTERACTIVE and only runs job handling code
if that option is on.
Also, I had missed the need for an update of shgd->current_pid
here. Since 843b546c replaced lots of getpid() calls by reading
that variable, this could cause ksh to SIGCHLD the wrong process.
But even after adding the shgd->current_pid update to the RH patch,
I was still able to make it crash.
src/cmd/ksh93/sh/xec.c: sh_exec(): case TPAR:
- Disable this optimisation outright for interactive or job
control-enabled shells. I really don't trust it at all, but there
have been no problem reports for scripts without job control, so
keep it until such reports surface.
- Update shgd->current_pid so the child doesn't end up signalling
the wrong process (re: 843b546c).
___________________________________________________________________
P.S.:
It was noted in the Red Hat discussion that ( ... & ) does a double
fork, i.e., a virtual/non-forked subshell always forks before
forking the background job. This extra fork is done by the
following two lines in under 'case TFORK:' in sh_exec() in xec.c:
https://github.com/ksh93/ksh/blob/bd283959/src/cmd/ksh93/sh/xec.c#L1534-L1535
1534 if((type&(FAMP|TFORK))==(FAMP|TFORK))
1535 sh_subfork();
This is executed if we're in a virtual/non-forked subshell, i.e.
shp->subshell is true (note it is false for forked subshells). So
it forks the virtual subshell (the first fork) before running the
background job (the second fork). A background job is recognised by
'type' having both the FAMP (AMP == ampersand == &) and TFORK bits
set and no others.
So the obvious way to remove the double fork is to comment out
these two lines. Indeed, testing that confirms it gone and ksh
appears to work fine at first glance. Does it really? Nearly!
There are just four regression failures in a 'bin/shtests -p' run:
options.sh[394]: & job delayed by --pipefail, expected '1212 or 1221', got '1122'
signal.sh[280]: subshell ignoring signal does not send signal to parent (expected 'SIGUSR1', got 'done')
signal.sh[289]: subshell catching signal does not send signal to parent (expected 'SIGUSR1', got 'done')
subshell.sh[467]: sleep& in subshell hanging
So, those two lines cannot be removed now and we have to keep the
double fork. But removing them doesn't appear to break very much,
so it may be possible to add some tests so we only do an extra fork
when really necessary. That is a project for another day.
For one Red Hat customer, the following reproducer consistently
crashed, tough I was not able to reproduce it and neither was RH.
However, the crash analysis is sound (see below).
function dlog
{
fc -ln -0
}
trap dlog DEBUG
>/tmp/blah
Original patch: https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20140801-arraylen.patch
The Red Hat bug thread is closed to the public as it also contains
some correspondence with their customer. But it has an excellent
crash analysis from Thomas Gardner which I'm including here for the
record (the line numbers are for their ksh at the time, not 93u+m).
===begin analysis===
> The creation of an empty file instead of a command that executes
> anything causes the coredump.
[...]
> Here is my analysis on the core that was provided by the customer:
>
> (gdb) bt
> #0 sh_fmtq (string=0x1 <Address 0x1 out of bounds>)
> at /usr/src/debug/ksh-20120801/src/cmd/ksh93/sh/string.c:340
> #1 0x0000000000457e96 in out_string (cp=<value optimized out>, c=32,
> quoted=<value optimized out>, iop=<value optimized out>)
> at /usr/src/debug/ksh-20120801/src/cmd/ksh93/sh/xec.c:444
> #2 0x000000000045804c in sh_debug (shp=0x76d180, trap=0x7f2f13a821e0 "dlog",
> name=<value optimized out>, subscript=<value optimized out>,
> argv=0x76e070, flags=<value optimized out>)
> at /usr/src/debug/ksh-20120801/src/cmd/ksh93/sh/xec.c:548
> #3 0x000000000045a867 in sh_exec (t=0x7f2f13aafad0, flags=4)
> at /usr/src/debug/ksh-20120801/src/cmd/ksh93/sh/xec.c:1265
> [...need go no further...]
>
> In frame 2, we can see it cycling through your classic
> (char **)argv array like:
>
> 543 while(cp = *argv++)
> 544 {
> 545 if((flags&ARG_EXP) && argv[1]==0)
> 546 out_pattern(iop, cp,' ');
> 547 else
> 548 out_string(iop, cp,' ',n?0: (flags&(ARG_RAW|ARG_NOGLOB))||*argv);
> 549 }
> 550 if(flags&ARG_ASSIGN)
> 551 sfputc(iop,')');
> 552 else if(iop==stkstd)
>
> (we seg-fault after going down the out_string function in line
> 548 up there). The string pointer that points to = 0x1 up in
> frame #0 (sh_fmtq) traces back to the "cp" variable in line 548
> up there. The "argv" variable being referenced up there just gets
> passed in as the fifth argument to this function.
>
> In frame #3 (sh_exec, line 1265), the line that makes the call
> that takes us to frame 2 is:
>
> 1265 int n = sh_debug(shp,trap,(char*)0,(char*)0, com, ARG_R AW);
>
> so "com" (the fifth argument) is what's going wrong as it
> descends down through these calls. Looking at where it comes
> from, well, it's assigned here:
>
> 1241 if(argn==0)
> 1242 {
> 1243 /* fake 'true' built-in */
> 1244 np = SYSTRUE;
> 1245 *argv = nv_name(np);
> 1246 com = argv;
> 1247 }
>
> because as we can see:
>
> (gdb) f 3
> #3 0x000000000045a867 in sh_exec (t=0x7f2f13aafad0, flags=4)
> at /usr/src/debug/ksh-20120801/src/cmd/ksh93/sh/xec.c:1265
> 1265 int n = sh_debug(shp,trap,(char*)0,(char*)0, com, ARG_RAW);
> (gdb) p argn
> $2 = 0
> (gdb)
>
> argn is == 0 here. The tip-off here is that nv_name clearly
> returns a simple pointer to an array of characters, not an array
> of pointers to arrays of characters as is evidenced by the fact
> that the assignment is "*argv = nv_name(np);" not "argv =
> nv_name(np);". Looking at the function nv_name proves that it
> does indeed return a single pointer to an array of characters,
> not a pointer to an array of pointers to arrays of characters.
> Now, com is defined as a 'char **':
>
> 1002 char *cp=0, **com=0, *comn;
>
> (as it is expected to be in the calls that follow) also, that
> argv is also defined as the effective equivalent a 'char **':
>
> 1237 static char *argv[1];
>
> Yup, argv is actually an array of pointers (char ** equivalent),
> but that array is restricted to having exactly one element.
> Recalling the assignment in the previously quoted line:
>
> 1245 *argv = nv_name(np);
>
> we see that the one and only element in that argv array is
> getting assigned a pointer to an array of characters here.
> Nothing necessarily wrong with that, but remember the loop we
> looked at earlier in frame #2 (sh_debug). It went like:
>
> 543 while(cp = *argv++)
> 544 {
> 545 if((flags&ARG_EXP) && argv[1]==0)
> 546 out_pattern(iop, cp,' ');
> 547 else
> 548 out_string(iop, cp,' ',n?0: (flags&(ARG_RAW|ARG_NOGLOB))||*argv);
> 549 }
>
> which is clearly expecting argv in this context (com in frame 3,
> which really points to that static local single element array
> that is also pointed to by argv in frame 2) to be an array of
> pointers of indefinite size, each element being a pointer, but
> whose last element will be a null pointer. Well, in frame 3 it is
> clearly an array with only a single element, and that one element
> is NOT pointing to null. Watch this:
>
> (gdb) f 3
> #3 0x000000000045a867 in sh_exec (t=0x7f2f13aafad0, flags=4)
> at /usr/src/debug/ksh-20120801/src/cmd/ksh93/sh/xec.c:1265
> 1265 int n = sh_debug(shp,trap,(char*)0,(char*)0, com, ARG_RAW);
> (gdb) p com
> $8 = (char **) 0x76e060
> (gdb) p &argv
> $9 = (char *(*)[1]) 0x76e060
> (gdb) p com[0]
> $11 = 0x5009c6 "true"
> (gdb) p com[1]
> $10 = 0x1 <Address 0x1 out of bounds>
> (gdb) p argv[0]
> $12 = 0x5009c6 "true"
> (gdb) p argv[1]
> $13 = 0x1 <Address 0x1 out of bounds>
> (gdb)
>
> So, as expected, com and &argv point to the same place, the first
> element points to the constant string "true", but since the array
> is defined as having only one element, when you refer to a second
> element in that array, you get well, whatever random crap happens
> to be in that memory location. When we try to reproduce this
> problem, apparently we're getting 0 there (or we're not quite
> following this same code path, which is also possible), but the
> customer happens to have a "1" in that memory location.
===end analysis===
src/cmd/ksh93/sh/xec.c: sh_exec():
- When processing TCOM (simple command) with an empty/null command,
increase the size of the static dummy argv[1] array to argv[2],
ensuring a terminating NULL element so that 'while(cp = *argv++)'
loops don't crash. (Note that static objects are automatically
initialised to zero in C.)
src/cmd/ksh93/tests/io.sh:
- Adapt the reproducer, testing a null-command redirection 1000x.
Of course I was wrong to say the bug had nothing to do with
functions; traps in ksh functions are local, are handled the same
way as traps that are local to virtual subshells, and had the same
crashing bug. So this adds a test for that as well.
Contrary to the RH bug report, this is yet another bug with
virtual/non-forked subshells and has nothing to do with functions.
If a signal is ignored (empty trap) in the main shell while any
trap (empty or not) is set on the same signal in a subshell, a
crash eventually occurred upon restoring state when leaving the
subshell.
Original patch:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-trapcom.patch
Prior discussion:
https://bugzilla.redhat.com/1117404
Paulo Andrade wrote there:
> The problem is that the sh_subshell function was saving pointers
> that could change, and when restoring, bad things would happen.
[...]
> The only comment I added:
> /* contents of shp->st.trapcom may change */
> may be a bit misleading, the "bad" save/restore already knows it,
> probably I should have added a better description telling that the
> data is, usually, modified in code like:
>
> tmp = buf[i]; buf[i] = strdup(tmp); free(tmp);
>
> so the shp->st.trapcom needs a "deep copy", as done in the
> patch, to properly save/restore pointers.
src/cmd/ksh93/sh/subshell.c,
src/cmd/ksh93/sh/xec.c:
- sh_subshell(), sh_funscope(): Make *savsig/*savstak into a
**savsig array. Use strdup(3) to save the data and get known
pointers that will not change. Free these upon restore.
- Change the comment from the patch as Paulo wished he had done.
src/cmd/ksh93/tests/subshell.sh:
- Test 2500 times. This should trigger the crash most of the time.
Another Red Hat patch. "Prior to this update, the result of a
command substitution was lost if a file descriptor used for the
substitution was previously explicitly closed. With this update,
ksh no longer reuses file descriptors that were closed during the
execution of a command substitution. Now, command substitutions
work as expected in the described situation."
Prior discussion:
https://bugzilla.redhat.com/1116072
Original patch:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20140929-safefd.patch
src/cmd/ksh93/include/io.h,
src/cmd/ksh93/sh/io.c:
- Add sh_iosafefd() function to get a file descriptor that is not
in use or otherwise occupied (including marked as closed).
src/cmd/ksh93/sh/subshell.c: sh_subshell():
- Use that function to obtain a safe FD upon restoring state when
exiting a command substitution. I don't really know the how and
why -- all that I/O magic is still beyond me and the code is
uncommented as usual.
src/cmd/ksh93/tests/subshell.sh:
- Add regression test from the reproducer in the bug, reduced to
the minimum necessary.
The undocumented alarm builtin executes actions unsafely so that
'read' with an IFS assignment crashed when an alarm was triggered.
This applies an edited version of a Red Hat patch:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-alarmifs.patch
Prior discussion:
https://bugzilla.redhat.com/1176670
src/cmd/ksh93/bltins/alarm.c:
- Add a TODO note based on dgk's 2014 email cited in the RH bug.
- When executing the trap function, save and restore the IFS table.
src/cmd/ksh93/sh/init.c: get_ifs():
- Remove now-unnecessary SHOPT_MULTIBYTE preprocessor directives as
8477d2ce lets the compiler optimise out multibyte code if needed.
- Initialise the 0 position of the IFS table to S_EOF. This
corresponds with the static state tables in data/lexstates.c.
src/cmd/ksh93/tests/builtins.sh:
- Crash test.
This applies the following Red Hat patch:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-cdfork.patch
The associated bug report is public, but nearly all info (such as
a reproducer) has been wiped: https://bugzilla.redhat.com/1168611
However, the errata blurb is mildly informative:
"Previously, ksh sometimes incorrectly initialized a variable
holding the path of the working directory. If a program changed the
working directory between forking and ksh execution, then ksh could
contain an incorrect value in the working directory variable. With
this update, initialization of the working directory variable has
been corrected, and ksh now contains the correct value in the
aforementioned situation."
Also, the patch makes a lot of sense on the face of it. It removes
an optimisation in path_pwd() that checks for the directory defined
by e_crondir[] in data/msg.c, which is:
const char e_crondir[] = "/usr/spool/cron/atjobs";
Of /usr/spool not existed on any system for decades as it is common
to mount usr as read-only, so all the writable stuff was moved to
/var. So that would never check out. And if 'flag' is nonzero, the
optimizing 'count++' is executed regardless of whether that
directory exists, ensuring that it never gets the real PWD and
defaults to returning ".".
src/cmd/ksh93/sh/path.c:
- Apply patch as described.
- Mark 'flag' variable as NOT_USED to suppress compiler warning.
Keep it for backwards compat, as some programs that link with
libshell might use this function (though it's undocumented).
src/cmd/ksh93/include/path.h,
src/cmd/ksh93/data/msg.c:
- Remove now-unused e_crondir[].
This imports a new version of the code to import environment
variable values that was sent to Red Hat from upstream in 2014.
It avoids importing environment variables whose names are not valid
in the shell language, as it would be impossible to change or unset
them. However, they stay in the environment to be passed to child
processes.
Prior discussion: https://bugzilla.redhat.com/1147645
Original patch: https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-oldenvinit.patch
src/cmd/ksh93/sh/init.c:
- env_init(): Import new, simplified code to import environment
variable name/value pairs. Instead of doing the heavy lifting
itself, this version uses nv_open(), passing the NV_IDENT flag to
reject and skip invalid names.
- Get rid of gotos and a static var by splitting off the code to
import attributes into a new env_import_attributes() function.
This is a better way to avoid importing attributes when
initialising the shell in POSIX mode (re: 00d43960
- Remove an nv_mapchar() call that was based on some unclear
flaggery which was also removed by upstream as sent to Red Hat.
I don't know what that did, if anything; looks like it might have
had something to do with typeset -u/-l, but those particular
attributes have never been successfully inherited through the
environment.
(Maybe that's another bug, or maybe I just don't care as
inheriting attributes is a misfeature anyway; we have to put up
with it because legacy scripts might use it. Maybe someone can
prove it's an unacceptable security risk to import attributes
like readonly from an environment variable that is inherently
vulnerable to manipulation. That would be nice, as a CVE ID
would give us a solid reason to get rid of this nonsense.)
- Remove an 'else cp += 2;' that was very clearly a no-op; 'cp' is
immediately overwritten on the next loop iteration and not used
past the loop.
src/cmd/ksh93/tests/variables.sh:
- Test.
According to 'whence --man', 'whence -f' should ignore functions:
-f Do not check for functions.
Right now this is only accomplished partially. As of commit
a329c22d 'whence -f' avoids any output when encountering a
function (in ksh93u+ 'whence -f' has incorrect output). The
return value is still wrong though:
$ foo() { true; }
$ whence -f foo; echo $?
0
This commit fixes the return value and makes 'type -f' error out
when given a function (like in Bash).
src/cmd/ksh93/bltins/whence.c:
- If -f was passed, set 'cp' to NULL since functions should be
ignored (as documented).
- Simplify return value by avoiding bitwise logic.
src/cmd/ksh93/tests/builtins.sh:
- Add regression tests for 'whence -f' and 'type -f'.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
Another Red Hat patch of a patch. With the new comsub mechanism,
functions could sometimes return the wrong exit status when invoked
from a command substitution.
Original patch:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-fununset.patch
I have determined that the extra setexit() in the Red Hat patch,
which copies the current exit status to $?, is not needed, as the
code for running functions already sets $? on termination. I've
added extra regression tests to prove this.
By the way, the setexit() macro is defined like this in defs.h:
#define exitset() (sh.savexit=sh.exitval)
That's more evidence (see also 3654ee73) that it does not
matter whether you address the shell's status struct via a
pointer. That macro is used in places that use shp pointers.
But, that aside...
src/cmd/ksh93/sh/subshell.c: sh_subshell():
- When waiting within a command substitution for a forked process
to end, save & restore sh.exitval (the exit status of the command
currently being run) so that job_wait() cannot override it.
src/cmd/ksh93/tests/functions.sh:
- Add tests based in part on the reproducer from rhbz#1116508.
src/cmd/ksh93/sh/xec.c: sh_funct():
- The np->nvalue.rp pointer was dereferenced before the check that
it is non-null. Do this check before dereferencing it.
Since at least 1999, whence -v on pdksh (and its successor mksh)
reports the path where an autoloadable function may be found:
$ mkdir ~/fun; FPATH=~/fun
$ echo 'myfn() { echo hi; }' >~/fun/myfn
$ whence -v myfn
myfn is a undefined (autoload from /home/user/fun/myfn) function
Whereas ksh93 only reports, rather uselessly:
myfn is an undefined function
As of this commit, whence -v/-a on ksh 93u+m does the same as
pdksh, but with correct grammar:
myfn is an undefined function (autoload from /home/user/fun/myfn)
This may be a small violation of my own "no new features" policy
for 93u+m, but I couldn't resist. This omission has been annoying
me, and it's just embarrassing to lack a pdksh feature :)
src/cmd/ksh93/include/path.h,
src/cmd/ksh93/data/msg.c:
- Add e_autoloadfrom[] = " (autoload from %s)" message.
src/cmd/ksh93/bltins/whence.c: whence():
- Report the path (if any) when reporting an undefined function.
This needs to be done in two places:
1. When a function has been explicitly marked undefined with
'autoload', we need to do a quick path_search() loop to find
the path. (These undefined functions take precedence over
regular commands, so are reported first.)
2. When a function is not explicitly autoloaded but merely
available in $FPATH, that path search was already done, so all
we need to do is report it. (These are reported last.)
Note that the output remains as on 93u+ if no function definition
file is found on $FPATH. This is also like pdksh/mksh.
src/cmd/ksh93/data/builtins.c:
- Bump 'whence' version date. The inline docs never detailed very
exactly what 'whence -v' reports, so no need for further edits.
src/cmd/ksh93/tests/path.sh:
- Regress-test the new whence behaviour plus actual autoloading,
including the command override behaviour of autoloaded functions.
The fixargs() function is invoked when ksh needs to run a script
without a #!/hashbang/path. Instead of letting the kernel invoke a
shell, ksh exfile()s the script itself from sh_main(). In the
forked child, it calls fixargs() to set the argument list in the
environment to the args of the new script, so that 'ps' and
/proc/PID/cmdline show the expected output.
But fixargs() is broken because, on systems other than HP-UX (on
which ksh uses pstat(2)), ksh simply inserts a terminating zero.
The arguments list is not a zero-terminated C string. Unix systems
expect the entire arguments buffer to be zeroed out, otherwise 'ps'
and /proc/*/cmdline will have fragments of previous command lines
in the output.
The Red Hat patch for this bug is:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-argvfix.patch
However, that fix is incomplete because 'command_len' was also
hardcoded to be limited to 64 characters (!), which still gave
invalid 'ps' output if the erased command line was longer.
src/cmd/ksh93/sh/main.c: fixargs():
- Remove CMD_LENGTH macro which was defined as 64.
- Remove code that limited the erasure of the arguments buffer to
CMD_LENGTH characters. That code also had quite a dodgy strdup()
call -- it copies arguments to the heap, but they are never freed
(or even used), so it's a memory leak. Also, none of this is
ever done if the length is calculated using pstat(2) on HP-UX,
which is a clear indication that it's unnecessary.
(I think this code block must have been some experiment they
forgot to remove. One reason why I think so is that a 64 byte
arguments limit never made sense, even in the 1980s when they
wrote ksh on 80-column CRT displays. Another indication of this
is that fixing it didn't require adding anything; the code to do
the right thing was already there, it was just being overridden.)
- Zero out the full arguments length as in the Red Hat patch.
src/cmd/ksh93/tests/basic.sh:
- Add test. It's sort of involved because 'ps' is one of the least
portable commands in practice, in spite of standardisation.
This fixes a regression introduced in commit f9c127e3.
When the legacy code for older versions of libast was
removed, the fmtident wrapper wasn't removed. As a result,
the version string output by Ctrl+Alt+V is garbled because
the fmtident wrapper doesn't do any formatting:
$ <Ctrl+Alt+V>
^J@(#)$Id: Version AJM 93u+m 2020-09-14
src/cmd/ksh93/sh/string.c:
- Remove the old version of fmtident that was overriding
the current version of fmtident provided by libast
(in src/lib/libast/string/fmtident.c).
There was no check for the -B/braceexpand option before calling
path_expand() to process brace expansion, making it impossible to
turn off brace expansion within command substitutions. Normally the
lexer flags brace expansion so that this code is not reached, but
shell code within command substitutions is handled differently.
Red Hat patches this by adding this check to the function itself:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20140301-fikspand.patch
But I think it's more logical to patch it at the point of decision.
src/cmd/ksh93/sh/macro.c: endfield():
- Decide to call either path_generate() or path_expand() based on
the state of the SH_BRACEEXPAND shell option.
- Fix '#if SHOPT_BRACEPAT' preprocessor check that previously
hardcoded this decision at compile time.
src/cmd/ksh93/tests/options.sh:
- Add tests.
The new command substitution mechanism imported in 970069a6 from
Red Hat patches introduced this bug: backtick-style command
substitutions hang when processing about 117KiB of data or more.
It is fixed by another Red Hat patch:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20140415-hokaido.patch
It saves the value of the shp->comsub flag so that it is set to 2
(usually meaning new-style $(comsubs)) in two specific cases even
when processing backtick comsubs. This stops the sh_subtmpfile()
function in subshell.c from creating a /tmp file. However, I think
that approach is quite ugly, so I'm taking a slightly different one
that has the same effect.
src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/subshell.c:
- Redefine sh_subtmpfile() to pass the comsub flag as an argument.
(Remove the shp pointer argument, which is redundant; a pointer
to the shell state can easily be obtained in the function.)
src/cmd/ksh93/sh/xec.c: sh_exec():
- Apply the Red Hat fix by passing flag 2 to sh_subtmpfile().
src/cmd/ksh93/tests/subshell.sh:
- Move regress test from ce68e1be from basic.sh to here; this is
the place for command substitution tests as they are subshells.
- Add regress test for this bug.
All other changed files:
- Update sh_subtmpfile() calls to pass on the shp->comsub flag.
When using typeset -l or -u on a variable that cannot be changed
when the shell is in restricted mode, ksh crashed.
This fixed is inspired by this Red Hat fix, which is incomplete:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-tpstl.patch
The crash was caused by the nv_shell() function. It walks though a
discipline function tree to get the pointer to the interpreter
associated with it. Evidently, the problem is that some pointer in
that walk is not set correctly for all special variables.
Thing is, ksh only has one shell language interpreter, and only one
global data structure (called 'sh') to keep its main state[*]. Yet,
the code is full of 'shp' pointers to that structure. Most (not
all) functions pass that pointer around to each other, accessing
that struct indirectly, ostensibly to account for the non-existent
possibility that there might be more than one interpreter state.
The "why" of that is an interesting cause for speculation that I
may get to sometime. For now, it is enough to know that, in the
code as it is, it matters not one iota what pointer to the shell
interpreter state is used; they all point to the same thing (unless
it's broken, as in this bug).
So, rather than fixing nv_shell() and/or associated pointer
assignments, this commit simply removes it, and replaces it with
calls to sh_getinterp(), which always returns a pointer to sh (see
init.c, where that function is defined as literally 'return &sh').
[*] Defined in shell.h, with the _SH_PRIVATE part in defs.h
src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/name.c:
- Remove nv_shell().
src/cmd/ksh93/sh/init.c:
- In all the discipline functions for special variables, initialise
shp using sh_getinterp() instead of nv_shell().
src/cmd/ksh93/tests/variables.sh:
- Add regression test for typeset -l/-u on all special variables.
Now that we have ${.sh.pid} a.k.a. shgd->current_pid, which is
updated using getpid() whenever forking a new process, there is no
need for anything else to ever call getpid(); we can use the stored
value instead. There were a lot of these syscalls kicking around,
some of them in performance-sensitive places.
The following lists only changes *other* than changing getpid() to
shgd->currentpid.
src/cmd/ksh93/include/defs.h:
- Comments: clarify what shgd->{pid,ppid,current_pid} are for.
src/cmd/ksh93/sh/main.c,
src/cmd/ksh93/sh/init.c:
- On reinit for a new script, update shgd->{pid,ppid,current_pid}
in the sh_reinit() function itself instead of calling sh_reinit()
from sh_main() and then updating those immediately after that
call. It just makes more sense this way. Nothing else ever calls
sh_reinit() so there are no side effects.
src/cmd/ksh93/sh/xec.c: _sh_fork():
- Update shgd->current_pid in the child early, so that the rest of
the function can use it instead of calling getpid() again.
- Remove reassignment of SH_PIDNOD->nvalue.lp value pointer to
shgd->current_pid (which makes ${.sh.pid} work in the shell).
It's constant and was already set on init.
This imports another fix from Red Hat/Fedora. Original patch:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-crash.patch
src/cmd/ksh93/include/jobs.h,
src/cmd/ksh93/sh/jobs.c,
src/cmd/ksh93/sh/subshell.c,
src/cmd/ksh93/sh/xec.c:
- Import the Red Hat fix with these differences:
- Rename the 'hack1_waitall' variable to 'bktick_waitall' and add
a comment describing what it's for.
- Remove unused 'pipefail' variable.
src/cmd/ksh93/tests/basic.sh:
- Regression test from reproducer given in the Red Hat bug report.
- Add special handling to SIGKILL it, as it might freeze hard.
var=$(< file) now reads the file even if the standard inout,
standard output and/or standard error file descriptors are closed.
Original patch:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-filecomsubst.patch
src/cmd/ksh93/sh/io.c: sh_redirect():
- When processing the '<' redirector as part of $(< ...), i.e. if
flag==3, make sure the FD of the file to read is > 2 by calling
sh_iomovefd(). Unlike the RedHat patch, this checks for flag==3
to avoid unnecessary sh_iomovefd() calls for normal redirections,
as there was no bug with those.
src/cmd/ksh93/tests/io.sh:
- Add test.
When ksh was compiled with SHOPT_SPAWN (the default), any command
substitution embedded in a here-document returned an empty string.
The bug was also present in 93u+ 2012-08-01 (although not in every
case as some systems compile it without SHOPT_SPAWN).
This fixes it by applying a slightly edited combination of two Red
Hat patches (the second containing a fix for the first), which
backport a new command substitution mechanism from the abandoned
ksh 93v- beta version. The originals are:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-macro.patchhttps://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-fd2lost.patch
src/cmd/ksh93/include/io.h:
- The iopipe() function from xec.c is now needed in sh_subshell()
(subshell.c), so rename it to sh_iounpipe() and declare it as an
extern here. The 93v- beta did it as well. (The Red Hat patch did
this without renaming it.)
src/cmd/ksh93/sh/xec.c:
- Backport new versions of iousepipe() and sh_iounpipe() from ksh
93v-. New 'type' flaggery is introduced to distinguish between
different command substitution conditions. What all that means
remains to be determined.
- sh_exec(): I made one change to the Red Hat patch myself: if in a
subshell and the type flags FAMP (for "ampersand" as in '&' as in
background job) and TFORK are set, continue to call sh_subfork()
to fork the subshell unconditionally, instead of only if we're in
a command substitution connected to an unseekable file. Maybe the
latter works for the 93v- code, but on 93u+(m) it causes a couple
of regressions, which are fixed by my change:
signal.sh[273]: subshell ignoring signal does not send signal to parent
signal.sh[276]: subshell catching signal does not send signal to parent
Details: https://github.com/ksh93/ksh/issues/104#issuecomment-696341902
src/cmd/ksh93/sh/macro.c,
src/cmd/ksh93/sh/subshell.c:
- Updates that go with those new functions.
Fixes: https://github.com/ksh93/ksh/issues/104
Affects: https://github.com/ksh93/ksh/issues/124
This fixes two memory leaks in old-style command substitutions
(one when invoking an alias, one when invoking an autoloaded
function), as well as a possible third leak with an unknown
reproducer, by applying this Red Hat patch:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-mlikfiks.patch
src/cmd/ksh93/sh/macro.c: comsubst():
- For as-yet unknown reasons, the alias leak did not occur when
adding a space at the end of the command substitution, as in
a=`some_alias `. This fix is a workaround that simply writes
an extra space to the stack. TODO: a real fix.
src/cmd/ksh93/sh/path.c: funload():
- Add missing free() before return. This fixes the leak with
autoloaded functions.
src/cmd/ksh93/sh/lex.c: alias_exceptf():
- This function is called "whenever an end of string is found with
alias". This adds a check for an SF_FINAL stream status flag when
deciding whether to call free(). In sfio.h this is commented as:
#define SF_FINAL 11 /* closing is done except stream free */
When I revert this change, none of the regression tests fail, so
I don't know how to trigger this supposed leak. But it makes some
sense given the sfio.h comment, so I'll keep it.
src/cmd/ksh93/tests/leaks.sh:
- Add the reproducers from rhbz#982142 as regression tests
(including an extra one for nested command substitutions that was
already fixed as of 93u+, but testing is good).
I replaced the external 'expr' and 'ls' commands by uses of
the 'true' builtin, otherwise the tests take far too long to run
with 16384 iterations. At least the alias leak was still behaving
identically after replacing 'ls' by 'true'.
Hopefully this doesn't introduce new bugs, but it does fix at
least the following:
1. When whence -v/-a found an "undefined" (i.e. autoloadable)
function in $FPATH, it actually loaded the function as a side
effect of reporting on its existence (!). Now it only reports.
2. 'whence' will now canonicalise paths properly. Examples:
$ whence ///usr/lib/../bin//./env
/usr/bin/env
$ (cd /; whence -v dev/../usr/bin//./env)
dev/../usr/bin//./env is /usr/bin/env
3. 'whence' no longer prefixes a spurious double slash when doing
something like 'cd / && whence bin/echo'. On Cygwin, an initial
double slash denotes a network server, so this was not just a
cosmetic problem.
4. 'whence -a' now reports a "tracked alias" (a.k.a. hash table
entry, i.e. cached $PATH search) even if an actual alias by the
same name exists. This needed fixing because in fact the hash
table entry continues to be used when bypassing the alias.
Aliases and "tracked aliases" are not remotely the same thing;
confusing nomenclature is not a reason to report wrong results.
5. When using 'hash' or 'alias -t' on a command that is also a
builtin to force caching a $PATH search for the external
command, 'whence -a' double-reported the path:
$ hash printf; whence -a printf
printf is a shell builtin
printf is /usr/bin/printf
printf is a tracked alias for /usr/bin/printf
This is now fixed so that the second output line is gone.
Plus, if there were multiple versions of the command on $PATH,
the tracked alias was reported at the end, which is the wrong
order. This is also fixed.
src/cmd/ksh93/bltins/whence.c: whence():
- Refactor the do...while loop that handles whence -v/-a for path
searches in such a way that the code actually makes sense and
stops looking like higher esotericism. Just doing this fixed#2,
#4 and #5 above (the latter two before I even noticed them). For
instance, the path_fullname() call to canonicalise paths was
already there; it was just never used.
- Remove broken 'notrack' flaggery for deciding whether to report a
hash table entry a.k.a. "tracked alias"; instead, check the hash
table (shp->track_tree).
src/cmd/ksh93/sh/path.c:
- path_search(): Re #3: When prefixing the PWD, first check if
we're in '/' and if so, don't prefix it; otherwise, adding the
next slash causes an initial double slash. (Since '/' is the only
valid single-character absolute path, all we need to do is check
if the second character pwd[1] is non-null.)
- path_search(): Re #1: Stop autoloading when called by 'whence':
* The 'flag==2' check to avoid autoloading a function was
broken. The flag value is 2 on the first whence() loop
iteration, but 3 on subsequent ones. Change to 'flag >= 2'.
* However, this only fixes it if the function file does not have
the x permission bit, as executable files are handled by
path_absolute() which unconditionally autoloads functions!
So, pass on our flag parameter when callling path_absolute().
- path_absolute(): Re #1: Add flag parameter. Do not autoload
functions if flag >= 2.
src/cmd/ksh93/include/path.h,
src/cmd/ksh93/bltins/typeset.c,
src/cmd/ksh93/sh/main.c,
src/cmd/ksh93/sh/xec.c:
- Re #1: Update path_absolute() calls, adding a 0 flag parameter.
src/cmd/ksh93/include/name.h:
- Remove now-unused pathcomp member from union Value. It was
introduced in 99065353 to allow examining the value of a tracked
alias. This commit uses nv_getval() instead.
src/cmd/ksh93/tests/builtins.sh,
src/cmd/ksh93/tests/path.sh:
- Add and tweak various related tests.
Fixes: https://github.com/ksh93/ksh/issues/84
{Brace,expansion} is potentially incompatible with POSIX scripts,
because in POSIX those are simple literal strings with no special
meaning. So the POSIX option should really turn that off.
As of b301d417, the 'posix' option was also forcing 'letoctal'
behaviour on, without actually setting that option. I've since
found that to be a botch; 'let' may recognise octals without that
option being set, and that looks like a bug.
So as of this commit, the '-o posix' option actually toggles both
of these options off/on and on/of, respectively. 'set +o posix'
toggles them inversely. However, it is now possible to control both
options (and their associated behaviour) independently in between
'set -o posix' and 'set +o posix'. Much better.
src/cmd/ksh93/sh/main.c: sh_main():
- If SH_POSIX was set on init, turn on SH_LETOCTAL by default
instead of SH_BRACEEXPAND.
src/cmd/ksh93/sh/args.c: sh_applyopts():
- Turn off SH_BRACEEXPAND and turn on SH_LETOCTAL when SH_POSIX is
turned on (but not if it was already on).
- Turn on SH_BRACEEXPAND and turn off SH_LETOCTAL when SH_POSIX is
turned off (but not if it was already off).
src/cmd/ksh93/sh/arith.c: arith():
- Revert to pre-b301d417 and only check SH_LETOCTAL option when
deciding whether 'let' should skip initial zeros.
src/cmd/ksh93/tests/options.sh:
- Update $- test to allow '-o posix' to switch B = braceexpand.
src/cmd/ksh93/sh.1:
- Update.
- Edit for clarity.
This allows running 'bin/shtests leaks' on a ksh without the
vmstate builtin and/or that is not compiled with AST vmalloc.
It falls back to 'ps -o rss= -p $$' to get the memory state.
This is in preparation for the beta and release versions, which
will not use vmalloc due to its defects[*]. Unfortunately,
abandoning vmalloc means abandoning the vmstate builtin which makes
it extremely efficient to test for memory leaks.
Because 'ps' only has a KiB granularity and also shows larger
artefacts/variations than vmalloc on most systems, we need many
more iterations (16384) and also tolerate a higher number of bytes
per iterations (8). So the run takes much longer. To tolerate only
2 bytes per iteration, we would need four times as many iterations,
which would make it take too long to run. Since a single word (e.g.
one pointer) on a 64-bit system is 8 bytes, it still seems very
unlikely for a real memory leak to be that small.
This is coded to make it easy to detect and add iteration and
tolerance parameters for a new method to get the memory state,
if some efficient or precise system-specific way is discovered.
I've also managed to trigger a false leak with shcomp in a UTF-8
locale on CentOS on a ksh with vmalloc/vmstate. So this increases
the tolerance for vmalloc from 2 to 4 bytes/iteration.
[*] Discussion: https://github.com/ksh93/ksh/issues/95
When compiling ksh with '-O0 -g -D_std_malloc' on my Mac, the
paths.sh regress test set crashed. This is the test that crashed:
print 'FPATH=../fun' > bin/.paths
cat <<- \EOF > fun/myfun
function myfun
{
print myfun
}
EOF
x=$(FPATH= PATH=$PWD/bin $SHELL -c ': $(whence less);myfun') 2> /dev/null
[[ $x == myfun ]] || err_exit 'function myfun not found'
The crash occurred on the second-to-last line. The backtrace
suggests an invalid use of strcpy() with overlapping memory:
0 libsystem_kernel.dylib __pthread_kill + 10
1 libsystem_pthread.dylib pthread_kill + 284
2 libsystem_c.dylib abort + 127
3 libsystem_c.dylib abort_report_np + 177
4 libsystem_c.dylib __chk_fail + 48
5 libsystem_c.dylib __chk_fail_overlap + 16
6 libsystem_c.dylib __chk_overlap + 34
7 libsystem_c.dylib __strcpy_chk + 64
8 ksh path_chkpaths + 1038 (path.c:1534)
9 ksh path_addcomp + 1032 (path.c:1481)
10 ksh path_addpath + 395 (path.c:1598)
11 ksh put_restricted + 626 (init.c:329)
[...]
src/cmd/ksh93/sh/path.c: path_chkpaths():
- When reading the '.paths' file, use memmove(3) instead of
strcpy(3) as the former does a non-destructive copy with
tolerance for overlap.
The following set of commands caused ksh to crash:
$ unalias history; unalias r
Memory fault
When ksh is compiled with -D_std_malloc, the crash always
occurs when the 'r' alias is removed with 'unalias r',
although with vmalloc 'unalias history' must be run first
for the crash to occur. With the native malloc, the crash
message is also different:
$ unalias history; unalias r
free(): invalid pointer
Abort
This crash happens because when an alias is unset, _nv_unset
removes the NV_NOFREE flag which results in an invalid use
of free(3) as nv_isattr no longer detects NV_NOFREE afterward.
The history and r aliases shouldn't be freed from memory by
nv_delete because those aliases are given the NV_NOFREE attribute.
src/cmd/ksh93/bltins/typeset.c:
- Save the state of NV_NOFREE for aliases to fix the crash
caused by 'unalias r'.
src/cmd/ksh93/tests/alias.sh:
- Use unalias on both history and r to check for the crash.
'unalias -a' can't be used to replicate the crash.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
This patch from Red Hat fixes the following:
1. ksh was ignoring the -m (-o monitor) option when specified on
the invocation command line.
2. Scripts did not properly terminate their background processes
on Ctrl+C if the -m option was turned off. Reproducer:
xterm &
read junk
When run as a script without turning on -m, pressing Ctrl+C
should terminate the xterm, and now does.
3. Scripts no longer attempt to set the terminal foreground process
group ID, as only interactive shells should be doing that.
This makes some progress on https://github.com/ksh93/ksh/issues/119
but we're a long way from fixing all of that.
src/cmd/ksh93/sh/main.c: exfile():
- On non-interactive shells, do not turn off the monitor option.
Instead, if it was turned on, turn on the SH_MONITOR state flag.
src/cmd/ksh93/edit/edit.c: ed_getchar():
- On Ctrl+C, issue SIGINT to the current process group using
killpg(2) instead of going via sh_fault(), which handles a
signal only for the current shell process.
src/cmd/ksh93/sh/jobs.c: job_reap(), job_reset(),
src/cmd/ksh93/sh/xec.c: sh_exec():
- Only attempt to set the terminal foreground process group ID
using tcsetpgrp(3) if the shell is interactive.
Original patch: https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-kshmfix.patch
This was applied to Red Hat's ksh 93u+ on 8 July 2013.
src/cmd/ksh93/data/signals.c includes two checks for the JOBS
identifier; if it is not defined then the interactive shell's
background job signal messages for SIGINT and SIGPIPE are empty.
The cause was that the "jobs.h" header, which defines that ID, was
not #included in signals.c. This commit adds that #include.
(ksh 93u+, ksh 93v- and ksh2020 all have this bug as well.)
Before:
$ sleep 30 &
[1] 86430
$ kill -s INT "$!"
[1] + sleep 30 &
$
After:
$ sleep 30 &
[1] 86445
$ kill -s INT "$!"
[1] + Interrupt sleep 30 &
$
In the vi and emacs line editors, repeat count parameters can now
also be used for the arrow keys and the forward-delete key. E.g.,
in emacs mode, <ESC> 7 <left-arrow> will now move the cursor seven
positions to the left. In vi control mode, this would be entered
as: 7 <left-arrow>.
src/cmd/ksh93/edit/emacs.c:
- ed_emacsread(): Upon getting ^[ (ESC), save current repeat count
in a new variable; restore and reset it upon the next character.
- escape(): Minor bugfix: when processing a ^[[x sequence where 'x'
is a character other than '~' (which would be DEL), also reinsert
the final character into the buffer so scripts can detect them.
src/cmd/ksh93/edit/vi.c:
- cntlmode(): Do not reset the repeat count if the command is '[',
the character following ESC in VT220 escape sequences.
- mvcursor():
* Do not use getcount() to get the character following '[', as
that was parsing repetition parameters in the wrong place.
There wouldn't be any, so this would reset the repeat count.
* After that, no more need for the special-casing of ^[[3~ (DEL)
introduced in f2a3f4e3. Move it to within the 'switch' block.
* When handling left and right arrows and Home and End keys, do
not modify cursor directly but ed_ungetchar() the corresponding
traditional command keys as with the rest. Otherwise a repeat
count parameter would now wrongly survive those keys.
src/cmd/ksh93/sh.1:
- Document control character notation used for vi mode docs.
- Since vi control mode beeps and aborts on ESC except if a
subsequent [ is already in the input buffer upon receiving ESC,
document that VT220 escape sequences only preserve repeat counts
when entered into the input buffer all at once.
- Don't skip the initial ESC in the documentation of the VT220
escape sequences. In control mode, skipping the initial ESC still
works as before, but that is now undocumented, as it's really
nothing more than an artefact of VT220 escape processing.
- Move the two long paragraphs on '-o viraw' and canonical (i.e.
line-based) input processing from the vi editor introduction to
the options section under 'viraw'. It is much too arcane for the
intro, and besides, ksh 93u+ (and hence also 93u+m) has
SHOPT_VIRAW enabled by default, so the shell is compiled to force
this option on at all times, making it even less relevant for
most users.
A memory leak occurred when typeset was used in a function called
from within a command substitution. This fix was backported from
the 93v- beta by Red Hat on 22 Jan 2014. Source:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-memlik3.patch
src/cmd/ksh93/include/name.h,
src/cmd/ksh93/sh/subshell.c:
- Replace the nv_subsaved() function by the version from ksh 93v-.
This version frees a table from memory if the NV_TABLE flag is
passed in the new second parameter, a bitmask for flags (which
was oddly named 'table'; I've renamed it to 'flags').
src/cmd/ksh93/sh/name.c:
- nv_delete(): When calling nv_subsaved(), pass on the NV_TABLE
flag if given.
- table_unset(): Call nv_delete() with the NV_TABLE flag.
src/cmd/ksh93/tests/leaks.sh:
- Add test based on the reproducer provided in Red Hat bug 1036470.
I now have access to some of the private bugs on the Red Hat bug
tracker. This one doesn't have a lot of information on the patch,
but it contains a good reproducer, so we can at least verify that
it works.
src/cmd/ksh93/sh/array.c,
src/cmd/ksh93/sh/name.c:
- Apply the patch associated with Red Hat bug #921455. Source:
https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-memlik.patch
This was applied to Red Hat's ksh on 04 Jul 2013.
src/cmd/ksh93/tests/leaks.sh:
- Add leak tests for associative and indexed arrays in functions
based on the reproducer from rhbz#921455.
- Both tests still leak (though much less) when run in a locale
other than C. For now, temporarily set the locale to C and add
a TODO note. Perhaps another Red Hat patch is yet to fix this.
One of the few AT&T fixes applied in early 2020 was a one-line
change to emacs.c tab handling, with only this info in the commit
message:
| - fix to emacs.c (I think from dgk)
So, it's unknown what that was meant to accomplish, but I did just
find that it breaks menu-driven pathname completion:
$ ls arch/darwin.i386-64/
1) bin/
2) fun/
3) include/
4) lib/
5) man/
6) src/
$ ls arch/darwin.i386-64/3 _
Typing 3+TAB should have inserted 'include/' but inserted a literal
tab instead. Reverting the vague "fix" fixes this bug.
On every modern system, the forward-delete key on PC/Mac keyboards
generates the VT220 sequence ESC [ 3 ~. Every other shell with an
editor handles this now, ksh93 seems to be the last not to.
src/cmd/ksh93/edit/emacs.c: escape():
- Handle the ^[[3 as part of normal escape processing, then read an
extra character to check for the final '~'. If detected, insert
an ERASECHAR key event.
src/cmd/ksh93/edit/vi.c: mvcursor():
- Replace the ^[[3~ sequence by an 'x' command. We have to
special-case its processing, because vi mode parses numbers as
repetition operators. The escape sequence contains a number,
making it incompatible with normal command handling. This means
number repetitions don't work with the forward-delete key. If
that annoys anyone enough to fix it, a patch would be welcome.
For now, it will do to make the forward-delete key stop
exhibiting bizarre behaviour (beep + change case + move forward).
src/cmd/ksh93/sh.1
- Copy-edit emacs documentation for VT220-style sequences; map them
to their actual key, otherwise it's meaningless to the reader.
- Document the new forward-delete key behaviour for emacs mode.
- Leave the forward-delete key for vi mode undocumented for now, as
repetitions don't work, so it doesn't really match the vi canon.
(OTOH, it doesn't work in vim, either...)
_sfcvt(), "convert a floating point value to ASCII", did not adjust
for negative decimal place movement as what happens with leading
zeroes. This caused ksh's 'printf %f' formatter to fail to round
floating point values correctly.
src/lib/libast/sfio/sfcvt.c:
- Removed constraint of <1e-8 for doubles by matching what was done
for long doubles having <.1.
- Corrected a condition when the next power of 10 occurred and that
new 1 digit was being overwritten by a 0.
src/cmd/ksh93/tests/math.sh:
- Validate that typeset -E/F formatting matches that of their
equivalent printf formatting options as well as checking for
correct float scaling of the fractional parts.
The fix was incomplete: expansions using '?' (${var?w(ord},
${var:?wo)rd}) still did not tolerate parentheses in the word
as regular characters.
It was also possible to simplify the fix by making use of the
ST_BRACE (sh_lexstate7[]) state table. See data/lexstates.c and
include/lexstates.h.
src/cmd/ksh93/sh/lex.c: sh_lex(): case S_MOD1:
- The previous fix tested for modifier operator characters : - + =
as part of the S_MOD2 case, though they are defined as S_MOD1 in
the ST_BRACE state table. It only worked because of the
fallthrough. And it turns out the S_MOD1 case already had a
similar fix, though incomplete. The new fix effectively cancelled
the old one out as any S_MOD1 character eventually led to
'continue'. So it can be simplified by removing most of that
code, without causing any change in behaviour. Only the mode
change to the ST_QUOTE state table followed by 'continue' is
necessary. This also fixes it for the '?' operator as that is
also defined as S_MOD1 in the ST_BRACE state table.
src/cmd/ksh93/sh/macro.c:
- When skipping a ${...} expansion using sh_lexskip(), use the
ST_QUOTE state table if the character c is an S_MOD1 modifier
operator character. This makes it consistent with the S_MOD1
handling in sh_lex().
src/cmd/ksh93/tests/variables.sh:
- Update regression tests to include ? and :? operators.
File descriptors are not properly closed, causing a leak, when
using a process substitution as an argument to a shell function.
See: https://github.com/ksh93/ksh/issues/67
Process substitution uses /dev/fd/NN pseudofiles if the kernel
provides them. This is tested in src/cmd/ksh93/features/options
which causes SHOPT_DEVFD to be defined if /dev/fd/9 can be used.
If not, ksh uses a fallback mechanism involving a temporary FIFO,
which works on all Unix variants.
As it happens, the leak only occurs when using the /dev/fd
mechanism. So, until a fix is found, we can work around the bug by
disabling it. The FIFO mechanism might be slightly less robust,
but it's an improvement over leaking file descriptors. Plus, there
is room for improving it.
src/cmd/ksh93/include/defs.h:
- Unconditionally redefine SHOPT_DEVFD as 0 for now.
src/cmd/ksh93/sh/args.c: sh_argprocsub():
- pathtemp() does appropriate access checks using access(2), but
there is an inherent race condition between calling it and
mkfifo(). Make the FIFO mechanism more robust by handling errors,
trying again if an error occurs that must have resulted from
losing that race, e.g. file name conflict or temp dir
permission/location change.
- Initially create the FIFO without any permissions, then chmod()
the appropriate user read/write permissions. Since mkfifo()
honours the umask and chmod() does not, this ensures that process
substitution continues to work if a shell script sets a umask
that disallows user read or write. (The /dev/fd/ mechanism does
not care about the umask, so neither should the fallback.)
To find the temporary files directory to use, the pathtemp()
function (generate a unique path to a temporary file) first checks
$TMPDIR and $TMPPATH, then falls back to /tmp, then to /usr/tmp as
a last resort. But all systems replaced /usr/tmp by /var/tmp
decades ago to allow mounting /usr as read-only, and a /usr/tmp
compatibility symlink is no longer commonly provided.
src/lib/libast/path/pathtemp.c:
- Change TMP2 definition from "/usr/tmp" to "/var/tmp".
src/lib/libast/features/mmap,
src/lib/libast/features/stdio:
- Change "/usr/tmp" to "/var/tmp" in feature tests.
Following a community discussion, it became clear that 'r' is
particularly problematic as a regular builtin, as the name can and
does conflict with at least one legit external command by that
name. There was a consensus against removing it altogether and
letting users set the alias in their login scripts. However,
aliases are easier to bypass, remove or rename than builtins are.
My compromise is to reinstate 'r' as a preset alias on interactive
shells only, along with 'history', as was done in 17f81ebe before
they were converted to builtins in 03224ae3. So this reintroduces
the notion of predefined aliases to ksh 93u+m, but only for
interactive shells that are not initialised in POSIX mode.
src/cmd/ksh93/Makefile,
src/cmd/ksh93/Mamfile,
src/cmd/ksh93/include/shtable.h,
src/cmd/ksh93/data/aliases.c:
- Restore aliases.c containing shtab_aliases[], a table specifying
the preset aliases.
src/cmd/ksh93/include/shtable.h,
src/cmd/ksh93/sh/init.c:
- Rename inittree() to sh_inittree() and make it extern, because we
need to use it in main.c (sh_main()).
src/cmd/ksh93/sh/main.c: sh_main():
- Init preset aliases from shtab_aliases[] only if the shell is
interactive and not in POSIX mode.
src/cmd/ksh93/bltins/typeset.c,
src/cmd/ksh93/tests/alias.sh:
- unall(): When unsetting an alias, pass on the NV_NOFREE attribute
to nv_delete() to avoid an erroneous attempt to free a preset
alias from read-only memory. See: 5d50f825
src/cmd/ksh93/data/builtins.c:
- Remove "history" and "r" entries from shtab_builtins[].
- Revert changes to inline fc/hist docs in sh_opthist[].
src/cmd/ksh93/bltins/hist.c: b_hist():
- Remove handling for 'history' and 'r' as builtins.
src/cmd/ksh93/sh.1:
- Update accordingly.
Resolves: https://github.com/ksh93/ksh/issues/125
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.
The 'exit' and 'return' commands without an argument failed to pass
down the exit status of the last-run command when incorporated in a
block with redirection, &&/|| list, 'case' statement, or 'while',
'until' or 'for' loop.
src/cmd/ksh93/bltins/cflow.c:
- Use $?, which is sh.savexit a.k.a. shp->savexit, as the default
exit status value if there is no argument, instead of
shp->oldexit. This fixes the default exit status behaviour to
match POSIX and other shells.
src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/include/shell.h:
- Remove now-unused sh.oldexit (a.k.a. shp->oldexit) private struct
member. It appeared to fulfill the same function as sh.savexit,
but in a slightly broken way.
- Move the savexit/$? declaration from the _SH_PRIVATE part of the
struct definition to the public API part. Since $? uses this,
it's clearly a publicly exposed value already, and this is
generally the one to use. (If anything, it's exitval that should
have been private.) This declares savexit right next to exitval,
rewriting the comments to clarify the difference between them.
src/cmd/ksh93/sh/fault.c,
src/cmd/ksh93/sh/subshell.c,
src/cmd/ksh93/sh/xec.c:
- Remove assignments to shp->oldexit.
src/cmd/ksh93/tests/basic.sh:
- Add thorough regression tests for the default exit status
behaviour of 'return' and 'exit' in various lexical contexts.
- Verify that 'for' and 'case' without any command, as well as a
lone redirection, still correctly reset the exit status to 0.
Fixes: #117
src/cmd/ksh93/include/jobs.h:
- The commit that removed legacy code mistakenly removed the
definition of vmbusy() required for ksh to compile with
-D_std_malloc. Ksh assumes vmbusy is always a macro, even
when _std_malloc is defined. This commit reintroduces the
_std_malloc definition of vmbusy to fix undefined
reference errors.
A coprocess cleanup test could fail on rare occasions because I had
lowered the 'sleep 1' between two test coprocesses to 'sleep .1'.
This increases the sleep to prevent future spurious fails.
Fixes: https://github.com/ksh93/ksh/issues/129
Using a process of elimination I've identified ${.sh.level}
(SH_LEVELNOD) as the cause of the crash. This node apparently
cannot be copied or moved without destabilising the shell. It
contains the current depth of function calls and it cannot be
changed by assignment, so this is not actually a problem.
Meanwhile, this commit re-fixes it for the other three.
src/cmd/ksh93/sh/subshell.c:
- Simplify sh_assignok() by removing special-casing for L_ARGNOD,
SH_SUBSCRNOD and SH_NAMENOD. 'add' now has 3 modes (0, 1, 2).
- The test for a ${ subshare; } was actually wrong. sp->subshare is
a saved backup value. We must test shp->subshare. (re: a9de50bf)
src/cmd/ksh93/bltins/typeset.c:
- setall(): Update the mode 3 sh_assignok() call.
src/cmd/ksh93/tests/variables.sh:
- Regress-test subshell leaks for all special variables except
${.sh.level}.
This reverts commit b3d37b00b0.
While ksh's own regression test suite passed just fine, when
running the modernish[*] regression tests uite, ksh either froze
hard (needing SIGKILL) or threw a spurious syntax error.
Cause unknown, but I'm certainly reverting until I find out.
This reintroduces a subshell leak for four special variables.
[*] https://github.com/modernish/modernish
${var:-wor)d} or ${var+w(ord}. The parentheses now correctly lose
their normal grammatical meaning within the braces. Fix by Eric
Scrivner (@etscrivner) from July 2018 backported from ksh2020.
This fix complies with POSIX:
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_02
src/cmd/ksh93/sh/lex.c: sh_lex():
- Set the ST_QUOTE state when analysing a modifier with parameter
expansions using operators ':', '-', '+', '='. This state causes
subsequent characters (including parentheses) to be considered
quoted, suppressing their normal grammatical meaning.
src/cmd/ksh93/sh/macro.c: varsub():
- Same for skipping the expansion.
Fixes: https://github.com/ksh93/ksh/issues/126
Prior discussion: https://github.com/att/ast/issues/475
The following special variables leaked out of a subshell:
$_, ${.sh.name}, ${.sh.level}, ${.sh.subscript}.
This was due to a faulty optimisation in sh_assignok().
bd3e2a80 fixed that in part, this fixes the rest.
src/cmd/ksh93/sh/subshell.c:
- Simplify sh_assignok() by removing special-casing for these four
special variables. The 'add' param reverts to a simple boolean.
- The test for a ${ subshare; } was actually wrong. sp->subshare is
a saved backup value. We must test shp->subshare. (re: a9de50bf)
src/cmd/ksh93/bltins/typeset.c:
- setall(), unall(): Update sh_assignok() calls.
src/cmd/ksh93/tests/variables.sh:
- Regress-test subshell leaks for all special variables.
Closes: #122
When exporting variables, ksh exports their attributes (such as
'integer' or 'readonly') in a magic environment variable called
"A__z" (string defined in e_envmarker[] in data/msg.c). Child
shells recognise that variable and restore the attributes.
This little-known feature is risky; the environment cannot
necessarily be trusted and that A__z variable is easy to manipulate
before or between ksh invocations, so you can cause a script's
variables to be of the wrong type, or readonly. Backwards
compatibility requires keeping it, at least for now. But it should
be disabled in the posix mode, as it violates POSIX.
To do this, we have to solve a catch-22 in init.c. We must parse
options to know whether to turn on posix mode; it may be specified
as '-o posix' on the command line. The option parsing loop depends
on an initialised environment[*], while environment initialisation
(i.e., importing attributes) should depend on the posix option.
The catch-22 can be solved because initialising just the values
before option parsing is enough to avoid regressions. Importing the
attributes can be delayed until after option parsing. That involves
basically splitting env_init() into two parts while keeping a local
static state variable between them.
src/cmd/ksh93/sh/init.c:
- env_init():
* Split the function in two stages based on a new
'import_attributes' parameter. Import values in the first
stage; import attributes from A__z in the second (if ever).
Make the 'next' variable static as it keeps a state needed for
the attributes import stage.
* Single point of truth, greppability: don't hardcode "A__z" in
separate character comparisons, but use e_envmarker[].
* Fix an indentation error.
- sh_init(): When initialising the environment (env_init), don't
import the attributes from A__z yet; parse options first, then
import attributes only if posix option is not set.
src/cmd/ksh93/sh/name.c:
- sh_envgen(): Don't export variable attributes to A__z if the
posix option is set.
src/cmd/ksh93/tests/attributes.sh:
- Check that variable attributes aren't imported or exported
if the POSIX option is set.
src/cmd/ksh93/sh.1:
- Update.
This was the last item on the TODO list for -o posix for now.
Closes: #20
[*] If environment initialisation is delayed until after option
parsing, bin/shtests shows various regressions, including:
restricted mode breaks; the locale is not initialised properly
so that multibyte variable names break; $SHLVL breaks.
This commit removes the following standards check on init:
strcmp(astconf("CONFORMANCE",0,0),"standard")==0
This also checks for the POSIXLY_CORRECT variable; the libast
configuration system uses it to set "CONFORMANCE" to "standard",
*but*, only if that parameter wasn't already initialised from the
_AST_FEATURES environment variable (see 'getconf --man').
Problem is, there is a harmful interaction between POSIXLY_CORRECT
and _AST_FEATURES. If the latter exists, it overrides the former.
Not only that, merely querying CONFORMANCE makes astconf create and
export the _AST_FEATURES variable, propagating the current setting
to child ksh processes, which will then ignore POSIXLY_CORRECT.
We could get around this by simply using getenv("POSIXLY_CORRECT").
But then the results may be inconsistent with the AST config state.
The whole thing may not be the best idea anyway. Honouring
POSIXLY_CORRECT at startup introduces a backwards compatibility
issue. Existing scripts or setups may export POSIXLY_CORRECT=y to
put external GNU utilities in standards mode, while still expecting
traditional ksh behaviour from newly initialised shells.
So it's probably better to just get rid of the check. This is not
bash, after all. If ksh is invoked as sh (the POSIX standard
command name), or with '-o posix' on the command line, you get the
standards mode; that ought to be good enough.
src/cmd/ksh93/sh/init.c: sh_init():
- Remove astconf call as per above.
In the SHOPT_BASH code, the -o posix option was given a '\374'
(0xFC, 252) single-letter option character. Reasons unclear. The
'set' builtin doesn't accept it. It can be omitted and the option
still works. And it caused the "$-" expansion (listing active
short-form options) to include that invalid high-bit character if
the -o posix option is active, which is clearly wrong.
src/cmd/ksh93/sh/args.c: optksh[], flagval[]:
- Remove '\374' one-letter option equivalent for SH_POSIX.
src/cmd/ksh93/tests/options.sh:
- Add test verifying that '-o posix' does not affect "$-".
The inclusion of the special parameter expansions ${!} and ${$}
(including the braces) in a here-document caused a syntax error.
Bug reported by @Saikiran-m on Github.
src/cmd/ksh93/data/lexstates.c: sh_lexstate7[]:
- Change the state for ! (33) and $ (36) from S_ERR to 0. State
table 7 is for skipping over ${...}, so this avoids the S_ERR
state being invoked in sh_lex() (lex.c) for these characters
while skipping ${...} in a here-doc.
src/cmd/ksh93/tests/heredoc.sh:
- Test evaluating the braces expansion form for all special
parameters (@ * # ! $ - ? 0) in a here-document.
Fixes: https://github.com/ksh93/ksh/issues/127
Since ksh 93u+m comes bundled with libast 20111111, there's no need
to support older versions, so this is another cleanup opportunity.
src/cmd/ksh93/include/defs.h:
- Throw an #error if AST_VERSION is undefined or < 20111111.
(Note that _AST_VERSION is the same as AST_VERSION, but the
latter is newer and preferred; see src/lib/libast/features/api)
All other changed files:
- Remove legacy code for versions older than the currently used
versions, which are:
_AST_VERSION 20111111
ERROR_VERSION 20100309
GLOB_VERSION 20060717
OPT_VERSION 20070319
SFIO_VERSION 20090915
VMALLOC_VERSION 20110808
SHOPT_ENV is an undocumented compile-time option implementing an
experimental method for handling environment variables, which is
implemented in env.h and env.c. There is no mention in the docs or
Makefile, and no mention in the mailing list archives. It adds no
new functionality, but at first glance it's a clean-looking
interface.
However, unfortunately, it's broken. Compiling with -DSHOPT_ENV
added to CCFLAGS causes bin/shtests to show these regressions:
functions.sh[341]: export not restored name=value function call -- expected 'base', got ''
functions.sh[1274]: Environment variable is not passed to a function
substring.sh[236]: export not restored name=value function call
variables.sh[782]: SHLVL should be 3 not 2
In addition, 'export' stops working on unset variables.
In the 93v- beta this code is still present, unchanged, though 93v-
made lots of incompatible changes. By the time ksh2020 noticed it,
it was no longer compiling, so it probably wasn't compiling in the
93v- beta either. Discussion: https://github.com/att/ast/issues/504
So the experiment was already abandoned by D. Korn and his team.
Meanwhile it was leaving sh/name.c with two versions of several
enviornment-related functions, and it's not clear which one is
actually compiled without doing detective work tracing header files
(most of the code was made conditional on _ENV_H, which is defined
in env.h, which is included by defs.h if SHOPT_ENV is defined).
This actively hinders understanding of the codebase. And any
changes to these functions would need to be implemented twice.
src/cmd/ksh93/include/env.h,
src/cmd/ksh93/sh/env.c:
- Removed.
src/cmd/ksh93/DESIGN,
src/cmd/ksh93/Makefile,
src/cmd/ksh93/Mamfile:
- Update accordingly.
All other changed files:
- Remove deactivated code behind SHOPT_ENV and _ENV_H.
An oops in tests/io.sh (re: c607c48c) wrote temporary files outside
$tmp and into src/cmd/ksh93/tests. Let's fix this properly so it
doesn't happen again.
src/cmd/ksh93/tests/shtests:
- Start each test set in its own temporary directory by default.
src/cmd/ksh93/tests/*.sh:
- Refuse to run if $tmp != $PWD.
- Related cleanups.
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.
ksh was enabling POSIX mode on init if it was invoked as any name
that merely started with 'sh' (after parsing initial 'r'). This
included shcomp, which was bad news.
src/cmd/ksh93/sh/init.c: sh_type():
- Check that the 'sh' is at the end of the string by checking
for a final zero byte.
- On Windows (_WINIX, see src/lib/libast/features/common), allow
for a file name extension (sh.exe) by checking for a dot as well.
eeee77ed implemented a POSIX compliance fix that caused a potential
incompatibility with existing ksh scripts; it made the (rarely
used) read/write redirection operator, <>, default to file
descriptor 0 (standard input) as POSIX specified, instead of 1
(standard output) which is traditional ksh93 behaviour. So ksh
scripts needed to change all <> to 1<> to override the new default.
This commit reverts that change, except in the new posix mode.
src/cmd/ksh93/sh/lex.c:
- Make FD for <> default to 0 in POSIX mode, 1 otherwise.
src/cmd/ksh93/tests/io.sh:
- Revert <> regression test changes from 60516872; we no longer
need 1<> instead of <> in ksh code.
If there are file descriptors > 2 opened with 'exec' or 'redirect',
ksh93 has always closed them when invoking another pogram. This is
contrary to POSIX which states:
Utilities other than the special built-ins […] shall be invoked
in a separate environment that consists of the following. The
initial value of these objects shall be the same as that for
the parent shell, except as noted below.
* Open files inherited on invocation of the shell, open files
controlled by the exec special built-in plus any
modifications, and additions specified by any redirections to
the utility
* […]
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_12
src/cmd/ksh93/sh/io.c: sh_redirect():
- When flag==2, do not close FDs > 2 if POSIX mode is active.
src/cmd/ksh93/tests/io.sh:
- Regress-test inheriting FD 7 with and without POSIX mode.
src/cmd/ksh93/sh.1:
- Update.
Though the "let" builtin is not itself a POSIX standard command, it
processes standard shell arithmetic, so it should recognise octals
by leading zeros as POSIX requires if the 'posix' option is on.
This overrides the setting of the 'letoctal' option.
Note that none of this applies to the ((...)) arithmetic command,
which has always recognised leading-octal zeros and does not listen
to 'letoctal'. So setting the posix mode makes this consistent.
src/cmd/ksh93/sh/arith.c:
- When running the 'let' builtin, test that both SH_LETOCTAL and
SH_POSIX are off before stripping leading zeros to disable octal
number recognition.
- Cosmetic: fix spurious newline.
src/cmd/ksh93/sh.1:
- Document the change.
src/cmd/ksh93/tests/shtests:
- Make sure to disable posix mode by default for regression tests.
On 16 June there was a call for volunteers to fix the bash
compatibility mode; it has never successfully compiled in 93u+.
Since no one showed up, it is now removed due to lack of interest.
A couple of things are kept, which are now globally enabled:
1. The &>file redirection shorthand (for >file 2>&1). As a matter
of fact, ksh93 already supported this natively, but only while
running rc/profile/login scripts, and it issued a warning. This
makse it globally available and removes the warning, bringing
ksh93 in line with mksh, bash and zsh.
2. The '-o posix' standard compliance option. It is now enabled on
startup if ksh is invoked as 'sh' or if the POSIXLY_CORRECT
variable exists in the environment. To begin with, it disables
the aforementioned &> redirection shorthand. Further compliance
tweaks will be added in subsequent commits. The differences will
be fairly minimal as ksh93 is mostly compliant already.
In all changed files, code was removed that was compiled (more
precisely, failed to compile/link) if the SHOPT_BASH preprocessor
identifier was defined. Below are other changes worth mentioning:
src/cmd/ksh93/sh/bash.c,
src/cmd/ksh93/data/bash_pre_rc.sh:
- Removed.
src/cmd/ksh93/data/lexstates.c,
src/cmd/ksh93/include/shlex.h,
src/cmd/ksh93/sh/lex.c:
- Globally enable &> redirection operator if SH_POSIX not active.
- Remove warning that was issued when &> was used in rc scripts.
src/cmd/ksh93/data/options.c,
src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/args.c:
- Keep SH_POSIX option (-o posix).
- Replace SH_TYPE_BASH shell type by SH_TYPE_POSIX.
src/cmd/ksh93/sh/init.c:
- sh_type(): Return SH_TYPE_POSIX shell type if ksh was invoked
as sh (or rsh, restricted sh).
- sh_init(): Enable posix option if the SH_TYPE_POSIX shell type
was detected, or if the CONFORMANCE ast config variable was set
to "standard" (which libast sets on init if POSIXLY_CORRECT
exists in the environment).
src/cmd/ksh93/tests/options.sh,
src/cmd/ksh93/tests/io.sh:
- Replace regression tests for &> and move to io.sh. Since &> is
now for general use, no longer test in an rc script, and don't
check that a warning is issued.
Closes: #9
Progresses: #20
Following a community objection to its removal, the inline 'test'
manual page along with its strange method of invocation is
restored. I've taken the opportunity to correct several mistakes,
add some missing info, do some copy-editing, and document the way
to get these docs in the main (k)sh.1 manual.
Discussion:
https://github.com/ksh93/ksh/commit/fa6a180f#commitcomment-41897553
Did you know that you could get a manual page for the 'test'/'['
builtin command using one of these strange command lines?
test --man --
[ --man -- ]
Neither did I. It's not documented or mentioned anywhere (and this
syntax violates POSIX). So nobody knows about it, which makes that
documentation useless. (The regular --man option doesn't work
because that would break 'test'.) I only found out how to invoke it
when I understood what the uncommented C code handling this does.
The test/[ command's self-documentation is unmaintained since 2003
and somewhat incomplete. It's also mostly redundant with the
documentation on Conditional Expressions in the main (k)sh.1 manual
page. But unlike the latter, this is resident in RAM, wasting
working memory in every shell process.
src/cmd/ksh93/sh.1:
- Add documentation for 'test'/'[' commands (yes, they were not
mentioned in the main manual page until now), describing them
in terms of differences from '[[' and recommending the latter.
src/cmd/ksh93/include/test.h,
src/cmd/ksh93/bltins/test.c,
src/cmd/ksh93/data/testops.c:
- Remove RAM-resident --man doc for test/[ command.
- Remove the bizarre option parsing that allowed invoking it.
Instead of a global 'sh_in_test_builtin' integer flag, it is nicer
to use the mechanism for shell state bits, which was designed for
this sort of thing.
src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/defs.c:
- Remove global sh_in_test_builtin integer.
- Define new SH_INTESTCMD state bit.
src/cmd/ksh93/bltins/test.c: _ERROR_exit_b_test(), b_test():
- Use the new state bit.
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.
As of 8477d2ce, the mbwide() macro (which tests if we're in a
multibyte locale, i.e. UTF-8) is redefined as a constant 0 if we're
compiling without SHOPT_MULTIBYTE. See src/cmd/ksh93/include/defs.h
The other multibyte macros use mbwide() as well, so they all revert
to the single-byte fallbacks in that case, and the multibyte code
in them is never compiled. See src/lib/libast/include/ast.h
Consequently we can now do a bit of cleanup and get rid of many of
the '#if SHOPT_MULTIBYTE' directives, as the compiler optimiser
will happily remove the multibyte-specific code. This increases the
legibility of the ksh code.
I'm taking the opportunity to fix a few typos and whitespace
formatting glitches as well.
The first of the two multibyte fixes from 8b5f11dc (which was for
using the first character of IFS as an output field separator when
expanding "$*" and similar) had a minor backwards compatibility
problem: if $IFS started with a byte sequence that is not a valid
UTF-8 character, then it treated IFS as empty in UTF-8 locales, so
the fields would be joined without any separator. The expected
behaviour would be for it to fall back to using the first byte of
IFS as it used to (and as bash and zsh do).
The new code handling this was also a bit kludgy and inefficient,
repeating the mbsize() calculation for every byte of the separator
character and for every field joined by the expansion.
src/cmd/ksh93/sh/macro.c: varsub():
- Rewrite code for joining fields for $* in a quoted or scalar
context and $@ in a scalar context, eliminating a confusing 'd'
variable and concentrating the routine in one block.
- When expanding $* with a multibyte separator (first character
of $IFS), only calculate the size in bytes once per expansion.
- If $IFS starts with a byte sequence that represents an invalid
multibyte character, fall back to using the first byte.
src/cmd/ksh93/tests/variables.sh:
- Tweak some regression tests, including one that overwrote $LANG.
- Add test for invalid multibyte character behaviour as per above.
src/cmd/ksh93/sh/path.c: canexecute():
- Close file descriptors inside of the err label. This fixes
a file descriptor leak that occurs when open succeeds but
fstat fails with EIO. The previous code only returned -1
after 'goto err', leaving the opened file descriptor
inaccessible. This bugfix was backported from ksh2020:
https://github.com/att/ast/commit/55cad1d
The package script searches for a good shell to run the build
scripts, preferring a ksh. But it also finds any recently compiled
development version of ksh in arch/*/bin that may be broken, or
have debug code, etc. -- and uses that in preference to anything
else. This is quite capable of breaking the build process.
The way to get around it is to do something like
bin/package make SHELL=/bin/ksh
which is annoying to have to keep doing.
bin/package,
src/cmd/INIT/package.sh:
- When finding a good shell, use the saved user path ($path), not
the current $PATH which includes arch/$HOSTTYPE/bin. Prefix this
temporary path with `getconf PATH`, the system's default path,
so that known-good system shells are found first.
All changed files:
- Put the shell in POSIX mode if it has an '-o posix' option.
- Remove nonsense disabling 'set -x' on bash. It's not broken.
bin/package, src/cmd/INIT/package.sh:
- Add check blocking native zsh mode (e.g., "$path" conflicts).
Using a 'sh -> zsh' symlink works, so recommend that.
- Remove old ksh93 version check for a supposed conflict with
libcmd. It was broken; it would revert to /bin/sh, but on illumos
distributions, /bin/sh is a ksh93 of a version that is supposedly
affected. It builds fine anyway.
- Rewrite checksh() to incorporate the shell compatibility checks
that were previously in two different places in 'package'.
bin/ignore, src/cmd/INIT/ignore.sh,
bin/silent, src/cmd/INIT/silent.sh:
- Change bad check for a full POSIX 'export' command (no, $RANDOM
has nothing to do with that) with a proper feature test.
The shell detection test assumed that any shell that has $RANDOM
and isn't bash must be ksh and have the 'print' built-in command.
This broke iffe on NetBSD 9.0 sh, which has $RANDOM but not 'print'.
src/cmd/INIT/iffe.sh:
- Rewrite shell detection test to test for features actually used
by iffe. "$shell" can now have the values 'bsh' for an ancient
pre-POSIX Bourne shell, 'bash' for bash, 'ksh' for a shell with
'let', 'typeset -u' and 'print' (ksh88, ksh93, mksh, pdksh, zsh),
and 'posix' for another POSIX or POSIX-ish shell with modern
command substitutions and parameter substitutions. (The 'posix'
value currently is not actively checked for anywhere, but avoids
unnecessary use of bsh fallbacks.)
If ksh was compiled with -DSHOPT_REGRESS=1, it would immediately
segfault on init. After fixing that, another segfault remained that
occurred when using the --regress= command line option with an
invalid option-argument.
The __regress__ builtin allows tracing a few things (see
'__regress__ --man' after compiling with -DSHOPT_REGRESS=1, or
usage[] in src/cmd/ksh93/bltins/regress.c). It seems of limited
use, but at least it can be used/tested now.
src/cmd/ksh93/sh/init.c: sh_init():
- Move the call to sh_regress_init() up. The crash on init was
caused by geteuid() being intercepted by regress.c before the
shp->regress (== sh.regress) pointer was initialised.
- The builtin can also be called using a --regress= option-argument
on the ksh command line. Before calling b___regress__() to parse
that, temporarily change error_info.exit so any usage error calls
exit(3) instead of sh_exit(), as the latter assumes a fully
defined shell state and this call is done before the shell is
fully initialised.
The INIT scripts are supposed to be Bourne shell scripts, not
POSIX, so we can't use $( ... ) command substitutions.
bin/package,
src/cmd/INIT/package.sh:
- Change a POSIX command substitution to old-style backticks.
This fixes a hanging bug that could occur on macOS when using the
'read' command to read from a FIFO and encountering end-of-file
without a final newline character. It also makes the 'read' command
perform 15-25% faster on macOS and Linux.
The previous version (ff385e5a) failed on SunOS/Solaris/Illumos
because those systems apparently don't (fully) support the POSIX
standard recv(2) syscall with MSG_PEEK[*], which is the feature
that iffe detects under the 'socket_peek' identifier. On Illumos,
using that methods causes a compilation failure (unknown identifier
MSG_PEEK); on Solaris 11.4, that method causes multiple regressions
in tests/io.sh, suggesting the method compiles but doesn't work at
all. Instead, SunOS/Solaris/Illumos requires the method using
ioctl(2)+I_PEEK and select(2). No other system that ksh currently
builds on requires this method, so it is now only used on
SunOS/Solaris/Illumos.
So far, this version of sfpkrd() has been tested to work correctly
on Linux, macOS, FreeBSD, NetBSD, OpenBSD, HP-UX, Solaris, and
OmniOS (an Illumos distribution).
It still fails to peek on Cygwin, but in the exact same way it
failed before, so that's no loss.
To test, run the 'io' test set: bin/shtests -p io
src/lib/libast/sfio/sfpkrd.c: sfpkrd():
- Remove long-obsolete Mac OS X and Solaris bug workarounds.
- Remove methods that are no longer needed.
On systems with a POSIX compliant recv(2), the only thing that
is required to avoid regressions is the code that was conditional
upon the socket_peek feature test, which tests for the correct
functioning of the recv(2) syscall. This has now been made
mandatory for non-SunOS/Solaris/Illumos systems (using an #error
directive if it is not detected), with the other methods removed.
The result performs 15-25% faster on macOS and Linux while
passing all the regression tests.
On macOS, avoiding the select(2) method fixes the hanging bug.
On SunOS/Solaris/Illumos (the '__sun' identifier), the method
using ioctl(2)+I_PEEK and select(2) (iffe feature IDs:
stream_peek and lib_select) is preserved.
Resolves: https://github.com/ksh93/ksh/issues/118 (again)
[*] https://pubs.opengroup.org/onlinepubs/9699919799/functions/recv.html
This fixes a hanging bug that could occur on macOS when using the
'read' command to read from a FIFO and encountering end-of-file
without a final newline character. It also makes the 'read' command
perform 15-25% faster on macOS and Linux and maybe other systems.
src/lib/libast/sfio/sfpkrd.c: sfpkrd():
- Get rid of the optional stuff that uses the poll(2) or select(2)
syscalls. The only thing that is required to avoid regressions is
the code that was conditional upon the socket_peek feature test,
which tests for the correct functioning of the recv(2) syscall.
This has now been made mandatory. The rest now uses what was
previously a fallback in plain C, resulting in a function that is
not only more readable, but actually faster than the syscalls.
Resolves: https://github.com/ksh93/ksh/issues/118
CDE <https://cdesktopenv.sf.net/> developer Chase writes, re dtksh:
| Everything is now completely working, and we are almost ready to
| add ksh93 as a submodule, but I have one last commit to get rid
| of some warnings we are facing. nval.h has some of these
| "compatiblity redefines" that are causing issues whenever we
| include it (warnings about redefining values) [...].
src/cmd/ksh93/include/nval.h:
- Replace ancient compatibility redefines by an unconditional
'#include <hash.h>'; ksh works fine with the "new" hash library.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
If a command's path was previously added to the hash table as a
'tracked alias', then the hash table entry was used, bypassing
the default utility path search activated by 'command -p'.
'command -p' activates a SH_DEFPATH shell state. The bug was caused
by a failure to check for this state before using the hash table.
This check needs to be added in four places.
src/cmd/ksh93/sh/path.c,
src/cmd/ksh93/sh/xec.c:
- path_search(), path_spawn(), sh_exec(), sh_ntfork(): Only consult
the hash table, which is shp->track_tree, if the SH_DEFPATH shell
state is not active.
src/cmd/ksh93/tests/path.sh:
- Add regress tests checking that 'command -p' and 'command -p -v'
still search in the default path if a hash table entry exists for
the command searched.
'command -x' (basically builtin xargs for 'command') worked for
long argument lists on *BSD and HP-UX, but not on macOS and Linux,
where it reliably entered into an infinite loop.
The problem was that it assumed that every byte of the environment
space can be used for arguments, without accounting for alignment
that some OSs do. MacOS seems to be the most wasteful one: it
aligns on 16-byte boundaries and requires some extra bytes per
argument as well.
src/cmd/ksh93/sh/path.c:
- path_xargs(): When calculating how much space to subtract per
argument, add 16 extra bytes to the length of each argument, then
align the result on 16-byte boundaries. The extra 16 bytes is
more than even macOS needs, but hopefully it is future-proof.
- path_spawn(): If path_xargs() does fail, do not enter a retry
loop (which always becomes an infinite loop if the argument list
exceeds OS limitations), but abort with an error message.
Four libast hash functions/macros (which ksh93 doesn't actually use)
were overridden with the following comment:
/*
* These following are for binary compatibility with the old hash library
* They will be removed someday
*/
This has been there for decades, and I just received word that they
cause problems for the dtksh (CDE) developers as dtksh does call
hashlook().
src/cmd/ksh93/sh/name.c:
- Remove 'hashscope', 'hashfree', 'hashname' and 'hashlook'
compatibility overrides.
A memory leak occurred upon leaving a virtual subshell if a
function was defined within it. If this was done more than 32766
(= 2^15-2 = the 'short' max value - 1) times, the shell crashed.
Discussion and reproducer: https://github.com/ksh93/ksh/issues/114
src/cmd/ksh93/sh/subshell.c: table_unset():
- A subshell-defined function was never freed because a broken
check for autoloaded functions (which must not be freed[*]). It
looked for an initial '/' in the canonical path of the script
file that defined the function, but that path is also stored for
regular functions. Now use a check that executes nv_search() in
fpathdict, the same method used in _nv_unset() in name.c for a
regular function unset.
src/cmd/ksh93/bltins/misc.c: b_dot_cmd():
- Fix an additional memory leak introduced in bd88cc7f, that caused
POSIX functions (which are run with b_dot_cmd() like dot scripts)
to leak extra. This fix avoids both the crash fixed there and the
memory leak by introducing a 'tofree' variable remembering the
filename to free. Thanks to Johnothan King for the patch.
src/lib/libast/include/stk.h,
src/lib/libast/misc/stk.c,
src/lib/libast/man/stk.3,
src/lib/libast/man/stak.3:
- Make the stack more resilient by extending the stack reference
counter 'stkref' from (signed) short to unsigned int. On modern
systems with 32-bit ints, this extends the maximum number of
elements on a stack from 2^15-1==32767 to 2^32-1==4294967295.
The ref counter can never be negative, so there is no reason for
signedness. sizeof(int) is defined as the size of a single CPU
word, so this should not affect performance at all.
On a 16-bit system (not that ksh still compiles there), this
doubles the max number of entries to 2^16-1=65535.
src/cmd/ksh93/tests/leaks.sh:
- Add leak regression tests for ksh functions, POSIX functions, dot
scripts run with '.', and dot scripts run with 'source'.
src/cmd/ksh93/tests/path.sh:
- Add an output builtin with a redirect to an autoloaded function
so that a crash[*] is triggered if the check for an autoloaded
function is ever removed from table_unset(), as was done in ksh
93v- (which crashed).
[*] Freeing autoloaded functions after leaving a virtual subshell
causes a crashing bug: https://github.com/att/ast/issues/803
Co-authored-by: Johnothan King <johnothanking@protonmail.com>
Fixes: https://github.com/ksh93/ksh/issues/114
The fix was incomplete because some tests have to unset HISTFILE,
which reverted them to using ~/.sh_history by default.
src/cmd/ksh93/tests/shtests:
- Instead of setting HISTFILE, set HOME to the temporary directory
$tmp, so nothing will write to the real user directory and the
default history file is $tmp/.sh_history.
src/cmd/ksh93/tests/attributes.sh:
- Restore HISTFILE after a test that requires setting HISTFILE=foo.
src/cmd/ksh93/bltins/print.c:
- If in UTF-8 locale, only bother to check for unreserved char if
the character is ASCII (< 128), and write unreserved chars with
a simple stakputc().
Co-authored-by: Martijn Dekker <martijn@inlv.org>
An intermittent crash occurred after running many thousands of
virtual/non-forked subshells. One reproducer is a crash in the
shbench fibonacci.ksh test, as documented here:
https://github.com/ksh-community/shbench/blob/f3d9e134/bench/fibonacci.ksh#L4-L10
The apparent cause was the signed and insufficiently large 'short'
data type of 'curenv' and related variables which wrapped around to
a negative number when overflowing. These IDs are necessary for the
'wait' builtin to obtain the exit status from a background job.
This fix is inspired by a patch based on ksh 93v-:
https://build.opensuse.org/package/view_file/shells/ksh/ksh93-longenv.dif?expand=1https://src.fedoraproject.org/rpms/ksh/blob/f24/f/ksh-20130628-longer.patch
However, we change the type to 'unsigned int' instead of 'long'. On
all remotely modern systems, ints are 32-bit values, and using this
type avoids a performance degradation on 32-bit sytems. Making them
unsigned prevents an overflow to negative values.
src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/include/jobs.h,
src/cmd/ksh93/include/nval.h,
src/cmd/ksh93/include/shell.h:
- Change the types of the static global 'subenv' and the subshell
structure members 'curenv', 'jobenv', 'subenv', 'p_env' and
'subshell' to one consistent type, unsigned int.
src/cmd/ksh93/sh/jobs.c,
src/cmd/ksh93/sh/macro.c:
src/cmd/ksh93/sh/name.c:
src/cmd/ksh93/sh/nvtype.c,
src/cmd/ksh93/sh/subshell.c:
- Updates to match new variable types.
src/cmd/ksh93/tests/subshell.sh:
- Show wrong exit status in message on failure of 'wait' builtin.
As of aa4669ad, astconf("PATH") is implemented as a hardcoded AST
configuration variable that always has a value, instead of one that
falls back on the OS. Its value is now obtained from the OS (with a
fallback) at configure time and not at runtime. This means that any
fallback for astconf("PATH") is now never used.
src/cmd/ksh93/data/msg.c,
src/cmd/ksh93/include/shell.h:
- Remove e_defpath[]. (The path "/bin:/usr/bin:" made no sense as a
default path anyway, as the final empty element is wrong: default
utilities should never be sought in the current working dir.)
src/cmd/ksh93/sh/path.c,
src/lib/libast/path/pathbin.c:
- abort() if astconf("PATH") returns null.
src/lib/libast/comp/conf.tab: PATH:
- If no 'getconf' utility can be found, use a fallback path that
finds more utilities by also searching in 'sbin' directories.
On some systems, this is needed to find chown(1).
src/cmd/ksh93/sh.1:
- Update doc re default path.
Using the bin/shtests -l/--locale option to run the regression
tests in your own locale broke the tests if you're in a locale that
uses ',' as the radix point, like my nl_NL.UTF-8, unless
LC_NUMERIC=C was exported manually. Let's automate that fix.
src/cmd/ksh93/tests/shtests: --locale:
- If LC_ALL was set, copy it to LANG and unset all LC_* vars.
This allows overriding the radix point with LC_NUMERIC if needed.
- If '1.0' is not a valid shell arithmetic expression, export
LC_NUMERIC=C to fix it.
The entity is not valid in XML, only in HTML. Since we must
be compatible with both, it can't be used. Thanks to Andras Farkas
for the bug report.
In addition, the generation of numeric entities for unprintable
characters was only valid while processing UTF-8 text while in a
UTF-8 locale. In all other conditions it produced invalid results.
This is not worth trying to fix.
Discussion:
https://groups.google.com/d/msgid/korn-shell/CAA0nTRta%3DPbOYduyBv%3DXCzumTcUCU8Lki%3DQQf2O8Erk2BFvO1g%40mail.gmail.com
src/cmd/ksh93/bltins/print.c:
- Remove conversion to entity.
- Remove conversion of non-graph characters to numeric entities.
Convert only the 5 semantically meaningful characters: < > & " '
src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/string.c:
- We don't need sh_isprint() in print.c anymore, so turn it back
into a static function.
src/cmd/ksh93/tests/builtins.sh:
- Update and trim regression tests.
The current fix appears to be only partially successful in
eliminating the intermittent crash, and also breaks '-o notify'
during the 60-second $TMOUT grace period. This replaces it.
The root cause appears to be that the state of job control becomes
somehow inconsistent when running external commands in a command
substitution expanded from the $PS1 prompt. The job_unpost() or
(sometimes) the job_list() function intermittently crash. These are
called if the SH_TTYWAIT state is active:
https://github.com/ksh93/ksh/blob/88e8fa67/src/cmd/ksh93/sh/jobs.c#L463-L469
Temporarily deactivating the SSH_TTYWAIT state while expanding
PS{1..4} prompts appears to fix the problem reliably.
It is quite possible that this fix merely masks a bug in the job
control system, but testing has shown that it stops ksh crashing
without side effects, so I'm calling it good for now.
Thanks to Marc Wilson for many hours of persistent testing.
src/cmd/ksh93/sh/jobs.c:
- Revert changes made in 33858689 and e805c7d9.
src/cmd/ksh93/sh/io.c: io_prompt():
- Save SH_TTYWAIT state and turn it off while expanding prompts.
Resolves: https://github.com/ksh93/ksh/issues/103
Resolves: https://github.com/ksh93/ksh/issues/112
This applies a number of fixes to the printf formatting directives
%H and %#H (as well as their equivalents %(html)q and %(url)q):
1. Both formatters have been made multibyte/UTF-8 aware, and no
longer delete multibyte characters. Invalid UTF-8 byte sequences
are rendered as ASCII question marks.
2. %H no longer wrongly encodes spaces as non-breaking spaces
( ) and instead correctly encodes the UTF-8 non-breaking
space as such.
3. %H now converts the single quote (') to '%#39;' instead of
''' which is not a valid entity in all HTML versions.
4. %#H failed to encode some reserved characters (e.g. '?') while
encoding some unreserved ones (e.g. '~'). It now percent-encodes
all characters except those 'unreserved' as per RFC3986 (ASCII
alphanumeric plus -._~).
Prior discussion:
https://groups.google.com/d/msgid/korn-shell/ce8d1467-4a6d-883b-45ad-fc3c7b90e681%40inlv.org
src/cmd/ksh93/include/defs.h:
src/cmd/ksh93/sh/string.c:
- defs.h: If compiling without SHOPT_MULTIBYTE, redefine the
mbwide() macro (which tests if we're in a multibyte locale) as 0.
This lets the compiler optimiser do the work that would otherwise
require a lot of tedious '#if SHOPT_MULTIBYTE' directives.
- string.c: Remove some now-unneeded '#if SHOPT_MULTIBYTE' stuff.
- defs.h, string.c: Rename is_invisible() to sh_isprint(), invert
the boolean return value, and make it an extern for use in
fmthtml() -- see below. If compiling without SHOPT_MULTIBYTE,
simply #define sh_isprint() as equivalent to isprint(3).
- defs.h: Add URI_RFC3986_UNRESERVED macro for fmthtml() containing
the characters "unreserved" for purposes of URI percent-encoding.
src/cmd/ksh93/bltins/print.c: fmthtml():
- Remove kludge that skipped all multibyte characters (!).
- Complete rewrite to implement fixes described above.
- Don't bother with '#if SHOPT_MULTIBYTE' directives (see above).
src/cmd/ksh93/data/builtins.c:
- sh_optprintf[]: %H: Add single quote to encoded chars doc.
- Edit credits and bump version date.
src/cmd/ksh93/tests/builtins.sh:
- Update and tweak old regression tests.
- Add a number of new tests for UTF-8 HTML and URI encoding, which
are only run when running tests in a UTF-8 locale (shtests -u).
Several regression tests invoke an "interactive" shell using 'ksh
-i'. This records all the commands tested in the shell's history
file. By default, that is the user's history file, ~/.sh_history.
As ksh continuously synchronises history among instances, a ksh
user who ran the regression tests ended up with a number of
mysterious extra commands in their command history.
src/cmd/ksh93/tests/shtests:
- Before running any tests, set and export HISTFILE to a new
history file in the temporary files directory.
There are convincing arguments why including '.' and '..' in the
result of pathname expansion is actively harmful. See:
https://www.austingroupbugs.net/view.php?id=1228https://github.com/ksh93/ksh/issues/58#issuecomment-653716846
pdksh, mksh and zsh already skip these special traversal names
in all cases. This commit makes ksh act like these shells.
Since passing '.' and especially '..' as arguments to commands like
'chmod -R' and 'cp -r' may cause harm, this change seems likely to
fix more legacy scripts than it breaks. I'm unaware of anyone ever
having come up with a concrete use case for the old behaviour.
This change also fixes the bug that '.' and '..' failed to be
ignored as documented if FIGNORE is set.
src/lib/libast/misc/glob.c: glob_dir():
- Explicitly skip any matching '.' and '..' in all cases.
src/cmd/ksh93/tests/glob.sh:
- Add test_glob() tests for '*' and '.*'.
src/cmd/ksh93/sh.1: File Name Generation:
- Update to match new behaviour.
Resolves: https://github.com/ksh93/ksh/issues/58
The 'redirect' builtin command did not error out before executing
any valid redirections. For example, 'redirect ls >foo.txt' issued
an "incorrect syntax" error, but still created 'foo.txt' and left
standard output permanently redirected to it.
src/cmd/ksh93/sh/xec.c: sh_exec():
- If we have redirections (io != NULL), and the command is
SYSREDIR, then check for arguments and error out if there are
any, before calling sh_redirect() to execute redirections.
(Note, the other check for arguments in b_exec() in bltins/misc.c
must be kept, as that applies if there are no redirections.)
src/cmd/ksh93/sh/io.c: sh_redirect():
- Edit comments to better explain what the flag values do.
src/cmd/ksh93/bltins/misc.c:
- Add a dummy b_redirect() function declaration "for the dictionary
generator" as has historically been done for other builtins that
share one C function. I'm not sure what that dictionary generator
is supposed to be, but this also improves greppability.
src/cmd/ksh93/data/builtins.c,
src/cmd/ksh93/sh.1:
- Fix misleading "I/O redirection arguments" term. I/O redirections
are not arguments at all; no argument parser ever sees them.
src/cmd/ksh93/tests/io.sh:
- Test both conditions that should make 'redirect' produce an
"incorrect syntax" error.
- Test that any redirections are not executed if erroneous
non-redirection arguments exist.
src/cmd/ksh93/tests/builtins.sh:
- "... should show usage info on unrecognized options" test:
Because 'redirect' now refuses to process redirections on error,
the error message was not captured. The fix is to run the builtin
in a braces block and add the redirection to the block.
The crash in job_list() or job_unpost() could still occur after the
previous patch if a signal was being handled after $TMOUT was
exceeded and the 60-second grace period was entered.
It *should* work to add a general check for !sh_isstate(SH_GRACE).
We know that the SH_GRACE state is set immediately after printing
the 60 second grace period warning message:
https://github.com/ksh93/ksh/blob/9de65210/src/cmd/ksh93/sh/io.c#L1869-L1870
(and that the crashes occur upon re-evaluating the $PS1 prompt
after setting the SH_GRACE state). We know that the SH_GRACE state
is not turned off again until either the user enters a line:
https://github.com/ksh93/ksh/blob/9de65210/src/cmd/ksh93/sh/main.c#L474
or the shell times out after the grace period:
https://github.com/ksh93/ksh/blob/9de65210/src/cmd/ksh93/sh/io.c#L1861
The SH_GRACE state flag is not used or changed in any other context
(verified with grep -rn SH_GRACE src/cmd/ksh93). So, logically,
this should suffice to make sure the crash stays gone.
src/cmd/ksh93/sh/jobs.c: job_reap():
- Do not list jobs when the SH_GRACE state (the 60 second timeout
grace period after TMOUT was exceeded) is active.
- Keep the previous check for job control just to be sure, and
because it makes sense.
Fixes: https://github.com/ksh93/ksh/issues/103 (again)
This variable is like Bash's $BASHPID, but in virtual subshells
it will retain its previous value as virtual subshells don't fork.
Both $BASHPID and ${.sh.pid} are different from $$ as the latter
is only set to the parent shell's process ID (i.e. it isn't set
to the process ID of the current subshell).
src/cmd/ksh93/include/defs.h:
- Add 'current_pid' for storing the current process ID at a valid
memory address.
- Change 'ppid' from 'int32_t' to 'pid_t', as the return value from
'getppid' is of the 'pid_t' data type.
src/cmd/ksh93/data/variables.c,
src/cmd/ksh93/include/variables.h,
src/cmd/ksh93/sh/init.c,
src/cmd/ksh93/sh/xec.c:
- Add the ${.sh.pid} variable as an alternative to $BASHPID.
The process ID is stored in a struct before ${.sh.pid} is set
as environment variables are pointers that must point to a
valid memory address. ${.sh.pid} is updated by the _sh_fork()
function, which is called when ksh forks a new process with
sh_fork() or sh_ntfork().
src/cmd/ksh93/tests/variables.sh:
- Add ${.sh.pid} to the list of special variables and add three
regression tests for ${.sh.pid}.
src/cmd/ksh93/tests/subshell.sh:
- Update the PATH forking regression test to use ${.sh.pid} and
remove the TODO note.
This bug caused an undefined state, which sometimes crashed the
shell in job_list() or job_unpost(), if $PS1 contains a command
substitution running an external command and the '-b'/'-o notify'
shell option is active. So far the only known way to trigger the
crash is by letting $TMOUT time out the interactive shell. See
https://github.com/ksh93/ksh/issues/103 for details.
src/cmd/ksh93/sh/jobs.c: job_reap():
- The check for the SH_NOTIFY option and the SH_TTYWAIT state
before listing jobs was insufficient. Job control is disabled in
command substitutions, so also check that job control is active
before listing jobs.
src/cmd/ksh93/sh.1:
- Fix TMOUT documentation. The 'read' command in fact only times
out when reading from a terminal, just like 'select'. Also
document the extra 60 second grace period when an interactive
shell prompt reads from a terminal.
Fixes: https://github.com/ksh93/ksh/issues/103
The liblist variable needs to be an extern for dtksh to build.
Quote from CDE developer Chase:
we use an old function that no longer appears in kornshell,
sh_getliblist, it seems to be replaced by the function sh_getlib,
which is fine, but it seems to return a "Shbltin_f" type, which I
can't seem to find any information on what it is. We need the void
pointer dlsym provides for some widget init stuff, I tried making
liblist an extern, but it kept giving me an error about libcomp_t
being undefined.
src/cmd/ksh93/bltins/typeset.c,
src/cmd/ksh93/include/shell.h:
- Fix the compiler error reported above by moving the type definition
for Libcomp_t to shell.h.
- Make liblist an extern since findsym.c in dtksh needs it to build.
The old sh_getliblist function doesn't need to be reintroduced
since the only purpose it served was to workaround the problem
of liblist being a static variable. Now that liblist is an extern,
dtksh fsym can use liblist directly to avoid sh_getliblist.
dtksh findsym.c:
https://sourceforge.net/p/cdesktopenv/code/ci/2.3.2/tree/cde/programs/dtksh/findsym.c
This commit fixes two bugs in the generation of $'...' shellquoted
strings:
1. A bug introduced in f9d28935. In UTF-8 locales, a byte that is
invalid in UTF-8, e.g. hex byte 86, would be shellquoted as
\u[86], which is not the same as the correct quoting, \x86.
2. A bug inherited from 93u+. Single bytes (e.g. hex 11) were
always quoted as \x11 and not \x[11], even if a subsequent
character was a hexadecimal digit. However, the parser reads
past two hexadecimal digits, so we got:
$ printf '%q\n' $'\x[11]1'
$'\x111'
$ printf $'\x111' | od -t x1
0000000 c4 91
0000002
After the bug fix, this works correctly:
$ printf '%q\n' $'\x[11]1'
$'\x[11]1'
$ printf $'\x[11]1' | od -t x1
0000000 11 31
0000002
src/cmd/ksh93/sh/string.c: sh_fmtq():
- Make the multibyte code for $'...' more readable, eliminating the
'isbyte' flag.
- When in a multibyte locale, make sure to shellquote both invalid
multibyte characters and unprintable ASCII characters as
hexadecimal bytes (\xNN). This reinstates 93u+ behaviour.
- When quoting bytes, use isxdigit(3) to determine if the next
character is a hex digit, and if so, protect the quoted byte with
square brackets.
src/cmd/ksh93/tests/quoting2.sh:
- Move the 'printf %q' shellquoting regression tests here from
builtins.sh; they test the shellquoting algorithm, not so much
the printf builtin itself.
- Add regression tests for these bugs.
A segfault happens when an array with an unset method
is turned into a multidimensional array. Reproducer:
function foo {
typeset -a a
a.unset() {
print unset
}
a[3][6][11][20]=7
}
foo
src/cmd/ksh93/sh/nvdisc:
- Fix the multidimensional array unset method crash by
checking if np->nvenv is an array, since multidimensional
arrays need to be handled as arrays. This bugfix was
backported from ksh93v- 2013-10-10-alpha.
src/cmd/ksh93/tests/arrays2.sh:
- Add the reproducer as a regression test for the crash
with multidimensional arrays.
Bug report on the old mailing list:
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01195.html
The required longjmp used to terminate scripts was not being run
when over-shifting in a POSIX function with a redirection. This
caused scripts to continue after an error in the shift builtin,
which is incorrect since shift is a special builtin. The
interpreter is sent into an indeterminate state that causes
undefined behavior as well:
$ cat reproducer.ksh
some_func() {
shift 10
}
for i in a b c d e f; do
echo "read $i"
[ "$i" != "c" ] && continue
some_func 2>&1
echo "$i = c"
done
$ ksh ./reproducer.ksh
read a
read b
read c
/tmp/k[2]: shift: 10: bad number
c = c
read d
/tmp/k[2]: shift: 10: bad number
d = c
read e
/tmp/k[2]: shift: 10: bad number
e = c
read f
/tmp/k[2]: shift: 10: bad number
f = c
src/cmd/ksh93/sh/xec.c: sh_exec():
- Do the necessary longjmp needed to terminate the script after
over-shifting in a POSIX function when the function call has a
redirection.
src/cmd/ksh93/tests/functions.sh:
- Add the over-shifting regression test from ksh93v- 2013-10-10-alpha.
Bug report and fix on the old mailing list:
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg00732.html
src/lib/libast/tm/tmxfmt.c:
- Making %l and %k aliases to %_I and %_H caused zero padding with
%0l and %0k to fail. Fix that by fully implementing %l and %k
without 'goto push'. This duplicates code from %I and %H, but it
is necessary for these formats to work correctly when zero padded.
src/cmd/ksh93/tests/builtins.sh:
- Add a regression test for manually specifying blank and zero
padding with sixteen different formats.
Some systems disallow executing files in /tmp and there is nothing
regular users can do about it. The build would fail with a
misleading error message about cc being a cross-compiler.
This commit makes the build system consistently use $TMPDIR with
/tmp as a fallback if that variable is not defined. This allows the
user to use another temporary directory with execute permission.
The error message in bin/package is also extended to signal the
possibility of a noexec temp dir.
It was working on Solaris 11.3, but there were still problems
building on Solaris 11.4 with GCC (as on the evaluation VM
downloaded directly from Oracle):
1. ksh immediately segfaulted. Experimenting with the compiler
flags Oracle uses revealed that we need to define _XPG6 for ksh
not to segfault. Why is a mystery.
2. The default path logic used by 'command -p' and the 'getconf
PATH' builtin command was still broken: the result did not
include any of the /usr/xpg?/bin directories where the standard
POSIX utilities actually live. Testing shows that the result of
the C language probe 'confstr(_CS_PATH,name,length)' is broken
on Solaris (it only yields the paths to the historic
non-standard utilities, defeating the purpose) unless _XPG7 is
defined; but the latter makes ksh segfault again. So another
solution is needed.
src/cmd/INIT/package.sh, bin/package:
- Add another hack to add the -D_XPG6 flag to CCFLAGS if we're
running SunOS aka Solaris. (I've tried to add a 'cc.sol11' script
to src/cmd/INIT/ instead, but for some reason that I just don't
have time to figure out, the INIT system ignores that on Solaris
with gcc, so this is the only way I could come up with. Any
patches for less hacky alternatives would be welcome.)
src/lib/libast/comp/conf.sh:
- Sanitise the code for finding the best 'getconf' utility.
src/lib/libast/comp/conf.tab: PATH:
- Since the C-languge getconf(_CS_PATH,...) is broken on Solaris
11.4, replace the C language probe with a shell script probe that
uses the external 'getconf' utility.
- To avoid ksh overriding the result of this probe with the result
of its own getconf(_CS_PATH,...) call, which would make Solaris
use the wrong value again, specify this as an AST configuration
entry instead of a POSIX entry. This should be good enough for
all systems; the OS 'getconf' utility should be reliable and the
default path value is constant for each OS, so can be hardcoded.
src/cmd/ksh93/tests/builtins.sh:
- Add another 'sleep .1' to the 'sleep -s 31' test as it was still
intermittently failing on Solaris and possibly other systems.
Solaris, Illumos distributions, and NetBSD need LDFLAGS set to link
explicitly to libm, otherwise, due to as-yet unknown reasons, the
src/lib/libdll/features/dll fails to write a valid header file and
compilation fails due to unknown identifiers such as Dllscan_t.
This commit adds the flag on those systems.
NixOS is a Linux distro that uses very different paths from the
usual Unix conventions (though it's POSIX compliant), and the
regression tests still needed a lot of tweaks to be compatible.
src/cmd/INIT/package.sh, bin/package:
- On SunOS (Solaris and illumos distros) and NetBSD, add '-lm' to
LDFLAGS before compiling.
src/cmd/INIT/mamprobe.sh, bin/mamprobe,
src/cmd/INIT/execrate.sh, bin/execrate:
- Instead of only in /bin, /usr/bin, /sbin and /usr/sbin, search
utilities in the path given by the OS 'getconf PATH', and use the
user's original $PATH as a fallback.
src/cmd/ksh93/tests/*.sh:
- Miscellaneous portability fixes, mainly elimination of unportable
hardcoded paths to commands.
- basic.sh: Remove test for 'time' keyword millisecond precision.
It was racy and could fail depending on system and system load.
This fixes 'command -p' for systems where getconf(1) lives
somewhere other than in /bin or /usr/bin, i.e. NixOS.
src/lib/libast/comp/conf.tab:
- To determine the default path value for AST 'getconf PATH' and
'command -p', compile a small C program to get the correct local
default path value (_CS_PATH) from the operating system so it
gets hardcoded in the ksh binary. This eliminates the need to to
invoke 'getconf PATH' to get this value, which fixes a catch-22
problem on systems where getconf(1) exists somewhere other than
/bin or /usr/bin.
A multibyte character immediately following an expansion of a
single-character name, e.g. $1 through $9, $?, $-, etc. was
corrupted when in a UTF-8 locale, e.g.:
$ set -- foo; echo "$1テスト"
foo?スト
Prior discussion:
https://www.mail-archive.com/ast-users@lists.research.att.com/msg01060.htmlhttps://bugzilla.redhat.com/show_bug.cgi?id=1256495
src/cmd/ksh93/sh/macro.c:
- Apply a Red Hat patch by Paulo Andrade that avoids calling
fcmbget() if backtracking more than one byte might be required.
src/cmd/ksh93/tests/basic.c:
- Test "テスト" following expansion of "$1", "$?" and "$#".
Co-authored-by: Martijn Dekker <martijn@inlv.org>
Multidimensional associative arrays are created with an extra array
member named '0', which is set to no value. Reproducer:
$ typeset -A foo
$ typeset -A foo[bar]
$ typeset -p foo
typeset -A foo=([bar]=([0]='') )
The bugfix prevents nv_setarray from creating the extra '[0]' member
when an associative array is empty. This bug was discussed on the old
mailing list:
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01574.html
src/cmd/ksh93/sh/array.c:
- Do not allow the creation of an extra array member when an array
is empty.
src/cmd/ksh93/tests/arrays.sh:
- Add a regression test for creating multidimensional associative
arrays, but use the output from 'typeset -p' instead of fgrep.
When the classic fork/exec mechanism was used (via sh_fork()) to
run an external command from within a non-forking subshell, SIGINT
was blocked until that subshell was exited. If a subsequent loop
was run in the subshell, it became uninterruptible, e.g.:
$ arch/*/bin/ksh -c '(/usr/bin/true; while :; do :; done); exit'
^C^C^C^C^C
src/cmd/ksh93/sh/xec.c:
- sh_fork() did not reset the savesig variable in the parent part
of the fork when running in a virtual subshell. This had the
effect of delaying signal handling until exiting the subshell.
There is no reason for that subshell check that I can discern, so
this removes it.
I've verified that this causes no regression test failures
even when ksh is compiled with -DSHOPT_SPAWN=0 which means the
classic fork/exec mechanism is always used.
Fixes: https://github.com/ksh93/ksh/issues/86
src/cmd/ksh93/tests/builtins.sh:
- Sleep longer after forking a background job to give the OS more
time to launch it; this will hopefully avoid an intermittent
regression test failure on the Github CI runners.
Due to the mysterious workings of vmalloc(3), occasionally a
spurious leak result still showed up. The leak is always smaller
in bytes than the number of test iterations, so it can't be a leak
in the thing tested.
src/cmd/ksh93/tests/leaks.sh:
- Run each test N=512 times.
- Use a 'err_exit_if_leak' function to add a tolerance of N/4 (128)
bytes to each test result check.
Resolves: https://github.com/ksh93/ksh/issues/100
The following is quoted from Marcin Cieślak [*]:
When running under FreeBSD /bin/sh (and not ksh) we get spurious
file named '=' created in the root. This is because the "checksh"
function runs /bin/sh -c '(( .sh.version >= 20111111 ))' which
produces a "=" file with /bin/sh as a side effect.
Fixes https://github.com/ksh93/ksh/issues/13
bin/package,
src/cmd/INIT/package.sh:
- Fix the creation of a spurious '=' file by making sure the shell
has support for (( ... )) expressions.
.gitignore:
- Remove the '=' file entry since it no longer has a purpose.
[*]: https://bsd.network/@saper/103196289917156347
This bugfix is from Marcin Cieślak's fork of the INIT build
system. Before this bugfix, running 'bin/package host cpu'
on FreeBSD would always report one CPU core, even if the CPU
is multi-core:
$ ./bin/package host cpu
1
bin/package,
src/cmd/INIT/package.sh:
- Correctly report the number of CPUs on FreeBSD by using
'sysctl -n hw.ncpu'.
src/cmd/INIT/mamake.c:
- Fix a rare build error by applying Oracle's patch to increase
mamake's buffer size[*]. Description from the original patch:
The build of KornShell might spuriously fail
with the following error.
...
/usr/bin/ksh: line 40: syntax error at line 44: `else unmatched
mamake [lib/libast]: *** exit code 3 making ast.req
mamake: *** exit code 139 making lib/libast
The patch increases the buffer size of mamake to avoid
spurious build failures.
I can't reproduce build error, but this patch should be merged
anyway because OpenSUSE also increases mamake's buffer size
in a patch titled 'workaround-stupid-build-system.diff'[**].
This indicates that the build failure is a heisenbug that can
occur on at least Linux and Solaris.
[*]: 7cad9dae78
[**]: https://build.opensuse.org/package/view_file/shells/ksh/workaround-stupid-build-system.diff?expand=1
src/*/*/Mamfile,
src/lib/libast/Makefile:
- There were a few instances where the CCFLAGS and LDFLAGS were missing
in the Mamfiles and a Makefile. This commit fixes the problem by merging
the changes from Debian's blhc.diff patch:
f8fea737c9/debian/patches/blhc.diff
Related discussion:
https://github.com/ksh93/ksh/issues/95#issuecomment-664010969
src/cmd/ksh93/tests/leaks.sh:
- When ksh is compiled to use the system's malloc(3) instead of AST
vmalloc(3), the vmstate builtin returns either nothing or zero.
Detect this as a regression test failure and refuse to run tests.
- Tweak iterations. Tests don't need 500 or 1000 runs for vmstate.
src/cmd/ksh93/data/builtins.c:
- Do not compile in vmstate builtin when using system's malloc(3).
If an array or upper/lowercase variable was declared with a null
initial value within a virtual/non-forked subshell, like:
( typeset -a foo; ... )
( typeset -A foo; ... )
( typeset -l foo; ... )
( typeset -u foo; ... )
then the type declaration leaked out of the subshell into the
parent shell environment, though without any values that may
subsequently have been assigned.
src/cmd/ksh93/bltins/typeset.c: setall():
- When deciding whether to create a virtual subshell scope for a
variable, use sh_assignok(), which was actually designed for the
purpose, instead of _nv_unset(). This allows getting rid of a
tangled mess of special-casing that never worked quite right.
src/cmd/ksh93/tests/arrays.sh:
- Add regression tests checking that array declarations don't leak
out of virtual subshells.
src/cmd/ksh93/tests/attributes.sh:
- Add regression tests for combining the 'export' and 'readonly'
attributes with every other possible typeset attribute on unset
variables. This also includes a subshell leak test for each one.
Fixes: https://github.com/ksh93/ksh/issues/88
When a builtin is given an unrecognized option, the usage information
for that builtin should be shown as 'Usage: builtin-name options'. The
sleep and suspend builtins were an exception to this. 'suspend' would
not show usage information and sleep wouldn't exit on error:
$ suspend -e
/usr/bin/ksh: suspend: -e: unknown option
$ time sleep -e 1
sleep: -e: unknown option
real 0m1.00s
user 0m0.00s
sys 0m0.00s
src/cmd/ksh93/bltins/sleep.c:
- Show usage information and exit when sleep is given an unknown
option. This bugfix was backported from ksh2020: https://github.com/att/ast/pull/1024
src/cmd/ksh93/bltins/trap.c:
- Use the normal method of parsing options with optget to fix the
suspend builtin's test failure.
src/cmd/ksh93/tests/builtins.sh:
- Add the ksh2020 regression test for getting the usage information
of each builtin. Enable all /opt/ast/bin builtins in a subshell
since those should be tested as well (aside from getconf and uname
because those builtins fallback to the real commands on error).
Add support for multibyte characters to $IFS
This commit fixes BUG_MULTIBIFS, which had two bug reports in the ksh2020 branch.
src/cmd/ksh93/sh/macro.c:
- Backport Eric Scrivner's fix for multibyte IFS characters (slightly modified
for compatibility with C89). Explanation from https://github.com/att/ast/pull/737:
Previously, the varsub method used for the macro expansion of $param, ${param},
and ${param op word} would incorrectly expand the internal field separator (IFS)
if it was a multibyte character. This was due to truncation based on the
incorrect assumption that the IFS would never be larger than a single byte.
This change fixes this issue by carefully tracking the number of bytes that
should be persisted in the IFS case and ensuring that all bytes are written
during expansion and substitution.
Bug report: https://github.com/att/ast/issues/13
- Fixed another bug that caused multibyte characters with the same initial byte
to be treated as the same character by the IFS. This bug was occurring because
the first byte of a multibyte character wasn't being written to the stack when
the IFS delimiter had the same initial byte:
$ IFS=£
$ v='§'
$ set -- $v
$ v="${1-}"
$ echo "$v" | hd # The first byte should be c2, but it isn't due to the bug
00000000 a7 0a |..|
00000002
Bug report: https://github.com/att/ast/issues/1372
src/cmd/ksh93/tests/variables.sh:
- Add (reworked) regression tests from ksh2020 for the multibyte IFS bugs.
- Add a regression test for att/ast#1372 based on the reproducer.
The following explanation is mostly taken from Tomas Klacko's report on
the old mailing list (which also contains a C program reproducer) [*]:
1. When ksh starts a binary, it sets its environment variable "_"
to "*number*/path/to/binary". Where "number" is the pid of the
ksh process.
2. The binary forks and the child executes a suid root shell script
which begins with #!/bin/sh. For this bug to occur, ksh must be /bin/sh.
3. The ksh process interpreting the suid shell script leaves the "_"
variable as not set (nv_getval(L_ARGNOD) returns NULL) because
the "number" from step 1 is not the pid of its parent process.
4-5. Because "_" is not set and the script is suid root, an infinite
loop occurs because when the SHELL environment variable contains
"/bin/sh" pathshell() returns "/bin/sh". This becomes an infinite
loop of /bin/sh /dev/fd/3 executing /bin/sh /dev/fd/3.
src/cmd/ksh93/sh/init.c: get_lastarg():
- Disable the check for if the "number" refers to the process id of
the parent process.
src/cmd/ksh93/sh/main.c: sh_main():
- Prevent an infinite loop when '$_' is not passed in from the environment.
Solaris applies this bugfix to their version of ksh:
https://github.com/oracle/solaris-userland/blob/master/components/ksh93/patches/190-17432413.patch
[*]: https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01680.html
When a command substitution is run on the same line as a here-document,
a syntax error occurs due to a regression introduced in ksh93u+ 2011-04-15:
true << EOF; true $(true)
EOF
syntax error at line 1: `<<EOF' here-document not contained within command substitution
The regression is caused by an error check that was added to make
the following script causes a syntax error (because the here-document
isn't completed inside of the command substitution):
$(true << EOF)
EOF
src/cmd/ksh93/sh/lex.c:
- Only throw an error when a here-document in a command substitution
isn't completed inside of the command substitution.
src/cmd/ksh93/tests/heredoc.sh:
- Add a regression test for running a command substitution on the
same line as a here-document.
- Add a missed regression test for using here-documents in command
substitutions. This is the original bug that was fixed in ksh93u+
2011-04-15 (it is why the error message was added), but a regression
test for here-documents in command substitutions wasn't added in
that version.
This bugfix was backported from ksh93v- 2013-10-10-alpha.
When ksh is compiled with SHOPT_SPAWN (the default), which uses
posix_spawn(3) or vfork(2) (via sh_ntfork()) to launch external
commands, at least two race conditions occur when launching
external commands while job control is active. See:
https://bugs.launchpad.net/ubuntu/+source/ksh/+bug/1887863/comments/3https://www.mail-archive.com/ast-developers@research.att.com/msg00717.html
The basic issue is that this performance optimisation is
incompatible with job control, because it uses a spawning mechanism
that doesn't copy the parent process' memory pages into the child
process, therefore no state that involves memory can be set before
exec-ing the external program. This makes it impossible to
correctly set the terminal's process group ID in the child process,
something that is essential for job control to work.
src/cmd/ksh93/sh/xec.c:
- Use sh_fork() instead of sh_ntfork() if job control is active.
This uses fork(2), which is 30%-ish slower on most sytems, but
allows for correctly setting the terminal process group.
src/cmd/ksh93/tests/basic.sh:
- Add regression test for the race condition reported in #79.
src/cmd/INIT/cc.darwin:
- Remove hardcoded flag to disable SHOPT_SPAWN on the Mac.
It should be safe to use now.
Fixes https://github.com/ksh93/ksh/issues/79
This code has always been completely undocumented since it was
added sometime between 2002 and 2004[*]. No one (including Google)
knows what it's for and no one is likely to find out.
Not only that, it doesn't compile. If SHOPT_AMP is defined, then it
errors out on an undefined function `print_fun` and an undefined
member `shpath` of 'struct Shell_s'. So it's clear that the code
had been abandoned by its authors for some time as of 2012.
src/cmd/ksh93/sh/xec.c:
- Remove vestigial SHOPT_AMP stuff, whatever that was.
[*] Found out by searching multishell ksh93 repo:
https://github.com/multishell/ksh93/
This merges some fixes to support building dtksh with -DBUILD_DTKSH.
These patches were sent through private email from the CDE developer
Chase. The reason these patches were submitted is because Chase wishes
to include ksh in CDE as an up-to-date git submodule. Quote from Chase:
"... my priority is to get your new version into our code as a git
submodule, and do it quickly before our code bases differ too widely."
Link to CDE project for anyone interested:
https://sourceforge.net/projects/cdesktopenv/
Although the patches were privately discussed, there are some public
emails on the CDE mailing list (links shortened due to long URLs):
ksh-chaos thread: https://bit.ly/3hjJ83b
dtksh alias thread: https://bit.ly/3hkzKfJ
The main fix is for suid_exec, which is now told that /usr/dt is a
valid directory to run from via preprocessor flags. A patch for
Shift-JIS was also submitted, but it isn't in this commit because it
isn't an effective fix for the existing Shift-JIS bugs. I will be
giving that patch some more testing.
From: Chase <nicetrynsa@protonmail.ch>
Co-authored by: Johnothan King <johnothanking@protonmail.com>
This applies ksh93-jobs.dif from OpenSUSE. Source:
https://build.opensuse.org/package/show/openSUSE:Leap:42.3:Update/ksh
src/cmd/ksh93/sh/jobs.c:
- jog_init(): Save errno in case close(JOBTTY) fails. If cause of
failure was interruption by a signal (EINTR), repeat close.
- job_kill(): Replace Red Hat fix for #35 with nicer OpenSUSE fix
that doesn't add a goto before declaring variables. Re: ff358f34
A file descriptor (at least 3, can't reproduce for 4 and up) opened
with 'exec' or 'redirect' in a virtual/non-forked subshell survived
that subshell after exiting it:
$ ksh -c '(redirect 3>&1); echo bug >&3'
bug
src/cmd/ksh93/sh/io.c:
- Apply a patch from OpenSUSE (ksh93-redirectleak.dif). Source:
https://build.opensuse.org/package/show/openSUSE:Leap:42.3:Update/ksh
src/cmd/ksh93/tests/io.sh:
- Add regression test.
Thanks to Marc Wilson for flagging this up.
ksh's built-in test, [ and [[ commands treat /dev/fd/* specially:
e.g. 'test /dev/fd/0' returns true even if it doesn't physically
exist, as on e.g. HP-UX. However, external commands need it to
exist physically.
src/cmd/ksh93/tests/subshell.sh:
- To decide whether to run a test with 'tee', use external 'test'
command to check if /dev/stdout and /dev/fd/1 actually exist.
'whence -a' is documented to list all possible interpretations of a
command, but failed to list a built-in command if a shell function
by the same name exists or is marked undefined using 'autoload'.
src/cmd/ksh93/bltins/whence.c: whence():
- Refactor and separate the code for reporting functions and
built-in commands so that both can be reported for one name.
src/cmd/ksh93/data/builtins.c: sh_optwhence[]:
- Correct 'whence --man' to document that:
* 'type' is equivalent to 'whence -v'
* '-a' output is like '-v'
src/cmd/ksh93/tests/builtins.sh:
- Test 'whence -a' with these combinations:
* a function, built-in and external command
* an undefined/autoload function, built-in and external command
Fixes https://github.com/ksh93/ksh/issues/83
$ ksh -c 'whence -a printf'
printf is a shell builtin
printf is /usr/bin/printf
printf is an undefined function
The third line should not appear.
src/cmd/ksh93/bltins/whence.c:
- Remove faulty extra check for undefined (= autoload) functions.
This was already handled earlier, on lines 192-193.
src/cmd/ksh93/tests/builtins.sh:
- Add regression test.
- For previous 'whence -a' test, don't bother with shell function.
Fixes https://github.com/ksh93/ksh/issues/26
Some regression tests have to be run with the -i option, making the
shell behave (mostly) as if it is interactive. This causes ksh to
print a final newline upon EOF (Ctrl+D). This is functional if the
shell is really interactive, i.e. if standard input is on a
terminal and we're not running a shell script: it ensures that a
parent shell's prompt appears on a new line. But for tests like
ksh -i -c 'testcommands'
or
ksh -i <<EOF
testcommands
EOF
it's a minor annoyance. Adding an explicit 'exit' is an effective
workaround, but we might as well fix it.
src/cmd/ksh93/sh/main.c: exfile(): done:
- If shell is "interactive", only print final newline if standard
input is on a terminal and we're not running a -c script.
This commit fixes two different crashes related to kshdb:
- When redirect is given an invalid file descriptor, a segfault
no longer occurs. Reproducer:
$ ksh -c 'redirect 9>&200000000000'
- Fix a crash due to free(3) being used on an invalid pointer.
This can be reproduced with kshdb (commands from att/ast#582):
$ git clone https://github.com/rocky/kshdb.git
$ cd kshdb
$ ksh autogen.sh
$ echo "print hi there" > $HOME/.kshdbrc
$ ./kshdb -L . test/example/dbg-test1.sh
src/cmd/ksh93/bltins/misc.c: b_dot_cmd():
- The string pointed to by shp->st.filename must be able to be
freed from memory with free(3), so duplicate the string with
strdup(3).
src/cmd/ksh93/sh/io.c: sh_redirect():
- Show an error message when a file descriptor is invalid to
fix a memory fault.
Commit 80d9ae2b removed the line that set the NV_EXPORT flag on an
alias when the obsolete ksh88 'alias -x' option was used. But it
turns out that flag actually did something: it caused 'whence -v'
to report the alias as an exported alias -- misleadingly, because
exported aliases have never actually exised in ksh93. Since '-x' no
longer sets that flag, that message is never printed.
src/cmd/ksh93/data/msg.c,
src/cmd/ksh93/include/path.h:
- Remove is_xalias[] = "%s is an exported alias for " message.
src/cmd/ksh93/bltins/whence.c:
- Remove dead code to check for (formerly) exported alias.
My tests with running shbench[*] on ksh binaries compiled by clang
and gcc yielded no performance difference between compiling with
'-O2' and '-Os'. So we might as well reduce ksh's size and memory
footprint by default.
[*] http://fossil.0branch.com/csb/https://github.com/ksh-community/shbench
src/cmd/INIT/make.probe:
- Change default gcc optimisation level from -O2 to -Os.
- Change default non-gcc optimisation level from -O to -Os.
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.
Support for the long-dead 3DFS userland versioning file system was
already removed from ksh93 (although I'd overlooked some minor
things), but libast still supported it. This removes that too.
src/lib/libast/include/fs3d.h,
src/lib/libast/man/fs3d.3,
src/lib/libast/misc/fs3d.c:
- Removed.
bin/package,
src/cmd/INIT/package.sh:
- Remove attempted use of removed vpath builtin.
src/cmd/ksh93/*:
- Remove minor 3dfs vestiges.
src/lib/lib{ast,cmd,coshell}/*:
- Remove code supporting 3dfs.
This commit backports the main changes to sh_delay from ksh93v-
and ksh2020, which fixes the following bugs:
- Microsecond amounts of less than one millisecond are no longer
ignored. The following loop will now take a minimum of one
second to complete:
for ((i = 0; i != 10000; i++)) do
sleep PT100U
done
- 'sleep 30' no longer adds an extra 30 milliseconds to the total
amount of time to sleep. This bug is hard to notice since 30
milliseconds can be considered within the margin of error. The
only reason why longer delays weren't affected is because the old
code masked the bug when the interval is greater than 30 seconds:
else if(n > 30)
{
sleep(n);
t -= n;
}
This caused 'sleep -s' to break with intervals greater than 30
seconds, so an actual fix is used instead of a workaround.
- 'sleep -s' now functions correctly with intervals of more than
30 seconds as the new code doesn't need the old workaround. This
is done by handling '-s' in sh_delay.
src/cmd/ksh93/bltins/sleep.c:
- Remove the replacement for sleep(3) from the sleep builtin.
- Replace the old sh_delay function with the newer one from ksh2020.
The new function uses tvsleep, which uses nanosleep(3) internally.
src/cmd/ksh93/include/shell.h,
src/cmd/ksh93/edit/edit.c,
src/cmd/ksh93/sh/jobs.c,
src/cmd/ksh93/sh/xec.c,
src/cmd/ksh93/shell.3:
- Update sh_delay documentation and usage since the function now
requires two arguments.
src/cmd/ksh93/tests/builtins.sh:
- Add a regression test for 'sleep -s' when the interval is greater
than 30 seconds. The other bugs can't be tested for in a feasible
manner across all systems:
https://github.com/ksh93/ksh/pull/72#issuecomment-657215616
With this change no more preset aliases exist, so the preset alias
tables can be safely removed. All ksh commands can now be used
without 'unalias -a' removing them, even in interactive shells.
Additionally, the history and r commands are no longer limited to
being used in interactive shells.
src/cmd/ksh93/bltins/hist.c:
- Implement the history and r commands as builtins. Also guarantee
lflag is set to one by avoiding 'lflag++'.
src/cmd/ksh93/Makefile,
src/cmd/ksh93/Mamfile,
src/cmd/ksh93/sh/main.c,
src/cmd/ksh93/sh/init.c,
src/cmd/ksh93/data/aliases.c:
- Remove the table of predefined aliases because the last few have
been removed. During init the alias tree is now initialized the
same way as the function tree.
src/cmd/ksh93/bltins/typeset.c:
- Remove the bugfix for unsetting predefined aliases because it is
now a no-op. Aliases are no longer able to have the NV_NOFREE
attribute.
src/cmd/ksh93/tests/alias.sh:
- Remove the regression test for unsetting predefined aliases since
those no longer exist.
src/cmd/ksh93/data/builtins.c:
- Update sh_opthist[] for 'hist --man', etc.
src/cmd/ksh93/sh.1:
- Remove the list of preset aliases since those no longer exist.
- Document history and r as builtins instead of preset aliases.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
These two default aliases are useful on interactive shells. In
scripts, they interfere with possible function or command names.
As of this commit, these final two default aliases are only loaded
for interactive shells, leaving zero default aliases for scripts.
This completes the project to get rid of misguided default aliases.
src/cmd/ksh93/include/shtable.h,
src/cmd/ksh93/data/aliases.c:
src/cmd/ksh93/sh/init.c:
- Add empty alias table shtab_noaliases[] for scripts.
- Rename inittree() to sh_inittree() and make it external.
- nv_init(), sh_reinit(): Initialise empty alias tree for scripts.
src/cmd/ksh93/sh/main.c: sh_main():
- If interactive, reinitialise alias tree for interactive shells.
src/cmd/ksh93/tests/alias.sh:
- To test default alias removal, launch shell with -i.
In a locale other than C/POSIX, ksh produces corrupted usage
messages for alternatives, e.g. this output of 'typeset -\?':
| Usage: typeset [-bflmnprstuxACHS] [-a[type]] [-i[base]] <..CUT..>
| [-T[tname]] [-Z[n]] [name[=value]...]
| Or:[name[=value]...]
| typeset[name[=value]...]
| [[name[=value]...]
| options[name[=value]...]
| ] -f [name...]
Correct output is:
| Usage: typeset [-bflmnprstuxACHS] [-a[type]] [-i[base]] <..CUT..>
| [-T[tname]] [-Z[n]] [name[=value]...]
| Or: typeset [ options ] -f [name...]
Similar corruption occurs in --help and --man output.
This bug is ancient: it's already in ksh 1993-12-28 s+.
ksh2020 has this fixed. A 'git bisect' run pinpointed the fix
to this commit, which fixes the ERROR_translating macro after
removing the AST-specific locale subsystem:
https://github.com/att/ast/commit/4abc061e
But making the same change in ksh 93u+m produced no results
(probably because we have not removed that subsystem).
However, disabling the use of translation macros in optget.sh
altogether (replacing them with dummies that were already coded in
a preprocessor directive fallback for a reduced standalone libast)
turns out to work. It's not as if there is actually any translation
anyway, so this effectively fixes this bug.
The actual cause of this bug remains mysterious, but should be
somewhere in the AST translation and/or locale subsystem.
src/lib/libast/misc/optget.c:
- Use fallback translation macros.
src/cmd/ksh93/tests/builtins.sh:
- Add regression tests for output of -?, --?x, --help and --man
for a usage string with an alternative ("Or:") usage message.
Before the fix, these failed when running the tests in the
C.UTF-8 locale (as in 'bin/shtests -u builtins').
A regression test failure was occurring on FreeBSD for
bin/shtests -u builtins
because UTF-8 characters were wrongly encoded as bytes in the
C.UTF-8 locale. The cause is that iswprint() always returns false
on FreeBSD if the ksh-specific C.UTF-8 locale is active, as the OS
doesn't support it.
That iswprint() call is redundant anyway; the new is_invisible()
function now handles this.
src/cmd/ksh93/sh/string.c: sh_fmtq():
- Remove redundant iswprint() test.
Many terminals (xterm being one example) give the Home and End keys
the escape sequences '^[[H' and '^[[F'. The first sequence is
handled in both editing modes by moving the cursor to start of
line, but ksh ignored the second sequence.
src/cmd/ksh93/edit/emacs.c,
src/cmd/ksh93/edit/vi.c:
- Add case labels for '^[[F' so that in both editing modes the End
key moves the cursor to the end of the line.
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.
Now that we have an iffe feature test for getrusage(3), introduced
in 70fc1da7, the millisecond-precision 'times' command from the
last version of ksh2020 can easily be backported.
src/cmd/ksh93/bltins/misc.c:
- Incorporate ksh2020 'times' command, with a couple of tweaks:
* Use locale's radix point instead of '.'.
* Pad seconds with initial zero if < 10.
src/cmd/ksh93/data/builtins.c:
- Update version date for 'times --man'.
src/cmd/ksh93/tests/builtins.sh:
- Update 'times' test for 3 digits after radix point.
The backported 'time' keyword code introduced a bug (shared by
ksh2020): the $TIMEFORMAT format sequences %0R, %0U and %0S output
a decimal fraction, acting as %1R, %1U and %1S.
A minor ksh2020 behaviour change that was also backported was that
the $TIMEFORMAT formatting no longer errored out on encountering an
invalid identifier, but continued. That behaviour is now reverted.
Neither of these two regressions occurred on older systems that
have to use times(3) instead of getrusage(2) or gettimeofday(2).
This commit also tweaks a regression test so that it doesn't fail
if the old times(3) interface is used.
src/cmd/ksh93/sh/xec.c: p_time():
- (Fix indentation of a for loop.)
- On modern systems, when outputting the result of $TIMEFORMAT
format sequences, only print fraction if precision is nonzero.
- On modern systems, when encountering an invalid format sequence,
abort formatting in the same way as done for old systems.
- On old systems, initialise 'n' in a more readable way when used
as the index for tm[].
src/cmd/ksh93/tests/basic.sh:
- Don't fail, but issue warning on old systems that use times(3).
Otherwise, check milliseconds: with the ksh 'sleep' builtin,
'TIMEFORMAT=%3R; time sleep .002' should always output '0.002'.
- Change regression test for TIMEFORMAT='%0S%' to check for the
correct output, '0%', instead of checking for an error message.
This commit backports the required fixes from ksh2020 for using
millisecond precision with the 'time' keyword. The bugfix refactors
a decent amount of code to rely on the BSD 'timeradd' and
'timersub' macros for calculating the total amount of time elapsed
(as these aren't standard, they are selectively implemented in an
iffe feature test for platforms without them). getrusage(3) is now
preferred since it usually has higher precision than times(3) (the
latter is used as a fallback).
There are three other fixes as well:
src/lib/libast/features/time:
- Test for getrusage with an iffe feature test rather than
assume _sys_times == _lib_getrusage.
src/cmd/ksh93/sh/xec.c:
- A single percent at the end of a format specifier is now
treated as a literal '%' (like in Bash).
- Zero-pad seconds if seconds < 10. This was already done for
the times builtin in commit 5c677a4c, although it wasn't
applied to the time keyword.
- Backport the ksh2020 bugfix for the time keyword by using
timeradd and timersub with gettimeofday (which is used with
a timeofday macro). Prefer getrusage when it is available.
- Allow compiling without the 'timeofday' ifdef for better
portability.
This is the order of priority for getting the elapsed time:
1) getrusage (most precise)
2) times + gettimeofday (best fallback)
3) only times (doesn't support millisecond precision)
This was tested by using debug '#undef' statements in xec.c.
src/cmd/ksh93/features/time:
- Implement feature tests for the 'timeradd' and 'timersub'
macros.
- Do a feature test for getrusage like in the libast time test.
src/cmd/ksh93/tests/basic.sh:
- Add test for millisecond precision.
- Add test for handling of '%' at the end of a format specifier.
- Add test for locale-specific radix point.
'set -b' had no effect; it should cause the shell to notify job
state changes immediately instead of waiting for the next prompt.
This fixes a regression that was introduced in ksh93t 2008-07-25.
The bugfix is from: https://github.com/att/ast/pull/1089
src/cmd/ksh93/sh/jobs.c:
- Save the tty wait state and avoid changing it if TTYWAIT was
already on to avoid breaking 'set -b'.
The last 'sh_offstate' is inside of an '#if' directive because it
is only required when ksh is compiled with SHOPT_COSHELL enabled.
src/cmd/ksh93/tests/pty.sh:
- Add a regression test for 'set -b' in interactive shells.
src/cmd/ksh93/tests/pty.sh:
- init: Remove superfluous lineno=$LINENO assignments. They aren't
needed if we avoid alias expansion on the err_exit function call.
- In the test "vi mode file name completion", append the main
shell's PID to /tmp/fakehome to make a slightly less insecure
temporary directory name. Unfortunately we cannot use $tmp as
that uses $TMPDIR which may cause a false pass. (re: 4cecde1d)
Apparently, on FreeBSD, the stty command does not work correctly
for setting 'erase' or 'kill' on a pty pseudoterminal. I've no
idea whether the bug is in FreeBSD stty or in AST pty, but in any
case, a workaround is needed for the time being.
src/cmd/ksh93/tests/pty.sh:
- Save terminal state on init; set a trap to restore it on exit.
- Issue 'stty erase ^H kill ^X' on the real terminal before
entering pty pseudoterminals.
Resolves#44.
For unknown reasons, the test for a memory leak in 'read -C stat
<<< "$data"' can show an intermittent minor variation in memory
usage when run with shcomp on certain versions of macOS.
The reported variations are 48 bytes or 80 bytes. This is too small
to be the result of an actual memory leak in the tested command;
it is repeated 500 times so that any real leak should show a
difference of at least 500 bytes.
src/cmd/ksh93/tests/leaks.sh:
- Add a tolerance of 128 bytes to get rid of the false failure.
Fixes#70 (hopefully).
This applies ksh-20100621-fdstatus.patch from Red Hat. Not very
much information is available, so this one is more or less taken
on faith. But it seems to make sense on the face of it: calling
sh_fcntl() instead of fcntl(2) directly makes the shell update its
internal file descriptor state more frequently.
It claims to fix Red Hat bug 924440. The report is currently closed
to the public: https://bugzilla.redhat.com/show_bug.cgi?id=924440
However, Kamil Dudka at Red Hat writes:
https://github.com/ksh93/ksh/issues/67#issuecomment-656379993
| Yes, the summary of RHBZ#924440 is "crash in bestreclaim() after
| traversing a memory block with a very large size". We did not have
| any in house reproducer for the bug. The mentioned patch was
| provided and verified by a customer.
...and Marc Wilson dug up a Red Hat erratum containing this info:
https://download.rhn.redhat.com/errata/RHBA-2013-1599.html
| Previously, the ksh shell did not resize the file descriptor list
| every time it was necessary. This could lead to memory corruption
| when several file descriptors were used. As a consequence, ksh
| terminated unexpectedly. This updated version resizes the file
| descriptor list every time it is needed, and ksh no longer
| crashes in the described scenario. (BZ#924440)
No reproducer means no regression test can be added now.
src/cmd/ksh93/sh/io.c,
src/cmd/ksh93/sh/subshell.c,
src/cmd/ksh93/sh/xec.c:
- Change several fcntl(2) calls to sh_fcntl(). This function calls
fcntl(2) and then updates the shell's file descriptor state.
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'.
This fixes an annoying issue in the shell's quoting algorithm
(used for xtrace (set -x), printf %q, and other things) for UTF-8
locales, that caused it to encode perfectly printable UTF-8
characters unnecessarily and inconsistently. For example:
$ (set -x; : 'aeu aéu')
+ : $'aeu a\u[e9]u'
$ (set -x; : 'aéu aeu')
+ : 'aéu aeu'
$ (set -x; : '正常終了 aeu')
+ : '正常終了 aeu'
$ (set -x; : 'aeu 正常終了')
+ : $'aeu \u[6b63]\u[5e38]\u[7d42]\u[4e86]'
This issue was originally reported by lijo george in May 2017:
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01958.html
src/cmd/ksh93/sh/string.c:
- Add is_invisible() function that returns true if a character is a
Unicode invisible (non-graph) character, excluding ASCII space.
Ref.: https://unicode.org/charts/PDF/U2000.pdf
- Use a fallback in is_invisible() if we cannot use the system's
iswprint(3); this is the case for the ksh C.UTF-8 locale if the
OS doesn't support that. Fall back to a hardcoded blacklist of
invisible and control characters and put up with not encoding
nonexistent characters into \u[xxxx] escapes.
Ref.: https://unicode.org/charts/PDF/U2000.pdf
- When deciding whether to switch to $'...' quoting mode (state=2),
use is_invisible() instead of testing for ASCII 0-127 range.
- In $'...' quoting mode, use is_invisible() to decide whether to
encode wide characters into \u[xxxx] escapes.
src/cmd/ksh93/tests/builtins.sh:
- Add regression tests for shellquoting Arabic, Japanese and Latin
UTF-8 characters, to be run only in a UTF-8 locale. The Arabic
sample text[*] contains a couple of direction markers that are
expected to be encoded into \u[xxxx] escapes.
[*] source: https://r12a.github.io/scripts/tutorial/summaries/arabic
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.
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.
There is a bug in path_alias() that may cause a memory leak when
clearing the hash table while setting/restoring PATH.
This applies a fix from Siteshwar Vashist:
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01945.html
Note that, contrary to Siteshwar's analysis linked above, this bug
has nothing directly to do with subshells, forked or otherwise; it
can also be reproduced by temporarily setting PATH for a command,
for example, 'PATH=/dev/null true', and then doing a PATH search.
Modified analysis:
ksh maintains the value of PATH as a linked list. When a local
scope for PATH is created (e.g. in a virtual subshell or when doing
something like PATH=/foo/bar command ...), ksh duplicates PATH by
increasing the refcount for every element in the linked list by
calling the path_dup() and path_alias() functions. However, when
the state of PATH is restored, this refcount is not decreased. Next
time when PATH is reset to a new value, ksh calls the path_delete()
function to delete the linked list that stored the older path. But
the path_delete() function does not free elements whose refcount is
greater than 1, causing a memory leak.
src/cmd/ksh93/sh/path.c: path_alias():
- Decrease refcount and free old item if needed.
(The 'old' variable was already introduced in 99065353, but
its value was never used there; this fixes that as well.)
src/cmd/ksh93/tests/leaks.sh:
- Add regression test. With the bug, setting/restoring PATH
(which clears the hash table) and doing a PATH search 16 times
causes about 1.5 KiB of memory to be leaked.
There is a bug in print_scan() function that may cause ksh to crash
while listing indexed arrays. The crash happens in nv_search() when
called from print_scan().
This applies a fix from Siteshwar Vashist:
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01944.html
src/cmd/ksh93/bltins/typeset.c:
- Call nv_scan() without the NV_IARRAY flag, even for a null scan.
src/cmd/ksh93/tests/arrays.sh:
- Add regression test for 'typeset -a' crash and check output.
There is a bug in sh_eval() that may cause ksh to crash due to a
double free() after sourcing multiple files with '.' or 'source'
if a longjmp is triggered, e.g. by a syntax error.
This applies a fix from Siteshwar Vashist:
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01943.html
src/cmd/ksh93/sh/xec.c: sh_eval():
- Zero file descriptor io_save after closing it. This prevents a
double free() after returning from a longjmp.
src/cmd/ksh93/tests/basic.sh:
- Add reproducer as regression test.
This shows a better layout in '--man' or '--nroff' output.
src/lib/libast/misc/optget.c:
- Incorporate the '--help | --man' addition in the printf format
instead of hardcoding it in the default options string.
This commit changes the behavior of four date formats accepted
by 'printf %()T' because the old behavior is not compatible with
modern implementations of date(1):
- %k and %l now return a blank-padded hour, the former based on a
24-hour clock and the latter a 12-hour clock (these are common
extensions present on Linux and *BSD).
- %f now returns a date with the format '%Y.%m.%d-%H:%M:%S'
(BusyBox extension).
- %q now returns the quarter of the current year (GNU extension).
src/cmd/ksh93/data/builtins.c:
- Copy the date format documentation from date in libcmd to
the printf man page (for documenting 'printf %T').
src/cmd/ksh93/tests/builtins.sh:
- Add four regression tests for the changed date formats.
src/cmd/ksh93/sh.1:
- Remove inaccurate information about the date formats accepted by
printf %T'. The KornShell uses a custom version of strftime(3)
that isn't guaranteed to accepts the same formats as the native
strftime function.
src/lib/libast/tm/tmxfmt.c:
- Change the behavior of %f, %k, %l and %q to the common behavior.
%k and %l are implemented as aliases to %_H and %_I to avoid
duplicating code.
src/lib/libcmd/date.c:
- Update the documentation for the AST date command since it is
also affected by the changes to 'printf %T'.
Fixes#62
Associative arrays weren't being properly freed from memory, which
was causing a memory leak.
This commit incorporates a patch and reproducer/regress test from:
https://www.mail-archive.com/ast-users@lists.research.att.com/msg01016.html
src/cmd/ksh93/sh/name.c:
- Properly free associative arrays from memory in nv_delete().
src/cmd/ksh93/tests/leaks.sh:
- Add regression test.
'ps' does not always give reliable results; on macOS, 'ps' appears
to produce nondeterministic (i.e. randomly varying) results for
'vsz' and 'rss', making it unusable for memory leak tests. See:
https://github.com/ksh93/ksh/pull/64#issuecomment-655094931
and further comments.
So let's compile in the vmstate builtin so that we can make sure to
measure things properly. It also reports bytes instead of 1024-byte
blocks, so smaller leaks can be detected.
To be decided: whether or not to disable the vmstate builtin for
release builds in order to save about 12K in the ksh binary.
src/cmd/ksh93/data/builtins.c:
- Add vmstate to the list of builtins that are compiled in.
src/cmd/ksh93/tests/leaks.sh:
- getmem(): get size using: vmstate --format='%(busy_size)u'
(Using busy_size instead of size seems to make more sense as it
excludes freed blocks. See vmstate --man)
- Introduce a $unit variable for reporting leaks and set it to
'bytes'; this makes it easier to change the unit in future.
- Since the tests are now more sensitive, initialise all variables
before use to avoid false leak detections.
- The last test seemed to need a few more 'read' invocations in
order to get memory usage to a steady state before the test.
The following set of commands can rarely cause a memory fault
when auditing[*] is enabled, although most of the time it will
simply cause ksh to write '(null)' to the auditing file in place
of a tty name:
$ [ -e /etc/ksh_audit ] || echo "/tmp/ksh_auditfile;$(id -u)" | sudo tee /etc/ksh_audit;
$ v=$(ksh 2> /dev/null +o rc -ic $'getopts a:bc: opt --man\nprint $?')
$ cat /tmp/ksh_auditfile
1000;1593599493;(null); getopts a:bc: opt --man
This happens because strdup is used unconditionally on the pointer
returned by 'ttyname', which can be NULL if stderr is closed. This
then causes 'hp->tty' to be set to null, as strdup returns NULL.
See https://github.com/att/ast/issues/1028
src/cmd/ksh93/edit/history.c:
- Make strdup duplicate 'notty' instead of NULL to prevent
crashes.
[*] https://blog.fpmurphy.com/2008/12/ksh93-auditing-and-accounting.html
If the processing of a multibyte character was interrupted in UTF-8
locales, e.g. by reading just one byte of a two-byte character 'ü'
(\303\274) with a command like:
print -nr $'\303\274' | read -n1 g
then the shellquoting algorithm was corrupted in such a way that
the final quote in simple single-quoted string was missing. This
bug may have had other, as yet undiscovered, effects as well. The
problem was with corrupted multibyte character processing and not
with the shell-quoting routine sh_fmtq() itself.
Full trace and discussion at: https://github.com/ksh93/ksh/issues/5
(which is also an attempt to begin to understand the esoteric
workings of the libast mb* macros that process UTF-8 characters).
src/lib/libast/comp/setlocale.c: utf8_mbtowc():
- If called from the mbinit() macro (i.e. if both pointer
parameters are null), reset the global multibyte character
synchronisation state variable. This fixes the problem with
interrupted processing leaving an inconsistent state, provided
that mbinit() is called before processing multibyte characters
(which it is, in most (?) places that do this). Before this fix,
calling mbinit() in UTF-8 locales was a no-op.
src/cmd/ksh93/sh/string.c: sh_fmtq():
- Call mbinit() before potentially processing multibyte characters.
Testing suggests that this could be superfluous, but at worst,
it's harmless; better be sure.
src/cmd/ksh93/tests/builtins.sh:
- Add regression test for shellquoting with 'printf %q' after
interrupting the processing of a multibyte characeter with
'read -n1'. This test only fails in a UTF-8 locale, e.g. when
running: bin/shtests -u builtins SHELL=/buggy/ksh-2012-08-01
Fixes#5.
src/cmd/ksh93/data/signals.c:
- SIGINFO was absent from the table of signals, which caused
commands like 'kill -INFO $$' to fail even on platforms with
SIGINFO (such as macOS and FreeBSD). Fix that by adding
it to the signal table.
src/cmd/ksh93/tests/signal.sh:
- Add a regression tests for using SIGINFO with the kill builtin.
The test will only be run if the external kill command supports
SIGINFO.
This gets rid of repetitive code in test scripts to create their
own temporary directories. Instead, shtests exports a $tmp to each
test script that is a subdirectory of its own temporary directory.
This has the advantage of having all test script temporary
directories in one hierarchy. Along with a new option to keep
temporary files, this makes it easy to inspect them if wanted.
This does make the test scripts less self-contained as they now
depend on a temporary directory being exported as $tmp. But they
already depended on $SHELL being the shell to test, so they already
were not quite self-contained.
src/cmd/ksh93/tests/shtests:
- Add -k/--keep option to keep temporary directory. Make the EXIT
trap report its location instead of deleting it.
- For each test, create a subdirectory of $tmp (named after the
test script plus the tested locale or 'shcomp') and export that
subdirectory to the test script as its own $tmp.
- If -k is not given, delete each script's temporary files
immediately after running it to minimise disk usage.
src/cmd/ksh93/tests/*.sh:
- Don't make own temp directory.
- Refuse to run if $tmp is not set.
- Miscellaneous tweaks.
src/cmd/ksh93/tests/pty.sh:
- Fix race condition in the test "raw Bourne mode literal tab
characters with wide characters enabled" by adding 'd 10' to add
a 10-millisecond delay before every write. Thanks to @JohnoKing:
https://github.com/ksh93/ksh/pull/57#issuecomment-653617531
- Fix locale for test "raw Bourne mode backslash handling" (should
be UTF-8, not UTF8) (re: a0dcdeea).
- Add a few more dummy # err_exit # comments to allow shtests to
count the number of tests.
This commit fixes the following bugs in the 'vi' editing mode
backslash escape feature. Ref.: Bolsky & Korn (1995), p. 113, which
states for \: "Similar to Control+V [...] except that it escapes
only the next Erase or Kill charactrer".
1. The vi mode now only escapes the next character if the last
character input was a backslash, fixing the bug demonstrated at:
https://asciinema.org/a/E3Rq3et07MMQG5BaF7vkXQTg0
2. Escaping backslashes are now disabled in vi.c if the vi mode is
disabled (note that vi.c handles raw editing mode in UTF-8
locales). This makes the behavior of the raw editing mode
consistent in C/POSIX and UTF-8 locales.
3. An odd interaction with Backspace when the character prior to a
separate buffer entered with Shift-C was a backslash has been
fixed. Demonstration at: https://asciinema.org/a/314833
^? will no longer be output repeatedly when attempting to erase
a separate buffer with a Backspace, although, to be consistent
with vi(1), you still cannot backspace past it before escaping
out of it. Ref.:
https://github.com/ksh93/ksh/issues/56#issuecomment-653586994
src/cmd/ksh93/edit/vi.c:
- Prevent a backslash from escaping the next input if the previous
input wasn't a backslash. This is done by unsetting a variable
named backslash if a backslash escaped a character. backslash is
set to the result of c == '\\' when the user enters a new
character.
- Disable escaping backslashes in the raw editing mode because
it should not be enabled there.
src/cmd/ksh93/tests/pty.sh:
- Add some tests for how ksh handles backslashes in each
editing mode to test for the bugs fixed by this commit.
Fixes#56.
ksh crashed if it encountered a .paths directory in any of the
directories in $PATH.
Ref: https://bugs.launchpad.net/ubuntu/+source/ksh/+bug/1534855
src/cmd/ksh93/sh/path.c: path_chkpaths():
- Refuse to read .paths if it's not a regular file
or a symlink to a regular file.
In <https://github.com/att/ast/commit/d2771913>, GCC version 10 was
specifically special-cased for skipping the -nostartfiles flag
along with versions 7, 8, and 9. It seems more future-proof to
specifically include it for versions up to 6 and remove it for any
version 7 and up.
src/cmd/INIT/make.probe:
- Remove the -nostartfiles for all version of gcc > 7.
Regular expressions that combine a repetition expression with
a parenthesized sub-expression throw a garbled syntax error:
$ [[ AATAAT =~ (AAT){2} ]]
ksh: syntax error: `~(E)(AAT){2} ]]
:'%Cred%h%Creseksh: syntax error: `~(E)(AAT){2} ]]
:'%Cred%h%Creseksh: syntax' unexpected
The syntax error occurs because ksh is not fully
accounting for '=~' when it runs into a curly bracket.
This fix disables the syntax error when the operator
is '=~' and adds handling for '(str){x}' (to allow for
more than one sub-expression). This bugfix and the
regression tests for it were backported from ksh93v-
2014-12-24-beta.
src/cmd/ksh93/sh/lex.c:
- Do not trigger a syntax error for '{x}' when the operator
is '=~' and add handling for multiple parentheses when
combined with '{x}'.
src/cmd/ksh93/tests/bracket.sh:
- Add two tests from ksh93v- to test sub-expressions
combined with the '{x}' quantifier.
src/cmd/ksh93/tests/leaks.sh:
- This script was never actually running the regression
tests because 'vmstate' isn't available as a builtin.
While this can be fixed by adding vmstate to the builtin
table, that has the downside of increasing the binary
size of ksh. This commit replaces all usage of 'vmstate'
with 'ps' and 'awk' as a different way to measure
memory usage. The memory leaks regression tests are now
always run.
- Rename old $n to $N due to new $n interfering with the old
regression test.
- Add before and after results for the number of 1024-byte
blocks leaked in each test.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
File descriptors that are too far out of range will cause the
read builtin to crash. The following example will generate
two crashes:
$ ksh -c 'read -u 2000000' || ksh -c 'read -u-2000000'
The fix is to error out when the given file descriptor is out
of range. This bugfix is from Tomas Klacko, although it was
modified to use 'sh_iovalidfd' and reject numbers greater
than 'INT_MAX':
https://www.mail-archive.com/ast-developers@lists.research.att.com/msg01912.html
The question about 'shp->fdstatus[-1]' only applies to ksh93v-
(ksh93u+ doesn't have any references to 'shp->fdstatus[-1]').
src/cmd/ksh93/bltins/read.c:
- File descriptors that are out of range should be rejected
with an error message (like invalid file descriptors that
are in range). The seemingly redundant check for negative
numbers is there because out of range negative numbers also
cause memory faults despite the later 'fd<0' check.
src/cmd/ksh93/tests/io.sh:
- Add three tests for attempting 'read -u' on various invalid
file descriptor numbers.
Not cleaning up the FIFO broke 'grep -r' in the arch directory,
making it hang forever. So we need a better way of cleaning it up.
bin/package,
src/cmd/INIT/package.sh:
- Unlink the FIFO early after sleeping a second in the background.
This works because the named directory entry is only needed to
establish the pipe, not to keep it going.
To get ksh to prefer UTC over GMT in 'printf %T' output, only the
change in format[] was needed. The corresponding change in zone[]
made it prefer UTC for London time, even in summer time, which is
wrong -- e.g.:
$ LANG=C TZ=Europe/London arch/*/bin/ksh -c 'date; printf %T\\n now'
Tue Jun 30 01:39:09 BST 2020
Tue Jun 30 00:39:09 UTC 2020
src/lib/libast/tm/tmdata.c:
- Revert change in zone[].
The regression test failed on systems where 'chmod' exists at more
than one location, e.g. Slackware where it's at both /bin/chmod and
/usr/bin/chmod.
src/cmd/ksh93/tests/builtins.sh: 'whence -a'/tracked aliases test:
- In the expected value, use modified 'whence -a -p chmod' output
to get all of the paths to chmod.
- On failure, report both expected and actual values.
* Fix the readonly builtin's scope in functions
This bug was first reported at https://github.com/att/ast/issues/881
'tdata.sh->prefix' is only set to the correct value when
'b_readonly' is called as 'export', which breaks 'readonly' in
functions because the correct scope isn't set. As a result, the
following example will only print a newline:
$ function show_bar { readonly foo=bar; echo $foo; }; show_bar
The fix is to move the required code out of the if statement for
'export', as it needs to be run for 'readonly' as well. This bugfix
is from https://github.com/att/ast/pull/906
src/cmd/ksh93/bltins/typeset.c:
- Set 'tdata.sh->prefix' to the correct value, otherwise 'readonly'
uses the wrong scope.
src/cmd/ksh93/tests/builtins.sh:
- Add the regression test from ksh2020, modified to run in a
subshell.
src/cmd/ksh93/data/builtins.c,
src/cmd/ksh93/sh.1:
- Add documentation of 'readonly' vs. 'typeset -r' difference:
'readonly' does not create a function-local scope.
Co-authored-by: Martijn Dekker <martijn@inlv.org>
The following set of commands ends with a memory fault under
certain circumstances because ksh attempts to free memory
twice, causing memory corruption:
$ testarray=(1 2)
$ compound testarray
$ unset testarray
$ eval testarray=
The fix is to make sure 'np->nvfun' is a valid pointer before
attempting to free memory in 'put_tree'. This patch is from
OpenSUSE: https://build.opensuse.org/package/view_file/shells/ksh/ksh93-nvtree-free.dif?expand=1
src/cmd/ksh93/sh/nvtree.c:
- Do not try to free memory when 'np->nvfun' and 'val'
are false.
src/cmd/ksh93/tests/comvar.sh:
- Add a regression test for the double free problem. The
reproducer must be run from an executable script
with 'ksh -c'.
Variables created with 'typeset -RF' were being treated as
short integers, even though they are actually floating point
values. As a result the following example will cause a crash:
$ typeset -RF foo=1
$ test "$foo"
This is fixed by checking for 'NV_DOUBLE' with 'nv_isattr',
which prevents ksh from treating floating point values as
short integers due to '== NV_INT16P' excluding 'NV_DOUBLE'.
This bugfix was backported from ksh93v- 2013-10-10-alpha.
src/cmd/ksh93/sh/array.c,
src/cmd/ksh93/sh/name.c,
src/cmd/ksh93/sh/nvdisc:
- Avoid treating floating point values as short integers by
checking for 'NV_DOUBLE' with 'nv_isattr'.
src/cmd/ksh93/tests/types.sh:
- Add a regression test for the 'typeset -RF' crash. The
crash cannot be replicated if 'typeset -RF' sets 'foo'
to zero.
ksh, even non-interactive, loads /etc/ksh.kshrc by default. On
some systems this can be a problem, e.g. OpenBSD, which installs a
default /etc/ksh.kshrc which is designed for its version of pdksh.
Quoth sh.1:
On systems that support a system wide /etc/ksh.kshrc
initialization file, if the filename generated by the expansion
of ENV begins with /./ or ././ the system wide initialization
file will not be executed.
src/cmd/ksh93/tests/shtests,
src/cmd/ksh93/tests/builtins.sh,
src/cmd/ksh93/tests/options.sh,
src/cmd/ksh93/tests/pty.sh:
- Instead of emptying or unsetting ENV, ensure it is exported with
a default value of /./dev/null so we skip loading the system-wide
profile and load an empty user profile.
- Where a specific ENV path was required for the tests, prefix it
with '/.' so it starts with '/./'.
The cd builtin was removing '.' from directory names when combined
with a preceding '../', which caused commands like 'cd ../.local'
to become 'cd ../local'. This patch fixes the problem by limiting
the extra handling to leading '..'. The bugfix comes from ksh93v-
2013-10-10-alpha, although this version is a shortened patch from
Solaris (as ksh93v- refactored a decent amount of the code for the
cd builtin).
src/cmd/ksh93/bltins/cd_pwd.c:
- cd should only check for leading '..', as trying to handle a lone
'.' only causes problems.
src/cmd/ksh93/tests/builtins.sh:
- Add a regression test for this problem based on the test present in
ksh93v- 2013-10-10-alpha.
Patch from Solaris:
https://github.com/oracle/solaris-userland/blob/860d27f/components/ksh93/patches/270-23319761.patch
Apparently some systems are still configured to use GMT instead of
UTC after all. This included our own GitHub CI runner config.
Oops. This made the previous commit fail to pass the CI test run.
We can't win this one, it's got to be either one or the other.
UTC is the international standard on which civil time is based.
GMT is often taken as synonymous for UTC, but in navigation,
it can differ from UTC by up to 0.9 seconds. Ref.:
https://en.wikipedia.org/w/index.php?title=Greenwich_Mean_Time&oldid=963422787
The more ambiguous term should not be the first preference.
src/cmd/ksh93/tests/builtins.sh:
- Before checking 'printf %T now' output against 'date' output,
change any ' GMT ' in the latter to ' UTC '.
.github/workflows/ci.yml:
- Set time zone to UTC, not GMT.
"UTC" is the modern name for what used to be "GMT", but ksh still
preferred GMT. On systems configured to use the UTC time zone, this
caused a 'printf %T' regression test failure in tests/builtins.sh
as the external 'data' utility will prefer UTC these days.
src/lib/libast/tm/tmdata.c:
- Reorder the name alternatives for UTC/GMT so that UTC is
the first preference.
src/cmd/ksh93/tests/builtins.sh:
- Report expected and actual values on 'printf %T' failure.
Related: #6
When neither '-o emacs' nor '-o vi' is active, there were a couple
of bugs with entering tab characters:
1. Tab completion was erroneously left active. The cause of this
was that raw Bourne edit mode is handled by ed_viread() in vi.c
on shells with wide character support, instead of the default
ed_read() in edit.c, and the former failed to check if vi mode
is active when processing tab characters.
2. When entering literal tab characters, the cursor was moved to
the right only one character, instead of the amount of
characters corresponding to the tab.
src/cmd/ksh93/edit/vi.c: getline():
- Before processing '\t' (tab) for command completion, check that
the 'vi' shell option (SH_VI) is active.
src/cmd/ksh93/edit/edit.c: ed_virt_to_phys():
- When translating literal tabs to on-terminal spaces and when
recalculating the cursor position, remove erroneous checks for
SH_VI; this is also needed in raw Bourne mode. According to my
own testing, this has no effect on emacs mode (knock on wood).
src/cmd/ksh93/tests/pty.sh:
- Add two regression tests. An odd race condition reveals itself in
either pty or in ksh's raw/Bourne edit mode; see comment in test.
Effect is we have to expect either literal tabs or tabs expanded
to spaces, until that is tracked down and fixed.
Fixes#43.
This commit fixes the bug reported in:
https://github.com/att/ast/issues/682
The following sequence fails in vi mode because ksh looks in the
wrong part of the 'virtual' buffer:
$ touch ~/testfile
$ ls ~/test<tab>
The fix is to change 'virtual[i]' to 'virtual[last_virt]' in the
bugged section of code. The other changes are to make sure listing
files in a directory with something like 'ls /etc/<tab>' calls the
code for Ctrl+L to preserve 'ls /etc/' rather than try (and fail)
to complete the directory name, producing 'ls /etc\n/'. This bugfix
was backported from ksh93v- 2013-10-10-alpha.
src/cmd/ksh93/edit/vi.c
- Backport the bugfix from ksh93v- 2013-10-10-alpha for this
problem.
src/cmd/ksh93/tests/pty.sh
- Add a regression test for this issue using pty, adjusted slightly
for a fake home directory in /tmp.
Somewhat notable changes in this commit:
- The 'set +r' bugfix (re: 74b41621) is now documented in the
changelog.
- Missing options have been added to the synopsis section of the
ksh man page.
- The minor formatting fix from https://github.com/ksh-community/ksh/pull/5
has been applied to the ksh man page.
- A few fixes from https://github.com/att/ast/commit/5e747cfb
have been applied to the ksh man page.
- The man page fixes from https://github.com/att/ast/pull/353
have been applied, being:
- An addition to document the behavior of 'set -H'.
- A fix for the cd section appending rksh93.
- A fix for some options being indented too far.
- Removal of a duplicate section documenting '-D'.
- Reordering the options for 'set' in alphabetical order.
- A minor fix for the documentation of 'ksh -i'.
src/cmd/ksh93/tests/bracket.sh:
- Disable tests for [[ -N ... ]] (test -N ...), because it is
expected to break on systems where $TMPDIR (or even the entire
root file system) is mounted with noatime for better performance.
Ref.: https://opensource.com/article/20/6/linux-noatime
(It also needs annoyingly long sleep times on older systems with
a 1-second timestamp granularity.)
Testing the behaviour of an external editor, even the standard one,
is outside the scope of the ksh regression tests.
src/cmd/ksh93/tests/pty.sh:
- Disable a test that invoked vi(1) and that failed, either
intermittently or consistently, on too many systems because
whatever vi(1) is installed locally doesn't write the string
"/tmp/" exactly as and/or when expected.
The output format is now identical to mksh's except for
the locale-dependent radix point ('.' or ',').
src/cmd/ksh93/bltins/misc.c:
- Output format tweak: pad seconds with initial zero if < 10.
- Use "too many operands" (e_toomanyops) error msg from 3ba4900e
if there are operands, instead of "bad syntax" (e_badsyntax).
- Consolidate repetitive calculating and printing code
into print_times().
- Get rid of some excessive variables.
src/cmd/ksh93/tests/builtins.sh:
- Update regression tests to match the above.
src/cmd/ksh93/data/builtins.c:
- Update sh_opttimes[] version string.
Well, that's what I get for backporting code without properly
checking it over. There was an elementary math error in how the
times builtin calculated seconds:
utime_sec = utime - utime_min;
which could cause output such as "1m98.38s" or "3m234.77s".
src/cmd/ksh93/bltins/misc.c: b_times():
- Use fmod(), i.e. floating point modulus, to calculate seconds.
When a nonexistent test script was given as an argument to
shtests, this was not counted as an error and shtests exited
successfully (with status 0).
src/cmd/ksh93/tests/shtests:
- Increase total error count if a test script is not found.
The code for handling process substitution with redirection was
never being run because IORAW is usually set when IOPROCSUB is
set. This commit fixes the problem by moving the required code
out of the !IORAW if statement. The following command now prints
'good' instead of writing 'ok' to a bizzare file:
$ ksh -c 'echo ok > >(sed s/ok/good/); wait'
good
This commit also fixes a bug that caused the process ID of the
asynchronous process to print when the shell was in interactive
mode. The following command no longer prints a process ID,
behaving like in Bash and zsh:
$ echo >(true)
/dev/fd/5
src/cmd/ksh93/sh/args.c:
- Temporarily turn off the interactive state while in a process
substitution to prevent the shell from printing the PID of
the asynchronous process.
src/cmd/ksh93/sh/io.c:
- Move the code for process substitution with redirection into
a separate if statement.
src/cmd/ksh93/tests/io.sh:
- Add two tests for both process substitution bugs fixed by this
commit.
src/cmd/ksh93/tests/shtests:
- Update shtests with a patch from Martijn Dekker to use
pretty-printing for the output from the times builtin (if it
is available).
Fixes#2
This commit fixes the bug described in att/ast#32. The fix and
following explanation is from att/ast#467:
While copying variables from function's local scope to a new scope,
variable attributes were not copied. Such variables were not marked
to be exported in the new function. For e.g.
function f2 { env | grep -i "^foo"; }
function f1 { env | grep -i "^foo"; f2; }
foo=bar f1
prints 'foo=bar' only once, but it should print be twice.
src/cmd/ksh93/sh/xec.c:
- When variables from the local scope of a function are copied into
the scope of a nested function, the attributes of the variables
need to be copied as well.
src/cmd/ksh93/tests/functions.sh:
- Add regression tests from ksh2020 to check environment variables
passed to functions.
src/cmd/ksh93/tests/builtins.sh:
- The output of 'printf %T now' and the external 'date'
command aren't guaranteed to be the same unless $LC_ALL
is set to 'C'. Set LC_ALL in these command substitutions
to fix a spurious test failure on Linux.
Ksh was trying to use the 'pw' variable as a valid pointer even
when it was NULL. This is fixed by doing the error check for
'pw' before doing anything else in 'job_kill'.
This bugfix is from Red Hat:
44e0a643a9/f/SOURCES/ksh-20130214-fixkill.patchFixes#34
The 'stop' and 'suspend' default aliases are now converted into
regular built-in commands so that 'unalias -a' does not remove
them, 'suspend' can do some sanity checks, and something like
cmd=stop; $cmd $!
will now work.
src/cmd/ksh93/bltins/trap.c:
- b_kill(): Incorporate 'stop' functionality, which is simply
setting the same flag and variable as '-s STOP' would have done.
- b_suspend(): Add simple builtin function that sends SIGSTOP to
the main shell. Check for no operands, and refuse to suspend a
login shell (which would leave the user stuck with no way out).
Also check that 'kill' succeeds; if we're in an asynchronous
subshell, it is possible the main shell no longer exists.
src/cmd/ksh93/data/aliases.c:
- Remove "stop" and "suspend" default aliases. (Why were these
conditional upon SIGTSTP when they actually issued SIGSTOP?)
src/cmd/ksh93/include/builtins.h,
src/cmd/ksh93/data/builtins.c,
src/cmd/ksh93/data/msg.c:
- Add declarations of "stop" and "suspend" regular built-ins.
- Add option strings (AST manual/--man pages) for them.
- Add e_toomanyops ("too many operands") reusable error message for
b_suspend(). Other new commands may want this at some point.
src/cmd/ksh93/sh.1:
- Remove "stop" and "suspend" default aliases.
- Document "stop" and "suspend" regular built-in commands.
This appears to be originating from:
2755 *) if test ! -d $INSTALLROOT
2756 then INSTALLROOT=$PACKAGEROOT;
where INSTALLROOT=PACKAGEROOT and 'clean' deletes everything under
INSTALLROOT thus deleting the entire git repo. This only applies when
there's no arch/$HOSTTYPE directory due to the condition above.
bin/package,
src/cmd/INIT/package.sh:
- Delete arch/$HOSTTYPE as stated in the documentation
for the clean action instead of $INSTALLROOT.
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.
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.
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
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.
'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.
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.
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).
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: #23Reopens: #21
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
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.
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.
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.
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.
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-.
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; }.
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.
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.
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.
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.
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#c14https://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.
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().
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.
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.
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
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).
'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.
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.
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.
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.
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.
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.
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.
src/cmd/ksh93/tests/io.sh:
- 16 tests for the 'redirect' builtin used the ((arithmetic
command)) to check the result. This does not tolerate the result
being a non-number, such as the empty string, which may occur on
test failure. So use [[ ... -eq ArithExpression ]] to check these
results instead; it simply returns false, failing gracefully.
This commit removes the undocumented 'login' and 'newgrp' builtin
commands. They already stopped blocking shell functions by that
name by changing from special to regular builtins in 04b91718 (a
change I forgot to mention in that commit message), but there is
another obnoxious aspect to these: being glorified hooks into
'exec', they replaced your shell session with the external commands
by the same name. This makes argument and error checking
impossible, so if you made so much as a typo, you would be
immediately logged out.
Even if that behaviour is wanted by a few, having it as the default
is user-hostile enough to be called a bug. It also violates the
POSIX definition of the 'newgrp' utility which explicitly says that
it "shall create a new shell execution environment", not replace
the existing one.
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/newgrp.html
Users who do want this behaviour can easily restore it by setting:
alias login='exec login'
alias newgrp='exec newgrp'
src/cmd/ksh93/bltins/misc.c:
- As there is no more 'login' builtin, combine b_exec() and
B_login() functions, which allows eliminating a few variables.
Note that most of 'exec' was actually implemented in B_login()!
src/cmd/ksh93/data/builtins.c:
- Remove "login" and "newgrp" table entries.
src/cmd/ksh93/include/builtins.h:
- Remove SYSLOGIN parser ID. As this was the first, all the others
needed renumbering.
src/cmd/ksh93/sh/xec.c:
- Remove SYSLOGIN parser check that made 'login' and 'newgrp' act
like 'exec' and replace the shell.
This commit converts the redirect='command exec' alias to a regular
'redirect' builtin command that only accepts I/O redirections, which
persist as in 'exec'. This means that:
* 'unlias -a' no longer removes the 'redirect' command;
* users no longer accidentally get logged out of their shells if
they type something intuitive but wrong, like 'redirect ls >file'.
This should not introduce any legitimate change in behaviour. If
someone did accidentally pass non-redirection arguments to
'redirect', unexpected behaviour would occur; this now produces
an 'incorrect syntax' error.
src/cmd/ksh93/bltins/misc.c: b_exec():
- Recognise 'redirect' when parsing options.
- If invoked as 'redirect', produce error if there are arguments.
src/cmd/ksh93/data/aliases.c:
- Remove redirect='command exec' alias.
src/cmd/ksh93/data/builtins.c:
- Update/improve comments re ordering.
- Add 'redirect' builtin entry.
- sh_optexec[]: Abbreviate redirection-related documentation;
refer to redirect(1) instead.
- sh_optredirect[]: Add documentation.
src/cmd/ksh93/include/builtins.h:
- Add SYSREDIR parser ID, renumbering those following it.
- Improve comments.
- Add extern sh_optredirect[].
src/cmd/ksh93/sh.1:
- exec: Abbreviate redirection-related documentation; refer to
'redirect' instead.
- redirect: Add documentation.
src/cmd/ksh93/sh/xec.c:
- Recognise SYSREDIR parser ID in addition to SYSEXEC when
determining whether to make redirections persistent.
src/cmd/ksh93/tests/io.sh:
- To regress-test the new builtin, change most 'command exec' uses
to 'redirect'.
- Add tests verifying the exit behaviour of 'exec', 'command exec',
'redirect' on redirections.
The exit status fix introduced in cherry-picked commit 22c3a6e1
introduced a problem: interrupting the build only interrupted the
main process, and the background job would happily continue.
bin/package, src/cmd/INIT/package.sh:
- Give up on the idea of a background job and put up with not
cleaning up the FIFO. It's in a build directory that is going to
get cleaned up anyway.
src/cmd/INIT/cc.darwin*:
- Build on ancient Mac OS X gcc versions (darwin 7+, Mac OS X
10.3+), more recent gcc versions (darwin 11+, Mac OS X 10.7+), as
well as current clang versions. So there are now three cc.darwin*
wrapper scripts for these different darwin versions.
bin/package, src/cmd/INIT/package.sh:
- Differentiate the host ID and pick the correct Darwin script based on
the detected OS release version.
Both 'alias' and 'unalias' are specified as regular builtins. Among
a few other things, that means it ought to be portable to use these
names for shell functions. But ksh93 disallowed that until now.
src/cmd/ksh93/data/builtins.c:
- Make 'unalias' a regular builtin by removing the BLT_SPC flag.
- (same fix for 'alias' was already done in afa68dca)
- Add the BLT_ENV flag to the 'alias' and 'hash' commands. In
include/name.h, this flag is commented: "non-stoppable, can
modify environment". The "non-stoppable" bit seems like a good
idea: these operations should not be interruptable as that would
cause an inconsistent state.
src/cmd/ksh93/sh.1:
- Remove the '-', indicating special builtin, from 'alias' entry.
- Minor cosmetic fix: space after the '-' for 'unset'.
(cherry picked from commit a4315d7672204acb543010b4d4916b22dcb9cb08)
The b_hash() function duplicated much of its code from b_alias(),
while b_alias() retained some code to support being called as
'hash'. There is no reason why 'hash' and 'alias' can't be handled
with a single function, as is the case several other builtins. Note
that option parsing can easily be made dependent on the name the
command was invoked with (in this case, argv[0]=='h').
The new hash builtin's -r option cleared the hash table by
assigning to PATH its existing value, triggering its associated
discipline function (put_restricted() in init.c) which then
actually cleared the hash table. That's a bit of a hack. It's nicer
if we can just do that directly. This requires taking a static
handler function rehash() from init.c, which invalidates one hash
table entry, and making it available to the builtin.
src/cmd/ksh93/bltins/typeset.c,
src/cmd/ksh93/include/builtins.h,
src/cmd/ksh93/include/nval.h,
src/cmd/ksh93/sh/init.c,
src/cmd/ksh93/sh/name.c:
- Merge b_hash() into b_alias().
- The -x option was still uselessly setting the NV_EXPORT flag.
Exported aliases were in ksh88 but were removed in ksh93.
- Rename rehash() handler function from init.c to nv_rehash
(avoiding a possible conflict with another rehash() in cd_pwd.c)
and move it to name.c just above nv_scan(), which it's meant to
be used with. Make it an extern so typeset.c can use it.
- b_alias(): Replace the PATH assignment by an nv_scan() call to
clear the hash table directly using the nv_rehash() handler.
src/cmd/ksh93/data/builtins.c:
- POSIX compliance fix: Remove BLT_SPC (special builtin) flag from
"alias" definition. 'alias' is specified as a regular builtin.
- sh_optalias[]: Fix uninformative -t option documentation.
- sh_opthash[]: Edit for conciseness and clarity.
src/cmd/ksh93/sh.1:
- Edit the 'alias -t' and 'hash' documentation.
- Remove the -- prefix from the 'alias' entry, which indicated that
it was supposed to be a declaration builtin like 'typeset', with
assignment-arguments expanding tildes and not being subject to
field splitting. However, my testing shows that 'alias' has never
actually behaved that way on ksh93. Even adding the BLT_DCL flag
in data/builtins.c doesn't seem to change that.
(cherry picked from commit afa68dca5c786daa13213973e8b0f9bf3a1dadf6)
The ksh man page documents that the restricted option cannot be
unset once it is set, which means `set +r` should be invalid.
While this was true for `set +o restricted`, `set +r` was causing
the restricted option to be unset. The fix for this problem comes
from one of Solaris' patches, which adds an error check to prevent
this behavior.
Solaris' patch:
https://github.com/oracle/solaris-userland/blob/master/components/ksh93/patches/020-CR6919590.patch
src/cmd/ksh93/sh/args.c:
- Add an error check to stop `set +r` from unsetting the
restricted option.
src/cmd/ksh93/tests/restricted.sh:
- Add two regression tests to make sure the restricted option
cannot be unset.
(cherry picked from commit bef4fee404d8e24b38fce66420c14a39ac4a123e)
Sometimes you have to put ^C on rapid repeat to get out of running
the tests. This is because each test scripts is launch as a child
shell (not a subshell) in the foreground, which thus handle the ^C.
It then exits with a status indicating SIGINT, but the shtest script
wasn't handling this and just kept going.
src/cmd/ksh93/tests/shtests:
- For reasons I don't understand yet, interrupting tests with ^C
tends to make them exit with status 130 (128+2) instead of what
I would expect for ksh93, 258 (256+2). Not a big deal: POSIX
specifies that any exit > 128 signifies a signal in any case,
and 'kill -l' works even on those values. But the checks need
changing to '> 128'.
- Check each result for SIGINT, and issue SIGINT to self if found.
(cherry picked from commit d0dfb37c6c71ac7b157060249125e0959130927d)
This commit replaces the old hash alias with a proper builtin.
I based this builtin off of the code alias uses for handling
`alias -t --`, but with the hack for `--` removed as it has
no use in the new builtin. `alias -t --` will no longer work,
that hack is now gone.
While I was testing this builtin, I found a bug with hash tables
in non-forking subshells. If the hash table of a non-forking
subshell is changed, the parent shell's hash table is also changed.
As an example, running `(hash -r)` was resetting the parent shell's
hash table. The workaround is to force the subshell to fork if the
hash table will be changed.
src/cmd/ksh93/bltins/typeset.c:
- Move the code for hash out of the alias builtin into a dedicated
hash builtin. `alias -t --` is no longer supported.
src/cmd/ksh93/data/aliases.c:
- Remove the old alias for hash from the table of predefined aliases.
src/cmd/ksh93/data/builtins.c:
- Fix the broken entry for the hash builtin and add a man page for
the new builtin.
src/cmd/ksh93/sh.1:
- Replace the entry for the hash alias with a more detailed entry
for the hash builtin.
src/cmd/ksh93/sh/name.c:
- Force non-forking subshells to fork if the PATH is being reset
to workaround a bug with the hash tree.
src/cmd/ksh93/tests/alias.sh:
- Add a regression test for resetting a hash table, then adding
a utility to the refreshed hash table.
src/cmd/ksh93/tests/subshell.sh:
- Add regression tests for changing the hash table in subshells.
(cherry picked from commit d8428a833afe9270b61745ba3d6df355fe1d5499)
Since we now have a shiny new POSIX compliant 'times' builtin,
we might as well use it.
src/cmd/ksh93/tests/shtests:
- Run 'times' at end of test run.
- Skip the pretty-printing until #7 is fixed.
(cherry picked from commit 2c27d9fbc239583004ec70377db98627eea5e294)
A column of whitespace in the NEWS file was removed for consistent
formatting. Most of the spelling errors were found with this
codespell dictionary:
https://github.com/orbitcowboy/codespell_dictionary
(cherry picked from commit 0e36b17abe5609c461a3e4da7041eb0fdf9991b7)
An oversight...
src/cmd/ksh93/sh.1:
- Undocument the fixed ${.sh.subshell} breakage.
(cherry picked from commit 42be8a3c6dd5a76d03e0456d7f53e38536757aa5)
The bug was really that I/O errors in output builtins were
undetectable by any means. Having a >0 exit status is sufficient.
Adding an error message risks making existing ksh scripts noisier,
or even breaking them if they redirect stderr to stdout.
Note to self: in future, implement the minimum change necessary to
fix bugs, nothing more. The fact that I needed to add four extra
2>/dev/null to the regression tests should have been a hint.
src/cmd/ksh93/bltins/print.c,
src/cmd/ksh93/data/msg.c,
src/cmd/ksh93/include/io.h:
- Remove "I/O error" message.
src/cmd/ksh93/tests/builtins.sh:
- Update to check for exit status only.
src/cmd/ksh93/tests/basic.sh,
src/cmd/ksh93/tests/coprocess.sh:
- Revert four new '2>/dev/null' to suppress the error message.
(cherry picked from commit 5e17be24d18455b575b6e98bc631c6935ffc795a)
src/cmd/ksh93/bltins/typeset.c: unall():
- There's no need to keep track of whether we're unsetting an alias
using a new 'isalias' vaariable, as we can just repeat the check
for 'troot==shp->alias_tree'.
(cherry picked from commit 68f3ae3acd0d19dd80805049a1c9afc1ae1e75c3)
This commit fixes a bug that caused unalias to return a zero status
when it tries to remove an alias twice. The following set of commands
will no longer end with an error:
$ alias foo=bar
$ unalias foo
$ unalias foo && echo 'Error'
This commit is based on the fix present in ksh2020, but it has been
extended with another bugfix. The initial fix for this problem tried to
remove aliases from the alias tree without accounting for NV_NOFREE. This
caused any attempt to remove a predefined aliases (e.g. `unalias float`)
to trigger an error with free, as all predefined aliases are in read-only
memory. The fix for this problem is to set NV_NOFREE when removing aliases
from the alias tree, but only if the alias is in read-only memory. All
other aliases must be freed from memory to prevent memory leaks.
I'll also note that I am using an `isalias` variable rather than the `type`
enum from ksh2020, as the `VARIABLE` value is never used and was replaced
with a bool called `aliases` in the ksh2020 release. The `isalias` variable
is an int as the ksh93u+ codebase does not use C99 bools.
Previous discussion: https://github.com/att/ast/issues/909
- src/cmd/ksh93/bltins/typeset.c:
Remove aliases from the alias tree by using nv_delete. NV_NOFREE
is only used when it is necessary.
- src/cmd/ksh93/tests/alias.sh:
Add two regression tests for the bugs fixed by this commit.
(cherry picked from commit 16d5ea9b52ba51f9d1bca115ce8f4f18e97abbc4)
Build scripts and the like ought to be able to check the results of
the build using the exit status of the build command.
bin/package,
src/cmd/INIT/package.sh:
- Add global error_status variable to keep track of the highest
exit status of all commands using capture(). Exit with that.
- capture():
* When capturing build output using 'tee', change the regular
pipe construct (which doesn't preserve the exit status of pipe
elements except the last one) to a couple of background jobs
connected with a FIFO (named pipe). This allows getting the
build's exit status using the "wait" command.
* Update error_status on every invocation of capture().
(cherry picked from commit 22c3a6e1b32f3ab7eeca45adb76863982dc3e334)
The default behavior of the package script is to ignore
errors. This has disastrous effects if any part of ksh
or libast fails to build due to something like a syntax
error, as the build will stop long after the first error
occurred.
This commit removes the -k flag from the $makeflags variable
in both copies of the package script. This forces the build
to fail if a compiler error occurs.
bin/package and src/cmd/INIT/package.sh:
- Remove the -k assignment to $makeflags so that compiler
errors cause the build to fail.
(cherry picked from commit 87bfaa19f3d336feb7b7bd4f5d4caf8d553f2547)
Setting the exit status allows build scripts and the like
to actually detect regression test failures.
src/cmd/ksh93/tests/shtests:
- Add counter for total number of errors.
- Report total to stdout at the end.
- Set the exit status to the total number of errors, up to 125
(higher statuses have special meanings).
(cherry picked from commit c0dd80b14d4d316b4e9eff4b1b67d4e47f23a6ba)
src/cmd/ksh93/tests/variables.sh:
- Tolerate a bit more time for the SECONDS verification test.
src/cmd/ksh93/tests/subshell.sh:
- Replace unportable 'head -c 1' by 'dd bs=1 count=1'
- Remove unnecessary uses of 'whence'.
src/cmd/ksh93/tests/builtins.sh:
- Add a regression test for a weirdly specific 'whence' bug exposed
by the aforementioned unneccessary uses of 'whence', which only
shows up on my old Power Mac G5 running Mac OS X 10.3. For all I
know it's a compiler bug, but let's add a more clear failure for
it here, in case that happens anywhere else.
(cherry picked from commit c3898bd1e6e40874845771d33a5b37220ef0b06e)
This reduces a bunch more unnecessarily long sleeps to give
asynchronous processes time to run, etc. (No, we don't need to be
compatible anymore with your cool 1985 Intel 80386DX 16 MHz
battlestation...) Running the test suite is almost tolerable now,
making me much more likely to actually run the regression test
suite and catch my own regressions.
In addition, there are various fixes to make the test suite
compatible with 'set -x' ('set -o xtrace') so that you can now
actually *use* the documented 'bin/shtests -x' option. Recommend
combining with '-p' to avoid tracing everything three times.
I've also added a really useful $PS4 trace prompt to shtests that
traces pretty much everything there is to trace. (It does use
expansions that modify ${.sh.match}, which affected several tests,
particularly in tests/substring.sh; for those we need to set a
temporary simpler $PS4.)
(cherry picked from commit c3a5d47cfe880b526cabb5370ddaced0e8626acd)
This regression was caused by the commit that introduced
the times builtin. The ordering of the #include directives
was causing the build to fail on Ubuntu 20.04 and Arch Linux.
src/cmd/ksh93/bltins/misc.c:
- Move the includes for standard headers to the bottom
of the includes to fix build failures on Linux.
(cherry picked from commit 3a42b206a0cc71bedc4773ce545435570e862e61)
This fixes two regression test failures in tests/functions.sh.
src/cmd/ksh93/bltins/cflow.c: b_exit():
- The exit status should of course only be cropped to 8 bits if
b_exit() will actually exit, not if it returns from a function.
(cherry picked from commit 2d1e7f87551159c942b16de2a98dc72697988d26)
Emacs editing mode is bugged in ksh93u+ and ksh2020. Let's
say you were to run the following commands after starting
a fresh instance of ksh:
$ alias foo='true'
$ unalias foo
If you type 'a' and then press the up arrow on your keyboard,
ksh will complete 'a' to `alias foo='true'` by doing a reverse
search for the last command that starts with 'a'.
Run the alias command again, then type 'u' and press the up
arrow key again. If ksh is in Vi mode, you will get `unalias foo`,
but in Emacs mode you will get `alias foo='true'` again.
This bug was occurring because ksh was only doing a reverse search
based on the first command that was completed using the up arrow.
All subsequent commands were ignored as ksh was saving the first
command and only based later searches off of it.
NEWS:
- Add instructions for reproducing this bug with the up arrow
key and information about why this bug was happening in the
first place.
src/cmd/ksh93/edit/emacs.c:
- Remove a bad check that was preventing ksh from using the
latest input for reverse search.
(cherry picked from commit 745b065487ad6bac693ec6f44752f96e87f9a63b)
The signal.sh regression tests took over a minute and a half to
complete, so with the default three runs (for different locales)
they took about five minutes. Annoying and unnecessary. As ksh
supports floating point shell arithmetic plus a 'sleep' builtin
that supports fractional seconds, this is easy to fix.
src/cmd/ksh93/tests/signal.sh:
- Slash all the 'sleep' arguments and associated factors by a
factor of 10. Since computers are now hundreds or thousands of
times faster than they were in the early 1990s, this should be
plenty safe enough and all the tests should still be valid.
- Change one unportable direct invocation of /bin/sleep to use the
builtin.
(cherry picked from commit 734e5953650e9a62048c45eb9a3b827bb9efcfb5)
This fixes two bugs: issuing the 'exit' command with a value > 256
would cause ksh 93u+ to kill itself with the corresponding signal
(try 'exit 265' to SIGKILL your interactive shell), and, if the
last command of a script exits due to a signal, the shell would
repeat that signal to itself, causing any parent ksh to also be
killed.
Discussion:
https://bugzilla.redhat.com/show_bug.cgi?id=1469624https://rainbow.chard.org/2017/03/21/ksh-deliberately-segfaults-if-the-last-command-in-a-script-crashes/
This commit is loosely based on a patch applied to the 93v- beta
and the abandoned ksh2020, but that patch was incomplete & broken:
$ ksh-2020.0.0 -c 'exit 265'; echo $?
137
Expected: 9. Since the exit was *not* due to a signal, the value
should simply be cropped to the 8 bits supported by the OS.
src/cmd/ksh93/bltins/cflow.c: b_exit():
- For the 'exit' builtin command, bitwise-AND the argument to
'exit' with SH_EXITMASK (8 bits, crop to 0-255) before passing it
on to sh_exit(). This restores the behaviour of <=2011 ksh93
versions and is in line with all other POSIX shells.
It also fixes this bogosity:
$ (exit 265); echo $? # non-forked subshell
265
$ (ulimit -t unlimited; exit 265); echo $? # forked subshell
9
Forked or non-forked should make no difference at all
(see commit message a0e0e29e for why).
src/cmd/ksh93/sh/fault.c: sh_done():
- If the current exit status is equal to the status for the last
signal that was received from a child process, remove the
SH_EXITSIG (9th) bit, so that the shell doesn't kill itself.
- If the shell's last child process exits due to a signal, exit
with a portable 8-bit exit status (128 + signal number). This
avoids the exit status being < 128 by being cropped to 8 bits.
src/cmd/ksh93/tests/signal.sh:
- Add regression test for exit with status > 256.
- Add regression test verifying the shell no longer kills itself.
(cherry picked from commit 98e0fc94393e175ce6adfee390327c320795bf12)
This commit gets rid of dead weight related to an obscure early
1990s Bell Labs versioning file system research project called
3DFS, which has not existed for decades and for which I have not
managed to find any evidence that it was ever used outside the lab.
This removes:
- the SHOPT_FS_3D compile option (which was forced on even when 0)
- the obnoxious default alias 2d='set -f;_2d' that turned off your
globbing and then tried to run a nonexistent _2d command
- undocumented builtins 'vmap' and 'vpath' that only errored out
- a non-functional -V unary operator for the test and [[ commands
- some specific code for Apollo workstations (last made in 1997),
which was inseparably intertwined with the 3DFS code
(cherry picked from commit 20cdf3709f4fb4e468057b534dcee819b1961fb6)
POSIX requires[*] that expanding any unset parameter other than $@
and $* is an error when 'set -u'/'set -o nounset' is active.
However, on ksh93, $! was exempt as well. That is a bug.
[*] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_25
src/cmd/ksh93/sh/macro.c:
- special(): Handle 'set -u' for special parameters if/when it is
about to return NIL. That code path is currently only possible to
reach for "$!", but this is future-proof and will do the right
thing if any other special parameter can ever have no value.
src/cmd/ksh93/tests/options.sh:
- Add and tweak 'set -u' regression tests.
(cherry picked from commit 75cc7a38cafe3a9929e1ed17d8b952babda22a09)
POSIX requires[*] that expanding any unset parameter other than $@
and $* is an error when 'set -u'/'set -o nounset' is active.
However, on ksh93, $1, $2, ... were exempt as well. That is a bug.
[*] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_25
src/cmd/ksh93/sh/macro.c:
- varsub(): Backport code for handling 'set -u' for positional
parameters from the ast 2016-10-01-beta branch.
src/cmd/ksh93/tests/options.sh:
- Add relevant regression tests.
src/cmd/ksh93/sh.1:
- Document that $@ and $* are exempt from 'set -u'.
(cherry picked from commit f954c6be0748c4c38a680a75f27564965fbd328e)
src/cmd/ksh93/tests/options.sh:
- Pipe hang bugfix test: The child shell has no idea how long to
sleep for if we don't export the variable telling it so.
(cherry picked from commit ede0960c4ec84f0f934b17072a892a4e40798e97)
According to Red Hat, this fixes "a bug on input buffer boundary
and/or temporary composing buffer of multibyte characters".
The patch was credited to Paulo Andrade <pandrade@redhat.com>.
To be honest, I don't know how to trigger this bug or what the code
removed by this fix really does, but this patch is in production
use at Red Hat, removes some smelly stuff, and is not triggering
any regression test failures, so I'll just take this one on faith.
https://bugzilla.redhat.com/show_bug.cgi?id=1417886https://github.com/att/ast/commit/4fa2020b
src/cmd/ksh93/sh/fcin.c:
- _fcmbget(): Remove some dodgy-looking buffer-fiddling code that
is marked as "for testing purposes with small buffers".
(cherry picked from commit 407760fdbddcb7f8ac92b5d1da29d3e09dac0369)
Discussion: https://bugzilla.redhat.com/show_bug.cgi?id=1451057
src/cmd/ksh93/sh/parse.c: funct():
- Make the savstak variable volatile and always initialise it to
avoid undefined behaviour.
(cherry picked from commit 5e56b28cd63ec2120c5f70a6e0abf2f8dbb7e7dc)
Both 'fc' and 'type' are already implemented as perfectly
functional builtins -- in fact they use the exact same C function
as 'hist' and 'whence', so the behaviour is clearly identical.
Except 'type' was aliased to 'whence -v', but b_whence() contains
explicit code to activate the v flag if invoked as 'type', so the
alias is not needed for that either.
It looks like someone decided to implement these aliases as proper
builtins (as they should be; they are POSIX standard commands and
'unalias -a' must not get rid of them), but then forgot to remove
these default aliases (and to update the man page).
I'm not even doing a NEWS entry for this as there is no change in
behaviour. The only difference is that you now get the correct
command name in error and usage messages for 'fc' and 'type'.
src/cmd/ksh93/data/aliases.c:
- Remove fc='hist' and type='whence -v' default aliases.
src/cmd/ksh93/sh.1:
- Remove those default aliases from the list.
- Document 'fc' and 'type' builtins.
(cherry picked from commit c73af6a5a36a72c681201c9e9c397f98bbf2a86d)
Continuing alias substitution after 'command' (due to the final
space in the alias) is inherently broken and doing so by default is
incompatible with the POSIX standard, as aliases may contain
arbitrary shell grammar.
For instance, until the previous commit, the POSIX standard 'times'
command was an alias: times='{ { time;} 2>&1;}' -- and so, of
course, 'command times' gave a syntax error, although this is
a perfectly valid POSIX idiom that must be supported.
'command' is specified by POSIX as a regular builtin, not an alias.
Therefore it should always bypass aliases just as it bypasses
functions to expose standard builtin and external commands.
I can only imagine that the reason for this command='command '
alias was that some standard commands themselves were implemented
as aliases, and POSIX requires that standard commands are
accessible with the 'command' prefix. But implementing standard
commands as aliases is itself inherently broken. It never worked
for 'command times', as shown; and in any case, removing all
aliases with 'unalias -a' should not get rid of standard commands.
Similarly, the default alias nohup='nohup ' is also harmful.
Anyone who really wants to keep this behaviour can just define
these aliases themselves in their script or ~/.kshrc file.
src/cmd/ksh93/data/aliases.c:
- Remove default alias command='command '.
- Remove default alias nohup='nohup '.
src/cmd/ksh93/sh.1
- Remove the above default aliases from the list.
- Mention that the 'command' builtin does not search for aliases.
(cherry picked from commit 5cfe7c4e2015b7445da24983af5008035c4b6e1e)
As of this commit, the 'times' command is a POSIX-compliant special
builtin command instead of an alias that doesn't produce the
required output. It displays the accumulated user and system CPU
times, one line with the times used by the shell and another with
those used by all of the shell's child processes.
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_27
This was originally written by Kurtis Rader and is now backported
and tweaked from the abandoned ksh2020 branch. I chose an earlier
and simpler version[*1] that uses times(3), with a precision of
hundredths of seconds, so it outputs the same precision as mksh and
zsh. Rader later wrote another version[*2] that uses getrusage(2),
giving it the same millisecond precision as bash. But that required
adding a feature test and a fallback to the old version, which is
non-trivial in the old INIT/iffe system. This simpler version is
enough to gain POSIX compliance and I think it will do very nicely
in this stable bugfix branch.
[*1] https://github.com/att/ast/pull/1332
[*2] https://github.com/att/ast/commit/038045f6
src/cmd/ksh93/bltins/misc.c
- Add b_times() function for 'times' builtin.
- Note we include <times.h>, not <sys/times.h>, so that we use the
AST feature-tested version with fallback on systems that need it.
src/cmd/ksh93/data/aliases.c:
- Remove times='{ { time;} 2>&1;}' builtin alias.
src/cmd/ksh93/data/builtins.c,
src/cmd/ksh93/include/builtins.h:
- Add entry for 'times' special builtin.
- Add --help/--man info for same.
src/cmd/ksh93/sh.1:
- Update manual page.
src/cmd/ksh93/tests/builtins.sh:
- Add a couple of simple regression tests.
(cherry picked from commit ebf71e619eb298ec1cf6b81d1828fa7cdf6e9203)
Allowing redefined macro warnings was not such a good idea, as the
Apple compiler flags redefine the _ast_int8_t macro right on the
command line, producing a warning for every cc invocation.
(cherry picked from commit 929930224f898cb1371471fe554fabb73b31d11f)
Only standard error needs to be on a terminal for a
shell to be interactive on init, not all output.
See sh_main() in src/cmd/ksh93/sh/main.c, under
"/* decide whether shell is interactive */".
Also, tcgetattr is not a syscall, so its manual page is
tcgetattr(3), not tcgetattr(2).
(cherry picked from commit 97f8bdb08650fd63f8e125257a86f5e6035fbdfa)
It didn't mention that the default path is obtained from the
operating system where available, and that the default string
documented is only a fallback (which, btw, should never occur on
modern systems).
See:
e_defpath[] in src/cmd/ksh93/data/msg.c
defpath_init() in src/cmd/ksh93/sh/path.c
src/cmd/ksh93/sh.1:
- Add a mention that the default path is equal to the output of
ksh's 'getconf PATH' builtin command, unless that fails.
(cherry picked from commit 12b8dc54626a14171d3671a26041733a32e0e07d)
This counter is documented as follows:
"The current depth for subshells and command substitution."
But before this commit, the actual behaviour was that the counter
was reset to zero whenever a subshell forked for any reason: a
pipe, background job, running 'ulimit', redirecting stdout in a
command substitution, and more. This behaviour was:
1. Not consistent with the documentation. Non-forked (a.k.a.
virtual) subshells are an internal implementation detail which
scripts should not have to be concerned with. The manual page
doesn't mention them at all.
2. Inherently broken. Since a subshell may fork for any number of
reasons, even mid-run, and those reasons may change with
bugfixes and further development, scripts have never actually
been able to rely on the value of ${.sh.subshell}.
So, this commit fixes the counter to count the levels of all
subshells, both virtual and forked.
src/cmd/ksh93/sh/xec.c: _sh_fork():
- Increase ${.sh.subshell} whenever we fork.
src/cmd/ksh93/sh/subshell.c:
- sh_subfork():
* Fix comment to properly explain what it does. It doesn't
"create" a subshell, it forks off an existing virtual subshell.
* Don't zero ${.sh.subshell}. Instead, since sh_fork() increases
it but we're forking an existing subshell, undo the increase.
- sh_subshell():
* Remove 'int16_t subshell' variable. It was unnecessary and
mostly unused. It was also the wrong type: it was assigned the
value from shp->subshell which is of type short.
* Increase and decrease the level of virtual subshells and
${.sh.subshell} independently.
src/cmd/ksh93/tests/variables.sh:
- Add regression tests for ${.sh.subshell} in virtual and forked
subshells of several kinds: comsub, parentheses, pipe, bg job.
- Undo wrong error test count fix from 04b4aef0.
(cherry picked from commit a0e0e29e7e0dbf21e4b3958ae02bde6665fb2696)
The sfputr() function (put out a null-terminated string) contained
a use of memccpy() that was invalid and could cause crashes,
because the buffer it was copying into could overlap or even be
identical with the buffer being copied from.
Among (probably) other things, this commit fixes a crash in 'print
-v' (print a compound variable structure) on macOS, that caused the
comvar.sh and comvario.sh regression tests to fail spectacularly.
Now they pass.
Issue discovered and fixed by Kurtis Rader in the abandoned ksh2020
branch; this commit backports the fix. He wrote:
| #if _lib_memccpy && !__ia64 /* these guys may never get it right */
|
| The problem is that assertion is wrong. It implies that the libc
| implementation of memccpy() on IA64 is broken. Which is
| incorrect. The problem is the AST sfputr() function is depending
| on what is explicitly undefined behavior in the face of
| overlapping source and destination buffers.
| [...] Using memccpy() simply complicates the code and is unlikely
| to be measurably, let alone noticeably, faster.
Further discussion/analysis: https://github.com/att/ast/issues/78
src/lib/libast/sfio/sfputr.c:
- Remove memccpy use. Always use the manual copying loop.
(cherry picked from commit fbe3c83335256cb714a4aa21f555083c9f1d71d8)
This fixes (or at least works around) a bug that caused special
variables such as PATH, LANG, LC_ALL, LINENO, etc. to lose their
effect after being unset in a subshell.
For example:
(unset PATH; PATH=/dev/null; ls); : wrongly ran 'ls'
(unset LC_ALL; LC_ALL=badlocale); : failed to print a diagnostic
This is yet another problem with non-forking/virtual subshells. If
you forced the subshell to fork (one way of doing this is using the
'ulimit' builtin, e.g. ulimit -t unlimited) before unsetting the
special variable, the problem vanished.
I've tried to localise the problem. I suspect the sh_assignok()
function, which is called from unall(), is to blame. This function
is supposed to make a copy of a variable node in the virtual
subshell's variable tree. Apparently, it fails to copy the
associated permanent discipline function settings (stored in the
np->nvfun->disc pointer) that gave these variables their special
effect, and which survive unset. However, my attempts to fix that
have been unsuccessful. If anyone can figure out a fix, please send
a patch/pull request!
Data point: This bug existed in 93u 2011-02-08, but did not yet
exist in M-1993-12-28-s+. So it is a regression.
Meanwhile, pending a proper fix, this commit adds a safe
workaround: it forces a non-forked subshell to fork before
unsetting such a special variable.
src/cmd/ksh93/bltins/typeset.c: unall():
- If we're in a non-forked, non-${ ...; } subshell, then before
unsetting any variable, check for variables with internal
trap/discipline functions, and call sh_subfork() if any are
found. To avoid crashing, this must be done before calling
sh_pushcontext(), so we need to loop through the args separately.
src/cmd/ksh93/tests/variables.sh:
- Remove the 'ulimit' that forced the fork; we do this in C now.
(cherry picked from commit 21b1a67156582e3cbd36936f4af908bb45211a4b)
Many tests used direct paths to some commands, mostly /bin/echo and
/bin/cat. This is unportable (breaks on e.g. NixOS).
The correct way is to obtain the direct path using 'whence -p'.
There was also one use of '/usr/bin/pstack' in tests/comvario.sh
that seemed bogus. Apparently this was supposed to analyse a core
file after a crash. Even on Solaris and Linux, where that command
exists, the argument is documented to be a PID, not a core file. If
this ever worked anywhere, then it was system-specific enough to be
useless here, so I've removed it.
(cherry picked from commit 4563b8bc651cd9cb18dc73f56a041f7ac5534395)
In making ksh build on new macOS, it stopped building on old Mac OS
X (old gcc-based Darwin). There is no real reason for this. We can
just restore the old cc wrapper script and use it if an old gcc
compiler is detected.
This is only tested on Mac OS X 10.3 on my old Power Mac G5 so far.
But at least that allows me to test fixes on that platform. Unusual
platforms sometimes expose corner case bugs...
bin/package, src/cmd/INIT/package.sh:
- If /usr/bin/cc is GCC, change 'darwin' host name to 'darwin_old'.
This removes the long-obsolete 'darwin7' host name.
src/cmd/INIT/cc.darwin_old:
- Restore the old cc.darwin script for darwin_old hosts.
(cherry picked from commit 93d4c6497ea8e9cc9f4977b75d06a673a2229f80)
When a test aborted due to a signal, the >256 exit code signifying
the signal was incorrectly reported as the number of errors.
src/cmd/ksh93/tests/shtests:
- For $? > 256, obtain and report the signal name using 'kill -l'.
(cherry picked from commit a7d8ae628e228fc3cadcf977fbffc87b90c7bc53)
Namerefs aren't broken in virtual/non-forked subshells after all
(phew). It is changing the locale that is somehow broken.
src/cmd/ksh93/tests/variables.sh:
- Pending further investigation, update the TODO and don't fork the
subshell until actually needed.
(cherry picked from commit efa3150396b383b6a68b2df45eab9005593b2e42)
This fixes a really stupid bug in my own code for unsetting a
function in a subshell. The algorithm for walking through the
subshell tree was broken, resulting in an infinite loop if there
were multiple levels of subshell.
src/cmd/ksh93/bltins/typeset.c:
- Correct the subshell function tree walk that deletes functions
from zombie parent scopes.
src/cmd/ksh93/tests/subshell.sh:
- Add a regression test for setting and unsetting identically named
functions in multiple levels of subshell.
(cherry picked from commit 972a7999c7f16469138daf3d86dfd6c0db3f4879)
The $! special parameter was not set if a background job
(somecommand &) or co-process (somecommand |&) was launched as the
only command within a braces block with an attached redirection,
for example:
{
somecommand &
} >&2
With the bug, $! was unchanged; now it contains the PID of
somecommand.
Ref.: https://github.com/att/ast/issues/1357
src/cmd/ksh93/sh/parse.c: item():
- When processing redirections following a compound command, always
create a parent node with the TSETIO (I/O redirection) token.
Before this commit, if the last command was of type TFORK (and
the last command only tested as TFORK if the bg job or coprocess
was the only command in a braces block, because the ksh parser
optimises away the braces in that case), then the parent node was
created with the TFORK token instead.
I have no idea what David Korn's intention was with that, but
this is clearly very wrong. Creating another TFORK node when
parsing the redirection caused sh_exec() in sh/xec.c to execute
the redirection in an extra forked, non-background subshell.
Since redirections are executed before anything else, this
subshell is what then launched the background job between the
braces, so $! (a.k.a. shp->bckpid) was updated in that subshell
only, and never in the main shell. The extra subshell also
prevented the background job from being noticed by job control
on interactive shells.
So, the fix is simply to remove the broken test for TFORK.
src/cmd/ksh93/tests/variables.sh:
- Add regression tests for a bg job and a co-process as the only
command within a braces block with attached redirection.
(cherry picked from commit ffe5df30e69f7b596941a98498014d8e838861f2)
src/cmd/ksh93/tests/variables.sh:
- Unset -x in a command substitution that redirects stderr to
stdout; this caused a spurious failure with tracing active.
- Execute the nameref tests in a subshell. They modified LINENO, so
that all the line numbers after this test were traced and/or
reported as 'foo'.
. This exposed a bug in namerefs: this is yet another thing that
is broken in non-forked/virtual subshells! That is for another
commit. For now, fork the subshell and leave a TODO.
(cherry picked from commit 8b7f8f9b14523a66bdf337612daef2501c2bb5ba)
src/cmd/ksh93/tests/bracket.sh:
- -r and -w are always true when running as root, so skip these.
- Fix several typos in the [[ tests (apparent copy/paste errors).
(cherry picked from commit b9325e9753b38f087d0c5501d474f7d1f15c4d36)
Fix a bug in autoloading functions. Directories in the path search
list which should be skipped (e.g. because they don't exist) did
not interact correctly with autoloaded functions, so that a
function to autoload was not always found.
Details:
https://github.com/att/ast/issues/1454
Fix backported (and cleaned up) from:
https://github.com/att/ast/commit/3bc58164
src/cmd/ksh93/sh/path.c:
- path_opentype(): Fix the path search loop so that entries marked
with PATH_SKIP are handled correctly.
src/cmd/ksh93/tests/functions.sh:
- Add regression test verifying an autoloaded function with a PATH
that triggered the bug.
The bug in path_opentype() fixed by this commit may affect other
scenarios but we know it affects autoloaded functions. Hence the
test for that scenario.
(cherry picked from commit a27903165775309f4f032de5d42ec1785f14cfbc)
Fix BUG_TESTERR1A: POSIX non-compliance of 'test'/'[' exit status
on error. The command now returns status 2 instead of 1 when given
an invalid number or arithmetic expression, e.g.: [ 123 -eq 123x ]
The problem was that the test builtin (b_test()) calls the generic
arithmetic evaluation subsystem (sh/arith.c, sh/streval.c) which
has no awareness of the test builtin. A simple solution would be to
always make the arithmetic subsystem use an exit status > 1 for
arithmetic errors, but globally changing this may cause backwards
compatibility issues. So it's best to change the behaviour of the
'test' builtin only. This requires the arithmetic subsystem to be
aware of whether it was called from the 'test' builtin or not. To
that end, this commit adds a global flag and overrides the
ERROR_exit macro where needed.
src/cmd/ksh93/include/defs.h,
src/cmd/ksh93/sh/defs.c:
- Declare and initialise a global sh_in_test_builtin flag.
- Declare internal function for ERROR_exit override in test.c.
src/cmd/ksh93/bltins/test.c:
- Add override for ERROR_exit macro using a function that checks if
the exit status is at least 2 if the error occurred while running
the test builtin.
- b_test(): Set sh_in_test_builtin flag while running test builtin.
src/cmd/ksh93/sh/arith.c,
src/cmd/ksh93/sh/streval.c:
- Override ERROR_exit macro using function from test.c.
src/cmd/ksh93/tests/bracket.sh:
- Add regression test verifying status > 1 on arith error in test.
(cherry picked from commit 5eeae5eb9fd5ed961a5096764ad11ab870a223a9)
Aliases can now be correctly unset within subshell environments
(such as ( ... ), $(command substitutions), etc), as well as
non-subshell "shared" command substitutions (${ ...; }). Before,
attempts to unset aliases within these were silently ignored.
Prior discussion: https://github.com/att/ast/issues/108
Subshell alias trees are only referenced in a few places in the
code, *and* have always been broken, so this commit gets rid of the
whole notion of a subshell alias tree. Instead, there is now just
one flat alias tree, and subshells fork into a separate process
when aliases are set or unset within them. It is not really
conceivable that this could be a performance-sensitive operation,
or even a common one, so this is a clean fix with no downside.
src/cmd/ksh93/include/defs.h:
- Remove sh_subaliastree() definition.
src/cmd/ksh93/sh/subshell.c:
- Remove salias element (pointer to subshell alias tree) from
subshell struct.
- Remove sh_subaliastree() function.
- sh_subshell(): Remove alias subshell tree cleanup.
src/cmd/ksh93/bltins/typeset.c:
- b_alias(): If in subshell, fork before setting alias.
- b_unalias(): If in subshell, fork before unsetting alias.
- unall(): Remove sh_subaliastree() call.
src/cmd/ksh93/sh/name.c:
- nv_open(): Remove sh_subaliastree() call.
src/cmd/ksh93/tests/subshell.sh:
- Add regression tests for unsetting or redefining aliases within
subshells.
(cherry picked from commit 12a15605b9521a2564a6e657905705a060e89095)
Functions can now be correctly redefined and unset in subshell
environments (such as ( ... ), $(command substitutions), etc).
Before this fix, attempts to do this were silently ignored (!!!),
causing the wrong code (i.e.: the function by the same name from
the parent shell environment) to be executed.
Redefining and unsetting functions within "shared" command
substitutions of the form '${ ...; }' is also fixed.
Prior discussion: https://github.com/att/ast/issues/73
src/cmd/ksh93/sh/parse.c:
- A fix from George Koelher (URL above). He writes:
| The parser can set t->comnamp to the wrong function.
| Suppose that the shell has executed
| foo() { echo WRONG; }
| and is now parsing
| (foo() { echo ok; } && foo)
| The parser was setting t->comnamp to the wrong foo. [This
| fix] doesn't set t->comnamp unless it was a builtin. Now the
| subshell can't call t->comnamp, so it looks for foo and finds
| the ok foo in the subshell's function tree.
src/cmd/ksh93/bltins/typeset.c:
- Unsetting functions in a virtual/non-forked subshell still
doesn't work: nv_open() fails to find the function. To work
around this problem, make 'unset -f' fork the subshell into its
own process with sh_subfork().
- The workaround exposed another bug: if we unset a function in a
subshell tree that overrode a function by the same name in the
main shell, then nv_delete() exposes the function from the main
shell scope. Since 'unset -f' now always forks a subshell, the
fix is to simply walk though troot's parent views and delete any
such zombie functions as well. (Without this, the 4 'more fun'
tests in tests/subshell.sh fail.)
src/cmd/ksh93/sh/subshell.c: sh_subfuntree():
- Fix function (re)definitions and unsetting in "shared" command
substitutions of the form '${ commandlist; }' (i.e.: if
sp->shp->subshare is true). Though internally this is a weird
form of virtual subshell, the manual page says it does not
execute in a subshell (meaning, all changes must survive it), so
a subshell function tree must not be created for these.
src/cmd/ksh93/tests/subshell.sh:
- Add regression tests related to these bugfixes. Test unsetting
and redefining a function in all three forms of virtual subshell.
(cherry picked from commit dde387825ab1bbd9f2eafc5dc38d5fd0bf9c3652)
These always fail when running as root, as root can access files
regardless of permission bits.
(cherry picked from commit a821fe13906ac8ef56162bebd7c3e976b973f91c)
