mirror of
https://github.com/hrydgard/ppsspp.git
synced 2026-10-01 14:58:14 +00:00
Debugger: Remove opcode-fusion display and fix cpu step size units
DisassemblyManager used to fuse lui+addiu/load/store into single pseudo-
instructions ("li", fused loads/stores) for display. This only applied to a
handful of opcodes, complicated DisassemblyManager, and was the root cause of
a stepping bug: Core_PerformCPUStep's Into/Over cases treated stepSize as a
byte count, while the WebSocket cpu.stepInto handler computed it as an
instruction count (needed to step over a whole fused macro in one go) - so a
plain, non-fused stepInto silently executed zero instructions.
Removed the fusion logic entirely (DisassemblyMacro, DISTYPE_MACRO) - every
disassembly line is now exactly one 4-byte instruction. With that,
"how many instructions does this line span" is always 1, so the
getInstructionSizeAt() byte-size queries in the legacy Windows and ImGui
debuggers are gone too; step requests just pass 1. Core_RequestCPUStep's
stepSize is now consistently in instructions everywhere.
Also fixes the PPSSPPHeadless build, broken since 0ed1f3e added
OpenWebDebugger() (which calls System_LaunchUrl) without a headless stub.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Hqm11k99viLfbJm2MkH4BH
This commit is contained in:
1 parent
0ed1f3eceb
commit
b1f0112cef
12 files changed
+16
-226
No files matched your search
+3
-5
@@ -274,8 +274,7 @@ bool Core_RequestCPUStep(CPUStepType type, int stepSize) {
|
||||
}
|
||||
|
||||
// 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.
|
||||
// stepSize is always in instructions (4 bytes each), never bytes.
|
||||
// Doesn't return the new address, as that's just mips->getPC().
|
||||
// Internal use.
|
||||
static void Core_PerformCPUStep(MIPSDebugInterface *cpu, CPUStepType stepType, int stepSize) {
|
||||
@@ -283,10 +282,9 @@ static void Core_PerformCPUStep(MIPSDebugInterface *cpu, CPUStepType stepType, i
|
||||
case CPUStepType::Into:
|
||||
{
|
||||
u32 currentPc = cpu->GetPC();
|
||||
u32 newAddress = currentPc + stepSize;
|
||||
// If the current PC is on a breakpoint, the user still wants the step to happen.
|
||||
g_breakpoints.SetSkipFirst(currentPc);
|
||||
for (int i = 0; i < (int)(newAddress - currentPc) / 4; i++) {
|
||||
for (int i = 0; i < stepSize; i++) {
|
||||
currentMIPS->SingleStep();
|
||||
}
|
||||
break;
|
||||
@@ -294,7 +292,7 @@ static void Core_PerformCPUStep(MIPSDebugInterface *cpu, CPUStepType stepType, i
|
||||
case CPUStepType::Over:
|
||||
{
|
||||
u32 currentPc = cpu->GetPC();
|
||||
u32 breakpointAddress = currentPc + stepSize;
|
||||
u32 breakpointAddress = currentPc + stepSize * 4;
|
||||
|
||||
g_breakpoints.SetSkipFirst(currentPc);
|
||||
MIPSAnalyst::MipsOpcodeInfo info = MIPSAnalyst::GetOpcodeInfo(cpu, cpu->GetPC());
|
||||
|
||||
@@ -84,6 +84,7 @@ BreakReason Core_BreakReason();
|
||||
|
||||
// This should be called externally.
|
||||
// Can fail if another step type was requested this frame.
|
||||
// stepSize is always in instructions (4 bytes each), never bytes - see Core_PerformCPUStep in Core.cpp.
|
||||
bool Core_RequestCPUStep(CPUStepType stepType, int stepSize);
|
||||
|
||||
bool Core_NextFrame();
|
||||
|
||||
@@ -595,26 +595,6 @@ void DisassemblyFunction::load()
|
||||
{
|
||||
generateBranchLines();
|
||||
|
||||
// gather all branch targets
|
||||
std::set<u32> branchTargets;
|
||||
{
|
||||
std::lock_guard<std::recursive_mutex> guard(lock_);
|
||||
for (size_t i = 0; i < lines.size(); i++)
|
||||
{
|
||||
switch (lines[i].type)
|
||||
{
|
||||
case LINE_DOWN:
|
||||
branchTargets.insert(lines[i].second);
|
||||
break;
|
||||
case LINE_UP:
|
||||
branchTargets.insert(lines[i].first);
|
||||
break;
|
||||
default:
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
DebugInterface *cpu = g_disassemblyManager.getCpu();
|
||||
u32 funcPos = address;
|
||||
u32 funcEnd = address+size;
|
||||
@@ -655,7 +635,6 @@ void DisassemblyFunction::load()
|
||||
}
|
||||
|
||||
MIPSAnalyst::MipsOpcodeInfo opInfo = MIPSAnalyst::GetOpcodeInfo(cpu,funcPos);
|
||||
u32 opAddress = funcPos;
|
||||
funcPos += 4;
|
||||
|
||||
// skip branches and their delay slots
|
||||
@@ -665,70 +644,6 @@ void DisassemblyFunction::load()
|
||||
continue;
|
||||
}
|
||||
|
||||
// lui
|
||||
if (MIPS_GET_OP(opInfo.encodedOpcode) == 0x0F && funcPos < funcEnd && funcPos != nextData)
|
||||
{
|
||||
MIPSOpcode next = Memory::Read_Instruction(funcPos);
|
||||
MIPSInfo nextInfo = MIPSGetInfo(next);
|
||||
|
||||
u32 immediate = ((opInfo.encodedOpcode & 0xFFFF) << 16) + (s16)(next.encoding & 0xFFFF);
|
||||
int rt = MIPS_GET_RT(opInfo.encodedOpcode);
|
||||
|
||||
int nextRs = MIPS_GET_RS(next.encoding);
|
||||
int nextRt = MIPS_GET_RT(next.encoding);
|
||||
|
||||
// both rs and rt of the second op have to match rt of the first,
|
||||
// otherwise there may be hidden consequences if the macro is displayed.
|
||||
// also, don't create a macro if something branches into the middle of it
|
||||
if (nextRs == rt && nextRt == rt && branchTargets.find(funcPos) == branchTargets.end())
|
||||
{
|
||||
DisassemblyMacro* macro = NULL;
|
||||
switch (MIPS_GET_OP(next.encoding))
|
||||
{
|
||||
case 0x09: // addiu
|
||||
macro = new DisassemblyMacro(opAddress);
|
||||
macro->setMacroLi(immediate,rt);
|
||||
funcPos += 4;
|
||||
break;
|
||||
case 0x20: // lb
|
||||
case 0x21: // lh
|
||||
case 0x23: // lw
|
||||
case 0x24: // lbu
|
||||
case 0x25: // lhu
|
||||
case 0x28: // sb
|
||||
case 0x29: // sh
|
||||
case 0x2B: // sw
|
||||
macro = new DisassemblyMacro(opAddress);
|
||||
|
||||
int dataSize = MIPSGetMemoryAccessSize(next);
|
||||
if (dataSize == 0) {
|
||||
delete macro;
|
||||
return;
|
||||
}
|
||||
|
||||
macro->setMacroMemory(MIPSGetName(next),immediate,rt,dataSize);
|
||||
funcPos += 4;
|
||||
break;
|
||||
}
|
||||
|
||||
if (macro != NULL)
|
||||
{
|
||||
if (opcodeSequenceStart != opAddress)
|
||||
addOpcodeSequence(opcodeSequenceStart,opAddress);
|
||||
|
||||
std::lock_guard<std::recursive_mutex> guard(lock_);
|
||||
entries[opAddress] = macro;
|
||||
for (int i = 0; i < macro->getNumLines(); i++)
|
||||
{
|
||||
lineAddresses.push_back(macro->getLineAddress(i));
|
||||
}
|
||||
|
||||
opcodeSequenceStart = funcPos;
|
||||
continue;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// just a normal opcode
|
||||
}
|
||||
|
||||
@@ -801,76 +716,6 @@ void DisassemblyOpcode::getBranchLines(u32 start, u32 size, std::vector<BranchLi
|
||||
}
|
||||
|
||||
|
||||
void DisassemblyMacro::setMacroLi(u32 _immediate, u8 _rt)
|
||||
{
|
||||
type = MACRO_LI;
|
||||
name = "li";
|
||||
immediate = _immediate;
|
||||
rt = _rt;
|
||||
numOpcodes = 2;
|
||||
}
|
||||
|
||||
void DisassemblyMacro::setMacroMemory(std::string_view _name, u32 _immediate, u8 _rt, int _dataSize)
|
||||
{
|
||||
type = MACRO_MEMORYIMM;
|
||||
name = _name;
|
||||
immediate = _immediate;
|
||||
rt = _rt;
|
||||
dataSize = _dataSize;
|
||||
numOpcodes = 2;
|
||||
}
|
||||
|
||||
bool DisassemblyMacro::disassemble(u32 address, DisassemblyLineInfo &dest, bool insertSymbols, DebugInterface *cpuDebug)
|
||||
{
|
||||
char buffer[64];
|
||||
dest.type = DISTYPE_MACRO;
|
||||
dest.info = MIPSAnalyst::GetOpcodeInfo(cpuDebug, address);
|
||||
|
||||
std::string addressSymbol;
|
||||
switch (type)
|
||||
{
|
||||
case MACRO_LI:
|
||||
dest.name = name;
|
||||
|
||||
addressSymbol = g_symbolMap->GetLabelString(immediate);
|
||||
if (!addressSymbol.empty() && insertSymbols) {
|
||||
snprintf(buffer, sizeof(buffer), "%s,%s", MIPSDebugInterface::GetRegName(0, rt).c_str(), addressSymbol.c_str());
|
||||
} else {
|
||||
snprintf(buffer, sizeof(buffer), "%s,0x%08X", MIPSDebugInterface::GetRegName(0, rt).c_str(), immediate);
|
||||
}
|
||||
|
||||
dest.params = buffer;
|
||||
|
||||
dest.info.hasRelevantAddress = true;
|
||||
dest.info.relevantAddress = immediate;
|
||||
break;
|
||||
case MACRO_MEMORYIMM:
|
||||
dest.name = name;
|
||||
|
||||
addressSymbol = g_symbolMap->GetLabelString(immediate);
|
||||
if (!addressSymbol.empty() && insertSymbols) {
|
||||
snprintf(buffer, sizeof(buffer), "%s,%s", MIPSDebugInterface::GetRegName(0, rt).c_str(), addressSymbol.c_str());
|
||||
} else {
|
||||
snprintf(buffer, sizeof(buffer), "%s,0x%08X", MIPSDebugInterface::GetRegName(0, rt).c_str(), immediate);
|
||||
}
|
||||
|
||||
dest.params = buffer;
|
||||
|
||||
dest.info.isDataAccess = true;
|
||||
dest.info.dataAddress = immediate;
|
||||
dest.info.dataSize = dataSize;
|
||||
|
||||
dest.info.hasRelevantAddress = true;
|
||||
dest.info.relevantAddress = immediate;
|
||||
break;
|
||||
default:
|
||||
return false;
|
||||
}
|
||||
|
||||
dest.totalSize = getTotalSize();
|
||||
return true;
|
||||
}
|
||||
|
||||
DisassemblyData::DisassemblyData(u32 _address, u32 _size, DataType _type): address(_address), size(_size), type(_type)
|
||||
{
|
||||
_dbg_assert_(PSP_GetBootState() == BootState::Complete);
|
||||
|
||||
@@ -32,7 +32,7 @@ typedef u64 HashType;
|
||||
typedef u32 HashType;
|
||||
#endif
|
||||
|
||||
enum DisassemblyLineType { DISTYPE_OPCODE, DISTYPE_MACRO, DISTYPE_DATA, DISTYPE_OTHER };
|
||||
enum DisassemblyLineType { DISTYPE_OPCODE, DISTYPE_DATA, DISTYPE_OTHER };
|
||||
|
||||
struct DisassemblyLineInfo
|
||||
{
|
||||
@@ -117,33 +117,6 @@ private:
|
||||
};
|
||||
|
||||
|
||||
class DisassemblyMacro: public DisassemblyEntry
|
||||
{
|
||||
public:
|
||||
DisassemblyMacro(u32 _address): address(_address) { }
|
||||
|
||||
void setMacroLi(u32 _immediate, u8 _rt);
|
||||
void setMacroMemory(std::string_view _name, u32 _immediate, u8 _rt, int _dataSize);
|
||||
|
||||
void recheck() override { };
|
||||
int getNumLines() override { return 1; };
|
||||
int getLineNum(u32 address, bool findStart) override { return 0; };
|
||||
u32 getLineAddress(int line) override { return address; };
|
||||
u32 getTotalSize() override { return numOpcodes * 4; };
|
||||
bool disassemble(u32 address, DisassemblyLineInfo& dest, bool insertSymbols, DebugInterface *cpuDebug) override;
|
||||
private:
|
||||
enum MacroType { MACRO_LI, MACRO_MEMORYIMM };
|
||||
|
||||
MacroType type;
|
||||
std::string name;
|
||||
u32 immediate;
|
||||
u32 address;
|
||||
u32 numOpcodes;
|
||||
u8 rt;
|
||||
int dataSize;
|
||||
};
|
||||
|
||||
|
||||
class DisassemblyData: public DisassemblyEntry
|
||||
{
|
||||
public:
|
||||
|
||||
@@ -78,8 +78,6 @@ void WebSocketDisasmState::WriteDisasmLine(JsonWriter &json, const DisassemblyLi
|
||||
json.pushDict();
|
||||
if (l.type == DISTYPE_OPCODE)
|
||||
json.writeString("type", "opcode");
|
||||
else if (l.type == DISTYPE_MACRO)
|
||||
json.writeString("type", "macro");
|
||||
else if (l.type == DISTYPE_DATA)
|
||||
json.writeString("type", "data");
|
||||
else if (l.type == DISTYPE_OTHER)
|
||||
|
||||
@@ -44,7 +44,6 @@ struct WebSocketSteppingState : public DebuggerSubscriber {
|
||||
|
||||
protected:
|
||||
uint32_t GetNextAddress(DebugInterface *cpuDebug);
|
||||
int GetNextInstructionCount(DebugInterface *cpuDebug);
|
||||
void PrepareResume();
|
||||
void AddThreadCondition(uint32_t breakpointAddress, uint32_t threadID);
|
||||
};
|
||||
@@ -103,8 +102,7 @@ void WebSocketSteppingState::Into(DebuggerRequest &req) {
|
||||
// If the current PC is on a breakpoint, the user doesn't want to do nothing.
|
||||
g_breakpoints.SetSkipFirst(currentMIPS->pc);
|
||||
|
||||
int c = GetNextInstructionCount(cpuDebug);
|
||||
Core_RequestCPUStep(CPUStepType::Into, c);
|
||||
Core_RequestCPUStep(CPUStepType::Into, 1);
|
||||
} else {
|
||||
uint32_t breakpointAddress = cpuDebug->GetPC();
|
||||
PrepareResume();
|
||||
@@ -267,10 +265,6 @@ uint32_t WebSocketSteppingState::GetNextAddress(DebugInterface *cpuDebug) {
|
||||
return g_disassemblyManager.getNthNextAddress(current, 1);
|
||||
}
|
||||
|
||||
int WebSocketSteppingState::GetNextInstructionCount(DebugInterface *cpuDebug) {
|
||||
return (GetNextAddress(cpuDebug) - cpuDebug->GetPC()) / 4;
|
||||
}
|
||||
|
||||
void WebSocketSteppingState::PrepareResume() {
|
||||
if (currentMIPS->inDelaySlot) {
|
||||
// Delay slot instructions are never joined, so we pass 1.
|
||||
|
||||
@@ -465,7 +465,7 @@ void ImDisasmView::FollowBranch() {
|
||||
DisassemblyLineInfo line;
|
||||
g_disassemblyManager.getLine(curAddress_, true, line, debugger_);
|
||||
|
||||
if (line.type == DISTYPE_OPCODE || line.type == DISTYPE_MACRO) {
|
||||
if (line.type == DISTYPE_OPCODE) {
|
||||
if (line.info.isBranch) {
|
||||
jumpStack_.push_back(curAddress_);
|
||||
gotoAddr(line.info.branchTarget);
|
||||
@@ -890,7 +890,7 @@ void ImDisasmView::updateStatusBarText() {
|
||||
g_disassemblyManager.getLine(curAddress_, true, line, debugger_);
|
||||
|
||||
text[0] = 0;
|
||||
if (line.type == DISTYPE_OPCODE || line.type == DISTYPE_MACRO) {
|
||||
if (line.type == DISTYPE_OPCODE) {
|
||||
if (line.info.hasRelevantAddress && IsLikelyStringAt(line.info.relevantAddress)) {
|
||||
snprintf(text, sizeof(text), "[%08X] = \"%s\"", line.info.relevantAddress, Memory::GetCharPointer(line.info.relevantAddress));
|
||||
}
|
||||
@@ -1162,13 +1162,6 @@ void ImDisasmView::scrollStepping(u32 newPc) {
|
||||
}
|
||||
}
|
||||
|
||||
u32 ImDisasmView::getInstructionSizeAt(u32 address) {
|
||||
u32 start = g_disassemblyManager.getStartAddress(address);
|
||||
u32 next = g_disassemblyManager.getNthNextAddress(start, 1);
|
||||
return next - address;
|
||||
}
|
||||
|
||||
|
||||
void ImDisasmWindow::Draw(MIPSDebugInterface *mipsDebug, ImConfig &cfg, ImControl &control, CoreState coreState) {
|
||||
disasmView_.setDebugger(mipsDebug);
|
||||
|
||||
@@ -1181,12 +1174,10 @@ void ImDisasmWindow::Draw(MIPSDebugInterface *mipsDebug, ImConfig &cfg, ImContro
|
||||
if (ImGui::IsWindowFocused()) {
|
||||
// Process stepping keyboard shortcuts.
|
||||
if (ImGui::IsKeyPressed(ImGuiKey_F10)) {
|
||||
u32 stepSize = disasmView_.getInstructionSizeAt(mipsDebug->GetPC());
|
||||
Core_RequestCPUStep(CPUStepType::Over, stepSize);
|
||||
Core_RequestCPUStep(CPUStepType::Over, 1);
|
||||
}
|
||||
if (ImGui::IsKeyPressed(ImGuiKey_F11)) {
|
||||
u32 stepSize = disasmView_.getInstructionSizeAt(mipsDebug->GetPC());
|
||||
Core_RequestCPUStep(CPUStepType::Into, stepSize);
|
||||
Core_RequestCPUStep(CPUStepType::Into, 1);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1219,8 +1210,7 @@ void ImDisasmWindow::Draw(MIPSDebugInterface *mipsDebug, ImConfig &cfg, ImContro
|
||||
ImGui::SameLine();
|
||||
|
||||
if (ImGui::RepeatButtonShift("Into")) {
|
||||
u32 stepSize = disasmView_.getInstructionSizeAt(mipsDebug->GetPC());
|
||||
Core_RequestCPUStep(CPUStepType::Into, stepSize);
|
||||
Core_RequestCPUStep(CPUStepType::Into, 1);
|
||||
}
|
||||
if (ImGui::IsItemHovered()) {
|
||||
ImGui::SetTooltip("F11");
|
||||
@@ -1228,8 +1218,7 @@ void ImDisasmWindow::Draw(MIPSDebugInterface *mipsDebug, ImConfig &cfg, ImContro
|
||||
|
||||
ImGui::SameLine();
|
||||
if (ImGui::SmallButton("Over")) {
|
||||
u32 stepSize = disasmView_.getInstructionSizeAt(mipsDebug->GetPC());
|
||||
Core_RequestCPUStep(CPUStepType::Over, stepSize);
|
||||
Core_RequestCPUStep(CPUStepType::Over, 1);
|
||||
}
|
||||
if (ImGui::IsItemHovered()) {
|
||||
ImGui::SetTooltip("F10");
|
||||
|
||||
@@ -58,7 +58,6 @@ public:
|
||||
}
|
||||
|
||||
void scrollStepping(u32 newPc);
|
||||
u32 getInstructionSizeAt(u32 address); // not const because it might have to analyze.
|
||||
|
||||
void gotoAddr(unsigned int addr) {
|
||||
if (positionLocked_ != 0)
|
||||
|
||||
@@ -602,7 +602,7 @@ void CtrlDisAsmView::followBranch()
|
||||
DisassemblyLineInfo line;
|
||||
g_disassemblyManager.getLine(curAddress, true, line, debugger);
|
||||
|
||||
if (line.type == DISTYPE_OPCODE || line.type == DISTYPE_MACRO)
|
||||
if (line.type == DISTYPE_OPCODE)
|
||||
{
|
||||
if (line.info.isBranch)
|
||||
{
|
||||
@@ -1116,7 +1116,7 @@ void CtrlDisAsmView::updateStatusBarText()
|
||||
g_disassemblyManager.getLine(curAddress,true,line, debugger);
|
||||
|
||||
text[0] = 0;
|
||||
if (line.type == DISTYPE_OPCODE || line.type == DISTYPE_MACRO)
|
||||
if (line.type == DISTYPE_OPCODE)
|
||||
{
|
||||
if (line.info.hasRelevantAddress && IsLikelyStringAt(line.info.relevantAddress)) {
|
||||
snprintf(text, sizeof(text), "[%08X] = \"%s\"", line.info.relevantAddress, Memory::GetCharPointer(line.info.relevantAddress));
|
||||
@@ -1350,9 +1350,3 @@ void CtrlDisAsmView::scrollStepping(u32 newPc)
|
||||
}
|
||||
}
|
||||
|
||||
u32 CtrlDisAsmView::getInstructionSizeAt(u32 address)
|
||||
{
|
||||
u32 start = g_disassemblyManager.getStartAddress(address);
|
||||
u32 next = g_disassemblyManager.getNthNextAddress(start,1);
|
||||
return next - address;
|
||||
}
|
||||
@@ -119,7 +119,6 @@ public:
|
||||
}
|
||||
|
||||
void scrollStepping(u32 newPc);
|
||||
u32 getInstructionSizeAt(u32 address);
|
||||
|
||||
void gotoAddr(unsigned int addr)
|
||||
{
|
||||
|
||||
@@ -206,8 +206,7 @@ void CDisasm::step(CPUStepType stepType) {
|
||||
ptr->setDontRedraw(true);
|
||||
lastTicks_ = CoreTiming::GetTicks();
|
||||
|
||||
u32 stepSize = ptr->getInstructionSizeAt(cpu->GetPC());
|
||||
Core_RequestCPUStep(stepType, stepSize);
|
||||
Core_RequestCPUStep(stepType, 1);
|
||||
}
|
||||
|
||||
void CDisasm::runToLine() {
|
||||
|
||||
@@ -92,6 +92,7 @@ bool System_AudioRecordingState() { return false; }
|
||||
void NativeFrame(GraphicsContext *graphicsContext) { }
|
||||
void NativeResized() { }
|
||||
|
||||
void System_LaunchUrl(LaunchUrlType urlType, std::string_view url) {}
|
||||
std::string System_GetProperty(SystemProperty prop) { return ""; }
|
||||
std::vector<std::string> System_GetPropertyStringVec(SystemProperty prop) { return std::vector<std::string>(); }
|
||||
int64_t System_GetPropertyInt(SystemProperty prop) {
|
||||
|
||||
Reference in new issue
Block a user