Skip to content

Add -unwind versions of the hooks. #64

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 2 commits into
base: master
Choose a base branch
from

Conversation

xNxExOx
Copy link

@xNxExOx xNxExOx commented Mar 15, 2025

And remove unused use in src/arch/x86/trampoline/mod.rs,

@Hpmason
Copy link
Owner

Hpmason commented Mar 15, 2025

Thanks for this PR. Based off the bit I've read, I appreciate reducing any UB in this crate. I haven't been following the nightly feature for this change, I was just wondering if you knew the status of it? Is this a feature that is coming in stable soon?

This is probably something we still want to put under a new feature flag (e.g. unwind-abis) just to keep a default lower MSRV. Plus, if it's gonna be a nightly for a longer or an unknown period of time, it'll also avoid breaking in new nightly versions for people who don't need to opt into it.

@xNxExOx
Copy link
Author

xNxExOx commented Mar 15, 2025

Oh that is actually a good point, for the project where I use it I am (and will be 😢) on nightly, but I was under impression, that this reached stable many moths ago, but I did not really checked that.

It was merged in 2021: Implement RFC 2945: "C-unwind" ABI #76570 , so it exists for quite some time.

And it is available just fine on stable playground

The default behavior changed at some point between nightly-2024-05-01-i686-pc-windows-msvc and nightly-2024-09-01-i686-pc-windows-msvc, that *-unwind are required for C++ exceptions to not cause crash. This was one of the breaking changes I needed to fix, in order to successfully update.

I can easily gate changes in src/macros.rs, but I do not know, how to easily gate changes in src/arch/x86/mod.rs, without duplicating the functions.

@Hpmason
Copy link
Owner

Hpmason commented Mar 15, 2025

Oh looks like the extern *-unwinds were added in 1.71 and finalized in 1.81 fully completed the transition (based off rust blog release notes). So it should be good to add without nightly.

I would still feature flag gate it since the MSRV of this crate is still 1.60. (I'll probably increase that in a newer crate release, but for now we can put it behind a feature.)

As for src/arch/x86/mod.rs, you can revert those changes since I think they only apply to tests, which they don't panic or call C++ code, so I don't see a big reason to update those tests or duplicate them.

@xNxExOx xNxExOx changed the title Add -unwind versions of the hooks, and make them helper functions -unwind too. Add -unwind versions of the hooks. Mar 16, 2025
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