-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Reduce the executable code size. #191
base: main
Are you sure you want to change the base?
Conversation
… two index tables are used to access the actual entities.
@jaykrell I think you were working on this code in the recent past? Do you have any thoughts on this? |
It is a micro optimization but yes makes sense just from the description. |
return (this->*pEntry->pfCopy)(pEntry, pbDst + 1, pbSrc + 1); | ||
} | ||
|
||
PBYTE CDetourDis::Copy0F78(REFCOPYENTRY, PBYTE pbDst, PBYTE pbSrc) | ||
{ | ||
// vmread, 66/extrq, F2/insertq | ||
|
||
static const COPYENTRY vmread = /* 78 */ ENTRY_CopyBytes2Mod; | ||
static const COPYENTRY extrq_insertq = /* 78 */ ENTRY_CopyBytes4; | ||
const BYTE vmread = /* 78 */ eENTRY_CopyBytes2Mod; |
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.
enum instead of byte?
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.
Eh, byte is maybe best and certainly works.
src/disasm.cpp
Outdated
@@ -660,7 +654,7 @@ PBYTE CDetourDis::CopyF6(REFCOPYENTRY pEntry, PBYTE pbDst, PBYTE pbSrc) | |||
|
|||
// TEST BYTE /0 | |||
if (0x00 == (0x38 & pbSrc[1])) { // reg(bits 543) of ModR/M == 0 | |||
static const COPYENTRY ce = /* f6 */ ENTRY_CopyBytes2Mod1; | |||
const COPYENTRY ce = /* f6 */ s_rceCopyMap[eENTRY_CopyBytes2Mod1]; |
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.
reference or pointer instead of value?
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.
Of course, I will make another commit.
/* 03 */ ENTRY_CopyBytes2Mod, // ADD /r | ||
/* 04 */ ENTRY_CopyBytes2, // ADD ib | ||
/* 05 */ ENTRY_CopyBytes3Or5, // ADD iw | ||
/* 00 */ eENTRY_CopyBytes2Mod, // ADD /r |
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.
You might rephrase the PR with the same intent and even resulting bytes, but without changing these lines. i.e. just to make the diff easier to read.
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.
Alternatively, someone else might try to make the same change and verify they get the same result. I understand, you just prepended 'e' to every identifer, over 512 lines, it is just hard to verify.
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.
This is what diff -ur
or git diff
gave, although less straight. Please let me know how to make this more human readable.
I used a regex to generate the eENTRY_* definitions (value/name pair) from the original ones.
I should point out. I changed all of this code so the main handler function is templatized, and the table is filled with (mostly?) instantiated pointers to it. So that ends up around 512*8=2k and perhaps faster, perhaps a little larger. |
Any updates on this? |
- Deduplicate code - Reduce constant table size (see also microsoft#191 by jdp1024)
- Deduplicate code and fix formatting - Reduce constant table size (see also microsoft#191 by jdp1024)
In the original version of Detours, s_rceCopyTable and s_rceCopyTable0F are two big arrays, both consisting of 256 COPYENTRY's, occupying 256*(8+8)*2=8192 bytes. To reduce the executable code size, those two arrays are changed to use the index to a COPYENTRY array instead of the actual entity.
s_rceCopyTable and s_rceCopyTable0F now are arrays of 256 BYTEs, the COPYENTRY array consists of only the 45 unique operands. All these occupy 2562+45(8+8)=1232 bytes, thus we save 7060 bytes with almost no performance sacrifice.
Microsoft Reviewers: Open in CodeFlow