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