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

Failing assertion while multiplying two BigUints #187

Closed
Armavica opened this issue Apr 18, 2016 · 2 comments · Fixed by #188
Closed

Failing assertion while multiplying two BigUints #187

Armavica opened this issue Apr 18, 2016 · 2 comments · Fixed by #188
Labels

Comments

@Armavica
Copy link

extern crate num;
use num::CheckedMul;
use num::bigint::BigUint;

fn main() {
    let s = "531137992816767098689588206552468627329593117727031923199444138200403559860852242739162502232636710047537552105951370000796528760829212940754539968588340162273730474622005920097370111";
    let a: BigUint = s.parse().unwrap();
    let b = a.clone();
    let _ = a.checked_mul(&b);
}

The code above crashes at this assertion in num-bigint:

thread '<main>' panicked at 'assertion failed: carry == 0', /home/virgile/.cargo/registry/src/github.com-88ac128001ac3a9a/num-bigint-0.1.32/src/lib.rs:783

while it runs fine with one digit less in s. The same problem occurs with an unchecked multiplication.

I am not sure how to interpret the comment below the assertion; is there an allocation I should make manually?

@cuviper
Copy link
Member

cuviper commented Apr 20, 2016

This is definitely a bug in the implementation, not something you did or should have done. The comments around that assertion are just development details from #133. In fact, @koverstreet if you see this, I'd appreciate if you can take a look.

FWIW, CheckedMul will always succeed on BigUint (barring bugs like this), so you can just use plain multiplication unless you need to be generic for other types too.

@cuviper cuviper added the T-bug label Apr 20, 2016
@koverstreet
Copy link
Contributor

I'll dig into it first thing tomorrow.

On Tue, Apr 19, 2016 at 7:37 PM, Josh Stone notifications@github.com
wrote:

This is definitely a bug in the implementation, not something you did or
should have done. The comments around that assertion are just development
details from #133 #133. In fact,
@koverstreet https://github.com/koverstreet if you see this, I'd
appreciate if you can take a look.

FWIW, CheckedMul will always succeed on BigUint (barring bugs like this),
so you can just use plain multiplication unless you need to be generic for
other types too.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#187 (comment)

koverstreet added a commit to koverstreet/num that referenced this issue Apr 20, 2016
When x.len() and y.len() are both equal and odd, we have x1.len() + y1.len() + 1
(the required size to multiply x1 and y1) > y.len() + 1

This fixes rust-num#187
koverstreet added a commit to koverstreet/num that referenced this issue Apr 20, 2016
When x.len() and y.len() are both equal and odd, we have x1.len() + y1.len() + 1
(the required size to multiply x1 and y1) > y.len() + 1

This fixes rust-num#187
@homu homu closed this as completed in #188 Apr 20, 2016
homu added a commit that referenced this issue Apr 20, 2016
bigint: Fix calculation of size for multiply temporaries

When x.len() and y.len() are both equal and odd, we have x1.len() + y1.len() + 1
(the required size to multiply x1 and y1) > y.len() + 1

This fixes #187
remexre pushed a commit to remexre/num that referenced this issue Jun 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants