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

Update rust ffi code to use C-unwind #3151

Closed
stevenengler opened this issue Sep 7, 2023 · 1 comment
Closed

Update rust ffi code to use C-unwind #3151

stevenengler opened this issue Sep 7, 2023 · 1 comment
Labels
Type: Bug Error or flaw producing unexpected results

Comments

@stevenengler
Copy link
Contributor

Most of our rust-C ffi code uses pub extern "C" fn, but these functions are not allowed by rust to panic (a panic is not allowed to cross the ffi boundary). While Shadow does do this and this is technically UB, it has seemed to work fine for us. But this may break in the future, so we should switch to pub extern "C-unwind" fn instead.

See:

@stevenengler stevenengler added the Type: Bug Error or flaw producing unexpected results label Sep 7, 2023
@stevenengler
Copy link
Contributor Author

I'm going to close this since in #3210 it's good enough for now. The long-term goal is to get rid of the C code, and if a new rust version in the future causes issues we can revisit. Shadow already seems to SIGABORT when panicking so our unwinding might not be working correctly anyways.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Error or flaw producing unexpected results
Projects
None yet
Development

No branches or pull requests

1 participant