Skip to content

Commit 2c20426

Browse files
lschuermannlschuermann
committed
tock-register-interface: replace register tests by const assertions
This replaces the automatically generated tests for register structs defined through the `register_structs!` macro by static assertions evaluated at compile time. This has some significant advantages over the current approach: - errors in the struct offsets and violation of other invariants is detected immediately on compilation and not only when running tests. This increases developer ergonomics significantly and reduces the potential for errors and weird behavior at runtime. - the crate no longer generates tests which rely on the standard library being present in the users' code. While the library itself still has tests which rely on the presence of the standard library, these tests are for internal validation only. In `no_std` environments, relying on the standard library even just for `#[test]` functions can be problematic. For developers, the choice has been to either use the standard library and have test, implement a workaround to provide the standard library when running tests but not otherwise, or not generate the tests at all. These changes enable us to make the library purely `#[no_std]` compatible and not impose any standard library requirements on user code. It does not come without drawbacks though. Because Rust's const panic features, especially w.r.t. string formatting, are still very limited, the provided debug information is significantly reduced. For instance, it as of now seems impossible to interpolate any const variable content into the assertion message. This is likely to be resolved in the future, when Rust allows more elaborate string formatting techniques at compile time. As an additional consequence, the assertions code is less readable. The generated error messages are quite prominent during the compilation process and easy to identify: error[E0080]: evaluation of constant value failed --> chips/lowrisc/src/uart.rs:16:1 | 16 | / register_structs! { 17 | | pub UartRegisters { 18 | | (0x00 => intr_state: ReadWrite<u32>), 19 | | (0x04 => intr_enable: ReadWrite<u32>), ... | 41 | | } 42 | | } | |_^ the evaluated program panicked at 'Invalid end offset for field val (expected 49 but actual value differs)', chips/lowrisc/src/uart.rs:16:1 Signed-off-by: Leon Schuermann <leon@is.currently.online>
1 parent d8de432 commit 2c20426

File tree

1 file changed

+212
-81
lines changed
  • libraries/tock-register-interface/src

1 file changed

+212
-81
lines changed

‎libraries/tock-register-interface/src/macros.rs

Lines changed: 212 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -79,123 +79,272 @@ macro_rules! register_fields {
7979
};
8080
}
8181

82+
// TODO: All of the rustdoc tests below use a `should_fail` attribute instead of
83+
// `should_panic` because a const panic will result in a failure to evaluate a
84+
// constant value, and thus a compiler error. However, this means that these
85+
// examples could break for unrelated reasons, trigger a compiler error, but not
86+
// test the desired assertion any longer. This should be switched to a
87+
// `should_panic`-akin attribute which works for const panics, once that is
88+
// available.
89+
/// Statically validate the size and offsets of the fields defined
90+
/// within the register struct through the [`register_structs!`]
91+
/// macro.
92+
///
93+
/// This macro expands to an expression which contains static
94+
/// assertions about various parameters of the individual fields in
95+
/// the register struct definition. It will test for:
96+
///
97+
/// - Proper start offset of padding fields. It will fail in cases
98+
/// such as
99+
///
100+
/// ```should_fail
101+
/// # #[macro_use]
102+
/// # extern crate tock_registers;
103+
/// # use tock_registers::register_structs;
104+
/// # use tock_registers::registers::ReadWrite;
105+
/// register_structs! {
106+
/// UartRegisters {
107+
/// (0x04 => _reserved),
108+
/// (0x08 => foo: ReadWrite<u32>),
109+
/// (0x0C => @END),
110+
/// }
111+
/// }
112+
/// # // This is required for rustdoc to not place this code snipped into an
113+
/// # // fn main() {...} function.
114+
/// # fn main() { }
115+
/// ```
116+
///
117+
/// In this example, the start offset of `_reserved` should have been `0x00`
118+
/// instead of `0x04`.
119+
///
120+
/// - Correct start offset and end offset (start offset of next field) in actual
121+
/// fields. It will fail in cases such as
122+
///
123+
/// ```should_fail
124+
/// # #[macro_use]
125+
/// # extern crate tock_registers;
126+
/// # use tock_registers::register_structs;
127+
/// # use tock_registers::registers::ReadWrite;
128+
/// register_structs! {
129+
/// UartRegisters {
130+
/// (0x00 => foo: ReadWrite<u32>),
131+
/// (0x05 => bar: ReadWrite<u32>),
132+
/// (0x08 => @END),
133+
/// }
134+
/// }
135+
/// # // This is required for rustdoc to not place this code snipped into an
136+
/// # // fn main() {...} function.
137+
/// # fn main() { }
138+
/// ```
139+
///
140+
/// In this example, the start offset of `bar` and thus the end offset of
141+
/// `foo` should have been `0x04` instead of `0x05`.
142+
///
143+
/// - Invalid alignment of fields.
144+
///
145+
/// - That the end marker matches the actual generated struct size. This will
146+
/// fail in cases such as
147+
///
148+
/// ```should_fail
149+
/// # #[macro_use]
150+
/// # extern crate tock_registers;
151+
/// # use tock_registers::register_structs;
152+
/// # use tock_registers::registers::ReadWrite;
153+
/// register_structs! {
154+
/// UartRegisters {
155+
/// (0x00 => foo: ReadWrite<u32>),
156+
/// (0x04 => bar: ReadWrite<u32>),
157+
/// (0x10 => @END),
158+
/// }
159+
/// }
160+
/// # // This is required for rustdoc to not place this code snipped into an
161+
/// # // fn main() {...} function.
162+
/// # fn main() { }
163+
/// ```
82164
#[macro_export]
83165
macro_rules! test_fields {
166+
// This macro works by iterating over all defined fields, until it hits an
167+
// ($size:expr => @END) field. Each iteration generates an expression which,
168+
// when evaluated, yields the current byte offset in the fields. Thus, when
169+
// reading a field or padding, the field or padding length must be added to
170+
// the returned size.
171+
//
172+
// By feeding this expression recursively into the macro, deeper invocations
173+
// can continue validating fields through knowledge of the current offset
174+
// and the remaining fields.
175+
//
176+
// The nested expression returned by this macro is guaranteed to be
177+
// const-evaluable.
178+
84179
// Macro entry point.
85180
(@root $struct:ident $(<$life:lifetime>)? { $($input:tt)* } ) => {
86-
$crate::test_fields!(@munch $struct $(<$life>)? sum ($($input)*) -> {});
181+
// Start recursion at offset 0.
182+
$crate::test_fields!(@munch $struct $(<$life>)? ($($input)*) : 0);
87183
};
88184

89-
// Print the tests once all fields have been munched.
90-
// We wrap the tests in a "detail" function that potentially takes a lifetime parameter, so that
91-
// the lifetime is declared inside it - therefore all types using the lifetime are well-defined.
92-
(@munch $struct:ident $(<$life:lifetime>)? $sum:ident
185+
// Consume the ($size:expr => @END) field, which MUST be the last field in
186+
// the register struct.
187+
(@munch $struct:ident $(<$life:lifetime>)?
93188
(
94189
$(#[$attr_end:meta])*
95190
($size:expr => @END),
96191
)
97-
-> {$($stmts:block)*}
192+
: $stmts:expr
98193
) => {
99-
{
100-
fn detail $(<$life>)? ()
101-
{
102-
let mut $sum: usize = 0;
103-
$($stmts)*
104-
let size = core::mem::size_of::<$struct $(<$life>)?>();
194+
const _: () = {
195+
// We've reached the end! Normally it is sufficient to compare the
196+
// struct's size to the reported end offet. However, we must
197+
// evaluate the previous iterations' expressions for them to have an
198+
// effect anyways, so we can perform an internal sanity check on
199+
// this value as well.
200+
const sum: usize = $stmts;
201+
202+
const fn struct_size $(<$life>)? () -> usize
203+
{
204+
core::mem::size_of::<$struct $(<$life>)?>()
205+
}
206+
105207
assert!(
106-
size == $size,
107-
"Invalid size for struct {} (expected {:#X} but was {:#X})",
108-
stringify!($struct),
109-
$size,
110-
size
208+
struct_size() == $size,
209+
"{}",
210+
concat!(
211+
"Invalid size for struct ",
212+
stringify!($struct),
213+
" (expected ",
214+
$size,
215+
", actual struct size differs)",
216+
),
111217
);
112-
}
113218

114-
detail();
115-
}
219+
// Internal sanity check. If we have reached this point and
220+
// correctly iterated over the struct's fields, the current offset
221+
// and the claimed end offset MUST be equal.
222+
assert!(sum == $size);
223+
};
116224
};
117225

118-
// Munch field.
119-
(@munch $struct:ident $(<$life:lifetime>)? $sum:ident
226+
// Consume a proper ($offset:expr => $field:ident: $ty:ty) field.
227+
(@munch $struct:ident $(<$life:lifetime>)?
120228
(
121229
$(#[$attr:meta])*
122230
($offset_start:expr => $vis:vis $field:ident: $ty:ty),
123231
$(#[$attr_next:meta])*
124232
($offset_end:expr => $($next:tt)*),
125233
$($after:tt)*
126234
)
127-
-> {$($output:block)*}
235+
: $output:expr
128236
) => {
129237
$crate::test_fields!(
130-
@munch $struct $(<$life>)? $sum (
238+
@munch $struct $(<$life>)? (
131239
$(#[$attr_next])*
132240
($offset_end => $($next)*),
133241
$($after)*
134-
) -> {
135-
$($output)*
136-
{
137-
assert!(
138-
$sum == $offset_start,
139-
"Invalid start offset for field {} (expected {:#X} but was {:#X})",
242+
) : {
243+
// Evaluate the previous iterations' expression to determine the
244+
// current offset.
245+
const sum: usize = $output;
246+
247+
// Validate the start offset of the current field. This check is
248+
// mostly relevant for when this is the first field in the
249+
// struct, as any subsequent start offset error will be detected
250+
// by an end offset error of the previous field.
251+
assert!(
252+
sum == $offset_start,
253+
"{}",
254+
concat!(
255+
"Invalid start offset for field ",
140256
stringify!($field),
257+
" (expected ",
141258
$offset_start,
142-
$sum
143-
);
144-
let align = core::mem::align_of::<$ty>();
259+
" but actual value differs)",
260+
),
261+
);
262+
263+
// Validate that the start offset of the current field within
264+
// the struct matches the type's minimum alignment constraint.
265+
const align: usize = core::mem::align_of::<$ty>();
266+
// Clippy can tell that (align - 1) is zero for some fields, so
267+
// we allow this lint and further encapsule the assert! as an
268+
// expression, such that the allow attr can apply.
269+
#[allow(clippy::bad_bit_mask)]
270+
{
145271
assert!(
146-
$sum & (align - 1) == 0,
147-
"Invalid alignment for field {} (expected alignment of {:#X} but offset was {:#X})",
148-
stringify!($field),
149-
align,
150-
$sum
272+
sum & (align - 1) == 0,
273+
"{}",
274+
concat!(
275+
"Invalid alignment for field ",
276+
stringify!($field),
277+
" (offset differs from expected)",
278+
),
151279
);
152-
$sum += core::mem::size_of::<$ty>();
153-
assert!(
154-
$sum == $offset_end,
155-
"Invalid end offset for field {} (expected {:#X} but was {:#X})",
280+
}
281+
282+
// Add the current field's length to the offset and validate the
283+
// end offset of the field based on the next field's claimed
284+
// start offset.
285+
const new_sum: usize = sum + core::mem::size_of::<$ty>();
286+
assert!(
287+
new_sum == $offset_end,
288+
"{}",
289+
concat!(
290+
"Invalid end offset for field ",
156291
stringify!($field),
292+
" (expected ",
157293
$offset_end,
158-
$sum
159-
);
160-
}
294+
" but actual value differs)",
295+
),
296+
);
297+
298+
// Provide the updated offset to the next iteration
299+
new_sum
161300
}
162301
);
163302
};
164303

165-
// Munch padding.
166-
(@munch $struct:ident $(<$life:lifetime>)? $sum:ident
304+
// Consume a padding ($offset:expr => $padding:ident) field.
305+
(@munch $struct:ident $(<$life:lifetime>)?
167306
(
168307
$(#[$attr:meta])*
169308
($offset_start:expr => $padding:ident),
170309
$(#[$attr_next:meta])*
171310
($offset_end:expr => $($next:tt)*),
172311
$($after:tt)*
173312
)
174-
-> {$($output:block)*}
313+
: $output:expr
175314
) => {
176315
$crate::test_fields!(
177-
@munch $struct $(<$life>)? $sum (
316+
@munch $struct $(<$life>)? (
178317
$(#[$attr_next])*
179318
($offset_end => $($next)*),
180319
$($after)*
181-
) -> {
182-
$($output)*
183-
{
184-
assert!(
185-
$sum == $offset_start,
186-
"Invalid start offset for padding {} (expected {:#X} but was {:#X})",
320+
) : {
321+
// Evaluate the previous iterations' expression to determine the
322+
// current offset.
323+
const sum: usize = $output;
324+
325+
// Validate the start offset of the current padding field. This
326+
// check is mostly relevant for when this is the first field in
327+
// the struct, as any subsequent start offset error will be
328+
// detected by an end offset error of the previous field.
329+
assert!(
330+
sum == $offset_start,
331+
concat!(
332+
"Invalid start offset for padding ",
187333
stringify!($padding),
334+
" (expected ",
188335
$offset_start,
189-
$sum
190-
);
191-
$sum = $offset_end;
192-
}
336+
" but actual value differs)",
337+
),
338+
);
339+
340+
// The padding field is automatically sized. Provide the start
341+
// offset of the next field to the next iteration.
342+
$offset_end
193343
}
194344
);
195345
};
196346
}
197347

198-
#[cfg(feature = "std_unit_tests")]
199348
#[macro_export]
200349
macro_rules! register_structs {
201350
{
@@ -208,33 +357,15 @@ macro_rules! register_structs {
208357
} => {
209358
$( $crate::register_fields!(@root $(#[$attr])* $vis_struct $name $(<$life>)? { $($fields)* } ); )*
210359

211-
#[cfg(test)]
212-
mod test_register_structs {
360+
mod static_validate_register_structs {
213361
$(
214362
#[allow(non_snake_case)]
215363
mod $name {
216364
use super::super::*;
217-
#[test]
218-
fn test_offsets() {
219-
$crate::test_fields!(@root $name $(<$life>)? { $($fields)* } )
220-
}
365+
366+
$crate::test_fields!(@root $name $(<$life>)? { $($fields)* } );
221367
}
222368
)*
223369
}
224370
};
225371
}
226-
227-
#[cfg(not(feature = "std_unit_tests"))]
228-
#[macro_export]
229-
macro_rules! register_structs {
230-
{
231-
$(
232-
$(#[$attr:meta])*
233-
$vis_struct:vis $name:ident $(<$life:lifetime>)? {
234-
$( $fields:tt )*
235-
}
236-
),*
237-
} => {
238-
$( $crate::register_fields!(@root $(#[$attr])* $vis_struct $name $(<$life>)? { $($fields)* } ); )*
239-
};
240-
}

0 commit comments

Comments
 (0)