From ea7f4ced07842f23b8590f6df0ca3aeff416d7b1 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Sat, 13 Sep 2025 10:26:40 +0200 Subject: [PATCH] uefi: remove support for unstable allocator_api feature The allocator_api feature [0] is old and not developed in years. Since then, understanding of memory safety and best practises has evolved. It is unlikely that in its current form the functionality will ever be merged. Therefore, we drop the complexity we have from this feature for now, leading to simpler code. [0] https://github.com/rust-lang/rust/issues/32838 --- uefi/CHANGELOG.md | 5 +++ uefi/src/lib.rs | 7 ++-- uefi/src/mem/util.rs | 57 +++---------------------------- uefi/src/proto/hii/database.rs | 4 --- uefi/src/proto/media/file/dir.rs | 37 -------------------- uefi/src/proto/media/file/mod.rs | 17 --------- uefi/src/proto/media/load_file.rs | 11 ------ uefi/src/proto/tcg/v1.rs | 15 +------- uefi/src/proto/tcg/v2.rs | 15 +------- uefi/src/runtime.rs | 12 +------ xtask/src/main.rs | 7 ++-- 11 files changed, 17 insertions(+), 170 deletions(-) diff --git a/uefi/CHANGELOG.md b/uefi/CHANGELOG.md index c5df6afbc..236cf67bb 100644 --- a/uefi/CHANGELOG.md +++ b/uefi/CHANGELOG.md @@ -25,6 +25,11 @@ - The documentation for UEFI protocols has been streamlined and improved. - Fixed memory safety bug in `SimpleNetwork::read_nv_data`. The `buffer` parameter is now mutable. +- Removed all internal usages including public APIs using the unstable + `allocator_api` feature. It may be reintroduced if it will have a chance of + getting stabilized in stable Rust. + - Removed `File::get_boxed_info_in` + - Removed `Directory::read_entry_boxed_in` # uefi - 0.35.0 (2025-05-04) diff --git a/uefi/src/lib.rs b/uefi/src/lib.rs index 718090349..541d2307a 100644 --- a/uefi/src/lib.rs +++ b/uefi/src/lib.rs @@ -142,10 +142,8 @@ //! - `log-debugcon`: Whether the logger set up by `logger` should also log //! to the debugcon device (available in QEMU or Cloud Hypervisor on x86). //! - `panic_handler`: Add a default panic handler that logs to `stdout`. -//! - `unstable`: Enable functionality that depends on [unstable -//! features] in the nightly compiler. -//! As example, in conjunction with the `alloc`-feature, this gate allows -//! the `allocator_api` on certain functions. +//! - `unstable`: Enable functionality that depends on [unstable features] in +//! the Rust compiler (nightly version). //! - `qemu`: Enable some code paths to adapt their execution when executed //! in QEMU, such as using the special `qemu-exit` device when the panic //! handler is called. @@ -229,7 +227,6 @@ //! [uefi-std-tr-issue]: https://github.com/rust-lang/rust/issues/100499 //! [unstable features]: https://doc.rust-lang.org/unstable-book/ -#![cfg_attr(all(feature = "unstable", feature = "alloc"), feature(allocator_api))] #![cfg_attr(docsrs, feature(doc_auto_cfg))] #![no_std] #![deny( diff --git a/uefi/src/mem/util.rs b/uefi/src/mem/util.rs index 4e4d6e890..60a451fda 100644 --- a/uefi/src/mem/util.rs +++ b/uefi/src/mem/util.rs @@ -4,17 +4,12 @@ use crate::data_types::Align; use crate::{Error, Result, ResultExt, Status}; +use ::alloc::alloc::{alloc, dealloc}; use ::alloc::boxed::Box; use core::alloc::Layout; use core::fmt::Debug; use core::slice; -#[cfg(not(feature = "unstable"))] -use ::alloc::alloc::{alloc, dealloc}; - -#[cfg(feature = "unstable")] -use {core::alloc::Allocator, core::ptr::NonNull}; - /// Helper to return owned versions of certain UEFI data structures on the heap in a [`Box`]. This /// function is intended to wrap low-level UEFI functions of this crate that /// - can consume an empty buffer without a panic to get the required buffer size in the errors @@ -23,31 +18,14 @@ use {core::alloc::Allocator, core::ptr::NonNull}; /// buffer size is sufficient, and /// - return a mutable typed reference that points to the same memory as the input buffer on /// success. -/// -/// # Feature `unstable` / `allocator_api` -/// By default, this function works with the allocator that is set as -/// `#[global_allocator]`. This might be UEFI allocator but depends on your -/// use case and how you set up the environment. -/// -/// If you activate the `unstable`-feature, all allocations uses the provided -/// allocator (via `allocator_api`) instead. In that case, the function takes an -/// additional parameter describing the specific [`Allocator`]. You can use -/// [`alloc::alloc::Global`] which defaults to the `#[global_allocator]`. -/// -/// [`Allocator`]: https://doc.rust-lang.org/alloc/alloc/trait.Allocator.html -/// [`alloc::alloc::Global`]: https://doc.rust-lang.org/alloc/alloc/struct.Global.html pub(crate) fn make_boxed< 'a, // The UEFI data structure. Data: Align + ?Sized + Debug + 'a, F: FnMut(&'a mut [u8]) -> Result<&'a mut Data, Option>, - #[cfg(feature = "unstable")] A: Allocator, >( // A function to read the UEFI data structure into a provided buffer. mut fetch_data_fn: F, - #[cfg(feature = "unstable")] - // Allocator of the `allocator_api` feature. You can use `Global` as default. - allocator: A, ) -> Result> { let required_size = match fetch_data_fn(&mut []).map_err(Error::split) { // This is the expected case: the empty buffer passed in is too @@ -70,7 +48,6 @@ pub(crate) fn make_boxed< // Allocate the buffer on the heap. let heap_buf: *mut u8 = { - #[cfg(not(feature = "unstable"))] { let ptr = unsafe { alloc(layout) }; if ptr.is_null() { @@ -78,13 +55,6 @@ pub(crate) fn make_boxed< } ptr } - - #[cfg(feature = "unstable")] - allocator - .allocate(layout) - .map_err(|_| >::into(Status::OUT_OF_RESOURCES))? - .as_ptr() - .cast::() }; // Read the data into the provided buffer. @@ -97,20 +67,12 @@ pub(crate) fn make_boxed< let data: &mut Data = match data { Ok(data) => data, Err(err) => { - #[cfg(not(feature = "unstable"))] - unsafe { - dealloc(heap_buf, layout) - }; - #[cfg(feature = "unstable")] - unsafe { - allocator.deallocate(NonNull::new(heap_buf).unwrap(), layout) - } + unsafe { dealloc(heap_buf, layout) }; return Err(err); } }; let data = unsafe { Box::from_raw(data) }; - Ok(data) } @@ -118,8 +80,6 @@ pub(crate) fn make_boxed< mod tests { use super::*; use crate::{ResultExt, StatusExt}; - #[cfg(feature = "unstable")] - use alloc::alloc::Global; /// Some simple dummy type to test [`make_boxed`]. #[derive(Debug)] @@ -212,27 +172,20 @@ mod tests { assert_eq!(&data.0.0, &[1, 2, 3, 4]); } - /// This unit tests checks the [`make_boxed`] utility. The test has different code and behavior - /// depending on whether the "unstable" feature is active or not. + /// This unit tests checks the [`make_boxed`] utility. + /// + /// This test is especially useful when run by miri. #[test] fn test_make_boxed_utility() { let fetch_data_fn = |buf| uefi_function_stub_read(buf); - #[cfg(not(feature = "unstable"))] let data: Box = make_boxed(fetch_data_fn).unwrap(); - - #[cfg(feature = "unstable")] - let data: Box = make_boxed(fetch_data_fn, Global).unwrap(); assert_eq!(&data.0, &[1, 2, 3, 4]); let fetch_data_fn = |buf| uefi_function_stub_read(buf); - #[cfg(not(feature = "unstable"))] let data: Box = make_boxed(fetch_data_fn).unwrap(); - #[cfg(feature = "unstable")] - let data: Box = make_boxed(fetch_data_fn, Global).unwrap(); - assert_eq!(&data.0.0, &[1, 2, 3, 4]); } } diff --git a/uefi/src/proto/hii/database.rs b/uefi/src/proto/hii/database.rs index 8c1688132..6cee354e7 100644 --- a/uefi/src/proto/hii/database.rs +++ b/uefi/src/proto/hii/database.rs @@ -44,12 +44,8 @@ impl HiiDatabase { } } - #[cfg(not(feature = "unstable"))] let buf = make_boxed::<[u8], _>(|buf| fetch_data_fn(self, buf))?; - #[cfg(feature = "unstable")] - let buf = make_boxed::<[u8], _, _>(|buf| fetch_data_fn(self, buf), alloc::alloc::Global)?; - Ok(buf) } } diff --git a/uefi/src/proto/media/file/dir.rs b/uefi/src/proto/media/file/dir.rs index f3ba174eb..aee266017 100644 --- a/uefi/src/proto/media/file/dir.rs +++ b/uefi/src/proto/media/file/dir.rs @@ -6,8 +6,6 @@ use crate::data_types::Align; use core::ffi::c_void; #[cfg(feature = "alloc")] use {crate::mem::make_boxed, alloc::boxed::Box}; -#[cfg(all(feature = "unstable", feature = "alloc"))] -use {alloc::alloc::Global, core::alloc::Allocator}; /// A `FileHandle` that is also a directory. /// @@ -80,42 +78,7 @@ impl Directory { maybe_info.expect("Should have more entries") }) }; - - #[cfg(not(feature = "unstable"))] let file_info = make_boxed::(fetch_data_fn)?; - - #[cfg(feature = "unstable")] - let file_info = make_boxed::(fetch_data_fn, Global)?; - - Ok(Some(file_info)) - } - - /// Wrapper around [`Self::read_entry`] that returns an owned copy of the data. It has the same - /// implications and requirements. On failure, the payload of `Err` is `()ยด. - /// - /// It allows to use a custom allocator via the `allocator_api` feature. - #[cfg(all(feature = "unstable", feature = "alloc"))] - pub fn read_entry_boxed_in( - &mut self, - allocator: A, - ) -> Result>> { - let read_entry_res = self.read_entry(&mut []); - - // If no more entries are available, return early. - if read_entry_res == Ok(None) { - return Ok(None); - } - - let fetch_data_fn = |buf| { - self.read_entry(buf) - // this is safe, as above, we checked that there are more entries - .map(|maybe_info: Option<&mut FileInfo>| { - maybe_info.expect("Should have more entries") - }) - }; - - let file_info = make_boxed::(fetch_data_fn, allocator)?; - Ok(Some(file_info)) } diff --git a/uefi/src/proto/media/file/mod.rs b/uefi/src/proto/media/file/mod.rs index 856cb0790..594b1b548 100644 --- a/uefi/src/proto/media/file/mod.rs +++ b/uefi/src/proto/media/file/mod.rs @@ -20,9 +20,6 @@ use core::fmt::Debug; use core::{mem, ptr}; use uefi_raw::protocol::file_system::FileProtocolV1; -#[cfg(all(feature = "unstable", feature = "alloc"))] -use {alloc::alloc::Global, core::alloc::Allocator}; - #[cfg(feature = "alloc")] use {crate::mem::make_boxed, alloc::boxed::Box}; @@ -198,21 +195,7 @@ pub trait File: Sized { #[cfg(feature = "alloc")] fn get_boxed_info(&mut self) -> Result> { let fetch_data_fn = |buf| self.get_info::(buf); - #[cfg(not(feature = "unstable"))] let file_info = make_boxed::(fetch_data_fn)?; - #[cfg(feature = "unstable")] - let file_info = make_boxed::(fetch_data_fn, Global)?; - Ok(file_info) - } - - /// Read the dynamically allocated info for a file. - #[cfg(all(feature = "unstable", feature = "alloc"))] - fn get_boxed_info_in( - &mut self, - allocator: A, - ) -> Result> { - let fetch_data_fn = |buf| self.get_info::(buf); - let file_info = make_boxed::(fetch_data_fn, allocator)?; Ok(file_info) } diff --git a/uefi/src/proto/media/load_file.rs b/uefi/src/proto/media/load_file.rs index ccb706514..346c6bbac 100644 --- a/uefi/src/proto/media/load_file.rs +++ b/uefi/src/proto/media/load_file.rs @@ -5,8 +5,6 @@ #[cfg(doc)] use crate::Status; use crate::proto::unsafe_protocol; -#[cfg(all(feature = "alloc", feature = "unstable"))] -use alloc::alloc::Global; use uefi_raw::protocol::media::{LoadFile2Protocol, LoadFileProtocol}; #[cfg(feature = "alloc")] use { @@ -90,12 +88,7 @@ impl LoadFile { status.to_result_with_err(|_| Some(size)).map(|_| buf) }; - #[cfg(not(feature = "unstable"))] let file: Box<[u8]> = make_boxed::<[u8], _>(fetch_data_fn)?; - - #[cfg(feature = "unstable")] - let file = make_boxed::<[u8], _, _>(fetch_data_fn, Global)?; - Ok(file) } } @@ -158,12 +151,8 @@ impl LoadFile2 { status.to_result_with_err(|_| Some(size)).map(|_| buf) }; - #[cfg(not(feature = "unstable"))] let file: Box<[u8]> = make_boxed::<[u8], _>(fetch_data_fn)?; - #[cfg(feature = "unstable")] - let file = make_boxed::<[u8], _, _>(fetch_data_fn, Global)?; - Ok(file) } } diff --git a/uefi/src/proto/tcg/v1.rs b/uefi/src/proto/tcg/v1.rs index 07a497a58..0394045b6 100644 --- a/uefi/src/proto/tcg/v1.rs +++ b/uefi/src/proto/tcg/v1.rs @@ -24,9 +24,6 @@ use uefi_raw::protocol::tcg::v1::{TcgBootServiceCapability, TcgProtocol}; #[cfg(feature = "alloc")] use {crate::mem::make_boxed, alloc::boxed::Box}; -#[cfg(all(feature = "unstable", feature = "alloc"))] -use alloc::alloc::Global; - pub use uefi_raw::protocol::tcg::v1::TcgVersion as Version; /// 20-byte SHA-1 digest. @@ -157,17 +154,7 @@ impl PcrEvent { digest: Sha1Digest, event_data: &[u8], ) -> Result> { - #[cfg(not(feature = "unstable"))] - { - make_boxed(|buf| Self::new_in_buffer(buf, pcr_index, event_type, digest, event_data)) - } - #[cfg(feature = "unstable")] - { - make_boxed( - |buf| Self::new_in_buffer(buf, pcr_index, event_type, digest, event_data), - Global, - ) - } + make_boxed(|buf| Self::new_in_buffer(buf, pcr_index, event_type, digest, event_data)) } /// PCR index for the event. diff --git a/uefi/src/proto/tcg/v2.rs b/uefi/src/proto/tcg/v2.rs index 383402274..308cc7571 100644 --- a/uefi/src/proto/tcg/v2.rs +++ b/uefi/src/proto/tcg/v2.rs @@ -26,9 +26,6 @@ use uefi_raw::protocol::tcg::v2::{Tcg2EventHeader as EventHeader, Tcg2Protocol}; #[cfg(feature = "alloc")] use {crate::mem::make_boxed, alloc::boxed::Box}; -#[cfg(all(feature = "unstable", feature = "alloc"))] -use alloc::alloc::Global; - pub use uefi_raw::protocol::tcg::v2::{ Tcg2EventLogFormat as EventLogFormat, Tcg2HashAlgorithmBitmap, Tcg2HashLogExtendEventFlags as HashLogExtendEventFlags, Tcg2Version as Version, @@ -183,17 +180,7 @@ impl PcrEventInputs { event_type: EventType, event_data: &[u8], ) -> Result> { - #[cfg(not(feature = "unstable"))] - { - make_boxed(|buf| Self::new_in_buffer(buf, pcr_index, event_type, event_data)) - } - #[cfg(feature = "unstable")] - { - make_boxed( - |buf| Self::new_in_buffer(buf, pcr_index, event_type, event_data), - Global, - ) - } + make_boxed(|buf| Self::new_in_buffer(buf, pcr_index, event_type, event_data)) } } diff --git a/uefi/src/runtime.rs b/uefi/src/runtime.rs index ec359a58e..e1be79d1c 100644 --- a/uefi/src/runtime.rs +++ b/uefi/src/runtime.rs @@ -24,9 +24,6 @@ use { alloc::{vec, vec::Vec}, }; -#[cfg(all(feature = "unstable", feature = "alloc"))] -use alloc::alloc::Global; - pub use uefi_raw::capsule::{CapsuleBlockDescriptor, CapsuleFlags, CapsuleHeader}; pub use uefi_raw::table::runtime::{ ResetType, TimeCapabilities, VariableAttributes, VariableVendor, @@ -187,14 +184,7 @@ pub fn get_variable_boxed( val }) }; - #[cfg(not(feature = "unstable"))] - { - make_boxed(get_var).map(|val| (val, out_attr)) - } - #[cfg(feature = "unstable")] - { - make_boxed(get_var, Global).map(|val| (val, out_attr)) - } + make_boxed(get_var).map(|val| (val, out_attr)) } /// Gets each variable key (name and vendor) one at a time. diff --git a/xtask/src/main.rs b/xtask/src/main.rs index d47f7b595..4b739b682 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -214,13 +214,10 @@ fn run_host_tests(test_opt: &TestOpt) -> Result<()> { packages.push(Package::UefiMacros); } - // Run uefi-rs and uefi-macros tests. + // Run uefi-rs and uefi-macros tests with `unstable` feature. let cargo = Cargo { action: CargoAction::Test, - // At least one unit test, for make_boxed() currently, has different behaviour dependent on - // the unstable feature. Because of this, we need to allow to test both variants. Runtime - // features is set to no as it is not possible as as soon a #[global_allocator] is - // registered, the Rust runtime executing the tests uses it as well. + // Some tests may behave differently depending on the unstable feature. features: Feature::more_code(*test_opt.unstable, false), packages, release: false,