Skip to content

Conversation

@divergentdave
Copy link
Contributor

This PR moves the panic payload storage from the Machine state to per-thread state. Prior to this change, if one thread panicked while another was still unwinding, Miri would fail with thread 'rustc' panicked at 'the panic runtime should avoid double-panics', src/shims/panic.rs:51:9. I ran into this issue while prototyping a round-robin scheduler, but it's also reachable with the current scheduler and contrived programs that use blocking API calls to cause thread switching during unwinding. I wrote a test case along those lines for this change.

@RalfJung
Copy link
Member

Thanks, this is a great catch!

I am wondering now, presumably last_error should also be per-thread?

@divergentdave
Copy link
Contributor Author

Good call about last_error, I tacked that on in 4b3af72. (since it would cause merge conflicts if separated)

if let Some(errno_place) = this.active_thread_ref().last_error {
Ok(errno_place)
} else {
let errno_layout = this.machine.layouts.u32;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let errno_layout = this.machine.layouts.u32;
// Allocate new place, set initial value to 0.
let errno_layout = this.machine.layouts.u32;

@RalfJung
Copy link
Member

RalfJung commented Sep 2, 2020

This looks great, thanks. :) Please fix the last comment nit and squash the "Commit missing new file" commit, then we can land this.

@RalfJung
Copy link
Member

RalfJung commented Sep 3, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Sep 3, 2020

📌 Commit a6746ad has been approved by RalfJung

@bors
Copy link
Contributor

bors commented Sep 3, 2020

⌛ Testing commit a6746ad with merge c28a8ee...

@bors
Copy link
Contributor

bors commented Sep 3, 2020

☀️ Test successful - checks-travis, status-appveyor
Approved by: RalfJung
Pushing c28a8ee to master...

@bors bors merged commit c28a8ee into rust-lang:master Sep 3, 2020
@divergentdave divergentdave deleted the thread-panic-payload branch October 26, 2020 21:49
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