Bug 925233 - OdinMonkey: fix neutering interaction with asm.js (r=sfink)
authorLuke Wagner <luke@mozilla.com>
Thu, 24 Oct 2013 09:00:04 -0500
changeset 165784 dc598f50a09a29c7d8999c6988af4a4545c708d3
parent 165783 6d4ff510c1179135cbda49e3568924bb7b18f0a2
child 165785 38414f29834c053dd4a85a8f74af88ed64e78b69
push id3066
push userakeybl@mozilla.com
push dateMon, 09 Dec 2013 19:58:46 +0000
treeherdermozilla-beta@a31a0dce83aa [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs925233
milestone27.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 925233 - OdinMonkey: fix neutering interaction with asm.js (r=sfink)
js/src/jit-test/tests/asm.js/testNeuter.js
js/src/jit/AsmJS.h
js/src/jit/AsmJSLink.cpp
js/src/jit/AsmJSModule.h
js/src/vm/ObjectImpl.h
js/src/vm/TypedArrayObject.cpp
js/src/vm/TypedArrayObject.h
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/asm.js/testNeuter.js
@@ -0,0 +1,74 @@
+load(libdir + "asm.js");
+
+function f(stdlib, foreign, buffer) {
+    "use asm";
+    var i32 = new stdlib.Int32Array(buffer);
+    function set(i,j) {
+        i=i|0;
+        j=j|0;
+        i32[i>>2] = j;
+    }
+    function get(i) {
+        i=i|0;
+        return i32[i>>2]|0
+    }
+    return {get:get, set:set}
+}
+if (isAsmJSCompilationAvailable())
+    assertEq(isAsmJSModule(f), true);
+
+var i32 = new Int32Array(1024);
+var buffer = i32.buffer;
+var {get, set} = f(this, null, buffer);
+if (isAsmJSCompilationAvailable())
+    assertEq(isAsmJSFunction(get) && isAsmJSFunction(set), true);
+
+set(4, 42);
+assertEq(get(4), 42);
+
+neuter(buffer);
+
+// These operations may throw internal errors
+try {
+    assertEq(get(4), 0);
+    set(0, 42);
+    assertEq(get(0), 0);
+} catch (e) {
+    assertEq(String(e).indexOf("InternalError"), 0);
+}
+
+function f2(stdlib, foreign, buffer) {
+    "use asm";
+    var i32 = new stdlib.Int32Array(buffer);
+    var ffi = foreign.ffi;
+    function inner(i) {
+        i=i|0;
+        ffi();
+        return i32[i>>2]|0
+    }
+    return inner
+}
+if (isAsmJSCompilationAvailable())
+    assertEq(isAsmJSModule(f2), true);
+
+var i32 = new Int32Array(1024);
+var buffer = i32.buffer;
+var threw = false;
+function ffi() {
+    try {
+        neuter(buffer);
+    } catch (e) {
+        assertEq(String(e).indexOf("InternalError"), 0);
+        threw = true;
+    }
+}
+var inner = f2(this, {ffi:ffi}, buffer);
+if (isAsmJSCompilationAvailable())
+    assertEq(isAsmJSFunction(inner), true);
+i32[2] = 13;
+var result = inner(8);
+if (threw)
+    assertEq(result, 13);
+else
+    assertEq(result, 0);
+
--- a/js/src/jit/AsmJS.h
+++ b/js/src/jit/AsmJS.h
@@ -55,16 +55,17 @@ class AsmJSActivation
     void *resumePC_;
 
   public:
     AsmJSActivation(JSContext *cx, AsmJSModule &module);
     ~AsmJSActivation();
 
     JSContext *cx() { return cx_; }
     AsmJSModule &module() const { return module_; }
+    AsmJSActivation *prev() const { return prev_; }
 
     // Read by JIT code:
     static unsigned offsetOfContext() { return offsetof(AsmJSActivation, cx_); }
     static unsigned offsetOfResumePC() { return offsetof(AsmJSActivation, resumePC_); }
 
     // Initialized by JIT code:
     static unsigned offsetOfErrorRejoinSP() { return offsetof(AsmJSActivation, errorRejoinSP_); }
 
--- a/js/src/jit/AsmJSLink.cpp
+++ b/js/src/jit/AsmJSLink.cpp
@@ -326,16 +326,25 @@ CallAsmJS(JSContext *cx, unsigned argc, 
     AsmJSModule &module = moduleObj->as<AsmJSModuleObject>().module();
 
     // An exported function points to the code as well as the exported
     // function's signature, which implies the dynamic coercions performed on
     // the arguments.
     unsigned exportIndex = callee->getExtendedSlot(ASM_EXPORT_INDEX_SLOT).toInt32();
     const AsmJSModule::ExportedFunction &func = module.exportedFunction(exportIndex);
 
+    // An asm.js module is specialized to its heap's base address and length
+    // which is normally immutable except for the neuter operation that occurs
+    // when an ArrayBuffer is transfered. Throw an internal error if we try to
+    // run with a neutered heap.
+    if (module.maybeHeapBufferObject() && module.maybeHeapBufferObject()->isNeutered()) {
+        js_ReportOverRecursed(cx);
+        return false;
+    }
+
     // The calling convention for an external call into asm.js is to pass an
     // array of 8-byte values where each value contains either a coerced int32
     // (in the low word) or double value, with the coercions specified by the
     // asm.js signature. The external entry point unpacks this array into the
     // system-ABI-specified registers and stack memory and then calls into the
     // internal entry point. The return value is stored in the first element of
     // the array (which, therefore, must have length >= 1).
 
--- a/js/src/jit/AsmJSModule.h
+++ b/js/src/jit/AsmJSModule.h
@@ -724,16 +724,20 @@ class AsmJSModule
     }
     bool isLinked() const {
         return linked_;
     }
     uint8_t *maybeHeap() const {
         JS_ASSERT(linked_);
         return heapDatum();
     }
+    ArrayBufferObject *maybeHeapBufferObject() const {
+        JS_ASSERT(linked_);
+        return maybeHeap_;
+    }
     size_t heapLength() const {
         JS_ASSERT(linked_);
         return maybeHeap_ ? maybeHeap_->byteLength() : 0;
     }
 
     void initGlobalArgumentName(PropertyName *n) {
         JS_ASSERT_IF(n, n->isTenured());
         globalArgumentName_ = n;
--- a/js/src/vm/ObjectImpl.h
+++ b/js/src/vm/ObjectImpl.h
@@ -750,29 +750,31 @@ ArraySetLength(typename ExecutionModeTra
  * Elements do not track property creation order, so enumerating the elements
  * of an object does not necessarily visit indexes in the order they were
  * created.
  */
 class ObjectElements
 {
   public:
     enum Flags {
-        CONVERT_DOUBLE_ELEMENTS = 0x1,
-        ASMJS_ARRAY_BUFFER = 0x2,
+        CONVERT_DOUBLE_ELEMENTS     = 0x1,
+        ASMJS_ARRAY_BUFFER          = 0x2,
+        NEUTERED_BUFFER             = 0x4,
 
         // Present only if these elements correspond to an array with
         // non-writable length; never present for non-arrays.
-        NONWRITABLE_ARRAY_LENGTH = 0x4
+        NONWRITABLE_ARRAY_LENGTH    = 0x8
     };
 
   private:
     friend class ::JSObject;
     friend class ObjectImpl;
     friend class ArrayObject;
     friend class ArrayBufferObject;
+    friend class TypedArrayObject;
     friend class Nursery;
 
     template <ExecutionMode mode>
     friend bool
     ArraySetLength(typename ExecutionModeTraits<mode>::ContextType cx,
                    Handle<ArrayObject*> obj, HandleId id,
                    unsigned attrs, HandleValue value, bool setterIsStrict);
 
@@ -812,16 +814,22 @@ class ObjectElements
         flags |= CONVERT_DOUBLE_ELEMENTS;
     }
     bool isAsmJSArrayBuffer() const {
         return flags & ASMJS_ARRAY_BUFFER;
     }
     void setIsAsmJSArrayBuffer() {
         flags |= ASMJS_ARRAY_BUFFER;
     }
+    bool isNeuteredBuffer() const {
+        return flags & NEUTERED_BUFFER;
+    }
+    void setIsNeuteredBuffer() {
+        flags |= NEUTERED_BUFFER;
+    }
     bool hasNonwritableArrayLength() const {
         return flags & NONWRITABLE_ARRAY_LENGTH;
     }
     void setNonwritableArrayLength() {
         flags |= NONWRITABLE_ARRAY_LENGTH;
     }
 
   public:
--- a/js/src/vm/TypedArrayObject.cpp
+++ b/js/src/vm/TypedArrayObject.cpp
@@ -26,16 +26,17 @@
 #ifdef XP_WIN
 # include "jswin.h"
 #endif
 #include "jswrapper.h"
 
 #include "gc/Barrier.h"
 #include "gc/Marking.h"
 #include "jit/AsmJS.h"
+#include "jit/AsmJSModule.h"
 #include "vm/GlobalObject.h"
 #include "vm/Interpreter.h"
 #include "vm/NumericConversions.h"
 #include "vm/WrapperObject.h"
 
 #include "jsatominlines.h"
 #include "jsinferinlines.h"
 #include "jsobjinlines.h"
@@ -325,37 +326,42 @@ InitViewList(ArrayBufferObject *obj, Arr
 
 static EncapsulatedPtr<ArrayBufferViewObject> &
 GetViewListRef(ArrayBufferObject *obj)
 {
     JS_ASSERT(obj->runtimeFromMainThread()->isHeapBusy());
     return reinterpret_cast<OldObjectRepresentationHack*>(obj->getElementsHeader())->views;
 }
 
-void
-ArrayBufferObject::neuterViews(JSContext *maybecx)
+bool
+ArrayBufferObject::neuterViews(JSContext *cx)
 {
     ArrayBufferViewObject *view;
     for (view = GetViewList(this); view; view = view->nextView()) {
         view->neuter();
 
         // Notify compiled jit code that the base pointer has moved.
-        if (maybecx)
-            MarkObjectStateChange(maybecx, view);
+        MarkObjectStateChange(cx, view);
     }
 
     // neuterAsmJSArrayBuffer adjusts state specific to the ArrayBuffer data
     // itself, but it only affects the behavior of views
-    if (isAsmJSArrayBuffer())
-        ArrayBufferObject::neuterAsmJSArrayBuffer(*this);
+    if (isAsmJSArrayBuffer()) {
+        if (!ArrayBufferObject::neuterAsmJSArrayBuffer(cx, *this))
+            return false;
+    }
+
+    return true;
 }
 
 void
 ArrayBufferObject::changeContents(JSContext *maybecx, ObjectElements *newHeader)
 {
+   JS_ASSERT(!isAsmJSArrayBuffer());
+
     // Grab out data before invalidating it.
     uint32_t byteLengthCopy = byteLength();
     uintptr_t oldDataPointer = uintptr_t(dataPointer());
     ArrayBufferViewObject *viewListHead = GetViewList(this);
 
     // Update all views.
     uintptr_t newDataPointer = uintptr_t(newHeader->elements());
     for (ArrayBufferViewObject *view = viewListHead; view; view = view->nextView()) {
@@ -386,16 +392,18 @@ ArrayBufferObject::neuter(JSContext *cx)
         changeContents(cx, ObjectElements::fromElements(fixedElements()));
 
         FreeOp fop(cx->runtime(), false);
         fop.free_(oldHeader);
     }
 
     uint32_t byteLen = 0;
     updateElementsHeader(getElementsHeader(), byteLen);
+
+    getElementsHeader()->setIsNeuteredBuffer();
 }
 
 bool
 ArrayBufferObject::copyData(JSContext *maybecx)
 {
     ObjectElements *newHeader = AllocateArrayBufferContents(maybecx, byteLength(), dataPointer());
     if (!newHeader)
         return false;
@@ -518,32 +526,16 @@ ArrayBufferObject::releaseAsmJSArrayBuff
     uint8_t *p = buffer.dataPointer() - AsmJSPageSize ;
     JS_ASSERT(uintptr_t(p) % AsmJSPageSize == 0);
 # ifdef XP_WIN
     VirtualFree(p, 0, MEM_RELEASE);
 # else
     munmap(p, AsmJSMappedSize);
 # endif
 }
-
-void
-ArrayBufferObject::neuterAsmJSArrayBuffer(ArrayBufferObject &buffer)
-{
-    // Protect all the pages so that any read/write will generate a fault which
-    // the AsmJSMemoryFaultHandler will turn into the expected result value.
-    JS_ASSERT(buffer.isAsmJSArrayBuffer());
-    JS_ASSERT(buffer.byteLength() % AsmJSAllocationGranularity == 0);
-#ifdef XP_WIN
-    if (!VirtualAlloc(buffer.dataPointer(), buffer.byteLength(), MEM_RESERVE, PAGE_NOACCESS))
-        MOZ_CRASH();
-#else
-    if (mprotect(buffer.dataPointer(), buffer.byteLength(), PROT_NONE))
-        MOZ_CRASH();
-#endif
-}
 #else  /* defined(JS_ION) && defined(JS_CPU_X64) */
 bool
 ArrayBufferObject::prepareForAsmJS(JSContext *cx, Handle<ArrayBufferObject*> buffer)
 {
     if (buffer->isAsmJSArrayBuffer())
         return true;
 
     if (!buffer->copyData(cx))
@@ -554,23 +546,32 @@ ArrayBufferObject::prepareForAsmJS(JSCon
     return true;
 }
 
 void
 ArrayBufferObject::releaseAsmJSArrayBuffer(FreeOp *fop, JSObject *obj)
 {
     fop->free_(obj->as<ArrayBufferObject>().getElementsHeader());
 }
-
-void
-ArrayBufferObject::neuterAsmJSArrayBuffer(ArrayBufferObject &buffer)
+#endif
+
+bool
+ArrayBufferObject::neuterAsmJSArrayBuffer(JSContext *cx, ArrayBufferObject &buffer)
 {
-    // TODO: be ever-so-slightly unsound (but safe) for now.
+    AsmJSActivation *act = cx->mainThread().asmJSActivationStackFromOwnerThread();
+    for (; act; act = act->prev()) {
+        if (act->module().maybeHeapBufferObject() == &buffer)
+            break;
+    }
+    if (!act)
+        return true;
+
+    js_ReportOverRecursed(cx);
+    return false;
 }
-#endif
 
 void
 ArrayBufferObject::addView(ArrayBufferViewObject *view)
 {
     // This view should never have been associated with a buffer before
     JS_ASSERT(view->bufferLink() == UNSET_BUFFER_LINK);
 
     // Note that pre-barriers are not needed here because either the list was
@@ -674,17 +675,18 @@ ArrayBufferObject::stealContents(JSConte
     ObjectElements *header = reinterpret_cast<ObjectElements*>(buffer.getTransferableContents(cx, &own));
     if (!header)
         return false;
     *contents = header;
     *data = reinterpret_cast<uint8_t *>(header + 1);
 
     // Neuter the views, which may also mprotect(PROT_NONE) the buffer. So do
     // it after copying out the data.
-    buffer.neuterViews(cx);
+    if (!buffer.neuterViews(cx))
+        return false;
 
     if (!own) {
         // If header has dynamically allocated elements, revert it back to
         // fixed-element storage before neutering it.
         buffer.changeContents(cx, ObjectElements::fromElements(buffer.fixedElements()));
     }
     buffer.neuter(cx);
 
@@ -4019,22 +4021,24 @@ JS_GetArrayBufferData(JSObject *obj)
     if (!obj)
         return nullptr;
     ArrayBufferObject &buffer = obj->as<ArrayBufferObject>();
     if (!buffer.ensureNonInline(NULL))
         return nullptr;
     return buffer.dataPointer();
 }
 
-JS_FRIEND_API(void)
-JS_NeuterArrayBuffer(JSObject *obj, JSContext *cx)
+JS_FRIEND_API(bool)
+JS_NeuterArrayBuffer(JSContext *cx, JSObject *obj)
 {
     ArrayBufferObject &buffer = obj->as<ArrayBufferObject>();
-    buffer.neuterViews(cx);
+    if (!buffer.neuterViews(cx))
+        return false;
     buffer.neuter(cx);
+    return true;
 }
 
 JS_FRIEND_API(JSObject *)
 JS_NewArrayBuffer(JSContext *cx, uint32_t nbytes)
 {
     JS_ASSERT(nbytes <= INT32_MAX);
     return ArrayBufferObject::create(cx, nbytes);
 }
--- a/js/src/vm/TypedArrayObject.h
+++ b/js/src/vm/TypedArrayObject.h
@@ -193,17 +193,17 @@ class ArrayBufferObject : public JSObjec
      * the returned contents (which is the case for inline or asm.js buffers),
      * and false if the ArrayBuffer still owns the pointer.
      */
     ObjectElements *getTransferableContents(JSContext *maybecx, bool *callerOwns);
 
     /*
      * Neuter all views of an ArrayBuffer.
      */
-    void neuterViews(JSContext *maybecx);
+    bool neuterViews(JSContext *cx);
 
     inline uint8_t * dataPointer() const {
         return (uint8_t *) elements;
     }
 
     /*
      * Discard the ArrayBuffer contents. For asm.js buffers, at least, should
      * be called after neuterViews().
@@ -216,18 +216,21 @@ class ArrayBufferObject : public JSObjec
      */
     bool hasData() const {
         return getClass() == &class_;
     }
 
     bool isAsmJSArrayBuffer() const {
         return getElementsHeader()->isAsmJSArrayBuffer();
     }
+    bool isNeutered() const {
+        return getElementsHeader()->isNeuteredBuffer();
+    }
     static bool prepareForAsmJS(JSContext *cx, Handle<ArrayBufferObject*> buffer);
-    static void neuterAsmJSArrayBuffer(ArrayBufferObject &buffer);
+    static bool neuterAsmJSArrayBuffer(JSContext *cx, ArrayBufferObject &buffer);
     static void releaseAsmJSArrayBuffer(FreeOp *fop, JSObject *obj);
 };
 
 /*
  * ArrayBufferViewObject
  *
  * Common definitions shared by all ArrayBufferViews.
  */