From 8de20781e87fd8dc0d71e6a1b10c08c953dd8fb1 Mon Sep 17 00:00:00 2001 From: Lars Wrenger Date: Sat, 11 Nov 2023 20:49:09 +0100 Subject: [PATCH] :bug: Fix bounds check for signed types #24 Coincidentally, this also fixes #22 --- Cargo.lock | 14 +++++++------- Cargo.toml | 2 +- src/lib.rs | 52 +++++++++++++++++++++++++++++++++++---------------- tests/test.rs | 18 ++++++++++++++++++ 4 files changed, 62 insertions(+), 24 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5a923d4..7b9ff4b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4,7 +4,7 @@ version = 3 [[package]] name = "bitfield-struct" -version = "0.5.4" +version = "0.5.5" dependencies = [ "proc-macro2", "quote", @@ -13,9 +13,9 @@ dependencies = [ [[package]] name = "proc-macro2" -version = "1.0.66" +version = "1.0.69" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "18fb31db3f9bddb2ea821cde30a9f70117e3f119938b5ee630b7403aa6e2ead9" +checksum = "134c189feb4956b20f6f547d2cf727d4c0fe06722b20a0eec87ed445a97f92da" dependencies = [ "unicode-ident", ] @@ -31,9 +31,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.29" +version = "2.0.39" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c324c494eba9d92503e6f1ef2e6df781e78f6a7705a0202d9801b198807d518a" +checksum = "23e78b90f2fcf45d3e842032ce32e3f2d1545ba6636271dcbf24fa306d87be7a" dependencies = [ "proc-macro2", "quote", @@ -42,6 +42,6 @@ dependencies = [ [[package]] name = "unicode-ident" -version = "1.0.11" +version = "1.0.12" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "301abaae475aa91687eb82514b328ab47a211a533026cb25fc3e519b86adfc3c" +checksum = "3354b9ac3fae1ff6755cb6db53683adb661634f67557942dea4facebec0fee4b" diff --git a/Cargo.toml b/Cargo.toml index 395805e..8e5d324 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "bitfield-struct" -version = "0.5.4" +version = "0.5.5" edition = "2021" authors = ["Lars Wrenger "] description = "Struct-like procedural macro for bitfields." diff --git a/src/lib.rs b/src/lib.rs index d0e0c39..07f54ce 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -449,7 +449,7 @@ impl Member { mut default, into, from, - } = parse_field(&attrs, &ty, ignore)?; + } = parse_field(&base_ty, &attrs, &ty, ignore)?; if bits > 0 && !ignore { if offset + bits > base_bits { @@ -540,8 +540,17 @@ impl ToTokens for Member { bits, base_ty, default: _, - inner: Some(MemberInner { ident, ty, attrs, vis, into, from }), - } = self else { + inner: + Some(MemberInner { + ident, + ty, + attrs, + vis, + into, + from, + }), + } = self + else { return Default::default(); }; @@ -621,7 +630,12 @@ struct Field { } /// Parses the `bits` attribute that allows specifying a custom number of bits. -fn parse_field(attrs: &[syn::Attribute], ty: &syn::Type, ignore: bool) -> syn::Result { +fn parse_field( + base_ty: &syn::Type, + attrs: &[syn::Attribute], + ty: &syn::Type, + ignore: bool, +) -> syn::Result { fn malformed(mut e: syn::Error, attr: &syn::Attribute) -> syn::Error { e.combine(syn::Error::new(attr.span(), "malformed #[bits] attribute")); e @@ -641,8 +655,8 @@ fn parse_field(attrs: &[syn::Attribute], ty: &syn::Type, ignore: bool) -> syn::R bits: ty_bits, ty: ty.clone(), default: quote!(0), - into: TokenStream::new(), - from: TokenStream::new(), + into: quote!(), + from: quote!(), }, TypeClass::UInt => Field { bits: ty_bits, @@ -654,7 +668,7 @@ fn parse_field(attrs: &[syn::Attribute], ty: &syn::Type, ignore: bool) -> syn::R TypeClass::Other => Field { bits: ty_bits, ty: ty.clone(), - default: TokenStream::new(), + default: quote!(), into: quote!(#ty::into_bits(this)), from: quote!(#ty::from_bits(this)), }, @@ -663,11 +677,12 @@ fn parse_field(attrs: &[syn::Attribute], ty: &syn::Type, ignore: bool) -> syn::R // Find and parse the bits attribute for attr in attrs { let syn::Attribute { - style: syn::AttrStyle::Outer, - meta: syn::Meta::List(syn::MetaList { path, tokens, .. }), - .. - } = attr else { - continue + style: syn::AttrStyle::Outer, + meta: syn::Meta::List(syn::MetaList { path, tokens, .. }), + .. + } = attr + else { + continue; }; if path.is_ident("bits") { let span = tokens.span(); @@ -678,6 +693,7 @@ fn parse_field(attrs: &[syn::Attribute], ty: &syn::Type, ignore: bool) -> syn::R from, } = syn::parse2(tokens.clone()).map_err(|e| malformed(e, attr))?; + // bit size if let Some(bits) = bits { if bits == 0 { return Err(syn::Error::new(span, "bits cannot bit 0")); @@ -687,6 +703,7 @@ fn parse_field(attrs: &[syn::Attribute], ty: &syn::Type, ignore: bool) -> syn::R } ret.bits = bits; } + // padding if ignore && (into.is_some() || from.is_some()) { return Err(syn::Error::new( default.span(), @@ -694,6 +711,7 @@ fn parse_field(attrs: &[syn::Attribute], ty: &syn::Type, ignore: bool) -> syn::R )); } + // conversion if let Some(into) = into { ret.into = quote!(#into(this)); } @@ -726,9 +744,11 @@ fn parse_field(attrs: &[syn::Attribute], ty: &syn::Type, ignore: bool) -> syn::R if ret.into.is_empty() { // Bounds check and remove leading ones from negative values ret.into = quote! {{ - #[allow(unused_comparisons)] - debug_assert!(if this >= 0 { this & !#mask == 0 } else { !this & !#mask == 0 }, "value out of bounds"); - (this & #mask) as _ + debug_assert!({ + let m = #ty::MIN >> (#ty::BITS - #bits); + m <= this && this <= -(m + 1) + }); + (this as #base_ty & #mask) }}; } if ret.from.is_empty() { @@ -857,7 +877,7 @@ impl Parse for Params { /// Returns the number of bits for a given type fn type_bits(ty: &syn::Type) -> (TypeClass, usize) { - let syn::Type::Path(syn::TypePath{ path, .. }) = ty else { + let syn::Type::Path(syn::TypePath { path, .. }) = ty else { return (TypeClass::Other, 0); }; let Some(ident) = path.get_ident() else { diff --git a/tests/test.rs b/tests/test.rs index 98e0530..e043ad8 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -163,6 +163,24 @@ fn negative() { assert_eq!(v.negative(), -4); } +#[test] +fn negative_signed() { + #[bitfield(u64)] + struct MyBitfield { + negative: i32, + #[bits(10)] + positive: u16, + #[bits(22)] + __: u32, + } + + let val = MyBitfield::new() + .with_negative(-1) + .with_positive(0b11_1111_1111); + assert_eq!(val.negative(), -1); + assert_eq!(val.positive(), 0b11_1111_1111); +} + #[test] fn entirely_negative() { #[bitfield(u32)]