zsh-workers
 help / color / mirror / code / Atom feed
From: Daniel Shahaf <d.s@daniel.shahaf.name>
To: Roman Perepelitsa <roman.perepelitsa@gmail.com>
Cc: Zsh hackers list <zsh-workers@zsh.org>
Subject: Re: local_traps doesn't restore traps set from functions
Date: Thu, 7 May 2020 21:21:36 +0000	[thread overview]
Message-ID: <20200507212136.3ff1a843@tarpaulin.shahaf.local2> (raw)
In-Reply-To: <CAN=4vMrq8S=W+C8Vcp_pZGkc0eNymK3d+x0E3yib6BHu5ZRhDg@mail.gmail.com>

Roman Perepelitsa wrote on Wed, 06 May 2020 15:31 +0200:
> Is this working as intended?
> 
>   ❯ zsh -f
>   adam% () { trap '' INT }
>   adam% () { emulate -L zsh; trap 'echo INT' INT }
>   adam% kill -INT $$
>   INT
>   adam%
> 

I'd say it doesn't, since the trap _is_ set before the second function is called —
.
    % () { trap '' INT }
    % kill -INT $$
    % 
.
— and the documentation is quite clear about the semantics in this case:

       LOCAL_TRAPS <K>
              If this option is set when a signal trap is set inside a
              function, then the previous status of the trap for that signal
              will be restored when the function exits.

> It appears that local_traps doesn't restore traps that were originally
> set from functions.

Precisely.

The following patch fixes it:

[[[
diff --git a/Src/signals.c b/Src/signals.c
index 4adf03202..3e4fdf370 100644
--- a/Src/signals.c
+++ b/Src/signals.c
@@ -1154,6 +1154,29 @@ endtrapscope(void)
 	}
     }
 
+    /* Fixup locallevel of signals. */
+    {
+	int i;
+	for (i = 0; i < VSIGCOUNT; ++i) {
+	    int locallevel_of_signal = (sigtrapped[i] >> ZSIG_SHIFT);
+	    if (locallevel_of_signal > locallevel) {
+		DPUTS3(locallevel_of_signal != locallevel + 1,
+		    "BUG: signal %s's locallevel unexpected: %d>>ZSIG_SHIFT seen, v. %d+1 expected",
+		    sigs[i], sigtrapped[i], locallevel);
+
+		/* 
+		 * Decrement the locallevel embedded in sigtrapped[i].
+		 *
+		 * Keep the low ZSIG_SHIFT bits unchanged.
+		 */
+		sigtrapped[i] =
+		    (sigtrapped[i] & ((1u << ZSIG_SHIFT) - 1u))
+		    |
+		    (locallevel << ZSIG_SHIFT);
+	    }
+	}
+    }
+
     if (exittr) {
 	/*
 	 * We already made sure this wasn't set as a POSIX exit trap.
diff --git a/Src/zsh.h b/Src/zsh.h
index 1f2d774a1..71ba6e6e0 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -2940,8 +2940,8 @@ struct heap {
 #define ZSIG_FUNC	(1<<2)	/* Trap is a function, not an eval list */
 /* Mask to get the above flags */
 #define ZSIG_MASK	(ZSIG_TRAPPED|ZSIG_IGNORED|ZSIG_FUNC)
-/* No. of bits to shift local level when storing in sigtrapped */
 #define ZSIG_ALIAS	(1<<3)  /* Trap is stored under an alias */
+/* No. of bits to shift locallevel when storing in sigtrapped */
 #define ZSIG_SHIFT	4
 
 /*
]]]

[[[
% () { trap '' INT }
% () { emulate -L zsh; trap 'echo INT' INT }
% kill -INT $$
% 
]]]

However, I don't know this area of the code well enough to be sure the
patch doesn't break anything.  Reviews would be appreciated.

Also, if someone could write a regression test for this, that'd be great.

Cheers,

Daniel

  reply	other threads:[~2020-05-07 21:22 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-06 13:31 Roman Perepelitsa
2020-05-07 21:21 ` Daniel Shahaf [this message]
2020-05-08  6:51   ` Roman Perepelitsa
2020-05-09 14:14     ` Daniel Shahaf
2020-05-09 15:24   ` Peter Stephenson
2020-05-09 15:42     ` Daniel Shahaf
2020-05-09 19:51       ` Daniel Shahaf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200507212136.3ff1a843@tarpaulin.shahaf.local2 \
    --to=d.s@daniel.shahaf.name \
    --cc=roman.perepelitsa@gmail.com \
    --cc=zsh-workers@zsh.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).