Skip to content

fix: added asserts for total_num_variables >= point.len() check.#208

Merged
TomWambsgans merged 1 commit into
leanEthereum:mainfrom
ImanKalyanChakraborty:fix/add-asserts-for-possible-underflow
May 1, 2026
Merged

fix: added asserts for total_num_variables >= point.len() check.#208
TomWambsgans merged 1 commit into
leanEthereum:mainfrom
ImanKalyanChakraborty:fix/add-asserts-for-possible-underflow

Conversation

@ImanKalyanChakraborty
Copy link
Copy Markdown
Contributor

Summary

Fixes a potential integer underflow bug in SparseStatement::selector_num_variables() where unchecked usize subtraction could silently wrap to a huge value in release builds, or panic in debug builds.

Fix

Two targeted changes in crates/whir/src/lib.rs:

1. Assert the invariant at construction time in new() and new_next():

assert!(
    total_num_variables >= point.len(),
    "total_num_variables ({}) must be >= point.len() ({})",
    total_num_variables,
    point.len()
);

2. Use checked_sub in selector_num_variables() as defense-in-depth:

pub fn selector_num_variables(&self) -> usize {
    self.total_num_variables
        .checked_sub(self.inner_num_variables())
        .expect("invariant violated: total_num_variables < point.len()")
}

Note: unique_value() and dense() are inherently safe because unique_value always uses an empty point (len = 0), and dense sets total_num_variables = point.len() exactly, therfore no assertion is needed there.

Changes

  • crates/whir/src/lib.rs: Add assert! in SparseStatement::new() and SparseStatement::new_next(), use checked_sub in selector_num_variables()

Testing

All cargo run commands meantioned in the README are running properly.

References

Fixes #182

@ImanKalyanChakraborty
Copy link
Copy Markdown
Contributor Author

Hi @TomWambsgans, I have added the PR. Since you mentioned using assert instead of Result, not much modifications are necessary in other files. Only crates/whir/src/lib.rs is enough.

@TomWambsgans
Copy link
Copy Markdown
Collaborator

thanks

@TomWambsgans TomWambsgans merged commit fc1a990 into leanEthereum:main May 1, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unsigned underflow yields panicable constraint sizes

2 participants