Merge pull request #19655 from hrydgard/more-im-debugger

More ImDebugger stuff

Common/System/Request.h

@@ -13,6 +13,7 @@ typedef std::function<void(const char *responseString, int responseValue)> Reque
 typedef std::function<void()> RequestFailedCallback;
 
 typedef int RequesterToken;
+
 #define NO_REQUESTER_TOKEN -1
 #define NON_EPHEMERAL_TOKEN -2
 
@@ -101,13 +102,18 @@ enum class BrowseFileType {
 	DB,
 	SOUND_EFFECT,
 	ZIP,
+	SYMBOL_MAP,
 	ANY,
 };
 
 inline void System_BrowseForFile(RequesterToken token, std::string_view title, BrowseFileType type, RequestCallback callback, RequestFailedCallback failedCallback = nullptr) {
 	g_requestManager.MakeSystemRequest(SystemRequestType::BROWSE_FOR_FILE, token, callback, failedCallback, title, "", (int)type);
 }
 
+inline void System_BrowseForFileSave(RequesterToken token, std::string_view title, std::string_view defaultFilename, BrowseFileType type, RequestCallback callback, RequestFailedCallback failedCallback = nullptr) {
+	g_requestManager.MakeSystemRequest(SystemRequestType::BROWSE_FOR_FILE_SAVE, token, callback, failedCallback, title, defaultFilename, (int)type);
+}
+
 void System_BrowseForFolder(RequesterToken token, std::string_view title, const Path &initialPath, RequestCallback callback, RequestFailedCallback failedCallback = nullptr);
 
 // The returned string is username + '\n' + password.

Common/System/System.h

@@ -70,6 +70,8 @@ enum class SystemRequestType {
 	BROWSE_FOR_FILE,
 	BROWSE_FOR_FOLDER,
 
+	BROWSE_FOR_FILE_SAVE,
+
 	EXIT_APP,
 	RESTART_APP,  // For graphics backend changes
 	RECREATE_ACTIVITY,  // Android

Core/Core.cpp

@@ -56,15 +56,15 @@
 static std::mutex g_stepMutex;
 struct StepCommand {
 	CPUStepType type;
-	int param;
+	int stepSize;
 	const char *reason;
 	u32 relatedAddr;
 	bool empty() const {
 		return type == CPUStepType::None;
 	}
 	void clear() {
 		type = CPUStepType::None;
-		param = 0;
+		stepSize = 0;
 		reason = "";
 		relatedAddr = 0;
 	}
@@ -243,11 +243,12 @@ bool Core_RequestSingleStep(CPUStepType type, int stepSize) {
 		ERROR_LOG(Log::CPU, "Can't submit two steps in one frame");
 		return false;
 	}
+	// Out-steps don't need a size.
+	_dbg_assert_(stepSize != 0 || type == CPUStepType::Out);
 	g_stepCommand = { type, stepSize };
 	return true;
 }
 
-// See comment in header.
 // Handles more advanced step types (used by the debugger).
 // stepSize is to support stepping through compound instructions like fused lui+ladd (li).
 // Yes, our disassembler does support those.
@@ -264,16 +265,18 @@ static void Core_PerformStep(MIPSDebugInterface *cpu, CPUStepType stepType, int
 		for (int i = 0; i < (int)(newAddress - currentPc) / 4; i++) {
 			currentMIPS->SingleStep();
 		}
-		return;
+		break;
 	}
 	case CPUStepType::Over:
 	{
 		u32 currentPc = cpu->GetPC();
 		u32 breakpointAddress = currentPc + stepSize;
 
 		g_breakpoints.SetSkipFirst(currentPc);
-
 		MIPSAnalyst::MipsOpcodeInfo info = MIPSAnalyst::GetOpcodeInfo(cpu, cpu->GetPC());
+
+		// TODO: Doing a step over in a delay slot is a bit .. unclear. Maybe just do a single step.
+
 		if (info.isBranch) {
 			if (info.isConditional == false) {
 				if (info.isLinkedBranch) { // jal, jalr
@@ -290,10 +293,14 @@ static void Core_PerformStep(MIPSDebugInterface *cpu, CPUStepType stepType, int
 					breakpointAddress = currentPc + 2 * cpu->getInstructionSize(0);
 				}
 			}
+			g_breakpoints.AddBreakPoint(breakpointAddress, true);
+			Core_Resume();
+		} else {
+			// If not a branch, just do a simple single-step, no point in involving the breakpoint machinery.
+			for (int i = 0; i < (int)(breakpointAddress - currentPc) / 4; i++) {
+				currentMIPS->SingleStep();
+			}
 		}
-
-		g_breakpoints.AddBreakPoint(breakpointAddress, true);
-		Core_Resume();
 		break;
 	}
 	case CPUStepType::Out:
@@ -361,7 +368,7 @@ void Core_ProcessStepping(MIPSDebugInterface *cpu) {
 	Core_ResetException();
 
 	if (!g_stepCommand.empty()) {
-		Core_PerformStep(cpu, g_stepCommand.type, g_stepCommand.param);
+		Core_PerformStep(cpu, g_stepCommand.type, g_stepCommand.stepSize);
 		if (g_stepCommand.type == CPUStepType::Into) {
 			// We're already done. The other step types will resume the CPU.
 			System_Notify(SystemNotification::DISASSEMBLY_AFTERSTEP);
@@ -418,11 +425,19 @@ void Core_Break(const char *reason, u32 relatedAddress) {
 	{
 		std::lock_guard<std::mutex> lock(g_stepMutex);
 		if (!g_stepCommand.empty()) {
-			// Already broke.
-			ERROR_LOG(Log::CPU, "Core_Break called with a break already in progress: %s", g_stepCommand.reason);
-			return;
+			// If we're in a failed step that uses a temp breakpoint, we need to be able to override it here.
+			switch (g_stepCommand.type) {
+			case CPUStepType::Over:
+			case CPUStepType::Out:
+				// Allow overwriting the command.
+				break;
+			default:
+				ERROR_LOG(Log::CPU, "Core_Break called with a step-command already in progress: %s", g_stepCommand.reason);
+				return;
+			}
 		}
 		mipsTracer.stop_tracing();
+		g_stepCommand.type = CPUStepType::None;
 		g_stepCommand.reason = reason;
 		g_stepCommand.relatedAddr = relatedAddress;
 		steppingCounter++;

Core/Debugger/Breakpoints.cpp

@@ -33,12 +33,6 @@
 
 BreakpointManager g_breakpoints;
 
-std::atomic<bool> anyBreakPoints_(false);
-std::atomic<bool> anyMemChecks_(false);
-
-static std::mutex breakPointsMutex_;
-static std::mutex memCheckMutex_;
-
 void MemCheck::Log(u32 addr, bool write, int size, u32 pc, const char *reason) {
 	if (result & BREAK_ACTION_LOG) {
 		const char *type = write ? "Write" : "Read";
@@ -512,8 +506,7 @@ BreakAction BreakpointManager::ExecMemCheck(u32 address, bool write, int size, u
 	return BREAK_ACTION_IGNORE;
 }
 
-BreakAction BreakpointManager::ExecOpMemCheck(u32 address, u32 pc)
-{
+BreakAction BreakpointManager::ExecOpMemCheck(u32 address, u32 pc) {
 	// Note: currently, we don't check "on changed" for HLE (ExecMemCheck.)
 	// We'd need to more carefully specify memory changes in HLE for that.
 	int size = MIPSAnalyst::OpMemoryAccessSize(pc);
@@ -550,13 +543,12 @@ BreakAction BreakpointManager::ExecOpMemCheck(u32 address, u32 pc)
 	return BREAK_ACTION_IGNORE;
 }
 
-void BreakpointManager::SetSkipFirst(u32 pc)
-{
+void BreakpointManager::SetSkipFirst(u32 pc) {
 	breakSkipFirstAt_ = pc;
 	breakSkipFirstTicks_ = CoreTiming::GetTicks();
 }
-u32 BreakpointManager::CheckSkipFirst()
-{
+
+u32 BreakpointManager::CheckSkipFirst() {
 	u32 pc = breakSkipFirstAt_;
 	if (breakSkipFirstTicks_ == CoreTiming::GetTicks())
 		return pc;
@@ -619,26 +611,16 @@ std::vector<MemCheck> BreakpointManager::GetMemCheckRanges(bool write) {
 	return memCheckRangesRead_;
 }
 
-std::vector<MemCheck> BreakpointManager::GetMemChecks()
-{
+std::vector<MemCheck> BreakpointManager::GetMemChecks() {
 	std::lock_guard<std::mutex> guard(memCheckMutex_);
 	return memChecks_;
 }
 
-std::vector<BreakPoint> BreakpointManager::GetBreakpoints()
-{
+std::vector<BreakPoint> BreakpointManager::GetBreakpoints() {
 	std::lock_guard<std::mutex> guard(breakPointsMutex_);
 	return breakPoints_;
 }
 
-bool BreakpointManager::HasBreakPoints() {
-	return anyBreakPoints_;
-}
-
-bool BreakpointManager::HasMemChecks() {
-	return anyMemChecks_;
-}
-
 void BreakpointManager::Update(u32 addr) {
 	if (MIPSComp::jit && addr != -1) {
 		bool resume = false;

Core/Debugger/Breakpoints.h

@@ -18,6 +18,8 @@
 #pragma once
 
 #include <vector>
+#include <atomic>
+#include <mutex>
 
 #include "Core/Debugger/DebugInterface.h"
 
@@ -174,8 +176,12 @@ class BreakpointManager {
 		return memChecks_;
 	}
 
-	bool HasBreakPoints();
-	bool HasMemChecks();
+	bool HasBreakPoints() const {
+		return anyBreakPoints_;
+	}
+	bool HasMemChecks() const {
+		return anyMemChecks_;
+	}
 
 	void Update(u32 addr = 0);
 
@@ -189,6 +195,12 @@ class BreakpointManager {
 	MemCheck *GetMemCheckLocked(u32 address, int size);
 	void UpdateCachedMemCheckRanges();
 
+	std::atomic<bool> anyBreakPoints_;
+	std::atomic<bool> anyMemChecks_;
+
+	std::mutex breakPointsMutex_;
+	std::mutex memCheckMutex_;
+
 	std::vector<BreakPoint> breakPoints_;
 	u32 breakSkipFirstAt_ = 0;
 	u64 breakSkipFirstTicks_ = 0;

Core/FileSystems/DirectoryFileSystem.cpp

@@ -964,7 +964,7 @@ VFSFileSystem::~VFSFileSystem() {
 	entries.clear();
 }
 
-std::string VFSFileSystem::GetLocalPath(const std::string &localPath) {
+std::string VFSFileSystem::GetLocalPath(const std::string &localPath) const {
 	return basePath + localPath;
 }
 

Core/FileSystems/DirectoryFileSystem.h

@@ -145,5 +145,5 @@ class VFSFileSystem : public IFileSystem {
 	std::string basePath;
 	IHandleAllocator *hAlloc;
 
-	std::string GetLocalPath(const std::string &localpath);
+	std::string GetLocalPath(const std::string &localpath) const;
 };

Core/FileSystems/MetaFileSystem.h

@@ -65,6 +65,11 @@ class MetaFileSystem : public IHandleAllocator, public IFileSystem {
 	void UnmountAll();
 	void Unmount(const std::string &prefix);
 
+	// Would like to make this const, but...
+	std::vector<MountPoint> &GetMounts() {
+		return fileSystems;
+	}
+
 	// The pointer returned from these are for temporary usage only. Do not store.
 	IFileSystem *GetSystem(const std::string &prefix);
 	IFileSystem *GetSystemFromFilename(const std::string &filename);

Core/FileSystems/VirtualDiscFileSystem.cpp

@@ -243,7 +243,7 @@ void VirtualDiscFileSystem::DoState(PointerWrap &p)
 	// We don't savestate handlers (loaded on fs load), but if they change, it may not load properly.
 }
 
-Path VirtualDiscFileSystem::GetLocalPath(std::string localpath) {
+Path VirtualDiscFileSystem::GetLocalPath(std::string localpath) const {
 	if (localpath.empty())
 		return basePath;
 
@@ -303,18 +303,14 @@ int VirtualDiscFileSystem::getFileListIndex(std::string &fileName)
 	return (int)fileList.size()-1;
 }
 
-int VirtualDiscFileSystem::getFileListIndex(u32 accessBlock, u32 accessSize, bool blockMode)
-{
-	for (size_t i = 0; i < fileList.size(); i++)
-	{
-		if (fileList[i].firstBlock <= accessBlock)
-		{
+int VirtualDiscFileSystem::getFileListIndex(u32 accessBlock, u32 accessSize, bool blockMode) const {
+	for (size_t i = 0; i < fileList.size(); i++) {
+		if (fileList[i].firstBlock <= accessBlock) {
 			u32 sectorOffset = (accessBlock-fileList[i].firstBlock)*2048;
 			u32 totalFileSize = blockMode ? (fileList[i].totalSize+2047) & ~2047 : fileList[i].totalSize;
 
 			u32 endOffset = sectorOffset+accessSize;
-			if (endOffset <= totalFileSize)
-			{
+			if (endOffset <= totalFileSize) {
 				return (int)i;
 			}
 		}

Core/FileSystems/VirtualDiscFileSystem.h

@@ -60,8 +60,8 @@ class VirtualDiscFileSystem: public IFileSystem {
 	void LoadFileListIndex();
 	// Warning: modifies input string.
 	int getFileListIndex(std::string &fileName);
-	int getFileListIndex(u32 accessBlock, u32 accessSize, bool blockMode = false);
-	Path GetLocalPath(std::string localpath);
+	int getFileListIndex(u32 accessBlock, u32 accessSize, bool blockMode = false) const;
+	Path GetLocalPath(std::string localpath) const;
 
 	typedef void *HandlerLibrary;
 	typedef int HandlerHandle;

UI/EmuScreen.cpp

@@ -963,23 +963,35 @@ bool EmuScreen::UnsyncKey(const KeyInput &key) {
 		case NKCODE_SHIFT_RIGHT: keyShiftRight_ = down; break;
 		case NKCODE_ALT_LEFT: keyAltLeft_ = down; break;
 		case NKCODE_ALT_RIGHT: keyAltRight_ = down; break;
+		default: break;
 		}
 	}
 
 	if (UI::IsFocusMovementEnabled() || (g_Config.bShowImDebugger && imguiInited_)) {
 		// Note: Allow some Vkeys through, so we can toggle the imgui for example (since we actually block the control mapper otherwise in imgui mode).
 		// We need to manually implement it here :/
-		if (g_Config.bShowImDebugger && imguiInited_ && (key.flags & (KEY_UP | KEY_DOWN))) {
-			InputMapping mapping(key.deviceId, key.keyCode);
-			std::vector<int> pspButtons;
-			bool mappingFound = KeyMap::InputMappingToPspButton(mapping, &pspButtons);
-			if (mappingFound) {
-				for (auto b : pspButtons) {
-					if (b == VIRTKEY_TOGGLE_DEBUGGER || b == VIRTKEY_PAUSE) {
-						return controlMapper_.Key(key, &pauseTrigger_);
+		if (g_Config.bShowImDebugger && imguiInited_) {
+			if (key.flags & (KEY_UP | KEY_DOWN)) {
+				InputMapping mapping(key.deviceId, key.keyCode);
+				std::vector<int> pspButtons;
+				bool mappingFound = KeyMap::InputMappingToPspButton(mapping, &pspButtons);
+				if (mappingFound) {
+					for (auto b : pspButtons) {
+						if (b == VIRTKEY_TOGGLE_DEBUGGER || b == VIRTKEY_PAUSE) {
+							return controlMapper_.Key(key, &pauseTrigger_);
+						}
 					}
 				}
 			}
+			UI::EnableFocusMovement(false);
+			// Enable gamepad controls while running imgui (but ignore mouse/keyboard).
+			switch (key.deviceId) {
+			case DEVICE_ID_KEYBOARD:
+			case DEVICE_ID_MOUSE:
+				break;
+			default:
+				controlMapper_.Key(key, &pauseTrigger_);
+			}
 		}
 
 		return UIScreen::UnsyncKey(key);