Skip to content

FFI panic is UB #26761

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

Merged
merged 1 commit into from
Jul 7, 2015
Merged

FFI panic is UB #26761

merged 1 commit into from
Jul 7, 2015

Conversation

steveklabnik
Copy link
Member

I incorrectly stated that it's an abort.

r? @gankro

@Gankra
Copy link
Contributor

Gankra commented Jul 3, 2015

r? @alexcrichton

@rust-highfive rust-highfive assigned alexcrichton and unassigned Gankra Jul 3, 2015
@Gankra
Copy link
Contributor

Gankra commented Jul 3, 2015

(lgtm but alex is the expert -- though I of course favour using Scare Caps for Undefined Behaviour)

@@ -544,8 +545,8 @@ pub extern fn oh_no() -> ! {
# fn main() {}
```

If you’re writing code that may panic, you should run it in another thread,
so that the panic doesn’t bubble up to C:
But it could do anything. If you’re writing code that may panic, you should run
Copy link
Member

Choose a reason for hiding this comment

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

I find this kinda confusing that this starts out by saying that the code could abort but then follows it up by saying it could do anything instead. Overall there's not a great thing we can say here without a stable catch_panic. I think the core truth here is that unwinding just shouldn't cross FFI boundaries, and the way to do that is catch_panic, once stabilized.

@steveklabnik
Copy link
Member Author

@alexcrichton updated

I incorrectly stated that it's an abort.
@alexcrichton
Copy link
Member

@bors: r+ ee43c5e rollup

steveklabnik added a commit to steveklabnik/rust that referenced this pull request Jul 7, 2015
…chton

I incorrectly stated that it's an abort.

r? @gankro
bors added a commit that referenced this pull request Jul 7, 2015
@bors bors merged commit ee43c5e into rust-lang:master Jul 7, 2015
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.

4 participants