debugger: add single-stepping commands

Add 'step' (s) and 'next' (n) commands for single-stepping bytecode.
'step' is the 'step into' type which descends into function calls,
wheras 'next' is the 'step over' type which continues until the next
instruction within the current function.

include/debugger.h

@@ -62,6 +62,8 @@ void dbg_cmd_add_module(const char *name, unsigned nr_commands, struct dbg_cmd *
 void dbg_handle_breakpoint(void);
 bool dbg_set_function_breakpoint(const char *_name, void(*cb)(struct breakpoint*), void *data);
 bool dbg_set_address_breakpoint(uint32_t address, void(*cb)(struct breakpoint*), void *data);
+bool dbg_set_step_over_breakpoint(void);
+bool dbg_set_step_into_breakpoint(void);
 void dbg_print_frame(unsigned no);
 void dbg_print_stack_trace(void);
 void dbg_print_dasm(void);

include/vm.h

@@ -88,8 +88,12 @@ static inline void stack_push_value(union vm_value v)
 #define stack_set(n, v) (stack_set_value((n), vm_value_cast(v)))
 #define stack_push(v) (stack_push_value(vm_value_cast(v)))
 union vm_value stack_pop(void);
+union vm_value stack_peek(int n);
+union vm_value *stack_peek_var(void);
 
+union vm_value local_get(int varno);
 union vm_value global_get(int varno);
+union vm_value member_get(int varno);
 void global_set(int varno, union vm_value val, bool call_dtors);
 struct page *local_page(void);
 struct page *get_local_page(int frame_no);
@@ -117,6 +121,8 @@ _Noreturn void vm_exit(int code);
 	_vm_error("*ERROR*(%s:%s:%d): " fmt "\n", __FILE__, __func__, __LINE__, ##__VA_ARGS__)
 
 #ifdef VM_PRIVATE
+#include "little_endian.h"
+#include "system4/ain.h"
 
 struct function_call {
 	int32_t fno;
@@ -131,6 +137,23 @@ extern int32_t call_stack_ptr;
 
 extern size_t instr_ptr;
 
+// Read argument N for the current instruction.
+static inline int32_t get_argument(int n)
+{
+	return LittleEndian_getDW(ain->code, instr_ptr + 2 + n*4);
+}
+
+// XXX: not strictly portable
+static inline float get_argument_float(int n)
+{
+	union vm_value v;
+	v.i = LittleEndian_getDW(ain->code, instr_ptr + 2 + n*4);
+	return v.f;
+}
+
+uint32_t get_switch_address(int no, int val);
+uint32_t get_strswitch_address(int no, struct string *str);
+
 int vm_save_image(const char *key, const char *path);
 void vm_load_image(const char *key, const char *path);
 struct page *vm_load_image_comments(const char *key, const char *path, int *success);

src/debug.c

@@ -114,6 +114,20 @@ static void add_breakpoint(uint32_t addr, struct breakpoint *bp)
 	slot->value = bp;
 }
 
+static void delete_breakpoint(uint32_t addr, struct breakpoint *bp)
+{
+	// restore opcode
+	LittleEndian_putW(ain->code, addr, bp->restore_op);
+
+	// remove from hash table
+	struct ht_slot *slot = ht_put_int(bp_table, addr, NULL);
+	assert(slot->value == bp);
+	slot->value = NULL;
+
+	free(bp->message);
+	free(bp);
+}
+
 static struct breakpoint *get_breakpoint(uint32_t addr)
 {
 	if (!bp_table)
@@ -175,6 +189,176 @@ bool dbg_set_address_breakpoint(uint32_t address, void(*cb)(struct breakpoint*),
 	return true;
 }
 
+static void dbg_step_breakpoint_cb(struct breakpoint *bp)
+{
+	// XXX: mutually recursive functions could trigger breakpoint early.
+	//      use size of call stack to check for this case
+	if ((intptr_t)bp->data != call_stack_ptr)
+		return;
+	delete_breakpoint(instr_ptr, bp);
+	dbg_cmd_repl();
+}
+
+static void dbg_set_step_breakpoint(int32_t address, int call_index)
+{
+	// if a breakpoint is already set on the next address, leave it
+	enum opcode op = LittleEndian_getW(ain->code, address);
+	if ((op & OPTYPE_MASK) == BREAKPOINT)
+		return;
+
+	struct breakpoint *bp = xcalloc(1, sizeof(struct breakpoint));
+	bp->restore_op = op & ~OPTYPE_MASK;
+	assert(bp->restore_op >= 0 && bp->restore_op < NR_OPCODES);
+	bp->cb = dbg_step_breakpoint_cb;
+	bp->data = (void*)(intptr_t)call_index;
+	bp->message = NULL;
+	LittleEndian_putW(ain->code, address, BREAKPOINT | bp->restore_op);
+	add_breakpoint(address, bp);
+}
+
+static int32_t get_function_address(int fno)
+{
+	assert(fno > 0 && fno < ain->nr_functions);
+	return ain->functions[fno].address;
+}
+
+/* Determine the next address at the current instruction. */
+static int32_t dbg_next_address(bool into, int *call_index)
+{
+	*call_index = call_stack_ptr;
+	enum opcode current = LittleEndian_getW(ain->code, instr_ptr) & ~OPTYPE_MASK;
+	assert(current >= 0 && current < NR_OPCODES);
+	switch (current) {
+	case JUMP:
+		return get_argument(0);
+	case IFZ:
+		if (stack_peek(0).i == 0)
+			return get_argument(0);
+		break;
+	case IFNZ:
+		if (stack_peek(0).i)
+			return get_argument(0);
+		break;
+	case RETURN: {
+		*call_index = call_stack_ptr-1;
+		return call_stack[call_stack_ptr-1].return_address;
+	}
+	case _MSG:
+		if (!into)
+			break;
+		// TODO: step into message function
+		return -1;
+	case SWITCH:
+		return get_switch_address(get_argument(0), stack_peek(0).i);
+	case STRSWITCH:
+		return get_strswitch_address(get_argument(0), heap_get_string(stack_peek(0).i));
+	case SJUMP: {
+		if (!into)
+			break;
+		// XXX: can't determine RETURN address of scenario call (VM_RETURN)
+		return -1;
+		/*
+		int fno = heap[stack_peek(0).i].page->index;
+		return ain->functions[fno].address;
+		*/
+	}
+	case CALLFUNC:
+	case CALLMETHOD:
+	case THISCALLMETHOD_NOPARAM:
+		if (!into)
+			break;
+		*call_index = call_stack_ptr + 1;
+		return get_function_address(get_argument(0));
+	case CALLFUNC2:
+		if (!into)
+			break;
+		*call_index = call_stack_ptr + 1;
+		return get_function_address(stack_peek(1).i);
+	case SH_IF_LOC_LT_IMM:
+		if (local_get(get_argument(0)).i < get_argument(1))
+			return get_argument(2);
+		break;
+	case SH_IF_LOC_GE_IMM:
+		if (local_get(get_argument(0)).i >= get_argument(1))
+			return get_argument(2);
+		break;
+	case SH_IF_STRUCTREF_NE_LOCALREF:
+		if (member_get(get_argument(0)).i != local_get(get_argument(1)).i)
+			return get_argument(2);
+		break;
+	case SH_IF_STRUCTREF_GT_IMM:
+		if (member_get(get_argument(0)).i > get_argument(1))
+			return get_argument(2);
+		break;
+	case SH_STRUCTREF2_CALLMETHOD_NO_PARAM:
+		if (!into)
+			break;
+		*call_index = call_stack_ptr + 1;
+		return get_function_address(get_argument(2));
+	case SH_IF_STRUCTREF_Z:
+		if (!member_get(get_argument(0)).i)
+			return get_argument(1);
+		break;
+	case SH_IF_STRUCT_A_NOT_EMPTY: {
+		struct page *array = heap_get_page(member_get(get_argument(0)).i);
+		if (array && array->nr_vars)
+			return get_argument(1);
+		break;
+	}
+	case SH_IF_LOC_GT_IMM:
+		if (local_get(get_argument(0)).i > get_argument(1))
+			return get_argument(2);
+		break;
+	case SH_IF_STRUCTREF_NE_IMM:
+		if (member_get(get_argument(0)).i != get_argument(1))
+			return get_argument(2);
+		break;
+	case SH_IF_STRUCTREF_EQ_IMM:
+		if (member_get(get_argument(0)).i == get_argument(1))
+			return get_argument(2);
+		break;
+	case SH_IF_SREF_NE_STR0: {
+		struct string *a = heap_get_string(stack_peek_var()->i);
+		struct string *b = ain->strings[get_argument(0)];
+		if (strcmp(a->text, b->text))
+			return get_argument(1);
+		break;
+	}
+	case DG_CALL: {
+		if (!into)
+			return get_argument(1);
+		// XXX: can't determine RETURN address of delegate call (VM_RETURN)
+		return -1;
+	}
+	default:
+		// XXX: catch any unhandled control-flow instructions
+		if (instructions[current].ip_inc == 0)
+			return -1;
+		break;
+	}
+	return instr_ptr + instruction_width(current);
+}
+
+bool dbg_set_step_over_breakpoint(void)
+{
+	int call_index;
+	int32_t address = dbg_next_address(false, &call_index);
+	if (address < 0)
+		return false;
+	dbg_set_step_breakpoint(address, call_index);
+	return true;
+}
+
+bool dbg_set_step_into_breakpoint(void)
+{
+	int call_index;
+	int32_t address = dbg_next_address(true, &call_index);
+	if (address < 0)
+		return false;
+	dbg_set_step_breakpoint(address, call_index);
+	return true;
+}
+
 static void _dbg_handle_breakpoint(void *data)
 {
 	struct breakpoint *bp = data;

src/debugger_cmd.c

@@ -39,6 +39,7 @@
 #include "xsystem4.h"
 
 struct dbg_cmd_node;
+static bool stepping = false;
 
 struct dbg_cmd_list {
 	unsigned nr_commands;
@@ -75,6 +76,7 @@ static void dbg_cmd_breakpoint(unsigned nr_args, char **args)
 
 static void dbg_cmd_continue(unsigned nr_args, char **args)
 {
+	stepping = false;
 	dbg_continue();
 }
 
@@ -179,6 +181,24 @@ static void dbg_cmd_scene(unsigned nr_args, char **args)
 	scene_print();
 }
 
+static void dbg_cmd_next(unsigned nr_args, char **args)
+{
+	if (!dbg_set_step_over_breakpoint())
+		DBG_ERROR("Can't step over this instruction");
+	else
+		stepping = true;
+	dbg_continue();
+}
+
+static void dbg_cmd_step(unsigned nr_args, char **args)
+{
+	if (!dbg_set_step_into_breakpoint())
+		DBG_ERROR("Can't step into this instruction");
+	else
+		stepping = true;
+	dbg_continue();
+}
+
 #ifdef HAVE_SCHEME
 static void dbg_cmd_scheme(unsigned nr_args, char **args)
 {
@@ -200,9 +220,11 @@ static struct dbg_cmd dbg_default_commands[] = {
 	{ "locals",     "l",   "[frame-number] - Print local variables", 0, 1, dbg_cmd_locals },
 	{ "log",        NULL,  "<function-name> - Log function calls",   1, 1, dbg_cmd_log },
 	{ "members",    "m",   "[frame-number] - Print struct members",  0, 1, dbg_cmd_members },
+	{ "next",       "n",   "- Step to next instruction (in current function)", 0, 0, dbg_cmd_next },
 	{ "print",      "p",   "<variable-name> - Print a variable",     1, 1, dbg_cmd_print },
 	{ "quit",       "q",   "- Quit xsystem4",                        0, 0, dbg_cmd_quit },
 	{ "scene",      NULL,  "- Display scene graph",                  0, 0, dbg_cmd_scene },
+	{ "step",       "s",   "- Step to next instruction",             0, 0, dbg_cmd_step },
 #ifdef HAVE_SCHEME
 	{ "scheme",     "scm", "- Drop into Scheme REPL",                0, 0, dbg_cmd_scheme },
 #endif
@@ -400,7 +422,10 @@ static void execute_line(char *line)
 
 void dbg_cmd_repl(void)
 {
-	puts("Entering the debugger REPL. Type 'help' for a list of commands.");
+	if (stepping)
+		dbg_print_vm_state();
+	else
+		puts("Entering the debugger REPL. Type 'help' for a list of commands.");
 	while (1) {
 		char *line = cmd_gets();
 		if (line)

src/vm.c

@@ -77,20 +77,6 @@ static int16_t get_opcode(size_t addr)
 	return LittleEndian_getW(ain->code, addr);
 }
 
-// Read argument N for the current instruction.
-static int32_t get_argument(int n)
-{
-	return LittleEndian_getDW(ain->code, instr_ptr + 2 + n*4);
-}
-
-// XXX: not strictly portable
-static float get_argument_float(int n)
-{
-	union vm_value v;
-	v.i = LittleEndian_getDW(ain->code, instr_ptr + 2 + n*4);
-	return v.f;
-}
-
 static const char *current_instruction_name(void)
 {
 	int16_t opcode = get_opcode(instr_ptr);
@@ -117,7 +103,7 @@ struct page *get_local_page(int frame_no)
 	return slot < 1 ? NULL : heap[slot].page;
 }
 
-static union vm_value local_get(int varno)
+union vm_value local_get(int varno)
 {
 	return local_page()->values[varno];
 }
@@ -178,7 +164,7 @@ struct page *get_struct_page(int frame_no)
 	return slot < 1 ? NULL : heap[slot].page;
 }
 
-static union vm_value member_get(int varno)
+union vm_value member_get(int varno)
 {
 	return struct_page()->values[varno];
 }
@@ -188,7 +174,7 @@ static void member_set(int varno, int32_t value)
 	struct_page()->values[varno].i = value;
 }
 
-static union vm_value stack_peek(int n)
+union vm_value stack_peek(int n)
 {
 	return stack[stack_ptr - (1 + n)];
 }
@@ -216,6 +202,17 @@ static union vm_value *stack_pop_var(void)
 	return &heap[heap_index].page->values[page_index];
 }
 
+union vm_value *stack_peek_var(void)
+{
+	int32_t page_index = stack_peek(0).i;
+	int32_t heap_index = stack_peek(1).i;
+	if (unlikely(!heap_index_valid(heap_index)))
+		VM_ERROR("Out of bounds heap index: %d/%d", heap_index, page_index);
+	if (unlikely(!heap[heap_index].page || page_index >= heap[heap_index].page->nr_vars))
+		VM_ERROR("Out of bounds page index: %d/%d", heap_index, page_index);
+	return &heap[heap_index].page->values[page_index];
+}
+
 static void stack_push_string(struct string *s)
 {
 	int32_t heap_slot = heap_alloc_slot(VM_STRING);
@@ -662,34 +659,32 @@ static void system_call(enum syscall_code code)
 	}
 }
 
-void exec_switch(int no, int val)
+uint32_t get_switch_address(int no, int val)
 {
 	struct ain_switch *s = &ain->switches[no];
 	for (int i = 0; i < s->nr_cases; i++) {
 		if (s->cases[i].value == val) {
-			instr_ptr = s->cases[i].address;
-			return;
+			return s->cases[i].address;
 		}
 	}
 	if (s->default_address > 0)
-		instr_ptr = s->default_address;
+		return s->default_address;
 	else
-		instr_ptr += instruction_width(SWITCH);
+		return instr_ptr + instruction_width(SWITCH);
 }
 
-void exec_strswitch(int no, struct string *str)
+uint32_t get_strswitch_address(int no, struct string *str)
 {
 	struct ain_switch *s = &ain->switches[no];
 	for (int i = 0; i < s->nr_cases; i++) {
 		if (!strcmp(str->text, ain->strings[s->cases[i].value]->text)) {
-			instr_ptr = s->cases[i].address;
-			return;
+			return s->cases[i].address;
 		}
 	}
 	if (s->default_address > 0)
-		instr_ptr = s->default_address;
+		return s->default_address;
 	else
-		instr_ptr += instruction_width(STRSWITCH);
+		return instr_ptr + instruction_width(STRSWITCH);
 }
 
 static void echo_message(int i)
@@ -957,12 +952,12 @@ static enum opcode execute_instruction(enum opcode opcode)
 		break;
 	}
 	case SWITCH: {
-		exec_switch(get_argument(0), stack_pop().i);
+		instr_ptr = get_switch_address(get_argument(0), stack_pop().i);
 		break;
 	}
 	case STRSWITCH: {
 		int str = stack_pop().i;
-		exec_strswitch(get_argument(0), heap_get_string(str));
+		instr_ptr = get_strswitch_address(get_argument(0), heap_get_string(str));
 		heap_unref(str);
 		break;
 	}