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

Conversion from 64-bit Number to BigInt is lossy #557

Open
gkaracha opened this issue Apr 12, 2020 · 0 comments · May be fixed by #719
Open

Conversion from 64-bit Number to BigInt is lossy #557

gkaracha opened this issue Apr 12, 2020 · 0 comments · May be fixed by #719
Labels
P2 major: an upcoming release type: bug

Comments

@gkaracha
Copy link
Member

Describe the bug
While solving issue #373 (PR #542), we found out that the conversion of 64-bit JavaScript Numbers into BigInts is lossy. Though somewhat expected [1], this has repercussions for Asterius, since the 64-bit versions of memset do not work as expected when contents above MAX_SAFE_INTEGER are used (this affects types that are represented using 64 bits such as Word, Word64, and Ptr). For example, running this piece of code

do
  dst <- newPinnedByteArray 32
  setByteArray dst (3*8) 1 (0xcccccccccccccccc :: Word64)
  w <- readByteArray dst 3
  print (w == (0xcccccccccccccccc :: Word64))

unintuitively prints False.

[1]: The maximum integer that can be safely represented in JavaScript is Number.MAX_SAFE_INTEGER = 2^53 - 1, because JavaScript uses IEEE754 double precision floating point number representation (see here).

To Reproduce
The easiest way to reproduce the issue is to run:

stack test asterius:primitive

This test uses all Haskell variants of memset on a given array, and checks that storing (using memset) and retrieving should yield the same result. Hence, it is expected that running the test prints only Trues (each False is a case where this above property is violated).

Expected behavior
Running stack test asterius:primitive should only print ByteArrays and True.

Environment

  • OS name + version: Ubuntu 18.04.4 LTS (bionic)
  • GHC version: 8.8.3
  • Cabal version: 3.0.1.0
  • Version of Asterius: current HEAD (5ad234e)

Additional context

The conversion from Number to BigInt happens in asterius/rts/rts.memory.mjs where a JS version of memset is implemented:

memset(_dst, c, n, size = 1) {
  // We only allow 1, 2, 4, 8. Any other size should get a runtime error.
  const ty = {
    1 : Uint8Array,
    2 : Uint16Array,
    4 : Uint32Array,
    8 : BigUint64Array
  }; 
  const buf = this.expose(_dst, n, ty[size]);

  if (size === 8) {
    // TODO: The conversion BigInt(c) is lossy. Numbers are represented as 
    // IEEE754 double precision floating point numbers, for which the maximum
    // (representable) safe integer in JavaScript is (Number.MAX_SAFE_INTEGER
    // = 2^53 - 1).
    buf.fill(BigInt(c));
  } else {
    buf.fill(c);
  }  
}

The input values are always Numbers, even when we want them to represent 64-bit integers, so the conversion BigInt(c) is unavoidable.

@gkaracha gkaracha added P2 major: an upcoming release type: bug labels Apr 12, 2020
@gkaracha gkaracha linked a pull request Jul 22, 2020 that will close this issue
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P2 major: an upcoming release type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant