DebugTools: Scan for functions from the ELF instead of from memory

pcsx2/DebugTools/DebugInterface.cpp

@@ -1181,3 +1181,84 @@ std::vector<std::unique_ptr<BiosThread>> R3000DebugInterface::GetThreadList() co
 {
 	return getIOPThreads();
 }
+
+ElfMemoryReader::ElfMemoryReader(const ccc::ElfFile& elf)
+	: m_elf(elf)
+{
+}
+
+u32 ElfMemoryReader::read8(u32 address)
+{
+	ccc::Result<u8> result = m_elf.get_object_virtual<u8>(address);
+	if (!result.success())
+		return 0;
+
+	return *result;
+}
+
+u32 ElfMemoryReader::read8(u32 address, bool& valid)
+{
+	ccc::Result<u8> result = m_elf.get_object_virtual<u8>(address);
+	valid = result.success();
+	if (!valid)
+		return 0;
+
+	return *result;
+}
+
+u32 ElfMemoryReader::read16(u32 address)
+{
+	ccc::Result<u16> result = m_elf.get_object_virtual<u16>(address);
+	if (!result.success())
+		return 0;
+
+	return *result;
+}
+
+u32 ElfMemoryReader::read16(u32 address, bool& valid)
+{
+	ccc::Result<u16> result = m_elf.get_object_virtual<u16>(address);
+	valid = result.success();
+	if (!valid)
+		return 0;
+
+	return *result;
+}
+
+u32 ElfMemoryReader::read32(u32 address)
+{
+	ccc::Result<u32> result = m_elf.get_object_virtual<u32>(address);
+	if (!result.success())
+		return 0;
+
+	return *result;
+}
+
+u32 ElfMemoryReader::read32(u32 address, bool& valid)
+{
+	ccc::Result<u32> result = m_elf.get_object_virtual<u32>(address);
+	valid = result.success();
+	if (!valid)
+		return 0;
+
+	return *result;
+}
+
+u64 ElfMemoryReader::read64(u32 address)
+{
+	ccc::Result<u64> result = m_elf.get_object_virtual<u64>(address);
+	if (!result.success())
+		return 0;
+
+	return *result;
+}
+
+u64 ElfMemoryReader::read64(u32 address, bool& valid)
+{
+	ccc::Result<u64> result = m_elf.get_object_virtual<u64>(address);
+	valid = result.success();
+	if (!valid)
+		return 0;
+
+	return *result;
+}

pcsx2/DebugTools/DebugInterface.h

@@ -32,15 +32,9 @@ enum BreakPointCpu
 	BREAKPOINT_IOP_AND_EE = 0x03
 };
 
-class DebugInterface
+class MemoryReader
 {
 public:
-	enum RegisterType
-	{
-		NORMAL,
-		SPECIAL
-	};
-
 	virtual u32 read8(u32 address) = 0;
 	virtual u32 read8(u32 address, bool& valid) = 0;
 	virtual u32 read16(u32 address) = 0;
@@ -49,6 +43,17 @@ public:
 	virtual u32 read32(u32 address, bool& valid) = 0;
 	virtual u64 read64(u32 address) = 0;
 	virtual u64 read64(u32 address, bool& valid) = 0;
+};
+
+class DebugInterface : public MemoryReader
+{
+public:
+	enum RegisterType
+	{
+		NORMAL,
+		SPECIAL
+	};
+
 	virtual u128 read128(u32 address) = 0;
 	virtual void write8(u32 address, u8 value) = 0;
 	virtual void write16(u32 address, u16 value) = 0;
@@ -140,7 +145,6 @@ public:
 	BreakPointCpu getCpuType() override;
 };
 
-
 class R3000DebugInterface : public DebugInterface
 {
 public:
@@ -183,5 +187,24 @@ public:
 	BreakPointCpu getCpuType() override;
 };
 
+// Provides access to the loadable segments from the ELF as they are on disk.
+class ElfMemoryReader : public MemoryReader
+{
+public:
+	ElfMemoryReader(const ccc::ElfFile& elf);
+
+	u32 read8(u32 address) override;
+	u32 read8(u32 address, bool& valid) override;
+	u32 read16(u32 address) override;
+	u32 read16(u32 address, bool& valid) override;
+	u32 read32(u32 address) override;
+	u32 read32(u32 address, bool& valid) override;
+	u64 read64(u32 address) override;
+	u64 read64(u32 address, bool& valid) override;
+
+protected:
+	const ccc::ElfFile& m_elf;
+};
+
 extern R5900DebugInterface r5900Debug;
 extern R3000DebugInterface r3000Debug;

pcsx2/DebugTools/MIPSAnalyst.cpp

@@ -23,9 +23,9 @@
 
 namespace MIPSAnalyst
 {
-	u32 GetJumpTarget(u32 addr)
+	u32 GetJumpTarget(u32 addr, MemoryReader& reader)
 	{
-		u32 op = r5900Debug.read32(addr);
+		u32 op = reader.read32(addr);
 		const R5900::OPCODE& opcode = R5900::GetInstruction(op);
 
 		if ((opcode.flags & IS_BRANCH) && (opcode.flags & BRANCHTYPE_MASK) == BRANCHTYPE_JUMP)
@@ -37,9 +37,9 @@ namespace MIPSAnalyst
 			return INVALIDTARGET;
 	}
 
-	u32 GetBranchTarget(u32 addr)
+	u32 GetBranchTarget(u32 addr, MemoryReader& reader)
 	{
-		u32 op = r5900Debug.read32(addr);
+		u32 op = reader.read32(addr);
 		const R5900::OPCODE& opcode = R5900::GetInstruction(op);
 
 		int branchType = (opcode.flags & BRANCHTYPE_MASK);
@@ -49,9 +49,9 @@ namespace MIPSAnalyst
 			return INVALIDTARGET;
 	}
 
-	u32 GetBranchTargetNoRA(u32 addr)
+	u32 GetBranchTargetNoRA(u32 addr, MemoryReader& reader)
 	{
-		u32 op = r5900Debug.read32(addr);
+		u32 op = reader.read32(addr);
 		const R5900::OPCODE& opcode = R5900::GetInstruction(op);
 
 		int branchType = (opcode.flags & BRANCHTYPE_MASK);
@@ -66,9 +66,9 @@ namespace MIPSAnalyst
 			return INVALIDTARGET;
 	}
 
-	u32 GetSureBranchTarget(u32 addr)
+	u32 GetSureBranchTarget(u32 addr, MemoryReader& reader)
 	{
-		u32 op = r5900Debug.read32(addr);
+		u32 op = reader.read32(addr);
 		const R5900::OPCODE& opcode = R5900::GetInstruction(op);
 
 		if ((opcode.flags & IS_BRANCH) && (opcode.flags & BRANCHTYPE_MASK) == BRANCHTYPE_BRANCH)
@@ -114,7 +114,7 @@ namespace MIPSAnalyst
 			return INVALIDTARGET;
 	}
 
-	static u32 ScanAheadForJumpback(u32 fromAddr, u32 knownStart, u32 knownEnd) {
+	static u32 ScanAheadForJumpback(u32 fromAddr, u32 knownStart, u32 knownEnd, MemoryReader& reader) {
 		static const u32 MAX_AHEAD_SCAN = 0x1000;
 		// Maybe a bit high... just to make sure we don't get confused by recursive tail recursion.
 		static const u32 MAX_FUNC_SIZE = 0x20000;
@@ -133,10 +133,10 @@ namespace MIPSAnalyst
 		u32 furthestJumpbackAddr = INVALIDTARGET;
 
 		for (u32 ahead = fromAddr; ahead < fromAddr + MAX_AHEAD_SCAN; ahead += 4) {
-			u32 aheadOp = r5900Debug.read32(ahead);
-			u32 target = GetBranchTargetNoRA(ahead);
+			u32 aheadOp = reader.read32(ahead);
+			u32 target = GetBranchTargetNoRA(ahead, reader);
 			if (target == INVALIDTARGET && ((aheadOp & 0xFC000000) == 0x08000000)) {
-				target = GetJumpTarget(ahead);
+				target = GetJumpTarget(ahead, reader);
 			}
 
 			if (target != INVALIDTARGET) {
@@ -157,10 +157,10 @@ namespace MIPSAnalyst
 
 		if (closestJumpbackAddr != INVALIDTARGET && furthestJumpbackAddr == INVALIDTARGET) {
 			for (u32 behind = closestJumpbackTarget; behind < fromAddr; behind += 4) {
-				u32 behindOp = r5900Debug.read32(behind);
-				u32 target = GetBranchTargetNoRA(behind);
+				u32 behindOp = reader.read32(behind);
+				u32 target = GetBranchTargetNoRA(behind, reader);
 				if (target == INVALIDTARGET && ((behindOp & 0xFC000000) == 0x08000000)) {
-					target = GetJumpTarget(behind);
+					target = GetJumpTarget(behind, reader);
 				}
 
 				if (target != INVALIDTARGET) {
@@ -174,7 +174,7 @@ namespace MIPSAnalyst
 		return furthestJumpbackAddr;
 	}
 
-	void ScanForFunctions(ccc::SymbolDatabase& database, u32 startAddr, u32 endAddr) {
+	void ScanForFunctions(ccc::SymbolDatabase& database, MemoryReader& reader, u32 startAddr, u32 endAddr) {
 		std::vector<MIPSAnalyst::AnalyzedFunction> functions;
 		AnalyzedFunction currentFunction = {startAddr};
 
@@ -199,9 +199,9 @@ namespace MIPSAnalyst
 				continue;
 			}
 
-			u32 op = r5900Debug.read32(addr);
+			u32 op = reader.read32(addr);
 
-			u32 target = GetBranchTargetNoRA(addr);
+			u32 target = GetBranchTargetNoRA(addr, reader);
 			if (target != INVALIDTARGET) {
 				isStraightLeaf = false;
 				if (target > furthestBranch) {
@@ -218,7 +218,7 @@ namespace MIPSAnalyst
 					}
 				}
 			} else if ((op & 0xFC000000) == 0x08000000) {
-				u32 sureTarget = GetJumpTarget(addr);
+				u32 sureTarget = GetJumpTarget(addr, reader);
 				// Check for a tail call.  Might not even have a jr ra.
 				if (sureTarget != INVALIDTARGET && sureTarget < currentFunction.start) {
 					if (furthestBranch > addr) {
@@ -230,7 +230,7 @@ namespace MIPSAnalyst
 				} else if (sureTarget != INVALIDTARGET && sureTarget > addr && sureTarget > furthestBranch) {
 					// A jump later.  Probably tail, but let's check if it jumps back.
 					u32 knownEnd = furthestBranch == 0 ? addr : furthestBranch;
-					u32 jumpback = ScanAheadForJumpback(sureTarget, currentFunction.start, knownEnd);
+					u32 jumpback = ScanAheadForJumpback(sureTarget, currentFunction.start, knownEnd, reader);
 					if (jumpback != INVALIDTARGET && jumpback > addr && jumpback > knownEnd) {
 						furthestBranch = jumpback;
 					} else {
@@ -256,10 +256,10 @@ namespace MIPSAnalyst
 
 			if (looking) {
 				if (addr >= furthestBranch) {
-					u32 sureTarget = GetSureBranchTarget(addr);
+					u32 sureTarget = GetSureBranchTarget(addr, reader);
 					// Regular j only, jals are to new funcs.
 					if (sureTarget == INVALIDTARGET && ((op & 0xFC000000) == 0x08000000)) {
-						sureTarget = GetJumpTarget(addr);
+						sureTarget = GetJumpTarget(addr, reader);
 					}
 
 					if (sureTarget != INVALIDTARGET && sureTarget < addr) {
@@ -268,7 +268,7 @@ namespace MIPSAnalyst
 						// Okay, we have a downward jump.  Might be an else or a tail call...
 						// If there's a jump back upward in spitting distance of it, it's an else.
 						u32 knownEnd = furthestBranch == 0 ? addr : furthestBranch;
-						u32 jumpback = ScanAheadForJumpback(sureTarget, currentFunction.start, knownEnd);
+						u32 jumpback = ScanAheadForJumpback(sureTarget, currentFunction.start, knownEnd, reader);
 						if (jumpback != INVALIDTARGET && jumpback > addr && jumpback > knownEnd) {
 							furthestBranch = jumpback;
 						}
@@ -287,7 +287,7 @@ namespace MIPSAnalyst
 				// Most functions are aligned to 8 or 16 bytes, so add padding
 				// to this one unless a symbol exists implying a new function
 				// follows immediately.
-				while (next_symbol == nullptr && ((addr+8) % 16) && r5900Debug.read32(addr+8) == 0)
+				while (next_symbol == nullptr && ((addr+8) % 16) && reader.read32(addr+8) == 0)
 					addr += 4;
 
 				currentFunction.end = addr + 4;
@@ -309,8 +309,10 @@ namespace MIPSAnalyst
 		functions.push_back(currentFunction);
 		
 		ccc::Result<ccc::SymbolSourceHandle> source = database.get_symbol_source("Analysis");
-		if(!source->valid())
+		if (!source.success()) {
+			Console.Error("MIPSAnalyst: %s", source.error().message.c_str());
 			return;
+		}
 
 		for (const AnalyzedFunction& function : functions) {
 			ccc::FunctionHandle handle = database.functions.first_handle_from_starting_address(function.start);
@@ -336,8 +338,10 @@ namespace MIPSAnalyst
 
 				ccc::Result<ccc::Function*> symbol_result = database.functions.create_symbol(
 					std::move(name), function.start, *source, nullptr);
-				if (!symbol_result.success())
+				if (!symbol_result.success()) {
+					Console.Error("MIPSAnalyst: %s", symbol_result.error().message.c_str());
 					return;
+				}
 				symbol = *symbol_result;
 			}
 

pcsx2/DebugTools/MIPSAnalyst.h

@@ -3,11 +3,9 @@
 
 #pragma once
 
+#include "DebugInterface.h"
 #include "SymbolGuardian.h"
 
-class DebugInterface;
-
-
 #define MIPS_GET_OP(op)   ((op>>26) & 0x3F)
 #define MIPS_GET_FUNC(op) (op & 0x3F)
 #define MIPS_GET_SA(op)   ((op>>6) & 0x1F)
@@ -29,7 +27,7 @@ namespace MIPSAnalyst
 		char name[64];
 	};
 
-	void ScanForFunctions(ccc::SymbolDatabase& database, u32 startAddr, u32 endAddr);
+	void ScanForFunctions(ccc::SymbolDatabase& database, MemoryReader& reader, u32 startAddr, u32 endAddr);
 
 	enum LoadStoreLRType { LOADSTORE_NORMAL, LOADSTORE_LEFT, LOADSTORE_RIGHT };
 

pcsx2/DebugTools/SymbolGuardian.cpp

@@ -134,16 +134,15 @@ void SymbolGuardian::ImportElf(std::vector<u8> elf, std::string elf_file_name, c
 		return;
 	}
 
-	std::unique_ptr<ccc::ElfSymbolFile> symbol_file =
-		std::make_unique<ccc::ElfSymbolFile>(std::move(*parsed_elf), std::move(elf_file_name));
+	ccc::ElfSymbolFile symbol_file(std::move(*parsed_elf), std::move(elf_file_name));
 
 	ShutdownWorkerThread();
 
-	m_import_thread = std::thread([this, nocash_path, file = std::move(symbol_file)]() {
+	m_import_thread = std::thread([this, nocash_path, worker_symbol_file = std::move(symbol_file)]() {
 		Threading::SetNameOfCurrentThread("Symbol Worker");
 
 		ccc::SymbolDatabase temp_database;
-		ImportSymbolTables(temp_database, *file, &m_interrupt_import_thread);
+		ImportSymbolTables(temp_database, worker_symbol_file, &m_interrupt_import_thread);
 
 		if (m_interrupt_import_thread)
 			return;
@@ -153,40 +152,24 @@ void SymbolGuardian::ImportElf(std::vector<u8> elf, std::string elf_file_name, c
 		if (m_interrupt_import_thread)
 			return;
 
-		ComputeOriginalFunctionHashes(temp_database, file->elf());
+		ComputeOriginalFunctionHashes(temp_database, worker_symbol_file.elf());
 
 		if (m_interrupt_import_thread)
 			return;
 
-		// Wait for the ELF to be loaded into memory, otherwise the call to
-		// ScanForFunctions below won't work.
-		while (true)
-		{
-			bool has_booted_elf = false;
-			Host::RunOnCPUThread([&]() {
-				has_booted_elf = VMManager::Internal::HasBootedELF();
-			},
-				true);
-
-			if (has_booted_elf)
-				break;
-
-			std::this_thread::sleep_for(std::chrono::milliseconds(100));
-
-			if (m_interrupt_import_thread)
-				return;
-		}
-
-		Host::RunOnCPUThread([this, &temp_database, &file, nocash_path]() {
 		ReadWrite([&](ccc::SymbolDatabase& database) {
 			database.merge_from(temp_database);
 
-				const ccc::ElfProgramHeader* entry_segment = file->elf().entry_point_segment();
+			if (m_interrupt_import_thread)
+				return;
+
+			const ccc::ElfProgramHeader* entry_segment = worker_symbol_file.elf().entry_point_segment();
 			if (entry_segment)
-					MIPSAnalyst::ScanForFunctions(database, entry_segment->vaddr, entry_segment->vaddr + entry_segment->filesz);
+			{
+				ElfMemoryReader reader(worker_symbol_file.elf());
+				MIPSAnalyst::ScanForFunctions(database, reader, entry_segment->vaddr, entry_segment->vaddr + entry_segment->filesz);
+			}
 		});
-		},
-			true);
 	});
 }