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

Right shift gives wrong result #2780

Closed
shuklaayush opened this issue Sep 21, 2023 · 3 comments
Closed

Right shift gives wrong result #2780

shuklaayush opened this issue Sep 21, 2023 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@shuklaayush
Copy link
Contributor

Aim

One of the tests in the noir-bigint library is failing because 0x80000000000000 >> 0x37 (as u56) is evaluating to 0 instead of 1

I wasn't able to create a minimal reproducible example but you can check it if you run

nargo test --package biguint --show-output test_div3

This test is failing and the relevant lines of code are here

image

Expected Behavior

Right shift should be calculated properly

Bug

0x80000000000000 >> 0x37 is evaluating to 0 instead of 1

To Reproduce

  1. Clone https://github.com/shuklaayush/noir-bigint/tree/debug/print-shl1-debug
  2. Run nargo test --package biguint --show-output test_div3

Installation Method

Binary

Nargo Version

nargo 0.12.0 (git version hash: 7b3df8e, 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 Sep 21, 2023
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir Sep 21, 2023
@vezenovm
Copy link
Contributor

I was unable to reproduce a runtime or comptime bitshift of 0x80000000000000 >> 0x37 that did not give 1. It looks like your program has an overflow. I see in your program you even have a TODO about returning early when n is zero here, because this can cause you to potentially right shift by 0x38 which is 56 and would cause an overflow. When running this program with #2713 I fail with Failed assertion: 'attempt to subtract with overflow'. I am going to leave this issue open until the overflow PR is finished.

So the bug looks to actually be that the prints are out of order when used in a loop in this manner. For example I should always have prints in this format:

"------------------"
"self.limbs[i - 1]"
0x00
"rshift"
0x37
"self.limbs[i - 1] >> rshift"
0x00
"0x80000000000000 >> rshift"
0x01

but I got this sometimes:

"------------------"
"self.limbs[i - 1]"
0x00
"rshift"
0x37
"self.limbs[i - 1] >> rshift"
0x00
"0x80000000000000 >> rshift"
0x01
0x01
0x38

I am going to reproduce the printing issue and make a separate issue for that.

@shuklaayush
Copy link
Contributor Author

this can cause you to potentially right shift by 0x38 which is 56 and would cause an overflow

I don't believe right shifting should cause an overflow with any inputs. If the shift is by 56, then shouldn't it just return 0?

When running this program with #2713 I fail with Failed assertion: 'attempt to subtract with overflow'.

I'll take a look but I would assume this is coming from other parts of the code that depends on wrapping operations

If you look at the screenshot, then one of the series of logs is the following

"------------------"
"self.limbs[i - 1]"
0x80000000000000
"rshift"
0x37
"self.limbs[i - 1] >> rshift"
0x00
"0x80000000000000 >> rshift"
0x01
"------------------"

This is wrong since 0x80000000000000 >> 0x37 should be 0x01 and not 0x00. I wasn't able to create a minimal example that reproduces this. Maybe it's because the prints are out of order, but they seem pretty orderly on my machine

@shuklaayush
Copy link
Contributor Author

I don't believe right shifting should cause an overflow with any inputs. If the shift is by 56, then shouldn't it just return 0?

Actually, I was wrong. This is considered overflowing behaviour in rust. The test works if I handle the overflowing case separately

@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Noir Oct 3, 2023
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

No branches or pull requests

2 participants