From e640ef9d7b07a007e417dc2852332baf024e6a51 Mon Sep 17 00:00:00 2001 From: magumagu Date: Fri, 23 Jan 2015 21:27:34 -0800 Subject: [PATCH 1/2] Improve illegal instruction handling. This should more reliably show an error message for illegal instructions. --- .../Core/PowerPC/Interpreter/Interpreter.cpp | 12 ++++-------- .../PowerPC/Interpreter/Interpreter_Tables.cpp | 16 ++++++++-------- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/Source/Core/Core/PowerPC/Interpreter/Interpreter.cpp b/Source/Core/Core/PowerPC/Interpreter/Interpreter.cpp index 0f549a74b9..55be204115 100644 --- a/Source/Core/Core/PowerPC/Interpreter/Interpreter.cpp +++ b/Source/Core/Core/PowerPC/Interpreter/Interpreter.cpp @@ -281,14 +281,10 @@ void Interpreter::Run() void Interpreter::unknown_instruction(UGeckoInstruction _inst) { - if (_inst.hex != 0) - { - std::string disasm = GekkoDisassembler::Disassemble(PowerPC::HostRead_U32(last_pc), last_pc); - NOTICE_LOG(POWERPC, "Last PC = %08x : %s", last_pc, disasm.c_str()); - Dolphin_Debugger::PrintCallstack(); - _assert_msg_(POWERPC, 0, "\nIntCPU: Unknown instruction %08x at PC = %08x last_PC = %08x LR = %08x\n", _inst.hex, PC, last_pc, LR); - } - + std::string disasm = GekkoDisassembler::Disassemble(PowerPC::HostRead_U32(last_pc), last_pc); + NOTICE_LOG(POWERPC, "Last PC = %08x : %s", last_pc, disasm.c_str()); + Dolphin_Debugger::PrintCallstack(); + _assert_msg_(POWERPC, 0, "\nIntCPU: Unknown instruction %08x at PC = %08x last_PC = %08x LR = %08x\n", _inst.hex, PC, last_pc, LR); } void Interpreter::ClearCache() diff --git a/Source/Core/Core/PowerPC/Interpreter/Interpreter_Tables.cpp b/Source/Core/Core/PowerPC/Interpreter/Interpreter_Tables.cpp index 639a899f57..fb7275f9a6 100644 --- a/Source/Core/Core/PowerPC/Interpreter/Interpreter_Tables.cpp +++ b/Source/Core/Core/PowerPC/Interpreter/Interpreter_Tables.cpp @@ -85,14 +85,14 @@ static GekkoOPTemplate primarytable[] = {61, Interpreter::psq_stu, {"psq_stu", OPTYPE_STOREPS, FL_IN_FLOAT_S | FL_OUT_A | FL_IN_A | FL_USE_FPU | FL_LOADSTORE, 1, 0, 0, 0}}, //missing: 0, 5, 6, 9, 22, 30, 62, 58 - {0, Interpreter::unknown_instruction, {"unknown_instruction", OPTYPE_UNKNOWN, 0, 0, 0, 0, 0}}, - {5, Interpreter::unknown_instruction, {"unknown_instruction", OPTYPE_UNKNOWN, 0, 0, 0, 0, 0}}, - {6, Interpreter::unknown_instruction, {"unknown_instruction", OPTYPE_UNKNOWN, 0, 0, 0, 0, 0}}, - {9, Interpreter::unknown_instruction, {"unknown_instruction", OPTYPE_UNKNOWN, 0, 0, 0, 0, 0}}, - {22, Interpreter::unknown_instruction, {"unknown_instruction", OPTYPE_UNKNOWN, 0, 0, 0, 0, 0}}, - {30, Interpreter::unknown_instruction, {"unknown_instruction", OPTYPE_UNKNOWN, 0, 0, 0, 0, 0}}, - {62, Interpreter::unknown_instruction, {"unknown_instruction", OPTYPE_UNKNOWN, 0, 0, 0, 0, 0}}, - {58, Interpreter::unknown_instruction, {"unknown_instruction", OPTYPE_UNKNOWN, 0, 0, 0, 0, 0}}, + {0, Interpreter::unknown_instruction, {"unknown_instruction", OPTYPE_UNKNOWN, FL_ENDBLOCK, 0, 0, 0, 0}}, + {5, Interpreter::unknown_instruction, {"unknown_instruction", OPTYPE_UNKNOWN, FL_ENDBLOCK, 0, 0, 0, 0}}, + {6, Interpreter::unknown_instruction, {"unknown_instruction", OPTYPE_UNKNOWN, FL_ENDBLOCK, 0, 0, 0, 0}}, + {9, Interpreter::unknown_instruction, {"unknown_instruction", OPTYPE_UNKNOWN, FL_ENDBLOCK, 0, 0, 0, 0}}, + {22, Interpreter::unknown_instruction, {"unknown_instruction", OPTYPE_UNKNOWN, FL_ENDBLOCK, 0, 0, 0, 0}}, + {30, Interpreter::unknown_instruction, {"unknown_instruction", OPTYPE_UNKNOWN, FL_ENDBLOCK, 0, 0, 0, 0}}, + {62, Interpreter::unknown_instruction, {"unknown_instruction", OPTYPE_UNKNOWN, FL_ENDBLOCK, 0, 0, 0, 0}}, + {58, Interpreter::unknown_instruction, {"unknown_instruction", OPTYPE_UNKNOWN, FL_ENDBLOCK, 0, 0, 0, 0}}, }; static GekkoOPTemplate table4[] = From 985087c7e2e17e66eb4e4bb7f70ab25a4a86c838 Mon Sep 17 00:00:00 2001 From: magumagu Date: Wed, 11 Feb 2015 21:49:53 -0800 Subject: [PATCH 2/2] Make all unknown opcodes behave consistently. Consistently fall back to the interpreter for unknown instructions, and make sure GetOpInfo() always returns a non-null pointer. --- .../Interpreter/Interpreter_Tables.cpp | 26 +++++++++---------- Source/Core/Core/PowerPC/Jit64/Jit.cpp | 5 ---- Source/Core/Core/PowerPC/Jit64/Jit.h | 1 - .../Core/Core/PowerPC/Jit64/Jit64_Tables.cpp | 10 +++---- Source/Core/Core/PowerPC/Jit64IL/JitIL.cpp | 6 ----- Source/Core/Core/PowerPC/Jit64IL/JitIL.h | 1 - .../Core/PowerPC/Jit64IL/JitIL_Tables.cpp | 10 +++---- Source/Core/Core/PowerPC/JitArm32/Jit.cpp | 4 --- Source/Core/Core/PowerPC/JitArm32/Jit.h | 1 - .../Core/PowerPC/JitArm32/JitArm_Tables.cpp | 10 +++---- Source/Core/Core/PowerPC/JitArm64/Jit.cpp | 5 ---- Source/Core/Core/PowerPC/JitArm64/Jit.h | 1 - .../Core/PowerPC/JitArm64/JitArm64_Tables.cpp | 10 +++---- .../Core/Core/PowerPC/JitILCommon/JitILBase.h | 1 - Source/Core/Core/PowerPC/PPCAnalyst.cpp | 5 ---- 15 files changed, 33 insertions(+), 63 deletions(-) diff --git a/Source/Core/Core/PowerPC/Interpreter/Interpreter_Tables.cpp b/Source/Core/Core/PowerPC/Interpreter/Interpreter_Tables.cpp index fb7275f9a6..c8c16a6640 100644 --- a/Source/Core/Core/PowerPC/Interpreter/Interpreter_Tables.cpp +++ b/Source/Core/Core/PowerPC/Interpreter/Interpreter_Tables.cpp @@ -13,6 +13,8 @@ struct GekkoOPTemplate GekkoOPInfo opinfo; }; +static GekkoOPInfo unknownopinfo = { "unknown_instruction", OPTYPE_UNKNOWN, FL_ENDBLOCK, 0, 0, 0, 0 }; + static GekkoOPTemplate primarytable[] = { {4, Interpreter::RunTable4, {"RunTable4", OPTYPE_SUBTABLE | (4<<24), 0, 0, 0, 0, 0}}, @@ -85,14 +87,6 @@ static GekkoOPTemplate primarytable[] = {61, Interpreter::psq_stu, {"psq_stu", OPTYPE_STOREPS, FL_IN_FLOAT_S | FL_OUT_A | FL_IN_A | FL_USE_FPU | FL_LOADSTORE, 1, 0, 0, 0}}, //missing: 0, 5, 6, 9, 22, 30, 62, 58 - {0, Interpreter::unknown_instruction, {"unknown_instruction", OPTYPE_UNKNOWN, FL_ENDBLOCK, 0, 0, 0, 0}}, - {5, Interpreter::unknown_instruction, {"unknown_instruction", OPTYPE_UNKNOWN, FL_ENDBLOCK, 0, 0, 0, 0}}, - {6, Interpreter::unknown_instruction, {"unknown_instruction", OPTYPE_UNKNOWN, FL_ENDBLOCK, 0, 0, 0, 0}}, - {9, Interpreter::unknown_instruction, {"unknown_instruction", OPTYPE_UNKNOWN, FL_ENDBLOCK, 0, 0, 0, 0}}, - {22, Interpreter::unknown_instruction, {"unknown_instruction", OPTYPE_UNKNOWN, FL_ENDBLOCK, 0, 0, 0, 0}}, - {30, Interpreter::unknown_instruction, {"unknown_instruction", OPTYPE_UNKNOWN, FL_ENDBLOCK, 0, 0, 0, 0}}, - {62, Interpreter::unknown_instruction, {"unknown_instruction", OPTYPE_UNKNOWN, FL_ENDBLOCK, 0, 0, 0, 0}}, - {58, Interpreter::unknown_instruction, {"unknown_instruction", OPTYPE_UNKNOWN, FL_ENDBLOCK, 0, 0, 0, 0}}, }; static GekkoOPTemplate table4[] = @@ -364,10 +358,16 @@ void InitTables() return; //clear + for (int i = 0; i < 64; i++) + { + Interpreter::m_opTable[i] = Interpreter::unknown_instruction; + m_infoTable[i] = &unknownopinfo; + } + for (int i = 0; i < 32; i++) { Interpreter::m_opTable59[i] = Interpreter::unknown_instruction; - m_infoTable59[i] = nullptr; + m_infoTable59[i] = &unknownopinfo; } for (int i = 0; i < 1024; i++) @@ -376,10 +376,10 @@ void InitTables() Interpreter::m_opTable19[i] = Interpreter::unknown_instruction; Interpreter::m_opTable31[i] = Interpreter::unknown_instruction; Interpreter::m_opTable63[i] = Interpreter::unknown_instruction; - m_infoTable4[i] = nullptr; - m_infoTable19[i] = nullptr; - m_infoTable31[i] = nullptr; - m_infoTable63[i] = nullptr; + m_infoTable4[i] = &unknownopinfo; + m_infoTable19[i] = &unknownopinfo; + m_infoTable31[i] = &unknownopinfo; + m_infoTable63[i] = &unknownopinfo; } for (auto& tpl : primarytable) diff --git a/Source/Core/Core/PowerPC/Jit64/Jit.cpp b/Source/Core/Core/PowerPC/Jit64/Jit.cpp index 7bfcb17002..187cb1fdac 100644 --- a/Source/Core/Core/PowerPC/Jit64/Jit.cpp +++ b/Source/Core/Core/PowerPC/Jit64/Jit.cpp @@ -246,11 +246,6 @@ void Jit64::WriteCallInterpreter(UGeckoInstruction inst) ABI_PopRegistersAndAdjustStack({}, 0); } -void Jit64::unknown_instruction(UGeckoInstruction inst) -{ - PanicAlert("unknown_instruction %08x - Fix me ;)", inst.hex); -} - void Jit64::FallBackToInterpreter(UGeckoInstruction _inst) { WriteCallInterpreter(_inst.hex); diff --git a/Source/Core/Core/PowerPC/Jit64/Jit.h b/Source/Core/Core/PowerPC/Jit64/Jit.h index 311c5be541..323358bacb 100644 --- a/Source/Core/Core/PowerPC/Jit64/Jit.h +++ b/Source/Core/Core/PowerPC/Jit64/Jit.h @@ -152,7 +152,6 @@ public: void FloatCompare(UGeckoInstruction inst, bool upper = false); // OPCODES - void unknown_instruction(UGeckoInstruction _inst); void FallBackToInterpreter(UGeckoInstruction _inst); void DoNothing(UGeckoInstruction _inst); void HLEFunction(UGeckoInstruction _inst); diff --git a/Source/Core/Core/PowerPC/Jit64/Jit64_Tables.cpp b/Source/Core/Core/PowerPC/Jit64/Jit64_Tables.cpp index c33f22e8d5..cbeb9c5ed9 100644 --- a/Source/Core/Core/PowerPC/Jit64/Jit64_Tables.cpp +++ b/Source/Core/Core/PowerPC/Jit64/Jit64_Tables.cpp @@ -398,15 +398,15 @@ void InitTables() //clear for (auto& tpl : dynaOpTable59) { - tpl = &Jit64::unknown_instruction; + tpl = &Jit64::FallBackToInterpreter; } for (int i = 0; i < 1024; i++) { - dynaOpTable4 [i] = &Jit64::unknown_instruction; - dynaOpTable19[i] = &Jit64::unknown_instruction; - dynaOpTable31[i] = &Jit64::unknown_instruction; - dynaOpTable63[i] = &Jit64::unknown_instruction; + dynaOpTable4 [i] = &Jit64::FallBackToInterpreter; + dynaOpTable19[i] = &Jit64::FallBackToInterpreter; + dynaOpTable31[i] = &Jit64::FallBackToInterpreter; + dynaOpTable63[i] = &Jit64::FallBackToInterpreter; } for (auto& tpl : primarytable) diff --git a/Source/Core/Core/PowerPC/Jit64IL/JitIL.cpp b/Source/Core/Core/PowerPC/Jit64IL/JitIL.cpp index fcf45aec1b..07ff7fe4b4 100644 --- a/Source/Core/Core/PowerPC/Jit64IL/JitIL.cpp +++ b/Source/Core/Core/PowerPC/Jit64IL/JitIL.cpp @@ -305,12 +305,6 @@ void JitIL::WriteCallInterpreter(UGeckoInstruction inst) } } -void JitIL::unknown_instruction(UGeckoInstruction inst) -{ - // CCPU::Break(); - PanicAlert("unknown_instruction %08x - Fix me ;)", inst.hex); -} - void JitIL::FallBackToInterpreter(UGeckoInstruction _inst) { ibuild.EmitFallBackToInterpreter( diff --git a/Source/Core/Core/PowerPC/Jit64IL/JitIL.h b/Source/Core/Core/PowerPC/Jit64IL/JitIL.h index d846c7af10..27b048727a 100644 --- a/Source/Core/Core/PowerPC/Jit64IL/JitIL.h +++ b/Source/Core/Core/PowerPC/Jit64IL/JitIL.h @@ -99,7 +99,6 @@ public: void WriteCode(u32 exitAddress); // OPCODES - void unknown_instruction(UGeckoInstruction _inst) override; void FallBackToInterpreter(UGeckoInstruction _inst) override; void DoNothing(UGeckoInstruction _inst) override; void HLEFunction(UGeckoInstruction _inst) override; diff --git a/Source/Core/Core/PowerPC/Jit64IL/JitIL_Tables.cpp b/Source/Core/Core/PowerPC/Jit64IL/JitIL_Tables.cpp index 28aaa157e0..19df5e1617 100644 --- a/Source/Core/Core/PowerPC/Jit64IL/JitIL_Tables.cpp +++ b/Source/Core/Core/PowerPC/Jit64IL/JitIL_Tables.cpp @@ -403,15 +403,15 @@ void InitTables() //clear for (auto& tpl : dynaOpTable59) { - tpl = &JitIL::unknown_instruction; + tpl = &JitIL::FallBackToInterpreter; } for (int i = 0; i < 1024; i++) { - dynaOpTable4 [i] = &JitIL::unknown_instruction; - dynaOpTable19[i] = &JitIL::unknown_instruction; - dynaOpTable31[i] = &JitIL::unknown_instruction; - dynaOpTable63[i] = &JitIL::unknown_instruction; + dynaOpTable4 [i] = &JitIL::FallBackToInterpreter; + dynaOpTable19[i] = &JitIL::FallBackToInterpreter; + dynaOpTable31[i] = &JitIL::FallBackToInterpreter; + dynaOpTable63[i] = &JitIL::FallBackToInterpreter; } for (auto& tpl : primarytable) diff --git a/Source/Core/Core/PowerPC/JitArm32/Jit.cpp b/Source/Core/Core/PowerPC/JitArm32/Jit.cpp index ab796df34c..5f96418dd6 100644 --- a/Source/Core/Core/PowerPC/JitArm32/Jit.cpp +++ b/Source/Core/Core/PowerPC/JitArm32/Jit.cpp @@ -64,10 +64,6 @@ void JitArm::WriteCallInterpreter(UGeckoInstruction inst) MOVI2R(R12, (u32)instr); BL(R12); } -void JitArm::unknown_instruction(UGeckoInstruction inst) -{ - PanicAlert("unknown_instruction %08x - Fix me ;)", inst.hex); -} void JitArm::FallBackToInterpreter(UGeckoInstruction _inst) { diff --git a/Source/Core/Core/PowerPC/JitArm32/Jit.h b/Source/Core/Core/PowerPC/JitArm32/Jit.h index 2d961a20f9..fe9537b1fc 100644 --- a/Source/Core/Core/PowerPC/JitArm32/Jit.h +++ b/Source/Core/Core/PowerPC/JitArm32/Jit.h @@ -134,7 +134,6 @@ public: void SafeLoadToReg(ArmGen::ARMReg dest, s32 addr, s32 offsetReg, int accessSize, s32 offset, bool signExtend, bool reverse, bool update); // OPCODES - void unknown_instruction(UGeckoInstruction _inst); void FallBackToInterpreter(UGeckoInstruction _inst); void DoNothing(UGeckoInstruction _inst); void HLEFunction(UGeckoInstruction _inst); diff --git a/Source/Core/Core/PowerPC/JitArm32/JitArm_Tables.cpp b/Source/Core/Core/PowerPC/JitArm32/JitArm_Tables.cpp index 79a7399d62..6d1099d6bc 100644 --- a/Source/Core/Core/PowerPC/JitArm32/JitArm_Tables.cpp +++ b/Source/Core/Core/PowerPC/JitArm32/JitArm_Tables.cpp @@ -402,15 +402,15 @@ void InitTables() //clear for (int i = 0; i < 32; i++) { - dynaOpTable59[i] = &JitArm::unknown_instruction; + dynaOpTable59[i] = &JitArm::FallBackToInterpreter; } for (int i = 0; i < 1024; i++) { - dynaOpTable4 [i] = &JitArm::unknown_instruction; - dynaOpTable19[i] = &JitArm::unknown_instruction; - dynaOpTable31[i] = &JitArm::unknown_instruction; - dynaOpTable63[i] = &JitArm::unknown_instruction; + dynaOpTable4 [i] = &JitArm::FallBackToInterpreter; + dynaOpTable19[i] = &JitArm::FallBackToInterpreter; + dynaOpTable31[i] = &JitArm::FallBackToInterpreter; + dynaOpTable63[i] = &JitArm::FallBackToInterpreter; } for (int i = 0; i < (int)(sizeof(primarytable) / sizeof(GekkoOPTemplate)); i++) diff --git a/Source/Core/Core/PowerPC/JitArm64/Jit.cpp b/Source/Core/Core/PowerPC/JitArm64/Jit.cpp index ff67956cde..6d2c0ddf73 100644 --- a/Source/Core/Core/PowerPC/JitArm64/Jit.cpp +++ b/Source/Core/Core/PowerPC/JitArm64/Jit.cpp @@ -43,11 +43,6 @@ void JitArm64::Shutdown() asm_routines.Shutdown(); } -void JitArm64::unknown_instruction(UGeckoInstruction inst) -{ - WARN_LOG(DYNA_REC, "unknown_instruction %08x - Fix me ;)", inst.hex); -} - void JitArm64::FallBackToInterpreter(UGeckoInstruction inst) { gpr.Flush(FlushMode::FLUSH_INTERPRETER, js.op); diff --git a/Source/Core/Core/PowerPC/JitArm64/Jit.h b/Source/Core/Core/PowerPC/JitArm64/Jit.h index 87e09ce176..d9d212a3a3 100644 --- a/Source/Core/Core/PowerPC/JitArm64/Jit.h +++ b/Source/Core/Core/PowerPC/JitArm64/Jit.h @@ -57,7 +57,6 @@ public: } // OPCODES - void unknown_instruction(UGeckoInstruction inst); void FallBackToInterpreter(UGeckoInstruction inst); void DoNothing(UGeckoInstruction inst); void HLEFunction(UGeckoInstruction inst); diff --git a/Source/Core/Core/PowerPC/JitArm64/JitArm64_Tables.cpp b/Source/Core/Core/PowerPC/JitArm64/JitArm64_Tables.cpp index b5997e9f7f..17cc7164ac 100644 --- a/Source/Core/Core/PowerPC/JitArm64/JitArm64_Tables.cpp +++ b/Source/Core/Core/PowerPC/JitArm64/JitArm64_Tables.cpp @@ -400,15 +400,15 @@ void InitTables() //clear for (int i = 0; i < 32; i++) { - dynaOpTable59[i] = &JitArm64::unknown_instruction; + dynaOpTable59[i] = &JitArm64::FallBackToInterpreter; } for (int i = 0; i < 1024; i++) { - dynaOpTable4 [i] = &JitArm64::unknown_instruction; - dynaOpTable19[i] = &JitArm64::unknown_instruction; - dynaOpTable31[i] = &JitArm64::unknown_instruction; - dynaOpTable63[i] = &JitArm64::unknown_instruction; + dynaOpTable4 [i] = &JitArm64::FallBackToInterpreter; + dynaOpTable19[i] = &JitArm64::FallBackToInterpreter; + dynaOpTable31[i] = &JitArm64::FallBackToInterpreter; + dynaOpTable63[i] = &JitArm64::FallBackToInterpreter; } for (int i = 0; i < (int)(sizeof(primarytable) / sizeof(GekkoOPTemplate)); i++) diff --git a/Source/Core/Core/PowerPC/JitILCommon/JitILBase.h b/Source/Core/Core/PowerPC/JitILCommon/JitILBase.h index 16a3f97691..70bd92f41c 100644 --- a/Source/Core/Core/PowerPC/JitILCommon/JitILBase.h +++ b/Source/Core/Core/PowerPC/JitILCommon/JitILBase.h @@ -32,7 +32,6 @@ public: virtual const CommonAsmRoutinesBase *GetAsmRoutines() = 0; // OPCODES - virtual void unknown_instruction(UGeckoInstruction inst) = 0; virtual void FallBackToInterpreter(UGeckoInstruction inst) = 0; virtual void DoNothing(UGeckoInstruction inst) = 0; virtual void HLEFunction(UGeckoInstruction inst) = 0; diff --git a/Source/Core/Core/PowerPC/PPCAnalyst.cpp b/Source/Core/Core/PowerPC/PPCAnalyst.cpp index 739cf22b8e..32105f468a 100644 --- a/Source/Core/Core/PowerPC/PPCAnalyst.cpp +++ b/Source/Core/Core/PowerPC/PPCAnalyst.cpp @@ -681,11 +681,6 @@ u32 PPCAnalyzer::Analyze(u32 address, CodeBlock *block, CodeBuffer *buffer, u32 num_inst++; memset(&code[i], 0, sizeof(CodeOp)); GekkoOPInfo *opinfo = GetOpInfo(inst); - if (!opinfo) - { - PanicAlert("Invalid PowerPC opcode: %x.", inst.hex); - Crash(); - } code[i].opinfo = opinfo; code[i].address = address;