Skip to content

Changed uint to use $imul #1307

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

Merged
merged 2 commits into from
May 23, 2024
Merged

Changed uint to use $imul #1307

merged 2 commits into from
May 23, 2024

Conversation

grantnelson-wf
Copy link
Collaborator

@grantnelson-wf grantnelson-wf commented May 22, 2024

When working in go1.20 I ran into crypto mathematics that failed because uint is using *. The math overflowed the JS int causing it to switch to using an exponent and dropping the LSBs. Crypto needs those LSBs and expected the value to rollover. Using $imul instead of * fixes this problem. Since uint is equivalent to uint32 in GopherJS and int32, uintptr, uint32, and int were already using $imul I figured that uint should probably be using it too.

GopherJS emulates a 32-bit environment. This means that int, uint and uintptr have a precision of 32 bits.

Even though this issue was discovered while working on go1.20, the fix isn't specifically related to go1.20. This should help anyone using master and also expecting uint to rollover the same as uint32.

@grantnelson-wf grantnelson-wf marked this pull request as ready for review May 22, 2024 16:34
@flimzy flimzy merged commit 7abe9dc into gopherjs:master May 23, 2024
8 checks passed
@grantnelson-wf grantnelson-wf deleted the uintMul branch May 23, 2024 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants