From owner-freebsd-bugs@FreeBSD.ORG Fri Aug 14 14:20:05 2009 Return-Path: <owner-freebsd-bugs@FreeBSD.ORG> Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 1BADC106568D for <freebsd-bugs@hub.freebsd.org>; Fri, 14 Aug 2009 14:20:05 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 09DB88FC55 for <freebsd-bugs@hub.freebsd.org>; Fri, 14 Aug 2009 14:20:05 +0000 (UTC) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.14.3/8.14.3) with ESMTP id n7EEK4WI073549 for <freebsd-bugs@freefall.freebsd.org>; Fri, 14 Aug 2009 14:20:04 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.3/8.14.3/Submit) id n7EEK4YE073548; Fri, 14 Aug 2009 14:20:04 GMT (envelope-from gnats) Date: Fri, 14 Aug 2009 14:20:04 GMT Message-Id: <200908141420.n7EEK4YE073548@freefall.freebsd.org> To: freebsd-bugs@FreeBSD.org From: Jilles Tjoelker <jilles@stack.nl> Cc: Subject: Re: bin/137640: [PATCH] sh(1) crash when redefining current function X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Jilles Tjoelker <jilles@stack.nl> List-Id: Bug reports <freebsd-bugs.freebsd.org> List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-bugs>, <mailto:freebsd-bugs-request@freebsd.org?subject=unsubscribe> List-Archive: <http://lists.freebsd.org/pipermail/freebsd-bugs> List-Post: <mailto:freebsd-bugs@freebsd.org> List-Help: <mailto:freebsd-bugs-request@freebsd.org?subject=help> List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-bugs>, <mailto:freebsd-bugs-request@freebsd.org?subject=subscribe> X-List-Received-Date: Fri, 14 Aug 2009 14:20:05 -0000 The following reply was made to PR bin/137640; it has been noted by GNATS. From: Jilles Tjoelker <jilles@stack.nl> To: bug-followup@FreeBSD.org, jilles@stack.nl Cc: Subject: Re: bin/137640: [PATCH] sh(1) crash when redefining current function Date: Fri, 14 Aug 2009 16:17:24 +0200 --azLHFNyN32YCQGCU Content-Type: text/plain; charset=us-ascii Content-Disposition: inline That patch does not work, try this. --azLHFNyN32YCQGCU Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="func-redef-fix.patch" Fix crash when undefining or redefining a currently executing function. Memory may leak if multiple SIGINTs arrive in interactive mode, this will be fixed later by changing SIGINT handling. diff --git a/eval.c b/eval.c --- a/eval.c +++ b/eval.c @@ -791,6 +791,7 @@ evalcommand(union node *cmd, int flags, INTOFF; savelocalvars = localvars; localvars = NULL; + reffunc(cmdentry.u.func); INTON; savehandler = handler; if (setjmp(jmploc.loc)) { @@ -800,6 +801,7 @@ evalcommand(union node *cmd, int flags, freeparam(&shellparam); shellparam = saveparam; } + unreffunc(cmdentry.u.func); poplocalvars(); localvars = savelocalvars; handler = savehandler; @@ -811,11 +813,12 @@ evalcommand(union node *cmd, int flags, funcnest++; exitstatus = oexitstatus; if (flags & EV_TESTED) - evaltree(cmdentry.u.func, EV_TESTED); + evaltree(&cmdentry.u.func->n, EV_TESTED); else - evaltree(cmdentry.u.func, 0); + evaltree(&cmdentry.u.func->n, 0); funcnest--; INTOFF; + unreffunc(cmdentry.u.func); poplocalvars(); localvars = savelocalvars; freeparam(&shellparam); diff --git a/exec.c b/exec.c --- a/exec.c +++ b/exec.c @@ -286,7 +286,7 @@ printentry(struct tblentry *cmdp, int ve out1fmt("function %s", cmdp->cmdname); if (verbose) { INTOFF; - name = commandtext(cmdp->param.func); + name = commandtext(&cmdp->param.func->n); out1c(' '); out1str(name); ckfree(name); @@ -583,7 +583,7 @@ deletefuncs(void) while ((cmdp = *pp) != NULL) { if (cmdp->cmdtype == CMDFUNCTION) { *pp = cmdp->next; - freefunc(cmdp->param.func); + unreffunc(cmdp->param.func); ckfree(cmdp); } else { pp = &cmdp->next; @@ -670,7 +670,7 @@ addcmdentry(char *name, struct cmdentry INTOFF; cmdp = cmdlookup(name, 1); if (cmdp->cmdtype == CMDFUNCTION) { - freefunc(cmdp->param.func); + unreffunc(cmdp->param.func); } cmdp->cmdtype = entry->cmdtype; cmdp->param = entry->u; @@ -705,7 +705,7 @@ unsetfunc(char *name) struct tblentry *cmdp; if ((cmdp = cmdlookup(name, 0)) != NULL && cmdp->cmdtype == CMDFUNCTION) { - freefunc(cmdp->param.func); + unreffunc(cmdp->param.func); delete_cmd_entry(); return (0); } diff --git a/exec.h b/exec.h --- a/exec.h +++ b/exec.h @@ -46,11 +46,12 @@ enum { TYPECMD_TYPE /* type */ }; +union node; struct cmdentry { int cmdtype; union param { int index; - union node *func; + struct funcdef *func; } u; int special; }; diff --git a/mknodes.c b/mknodes.c --- a/mknodes.c +++ b/mknodes.c @@ -248,8 +248,13 @@ output(char *file) fputs("\tstruct nodelist *next;\n", hfile); fputs("\tunion node *n;\n", hfile); fputs("};\n\n\n", hfile); - fputs("union node *copyfunc(union node *);\n", hfile); - fputs("void freefunc(union node *);\n", hfile); + fputs("struct funcdef {\n", hfile); + fputs("\tunsigned int refcount;\n", hfile); + fputs("\tunion node n;\n", hfile); + fputs("};\n\n\n", hfile); + fputs("struct funcdef *copyfunc(union node *);\n", hfile); + fputs("void reffunc(struct funcdef *);\n", hfile); + fputs("void unreffunc(struct funcdef *);\n", hfile); fputs(writer, cfile); while (fgets(line, sizeof line, patfile) != NULL) { diff --git a/nodes.c.pat b/nodes.c.pat --- a/nodes.c.pat +++ b/nodes.c.pat @@ -35,6 +35,7 @@ #include <sys/param.h> #include <stdlib.h> +#include <stddef.h> /* * Routine for dealing with parsed shell commands. */ @@ -65,17 +66,22 @@ STATIC char *nodesavestr(char *); * Make a copy of a parse tree. */ -union node * +struct funcdef * copyfunc(union node *n) { + struct funcdef *fn; + if (n == NULL) return NULL; - funcblocksize = 0; + funcblocksize = offsetof(struct funcdef, n); funcstringsize = 0; calcsize(n); - funcblock = ckmalloc(funcblocksize + funcstringsize); - funcstring = (char *)funcblock + funcblocksize; - return copynode(n); + fn = ckmalloc(funcblocksize + funcstringsize); + fn->refcount = 1; + funcblock = (char *)fn + offsetof(struct funcdef, n); + funcstring = (char *)fn + funcblocksize; + copynode(n); + return fn; } @@ -144,14 +150,25 @@ nodesavestr(char *s) } +void +reffunc(struct funcdef *fn) +{ + fn->refcount++; +} + /* - * Free a parse tree. + * Decrement the reference count of a function definition, freeing it + * if it falls to 0. */ void -freefunc(union node *n) +unreffunc(struct funcdef *fn) { - if (n) - ckfree(n); + if (fn) { + fn->refcount--; + if (fn->refcount > 0) + return; + ckfree(fn); + } } --azLHFNyN32YCQGCU--