From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3441 invoked by alias); 11 Oct 2012 14:36:08 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Seq: 30724 Received: (qmail 7513 invoked from network); 11 Oct 2012 14:35:56 -0000 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 Received-SPF: neutral (ns1.primenet.com.au: 209.85.219.43 is neither permitted nor denied by SPF record at ntlworld.com) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:x-originating-ip:in-reply-to:references:date :message-id:subject:from:to:content-type:x-gm-message-state; bh=qVbk/+FIkbWdkPiHRcKE1gX9y0MAeYIj2M1L/GGiTz4=; b=IN1g60cyoLJSoOPXX3Z5fmqq0E685pwAUCBf5us6lxrocFkUtvv7hLrKGB2iQ26PIY oozzQjh6IeFR+JN3aIWL21zJhY3dogxKlqm8RTprlj72Tu+PHbVQ+6g15pwyVw2TvqiS Falo+ANiqaAymxRvNYq2W4Lmqf8GeCs74/ATGxW7hfLEmfBNjpLWCxD+YA5yP81T6Rlx zuFss1gO9R6sHLJEAOaum9yKyErAsKWre17xz8aO0LBdvjPN45J9mh/olx99Mr8U0Gua XdFunKaR2mQJiL9JtyFdo5NjWvjZXUlAYWDmelYa/rkIZvJkas8g8pxXFLFQ79Vfj4c8 ViBw== MIME-Version: 1.0 X-Originating-IP: [80.239.194.50] In-Reply-To: <5076CDC9.9040908@googlemail.com> References: <5076CDC9.9040908@googlemail.com> Date: Thu, 11 Oct 2012 15:35:50 +0100 Message-ID: Subject: Re: Bug in ZSH 5.0.0 - Segmentation fault after sending process to background From: Peter Stephenson To: zsh-workers@zsh.org Content-Type: text/plain; charset=ISO-8859-1 X-Gm-Message-State: ALoCoQkw6HqUqRbsqRyv9t4YGTogC0ZE3LpPgmafuCWd1KBdtr/cvBcmt98k7Meoxh0IIj+c92P4 On 11 October 2012 14:46, BlueC0re wrote: > zsh version 5.0.0 crashes with a segfault if you have a custom precmd > specified (containing an additional command block), and sending a > process to the background. Yes, it does. Thanks for the reproduction information. That's bad, right? I think I understand much of this. Certain very simple commands are optimised to be run by execsimple(), bypassing job control, so "thisjob" won't be set up the way you expect. This causes the observed mayhem if the code actually does look at job control. I've added some tests to ensure thisjob is valid where necessary, and also ensured that it's not valid for code called from execsimple(), since it's never been set up --- or if it has we have no reason to suppose it's a job we're interested in. While I can't guarantee invalidating thisjob in execsimple is harmless, the case here suggests that if it causes anything else that's probably something we want to find out about rather than sweeping under the carpet. Using my own initialisation files I can see an execshfunc() being called from execsimple(). This doesn't strike me as being particularly "simple", although execsimple says the requirement is the code runs entirely within the shell, which is true (code run from the function that doesn't will cause its own jobs to be created). The change in hasprocs() is just there to make debugging a little easier. Index: Src/exec.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/exec.c,v retrieving revision 1.212 diff -p -u -r1.212 exec.c --- Src/exec.c 7 Oct 2012 19:46:46 -0000 1.212 +++ Src/exec.c 11 Oct 2012 14:19:34 -0000 @@ -404,7 +404,17 @@ execcursh(Estate state, int do_exec) /* Skip word only used for try/always */ state->pc++; - if (!list_pipe && thisjob != list_pipe_job && !hasprocs(thisjob)) + /* + * The test thisjob != -1 was added because sometimes thisjob + * can be invalid at this point. The case in question was + * in a precmd function after operations involving background + * jobs. + * + * This is because sometimes we bypass job control to execute + * very simple functions via execssimple(). + */ + if (!list_pipe && thisjob != -1 && thisjob != list_pipe_job && + !hasprocs(thisjob)) deletejob(jobtab + thisjob, 0); cmdpush(CS_CURSH); execlist(state, 1, do_exec); @@ -1064,7 +1074,7 @@ static int execsimple(Estate state) { wordcode code = *state->pc++; - int lv; + int lv, otj; if (errflag) return (lastval = 1); @@ -1075,6 +1085,13 @@ execsimple(Estate state) code = wc_code(*state->pc++); + /* + * Because we're bypassing job control, ensure the called + * code doesn't see the current job. + */ + otj = thisjob; + thisjob = -1; + if (code == WC_ASSIGN) { cmdoutval = 0; addvars(state, state->pc - 1, 0); @@ -1086,6 +1103,8 @@ execsimple(Estate state) } else lv = (execfuncs[code - WC_CURSH])(state, 0); + thisjob = otj; + return lastval = lv; } @@ -4313,7 +4332,9 @@ execshfunc(Shfunc shf, LinkList args) if (errflag) return; - if (!list_pipe && thisjob != list_pipe_job && !hasprocs(thisjob)) { + /* thisjob may be invalid if we're called via execsimple: see execcursh */ + if (!list_pipe && thisjob != -1 && thisjob != list_pipe_job && + !hasprocs(thisjob)) { /* Without this deletejob the process table * * would be filled by a recursive function. */ last_file_list = jobtab[thisjob].filelist; Index: Src/jobs.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/jobs.c,v retrieving revision 1.91 diff -p -u -r1.91 jobs.c --- Src/jobs.c 21 Sep 2012 12:45:29 -0000 1.91 +++ Src/jobs.c 11 Oct 2012 14:19:34 -0000 @@ -209,7 +209,13 @@ findproc(pid_t pid, Job *jptr, Process * int hasprocs(int job) { - Job jn = jobtab + job; + Job jn; + + if (job < 0) { + DPUTS(1, "job number invalid in hasprocs"); + return 0; + } + jn = jobtab + job; return jn->procs || jn->auxprocs; } pws