Re: Re : Re: Re : [BUG] Crash due to malloc call in signal handler

Re: Re : Re: Re : [BUG] Crash due to malloc call in signal handler

[Peter Stephenson]

On Tue, 2019-12-17 at 19:32 +0100, Antoine C. wrote:
> Good news(!), I finally get another crash with a backtrace, despite 
> Peter's last patch. This one, like the previous, is triggered by debug
> code because of two simultaneous free() calls from main and signal 
> contexts.

Great, thanks.

OK, the suspect here is the arithmetic code --- it looks like it's
running unprotected against signals, despite the fact it can set
variables.  Arithmetic is a quick operation, so hopefully we can block
fairly high up the stack.  I should get a chance to look later
(but I don't think this is rocket science from this point on so maybe
someone will beat me to it).

pws

> #9  0x000055e115ce40f4 in zhandler (sig=17) at signals.c:648
...
> #23 0x000055e115c7cddf in execarith (state=0x7ffea5b39110, do_exec=0) at exec.c:5111

Re: Re : Re: Re : [BUG] Crash due to malloc call in signal handler

[Peter Stephenson]

On Wed, 2019-12-18 at 10:01 +0000, Peter Stephenson wrote:
> OK, the suspect here is the arithmetic code --- it looks like it's
> running unprotected against signals, despite the fact it can set
> variables.  Arithmetic is a quick operation, so hopefully we can block
> fairly high up the stack.  I should get a chance to look later
> (but I don't think this is rocket science from this point on so maybe
> someone will beat me to it).
> 
> pws
> 
> > 
> > #9  0x000055e115ce40f4 in zhandler (sig=17) at signals.c:648
> ...
> > 
> > #23 0x000055e115c7cddf in execarith (state=0x7ffea5b39110, do_exec=0) at exec.c:5111

Yes, it really does look this simple.  All top-level "exec" functions
should be able to queue and unqueue signals without side effects, any
issues being handled lower down, and execarith() is a good deal simpler
than most of the others (which is probably why it never acquired the
protection in the first place).

I'll commit this fairly quickly --- it's obviously needed and in the
event of side effects I'd rather find out sooner than later.

pws

diff --git a/Src/exec.c b/Src/exec.c
index 50027654a..fac095d64 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -5101,6 +5101,7 @@ execarith(Estate state, UNUSED(int do_exec))
     mnumber val = zero_mnumber;
     int htok = 0;
 
+    queue_signals();
     if (isset(XTRACE)) {
 	printprompt4();
 	fprintf(xtrerr, "((");
@@ -5120,6 +5121,8 @@ execarith(Estate state, UNUSED(int do_exec))
 	fprintf(xtrerr, " ))\n");
 	fflush(xtrerr);
     }
+    unqueue_signals();
+
     if (errflag) {
 	errflag &= ~ERRFLAG_ERROR;
 	return 2;

Re : Re: Re : Re: Re : [BUG] Crash due to malloc call in signal handler

[Antoine C.]

> I'll commit this fairly quickly --- it's obviously needed and in the
> event of side effects I'd rather find out sooner than later.

> diff --git a/Src/exec.c b/Src/exec.c
> index 50027654a..fac095d64 100644
> [...]

Just a quick update regarding the malloc crash: I did not 
experienced any more crash after applying this patch and 
the previous one.

Thanks !
Antoine