From 8f3f6bda2005a1324dafdfb8a35f423e9691472d Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Mon, 12 Apr 2021 03:10:27 -0400 Subject: [PATCH] Fix numpy in chromium v89 (#1449) --- packages/numpy/meta.yaml | 1 + .../fix-i32-load-memory-access-oob.patch | 115 ++++++++++++++++++ 2 files changed, 116 insertions(+) create mode 100644 packages/numpy/patches/fix-i32-load-memory-access-oob.patch diff --git a/packages/numpy/meta.yaml b/packages/numpy/meta.yaml index 2bd617e66..710132dce 100644 --- a/packages/numpy/meta.yaml +++ b/packages/numpy/meta.yaml @@ -7,6 +7,7 @@ source: sha256: 16507ba6617f62ae3c6ab1725ae6f550331025d4d9a369b83f6d5a470446c342 patches: + - patches/fix-i32-load-memory-access-oob.patch - patches/add-emscripten-cpu.patch - patches/disable-maybe-uninitialized.patch - patches/dont-include-execinfo.patch diff --git a/packages/numpy/patches/fix-i32-load-memory-access-oob.patch b/packages/numpy/patches/fix-i32-load-memory-access-oob.patch new file mode 100644 index 000000000..71017ec1a --- /dev/null +++ b/packages/numpy/patches/fix-i32-load-memory-access-oob.patch @@ -0,0 +1,115 @@ +From 6cf164b6a1a7cc151aa52ab5bb9e20a95a6a5e67 Mon Sep 17 00:00:00 2001 +From: Hood +Date: Fri, 2 Apr 2021 14:24:24 -0700 +Subject: [PATCH] Fix "load" memory access out of bounds on Chrome v89 #1384 + +Resolves https://github.com/pyodide/pyodide/issues/1384. + +There seems to be a bug in the v8 wasm runtime. The failure occurs in the numpy +definition of PyArray_IntpFromIndexSequence in the following code: +```C +vals[i] = PyArray_PyIntAsIntp(op); // line 958 +Py_DECREF(op); // line 959 +if(vals[i] == -1) { // line 960, fails here "RuntimeError: memory access out of bounds" + +} +``` +This is compiled to the following: +```wat +;; First calculate &vals[i]. vals is an array of i32, so this is vals + 4*i. +;; line 958: vals[i] = PyArray_PyIntAsIntp(op); +local.get $var1 ;; vals +local.get $var3 ;; i +i32.const 2 +i32.shl ;; i << 2 +i32.add ;; &vals[i] == vals + 4*i +local.tee $var7 ;; v7 = &vals[i], leave &vals[i] on stack +(; Snipped out argument setup and inlined PyArray_PyIntAsIntp ;) +call $PyArray_PyIntAsIntp +i32.store ;; vals[i] = PyArray_PyIntAsIntp(op) ;; Store to vals[i] +;; line 959: Py_DECREF(op); +(; Snipped out Py_DECREF(op) ;) +block $label1 + ;; line 960: if(vals[i] == -1) { + local.get $var7 ;; &vals[i] + i32.load ;; Load vals[i] back <== Fails here "RuntimeError: memory access out of bounds" + i32.const -1 + i32.ne ;; vals[i] != -1 + br_if $label1 +``` + +It doesn't make a whole lot of sense that it fails here, we just succesfully +stored to vals[i] so why does it fail when we load back? Unforunately the bug +disappears when devtools are open making it very difficult to assess what state +the wasm VM is in when the bug is triggered. + +The goal here was to refactor the source to remove the offending load +instruction. There are several options for how to change it, I chose the +following: +```C +int x = PyArray_PyIntAsIntp(op); // line 958 +Py_DECREF(op); // line 959 +if(x == -1) { // line 960 + +} +vals[i] = x; // line 971 +``` +Updated WAT looks like what you'd expect: +```wat + ;; line 958: int x = PyArray_PyIntAsIntp(op); + (; Snipped out argument setup and inlined PyArray_PyIntAsIntp ;) + call $PyArray_PyIntAsIntp + ;; Now instead of storing result into vals[i], we store it in a local variable + local.set $var6 + ;; line 959: Py_DECREF(op) + (; Snipped out Py_DECREF(op) ;) + ;; line 960: if(x == -1) + block $label1 + local.get $var6 ;; now check value of local variable + i32.const -1 + i32.ne + br_if $label1 + (; Snipped out body of conditional ;) + end $label1 + ;; line 971: vals[i] = x + local.get $var1 ;; vals + local.get $var3 ;; i + i32.const 2 + i32.shl ;; 4*i + i32.add ;; vals + 4*i + local.get $var6 ;; x + i32.store +``` +Indeed the offending `i32.load` is gone and so is the crash. + +--- + numpy/core/src/multiarray/conversion_utils.c | 5 +++-- + 1 file changed, 3 insertions(+), 2 deletions(-) + +diff --git a/numpy/core/src/multiarray/conversion_utils.c b/numpy/core/src/multiarray/conversion_utils.c +index 52cb58726..5467f4d78 100644 +--- a/numpy/core/src/multiarray/conversion_utils.c ++++ b/numpy/core/src/multiarray/conversion_utils.c +@@ -955,9 +955,9 @@ PyArray_IntpFromIndexSequence(PyObject *seq, npy_intp *vals, npy_intp maxvals) + return -1; + } + +- vals[i] = PyArray_PyIntAsIntp(op); ++ npy_intp x = PyArray_PyIntAsIntp(op); + Py_DECREF(op); +- if(vals[i] == -1) { ++ if(x == -1) { + err = PyErr_Occurred(); + if (err && + PyErr_GivenExceptionMatches(err, PyExc_OverflowError)) { +@@ -968,6 +968,7 @@ PyArray_IntpFromIndexSequence(PyObject *seq, npy_intp *vals, npy_intp maxvals) + return -1; + } + } ++ vals[i] = x; + } + } + return nd; +-- + +