From 391c7319f53b5f25048c2d3f9e66c9d5f2c45d19 Mon Sep 17 00:00:00 2001 From: Michael Maltese Date: Mon, 15 May 2017 17:12:15 -0700 Subject: [PATCH 1/6] DSPDisassembler: get rid of double-pass and temp file --- Source/Core/Core/DSP/DSPCodeUtil.cpp | 8 +-- Source/Core/Core/DSP/DSPDisassembler.cpp | 69 +++++-------------- Source/Core/Core/DSP/DSPDisassembler.h | 8 +-- Source/Core/Core/HW/DSPLLE/DSPSymbols.cpp | 2 +- Source/UnitTests/Core/DSP/DSPAssemblyTest.cpp | 2 +- 5 files changed, 25 insertions(+), 64 deletions(-) diff --git a/Source/Core/Core/DSP/DSPCodeUtil.cpp b/Source/Core/Core/DSP/DSPCodeUtil.cpp index 78a1611d9f..a5c789716b 100644 --- a/Source/Core/Core/DSP/DSPCodeUtil.cpp +++ b/Source/Core/Core/DSP/DSPCodeUtil.cpp @@ -56,7 +56,7 @@ bool Disassemble(const std::vector& code, bool line_numbers, std::string& t settings.decode_registers = true; DSPDisassembler disasm(settings); - bool success = disasm.Disassemble(0, code, 0x0000, text); + bool success = disasm.Disassemble(code, 0x0000, text); return success; } @@ -79,9 +79,9 @@ bool Compare(const std::vector& code1, const std::vector& code2) { std::string line1, line2; u16 pc = i; - disassembler.DisassembleOpcode(&code1[0], 0x0000, 2, &pc, line1); + disassembler.DisassembleOpcode(&code1[0], 0x0000, &pc, line1); pc = i; - disassembler.DisassembleOpcode(&code2[0], 0x0000, 2, &pc, line2); + disassembler.DisassembleOpcode(&code2[0], 0x0000, &pc, line2); printf("!! %04x : %04x vs %04x - %s vs %s\n", i, code1[i], code2[i], line1.c_str(), line2.c_str()); } @@ -94,7 +94,7 @@ bool Compare(const std::vector& code1, const std::vector& code2) { u16 pc = i; std::string line; - disassembler.DisassembleOpcode(&longest[0], 0x0000, 2, &pc, line); + disassembler.DisassembleOpcode(&longest[0], 0x0000, &pc, line); printf("!! %s\n", line.c_str()); } } diff --git a/Source/Core/Core/DSP/DSPDisassembler.cpp b/Source/Core/Core/DSP/DSPDisassembler.cpp index e43f400390..ebe8121cf2 100644 --- a/Source/Core/Core/DSP/DSPDisassembler.cpp +++ b/Source/Core/Core/DSP/DSPDisassembler.cpp @@ -55,19 +55,15 @@ DSPDisassembler::~DSPDisassembler() uo << StringFromFormat("Unknown opcodes count: %d\n", count); } -bool DSPDisassembler::Disassemble(int start_pc, const std::vector& code, int base_addr, - std::string& text) +bool DSPDisassembler::Disassemble(const std::vector& code, int base_addr, std::string& text) { - const char* tmp1 = "tmp1.bin"; - - // First we have to dump the code to a bin file. + for (u16 pc = 0; pc < code.size();) { - File::IOFile f(tmp1, "wb"); - f.WriteArray(&code[0], code.size()); + if (!DisassembleOpcode(code.data(), base_addr, &pc, text)) + return false; + text.append("\n"); } - - // Run the two passes. - return DisassembleFile(tmp1, base_addr, 1, text) && DisassembleFile(tmp1, base_addr, 2, text); + return true; } std::string DSPDisassembler::DisassembleParameters(const DSPOPCTemplate& opc, u16 op1, u16 op2) @@ -170,11 +166,9 @@ static std::string MakeLowerCase(std::string in) return in; } -bool DSPDisassembler::DisassembleOpcode(const u16* binbuf, int base_addr, int pass, u16* pc, +bool DSPDisassembler::DisassembleOpcode(const u16* binbuf, int base_addr, u16* pc, std::string& dest) { - std::string buf(" "); - if ((*pc & 0x7fff) >= 0x1000) { ++pc; @@ -217,7 +211,7 @@ bool DSPDisassembler::DisassembleOpcode(const u16* binbuf, int base_addr, int pa // printing if (settings_.show_pc) - buf += StringFromFormat("%04x ", *pc); + dest += StringFromFormat("%04x ", *pc); u16 op2; @@ -226,13 +220,13 @@ bool DSPDisassembler::DisassembleOpcode(const u16* binbuf, int base_addr, int pa { op2 = binbuf[(*pc + 1) & 0x0fff]; if (settings_.show_hex) - buf += StringFromFormat("%04x %04x ", op1, op2); + dest += StringFromFormat("%04x %04x ", op1, op2); } else { op2 = 0; if (settings_.show_hex) - buf += StringFromFormat("%04x ", op1); + dest += StringFromFormat("%04x ", op1); } std::string opname = opc->name; @@ -248,30 +242,30 @@ bool DSPDisassembler::DisassembleOpcode(const u16* binbuf, int base_addr, int pa ext_buf = MakeLowerCase(ext_buf); if (settings_.print_tabs) - buf += StringFromFormat("%s\t", ext_buf.c_str()); + dest += StringFromFormat("%s\t", ext_buf.c_str()); else - buf += StringFromFormat("%-12s", ext_buf.c_str()); + dest += StringFromFormat("%-12s", ext_buf.c_str()); if (opc->param_count > 0) - buf += DisassembleParameters(*opc, op1, op2); + dest += DisassembleParameters(*opc, op1, op2); // Handle opcode extension. if (is_extended) { if (opc->param_count > 0) - buf += " "; + dest += " "; - buf += ": "; + dest += ": "; if (opc_ext->param_count > 0) - buf += DisassembleParameters(*opc_ext, op1, op2); + dest += DisassembleParameters(*opc_ext, op1, op2); } if (opc->opcode_mask == 0) { // unknown opcode unk_opcodes[op1]++; - buf += "\t\t; *** UNKNOWN OPCODE ***"; + dest += "\t\t; *** UNKNOWN OPCODE ***"; } if (is_extended) @@ -279,35 +273,6 @@ bool DSPDisassembler::DisassembleOpcode(const u16* binbuf, int base_addr, int pa else *pc += opc->size; - if (pass == 2) - dest.append(buf); - - return true; -} - -bool DSPDisassembler::DisassembleFile(const std::string& name, int base_addr, int pass, - std::string& output) -{ - File::IOFile in(name, "rb"); - if (!in) - { - printf("gd_dis_file: No input\n"); - return false; - } - - const int size = ((int)in.GetSize() & ~1) / 2; - std::vector binbuf(size); - in.ReadArray(binbuf.data(), size); - in.Close(); - - // Actually do the disassembly. - for (u16 pc = 0; pc < size;) - { - DisassembleOpcode(binbuf.data(), base_addr, pass, &pc, output); - if (pass == 2) - output.append("\n"); - } - return true; } } // namespace DSP diff --git a/Source/Core/Core/DSP/DSPDisassembler.h b/Source/Core/Core/DSP/DSPDisassembler.h index 78b3c31ddc..d3c2f97260 100644 --- a/Source/Core/Core/DSP/DSPDisassembler.h +++ b/Source/Core/Core/DSP/DSPDisassembler.h @@ -36,16 +36,12 @@ public: explicit DSPDisassembler(const AssemblerSettings& settings); ~DSPDisassembler(); - bool Disassemble(int start_pc, const std::vector& code, int base_addr, std::string& text); + bool Disassemble(const std::vector& code, int base_addr, std::string& text); // Warning - this one is trickier to use right. - // Use pass == 2 if you're just using it by itself. - bool DisassembleOpcode(const u16* binbuf, int base_addr, int pass, u16* pc, std::string& dest); + bool DisassembleOpcode(const u16* binbuf, int base_addr, u16* pc, std::string& dest); private: - // Moves PC forward and writes the result to dest. - bool DisassembleFile(const std::string& name, int base_addr, int pass, std::string& output); - std::string DisassembleParameters(const DSPOPCTemplate& opc, u16 op1, u16 op2); std::map unk_opcodes; diff --git a/Source/Core/Core/HW/DSPLLE/DSPSymbols.cpp b/Source/Core/Core/HW/DSPLLE/DSPSymbols.cpp index 844f24b6db..093389761f 100644 --- a/Source/Core/Core/HW/DSPLLE/DSPSymbols.cpp +++ b/Source/Core/Core/HW/DSPLLE/DSPSymbols.cpp @@ -230,7 +230,7 @@ void AutoDisassembly(u16 start_addr, u16 end_addr) addr_to_line[addr] = line_counter; std::string buf; - if (!disasm.DisassembleOpcode(ptr, 0, 2, &addr, buf)) + if (!disasm.DisassembleOpcode(ptr, 0, &addr, buf)) { ERROR_LOG(DSPLLE, "disasm failed at %04x", addr); break; diff --git a/Source/UnitTests/Core/DSP/DSPAssemblyTest.cpp b/Source/UnitTests/Core/DSP/DSPAssemblyTest.cpp index 5d8a7838ce..b1b0ba68d8 100644 --- a/Source/UnitTests/Core/DSP/DSPAssemblyTest.cpp +++ b/Source/UnitTests/Core/DSP/DSPAssemblyTest.cpp @@ -23,7 +23,7 @@ static bool RoundTrippableDissassemble(const std::vector& code, std::string settings.show_pc = false; DSP::DSPDisassembler disasm(settings); - return disasm.Disassemble(0x0000, code, 0x0000, text); + return disasm.Disassemble(code, 0x0000, text); } // This test goes from text ASM to binary to text ASM and once again back to binary. From 54ef5299bbce467201ead6f33c61511ae358870a Mon Sep 17 00:00:00 2001 From: Michael Maltese Date: Mon, 15 May 2017 17:16:55 -0700 Subject: [PATCH 2/6] DSPDisassembler: remove UnkOps.txt write on destruction --- Source/Core/Core/DSP/DSPDisassembler.cpp | 31 ------------------------ Source/Core/Core/DSP/DSPDisassembler.h | 2 -- 2 files changed, 33 deletions(-) diff --git a/Source/Core/Core/DSP/DSPDisassembler.cpp b/Source/Core/Core/DSP/DSPDisassembler.cpp index ebe8121cf2..d6b94a9597 100644 --- a/Source/Core/Core/DSP/DSPDisassembler.cpp +++ b/Source/Core/Core/DSP/DSPDisassembler.cpp @@ -25,36 +25,6 @@ DSPDisassembler::DSPDisassembler(const AssemblerSettings& settings) : settings_( { } -DSPDisassembler::~DSPDisassembler() -{ - // Some old code for logging unknown ops. - std::string filename = File::GetUserPath(D_DUMPDSP_IDX) + "UnkOps.txt"; - std::ofstream uo(filename); - if (!uo) - return; - - int count = 0; - for (const auto& entry : unk_opcodes) - { - if (entry.second > 0) - { - count++; - uo << StringFromFormat("OP%04x\t%d", entry.first, entry.second); - for (int j = 15; j >= 0; j--) // print op bits - { - if ((j & 0x3) == 3) - uo << "\tb"; - - uo << StringFromFormat("%d", (entry.first >> j) & 0x1); - } - - uo << "\n"; - } - } - - uo << StringFromFormat("Unknown opcodes count: %d\n", count); -} - bool DSPDisassembler::Disassemble(const std::vector& code, int base_addr, std::string& text) { for (u16 pc = 0; pc < code.size();) @@ -264,7 +234,6 @@ bool DSPDisassembler::DisassembleOpcode(const u16* binbuf, int base_addr, u16* p if (opc->opcode_mask == 0) { // unknown opcode - unk_opcodes[op1]++; dest += "\t\t; *** UNKNOWN OPCODE ***"; } diff --git a/Source/Core/Core/DSP/DSPDisassembler.h b/Source/Core/Core/DSP/DSPDisassembler.h index d3c2f97260..8332f0f204 100644 --- a/Source/Core/Core/DSP/DSPDisassembler.h +++ b/Source/Core/Core/DSP/DSPDisassembler.h @@ -34,7 +34,6 @@ class DSPDisassembler { public: explicit DSPDisassembler(const AssemblerSettings& settings); - ~DSPDisassembler(); bool Disassemble(const std::vector& code, int base_addr, std::string& text); @@ -43,7 +42,6 @@ public: private: std::string DisassembleParameters(const DSPOPCTemplate& opc, u16 op1, u16 op2); - std::map unk_opcodes; const AssemblerSettings settings_; From 0f98cd636b80773cacc85239c701bf03ead2c29e Mon Sep 17 00:00:00 2001 From: Michael Maltese Date: Mon, 15 May 2017 17:18:43 -0700 Subject: [PATCH 3/6] DSPDisassembler: remove unused labels member from dspdisassembler --- Source/Core/Core/DSP/DSPDisassembler.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/Source/Core/Core/DSP/DSPDisassembler.h b/Source/Core/Core/DSP/DSPDisassembler.h index 8332f0f204..0eb4c301c7 100644 --- a/Source/Core/Core/DSP/DSPDisassembler.h +++ b/Source/Core/Core/DSP/DSPDisassembler.h @@ -12,7 +12,6 @@ #include "Common/CommonTypes.h" #include "Core/DSP/DSPTables.h" -#include "Core/DSP/LabelMap.h" namespace DSP { @@ -44,7 +43,5 @@ private: std::string DisassembleParameters(const DSPOPCTemplate& opc, u16 op1, u16 op2); const AssemblerSettings settings_; - - LabelMap labels; }; } // namespace DSP From 2564823522965e2a2dcfad757f6833f6740e6f7e Mon Sep 17 00:00:00 2001 From: Michael Maltese Date: Mon, 15 May 2017 17:30:21 -0700 Subject: [PATCH 4/6] DSPDisassembler: cleanup disassembler text handling --- Source/Core/Core/DSP/DSPDisassembler.cpp | 33 ++++++------------------ 1 file changed, 8 insertions(+), 25 deletions(-) diff --git a/Source/Core/Core/DSP/DSPDisassembler.cpp b/Source/Core/Core/DSP/DSPDisassembler.cpp index d6b94a9597..7422852f18 100644 --- a/Source/Core/Core/DSP/DSPDisassembler.cpp +++ b/Source/Core/Core/DSP/DSPDisassembler.cpp @@ -129,13 +129,6 @@ std::string DSPDisassembler::DisassembleParameters(const DSPOPCTemplate& opc, u1 return buf; } -static std::string MakeLowerCase(std::string in) -{ - std::transform(in.begin(), in.end(), in.begin(), ::tolower); - - return in; -} - bool DSPDisassembler::DisassembleOpcode(const u16* binbuf, int base_addr, u16* pc, std::string& dest) { @@ -150,10 +143,9 @@ bool DSPDisassembler::DisassembleOpcode(const u16* binbuf, int base_addr, u16* p // Find main opcode const DSPOPCTemplate* opc = FindOpInfoByOpcode(op1); - const DSPOPCTemplate fake_op = {"CW", 0x0000, 0x0000, DSP::Interpreter::nop, - nullptr, 1, 1, {{P_VAL, 2, 0, 0, 0xffff}}, - false, false, false, false, - false}; + const DSPOPCTemplate fake_op = { + "CW", 0x0000, 0x0000, nullptr, nullptr, 1, 1, {{P_VAL, 2, 0, 0, 0xffff}}, + false, false, false, false, false}; if (!opc) opc = &fake_op; @@ -200,21 +192,15 @@ bool DSPDisassembler::DisassembleOpcode(const u16* binbuf, int base_addr, u16* p } std::string opname = opc->name; - if (settings_.lower_case_ops) - opname = MakeLowerCase(opname); - - std::string ext_buf; if (is_extended) - ext_buf = StringFromFormat("%s%c%s", opname.c_str(), settings_.ext_separator, opc_ext->name); - else - ext_buf = opname; + opname += StringFromFormat("%c%s", settings_.ext_separator, opc_ext->name); if (settings_.lower_case_ops) - ext_buf = MakeLowerCase(ext_buf); + std::transform(opname.begin(), opname.end(), opname.begin(), ::tolower); if (settings_.print_tabs) - dest += StringFromFormat("%s\t", ext_buf.c_str()); + dest += StringFromFormat("%s\t", opname.c_str()); else - dest += StringFromFormat("%-12s", ext_buf.c_str()); + dest += StringFromFormat("%-12s", opname.c_str()); if (opc->param_count > 0) dest += DisassembleParameters(*opc, op1, op2); @@ -237,10 +223,7 @@ bool DSPDisassembler::DisassembleOpcode(const u16* binbuf, int base_addr, u16* p dest += "\t\t; *** UNKNOWN OPCODE ***"; } - if (is_extended) - *pc += opc_ext->size; - else - *pc += opc->size; + *pc += is_extended ? opc_ext->size : opc->size; return true; } From 1d0185d7d56e6f96d0cfa89096d8caeecb26d841 Mon Sep 17 00:00:00 2001 From: Michael Maltese Date: Wed, 24 May 2017 15:20:31 -0700 Subject: [PATCH 5/6] DSPDisassembler: remove unused base_addr parameter --- Source/Core/Core/DSP/DSPCodeUtil.cpp | 8 ++++---- Source/Core/Core/DSP/DSPDisassembler.cpp | 7 +++---- Source/Core/Core/DSP/DSPDisassembler.h | 4 ++-- Source/Core/Core/HW/DSPLLE/DSPSymbols.cpp | 2 +- Source/UnitTests/Core/DSP/DSPAssemblyTest.cpp | 2 +- 5 files changed, 11 insertions(+), 12 deletions(-) diff --git a/Source/Core/Core/DSP/DSPCodeUtil.cpp b/Source/Core/Core/DSP/DSPCodeUtil.cpp index a5c789716b..b33dcb416f 100644 --- a/Source/Core/Core/DSP/DSPCodeUtil.cpp +++ b/Source/Core/Core/DSP/DSPCodeUtil.cpp @@ -56,7 +56,7 @@ bool Disassemble(const std::vector& code, bool line_numbers, std::string& t settings.decode_registers = true; DSPDisassembler disasm(settings); - bool success = disasm.Disassemble(code, 0x0000, text); + bool success = disasm.Disassemble(code, text); return success; } @@ -79,9 +79,9 @@ bool Compare(const std::vector& code1, const std::vector& code2) { std::string line1, line2; u16 pc = i; - disassembler.DisassembleOpcode(&code1[0], 0x0000, &pc, line1); + disassembler.DisassembleOpcode(&code1[0], &pc, line1); pc = i; - disassembler.DisassembleOpcode(&code2[0], 0x0000, &pc, line2); + disassembler.DisassembleOpcode(&code2[0], &pc, line2); printf("!! %04x : %04x vs %04x - %s vs %s\n", i, code1[i], code2[i], line1.c_str(), line2.c_str()); } @@ -94,7 +94,7 @@ bool Compare(const std::vector& code1, const std::vector& code2) { u16 pc = i; std::string line; - disassembler.DisassembleOpcode(&longest[0], 0x0000, &pc, line); + disassembler.DisassembleOpcode(&longest[0], &pc, line); printf("!! %s\n", line.c_str()); } } diff --git a/Source/Core/Core/DSP/DSPDisassembler.cpp b/Source/Core/Core/DSP/DSPDisassembler.cpp index 7422852f18..ce911aeca9 100644 --- a/Source/Core/Core/DSP/DSPDisassembler.cpp +++ b/Source/Core/Core/DSP/DSPDisassembler.cpp @@ -25,11 +25,11 @@ DSPDisassembler::DSPDisassembler(const AssemblerSettings& settings) : settings_( { } -bool DSPDisassembler::Disassemble(const std::vector& code, int base_addr, std::string& text) +bool DSPDisassembler::Disassemble(const std::vector& code, std::string& text) { for (u16 pc = 0; pc < code.size();) { - if (!DisassembleOpcode(code.data(), base_addr, &pc, text)) + if (!DisassembleOpcode(code.data(), &pc, text)) return false; text.append("\n"); } @@ -129,8 +129,7 @@ std::string DSPDisassembler::DisassembleParameters(const DSPOPCTemplate& opc, u1 return buf; } -bool DSPDisassembler::DisassembleOpcode(const u16* binbuf, int base_addr, u16* pc, - std::string& dest) +bool DSPDisassembler::DisassembleOpcode(const u16* binbuf, u16* pc, std::string& dest) { if ((*pc & 0x7fff) >= 0x1000) { diff --git a/Source/Core/Core/DSP/DSPDisassembler.h b/Source/Core/Core/DSP/DSPDisassembler.h index 0eb4c301c7..e02e221d9b 100644 --- a/Source/Core/Core/DSP/DSPDisassembler.h +++ b/Source/Core/Core/DSP/DSPDisassembler.h @@ -34,10 +34,10 @@ class DSPDisassembler public: explicit DSPDisassembler(const AssemblerSettings& settings); - bool Disassemble(const std::vector& code, int base_addr, std::string& text); + bool Disassemble(const std::vector& code, std::string& text); // Warning - this one is trickier to use right. - bool DisassembleOpcode(const u16* binbuf, int base_addr, u16* pc, std::string& dest); + bool DisassembleOpcode(const u16* binbuf, u16* pc, std::string& dest); private: std::string DisassembleParameters(const DSPOPCTemplate& opc, u16 op1, u16 op2); diff --git a/Source/Core/Core/HW/DSPLLE/DSPSymbols.cpp b/Source/Core/Core/HW/DSPLLE/DSPSymbols.cpp index 093389761f..868fd3443c 100644 --- a/Source/Core/Core/HW/DSPLLE/DSPSymbols.cpp +++ b/Source/Core/Core/HW/DSPLLE/DSPSymbols.cpp @@ -230,7 +230,7 @@ void AutoDisassembly(u16 start_addr, u16 end_addr) addr_to_line[addr] = line_counter; std::string buf; - if (!disasm.DisassembleOpcode(ptr, 0, &addr, buf)) + if (!disasm.DisassembleOpcode(ptr, &addr, buf)) { ERROR_LOG(DSPLLE, "disasm failed at %04x", addr); break; diff --git a/Source/UnitTests/Core/DSP/DSPAssemblyTest.cpp b/Source/UnitTests/Core/DSP/DSPAssemblyTest.cpp index b1b0ba68d8..4dd3395a46 100644 --- a/Source/UnitTests/Core/DSP/DSPAssemblyTest.cpp +++ b/Source/UnitTests/Core/DSP/DSPAssemblyTest.cpp @@ -23,7 +23,7 @@ static bool RoundTrippableDissassemble(const std::vector& code, std::string settings.show_pc = false; DSP::DSPDisassembler disasm(settings); - return disasm.Disassemble(code, 0x0000, text); + return disasm.Disassemble(code, text); } // This test goes from text ASM to binary to text ASM and once again back to binary. From 7dab92d8b568db8b4fc208614d23ae696029389a Mon Sep 17 00:00:00 2001 From: Michael Maltese Date: Fri, 30 Jun 2017 01:45:33 -0700 Subject: [PATCH 6/6] DSPDisassembler: fail when buffer too large --- Source/Core/Core/DSP/DSPDisassembler.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Source/Core/Core/DSP/DSPDisassembler.cpp b/Source/Core/Core/DSP/DSPDisassembler.cpp index ce911aeca9..af736cad2a 100644 --- a/Source/Core/Core/DSP/DSPDisassembler.cpp +++ b/Source/Core/Core/DSP/DSPDisassembler.cpp @@ -27,6 +27,12 @@ DSPDisassembler::DSPDisassembler(const AssemblerSettings& settings) : settings_( bool DSPDisassembler::Disassemble(const std::vector& code, std::string& text) { + if (code.size() > std::numeric_limits::max()) + { + text.append("; code too large for 16-bit addressing\n"); + return false; + } + for (u16 pc = 0; pc < code.size();) { if (!DisassembleOpcode(code.data(), &pc, text))