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

Low level arithmetics bug: -3 mod 7 == 3 #12514

Closed
snjax opened this issue Oct 24, 2019 · 2 comments
Closed

Low level arithmetics bug: -3 mod 7 == 3 #12514

snjax opened this issue Oct 24, 2019 · 2 comments
Assignees

Comments

@snjax
Copy link

snjax commented Oct 24, 2019

Function mod returns the wrong result.

Nim versions affected by the bug: >= 1.0

Example

echo -3 mod 7

Current Output

3

Expected Output

-3

Possible Solution

rollback mod operation to 0.20.2 version

@snjax snjax changed the title Low level arithmetics bug: -3 mod 7 == 3, -3 %% 7 = 6 Low level arithmetics bug: -3 mod 7 == 3 Oct 24, 2019
@narimiran
Copy link
Member

narimiran commented Oct 25, 2019

An easier example to see the buggy behaviour:

echo -3 mod 5
echo -13 mod 5
3
-3

Another example:

echo ( 7 mod  5)
echo (-7 mod  5)
echo ( 7 mod -5)
echo (-7 mod -5)
echo "----"
echo ( 2 mod  5)
echo (-2 mod  5)
echo ( 2 mod -5)
echo (-2 mod -5)
2
-2
2
-2
----
2
2
2
2

I can confirm all these worked correctly in 0.20.x version.

@mratsim
Copy link
Collaborator

mratsim commented Oct 25, 2019

again? #12332

Seems like I jumped the gun. This is actually more intricate see C vs Python below.


It is very important for us that we can trust all Nim low-level operators to do give the same results at runtime, during constant folding and in the VM.

We implemented an anti-regression suite in stint (status-im/nim-stint#91) but those regressions, bugs and flawed tests (#11138, #9572) are very time-consuming.

This is a necessary condition to build healthy big int libraries, crypto libraries and number theory related libraries in Nim.

I suggest we have an extensive run-time, compile-time and semantic fold test suite for basic operators.


C output:

C uses the underlying hardware convention. On x86_64, the sign of the remainder is the sign of the dividend.

#include <stdio.h>
#include <stdint.h>

int main(int argc, char *argv[])
{
  int64_t dividend, divisor;

  dividend = -3;
  divisor = 5;
  printf("%d mod %d = %d\n", dividend, divisor, dividend % divisor);

  dividend = -13;
  divisor = 5;
  printf("%d mod %d = %d\n", dividend, divisor, dividend % divisor);

  printf("----------------------------------------------------------\n");
  printf("%d mod %d = %d\n",  7,  5,  7 %  5);
  printf("%d mod %d = %d\n", -7,  5, -7 %  5);
  printf("%d mod %d = %d\n",  7, -5,  7 % -5);
  printf("%d mod %d = %d\n", -7, -5, -7 % -5);
  printf("----\n");
  printf("%d mod %d = %d\n",  2,  5,  2 %  5);
  printf("%d mod %d = %d\n", -2,  5, -2 %  5);
  printf("%d mod %d = %d\n",  2, -5,  2 % -5);
  printf("%d mod %d = %d\n", -2, -5, -2 % -5);
}
-3 mod 5 = -3
-13 mod 5 = -3
----------------------------------------------------------
7 mod 5 = 2
-7 mod 5 = -2
7 mod -5 = 2
-7 mod -5 = -2
----
2 mod 5 = 2
-2 mod 5 = -2
2 mod -5 = 2
-2 mod -5 = -2

Python output:

Python has the convention that remainder is non-negative

print(f"-3 mod 5 = {-3 % 5}")
print(f"-13 mod 5 = {-13 % 5}")
print("----------------------------------------------------------")
print(f" 7 mod  5 = { 7 %  5}")
print(f"-7 mod  5 = {-7 %  5}")
print(f" 7 mod -5 = { 7 % -5}")
print(f"-7 mod -5 = {-7 % -5}")
print("----")
print(f" 2 mod  5 = { 2 %  5}")
print(f"-2 mod  5 = {-2 %  5}")
print(f" 2 mod -5 = { 2 % -5}")
print(f"-2 mod -5 = {-2 % -5}")
-3 mod 5 = 2
-13 mod 5 = 2
----------------------------------------------------------
 7 mod  5 = 2
-7 mod  5 = 3
 7 mod -5 = -3
-7 mod -5 = -2
----
 2 mod  5 = 2
-2 mod  5 = 3
 2 mod -5 = -3
-2 mod -5 = -2

I suggest we choose the same convention as C and assembly.

Note that once a convention is chosen, the output of div and mod must respect the equation A = quotient * B + remainder with A mod B = remainder and A div B = quotient


Important notice

As mentioned mod and div in C are "implementation-defined" by the hardware. We should just mention that in the VM for modulo/remainder operations we follow x86 convention.

krux02 added a commit to krux02/Nim that referenced this issue Oct 25, 2019
@Araq Araq closed this as completed in 3c567bc Oct 27, 2019
narimiran pushed a commit that referenced this issue Oct 30, 2019
(cherry picked from commit 3c567bc)
kiyolee pushed a commit to kiyolee/nim that referenced this issue Nov 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants