Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent output on calling function with same args #3607

Closed
shuklaayush opened this issue Nov 28, 2023 · 1 comment · Fixed by #3664
Closed

Inconsistent output on calling function with same args #3607

shuklaayush opened this issue Nov 28, 2023 · 1 comment · Fixed by #3664
Assignees
Labels
bug Something isn't working

Comments

@shuklaayush
Copy link
Contributor

Aim

Getting this weird error where the same function results in a different output based on whether it's called directly or inside a complex call stack

Expected Behavior

Output should be the same

Bug

The shl1 functoin in noir-bigint shifts a BigInt left by 1 bit.

Running it on a constant input gives the right result

nargo test shl6 --show-output
[biguint] Running 1 test functions
[biguint] Testing test_shl6...
BigUint56 { limbs: [0x80000000000000, 0x00, 0x00, 0x00, 0x00] }
BigUint56 { limbs: [0x00, 0x01, 0x00, 0x00, 0x00] }
ok

While when it is called from within div, it outputs 0 which is wrong

nargo test div3 --show-output
[biguint] Running 1 test functions
[biguint] Testing test_div3...
...
--------------------------------------------------------------------------------
BigUint56 { limbs: [0x80000000000000, 0x00, 0x00, 0x00, 0x00] }
BigUint56 { limbs: [0x00, 0x00, 0x00, 0x00, 0x00] }

To Reproduce

  1. Clone repository git clone https://github.com/shuklaayush/noir-bigint.git
  2. Switch to branch git checkout bug/inconsistent-shl1
  3. Run nargo test shl6 --show-output
  4. Run nargo test div3 --show-output and see last log

Installation Method

Binary

Nargo Version

nargo version = 0.19.4 noirc version = 0.19.4+1ef854686a416eb547cc0919d669627f43d01450 (git version hash: 1ef8546, is dirty: false)

Additional Context

No response

Would you like to submit a PR for this Issue?

Maybe

Support Needs

No response

@shuklaayush shuklaayush added the bug Something isn't working label Nov 28, 2023
@guipublic
Copy link
Contributor

After a lot of efforts and sweating, I managed to strip this issue down to a minimal example (30 opcodes):

use dep::biguint::BigUint56;

fn main( ) { 
    let a = BigUint56 { limbs: [0x02, 0x00, 0x00, 0x00, 0x00] };
    let b = BigUint56 { limbs: [0x10000000000000, 0x00, 0x00, 0x00, 0x00] };
    let c = BigUint56 { limbs: [0x00, 0x08, 0x00, 0x00, 0x00] };
    let (q, r) = indiv(a, b,c); 
    assert(q.eq(BigUint56 { limbs: [0, 1, 0, 0, 0] }));
}

fn indiv(mut rem: BigUint56, mut quo: BigUint56, mut c: BigUint56) -> (BigUint56, BigUint56) {
    for _i in 0..4 {
        if rem.limbs[0]==c.limbs[0] {
            assert(false);
            quo = quo.shl1();
        } else {
            quo = quo.shl1();
        }
        c = c.shr1();                 
    }
    (quo, rem)
}

Executing this program fails, but if you comment the first quo = quo.shl1(); (just after the assert(false)), then it works.
One can see that it is the same as the original issue by adding this at the end of shl_limb (in biguint):

if( (self.limbs[0]!= 0)  & (res.limbs[0] ==0)) {
    assert(self.limbs[0] == 0x80000000000000);
    println(self.limbs[0]>> 55);
}

When it fails, it prints 0x0 (which is not correct), and when it succeed (by commenting the first quo = quo.shl1();), then it prints 0x1 (which is correct).

github-merge-queue bot pushed a commit that referenced this issue Dec 1, 2023
# Description

## Problem\*

Resolves #3607 

## Summary\*
Identical divisions happening in both 'else' and 'then' should not be
simplified, because the non-taken branch will output (0,0) instead of
the division result.


## Additional Context



## Documentation\*

Check one:
- [X] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[Exceptional Case]** 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants