From cfbf1bf7cd300ddbfd2826488469c0921738685a Mon Sep 17 00:00:00 2001
From: Gary Guo <gary@garyguo.net>
Date: Wed, 28 Jun 2023 15:16:11 +0100
Subject: [PATCH 1/6] Do not create new resume block if there isn't one already
---
.../src/remove_noop_landing_pads.rs | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/compiler/rustc_mir_transform/src/remove_noop_landing_pads.rs b/compiler/rustc_mir_transform/src/remove_noop_landing_pads.rs
index 4941c9edce305..4e85c76fbc066 100644
--- a/compiler/rustc_mir_transform/src/remove_noop_landing_pads.rs
+++ b/compiler/rustc_mir_transform/src/remove_noop_landing_pads.rs
@@ -6,8 +6,8 @@ use rustc_middle::ty::TyCtxt;
use rustc_target::spec::PanicStrategy;
/// A pass that removes noop landing pads and replaces jumps to them with
-/// `None`. This is important because otherwise LLVM generates terrible
-/// code for these.
+/// `UnwindAction::Continue`. This is important because otherwise LLVM generates
+/// terrible code for these.
pub struct RemoveNoopLandingPads;
impl<'tcx> MirPass<'tcx> for RemoveNoopLandingPads {
@@ -84,7 +84,17 @@ impl RemoveNoopLandingPads {
fn remove_nop_landing_pads(&self, body: &mut Body<'_>) {
debug!("body: {:#?}", body);
- // make sure there's a resume block
+ // Skip the pass if there are no blocks with a resume terminator.
+ let has_resume = body
+ .basic_blocks
+ .iter_enumerated()
+ .any(|(_bb, block)| matches!(block.terminator().kind, TerminatorKind::Resume));
+ if !has_resume {
+ debug!("remove_noop_landing_pads: no resume block in MIR");
+ return;
+ }
+
+ // make sure there's a resume block without any statements
let resume_block = {
let mut patch = MirPatch::new(body);
let resume_block = patch.resume_block();
From cec8e09edf55fbf474c4414e3fec26d85c7869ff Mon Sep 17 00:00:00 2001
From: Gary Guo <gary@garyguo.net>
Date: Wed, 28 Jun 2023 16:05:17 +0100
Subject: [PATCH 2/6] Run `AbortUnwindingCalls` after generator transform
---
compiler/rustc_mir_transform/src/generator.rs | 16 +++++++++++++
...await.a-{closure#0}.generator_resume.0.mir | 2 +-
...await.b-{closure#0}.generator_resume.0.mir | 2 +-
tests/run-make/panic-abort-eh_frame/Makefile | 2 +-
tests/run-make/panic-abort-eh_frame/foo.rs | 24 +++++++++++++++++++
5 files changed, 43 insertions(+), 3 deletions(-)
diff --git a/compiler/rustc_mir_transform/src/generator.rs b/compiler/rustc_mir_transform/src/generator.rs
index 797a1a86846f6..2bd759ab1e5dc 100644
--- a/compiler/rustc_mir_transform/src/generator.rs
+++ b/compiler/rustc_mir_transform/src/generator.rs
@@ -50,8 +50,10 @@
//! For generators with state 1 (returned) and state 2 (poisoned) it does nothing.
//! Otherwise it drops all the values in scope at the last suspension point.
+use crate::abort_unwinding_calls;
use crate::deref_separator::deref_finder;
use crate::errors;
+use crate::pass_manager as pm;
use crate::simplify;
use crate::MirPass;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
@@ -64,6 +66,7 @@ use rustc_index::{Idx, IndexVec};
use rustc_middle::mir::dump_mir;
use rustc_middle::mir::visit::{MutVisitor, PlaceContext, Visitor};
use rustc_middle::mir::*;
+use rustc_middle::ty::InstanceDef;
use rustc_middle::ty::{self, AdtDef, Ty, TyCtxt};
use rustc_middle::ty::{GeneratorArgs, GenericArgsRef};
use rustc_mir_dataflow::impls::{
@@ -1147,6 +1150,17 @@ fn create_generator_drop_shim<'tcx>(
// unrelated code from the resume part of the function
simplify::remove_dead_blocks(tcx, &mut body);
+ // Update the body's def to become the drop glue.
+ let drop_in_place = tcx.require_lang_item(LangItem::DropInPlace, None);
+ body.source.instance = InstanceDef::DropGlue(drop_in_place, Some(gen_ty));
+
+ pm::run_passes_no_validate(
+ tcx,
+ &mut body,
+ &[&abort_unwinding_calls::AbortUnwindingCalls],
+ None,
+ );
+
dump_mir(tcx, false, "generator_drop", &0, &body, |_, _| Ok(()));
body
@@ -1317,6 +1331,8 @@ fn create_generator_resume_function<'tcx>(
// unrelated code from the drop part of the function
simplify::remove_dead_blocks(tcx, body);
+ pm::run_passes_no_validate(tcx, body, &[&abort_unwinding_calls::AbortUnwindingCalls], None);
+
dump_mir(tcx, false, "generator_resume", &0, body, |_, _| Ok(()));
}
diff --git a/tests/mir-opt/building/async_await.a-{closure#0}.generator_resume.0.mir b/tests/mir-opt/building/async_await.a-{closure#0}.generator_resume.0.mir
index 074ebddf78bb4..9be5b8509c728 100644
--- a/tests/mir-opt/building/async_await.a-{closure#0}.generator_resume.0.mir
+++ b/tests/mir-opt/building/async_await.a-{closure#0}.generator_resume.0.mir
@@ -30,7 +30,7 @@ fn a::{closure#0}(_1: Pin<&mut [async fn body@$DIR/async_await.rs:11:14: 11:16]>
}
bb2: {
- assert(const false, "`async fn` resumed after completion") -> [success: bb2, unwind continue];
+ assert(const false, "`async fn` resumed after completion") -> [success: bb2, unwind unreachable];
}
bb3: {
diff --git a/tests/mir-opt/building/async_await.b-{closure#0}.generator_resume.0.mir b/tests/mir-opt/building/async_await.b-{closure#0}.generator_resume.0.mir
index f774f32eb23e8..80ac92d59f3c2 100644
--- a/tests/mir-opt/building/async_await.b-{closure#0}.generator_resume.0.mir
+++ b/tests/mir-opt/building/async_await.b-{closure#0}.generator_resume.0.mir
@@ -310,7 +310,7 @@ fn b::{closure#0}(_1: Pin<&mut [async fn body@$DIR/async_await.rs:14:18: 17:2]>,
}
bb28: {
- assert(const false, "`async fn` resumed after completion") -> [success: bb28, unwind continue];
+ assert(const false, "`async fn` resumed after completion") -> [success: bb28, unwind unreachable];
}
bb29: {
diff --git a/tests/run-make/panic-abort-eh_frame/Makefile b/tests/run-make/panic-abort-eh_frame/Makefile
index 1cb7bf575cbdf..7020455b742d0 100644
--- a/tests/run-make/panic-abort-eh_frame/Makefile
+++ b/tests/run-make/panic-abort-eh_frame/Makefile
@@ -6,5 +6,5 @@
include ../tools.mk
all:
- $(RUSTC) foo.rs --crate-type=lib --emit=obj=$(TMPDIR)/foo.o -Cpanic=abort
+ $(RUSTC) foo.rs --crate-type=lib --emit=obj=$(TMPDIR)/foo.o -Cpanic=abort --edition 2021 -Z validate-mir
objdump --dwarf=frames $(TMPDIR)/foo.o | $(CGREP) -v 'DW_CFA'
diff --git a/tests/run-make/panic-abort-eh_frame/foo.rs b/tests/run-make/panic-abort-eh_frame/foo.rs
index e185352945538..e2274d469e73c 100644
--- a/tests/run-make/panic-abort-eh_frame/foo.rs
+++ b/tests/run-make/panic-abort-eh_frame/foo.rs
@@ -1,5 +1,13 @@
#![no_std]
+use core::future::Future;
+
+pub struct NeedsDrop;
+
+impl Drop for NeedsDrop {
+ fn drop(&mut self) {}
+}
+
#[panic_handler]
fn handler(_: &core::panic::PanicInfo<'_>) -> ! {
loop {}
@@ -8,3 +16,19 @@ fn handler(_: &core::panic::PanicInfo<'_>) -> ! {
pub unsafe fn oops(x: *const u32) -> u32 {
*x
}
+
+pub async fn foo(_: NeedsDrop) {
+ async fn bar() {}
+ bar().await;
+}
+
+pub fn poll_foo(x: &mut core::task::Context<'_>) {
+ let _g = NeedsDrop;
+ let mut p = core::pin::pin!(foo(NeedsDrop));
+ let _ = p.as_mut().poll(x);
+ let _ = p.as_mut().poll(x);
+}
+
+pub fn drop_foo() {
+ drop(foo(NeedsDrop));
+}
From 56b933763e9799f9deef86153d60988571188b49 Mon Sep 17 00:00:00 2001
From: Gary Guo <gary@garyguo.net>
Date: Wed, 28 Jun 2023 15:15:34 +0100
Subject: [PATCH 3/6] Add MIR validation for unwind out from nounwind functions
---
.../src/transform/validate.rs | 48 ++++++++++++++++---
1 file changed, 41 insertions(+), 7 deletions(-)
diff --git a/compiler/rustc_const_eval/src/transform/validate.rs b/compiler/rustc_const_eval/src/transform/validate.rs
index 33e73ad665359..50ae30711a659 100644
--- a/compiler/rustc_const_eval/src/transform/validate.rs
+++ b/compiler/rustc_const_eval/src/transform/validate.rs
@@ -18,6 +18,7 @@ use rustc_mir_dataflow::impls::MaybeStorageLive;
use rustc_mir_dataflow::storage::always_storage_live_locals;
use rustc_mir_dataflow::{Analysis, ResultsCursor};
use rustc_target::abi::{Size, FIRST_VARIANT};
+use rustc_target::spec::abi::Abi;
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
enum EdgeKind {
@@ -58,6 +59,25 @@ impl<'tcx> MirPass<'tcx> for Validator {
.iterate_to_fixpoint()
.into_results_cursor(body);
+ let can_unwind = if mir_phase <= MirPhase::Runtime(RuntimePhase::Initial) {
+ // In this case `AbortUnwindingCalls` haven't yet been executed.
+ true
+ } else if !tcx.def_kind(def_id).is_fn_like() {
+ true
+ } else {
+ let body_ty = tcx.type_of(def_id).skip_binder();
+ let body_abi = match body_ty.kind() {
+ ty::FnDef(..) => body_ty.fn_sig(tcx).abi(),
+ ty::Closure(..) => Abi::RustCall,
+ ty::Generator(..) => Abi::Rust,
+ _ => {
+ span_bug!(body.span, "unexpected body ty: {:?} phase {:?}", body_ty, mir_phase)
+ }
+ };
+
+ ty::layout::fn_can_unwind(tcx, Some(def_id), body_abi)
+ };
+
let mut cfg_checker = CfgChecker {
when: &self.when,
body,
@@ -68,6 +88,7 @@ impl<'tcx> MirPass<'tcx> for Validator {
storage_liveness,
place_cache: FxHashSet::default(),
value_cache: FxHashSet::default(),
+ can_unwind,
};
cfg_checker.visit_body(body);
cfg_checker.check_cleanup_control_flow();
@@ -99,6 +120,9 @@ struct CfgChecker<'a, 'tcx> {
storage_liveness: ResultsCursor<'a, 'tcx, MaybeStorageLive<'static>>,
place_cache: FxHashSet<PlaceRef<'tcx>>,
value_cache: FxHashSet<u128>,
+ // If `false`, then the MIR must not contain `UnwindAction::Continue` or
+ // `TerminatorKind::Resume`.
+ can_unwind: bool,
}
impl<'a, 'tcx> CfgChecker<'a, 'tcx> {
@@ -237,13 +261,17 @@ impl<'a, 'tcx> CfgChecker<'a, 'tcx> {
match unwind {
UnwindAction::Cleanup(unwind) => {
if is_cleanup {
- self.fail(location, "unwind on cleanup block");
+ self.fail(location, "`UnwindAction::Cleanup` in cleanup block");
}
self.check_edge(location, unwind, EdgeKind::Unwind);
}
UnwindAction::Continue => {
if is_cleanup {
- self.fail(location, "unwind on cleanup block");
+ self.fail(location, "`UnwindAction::Continue` in cleanup block");
+ }
+
+ if !self.can_unwind {
+ self.fail(location, "`UnwindAction::Continue` in no-unwind function");
}
}
UnwindAction::Unreachable | UnwindAction::Terminate => (),
@@ -464,13 +492,19 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> {
);
}
}
- TerminatorKind::Resume | TerminatorKind::Terminate => {
+ TerminatorKind::Resume => {
let bb = location.block;
if !self.body.basic_blocks[bb].is_cleanup {
- self.fail(
- location,
- "Cannot `Resume` or `Terminate` from non-cleanup basic block",
- )
+ self.fail(location, "Cannot `Resume` from non-cleanup basic block")
+ }
+ if !self.can_unwind {
+ self.fail(location, "Cannot `Resume` in a function that cannot unwind")
+ }
+ }
+ TerminatorKind::Terminate => {
+ let bb = location.block;
+ if !self.body.basic_blocks[bb].is_cleanup {
+ self.fail(location, "Cannot `Terminate` from non-cleanup basic block")
}
}
TerminatorKind::Return => {
From 907e431f9318eb83dd5aaab6dee67a88a97df787 Mon Sep 17 00:00:00 2001
From: Gary Guo <gary@garyguo.net>
Date: Wed, 28 Jun 2023 15:29:16 +0100
Subject: [PATCH 4/6] Perform MIR validation on drop glue of generator
---
compiler/rustc_mir_transform/src/shim.rs | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/compiler/rustc_mir_transform/src/shim.rs b/compiler/rustc_mir_transform/src/shim.rs
index 5e8ba4f544c97..223dc59c68d17 100644
--- a/compiler/rustc_mir_transform/src/shim.rs
+++ b/compiler/rustc_mir_transform/src/shim.rs
@@ -71,8 +71,17 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> Body<'
// of this function. Is this intentional?
if let Some(ty::Generator(gen_def_id, args, _)) = ty.map(Ty::kind) {
let body = tcx.optimized_mir(*gen_def_id).generator_drop().unwrap();
- let body = EarlyBinder::bind(body.clone()).instantiate(tcx, args);
+ 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,
+ &[],
+ Some(MirPhase::Runtime(RuntimePhase::Optimized)),
+ );
+
return body;
}
From eb4d6d9ff72d09eee908340d8d8726b5644ae86e Mon Sep 17 00:00:00 2001
From: Gary Guo <gary@garyguo.net>
Date: Fri, 18 Aug 2023 16:19:26 +0100
Subject: [PATCH 5/6] Add missing instantiation of generator ty in validator
---
compiler/rustc_const_eval/src/transform/validate.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/compiler/rustc_const_eval/src/transform/validate.rs b/compiler/rustc_const_eval/src/transform/validate.rs
index 50ae30711a659..783b52d004020 100644
--- a/compiler/rustc_const_eval/src/transform/validate.rs
+++ b/compiler/rustc_const_eval/src/transform/validate.rs
@@ -699,7 +699,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
return;
};
- f_ty.ty
+ ty::EarlyBinder::bind(f_ty.ty).instantiate(self.tcx, args)
} else {
let Some(&f_ty) = args.as_generator().prefix_tys().get(f.index())
else {
From 0a7202d4769fe417988b7e74333e3014a51ad462 Mon Sep 17 00:00:00 2001
From: Gary Guo <gary@garyguo.net>
Date: Fri, 18 Aug 2023 16:36:22 +0100
Subject: [PATCH 6/6] Change generator_drop's instance to that of generator for
dump_mir
Otherwise the file name generated for generator_drop will become
core.ptr-drop_in_place.[generator@<FILEPATH>_<NUMBERS>].generator_drop.0.mir
instead of main-{closure#0}.generator_drop.0.mir which breaks a mir-opt
test.
---
compiler/rustc_mir_transform/src/generator.rs | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/compiler/rustc_mir_transform/src/generator.rs b/compiler/rustc_mir_transform/src/generator.rs
index 2bd759ab1e5dc..ff4822f333f08 100644
--- a/compiler/rustc_mir_transform/src/generator.rs
+++ b/compiler/rustc_mir_transform/src/generator.rs
@@ -1151,8 +1151,11 @@ fn create_generator_drop_shim<'tcx>(
simplify::remove_dead_blocks(tcx, &mut body);
// Update the body's def to become the drop glue.
+ // This needs to be updated before the AbortUnwindingCalls pass.
+ let gen_instance = body.source.instance;
let drop_in_place = tcx.require_lang_item(LangItem::DropInPlace, None);
- body.source.instance = InstanceDef::DropGlue(drop_in_place, Some(gen_ty));
+ let drop_instance = InstanceDef::DropGlue(drop_in_place, Some(gen_ty));
+ body.source.instance = drop_instance;
pm::run_passes_no_validate(
tcx,
@@ -1161,7 +1164,11 @@ fn create_generator_drop_shim<'tcx>(
None,
);
+ // Temporary change MirSource to generator's instance so that dump_mir produces more sensible
+ // filename.
+ body.source.instance = gen_instance;
dump_mir(tcx, false, "generator_drop", &0, &body, |_, _| Ok(()));
+ body.source.instance = drop_instance;
body
}