Skip to content

Commit

Permalink
Fix coroutine validation for mixed panic strategy
Browse files Browse the repository at this point in the history
Validation introduced in #113124 allows UnwindAction::Continue and
TerminatorKind::Resume to occur only in functions with ABI that can
unwind. The function ABI depends on the panic strategy, which can vary
across crates.

Usually MIR is built and validated in the same crate. The coroutine drop
glue thus far was an exception. As a result validation could fail when
mixing different panic strategies.

Avoid the problem by executing AbortUnwindingCalls along with the
validation.
  • Loading branch information
tmiasko committed Nov 28, 2023
1 parent 5facb42 commit 5161b22
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 12 deletions.
10 changes: 0 additions & 10 deletions compiler/rustc_mir_transform/src/coroutine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@
//! Otherwise it drops all the values in scope at the last suspension point.
use crate::abort_unwinding_calls;
use crate::add_call_guards;
use crate::deref_separator::deref_finder;
use crate::errors;
use crate::pass_manager as pm;
Expand Down Expand Up @@ -1168,18 +1167,9 @@ fn create_coroutine_drop_shim<'tcx>(
simplify::remove_dead_blocks(&mut body);

// Update the body's def to become the drop glue.
// This needs to be updated before the AbortUnwindingCalls pass.
let coroutine_instance = body.source.instance;
let drop_in_place = tcx.require_lang_item(LangItem::DropInPlace, None);
let drop_instance = InstanceDef::DropGlue(drop_in_place, Some(coroutine_ty));
body.source.instance = drop_instance;

pm::run_passes_no_validate(
tcx,
&mut body,
&[&abort_unwinding_calls::AbortUnwindingCalls, &add_call_guards::CriticalCallEdges],
None,
);

// Temporary change MirSource to coroutine's instance so that dump_mir produces more sensible
// filename.
Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_mir_transform/src/shim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,13 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> Body<'
let mut body = EarlyBinder::bind(body.clone()).instantiate(tcx, args);
debug!("make_shim({:?}) = {:?}", instance, body);

// Run empty passes to mark phase change and perform validation.
pm::run_passes(
tcx,
&mut body,
&[],
&[
&abort_unwinding_calls::AbortUnwindingCalls,
&add_call_guards::CriticalCallEdges,
],
Some(MirPhase::Runtime(RuntimePhase::Optimized)),
);

Expand Down
11 changes: 11 additions & 0 deletions tests/ui/coroutine/auxiliary/unwind-aux.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// compile-flags: -Cpanic=unwind --crate-type=lib
// no-prefer-dynamic
// edition:2021

#![feature(coroutines)]
pub fn run<T>(a: T) {
let _ = move || {
drop(a);
yield;
};
}
13 changes: 13 additions & 0 deletions tests/ui/coroutine/unwind-abort-mix.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Ensure that coroutine drop glue is valid when mixing different panic
// strategies. Regression test for #116953.
//
// no-prefer-dynamic
// build-pass
// aux-build:unwind-aux.rs
// compile-flags: -Cpanic=abort
// needs-unwind
extern crate unwind_aux;

pub fn main() {
unwind_aux::run(String::new());
}

0 comments on commit 5161b22

Please sign in to comment.