From fda565e703a38e5663e8fffaff786eb9bd6cbfa3 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 25 Jan 2024 09:58:40 +0100 Subject: [PATCH] Make conversions safer. --- number/src/macros.rs | 27 ++++++++++++++++++--------- number/src/serialize.rs | 4 ++-- number/src/traits.rs | 5 ++--- parser/src/powdr.lalrpop | 2 +- pil-analyzer/src/evaluator.rs | 24 ++++++++++++++++++++++++ pipeline/src/lib.rs | 2 +- 6 files changed, 48 insertions(+), 16 deletions(-) diff --git a/number/src/macros.rs b/number/src/macros.rs index 2eca334c5..c78fefd48 100644 --- a/number/src/macros.rs +++ b/number/src/macros.rs @@ -253,6 +253,18 @@ macro_rules! powdr_field { } } + impl FromStr for $name { + type Err = String; + fn from_str(s: &str) -> Result { + let n = BigUint::from_str(s).map_err(|e| e.to_string())?; + if n >= <$ark_type>::MODULUS.into() { + Err(format!("Decimal number \"{s}\" too large for field.")) + } else { + Ok(n.into()) + } + } + } + impl fmt::LowerHex for $name { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fmt::LowerHex::fmt(&self.to_integer(), f) @@ -267,16 +279,13 @@ macro_rules! powdr_field { Some(KnownField::$name) } - fn from_str(s: &str) -> Self { - Self { - value: <$ark_type>::from_str(s).unwrap(), - } - } - fn from_str_radix(s: &str, radix: u32) -> Result { - BigUint::from_str_radix(s, radix) - .map(|n| n.into()) - .map_err(|e| e.to_string()) + let n = BigUint::from_str_radix(s, radix).map_err(|e| e.to_string())?; + if n >= <$ark_type>::MODULUS.into() { + Err(format!("Hexadecimal number \"0x{s}\" too large for field.")) + } else { + Ok(n.into()) + } } fn to_degree(&self) -> DegreeType { diff --git a/number/src/serialize.rs b/number/src/serialize.rs index 48767ef53..f46d988a8 100644 --- a/number/src/serialize.rs +++ b/number/src/serialize.rs @@ -68,9 +68,9 @@ pub fn read_polys_csv_file(file: &mut impl Read) -> Vec<(String let value = if let Some(value) = value.strip_prefix("0x") { T::from_str_radix(value, 16).unwrap() } else if let Some(value) = value.strip_prefix('-') { - -T::from_str(value) + -T::from_str(value).unwrap() } else { - T::from_str(value) + T::from_str(value).unwrap() }; polys[idx].1.push(value); } diff --git a/number/src/traits.rs b/number/src/traits.rs index 8028ab54d..18027cb43 100644 --- a/number/src/traits.rs +++ b/number/src/traits.rs @@ -1,4 +1,4 @@ -use std::{fmt, hash::Hash, ops::*}; +use std::{fmt, hash::Hash, ops::*, str::FromStr}; use num_traits::{One, Zero}; @@ -78,6 +78,7 @@ pub trait FieldElement: + fmt::Debug + From + From + + FromStr + From + From + From @@ -110,8 +111,6 @@ pub trait FieldElement: fn from_bytes_le(bytes: &[u8]) -> Self; - fn from_str(s: &str) -> Self; - fn from_str_radix(s: &str, radix: u32) -> Result; /// Returns true if the value is in the "lower half" of the field, diff --git a/parser/src/powdr.lalrpop b/parser/src/powdr.lalrpop index 150f53dbc..4952c2e57 100644 --- a/parser/src/powdr.lalrpop +++ b/parser/src/powdr.lalrpop @@ -576,7 +576,7 @@ PublicIdentifier: String = { } FieldElement: T = { - r"[0-9][0-9_]*" => T::from_str(&<>.replace('_', "")), + r"[0-9][0-9_]*" => T::from_str(&<>.replace('_', "")).unwrap(), r"0x[0-9A-Fa-f][0-9A-Fa-f_]*" => T::from_str_radix(&<>[2..].replace('_', ""), 16).unwrap(), } diff --git a/pil-analyzer/src/evaluator.rs b/pil-analyzer/src/evaluator.rs index 31e9c5a78..27d51a1af 100644 --- a/pil-analyzer/src/evaluator.rs +++ b/pil-analyzer/src/evaluator.rs @@ -591,4 +591,28 @@ mod test { "#; parse_and_evaluate_symbol(src, "F.x"); } + + #[test] + #[should_panic = r#"Hexadecimal number \"0x9999999999999999999999999999999\" too large for field"#] + pub fn hex_number_outside_field() { + // This tests that the parser does not lose precision when parsing large integers. + // We are currently going through FieldElements in the parser, which have limited precision. + // As soon as we use Integers there, this test should succeed. + let src = r#" + let N = 0x9999999999999999999999999999999; + "#; + parse_and_evaluate_symbol(src, "N"); + } + + #[test] + #[should_panic = r#"Decimal number \"9999999999999999999999999999999\" too large for field"#] + pub fn decimal_number_outside_field() { + // This tests that the parser does not lose precision when parsing large integers. + // We are currently going through FieldElements in the parser, which have limited precision. + // As soon as we use Integers there, this test should succeed. + let src = r#" + let N = 9999999999999999999999999999999; + "#; + parse_and_evaluate_symbol(src, "N"); + } } diff --git a/pipeline/src/lib.rs b/pipeline/src/lib.rs index 8f64faa43..ae9cd857f 100644 --- a/pipeline/src/lib.rs +++ b/pipeline/src/lib.rs @@ -99,7 +99,7 @@ pub fn inputs_to_query_callback(inputs: Vec) -> impl QueryCa // called again. Ok(Some(0.into())) } - ["\"hint\"", value] => Ok(Some(T::from_str(value))), + ["\"hint\"", value] => Ok(Some(T::from_str(value).unwrap())), k => Err(format!("Unsupported query: {}", k.iter().format(", "))), } }