fixes for memory leak for user-supplied sorting function.

5 files changed, 77 insertions, 66 deletions

diff --git a/ChangeLog b/ChangeLog
index 6822c6f4..79262466 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,18 @@
+2017-04-04         Arnold D. Robbins     <arnold@skeeve.com>
+
+	* awk.h (INSTRUCTION): Add pool_size member.
+	[MAX_INSTRUCTION_ALLOC]: New macro.
+	(INSTRUCTION_POOL): New type.
+	(struct context): Use INSTRUCTION_POOL.
+	* array.c (assoc_list): Reorg the code a bit to make sure
+	to alway free the INSTRUCTIONs allocated for calling a
+	user-supplied sorting function. Based on code by
+	Andrew Schorr.
+	* symbol.c (free_bcpool): Rework to use an INSTRUCTION_POOL.
+	(bcfree, bcalloc): Rework to use separate chains in
+	the instruction pool.
+	(set_context): Update appropriately.
+
 2017-03-27         Arnold D. Robbins     <arnold@skeeve.com>
 
 	Cause EPIPE errors to stdout to generate a real SIGPIPE.
diff --git a/array.c b/array.c
index cee1c729..3159bfdc 100644
--- a/array.c
+++ b/array.c
@@ -1353,12 +1353,22 @@ assoc_list(NODE *symbol, const char *sort_str, sort_context_t sort_ctxt)
 	list = symbol->alist(symbol, & akind);
 	assoc_kind = (assoc_kind_t) akind.flags;	/* symbol->alist can modify it */
 
-	if (list == NULL || ! cmp_func || (assoc_kind & (AASC|ADESC)) != 0)
-		return list;	/* empty list or unsorted, or list already sorted */
+	/* check for empty list or unsorted, or list already sorted */
+	if (list != NULL && cmp_func != NULL && (assoc_kind & (AASC|ADESC)) == 0) {
+		num_elems = assoc_length(symbol);
 
-	num_elems = assoc_length(symbol);
+		qsort(list, num_elems, elem_size * sizeof(NODE *), cmp_func); /* shazzam! */
 
-	qsort(list, num_elems, elem_size * sizeof(NODE *), cmp_func); /* shazzam! */
+		if (sort_ctxt == SORTED_IN && (assoc_kind & (AINDEX|AVALUE)) == (AINDEX|AVALUE)) {
+			/* relocate all index nodes to the first half of the list. */
+			for (j = 1; j < num_elems; j++)
+				list[j] = list[2 * j];
+
+			/* give back extra memory */
+
+			erealloc(list, NODE **, num_elems * sizeof(NODE *), "assoc_list");
+		}
+	}
 
 	if (cmp_func == sort_user_func) {
 		code = POP_CODE();
@@ -1367,15 +1377,5 @@ assoc_list(NODE *symbol, const char *sort_str, sort_context_t sort_ctxt)
 		bcfree(code);                   /* Op_func_call */
 	}
 
-	if (sort_ctxt == SORTED_IN && (assoc_kind & (AINDEX|AVALUE)) == (AINDEX|AVALUE)) {
-		/* relocate all index nodes to the first half of the list. */
-		for (j = 1; j < num_elems; j++)
-			list[j] = list[2 * j];
-
-		/* give back extra memory */
-
-		erealloc(list, NODE **, num_elems * sizeof(NODE *), "assoc_list");
-	}
-
 	return list;
 }
diff --git a/awk.h b/awk.h
index c1e9b4a9..6b7cdb7a 100644
--- a/awk.h
+++ b/awk.h
@@ -783,6 +783,7 @@ typedef struct exp_instruction {
 	} x;
 
 	short source_line;
+	short pool_size;	// memory management in symbol.c
 	OPCODE opcode;
 } INSTRUCTION;
 
@@ -1031,9 +1032,15 @@ typedef struct srcfile {
 	int lasttok;
 } SRCFILE;
 
+// structure for INSTRUCTION pool, needed mainly for debugger
+typedef struct instruction_pool {
+#define MAX_INSTRUCTION_ALLOC	3	// we don't call bcalloc with more than this
+	INSTRUCTION pool[MAX_INSTRUCTION_ALLOC + 1];
+} INSTRUCTION_POOL;
+
 /* structure for execution context */
 typedef struct context {
-	INSTRUCTION pools;
+	INSTRUCTION_POOL pools;
 	NODE symbols;
 	INSTRUCTION rule_list;
 	SRCFILE srcfiles;
diff --git a/symbol.c b/symbol.c
index 65ed4d90..b2a6a6c2 100644
--- a/symbol.c
+++ b/symbol.c
@@ -37,7 +37,7 @@ static NODE *symbol_list;
 static void (*install_func)(NODE *) = NULL;
 static NODE *make_symbol(const char *name, NODETYPE type);
 static NODE *install(const char *name, NODE *parm, NODETYPE type);
-static void free_bcpool(INSTRUCTION *pl);
+static void free_bcpool(INSTRUCTION_POOL *pl);
 
 static AWK_CONTEXT *curr_ctxt = NULL;
 static int ctxt_level;
@@ -693,21 +693,19 @@ check_param_names(void)
 	return result;
 }
 
-#define pool_size	d.dl
 #define freei		x.xi
-static INSTRUCTION *pool_list;
-
-/* INSTR_CHUNK must be > largest code size (3) */
-#define INSTR_CHUNK 127
+static INSTRUCTION_POOL pools;
 
 /* bcfree --- deallocate instruction */
 
 void
 bcfree(INSTRUCTION *cp)
 {
+	assert(cp->pool_size >= 1 && cp->pool_size <= MAX_INSTRUCTION_ALLOC);
+
 	cp->opcode = 0;
-	cp->nexti = pool_list->freei;
-	pool_list->freei = cp;
+	cp->nexti = pools.pool[cp->pool_size].freei;
+	pools.pool[cp->pool_size].freei = cp;
 }
 
 /* bcalloc --- allocate a new instruction */
@@ -717,37 +715,16 @@ bcalloc(OPCODE op, int size, int srcline)
 {
 	INSTRUCTION *cp;
 
-	if (size > 1) {
-		/* wide instructions Op_rule, Op_func_call .. */
-		emalloc(cp, INSTRUCTION *, (size + 1) * sizeof(INSTRUCTION), "bcalloc");
-		cp->pool_size = size;
-		cp->nexti = pool_list->nexti;
-		pool_list->nexti = cp++;
+	assert(size >= 1 && size <= MAX_INSTRUCTION_ALLOC);
+
+	if ((cp = pools.pool[size].freei) != NULL) {
+		pools.pool[size].freei = cp->nexti;
 	} else {
-		INSTRUCTION *pool;
-
-		pool = pool_list->freei;
-		if (pool == NULL) {
-			INSTRUCTION *last;
-			emalloc(cp, INSTRUCTION *, (INSTR_CHUNK + 1) * sizeof(INSTRUCTION), "bcalloc");
-
-			cp->pool_size = INSTR_CHUNK;
-			cp->nexti = pool_list->nexti;
-			pool_list->nexti = cp;
-			pool = ++cp;
-			last = &pool[INSTR_CHUNK - 1];
-			for (; cp <= last; cp++) {
-				cp->opcode = 0;
-				cp->nexti = cp + 1;
-			}
-			--cp;
-			cp->nexti = NULL;
-		}
-		cp = pool;
-		pool_list->freei = cp->nexti;
+		emalloc(cp, INSTRUCTION *, (size + 1) * sizeof(INSTRUCTION), "bcalloc");
 	}
 
 	memset(cp, 0, size * sizeof(INSTRUCTION));
+	cp->pool_size = size;
 	cp->opcode = op;
 	cp->source_line = srcline;
 	return cp;
@@ -773,7 +750,7 @@ new_context()
 static void
 set_context(AWK_CONTEXT *ctxt)
 {
-	pool_list = & ctxt->pools;
+	pools = ctxt->pools;
 	symbol_list = & ctxt->symbols;
 	srcfiles = & ctxt->srcfiles;
 	rule_list = & ctxt->rule_list;
@@ -915,24 +892,16 @@ free_bc_internal(INSTRUCTION *cp)
 /* free_bcpool --- free list of instruction memory pools */
 
 static void
-free_bcpool(INSTRUCTION *pl)
+free_bcpool(INSTRUCTION_POOL *pl)
 {
-	INSTRUCTION *pool, *tmp;
-
-	for (pool = pl->nexti; pool != NULL; pool = tmp) {
-		INSTRUCTION *cp, *last;
-		long psiz;
-		psiz = pool->pool_size;
-		if (psiz == INSTR_CHUNK)
-			last = pool + psiz;
-		else
-			last = pool + 1;
-		for (cp = pool + 1; cp <= last ; cp++) {
+	INSTRUCTION *cp, *next;
+	int i;
+
+	for (i = 1; i <= MAX_INSTRUCTION_ALLOC; i++) {
+		for (cp = pl->pool[i].nexti; cp != NULL; cp = next) {
+			next = cp->nexti;
 			if (cp->opcode != 0)
 				free_bc_internal(cp);
 		}
-		tmp = pool->nexti;
-		efree(pool);
 	}
-	memset(pl, 0, sizeof(INSTRUCTION));
 }
diff --git a/test/memleak.awk b/test/memleak.awk
new file mode 100644
index 00000000..3937658f
--- /dev/null
+++ b/test/memleak.awk
@@ -0,0 +1,20 @@
+# This program doesn't do anything except allow us to
+# check for memory leak from using a user-supplied
+# sorting function.
+#
+# From Andrew Schorr.
+
+function my_func(i1, v1, i2, v2) {
+	return v2-v1
+}
+
+BEGIN {
+	a[1] = "3"
+	a[2] = "2"
+	a[3] = "4"
+	for (i = 0; i < 10000; i++) {
+		n = asort(a, b, "my_func")
+		s += n
+	}
+	print s
+}