From 98a2292919147d775ec196f42fe821b600180019 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Sat, 3 Oct 2020 14:38:01 -0700 Subject: [PATCH 1/6] Allow `Abort` terminators in a const-context These appear along the cleanup path inside functions with `#[unwind(aborts)]`. We don't const-check the cleanup path anyways, since const-eval already has "abort-on-panic" semantics and there's often drops that would otherwise be forbidden, so the check wasn't really preventing anything anyways. --- compiler/rustc_mir/src/transform/check_consts/ops.rs | 12 ------------ .../src/transform/check_consts/validation.rs | 4 ++-- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_mir/src/transform/check_consts/ops.rs b/compiler/rustc_mir/src/transform/check_consts/ops.rs index 32e233e337d..9a1b77e994d 100644 --- a/compiler/rustc_mir/src/transform/check_consts/ops.rs +++ b/compiler/rustc_mir/src/transform/check_consts/ops.rs @@ -41,18 +41,6 @@ pub trait NonConstOp: std::fmt::Debug { fn build_error(&self, ccx: &ConstCx<'_, 'tcx>, span: Span) -> DiagnosticBuilder<'tcx>; } -#[derive(Debug)] -pub struct Abort; -impl NonConstOp for Abort { - fn status_in_item(&self, ccx: &ConstCx<'_, '_>) -> Status { - mcf_status_in_item(ccx) - } - - fn build_error(&self, ccx: &ConstCx<'_, 'tcx>, span: Span) -> DiagnosticBuilder<'tcx> { - mcf_build_error(ccx, span, "abort is not stable in const fn") - } -} - #[derive(Debug)] pub struct FloatingPointOp; impl NonConstOp for FloatingPointOp { diff --git a/compiler/rustc_mir/src/transform/check_consts/validation.rs b/compiler/rustc_mir/src/transform/check_consts/validation.rs index 4e714bfeed3..ab78e3d541c 100644 --- a/compiler/rustc_mir/src/transform/check_consts/validation.rs +++ b/compiler/rustc_mir/src/transform/check_consts/validation.rs @@ -874,13 +874,13 @@ impl Visitor<'tcx> for Validator<'mir, 'tcx> { } TerminatorKind::InlineAsm { .. } => self.check_op(ops::InlineAsm), - TerminatorKind::Abort => self.check_op(ops::Abort), TerminatorKind::GeneratorDrop | TerminatorKind::Yield { .. } => { self.check_op(ops::Generator(hir::GeneratorKind::Gen)) } - TerminatorKind::Assert { .. } + TerminatorKind::Abort + | TerminatorKind::Assert { .. } | TerminatorKind::FalseEdge { .. } | TerminatorKind::FalseUnwind { .. } | TerminatorKind::Goto { .. } From a5f083133e698adefe11b9e8956387d5b4c99548 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Sat, 3 Oct 2020 14:48:01 -0700 Subject: [PATCH 2/6] Add check-pass test for `#[unwind(aborts)]` on a `const fn` --- src/test/ui/consts/unwind-abort.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 src/test/ui/consts/unwind-abort.rs diff --git a/src/test/ui/consts/unwind-abort.rs b/src/test/ui/consts/unwind-abort.rs new file mode 100644 index 00000000000..f9011f908a7 --- /dev/null +++ b/src/test/ui/consts/unwind-abort.rs @@ -0,0 +1,17 @@ +// check-pass + +#![feature(unwind_attributes, const_panic)] + +// `#[unwind(aborts)]` is okay for a `const fn`. We don't unwind in const-eval anyways. +#[unwind(aborts)] +const fn foo() { + panic!() +} + +const fn bar() { + foo(); +} + +fn main() { + bar(); +} From 14c3705c6cfc5f88ba64e00129c99cc1f8a41490 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Sat, 3 Oct 2020 15:09:43 -0700 Subject: [PATCH 3/6] Ensure that the const-eval engine handles `#[unwind(aborts)]` --- src/test/ui/consts/const-eval/unwind-abort.rs | 12 +++++++++++ .../ui/consts/const-eval/unwind-abort.stderr | 21 +++++++++++++++++++ 2 files changed, 33 insertions(+) create mode 100644 src/test/ui/consts/const-eval/unwind-abort.rs create mode 100644 src/test/ui/consts/const-eval/unwind-abort.stderr diff --git a/src/test/ui/consts/const-eval/unwind-abort.rs b/src/test/ui/consts/const-eval/unwind-abort.rs new file mode 100644 index 00000000000..146c841bf1f --- /dev/null +++ b/src/test/ui/consts/const-eval/unwind-abort.rs @@ -0,0 +1,12 @@ +#![feature(unwind_attributes, const_panic)] + +#[unwind(aborts)] +const fn foo() { + panic!() //~ evaluation of constant value failed +} + +const _: () = foo(); //~ any use of this value will cause an error + +fn main() { + let _ = foo(); +} diff --git a/src/test/ui/consts/const-eval/unwind-abort.stderr b/src/test/ui/consts/const-eval/unwind-abort.stderr new file mode 100644 index 00000000000..084beb19eb9 --- /dev/null +++ b/src/test/ui/consts/const-eval/unwind-abort.stderr @@ -0,0 +1,21 @@ +error[E0080]: evaluation of constant value failed + --> $DIR/unwind-abort.rs:5:5 + | +LL | panic!() + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/unwind-abort.rs:5:5 + | + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: any use of this value will cause an error + --> $DIR/unwind-abort.rs:8:15 + | +LL | const _: () = foo(); + | --------------^^^^^- + | | + | referenced constant has errors + | + = note: `#[deny(const_err)]` on by default + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0080`. From 25fdbaff444580bb07783861579653bf81657f5b Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Sun, 4 Oct 2020 10:39:12 -0700 Subject: [PATCH 4/6] Discuss cleanup blocks and `span_bug` on `Abort` --- .../src/transform/check_consts/validation.rs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_mir/src/transform/check_consts/validation.rs b/compiler/rustc_mir/src/transform/check_consts/validation.rs index ab78e3d541c..cb9feba260f 100644 --- a/compiler/rustc_mir/src/transform/check_consts/validation.rs +++ b/compiler/rustc_mir/src/transform/check_consts/validation.rs @@ -434,11 +434,13 @@ impl Visitor<'tcx> for Validator<'mir, 'tcx> { fn visit_basic_block_data(&mut self, bb: BasicBlock, block: &BasicBlockData<'tcx>) { trace!("visit_basic_block_data: bb={:?} is_cleanup={:?}", bb, block.is_cleanup); - // Just as the old checker did, we skip const-checking basic blocks on the unwind path. - // These blocks often drop locals that would otherwise be returned from the function. + // We don't const-check basic blocks on the cleanup path since we never unwind during + // const-eval: a panic causes an immediate compile error. In other words, cleanup blocks + // are unreachable during const-eval. // - // FIXME: This shouldn't be unsound since a panic at compile time will cause a compiler - // error anyway, but maybe we should do more here? + // We can't be more conservative (e.g., by const-checking cleanup blocks anyways) because + // locals that would never be dropped during normal execution are sometimes dropped during + // unwinding, which means backwards-incompatible live-drop errors. if block.is_cleanup { return; } @@ -879,8 +881,11 @@ impl Visitor<'tcx> for Validator<'mir, 'tcx> { self.check_op(ops::Generator(hir::GeneratorKind::Gen)) } - TerminatorKind::Abort - | TerminatorKind::Assert { .. } + TerminatorKind::Abort => { + span_bug!(self.span, "`Abort` terminator outside of cleanup block") + } + + TerminatorKind::Assert { .. } | TerminatorKind::FalseEdge { .. } | TerminatorKind::FalseUnwind { .. } | TerminatorKind::Goto { .. } From fe97990e2395d46aceb9472fd04704c623c8eb0e Mon Sep 17 00:00:00 2001 From: ecstatic-morse Date: Sun, 4 Oct 2020 13:02:54 -0700 Subject: [PATCH 5/6] Add comment to `Abort` match arm Co-authored-by: Ralf Jung --- compiler/rustc_mir/src/transform/check_consts/validation.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/rustc_mir/src/transform/check_consts/validation.rs b/compiler/rustc_mir/src/transform/check_consts/validation.rs index cb9feba260f..94806116eaf 100644 --- a/compiler/rustc_mir/src/transform/check_consts/validation.rs +++ b/compiler/rustc_mir/src/transform/check_consts/validation.rs @@ -882,6 +882,7 @@ impl Visitor<'tcx> for Validator<'mir, 'tcx> { } TerminatorKind::Abort => { + // Cleanup blocks are skipped for const checking (see `visit_basic_block_data`). span_bug!(self.span, "`Abort` terminator outside of cleanup block") } From 6ae1da3198bd2d110652900082800e01a8e37149 Mon Sep 17 00:00:00 2001 From: ecstatic-morse Date: Sun, 4 Oct 2020 13:25:45 -0700 Subject: [PATCH 6/6] But whatever --- src/test/ui/consts/const-eval/unwind-abort.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/ui/consts/const-eval/unwind-abort.rs b/src/test/ui/consts/const-eval/unwind-abort.rs index 146c841bf1f..b8b95dea1e7 100644 --- a/src/test/ui/consts/const-eval/unwind-abort.rs +++ b/src/test/ui/consts/const-eval/unwind-abort.rs @@ -6,6 +6,7 @@ const fn foo() { } const _: () = foo(); //~ any use of this value will cause an error +// Ensure that the CTFE engine handles calls to `#[unwind(aborts)]` gracefully fn main() { let _ = foo();