Repository: pyodide/pyodide
Pull request: Fix large integer conversions
This resolves #2482. I added hypothesis test coverage but it never looks at integers as large as 2^63 even after many many trials so it doesn't find the bug.


### Checklists

- [x] Add a [CHANGELOG](https://github.com/pyodide/pyodide/blob/main/docs/project/changelog.md) entry
- [x] Add / update tests


Problem statement:
Power operator incorrect result
## 🐛 Bug

I have an incorrect result when using the power operator ** with some values

### To Reproduce

```print(5 ** 55)```
result : -62726610764649328357466690409063133331

### Expected behavior

```print(5 ** 55)```
result : 277555756156289135105907917022705078125

### Environment

- Pyodide Version: Current demo in browser version (showing python 3.10.2)
- Browser version: Chrome 101.0.4951.41 (Official Build) (arm64)

Hints from issue discussion:
Thanks for the report. It's interesting that we didn't catch this for a long time.
~I am getting the correct results from Python 3.11 alpha 6 REPL of @tiran's (https://cheimes.fedorapeople.org/python-wasm/)~

~@tiran Do you have any tips on this issue?~

Edit:

Oh, sorry, never mind. It looks like it's our console.html that is malfunctioning.

![image](https://user-images.githubusercontent.com/24893111/166670312-3c2b1f8c-2be3-4ea5-8efd-cc87836e5b55.png)

That's good but probably this bug is that python2js misencodes a BigInt so it's not just a repl bug.
Okay, actually `python2js` translates this correctly. Thanks again to hypothesis for locating several bugs like this that were in the original implementation @ZAC-HD.

So @ryanking13 is right, it is most likely a problem in `console.html`.
Aha it is a type translations problem but the problem is in the other direction: 
```py
from js import eval
y = eval('277555756156289135105907917022705078125n')
print(y) 
```
should print `277555756156289135105907917022705078125` but instead it prints `-62726610764649328357466690409063133331`

We didn't catch this with hypothesis since we told it not to look for it: `test_number_conversions` specifically restricts to small integers:
```py
integers(min_value=-(2**53), max_value=2**53),
```
Interesting. Here is another code for minimal reproduction.

```html
<!DOCTYPE html>
<html>
  <head>
      <script src="https://cdn.jsdelivr.net/pyodide/v0.20.0/full/pyodide.js"></script>
  </head>
  <body>
    <script type="text/javascript">
      async function main(){
        let pyodide = await loadPyodide();
		let x = pyodide.runPython(`
			def x(v): return v
			x
		`);
		let v = BigInt(2)**63n;
		console.log("v:", v);
		console.log("x(v):", x(v));
      }
      main();
    </script>
  </body>
</html>
```

```
v: 9223372036854775808n
x(v): -9223372036854775808n
```
Hypothesis doesn't seem to be able to find it even after 10,000 tries. I guess it's pretty rare.
The bug is in this function on line 44 of js2python.js:
```js
  function js2python_bigint(value) {
    let value_orig = value;
    let length = 0;
    if (value < 0) {
      value = -value;
    }
    while (value) {
      length++;
      value >>= BigInt(32);
    }
    let stackTop = stackSave();
    let ptr = stackAlloc(length * 4);
    value = value_orig;
    for (let i = 0; i < length; i++) {
      ASSIGN_U32(ptr, i, Number(value & BigInt(0xffffffff)));
      value >>= BigInt(32);
    }
    let result = __PyLong_FromByteArray(
      ptr,
      length * 4 /* length in bytes */,
      true /* little endian */,
      true /* signed? */
    );
    stackRestore(stackTop);
    return result;
  }
```
The problem seems to occur for all integers in the range from `2^{32*i - 1}` to `2^{32*i}` for each i. We are setting the sign bit by mistake.

Commit message: Update changelog

Patch to review:
diff --git a/Makefile.envs b/Makefile.envs
index 85c91449468..0afd23a1ad8 100644
--- a/Makefile.envs
+++ b/Makefile.envs
@@ -52,7 +52,7 @@ export DBG_LDFLAGS_SOURCEMAPDEBUG=-gseparate-dwarf
 export DBGFLAGS=$(DBGFLAGS_NODEBUG)
 
 # Include debug symbols but no source maps (most useful)
-#export DBGFLAGS=$(DBGFLAGS_WASMDEBUG)
+export DBGFLAGS=$(DBGFLAGS_WASMDEBUG)
 
 # Debug with source maps (less useful than WASMDEBUG but easier if it helps)
 #export DBGFLAGS=$(DBGFLAGS_SOURCEMAPDEBUG)
diff --git a/docs/project/changelog.md b/docs/project/changelog.md
index af06417454e..c78fcf5bab1 100644
--- a/docs/project/changelog.md
+++ b/docs/project/changelog.md
@@ -36,9 +36,15 @@ substitutions:
   `pyodide.runPython(code, { globals : some_dict})`;
   {pr}`2391`
 
-- {{ Bugfix }} The build will error out earlier if `cmake` or `libtool` are not installed.
+- {{ Fix }} The build will error out earlier if `cmake` or `libtool` are not installed.
+  {pr}`2423`
 
 - {{ Enhancement }} Add SHA-256 hash of package to entries in `packages.json`
+  {pr}`2455`
+
+- {{ Fix }} BigInt's between 2^{32\*n - 1} and 2^{32\*n} no longer get
+  translated to negative Python ints.
+  {pr}`2484`
 
 ### Packages
 
diff --git a/src/core/js2python.js b/src/core/js2python.js
index 2a72a9fce04..92f11b71566 100644
--- a/src/core/js2python.js
+++ b/src/core/js2python.js
@@ -47,6 +47,7 @@ JS_FILE(js2python_init, () => {
     if (value < 0) {
       value = -value;
     }
+    value <<= BigInt(1);
     while (value) {
       length++;
       value >>= BigInt(32);
diff --git a/src/tests/test_typeconversions.py b/src/tests/test_typeconversions.py
index 0811b5adb2c..6067e8a7354 100644
--- a/src/tests/test_typeconversions.py
+++ b/src/tests/test_typeconversions.py
@@ -141,6 +141,76 @@ def test_bigint_conversions(selenium_module_scope, n):
         )
 
 
+@given(
+    n=strategies.one_of(
+        strategies.integers(min_value=2**53 + 1),
+        strategies.integers(max_value=-(2**53) - 1),
+    )
+)
+@settings(deadline=2000)
+def test_big_int_conversions2(selenium_module_scope, n):
+    with selenium_context_manager(selenium_module_scope) as selenium:
+        import json
+
+        print("n:", n, end=" ")
+        s = json.dumps(n)
+        selenium.run_js(
+            f"""
+            self.x_js = eval('{s}n'); // JSON.parse apparently doesn't work
+            pyodide.runPython(`
+                import json
+                x_py = json.loads({s!r})
+            `);
+            """
+        )
+        try:
+            assert selenium.run_js("""return pyodide.runPython('x_py') === x_js;""")
+            assert selenium.run(
+                """
+                from js import x_js
+                x_js == x_py
+                """
+            )
+        except Exception:
+            print("failed =(")
+            raise
+        else:
+            print("worked!!")
+
+
+@given(
+    n=strategies.integers(),
+    exp=strategies.integers(min_value=1, max_value=10),
+)
+def test_big_int_conversions3(selenium_module_scope, n, exp):
+    with selenium_context_manager(selenium_module_scope) as selenium:
+        import json
+
+        val = 2 ** (32 * exp) - n
+        s = json.dumps(val)
+        selenium.run_js(
+            f"""
+            self.x_js = eval('{s}n'); // JSON.parse apparently doesn't work
+            pyodide.runPython(`
+                import json
+                x_py = json.loads({s!r})
+            `);
+            """
+        )
+        [x1, x2] = selenium.run_js(
+            """return [pyodide.runPython('x_py').toString(), x_js.toString()]"""
+        )
+        assert x1 == x2
+        [x1, x2] = selenium.run(
+            """
+            from js import x_js
+            from pyodide import to_js
+            to_js([str(x_js), str(x_py)])
+            """
+        )
+        assert x1 == x2
+
+
 # Generate an object of any type
 @pytest.mark.skip_refcount_check
 @pytest.mark.skip_pyproxy_check


Code review comments:
diff_hunk: @@ -52,7 +52,7 @@ export DBG_LDFLAGS_SOURCEMAPDEBUG=-gseparate-dwarf
 export DBGFLAGS=$(DBGFLAGS_NODEBUG)
 
 # Include debug symbols but no source maps (most useful)
-#export DBGFLAGS=$(DBGFLAGS_WASMDEBUG)
+export DBGFLAGS=$(DBGFLAGS_WASMDEBUG)
path: Makefile.envs
text: @user1:
Needs reverting.