Skip to content

Commit 7c3bed7

Browse files
rmsynromancardenas
rmsyn
and
romancardenas
committed
fixup: riscv: read-only CSR compile-time asserts
Applies compile-time checks in read-only CSR macros. Authored-by: rmsyn <[email protected]> Co-authored-by: romancardenas <[email protected]>
1 parent 7ad499a commit 7c3bed7

File tree

4 files changed

+39
-113
lines changed

4 files changed

+39
-113
lines changed

‎riscv/src/register/macros.rs‎

Lines changed: 34 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -747,8 +747,6 @@ macro_rules! read_write_csr_field {
747747
($ty:ident,
748748
$(#[$field_doc:meta])+
749749
$field:ident,
750-
$(#[$try_field_doc:meta])+
751-
$try_field:ident,
752750
$(#[$set_field_doc:meta])+
753751
$set_field:ident,
754752
$(#[$try_set_field_doc:meta])+
@@ -758,10 +756,7 @@ macro_rules! read_write_csr_field {
758756
$crate::read_only_csr_field!(
759757
$ty,
760758
$(#[$field_doc])+
761-
$field,
762-
$(#[$try_field_doc])+
763-
$try_field,
764-
bit: $bit,
759+
$field: $bit,
765760
);
766761

767762
$crate::write_only_csr_field!(
@@ -807,8 +802,6 @@ macro_rules! read_write_csr_field {
807802
($ty:ident,
808803
$(#[$field_doc:meta])+
809804
$field:ident,
810-
$(#[$try_field_doc:meta])+
811-
$try_field:ident,
812805
$(#[$set_field_doc:meta])+
813806
$set_field:ident,
814807
$(#[$try_set_field_doc:meta])+
@@ -818,10 +811,7 @@ macro_rules! read_write_csr_field {
818811
$crate::read_only_csr_field!(
819812
$ty,
820813
$(#[$field_doc])+
821-
$field,
822-
$(#[$try_field_doc])+
823-
$try_field,
824-
range: [$bit_start : $bit_end],
814+
$field: [$bit_start : $bit_end],
825815
);
826816

827817
$crate::write_only_csr_field!(
@@ -886,31 +876,14 @@ macro_rules! read_write_csr_field {
886876
macro_rules! read_only_csr_field {
887877
($ty:ident,
888878
$(#[$field_doc:meta])+
889-
$field:ident,
890-
$(#[$try_field_doc:meta])+
891-
$try_field:ident,
892-
bit: $bit:literal$(,)?) => {
879+
$field:ident: $bit:literal$(,)?) => {
880+
const _: () = assert!($bit < usize::BITS);
881+
893882
impl $ty {
894883
$(#[$field_doc])+
895884
#[inline]
896885
pub fn $field(&self) -> bool {
897-
self.$try_field().unwrap()
898-
}
899-
900-
$(#[$try_field_doc])+
901-
#[inline]
902-
pub fn $try_field(&self) -> $crate::result::Result<bool> {
903-
let max_width = core::mem::size_of_val(&self.bits) * 8;
904-
905-
if $bit < max_width {
906-
Ok($crate::bits::bf_extract(self.bits, $bit, 1) != 0)
907-
} else {
908-
Err($crate::result::Error::IndexOutOfBounds {
909-
index: $bit,
910-
min: 0,
911-
max: max_width,
912-
})
913-
}
886+
$crate::bits::bf_extract(self.bits, $bit, 1) != 0
914887
}
915888
}
916889
};
@@ -921,6 +894,9 @@ macro_rules! read_only_csr_field {
921894
$(#[$try_field_doc:meta])+
922895
$try_field:ident,
923896
range: $bit_start:literal..=$bit_end:literal$(,)?) => {
897+
const _: () = assert!($bit_end < usize::BITS);
898+
const _: () = assert!($bit_start <= $bit_end);
899+
924900
impl $ty {
925901
$(#[$field_doc])+
926902
#[inline]
@@ -931,9 +907,7 @@ macro_rules! read_only_csr_field {
931907
$(#[$try_field_doc])+
932908
#[inline]
933909
pub fn $try_field(&self, index: usize) -> $crate::result::Result<bool> {
934-
let max_width = core::mem::size_of_val(&self.bits) * 8;
935-
936-
if $bit_end < max_width && ($bit_start..=$bit_end).contains(&index) {
910+
if ($bit_start..=$bit_end).contains(&index) {
937911
Ok($crate::bits::bf_extract(self.bits, index, 1) != 0)
938912
} else {
939913
Err($crate::result::Error::IndexOutOfBounds {
@@ -948,32 +922,16 @@ macro_rules! read_only_csr_field {
948922

949923
($ty:ident,
950924
$(#[$field_doc:meta])+
951-
$field:ident,
952-
$(#[$try_field_doc:meta])+
953-
$try_field:ident,
954-
range: [$bit_start:literal : $bit_end:literal]$(,)?) => {
925+
$field:ident: [$bit_start:literal : $bit_end:literal]$(,)?) => {
926+
const _: () = assert!($bit_end < usize::BITS);
927+
const _: () = assert!($bit_start <= $bit_end);
928+
955929
impl $ty {
956930
$(#[$field_doc])+
957931
#[inline]
958932
pub fn $field(&self) -> usize {
959933
$crate::bits::bf_extract(self.bits, $bit_start, $bit_end - $bit_start + 1)
960934
}
961-
962-
$(#[$try_field_doc])+
963-
#[inline]
964-
pub fn $try_field(&self) -> $crate::result::Result<usize> {
965-
let max_width = core::mem::size_of_val(&self.bits) * 8;
966-
967-
if $bit_end < max_width && $bit_start <= $bit_end {
968-
Ok($crate::bits::bf_extract(self.bits, $bit_start, $bit_end - $bit_start + 1))
969-
} else {
970-
Err($crate::result::Error::IndexOutOfBounds {
971-
index: $bit_start,
972-
min: $bit_start,
973-
max: $bit_end,
974-
})
975-
}
976-
}
977935
}
978936
};
979937

@@ -1017,35 +975,28 @@ macro_rules! read_only_csr_field {
1017975
$field_ty:ident,
1018976
range: [$field_start:literal : $field_end:literal]$(,)?
1019977
) => {
1020-
impl $ty {
1021-
$(#[$field_doc])+
1022-
#[inline]
1023-
pub fn $field(&self) -> $field_ty {
1024-
self.$try_field().unwrap()
1025-
}
978+
const _: () = assert!($field_end < usize::BITS);
979+
const _: () = assert!($field_start <= $field_end);
1026980

1027-
$(#[$try_field_doc])+
1028-
#[inline]
1029-
pub fn $try_field(&self) -> $crate::result::Result<$field_ty> {
1030-
let max_width = core::mem::size_of_val(&self.bits) * 8;
1031-
1032-
if $field_end < max_width && $field_start < $field_end {
1033-
let value = $crate::bits::bf_extract(
1034-
self.bits,
1035-
$field_start,
1036-
$field_end - $field_start + 1,
1037-
);
981+
impl $ty {
982+
$(#[$field_doc])+
983+
#[inline]
984+
pub fn $field(&self) -> $field_ty {
985+
self.$try_field().unwrap()
986+
}
1038987

1039-
$field_ty::from_usize(value)
1040-
} else {
1041-
Err($crate::result::Error::IndexOutOfBounds {
1042-
index: $field_start,
1043-
min: $field_start,
1044-
max: $field_end,
1045-
})
1046-
}
1047-
}
1048-
}
988+
$(#[$try_field_doc])+
989+
#[inline]
990+
pub fn $try_field(&self) -> $crate::result::Result<$field_ty> {
991+
let value = $crate::bits::bf_extract(
992+
self.bits,
993+
$field_start,
994+
$field_end - $field_start + 1,
995+
);
996+
997+
$field_ty::from_usize(value)
998+
}
999+
}
10491000
};
10501001
}
10511002

‎riscv/src/register/mcountinhibit.rs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@ read_write_csr_field! {
1515
Mcountinhibit,
1616
/// Gets the `cycle[h]` inhibit field value.
1717
cy,
18-
/// Attempts to get the `cycle[h]` inhibit field value.
19-
try_cy,
2018
/// Sets the `cycle[h]` inhibit field value.
2119
///
2220
/// **NOTE**: only updates the in-memory value without touching the CSR.
@@ -32,8 +30,6 @@ read_write_csr_field! {
3230
Mcountinhibit,
3331
/// Gets the `instret[h]` inhibit field value.
3432
ir,
35-
/// Attempts to get the `instret[h]` inhibit field value.
36-
try_ir,
3733
/// Sets the `instret[h]` inhibit field value.
3834
///
3935
/// **NOTE**: only updates the in-memory value without touching the CSR.

‎riscv/src/register/tests/read_only_csr.rs‎

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,7 @@ read_only_csr! {
99
read_only_csr_field! {
1010
Mtest,
1111
/// test single-bit field
12-
single,
13-
/// try-getter test single-bit field
14-
try_single,
15-
bit: 0,
12+
single: 0,
1613
}
1714

1815
read_only_csr_field! {
@@ -27,10 +24,7 @@ read_only_csr_field! {
2724
read_only_csr_field!(
2825
Mtest,
2926
/// multi-bit field
30-
multi_field,
31-
/// try-getter multi-bit field
32-
try_multi_field,
33-
range: [4:7],
27+
multi_field: [4:7],
3428
);
3529

3630
read_only_csr_field!(
@@ -41,7 +35,7 @@ read_only_csr_field!(
4135
try_field_enum,
4236
/// field enum type with valid field variants
4337
MtestFieldEnum {
44-
range: [7:11],
38+
range: [8:11],
4539
default: Field1,
4640
Field1 = 1,
4741
Field2 = 2,
@@ -70,11 +64,9 @@ fn test_mtest_read_only() {
7064

7165
// check that single bit field getter/setters work.
7266
assert_eq!(mtest.single(), false);
73-
assert_eq!(mtest.try_single(), Ok(false));
7467

7568
mtest = Mtest::from_bits(1);
7669
assert_eq!(mtest.single(), true);
77-
assert_eq!(mtest.try_single(), Ok(true));
7870

7971
mtest = Mtest::from_bits(0);
8072

@@ -94,25 +86,20 @@ fn test_mtest_read_only() {
9486

9587
// check that multi-bit field getter/setters work.
9688
assert_eq!(mtest.multi_field(), 0);
97-
assert_eq!(mtest.try_multi_field(), Ok(0));
9889

9990
mtest = Mtest::from_bits(0xf << 4);
10091
assert_eq!(mtest.multi_field(), 0xf);
101-
assert_eq!(mtest.try_multi_field(), Ok(0xf));
10292

10393
mtest = Mtest::from_bits(0x3 << 4);
10494
assert_eq!(mtest.multi_field(), 0x3);
105-
assert_eq!(mtest.try_multi_field(), Ok(0x3));
10695

10796
// check that only bits in the field are set.
10897
mtest = Mtest::from_bits(0xff << 4);
10998
assert_eq!(mtest.multi_field(), 0xf);
110-
assert_eq!(mtest.try_multi_field(), Ok(0xf));
11199
assert_eq!(mtest.bits(), 0xff << 4);
112100

113101
mtest = Mtest::from_bits(0x0 << 4);
114102
assert_eq!(mtest.multi_field(), 0x0);
115-
assert_eq!(mtest.try_multi_field(), Ok(0x0));
116103

117104
assert_eq!(mtest.try_field_enum(), Err(Error::InvalidVariant(0)),);
118105

@@ -124,12 +111,12 @@ fn test_mtest_read_only() {
124111
]
125112
.into_iter()
126113
.for_each(|variant| {
127-
mtest = Mtest::from_bits(variant.into_usize() << 7);
114+
mtest = Mtest::from_bits(variant.into_usize() << 8);
128115
assert_eq!(mtest.field_enum(), variant);
129116
assert_eq!(mtest.try_field_enum(), Ok(variant));
130117
});
131118

132119
// check that setting an invalid variant returns `None`
133-
mtest = Mtest::from_bits(0xbad << 7);
120+
mtest = Mtest::from_bits(0xbad << 8);
134121
assert_eq!(mtest.try_field_enum(), Err(Error::InvalidVariant(13)),);
135122
}

‎riscv/src/register/tests/read_write_csr.rs‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@ read_write_csr_field! {
1010
Mtest,
1111
/// test single-bit field
1212
single,
13-
/// try-get test single-bit field
14-
try_single,
1513
/// setter test single-bit field
1614
set_single,
1715
/// try-setter test single-bit field
@@ -36,8 +34,6 @@ read_write_csr_field!(
3634
Mtest,
3735
/// multi-bit field
3836
multi_field,
39-
/// try-get multi-bit field
40-
try_multi_field,
4137
/// setter multi-bit field
4238
set_multi_field,
4339
/// try-setter multi-bit field
@@ -119,21 +115,17 @@ fn test_mtest_read_write() {
119115

120116
mtest.set_multi_field(0xf);
121117
assert_eq!(mtest.multi_field(), 0xf);
122-
assert_eq!(mtest.try_multi_field(), Ok(0xf));
123118

124119
mtest.set_multi_field(0x3);
125120
assert_eq!(mtest.multi_field(), 0x3);
126-
assert_eq!(mtest.try_multi_field(), Ok(0x3));
127121

128122
// check that only bits in the field are set.
129123
mtest.set_multi_field(0xff);
130124
assert_eq!(mtest.multi_field(), 0xf);
131-
assert_eq!(mtest.try_multi_field(), Ok(0xf));
132125
assert_eq!(mtest.bits(), 0xf << 4);
133126

134127
mtest.set_multi_field(0x0);
135128
assert_eq!(mtest.multi_field(), 0x0);
136-
assert_eq!(mtest.try_multi_field(), Ok(0x0));
137129

138130
assert_eq!(mtest.try_field_enum(), Err(Error::InvalidVariant(0)),);
139131

0 commit comments

Comments
 (0)