From 8838070a1b71c650b64bb05fba07f78134962ed4 Mon Sep 17 00:00:00 2001 From: Bradlee Speice Date: Sun, 20 Jan 2019 13:19:02 -0500 Subject: [PATCH 1/6] Add an `is_active` function, and remove panic on not using --- .gitignore | 1 + src/lib.rs | 107 +++++++++++++++++++++++++++++++++++------- tests/inactive.rs | 4 ++ tests/unused_panic.rs | 11 ----- 4 files changed, 94 insertions(+), 29 deletions(-) create mode 100644 tests/inactive.rs delete mode 100644 tests/unused_panic.rs diff --git a/.gitignore b/.gitignore index 6936990..2fcd2ae 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ /target **/*.rs.bk Cargo.lock +*.swp diff --git a/src/lib.rs b/src/lib.rs index 53ece0d..5e31d2f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -17,11 +17,17 @@ //! Actually making use of QADAPT is straight-forward. To set up the allocator, //! place the following snippet in either your program binaries (main.rs) or tests: //! -//! ```rust,ignore +//! ```rust,no_run //! use qadapt::QADAPT; //! //! #[global_allocator] //! static Q: QADAPT = QADAPT; +//! +//! fn main() { +//! if cfg!(debug_assertions) { +//! assert!(qadapt::is_active()); +//! } +//! } //! ``` //! //! After that, there are two ways of telling QADAPT that it should trigger a panic: @@ -29,6 +35,11 @@ //! 1. Annotate functions with the `#[no_alloc]` proc macro: //! ```rust,no_run //! use qadapt::no_alloc; +//! use qadapt::QADAPT; +//! use std::panic::catch_unwind; +//! +//! #[global_allocator] +//! static Q: QADAPT = QADAPT; //! //! // This function is fine, there are no allocations here //! #[no_alloc] @@ -44,15 +55,21 @@ //! //! fn main() { //! do_math(); -//! does_panic(); +//! +//! let err = catch_unwind(|| does_panic()); +//! assert!(err.is_err()); //! } //! ``` //! //! 2. Evaluate expressions with the `assert_no_alloc!` macro -//! ```rust,no_run +//! ```rust //! use qadapt::assert_no_alloc; +//! use qadapt::QADAPT; //! -//! fn do_work() { +//! #[global_allocator] +//! static Q: QADAPT = QADAPT; +//! +//! fn main() { //! // This code is allowed to trigger an allocation //! let b = Box::new(8); //! @@ -60,6 +77,7 @@ //! let x = assert_no_alloc!(*b + 2); //! assert_eq!(x, 10); //! } +//! ``` #![deny(missing_docs)] // thread_id is necessary because `std::thread::current()` panics if we have not yet @@ -84,11 +102,17 @@ thread_local! { /// To make use of the allocator, include this code block in your program /// binaries/tests: /// -/// ```rust,ignore +/// ```rust,no_run /// use qadapt::QADAPT; /// /// #[global_allocator] /// static Q: QADAPT = QADAPT; +/// +/// fn main() { +/// if cfg!(debug_assertions) { +/// assert!(qadapt::is_active()); +/// } +/// } /// ``` pub struct QADAPT; @@ -99,9 +123,13 @@ static SYSTEM_ALLOC: System = System; /// /// **Example**: /// -/// ```rust,no_run +/// ```rust /// use qadapt::enter_protected; /// use qadapt::exit_protected; +/// use qadapt::QADAPT; +/// +/// #[global_allocator] +/// static Q: QADAPT = QADAPT; /// /// fn main() { /// // Force an allocation by using a Box @@ -119,14 +147,10 @@ static SYSTEM_ALLOC: System = System; pub fn enter_protected() { #[cfg(debug_assertions)] { - if thread::panicking() { + if thread::panicking() || !is_active() { return; } - if !*IS_ACTIVE.read() { - panic!("QADAPT not initialized when using allocation guards; please verify `#[global_allocator]` is set!"); - } - PROTECTION_LEVEL .try_with(|v| { *v.write() += 1; @@ -140,9 +164,13 @@ pub fn enter_protected() { /// /// **Example**: /// -/// ```rust,no_run +/// ```rust /// use qadapt::enter_protected; /// use qadapt::exit_protected; +/// use qadapt::QADAPT; +/// +/// #[global_allocator] +/// static Q: QADAPT = QADAPT; /// /// fn main() { /// // Force an allocation by using a Box @@ -160,7 +188,7 @@ pub fn enter_protected() { pub fn exit_protected() { #[cfg(debug_assertions)] { - if thread::panicking() { + if thread::panicking() || !is_active() { return; } @@ -183,8 +211,12 @@ pub fn exit_protected() { /// /// **Example**: /// -/// ```rust,no_run +/// ```rust /// use qadapt::assert_no_alloc; +/// use qadapt::QADAPT; +/// +/// #[global_allocator] +/// static Q: QADAPT = QADAPT; /// /// fn main() { /// assert_no_alloc!(2 + 2); @@ -198,6 +230,11 @@ pub fn exit_protected() { /// /// ```rust,no_run /// use qadapt::assert_no_alloc; +/// use qadapt::QADAPT; +/// use std::panic::catch_unwind; +/// +/// #[global_allocator] +/// static Q: QADAPT = QADAPT; /// /// fn early_return() -> usize { /// assert_no_alloc!(return 8); @@ -206,10 +243,12 @@ pub fn exit_protected() { /// fn main() { /// let x = early_return(); /// -/// // This triggers a panic - `Box::new` forces an allocation, -/// // and QADAPT still thinks we're in a protected region because -/// // of a return in the `early_return()` function -/// let b = Box::new(x); +/// // Even though only the `early_return` function contains +/// // QADAPT allocation guards, this triggers a panic: +/// // `Box::new` forces an allocation, and QADAPT still thinks +/// // we're in a protected region because of the return in `early_return()` +/// let res = catch_unwind(|| Box::new(x)); +/// assert!(res.is_err()); /// } #[macro_export] macro_rules! assert_no_alloc { @@ -231,8 +270,12 @@ static INTERNAL_ALLOCATION: RwLock = RwLock::new(usize::max_value()); /// ```rust,no_run /// use qadapt::enter_protected; /// use qadapt::exit_protected; +/// use qadapt::QADAPT; /// use qadapt::protection_level; /// +/// #[global_allocator] +/// static Q: QADAPT = QADAPT; +/// /// fn main() { /// enter_protected(); /// // We're now in an allocation-protected code region @@ -254,6 +297,34 @@ pub fn protection_level() -> usize { } } +/// Determine whether qadapt is running as the current global allocator. Useful for +/// double-checking that you will in fact panic if allocations happen in guarded code. +/// +/// **Note**: when running in `release` profile, `is_active()` will always return false. +/// +/// **Example**: +/// +/// ```rust,no_run +/// use qadapt::is_active; +/// use qadapt::QADAPT; +/// +/// #[global_allocator] +/// static Q: QADAPT = QADAPT; +/// +/// pub fn main() { +/// if cfg!(debug_assertions) { +/// assert!(is_active()); +/// } +/// } +/// ``` +pub fn is_active() -> bool { + if cfg!(debug_assertions) { + *IS_ACTIVE.read() + } else { + false + } +} + fn claim_internal_alloc() { loop { match INTERNAL_ALLOCATION.write() { diff --git a/tests/inactive.rs b/tests/inactive.rs new file mode 100644 index 0000000..cf08118 --- /dev/null +++ b/tests/inactive.rs @@ -0,0 +1,4 @@ +#[test] +fn is_inactive() { + assert!(!qadapt::is_active()); +} diff --git a/tests/unused_panic.rs b/tests/unused_panic.rs deleted file mode 100644 index e185a82..0000000 --- a/tests/unused_panic.rs +++ /dev/null @@ -1,11 +0,0 @@ -use qadapt::enter_protected; - -#[test] -#[should_panic] -fn guard_without_initialization() { - if cfg!(debug_assertions) { - enter_protected(); - } else { - panic!("Intentional") - } -} From 87beb53ccd846bc725ffc54af7f60a3308894d80 Mon Sep 17 00:00:00 2001 From: Bradlee Speice Date: Sun, 20 Jan 2019 13:29:38 -0500 Subject: [PATCH 2/6] Minor note about protection level during release --- src/lib.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 5e31d2f..cb15e1a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -263,7 +263,9 @@ macro_rules! assert_no_alloc { static IS_ACTIVE: RwLock = RwLock::new(false); static INTERNAL_ALLOCATION: RwLock = RwLock::new(usize::max_value()); -/// Get the current "protection level" in QADAPT: calls to enter_protected() - exit_protected() +/// Get the current "protection level" in QADAPT: calls to `enter_protected() - exit_protected()`. +/// +/// **Note**: For release builds, `protection_level()` will always return 0. /// /// **Example**: /// @@ -290,11 +292,7 @@ static INTERNAL_ALLOCATION: RwLock = RwLock::new(usize::max_value()); /// // It's now safe to allocate/drop /// } pub fn protection_level() -> usize { - if cfg!(debug_assertions) { - PROTECTION_LEVEL.try_with(|v| *v.read()).unwrap_or(0) - } else { - 0 - } + PROTECTION_LEVEL.try_with(|v| *v.read()).unwrap_or(0) } /// Determine whether qadapt is running as the current global allocator. Useful for From 08dac29b82a4dd7e2c8e566dc7628425e003e747 Mon Sep 17 00:00:00 2001 From: Bradlee Speice Date: Sun, 20 Jan 2019 14:40:57 -0500 Subject: [PATCH 3/6] Remove `no_run` from doctests Uses `qadapt::is_active` to work around rust-lang/cargo#6570. Currently assumes that *some* allocation will have happened by the time `fn main()` is called, which is sub-par. --- src/lib.rs | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index cb15e1a..fb29bec 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -17,23 +17,27 @@ //! Actually making use of QADAPT is straight-forward. To set up the allocator, //! place the following snippet in either your program binaries (main.rs) or tests: //! -//! ```rust,no_run +//! ```rust //! use qadapt::QADAPT; //! //! #[global_allocator] //! static Q: QADAPT = QADAPT; //! //! fn main() { +//! # // Because `debug_assertions` are on for doctests in release mode +//! # // we have to add an extra guard. +//! # if qadapt::is_active() { //! if cfg!(debug_assertions) { //! assert!(qadapt::is_active()); //! } +//! # } //! } //! ``` //! //! After that, there are two ways of telling QADAPT that it should trigger a panic: //! //! 1. Annotate functions with the `#[no_alloc]` proc macro: -//! ```rust,no_run +//! ```rust //! use qadapt::no_alloc; //! use qadapt::QADAPT; //! use std::panic::catch_unwind; @@ -57,7 +61,9 @@ //! do_math(); //! //! let err = catch_unwind(|| does_panic()); +//! # if qadapt::is_active() { //! assert!(err.is_err()); +//! # } //! } //! ``` //! @@ -96,22 +102,27 @@ use std::thread; thread_local! { static PROTECTION_LEVEL: RwLock = RwLock::new(0); } +static IS_ACTIVE: RwLock = RwLock::new(false); +static INTERNAL_ALLOCATION: RwLock = RwLock::new(usize::max_value()); + /// The QADAPT allocator itself /// /// To make use of the allocator, include this code block in your program /// binaries/tests: /// -/// ```rust,no_run +/// ```rust /// use qadapt::QADAPT; /// /// #[global_allocator] /// static Q: QADAPT = QADAPT; /// /// fn main() { +/// # if qadapt::is_active() { /// if cfg!(debug_assertions) { /// assert!(qadapt::is_active()); /// } +/// # } /// } /// ``` pub struct QADAPT; @@ -228,7 +239,7 @@ pub fn exit_protected() { /// in code that was not intended to be allocation-free. The compiler will warn you /// that there is an unreachable statement if this happens. /// -/// ```rust,no_run +/// ```rust /// use qadapt::assert_no_alloc; /// use qadapt::QADAPT; /// use std::panic::catch_unwind; @@ -247,8 +258,10 @@ pub fn exit_protected() { /// // QADAPT allocation guards, this triggers a panic: /// // `Box::new` forces an allocation, and QADAPT still thinks /// // we're in a protected region because of the return in `early_return()` +/// # if qadapt::is_active() { /// let res = catch_unwind(|| Box::new(x)); /// assert!(res.is_err()); +/// # } /// } #[macro_export] macro_rules! assert_no_alloc { @@ -260,16 +273,13 @@ macro_rules! assert_no_alloc { }}; } -static IS_ACTIVE: RwLock = RwLock::new(false); -static INTERNAL_ALLOCATION: RwLock = RwLock::new(usize::max_value()); - /// Get the current "protection level" in QADAPT: calls to `enter_protected() - exit_protected()`. /// /// **Note**: For release builds, `protection_level()` will always return 0. /// /// **Example**: /// -/// ```rust,no_run +/// ```rust /// use qadapt::enter_protected; /// use qadapt::exit_protected; /// use qadapt::QADAPT; @@ -279,6 +289,7 @@ static INTERNAL_ALLOCATION: RwLock = RwLock::new(usize::max_value()); /// static Q: QADAPT = QADAPT; /// /// fn main() { +/// # if qadapt::is_active() { /// enter_protected(); /// // We're now in an allocation-protected code region /// assert_eq!(1, protection_level()); @@ -288,6 +299,7 @@ static INTERNAL_ALLOCATION: RwLock = RwLock::new(usize::max_value()); /// assert_eq!(2, protection_level()); /// exit_protected(); /// exit_protected(); +/// # } /// /// // It's now safe to allocate/drop /// } @@ -302,7 +314,7 @@ pub fn protection_level() -> usize { /// /// **Example**: /// -/// ```rust,no_run +/// ```rust /// use qadapt::is_active; /// use qadapt::QADAPT; /// @@ -310,9 +322,11 @@ pub fn protection_level() -> usize { /// static Q: QADAPT = QADAPT; /// /// pub fn main() { +/// # if qadapt::is_active() { /// if cfg!(debug_assertions) { /// assert!(is_active()); /// } +/// # } /// } /// ``` pub fn is_active() -> bool { From 640269ff1075ab4c9702bd68a4a17d72543eaeb7 Mon Sep 17 00:00:00 2001 From: Bradlee Speice Date: Sun, 20 Jan 2019 15:18:41 -0500 Subject: [PATCH 4/6] Documentation updates --- src/lib.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index fb29bec..6cb6f90 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -105,7 +105,6 @@ thread_local! { static IS_ACTIVE: RwLock = RwLock::new(false); static INTERNAL_ALLOCATION: RwLock = RwLock::new(usize::max_value()); - /// The QADAPT allocator itself /// /// To make use of the allocator, include this code block in your program @@ -307,10 +306,12 @@ pub fn protection_level() -> usize { PROTECTION_LEVEL.try_with(|v| *v.read()).unwrap_or(0) } -/// Determine whether qadapt is running as the current global allocator. Useful for -/// double-checking that you will in fact panic if allocations happen in guarded code. +/// Determine whether QADAPT will trigger thread panics if an allocation happens +/// during protected code. This should be used for making sure that QADAPT is +/// properly set up and initialized. /// -/// **Note**: when running in `release` profile, `is_active()` will always return false. +/// Note that this will return `false` in release builds even if QADAPT is set +/// as the `#[global_allocator]`. /// /// **Example**: /// @@ -331,6 +332,9 @@ pub fn protection_level() -> usize { /// ``` pub fn is_active() -> bool { if cfg!(debug_assertions) { + // Because there are heap allocations that happen before `fn main()`, + // we don't need to force an extra allocation here to guarantee that + // IS_ACTIVE is set *IS_ACTIVE.read() } else { false From 8df83a56b1f129d3c90af75bcc7f29616b717b96 Mon Sep 17 00:00:00 2001 From: Bradlee Speice Date: Sun, 20 Jan 2019 15:20:59 -0500 Subject: [PATCH 5/6] Add a release-only test for `is_active()` behavior --- tests/inactive_release.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 tests/inactive_release.rs diff --git a/tests/inactive_release.rs b/tests/inactive_release.rs new file mode 100644 index 0000000..05a5627 --- /dev/null +++ b/tests/inactive_release.rs @@ -0,0 +1,10 @@ +use qadapt::QADAPT; + +#[global_allocator] +static Q: QADAPT = QADAPT; + +#[cfg(not(debug_assertions))] +#[test] +fn release_only_inactive() { + assert!(!qadapt::is_active()); +} From 029ca6aba9597d6e6ab860c48e065515366b8ee7 Mon Sep 17 00:00:00 2001 From: Bradlee Speice Date: Sun, 20 Jan 2019 15:25:41 -0500 Subject: [PATCH 6/6] Don't have doctests depend on debug_assertions --- src/lib.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 6cb6f90..cc88ad4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -27,9 +27,7 @@ //! # // Because `debug_assertions` are on for doctests in release mode //! # // we have to add an extra guard. //! # if qadapt::is_active() { -//! if cfg!(debug_assertions) { -//! assert!(qadapt::is_active()); -//! } +//! assert!(qadapt::is_active()); //! # } //! } //! ``` @@ -118,9 +116,7 @@ static INTERNAL_ALLOCATION: RwLock = RwLock::new(usize::max_value()); /// /// fn main() { /// # if qadapt::is_active() { -/// if cfg!(debug_assertions) { -/// assert!(qadapt::is_active()); -/// } +/// assert!(qadapt::is_active()); /// # } /// } /// ``` @@ -324,9 +320,7 @@ pub fn protection_level() -> usize { /// /// pub fn main() { /// # if qadapt::is_active() { -/// if cfg!(debug_assertions) { -/// assert!(is_active()); -/// } +/// assert!(is_active()); /// # } /// } /// ```