-
Notifications
You must be signed in to change notification settings - Fork 473
Add XArray abstraction #1019
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: rust-dev
Are you sure you want to change the base?
Add XArray abstraction #1019
Changes from all commits
520a56a
3d8d9bc
7671527
0ad204c
70a61eb
a250bde
00383f7
8b2e73a
eb9199d
6bed841
600eb66
6d4220b
10452fc
6a4e6dc
f2f2787
53dd54f
02ddf5a
276f14d
c30db85
4009de1
e9e95cd
eff7a66
72fbe4a
8e87a0d
7c6e564
522ba05
b1555e1
19b33fc
c1c510f
91f4147
6c19acb
3016eac
90ddba2
702746d
976067b
ac520c2
642ff49
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,156 @@ | ||
// SPDX-License-Identifier: GPL-2.0 | ||
|
||
//! KUnit-based macros for Rust unit tests. | ||
//! | ||
//! C header: [`include/kunit/test.h`](../../../../../include/kunit/test.h) | ||
//! | ||
//! Reference: <https://docs.kernel.org/dev-tools/kunit/index.html> | ||
use core::{ffi::c_void, fmt}; | ||
|
||
/// Prints a KUnit error. | ||
/// | ||
/// Public but hidden since it should only be used from KUnit generated code. | ||
#[doc(hidden)] | ||
pub fn err(args: fmt::Arguments<'_>) { | ||
// SAFETY: The format string is null-terminated and the `%pA` specifier matches the argument we | ||
// are passing. | ||
#[cfg(CONFIG_PRINTK)] | ||
unsafe { | ||
bindings::_printk( | ||
b"\x013%pA\0".as_ptr() as _, | ||
&args as *const _ as *const c_void, | ||
); | ||
} | ||
} | ||
|
||
/// Prints a KUnit error. | ||
/// | ||
/// Public but hidden since it should only be used from KUnit generated code. | ||
#[doc(hidden)] | ||
pub fn info(args: fmt::Arguments<'_>) { | ||
// SAFETY: The format string is null-terminated and the `%pA` specifier matches the argument we | ||
// are passing. | ||
#[cfg(CONFIG_PRINTK)] | ||
unsafe { | ||
bindings::_printk( | ||
b"\x016%pA\0".as_ptr() as _, | ||
&args as *const _ as *const c_void, | ||
); | ||
} | ||
} | ||
|
||
/// Asserts that a boolean expression is `true` at runtime. | ||
/// | ||
/// Public but hidden since it should only be used from generated tests. | ||
/// | ||
/// Unlike the one in `core`, this one does not panic; instead, it is mapped to the KUnit | ||
/// facilities. See [`assert!`] for more details. | ||
#[doc(hidden)] | ||
#[macro_export] | ||
macro_rules! kunit_assert { | ||
($name:literal, $condition:expr $(,)?) => { | ||
'out: { | ||
// Do nothing if the condition is `true`. | ||
if $condition { | ||
break 'out; | ||
} | ||
|
||
static LINE: i32 = core::line!() as i32; | ||
static FILE: &'static $crate::str::CStr = $crate::c_str!(core::file!()); | ||
static CONDITION: &'static $crate::str::CStr = $crate::c_str!(stringify!($condition)); | ||
|
||
// SAFETY: FFI call without safety requirements. | ||
let kunit_test = unsafe { $crate::bindings::kunit_get_current_test() }; | ||
if kunit_test.is_null() { | ||
// The assertion failed but this task is not running a KUnit test, so we cannot call | ||
// KUnit, but at least print an error to the kernel log. This may happen if this | ||
// macro is called from an spawned thread in a test (see | ||
// `scripts/rustdoc_test_gen.rs`) or if some non-test code calls this macro by | ||
// mistake (it is hidden to prevent that). | ||
// | ||
// This mimics KUnit's failed assertion format. | ||
$crate::kunit::err(format_args!( | ||
" # {}: ASSERTION FAILED at {FILE}:{LINE}\n", | ||
$name | ||
)); | ||
$crate::kunit::err(format_args!( | ||
" Expected {CONDITION} to be true, but is false\n" | ||
)); | ||
$crate::kunit::err(format_args!( | ||
" Failure not reported to KUnit since this is a non-KUnit task\n" | ||
)); | ||
break 'out; | ||
} | ||
|
||
#[repr(transparent)] | ||
struct Location($crate::bindings::kunit_loc); | ||
|
||
#[repr(transparent)] | ||
struct UnaryAssert($crate::bindings::kunit_unary_assert); | ||
|
||
// SAFETY: There is only a static instance and in that one the pointer field points to | ||
// an immutable C string. | ||
unsafe impl Sync for Location {} | ||
|
||
// SAFETY: There is only a static instance and in that one the pointer field points to | ||
// an immutable C string. | ||
unsafe impl Sync for UnaryAssert {} | ||
|
||
static LOCATION: Location = Location($crate::bindings::kunit_loc { | ||
file: FILE.as_char_ptr(), | ||
line: LINE, | ||
}); | ||
static ASSERTION: UnaryAssert = UnaryAssert($crate::bindings::kunit_unary_assert { | ||
assert: $crate::bindings::kunit_assert {}, | ||
condition: CONDITION.as_char_ptr(), | ||
expected_true: true, | ||
}); | ||
|
||
// SAFETY: | ||
// - FFI call. | ||
// - The `kunit_test` pointer is valid because we got it from | ||
// `kunit_get_current_test()` and it was not null. This means we are in a KUnit | ||
// test, and that the pointer can be passed to KUnit functions and assertions. | ||
// - The string pointers (`file` and `condition` above) point to null-terminated | ||
// strings since they are `CStr`s. | ||
// - The function pointer (`format`) points to the proper function. | ||
// - The pointers passed will remain valid since they point to `static`s. | ||
// - The format string is allowed to be null. | ||
// - There are, however, problems with this: first of all, this will end up stopping | ||
// the thread, without running destructors. While that is problematic in itself, | ||
// it is considered UB to have what is effectively a forced foreign unwind | ||
// with `extern "C"` ABI. One could observe the stack that is now gone from | ||
// another thread. We should avoid pinning stack variables to prevent library UB, | ||
// too. For the moment, given that test failures are reported immediately before the | ||
// next test runs, that test failures should be fixed and that KUnit is explicitly | ||
// documented as not suitable for production environments, we feel it is reasonable. | ||
unsafe { | ||
$crate::bindings::kunit_do_failed_assertion( | ||
kunit_test, | ||
core::ptr::addr_of!(LOCATION.0), | ||
$crate::bindings::kunit_assert_type_KUNIT_ASSERTION, | ||
core::ptr::addr_of!(ASSERTION.0.assert), | ||
Some($crate::bindings::kunit_unary_assert_format), | ||
core::ptr::null(), | ||
); | ||
} | ||
} | ||
}; | ||
} | ||
|
||
/// Asserts that two expressions are equal to each other (using [`PartialEq`]). | ||
/// | ||
/// Public but hidden since it should only be used from generated tests. | ||
/// | ||
/// Unlike the one in `core`, this one does not panic; instead, it is mapped to the KUnit | ||
/// facilities. See [`assert!`] for more details. | ||
#[doc(hidden)] | ||
#[macro_export] | ||
macro_rules! kunit_assert_eq { | ||
($name:literal, $left:expr, $right:expr $(,)?) => {{ | ||
// For the moment, we just forward to the expression assert because, for binary asserts, | ||
// KUnit supports only a few types (e.g. integers). | ||
$crate::kunit_assert!($name, $left == $right); | ||
}}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,6 +34,8 @@ mod build_assert; | |
pub mod error; | ||
pub mod init; | ||
pub mod ioctl; | ||
#[cfg(CONFIG_KUNIT)] | ||
pub mod kunit; | ||
pub mod prelude; | ||
pub mod print; | ||
mod static_assert; | ||
|
@@ -43,6 +45,7 @@ pub mod str; | |
pub mod sync; | ||
pub mod task; | ||
pub mod types; | ||
pub mod xarray; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not urgent, but should we introduce a |
||
|
||
#[doc(hidden)] | ||
pub use bindings; | ||
|
@@ -93,7 +96,4 @@ fn panic(info: &core::panic::PanicInfo<'_>) -> ! { | |
pr_emerg!("{}\n", info); | ||
// SAFETY: FFI call. | ||
unsafe { bindings::BUG() }; | ||
// Bindgen currently does not recognize `__noreturn` so `BUG` returns `()` | ||
// instead of `!`. See <https://github.com/rust-lang/rust-bindgen/issues/2094>. | ||
loop {} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,250 @@ | ||
// SPDX-License-Identifier: GPL-2.0 | ||
|
||
//! XArray abstraction. | ||
//! | ||
//! C header: [`include/linux/xarray.h`](../../include/linux/xarray.h) | ||
use crate::{ | ||
bindings, | ||
error::{Error, Result}, | ||
prelude::*, | ||
types::{ForeignOwnable, Opaque, ScopeGuard}, | ||
}; | ||
use core::{ | ||
marker::{PhantomData, PhantomPinned}, | ||
pin::Pin, | ||
ptr::{addr_of_mut, NonNull}, | ||
}; | ||
|
||
/// Flags passed to `XArray::new` to configure the `XArray`. | ||
type Flags = bindings::gfp_t; | ||
|
||
/// Flag values passed to `XArray::new` to configure the `XArray`. | ||
pub mod flags { | ||
/// Use IRQ-safe locking. | ||
pub const LOCK_IRQ: super::Flags = bindings::BINDINGS_XA_FLAGS_LOCK_IRQ; | ||
/// Use softirq-safe locking. | ||
pub const LOCK_BH: super::Flags = bindings::BINDINGS_XA_FLAGS_LOCK_BH; | ||
/// Track which entries are free (distinct from None). | ||
pub const TRACK_FREE: super::Flags = bindings::BINDINGS_XA_FLAGS_TRACK_FREE; | ||
/// Initialize array index 0 as busy. | ||
pub const ZERO_BUSY: super::Flags = bindings::BINDINGS_XA_FLAGS_ZERO_BUSY; | ||
/// Use GFP_ACCOUNT for internal memory allocations. | ||
pub const ACCOUNT: super::Flags = bindings::BINDINGS_XA_FLAGS_ACCOUNT; | ||
/// Create an allocating `XArray` starting at index 0. | ||
pub const ALLOC: super::Flags = bindings::BINDINGS_XA_FLAGS_ALLOC; | ||
/// Create an allocating `XArray` starting at index 1. | ||
pub const ALLOC1: super::Flags = bindings::BINDINGS_XA_FLAGS_ALLOC1; | ||
} | ||
|
||
/// An array which efficiently maps sparse integer indices to owned objects. | ||
/// | ||
/// This is similar to a `Vec<Option<T>>`, but more efficient when there are holes in the | ||
/// index space, and can be efficiently grown. | ||
/// | ||
/// This structure is expected to often be used with an inner type that can be efficiently | ||
/// cloned, such as an `Arc<T>`. | ||
/// | ||
/// # Examples | ||
/// | ||
/// In this example, we create a new `XArray` and demonstrate | ||
/// rudimentary read/write operations. | ||
Comment on lines
+50
to
+51
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this example is very helpful! Additionally it might be a good idea to take each example and put it on the relevant functions as well, especially the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't realize that worked! I think at one point in time I could only get the kunit stuff working if the tests were on the top of the struct. I see now that it does...what do we consider better, a long doctest at the top with interactions between different functions, or individual tests for each function? (Or both...with some repetition?) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would prefer to have a examples on functions. Big examples only make sense if you want to demonstrate how things are used together. I would recommend this when the API is excessively complicated. I dont think that is the case here. |
||
/// | ||
/// ``` | ||
/// use core::{ | ||
/// borrow::Borrow, | ||
/// ops::{ Deref, DerefMut } | ||
/// }; | ||
/// use kernel::{ | ||
/// pin_init, | ||
/// sync::Arc, | ||
/// xarray::{ XArray, flags::LOCK_IRQ } | ||
/// }; | ||
/// | ||
/// let xarr = Box::pin_init(XArray::<Arc<usize>>::new(LOCK_IRQ)).unwrap(); | ||
/// let xarr = xarr.as_ref(); | ||
/// | ||
/// // `get` and `set` are used to read/write values. | ||
/// assert!(xarr.get(0).is_none()); | ||
/// xarr.set(0, Arc::try_new(0)?); | ||
/// assert_eq!(*xarr.get(0).unwrap(), 0); | ||
/// | ||
/// // `replace` is like `set`, but returns the old value. | ||
/// let old = xarr.replace(0, Arc::try_new(1)?)?.unwrap(); | ||
/// assert_eq!(*old, 0); | ||
/// assert_eq!(*xarr.get(0).unwrap(), 1); | ||
/// | ||
/// // `replace` returns `None` if there was no previous value. | ||
/// assert!(xarr.replace(1, Arc::try_new(1)?)?.is_none()); | ||
/// assert_eq!(*xarr.get(1).unwrap(), 1); | ||
/// | ||
/// // Similarly, `remove` returns the old value, or `None` if it didn't exist. | ||
/// assert_eq!(*xarr.remove(0).unwrap(), 1); | ||
/// assert!(xarr.get(0).is_none()); | ||
/// assert!(xarr.remove(0).is_none()); | ||
/// | ||
/// # Ok::<(), Error>(()) | ||
/// ``` | ||
/// | ||
/// In this example, we create a new `XArray` and demonstrate | ||
/// reading and/or mutating values that are not `T::Borrowed<'a>: Into<T>`. | ||
/// | ||
/// ``` | ||
/// use core::{ | ||
/// borrow::Borrow, | ||
/// ops::{ Deref, DerefMut } | ||
/// }; | ||
/// use kernel::xarray::{XArray, flags::LOCK_IRQ}; | ||
/// | ||
/// let xarr = Box::pin_init(XArray::<Box<usize>>::new(LOCK_IRQ)).unwrap(); | ||
/// let xarr = xarr.as_ref(); | ||
/// | ||
/// // If the type is not `T::Borrowed<'a>: Into<T>`, use `get_scoped` to access a shared reference | ||
/// // from a closure. | ||
/// assert!(xarr.get_scoped(0, |x| x.is_none())); | ||
/// xarr.set(0, Box::try_new(0)?); | ||
/// xarr.get_scoped(0, |x| assert_eq!(*x.unwrap(), 0)); | ||
/// | ||
/// // `get_scoped` also gives mutable access inside the closure. | ||
/// xarr.get_scoped(0, |x| { | ||
/// let mut_x = x.unwrap(); | ||
/// assert_eq!(mut_x, &0); | ||
/// *mut_x = 1; | ||
/// }); | ||
/// | ||
/// xarr.get_scoped(0, |x| assert_eq!(*x.unwrap(), 1)); | ||
/// | ||
/// # Ok::<(), Error>(()) | ||
/// ``` | ||
/// | ||
pub struct XArray<T: ForeignOwnable> { | ||
xa: Opaque<bindings::xarray>, | ||
_p: PhantomData<T>, | ||
_q: PhantomPinned, | ||
} | ||
|
||
impl<T: ForeignOwnable> XArray<T> { | ||
/// Creates a new `XArray` with the given flags. | ||
pub fn new(flags: Flags) -> impl PinInit<Self> { | ||
unsafe { | ||
kernel::init::pin_init_from_closure(move |slot: *mut XArray<T>| { | ||
bindings::xa_init_flags(Opaque::raw_get(addr_of_mut!((*slot).xa)), flags); | ||
Ok(()) | ||
}) | ||
} | ||
} | ||
|
||
/// Replaces an entry with a new value, returning the old value (if any). | ||
pub fn replace(&self, index: u64, value: T) -> Result<Option<T>> { | ||
let new = value.into_foreign(); | ||
// SAFETY: `new` just came from into_foreign(), and we dismiss this guard if | ||
// the xa_store operation succeeds and takes ownership of the pointer. | ||
let guard = ScopeGuard::new(|| unsafe { | ||
drop(T::from_foreign(new)); | ||
}); | ||
|
||
// SAFETY: `self.xa` is always valid by the type invariant, and we are storing | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This invariant should be documented. |
||
// a `T::into_foreign()` result which upholds the later invariants. | ||
let old = unsafe { | ||
bindings::xa_store(self.xa.get(), index, new as *mut _, bindings::GFP_KERNEL) | ||
}; | ||
|
||
let ret = unsafe { bindings::xa_err(old) }; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing |
||
if ret != 0 { | ||
Err(Error::from_errno(ret)) | ||
} else if old.is_null() { | ||
guard.dismiss(); | ||
Ok(None) | ||
} else { | ||
guard.dismiss(); | ||
// SAFETY: The old value must have been stored by either this function or | ||
// `alloc_limits_opt`, both of which ensure non-NULL entries are valid | ||
// ForeignOwnable pointers. | ||
Ok(Some(unsafe { T::from_foreign(old) })) | ||
} | ||
} | ||
|
||
/// Replaces an entry with a new value, dropping the old value (if any). | ||
pub fn set(&self, index: u64, value: T) -> Result { | ||
self.replace(index, value)?; | ||
Ok(()) | ||
} | ||
|
||
/// Looks up a reference to an entry in the array, cloning it | ||
/// and returning the cloned value to the user. | ||
/// | ||
pub fn get(&self, index: u64) -> Option<T> | ||
where | ||
for<'a> T::BorrowedMut<'a>: Into<T>, | ||
{ | ||
self.get_scoped(index, |x| x.map(|v| v.into())) | ||
} | ||
|
||
/// Looks up and a reference to an entry in the array, calling the user | ||
/// provided function on the resulting `Option<&T>` to return a value | ||
/// computed from the reference. Use this function if you need | ||
/// (possibly mutable) access to a `&T` that is not `T::Borrowed<'a>: Into<T>`. | ||
/// | ||
pub fn get_scoped<F, R>(&self, index: u64, f: F) -> R | ||
where | ||
F: FnOnce(Option<T::BorrowedMut<'_>>) -> R, | ||
{ | ||
// SAFETY: `self.xa` is always valid by the type invariant. | ||
unsafe { bindings::xa_lock(self.xa.get()) }; | ||
|
||
// SAFETY: `self.xa` is always valid by the type invariant. | ||
let p = unsafe { bindings::xa_load(self.xa.get(), index) }; | ||
let t = NonNull::new(p as *mut T).map(|p| unsafe { T::borrow_mut(p.as_ptr() as _) }); | ||
let r = f(t); | ||
|
||
// SAFETY: `self.xa` is always valid by the type invariant. | ||
unsafe { bindings::xa_unlock(self.xa.get()) }; | ||
|
||
r | ||
} | ||
|
||
/// Removes and returns an entry, returning it if it existed. | ||
pub fn remove(&self, index: u64) -> Option<T> { | ||
let p = unsafe { bindings::xa_erase(self.xa.get(), index) }; | ||
if p.is_null() { | ||
None | ||
} else { | ||
Some(unsafe { T::from_foreign(p) }) | ||
} | ||
} | ||
} | ||
|
||
impl<T: ForeignOwnable> Drop for XArray<T> { | ||
fn drop(&mut self) { | ||
// SAFETY: `self.xa` is valid by the type invariant, and as we have the only reference to | ||
// the `XArray` we can safely iterate its contents and drop everything. | ||
unsafe { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this block should be split into smaller parts. that way every unsafe call is visible and can be annotated appropriately. |
||
let mut index: core::ffi::c_ulong = 0; | ||
let mut entry = bindings::xa_find( | ||
self.xa.get(), | ||
&mut index, | ||
core::ffi::c_ulong::MAX, | ||
bindings::BINDINGS_XA_PRESENT, | ||
); | ||
while !entry.is_null() { | ||
T::from_foreign(entry); | ||
entry = bindings::xa_find_after( | ||
self.xa.get(), | ||
&mut index, | ||
core::ffi::c_ulong::MAX, | ||
bindings::BINDINGS_XA_PRESENT, | ||
); | ||
} | ||
|
||
// Locked locks are not safe to drop. Normally we would want to try_lock()/unlock() here | ||
// for safety or something similar, but in this case xa_destroy() is guaranteed to | ||
// acquire the lock anyway. This will deadlock if a lock guard was improperly dropped, | ||
// but that is not UB, so it's sufficient for soundness purposes. | ||
bindings::xa_destroy(self.xa.get()); | ||
} | ||
} | ||
} | ||
|
||
// SAFETY: XArray is thread-safe and all mutation operations are internally locked. | ||
unsafe impl<T: Send + ForeignOwnable> Send for XArray<T> {} | ||
unsafe impl<T: Sync + ForeignOwnable> Sync for XArray<T> {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,94 @@ | ||
use proc_macro::{Delimiter, Group, Ident, Spacing, Span, TokenTree}; | ||
|
||
fn concat(tokens: &[TokenTree], group_span: Span) -> TokenTree { | ||
let mut tokens = tokens.iter(); | ||
let mut segments = Vec::new(); | ||
let mut span = None; | ||
loop { | ||
match tokens.next() { | ||
None => break, | ||
Some(TokenTree::Literal(lit)) => segments.push((lit.to_string(), lit.span())), | ||
Some(TokenTree::Ident(ident)) => { | ||
let mut value = ident.to_string(); | ||
if value.starts_with("r#") { | ||
value.replace_range(0..2, ""); | ||
} | ||
segments.push((value, ident.span())); | ||
} | ||
Some(TokenTree::Punct(p)) if p.as_char() == ':' => { | ||
let Some(TokenTree::Ident(ident)) = tokens.next() else { | ||
panic!("expected identifier as modifier"); | ||
}; | ||
|
||
let (mut value, sp) = segments.pop().expect("expected identifier before modifier"); | ||
match ident.to_string().as_str() { | ||
// Set the overall span of concatenated token as current span | ||
"span" => { | ||
assert!( | ||
span.is_none(), | ||
"span modifier should only appear at most once" | ||
); | ||
span = Some(sp); | ||
} | ||
"lower" => value = value.to_lowercase(), | ||
"upper" => value = value.to_uppercase(), | ||
v => panic!("unknown modifier `{v}`"), | ||
}; | ||
segments.push((value, sp)); | ||
} | ||
_ => panic!("unexpected token in paste segments"), | ||
}; | ||
} | ||
|
||
let pasted: String = segments.into_iter().map(|x| x.0).collect(); | ||
TokenTree::Ident(Ident::new(&pasted, span.unwrap_or(group_span))) | ||
} | ||
|
||
pub(crate) fn expand(tokens: &mut Vec<TokenTree>) { | ||
for token in tokens.iter_mut() { | ||
if let TokenTree::Group(group) = token { | ||
let delimiter = group.delimiter(); | ||
let span = group.span(); | ||
let mut stream: Vec<_> = group.stream().into_iter().collect(); | ||
// Find groups that looks like `[< A B C D >]` | ||
if delimiter == Delimiter::Bracket | ||
&& stream.len() >= 3 | ||
&& matches!(&stream[0], TokenTree::Punct(p) if p.as_char() == '<') | ||
&& matches!(&stream[stream.len() - 1], TokenTree::Punct(p) if p.as_char() == '>') | ||
{ | ||
// Replace the group with concatenated token | ||
*token = concat(&stream[1..stream.len() - 1], span); | ||
} else { | ||
// Recursively expand tokens inside the group | ||
expand(&mut stream); | ||
let mut group = Group::new(delimiter, stream.into_iter().collect()); | ||
group.set_span(span); | ||
*token = TokenTree::Group(group); | ||
} | ||
} | ||
} | ||
|
||
// Path segments cannot contain invisible delimiter group, so remove them if any. | ||
for i in (0..tokens.len().saturating_sub(3)).rev() { | ||
// Looking for a double colon | ||
if matches!( | ||
(&tokens[i + 1], &tokens[i + 2]), | ||
(TokenTree::Punct(a), TokenTree::Punct(b)) | ||
if a.as_char() == ':' && a.spacing() == Spacing::Joint && b.as_char() == ':' | ||
) { | ||
match &tokens[i + 3] { | ||
TokenTree::Group(group) if group.delimiter() == Delimiter::None => { | ||
tokens.splice(i + 3..i + 4, group.stream()); | ||
} | ||
_ => (), | ||
} | ||
|
||
match &tokens[i] { | ||
TokenTree::Group(group) if group.delimiter() == Delimiter::None => { | ||
tokens.splice(i..i + 1, group.stream()); | ||
} | ||
_ => (), | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,7 +30,7 @@ rustc) | |
echo 1.68.2 | ||
;; | ||
bindgen) | ||
echo 0.56.0 | ||
echo 0.65.1 | ||
;; | ||
*) | ||
echo "$1: unknown tool" >&2 | ||
|
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
// SPDX-License-Identifier: GPL-2.0 | ||
|
||
//! Test builder for `rustdoc`-generated tests. | ||
//! | ||
//! This script is a hack to extract the test from `rustdoc`'s output. Ideally, `rustdoc` would | ||
//! have an option to generate this information instead, e.g. as JSON output. | ||
//! | ||
//! The `rustdoc`-generated test names look like `{file}_{line}_{number}`, e.g. | ||
//! `...path_rust_kernel_sync_arc_rs_42_0`. `number` is the "test number", needed in cases like | ||
//! a macro that expands into items with doctests is invoked several times within the same line. | ||
//! | ||
//! However, since these names are used for bisection in CI, the line number makes it not stable | ||
//! at all. In the future, we would like `rustdoc` to give us the Rust item path associated with | ||
//! the test, plus a "test number" (for cases with several examples per item) and generate a name | ||
//! from that. For the moment, we generate ourselves a new name, `{file}_{number}` instead, in | ||
//! the `gen` script (done there since we need to be aware of all the tests in a given file). | ||
use std::fs::File; | ||
use std::io::{BufWriter, Read, Write}; | ||
|
||
fn main() { | ||
let mut stdin = std::io::stdin().lock(); | ||
let mut body = String::new(); | ||
stdin.read_to_string(&mut body).unwrap(); | ||
|
||
// Find the generated function name looking for the inner function inside `main()`. | ||
// | ||
// The line we are looking for looks like one of the following: | ||
// | ||
// ``` | ||
// fn main() { #[allow(non_snake_case)] fn _doctest_main_rust_kernel_file_rs_28_0() { | ||
// fn main() { #[allow(non_snake_case)] fn _doctest_main_rust_kernel_file_rs_37_0() -> Result<(), impl core::fmt::Debug> { | ||
// ``` | ||
// | ||
// It should be unlikely that doctest code matches such lines (when code is formatted properly). | ||
let rustdoc_function_name = body | ||
.lines() | ||
.find_map(|line| { | ||
Some( | ||
line.split_once("fn main() {")? | ||
.1 | ||
.split_once("fn ")? | ||
.1 | ||
.split_once("()")? | ||
.0, | ||
) | ||
.filter(|x| x.chars().all(|c| c.is_alphanumeric() || c == '_')) | ||
}) | ||
.expect("No test function found in `rustdoc`'s output."); | ||
|
||
// Qualify `Result` to avoid the collision with our own `Result` coming from the prelude. | ||
let body = body.replace( | ||
&format!("{rustdoc_function_name}() -> Result<(), impl core::fmt::Debug> {{"), | ||
&format!("{rustdoc_function_name}() -> core::result::Result<(), impl core::fmt::Debug> {{"), | ||
); | ||
|
||
// For tests that get generated with `Result`, like above, `rustdoc` generates an `unwrap()` on | ||
// the return value to check there were no returned errors. Instead, we use our assert macro | ||
// since we want to just fail the test, not panic the kernel. | ||
// | ||
// We save the result in a variable so that the failed assertion message looks nicer. | ||
let body = body.replace( | ||
&format!("}} {rustdoc_function_name}().unwrap() }}"), | ||
&format!("}} let test_return_value = {rustdoc_function_name}(); assert!(test_return_value.is_ok()); }}"), | ||
); | ||
|
||
// Figure out a smaller test name based on the generated function name. | ||
let name = rustdoc_function_name.split_once("_rust_kernel_").unwrap().1; | ||
|
||
let path = format!("rust/test/doctests/kernel/{name}"); | ||
|
||
write!(BufWriter::new(File::create(path).unwrap()), "{body}").unwrap(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,162 @@ | ||
// SPDX-License-Identifier: GPL-2.0 | ||
|
||
//! Generates KUnit tests from saved `rustdoc`-generated tests. | ||
//! | ||
//! KUnit passes a context (`struct kunit *`) to each test, which should be forwarded to the other | ||
//! KUnit functions and macros. | ||
//! | ||
//! However, we want to keep this as an implementation detail because: | ||
//! | ||
//! - Test code should not care about the implementation. | ||
//! | ||
//! - Documentation looks worse if it needs to carry extra details unrelated to the piece | ||
//! being described. | ||
//! | ||
//! - Test code should be able to define functions and call them, without having to carry | ||
//! the context. | ||
//! | ||
//! - Later on, we may want to be able to test non-kernel code (e.g. `core`, `alloc` or | ||
//! third-party crates) which likely use the standard library `assert*!` macros. | ||
//! | ||
//! For this reason, instead of the passed context, `kunit_get_current_test()` is used instead | ||
//! (i.e. `current->kunit_test`). | ||
//! | ||
//! Note that this means other threads/tasks potentially spawned by a given test, if failing, will | ||
//! report the failure in the kernel log but will not fail the actual test. Saving the pointer in | ||
//! e.g. a `static` per test does not fully solve the issue either, because currently KUnit does | ||
//! not support assertions (only expectations) from other tasks. Thus leave that feature for | ||
//! the future, which simplifies the code here too. We could also simply not allow `assert`s in | ||
//! other tasks, but that seems overly constraining, and we do want to support them, eventually. | ||
use std::io::{BufWriter, Read, Write}; | ||
use std::{fs, fs::File}; | ||
|
||
fn main() { | ||
let mut paths = fs::read_dir("rust/test/doctests/kernel") | ||
.unwrap() | ||
.map(|entry| entry.unwrap().path()) | ||
.collect::<Vec<_>>(); | ||
|
||
// Sort paths for clarity. | ||
paths.sort(); | ||
|
||
let mut rust_tests = String::new(); | ||
let mut c_test_declarations = String::new(); | ||
let mut c_test_cases = String::new(); | ||
let mut body = String::new(); | ||
let mut last_file = String::new(); | ||
let mut number = 0; | ||
for path in paths { | ||
// The `name` follows the `{file}_{line}_{number}` pattern (see description in | ||
// `scripts/rustdoc_test_builder.rs`). Discard the `number`. | ||
let name = path.file_name().unwrap().to_str().unwrap().to_string(); | ||
|
||
// Extract the `file` and the `line`, discarding the `number`. | ||
let (file, line) = name.rsplit_once('_').unwrap().0.rsplit_once('_').unwrap(); | ||
|
||
// Generate an ID sequence ("test number") for each one in the file. | ||
if file == last_file { | ||
number += 1; | ||
} else { | ||
number = 0; | ||
last_file = file.to_string(); | ||
} | ||
|
||
// Generate a KUnit name (i.e. test name and C symbol) for this test. | ||
// | ||
// We avoid the line number, like `rustdoc` does, to make things slightly more stable for | ||
// bisection purposes. However, to aid developers in mapping back what test failed, we will | ||
// print a diagnostics line in the KTAP report. | ||
let kunit_name = format!("rust_doctest_kernel_{file}_{number}"); | ||
|
||
// Read the test's text contents to dump it below. | ||
body.clear(); | ||
File::open(path).unwrap().read_to_string(&mut body).unwrap(); | ||
|
||
let line = line.parse::<core::ffi::c_int>().unwrap(); | ||
|
||
use std::fmt::Write; | ||
write!( | ||
rust_tests, | ||
r#"/// Generated `{name}` KUnit test case from a Rust documentation test. | ||
#[no_mangle] | ||
pub extern "C" fn {kunit_name}(__kunit_test: *mut kernel::bindings::kunit) {{ | ||
/// Overrides the usual [`assert!`] macro with one that calls KUnit instead. | ||
#[allow(unused)] | ||
macro_rules! assert {{ | ||
($cond:expr $(,)?) => {{{{ | ||
kernel::kunit_assert!("{kunit_name}", $cond); | ||
}}}} | ||
}} | ||
/// Overrides the usual [`assert_eq!`] macro with one that calls KUnit instead. | ||
#[allow(unused)] | ||
macro_rules! assert_eq {{ | ||
($left:expr, $right:expr $(,)?) => {{{{ | ||
kernel::kunit_assert_eq!("{kunit_name}", $left, $right); | ||
}}}} | ||
}} | ||
// Many tests need the prelude, so provide it by default. | ||
#[allow(unused)] | ||
use kernel::prelude::*; | ||
// Display line number so that developers can map the test easily to the source code. | ||
kernel::kunit::info(format_args!(" # Doctest from line {line}\n")); | ||
{{ | ||
{body} | ||
main(); | ||
}} | ||
}} | ||
"# | ||
) | ||
.unwrap(); | ||
|
||
write!(c_test_declarations, "void {kunit_name}(struct kunit *);\n").unwrap(); | ||
write!(c_test_cases, " KUNIT_CASE({kunit_name}),\n").unwrap(); | ||
} | ||
|
||
let rust_tests = rust_tests.trim(); | ||
let c_test_declarations = c_test_declarations.trim(); | ||
let c_test_cases = c_test_cases.trim(); | ||
|
||
write!( | ||
BufWriter::new(File::create("rust/doctests_kernel_generated.rs").unwrap()), | ||
r#"//! `kernel` crate documentation tests. | ||
const __LOG_PREFIX: &[u8] = b"rust_doctests_kernel\0"; | ||
{rust_tests} | ||
"# | ||
) | ||
.unwrap(); | ||
|
||
write!( | ||
BufWriter::new(File::create("rust/doctests_kernel_generated_kunit.c").unwrap()), | ||
r#"/* | ||
* `kernel` crate documentation tests. | ||
*/ | ||
#include <kunit/test.h> | ||
{c_test_declarations} | ||
static struct kunit_case test_cases[] = {{ | ||
{c_test_cases} | ||
{{ }} | ||
}}; | ||
static struct kunit_suite test_suite = {{ | ||
.name = "rust_doctests_kernel", | ||
.test_cases = test_cases, | ||
}}; | ||
kunit_test_suite(test_suite); | ||
MODULE_LICENSE("GPL"); | ||
"# | ||
) | ||
.unwrap(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also discovered this yesterday. Could you send this patch directly to the mailing list?
@ojeda probably should take this as a fix.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is https://rust-for-linux.zulipchat.com/#narrow/stream/291565-Help/topic/Build.20errors/near/365685550 (applied at the rebased ojeda@698129c), i.e. I was going to fix it directly when we take the upgrade to 0.65.1.