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

Rust 1.81.0 causes SIGABRT in test environment #1332

Closed
leighmcculloch opened this issue Sep 10, 2024 · 1 comment · Fixed by #1333
Closed

Rust 1.81.0 causes SIGABRT in test environment #1332

leighmcculloch opened this issue Sep 10, 2024 · 1 comment · Fixed by #1333
Assignees
Labels
bug Something isn't working

Comments

@leighmcculloch
Copy link
Member

What version are you using?

v21.6.0

What did you do?

Run the test_auth crate's tests that are in this repo.

What did you expect to see?

Passing tests as occurs on Rust 1.80.0.

What did you see instead?

thread caused non-unwinding panic. aborting.
Caused by:
  process didn't exit successfully: `/Users/leighmcculloch/Code/rs-soroban-sdk/target/debug/deps/soroban_sdk-a1f019b2dff9103a` (signal: 6, SIGABRT: process abort signal)
@leighmcculloch leighmcculloch added the bug Something isn't working label Sep 10, 2024
@leighmcculloch leighmcculloch self-assigned this Sep 10, 2024
@leighmcculloch
Copy link
Member Author

Fix is at:

github-merge-queue bot pushed a commit that referenced this issue Sep 10, 2024
### What

Fixes for Rust 1.81:

1. Update semver-checks to 0.35.0.

2. Don't use the `extern` function for invoking a contract function when
executing tests.

### Why

1. Semver checks is failing to run on the new version of Rust and needs
updating.

2. Rust 1.81.0 changed the behavior of panics on `extern "C"` functions,
such that panics cannot be caught by
[std::panic::catch_unwind](https://doc.rust-lang.org/std/panic/fn.catch_unwind.html).

Contracts expose their functions as an `extern` function that defaults
to `extern "C"`. The test environment catches panics inside contract
functions and resurfaces them as errors by using catch_unwind.

The solution used was to make it so that the test environment never
calls through the extern function, and instead calls through a
non-extern function that does the same work. This seems like the sanest
way to take away any dependence / relationship between the test
environment and the Rust C-ABI so that any future changes to the C-ABI
don't affect the test environment.

An alternative solution would be to mark the contract function as
`extern "C-unwind"` so that it retained unwind functionality, but that
would continue to bind the SDK test environment to behaviour changes in
extern fns.

In the solution in this change the Abi qualifier `"C"` has been added.
When the Abi qualifier is not specified it defaults to `"C"`, but for
the sake of being explicit and removing any doubt from a reader it is
now specified. The Rust reference confirms that `"C"` is the default:
-
https://doc.rust-lang.org/reference/items/functions.html#extern-function-qualifier.
   
   More details on this change to Rust 1.81.0 can be found at:
-
https://blog.rust-lang.org/2024/09/05/Rust-1.81.0.html#abort-on-uncaught-panics-in-extern-c-functions
   - rust-lang/rust#74990
    
   Close #1332
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant