Fix bug in PrintCallstack caused by trying to use MMIO

Also tidy up methods in Debugger_SymbolMap.cpp

Source/Core/Core/Debugger/Debugger_SymbolMap.cpp

@@ -23,9 +23,9 @@ void AddAutoBreakpoints()
 		"PPCHalt",
 	};
 
-	for (u32 i = 0; i < sizeof(bps) / sizeof(const char *); i++)
+	for (const char* bp : bps)
 	{
-		Symbol *symbol = g_symbolDB.GetSymbolFromName(bps[i]);
+		Symbol *symbol = g_symbolDB.GetSymbolFromName(bp);
 		if (symbol)
 			PowerPC::breakpoints.Add(symbol->address, false);
 	}
@@ -33,18 +33,43 @@ void AddAutoBreakpoints()
 #endif
 }
 
+// Returns true if the address is not a valid RAM address or NULL.
+bool IsStackBottom(u32 addr)
+{
+	return !addr || !Memory::IsRAMAddress(addr);
+}
+
+void WalkTheStack(const std::function<void(u32)>& stack_step)
+{
+	if (!IsStackBottom(PowerPC::ppcState.gpr[1]))
+	{
+		u32 addr = Memory::ReadUnchecked_U32(PowerPC::ppcState.gpr[1]);  // SP
+
+		// Walk the stack chain
+		for (int count = 0;
+		     !IsStackBottom(addr + 4) && (count++ < 20);
+		     ++count)
+		{
+			u32 func_addr = Memory::ReadUnchecked_U32(addr + 4);
+			stack_step(func_addr);
+
+			if (IsStackBottom(addr))
+				break;
+
+			addr = Memory::ReadUnchecked_U32(addr);
+		}
+	}
+}
+
 // Returns callstack "formatted for debugging" - meaning that it
 // includes LR as the last item, and all items are the last step,
 // instead of "pointing ahead"
 bool GetCallstack(std::vector<CallstackEntry> &output)
 {
-	if (Core::GetState() == Core::CORE_UNINITIALIZED)
+	if (Core::GetState() == Core::CORE_UNINITIALIZED ||
+	    !Memory::IsRAMAddress(PowerPC::ppcState.gpr[1]))
 		return false;
 
-	if (!Memory::IsRAMAddress(PowerPC::ppcState.gpr[1]))
-		return false;
-
-	u32 addr = Memory::ReadUnchecked_U32(PowerPC::ppcState.gpr[1]);  // SP
 	if (LR == 0)
 	{
 		CallstackEntry entry;
@@ -54,94 +79,62 @@ bool GetCallstack(std::vector<CallstackEntry> &output)
 		return false;
 	}
 
-	int count = 1;
-
 	CallstackEntry entry;
-	entry.Name = StringFromFormat(" * %s [ LR = %08x ]\n", g_symbolDB.GetDescription(LR), LR - 4);
+	entry.Name = StringFromFormat(" * %s [ LR = %08x ]\n", g_symbolDB.GetDescription(LR).c_str(), LR - 4);
 	entry.vAddress = LR - 4;
 	output.push_back(entry);
-	count++;
 
-	//walk the stack chain
+	WalkTheStack([&entry, &output](u32 func_addr) {
-	while ((addr != 0xFFFFFFFF) && (addr != 0) && (count++ < 20) && (PowerPC::ppcState.gpr[1] != 0))
+		std::string func_desc = g_symbolDB.GetDescription(func_addr);
-	{
+		if (func_desc.empty() || func_desc == "Invalid")
-		if (!Memory::IsRAMAddress(addr + 4))
+			func_desc = "(unknown)";
-			return false;
+		entry.Name = StringFromFormat(" * %s [ addr = %08x ]\n",
-
+		                              func_desc.c_str(), func_addr - 4);
-		u32 func = Memory::ReadUnchecked_U32(addr + 4);
+		entry.vAddress = func_addr - 4;
-		const char *str = g_symbolDB.GetDescription(func);
-		if (!str || strlen(str) == 0 || !strcmp(str, "Invalid"))
-			str = "(unknown)";
-
-		entry.Name = StringFromFormat(" * %s [ addr = %08x ]\n", str, func - 4);
-		entry.vAddress = func - 4;
 		output.push_back(entry);
-
+	});
-		if (!Memory::IsRAMAddress(addr))
-			return false;
-
-		addr = Memory::ReadUnchecked_U32(addr);
-	}
 
 	return true;
 }
 
 void PrintCallstack()
 {
-	u32 addr = Memory::ReadUnchecked_U32(PowerPC::ppcState.gpr[1]);  // SP
-
 	printf("== STACK TRACE - SP = %08x ==", PowerPC::ppcState.gpr[1]);
 
 	if (LR == 0) {
 		printf(" LR = 0 - this is bad");
 	}
-	int count = 1;
+	if (g_symbolDB.GetDescription(PC) != g_symbolDB.GetDescription(LR))
-	if (g_symbolDB.GetDescription(PowerPC::ppcState.pc) != g_symbolDB.GetDescription(LR))
 	{
-		printf(" * %s  [ LR = %08x ]", g_symbolDB.GetDescription(LR), LR);
+		printf(" * %s  [ LR = %08x ]", g_symbolDB.GetDescription(LR).c_str(), LR);
-		count++;
-	}
-
-	//walk the stack chain
-	while ((addr != 0xFFFFFFFF) && (addr != 0) && (count++ < 20) && (PowerPC::ppcState.gpr[1] != 0))
-	{
-		u32 func = Memory::ReadUnchecked_U32(addr + 4);
-		const char *str = g_symbolDB.GetDescription(func);
-		if (!str || strlen(str) == 0 || !strcmp(str, "Invalid"))
-			str = "(unknown)";
-		printf( " * %s [ addr = %08x ]", str, func);
-		addr = Memory::ReadUnchecked_U32(addr);
 	}
+	WalkTheStack([](u32 func_addr) {
+		std::string func_desc = g_symbolDB.GetDescription(func_addr);
+		if (func_desc.empty() || func_desc == "Invalid")
+			func_desc = "(unknown)";
+		printf(" * %s [ addr = %08x ]", func_desc.c_str(), func_addr);
+	});
 }
 
 void PrintCallstack(LogTypes::LOG_TYPE type, LogTypes::LOG_LEVELS level)
 {
-	u32 addr = Memory::ReadUnchecked_U32(PowerPC::ppcState.gpr[1]);  // SP
-
 	GENERIC_LOG(type, level, "== STACK TRACE - SP = %08x ==",
 				PowerPC::ppcState.gpr[1]);
 
 	if (LR == 0) {
 		GENERIC_LOG(type, level, " LR = 0 - this is bad");
 	}
-	int count = 1;
+	if (g_symbolDB.GetDescription(PC) != g_symbolDB.GetDescription(LR))
-	if (g_symbolDB.GetDescription(PowerPC::ppcState.pc) != g_symbolDB.GetDescription(LR))
 	{
 		GENERIC_LOG(type, level, " * %s  [ LR = %08x ]",
-					g_symbolDB.GetDescription(LR), LR);
+					g_symbolDB.GetDescription(LR).c_str(), LR);
-		count++;
-	}
-
-	//walk the stack chain
-	while ((addr != 0xFFFFFFFF) && (addr != 0) && (count++ < 20) && (PowerPC::ppcState.gpr[1] != 0))
-	{
-		u32 func = Memory::ReadUnchecked_U32(addr + 4);
-		const char *str = g_symbolDB.GetDescription(func);
-		if (!str || strlen(str) == 0 || !strcmp(str, "Invalid"))
-			str = "(unknown)";
-		GENERIC_LOG(type, level, " * %s [ addr = %08x ]", str, func);
-		addr = Memory::ReadUnchecked_U32(addr);
 	}
+	WalkTheStack([type, level](u32 func_addr) {
+		std::string func_desc = g_symbolDB.GetDescription(func_addr);
+		if (func_desc.empty() || func_desc == "Invalid")
+			func_desc = "(unknown)";
+		GENERIC_LOG(type, level, " * %s [ addr = %08x ]", func_desc.c_str(), func_addr);
+	});
 }
 
 void PrintDataBuffer(LogTypes::LOG_TYPE type, u8* _pData, size_t _Size, const char* _title)
@@ -149,15 +142,15 @@ void PrintDataBuffer(LogTypes::LOG_TYPE type, u8* _pData, size_t _Size, const ch
 	GENERIC_LOG(type, LogTypes::LDEBUG, "%s", _title);
 	for (u32 j = 0; j < _Size;)
 	{
-		std::string Temp;
+		std::string hex_line = "";
 		for (int i = 0; i < 16; i++)
 		{
-			Temp.append(StringFromFormat("%02x ", _pData[j++]));
+			hex_line += StringFromFormat("%02x ", _pData[j++]);
 
 			if (j >= _Size)
 				break;
 		}
-		GENERIC_LOG(type, LogTypes::LDEBUG, "   Data: %s", Temp.c_str());
+		GENERIC_LOG(type, LogTypes::LDEBUG, "   Data: %s", hex_line.c_str());
 	}
 }
 

Source/Core/Core/PowerPC/PPCSymbolDB.cpp

@@ -104,11 +104,11 @@ Symbol *PPCSymbolDB::GetSymbolFromAddr(u32 addr)
 	return 0;
 }
 
-const char *PPCSymbolDB::GetDescription(u32 addr)
+const std::string PPCSymbolDB::GetDescription(u32 addr)
 {
 	Symbol *symbol = GetSymbolFromAddr(addr);
 	if (symbol)
-		return symbol->name.c_str();
+		return symbol->name;
 	else
 		return " --- ";
 }

Source/Core/Core/PowerPC/PPCSymbolDB.h

@@ -30,7 +30,7 @@ public:
 
 	Symbol *GetSymbolFromAddr(u32 addr) override;
 
-	const char *GetDescription(u32 addr);
+	const std::string GetDescription(u32 addr);
 
 	void FillInCallers();