Skip to content

Add __chkstk for arm64ec-pc-windows-msvc #812

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

codeOverFlow
Copy link

This is my first PR so I tried one that did not looked overly complicated.

This is base on llvm/llvm-project@946a81f

As this is my first PR fill free to fire all mistake you can spot :)

I am also not sure for the #[cfg] gate I put.

@tgross35
Copy link
Contributor

tgross35 commented Apr 7, 2025

I’ll take a look at the implementation soon, do you have a repo that gets linker errors without this function?

@beetrees
Copy link
Contributor

beetrees commented Apr 8, 2025

@codeOverFlow
Copy link
Author

codeOverFlow commented Apr 8, 2025 via email

@tgross35
Copy link
Contributor

tgross35 commented Apr 8, 2025

Could you clarify whether this is attempting to solve a problem you have experienced (e.g. linker error) or just adding an implementation that is missing?

We don't really want to add these implementations if Rust never emits them, but it's possible that is happening in circumstances we aren't yet testing. thumbv7a-pc-windows-msvc and thumbv7a-uwp-windows-msvc are the only Windows targets for 32-bit Arm and arm64ec-pc-windows-msvc is the only arm64ec target; I'm not sure that either of these need __chkstk or __chkstk_arm64ec, but maybe they do for #![no_std] binaries? Cc @bdbaia and @dpaoliello (listed target maintainers) in case you have any ideas.

If it's possible to create a linker error looking for these symbols then we can add them, but otherwise we could skip it and just update README to mention these in the "Unimplemented functions" section.

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.

3 participants