From a89eb67124715e3b474e0871d4e565819b12b833 Mon Sep 17 00:00:00 2001 From: Peace-Maker Date: Sat, 18 Aug 2018 12:54:30 +0200 Subject: [PATCH] Fix detour of functions returning a float Floats are always returned in FPU register st0. Since the value in st0 doesn't matter in a pre-hook before the function was executed, don't try to save and restore the value of the FPU stack top for a pre-hook. Only replace st0 after a post hook. --- DynamicHooks/hook.cpp | 34 ++++++++++++++++++++++++++-------- DynamicHooks/hook.h | 4 ++-- dynhooks_sourcepawn.cpp | 2 +- 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/DynamicHooks/hook.cpp b/DynamicHooks/hook.cpp index a695266..3abf764 100644 --- a/DynamicHooks/hook.cpp +++ b/DynamicHooks/hook.cpp @@ -227,7 +227,7 @@ void* CHook::CreateBridge() masm.cmpb(r8_al, ReturnAction_Supercede); // Restore the previously saved registers, so any changes will be applied - Write_RestoreRegisters(masm); + Write_RestoreRegisters(masm, HOOKTYPE_PRE); masm.j(equal, &label_supercede); @@ -293,7 +293,7 @@ void* CHook::CreatePostCallback() Write_CallHandler(masm, HOOKTYPE_POST); // Restore the previously saved registers, so any changes will be applied - Write_RestoreRegisters(masm); + Write_RestoreRegisters(masm, HOOKTYPE_POST); // Save scratch registers that are used by GetReturnAddress static void* pEAX = NULL; @@ -337,16 +337,19 @@ void CHook::Write_CallHandler(sp::MacroAssembler& masm, HookType_t type) ReturnAction_t (__cdecl CHook::*HookHandler)(HookType_t) = &CHook::HookHandler; // Save the registers so that we can access them in our handlers - Write_SaveRegisters(masm); + Write_SaveRegisters(masm, type); + + // Align the stack to 16 bytes. + masm.subl(esp, 8); // Call the global hook handler masm.push(type); masm.push(intptr_t(this)); masm.call(ExternalAddress((void *&)HookHandler)); - masm.addl(esp, 8); + masm.addl(esp, 16); } -void CHook::Write_SaveRegisters(sp::MacroAssembler& masm) +void CHook::Write_SaveRegisters(sp::MacroAssembler& masm, HookType_t type) { ke::Vector vecRegistersToSave = m_pCallingConvention->GetRegisters(); for(size_t i = 0; i < vecRegistersToSave.length(); i++) @@ -394,7 +397,12 @@ void CHook::Write_SaveRegisters(sp::MacroAssembler& masm) // ======================================================================== // >> 80-bit FPU registers // ======================================================================== - case ST0: masm.fst32(Operand(ExternalAddress(m_pRegisters->m_st0->m_pAddress))); break; + case ST0: + // Don't mess with the FPU stack in a pre-hook. The float return is returned in st0, + // so only load it in a post hook to avoid writing back NaN. + if (type == HOOKTYPE_POST) + masm.fst32(Operand(ExternalAddress(m_pRegisters->m_st0->m_pAddress))); + break; //case ST1: masm.movl(tword_ptr_abs(Ptr(m_pRegisters->m_st1->m_pAddress)), st1); break; //case ST2: masm.movl(tword_ptr_abs(Ptr(m_pRegisters->m_st2->m_pAddress)), st2); break; //case ST3: masm.movl(tword_ptr_abs(Ptr(m_pRegisters->m_st3->m_pAddress)), st3); break; @@ -408,7 +416,7 @@ void CHook::Write_SaveRegisters(sp::MacroAssembler& masm) } } -void CHook::Write_RestoreRegisters(sp::MacroAssembler& masm) +void CHook::Write_RestoreRegisters(sp::MacroAssembler& masm, HookType_t type) { ke::Vector vecRegistersToSave = m_pCallingConvention->GetRegisters(); for (size_t i = 0; i < vecRegistersToSave.length(); i++) @@ -456,7 +464,17 @@ void CHook::Write_RestoreRegisters(sp::MacroAssembler& masm) // ======================================================================== // >> 80-bit FPU registers // ======================================================================== - case ST0: masm.fld32(Operand(ExternalAddress(m_pRegisters->m_st0->m_pAddress))); break; + case ST0: + if (type == HOOKTYPE_POST) { + // Replace the top of the FPU stack. + // Copy st0 to st0 and pop -> just pop the FPU stack. + masm.fstp(st0); + // Push a value to the FPU stack. + // TODO: Only write back when changed? Save full 80bits for that case. + // Avoid truncation of the data if it's unchanged. + masm.fld32(Operand(ExternalAddress(m_pRegisters->m_st0->m_pAddress))); + } + break; //case ST1: masm.movl(st1, tword_ptr_abs(Ptr(m_pRegisters->m_st1->m_pAddress))); break; //case ST2: masm.movl(st2, tword_ptr_abs(Ptr(m_pRegisters->m_st2->m_pAddress))); break; //case ST3: masm.movl(st3, tword_ptr_abs(Ptr(m_pRegisters->m_st3->m_pAddress))); break; diff --git a/DynamicHooks/hook.h b/DynamicHooks/hook.h index 8ee23ae..2ebe347 100644 --- a/DynamicHooks/hook.h +++ b/DynamicHooks/hook.h @@ -174,8 +174,8 @@ private: void Write_ModifyReturnAddress(sp::MacroAssembler& masm); void Write_CallHandler(sp::MacroAssembler& masm, HookType_t type); - void Write_SaveRegisters(sp::MacroAssembler& masm); - void Write_RestoreRegisters(sp::MacroAssembler& masm); + void Write_SaveRegisters(sp::MacroAssembler& masm, HookType_t type); + void Write_RestoreRegisters(sp::MacroAssembler& masm, HookType_t type); void* CreatePostCallback(); diff --git a/dynhooks_sourcepawn.cpp b/dynhooks_sourcepawn.cpp index 016f376..9892200 100644 --- a/dynhooks_sourcepawn.cpp +++ b/dynhooks_sourcepawn.cpp @@ -387,7 +387,7 @@ ReturnAction_t HandleDetour(HookType_t hookType, CHook* pDetour) } else if (pWrapper->returnType == ReturnType_Float) { - *(float *)tempRetBuf = *(float *)returnStruct->newResult; + *(float *)&tempRetBuf = *(float *)returnStruct->newResult; } else {