Zsh Mailing List Archive
Messages sorted by:
Reverse Date,
Date,
Thread,
Author
Re: local_traps doesn't restore traps set from functions
- X-seq: zsh-workers 45801
- From: Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>
- To: Zsh hackers list <zsh-workers@xxxxxxx>
- Subject: Re: local_traps doesn't restore traps set from functions
- Date: Sat, 9 May 2020 19:51:02 +0000
- In-reply-to: <20200509154210.401d82d9@tarpaulin.shahaf.local2>
- List-help: <mailto:zsh-workers-help@zsh.org>
- List-id: Zsh Workers List <zsh-workers.zsh.org>
- List-post: <mailto:zsh-workers@zsh.org>
- List-unsubscribe: <mailto:zsh-workers-unsubscribe@zsh.org>
- Mailing-list: contact zsh-workers-help@xxxxxxx; run by ezmlm
- References: <CAN=4vMrq8S=W+C8Vcp_pZGkc0eNymK3d+x0E3yib6BHu5ZRhDg@mail.gmail.com> <20200507212136.3ff1a843@tarpaulin.shahaf.local2> <1305621354.302160.1589037862169@mail2.virginmedia.com> <20200509154210.401d82d9@tarpaulin.shahaf.local2>
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 <d.s@xxxxxxxxxxxxxxxxxx> 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);
}
Messages sorted by:
Reverse Date,
Date,
Thread,
Author