From 6da10463f910f8647bfea7b0a17efabf314fda9b Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sat, 29 Apr 2023 09:48:33 -0700 Subject: [PATCH] Debugger: Make reg names safer, stop using v000. Better to use S000, etc. as that's more clear throughout. --- Core/Debugger/DebugInterface.h | 2 +- Core/Debugger/DisassemblyManager.cpp | 8 ++--- Core/MIPS/ARM/ArmRegCacheFPU.cpp | 6 ++-- Core/MIPS/IR/IRInst.cpp | 4 +-- Core/MIPS/MIPSDebugInterface.cpp | 36 +++++++++---------- Core/MIPS/MIPSDebugInterface.h | 2 +- Core/MIPS/MIPSDis.cpp | 6 ++-- Core/MIPS/MIPSDisVFPU.cpp | 15 ++++---- Core/MIPS/MIPSVFPUUtils.cpp | 52 +++++++++++---------------- Core/MIPS/MIPSVFPUUtils.h | 7 ++-- Windows/Debugger/CtrlDisAsmView.cpp | 2 +- Windows/Debugger/CtrlRegisterList.cpp | 2 +- unittest/TestArmEmitter.cpp | 4 +-- unittest/UnitTest.cpp | 6 ++-- 14 files changed, 70 insertions(+), 82 deletions(-) diff --git a/Core/Debugger/DebugInterface.h b/Core/Debugger/DebugInterface.h index 522e329a49..7a77b6ac0a 100644 --- a/Core/Debugger/DebugInterface.h +++ b/Core/Debugger/DebugInterface.h @@ -61,7 +61,7 @@ public: virtual int GetNumCategories() {return 0;} virtual int GetNumRegsInCategory(int cat) {return 0;} virtual const char *GetCategoryName(int cat) {return 0;} - virtual const char *GetRegName(int cat, int index) {return 0;} + virtual std::string GetRegName(int cat, int index) { return ""; } virtual void PrintRegValue(int cat, int index, char *out, size_t outSize) { snprintf(out, outSize, "%08X", GetGPR32Value(index)); } diff --git a/Core/Debugger/DisassemblyManager.cpp b/Core/Debugger/DisassemblyManager.cpp index e5eb5c87ae..ecea54a868 100644 --- a/Core/Debugger/DisassemblyManager.cpp +++ b/Core/Debugger/DisassemblyManager.cpp @@ -847,9 +847,9 @@ bool DisassemblyMacro::disassemble(u32 address, DisassemblyLineInfo &dest, bool addressSymbol = g_symbolMap->GetLabelString(immediate); if (!addressSymbol.empty() && insertSymbols) { - snprintf(buffer, sizeof(buffer), "%s,%s", cpuDebug->GetRegName(0, rt), addressSymbol.c_str()); + snprintf(buffer, sizeof(buffer), "%s,%s", cpuDebug->GetRegName(0, rt).c_str(), addressSymbol.c_str()); } else { - snprintf(buffer, sizeof(buffer), "%s,0x%08X", cpuDebug->GetRegName(0, rt), immediate); + snprintf(buffer, sizeof(buffer), "%s,0x%08X", cpuDebug->GetRegName(0, rt).c_str(), immediate); } dest.params = buffer; @@ -862,9 +862,9 @@ bool DisassemblyMacro::disassemble(u32 address, DisassemblyLineInfo &dest, bool addressSymbol = g_symbolMap->GetLabelString(immediate); if (!addressSymbol.empty() && insertSymbols) { - snprintf(buffer, sizeof(buffer), "%s,%s", cpuDebug->GetRegName(0, rt), addressSymbol.c_str()); + snprintf(buffer, sizeof(buffer), "%s,%s", cpuDebug->GetRegName(0, rt).c_str(), addressSymbol.c_str()); } else { - snprintf(buffer, sizeof(buffer), "%s,0x%08X", cpuDebug->GetRegName(0, rt), immediate); + snprintf(buffer, sizeof(buffer), "%s,0x%08X", cpuDebug->GetRegName(0, rt).c_str(), immediate); } dest.params = buffer; diff --git a/Core/MIPS/ARM/ArmRegCacheFPU.cpp b/Core/MIPS/ARM/ArmRegCacheFPU.cpp index f2478595c0..88e4334163 100644 --- a/Core/MIPS/ARM/ArmRegCacheFPU.cpp +++ b/Core/MIPS/ARM/ArmRegCacheFPU.cpp @@ -618,7 +618,7 @@ void ArmRegCacheFPU::QFlush(int quad) { } if (qr[quad].isDirty && !qr[quad].isTemp) { - INFO_LOG(JIT, "Flushing Q%i (%s)", quad, GetVectorNotation(qr[quad].mipsVec, qr[quad].sz)); + INFO_LOG(JIT, "Flushing Q%i (%s)", quad, GetVectorNotation(qr[quad].mipsVec, qr[quad].sz).c_str()); ARMReg q = QuadAsQ(quad); // Unlike reads, when writing to the register file we need to be careful to write the correct @@ -881,7 +881,7 @@ ARMReg ArmRegCacheFPU::QMapReg(int vreg, VectorSize sz, int flags) { // We didn't find the extra register, but we got a list of regs to flush. Flush 'em. // Here we can check for opportunities to do a "transpose-flush" of row vectors, etc. if (!quadsToFlush.empty()) { - INFO_LOG(JIT, "New mapping %s collided with %d quads, flushing them.", GetVectorNotation(vreg, sz), (int)quadsToFlush.size()); + INFO_LOG(JIT, "New mapping %s collided with %d quads, flushing them.", GetVectorNotation(vreg, sz).c_str(), (int)quadsToFlush.size()); } for (size_t i = 0; i < quadsToFlush.size(); i++) { QFlush(quadsToFlush[i]); @@ -973,7 +973,7 @@ ARMReg ArmRegCacheFPU::QMapReg(int vreg, VectorSize sz, int flags) { qr[quad].isDirty = (flags & MAP_DIRTY) != 0; qr[quad].spillLock = true; - INFO_LOG(JIT, "Mapped Q%i to vfpu %i (%s), sz=%i, dirty=%i", quad, vreg, GetVectorNotation(vreg, sz), (int)sz, qr[quad].isDirty); + INFO_LOG(JIT, "Mapped Q%i to vfpu %i (%s), sz=%i, dirty=%i", quad, vreg, GetVectorNotation(vreg, sz).c_str(), (int)sz, qr[quad].isDirty); if (sz == V_Single || sz == V_Pair) { return D_0(QuadAsQ(quad)); } else { diff --git a/Core/MIPS/IR/IRInst.cpp b/Core/MIPS/IR/IRInst.cpp index 13b2509f18..6d3e6f9572 100644 --- a/Core/MIPS/IR/IRInst.cpp +++ b/Core/MIPS/IR/IRInst.cpp @@ -208,7 +208,7 @@ int IRWriter::AddConstantFloat(float value) { return AddConstant(val); } -const char *GetGPRName(int r) { +static std::string GetGPRName(int r) { if (r < 32) { return currentDebugMIPS->GetRegName(0, r); } @@ -259,7 +259,7 @@ void DisassembleParam(char *buf, int bufSize, u8 param, char type, u32 constant) switch (type) { case 'G': - snprintf(buf, bufSize, "%s", GetGPRName(param)); + snprintf(buf, bufSize, "%s", GetGPRName(param).c_str()); break; case 'F': if (param >= 32) { diff --git a/Core/MIPS/MIPSDebugInterface.cpp b/Core/MIPS/MIPSDebugInterface.cpp index f2063ddc1d..747a9a5083 100644 --- a/Core/MIPS/MIPSDebugInterface.cpp +++ b/Core/MIPS/MIPSDebugInterface.cpp @@ -27,6 +27,7 @@ #include "Core/Debugger/SymbolMap.h" #include "Core/Debugger/DebugInterface.h" #include "Core/MIPS/MIPSDebugInterface.h" +#include "Core/MIPS/MIPSVFPUUtils.h" #include "Core/HLE/sceKernelThread.h" #include "Core/MemMap.h" #include "Core/MIPS/MIPSTables.h" @@ -60,12 +61,12 @@ public: char reg[8]; snprintf(reg, sizeof(reg), "r%d", i); - if (strcasecmp(str, reg) == 0 || strcasecmp(str, cpu->GetRegName(0, i)) == 0) + if (strcasecmp(str, reg) == 0 || strcasecmp(str, cpu->GetRegName(0, i).c_str()) == 0) { referenceIndex = i; return true; } - else if (strcasecmp(str, cpu->GetRegName(1, i)) == 0) + else if (strcasecmp(str, cpu->GetRegName(1, i).c_str()) == 0) { referenceIndex = REF_INDEX_FPU | i; return true; @@ -81,7 +82,7 @@ public: for (int i = 0; i < 128; i++) { - if (strcasecmp(str, cpu->GetRegName(2, i)) == 0) + if (strcasecmp(str, cpu->GetRegName(2, i).c_str()) == 0) { referenceIndex = REF_INDEX_VFPU | i; return true; @@ -261,9 +262,7 @@ const char *MIPSDebugInterface::GetName() return ("R4"); } -// NOT threadsafe. -const char *MIPSDebugInterface::GetRegName(int cat, int index) -{ +std::string MIPSDebugInterface::GetRegName(int cat, int index) { static const char *regName[32] = { "zero", "at", "v0", "v1", "a0", "a1", "a2", "a3", @@ -274,23 +273,20 @@ const char *MIPSDebugInterface::GetRegName(int cat, int index) "t8", "t9", "k0", "k1", "gp", "sp", "fp", "ra" }; + static const char *fpRegName[32] = { + "f0", "f1", "f2", "f3", "f4", "f5", "f6", "f7", + "f8", "f9", "f10", "f11", "f12", "f13", "f14", "f15", + "f16", "f16", "f18", "f19", "f20", "f21", "f22", "f23", + "f24", "f25", "f26", "f27", "f28", "f29", "f30", "f31", + }; - // really nasty hack so that this function can be called several times on one line of c++. - static int access = 0; - access++; - access &= 3; - static char temp[4][16]; - - if (cat == 0) { + if (cat == 0 && (unsigned)index < sizeof(regName)) { return regName[index]; - } else if (cat == 1) { - snprintf(temp[access], sizeof(temp[access]), "f%d", index); - return temp[access]; + } else if (cat == 1 && (unsigned)index < sizeof(fpRegName)) { + return fpRegName[index]; } else if (cat == 2) { - snprintf(temp[access], sizeof(temp[access]), "v%03x", index); - return temp[access]; - } else { - return "???"; + return GetVectorNotation(index, V_Single); } + return "???"; } diff --git a/Core/MIPS/MIPSDebugInterface.h b/Core/MIPS/MIPSDebugInterface.h index cb6781455f..6d5cec88a2 100644 --- a/Core/MIPS/MIPSDebugInterface.h +++ b/Core/MIPS/MIPSDebugInterface.h @@ -62,7 +62,7 @@ public: static int r[3] = { 32, 32, 128 }; return r[cat]; } - const char *GetRegName(int cat, int index) override; + std::string GetRegName(int cat, int index) override; void PrintRegValue(int cat, int index, char *out, size_t outSize) override { switch (cat) { diff --git a/Core/MIPS/MIPSDis.cpp b/Core/MIPS/MIPSDis.cpp index 14ea4ff774..a576550798 100644 --- a/Core/MIPS/MIPSDis.cpp +++ b/Core/MIPS/MIPSDis.cpp @@ -34,9 +34,9 @@ #define _POS ((op>>6 ) & 0x1F) #define _SIZE ((op>>11) & 0x1F) -#define RN(i) currentDebugMIPS->GetRegName(0,i) -#define FN(i) currentDebugMIPS->GetRegName(1,i) -//#define VN(i) currentMIPS->GetRegName(2,i) +#define RN(i) (currentDebugMIPS->GetRegName(0, i).c_str()) +#define FN(i) (currentDebugMIPS->GetRegName(1, i).c_str()) +//#define VN(i) (currentDebugMIPS->GetRegName(2, i).c_str()) namespace MIPSDis { diff --git a/Core/MIPS/MIPSDisVFPU.cpp b/Core/MIPS/MIPSDisVFPU.cpp index fdc4aece46..4d35a0616c 100644 --- a/Core/MIPS/MIPSDisVFPU.cpp +++ b/Core/MIPS/MIPSDisVFPU.cpp @@ -34,9 +34,9 @@ #define _SIZE ((op>>11) & 0x1F) -#define RN(i) currentDebugMIPS->GetRegName(0,i) -#define FN(i) currentDebugMIPS->GetRegName(1,i) -//#define VN(i) currentDebugMIPS->GetRegName(2,i) +#define RN(i) (currentDebugMIPS->GetRegName(0, i).c_str()) +#define FN(i) (currentDebugMIPS->GetRegName(1, i).c_str()) +//#define VN(i) (currentDebugMIPS->GetRegName(2, i).c_str()) #define S_not(a,b,c) (a<<2)|(b)|(c<<5) @@ -48,8 +48,7 @@ #define VertOff 1 #define MtxOff 4 -inline const char *VN(int v, VectorSize size) -{ +inline std::string VNStr(int v, VectorSize size) { static const char *vfpuCtrlNames[VFPU_CTRL_MAX] = { "SPFX", "TPFX", @@ -77,11 +76,13 @@ inline const char *VN(int v, VectorSize size) return GetVectorNotation(v, size); } -inline const char *MN(int v, MatrixSize size) -{ +inline std::string MNStr(int v, MatrixSize size) { return GetMatrixNotation(v, size); } +#define VN(v, s) (VNStr(v, s).c_str()) +#define MN(v, s) (MNStr(v, s).c_str()) + inline const char *VSuff(MIPSOpcode op) { int a = (op>>7)&1; diff --git a/Core/MIPS/MIPSVFPUUtils.cpp b/Core/MIPS/MIPSVFPUUtils.cpp index e80bb77ac8..9f3494a63b 100644 --- a/Core/MIPS/MIPSVFPUUtils.cpp +++ b/Core/MIPS/MIPSVFPUUtils.cpp @@ -23,6 +23,7 @@ #include "Common/BitScan.h" #include "Common/CommonFuncs.h" #include "Common/File/VFS/VFS.h" +#include "Common/StringUtils.h" #include "Core/Reporting.h" #include "Core/MIPS/MIPS.h" #include "Core/MIPS/MIPSVFPUUtils.h" @@ -538,11 +539,7 @@ MatrixOverlapType GetMatrixOverlap(int mtx1, int mtx2, MatrixSize msize) { return OVERLAP_NONE; } -const char *GetVectorNotation(int reg, VectorSize size) -{ - static char temp[4][16]; - static int yo = 0; yo++; yo &= 3; - +std::string GetVectorNotation(int reg, VectorSize size) { int mtx = (reg>>2)&7; int col = reg&3; int row = 0; @@ -557,34 +554,27 @@ const char *GetVectorNotation(int reg, VectorSize size) } if (transpose && c == 'C') c='R'; if (transpose) - snprintf(temp[yo], sizeof(temp[yo]), "%c%i%i%i",c,mtx,row,col); - else - snprintf(temp[yo], sizeof(temp[yo]), "%c%i%i%i",c,mtx,col,row); - return temp[yo]; + return StringFromFormat("%c%i%i%i", c, mtx, row, col); + return StringFromFormat("%c%i%i%i", c, mtx, col, row); } -const char *GetMatrixNotation(int reg, MatrixSize size) -{ - static char temp[4][16]; - static int yo=0;yo++;yo&=3; - int mtx = (reg>>2)&7; - int col = reg&3; - int row = 0; - int transpose = (reg>>5)&1; - char c; - switch (size) - { - case M_2x2: c='M'; row=(reg>>5)&2; break; - case M_3x3: c='M'; row=(reg>>6)&1; break; - case M_4x4: c='M'; row=(reg>>5)&2; break; - default: c='?'; break; - } - if (transpose && c=='M') c='E'; - if (transpose) - snprintf(temp[yo], sizeof(temp[yo]), "%c%i%i%i",c,mtx,row,col); - else - snprintf(temp[yo], sizeof(temp[yo]), "%c%i%i%i",c,mtx,col,row); - return temp[yo]; +std::string GetMatrixNotation(int reg, MatrixSize size) { + int mtx = (reg>>2)&7; + int col = reg&3; + int row = 0; + int transpose = (reg>>5)&1; + char c; + switch (size) + { + case M_2x2: c='M'; row=(reg>>5)&2; break; + case M_3x3: c='M'; row=(reg>>6)&1; break; + case M_4x4: c='M'; row=(reg>>5)&2; break; + default: c='?'; break; + } + if (transpose && c=='M') c='E'; + if (transpose) + return StringFromFormat("%c%i%i%i", c, mtx, row, col); + return StringFromFormat("%c%i%i%i", c, mtx, col, row); } bool GetVFPUCtrlMask(int reg, u32 *mask) { diff --git a/Core/MIPS/MIPSVFPUUtils.h b/Core/MIPS/MIPSVFPUUtils.h index f26340f356..2daee3c367 100644 --- a/Core/MIPS/MIPSVFPUUtils.h +++ b/Core/MIPS/MIPSVFPUUtils.h @@ -16,8 +16,9 @@ // https://github.com/hrydgard/ppsspp and http://www.ppsspp.org/. #pragma once -#include +#include +#include #include "Common/CommonTypes.h" #include "Core/MIPS/MIPS.h" @@ -216,8 +217,8 @@ VectorSize MatrixVectorSize(MatrixSize sz); int GetNumVectorElements(VectorSize sz); int GetMatrixSideSafe(MatrixSize sz); int GetMatrixSide(MatrixSize sz); -const char *GetVectorNotation(int reg, VectorSize size); -const char *GetMatrixNotation(int reg, MatrixSize size); +std::string GetVectorNotation(int reg, VectorSize size); +std::string GetMatrixNotation(int reg, MatrixSize size); inline bool IsMatrixTransposed(int matrixReg) { return (matrixReg >> 5) & 1; } diff --git a/Windows/Debugger/CtrlDisAsmView.cpp b/Windows/Debugger/CtrlDisAsmView.cpp index e8d176b1bc..89529d06e6 100644 --- a/Windows/Debugger/CtrlDisAsmView.cpp +++ b/Windows/Debugger/CtrlDisAsmView.cpp @@ -302,7 +302,7 @@ void CtrlDisAsmView::assembleOpcode(u32 address, std::string defaultText) { for (int reg = 0; reg < debugger->GetNumRegsInCategory(cat); reg++) { - if (strcasecmp(debugger->GetRegName(cat,reg),registerName.c_str()) == 0) + if (strcasecmp(debugger->GetRegName(cat,reg).c_str(), registerName.c_str()) == 0) { debugger->SetRegValue(cat,reg,value); Reporting::NotifyDebugger(); diff --git a/Windows/Debugger/CtrlRegisterList.cpp b/Windows/Debugger/CtrlRegisterList.cpp index 7f442650ed..2a5bc7a6c2 100644 --- a/Windows/Debugger/CtrlRegisterList.cpp +++ b/Windows/Debugger/CtrlRegisterList.cpp @@ -251,7 +251,7 @@ void CtrlRegisterList::onPaint(WPARAM wParam, LPARAM lParam) if (iGetNumRegsInCategory(category)) { char temp[256]; - int temp_len = snprintf(temp, sizeof(temp), "%s", cpu->GetRegName(category, i)); + int temp_len = snprintf(temp, sizeof(temp), "%s", cpu->GetRegName(category, i).c_str()); SetTextColor(hdc,0x600000); TextOutA(hdc,17,rowY1,temp,temp_len); SetTextColor(hdc,0x000000); diff --git a/unittest/TestArmEmitter.cpp b/unittest/TestArmEmitter.cpp index 848655058a..96d96cf66e 100644 --- a/unittest/TestArmEmitter.cpp +++ b/unittest/TestArmEmitter.cpp @@ -243,8 +243,8 @@ bool TestArmEmitter() { int R001 = GetRowName(0, M_4x4, 1, 0); int R002 = GetRowName(0, M_4x4, 2, 0); int R003 = GetRowName(0, M_4x4, 3, 0); - printf("Col 010: %s\n", GetVectorNotation(C010, V_Quad)); - printf("Row 003: %s\n", GetVectorNotation(R003, V_Quad)); + printf("Col 010: %s\n", GetVectorNotation(C010, V_Quad).c_str()); + printf("Row 003: %s\n", GetVectorNotation(R003, V_Quad).c_str()); MIPSAnalyst::AnalysisResults results; memset(&results, 0, sizeof(results)); diff --git a/unittest/UnitTest.cpp b/unittest/UnitTest.cpp index 605ba0bfb8..38c9d67dbc 100644 --- a/unittest/UnitTest.cpp +++ b/unittest/UnitTest.cpp @@ -431,7 +431,7 @@ bool TestMatrixTranspose() { } void TestGetMatrix(int matrix, MatrixSize sz) { - INFO_LOG(SYSTEM, "Testing matrix %s", GetMatrixNotation(matrix, sz)); + INFO_LOG(SYSTEM, "Testing matrix %s", GetMatrixNotation(matrix, sz).c_str()); u8 fullMatrix[16]; u8 cols[4]; @@ -449,8 +449,8 @@ void TestGetMatrix(int matrix, MatrixSize sz) { // int rowName = GetRowName(matrix, sz, i, 0); int colName = cols[i]; int rowName = rows[i]; - INFO_LOG(SYSTEM, "Column %i: %s", i, GetVectorNotation(colName, vsz)); - INFO_LOG(SYSTEM, "Row %i: %s", i, GetVectorNotation(rowName, vsz)); + INFO_LOG(SYSTEM, "Column %i: %s", i, GetVectorNotation(colName, vsz).c_str()); + INFO_LOG(SYSTEM, "Row %i: %s", i, GetVectorNotation(rowName, vsz).c_str()); u8 colRegs[4]; u8 rowRegs[4];