Skip to content

Fix passing variant_t parameters + fix codegen stack on Win64#2117

Merged
psychonic merged 2 commits intoalliedmodders:masterfrom
KitRifty:x64-fix-shadow-space-alloc
May 5, 2024
Merged

Fix passing variant_t parameters + fix codegen stack on Win64#2117
psychonic merged 2 commits intoalliedmodders:masterfrom
KitRifty:x64-fix-shadow-space-alloc

Conversation

@KitRifty
Copy link
Contributor

This fixes AcceptEntityInput, FireEntityOutput, and entity output hooks to work on Windows 64-bit. This also fixes a crash associated with using the TF2_DisguisePlayer native.

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

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:

; Write_Execution_Prologue
push    rbp
mov     rbp, rsp
; arg stack
push    rbx
mov     rbx, rcx
push    r15
mov     r15, rsp
and     rsp, 0FFFFFFFFFFFFFFF0h
sub     rsp, 10h

; Write_PushThisPtr
mov     rcx, qword ptr [rbx]

; Write_PushPOD param_1
mov     edx, dword ptr [rbx+8]

; Write_PushPOD param_2
mov     r8d, dword ptr [rbx+0Ch]

; Write_PushPOD param_3
mov     r9, qword ptr [rbx+10h]

; Write_PushPOD param_4
movzx   rax, byte ptr [rbx+18h]
mov     qword ptr [rsp+20h], rax    ; <-- overran parameter space

; Write_CallFunction (direct)
mov     rax, 7FFF62C4EC20h
call    rax

; Write_RectifyStack
add     rsp, 10h

; Write_Function_Epilogue
mov     rsp, r15
pop     r15
pop     rbx
mov     rsp, rbp
pop     rbp
ret 

After forcing g_StackUsage to be 32 on JIT_CallCompile's first pass, the generated code now allocates enough space:

; Write_Execution_Prologue
push    rbp
mov     rbp, rsp
; arg stack
push    rbx
mov     rbx, rcx
push    r15
mov     r15, rsp
and     rsp, 0FFFFFFFFFFFFFFF0h
-sub     rsp, 10h
+sub     rsp, 30h

; Write_PushThisPtr
mov     rcx, qword ptr [rbx]

; Write_PushPOD param_1
mov     edx, dword ptr [rbx+8]

; Write_PushPOD param_2
mov     r8d, dword ptr [rbx+0Ch]

; Write_PushPOD param_3
mov     r9, qword ptr [rbx+10h]

; Write_PushPOD param_4
movzx   rax, byte ptr [rbx+18h]
mov     qword ptr [rsp+20h], rax

; Write_CallFunction (direct)
mov     rax, 7FFF62C4EC20h
call    rax

; Write_RectifyStack
-add     rsp, 10h
+add     rsp, 30h

; Write_Function_Epilogue
mov     rsp, r15
pop     r15
pop     rbx
mov     rsp, rbp
pop     rbp
ret 
  • Fixed incorrect handling of objects passed by value.

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.

if (info->size < 64 && (info->size & (info->size - 1)) == 0)

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:

; 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
mov     qword ptr [rsp+20h], rax    ; <-- rax is undefined

; 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   

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   

@psychonic
Copy link
Member

Thanks

@psychonic psychonic merged commit f08f1ef into alliedmodders:master May 5, 2024
@KitRifty KitRifty deleted the x64-fix-shadow-space-alloc branch May 5, 2024 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants