Merge pull request #17260 from unknownbrackets/debugger-minor

Debugger: Accept format for watches and stack walk tweak

Common/UI/PopupScreens.cpp

@@ -319,14 +319,13 @@ void SliderPopupScreen::CreatePopupContents(UI::ViewGroup *parent) {
 		lin->Add(new TextView(units_))->SetTextColor(dc.theme->itemStyle.fgColor);
 
 	if (defaultValue_ != NO_DEFAULT_FLOAT) {
-		//LinearLayout *lin2 = vert->Add(new LinearLayout(ORIENT_HORIZONTAL, new LinearLayoutParams(UI::Margins(10, 10))));
 		lin->Add(new Button(di->T("Reset")))->OnClick.Add([=](UI::EventParams &) {
 			sliderValue_ = defaultValue_;
 			changing_ = true;
 			UpdateTextBox();
 			changing_ = false;
 			return UI::EVENT_DONE;
-			});
+		});
 	}
 
 	if (!negativeLabel_.empty())

Core/MIPS/MIPSStackWalk.cpp

@@ -82,8 +82,17 @@ namespace MIPSStackWalk {
 		const u32 LONGEST_FUNCTION = 1024 * 1024;
 		// TODO: Check if found entry is in the same symbol?  Might be wrong sometimes...
 
+		if (entry != INVALIDTARGET && frame.pc == entry) {
+			// This happens when we're at the start of a function.  Our ra is already correct.
+			frame.entry = entry;
+			// This function may consume stack, but the frame hasn't used it yet.
+			frame.stackSize = 0;
+			return true;
+		}
+
 		int ra_offset = -1;
-		const u32 start = frame.pc;
+		// Start one instruction before the current frame pc, as that hasn't run yet.
+		const u32 start = frame.pc - 4;
 		u32 stop = entry;
 		if (entry == INVALIDTARGET) {
 			if (start >= PSP_GetUserMemoryBase()) {
@@ -190,6 +199,4 @@ namespace MIPSStackWalk {
 
 		return frames;
 	}
-
-
 };

Windows/Debugger/Debugger_Lists.cpp

@@ -888,6 +888,7 @@ bool CtrlWatchList::WindowMessage(UINT msg, WPARAM wParam, LPARAM lParam, LRESUL
 
 void CtrlWatchList::GetColumnText(wchar_t *dest, int row, int col) {
 	uint32_t value = 0;
+	float valuef = 0.0f;
 	switch (col) {
 	case WL_NAME:
 		wcsncpy(dest, ConvertUTF8ToWString(watches_[row].name).c_str(), 255);
@@ -899,7 +900,24 @@ void CtrlWatchList::GetColumnText(wchar_t *dest, int row, int col) {
 		break;
 	case WL_VALUE:
 		if (cpu_->parseExpression(watches_[row].expression, value)) {
-			wsprintf(dest, L"0x%08x", value);
+			switch (watches_[row].format) {
+			case WatchFormat::HEX:
+				wsprintf(dest, L"0x%08X", value);
+				break;
+			case WatchFormat::INT:
+				wsprintf(dest, L"%d", (int32_t)value);
+				break;
+			case WatchFormat::FLOAT:
+				memcpy(&valuef, &value, sizeof(valuef));
+				swprintf_s(dest, 255, L"%f", valuef);
+				break;
+			case WatchFormat::STR:
+				if (Memory::IsValidAddress(value))
+					swprintf_s(dest, 255, L"%.255S", Memory::GetCharPointer(value));
+				else
+					wsprintf(dest, L"(0x%08X)", value);
+				break;
+			}
 		} else {
 			wcscpy(dest, L"(failed to evaluate)");
 		}
@@ -944,6 +962,7 @@ void CtrlWatchList::AddWatch() {
 		if (cpu_->initExpression(win.GetExpression().c_str(), info.expression)) {
 			info.name = win.GetName();
 			info.originalExpression = win.GetExpression();
+			info.format = win.GetFormat();
 			watches_.push_back(info);
 			RefreshValues();
 		} else {
@@ -955,12 +974,14 @@ void CtrlWatchList::AddWatch() {
 }
 
 void CtrlWatchList::EditWatch(int pos) {
+	auto &watch = watches_[pos];
 	WatchItemWindow win(nullptr, GetHandle(), cpu_);
-	win.Init(watches_[pos].name, watches_[pos].originalExpression);
+	win.Init(watch.name, watch.originalExpression, watch.format);
 	if (win.Exec()) {
-		if (cpu_->initExpression(win.GetExpression().c_str(), watches_[pos].expression)) {
-			watches_[pos].name = win.GetName();
-			watches_[pos].originalExpression = win.GetExpression();
+		if (cpu_->initExpression(win.GetExpression().c_str(), watch.expression)) {
+			watch.name = win.GetName();
+			watch.originalExpression = win.GetExpression();
+			watch.format = win.GetFormat();
 			RefreshValues();
 		} else {
 			char errorMessage[512];

Windows/Debugger/Debugger_Lists.h

@@ -7,6 +7,13 @@
 #include "../../Core/MIPS/MIPSStackWalk.h"
 #include "Windows/W32Util/Misc.h"
 
+enum class WatchFormat {
+	HEX,
+	INT,
+	FLOAT,
+	STR,
+};
+
 class CtrlThreadList: public GenericListControl
 {
 public:
@@ -108,6 +115,7 @@ class CtrlWatchList : public GenericListControl {
 		std::string name;
 		std::string originalExpression;
 		PostfixExpression expression;
+		WatchFormat format = WatchFormat::HEX;
 		uint32_t currentValue = 0;
 		uint32_t lastValue = 0;
 		int steppingCounter = -1;

Windows/Debugger/WatchItemWindow.cpp

@@ -39,6 +39,16 @@ INT_PTR WatchItemWindow::DlgFunc(HWND hWnd, UINT iMsg, WPARAM wParam, LPARAM lPa
 	case WM_INITDIALOG:
 		SetWindowTextW(GetDlgItem(hWnd, IDC_BREAKPOINT_ADDRESS), ConvertUTF8ToWString(name_).c_str());
 		SetWindowTextW(GetDlgItem(hWnd, IDC_BREAKPOINT_CONDITION), ConvertUTF8ToWString(expression_).c_str());
+
+		// We only need to set one state on dialog init.
+		if (format_ == WatchFormat::HEX)
+			SendMessage(GetDlgItem(hWnd, IDC_DISASM_FMT_HEX), BM_SETCHECK, BST_CHECKED, 0);
+		else if (format_ == WatchFormat::INT)
+			SendMessage(GetDlgItem(hWnd, IDC_DISASM_FMT_INT), BM_SETCHECK, BST_CHECKED, 0);
+		else if (format_ == WatchFormat::FLOAT)
+			SendMessage(GetDlgItem(hWnd, IDC_DISASM_FMT_FLOAT), BM_SETCHECK, BST_CHECKED, 0);
+		else if (format_ == WatchFormat::STR)
+			SendMessage(GetDlgItem(hWnd, IDC_DISASM_FMT_STR), BM_SETCHECK, BST_CHECKED, 0);
 		return TRUE;
 
 	case WM_COMMAND:
@@ -83,6 +93,10 @@ bool WatchItemWindow::Exec() {
 	return DialogBoxParam(GetModuleHandle(0), MAKEINTRESOURCE(IDD_CPUWATCH), parentHwnd_, StaticDlgFunc, (LPARAM)this) != 0;
 }
 
+static bool IsControlChecked(HWND hWnd, int id) {
+	return SendMessage(GetDlgItem(hWnd, id), BM_GETCHECK, 0, 0) != 0;
+}
+
 bool WatchItemWindow::FetchDialogData(HWND hwnd) {
 	wchar_t textValue[512];
 
@@ -99,5 +113,14 @@ bool WatchItemWindow::FetchDialogData(HWND hwnd) {
 		return false;
 	}
 
+	if (IsControlChecked(hwnd, IDC_DISASM_FMT_HEX))
+		format_ = WatchFormat::HEX;
+	else if (IsControlChecked(hwnd, IDC_DISASM_FMT_INT))
+		format_ = WatchFormat::INT;
+	else if (IsControlChecked(hwnd, IDC_DISASM_FMT_FLOAT))
+		format_ = WatchFormat::FLOAT;
+	else if (IsControlChecked(hwnd, IDC_DISASM_FMT_STR))
+		format_ = WatchFormat::STR;
+
 	return true;
 }

Windows/Debugger/WatchItemWindow.h

@@ -21,14 +21,16 @@
 #include "Common/CommonWindows.h"
 #include "Common/CommonTypes.h"
 #include "Core/Debugger/DebugInterface.h"
+#include "Windows/Debugger/Debugger_Lists.h"
 
 class WatchItemWindow {
 public:
 	WatchItemWindow(HINSTANCE inst, HWND parent, DebugInterface *cpu) : parentHwnd_(parent), cpu_(cpu) {}
 
-	void Init(const std::string &name, const std::string &expression) {
+	void Init(const std::string &name, const std::string &expression, WatchFormat fmt) {
 		name_ = name;
 		expression_ = expression;
+		format_ = fmt;
 	}
 
 	bool Exec();
@@ -39,6 +41,9 @@ class WatchItemWindow {
 	const std::string &GetExpression() const {
 		return expression_;
 	}
+	WatchFormat GetFormat() const {
+		return format_;
+	}
 
 private:
 	static INT_PTR CALLBACK StaticDlgFunc(HWND hWnd, UINT iMsg, WPARAM wParam, LPARAM lParam);
@@ -50,4 +55,5 @@ class WatchItemWindow {
 
 	std::string name_;
 	std::string expression_;
+	WatchFormat format_ = WatchFormat::HEX;
 };

Windows/ppsspp.rc

@@ -389,7 +389,7 @@ BEGIN
     PUSHBUTTON      "Cancel",IDCANCEL,173,79,50,14
 END
 
-IDD_CPUWATCH DIALOGEX 0, 0, 236, 70
+IDD_CPUWATCH DIALOGEX 0, 0, 236, 90
 STYLE DS_SETFONT | DS_MODALFRAME | DS_FIXEDSYS | WS_POPUP | WS_CAPTION | WS_SYSMENU
 CAPTION "Watch"
 FONT 8, "MS Shell Dlg", 400, 0, 0x1
@@ -398,8 +398,12 @@ BEGIN
     EDITTEXT        IDC_BREAKPOINT_ADDRESS,56,7,173,14,ES_AUTOHSCROLL
     LTEXT           "Expression",IDC_STATIC,7,28,36,8
     EDITTEXT        IDC_BREAKPOINT_CONDITION,56,26,173,14,ES_AUTOHSCROLL
-    DEFPUSHBUTTON   "OK",IDC_BREAKPOINT_OK,144,47,41,14
-    PUSHBUTTON      "Cancel",IDC_BREAKPOINT_CANCEL,186,47,42,14
+    CONTROL         "Hex",IDC_DISASM_FMT_HEX,"Button",BS_AUTORADIOBUTTON | WS_GROUP,56,45,34,9
+    CONTROL         "Integer",IDC_DISASM_FMT_INT,"Button",BS_AUTORADIOBUTTON,90,45,34,9
+    CONTROL         "Float",IDC_DISASM_FMT_FLOAT,"Button",BS_AUTORADIOBUTTON,134,45,34,9
+    CONTROL         "String",IDC_DISASM_FMT_STR,"Button",BS_AUTORADIOBUTTON,178,45,34,9
+    DEFPUSHBUTTON   "OK",IDC_BREAKPOINT_OK,144,66,41,14
+    PUSHBUTTON      "Cancel",IDC_BREAKPOINT_CANCEL,186,66,42,14
 END
 
 

Windows/resource.h

@@ -337,8 +337,12 @@
 #define ID_GEDBG_TRACK_PIXEL             40226
 #define ID_GEDBG_TRACK_PIXEL_STOP        40227
 #define ID_DISASM_NOPINSTRUCTION         40228
-#define IDC_WATCHLIST                    40229
-#define ID_DISASM_DELETEBREAKPOINT       40230
+#define IDC_WATCHLIST                    40230
+#define ID_DISASM_DELETEBREAKPOINT       40231
+#define IDC_DISASM_FMT_HEX               40232
+#define IDC_DISASM_FMT_INT               40233
+#define IDC_DISASM_FMT_FLOAT             40234
+#define IDC_DISASM_FMT_STR               40235
 
 
 // Dummy option to let the buffered rendering hotkey cycle through all the options.
@@ -352,7 +356,7 @@
 #ifdef APSTUDIO_INVOKED
 #ifndef APSTUDIO_READONLY_SYMBOLS
 #define _APS_NEXT_RESOURCE_VALUE        258
-#define _APS_NEXT_COMMAND_VALUE         40231
+#define _APS_NEXT_COMMAND_VALUE         40236
 #define _APS_NEXT_CONTROL_VALUE         1202
 #define _APS_NEXT_SYMED_VALUE           101
 #endif