On 10/28/18 8:13 PM, Anton Lindqvist wrote:
> Hi,
> Bug reported by miod@, how to reproduce:
>
> $ command -v r
> alias r='fc -s'
> $ sleep 5
> $ r sleep
> ^C # abort sleep before finishing
> $ r sleep
> ksh: fc: history function called recursively
>
> The c_fc() function has some internal state used to prevent recursive
> invocations that gets out of sync when the re-executed command is
> aborted by SIGINT. My proposal is to temporarily register a SIGINT
> handler before re-executing the command in order to reset the call depth
> counter upon signal delivery.
>
> Comments? OK?
>
You diff always unsets the shtrap pointer, which most likely unsets
custom SIGINT handlers installed by the user (untested). Next to that
this feels like special-casing and I'm afraid that there are some cases
we might be overlooking here.
The source with this issue is that a SIGINT triggers a list of
siglongjmps, which don't allow us to return to c_fc and decrement the
pointer. The diff below detects if we're on the lowest level of the
unwinding by abusing the interactive variable. This works because
calling fc from a non-interactive session is prohibited.
I'm not 100% convinced that the placement of fc_depth = 0 should be
placed in shell(), or that we maybe should place it in unwind(). The
reason I choose for shell() is because interactive is guaranteed to
be setup for checking, because it's already there.
This diff is only lightly tested and the code here is very hard to
untangle, so extreme scrutiny is desirable.
martijn@
Index: history.c
===================================================================
RCS file: /cvs/src/bin/ksh/history.c,v
retrieving revision 1.80
diff -u -p -r1.80 history.c
--- history.c 15 Jan 2018 22:30:38 -0000 1.80
+++ history.c 29 Oct 2018 08:19:12 -0000
@@ -48,6 +48,8 @@ static uint32_t line_co;
static struct stat last_sb;
+volatile sig_atomic_t fc_depth;
+
int
c_fc(char **wp)
{
@@ -58,9 +60,8 @@ c_fc(char **wp)
int optc, ret;
char *first = NULL, *last = NULL;
char **hfirst, **hlast, **hp;
- static int depth;
- if (depth != 0) {
+ if (fc_depth != 0) {
bi_errorf("history function called recursively");
return 1;
}
@@ -146,9 +147,9 @@ c_fc(char **wp)
hist_get_newest(false);
if (!hp)
return 1;
- depth++;
+ fc_depth++;
ret = hist_replace(hp, pat, rep, gflag);
- depth--;
+ fc_depth--;
return ret;
}
@@ -268,9 +269,9 @@ c_fc(char **wp)
shf_close(shf);
*xp = '\0';
strip_nuls(Xstring(xs, xp), Xlength(xs, xp));
- depth++;
+ fc_depth++;
ret = hist_execute(Xstring(xs, xp));
- depth--;
+ fc_depth--;
return ret;
}
}
Index: main.c
===================================================================
RCS file: /cvs/src/bin/ksh/main.c,v
retrieving revision 1.93
diff -u -p -r1.93 main.c
--- main.c 29 Sep 2018 14:13:19 -0000 1.93
+++ main.c 29 Oct 2018 08:19:12 -0000
@@ -549,6 +549,7 @@ shell(Source *volatile s, volatile int t
case LERROR:
case LSHELL:
if (interactive) {
+ fc_depth = 0;
if (i == LINTR)
shellf("\n");
/* Reset any eof that was read as part of a
Index: sh.h
===================================================================
RCS file: /cvs/src/bin/ksh/sh.h,v
retrieving revision 1.73
diff -u -p -r1.73 sh.h
--- sh.h 18 May 2018 13:25:20 -0000 1.73
+++ sh.h 29 Oct 2018 08:19:12 -0000
@@ -249,6 +249,7 @@ extern volatile sig_atomic_t intrsig; /*
extern volatile sig_atomic_t fatal_trap; /* received a fatal signal */
extern volatile sig_atomic_t got_sigwinch;
extern Trap sigtraps[NSIG+1];
+extern volatile sig_atomic_t fc_depth; /* c_fc depth counter */
/*
* TMOUT support
