-
-
Notifications
You must be signed in to change notification settings - Fork 424
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add better support for detouring at instruction addresses with DHooks #1969
Open
chrb22
wants to merge
14
commits into
alliedmodders:master
Choose a base branch
from
chrb22:dhooks-instruction
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
c1a5bf9
Make post-hook creation optional
chrb22 154ceb2
Add instruction "calling convention"
chrb22 45020bd
Add errors to instruction hook setup
chrb22 7ec67d9
Initialize `m_pNewRetAddr` as `NULL`
chrb22 657b3dd
Remove wrongly created error check
chrb22 42b1e5e
Add checks for instruction hooks in gamedata parsing
chrb22 e906917
Add parameter check back
chrb22 19f15c0
Save all registers in instruction hooks
chrb22 ddb9066
Implement `x86Instruction::GetArgRegisterSize` properly
chrb22 e0e3cfb
Add ability to supercede instruction hook
chrb22 abba523
Add jump target warning to include file
chrb22 eb780f0
Add constructor for `SignatureWrapper`
chrb22 5adeef8
Add trailing commas to `dhooks.inc` enums
chrb22 1799c7e
Lower DHooks jump patch size to 5 bytes
chrb22 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
121 changes: 121 additions & 0 deletions
121
extensions/dhooks/DynamicHooks/conventions/x86Instruction.cpp
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,121 @@ | ||
/** | ||
* ============================================================================= | ||
* DynamicHooks | ||
* Copyright (C) 2015 Robin Gohmert. All rights reserved. | ||
* Copyright (C) 2018-2021 AlliedModders LLC. All rights reserved. | ||
* ============================================================================= | ||
* | ||
* This software is provided 'as-is', without any express or implied warranty. | ||
* In no event will the authors be held liable for any damages arising from | ||
* the use of this software. | ||
* | ||
* Permission is granted to anyone to use this software for any purpose, | ||
* including commercial applications, and to alter it and redistribute it | ||
* freely, subject to the following restrictions: | ||
* | ||
* 1. The origin of this software must not be misrepresented; you must not | ||
* claim that you wrote the original software. If you use this software in a | ||
* product, an acknowledgment in the product documentation would be | ||
* appreciated but is not required. | ||
* | ||
* 2. Altered source versions must be plainly marked as such, and must not be | ||
* misrepresented as being the original software. | ||
* | ||
* 3. This notice may not be removed or altered from any source distribution. | ||
* | ||
* asm.h/cpp from devmaster.net (thanks cybermind) edited by pRED* to handle gcc | ||
* -fPIC thunks correctly | ||
* | ||
* Idea and trampoline code taken from DynDetours (thanks your-name-here). | ||
* | ||
* Adopted to provide similar features to SourceHook by AlliedModders LLC. | ||
*/ | ||
|
||
// ============================================================================ | ||
// >> INCLUDES | ||
// ============================================================================ | ||
#include "x86Instruction.h" | ||
#include <string.h> | ||
|
||
|
||
// ============================================================================ | ||
// >> x86Instruction | ||
// ============================================================================ | ||
x86Instruction::x86Instruction(std::vector<DataTypeSized_t> &vecArgTypes, DataTypeSized_t returnType, int iAlignment) : | ||
ICallingConvention(vecArgTypes, returnType, iAlignment) | ||
{ | ||
} | ||
|
||
x86Instruction::~x86Instruction() | ||
{ | ||
} | ||
|
||
std::vector<Register_t> x86Instruction::GetRegisters() | ||
{ | ||
std::vector<Register_t> registers = | ||
{ | ||
AL, CL, DL, BL, AH, CH, DH, BH, | ||
EAX, ECX, EDX, EBX, ESP, EBP, ESI, EDI, | ||
XMM0, XMM1, XMM2, XMM3, XMM4, XMM5, XMM6, XMM7, | ||
ST0 | ||
}; | ||
|
||
return registers; | ||
} | ||
|
||
int x86Instruction::GetPopSize() | ||
{ | ||
return 0; | ||
} | ||
|
||
int x86Instruction::GetArgStackSize() | ||
{ | ||
return 0; | ||
} | ||
|
||
void** x86Instruction::GetStackArgumentPtr(CRegisters* pRegisters) | ||
{ | ||
return NULL; | ||
} | ||
|
||
int x86Instruction::GetArgRegisterSize() | ||
{ | ||
int iArgRegisterSize = 0; | ||
|
||
for (size_t i = 0; i < m_vecArgTypes.size(); i++) | ||
{ | ||
if (m_vecArgTypes[i].custom_register != None) | ||
iArgRegisterSize += m_vecArgTypes[i].size; | ||
} | ||
|
||
return iArgRegisterSize; | ||
} | ||
|
||
void* x86Instruction::GetArgumentPtr(unsigned int iIndex, CRegisters* pRegisters) | ||
{ | ||
if (iIndex >= m_vecArgTypes.size()) | ||
return NULL; | ||
|
||
// Check if this argument wasn't passed in a register. | ||
if (m_vecArgTypes[iIndex].custom_register == None) | ||
return NULL; | ||
|
||
CRegister *pRegister = pRegisters->GetRegister(m_vecArgTypes[iIndex].custom_register); | ||
if (!pRegister) | ||
return NULL; | ||
|
||
return pRegister->m_pAddress; | ||
} | ||
|
||
void x86Instruction::ArgumentPtrChanged(unsigned int iIndex, CRegisters* pRegisters, void* pArgumentPtr) | ||
{ | ||
} | ||
|
||
void* x86Instruction::GetReturnPtr(CRegisters* pRegisters) | ||
{ | ||
return NULL; | ||
} | ||
|
||
void x86Instruction::ReturnPtrChanged(CRegisters* pRegisters, void* pReturnPtr) | ||
{ | ||
} |
68 changes: 68 additions & 0 deletions
68
extensions/dhooks/DynamicHooks/conventions/x86Instruction.h
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
/** | ||
* ============================================================================= | ||
* DynamicHooks | ||
* Copyright (C) 2015 Robin Gohmert. All rights reserved. | ||
* Copyright (C) 2018-2021 AlliedModders LLC. All rights reserved. | ||
* ============================================================================= | ||
* | ||
* This software is provided 'as-is', without any express or implied warranty. | ||
* In no event will the authors be held liable for any damages arising from | ||
* the use of this software. | ||
* | ||
* Permission is granted to anyone to use this software for any purpose, | ||
* including commercial applications, and to alter it and redistribute it | ||
* freely, subject to the following restrictions: | ||
* | ||
* 1. The origin of this software must not be misrepresented; you must not | ||
* claim that you wrote the original software. If you use this software in a | ||
* product, an acknowledgment in the product documentation would be | ||
* appreciated but is not required. | ||
* | ||
* 2. Altered source versions must be plainly marked as such, and must not be | ||
* misrepresented as being the original software. | ||
* | ||
* 3. This notice may not be removed or altered from any source distribution. | ||
* | ||
* asm.h/cpp from devmaster.net (thanks cybermind) edited by pRED* to handle gcc | ||
* -fPIC thunks correctly | ||
* | ||
* Idea and trampoline code taken from DynDetours (thanks your-name-here). | ||
* | ||
* Adopted to provide similar features to SourceHook by AlliedModders LLC. | ||
*/ | ||
|
||
#ifndef _X86_INSTRUCTION_H | ||
#define _X86_INSTRUCTION_H | ||
|
||
// ============================================================================ | ||
// >> INCLUDES | ||
// ============================================================================ | ||
#include "../convention.h" | ||
|
||
|
||
// ============================================================================ | ||
// >> CLASSES | ||
// ============================================================================ | ||
class x86Instruction: public ICallingConvention | ||
{ | ||
public: | ||
x86Instruction(std::vector<DataTypeSized_t> &vecArgTypes, DataTypeSized_t returnType, int iAlignment=4); | ||
virtual ~x86Instruction(); | ||
|
||
virtual std::vector<Register_t> GetRegisters(); | ||
virtual int GetPopSize(); | ||
virtual int GetArgStackSize(); | ||
virtual void** GetStackArgumentPtr(CRegisters* pRegisters); | ||
virtual int GetArgRegisterSize(); | ||
|
||
virtual void* GetArgumentPtr(unsigned int iIndex, CRegisters* pRegisters); | ||
virtual void ArgumentPtrChanged(unsigned int iIndex, CRegisters* pRegisters, void* pArgumentPtr); | ||
|
||
virtual void* GetReturnPtr(CRegisters* pRegisters); | ||
virtual void ReturnPtrChanged(CRegisters* pRegisters, void* pReturnPtr); | ||
|
||
private: | ||
void* m_pReturnBuffer; | ||
}; | ||
|
||
#endif // _X86_INSTRUCTION_H |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be useful if you could skip instructions similar to the "supercede" mode for function calls.
Imagine a mid-function detour on something like
mov eax, 0x1000
where you'd want to change the constant. If you add a detour, change eax, and return, the original instruction would still be executed and your detour had no effect.I guess you can work around this by detouring after that instruction for now, but that could force you into jump target territory for no reason. I guess we can improve that in an additional PR. This feature is pretty advanced and you should be knowing what you're doing when going this way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't even consider jump targets. Having a post routine would improve the situation, but it won't solve all cases as you mentioned yourself:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, don't really need a whole post routine for a single instruction, just a supercede implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess that would just be conditionally replacing the
m_pTrampoline
jump with am_pTrampoline + instruction size
jump?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay that does seem to work. One problem is that you can't change params and supercede at the same time currently. Should I add
MRES_ChangedSupercede = -3
toenum MRESReturn
and implement that? Or just have it update params automatically when usingMRES_Supercede
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thought about changing
MRESReturn
based on what you said about skipping a specified number of bytesDoes returning
MRES_Handled
actually do something different to returningMRES_Ignored
?