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

Prevent overflow #471

Merged
merged 6 commits into from
Jan 7, 2020
Merged

Prevent overflow #471

merged 6 commits into from
Jan 7, 2020

Conversation

minad
Copy link
Member

@minad minad commented Dec 4, 2019

disabled the overestimate test for the INT_MAX-1 number for now since this number is not allowed anymore.

tommath_private.h Outdated Show resolved Hide resolved
@minad
Copy link
Member Author

minad commented Dec 5, 2019

@czurnieden could you do me a favor and recompute the biggest number table for the overestimate test which is disabled now?

@czurnieden
Copy link
Contributor

could you do me a favor and recompute the biggest number table for the overestimate test which is disabled now?

If you want to make the maximum size dependent on MP_DIGIT_BIT we would need several tables, one for each size/MP_xBIT and two for MP_16BIT because sizeof(int) can be 2.

That may take a day or two ;-)

@minad
Copy link
Member Author

minad commented Dec 5, 2019

Uuh could we do it somehow differently?

@minad
Copy link
Member Author

minad commented Dec 5, 2019

@czurnieden I added a randomized test for now.

@minad
Copy link
Member Author

minad commented Dec 5, 2019

@sjaeckel Right, I added a workaround using the tommath_c89.h header.

@minad
Copy link
Member Author

minad commented Dec 6, 2019

@sjaeckel Good to merge?

@minad
Copy link
Member Author

minad commented Dec 16, 2019

@sjaeckel Ping. Can we merge this?

mp_grow.c Outdated Show resolved Hide resolved
@czurnieden
Copy link
Contributor

What is your use case, may I ask?

A new and improved mp_log_n.
And also extended to full bigint if my calculations are not too much off.

It nagged me from the start on, that it has a lot of full bigint operations with up to twice the number of digits of the input, the trial exponentiations can overflow MP_MAX_DIGIT_COUNT * MP_DIGIT_BIT (the squarings too, but you could put a lid on them), and that the base is, although not theoretically but in praxi restricted to a small native integer instead of the actual upper limit (MP_MAX_DIGIT_COUNT * MP_DIGIT_BIT) - 1 .

You can get that down to one trial exponentiation that does not overflow and up to two small trials. One of the trials is by multiplying with the base which can overflow but that overflow is usable by the trial, that's why I wanted the distinct error. The other trial is a division by the base. It is exact, but as long as we don't have "exact division" also slower than the multiplication for bigint bases.

That together with a rather large "hidden constant" might make it significantly slower than the old method for small input but I have no benchmarks at this stage.

So don't hold your breath ;-)

@sjaeckel sjaeckel merged commit ffd8066 into develop Jan 7, 2020
@sjaeckel sjaeckel deleted the prevent-overflow branch January 7, 2020 17:35
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 this pull request may close these issues.

3 participants