From 53252fd521ce7818a1d97824be30466590d879f5 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Mon, 21 Oct 2024 08:51:57 +0100 Subject: [PATCH] fix: enforce correctness of decompositions performed at compile time (#6278) # Description ## Problem\* Resolves https://github.com/noir-lang/noir/issues/6244 ## Summary\* We were not checking that a radix decomposition simplification performed at compile time was valid, resulting in a truncation to fit the result array. This PR prevents the simplification in this case so we can instead error at runtime. ## Additional Context ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- .../src/ssa/ir/instruction/call.rs | 32 ++++++++++--------- .../Nargo.toml | 7 ++++ .../src/main.nr | 5 +++ .../Nargo.toml | 7 ++++ .../src/main.nr | 4 +++ 5 files changed, 40 insertions(+), 15 deletions(-) create mode 100644 test_programs/execution_failure/invalid_comptime_bits_decomposition/Nargo.toml create mode 100644 test_programs/execution_failure/invalid_comptime_bits_decomposition/src/main.nr create mode 100644 test_programs/execution_failure/invalid_comptime_bytes_decomposition/Nargo.toml create mode 100644 test_programs/execution_failure/invalid_comptime_bytes_decomposition/src/main.nr diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs index 3b202b38b11..8b540d45664 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs @@ -60,9 +60,7 @@ pub(super) fn simplify_call( } else { unreachable!("ICE: Intrinsic::ToRadix return type must be array") }; - let result_array = constant_to_radix(endian, field, 2, limb_count, dfg); - - SimplifyResult::SimplifiedTo(result_array) + constant_to_radix(endian, field, 2, limb_count, dfg) } else { SimplifyResult::None } @@ -79,10 +77,7 @@ pub(super) fn simplify_call( } else { unreachable!("ICE: Intrinsic::ToRadix return type must be array") }; - - let result_array = constant_to_radix(endian, field, radix, limb_count, dfg); - - SimplifyResult::SimplifiedTo(result_array) + constant_to_radix(endian, field, radix, limb_count, dfg) } else { SimplifyResult::None } @@ -611,7 +606,7 @@ fn constant_to_radix( radix: u32, limb_count: u32, dfg: &mut DataFlowGraph, -) -> ValueId { +) -> SimplifyResult { let bit_size = u32::BITS - (radix - 1).leading_zeros(); let radix_big = BigUint::from(radix); assert_eq!(BigUint::from(2u128).pow(bit_size), radix_big, "ICE: Radix must be a power of 2"); @@ -619,14 +614,21 @@ fn constant_to_radix( // Decompose the integer into its radix digits in little endian form. let decomposed_integer = big_integer.to_radix_le(radix); - let mut limbs = vecmap(0..limb_count, |i| match decomposed_integer.get(i as usize) { - Some(digit) => FieldElement::from_be_bytes_reduce(&[*digit]), - None => FieldElement::zero(), - }); - if endian == Endian::Big { - limbs.reverse(); + if limb_count < decomposed_integer.len() as u32 { + // `field` cannot be represented as `limb_count` bits. + // defer error to acir_gen. + SimplifyResult::None + } else { + let mut limbs = vecmap(0..limb_count, |i| match decomposed_integer.get(i as usize) { + Some(digit) => FieldElement::from_be_bytes_reduce(&[*digit]), + None => FieldElement::zero(), + }); + if endian == Endian::Big { + limbs.reverse(); + } + let result_array = make_constant_array(dfg, limbs, Type::unsigned(bit_size)); + SimplifyResult::SimplifiedTo(result_array) } - make_constant_array(dfg, limbs, Type::unsigned(bit_size)) } fn to_u8_vec(dfg: &DataFlowGraph, values: im::Vector>) -> Vec { diff --git a/test_programs/execution_failure/invalid_comptime_bits_decomposition/Nargo.toml b/test_programs/execution_failure/invalid_comptime_bits_decomposition/Nargo.toml new file mode 100644 index 00000000000..c4efe5b4bb4 --- /dev/null +++ b/test_programs/execution_failure/invalid_comptime_bits_decomposition/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "invalid_comptime_bits_decomposition" +type = "bin" +authors = [""] +compiler_version = ">=0.30.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_failure/invalid_comptime_bits_decomposition/src/main.nr b/test_programs/execution_failure/invalid_comptime_bits_decomposition/src/main.nr new file mode 100644 index 00000000000..a9ac06c42c6 --- /dev/null +++ b/test_programs/execution_failure/invalid_comptime_bits_decomposition/src/main.nr @@ -0,0 +1,5 @@ +fn main() -> pub [u1; 1] { + let large_number: Field = 2; + + large_number.to_be_bits() +} diff --git a/test_programs/execution_failure/invalid_comptime_bytes_decomposition/Nargo.toml b/test_programs/execution_failure/invalid_comptime_bytes_decomposition/Nargo.toml new file mode 100644 index 00000000000..7ec63576af3 --- /dev/null +++ b/test_programs/execution_failure/invalid_comptime_bytes_decomposition/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "invalid_comptime_bytes_decomposition" +type = "bin" +authors = [""] +compiler_version = ">=0.30.0" + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_failure/invalid_comptime_bytes_decomposition/src/main.nr b/test_programs/execution_failure/invalid_comptime_bytes_decomposition/src/main.nr new file mode 100644 index 00000000000..29be8508575 --- /dev/null +++ b/test_programs/execution_failure/invalid_comptime_bytes_decomposition/src/main.nr @@ -0,0 +1,4 @@ +fn main() -> pub [u8; 1] { + let large_number: Field = 256; + large_number.to_be_bytes() +}