Skip to content
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

runtime: _rt0_amd64_windows_lib stack frame not aligned, violates Microsoft x64 ABI #41075

Closed
zhangyoufu opened this issue Aug 27, 2020 · 3 comments · Fixed by ferrmin/go#14
Closed
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-Windows
Milestone

Comments

@zhangyoufu
Copy link
Contributor

zhangyoufu commented Aug 27, 2020

The actual frame size of _rt0_amd64_windows_lib is 0x58 (incl. return address and saved rbp), which does not conform to Microsoft x64 ABI (align to 0x10).

// When building with -buildmode=(c-shared or c-archive), this
// symbol is called. For dynamic libraries it is called when the
// library is loaded. For static libraries it is called when the
// final executable starts, during the C runtime initialization
// phase.
// Leave space for four pointers on the stack as required
// by the Windows amd64 calling convention.
TEXT _rt0_amd64_windows_lib(SB),NOSPLIT,$0x48
MOVQ BP, 0x20(SP)
MOVQ BX, 0x28(SP)
MOVQ AX, 0x30(SP)
MOVQ CX, 0x38(SP)
MOVQ DX, 0x40(SP)
// Create a new thread to do the runtime initialization and return.
MOVQ _cgo_sys_thread_create(SB), AX
MOVQ $_rt0_amd64_windows_lib_go(SB), CX
MOVQ $0, DX
CALL AX
MOVQ 0x20(SP), BP
MOVQ 0x28(SP), BX
MOVQ 0x30(SP), AX
MOVQ 0x38(SP), CX
MOVQ 0x40(SP), DX
RET

I ran into this issue when I managed to run latest Go on Windows Server 2003 x64 (unsupported, yes I know). misc/cgo/testcarchive failed with runtime: failed to create new OS thread (22), because NtCreateThread does not accept an unaligned ThreadContext pointer (at 0xXXXXXX8).

Save and restore these registers looks strange for me. RBX is callee-saved, but we didn't tamper it. RAX/RCX/RDX are volatile, we don't have to save and restore them.

TEXT _rt0_amd64_windows_lib(SB),NOSPLIT|NOFRAME,$0
        // Create a new thread to do the runtime initialization and return.
        MOVQ    $_rt0_amd64_windows_lib_go(SB), CX
        MOVQ    $0, DX
        MOVQ    _cgo_sys_thread_create(SB), AX
        JMP     AX

This works for me. I don't think this change will break JNI usage, as mentioned in #30944.

@ianlancetaylor ianlancetaylor added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows labels Aug 27, 2020
@ianlancetaylor ianlancetaylor added this to the Go1.16 milestone Aug 27, 2020
zhangyoufu added a commit to zhangyoufu/go that referenced this issue Aug 29, 2020
@alexbrainman
Copy link
Member

I ran into this issue when I managed to run latest Go on Windows Server 2003 x64 (unsupported, yes I know). misc/cgo/testcarchive failed with runtime: failed to create new OS thread (22), because NtCreateThread does not accept an unaligned ThreadContext pointer (at 0xXXXXXX8).

Is there any way for me to reproduce this problem? Regardless, please send your change, if you like, and I will review it.

Save and restore these registers looks strange for me. RBX is callee-saved, but we didn't tamper it. RAX/RCX/RDX are volatile, we don't have to save and restore them.

We definitely need to leave 4 words on stack for AX, BX. DX and CX. I agree we don't need to restore BX. DX and CX. I am not familiar with BP here. But I suggest you leave code alone as is, just add code to align stack before CreateThread.

Thank you.

Alex

@aclements aclements added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Dec 8, 2020
@odeke-em
Copy link
Member

odeke-em commented Feb 5, 2021

Thank you for the report and suggestion for the change, @zhangyoufu! Thank you @alexbrainman for the response, and for imploring a CL; @zhangyoufu please go ahead, the tree for Go1.17 will open soon and it would be nice to have you as a contributor. Punting this to Go1.17.

@gopherbot
Copy link

Change https://golang.org/cl/295329 mentions this issue: runtime: fix stack alignment for Windows amd64 lib entry

zhangyoufu added a commit to zhangyoufu/go that referenced this issue Feb 23, 2021
Windows amd64 calling convention requires 16-bytes aligned
stack pointer. Before this patch, the real frame size is
0x48 (frame size) + 0x10 (frame pointer & return address),
which does not satisfy the alignment requirement.

_cgo_sys_thread_create eventually calls NtCreateThread,
which receives a pointer to a ThreadContext structure
allocated from (mis-aligned) stack, and may fail with
STATUS_DATATYPE_MISALIGNMENT on some implementations.

BP is saved/restored by prolog/epilog.
AX, CX, DX are volatile, no need to save and restore.

Fixes golang#41075
@golang golang locked and limited conversation to collaborators Mar 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-Windows
Projects
None yet
6 participants