From 20b2dbf0b6a9d5b95dbe599928dcee62a2ade463 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 7 Nov 2023 21:14:38 +0000 Subject: [PATCH 1/7] Build pre-coroutine-transform coroutine body (cherry picked from commit 0ba7d19769c068729761eba72e779b53101a19bc) --- compiler/rustc_mir_build/src/build/mod.rs | 12 +----------- .../ui/mir/build-async-error-body-correctly.rs | 8 ++++++++ .../mir/build-async-error-body-correctly.stderr | 17 +++++++++++++++++ 3 files changed, 26 insertions(+), 11 deletions(-) create mode 100644 tests/ui/mir/build-async-error-body-correctly.rs create mode 100644 tests/ui/mir/build-async-error-body-correctly.stderr diff --git a/compiler/rustc_mir_build/src/build/mod.rs b/compiler/rustc_mir_build/src/build/mod.rs index 7c729016521b0..886d805454db4 100644 --- a/compiler/rustc_mir_build/src/build/mod.rs +++ b/compiler/rustc_mir_build/src/build/mod.rs @@ -656,17 +656,7 @@ fn construct_error(tcx: TyCtxt<'_>, def_id: LocalDefId, guar: ErrorGuaranteed) - let args = args.as_coroutine(); let yield_ty = args.yield_ty(); let return_ty = args.return_ty(); - let self_ty = Ty::new_adt( - tcx, - tcx.adt_def(tcx.lang_items().pin_type().unwrap()), - tcx.mk_args(&[Ty::new_mut_ref(tcx, tcx.lifetimes.re_erased, coroutine_ty).into()]), - ); - let coroutine_state = Ty::new_adt( - tcx, - tcx.adt_def(tcx.lang_items().coroutine_state().unwrap()), - tcx.mk_args(&[yield_ty.into(), return_ty.into()]), - ); - (vec![self_ty, args.resume_ty()], coroutine_state, Some(yield_ty)) + (vec![coroutine_ty, args.resume_ty()], return_ty, Some(yield_ty)) } dk => bug!("{:?} is not a body: {:?}", def_id, dk), }; diff --git a/tests/ui/mir/build-async-error-body-correctly.rs b/tests/ui/mir/build-async-error-body-correctly.rs new file mode 100644 index 0000000000000..1787f80c07e5e --- /dev/null +++ b/tests/ui/mir/build-async-error-body-correctly.rs @@ -0,0 +1,8 @@ +// edition: 2021 + +async fn asyncfn() { + let binding = match true {}; + //~^ ERROR non-exhaustive patterns: type `bool` is non-empty +} + +fn main() {} diff --git a/tests/ui/mir/build-async-error-body-correctly.stderr b/tests/ui/mir/build-async-error-body-correctly.stderr new file mode 100644 index 0000000000000..3d18c249afe21 --- /dev/null +++ b/tests/ui/mir/build-async-error-body-correctly.stderr @@ -0,0 +1,17 @@ +error[E0004]: non-exhaustive patterns: type `bool` is non-empty + --> $DIR/build-async-error-body-correctly.rs:4:25 + | +LL | let binding = match true {}; + | ^^^^ + | + = note: the matched value is of type `bool` +help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern as shown + | +LL ~ let binding = match true { +LL + _ => todo!(), +LL ~ }; + | + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0004`. From ae0b24fe5504b469bdb824646a93da96a66a4ea8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Tue, 28 Nov 2023 00:00:00 +0000 Subject: [PATCH 2/7] Fix coroutine validation for mixed panic strategy 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. (cherry picked from commit 5161b22143bf9a726cfd69d19bef11552d0ed519) --- compiler/rustc_mir_transform/src/coroutine.rs | 9 --------- compiler/rustc_mir_transform/src/shim.rs | 6 ++++-- tests/ui/coroutine/auxiliary/unwind-aux.rs | 11 +++++++++++ tests/ui/coroutine/unwind-abort-mix.rs | 13 +++++++++++++ 4 files changed, 28 insertions(+), 11 deletions(-) create mode 100644 tests/ui/coroutine/auxiliary/unwind-aux.rs create mode 100644 tests/ui/coroutine/unwind-abort-mix.rs diff --git a/compiler/rustc_mir_transform/src/coroutine.rs b/compiler/rustc_mir_transform/src/coroutine.rs index dfafd8598306b..abaed103fab1f 100644 --- a/compiler/rustc_mir_transform/src/coroutine.rs +++ b/compiler/rustc_mir_transform/src/coroutine.rs @@ -1153,18 +1153,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], - None, - ); // Temporary change MirSource to coroutine's instance so that dump_mir produces more sensible // filename. diff --git a/compiler/rustc_mir_transform/src/shim.rs b/compiler/rustc_mir_transform/src/shim.rs index 4ae5ea4c8d68f..ab7961321b3f9 100644 --- a/compiler/rustc_mir_transform/src/shim.rs +++ b/compiler/rustc_mir_transform/src/shim.rs @@ -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)), ); diff --git a/tests/ui/coroutine/auxiliary/unwind-aux.rs b/tests/ui/coroutine/auxiliary/unwind-aux.rs new file mode 100644 index 0000000000000..215d676911633 --- /dev/null +++ b/tests/ui/coroutine/auxiliary/unwind-aux.rs @@ -0,0 +1,11 @@ +// compile-flags: -Cpanic=unwind --crate-type=lib +// no-prefer-dynamic +// edition:2021 + +#![feature(coroutines)] +pub fn run(a: T) { + let _ = move || { + drop(a); + yield; + }; +} diff --git a/tests/ui/coroutine/unwind-abort-mix.rs b/tests/ui/coroutine/unwind-abort-mix.rs new file mode 100644 index 0000000000000..869b3e4f43347 --- /dev/null +++ b/tests/ui/coroutine/unwind-abort-mix.rs @@ -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()); +} From ebd43bdb66774b5aa7fb8c84f1172b1ee61c9521 Mon Sep 17 00:00:00 2001 From: Alona Enraght-Moony Date: Tue, 28 Nov 2023 15:12:46 +0000 Subject: [PATCH 3/7] Precommit test for https://github.com/rust-lang/rust/issues/118328. (cherry picked from commit b1a6cf4a0e7d6727516e943d86fdab8587b94722) --- tests/mir-opt/const_prop/issue_118328.rs | 20 ++++++++++++++++++ .../issue_118328.size_of.ConstProp.diff | 21 +++++++++++++++++++ 2 files changed, 41 insertions(+) create mode 100644 tests/mir-opt/const_prop/issue_118328.rs create mode 100644 tests/mir-opt/const_prop/issue_118328.size_of.ConstProp.diff diff --git a/tests/mir-opt/const_prop/issue_118328.rs b/tests/mir-opt/const_prop/issue_118328.rs new file mode 100644 index 0000000000000..5072091ddfc30 --- /dev/null +++ b/tests/mir-opt/const_prop/issue_118328.rs @@ -0,0 +1,20 @@ +// unit-test: ConstProp +// compile-flags: -O +// skip-filecheck +#![allow(unused_assignments)] + +struct SizeOfConst(std::marker::PhantomData); +impl SizeOfConst { + const SIZE: usize = std::mem::size_of::(); +} + +// EMIT_MIR issue_118328.size_of.ConstProp.diff +fn size_of() -> usize { + let mut a = 0; + a = SizeOfConst::::SIZE; + a +} + +fn main() { + assert_eq!(size_of::(), std::mem::size_of::()); +} diff --git a/tests/mir-opt/const_prop/issue_118328.size_of.ConstProp.diff b/tests/mir-opt/const_prop/issue_118328.size_of.ConstProp.diff new file mode 100644 index 0000000000000..0f44c49bc4da1 --- /dev/null +++ b/tests/mir-opt/const_prop/issue_118328.size_of.ConstProp.diff @@ -0,0 +1,21 @@ +- // MIR for `size_of` before ConstProp ++ // MIR for `size_of` after ConstProp + + fn size_of() -> usize { + let mut _0: usize; + let mut _1: usize; + scope 1 { + debug a => _1; + } + + bb0: { + StorageLive(_1); + _1 = const 0_usize; + _1 = const _; +- _0 = _1; ++ _0 = const 0_usize; + StorageDead(_1); + return; + } + } + From cb4f87af3490997fa31e751ecf62647acd058bd3 Mon Sep 17 00:00:00 2001 From: Alona Enraght-Moony Date: Tue, 28 Nov 2023 21:43:23 +0000 Subject: [PATCH 4/7] ConstProp: Remove const when rvalue check fails. (cherry picked from commit 9121a41450e905fe5a12c11c955acc14ab1f92fe) --- compiler/rustc_mir_transform/src/const_prop.rs | 7 ++++++- .../mir-opt/const_prop/issue_118328.size_of.ConstProp.diff | 3 +-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_mir_transform/src/const_prop.rs b/compiler/rustc_mir_transform/src/const_prop.rs index 0adbb078105af..f7f882310e603 100644 --- a/compiler/rustc_mir_transform/src/const_prop.rs +++ b/compiler/rustc_mir_transform/src/const_prop.rs @@ -440,6 +440,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { // FIXME we need to revisit this for #67176 if rvalue.has_param() { + trace!("skipping, has param"); return None; } if !rvalue @@ -708,7 +709,11 @@ impl<'tcx> Visitor<'tcx> for ConstPropagator<'_, 'tcx> { fn visit_assign(&mut self, place: &Place<'tcx>, rvalue: &Rvalue<'tcx>, location: Location) { self.super_assign(place, rvalue, location); - let Some(()) = self.check_rvalue(rvalue) else { return }; + let Some(()) = self.check_rvalue(rvalue) else { + trace!("rvalue check failed, removing const"); + Self::remove_const(&mut self.ecx, place.local); + return; + }; match self.ecx.machine.can_const_prop[place.local] { // Do nothing if the place is indirect. diff --git a/tests/mir-opt/const_prop/issue_118328.size_of.ConstProp.diff b/tests/mir-opt/const_prop/issue_118328.size_of.ConstProp.diff index 0f44c49bc4da1..ad8318832d63b 100644 --- a/tests/mir-opt/const_prop/issue_118328.size_of.ConstProp.diff +++ b/tests/mir-opt/const_prop/issue_118328.size_of.ConstProp.diff @@ -12,8 +12,7 @@ StorageLive(_1); _1 = const 0_usize; _1 = const _; -- _0 = _1; -+ _0 = const 0_usize; + _0 = _1; StorageDead(_1); return; } From 98d3e99b8c1aaf745be6e53c121e052d1e9ae31a Mon Sep 17 00:00:00 2001 From: Alona Enraght-Moony Date: Tue, 28 Nov 2023 22:31:53 +0000 Subject: [PATCH 5/7] Rename and add another test (cherry picked from commit 6e956c0a383444a25cc0e690cbc9fa6f859b07b4) --- .../overwrite_with_const_with_params.rs | 26 +++++++++++++++++++ ..._const_with_params.size_of.ConstProp.diff} | 0 .../overwrite_with_const_with_params.rs} | 7 ++--- 3 files changed, 30 insertions(+), 3 deletions(-) create mode 100644 tests/mir-opt/const_prop/overwrite_with_const_with_params.rs rename tests/mir-opt/const_prop/{issue_118328.size_of.ConstProp.diff => overwrite_with_const_with_params.size_of.ConstProp.diff} (100%) rename tests/{mir-opt/const_prop/issue_118328.rs => ui/const_prop/overwrite_with_const_with_params.rs} (79%) diff --git a/tests/mir-opt/const_prop/overwrite_with_const_with_params.rs b/tests/mir-opt/const_prop/overwrite_with_const_with_params.rs new file mode 100644 index 0000000000000..4cf6d7c139643 --- /dev/null +++ b/tests/mir-opt/const_prop/overwrite_with_const_with_params.rs @@ -0,0 +1,26 @@ +// unit-test: ConstProp +// compile-flags: -O + +// Regression test for https://github.com/rust-lang/rust/issues/118328 + +#![allow(unused_assignments)] + +struct SizeOfConst(std::marker::PhantomData); +impl SizeOfConst { + const SIZE: usize = std::mem::size_of::(); +} + +// EMIT_MIR overwrite_with_const_with_params.size_of.ConstProp.diff +fn size_of() -> usize { + // CHECK-LABEL: fn size_of( + // CHECK: _1 = const 0_usize; + // CHECK-NEXT: _1 = const _; + // CHECK-NEXT: _0 = _1; + let mut a = 0; + a = SizeOfConst::::SIZE; + a +} + +fn main() { + assert_eq!(size_of::(), std::mem::size_of::()); +} diff --git a/tests/mir-opt/const_prop/issue_118328.size_of.ConstProp.diff b/tests/mir-opt/const_prop/overwrite_with_const_with_params.size_of.ConstProp.diff similarity index 100% rename from tests/mir-opt/const_prop/issue_118328.size_of.ConstProp.diff rename to tests/mir-opt/const_prop/overwrite_with_const_with_params.size_of.ConstProp.diff diff --git a/tests/mir-opt/const_prop/issue_118328.rs b/tests/ui/const_prop/overwrite_with_const_with_params.rs similarity index 79% rename from tests/mir-opt/const_prop/issue_118328.rs rename to tests/ui/const_prop/overwrite_with_const_with_params.rs index 5072091ddfc30..6f533919a474e 100644 --- a/tests/mir-opt/const_prop/issue_118328.rs +++ b/tests/ui/const_prop/overwrite_with_const_with_params.rs @@ -1,6 +1,8 @@ -// unit-test: ConstProp // compile-flags: -O -// skip-filecheck +// run-pass + +// Regression test for https://github.com/rust-lang/rust/issues/118328 + #![allow(unused_assignments)] struct SizeOfConst(std::marker::PhantomData); @@ -8,7 +10,6 @@ impl SizeOfConst { const SIZE: usize = std::mem::size_of::(); } -// EMIT_MIR issue_118328.size_of.ConstProp.diff fn size_of() -> usize { let mut a = 0; a = SizeOfConst::::SIZE; From 76e6681b1f79bc7ed1c177bb95a37acead80a08c Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Wed, 29 Nov 2023 17:36:45 -0600 Subject: [PATCH 6/7] Dispose llvm::TargetMachines prior to llvm::Context being disposed If the TargetMachine is disposed after the Context is disposed, it can lead to use after frees in some cases. I've observed this happening occasionally on code compiled for aarch64-pc-windows-msvc using `-Zstack-protector=strong` but other users have reported AVs from host aarch64-pc-windows-msvc compilers as well. (cherry picked from commit 3323e4dc04e57cc64ac77dbff2f6bf50ac6832f0) --- compiler/rustc_codegen_llvm/src/back/lto.rs | 3 ++- compiler/rustc_codegen_llvm/src/lib.rs | 21 ++++++++++++++++----- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/back/lto.rs b/compiler/rustc_codegen_llvm/src/back/lto.rs index 8655aeec13dd6..db297425b03bf 100644 --- a/compiler/rustc_codegen_llvm/src/back/lto.rs +++ b/compiler/rustc_codegen_llvm/src/back/lto.rs @@ -25,6 +25,7 @@ use std::ffi::{CStr, CString}; use std::fs::File; use std::io; use std::iter; +use std::mem::ManuallyDrop; use std::path::Path; use std::slice; use std::sync::Arc; @@ -734,7 +735,7 @@ pub unsafe fn optimize_thin_module( let llcx = llvm::LLVMRustContextCreate(cgcx.fewer_names); let llmod_raw = parse_module(llcx, module_name, thin_module.data(), &diag_handler)? as *const _; let mut module = ModuleCodegen { - module_llvm: ModuleLlvm { llmod_raw, llcx, tm }, + module_llvm: ModuleLlvm { llmod_raw, llcx, tm: ManuallyDrop::new(tm) }, name: thin_module.name().to_string(), kind: ModuleKind::Regular, }; diff --git a/compiler/rustc_codegen_llvm/src/lib.rs b/compiler/rustc_codegen_llvm/src/lib.rs index 8a6a5f79b3bb9..4ea2d30ad6d20 100644 --- a/compiler/rustc_codegen_llvm/src/lib.rs +++ b/compiler/rustc_codegen_llvm/src/lib.rs @@ -53,6 +53,7 @@ use rustc_span::symbol::Symbol; use std::any::Any; use std::ffi::CStr; use std::io::Write; +use std::mem::ManuallyDrop; mod back { pub mod archive; @@ -408,8 +409,9 @@ pub struct ModuleLlvm { llcx: &'static mut llvm::Context, llmod_raw: *const llvm::Module, - // independent from llcx and llmod_raw, resources get disposed by drop impl - tm: OwnedTargetMachine, + // This field is `ManuallyDrop` because it is important that the `TargetMachine` + // is disposed prior to the `Context` being disposed otherwise UAFs can occur. + tm: ManuallyDrop, } unsafe impl Send for ModuleLlvm {} @@ -420,7 +422,11 @@ impl ModuleLlvm { unsafe { let llcx = llvm::LLVMRustContextCreate(tcx.sess.fewer_names()); let llmod_raw = context::create_module(tcx, llcx, mod_name) as *const _; - ModuleLlvm { llmod_raw, llcx, tm: create_target_machine(tcx, mod_name) } + ModuleLlvm { + llmod_raw, + llcx, + tm: ManuallyDrop::new(create_target_machine(tcx, mod_name)), + } } } @@ -428,7 +434,11 @@ impl ModuleLlvm { unsafe { let llcx = llvm::LLVMRustContextCreate(tcx.sess.fewer_names()); let llmod_raw = context::create_module(tcx, llcx, mod_name) as *const _; - ModuleLlvm { llmod_raw, llcx, tm: create_informational_target_machine(tcx.sess) } + ModuleLlvm { + llmod_raw, + llcx, + tm: ManuallyDrop::new(create_informational_target_machine(tcx.sess)), + } } } @@ -449,7 +459,7 @@ impl ModuleLlvm { } }; - Ok(ModuleLlvm { llmod_raw, llcx, tm }) + Ok(ModuleLlvm { llmod_raw, llcx, tm: ManuallyDrop::new(tm) }) } } @@ -461,6 +471,7 @@ impl ModuleLlvm { impl Drop for ModuleLlvm { fn drop(&mut self) { unsafe { + drop(ManuallyDrop::take(&mut self.tm)); llvm::LLVMContextDispose(&mut *(self.llcx as *mut _)); } } From 6c4560855899cc082a773d1588dce93b1f9e3fda Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Wed, 29 Nov 2023 18:00:41 -0600 Subject: [PATCH 7/7] Update compiler/rustc_codegen_llvm/src/lib.rs Co-authored-by: Josh Stone (cherry picked from commit 10110787154154cddfaaacdeebd7a4406223a25b) --- compiler/rustc_codegen_llvm/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_codegen_llvm/src/lib.rs b/compiler/rustc_codegen_llvm/src/lib.rs index 4ea2d30ad6d20..a4e0270124173 100644 --- a/compiler/rustc_codegen_llvm/src/lib.rs +++ b/compiler/rustc_codegen_llvm/src/lib.rs @@ -471,7 +471,7 @@ impl ModuleLlvm { impl Drop for ModuleLlvm { fn drop(&mut self) { unsafe { - drop(ManuallyDrop::take(&mut self.tm)); + ManuallyDrop::drop(&mut self.tm); llvm::LLVMContextDispose(&mut *(self.llcx as *mut _)); } }