From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28184 invoked by alias); 16 May 2017 11:05:41 -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: 41113 Received: (qmail 26023 invoked from network); 16 May 2017 11:05:41 -0000 X-Qmail-Scanner-Diagnostics: from mailout1.w1.samsung.com 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(210.118.77.11):SA:0(-5.0/5.0):. Processed in 1.125566 secs); 16 May 2017 11:05:41 -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=-5.0 required=5.0 tests=RCVD_IN_DNSWL_HI, RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=unavailable autolearn_force=no version=3.4.1 X-Envelope-From: p.stephenson@samsung.com X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | Received-SPF: none (ns1.primenet.com.au: domain at samsung.com does not designate permitted sender hosts) X-AuditID: cbfec7f2-f797e6d000004438-91-591adcfc3a04 Date: Tue, 16 May 2017 12:05:27 +0100 From: Peter Stephenson To: zsh-workers@zsh.org Subject: Re: zsh 5.3.1 crashes on completion Message-id: <20170516120527.572a5cc9@pwslap01u.europe.root.pri> In-reply-to: <20170516104831.14aa23c6@pwslap01u.europe.root.pri> Organization: Samsung Cambridge Solution Centre X-Mailer: Claws Mail 3.7.9 (GTK+ 2.22.0; i386-redhat-linux-gnu) MIME-version: 1.0 Content-type: text/plain; charset=US-ASCII Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrFIsWRmVeSWpSXmKPExsWy7djPc7p/7khFGqx+KmxxsPkhkwOjx6qD H5gCGKO4bFJSczLLUov07RK4Mh6cu8FWMNOgYtqa/8wNjF9Uuhg5OSQETCS6/69ih7DFJC7c W88GYgsJLGWUuLjfoouRC8juZZJ4vfgzE0zDhmsX2SESyxglTlxZwQrRMY1JYs8eN4jEGUaJ Rdt/QFWdZZTo2DuZBaSKRUBV4uTO42A2m4ChxNRNsxlBbBEBcYmza88DxTk4hAV0JPbvLwAx eQXsJVZsAlvMKeAgsXTjSjCbX0Bf4urfT1AH2UvMvHIGbAqvgKDEj8n3wKYzA03Ztu0xO4Qt L7F5zVtmkHMkBP6zSfTt/csOMl9CQFZi0wFmiDkuEv3z7kJDQlji1fEtULaMxOXJ3SwQdj+j xJNuX4g5MxglTp/ZwQaRsJbou32REWIZn8SkbdOZIebzSnS0CUGUeEgsftnECGE7SuyfMpN1 AqPiLCRnz0Jy9iwkZy9gZF7FKJJaWpybnlpsrFecmFtcmpeul5yfu4kRmAJO/zv+aQfj1xNW hxgFOBiVeHhXrJCMFGJNLCuuzD3EKMHBrCTCm3ZBKlKINyWxsiq1KD++qDQntfgQozQHi5I4 L9epaxFCAumJJanZqakFqUUwWSYOTqkGRn+PFreV8f4/jW7FfWR/MKuvKP2ZvuGtRS4Xl/z4 +Pxi/Dvv41Pdr0x+G/TkeQzf5HnhU3uq+F8snKO68aSfuZQbr1DgdAXDFZfk5nXXLG7fcuNs 8IfD/XZLvgdqPGN0sFklETh3vfzCquTd23zSZ147rPfhytRFJqssD9TMkhM6FFUipS7ytUOJ pTgj0VCLuag4EQA9uvyl/QIAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrEIsWRmVeSWpSXmKPExsVy+t/xy7oyd6UiDW5MFLU42PyQyYHRY9XB D0wBjFFuNhmpiSmpRQqpecn5KZl56bZKoSFuuhZKCnmJuam2ShG6viFBSgpliTmlQJ6RARpw cA5wD1bSt0twy3hw7gZbwUyDimlr/jM3MH5R6WLk5JAQMJHYcO0iO4QtJnHh3no2EFtIYAmj xLWXZRD2DCaJrZejuhi5gOxzjBJXnl+AKjrLKHFhYRqIzSKgKnFy53EWEJtNwFBi6qbZjCC2 iIC4xNm154HiHBzCAjoS+/cXgJi8AvYSKzYxgVRwCjhILN24kgli/Bdmia9Hr4C18gvoS1z9 +4kJ4jZ7iZlXzoDFeQUEJX5Mvge2illAS2LztiZWCFteYvOat8wQp6lL3Li7m30Co/AsJC2z kLTMQtKygJF5FaNIamlxbnpusZFecWJucWleul5yfu4mRmD8bDv2c8sOxq53wYcYBTgYlXh4 V6yQjBRiTSwrrsw9xCjBwawkwpt2QSpSiDclsbIqtSg/vqg0J7X4EKMpMFwmMkuJJucDYzuv JN7QxNDc0tDI2MLC3MhISZx36ocr4UIC6YklqdmpqQWpRTB9TBycUg2MJQvjD7dd1Fc8J1Dw 69fq70cOtjbO9XD0/lk5YdpzZin2lemHlt3eebM5W0Lh3U3ta52Pd1mlBkb5H1rn4C6SpMHx sSL8/RTjxP7Xq4OLlVSqjC2trG9sdM9qkuqOkXBYHvwovbynfJ/d78ebVdKO+B66Kyd7JTXs VZ74BI0jM7JkYo6y671QYinOSDTUYi4qTgQANSCSGrUCAAA= X-MTR: 20000000000000000@CPGS X-CMS-MailID: 20170516110531eucas1p23521af666eb632e60e775d944e4b133a X-Msg-Generator: CA X-Sender-IP: 182.198.249.180 X-Local-Sender: =?UTF-8?B?UGV0ZXIgU3RlcGhlbnNvbhtTQ1NDLURhdGEgUGxhbmUb?= =?UTF-8?B?7IK87ISx7KCE7J6QG1ByaW5jaXBhbCBFbmdpbmVlciwgU29mdHdhcmU=?= X-Global-Sender: =?UTF-8?B?UGV0ZXIgU3RlcGhlbnNvbhtTQ1NDLURhdGEgUGxhbmUbU2Ft?= =?UTF-8?B?c3VuZyBFbGVjdHJvbmljcxtQcmluY2lwYWwgRW5naW5lZXIsIFNvZnR3YXJl?= X-Sender-Code: =?UTF-8?B?QzEwG0VIURtDMTBDRDA1Q0QwNTAwNTg=?= CMS-TYPE: 201P X-HopCount: 7 X-CMS-RootMailID: 20170513182335epcas3p2ddebe4bda1f5c273cce7a5902d621aa7 X-RootMTR: 20170513182335epcas3p2ddebe4bda1f5c273cce7a5902d621aa7 References: <170511141903.ZM13639@torch.brasslantern.com> <20170512161603.GA16688@fujitsu.shahaf.local2> <783A691B-B69F-4673-B237-B316CEF26346@remrain.com> <170513112313.ZM14712@torch.brasslantern.com> <20170515102819.39284ef6@pwslap01u.europe.root.pri> <170515133354.ZM20191@torch.brasslantern.com> <20170516104831.14aa23c6@pwslap01u.europe.root.pri> On Tue, 16 May 2017 10:48:31 +0100 Peter Stephenson wrote: > zcontext_save() in loop() will run, since this isn't a top-level loop, > which will save and clear chline. So it should be the case the the loop > we have for fc has a clean history state. So I wonder if something else to do > with the state (that maybe we should be saving and clearing at the same > time?) is screwy owing to not having finished the top-level history > line. (BTW I needed to use standard malloc for this, it wasn't showing up with zsh's own.) Hmm... I think the bad free comes because curline, which points to chline, is part of the history ring and is being freed when we free that. So at hend() chline is already free. Background: curline is a static structure with direct references to the current history line. We insert it into the history ring for convenience in searching as necessary. However, its memory status is different from that of other history ring entries, which is obviously a hostage to fortune. This attempts to be smarter about linking curline into the history ring and unlinking it. This seems to have good effects, but definitely needs a further look over. pws diff --git a/Src/hashtable.c b/Src/hashtable.c index ba5faad..c34744c 100644 --- a/Src/hashtable.c +++ b/Src/hashtable.c @@ -1448,6 +1448,7 @@ freehistdata(Histent he, int unlink) if (!(he->node.flags & (HIST_DUP | HIST_TMPSTORE))) removehashnode(histtab, he->node.nam); + DPUTS(he->node.nam == chline, "Attempt to free chline in history data"); zsfree(he->node.nam); if (he->nwords) zfree(he->words, he->nwords*2*sizeof(short)); diff --git a/Src/hist.c b/Src/hist.c index 350688d..636c315 100644 --- a/Src/hist.c +++ b/Src/hist.c @@ -87,6 +87,9 @@ mod_export zlong curhist; /**/ struct histent curline; +/***/ +int curline_linked; + /* current line count of allocated history entries */ /**/ @@ -261,6 +264,9 @@ hist_context_save(struct hist_stack *hs, int toplevel) */ hs->cstack = cmdstack; hs->csp = cmdsp; + hs->curline_linked = curline_linked; + + unlinkcurline(); stophist = 0; chline = NULL; @@ -300,6 +306,10 @@ hist_context_restore(const struct hist_stack *hs, int toplevel) zfree(cmdstack, CMDSTACKSZ); cmdstack = hs->cstack; cmdsp = hs->csp; + unlinkcurline(); + curline_linked = hs->curline_linked; + if (curline_linked) + linkcurline(); } /* @@ -990,6 +1000,7 @@ nohwe(void) /* these functions handle adding/removing curline to/from the hist_ring */ +/**/ static void linkcurline(void) { @@ -1002,11 +1013,15 @@ linkcurline(void) hist_ring = &curline; } curline.histnum = ++curhist; + curline_linked = 1; } +/**/ static void unlinkcurline(void) { + if (!curline_linked) + return; curline.up->down = curline.down; curline.down->up = curline.up; if (hist_ring == &curline) { @@ -1016,6 +1031,7 @@ unlinkcurline(void) hist_ring = curline.up; } curhist--; + curline_linked = 0; } /* initialize the history mechanism */ @@ -1035,6 +1051,7 @@ hbegin(int dohist) stophist = (!interact || unset(SHINSTDIN)) ? 2 : 0; else stophist = 0; + DPUTS(chline != NULL, "chline set at start of history"); /* * pws: We used to test for "|| (inbufflags & INP_ALIAS)" * in this test, but at this point we don't have input @@ -1292,11 +1309,10 @@ putoldhistentryontop(short keep_going) Histent prepnexthistent(void) { - Histent he; - int curline_in_ring = hist_ring == &curline; + Histent he; + int relink_curline = curline_linked; - if (curline_in_ring) - unlinkcurline(); + unlinkcurline(); if (hist_ring && hist_ring->node.flags & HIST_TMPSTORE) { curhist--; freehistnode(&hist_ring->node); @@ -1320,7 +1336,7 @@ prepnexthistent(void) he = hist_ring; } he->histnum = ++curhist; - if (curline_in_ring) + if (relink_curline) linkcurline(); return he; } @@ -1395,8 +1411,7 @@ hend(Eprog prog) queue_signals(); if (histdone & HISTFLAG_SETTY) settyinfo(&shttyinfo); - if (!(histactive & HA_NOINC)) - unlinkcurline(); + unlinkcurline(); if (histactive & HA_NOINC) { zfree(chline, hlinesz); zfree(chwords, chwordlen*sizeof(short)); @@ -3624,7 +3639,7 @@ int pushhiststack(char *hf, zlong hs, zlong shs, int level) { struct histsave *h; - int curline_in_ring = (histactive & HA_ACTIVE) && hist_ring == &curline; + int relink_curline = curline_linked; if (histsave_stack_pos == histsave_stack_size) { histsave_stack_size += 5; @@ -3632,8 +3647,7 @@ pushhiststack(char *hf, zlong hs, zlong shs, int level) histsave_stack_size * sizeof (struct histsave)); } - if (curline_in_ring) - unlinkcurline(); + unlinkcurline(); h = &histsave_stack[histsave_stack_pos++]; @@ -3668,7 +3682,7 @@ pushhiststack(char *hf, zlong hs, zlong shs, int level) savehistsiz = shs; inithist(); /* sets histtab */ - if (curline_in_ring) + if (relink_curline) linkcurline(); return histsave_stack_pos; @@ -3680,13 +3694,12 @@ int pophiststack(void) { struct histsave *h; - int curline_in_ring = (histactive & HA_ACTIVE) && hist_ring == &curline; + int relink_curline = curline_linked; if (histsave_stack_pos == 0) return 0; - if (curline_in_ring) - unlinkcurline(); + unlinkcurline(); deletehashtable(histtab); zsfree(lasthist.text); @@ -3709,7 +3722,7 @@ pophiststack(void) histsiz = h->histsiz; savehistsiz = h->savehistsiz; - if (curline_in_ring) + if (relink_curline) linkcurline(); return histsave_stack_pos + 1; diff --git a/Src/zsh.h b/Src/zsh.h index 22f73f8..405b274 100644 --- a/Src/zsh.h +++ b/Src/zsh.h @@ -2931,6 +2931,7 @@ struct hist_stack { void (*addtoline) _((int)); unsigned char *cstack; int csp; + int curline_linked; }; /*