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

[libtorrent] fix build error on windows arm64 target #29626

Merged
merged 2 commits into from
Feb 16, 2023

Conversation

wbsdty331
Copy link
Contributor

fixes: #17320
libtorrent only focus AMD64 and i386 platform, when build for arm64 it will throw error. This pr fix it.

reference: arvidn/libtorrent#7313

If upstream merge my pr and release new version, I will delete this path from vcpkg repoistry.

@wbsdty331 wbsdty331 changed the title fix build error on windows arm64 target [libtorrent] fix build error on windows arm64 target Feb 13, 2023
@LilyWangLL LilyWangLL added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Feb 14, 2023
@wbsdty331 wbsdty331 marked this pull request as ready for review February 14, 2023 03:51
@LilyWangLL LilyWangLL added the info:reviewed Pull Request changes follow basic guidelines label Feb 15, 2023
+ stack_frame.AddrPC.Offset = context_record.Pc;
+ stack_frame.AddrFrame.Offset = context_record.Fp;
+ stack_frame.AddrStack.Offset = context_record.Sp;
#endif
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you give some more details as to why this patch fixes / enables arm64 support?

Copy link
Contributor Author

@wbsdty331 wbsdty331 Feb 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to build arm64 libtorrent with MSVC arm64 build toolchin and VS2022,but I got an error with assert.cpp.
let me see the original code:

#if defined(_WIN64)
	int const machine_type = IMAGE_FILE_MACHINE_AMD64;
	stack_frame.AddrPC.Offset = context_record.Rip;
	stack_frame.AddrFrame.Offset = context_record.Rbp;
	stack_frame.AddrStack.Offset = context_record.Rsp;
#else
	int const machine_type = IMAGE_FILE_MACHINE_I386;
	stack_frame.AddrPC.Offset = context_record.Eip;
	stack_frame.AddrFrame.Offset = context_record.Ebp;
	stack_frame.AddrStack.Offset = context_record.Esp;
#endif

from MSDN Predefined preprocessor macros we can know:

_WIN64 Defined as 1 when the compilation target is 64-bit ARM or x64. Otherwise, undefined.

original code only focused on i386 and x86_64 platform, but stackframe have dependencies on different CPU registers (Rip rbp Ebp and so on),

It means arm64 architectures in this define is also true. for x86_64 cpu, the code is ok, but arm64 processor don't have Rip Rbp and Rsp registers, and MACHINE_TYPE is also wrong, so you must use arm64 CPU register and MACHINE_TYPE to use stackframe.

ref: https://learn.microsoft.com/en-us/cpp/build/arm64-windows-abi-conventions?view=msvc-170

@JavierMatosD JavierMatosD added requires:author-response info:reviewed Pull Request changes follow basic guidelines and removed info:reviewed Pull Request changes follow basic guidelines requires:author-response labels Feb 15, 2023
@JavierMatosD
Copy link
Contributor

Thank you for the explanation! :)

@JavierMatosD JavierMatosD merged commit 70cc9c5 into microsoft:master Feb 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[libtorrent] build failure
3 participants