Fix passing variant_t parameters + fix codegen stack on Win64#2117
Merged
psychonic merged 2 commits intoalliedmodders:masterfrom May 5, 2024
Merged
Fix passing variant_t parameters + fix codegen stack on Win64#2117psychonic merged 2 commits intoalliedmodders:masterfrom
psychonic merged 2 commits intoalliedmodders:masterfrom
Conversation
Fix wrong cond check for passing objects by value
psychonic
approved these changes
May 5, 2024
Member
|
Thanks |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This fixes
AcceptEntityInput,FireEntityOutput, and entity output hooks to work on Windows 64-bit. This also fixes a crash associated with using theTF2_DisguisePlayernative.This was tested on a TF2 Windows 64-bit server with a custom build of SourceMod built using this version of the SDK (though alliedmodders/hl2sdk#198 might work too). I didn't test on other games running on 64-bit at this time but hopefully should follow the same rules.
Explanation of fixes
FireOutputdetour no longer has to manually spread out thevariant_tstruct parameter.TF2's Disguise function takes 5 parameters in total (including
this) so the 5th parameter will be pushed onto the stack. However the initial code did not allocate enough stack space to store the 5th parameter, causing the code to overwrite part of the stack outside of the allocated stack space. Initially, the assembly for calling the function looked like this:After forcing
g_StackUsageto be 32 onJIT_CallCompile's first pass, the generated code now allocates enough space:The x64 convention states that objects not of size 1, 2, 4, or 8 bytes must be passed by reference. Yet the check to pass an object by value or reference is incorrect.
sourcemod/extensions/bintools/jit_call_x64.cpp
Line 790 in e262064
I'm not sure why it was made this way (though an explanation would be welcome). This made it so that only structures of 1, 2, 4, 8, 16, and 32 bytes to be passed by reference which makes no sense. The initial version caused the following codegen for calling
AcceptInput:Correcting the check yields the following codegen:
; Write_Execution_Prologue push rbp mov rbp, rsp ; return buffer push r14 mov r14, rdx ; arg stack push rbx mov rbx, rcx push r15 mov r15, rsp and rsp, 0FFFFFFFFFFFFFFF0h sub rsp, 40h ; Write_PushThisPtr mov rcx, qword ptr [rbx] ; Write_PushPOD param_1 mov rdx, qword ptr [rbx+8] ; Write_PushPOD param_2 mov r8, qword ptr [rbx+10h] ; Write_PushPOD param_3 mov r9, qword ptr [rbx+18h] ; Write_PushObject param_4 +lea rax, [rbx+20h] mov qword ptr [rsp+20h], rax ; Write_PushPOD param_5 mov r10d, dword ptr [rbx+34h] mov qword ptr [rsp+28h], r10 ; Write_CallFunction (vtable) mov r10, qword ptr [rcx] mov r11, qword ptr [r10+130h] call r11 ; Write_RectifyStack add rsp, 40h ; Write_MovRet2Buf mov byte ptr [r14], al ; Write_Function_Epilogue mov rsp, r15 pop r15 pop rbx pop r14 mov rsp, rbp pop rbp ret