| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ZIps86MbJF/iGIzd@boqun-archlinux>
Date: Wed, 14 Jun 2023 18:44:19 -0700
From: Boqun Feng <boqun.feng@...il.com>
To: Miguel Ojeda <ojeda@...nel.org>
Cc: David Gow <davidgow@...gle.com>,
Brendan Higgins <brendan.higgins@...ux.dev>,
Wedson Almeida Filho <wedsonaf@...il.com>,
Alex Gaynor <alex.gaynor@...il.com>,
Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Benno Lossin <benno.lossin@...ton.me>,
Alice Ryhl <aliceryhl@...gle.com>,
Andreas Hindborg <nmi@...aspace.dk>,
Philip Li <philip.li@...el.com>, kunit-dev@...glegroups.com,
linux-kselftest@...r.kernel.org, rust-for-linux@...r.kernel.org,
linux-kernel@...r.kernel.org, patches@...ts.linux.dev
Subject: Re: [PATCH 0/6] KUnit integration for Rust doctests
On Wed, Jun 14, 2023 at 08:08:24PM +0200, Miguel Ojeda wrote:
> This is the initial KUnit integration for running Rust documentation
> tests within the kernel.
>
> Thank you to the KUnit team for all the input and feedback on this
> over the months, as well as the Intel LKP 0-Day team!
>
> This may be merged through either the KUnit or the Rust trees. If
> the KUnit team wants to merge it, then that would be great.
>
> Please see the message in the main commit for the details.
>
Great work! I've played this for a while, and it's really useful ;-)
One thing though, maybe we can provide more clues for users to locate
the corresponding Doctests? For example, I did the following to trigger
an assertion:
diff --git a/rust/kernel/sync/lock/spinlock.rs b/rust/kernel/sync/lock/spinlock.rs
index 91eb2c9e9123..9ead152e2c7e 100644
--- a/rust/kernel/sync/lock/spinlock.rs
+++ b/rust/kernel/sync/lock/spinlock.rs
@@ -58,7 +58,7 @@ macro_rules! new_spinlock {
///
/// // Allocate a boxed `Example`.
/// let e = Box::pin_init(Example::new())?;
-/// assert_eq!(e.c, 10);
+/// assert_eq!(e.c, 11);
/// assert_eq!(e.d.lock().a, 20);
/// assert_eq!(e.d.lock().b, 30);
/// # Ok::<(), Error>(())
Originally I got:
[..] # Doctest from line 35
[..] # rust_doctest_kernel_sync_lock_spinlock_rs_0: ASSERTION FAILED at rust/doctests_kernel_generated.rs:2437
[..] Expected e.c == 11 to be true, but is false
[..] [FAILED] rust_doctest_kernel_sync_lock_spinlock_rs_0
The assertion warning only says line 35 but which file? Yes, the
".._sync_lock_spinlock_rs" name does provide the lead, however since we
generate the test code, so we actually know the line # for each real
test body, so I come up a way to give us the following:
[..] # rust_doctest_kernel_sync_lock_spinlock_rs_0: ASSERTION FAILED at rust/kernel/sync/lock/spinlock.rs:61
[..] Expected e.c == 11 to be true, but is false
[..] [FAILED] rust_doctest_kernel_sync_lock_spinlock_rs_0
Thoughts?
Regards,
Boqun
----------------->8
diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index 3c94efcd7f76..807fe3633567 100644
--- a/rust/kernel/kunit.rs
+++ b/rust/kernel/kunit.rs
@@ -49,15 +49,15 @@ pub fn info(args: fmt::Arguments<'_>) {
#[doc(hidden)]
#[macro_export]
macro_rules! kunit_assert {
- ($name:literal, $condition:expr $(,)?) => {
+ ($name:literal, $diff:expr, $file:expr, $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 LINE: i32 = core::line!() as i32 - $diff;
+ static FILE: &'static $crate::str::CStr = $crate::c_str!($file);
static CONDITION: &'static $crate::str::CStr = $crate::c_str!(stringify!($condition));
// SAFETY: FFI call without safety requirements.
@@ -148,9 +148,9 @@ unsafe impl Sync for UnaryAssert {}
#[doc(hidden)]
#[macro_export]
macro_rules! kunit_assert_eq {
- ($name:literal, $left:expr, $right:expr $(,)?) => {{
+ ($name:literal, $diff:expr, $file:expr, $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);
+ $crate::kunit_assert!($name, $diff, $file, $left == $right);
}};
}
diff --git a/scripts/rustdoc_test_gen.rs b/scripts/rustdoc_test_gen.rs
index 793885c32c0d..4786a2ef0dc6 100644
--- a/scripts/rustdoc_test_gen.rs
+++ b/scripts/rustdoc_test_gen.rs
@@ -75,6 +75,11 @@ fn main() {
let line = line.parse::<core::ffi::c_int>().unwrap();
+ let src_file = format!("rust/kernel/{}", file.replace("_rs", ".rs").replace("_", "/"));
+
+ // Calculate how many lines before `main` function (including the `main` function line).
+ let body_offset = body.lines().take_while(|l| !l.contains("fn main() {")).count() + 1;
+
use std::fmt::Write;
write!(
rust_tests,
@@ -85,7 +90,7 @@ pub extern "C" fn {kunit_name}(__kunit_test: *mut kernel::bindings::kunit) {{
#[allow(unused)]
macro_rules! assert {{
($cond:expr $(,)?) => {{{{
- kernel::kunit_assert!("{kunit_name}", $cond);
+ kernel::kunit_assert!("{kunit_name}", anchor - {line}, "{src_file}", $cond);
}}}}
}}
@@ -93,7 +98,7 @@ macro_rules! assert {{
#[allow(unused)]
macro_rules! assert_eq {{
($left:expr, $right:expr $(,)?) => {{{{
- kernel::kunit_assert_eq!("{kunit_name}", $left, $right);
+ kernel::kunit_assert_eq!("{kunit_name}", anchor - {line}, "{src_file}", $left, $right);
}}}}
}}
@@ -101,9 +106,8 @@ macro_rules! assert_eq {{
#[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"));
-
+ // The anchor where the test code body starts.
+ static anchor: i32 = core::line!() as i32 + {body_offset} + 1;
{{
{body}
main();
Powered by blists - more mailing lists