Debugger: Shaving recursive yaks takes a lot of work

CHANGES

@@ -53,6 +53,7 @@ Other fixes:
  - Core: Don't attempt to restore rewind diffs past start of rewind
  - Core: Fix the runloop resuming after a game has crashed (fixes mgba.io/i/2451)
  - Core: Fix crash if library can't be opened
+ - Debugger: Fix crash with extremely long CLI strings
  - FFmpeg: Fix crash when encoding audio with some containers
  - FFmpeg: Fix GIF recording (fixes mgba.io/i/2393)
  - GB: Fix temporary saves

include/mgba-util/vector.h

@@ -98,6 +98,7 @@ CXX_GUARD_START
 	} \
 
 DECLARE_VECTOR(StringList, char*);
+DECLARE_VECTOR(IntList, int);
 
 CXX_GUARD_END
 

include/mgba/internal/debugger/parser.h

@@ -43,7 +43,7 @@ enum Operation {
 
 struct Token {
 	enum TokenType {
-		TOKEN_ERROR_TYPE,
+		TOKEN_ERROR_TYPE = 0,
 		TOKEN_UINT_TYPE,
 		TOKEN_IDENTIFIER_TYPE,
 		TOKEN_OPERATOR_TYPE,
@@ -60,8 +60,10 @@ struct Token {
 
 struct ParseTree {
 	struct Token token;
+	struct ParseTree* p;
 	struct ParseTree* lhs;
 	struct ParseTree* rhs;
+	int precedence;
 };
 
 size_t lexExpression(struct LexVector* lv, const char* string, size_t length, const char* eol);

src/arm/debugger/debugger.c

@@ -190,14 +190,12 @@ static struct ARMDebugBreakpoint* _lookupBreakpoint(struct ARMDebugBreakpointLis
 static void _destroyBreakpoint(struct ARMDebugBreakpoint* breakpoint) {
 	if (breakpoint->d.condition) {
 		parseFree(breakpoint->d.condition);
-		free(breakpoint->d.condition);
 	}
 }
 
 static void _destroyWatchpoint(struct mWatchpoint* watchpoint) {
 	if (watchpoint->condition) {
 		parseFree(watchpoint->condition);
-		free(watchpoint->condition);
 	}
 }
 

src/debugger/cli-debugger.c

@@ -226,11 +226,9 @@ static bool _parseExpression(struct mDebugger* debugger, struct CLIDebugVector*
 	}
 	if (!mDebuggerEvaluateParseTree(debugger, tree, intValue, segmentValue)) {
 		parseFree(tree);
-		free(tree);
 		return false;
 	}
 	parseFree(tree);
-	free(tree);
 	return true;
 }
 
@@ -599,7 +597,6 @@ static struct ParseTree* _parseTree(const char** string) {
 	if (error) {
 		if (tree) {
 			parseFree(tree);
-			free(tree);
 		}
 		return NULL;
 	} else {

src/debugger/parser.c

@@ -11,6 +11,8 @@
 
 DEFINE_VECTOR(LexVector, struct Token);
 
+DEFINE_VECTOR(IntList, int32_t);
+
 enum LexState {
 	LEX_ERROR = -1,
 	LEX_ROOT = 0,
@@ -493,16 +495,21 @@ static const int _operatorPrecedence[] = {
 	[OP_DEREFERENCE] = 2,
 };
 
-static struct ParseTree* _parseTreeCreate() {
+static struct ParseTree* _parseTreeCreate(void) {
 	struct ParseTree* tree = malloc(sizeof(struct ParseTree));
 	tree->token.type = TOKEN_ERROR_TYPE;
-	tree->rhs = 0;
-	tree->lhs = 0;
+	tree->p = NULL;
+	tree->rhs = NULL;
+	tree->lhs = NULL;
+	tree->precedence = INT_MAX;
 	return tree;
 }
 
-static size_t _parseExpression(struct ParseTree* tree, struct LexVector* lv, size_t i, int precedence, int* openParens) {
-	struct ParseTree* newTree = 0;
+static size_t _parseExpression(struct ParseTree* tree, struct LexVector* lv, int* openParens) {
+	struct ParseTree* newTree = NULL;
+	bool pop = false;
+	int precedence = INT_MAX;
+	size_t i = 0;
 	while (i < LexVectorSize(lv)) {
 		struct Token* token = LexVectorGetPointer(lv, i);
 		int newPrecedence;
@@ -517,27 +524,36 @@ static size_t _parseExpression(struct ParseTree* tree, struct LexVector* lv, siz
 				++i;
 			} else {
 				tree->token.type = TOKEN_ERROR_TYPE;
-				return i + 1;
+				++i;
+				pop = true;
 			}
 			break;
 		case TOKEN_SEGMENT_TYPE:
 			tree->lhs = _parseTreeCreate();
 			tree->lhs->token.type = TOKEN_UINT_TYPE;
 			tree->lhs->token.uintValue = token->uintValue;
+			tree->lhs->p = tree;
+			tree->lhs->precedence = precedence;
 			tree->rhs = _parseTreeCreate();
+			tree->rhs->p = tree;
+			tree->rhs->precedence = precedence;
 			tree->token.type = TOKEN_SEGMENT_TYPE;
-			i = _parseExpression(tree->rhs, lv, i + 1, precedence, openParens);
+			tree = tree->rhs;
+			++i;
 			break;
 		case TOKEN_OPEN_PAREN_TYPE:
 			++*openParens;
-			i = _parseExpression(tree, lv, i + 1, INT_MAX, openParens);
+			precedence = INT_MAX;
+			++i;
 			break;
 		case TOKEN_CLOSE_PAREN_TYPE:
 			if (*openParens <= 0) {
 				tree->token.type = TOKEN_ERROR_TYPE;
 			}
 			--*openParens;
-			return i + 1;
+			++i;
+			pop = true;
+			break;
 		case TOKEN_OPERATOR_TYPE:
 			if (tree->token.type == TOKEN_ERROR_TYPE) {
 				switch (token->operatorValue) {
@@ -557,21 +573,44 @@ static size_t _parseExpression(struct ParseTree* tree, struct LexVector* lv, siz
 			newPrecedence = _operatorPrecedence[token->operatorValue];
 			if (newPrecedence < precedence) {
 				newTree = _parseTreeCreate();
-				*newTree = *tree;
+				memcpy(newTree, tree, sizeof(*tree));
+				if (newTree->lhs) {
+					newTree->lhs->p = newTree;
+				}
+				if (newTree->rhs) {
+					newTree->rhs->p = newTree;
+				}
+				newTree->p = tree;
 				tree->lhs = newTree;
 				tree->rhs = _parseTreeCreate();
+				tree->rhs->p = tree;
+				tree->rhs->precedence = newPrecedence;
+				precedence = newPrecedence;
 				tree->token = *token;
-				i = _parseExpression(tree->rhs, lv, i + 1, newPrecedence, openParens);
-				if (tree->token.type == TOKEN_ERROR_TYPE) {
-					tree->token.type = TOKEN_ERROR_TYPE;
-				}
+				tree = tree->rhs;
+				++i;
 			} else {
-				return i;
+				pop = true;
 			}
 			break;
 		case TOKEN_ERROR_TYPE:
 			tree->token.type = TOKEN_ERROR_TYPE;
-			return i + 1;
+			++i;
+			pop = true;
+			break;
+		}
+
+		if (pop) {
+			if (tree->token.type == TOKEN_ERROR_TYPE && tree->p) {
+				tree->p->token.type = TOKEN_ERROR_TYPE;
+			}
+			tree = tree->p;
+			pop = false;
+			if (!tree) {
+				break;
+			} else {
+				precedence = tree->precedence;
+			}
 		}
 	}
 
@@ -584,11 +623,13 @@ void parseLexedExpression(struct ParseTree* tree, struct LexVector* lv) {
 	}
 
 	tree->token.type = TOKEN_ERROR_TYPE;
-	tree->lhs = 0;
-	tree->rhs = 0;
+	tree->lhs = NULL;
+	tree->rhs = NULL;
+	tree->p = NULL;
+	tree->precedence = INT_MAX;
 
 	int openParens = 0;
-	_parseExpression(tree, lv, 0, INT_MAX, &openParens);
+	_parseExpression(tree, lv, &openParens);
 	if (openParens) {
 		if (tree->token.type == TOKEN_IDENTIFIER_TYPE) {
 			free(tree->token.identifierValue);
@@ -607,23 +648,40 @@ void lexFree(struct LexVector* lv) {
 	}
 }
 
-void parseFree(struct ParseTree* tree) {
-	if (!tree) {
-		return;
-	}
-
-	if (tree->lhs) {
-		parseFree(tree->lhs);
-		free(tree->lhs);
-	}
-	if (tree->rhs) {
-		parseFree(tree->rhs);
-		free(tree->rhs);
-	}
-
+static void _freeTree(struct ParseTree* tree) {
 	if (tree->token.type == TOKEN_IDENTIFIER_TYPE) {
 		free(tree->token.identifierValue);
 	}
+	free(tree);
+}
+
+void parseFree(struct ParseTree* tree) {
+	while (tree) {
+		if (tree->lhs) {
+			tree = tree->lhs;
+			continue;
+		}
+		if (tree->rhs) {
+			tree = tree->rhs;
+			continue;
+		}
+		if (tree->p) {
+			if (tree->p->lhs == tree) {
+				tree = tree->p;
+				_freeTree(tree->lhs);
+				tree->lhs = NULL;
+			} else if (tree->p->rhs == tree) {
+				tree = tree->p;
+				_freeTree(tree->rhs);
+				tree->rhs = NULL;
+			} else {
+				abort();
+			}
+		} else {
+			_freeTree(tree);
+			break;
+		}
+	}
 }
 
 static bool _performOperation(struct mDebugger* debugger, enum Operation operation, int32_t current, int32_t next, int32_t* value, int* segment) {
@@ -721,19 +779,22 @@ bool mDebuggerEvaluateParseTree(struct mDebugger* debugger, struct ParseTree* tr
 	if (!value) {
 		return false;
 	}
-	int32_t lhs, rhs;
+	struct IntList stack;
+	int nextBranch;
+	bool ok = true;
+	int32_t tmpVal, tmpSegment;
+
+	IntListInit(&stack, 0);
+	while (ok) {
 		switch (tree->token.type) {
 		case TOKEN_UINT_TYPE:
-		if (segment) {
-			*segment = -1;
-		}
-		*value = tree->token.uintValue;
-		return true;
+			nextBranch = 2;
+			tmpSegment = -1;
+			tmpVal = tree->token.uintValue;
+			break;
 		case TOKEN_SEGMENT_TYPE:
-		if (!mDebuggerEvaluateParseTree(debugger, tree->rhs, value, segment)) {
-			return false;
-		}
-		return mDebuggerEvaluateParseTree(debugger, tree->lhs, segment, NULL);
+			nextBranch = 0;
+			break;
 		case TOKEN_OPERATOR_TYPE:
 			switch (tree->token.operatorValue) {
 			case OP_ASSIGN:
@@ -755,22 +816,87 @@ bool mDebuggerEvaluateParseTree(struct mDebugger* debugger, struct ParseTree* tr
 			case OP_GE:
 			case OP_SHIFT_L:
 			case OP_SHIFT_R:
-			if (!mDebuggerEvaluateParseTree(debugger, tree->lhs, &lhs, segment)) {
-				return false;
-			}
-			// Fall through
+				nextBranch = 0;
+				break;
 			default:
-			if (!mDebuggerEvaluateParseTree(debugger, tree->rhs, &rhs, segment)) {
-				return false;
-			}
+				nextBranch = 1;
 				break;
 			}
-		return _performOperation(debugger, tree->token.operatorValue, lhs, rhs, value, segment);
+			break;
 		case TOKEN_IDENTIFIER_TYPE:
-		return mDebuggerLookupIdentifier(debugger, tree->token.identifierValue, value, segment);
+			if (!mDebuggerLookupIdentifier(debugger, tree->token.identifierValue, &tmpVal, &tmpSegment)) {
+				ok = false;
+			}
+			nextBranch = 2;
+			break;
 		case TOKEN_ERROR_TYPE:
 		default:
+			ok = false;
 			break;
 		}
-	return false;
+		if (!ok) {
+			break;
+		}
+
+		bool gotTree = false;
+		while (!gotTree && tree) {
+			int32_t lhs, rhs;
+
+			switch (nextBranch) {
+			case 0:
+				*IntListAppend(&stack) = tmpVal;
+				*IntListAppend(&stack) = tmpSegment;
+				*IntListAppend(&stack) = nextBranch;
+				tree = tree->lhs;
+				gotTree = true;
+				break;
+			case 1:
+				*IntListAppend(&stack) = tmpVal;
+				*IntListAppend(&stack) = tmpSegment;
+				*IntListAppend(&stack) = nextBranch;
+				tree = tree->rhs;
+				gotTree = true;
+				break;
+			case 2:
+				if (!IntListSize(&stack)) {
+					tree = NULL;
+					break;
+				}
+				nextBranch = *IntListGetPointer(&stack, IntListSize(&stack) - 1);
+				IntListResize(&stack, -1);
+				tree = tree->p;
+				if (nextBranch == 0) {
+					++nextBranch;
+				} else if (tree) {
+					nextBranch = 2;
+					switch (tree->token.type) {
+					case TOKEN_OPERATOR_TYPE:
+						rhs = tmpVal;
+						lhs = *IntListGetPointer(&stack, IntListSize(&stack) - 2);
+						tmpSegment = *IntListGetPointer(&stack, IntListSize(&stack) - 1);
+						ok = _performOperation(debugger, tree->token.operatorValue, lhs, rhs, &tmpVal, &tmpSegment);
+						break;
+					case TOKEN_SEGMENT_TYPE:
+						tmpSegment = *IntListGetPointer(&stack, IntListSize(&stack) - 2);
+						break;
+					default:
+						break;
+					}
+				}
+				IntListResize(&stack, -2);
+				break;
+			}
+		}
+		if (!tree) {
+			break;
+		}
+	}
+	IntListDeinit(&stack);
+	if (ok) {
+		*value = tmpVal;
+		if (segment) {
+			*segment = tmpSegment;
+		}
+	}
+	return ok;
 }

src/debugger/test/parser.c

@@ -9,7 +9,7 @@
 
 struct LPTest {
 	struct LexVector lv;
-	struct ParseTree tree;
+	struct ParseTree* tree;
 };
 
 #define PARSE(STR) \
@@ -18,7 +18,8 @@ struct LPTest {
 	LexVectorClear(&lp->lv); \
 	size_t adjusted = lexExpression(&lp->lv, STR, strlen(STR), ""); \
 	assert_false(adjusted > strlen(STR)); \
-	struct ParseTree* tree = &lp->tree; \
+	lp->tree = malloc(sizeof(*lp->tree)); \
+	struct ParseTree* tree = lp->tree; \
 	parseLexedExpression(tree, &lp->lv)
 
 static int parseSetup(void** state) {
@@ -30,7 +31,7 @@ static int parseSetup(void** state) {
 
 static int parseTeardown(void** state) {
 	struct LPTest* lp = *state;
-	parseFree(&lp->tree);
+	parseFree(lp->tree);
 	lexFree(&lp->lv);
 	LexVectorDeinit(&lp->lv);
 	free(lp);

src/sm83/debugger/debugger.c

@@ -28,14 +28,12 @@ static struct mBreakpoint* _lookupBreakpoint(struct mBreakpointList* breakpoints
 static void _destroyBreakpoint(struct mBreakpoint* breakpoint) {
 	if (breakpoint->condition) {
 		parseFree(breakpoint->condition);
-		free(breakpoint->condition);
 	}
 }
 
 static void _destroyWatchpoint(struct mWatchpoint* watchpoint) {
 	if (watchpoint->condition) {
 		parseFree(watchpoint->condition);
-		free(watchpoint->condition);
 	}
 }