From a3c2305ade9c990b861062413995e10656c89306 Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Mon, 6 Aug 2018 15:27:22 -0400 Subject: [PATCH 01/43] Initial attempt at designing a safe Context type, getcontext(), and setcontext() --- .gitignore | 2 ++ Cargo.toml | 6 +++++ src/lib.rs | 6 +++++ src/ucontext.rs | 62 +++++++++++++++++++++++++++++++++++++++++++++++++ src/uninit.rs | 9 +++++++ 5 files changed, 85 insertions(+) create mode 100644 .gitignore create mode 100644 Cargo.toml create mode 100644 src/lib.rs create mode 100644 src/ucontext.rs create mode 100644 src/uninit.rs diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..d753a25 --- /dev/null +++ b/.gitignore @@ -0,0 +1,2 @@ +target/ +*.lock diff --git a/Cargo.toml b/Cargo.toml new file mode 100644 index 0000000..15f735b --- /dev/null +++ b/Cargo.toml @@ -0,0 +1,6 @@ +[package] +name = "ucontext" +version = "0.0.0" + +[dependencies] +libc = "*" diff --git a/src/lib.rs b/src/lib.rs new file mode 100644 index 0000000..85241bd --- /dev/null +++ b/src/lib.rs @@ -0,0 +1,6 @@ +extern crate libc; + +mod ucontext; +mod uninit; + +pub use ucontext::*; diff --git a/src/ucontext.rs b/src/ucontext.rs new file mode 100644 index 0000000..c300fb2 --- /dev/null +++ b/src/ucontext.rs @@ -0,0 +1,62 @@ +use libc::ucontext_t; +use std::io::Error; +use std::io::Result; +use std::rc::Rc; +use uninit::Uninit; + +pub struct Context { + context: ucontext_t, + ready: bool, + guard: Rc<()>, +} + +impl Context { + fn new() -> Self { + Self { + context: ucontext_t::uninit(), + ready: false, + guard: Rc::new(()), + } + + } +} + +pub fn getcontext(a: A, b: B) -> Result<()> { + use libc::getcontext; + + let mut context = Context::new(); + let guard = Rc::downgrade(&context.guard); + if unsafe { + getcontext(&mut context.context) + } != 0 { + Err(Error::last_os_error())?; + } + + if ! context.ready { + context.ready = true; + a(context); + } else { + b(); + } + + drop(guard); + Ok(()) +} + +pub fn setcontext(context: Context) -> Option { + use libc::setcontext; + + let guarded = Rc::weak_count(&context.guard); + if guarded == 0 { + None?; + } + debug_assert!(guarded == 1); + + drop(context.guard); + unsafe { + setcontext(&context.context); + } + Some(Error::last_os_error()) +} + +unsafe impl Uninit for ucontext_t {} diff --git a/src/uninit.rs b/src/uninit.rs new file mode 100644 index 0000000..10555c9 --- /dev/null +++ b/src/uninit.rs @@ -0,0 +1,9 @@ +pub unsafe trait Uninit: Sized { + fn uninit() -> Self { + use std::mem::uninitialized; + + unsafe { + uninitialized() + } + } +} From 99460bac3c7865bdbe8b51d709f5920fe415086d Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Mon, 6 Aug 2018 15:28:29 -0400 Subject: [PATCH 02/43] Initial safety, correctness, and leakiness checks --- src/lib.rs | 2 ++ src/tests.rs | 37 +++++++++++++++++++++++++++++++++++++ tests/tests.rs | 26 ++++++++++++++++++++++++++ 3 files changed, 65 insertions(+) create mode 100644 src/tests.rs create mode 100644 tests/tests.rs diff --git a/src/lib.rs b/src/lib.rs index 85241bd..631ca71 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,5 +1,7 @@ extern crate libc; +#[doc(hidden)] +mod tests; mod ucontext; mod uninit; diff --git a/src/tests.rs b/src/tests.rs new file mode 100644 index 0000000..ae5a73d --- /dev/null +++ b/src/tests.rs @@ -0,0 +1,37 @@ +#![doc(test(attr(deny(warnings))))] + +//! ```compile_fail +//! use std::mem::uninitialized; +//! use ucontext::Context; +//! +//! fn assert_clone(_: T) {} +//! +//! let context: Context = unsafe { +//! uninitialized() +//! }; +//! assert_clone(context); +//! ``` + +//! ```compile_fail +//! use std::mem::uninitialized; +//! use ucontext::Context; + +//! fn assert_send(_: T) {} +//! +//! let context: Context = unsafe { +//! uninitialized() +//! }; +//! assert_send(context); +//! ``` + +//! ```compile_fail +//! use std::mem::uninitialized; +//! use ucontext::Context; +//! +//! fn assert_sync(_: T) {} +//! +//! let context: Context = unsafe { +//! uninitialized() +//! }; +//! assert_sync(context); +//! ``` diff --git a/tests/tests.rs b/tests/tests.rs new file mode 100644 index 0000000..c4cde4b --- /dev/null +++ b/tests/tests.rs @@ -0,0 +1,26 @@ +extern crate ucontext; + +#[test] +fn getcontext_donothing() { + use ucontext::getcontext; + + let mut reached = false; + getcontext(|_| reached = true, || unreachable!()).unwrap(); + assert!(reached); +} + +#[test] +fn getcontext_setcontext() { + use ucontext::getcontext; + use ucontext::setcontext; + + let mut reached = false; + getcontext( + |context| { + setcontext(context); + unreachable!(); + }, + || reached = true, + ).unwrap(); + assert!(reached); +} From 7deddbf2d23284ac11ebc80e4c2893c99ccd3603 Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Mon, 6 Aug 2018 23:06:06 -0400 Subject: [PATCH 03/43] Add script and scaffolding for running LLVM sanitizers as a supplement to valgrind, which appears to be producing false positives as a result of the stack switching. --- .gitignore | 1 + salgrind | 7 +++++++ tests/tests.rs | 11 +++++++++-- 3 files changed, 17 insertions(+), 2 deletions(-) create mode 100755 salgrind diff --git a/.gitignore b/.gitignore index d753a25..bb3e3a5 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ +src/main.rs target/ *.lock diff --git a/salgrind b/salgrind new file mode 100755 index 0000000..a5965a2 --- /dev/null +++ b/salgrind @@ -0,0 +1,7 @@ +#!/bin/sh +set -e + +trap "rm -f src/main.rs target/debug/ucontext" EXIT +ln -s ../tests/tests.rs src/main.rs +cargo rustc --bins -- -Zsanitizer="$1" +target/debug/ucontext diff --git a/tests/tests.rs b/tests/tests.rs index c4cde4b..b7133ef 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -1,6 +1,13 @@ +#![cfg_attr(not(test), allow(dead_code))] + extern crate ucontext; -#[test] +fn main() { + getcontext_donothing(); + getcontext_setcontext(); +} + +#[cfg_attr(test, test)] fn getcontext_donothing() { use ucontext::getcontext; @@ -9,7 +16,7 @@ fn getcontext_donothing() { assert!(reached); } -#[test] +#[cfg_attr(test, test)] fn getcontext_setcontext() { use ucontext::getcontext; use ucontext::setcontext; From 71e99b651dee38a400607e3f864136a984ae11e0 Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Tue, 7 Aug 2018 01:32:04 -0400 Subject: [PATCH 04/43] src/ucontext.rs: Change getcontext() signature to reflect the fact that a context may be resumed multiple times --- src/ucontext.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ucontext.rs b/src/ucontext.rs index c300fb2..7b0c434 100644 --- a/src/ucontext.rs +++ b/src/ucontext.rs @@ -21,7 +21,7 @@ impl Context { } } -pub fn getcontext(a: A, b: B) -> Result<()> { +pub fn getcontext(a: A, mut b: B) -> Result<()> { use libc::getcontext; let mut context = Context::new(); From d33abc8c8433c91be60aa876679ee7a402d6394a Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Tue, 7 Aug 2018 01:34:35 -0400 Subject: [PATCH 05/43] src: Fix a lifetime bug by moving the resume flag onto the stack Storing it in the context was incorrect because it is dropped at the end of the first closure! --- src/lib.rs | 1 + src/ucontext.rs | 13 +++++++++---- src/volatile.rs | 25 +++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 4 deletions(-) create mode 100644 src/volatile.rs diff --git a/src/lib.rs b/src/lib.rs index 631ca71..255f233 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,5 +4,6 @@ extern crate libc; mod tests; mod ucontext; mod uninit; +mod volatile; pub use ucontext::*; diff --git a/src/ucontext.rs b/src/ucontext.rs index 7b0c434..7b4ee81 100644 --- a/src/ucontext.rs +++ b/src/ucontext.rs @@ -6,7 +6,6 @@ use uninit::Uninit; pub struct Context { context: ucontext_t, - ready: bool, guard: Rc<()>, } @@ -14,7 +13,6 @@ impl Context { fn new() -> Self { Self { context: ucontext_t::uninit(), - ready: false, guard: Rc::new(()), } @@ -23,8 +21,15 @@ impl Context { pub fn getcontext(a: A, mut b: B) -> Result<()> { use libc::getcontext; + use volatile::VolBool; let mut context = Context::new(); + + // Storing this flag on the stack is not unsound because guard enforces the invariant that + // this stack frame outlives any resumable context. Storing it on the stack is not a leaky + // because client code that never resumes the context was already responsible for cleaning + // up this function's stack. + let mut unused = VolBool::new(true); let guard = Rc::downgrade(&context.guard); if unsafe { getcontext(&mut context.context) @@ -32,8 +37,8 @@ pub fn getcontext(a: A, mut b: B) -> Result<()> Err(Error::last_os_error())?; } - if ! context.ready { - context.ready = true; + if unused.load() { + unused.store(false); a(context); } else { b(); diff --git a/src/volatile.rs b/src/volatile.rs new file mode 100644 index 0000000..4120094 --- /dev/null +++ b/src/volatile.rs @@ -0,0 +1,25 @@ +pub struct VolBool (bool); + +impl VolBool { + pub fn new(val: bool) -> Self { + VolBool (val) + } + + pub fn load(&self) -> bool { + use std::ptr::read_volatile; + + let VolBool (val) = self; + unsafe { + read_volatile(val) + } + } + + pub fn store(&mut self, val: bool) { + use std::ptr::write_volatile; + + let VolBool (ue) = self; + unsafe { + write_volatile(ue, val); + } + } +} From 2fb9e5bb78a869fdb09ceeeaf0c877f81b8b39db Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Tue, 7 Aug 2018 03:12:31 -0400 Subject: [PATCH 06/43] src/ucontext.rs: More descriptive error if this assertion somehow fails --- src/ucontext.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ucontext.rs b/src/ucontext.rs index 7b4ee81..f6f2a0c 100644 --- a/src/ucontext.rs +++ b/src/ucontext.rs @@ -55,7 +55,7 @@ pub fn setcontext(context: Context) -> Option { if guarded == 0 { None?; } - debug_assert!(guarded == 1); + debug_assert!(guarded == 1, "setcontext() found multiple corresponding stack frames (?)"); drop(context.guard); unsafe { From 181ec447c204d6de777ce528ddcff4e6b40b218c Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Tue, 7 Aug 2018 03:13:21 -0400 Subject: [PATCH 07/43] src: Perform platform-specific pointer fixes in setcontext() to accommodate possible self-reference --- src/invar.rs | 3 +++ src/lib.rs | 2 ++ src/platform.rs | 28 ++++++++++++++++++++++++++++ src/ucontext.rs | 6 ++++-- 4 files changed, 37 insertions(+), 2 deletions(-) create mode 100644 src/invar.rs create mode 100644 src/platform.rs diff --git a/src/invar.rs b/src/invar.rs new file mode 100644 index 0000000..664e9f7 --- /dev/null +++ b/src/invar.rs @@ -0,0 +1,3 @@ +pub trait MoveInvariant { + fn after_move(&mut self) {} +} diff --git a/src/lib.rs b/src/lib.rs index 255f233..775309c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,5 +1,7 @@ extern crate libc; +mod invar; +mod platform; #[doc(hidden)] mod tests; mod ucontext; diff --git a/src/platform.rs b/src/platform.rs new file mode 100644 index 0000000..e4b3d1b --- /dev/null +++ b/src/platform.rs @@ -0,0 +1,28 @@ +use invar::MoveInvariant; +use libc::ucontext_t; +pub use self::imp::*; + +#[cfg(target_os = "linux")] +mod imp { + use libc::_libc_fpstate; + use super::*; + + impl MoveInvariant for ucontext_t { + fn after_move(&mut self) { + let start = self as *mut ucontext_t; + let end = unsafe { + start.add(1) + } as *mut _libc_fpstate; + self.uc_mcontext.fpregs = unsafe { + end.sub(1) + }; + } + } +} + +#[cfg(not(target_os = "linux"))] +mod imp { + use super::*; + + impl MoveInvariant for ucontext_t {} +} diff --git a/src/ucontext.rs b/src/ucontext.rs index f6f2a0c..d02afe1 100644 --- a/src/ucontext.rs +++ b/src/ucontext.rs @@ -1,3 +1,4 @@ +use invar::MoveInvariant; use libc::ucontext_t; use std::io::Error; use std::io::Result; @@ -48,7 +49,7 @@ pub fn getcontext(a: A, mut b: B) -> Result<()> Ok(()) } -pub fn setcontext(context: Context) -> Option { +pub fn setcontext(mut context: Context) -> Option { use libc::setcontext; let guarded = Rc::weak_count(&context.guard); @@ -56,8 +57,9 @@ pub fn setcontext(context: Context) -> Option { None?; } debug_assert!(guarded == 1, "setcontext() found multiple corresponding stack frames (?)"); - drop(context.guard); + + context.context.after_move(); unsafe { setcontext(&context.context); } From 5daa86db159da42a6eab55615d09d74da039b1c9 Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Tue, 7 Aug 2018 03:34:15 -0400 Subject: [PATCH 08/43] src/ucontext.rs: Document public interface so far --- src/ucontext.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/ucontext.rs b/src/ucontext.rs index d02afe1..fea18c2 100644 --- a/src/ucontext.rs +++ b/src/ucontext.rs @@ -5,6 +5,7 @@ use std::io::Result; use std::rc::Rc; use uninit::Uninit; +/// A continuation that may be resumed using `setcontext()`. pub struct Context { context: ucontext_t, guard: Rc<()>, @@ -20,6 +21,9 @@ impl Context { } } +/// Calls `a()`, which may perform a `setcontext()` on its argument. If and only if it does so, +/// `b()` is executed before this function returns. However, `b()` may also perform such a +/// `setcontext()`, in which case it is restarted from the beginning, preserving in-memory changes. pub fn getcontext(a: A, mut b: B) -> Result<()> { use libc::getcontext; use volatile::VolBool; @@ -27,7 +31,7 @@ pub fn getcontext(a: A, mut b: B) -> Result<()> let mut context = Context::new(); // Storing this flag on the stack is not unsound because guard enforces the invariant that - // this stack frame outlives any resumable context. Storing it on the stack is not a leaky + // this stack frame outlives any resumable context. Storing it on the stack is not leaky // because client code that never resumes the context was already responsible for cleaning // up this function's stack. let mut unused = VolBool::new(true); @@ -49,6 +53,8 @@ pub fn getcontext(a: A, mut b: B) -> Result<()> Ok(()) } +/// Attempts to resume `context`, never returning on success. Otherwise, returns `None` if +/// `context`'s stack frame has expired or `Some` to indicate a platform error. pub fn setcontext(mut context: Context) -> Option { use libc::setcontext; From b7d1e3e7559c3ec10f4751c644a1c3a28896b56b Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Tue, 7 Aug 2018 03:48:18 -0400 Subject: [PATCH 09/43] Revert "src/ucontext.rs: Change getcontext() signature to reflect the fact that a context may be resumed multiple times" This reverts commit 8863dd3d25031c1bbc83976b25b863916e7bef07, because he described scenario of resuming the same context multiple times is impossible because setcontext() accepts takes ownership. --- src/ucontext.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/ucontext.rs b/src/ucontext.rs index fea18c2..b6402dc 100644 --- a/src/ucontext.rs +++ b/src/ucontext.rs @@ -22,9 +22,8 @@ impl Context { } /// Calls `a()`, which may perform a `setcontext()` on its argument. If and only if it does so, -/// `b()` is executed before this function returns. However, `b()` may also perform such a -/// `setcontext()`, in which case it is restarted from the beginning, preserving in-memory changes. -pub fn getcontext(a: A, mut b: B) -> Result<()> { +/// `b()` is executed before this function returns. +pub fn getcontext(a: A, b: B) -> Result<()> { use libc::getcontext; use volatile::VolBool; From bb39d40f8273f6b6a935dd8ab66fd772bc0cdf86 Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Tue, 7 Aug 2018 04:10:08 -0400 Subject: [PATCH 10/43] tests/tests.rs: Add a test to ensure contexts cannot be resumed once the stack frame has been destroyed --- tests/tests.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/tests.rs b/tests/tests.rs index b7133ef..72c13fb 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -5,6 +5,7 @@ extern crate ucontext; fn main() { getcontext_donothing(); getcontext_setcontext(); + getcontext_succeedatnothing(); } #[cfg_attr(test, test)] @@ -31,3 +32,13 @@ fn getcontext_setcontext() { ).unwrap(); assert!(reached); } + +#[cfg_attr(test, test)] +fn getcontext_succeedatnothing() { + use ucontext::getcontext; + use ucontext::setcontext; + + let mut invalid = None; + getcontext(|context| invalid = Some(context), || unreachable!()).unwrap(); + assert!(setcontext(invalid.unwrap()).is_none()); +} From eaaf491a8500ade6d2c63893f299538ba5c88eac Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Tue, 7 Aug 2018 04:10:55 -0400 Subject: [PATCH 11/43] tests/tests.rs: Verify that each test is making it through to the end as opposed to just never reaching its assertions! --- tests/tests.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/tests.rs b/tests/tests.rs index 72c13fb..8338365 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -8,6 +8,7 @@ fn main() { getcontext_succeedatnothing(); } +#[cfg_attr(test, should_panic(expected = "done"))] #[cfg_attr(test, test)] fn getcontext_donothing() { use ucontext::getcontext; @@ -15,8 +16,10 @@ fn getcontext_donothing() { let mut reached = false; getcontext(|_| reached = true, || unreachable!()).unwrap(); assert!(reached); + panic!("done"); } +#[cfg_attr(test, should_panic(expected = "done"))] #[cfg_attr(test, test)] fn getcontext_setcontext() { use ucontext::getcontext; @@ -31,8 +34,10 @@ fn getcontext_setcontext() { || reached = true, ).unwrap(); assert!(reached); + panic!("done"); } +#[cfg_attr(test, should_panic(expected = "done"))] #[cfg_attr(test, test)] fn getcontext_succeedatnothing() { use ucontext::getcontext; @@ -41,4 +46,5 @@ fn getcontext_succeedatnothing() { let mut invalid = None; getcontext(|context| invalid = Some(context), || unreachable!()).unwrap(); assert!(setcontext(invalid.unwrap()).is_none()); + panic!("done"); } From 0acc81ed8b759a5cf9249486aa358bff5c502671 Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Tue, 7 Aug 2018 12:42:01 -0400 Subject: [PATCH 12/43] src/ucontext.rs: Return the closure result from getcontext() --- src/ucontext.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/ucontext.rs b/src/ucontext.rs index b6402dc..8090c9f 100644 --- a/src/ucontext.rs +++ b/src/ucontext.rs @@ -23,7 +23,7 @@ impl Context { /// Calls `a()`, which may perform a `setcontext()` on its argument. If and only if it does so, /// `b()` is executed before this function returns. -pub fn getcontext(a: A, b: B) -> Result<()> { +pub fn getcontext T, B: FnOnce() -> T>(a: A, b: B) -> Result { use libc::getcontext; use volatile::VolBool; @@ -41,15 +41,16 @@ pub fn getcontext(a: A, b: B) -> Result<()> { Err(Error::last_os_error())?; } + let res; if unused.load() { unused.store(false); - a(context); + res = a(context); } else { - b(); + res = b(); } drop(guard); - Ok(()) + Ok(res) } /// Attempts to resume `context`, never returning on success. Otherwise, returns `None` if From 7620be28b4412eb71b4d8acfe05487306e4f2545 Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Wed, 8 Aug 2018 15:09:13 -0400 Subject: [PATCH 13/43] tests/tests.rs: Simplify test by using the return type to obviate the need for Option --- tests/tests.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/tests.rs b/tests/tests.rs index 8338365..f182f56 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -43,8 +43,7 @@ fn getcontext_succeedatnothing() { use ucontext::getcontext; use ucontext::setcontext; - let mut invalid = None; - getcontext(|context| invalid = Some(context), || unreachable!()).unwrap(); - assert!(setcontext(invalid.unwrap()).is_none()); + let invalid = getcontext(|context| context, || unreachable!()).unwrap(); + assert!(setcontext(invalid).is_none()); panic!("done"); } From 7bfe9db27e840f932238e9c04570872166cf5e12 Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Tue, 7 Aug 2018 12:46:34 -0400 Subject: [PATCH 14/43] tests/tests.rs: Add a test that demonstrates unsoundness --- tests/tests.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/tests.rs b/tests/tests.rs index f182f56..2deeb17 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -6,6 +6,7 @@ fn main() { getcontext_donothing(); getcontext_setcontext(); getcontext_succeedatnothing(); + getcontext_nested(); } #[cfg_attr(test, should_panic(expected = "done"))] @@ -47,3 +48,30 @@ fn getcontext_succeedatnothing() { assert!(setcontext(invalid).is_none()); panic!("done"); } + +#[cfg_attr(test, should_panic(expected = "done"))] +#[cfg_attr(test, test)] +fn getcontext_nested() { + use std::cell::Cell; + use ucontext::getcontext; + use ucontext::setcontext; + + let mut reached = true; + let context = Cell::new(None); + getcontext( + |outer| getcontext( + |inner| { + context.set(Some(inner)); + setcontext(outer); + unreachable!(); + }, + || reached = true, + ).unwrap(), + || { + setcontext(context.take().unwrap()); + unreachable!(); + }, + ).unwrap(); + assert!(reached); + panic!("done"); +} From 6ec56f12a9b320bf073a8bf938e045b83972317a Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Wed, 8 Aug 2018 12:03:43 -0400 Subject: [PATCH 15/43] tests/tests.rs: Mark the unsoundness test ignored because it's probably not fixable without withoutboats's pinning trick. --- tests/tests.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/tests.rs b/tests/tests.rs index 2deeb17..aebf19b 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -6,7 +6,7 @@ fn main() { getcontext_donothing(); getcontext_setcontext(); getcontext_succeedatnothing(); - getcontext_nested(); + //getcontext_nested(); } #[cfg_attr(test, should_panic(expected = "done"))] @@ -49,6 +49,7 @@ fn getcontext_succeedatnothing() { panic!("done"); } +#[cfg_attr(test, ignore)] #[cfg_attr(test, should_panic(expected = "done"))] #[cfg_attr(test, test)] fn getcontext_nested() { From 3c27621df5511a5fd2ef310aad123316ba320614 Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Wed, 8 Aug 2018 15:14:26 -0400 Subject: [PATCH 16/43] src/ucontext.rs: Make guard optional in preparation for implementing makecontext() --- src/ucontext.rs | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/ucontext.rs b/src/ucontext.rs index 8090c9f..f2e85d4 100644 --- a/src/ucontext.rs +++ b/src/ucontext.rs @@ -8,17 +8,21 @@ use uninit::Uninit; /// A continuation that may be resumed using `setcontext()`. pub struct Context { context: ucontext_t, - guard: Rc<()>, + guard: Option>, } impl Context { fn new() -> Self { Self { context: ucontext_t::uninit(), - guard: Rc::new(()), + guard: None, } } + + fn guard(&mut self) -> &Rc<()> { + self.guard.get_or_insert_with(|| Rc::new(())) + } } /// Calls `a()`, which may perform a `setcontext()` on its argument. If and only if it does so, @@ -34,7 +38,7 @@ pub fn getcontext T, B: FnOnce() -> T>(a: A, b: B) -> R // because client code that never resumes the context was already responsible for cleaning // up this function's stack. let mut unused = VolBool::new(true); - let guard = Rc::downgrade(&context.guard); + let guard = Rc::downgrade(context.guard()); if unsafe { getcontext(&mut context.context) } != 0 { @@ -58,12 +62,13 @@ pub fn getcontext T, B: FnOnce() -> T>(a: A, b: B) -> R pub fn setcontext(mut context: Context) -> Option { use libc::setcontext; - let guarded = Rc::weak_count(&context.guard); - if guarded == 0 { - None?; + if let Some(guard) = context.guard.take() { + let guarded = Rc::weak_count(&guard); + if guarded == 0 { + None?; + } + debug_assert!(guarded == 1, "setcontext() found multiple corresponding stack frames (?)"); } - debug_assert!(guarded == 1, "setcontext() found multiple corresponding stack frames (?)"); - drop(context.guard); context.context.after_move(); unsafe { From f2a4fe692078d5c96ab7228353863d42a7156428 Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Wed, 8 Aug 2018 15:14:45 -0400 Subject: [PATCH 17/43] src/ucontext.rs: Implement makecontext() --- src/lib.rs | 2 ++ src/ucontext.rs | 17 +++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index 775309c..14d4d4b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -8,4 +8,6 @@ mod ucontext; mod uninit; mod volatile; +pub use libc::MINSIGSTKSZ; +pub use libc::SIGSTKSZ; pub use ucontext::*; diff --git a/src/ucontext.rs b/src/ucontext.rs index f2e85d4..051a643 100644 --- a/src/ucontext.rs +++ b/src/ucontext.rs @@ -57,6 +57,23 @@ pub fn getcontext T, B: FnOnce() -> T>(a: A, b: B) -> R Ok(res) } +pub fn makecontext(function: extern "C" fn(), stack: &mut [u8], successor: Option<&mut Context>) -> Result { + use libc::makecontext; + + let mut context = getcontext(|context| context, || unreachable!())?; + context.guard.take(); + context.context.uc_stack.ss_sp = stack.as_mut_ptr() as _; + context.context.uc_stack.ss_size = stack.len(); + if let Some(successor) = successor { + context.context.uc_link = &mut successor.context; + } + + unsafe { + makecontext(&mut context.context, function, 0); + } + Ok(context) +} + /// Attempts to resume `context`, never returning on success. Otherwise, returns `None` if /// `context`'s stack frame has expired or `Some` to indicate a platform error. pub fn setcontext(mut context: Context) -> Option { From 33274b56cc863eaaa0a89d473d4cec3cadbc0862 Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Thu, 9 Aug 2018 02:04:42 -0400 Subject: [PATCH 18/43] tests/tests.rs: Add a makecontext() test --- tests/tests.rs | 44 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/tests/tests.rs b/tests/tests.rs index aebf19b..2b49ce2 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -2,18 +2,21 @@ extern crate ucontext; +use ucontext::getcontext; +use ucontext::makecontext; +use ucontext::setcontext; + fn main() { getcontext_donothing(); getcontext_setcontext(); getcontext_succeedatnothing(); //getcontext_nested(); + makecontext_setcontext(); } #[cfg_attr(test, should_panic(expected = "done"))] #[cfg_attr(test, test)] fn getcontext_donothing() { - use ucontext::getcontext; - let mut reached = false; getcontext(|_| reached = true, || unreachable!()).unwrap(); assert!(reached); @@ -23,9 +26,6 @@ fn getcontext_donothing() { #[cfg_attr(test, should_panic(expected = "done"))] #[cfg_attr(test, test)] fn getcontext_setcontext() { - use ucontext::getcontext; - use ucontext::setcontext; - let mut reached = false; getcontext( |context| { @@ -41,9 +41,6 @@ fn getcontext_setcontext() { #[cfg_attr(test, should_panic(expected = "done"))] #[cfg_attr(test, test)] fn getcontext_succeedatnothing() { - use ucontext::getcontext; - use ucontext::setcontext; - let invalid = getcontext(|context| context, || unreachable!()).unwrap(); assert!(setcontext(invalid).is_none()); panic!("done"); @@ -54,8 +51,6 @@ fn getcontext_succeedatnothing() { #[cfg_attr(test, test)] fn getcontext_nested() { use std::cell::Cell; - use ucontext::getcontext; - use ucontext::setcontext; let mut reached = true; let context = Cell::new(None); @@ -76,3 +71,32 @@ fn getcontext_nested() { assert!(reached); panic!("done"); } + +#[cfg_attr(test, should_panic(expected = "done"))] +#[cfg_attr(test, test)] +fn makecontext_setcontext() { + use std::cell::Cell; + use ucontext::MINSIGSTKSZ; + + thread_local! { + static REACHED: Cell = Cell::new(false); + } + + extern "C" fn call() { + REACHED.with(|reached| reached.set(true)); + } + + let mut reached = false; + getcontext( + |mut successor| { + let mut stack = [0u8; MINSIGSTKSZ]; + let predecessor = makecontext(call, &mut stack, Some(&mut successor)).unwrap(); + setcontext(predecessor); + unreachable!(); + }, + || reached = true, + ).unwrap(); + assert!(REACHED.with(|reached| reached.get())); + assert!(reached); + panic!("done"); +} From 9d1f98a399aecf4c3e2ba728f906363715e1415d Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Thu, 9 Aug 2018 02:07:09 -0400 Subject: [PATCH 19/43] src/ucontext.rs: Have makecontext() use platform getcontext() to avoid redundant/immediately reversed steps --- src/ucontext.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/ucontext.rs b/src/ucontext.rs index 051a643..dc0ee91 100644 --- a/src/ucontext.rs +++ b/src/ucontext.rs @@ -58,10 +58,16 @@ pub fn getcontext T, B: FnOnce() -> T>(a: A, b: B) -> R } pub fn makecontext(function: extern "C" fn(), stack: &mut [u8], successor: Option<&mut Context>) -> Result { + use libc::getcontext; use libc::makecontext; - let mut context = getcontext(|context| context, || unreachable!())?; - context.guard.take(); + let mut context = Context::new(); + if unsafe { + getcontext(&mut context.context) + } != 0 { + Err(Error::last_os_error())?; + } + context.context.uc_stack.ss_sp = stack.as_mut_ptr() as _; context.context.uc_stack.ss_size = stack.len(); if let Some(successor) = successor { From cd01127f775ebdca167b6859471916a773f7e450 Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Thu, 9 Aug 2018 18:08:53 -0400 Subject: [PATCH 20/43] src/ucontext.rs: Document makecontext() --- src/ucontext.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ucontext.rs b/src/ucontext.rs index dc0ee91..70c24d0 100644 --- a/src/ucontext.rs +++ b/src/ucontext.rs @@ -57,6 +57,8 @@ pub fn getcontext T, B: FnOnce() -> T>(a: A, b: B) -> R Ok(res) } +/// Configures a context to invoke `function()` on a separate `stack`, optionally resuming the +/// program at the `successor` context upon said function's return (or, by default, exiting). pub fn makecontext(function: extern "C" fn(), stack: &mut [u8], successor: Option<&mut Context>) -> Result { use libc::getcontext; use libc::makecontext; From bcfc7ea4ad1808069b28c392c098a28569c49b5e Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Thu, 9 Aug 2018 23:35:48 -0400 Subject: [PATCH 21/43] Support swapping out a context to jump back to a different place from a signal handler --- src/lib.rs | 1 + src/ucontext.rs | 28 ++++++++++++ tests/tests.rs | 118 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 147 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index 14d4d4b..90f0ed2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -8,6 +8,7 @@ mod ucontext; mod uninit; mod volatile; +pub use invar::MoveInvariant; pub use libc::MINSIGSTKSZ; pub use libc::SIGSTKSZ; pub use ucontext::*; diff --git a/src/ucontext.rs b/src/ucontext.rs index 70c24d0..e8e4c44 100644 --- a/src/ucontext.rs +++ b/src/ucontext.rs @@ -23,6 +23,34 @@ impl Context { fn guard(&mut self) -> &Rc<()> { self.guard.get_or_insert_with(|| Rc::new(())) } + + /// Exchange the functional portion of this context with another one. When called on a + /// a particular context within a signal handler, this causes that context to be restored + /// upon return from the handler. Note that the handler's original context is stored back + /// unguarded, but that a subsequent `setcontext()`s is UB according to SUSv2. + pub fn swap(&mut self, other: &mut ucontext_t) { + use std::mem::swap; + + let this = &mut self.context; + + this.after_move(); + swap(&mut this.uc_mcontext, &mut other.uc_mcontext); + let this_fp = unsafe { + this.uc_mcontext.fpregs.as_mut().unwrap() + }; + let other_fp = unsafe { + other.uc_mcontext.fpregs.as_mut().unwrap() + }; + swap(this_fp, other_fp); + swap(&mut this.uc_mcontext.fpregs, &mut other.uc_mcontext.fpregs); + + swap(&mut this.uc_flags, &mut other.uc_flags); + swap(&mut this.uc_link, &mut other.uc_link); + swap(&mut this.uc_stack, &mut other.uc_stack); + swap(&mut this.uc_sigmask, &mut other.uc_sigmask); + + self.guard.take(); + } } /// Calls `a()`, which may perform a `setcontext()` on its argument. If and only if it does so, diff --git a/tests/tests.rs b/tests/tests.rs index 2b49ce2..d34d7b1 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -1,7 +1,10 @@ #![cfg_attr(not(test), allow(dead_code))] +extern crate libc; extern crate ucontext; +use libc::ucontext_t; +use ucontext::Context; use ucontext::getcontext; use ucontext::makecontext; use ucontext::setcontext; @@ -12,6 +15,9 @@ fn main() { getcontext_succeedatnothing(); //getcontext_nested(); makecontext_setcontext(); + context_moveinvariant(); + context_swapinvariant(); + getcontext_killswap(); } #[cfg_attr(test, should_panic(expected = "done"))] @@ -100,3 +106,115 @@ fn makecontext_setcontext() { assert!(reached); panic!("done"); } + +fn ucontext(context: &mut Context) -> &mut ucontext_t { + use std::mem::transmute; + + unsafe { + transmute(context) + } +} + +fn uc_inbounds(within: *const ucontext_t, context: *const ucontext_t) -> bool { + within > context && within < unsafe { + context.add(1) + } +} + +#[cfg_attr(test, test)] +fn context_moveinvariant() { + use ucontext::MoveInvariant; + + let mut context = getcontext(|context| context, || unreachable!()).unwrap(); + let context = ucontext(&mut context); + context.after_move(); + assert!(uc_inbounds(context.uc_mcontext.fpregs as _, context)); +} + +#[cfg_attr(test, test)] +fn context_swapinvariant() { + use ucontext::MoveInvariant; + + let mut first = getcontext(|context| context, || unreachable!()).unwrap(); + let mut second = getcontext(|context| context, || unreachable!()).unwrap(); + + let second = ucontext(&mut second); + { + let first = ucontext(&mut first); + first.after_move(); + second.after_move(); + first.uc_link = first.uc_mcontext.fpregs as _; + second.uc_link = second.uc_mcontext.fpregs as _; + assert!(uc_inbounds(first.uc_link, first)); + assert!(uc_inbounds(second.uc_link, second)); + } + + first.swap(second); + let first = ucontext(&mut first); + assert!(uc_inbounds(first.uc_link, second)); + assert!(uc_inbounds(second.uc_link, first)); + assert!(uc_inbounds(first.uc_mcontext.fpregs as _, first)); + assert!(uc_inbounds(second.uc_mcontext.fpregs as _, second)); +} + +#[cfg_attr(test, should_panic(expected = "done"))] +#[cfg_attr(test, test)] +fn getcontext_killswap() { + use libc::SA_SIGINFO; + use libc::SIGUSR1; + use libc::pthread_kill; + use libc::pthread_self; + use libc::sigaction; + use libc::siginfo_t; + use std::cell::Cell; + use std::io::Error; + use std::mem::zeroed; + use std::os::raw::c_int; + use std::ptr::null_mut; + + thread_local! { + static CONTEXT: Cell> = Cell::new(None); + } + + extern "C" fn handler( + _: c_int, + _: Option<&mut siginfo_t>, + context: Option<&mut ucontext_t>, + ) { + let context = context.unwrap(); + CONTEXT.with(|global| { + let mut global = global.take().unwrap(); + global.swap(context); + }); + } + + let config = sigaction { + sa_flags: SA_SIGINFO, + sa_sigaction: handler as _, + sa_restorer: None, + sa_mask: unsafe { + zeroed() + }, + }; + if unsafe { + sigaction(SIGUSR1, &config, null_mut()) + } != 0 { + panic!(Error::last_os_error()); + } + + let mut reached = false; + getcontext( + |context| { + CONTEXT.with(|global| global.set(Some(context))); + unsafe { + pthread_kill(pthread_self(), SIGUSR1); + } + panic!(Error::last_os_error()); + }, + || { + reached = true; + } + ).unwrap(); + assert!(reached); + panic!("done"); +} From a74b289c0387ac0c05f6ed9e55aebd0be0e5820f Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Thu, 9 Aug 2018 23:36:45 -0400 Subject: [PATCH 22/43] src/ucontext.rs: Resolve segfault in signal handler by preserving the segment registers since x86_64 doesn't permit changing the corresponding control register from unprivileged code. --- src/ucontext.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/ucontext.rs b/src/ucontext.rs index e8e4c44..600287c 100644 --- a/src/ucontext.rs +++ b/src/ucontext.rs @@ -5,6 +5,8 @@ use std::io::Result; use std::rc::Rc; use uninit::Uninit; +const REG_CSGSFS: usize = 18; + /// A continuation that may be resumed using `setcontext()`. pub struct Context { context: ucontext_t, @@ -35,6 +37,8 @@ impl Context { this.after_move(); swap(&mut this.uc_mcontext, &mut other.uc_mcontext); + swap(&mut this.uc_mcontext.gregs[REG_CSGSFS], &mut other.uc_mcontext.gregs[REG_CSGSFS]); + let this_fp = unsafe { this.uc_mcontext.fpregs.as_mut().unwrap() }; From 82caca35412a8d7fc0d884c0082b97f86ce5649c Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Fri, 10 Aug 2018 05:23:04 -0400 Subject: [PATCH 23/43] src: Fix crashes due to dropping the uninitialized ucontext_t which is UB. --- src/lib.rs | 1 + src/ucontext.rs | 38 ++++++++++++++++++++++++++------------ src/zero.rs | 9 +++++++++ 3 files changed, 36 insertions(+), 12 deletions(-) create mode 100644 src/zero.rs diff --git a/src/lib.rs b/src/lib.rs index 90f0ed2..ec4f8a1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -7,6 +7,7 @@ mod tests; mod ucontext; mod uninit; mod volatile; +mod zero; pub use invar::MoveInvariant; pub use libc::MINSIGSTKSZ; diff --git a/src/ucontext.rs b/src/ucontext.rs index 600287c..8eaf423 100644 --- a/src/ucontext.rs +++ b/src/ucontext.rs @@ -4,6 +4,7 @@ use std::io::Error; use std::io::Result; use std::rc::Rc; use uninit::Uninit; +use zero::Zero; const REG_CSGSFS: usize = 18; @@ -14,6 +15,8 @@ pub struct Context { } impl Context { + /// NB: The returned object contains uninitialized data, and cannot be safely dropped until + /// it has either been initialized or zeroed! fn new() -> Self { Self { context: ucontext_t::uninit(), @@ -57,10 +60,29 @@ impl Context { } } +#[inline(always)] +fn checkpoint(context: &mut ucontext_t) -> Result<()> { + use libc::getcontext; + use std::mem::zeroed; + use std::ptr::write; + + if unsafe { + context.uc_mcontext.gregs = zeroed(); + getcontext(context) + } != 0 { + // Zero the uninitialized context before dropping it! + unsafe { + write(context, ucontext_t::zero()); + } + Err(Error::last_os_error())?; + } + + Ok(()) +} + /// Calls `a()`, which may perform a `setcontext()` on its argument. If and only if it does so, /// `b()` is executed before this function returns. pub fn getcontext T, B: FnOnce() -> T>(a: A, b: B) -> Result { - use libc::getcontext; use volatile::VolBool; let mut context = Context::new(); @@ -71,11 +93,7 @@ pub fn getcontext T, B: FnOnce() -> T>(a: A, b: B) -> R // up this function's stack. let mut unused = VolBool::new(true); let guard = Rc::downgrade(context.guard()); - if unsafe { - getcontext(&mut context.context) - } != 0 { - Err(Error::last_os_error())?; - } + checkpoint(&mut context.context)?; let res; if unused.load() { @@ -92,15 +110,10 @@ pub fn getcontext T, B: FnOnce() -> T>(a: A, b: B) -> R /// Configures a context to invoke `function()` on a separate `stack`, optionally resuming the /// program at the `successor` context upon said function's return (or, by default, exiting). pub fn makecontext(function: extern "C" fn(), stack: &mut [u8], successor: Option<&mut Context>) -> Result { - use libc::getcontext; use libc::makecontext; let mut context = Context::new(); - if unsafe { - getcontext(&mut context.context) - } != 0 { - Err(Error::last_os_error())?; - } + checkpoint(&mut context.context)?; context.context.uc_stack.ss_sp = stack.as_mut_ptr() as _; context.context.uc_stack.ss_size = stack.len(); @@ -135,3 +148,4 @@ pub fn setcontext(mut context: Context) -> Option { } unsafe impl Uninit for ucontext_t {} +unsafe impl Zero for ucontext_t {} diff --git a/src/zero.rs b/src/zero.rs new file mode 100644 index 0000000..3461cc4 --- /dev/null +++ b/src/zero.rs @@ -0,0 +1,9 @@ +pub unsafe trait Zero: Sized { + fn zero() -> Self { + use std::mem::zeroed; + + unsafe { + zeroed() + } + } +} From b811ce48a520b34d34290bd09e8c3d9b35dc8b61 Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Fri, 10 Aug 2018 05:25:54 -0400 Subject: [PATCH 24/43] tests/tests.rs: Add signal handler swap test for makecontext() as well --- tests/tests.rs | 57 ++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 44 insertions(+), 13 deletions(-) diff --git a/tests/tests.rs b/tests/tests.rs index d34d7b1..5efc9e9 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -17,7 +17,8 @@ fn main() { makecontext_setcontext(); context_moveinvariant(); context_swapinvariant(); - getcontext_killswap(); + killswap_getcontext(); + killswap_makecontext(); } #[cfg_attr(test, should_panic(expected = "done"))] @@ -157,9 +158,7 @@ fn context_swapinvariant() { assert!(uc_inbounds(second.uc_mcontext.fpregs as _, second)); } -#[cfg_attr(test, should_panic(expected = "done"))] -#[cfg_attr(test, test)] -fn getcontext_killswap() { +fn killswap() -> fn(Context) { use libc::SA_SIGINFO; use libc::SIGUSR1; use libc::pthread_kill; @@ -202,19 +201,51 @@ fn getcontext_killswap() { panic!(Error::last_os_error()); } + fn fun(context: Context) { + CONTEXT.with(|global| global.set(Some(context))); + unsafe { + pthread_kill(pthread_self(), SIGUSR1); + } + panic!(Error::last_os_error()); + } + + fun +} + +#[cfg_attr(test, should_panic(expected = "done"))] +#[cfg_attr(test, test)] +fn killswap_getcontext() { let mut reached = false; + getcontext(killswap(), || reached = true).unwrap(); + assert!(reached); + panic!("done"); +} + +#[cfg_attr(test, should_panic(expected = "done"))] +#[cfg_attr(test, test)] +fn killswap_makecontext() { + use std::cell::Cell; + use libc::MINSIGSTKSZ; + + thread_local! { + static REACHED: Cell = Cell::new(false); + } + + extern "C" fn call() { + REACHED.with(|reached| reached.set(true)); + } + + let mut reached = false; + let mut stack = [0u8; MINSIGSTKSZ]; getcontext( - |context| { - CONTEXT.with(|global| global.set(Some(context))); - unsafe { - pthread_kill(pthread_self(), SIGUSR1); - } - panic!(Error::last_os_error()); + |mut context| { + let context = makecontext(call, &mut stack, Some(&mut context)).unwrap(); + killswap()(context); + unreachable!(); }, - || { - reached = true; - } + || reached = true, ).unwrap(); assert!(reached); + assert!(REACHED.with(|reached| reached.get())); panic!("done"); } From a78bc4b187de95c50ec0b13d7dbadaa271c1c887 Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Fri, 10 Aug 2018 07:40:13 -0400 Subject: [PATCH 25/43] tests/tests.rs: Verify stack addresses in makecontext()/swap() test --- tests/tests.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/tests.rs b/tests/tests.rs index 5efc9e9..4946395 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -221,6 +221,15 @@ fn killswap_getcontext() { panic!("done"); } +fn stack_inbounds(within: &ucontext_t, stack: &[u8]) -> bool { + const REG_RSP: usize = 15; + + let within: *const _ = within.uc_mcontext.gregs[REG_RSP] as _; + within > stack.as_ptr() && within < unsafe { + stack.as_ptr().add(stack.len()) + } +} + #[cfg_attr(test, should_panic(expected = "done"))] #[cfg_attr(test, test)] fn killswap_makecontext() { @@ -239,6 +248,7 @@ fn killswap_makecontext() { let mut stack = [0u8; MINSIGSTKSZ]; getcontext( |mut context| { + assert!(! stack_inbounds(ucontext(&mut context), &stack)); let context = makecontext(call, &mut stack, Some(&mut context)).unwrap(); killswap()(context); unreachable!(); @@ -247,5 +257,8 @@ fn killswap_makecontext() { ).unwrap(); assert!(reached); assert!(REACHED.with(|reached| reached.get())); + + let mut context = getcontext(|context| context, || unreachable!()).unwrap(); + assert!(! stack_inbounds(ucontext(&mut context), &stack)); panic!("done"); } From 9cd8b346651958485ad76e7fd0fddb69d6aa01d2 Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Fri, 10 Aug 2018 07:40:44 -0400 Subject: [PATCH 26/43] tests/tests.rs: Guard completion panic!()s so they don't interfere with sanitizer runs --- tests/tests.rs | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/tests/tests.rs b/tests/tests.rs index 4946395..f8e6e0a 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -27,7 +27,9 @@ fn getcontext_donothing() { let mut reached = false; getcontext(|_| reached = true, || unreachable!()).unwrap(); assert!(reached); - panic!("done"); + if cfg!(test) { + panic!("done"); + } } #[cfg_attr(test, should_panic(expected = "done"))] @@ -42,7 +44,9 @@ fn getcontext_setcontext() { || reached = true, ).unwrap(); assert!(reached); - panic!("done"); + if cfg!(test) { + panic!("done"); + } } #[cfg_attr(test, should_panic(expected = "done"))] @@ -50,7 +54,9 @@ fn getcontext_setcontext() { fn getcontext_succeedatnothing() { let invalid = getcontext(|context| context, || unreachable!()).unwrap(); assert!(setcontext(invalid).is_none()); - panic!("done"); + if cfg!(test) { + panic!("done"); + } } #[cfg_attr(test, ignore)] @@ -76,7 +82,9 @@ fn getcontext_nested() { }, ).unwrap(); assert!(reached); - panic!("done"); + if cfg!(test) { + panic!("done"); + } } #[cfg_attr(test, should_panic(expected = "done"))] @@ -105,7 +113,9 @@ fn makecontext_setcontext() { ).unwrap(); assert!(REACHED.with(|reached| reached.get())); assert!(reached); - panic!("done"); + if cfg!(test) { + panic!("done"); + } } fn ucontext(context: &mut Context) -> &mut ucontext_t { @@ -218,7 +228,9 @@ fn killswap_getcontext() { let mut reached = false; getcontext(killswap(), || reached = true).unwrap(); assert!(reached); - panic!("done"); + if cfg!(test) { + panic!("done"); + } } fn stack_inbounds(within: &ucontext_t, stack: &[u8]) -> bool { @@ -260,5 +272,7 @@ fn killswap_makecontext() { let mut context = getcontext(|context| context, || unreachable!()).unwrap(); assert!(! stack_inbounds(ucontext(&mut context), &stack)); - panic!("done"); + if cfg!(test) { + panic!("done"); + } } From d4069b2dacbd1abe199a068194b45334d6d8bc6c Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Fri, 10 Aug 2018 12:07:04 -0400 Subject: [PATCH 27/43] src/uconntext.rs: Port sigsetcontext() --- src/ucontext.rs | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/src/ucontext.rs b/src/ucontext.rs index 8eaf423..4e649a9 100644 --- a/src/ucontext.rs +++ b/src/ucontext.rs @@ -1,4 +1,5 @@ use invar::MoveInvariant; +use libc::sigset_t; use libc::ucontext_t; use std::io::Error; use std::io::Result; @@ -147,5 +148,52 @@ pub fn setcontext(mut context: Context) -> Option { Some(Error::last_os_error()) } +pub fn sigsetcontext(context: Context) -> Error { + use libc::SA_SIGINFO; + use libc::SIGVTALRM; + use libc::pthread_kill; + use libc::pthread_self; + use libc::sigaction; + use libc::siginfo_t; + use std::cell::Cell; + use std::os::raw::c_int; + use std::ptr::null_mut; + use std::sync::ONCE_INIT; + use std::sync::Once; + + static INIT: Once = ONCE_INIT; + + thread_local! { + static CONTEXT: Cell> = Cell::new(None); + } + + INIT.call_once(|| { + extern "C" fn handler(_: c_int, _: Option<&siginfo_t>, context: Option<&mut ucontext_t>) { + let context = context.unwrap(); + let mut protext = CONTEXT.with(|protext| protext.take()).unwrap(); + protext.swap(context); + } + + let config = sigaction { + sa_flags: SA_SIGINFO, + sa_sigaction: handler as _, + sa_restorer: None, + sa_mask: sigset_t::zero(), + }; + if unsafe { + sigaction(SIGVTALRM, &config, null_mut()) + } != 0 { + panic!(Error::last_os_error()); + } + }); + + CONTEXT.with(|protext| protext.set(Some(context))); + unsafe { + pthread_kill(pthread_self(), SIGVTALRM); + } + Error::last_os_error() +} + unsafe impl Uninit for ucontext_t {} +unsafe impl Zero for sigset_t {} unsafe impl Zero for ucontext_t {} From a72fdd1704cb245b1b9c24fc93506aafe34ae3ff Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Tue, 14 Aug 2018 10:51:53 -0400 Subject: [PATCH 28/43] tests/tests.rs: Prepare signal handler--swapping test framework for testing sigsetcontext() --- tests/tests.rs | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/tests/tests.rs b/tests/tests.rs index f8e6e0a..22bc754 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -4,6 +4,7 @@ extern crate libc; extern crate ucontext; use libc::ucontext_t; +use std::cell::RefCell; use ucontext::Context; use ucontext::getcontext; use ucontext::makecontext; @@ -168,6 +169,10 @@ fn context_swapinvariant() { assert!(uc_inbounds(second.uc_mcontext.fpregs as _, second)); } +thread_local! { + static CONTEXT: RefCell> = RefCell::new(None); +} + fn killswap() -> fn(Context) { use libc::SA_SIGINFO; use libc::SIGUSR1; @@ -175,26 +180,18 @@ fn killswap() -> fn(Context) { use libc::pthread_self; use libc::sigaction; use libc::siginfo_t; - use std::cell::Cell; use std::io::Error; use std::mem::zeroed; use std::os::raw::c_int; use std::ptr::null_mut; - thread_local! { - static CONTEXT: Cell> = Cell::new(None); - } - extern "C" fn handler( _: c_int, _: Option<&mut siginfo_t>, context: Option<&mut ucontext_t>, ) { let context = context.unwrap(); - CONTEXT.with(|global| { - let mut global = global.take().unwrap(); - global.swap(context); - }); + CONTEXT.with(|global| global.borrow_mut().as_mut().unwrap().swap(context)); } let config = sigaction { @@ -212,11 +209,10 @@ fn killswap() -> fn(Context) { } fn fun(context: Context) { - CONTEXT.with(|global| global.set(Some(context))); + CONTEXT.with(|global| global.replace(Some(context))); unsafe { pthread_kill(pthread_self(), SIGUSR1); } - panic!(Error::last_os_error()); } fun From 3ed60dc0c929f7ab52f47d9f0c11f44de96886c0 Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Tue, 14 Aug 2018 10:54:06 -0400 Subject: [PATCH 29/43] tests/tests.rs: Add a test for sigsetcontext() At this point, the debug builds of all tests are passing! --- tests/tests.rs | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/tests/tests.rs b/tests/tests.rs index 22bc754..9258a1a 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -9,6 +9,7 @@ use ucontext::Context; use ucontext::getcontext; use ucontext::makecontext; use ucontext::setcontext; +use ucontext::sigsetcontext; fn main() { getcontext_donothing(); @@ -20,6 +21,7 @@ fn main() { context_swapinvariant(); killswap_getcontext(); killswap_makecontext(); + killswap_sigsetcontext(); } #[cfg_attr(test, should_panic(expected = "done"))] @@ -272,3 +274,44 @@ fn killswap_makecontext() { panic!("done"); } } + +#[cfg_attr(test, should_panic(expected = "done"))] +#[cfg_attr(test, test)] +fn killswap_sigsetcontext() { + use std::cell::Cell; + use libc::MINSIGSTKSZ; + + thread_local! { + static CHECKPOINT: Cell> = Cell::new(None); + } + + extern "C" fn call() { + let context = CHECKPOINT.with(|checkpoint| checkpoint.take()).unwrap(); + killswap()(context); + } + + let mut reached = false; + getcontext( + |mut call_complete| { + let mut stack = [0u8; MINSIGSTKSZ]; + let call_gate = makecontext(call, &mut stack, Some(&mut call_complete)).unwrap(); + getcontext( + |call_pause| { + CHECKPOINT.with(|checkpoint| checkpoint.set(Some(call_pause))); + setcontext(call_gate); + unreachable!(); + }, + || { + let call_resume = CONTEXT.with(|context| context.borrow_mut().take()).unwrap(); + sigsetcontext(call_resume); + unreachable!(); + }, + ).unwrap(); + }, + || reached = true, + ).unwrap(); + assert!(reached); + if cfg!(test) { + panic!("done"); + } +} From cc33afb49ca761437ed38267a881366daad136bc Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Tue, 14 Aug 2018 18:34:24 -0400 Subject: [PATCH 30/43] src/ucontext.rs: Fix a double free bug that was causing release builds of the tests to crash On the second path through the checkpoint() routine (i.e. after client code had restored the context), the Rust compiler didn't see any use of the context object, and so attempted to drop it when it went out of scope. However, before the time travel, ownership of this object had been passed to the first closure, which had already subsequently deallocated it via setcontext()! We now pass the tests for both types of builds! --- src/ucontext.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ucontext.rs b/src/ucontext.rs index 4e649a9..5c241b4 100644 --- a/src/ucontext.rs +++ b/src/ucontext.rs @@ -84,6 +84,7 @@ fn checkpoint(context: &mut ucontext_t) -> Result<()> { /// Calls `a()`, which may perform a `setcontext()` on its argument. If and only if it does so, /// `b()` is executed before this function returns. pub fn getcontext T, B: FnOnce() -> T>(a: A, b: B) -> Result { + use std::mem::forget; use volatile::VolBool; let mut context = Context::new(); @@ -101,6 +102,7 @@ pub fn getcontext T, B: FnOnce() -> T>(a: A, b: B) -> R unused.store(false); res = a(context); } else { + forget(context); res = b(); } From 056ea49f5a973c453652b604c001af237b15e484 Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Thu, 16 Aug 2018 02:34:15 -0400 Subject: [PATCH 31/43] Make setcontext() take a shared reference to the Context instead of acquiring ownershipv of it This is possible because of the platform setcontext()'s guarantee that it will not modify the context (and therefore that said context may be restored multiple times). (Note that unfortunately POSIX defined setcontext() with a const-incorrect signature.) This implementation requires interior mutability so that setcontext() is still able to perform the MoveInvariant fixup. Note that the change causes UB in at least the killswap_sigsetcontext() test, which now segfaults. --- src/ucontext.rs | 39 ++++++++++++++++++++++----------------- tests/tests.rs | 17 +++++++++-------- 2 files changed, 31 insertions(+), 25 deletions(-) diff --git a/src/ucontext.rs b/src/ucontext.rs index 5c241b4..60bf452 100644 --- a/src/ucontext.rs +++ b/src/ucontext.rs @@ -1,6 +1,7 @@ use invar::MoveInvariant; use libc::sigset_t; use libc::ucontext_t; +use std::cell::RefCell; use std::io::Error; use std::io::Result; use std::rc::Rc; @@ -11,7 +12,7 @@ const REG_CSGSFS: usize = 18; /// A continuation that may be resumed using `setcontext()`. pub struct Context { - context: ucontext_t, + context: RefCell, guard: Option>, } @@ -20,7 +21,7 @@ impl Context { /// it has either been initialized or zeroed! fn new() -> Self { Self { - context: ucontext_t::uninit(), + context: RefCell::new(ucontext_t::uninit()), guard: None, } @@ -37,7 +38,7 @@ impl Context { pub fn swap(&mut self, other: &mut ucontext_t) { use std::mem::swap; - let this = &mut self.context; + let mut this = self.context.borrow_mut(); this.after_move(); swap(&mut this.uc_mcontext, &mut other.uc_mcontext); @@ -95,7 +96,7 @@ pub fn getcontext T, B: FnOnce() -> T>(a: A, b: B) -> R // up this function's stack. let mut unused = VolBool::new(true); let guard = Rc::downgrade(context.guard()); - checkpoint(&mut context.context)?; + checkpoint(&mut context.context.borrow_mut())?; let res; if unused.load() { @@ -115,27 +116,30 @@ pub fn getcontext T, B: FnOnce() -> T>(a: A, b: B) -> R pub fn makecontext(function: extern "C" fn(), stack: &mut [u8], successor: Option<&mut Context>) -> Result { use libc::makecontext; - let mut context = Context::new(); - checkpoint(&mut context.context)?; + let context = Context::new(); + { + let mut ucontext = context.context.borrow_mut(); - context.context.uc_stack.ss_sp = stack.as_mut_ptr() as _; - context.context.uc_stack.ss_size = stack.len(); - if let Some(successor) = successor { - context.context.uc_link = &mut successor.context; - } + checkpoint(&mut ucontext)?; + ucontext.uc_stack.ss_sp = stack.as_mut_ptr() as _; + ucontext.uc_stack.ss_size = stack.len(); + if let Some(successor) = successor { + ucontext.uc_link = successor.context.as_ptr(); + } - unsafe { - makecontext(&mut context.context, function, 0); + unsafe { + makecontext(&mut *ucontext, function, 0); + } } Ok(context) } /// Attempts to resume `context`, never returning on success. Otherwise, returns `None` if /// `context`'s stack frame has expired or `Some` to indicate a platform error. -pub fn setcontext(mut context: Context) -> Option { +pub fn setcontext(context: &Context) -> Option { use libc::setcontext; - if let Some(guard) = context.guard.take() { + if let Some(guard) = context.guard.as_ref() { let guarded = Rc::weak_count(&guard); if guarded == 0 { None?; @@ -143,9 +147,10 @@ pub fn setcontext(mut context: Context) -> Option { debug_assert!(guarded == 1, "setcontext() found multiple corresponding stack frames (?)"); } - context.context.after_move(); + let mut ucontext = context.context.borrow_mut(); + ucontext.after_move(); unsafe { - setcontext(&context.context); + setcontext(&*ucontext); } Some(Error::last_os_error()) } diff --git a/tests/tests.rs b/tests/tests.rs index 9258a1a..0d6cbcf 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -41,7 +41,7 @@ fn getcontext_setcontext() { let mut reached = false; getcontext( |context| { - setcontext(context); + setcontext(&context); unreachable!(); }, || reached = true, @@ -56,7 +56,7 @@ fn getcontext_setcontext() { #[cfg_attr(test, test)] fn getcontext_succeedatnothing() { let invalid = getcontext(|context| context, || unreachable!()).unwrap(); - assert!(setcontext(invalid).is_none()); + assert!(setcontext(&invalid).is_none()); if cfg!(test) { panic!("done"); } @@ -74,13 +74,13 @@ fn getcontext_nested() { |outer| getcontext( |inner| { context.set(Some(inner)); - setcontext(outer); + setcontext(&outer); unreachable!(); }, || reached = true, ).unwrap(), || { - setcontext(context.take().unwrap()); + setcontext(&context.take().unwrap()); unreachable!(); }, ).unwrap(); @@ -109,7 +109,7 @@ fn makecontext_setcontext() { |mut successor| { let mut stack = [0u8; MINSIGSTKSZ]; let predecessor = makecontext(call, &mut stack, Some(&mut successor)).unwrap(); - setcontext(predecessor); + setcontext(&predecessor); unreachable!(); }, || reached = true, @@ -124,9 +124,10 @@ fn makecontext_setcontext() { fn ucontext(context: &mut Context) -> &mut ucontext_t { use std::mem::transmute; - unsafe { + let context: &mut RefCell<_> = unsafe { transmute(context) - } + }; + context.get_mut() } fn uc_inbounds(within: *const ucontext_t, context: *const ucontext_t) -> bool { @@ -298,7 +299,7 @@ fn killswap_sigsetcontext() { getcontext( |call_pause| { CHECKPOINT.with(|checkpoint| checkpoint.set(Some(call_pause))); - setcontext(call_gate); + setcontext(&call_gate); unreachable!(); }, || { From 4c5315c3b2d77bba4a662c317f2d49c38c31c101 Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Thu, 16 Aug 2018 04:27:14 -0400 Subject: [PATCH 32/43] src/ucontext.rs: Fix bug where getcontext() took a RefMut once before saving the checkpoint but double-freed it thereafter The tests now pass again on both debug and release builds. --- src/ucontext.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/ucontext.rs b/src/ucontext.rs index 60bf452..58e916e 100644 --- a/src/ucontext.rs +++ b/src/ucontext.rs @@ -63,18 +63,18 @@ impl Context { } #[inline(always)] -fn checkpoint(context: &mut ucontext_t) -> Result<()> { +fn checkpoint(context: &Context) -> Result<()> { use libc::getcontext; use std::mem::zeroed; use std::ptr::write; if unsafe { - context.uc_mcontext.gregs = zeroed(); - getcontext(context) + context.context.borrow_mut().uc_mcontext.gregs = zeroed(); + getcontext(context.context.as_ptr()) } != 0 { // Zero the uninitialized context before dropping it! unsafe { - write(context, ucontext_t::zero()); + write(context.context.as_ptr(), ucontext_t::zero()); } Err(Error::last_os_error())?; } @@ -96,7 +96,7 @@ pub fn getcontext T, B: FnOnce() -> T>(a: A, b: B) -> R // up this function's stack. let mut unused = VolBool::new(true); let guard = Rc::downgrade(context.guard()); - checkpoint(&mut context.context.borrow_mut())?; + checkpoint(&context)?; let res; if unused.load() { @@ -117,10 +117,9 @@ pub fn makecontext(function: extern "C" fn(), stack: &mut [u8], successor: Optio use libc::makecontext; let context = Context::new(); + checkpoint(&context)?; { let mut ucontext = context.context.borrow_mut(); - - checkpoint(&mut ucontext)?; ucontext.uc_stack.ss_sp = stack.as_mut_ptr() as _; ucontext.uc_stack.ss_size = stack.len(); if let Some(successor) = successor { From 405a1c10f6204e47a74cecf287cb03f30936863b Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Thu, 16 Aug 2018 04:39:26 -0400 Subject: [PATCH 33/43] src: Elide unnecessary {,extra} borrow_mut() in {getcontext(),makecontext()} --- src/platform.rs | 4 ++++ src/ucontext.rs | 7 ++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/platform.rs b/src/platform.rs index e4b3d1b..85bfc04 100644 --- a/src/platform.rs +++ b/src/platform.rs @@ -4,8 +4,10 @@ pub use self::imp::*; #[cfg(target_os = "linux")] mod imp { + use libc::greg_t; use libc::_libc_fpstate; use super::*; + use zero::Zero; impl MoveInvariant for ucontext_t { fn after_move(&mut self) { @@ -18,6 +20,8 @@ mod imp { }; } } + + unsafe impl Zero for [greg_t; 23] {} } #[cfg(not(target_os = "linux"))] diff --git a/src/ucontext.rs b/src/ucontext.rs index 58e916e..1bfe438 100644 --- a/src/ucontext.rs +++ b/src/ucontext.rs @@ -20,8 +20,11 @@ impl Context { /// NB: The returned object contains uninitialized data, and cannot be safely dropped until /// it has either been initialized or zeroed! fn new() -> Self { + let mut context = ucontext_t::uninit(); + context.uc_mcontext.gregs = Zero::zero(); + Self { - context: RefCell::new(ucontext_t::uninit()), + context: RefCell::new(context), guard: None, } @@ -65,11 +68,9 @@ impl Context { #[inline(always)] fn checkpoint(context: &Context) -> Result<()> { use libc::getcontext; - use std::mem::zeroed; use std::ptr::write; if unsafe { - context.context.borrow_mut().uc_mcontext.gregs = zeroed(); getcontext(context.context.as_ptr()) } != 0 { // Zero the uninitialized context before dropping it! From bf6974b9af984756c064e3834cf81afc24e9de61 Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Thu, 16 Aug 2018 07:52:49 -0400 Subject: [PATCH 34/43] Detect illegal intra-stack use of nested contexts by using a per-thread shadow stack of guards This change allows us to invalidate any nested getcontext() invocations' contexts when we return from a call to getcontext() or when we make an intra-stack call to setcontext(). This should also pave the way to leak-free cleanup after setcontext(), which is a concern now that this function accepts a reference rather than a value. --- src/ucontext.rs | 31 +++++++++++++++++++------------ tests/tests.rs | 9 ++++----- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/src/ucontext.rs b/src/ucontext.rs index 1bfe438..1d6b8e6 100644 --- a/src/ucontext.rs +++ b/src/ucontext.rs @@ -5,15 +5,20 @@ use std::cell::RefCell; use std::io::Error; use std::io::Result; use std::rc::Rc; +use std::rc::Weak; use uninit::Uninit; use zero::Zero; const REG_CSGSFS: usize = 18; +thread_local! { + static GUARDS: RefCell>> = RefCell::new(Vec::new()); +} + /// A continuation that may be resumed using `setcontext()`. pub struct Context { context: RefCell, - guard: Option>, + guard: Option>, } impl Context { @@ -30,10 +35,6 @@ impl Context { } - fn guard(&mut self) -> &Rc<()> { - self.guard.get_or_insert_with(|| Rc::new(())) - } - /// Exchange the functional portion of this context with another one. When called on a /// a particular context within a signal handler, this causes that context to be restored /// upon return from the handler. Note that the handler's original context is stored back @@ -96,7 +97,14 @@ pub fn getcontext T, B: FnOnce() -> T>(a: A, b: B) -> R // because client code that never resumes the context was already responsible for cleaning // up this function's stack. let mut unused = VolBool::new(true); - let guard = Rc::downgrade(context.guard()); + let guard = GUARDS.with(|guards| { + let mut guards = guards.borrow_mut(); + let guard = Rc::new(guards.len()); + let res = Rc::downgrade(&guard); + guards.push(guard); + res + }); + context.guard = Some(guard.clone()); checkpoint(&context)?; let res; @@ -108,7 +116,9 @@ pub fn getcontext T, B: FnOnce() -> T>(a: A, b: B) -> R res = b(); } - drop(guard); + GUARDS.with(move |guards| { + guards.borrow_mut().truncate(*guard.upgrade().unwrap()) + }); Ok(res) } @@ -140,11 +150,8 @@ pub fn setcontext(context: &Context) -> Option { use libc::setcontext; if let Some(guard) = context.guard.as_ref() { - let guarded = Rc::weak_count(&guard); - if guarded == 0 { - None?; - } - debug_assert!(guarded == 1, "setcontext() found multiple corresponding stack frames (?)"); + let guard = guard.upgrade()?; + GUARDS.with(|guards| guards.borrow_mut().truncate(*guard + 1)); } let mut ucontext = context.context.borrow_mut(); diff --git a/tests/tests.rs b/tests/tests.rs index 0d6cbcf..22965cb 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -15,7 +15,7 @@ fn main() { getcontext_donothing(); getcontext_setcontext(); getcontext_succeedatnothing(); - //getcontext_nested(); + getcontext_nested(); makecontext_setcontext(); context_moveinvariant(); context_swapinvariant(); @@ -62,7 +62,6 @@ fn getcontext_succeedatnothing() { } } -#[cfg_attr(test, ignore)] #[cfg_attr(test, should_panic(expected = "done"))] #[cfg_attr(test, test)] fn getcontext_nested() { @@ -77,11 +76,11 @@ fn getcontext_nested() { setcontext(&outer); unreachable!(); }, - || reached = true, + || unreachable!(), ).unwrap(), || { - setcontext(&context.take().unwrap()); - unreachable!(); + assert!(setcontext(&context.take().unwrap()).is_none()); + reached = true; }, ).unwrap(); assert!(reached); From bf36e17ed817ebc3f6aa2ff17c081a2910b2acb5 Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Thu, 16 Aug 2018 19:46:51 -0400 Subject: [PATCH 35/43] Have setcontext() free the Weak pointers of Contexts that have been left behind in a forever orphaned stack frame This patch compares stack addresses to determine whether the Context escaped the closure's scope. It solves some, but not all, of our leak issues. --- .gitignore | 2 ++ build.rs | 60 +++++++++++++++++++++++++++++++++++++++++ native/Makefile | 6 +++++ native/arch/x86_64/sp.s | 7 +++++ src/lib.rs | 1 + src/sp.rs | 12 +++++++++ src/ucontext.rs | 35 +++++++++++++++++++----- 7 files changed, 116 insertions(+), 7 deletions(-) create mode 100644 build.rs create mode 100644 native/Makefile create mode 100644 native/arch/x86_64/sp.s create mode 100644 src/sp.rs diff --git a/.gitignore b/.gitignore index bb3e3a5..82cb277 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,5 @@ src/main.rs target/ +*.a *.lock +*.o diff --git a/build.rs b/build.rs new file mode 100644 index 0000000..f929e18 --- /dev/null +++ b/build.rs @@ -0,0 +1,60 @@ +use std::env::VarError; +use std::io::Error; +use std::process::ExitStatus; + +const LIBS: [&str; 1] = [ + "libsp.a", +]; + +const SRCDIR: &str = "native"; + +pub fn main() -> Result<(), Failure> { + use std::env::var; + use std::process::Command; + + let destdir = format!("target/{}/deps", var("PROFILE")?); + let srcdir = format!("arch/{}", var("CARGO_CFG_TARGET_ARCH")?); + for lib in &LIBS { + let filename = format!("{}/{}", srcdir, lib); + Command::new("make").arg("-C").arg(SRCDIR).arg(&filename).status().success()?; + let filename = format!("{}/{}", SRCDIR, filename); + Command::new("mv").arg(&filename).arg(&destdir).status().success()?; + } + + Ok(()) +} + +#[derive(Debug)] +pub enum Failure { + VarError(VarError), + Error(Error), + ExitStatus(ExitStatus), +} + +impl From for Failure { + fn from(ve: VarError) -> Self { + Failure::VarError(ve) + } +} + +trait FailureResult { + type FailRes; + + fn success(self) -> Self::FailRes; +} + +impl FailureResult for Result { + type FailRes = Result<(), Failure>; + + fn success(self) -> Self::FailRes { + match self { + Ok(exit_status) => + if exit_status.success() { + Ok(()) + } else { + Err(Failure::ExitStatus(exit_status)) + }, + Err(error) => Err(Failure::Error(error)), + } + } +} diff --git a/native/Makefile b/native/Makefile new file mode 100644 index 0000000..dbbac31 --- /dev/null +++ b/native/Makefile @@ -0,0 +1,6 @@ +override ASFLAGS := $(ASFLAGS) +override LDFLAGS := $(LDFLAGS) +override LDLIBS := $(LDLIBS) + +lib%.a: %.o + $(AR) rs $@ $^ diff --git a/native/arch/x86_64/sp.s b/native/arch/x86_64/sp.s new file mode 100644 index 0000000..b0945cc --- /dev/null +++ b/native/arch/x86_64/sp.s @@ -0,0 +1,7 @@ + .text + + .globl sp + .type sp, @function +sp: + mov %rsp, %rax + ret diff --git a/src/lib.rs b/src/lib.rs index ec4f8a1..a410f93 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2,6 +2,7 @@ extern crate libc; mod invar; mod platform; +mod sp; #[doc(hidden)] mod tests; mod ucontext; diff --git a/src/sp.rs b/src/sp.rs new file mode 100644 index 0000000..5276719 --- /dev/null +++ b/src/sp.rs @@ -0,0 +1,12 @@ +use libc::uintptr_t; + +pub fn sp() -> uintptr_t { + #[link(name = "sp")] + extern "C" { + fn sp() -> uintptr_t; + } + + unsafe { + sp() + } +} diff --git a/src/ucontext.rs b/src/ucontext.rs index 1d6b8e6..1992a64 100644 --- a/src/ucontext.rs +++ b/src/ucontext.rs @@ -1,6 +1,7 @@ use invar::MoveInvariant; use libc::sigset_t; use libc::ucontext_t; +use std::cell::Cell; use std::cell::RefCell; use std::io::Error; use std::io::Result; @@ -10,6 +11,7 @@ use uninit::Uninit; use zero::Zero; const REG_CSGSFS: usize = 18; +const REG_RSP: usize = 15; thread_local! { static GUARDS: RefCell>> = RefCell::new(Vec::new()); @@ -18,7 +20,7 @@ thread_local! { /// A continuation that may be resumed using `setcontext()`. pub struct Context { context: RefCell, - guard: Option>, + guard: Cell>>, } impl Context { @@ -30,7 +32,7 @@ impl Context { Self { context: RefCell::new(context), - guard: None, + guard: Cell::new(None), } } @@ -90,7 +92,7 @@ pub fn getcontext T, B: FnOnce() -> T>(a: A, b: B) -> R use std::mem::forget; use volatile::VolBool; - let mut context = Context::new(); + let context = Context::new(); // Storing this flag on the stack is not unsound because guard enforces the invariant that // this stack frame outlives any resumable context. Storing it on the stack is not leaky @@ -104,7 +106,7 @@ pub fn getcontext T, B: FnOnce() -> T>(a: A, b: B) -> R guards.push(guard); res }); - context.guard = Some(guard.clone()); + context.guard.set(Some(guard.clone())); checkpoint(&context)?; let res; @@ -148,10 +150,29 @@ pub fn makecontext(function: extern "C" fn(), stack: &mut [u8], successor: Optio /// `context`'s stack frame has expired or `Some` to indicate a platform error. pub fn setcontext(context: &Context) -> Option { use libc::setcontext; + use sp::sp; + + fn between(left: usize, between: usize, right: usize) -> bool { + use std::cmp::max; + use std::cmp::min; + use std::mem::size_of; - if let Some(guard) = context.guard.as_ref() { - let guard = guard.upgrade()?; - GUARDS.with(|guards| guards.borrow_mut().truncate(*guard + 1)); + let size = 4 * size_of::(); + min(left, right) - size <= between && between <= max(left, right) + size + } + + if let Some(guard) = context.guard.take().take() { + GUARDS.with(|guards| { + let guard = guard.upgrade()?; + guards.borrow_mut().truncate(*guard + 1); + Some(()) + })?; + + let ours = sp(); + let theirs = context.context.borrow().uc_mcontext.gregs[REG_RSP]; + if ! between(ours, &guard as *const _ as _, theirs as _) { + context.guard.set(Some(guard)); + } } let mut ucontext = context.context.borrow_mut(); From e2107f8f0405222fe72096979a76453ea7f6c3c8 Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Thu, 16 Aug 2018 19:49:50 -0400 Subject: [PATCH 36/43] src/ucontext.rs: Eliminate another leak by having getcontext() store the index instead of its own Weak pointer thereto This is necessary because getcontext()'s stack frame can itself become orphaned (e.g. in the getcontext_nested() test). --- src/ucontext.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/ucontext.rs b/src/ucontext.rs index 1992a64..61936b1 100644 --- a/src/ucontext.rs +++ b/src/ucontext.rs @@ -99,14 +99,15 @@ pub fn getcontext T, B: FnOnce() -> T>(a: A, b: B) -> R // because client code that never resumes the context was already responsible for cleaning // up this function's stack. let mut unused = VolBool::new(true); - let guard = GUARDS.with(|guards| { + let idx = GUARDS.with(|guards| { let mut guards = guards.borrow_mut(); - let guard = Rc::new(guards.len()); - let res = Rc::downgrade(&guard); + let idx = guards.len(); + let guard = Rc::new(idx); + let guardian = Rc::downgrade(&guard); + context.guard.set(Some(guardian)); guards.push(guard); - res + idx }); - context.guard.set(Some(guard.clone())); checkpoint(&context)?; let res; @@ -119,7 +120,7 @@ pub fn getcontext T, B: FnOnce() -> T>(a: A, b: B) -> R } GUARDS.with(move |guards| { - guards.borrow_mut().truncate(*guard.upgrade().unwrap()) + guards.borrow_mut().truncate(idx) }); Ok(res) } From bcd484fd458b64c5f4150e41a0274d1e6e5130b1 Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Fri, 17 Aug 2018 13:05:04 -0400 Subject: [PATCH 37/43] Attempt to store successor context references in contexts returned from makecontext() The goal behind this change was to alleviate the makecontext() leaks by enabling setcontext() to follow the resulting linked list, validating and destructiong (if on an orphaned stack) the contexts in the chain of "succession." Unfortunately, this approach doesn't work because the compiler knows that UnsafeCells, like mut references, are invariant in their type; as such, it becomes impossible to call makecontext() on a valid checkpoint from getcontext() (i.e. from within the closure passed thereto). --- src/ucontext.rs | 49 +++++++++++++++++++++++++++++++++++-------------- tests/tests.rs | 12 ++++++++---- 2 files changed, 43 insertions(+), 18 deletions(-) diff --git a/src/ucontext.rs b/src/ucontext.rs index 61936b1..75eddb8 100644 --- a/src/ucontext.rs +++ b/src/ucontext.rs @@ -18,12 +18,19 @@ thread_local! { } /// A continuation that may be resumed using `setcontext()`. -pub struct Context { +pub struct Context<'a> { context: RefCell, - guard: Cell>>, + guard: Cell>, } -impl Context { +enum Guard<'a> { + OnStack(Weak), + StackGate(Option<&'a Context<'a>>), + Expired, + Interrupted, +} + +impl<'a> Context<'a> { /// NB: The returned object contains uninitialized data, and cannot be safely dropped until /// it has either been initialized or zeroed! fn new() -> Self { @@ -32,7 +39,7 @@ impl Context { Self { context: RefCell::new(context), - guard: Cell::new(None), + guard: Cell::new(Guard::Expired), } } @@ -64,7 +71,13 @@ impl Context { swap(&mut this.uc_stack, &mut other.uc_stack); swap(&mut this.uc_sigmask, &mut other.uc_sigmask); - self.guard.take(); + self.guard.set(Guard::Interrupted); + } +} + +impl<'a> Default for Guard<'a> { + fn default() -> Self { + Guard::Expired } } @@ -88,7 +101,7 @@ fn checkpoint(context: &Context) -> Result<()> { /// Calls `a()`, which may perform a `setcontext()` on its argument. If and only if it does so, /// `b()` is executed before this function returns. -pub fn getcontext T, B: FnOnce() -> T>(a: A, b: B) -> Result { +pub fn getcontext<'a, T: 'a, A: FnOnce(Context<'a>) -> T, B: FnOnce() -> T>(a: A, b: B) -> Result { use std::mem::forget; use volatile::VolBool; @@ -104,7 +117,7 @@ pub fn getcontext T, B: FnOnce() -> T>(a: A, b: B) -> R let idx = guards.len(); let guard = Rc::new(idx); let guardian = Rc::downgrade(&guard); - context.guard.set(Some(guardian)); + context.guard.set(Guard::OnStack(guardian)); guards.push(guard); idx }); @@ -122,23 +135,28 @@ pub fn getcontext T, B: FnOnce() -> T>(a: A, b: B) -> R GUARDS.with(move |guards| { guards.borrow_mut().truncate(idx) }); + // FIXME: MUST DROP first AND second! + //drop(a); + //drop(b); Ok(res) } /// Configures a context to invoke `function()` on a separate `stack`, optionally resuming the /// program at the `successor` context upon said function's return (or, by default, exiting). -pub fn makecontext(function: extern "C" fn(), stack: &mut [u8], successor: Option<&mut Context>) -> Result { +pub fn makecontext<'a>(function: extern "C" fn(), stack: &'a mut [u8], successor: Option<&'a Context<'a>>) -> Result> { use libc::makecontext; + use std::ptr::null_mut; let context = Context::new(); + let link = successor.map(|link| link.context.as_ptr()).unwrap_or(null_mut()); + context.guard.set(Guard::StackGate(successor)); + checkpoint(&context)?; { let mut ucontext = context.context.borrow_mut(); ucontext.uc_stack.ss_sp = stack.as_mut_ptr() as _; ucontext.uc_stack.ss_size = stack.len(); - if let Some(successor) = successor { - ucontext.uc_link = successor.context.as_ptr(); - } + ucontext.uc_link = link; unsafe { makecontext(&mut *ucontext, function, 0); @@ -162,7 +180,7 @@ pub fn setcontext(context: &Context) -> Option { min(left, right) - size <= between && between <= max(left, right) + size } - if let Some(guard) = context.guard.take().take() { + if let Guard::OnStack(guard) = context.guard.take() { GUARDS.with(|guards| { let guard = guard.upgrade()?; guards.borrow_mut().truncate(*guard + 1); @@ -192,6 +210,7 @@ pub fn sigsetcontext(context: Context) -> Error { use libc::sigaction; use libc::siginfo_t; use std::cell::Cell; + use std::mem::transmute; use std::os::raw::c_int; use std::ptr::null_mut; use std::sync::ONCE_INIT; @@ -200,7 +219,7 @@ pub fn sigsetcontext(context: Context) -> Error { static INIT: Once = ONCE_INIT; thread_local! { - static CONTEXT: Cell> = Cell::new(None); + static CONTEXT: Cell>> = Cell::new(None); } INIT.call_once(|| { @@ -223,7 +242,9 @@ pub fn sigsetcontext(context: Context) -> Error { } }); - CONTEXT.with(|protext| protext.set(Some(context))); + CONTEXT.with(|protext| protext.set(Some(unsafe { + transmute(context) + }))); unsafe { pthread_kill(pthread_self(), SIGVTALRM); } diff --git a/tests/tests.rs b/tests/tests.rs index 22965cb..3f7af7e 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -120,7 +120,7 @@ fn makecontext_setcontext() { } } -fn ucontext(context: &mut Context) -> &mut ucontext_t { +fn ucontext<'a>(context: &'a mut Context<'a>) -> &'a mut ucontext_t { use std::mem::transmute; let context: &mut RefCell<_> = unsafe { @@ -172,7 +172,7 @@ fn context_swapinvariant() { } thread_local! { - static CONTEXT: RefCell> = RefCell::new(None); + static CONTEXT: RefCell>> = RefCell::new(None); } fn killswap() -> fn(Context) { @@ -211,7 +211,11 @@ fn killswap() -> fn(Context) { } fn fun(context: Context) { - CONTEXT.with(|global| global.replace(Some(context))); + use std::mem::transmute; + + CONTEXT.with(|global| global.replace(Some(unsafe { + transmute(context) + }))); unsafe { pthread_kill(pthread_self(), SIGUSR1); } @@ -282,7 +286,7 @@ fn killswap_sigsetcontext() { use libc::MINSIGSTKSZ; thread_local! { - static CHECKPOINT: Cell> = Cell::new(None); + static CHECKPOINT: Cell>> = Cell::new(None); } extern "C" fn call() { From 7b0941e4be7798b8f1097a819c8a17c61cd25f54 Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Fri, 17 Aug 2018 13:11:59 -0400 Subject: [PATCH 38/43] Revert "Attempt to store successor context references in contexts returned from makecontext()" This reverts commit bcd484fd458b64c5f4150e41a0274d1e6e5130b1. --- src/ucontext.rs | 49 ++++++++++++++----------------------------------- tests/tests.rs | 12 ++++-------- 2 files changed, 18 insertions(+), 43 deletions(-) diff --git a/src/ucontext.rs b/src/ucontext.rs index 75eddb8..61936b1 100644 --- a/src/ucontext.rs +++ b/src/ucontext.rs @@ -18,19 +18,12 @@ thread_local! { } /// A continuation that may be resumed using `setcontext()`. -pub struct Context<'a> { +pub struct Context { context: RefCell, - guard: Cell>, + guard: Cell>>, } -enum Guard<'a> { - OnStack(Weak), - StackGate(Option<&'a Context<'a>>), - Expired, - Interrupted, -} - -impl<'a> Context<'a> { +impl Context { /// NB: The returned object contains uninitialized data, and cannot be safely dropped until /// it has either been initialized or zeroed! fn new() -> Self { @@ -39,7 +32,7 @@ impl<'a> Context<'a> { Self { context: RefCell::new(context), - guard: Cell::new(Guard::Expired), + guard: Cell::new(None), } } @@ -71,13 +64,7 @@ impl<'a> Context<'a> { swap(&mut this.uc_stack, &mut other.uc_stack); swap(&mut this.uc_sigmask, &mut other.uc_sigmask); - self.guard.set(Guard::Interrupted); - } -} - -impl<'a> Default for Guard<'a> { - fn default() -> Self { - Guard::Expired + self.guard.take(); } } @@ -101,7 +88,7 @@ fn checkpoint(context: &Context) -> Result<()> { /// Calls `a()`, which may perform a `setcontext()` on its argument. If and only if it does so, /// `b()` is executed before this function returns. -pub fn getcontext<'a, T: 'a, A: FnOnce(Context<'a>) -> T, B: FnOnce() -> T>(a: A, b: B) -> Result { +pub fn getcontext T, B: FnOnce() -> T>(a: A, b: B) -> Result { use std::mem::forget; use volatile::VolBool; @@ -117,7 +104,7 @@ pub fn getcontext<'a, T: 'a, A: FnOnce(Context<'a>) -> T, B: FnOnce() -> T>(a: A let idx = guards.len(); let guard = Rc::new(idx); let guardian = Rc::downgrade(&guard); - context.guard.set(Guard::OnStack(guardian)); + context.guard.set(Some(guardian)); guards.push(guard); idx }); @@ -135,28 +122,23 @@ pub fn getcontext<'a, T: 'a, A: FnOnce(Context<'a>) -> T, B: FnOnce() -> T>(a: A GUARDS.with(move |guards| { guards.borrow_mut().truncate(idx) }); - // FIXME: MUST DROP first AND second! - //drop(a); - //drop(b); Ok(res) } /// Configures a context to invoke `function()` on a separate `stack`, optionally resuming the /// program at the `successor` context upon said function's return (or, by default, exiting). -pub fn makecontext<'a>(function: extern "C" fn(), stack: &'a mut [u8], successor: Option<&'a Context<'a>>) -> Result> { +pub fn makecontext(function: extern "C" fn(), stack: &mut [u8], successor: Option<&mut Context>) -> Result { use libc::makecontext; - use std::ptr::null_mut; let context = Context::new(); - let link = successor.map(|link| link.context.as_ptr()).unwrap_or(null_mut()); - context.guard.set(Guard::StackGate(successor)); - checkpoint(&context)?; { let mut ucontext = context.context.borrow_mut(); ucontext.uc_stack.ss_sp = stack.as_mut_ptr() as _; ucontext.uc_stack.ss_size = stack.len(); - ucontext.uc_link = link; + if let Some(successor) = successor { + ucontext.uc_link = successor.context.as_ptr(); + } unsafe { makecontext(&mut *ucontext, function, 0); @@ -180,7 +162,7 @@ pub fn setcontext(context: &Context) -> Option { min(left, right) - size <= between && between <= max(left, right) + size } - if let Guard::OnStack(guard) = context.guard.take() { + if let Some(guard) = context.guard.take().take() { GUARDS.with(|guards| { let guard = guard.upgrade()?; guards.borrow_mut().truncate(*guard + 1); @@ -210,7 +192,6 @@ pub fn sigsetcontext(context: Context) -> Error { use libc::sigaction; use libc::siginfo_t; use std::cell::Cell; - use std::mem::transmute; use std::os::raw::c_int; use std::ptr::null_mut; use std::sync::ONCE_INIT; @@ -219,7 +200,7 @@ pub fn sigsetcontext(context: Context) -> Error { static INIT: Once = ONCE_INIT; thread_local! { - static CONTEXT: Cell>> = Cell::new(None); + static CONTEXT: Cell> = Cell::new(None); } INIT.call_once(|| { @@ -242,9 +223,7 @@ pub fn sigsetcontext(context: Context) -> Error { } }); - CONTEXT.with(|protext| protext.set(Some(unsafe { - transmute(context) - }))); + CONTEXT.with(|protext| protext.set(Some(context))); unsafe { pthread_kill(pthread_self(), SIGVTALRM); } diff --git a/tests/tests.rs b/tests/tests.rs index 3f7af7e..22965cb 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -120,7 +120,7 @@ fn makecontext_setcontext() { } } -fn ucontext<'a>(context: &'a mut Context<'a>) -> &'a mut ucontext_t { +fn ucontext(context: &mut Context) -> &mut ucontext_t { use std::mem::transmute; let context: &mut RefCell<_> = unsafe { @@ -172,7 +172,7 @@ fn context_swapinvariant() { } thread_local! { - static CONTEXT: RefCell>> = RefCell::new(None); + static CONTEXT: RefCell> = RefCell::new(None); } fn killswap() -> fn(Context) { @@ -211,11 +211,7 @@ fn killswap() -> fn(Context) { } fn fun(context: Context) { - use std::mem::transmute; - - CONTEXT.with(|global| global.replace(Some(unsafe { - transmute(context) - }))); + CONTEXT.with(|global| global.replace(Some(context))); unsafe { pthread_kill(pthread_self(), SIGUSR1); } @@ -286,7 +282,7 @@ fn killswap_sigsetcontext() { use libc::MINSIGSTKSZ; thread_local! { - static CHECKPOINT: Cell>> = Cell::new(None); + static CHECKPOINT: Cell> = Cell::new(None); } extern "C" fn call() { From c4b3c680fc3f125d8e0cafa5e60014009c79577e Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Fri, 17 Aug 2018 14:26:23 -0400 Subject: [PATCH 39/43] src/ucontext.rs: Perform minor internal refactoring to make reuse of Context's ucontext_t portion easier This will be useful as we work on mitigating the ability to completely unsafely (re)activate call gates (and the fact that they currently leak the Weak corresponding to their successor context, all this by inventing a separate type to represent call gates instead of making Contexts themselves infinitely chainable. --- src/ucontext.rs | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/src/ucontext.rs b/src/ucontext.rs index 61936b1..e38b388 100644 --- a/src/ucontext.rs +++ b/src/ucontext.rs @@ -27,11 +27,8 @@ impl Context { /// NB: The returned object contains uninitialized data, and cannot be safely dropped until /// it has either been initialized or zeroed! fn new() -> Self { - let mut context = ucontext_t::uninit(); - context.uc_mcontext.gregs = Zero::zero(); - Self { - context: RefCell::new(context), + context: RefCell::new(ucontext_t::uninit()), guard: Cell::new(None), } @@ -69,16 +66,16 @@ impl Context { } #[inline(always)] -fn checkpoint(context: &Context) -> Result<()> { +fn checkpoint(context: *mut ucontext_t) -> Result<()> { use libc::getcontext; use std::ptr::write; if unsafe { - getcontext(context.context.as_ptr()) + getcontext(context) } != 0 { // Zero the uninitialized context before dropping it! unsafe { - write(context.context.as_ptr(), ucontext_t::zero()); + write(context, ucontext_t::zero()); } Err(Error::last_os_error())?; } @@ -108,7 +105,7 @@ pub fn getcontext T, B: FnOnce() -> T>(a: A, b: B) -> R guards.push(guard); idx }); - checkpoint(&context)?; + checkpoint(context.context.as_ptr())?; let res; if unused.load() { @@ -131,7 +128,7 @@ pub fn makecontext(function: extern "C" fn(), stack: &mut [u8], successor: Optio use libc::makecontext; let context = Context::new(); - checkpoint(&context)?; + checkpoint(context.context.as_ptr())?; { let mut ucontext = context.context.borrow_mut(); ucontext.uc_stack.ss_sp = stack.as_mut_ptr() as _; @@ -230,6 +227,17 @@ pub fn sigsetcontext(context: Context) -> Error { Error::last_os_error() } -unsafe impl Uninit for ucontext_t {} +unsafe impl Uninit for ucontext_t { + fn uninit() -> Self { + use std::mem::uninitialized; + + let mut this: Self = unsafe { + uninitialized() + }; + this.uc_mcontext.gregs = Zero::zero(); + this + } +} + unsafe impl Zero for sigset_t {} unsafe impl Zero for ucontext_t {} From 4c21c51d91278ad11bfe491392f43a91e2464a16 Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Wed, 22 Aug 2018 20:37:30 -0400 Subject: [PATCH 40/43] src/ucontext.rs: Make setcontext() fail if the Context's guard has been invalidated --- src/ucontext.rs | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/ucontext.rs b/src/ucontext.rs index e38b388..1761703 100644 --- a/src/ucontext.rs +++ b/src/ucontext.rs @@ -159,18 +159,17 @@ pub fn setcontext(context: &Context) -> Option { min(left, right) - size <= between && between <= max(left, right) + size } - if let Some(guard) = context.guard.take().take() { - GUARDS.with(|guards| { - let guard = guard.upgrade()?; - guards.borrow_mut().truncate(*guard + 1); - Some(()) - })?; - - let ours = sp(); - let theirs = context.context.borrow().uc_mcontext.gregs[REG_RSP]; - if ! between(ours, &guard as *const _ as _, theirs as _) { - context.guard.set(Some(guard)); - } + let guard = context.guard.take().take()?; + GUARDS.with(|guards| { + let guard = guard.upgrade()?; + guards.borrow_mut().truncate(*guard + 1); + Some(()) + })?; + + let ours = sp(); + let theirs = context.context.borrow().uc_mcontext.gregs[REG_RSP]; + if ! between(ours, &guard as *const _ as _, theirs as _) { + context.guard.set(Some(guard)); } let mut ucontext = context.context.borrow_mut(); From 52da6cf12ed24de1be7b1af1310225c5e334dcf4 Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Thu, 23 Aug 2018 10:48:30 -0400 Subject: [PATCH 41/43] Restructure makecontext() to perform a platform setcontext() internally This simplifies its interface greatly, since one doesn't need to worry about the lifetimes associated with some resulting "Context" object, and it becomes impossible to erroneously use such a(n untagged) call gate with sigsetcontext(). --- src/ucontext.rs | 107 ++++++++++++++++++++++++++++++++++-------------- tests/tests.rs | 26 ++++++------ 2 files changed, 89 insertions(+), 44 deletions(-) diff --git a/src/ucontext.rs b/src/ucontext.rs index 1761703..aa9f944 100644 --- a/src/ucontext.rs +++ b/src/ucontext.rs @@ -1,6 +1,7 @@ use invar::MoveInvariant; use libc::sigset_t; use libc::ucontext_t; +use libc::uintptr_t; use std::cell::Cell; use std::cell::RefCell; use std::io::Error; @@ -122,34 +123,7 @@ pub fn getcontext T, B: FnOnce() -> T>(a: A, b: B) -> R Ok(res) } -/// Configures a context to invoke `function()` on a separate `stack`, optionally resuming the -/// program at the `successor` context upon said function's return (or, by default, exiting). -pub fn makecontext(function: extern "C" fn(), stack: &mut [u8], successor: Option<&mut Context>) -> Result { - use libc::makecontext; - - let context = Context::new(); - checkpoint(context.context.as_ptr())?; - { - let mut ucontext = context.context.borrow_mut(); - ucontext.uc_stack.ss_sp = stack.as_mut_ptr() as _; - ucontext.uc_stack.ss_size = stack.len(); - if let Some(successor) = successor { - ucontext.uc_link = successor.context.as_ptr(); - } - - unsafe { - makecontext(&mut *ucontext, function, 0); - } - } - Ok(context) -} - -/// Attempts to resume `context`, never returning on success. Otherwise, returns `None` if -/// `context`'s stack frame has expired or `Some` to indicate a platform error. -pub fn setcontext(context: &Context) -> Option { - use libc::setcontext; - use sp::sp; - +fn validate_guard(context: &Context, stack: uintptr_t) -> Option<()> { fn between(left: usize, between: usize, right: usize) -> bool { use std::cmp::max; use std::cmp::min; @@ -166,12 +140,85 @@ pub fn setcontext(context: &Context) -> Option { Some(()) })?; - let ours = sp(); let theirs = context.context.borrow().uc_mcontext.gregs[REG_RSP]; - if ! between(ours, &guard as *const _ as _, theirs as _) { + if ! between(stack, &guard as *const _ as _, theirs as _) { context.guard.set(Some(guard)); } + Some(()) +} + +/// "Call gate" that invokes a `function()` on a separate `stack`, optionally resuming the program +/// at the `successor` context upon said function's return (or, by default, exiting). +pub fn makecontext(mut function: T, stack: &mut [u8], successor: Option<&Context>) -> Error { + use libc::makecontext; + use libc::setcontext; + use sp::sp; + use std::mem::transmute; + use std::os::raw::c_int; + + enum Link<'a> { + WithoutSuccessor(&'a mut dyn FnMut()), + WithSuccessor(LinkedSuccessor<'a>), + } + + struct LinkedSuccessor<'a> { + function: &'a mut dyn FnMut(), + successor: &'a Context, + stack: uintptr_t, + } + + extern "C" fn link(lower: c_int, upper: c_int) { + let link: *mut Link = (lower as usize | ((upper as usize) << 32)) as _; + let link = unsafe { + link.as_mut() + }.unwrap(); + + match link { + Link::WithoutSuccessor(link) => link(), + Link::WithSuccessor(link) => { + (link.function)(); + validate_guard(&link.successor, link.stack); + }, + } + } + + let mut context = ucontext_t::uninit(); + if let Err(or) = checkpoint(&mut context) { + return or; + } + + let args; + context.uc_stack.ss_sp = stack.as_mut_ptr() as _; + context.uc_stack.ss_size = stack.len(); + if let Some(successor) = successor { + context.uc_link = successor.context.as_ptr(); + args = Link::WithSuccessor(LinkedSuccessor { + function: &mut function, + successor: successor, + stack: sp(), + }); + } else { + args = Link::WithoutSuccessor(&mut function); + } + + let args: usize = &args as *const _ as _; + unsafe { + makecontext(&mut context, transmute(link as extern "C" fn(c_int, c_int)), 2, args, args >> 32); + setcontext(&context); + } + + unreachable!() +} + +/// Attempts to resume `context`, never returning on success. Otherwise, returns `None` if +/// `context`'s stack frame has expired or `Some` to indicate a platform error. +pub fn setcontext(context: &Context) -> Option { + use libc::setcontext; + use sp::sp; + + validate_guard(context, sp())?; + let mut ucontext = context.context.borrow_mut(); ucontext.after_move(); unsafe { diff --git a/tests/tests.rs b/tests/tests.rs index 22965cb..9f5640f 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -99,7 +99,7 @@ fn makecontext_setcontext() { static REACHED: Cell = Cell::new(false); } - extern "C" fn call() { + fn call() { REACHED.with(|reached| reached.set(true)); } @@ -107,8 +107,7 @@ fn makecontext_setcontext() { getcontext( |mut successor| { let mut stack = [0u8; MINSIGSTKSZ]; - let predecessor = makecontext(call, &mut stack, Some(&mut successor)).unwrap(); - setcontext(&predecessor); + makecontext(call, &mut stack, Some(&mut successor)); unreachable!(); }, || reached = true, @@ -243,15 +242,16 @@ fn stack_inbounds(within: &ucontext_t, stack: &[u8]) -> bool { #[cfg_attr(test, should_panic(expected = "done"))] #[cfg_attr(test, test)] fn killswap_makecontext() { - use std::cell::Cell; use libc::MINSIGSTKSZ; + use libc::uintptr_t; + use std::cell::Cell; thread_local! { - static REACHED: Cell = Cell::new(false); + static SUCCESSOR: Cell> = Cell::new(None); } - extern "C" fn call() { - REACHED.with(|reached| reached.set(true)); + fn call() { + killswap()(SUCCESSOR.with(|successor| successor.take()).unwrap()); } let mut reached = false; @@ -259,14 +259,13 @@ fn killswap_makecontext() { getcontext( |mut context| { assert!(! stack_inbounds(ucontext(&mut context), &stack)); - let context = makecontext(call, &mut stack, Some(&mut context)).unwrap(); - killswap()(context); + SUCCESSOR.with(|successor| successor.set(Some(context))); + makecontext(call, &mut stack, None); unreachable!(); }, || reached = true, ).unwrap(); assert!(reached); - assert!(REACHED.with(|reached| reached.get())); let mut context = getcontext(|context| context, || unreachable!()).unwrap(); assert!(! stack_inbounds(ucontext(&mut context), &stack)); @@ -278,14 +277,14 @@ fn killswap_makecontext() { #[cfg_attr(test, should_panic(expected = "done"))] #[cfg_attr(test, test)] fn killswap_sigsetcontext() { - use std::cell::Cell; use libc::MINSIGSTKSZ; + use std::cell::Cell; thread_local! { static CHECKPOINT: Cell> = Cell::new(None); } - extern "C" fn call() { + fn call() { let context = CHECKPOINT.with(|checkpoint| checkpoint.take()).unwrap(); killswap()(context); } @@ -294,11 +293,10 @@ fn killswap_sigsetcontext() { getcontext( |mut call_complete| { let mut stack = [0u8; MINSIGSTKSZ]; - let call_gate = makecontext(call, &mut stack, Some(&mut call_complete)).unwrap(); getcontext( |call_pause| { CHECKPOINT.with(|checkpoint| checkpoint.set(Some(call_pause))); - setcontext(&call_gate); + makecontext(call, &mut stack, Some(&mut call_complete)); unreachable!(); }, || { From b30404ebed689af5a98fc92366240cc53895cd13 Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Thu, 23 Aug 2018 10:54:19 -0400 Subject: [PATCH 42/43] src/ucontext.rs: Fix a makecontext() pointer casting mistake It turns out that sign extension during a widening cast is determined by the signedness of the *source* type; hence, going from c_int s directly to usize s was problematic. After this change, the makecontext_setcontext() test passes consistently. --- src/ucontext.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ucontext.rs b/src/ucontext.rs index aa9f944..61aa655 100644 --- a/src/ucontext.rs +++ b/src/ucontext.rs @@ -155,7 +155,7 @@ pub fn makecontext(mut function: T, stack: &mut [u8], successor: Opt use libc::setcontext; use sp::sp; use std::mem::transmute; - use std::os::raw::c_int; + use std::os::raw::c_uint; enum Link<'a> { WithoutSuccessor(&'a mut dyn FnMut()), @@ -168,7 +168,7 @@ pub fn makecontext(mut function: T, stack: &mut [u8], successor: Opt stack: uintptr_t, } - extern "C" fn link(lower: c_int, upper: c_int) { + extern "C" fn link(lower: c_uint, upper: c_uint) { let link: *mut Link = (lower as usize | ((upper as usize) << 32)) as _; let link = unsafe { link.as_mut() @@ -204,7 +204,7 @@ pub fn makecontext(mut function: T, stack: &mut [u8], successor: Opt let args: usize = &args as *const _ as _; unsafe { - makecontext(&mut context, transmute(link as extern "C" fn(c_int, c_int)), 2, args, args >> 32); + makecontext(&mut context, transmute(link as extern "C" fn(c_uint, c_uint)), 2, args, args >> 32); setcontext(&context); } From 1f4b3823f9edcf49daf6d2c31cc7e6b73bf21681 Mon Sep 17 00:00:00 2001 From: Sol Boucher Date: Mon, 10 Sep 2018 14:16:23 -0400 Subject: [PATCH 43/43] tests/tests.rs: Fix unused import warning --- tests/tests.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/tests.rs b/tests/tests.rs index 9f5640f..b659bff 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -243,7 +243,6 @@ fn stack_inbounds(within: &ucontext_t, stack: &[u8]) -> bool { #[cfg_attr(test, test)] fn killswap_makecontext() { use libc::MINSIGSTKSZ; - use libc::uintptr_t; use std::cell::Cell; thread_local! {