Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Fixing possible overflow during multiplication #1381

Merged
merged 1 commit into from Jun 22, 2016
Merged

Fixing possible overflow during multiplication #1381

merged 1 commit into from Jun 22, 2016

Conversation

tomusdrw
Copy link
Collaborator

It was haunting me since I finished working on uint. In a following code there can be only one overflow, but cannot be sure when it will occur.

Most probably closes #1380

@tomusdrw tomusdrw added the A0-pleasereview 🤓 Pull request needs code review. label Jun 22, 2016
ret[i + j + 1] = res;

// Only single overflow possible there
carry2 = (o1 | o2 | o3) as u64;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why only single?

Copy link
Collaborator Author

@tomusdrw tomusdrw Jun 22, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are multiplying two 64bit numbers - the result will fit in 128bits.
Those operations are adding the possible carry (so two 64bits, split into couple of parts) - the result will fit in 64bits + 1 carry.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but you then add result to the next word, don't you ?..
let (res, o3) = res.overflowing_add(ret[i + j + 1]);
which can be also overflow

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

though forget it, i think overflow tests are made for each word there

@NikVolf NikVolf added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Jun 22, 2016
@gavofyork gavofyork merged commit bc1f603 into master Jun 22, 2016
@gavofyork gavofyork deleted the uint-fix branch June 22, 2016 13:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

arithmetic operation overflow on ARM
3 participants