From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=1.0 required=5.0 tests=MAILING_LIST_MULTI, PDS_OTHER_BAD_TLD,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.4 Received: (qmail 3017 invoked from network); 9 May 2020 19:51:58 -0000 Received-SPF: pass (primenet.com.au: domain of zsh.org designates 203.24.36.2 as permitted sender) receiver=inbox.vuxu.org; client-ip=203.24.36.2 envelope-from= Received: from ns1.primenet.com.au (HELO primenet.com.au) (203.24.36.2) by inbox.vuxu.org with ESMTPUTF8; 9 May 2020 19:51:58 -0000 Received: (qmail 12935 invoked by alias); 9 May 2020 19:51:49 -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: 45801 Received: (qmail 15521 invoked by uid 1010); 9 May 2020 19:51:49 -0000 X-Qmail-Scanner-Diagnostics: from out1-smtp.messagingengine.com by f.primenet.com.au (envelope-from , uid 7791) with qmail-scanner-2.11 (clamdscan: 0.102.2/25801. spamassassin: 3.4.4. Clear:RC:0(66.111.4.25):SA:0(0.9/5.0):. Processed in 5.055975 secs); 09 May 2020 19:51:49 -0000 X-Envelope-From: d.s@daniel.shahaf.name X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | Received-SPF: none (ns1.primenet.com.au: domain at daniel.shahaf.name does not designate permitted sender hosts) X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduhedrkeehgddugeduucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucenucfjughrpeffhffvuffkjghfofggtgfgsehtje dttdertddvnecuhfhrohhmpeffrghnihgvlhcuufhhrghhrghfuceougdrshesuggrnhhi vghlrdhshhgrhhgrfhdrnhgrmhgvqeenucggtffrrghtthgvrhhnpeefudetgeevhedvhf etveetvdduleduieejueduueejjedtteeutdejhfdtgfeiteenucfkphepuddtledrieei rdduhedrvdefleenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfh hrohhmpegurdhssegurghnihgvlhdrshhhrghhrghfrdhnrghmvg X-ME-Proxy: Date: Sat, 9 May 2020 19:51:02 +0000 From: Daniel Shahaf To: Zsh hackers list Subject: Re: local_traps doesn't restore traps set from functions Message-ID: <20200509195102.6d6f5095@tarpaulin.shahaf.local2> In-Reply-To: <20200509154210.401d82d9@tarpaulin.shahaf.local2> References: <20200507212136.3ff1a843@tarpaulin.shahaf.local2> <1305621354.302160.1589037862169@mail2.virginmedia.com> <20200509154210.401d82d9@tarpaulin.shahaf.local2> X-Mailer: Claws Mail 3.17.3 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Daniel Shahaf wrote on Sat, 09 May 2020 15:42 +0000: > Peter Stephenson wrote on Sat, 09 May 2020 16:24 +0100: > > > On 07 May 2020 at 22:21 Daniel Shahaf wrote: > > > Roman Perepelitsa wrote on Wed, 06 May 2020 15:31 +0200: > > > > It appears that local_traps doesn't restore traps that were originally > > > > set from functions. > > > > > > Precisely. > > > > > > The following patch fixes it: > > > > OK, so the point here is that in this case the trap we're fiddling with > > is not going out of scope, nor having something else restored over it, > > it just happens to have been created inside a function. At this > > point, that fact becomes irrelevant, so we simply massage the information > > to make it behave as if it had been created at the current (new) function > > depth. > > > > That seems fine --- my only comment would be it would probably be sensible > > to insert some edited version of the previous paragraph as I for one > > am almost certain to wonder what's going on if I encounter this code > > in the future. > > Will do. Thanks for the review! Here's an interdiff, to be applied on top of 45790. Test will follow separately. diff --git a/Src/signals.c b/Src/signals.c index 5d0bae7f5..56b3071de 100644 --- a/Src/signals.c +++ b/Src/signals.c @@ -30,8 +30,16 @@ #include "zsh.mdh" #include "signals.pro" -/* Array describing the state of each signal: an element contains * - * 0 for the default action or some ZSIG_* flags ored together. */ +/* + * Array describing the state of each signal: an element contains + * 0 for the default action or some ZSIG_* flags ored together. + * + * The high bits (see ZSIG_SHIFT) identify the locallevel that owns the trap. + * Whenever execution returns to that locallevel (which is never larger/deeper + * than the current locallevel) from deeper levels, any function-scoped traps + * that shadow this trap will have gone out of scope. See the LOCALTRAPS + * condition in removetrap(). + */ /**/ mod_export int sigtrapped[VSIGCOUNT]; @@ -997,9 +1005,12 @@ removetrap(int sig) queue_signals(); trapped = sigtrapped[sig]; /* + * Save the trap if we should restore it at the end of the current function. + * * Note that we save the trap here even if there isn't an existing - * one, to aid in removing this one. However, if there's - * already one at the current locallevel we just overwrite it. + * one, to aid in removing this one. We also save the trap if it's owned + * by someone up the callstack. However, if the trap is owned by the + * current locallevel we just overwrite it. * * Note we save EXIT traps based on the *current* setting of * POSIXTRAPS --- so if there is POSIX EXIT trap set but @@ -1062,6 +1073,16 @@ removetrap(int sig) return NULL; } +/* + * Return the value of ARG after setting all but the N least significant bits + * to zero. + */ +static unsigned int +low_N_bits_of(unsigned int arg, unsigned char N) +{ + return arg & ((1u << N)-1u); +} + /**/ void starttrapscope(void) @@ -1114,6 +1135,9 @@ endtrapscope(void) sigtrapped[SIGEXIT] = 0; } + /* + * Restore traps from savetraps to sigtrapped. + */ if (savetraps) { while ((ln = firstnode(savetraps)) && (st = (struct savetrap *) ln->dat) && @@ -1122,6 +1146,8 @@ endtrapscope(void) remnode(savetraps, ln); + /* ### This doesn't mask ZSIG_IGNORED out of the boolean check, + * ### presumably because checking st->list is sufficient? */ if (st->flags && (st->list != NULL)) { /* prevent settrap from saving this */ dontsavetrap++; @@ -1154,7 +1180,11 @@ endtrapscope(void) } } - /* Fixup locallevel of signals. */ + /* + * If the function that's in the process of returning set a global trap, + * that trap is now owned by that function's caller. Update sigtrapped + * accordingly. + */ { int i; for (i = 0; i < VSIGCOUNT; ++i) { @@ -1170,7 +1200,7 @@ endtrapscope(void) * Keep the low ZSIG_SHIFT bits unchanged. */ sigtrapped[i] = - (sigtrapped[i] & ((1u << ZSIG_SHIFT) - 1u)) + low_N_bits_of(sigtrapped[i], ZSIG_SHIFT) | (locallevel << ZSIG_SHIFT); }