zsh-workers
 help / color / mirror / code / Atom feed
* Probabilistic crash on zsh 5.9 on x86_64
@ 2023-04-08 17:28 zsh bug report throwaway email thing
  2023-04-08 21:36 ` Mikael Magnusson
  0 siblings, 1 reply; 14+ messages in thread
From: zsh bug report throwaway email thing @ 2023-04-08 17:28 UTC (permalink / raw)
  To: zsh-workers

[-- Attachment #1: Type: text/plain, Size: 807 bytes --]

Hello,

I would like to report a bug in zsh 5.9 (x86_64-pc-linux-gnu) (on Arch Linux, but I also reproduced on Alpine in QEMU, so it is probably zsh and not libc. Also, this does not happen in archiso in qemu, which is also weird.).

Repro instructions: run the commands:
TRAPEXIT() { ls }
TRAPEXIT
# if that does not crash, keep typing TRAPEXIT until it does. sometimes it doesn't crash.

Expected behavior: zsh might throw an error or something? but it shouldn't crash
Actual behavior: there is an unknown chance that zsh throws an error and crashes like this:
zsh: TRAPEXIT: function not defined by file
malloc(): unaligned tcache chunk detected
[1] 34511 IOT instruction (core dumped) zsh

Nothing relevant in dmesg.

If I can help in any way, please contact me.

Kind regards,
an anonymous bug reporter

[-- Attachment #2: Type: text/html, Size: 2646 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Probabilistic crash on zsh 5.9 on x86_64
  2023-04-08 17:28 Probabilistic crash on zsh 5.9 on x86_64 zsh bug report throwaway email thing
@ 2023-04-08 21:36 ` Mikael Magnusson
  2023-04-11 16:14   ` Jun. T
  0 siblings, 1 reply; 14+ messages in thread
From: Mikael Magnusson @ 2023-04-08 21:36 UTC (permalink / raw)
  To: zsh bug report throwaway email thing; +Cc: zsh-workers

On 4/8/23, zsh bug report throwaway email thing
<zsh.throwaway.eDjb3nwqw@proton.me> wrote:
> Hello,
>
> I would like to report a bug in zsh 5.9 (x86_64-pc-linux-gnu) (on Arch
> Linux, but I also reproduced on Alpine in QEMU, so it is probably zsh and
> not libc. Also, this does not happen in archiso in qemu, which is also
> weird.).
>
> Repro instructions: run the commands:
> TRAPEXIT() { ls }
> TRAPEXIT
> # if that does not crash, keep typing TRAPEXIT until it does. sometimes it
> doesn't crash.
>
> Expected behavior: zsh might throw an error or something? but it shouldn't
> crash
> Actual behavior: there is an unknown chance that zsh throws an error and
> crashes like this:
> zsh: TRAPEXIT: function not defined by file
> malloc(): unaligned tcache chunk detected
> [1] 34511 IOT instruction (core dumped) zsh
>
> Nothing relevant in dmesg.
>
> If I can help in any way, please contact me.
>
> Kind regards,
> an anonymous bug reporter

It seems to happen reliably for me every time, with these messages,
% MALLOC_CHECK_=3 zsh -fc 'TRAPEXIT() { ls }; TRAPEXIT'
1: parse.c:2817: Heap EPROG has nref > 0
free(): invalid pointer
zsh: abort      MALLOC_CHECK_=3 zsh -fc 'TRAPEXIT() { ls }; TRAPEXIT'

valgrind is also extremely unhappy with this execution path.

-- 
Mikael Magnusson


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Probabilistic crash on zsh 5.9 on x86_64
  2023-04-08 21:36 ` Mikael Magnusson
@ 2023-04-11 16:14   ` Jun. T
  2023-04-11 16:29     ` Peter Stephenson
  0 siblings, 1 reply; 14+ messages in thread
From: Jun. T @ 2023-04-11 16:14 UTC (permalink / raw)
  To: zsh-workers


> 2023/04/09 6:36, Mikael Magnusson <mikachu@gmail.com> wrlte:

> It seems to happen reliably for me every time, with these messages,
> % MALLOC_CHECK_=3 zsh -fc 'TRAPEXIT() { ls }; TRAPEXIT'
> 1: parse.c:2817: Heap EPROG has nref > 0
> free(): invalid pointer
> zsh: abort      MALLOC_CHECK_=3 zsh -fc 'TRAPEXIT() { ls }; TRAPEXIT'

It seems memory pointed to by 'Eprog p' (in function freeeprog(),
parse.c:2817) is already freed.

If TRAPEXIT() is called directly, execshfunc(shf, ..) is called
with shf pointing to the node "TRAPEXIT" in shfunctab.
Then it calls

doshfunc(shf, ..)
  starttrapscope()			// exec.c:5821
    unsettrap()				// signals.c:1079
      shfunctab->freenode(shf)		// signals.c:982

this means shf is freed by freeshfuncnode(shf). But doshfunc()
continues to use shf (=shfunc in this function), and calls
  runshfunc(prog=shf->funcdef, ..)	// exec.c:5963
This leads to crash, of course.

The simplest thing we can do would be just to prohibit
users/scripts from calling TRAPEXIT() directly. I guess this
can be done by, for example, rejecting (with error message)
shf->node.nam=="TRAPEXIT" at the top of execshfunc(shf,..).

But then users can't test TRAPEXIT manually.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Probabilistic crash on zsh 5.9 on x86_64
  2023-04-11 16:14   ` Jun. T
@ 2023-04-11 16:29     ` Peter Stephenson
  2023-04-13 10:47       ` Peter Stephenson
  0 siblings, 1 reply; 14+ messages in thread
From: Peter Stephenson @ 2023-04-11 16:29 UTC (permalink / raw)
  To: zsh-workers, Jun. T

[-- Attachment #1: Type: text/plain, Size: 1657 bytes --]

(Please excuse illiterate response, I'm away but should be
back with a real computer in a day or so.)

This indicates something is wrong with the reference counting:
it shouldn't be possible for a shell code chunk to be visible
to the user without it being marked as having at least one reference.

pws


On 11 April 2023 17:14:15 BST, "Jun. T" <takimoto-j@kba.biglobe.ne.jp> wrote:
>
>> 2023/04/09 6:36, Mikael Magnusson <mikachu@gmail.com> wrlte:
>
>> It seems to happen reliably for me every time, with these messages,
>> % MALLOC_CHECK_=3 zsh -fc 'TRAPEXIT() { ls }; TRAPEXIT'
>> 1: parse.c:2817: Heap EPROG has nref > 0
>> free(): invalid pointer
>> zsh: abort      MALLOC_CHECK_=3 zsh -fc 'TRAPEXIT() { ls }; TRAPEXIT'
>
>It seems memory pointed to by 'Eprog p' (in function freeeprog(),
>parse.c:2817) is already freed.
>
>If TRAPEXIT() is called directly, execshfunc(shf, ..) is called
>with shf pointing to the node "TRAPEXIT" in shfunctab.
>Then it calls
>
>doshfunc(shf, ..)
>  starttrapscope()			// exec.c:5821
>    unsettrap()				// signals.c:1079
>      shfunctab->freenode(shf)		// signals.c:982
>
>this means shf is freed by freeshfuncnode(shf). But doshfunc()
>continues to use shf (=shfunc in this function), and calls
>  runshfunc(prog=shf->funcdef, ..)	// exec.c:5963
>This leads to crash, of course.
>
>The simplest thing we can do would be just to prohibit
>users/scripts from calling TRAPEXIT() directly. I guess this
>can be done by, for example, rejecting (with error message)
>shf->node.nam=="TRAPEXIT" at the top of execshfunc(shf,..).
>
>But then users can't test TRAPEXIT manually.
>

[-- Attachment #2: Type: text/html, Size: 2233 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Probabilistic crash on zsh 5.9 on x86_64
  2023-04-11 16:29     ` Peter Stephenson
@ 2023-04-13 10:47       ` Peter Stephenson
  2023-04-13 11:12         ` Peter Stephenson
  0 siblings, 1 reply; 14+ messages in thread
From: Peter Stephenson @ 2023-04-13 10:47 UTC (permalink / raw)
  To: zsh-workers, Jun. T

> On 11/04/2023 17:29 Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> This indicates something is wrong with the reference counting:
> it shouldn't be possible for a shell code chunk to be visible
> to the user without it being marked as having at least one reference.

This should reliably pick up the problem, though it needs tracing back
to where the function is set up.

It was crashing 100% for me, too, and this is the only time I see this debug
message, so I think this is safe to commit and should give us a heads up
on other cases.

diff --git a/Src/exec.c b/Src/exec.c
index 3b3d1235e..49c196bfd 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -5942,6 +5942,8 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval)
 	funcsave->fstack.filename = getshfuncfile(shfunc);
 
 	prog = shfunc->funcdef;
+	DPUTS1(!prog->nref, "function definition %s has zero reference count",
+	       (fname && *fname) ? fname : "<anon>");
 	if (prog->flags & EF_RUN) {
 	    Shfunc shf;
 
pws


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Probabilistic crash on zsh 5.9 on x86_64
  2023-04-13 10:47       ` Peter Stephenson
@ 2023-04-13 11:12         ` Peter Stephenson
  2023-04-13 13:02           ` Jun. T
  0 siblings, 1 reply; 14+ messages in thread
From: Peter Stephenson @ 2023-04-13 11:12 UTC (permalink / raw)
  To: zsh-workers

> On 13/04/2023 11:47 Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> > On 11/04/2023 17:29 Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> > This indicates something is wrong with the reference counting:
> > it shouldn't be possible for a shell code chunk to be visible
> > to the user without it being marked as having at least one reference.
> 
> This should reliably pick up the problem, though it needs tracing back
> to where the function is set up.

"watch" says the real culprit is this unsettrap() in starttrapscope().
I guess the save and restore action here needs a corresponding
useeprog / freeprog, not sure the best way of doing that yet.

    /*
     * SIGEXIT needs to be restored at the current locallevel,
     * so give it the next higher one. dosavetrap() is called
     * automatically where necessary.
     */
    if (sigtrapped[SIGEXIT] && !exit_trap_posix) {
	locallevel++;
	unsettrap(SIGEXIT);
	locallevel--;
    }

pws


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Probabilistic crash on zsh 5.9 on x86_64
  2023-04-13 11:12         ` Peter Stephenson
@ 2023-04-13 13:02           ` Jun. T
  2023-04-13 13:19             ` Peter Stephenson
  0 siblings, 1 reply; 14+ messages in thread
From: Jun. T @ 2023-04-13 13:02 UTC (permalink / raw)
  To: zsh-workers


> 2023/04/13 20:12、Peter Stephenson <p.w.stephenson@ntlworld.com>のメール:

> "watch" says the real culprit is this unsettrap() in starttrapscope().
> I guess the save and restore action here needs a corresponding
> useeprog / freeprog, not sure the best way of doing that yet.

As I wrote before, the entire 'struct shfunc' is freed by
shfunctab->freenode(shf) (signals.c:982), or freeshfuncnode(shf).
I think it is not just the problem of the reference count of Eprog
(shfnc.funcdef.nref).
Or maybe I misunderstood your post.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Probabilistic crash on zsh 5.9 on x86_64
  2023-04-13 13:02           ` Jun. T
@ 2023-04-13 13:19             ` Peter Stephenson
  2023-04-13 14:03               ` Peter Stephenson
  0 siblings, 1 reply; 14+ messages in thread
From: Peter Stephenson @ 2023-04-13 13:19 UTC (permalink / raw)
  To: zsh-workers

> On 13/04/2023 14:02 Jun. T <takimoto-j@kba.biglobe.ne.jp> wrote:
> > 2023/04/13 20:12、Peter Stephenson <p.w.stephenson@ntlworld.com>のメール:
> > "watch" says the real culprit is this unsettrap() in starttrapscope().
> > I guess the save and restore action here needs a corresponding
> > useeprog / freeprog, not sure the best way of doing that yet.
> 
> As I wrote before, the entire 'struct shfunc' is freed by
> shfunctab->freenode(shf) (signals.c:982), or freeshfuncnode(shf).
> I think it is not just the problem of the reference count of Eprog
> (shfnc.funcdef.nref).
> Or maybe I misunderstood your post.

"Freeing" really means reducing the reference count and then actually
freeing the structure only when the reference count hits zero.  So
because we want to keep this structure, the reference count shouldn't
be zero at this point (it shouldn't ever be allowed to go to zero
for a permanently allocated structure visible to user code, hence the
DPUTS test), and if it isn't then the problem goes away --- this
becomes just a normal function call.

However, it's not trivial because of the way we save and restore
TRAPEXIT (and possibly other traps).  We do this because the
TRAPEXIT shouldn't be running from within a nested function call.
So what we should logically do is take the code chunk out of the
trap list but marked in such a way that it won't get actually freed,
because we're going to put it back again later, and because someone
may access the same structure meanwhile (as it's permanently
allocated --- the whole point of reference counts being that it's
hard to know for sure who's referring to which code chunk at any
given time).

Unfortunately, it looks like this process is more complicated than
that, involving actual copies, so finding a place to bump the
reference count and then later reduce it again (which would free
the structure if it actually wasn't needed any more at that second
point) isn't trivial, and it's quite possible there is another solution
not involving reference counts.

Hope that's clearer but I'm not sure it is...

pws


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Probabilistic crash on zsh 5.9 on x86_64
  2023-04-13 13:19             ` Peter Stephenson
@ 2023-04-13 14:03               ` Peter Stephenson
  2023-04-13 14:13                 ` Peter Stephenson
  2023-04-13 16:40                 ` Jun. T
  0 siblings, 2 replies; 14+ messages in thread
From: Peter Stephenson @ 2023-04-13 14:03 UTC (permalink / raw)
  To: zsh-workers

> On 13/04/2023 14:19 Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> ...not trivial...
> ...hard to know...
> 
> ...Unfortunately...
> ...more complicated than that...
> ...isn't trivial...

Have a look at this --- it simply marks the prog in the shell function as
in use earlier and unmarks it later, so the shenanigans within to do
with traps all come out in the wash.  So, in theory, there's not much
to go wrong.  But let me know...

pws

diff --git a/Src/exec.c b/Src/exec.c
index 3b3d1235e..bf81a2a42 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -5779,12 +5779,25 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval)
     char *name = shfunc->node.nam;
     int flags = shfunc->node.flags;
     char *fname = dupstring(name);
-    Eprog prog;
+    Eprog prog, marked_prog;
     static int oflags;
     static int funcdepth;
     Heap funcheap;
 
     queue_signals();	/* Lots of memory and global state changes coming */
+    /*
+     * In case this is a special function such as a trap, mark it
+     * is in use right now, so it doesn't get freed early.  The
+     * worst that can happen is this hangs around in memory a little
+     * longer than strictly needed.
+     *
+     * Classic example of this happening is running TRAPEXIT directly.
+     *
+     * Because the shell function's contents may change, we'll ensure
+     * we use a consistent structure for use / free.
+     */
+    marked_prog = shfunc->funcdef;
+    useeprog(marked_prog);
 
     NEWHEAPS(funcheap) {
 	/*
@@ -5942,6 +5955,8 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval)
 	funcsave->fstack.filename = getshfuncfile(shfunc);
 
 	prog = shfunc->funcdef;
+	DPUTS1(!prog->nref, "function definition %s has zero reference count",
+	       (fname && *fname) ? fname : "<anon>");
 	if (prog->flags & EF_RUN) {
 	    Shfunc shf;
 
@@ -6046,6 +6061,7 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval)
 	}
     } OLDHEAPS;
 
+    freeeprog(marked_prog);
     unqueue_signals();
 
     /*
diff --git a/Test/C03traps.ztst b/Test/C03traps.ztst
index e0b6afb5f..de57765a0 100644
--- a/Test/C03traps.ztst
+++ b/Test/C03traps.ztst
@@ -1083,6 +1083,17 @@ F:Must be tested with a top-level script rather than source or function
 >trap1
 # As of 5.7.1-test-2, the output was "out1 fn1 trap1 fn2" (on separate lines).
 
+  TRAPEXIT() { echo This is TRAPEXIT; }
+  TRAPEXIT
+  TRAPEXIT
+  TRAPEXIT
+0:No memory problems with explicit call to TRAPEXIT.
+>This is TRAPEXIT
+>This is TRAPEXIT
+>This is TRAPEXIT
+>This is TRAPEXIT
+# Three explicit calls, one implicit call at function exit.
+
 %clean
 
   rm -f TRAPEXIT


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Probabilistic crash on zsh 5.9 on x86_64
  2023-04-13 14:03               ` Peter Stephenson
@ 2023-04-13 14:13                 ` Peter Stephenson
  2023-04-13 16:40                 ` Jun. T
  1 sibling, 0 replies; 14+ messages in thread
From: Peter Stephenson @ 2023-04-13 14:13 UTC (permalink / raw)
  To: zsh-workers

> On 13/04/2023 15:03 Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> > On 13/04/2023 14:19 Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> > ...not trivial...
> > ...hard to know...
> > 
> > ...Unfortunately...
> > ...more complicated than that...
> > ...isn't trivial...
> 
> Have a look at this --- it simply marks the prog in the shell function as
> in use earlier and unmarks it later, so the shenanigans within to do
> with traps all come out in the wash.  So, in theory, there's not much
> to go wrong.  But let me know...

One note (the only caveat that springs to mind, but probably worth mentioning)
is that if a function is freed explicitly or implicitly inside the hierarchy
that handles it, we don't actually gain the memory back until a bit later
than previously.  I think this would happen with the placeholder structure
for an autoloadable shell function, for example.  Hopefully that's not a
big deal, but unless I mention it someone's sure to come up with a case
where it is...

pws


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Probabilistic crash on zsh 5.9 on x86_64
  2023-04-13 14:03               ` Peter Stephenson
  2023-04-13 14:13                 ` Peter Stephenson
@ 2023-04-13 16:40                 ` Jun. T
  2023-04-13 16:55                   ` Peter Stephenson
  1 sibling, 1 reply; 14+ messages in thread
From: Jun. T @ 2023-04-13 16:40 UTC (permalink / raw)
  To: zsh-workers


> 2023/04/13 23:03、Peter Stephenson <p.w.stephenson@ntlworld.com>のメール:
> 
> Have a look at this --- it simply marks the prog in the shell function as
> in use earlier and unmarks it later, so the shenanigans within to do
> with traps all come out in the wash.  So, in theory, there's not much
> to go wrong.  But let me know...

> +    marked_prog = shfunc->funcdef;
> +    useeprog(marked_prog);
(snip)
> +    freeeprog(marked_prog);

Even with this, shfunc is still freed, although shfunc->funcdef is not freed.
But doshfunc() continues to use shfunc, at lines 5854, 5954, 5957, etc.
If it doesn't crash, it means the freed memory is not yet used by others.
But I think this is unsafe, and valgrind gives lots of warnings.




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Probabilistic crash on zsh 5.9 on x86_64
  2023-04-13 16:40                 ` Jun. T
@ 2023-04-13 16:55                   ` Peter Stephenson
  2023-04-14  8:29                     ` Peter Stephenson
  0 siblings, 1 reply; 14+ messages in thread
From: Peter Stephenson @ 2023-04-13 16:55 UTC (permalink / raw)
  To: zsh-workers

> On 13/04/2023 17:40 Jun. T <takimoto-j@kba.biglobe.ne.jp> wrote:
> 
>  
> > 2023/04/13 23:03、Peter Stephenson <p.w.stephenson@ntlworld.com>のメール:
> > 
> > Have a look at this --- it simply marks the prog in the shell function as
> > in use earlier and unmarks it later, so the shenanigans within to do
> > with traps all come out in the wash.  So, in theory, there's not much
> > to go wrong.  But let me know...
> 
> > +    marked_prog = shfunc->funcdef;
> > +    useeprog(marked_prog);
> (snip)
> > +    freeeprog(marked_prog);
> 
> Even with this, shfunc is still freed, although shfunc->funcdef is not freed.
> But doshfunc() continues to use shfunc, at lines 5854, 5954, 5957, etc.
> If it doesn't crash, it means the freed memory is not yet used by others.
> But I think this is unsafe, and valgrind gives lots of warnings.

OK, so the starttrapscope() just above that point is pulling the rug out
from under the function's feet.  We've looked up the TRAPEXIT function and
now that function's just been undefined.

So some better save / restore for the shell function is probably needed.

pws


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Probabilistic crash on zsh 5.9 on x86_64
  2023-04-13 16:55                   ` Peter Stephenson
@ 2023-04-14  8:29                     ` Peter Stephenson
  2023-04-14 12:21                       ` Mikael Magnusson
  0 siblings, 1 reply; 14+ messages in thread
From: Peter Stephenson @ 2023-04-14  8:29 UTC (permalink / raw)
  To: zsh-workers

> On 13/04/2023 17:55 Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> > On 13/04/2023 17:40 Jun. T <takimoto-j@kba.biglobe.ne.jp> wrote:
> > Even with this, shfunc is still freed, although shfunc->funcdef is not freed.
> > But doshfunc() continues to use shfunc, at lines 5854, 5954, 5957, etc.
> > If it doesn't crash, it means the freed memory is not yet used by others.
> > But I think this is unsafe, and valgrind gives lots of warnings.
> 
> OK, so the starttrapscope() just above that point is pulling the rug out
> from under the function's feet.  We've looked up the TRAPEXIT function and
> now that function's just been undefined.
> 
> So some better save / restore for the shell function is probably needed.

I can't think of any major surgery I'd like to do for this special case
--- TRAPEXIT being from removed from within an execution of TRAPEXIT so
that the trap doesn't go off inside the function being executed.

Instead, here's a simple local fix-up which seems to keep valgrind quiet.
It's not the most elegant thing I can imagine.

diff --git a/Src/exec.c b/Src/exec.c
index 3b3d1235e..274800b10 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -5779,12 +5779,25 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval)
     char *name = shfunc->node.nam;
     int flags = shfunc->node.flags;
     char *fname = dupstring(name);
-    Eprog prog;
+    Eprog prog, marked_prog;
     static int oflags;
     static int funcdepth;
     Heap funcheap;
 
     queue_signals();	/* Lots of memory and global state changes coming */
+    /*
+     * In case this is a special function such as a trap, mark it
+     * is in use right now, so it doesn't get freed early.  The
+     * worst that can happen is this hangs around in memory a little
+     * longer than strictly needed.
+     *
+     * Classic example of this happening is running TRAPEXIT directly.
+     *
+     * Because the shell function's contents may change, we'll ensure
+     * we use a consistent structure for use / free.
+     */
+    marked_prog = shfunc->funcdef;
+    useeprog(marked_prog);
 
     NEWHEAPS(funcheap) {
 	/*
@@ -5818,6 +5831,22 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval)
 	    memcpy(funcsave->pipestats, pipestats, bytes);
 	}
 
+	if (!strcmp(fname, "TRAPEXIT")) {
+	    /*
+	     * If we are executing TRAPEXIT directly, starttrapscope()
+	     * will pull the rug out from under us to ensure the
+	     * exit trap isn't run inside the function.  We just need
+	     * the information locally here, so copy it on the heap.
+	     *
+	     * The funcdef is separately handled by reference counting.
+	     */
+	    Shfunc shcopy = (Shfunc)zhalloc(sizeof(struct shfunc));
+	    memcpy(shcopy, shfunc, sizeof(struct shfunc));
+	    shcopy->node.nam = dupstring(shfunc->node.nam);
+	    shfunc = shcopy;
+	    name = shfunc->node.nam;
+	}
+
 	starttrapscope();
 	startpatternscope();
 
@@ -5942,6 +5971,8 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval)
 	funcsave->fstack.filename = getshfuncfile(shfunc);
 
 	prog = shfunc->funcdef;
+	DPUTS1(!prog->nref, "function definition %s has zero reference count",
+	       (fname && *fname) ? fname : "<anon>");
 	if (prog->flags & EF_RUN) {
 	    Shfunc shf;
 
@@ -6046,6 +6077,7 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval)
 	}
     } OLDHEAPS;
 
+    freeeprog(marked_prog);
     unqueue_signals();
 
     /*
diff --git a/Test/C03traps.ztst b/Test/C03traps.ztst
index e0b6afb5f..de57765a0 100644
--- a/Test/C03traps.ztst
+++ b/Test/C03traps.ztst
@@ -1083,6 +1083,17 @@ F:Must be tested with a top-level script rather than source or function
 >trap1
 # As of 5.7.1-test-2, the output was "out1 fn1 trap1 fn2" (on separate lines).
 
+  TRAPEXIT() { echo This is TRAPEXIT; }
+  TRAPEXIT
+  TRAPEXIT
+  TRAPEXIT
+0:No memory problems with explicit call to TRAPEXIT.
+>This is TRAPEXIT
+>This is TRAPEXIT
+>This is TRAPEXIT
+>This is TRAPEXIT
+# Three explicit calls, one implicit call at function exit.
+
 %clean
 
   rm -f TRAPEXIT


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Probabilistic crash on zsh 5.9 on x86_64
  2023-04-14  8:29                     ` Peter Stephenson
@ 2023-04-14 12:21                       ` Mikael Magnusson
  0 siblings, 0 replies; 14+ messages in thread
From: Mikael Magnusson @ 2023-04-14 12:21 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: zsh-workers

On 4/14/23, Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> Instead, here's a simple local fix-up which seems to keep valgrind quiet.
> It's not the most elegant thing I can imagine.
>
> diff --git a/Src/exec.c b/Src/exec.c
> index 3b3d1235e..274800b10 100644
> --- a/Src/exec.c
> +++ b/Src/exec.c
> @@ -5779,12 +5779,25 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int
...
>      queue_signals();	/* Lots of memory and global state changes coming */
> +    /*
> +     * In case this is a special function such as a trap, mark it
> +     * is in use right now, so it doesn't get freed early.  The

"mark it as in use"

-- 
Mikael Magnusson


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2023-04-14 12:21 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-08 17:28 Probabilistic crash on zsh 5.9 on x86_64 zsh bug report throwaway email thing
2023-04-08 21:36 ` Mikael Magnusson
2023-04-11 16:14   ` Jun. T
2023-04-11 16:29     ` Peter Stephenson
2023-04-13 10:47       ` Peter Stephenson
2023-04-13 11:12         ` Peter Stephenson
2023-04-13 13:02           ` Jun. T
2023-04-13 13:19             ` Peter Stephenson
2023-04-13 14:03               ` Peter Stephenson
2023-04-13 14:13                 ` Peter Stephenson
2023-04-13 16:40                 ` Jun. T
2023-04-13 16:55                   ` Peter Stephenson
2023-04-14  8:29                     ` Peter Stephenson
2023-04-14 12:21                       ` Mikael Magnusson

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).