Skip to content

Commit

Permalink
fix: wrapping in signed division (#5134)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves #5129

## Summary\*
A negative and null quotient was computed as -0=2^{bit_size}, which
wraps over the bit size.


## 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.
  • Loading branch information
guipublic committed May 30, 2024
1 parent 8ae3d75 commit 29baeb4
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -979,6 +979,7 @@ impl AcirContext {
let max_power_of_two = self.add_constant(
FieldElement::from(2_i128).pow(&FieldElement::from(bit_size as i128 - 1)),
);
let zero = self.add_constant(FieldElement::zero());
let one = self.add_constant(FieldElement::one());

// Get the sign bit of rhs by computing rhs / max_power_of_two
Expand All @@ -998,10 +999,19 @@ impl AcirContext {
// Unsigned to signed: derive q and r from q1,r1 and the signs of lhs and rhs
// Quotient sign is lhs sign * rhs sign, whose resulting sign bit is the XOR of the sign bits
let q_sign = self.xor_var(lhs_leading, rhs_leading, AcirType::unsigned(1))?;

let quotient = self.two_complement(q1, q_sign, bit_size)?;
let remainder = self.two_complement(r1, lhs_leading, bit_size)?;

// Issue #5129 - When q1 is zero and quotient sign is -1, we compute -0=2^{bit_size},
// which is not valid because we do not wrap integer operations
// Similar case can happen with the remainder.
let q_is_0 = self.eq_var(q1, zero)?;
let q_is_not_0 = self.not_var(q_is_0, AcirType::unsigned(1))?;
let quotient = self.mul_var(quotient, q_is_not_0)?;
let r_is_0 = self.eq_var(r1, zero)?;
let r_is_not_0 = self.not_var(r_is_0, AcirType::unsigned(1))?;
let remainder = self.mul_var(remainder, r_is_not_0)?;

Ok((quotient, remainder))
}

Expand Down
2 changes: 1 addition & 1 deletion test_programs/execution_success/signed_div/src/main.nr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ struct SignedDivOp {
result: i8,
}

unconstrained fn main(ops: [SignedDivOp; 15]) {
fn main(ops: [SignedDivOp; 15]) {
for i in 0..15 {
assert_eq(ops[i].lhs / ops[i].rhs, ops[i].result);
}
Expand Down

0 comments on commit 29baeb4

Please sign in to comment.