From 0ecaa57b62e42d7b0c149f9debf1927751e5ca70 Mon Sep 17 00:00:00 2001 From: Yuval Shekel Date: Tue, 31 Dec 2024 14:18:06 +0200 Subject: [PATCH] fix bug in poly-api regarding copy_coeffs() to a buffer larger than the poly --- .../default_backend/default_poly_backend.h | 30 +++++++++++++------ .../rust/icicle-core/src/polynomials/mod.rs | 7 ++++- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/icicle/include/icicle/polynomials/default_backend/default_poly_backend.h b/icicle/include/icicle/polynomials/default_backend/default_poly_backend.h index ef59f816f..d7fc13b40 100644 --- a/icicle/include/icicle/polynomials/default_backend/default_poly_backend.h +++ b/icicle/include/icicle/polynomials/default_backend/default_poly_backend.h @@ -259,7 +259,10 @@ namespace icicle { const int64_t deg_a = degree(a); const int64_t deg_b = degree(b); - ICICLE_ASSERT(deg_b >= 0) << "Polynomial division: divide by zero polynomial"; + if (deg_b < 0) { + ICICLE_LOG_ERROR << "Polynomial division: divide by zero polynomial. Skipping computation."; + return; + } // init: Q=0, R=a Q->allocate(deg_a - deg_b + 1, State::Coefficients, true /*=memset zeros*/); @@ -346,7 +349,10 @@ namespace icicle { numerator->transform_to_coefficients(); auto numerator_coeffs = get_context_storage_mutable(numerator); const auto N = numerator->get_nof_elements(); - ICICLE_ASSERT(vanishing_poly_degree <= N) << "divide_by_vanishing_polynomial(): degree is too large"; + if (vanishing_poly_degree > N) { + ICICLE_LOG_ERROR << "divide_by_vanishing_polynomial(): degree is too large. Skipping computation."; + return; + } out->allocate(N, State::Coefficients, true /*=set zeros*/); add_monomial_inplace(out, C::zero() - C::one(), 0); //-1 @@ -384,8 +390,10 @@ namespace icicle { void divide_by_vanishing_case_2N(PolyContext out, PolyContext numerator, uint64_t vanishing_poly_degree) { // in that special case the numertaor has 2N elements and output will be N elements - ICICLE_ASSERT(numerator->get_nof_elements() == 2 * vanishing_poly_degree) - << "invalid input size. Expecting numerator to be of size 2N"; + if (numerator->get_nof_elements() != 2 * vanishing_poly_degree) { + ICICLE_LOG_ERROR << "divide_by_vanishing_case_2N(): invalid input size. Skipping computation."; + return; + } // In the case where deg(P)=2N, I can transform numerator to Reversed-evals -> The second half is // a reversed-coset of size N with coset-gen the 2N-th root of unity. @@ -423,8 +431,10 @@ namespace icicle { void divide_by_vanishing_case_N(PolyContext out, PolyContext numerator, uint64_t vanishing_poly_degree) { // in that special case the numertaor has N elements and output will be N elements - ICICLE_ASSERT(numerator->get_nof_elements() == vanishing_poly_degree) - << "invalid input size. Expecting numerator to be of size N"; + if (numerator->get_nof_elements() != vanishing_poly_degree) { + ICICLE_LOG_ERROR << "divide_by_vanishing_case_N(): invalid input size. Skipping computation."; + return; + } const int N = vanishing_poly_degree; numerator->transform_to_coefficients(N); @@ -572,7 +582,8 @@ namespace icicle { ntt(d_evals, poly_size, NTTDir::kForward, ntt_config, d_evals); } break; default: - ICICLE_ASSERT(false) << "Invalid state to compute evaluations"; + ICICLE_LOG_ERROR << "Panic: Invalid state to compute evaluations"; + ICICLE_ASSERT(false); break; } } @@ -596,8 +607,9 @@ namespace icicle { const bool is_valid_end_idx = end_idx < nof_coeffs && end_idx >= start_idx; const bool is_valid_indices = is_valid_start_idx && is_valid_end_idx; if (!is_valid_indices) { - // return -1 instead? I could but 'get_coeff()' cannot with its current declaration - ICICLE_ASSERT(false) << "copy_coeffs() invalid indices"; + ICICLE_LOG_ERROR << "copy_coeffs() invalid indices (start=" << start_idx << ", end=" << end_idx + << ", nof_coeffs=" << nof_coeffs << "). Skipping copy..."; + return 0; } op->transform_to_coefficients(); diff --git a/wrappers/rust/icicle-core/src/polynomials/mod.rs b/wrappers/rust/icicle-core/src/polynomials/mod.rs index 96914bd57..387d2c4af 100644 --- a/wrappers/rust/icicle-core/src/polynomials/mod.rs +++ b/wrappers/rust/icicle-core/src/polynomials/mod.rs @@ -282,7 +282,7 @@ macro_rules! impl_univariate_polynomial_api { fn copy_coeffs + ?Sized>(&self, start_idx: u64, coeffs: &mut S) { let coeffs_len = coeffs.len() as u64; let nof_coeffs = self.get_nof_coeffs(); - let end_idx = cmp::min(nof_coeffs, start_idx + coeffs_len - 1); + let end_idx = cmp::min(nof_coeffs - 1, start_idx + coeffs_len - 1); unsafe { copy_coeffs(self.handle, coeffs.as_mut_ptr(), start_idx, end_idx); @@ -626,6 +626,11 @@ macro_rules! impl_polynomial_tests { f.copy_coeffs(0, HostSlice::from_mut_slice(&mut host_mem)); assert_eq!(host_mem, coeffs); + // read into larger buffer + let mut host_mem_large = vec![$field::zero(); coeffs.len() + 10]; + f.copy_coeffs(0, HostSlice::from_mut_slice(&mut host_mem_large)); + assert_eq!(host_mem_large[..coeffs.len()], coeffs); + // read coeffs to device memory let mut device_mem = DeviceVec::<$field>::device_malloc(coeffs.len()).unwrap(); f.copy_coeffs(0, &mut device_mem[..]);