From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13523 invoked by alias); 11 Apr 2018 12:45:22 -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: List-Unsubscribe: X-Seq: 42624 Received: (qmail 3953 invoked by uid 1010); 11 Apr 2018 12:45:21 -0000 X-Qmail-Scanner-Diagnostics: from park01.gkg.net by f.primenet.com.au (envelope-from , uid 7791) with qmail-scanner-2.11 (clamdscan: 0.99.2/21882. spamassassin: 3.4.1. Clear:RC:0(205.235.26.22):SA:0(-1.4/5.0):. Processed in 15.08761 secs); 11 Apr 2018 12:45:21 -0000 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-1.4 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS, SPF_PASS,T_DKIM_INVALID autolearn=no autolearn_force=no version=3.4.1 X-Envelope-From: SRS0=bS7L=HA=yahoo.co.uk=okiddle@bounces.park01.gkg.net X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | X-Virus-Scanned: by amavisd-new at gkg.net Authentication-Results: amavisd4.gkg.net (amavisd-new); dkim=pass (2048-bit key) header.d=yahoo.co.uk X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.co.uk; s=s2048; t=1523450682; bh=OLzOOLUKyD4MtBUbZQNp59WqdpwpmC/LY2ZCjAUqmx4=; h=From:To:Subject:Date:From:Subject; b=QOaZjCI1q+iLximsl2sBs0yxWHY2TyoWYr2U94EzLVI6bs9vKFDHi0hCfoRHbyYC6Ux8Iav/dN30L3tt+TXNnyj9onsPB4ctcKViU67t/mib7kOIyAJ6ZskESPSx/0DuTHtWdwGO6ecUrwmrfYp+MB7SD7K0qAGb8gDj883Frv/+TfwjmG7XNDbiRjqSijSVNktaZBx5we9QlUS4Cd3RzG6oWMGzqvIXfsVYRkyjG0Mka9IYBAAyzaEm1fRFooipEIuRPR/Gxq1eFg5O20ZgP8BVrNzP7p8fhKKUN42EsDMw/FgKtSVZWIn9zfUeMd7K+rIcFa0Gc8uXlLgM7zezWw== X-YMail-OSG: _cusT2QVM1mDzVddtGUzZkKbh1Ie4cnm9ye1lsmhlQhtLHprK6Fqt0SLNt5FVde 65nyrXAfLkHy0WfdBSQmWXmBqxTr_huXL_18H2JjtMx7.4em.kUePvv9vBc2hRM1On2grRgs.IM. IiAMl0nxxtrhDXL3w3.YMRBDHz.rRv2jHv5RlLkLooii02sReaBlEJhKZs_gaW9W95xaImKGs3h. faobGjoQHfwIQEgrYAh_aArWbX35g4pcDh4q3hsP7gMr0s4ttJwocPlru0pbEa1.yrTxrE1KLoqD yXGxtBipgFlJT4tcais9K2yVjc8waiHscoSM0ot3T.2WMZ_xLwfaPd8xA3enm6JkBuKNVRvPeobH Suz.okYAtfI47RFdYrsiz..n5HfYTcWPP66OzPInVzc8PSI5Sc5BuziFEjVEBVn0GWplySH0.wqd zDn9Tqw3KtCi7uzqXoVfaPwqVd_cpeGoYxX5qsi8Dl1BtuBnJmIoKpcpJR05e62n8FCISrdXvbwq iWz5uxNzJBw-- From: Oliver Kiddle To: Zsh workers Subject: PATCH: bug with redirections on multiple function definitions MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-ID: <6840.1523449402.1@thecus> Date: Wed, 11 Apr 2018 14:23:22 +0200 Message-ID: <6841.1523449402@thecus> My starting point was a seg fault but the easiest demonstration is the following. It may not come out exactly like this depending on your memory allocator because we are reading freed memory - the redirection on b is undefined. a a b() { body } < redirection functions b b () { body }body execfuncdef() is passed the redirection in redir_prog. For the first function defined, this is copied into the Shfunc structure but a copy is taken for subsequent functions. However, we have a duplicate name and in order to add a() a second time, the existing a() is removed which frees the memory pointed to by redir_prog. This typically gets reused when making a copy of the function body for b(). The fix below is to use the original value of redir_prog for the last function added. The nfunc variable was used to determine if we were on the first function name. I've replaced that with the nonempty test on the list and assign 0 to redir_prog when using it which perhaps better reflects the move semantics. Oliver diff --git a/Src/exec.c b/Src/exec.c index e154d1249..216057aa7 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -5042,7 +5042,7 @@ execfuncdef(Estate state, Eprog redir_prog) Shfunc shf; char *s = NULL; int signum, nprg, sbeg, nstrs, npats, len, plen, i, htok = 0, ret = 0; - int nfunc = 0, anon_func = 0; + int anon_func = 0; Wordcode beg = state->pc, end; Eprog prog; Patprog *pp; @@ -5118,12 +5118,16 @@ execfuncdef(Estate state, Eprog redir_prog) /* * redir_prog is permanently allocated --- but if * this function has multiple names we need an additional - * one. + * one. Original redir_prog used with the last name + * because earlier functions are freed in case of duplicate + * names. */ - if (nfunc++ && redir_prog) + if (names && nonempty(names) && redir_prog) shf->redir = dupeprog(redir_prog, 0); - else + else { shf->redir = redir_prog; + redir_prog = 0; + } shfunc_set_sticky(shf); if (!names) { @@ -5203,7 +5207,7 @@ execfuncdef(Estate state, Eprog redir_prog) } if (!anon_func) setunderscore(""); - if (!nfunc && redir_prog) { + if (redir_prog) { /* For completeness, shouldn't happen */ freeeprog(redir_prog); }