From ed9053ecfb538c8a9707c92e4ac1604144a7d0f3 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Mon, 13 Jun 2022 11:18:03 +0100 Subject: [PATCH] remove ( simple_command & ) optimisation (re: e3d7bf1d) I didn't trust this back in e3d7bf1d (which disabled it for interactive shells) and I trust it less now. In af6a32d1/6b380572, this was also disabled for virtual subshells as it caused program flow corruption there. Now, on macOS 10.14.6, a crash occurs when repeatedly running a command with this optimisation: $ ksh -c 'for((i=0;i<100;i++));do print -n "$i ";(sleep 1&);done' 0 1 2 3 4 5 6 7 Illegal instruction Oddly enough it seems that I can only reproduce this crash on macOS -- not on Linux, OpenBSD, or Solaris. It could be a macOS bug, particularly given the odd message in the stack trace below. I've had enough, though. Out it comes. Things now work fine, the reproducer is fixed on macOS, and it didn't optimise much anyway. The double-fork issue discussed in e3d7bf1d remains. ________ For future reference, here's an lldb debugger session with a stack trace. It crashes on calling calloc() (via sh_calloc(), via sh_newof()) in jobsave_create(). This is not an invalid pointer problem as we're allocating new memory, so it does look like an OS bug. The "BUG IN CLIENT OF LIBPLATFORM" message is interesting. $ lldb -- arch/*/bin/ksh -c 'for((i=0;i<100;i++));do print -n "$i ";(sleep 1&);done' (lldb) target create "arch/darwin.i386-64/bin/ksh" Current executable set to 'arch/darwin.i386-64/bin/ksh' (x86_64). (lldb) settings set -- target.run-args "-c" "for((i=0;i<100;i++));do print -n \"$i \";(sleep 1&);done" (lldb) run error: shell expansion failed (reason: lldb-argdumper exited with error 2). consider launching with 'process launch'. (lldb) process launch Process 35038 launched: '/usr/local/src/ksh93/ksh/arch/darwin.i386-64/bin/ksh' (x86_64) 0 1 2 3 4 5 6 7 8 9 Process 35038 stopped * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0) frame #0: 0x00007fff70deb1c2 libsystem_platform.dylib`_os_unfair_lock_recursive_abort + 23 libsystem_platform.dylib`_os_unfair_lock_recursive_abort: -> 0x7fff70deb1c2 <+23>: ud2 libsystem_platform.dylib`_os_unfair_lock_unowned_abort: 0x7fff70deb1c4 <+0>: movl %edi, %eax 0x7fff70deb1c6 <+2>: leaq 0x1a8a(%rip), %rcx ; "BUG IN CLIENT OF LIBPLATFORM: Unlock of an os_unfair_lock not owned by current thread" 0x7fff70deb1cd <+9>: movq %rcx, 0x361cb16c(%rip) ; gCRAnnotations + 8 Target 0: (ksh) stopped. (lldb) bt * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0) * frame #0: 0x00007fff70deb1c2 libsystem_platform.dylib`_os_unfair_lock_recursive_abort + 23 frame #1: 0x00007fff70de7c9a libsystem_platform.dylib`_os_unfair_lock_lock_slow + 239 frame #2: 0x00007fff70daa3bd libsystem_malloc.dylib`tiny_malloc_should_clear + 188 frame #3: 0x00007fff70daa20f libsystem_malloc.dylib`szone_malloc_should_clear + 66 frame #4: 0x00007fff70dab444 libsystem_malloc.dylib`malloc_zone_calloc + 99 frame #5: 0x00007fff70dab3c4 libsystem_malloc.dylib`calloc + 30 frame #6: 0x000000010003fa5d ksh`sh_calloc(nmemb=1, size=16) at init.c:264:13 frame #7: 0x000000010004f8a6 ksh`jobsave_create(pid=35055) at jobs.c:272:8 frame #8: 0x000000010004ed42 ksh`job_reap(sig=20) at jobs.c:363:9 frame #9: 0x000000010004ff6f ksh`job_waitsafe(sig=20) at jobs.c:511:3 frame #10: 0x00007fff70de9b5d libsystem_platform.dylib`_sigtramp + 29 frame #11: 0x00007fff70d39ac4 libsystem_kernel.dylib`__fork + 12 frame #12: 0x00007fff70c57d80 libsystem_c.dylib`fork + 17 frame #13: 0x000000010009590d ksh`sh_exec(t=0x0000000101005d30, flags=4) at xec.c:1883:16 frame #14: 0x0000000100096013 ksh`sh_exec(t=0x0000000101005d30, flags=4) at xec.c:2019:4 frame #15: 0x0000000100096c4f ksh`sh_exec(t=0x0000000101005a40, flags=5) at xec.c:2213:9 frame #16: 0x0000000100096013 ksh`sh_exec(t=0x0000000101005a40, flags=5) at xec.c:2019:4 frame #17: 0x000000010001c23f ksh`exfile(iop=0x0000000100405750, fno=-1) at main.c:603:4 frame #18: 0x000000010001b23c ksh`sh_main(ac=3, av=0x00007ffeefbff4f0, userinit=0x0000000000000000) at main.c:365:2 frame #19: 0x0000000100000776 ksh`main(argc=3, argv=0x00007ffeefbff4f0) at pmain.c:45:9 frame #20: 0x00007fff70bfe3d5 libdyld.dylib`start + 1 --- src/cmd/ksh93/sh/xec.c | 18 ------------------ src/cmd/ksh93/tests/variables.sh | 1 + 2 files changed, 1 insertion(+), 18 deletions(-) diff --git a/src/cmd/ksh93/sh/xec.c b/src/cmd/ksh93/sh/xec.c index 8271cfa67d0e..bfcbe40da62b 100644 --- a/src/cmd/ksh93/sh/xec.c +++ b/src/cmd/ksh93/sh/xec.c @@ -1868,24 +1868,6 @@ int sh_exec(register const Shnode_t *t, int flags) sh.exitval -= 128; sh_done(0); } - else if(((type=t->par.partre->tre.tretyp)&FAMP) && ((type&COMMSK)==TFORK) - && !job.jobcontrol && !sh.subshell) - { - /* Optimize '( simple_command & )' */ - pid_t pid; - sfsync(NIL(Sfio_t*)); - while((pid=fork())< 0) - _sh_fork(pid,0,0); - if(pid==0) - { - sh.current_pid = getpid(); - sh_reseed_rand((struct rand*)RANDNOD->nvfun); - sh.realsubshell++; - sh_exec(t->par.partre,flags); - sh.st.trapcom[0]=0; - sh_done(0); - } - } else sh_subshell(t->par.partre,flags,0); break; diff --git a/src/cmd/ksh93/tests/variables.sh b/src/cmd/ksh93/tests/variables.sh index 5c48b2113965..2bc4edc44ba7 100755 --- a/src/cmd/ksh93/tests/variables.sh +++ b/src/cmd/ksh93/tests/variables.sh @@ -69,6 +69,7 @@ done (( rand1 == rand2 )) && err_exit "Test 3: \$RANDOM seed in subshell doesn't change" \ "(both results are $rand1)" # $RANDOM should be reseeded for the ( simple_command & ) optimization +# (which was removed on 2022-06-13, but let's keep the test) for((i=0; i|r1 ( echo $RANDOM & ) >|r2