Fix multiple VM memory leaks

Also implement a feature to track leaked heap memory (DEBUG_HEAP). With this, it is now possible to hold CTRL through the entire Rance II digest version without crashing.
2026-09-30 02:48:14 +03:00 · 2019-11-27 18:11:07 -08:00
parent becda2c61d
commit 6039ab9bd0
5 changed files with 118 additions and 22 deletions
@@ -18,6 +18,7 @@
 #include "system4.h"
 #include "sdl_private.h"
 #include "input.h"
+#include "vm.h"

 bool key_state[VK_NR_KEYCODES];
 static enum sact_keycode sdl_keytable[];
@@ -72,7 +73,7 @@ void handle_events(void)
 	while (SDL_PollEvent(&e)) {
 		switch (e.type) {
 		case SDL_QUIT:
-			sys_exit(0);
+			vm_exit(0);
 			break;
 		case SDL_WINDOWEVENT:
 			switch (e.window.event) {
@@ -98,6 +98,7 @@ void variable_fini(union vm_value v, enum ain_data_type type)
 	case AIN_STRING:
 	case AIN_STRUCT:
 	case AIN_ARRAY_TYPE:
+	case AIN_REF_TYPE:
 		if (v.i == -1)
 			break;
 		heap_unref(v.i);
@@ -216,7 +217,7 @@ void create_struct(int no, union vm_value *var)
 	init_struct(no, var->i);
 }

-struct page *alloc_array(int rank, union vm_value *dimensions, int data_type, int struct_type)
+struct page *alloc_array(int rank, union vm_value *dimensions, int data_type, int struct_type, bool init_structs)
 {
 	if (rank < 1)
 		return NULL;
@@ -228,12 +229,12 @@ struct page *alloc_array(int rank, union vm_value *dimensions, int data_type, in

 	for (int i = 0; i < dimensions->i; i++) {
 		if (rank == 1) {
-			if (type == AIN_STRUCT)
+			if (type == AIN_STRUCT && init_structs)
 				create_struct(struct_type, &page->values[i]);
 			else
 				page->values[i] = variable_initval(type);
 		} else {
-			struct page *child = alloc_array(rank - 1, dimensions + 1, data_type, struct_type);
+			struct page *child = alloc_array(rank - 1, dimensions + 1, data_type, struct_type, init_structs);
 			int slot = heap_alloc_slot(VM_PAGE);
 			heap[slot].page = child;
 			page->values[i].i = slot;
@@ -242,14 +243,14 @@ struct page *alloc_array(int rank, union vm_value *dimensions, int data_type, in
 	return page;
 }

-struct page *realloc_array(struct page *src, int rank, union vm_value *dimensions, int data_type, int struct_type)
+struct page *realloc_array(struct page *src, int rank, union vm_value *dimensions, int data_type, int struct_type, bool init_structs)
 {
 	if (rank < 1)
 		ERROR("Tried to allocate 0-rank array");
 	if (!src && !dimensions->i)
 		return NULL;
 	if (!src)
-		return alloc_array(rank, dimensions, data_type, struct_type);
+		return alloc_array(rank, dimensions, data_type, struct_type, init_structs);
 	if (src->type != ARRAY_PAGE)
 		ERROR("Not an array");
 	if (src->rank != rank)
@@ -273,12 +274,12 @@ struct page *realloc_array(struct page *src, int rank, union vm_value *dimension
 	if (dimensions->i > src->nr_vars) {
 		for (int i = src->nr_vars; i < dimensions->i; i++) {
 			if (rank == 1) {
-				if (type == AIN_STRUCT)
+				if (type == AIN_STRUCT && init_structs)
 					create_struct(struct_type, &src->values[i]);
 				else
 					src->values[i] = variable_initval(type);
 			} else {
-				struct page *child = alloc_array(rank - 1, dimensions + 1, data_type, struct_type);
+				struct page *child = alloc_array(rank - 1, dimensions + 1, data_type, struct_type, init_structs);
 				int slot = heap_alloc_slot(VM_PAGE);
 				heap[slot].page = child;
 				src->values[i].i = slot;
@@ -358,12 +359,12 @@ void array_pushback(struct page **dst, union vm_value v, int data_type, int stru

 		int index = (*dst)->nr_vars;
 		union vm_value dims[1] = { (union vm_value) { .i = index + 1 } };
-		*dst = realloc_array(*dst, 1, dims, (*dst)->a_type, (*dst)->struct_type);
-		(*dst)->values[index] = vm_copy(v, array_type((*dst)->a_type));
+		*dst = realloc_array(*dst, 1, dims, (*dst)->a_type, (*dst)->struct_type, false);
+		(*dst)->values[index] = v;
 	} else {
 		union vm_value dims[1] = { (union vm_value) { .i = 1 } };
-		*dst = alloc_array(1, dims, data_type, struct_type);
-		(*dst)->values[0] = vm_copy(v, array_type((*dst)->a_type));
+		*dst = alloc_array(1, dims, data_type, struct_type, false);
+		(*dst)->values[0] = v;
 	}
 }

@@ -377,7 +378,7 @@ void array_popback(struct page **dst)
 		ERROR("Tried popping from a multi-dimensional array");

 	union vm_value dims[1] = { (union vm_value) { .i = (*dst)->nr_vars - 1 } };
-	*dst = realloc_array(*dst, 1, dims, (*dst)->a_type, (*dst)->struct_type);
+	*dst = realloc_array(*dst, 1, dims, (*dst)->a_type, (*dst)->struct_type, false);
 }

 bool array_erase(struct page **_page, int i)
@@ -433,7 +434,7 @@ void array_insert(struct page **_page, int i, union vm_value v, int data_type, i
 	for (int j = page->nr_vars - 1; j > i; j--) {
 		page->values[j] = page->values[j-1];
 	}
-	page->values[i] = vm_copy(v, array_type(page->a_type));
+	page->values[i] = v;
 }

 static int current_sort_function;
@@ -17,6 +17,7 @@
 #ifndef SYSTEM4_PAGE_H
 #define SYSTEM4_PAGE_H

+#include <stdbool.h>
 #include <stdint.h>
 #include "vm.h"
 #include "ain.h"
@@ -87,8 +88,8 @@ void init_struct(int no, int slot);
 void create_struct(int no, union vm_value *var);

 // arrays
-struct page *alloc_array(int rank, union vm_value *dimensions, int data_type, int struct_type);
-struct page *realloc_array(struct page *src, int rank, union vm_value *dimensions, int data_type, int struct_type);
+struct page *alloc_array(int rank, union vm_value *dimensions, int data_type, int struct_type, bool init_structs);
+struct page *realloc_array(struct page *src, int rank, union vm_value *dimensions, int data_type, int struct_type, bool init_structs);
 int array_numof(struct page *page, int rank);
 void array_copy(struct page *dst, int dst_i, struct page *src, int src_i, int n);
 int array_fill(struct page *dst, int dst_i, int n, union vm_value v);
@@ -103,6 +103,10 @@ int32_t heap_alloc_slot(enum vm_pointer_type type)
 	int32_t slot = heap_free_stack[heap_free_ptr++];
 	heap[slot].ref = 1;
 	heap[slot].type = type;
+#ifdef DEBUG_HEAP
+	heap[slot].alloc_addr = instr_ptr;
+	heap[slot].ref_addr = 0;
+#endif
 	return slot;
 }

@@ -114,6 +118,9 @@ static void heap_free_slot(int32_t slot)
 void heap_ref(int32_t slot)
 {
 	heap[slot].ref++;
+#ifdef DEBUG_HEAP
+	heap[slot].ref_addr = instr_ptr;
+#endif
 }

 static const char *vm_ptrtype_strtab[] = {
@@ -318,6 +325,11 @@ static void function_call(int fno, int return_address)
 	// pop arguments, store in local page
 	for (int i = f->nr_args - 1; i >= 0; i--) {
 		heap[slot].page->values[i] = stack_pop();
+		switch (f->vars[i].data_type) {
+		case AIN_REF_TYPE:
+			heap_ref(heap[slot].page->values[i].i);
+			break;
+		}
 	}
 	// initialize local variables
 	for (int i = f->nr_args; i < f->nr_vars; i++) {
@@ -373,7 +385,15 @@ static void hll_call(int libno, int fno)
 	}
 	union vm_value r = f->fun(args);
 	for (int i = 0; i < f->nr_arguments; i++) {
-		variable_fini(stack[stack_ptr + i], f->arguments[i].data_type);
+		// XXX: We don't increase the ref count when passing ref arguments to HLL
+		//      functions, so we need to avoid decreasing it via variable_fini
+		switch (f->arguments[i].data_type) {
+		case AIN_REF_TYPE:
+			break;
+		default:
+			variable_fini(stack[stack_ptr + i], f->arguments[i].data_type);
+			break;
+		}
 	}
 	if (f->data_type != AIN_VOID)
 		stack_push(r);
@@ -392,7 +412,7 @@ static void system_call(int32_t code)
 	struct string *str;
 	switch (code) {
 	case 0x0: // system.Exit(int nResult)
-		sys_exit(stack_pop().i);
+		vm_exit(stack_pop().i);
 		break;
 	case 0x3: // system.LockPeek()
 	case 0x4: // system.UnlockPeek()
@@ -668,7 +688,7 @@ static void execute_instruction(int16_t opcode)
 			sys_message("Assertion failed at %s:%d: %s\n", filename, i, value);
 			free(filename);
 			free(value);
-			sys_exit(1);
+			vm_exit(1);
 		}
 		heap_unref(a);
 		heap_unref(b);
@@ -1031,8 +1051,10 @@ static void execute_instruction(int16_t opcode)
 		a = stack_pop().i;
 		if (a == -1)
 			VM_ERROR("Assignment to null-pointer");
-		if (heap[a].page)
+		if (heap[a].page) {
 			delete_page(heap[a].page);
+			free_page(heap[a].page);
+		}
 		heap[a].page = copy_page(heap[b].page);
 		stack_push(b);
 		break;
@@ -1045,7 +1067,7 @@ static void execute_instruction(int16_t opcode)
 		pageno = stack_peek(a+1).i;
 		slot = heap[pageno].page->values[varno].i;
 		data_type = variable_type(heap[pageno].page, varno, &struct_type);
-		heap[slot].page = alloc_array(a, stack_peek_ptr(a-1), data_type, struct_type);
+		heap[slot].page = alloc_array(a, stack_peek_ptr(a-1), data_type, struct_type, true);
 		stack_ptr -= a + 2;
 		break;
 	case A_REALLOC:
@@ -1054,7 +1076,7 @@ static void execute_instruction(int16_t opcode)
 		pageno = stack_peek(a+1).i;
 		slot = heap[pageno].page->values[varno].i;
 		data_type = variable_type(heap[pageno].page, varno, &struct_type);
-		heap[slot].page = realloc_array(heap[slot].page, a, stack_peek_ptr(a-1), data_type, struct_type);
+		heap[slot].page = realloc_array(heap[slot].page, a, stack_peek_ptr(a-1), data_type, struct_type, true);
 		stack_ptr -= a + 2;
 		break;
 	case A_FREE:
@@ -1228,6 +1250,7 @@ void vm_execute_ain(struct ain *program)
 	link_libraries();

 	// Initialize globals
+	heap[0].ref = 1;
 	heap[0].page = alloc_page(GLOBAL_PAGE, 0, ain->nr_globals);
 	for (int i = 0; i < ain->nr_globals; i++) {
 		if (ain->globals[i].data_type == AIN_STRUCT) {
@@ -1290,3 +1313,68 @@ int vm_time(void)
 {
 	return clock() / (CLOCKS_PER_SEC / 1000);
 }
+
+#ifdef DEBUG_HEAP
+static void describe_page(struct page *page)
+{
+	if (!page) {
+		sys_message("NULL_PAGE\n");
+		return;
+	}
+
+	switch (page->type) {
+	case GLOBAL_PAGE:
+		sys_message("GLOBAL_PAGE\n");
+		break;
+	case LOCAL_PAGE:
+		sys_message("LOCAL_PAGE: %s\n", ain->functions[page->index].name);
+		break;
+	case STRUCT_PAGE:
+		sys_message("STRUCT_PAGE: %s\n", ain->structures[page->index].name);
+		break;
+	case ARRAY_PAGE:
+		sys_message("ARRAY_PAGE: %s\n", ain_strtype(ain, page->a_type, page->struct_type));
+		break;
+	}
+}
+
+static void describe_slot(size_t slot)
+{
+	sys_message("[%d](%d)(%08X)(%08X) = ", slot, heap[slot].ref, heap[slot].alloc_addr, heap[slot].ref_addr);
+	switch (heap[slot].type) {
+	case VM_PAGE:
+		describe_page(heap[slot].page);
+		break;
+	case VM_STRING:
+		if (heap[slot].s) {
+			char *u = sjis2utf(heap[slot].s->text, heap[slot].s->size);
+			sys_message("STRING: %s\n", u);
+			free(u);
+		} else {
+			sys_message("STRING: NULL\n");
+		}
+		break;
+	default:
+		sys_message("???\n");
+		break;
+	}
+}
+#endif
+
+noreturn void vm_exit(int code)
+{
+	// flush call stack
+	for (int i = call_stack_ptr - 1; i >= 0; i--) {
+		heap_unref(call_stack[i].page_slot);
+	}
+	// free globals
+	heap_unref(0);
+#ifdef DEBUG_HEAP
+	for (size_t i = 0; i < heap_size; i++) {
+		if (heap[i].ref > 0)
+			describe_slot(i);
+	}
+	sys_message("Number of leaked objects: %d\n", heap_free_ptr);
+#endif
+	sys_exit(code);
+}
@@ -51,6 +51,10 @@ struct vm_pointer {
 		struct string *s;
 		struct page *page;
 	};
+#ifdef DEBUG_HEAP
+	size_t alloc_addr;
+	size_t ref_addr;
+#endif
 };

 struct hll_function {
@@ -130,6 +134,7 @@ int vm_time(void);

 void vm_stack_trace(void);
 noreturn void _vm_error(const char *fmt, ...);
+noreturn void vm_exit(int code);

 #define VM_ERROR(fmt, ...) \
 	_vm_error("*ERROR*(%s:%s:%d): " fmt "\n", __FILE__, __func__, __LINE__, ##__VA_ARGS__)