From f74e8c7afcf803d6c78a1f05ab89c0e00bce5f0a Mon Sep 17 00:00:00 2001
From: Gary Guo <gary@garyguo.net>
Date: Fri, 14 Jan 2022 23:54:26 +0000
Subject: [PATCH 1/5] Guard against unwinding in cleanup code
---
compiler/rustc_codegen_ssa/src/mir/block.rs | 67 +++++++++++++++++----
compiler/rustc_codegen_ssa/src/mir/mod.rs | 4 ++
library/core/src/panicking.rs | 3 +-
3 files changed, 59 insertions(+), 15 deletions(-)
diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs
index 4c7a09ca1e94b..5cbbb34d2e625 100644
--- a/compiler/rustc_codegen_ssa/src/mir/block.rs
+++ b/compiler/rustc_codegen_ssa/src/mir/block.rs
@@ -135,21 +135,38 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> {
// If there is a cleanup block and the function we're calling can unwind, then
// do an invoke, otherwise do a call.
let fn_ty = bx.fn_decl_backend_type(&fn_abi);
- if let Some(cleanup) = cleanup.filter(|_| fn_abi.can_unwind) {
+
+ let unwind_block = if let Some(cleanup) = cleanup.filter(|_| fn_abi.can_unwind) {
+ Some(self.llblock(fx, cleanup))
+ } else if fx.mir[self.bb].is_cleanup
+ && fn_abi.can_unwind
+ && !base::wants_msvc_seh(fx.cx.tcx().sess)
+ {
+ // Exception must not propagate out of the execution of a cleanup (doing so
+ // can cause undefined behaviour). We insert a double unwind guard for
+ // functions that can potentially unwind to protect against this.
+ //
+ // This is not necessary for SEH which does not use successive unwinding
+ // like Itanium EH. EH frames in SEH are different from normal function
+ // frames and SEH will abort automatically if an exception tries to
+ // propagate out from cleanup.
+ Some(fx.double_unwind_guard())
+ } else {
+ None
+ };
+
+ if let Some(unwind_block) = unwind_block {
let ret_llbb = if let Some((_, target)) = destination {
fx.llbb(target)
} else {
fx.unreachable_block()
};
- let invokeret = bx.invoke(
- fn_ty,
- fn_ptr,
- &llargs,
- ret_llbb,
- self.llblock(fx, cleanup),
- self.funclet(fx),
- );
+ let invokeret =
+ bx.invoke(fn_ty, fn_ptr, &llargs, ret_llbb, unwind_block, self.funclet(fx));
bx.apply_attrs_callsite(&fn_abi, invokeret);
+ if fx.mir[self.bb].is_cleanup {
+ bx.apply_attrs_to_cleanup_callsite(invokeret);
+ }
if let Some((ret_dest, target)) = destination {
let mut ret_bx = fx.build_block(target);
@@ -486,9 +503,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
let span = terminator.source_info.span;
self.set_debug_loc(&mut bx, terminator.source_info);
- // Get the location information.
- let location = self.get_caller_location(&mut bx, terminator.source_info).immediate();
-
// Obtain the panic entry point.
let def_id = common::langcall(bx.tcx(), Some(span), "", LangItem::PanicNoUnwind);
let instance = ty::Instance::mono(bx.tcx(), def_id);
@@ -496,7 +510,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
let llfn = bx.get_fn_addr(instance);
// Codegen the actual panic invoke/call.
- helper.do_call(self, &mut bx, fn_abi, llfn, &[location], None, None);
+ helper.do_call(self, &mut bx, fn_abi, llfn, &[], None, None);
}
/// Returns `true` if this is indeed a panic intrinsic and codegen is done.
@@ -1398,6 +1412,33 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
})
}
+ fn double_unwind_guard(&mut self) -> Bx::BasicBlock {
+ self.double_unwind_guard.unwrap_or_else(|| {
+ assert!(!base::wants_msvc_seh(self.cx.sess()));
+
+ let mut bx = self.new_block("abort");
+ let llpersonality = self.cx.eh_personality();
+ let llretty = self.landing_pad_type();
+ bx.cleanup_landing_pad(llretty, llpersonality);
+
+ let def_id = common::langcall(bx.tcx(), None, "", LangItem::PanicNoUnwind);
+ let instance = ty::Instance::mono(bx.tcx(), def_id);
+ let fn_abi = bx.fn_abi_of_instance(instance, ty::List::empty());
+ let fn_ptr = bx.get_fn_addr(instance);
+ let fn_ty = bx.fn_decl_backend_type(&fn_abi);
+
+ let llret = bx.call(fn_ty, fn_ptr, &[], None);
+ bx.apply_attrs_callsite(&fn_abi, llret);
+ bx.apply_attrs_to_cleanup_callsite(llret);
+
+ bx.unreachable();
+ let llbb = bx.llbb();
+
+ self.double_unwind_guard = Some(llbb);
+ llbb
+ })
+ }
+
// FIXME(eddyb) replace with `build_sibling_block`/`append_sibling_block`
// (which requires having a `Bx` already, and not all callers do).
fn new_block(&self, name: &str) -> Bx {
diff --git a/compiler/rustc_codegen_ssa/src/mir/mod.rs b/compiler/rustc_codegen_ssa/src/mir/mod.rs
index 814e4d626e119..4f05d02526e55 100644
--- a/compiler/rustc_codegen_ssa/src/mir/mod.rs
+++ b/compiler/rustc_codegen_ssa/src/mir/mod.rs
@@ -62,6 +62,9 @@ pub struct FunctionCx<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> {
/// Cached unreachable block
unreachable_block: Option<Bx::BasicBlock>,
+ /// Cached double unwind guarding block
+ double_unwind_guard: Option<Bx::BasicBlock>,
+
/// The location where each MIR arg/var/tmp/ret is stored. This is
/// usually an `PlaceRef` representing an alloca, but not always:
/// sometimes we can skip the alloca and just store the value
@@ -169,6 +172,7 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
personality_slot: None,
cached_llbbs,
unreachable_block: None,
+ double_unwind_guard: None,
cleanup_kinds,
landing_pads: IndexVec::from_elem(None, mir.basic_blocks()),
funclets: IndexVec::from_fn_n(|_| None, mir.basic_blocks().len()),
diff --git a/library/core/src/panicking.rs b/library/core/src/panicking.rs
index 0798076411ac4..2b720fc4192a7 100644
--- a/library/core/src/panicking.rs
+++ b/library/core/src/panicking.rs
@@ -87,8 +87,7 @@ fn panic_bounds_check(index: usize, len: usize) -> ! {
#[cfg(not(bootstrap))]
#[cold]
-#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))]
-#[track_caller]
+#[inline(never)]
#[lang = "panic_no_unwind"] // needed by codegen for panic in nounwind function
fn panic_no_unwind() -> ! {
if cfg!(feature = "panic_immediate_abort") {
From 42b41da33823f10aa39ca594c76fe13be3f53e63 Mon Sep 17 00:00:00 2001
From: Gary Guo <gary@garyguo.net>
Date: Sun, 13 Feb 2022 13:19:57 +0000
Subject: [PATCH 2/5] Fix codegen tests
---
src/test/codegen/drop.rs | 11 ++++++-----
src/test/codegen/unwind-landingpad-cold.rs | 2 +-
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/test/codegen/drop.rs b/src/test/codegen/drop.rs
index 543b81b0b6246..5e9ef23784349 100644
--- a/src/test/codegen/drop.rs
+++ b/src/test/codegen/drop.rs
@@ -23,13 +23,14 @@ pub fn droppy() {
// FIXME(eddyb) the `void @` forces a match on the instruction, instead of the
// comment, that's `; call core::ptr::drop_in_place::<drop::SomeUniqueName>`
// for the `v0` mangling, should switch to matching on that once `legacy` is gone.
-// CHECK-NOT: invoke void @{{.*}}drop_in_place{{.*}}SomeUniqueName
-// CHECK: call void @{{.*}}drop_in_place{{.*}}SomeUniqueName
-// CHECK: call void @{{.*}}drop_in_place{{.*}}SomeUniqueName
// CHECK-NOT: call void @{{.*}}drop_in_place{{.*}}SomeUniqueName
// CHECK: invoke void @{{.*}}drop_in_place{{.*}}SomeUniqueName
-// CHECK-NOT: invoke void @{{.*}}drop_in_place{{.*}}SomeUniqueName
-// CHECK: call void @{{.*}}drop_in_place{{.*}}SomeUniqueName
+// CHECK-NOT: call void @{{.*}}drop_in_place{{.*}}SomeUniqueName
+// CHECK: invoke void @{{.*}}drop_in_place{{.*}}SomeUniqueName
+// CHECK-NOT: call void @{{.*}}drop_in_place{{.*}}SomeUniqueName
+// CHECK: invoke void @{{.*}}drop_in_place{{.*}}SomeUniqueName
+// CHECK-NOT: call void @{{.*}}drop_in_place{{.*}}SomeUniqueName
+// CHECK: invoke void @{{.*}}drop_in_place{{.*}}SomeUniqueName
// CHECK-NOT: call void @{{.*}}drop_in_place{{.*}}SomeUniqueName
// CHECK: invoke void @{{.*}}drop_in_place{{.*}}SomeUniqueName
// CHECK-NOT: invoke void @{{.*}}drop_in_place{{.*}}SomeUniqueName
diff --git a/src/test/codegen/unwind-landingpad-cold.rs b/src/test/codegen/unwind-landingpad-cold.rs
index 650d5b230f4c2..432f60f335b41 100644
--- a/src/test/codegen/unwind-landingpad-cold.rs
+++ b/src/test/codegen/unwind-landingpad-cold.rs
@@ -6,7 +6,7 @@
// get the `cold` attribute.
// CHECK-LABEL: @check_cold
-// CHECK: call void {{.+}}drop_in_place{{.+}} [[ATTRIBUTES:#[0-9]+]]
+// CHECK: invoke void {{.+}}drop_in_place{{.+}} [[ATTRIBUTES:#[0-9]+]]
// CHECK: attributes [[ATTRIBUTES]] = { cold }
#[no_mangle]
pub fn check_cold(f: fn(), x: Box<u32>) {
From 0804e47792606b97933d019d75219d170ef9a891 Mon Sep 17 00:00:00 2001
From: Gary Guo <gary@garyguo.net>
Date: Sun, 13 Feb 2022 17:26:38 +0000
Subject: [PATCH 3/5] Add test for foreign double unwind
---
.../foreign-double-unwind/Makefile | 10 ++++++
.../foreign-double-unwind/foo.cpp | 33 +++++++++++++++++++
.../foreign-double-unwind/foo.rs | 26 +++++++++++++++
3 files changed, 69 insertions(+)
create mode 100644 src/test/run-make-fulldeps/foreign-double-unwind/Makefile
create mode 100644 src/test/run-make-fulldeps/foreign-double-unwind/foo.cpp
create mode 100644 src/test/run-make-fulldeps/foreign-double-unwind/foo.rs
diff --git a/src/test/run-make-fulldeps/foreign-double-unwind/Makefile b/src/test/run-make-fulldeps/foreign-double-unwind/Makefile
new file mode 100644
index 0000000000000..27cf4d19ce067
--- /dev/null
+++ b/src/test/run-make-fulldeps/foreign-double-unwind/Makefile
@@ -0,0 +1,10 @@
+-include ../tools.mk
+
+all: foo
+ $(call RUN,foo) | $(CGREP) -v unreachable
+
+foo: foo.rs $(call NATIVE_STATICLIB,foo)
+ $(RUSTC) $< -lfoo $(EXTRARSCXXFLAGS)
+
+$(TMPDIR)/libfoo.o: foo.cpp
+ $(call COMPILE_OBJ_CXX,$@,$<)
diff --git a/src/test/run-make-fulldeps/foreign-double-unwind/foo.cpp b/src/test/run-make-fulldeps/foreign-double-unwind/foo.cpp
new file mode 100644
index 0000000000000..69a8f11c2db78
--- /dev/null
+++ b/src/test/run-make-fulldeps/foreign-double-unwind/foo.cpp
@@ -0,0 +1,33 @@
+#include <cstdio>
+#include <exception>
+
+void println(const char* s) {
+ puts(s);
+ fflush(stdout);
+}
+
+struct outer_exception {};
+struct inner_exception {};
+
+extern "C" {
+ void throw_cxx_exception() {
+ if (std::uncaught_exception()) {
+ println("throwing inner C++ exception");
+ throw inner_exception();
+ } else {
+ println("throwing outer C++ exception");
+ throw outer_exception();
+ }
+ }
+
+ void cxx_catch_callback(void (*cb)()) {
+ try {
+ cb();
+ println("unreachable: callback returns");
+ } catch (outer_exception) {
+ println("unreachable: caught outer exception in catch (...)");
+ } catch (inner_exception) {
+ println("unreachable: caught inner exception in catch (...)");
+ }
+ }
+}
diff --git a/src/test/run-make-fulldeps/foreign-double-unwind/foo.rs b/src/test/run-make-fulldeps/foreign-double-unwind/foo.rs
new file mode 100644
index 0000000000000..cae8aa9402d88
--- /dev/null
+++ b/src/test/run-make-fulldeps/foreign-double-unwind/foo.rs
@@ -0,0 +1,26 @@
+// Tests that C++ double unwinding through Rust code will be properly guarded
+// against instead of exhibiting undefined behaviour.
+
+#![feature(c_unwind)]
+
+extern "C-unwind" {
+ fn throw_cxx_exception();
+ fn cxx_catch_callback(cb: extern "C-unwind" fn());
+}
+
+struct ThrowOnDrop;
+
+impl Drop for ThrowOnDrop {
+ fn drop(&mut self) {
+ unsafe { throw_cxx_exception() };
+ }
+}
+
+extern "C-unwind" fn test_double_unwind() {
+ let _a = ThrowOnDrop;
+ let _b = ThrowOnDrop;
+}
+
+fn main() {
+ unsafe { cxx_catch_callback(test_double_unwind) };
+}
From 3406db94168f4eec9008df82427779370bd609d5 Mon Sep 17 00:00:00 2001
From: Gary Guo <gary@garyguo.net>
Date: Mon, 14 Feb 2022 19:44:24 +0000
Subject: [PATCH 4/5] Fix missing dbg info
---
compiler/rustc_codegen_ssa/src/mir/block.rs | 2 ++
1 file changed, 2 insertions(+)
diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs
index 5cbbb34d2e625..33f883f9b6c30 100644
--- a/compiler/rustc_codegen_ssa/src/mir/block.rs
+++ b/compiler/rustc_codegen_ssa/src/mir/block.rs
@@ -1417,6 +1417,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
assert!(!base::wants_msvc_seh(self.cx.sess()));
let mut bx = self.new_block("abort");
+ self.set_debug_loc(&mut bx, mir::SourceInfo::outermost(self.mir.span));
+
let llpersonality = self.cx.eh_personality();
let llretty = self.landing_pad_type();
bx.cleanup_landing_pad(llretty, llpersonality);
From 7d683f525a6cd807f7bcb1293c71bce613936e79 Mon Sep 17 00:00:00 2001
From: Gary Guo <gary@garyguo.net>
Date: Fri, 18 Feb 2022 18:19:30 +0000
Subject: [PATCH 5/5] Fix codegen test for MSVC
---
src/test/codegen/drop.rs | 13 +------------
src/test/codegen/unwind-landingpad-cold.rs | 2 +-
2 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/src/test/codegen/drop.rs b/src/test/codegen/drop.rs
index 5e9ef23784349..994028271583f 100644
--- a/src/test/codegen/drop.rs
+++ b/src/test/codegen/drop.rs
@@ -23,18 +23,7 @@ pub fn droppy() {
// FIXME(eddyb) the `void @` forces a match on the instruction, instead of the
// comment, that's `; call core::ptr::drop_in_place::<drop::SomeUniqueName>`
// for the `v0` mangling, should switch to matching on that once `legacy` is gone.
-// CHECK-NOT: call void @{{.*}}drop_in_place{{.*}}SomeUniqueName
-// CHECK: invoke void @{{.*}}drop_in_place{{.*}}SomeUniqueName
-// CHECK-NOT: call void @{{.*}}drop_in_place{{.*}}SomeUniqueName
-// CHECK: invoke void @{{.*}}drop_in_place{{.*}}SomeUniqueName
-// CHECK-NOT: call void @{{.*}}drop_in_place{{.*}}SomeUniqueName
-// CHECK: invoke void @{{.*}}drop_in_place{{.*}}SomeUniqueName
-// CHECK-NOT: call void @{{.*}}drop_in_place{{.*}}SomeUniqueName
-// CHECK: invoke void @{{.*}}drop_in_place{{.*}}SomeUniqueName
-// CHECK-NOT: call void @{{.*}}drop_in_place{{.*}}SomeUniqueName
-// CHECK: invoke void @{{.*}}drop_in_place{{.*}}SomeUniqueName
-// CHECK-NOT: invoke void @{{.*}}drop_in_place{{.*}}SomeUniqueName
-// CHECK: call void @{{.*}}drop_in_place{{.*}}SomeUniqueName
+// CHECK-COUNT-6: {{(call|invoke) void @.*}}drop_in_place{{.*}}SomeUniqueName
// CHECK-NOT: {{(call|invoke) void @.*}}drop_in_place{{.*}}SomeUniqueName
// The next line checks for the } that ends the function definition
// CHECK-LABEL: {{^[}]}}
diff --git a/src/test/codegen/unwind-landingpad-cold.rs b/src/test/codegen/unwind-landingpad-cold.rs
index 432f60f335b41..aa00b7936541e 100644
--- a/src/test/codegen/unwind-landingpad-cold.rs
+++ b/src/test/codegen/unwind-landingpad-cold.rs
@@ -6,7 +6,7 @@
// get the `cold` attribute.
// CHECK-LABEL: @check_cold
-// CHECK: invoke void {{.+}}drop_in_place{{.+}} [[ATTRIBUTES:#[0-9]+]]
+// CHECK: {{(call|invoke) void .+}}drop_in_place{{.+}} [[ATTRIBUTES:#[0-9]+]]
// CHECK: attributes [[ATTRIBUTES]] = { cold }
#[no_mangle]
pub fn check_cold(f: fn(), x: Box<u32>) {