From 4c3230bcde57814b14bdb1701d47929ffcfa138a Mon Sep 17 00:00:00 2001 From: comex Date: Mon, 2 Sep 2013 16:37:04 -0400 Subject: [PATCH] Remove accessType from BackPatch's signature in favor of getting it from DisassembleMov. It isn't easily accessible with sigaction or Mach exceptions (well, requires an additional system call in the latter), and isn't necessary. (and get rid of the enum, because it's only used once, and the comments are more expressive than enum names) --- Source/Core/Common/Src/x64Analyzer.cpp | 177 +++++++++--------- Source/Core/Common/Src/x64Analyzer.h | 15 +- Source/Core/Core/Src/ArmMemTools.cpp | 4 +- .../Src/PowerPC/JitArm32/JitArm_BackPatch.cpp | 2 +- .../Src/PowerPC/JitCommon/JitBackpatch.cpp | 25 +-- .../Core/Core/Src/PowerPC/JitCommon/JitBase.h | 4 +- Source/Core/Core/Src/PowerPC/JitInterface.cpp | 4 +- Source/Core/Core/Src/PowerPC/JitInterface.h | 2 +- Source/Core/Core/Src/x64MemTools.cpp | 5 +- 9 files changed, 109 insertions(+), 129 deletions(-) diff --git a/Source/Core/Common/Src/x64Analyzer.cpp b/Source/Core/Common/Src/x64Analyzer.cpp index 456830e467..9632c46125 100644 --- a/Source/Core/Common/Src/x64Analyzer.cpp +++ b/Source/Core/Common/Src/x64Analyzer.cpp @@ -4,7 +4,7 @@ #include "x64Analyzer.h" -bool DisassembleMov(const unsigned char *codePtr, InstructionInfo &info, int accessType) +bool DisassembleMov(const unsigned char *codePtr, InstructionInfo *info) { unsigned const char *startCodePtr = codePtr; u8 rex = 0; @@ -12,11 +12,11 @@ bool DisassembleMov(const unsigned char *codePtr, InstructionInfo &info, int acc u8 codeByte2 = 0; //Check for regular prefix - info.operandSize = 4; - info.zeroExtend = false; - info.signExtend = false; - info.hasImmediate = false; - info.isMemoryWrite = false; + info->operandSize = 4; + info->zeroExtend = false; + info->signExtend = false; + info->hasImmediate = false; + info->isMemoryWrite = false; u8 modRMbyte = 0; u8 sibByte = 0; @@ -26,7 +26,7 @@ bool DisassembleMov(const unsigned char *codePtr, InstructionInfo &info, int acc if (*codePtr == 0x66) { - info.operandSize = 2; + info->operandSize = 2; codePtr++; } else if (*codePtr == 0x67) @@ -40,7 +40,7 @@ bool DisassembleMov(const unsigned char *codePtr, InstructionInfo &info, int acc rex = *codePtr; if (rex & 8) //REX.W { - info.operandSize = 8; + info->operandSize = 8; } codePtr++; } @@ -72,11 +72,11 @@ bool DisassembleMov(const unsigned char *codePtr, InstructionInfo &info, int acc (codeByte2 & 0xF0) == 0x80 || ((codeByte2 & 0xF0) == 0xA0 && (codeByte2 & 0x07) <= 0x02) || (codeByte2 & 0xF8) == 0xC8) - { + { // No mod R/M byte - } + } else - { + { modRMbyte = *codePtr++; hasModRM = true; } @@ -85,21 +85,21 @@ bool DisassembleMov(const unsigned char *codePtr, InstructionInfo &info, int acc if (hasModRM) { ModRM mrm(modRMbyte, rex); - info.regOperandReg = mrm.reg; + info->regOperandReg = mrm.reg; if (mrm.mod < 3) { if (mrm.rm == 4) { //SIB byte sibByte = *codePtr++; - info.scaledReg = (sibByte >> 3) & 7; - info.otherReg = (sibByte & 7); - if (rex & 2) info.scaledReg += 8; - if (rex & 1) info.otherReg += 8; + info->scaledReg = (sibByte >> 3) & 7; + info->otherReg = (sibByte & 7); + if (rex & 2) info->scaledReg += 8; + if (rex & 1) info->otherReg += 8; } else { - //info.scaledReg = + //info->scaledReg = } } if (mrm.mod == 1 || mrm.mod == 2) @@ -112,100 +112,103 @@ bool DisassembleMov(const unsigned char *codePtr, InstructionInfo &info, int acc } if (displacementSize == 1) - info.displacement = (s32)(s8)*codePtr; + info->displacement = (s32)(s8)*codePtr; else - info.displacement = *((s32 *)codePtr); + info->displacement = *((s32 *)codePtr); codePtr += displacementSize; - if (accessType == 1) + switch (codeByte) { - info.isMemoryWrite = true; - //Write access - switch (codeByte) + // writes + case 0xC6: // mem <- imm8 { - case MOVE_8BIT: //move 8-bit immediate - { - info.hasImmediate = true; - info.immediate = *codePtr; - codePtr++; //move past immediate - } - break; - - case MOVE_16_32BIT: //move 16 or 32-bit immediate, easiest case for writes - { - if (info.operandSize == 2) - { - info.hasImmediate = true; - info.immediate = *(u16*)codePtr; - codePtr += 2; - } - else if (info.operandSize == 4) - { - info.hasImmediate = true; - info.immediate = *(u32*)codePtr; - codePtr += 4; - } - else if (info.operandSize == 8) - { - info.zeroExtend = true; - info.immediate = *(u32*)codePtr; - codePtr += 4; - } - } - break; - case MOVE_REG_TO_MEM: //move reg to memory - break; - - default: - PanicAlert("Unhandled disasm case in write handler!\n\nPlease implement or avoid."); - return false; + info->isMemoryWrite = true; + info->hasImmediate = true; + info->immediate = *codePtr; + codePtr++; //move past immediate } - } - else - { - // Memory read + break; - //mov eax, dword ptr [rax] == 8b 00 - switch (codeByte) + case 0xC7: // mem <- imm16/32 { - case 0x0F: + info->isMemoryWrite = true; + if (info->operandSize == 2) + { + info->hasImmediate = true; + info->immediate = *(u16*)codePtr; + codePtr += 2; + } + else if (info->operandSize == 4) + { + info->hasImmediate = true; + info->immediate = *(u32*)codePtr; + codePtr += 4; + } + else if (info->operandSize == 8) + { + info->zeroExtend = true; + info->immediate = *(u32*)codePtr; + codePtr += 4; + } + } + + case 0x89: // mem <- r16/32/64 + { + info->isMemoryWrite = true; + break; + } + + case 0x0F: // two-byte escape + { + info->isMemoryWrite = false; switch (codeByte2) { - case MOVZX_BYTE: //movzx on byte - info.zeroExtend = true; - info.operandSize = 1; + case 0xB6: // movzx on byte + info->zeroExtend = true; + info->operandSize = 1; break; - case MOVZX_SHORT: //movzx on short - info.zeroExtend = true; - info.operandSize = 2; + case 0xB7: // movzx on short + info->zeroExtend = true; + info->operandSize = 2; break; - case MOVSX_BYTE: //movsx on byte - info.signExtend = true; - info.operandSize = 1; + case 0xBE: // movsx on byte + info->signExtend = true; + info->operandSize = 1; break; - case MOVSX_SHORT: //movsx on short - info.signExtend = true; - info.operandSize = 2; + case 0xBF: // movsx on short + info->signExtend = true; + info->operandSize = 2; break; default: return false; } break; - case 0x8a: - if (info.operandSize == 4) + } + + case 0x8A: // r8 <- mem + { + info->isMemoryWrite = false; + if (info->operandSize == 4) { - info.operandSize = 1; + info->operandSize = 1; break; } - else + else return false; - case 0x8b: - break; //it's OK don't need to do anything - default: - return false; } + + case 0x8B: // r16/32/64 <- mem + { + info->isMemoryWrite = false; + break; + } + + break; + + default: + return false; } - info.instructionSize = (int)(codePtr - startCodePtr); + info->instructionSize = (int)(codePtr - startCodePtr); return true; } diff --git a/Source/Core/Common/Src/x64Analyzer.h b/Source/Core/Common/Src/x64Analyzer.h index 705bcd0e9c..028d291079 100644 --- a/Source/Core/Common/Src/x64Analyzer.h +++ b/Source/Core/Common/Src/x64Analyzer.h @@ -33,21 +33,12 @@ struct ModRM } }; -enum{ - MOVZX_BYTE = 0xB6, //movzx on byte - MOVZX_SHORT = 0xB7, //movzx on short - MOVSX_BYTE = 0xBE, //movsx on byte - MOVSX_SHORT = 0xBF, //movsx on short - MOVE_8BIT = 0xC6, //move 8-bit immediate - MOVE_16_32BIT = 0xC7, //move 16 or 32-bit immediate - MOVE_REG_TO_MEM = 0x89, //move reg to memory -}; - -enum AccessType{ +enum AccessType +{ OP_ACCESS_READ = 0, OP_ACCESS_WRITE = 1 }; -bool DisassembleMov(const unsigned char *codePtr, InstructionInfo &info, int accessType); +bool DisassembleMov(const unsigned char *codePtr, InstructionInfo *info); #endif // _X64ANALYZER_H_ diff --git a/Source/Core/Core/Src/ArmMemTools.cpp b/Source/Core/Core/Src/ArmMemTools.cpp index 34a0c34b7d..01e63a6a42 100644 --- a/Source/Core/Core/Src/ArmMemTools.cpp +++ b/Source/Core/Core/Src/ArmMemTools.cpp @@ -81,11 +81,9 @@ void sigsegv_handler(int signal, siginfo_t *info, void *raw_context) u32 em_address = (u32)(bad_address - memspace_bottom); - int access_type = 0; - CONTEXT fake_ctx; fake_ctx.reg_pc = ctx->arm_pc; - const u8 *new_rip = jit->BackPatch(fault_instruction_ptr, access_type, em_address, &fake_ctx); + const u8 *new_rip = jit->BackPatch(fault_instruction_ptr, em_address, &fake_ctx); if (new_rip) { ctx->arm_pc = fake_ctx.reg_pc; } diff --git a/Source/Core/Core/Src/PowerPC/JitArm32/JitArm_BackPatch.cpp b/Source/Core/Core/Src/PowerPC/JitArm32/JitArm_BackPatch.cpp index 9d01332f57..45b39ecd5b 100644 --- a/Source/Core/Core/Src/PowerPC/JitArm32/JitArm_BackPatch.cpp +++ b/Source/Core/Core/Src/PowerPC/JitArm32/JitArm_BackPatch.cpp @@ -96,7 +96,7 @@ bool DisamLoadStore(const u32 inst, ARMReg &rD, u8 &accessSize, bool &Store) } return true; } -const u8 *JitArm::BackPatch(u8 *codePtr, int accessType, u32 emAddress, void *ctx_void) +const u8 *JitArm::BackPatch(u8 *codePtr, u32 emAddress, void *ctx_void) { // TODO: This ctx needs to be filled with our information CONTEXT *ctx = (CONTEXT *)ctx_void; diff --git a/Source/Core/Core/Src/PowerPC/JitCommon/JitBackpatch.cpp b/Source/Core/Core/Src/PowerPC/JitCommon/JitBackpatch.cpp index c92d05d9d3..506435284e 100644 --- a/Source/Core/Core/Src/PowerPC/JitCommon/JitBackpatch.cpp +++ b/Source/Core/Core/Src/PowerPC/JitCommon/JitBackpatch.cpp @@ -160,7 +160,7 @@ const u8 *TrampolineCache::GetWriteTrampoline(const InstructionInfo &info) // 1) It's really necessary. We don't know anything about the context. // 2) It doesn't really hurt. Only instructions that access I/O will get these, and there won't be // that many of them in a typical program/game. -const u8 *Jitx86Base::BackPatch(u8 *codePtr, int accessType, u32 emAddress, void *ctx_void) +const u8 *Jitx86Base::BackPatch(u8 *codePtr, u32 emAddress, void *ctx_void) { #ifdef _M_X64 CONTEXT *ctx = (CONTEXT *)ctx_void; @@ -169,29 +169,15 @@ const u8 *Jitx86Base::BackPatch(u8 *codePtr, int accessType, u32 emAddress, void return 0; // this will become a regular crash real soon after this InstructionInfo info; - if (!DisassembleMov(codePtr, info, accessType)) { + if (!DisassembleMov(codePtr, &info)) { BackPatchError("BackPatch - failed to disassemble MOV instruction", codePtr, emAddress); } - /* - if (info.isMemoryWrite) { - if (!Memory::IsRAMAddress(emAddress, true)) { - PanicAlert("Exception: Caught write to invalid address %08x", emAddress); - return; - } - BackPatchError("BackPatch - determined that MOV is write, not yet supported and should have been caught before", - codePtr, emAddress); - }*/ - if (info.otherReg != RBX) PanicAlert("BackPatch : Base reg not RBX." "\n\nAttempted to access %08x.", emAddress); - - if (accessType == OP_ACCESS_WRITE) - PanicAlert("BackPatch : Currently only supporting reads." - "\n\nAttempted to write to %08x.", emAddress); - if (accessType == 0) + if (!info.isMemoryWrite) { XEmitter emitter(codePtr); int bswapNopCount; @@ -205,8 +191,11 @@ const u8 *Jitx86Base::BackPatch(u8 *codePtr, int accessType, u32 emAddress, void emitter.NOP((int)info.instructionSize + bswapNopCount - 5); return codePtr; } - else if (accessType == 1) + else { + PanicAlert("BackPatch : Currently only supporting reads." + "\n\nAttempted to write to %08x.", emAddress); + // TODO: special case FIFO writes. Also, support 32-bit mode. // Also, debug this so that it actually works correctly :P XEmitter emitter(codePtr - 2); diff --git a/Source/Core/Core/Src/PowerPC/JitCommon/JitBase.h b/Source/Core/Core/Src/PowerPC/JitCommon/JitBase.h index 826369e604..a5f7bd62be 100644 --- a/Source/Core/Core/Src/PowerPC/JitCommon/JitBase.h +++ b/Source/Core/Core/Src/PowerPC/JitCommon/JitBase.h @@ -74,7 +74,7 @@ public: virtual void Jit(u32 em_address) = 0; - virtual const u8 *BackPatch(u8 *codePtr, int accessType, u32 em_address, void *ctx) = 0; + virtual const u8 *BackPatch(u8 *codePtr, u32 em_address, void *ctx) = 0; virtual const CommonAsmRoutinesBase *GetAsmRoutines() = 0; @@ -89,7 +89,7 @@ protected: public: JitBlockCache *GetBlockCache() { return &blocks; } - const u8 *BackPatch(u8 *codePtr, int accessType, u32 em_address, void *ctx); + const u8 *BackPatch(u8 *codePtr, u32 em_address, void *ctx); bool IsInCodeSpace(u8 *ptr) { return IsInSpace(ptr); } }; diff --git a/Source/Core/Core/Src/PowerPC/JitInterface.cpp b/Source/Core/Core/Src/PowerPC/JitInterface.cpp index 1ca5626b1e..787ffebb6e 100644 --- a/Source/Core/Core/Src/PowerPC/JitInterface.cpp +++ b/Source/Core/Core/Src/PowerPC/JitInterface.cpp @@ -175,9 +175,9 @@ namespace JitInterface { return jit->IsInCodeSpace(ptr); } - const u8 *BackPatch(u8 *codePtr, int accessType, u32 em_address, void *ctx) + const u8 *BackPatch(u8 *codePtr, u32 em_address, void *ctx) { - return jit->BackPatch(codePtr, accessType, em_address, ctx); + return jit->BackPatch(codePtr, em_address, ctx); } void ClearCache() diff --git a/Source/Core/Core/Src/PowerPC/JitInterface.h b/Source/Core/Core/Src/PowerPC/JitInterface.h index bf5d750a1f..a0b7d62291 100644 --- a/Source/Core/Core/Src/PowerPC/JitInterface.h +++ b/Source/Core/Core/Src/PowerPC/JitInterface.h @@ -18,7 +18,7 @@ namespace JitInterface // Memory Utilities bool IsInCodeSpace(u8 *ptr); - const u8 *BackPatch(u8 *codePtr, int accessType, u32 em_address, void *ctx); + const u8 *BackPatch(u8 *codePtr, u32 em_address, void *ctx); // used by JIT to read instructions u32 Read_Opcode_JIT(const u32 _Address); diff --git a/Source/Core/Core/Src/x64MemTools.cpp b/Source/Core/Core/Src/x64MemTools.cpp index 2594598303..02b5abb809 100644 --- a/Source/Core/Core/Src/x64MemTools.cpp +++ b/Source/Core/Core/Src/x64MemTools.cpp @@ -99,7 +99,7 @@ LONG NTAPI Handler(PEXCEPTION_POINTERS pPtrs) //We could emulate the memory accesses here, but then they would still be around to take up //execution resources. Instead, we backpatch into a generic memory call and retry. - const u8 *new_rip = JitInterface::BackPatch(codePtr, accessType, emAddress, ctx); + const u8 *new_rip = JitInterface::BackPatch(codePtr, emAddress, ctx); // Rip/Eip needs to be updated. if (new_rip) @@ -214,7 +214,6 @@ void sigsegv_handler(int signal, siginfo_t *info, void *raw_context) // Backpatch time. // Seems we'll need to disassemble to get access_type - that work is probably // best done and debugged on the Windows side. - int access_type = 0; CONTEXT fake_ctx; @@ -225,7 +224,7 @@ void sigsegv_handler(int signal, siginfo_t *info, void *raw_context) fake_ctx.Eax = CREG_EAX(ctx); fake_ctx.Eip = CREG_EIP(ctx); #endif - const u8 *new_rip = jit->BackPatch(fault_instruction_ptr, access_type, em_address, &fake_ctx); + const u8 *new_rip = jit->BackPatch(fault_instruction_ptr, em_address, &fake_ctx); if (new_rip) { #ifdef _M_X64