fix: use platform independent method for counting new multiplication overflow from result limb count #916
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
When we count the new overflow for the multiplication result in field emulation, then part of the computation depends on the number of limbs of the multiplication result. Previously, we used
uint(math.Log2(float64(2*nbResLimbs-1)))+1
, but this lead to inconsistent results whennbResLimbs
was 0. In that case we take log2 of a negative value which isNaN
. However, on ARMuint(NaN) = 0
but on x86uint(NaN) = math.Max
.We now use
bits.Len
which is correct for all platforms and avoids doing cast to floating point values.Additionally, added a check that in case we compute number of limbs for two inputs both having zero inputs. Reviewing code-base, this edge case does not happen, but adding as a protective measure.
This bug appeared when we compoute inverse and division overflows. There, we reused multiplication precondition for computing overflow, but had omitted the number of limbs for the result from the hint. This PR also fixes these calls.
Type of change
How has this been tested?
Test suite.
How has this been benchmarked?
Not benchmarked.
Checklist:
golangci-lint
does not output errors locally