Bug 1081277 - OdinMonkey: move detachment logic into AsmJSModule (r=bbouvier,sfink)
authorLuke Wagner <luke@mozilla.com>
Tue, 14 Oct 2014 10:59:37 -0500
changeset 210617 20aa7722f330ac3a58c418fa2ebfb969ca4b7847
parent 210616 88497210b950bb6e77d603fc5677eecbfef9627f
child 210618 62185aa0fd863e426eadac63dbcfaf73e2fec2e4
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersbbouvier, sfink
bugs1081277
milestone36.0a1
Bug 1081277 - OdinMonkey: move detachment logic into AsmJSModule (r=bbouvier,sfink)
js/src/asmjs/AsmJSLink.cpp
js/src/asmjs/AsmJSModule.cpp
js/src/asmjs/AsmJSModule.h
js/src/jit-test/tests/asm.js/testNeuter.js
js/src/jit-test/tests/asm.js/testResize.js
js/src/vm/ArrayBufferObject.cpp
js/src/vm/ArrayBufferObject.h
--- a/js/src/asmjs/AsmJSLink.cpp
+++ b/js/src/asmjs/AsmJSLink.cpp
@@ -582,16 +582,21 @@ ChangeHeap(JSContext *cx, AsmJSModule &m
     if (heapLength & module.heapLengthMask() ||
         heapLength < module.minHeapLength() ||
         heapLength > module.maxHeapLength())
     {
         args.rval().set(BooleanValue(false));
         return true;
     }
 
+    if (!module.hasArrayView()) {
+        args.rval().set(BooleanValue(true));
+        return true;
+    }
+
     MOZ_ASSERT(IsValidAsmJSHeapLength(heapLength));
     MOZ_ASSERT(!IsDeprecatedAsmJSHeapLength(heapLength));
 
     if (!ArrayBufferObject::prepareForAsmJS(cx, newBuffer, module.usesSignalHandlersForOOB()))
         return false;
 
     args.rval().set(BooleanValue(module.changeHeap(newBuffer, cx)));
     return true;
@@ -684,25 +689,24 @@ CallAsmJS(JSContext *cx, unsigned argc, 
             if (!ToSimdConstant<Float32x4>(cx, v, &simd))
                 return false;
             memcpy(&coercedArgs[i], simd.asFloat32x4(), Simd128DataSize);
             break;
           }
         }
     }
 
-    // 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're
-    // about to run with a neutered heap.
-    if (module.maybeHeapBufferObject() &&
-        module.maybeHeapBufferObject()->is<ArrayBufferObject>() &&
-        module.maybeHeapBufferObject()->as<ArrayBufferObject>().isNeutered())
-    {
-        js_ReportOverRecursed(cx);
+    // The correct way to handle this situation would be to allocate a new range
+    // of PROT_NONE memory and module.changeHeap to this memory. That would
+    // cause every access to take the out-of-bounds signal-handler path which
+    // does the right thing. For now, just throw an out-of-memory exception
+    // since these can technically pop out anywhere and the full fix may
+    // actually OOM when trying to allocate the PROT_NONE memory.
+    if (module.hasDetachedHeap()) {
+        JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_OUT_OF_MEMORY);
         return false;
     }
 
     {
         // Push an AsmJSActivation to describe the asm.js frames we're about to
         // push when running this module. Additionally, push a JitActivation so
         // that the optimized asm.js-to-Ion FFI call path (which we want to be
         // very fast) can avoid doing so. The JitActivation is marked as
--- a/js/src/asmjs/AsmJSModule.cpp
+++ b/js/src/asmjs/AsmJSModule.cpp
@@ -49,16 +49,46 @@ using namespace js;
 using namespace jit;
 using namespace frontend;
 using mozilla::BinarySearch;
 using mozilla::PodCopy;
 using mozilla::PodEqual;
 using mozilla::Compression::LZ4;
 using mozilla::Swap;
 
+// At any time, the executable code of an asm.js module can be protected (as
+// part of RequestInterruptForAsmJSCode). When we touch the executable outside
+// of executing it (which the AsmJSFaultHandler will correctly handle), we need
+// to guard against this by unprotecting the code (if it has been protected) and
+// preventing it from being protected while we are touching it.
+class AutoUnprotectCode
+{
+    JSRuntime *rt_;
+    JSRuntime::AutoLockForInterrupt lock_;
+    const AsmJSModule &module_;
+    const bool protectedBefore_;
+
+  public:
+    AutoUnprotectCode(JSContext *cx, const AsmJSModule &module)
+      : rt_(cx->runtime()),
+        lock_(rt_),
+        module_(module),
+        protectedBefore_(module_.codeIsProtected(rt_))
+    {
+        if (protectedBefore_)
+            module_.unprotectCode(rt_);
+    }
+
+    ~AutoUnprotectCode()
+    {
+        if (protectedBefore_)
+            module_.protectCode(rt_);
+    }
+};
+
 static uint8_t *
 AllocateExecutableMemory(ExclusiveContext *cx, size_t bytes)
 {
 #ifdef XP_WIN
     unsigned permissions = PAGE_EXECUTE_READWRITE;
 #else
     unsigned permissions = PROT_READ | PROT_WRITE | PROT_EXEC;
 #endif
@@ -839,16 +869,45 @@ AsmJSModule::restoreToInitialState(Array
                                                PatchedImmPtr(originalValue));
         }
     }
 #endif
 
     restoreHeapToInitialState(maybePrevBuffer);
 }
 
+bool
+AsmJSModule::detachHeap(JSContext *cx)
+{
+    MOZ_ASSERT(isDynamicallyLinked());
+    MOZ_ASSERT(maybeHeap_);
+
+    AutoUnprotectCode auc(cx, *this);
+    restoreHeapToInitialState(maybeHeap_);
+
+    MOZ_ASSERT(hasDetachedHeap());
+    return true;
+}
+
+bool
+js::OnDetachAsmJSArrayBuffer(JSContext *cx, Handle<ArrayBufferObject*> buffer)
+{
+    for (AsmJSModule *m = cx->runtime()->linkedAsmJSModules; m; m = m->nextLinked()) {
+        if (buffer == m->maybeHeapBufferObject()) {
+            if (m->active()) {
+                JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_OUT_OF_MEMORY);
+                return false;
+            }
+            if (!m->detachHeap(cx))
+                return false;
+        }
+    }
+    return true;
+}
+
 static void
 AsmJSModuleObject_finalize(FreeOp *fop, JSObject *obj)
 {
     fop->delete_(&obj->as<AsmJSModuleObject>().module());
 }
 
 static void
 AsmJSModuleObject_trace(JSTracer *trc, JSObject *obj)
@@ -1480,46 +1539,16 @@ AsmJSModule::deserialize(ExclusiveContex
 #endif
     (cursor = staticLinkData_.deserialize(cx, cursor));
 
     loadedFromCache_ = true;
 
     return cursor;
 }
 
-// At any time, the executable code of an asm.js module can be protected (as
-// part of RequestInterruptForAsmJSCode). When we touch the executable outside
-// of executing it (which the AsmJSFaultHandler will correctly handle), we need
-// to guard against this by unprotecting the code (if it has been protected) and
-// preventing it from being protected while we are touching it.
-class AutoUnprotectCode
-{
-    JSRuntime *rt_;
-    JSRuntime::AutoLockForInterrupt lock_;
-    const AsmJSModule &module_;
-    const bool protectedBefore_;
-
-  public:
-    AutoUnprotectCode(JSContext *cx, const AsmJSModule &module)
-      : rt_(cx->runtime()),
-        lock_(rt_),
-        module_(module),
-        protectedBefore_(module_.codeIsProtected(rt_))
-    {
-        if (protectedBefore_)
-            module_.unprotectCode(rt_);
-    }
-
-    ~AutoUnprotectCode()
-    {
-        if (protectedBefore_)
-            module_.protectCode(rt_);
-    }
-};
-
 bool
 AsmJSModule::clone(JSContext *cx, ScopedJSDeletePtr<AsmJSModule> *moduleOut) const
 {
     AutoUnprotectCode auc(cx, *this);
 
     *moduleOut = cx->new_<AsmJSModule>(scriptSource_, srcStart_, srcBodyStart_, pod.strict_,
                                        pod.usesSignalHandlers_);
     if (!*moduleOut)
@@ -1564,16 +1593,18 @@ AsmJSModule::clone(JSContext *cx, Scoped
 
     out.restoreToInitialState(maybeHeap_, code_, cx);
     return true;
 }
 
 bool
 AsmJSModule::changeHeap(Handle<ArrayBufferObject*> newHeap, JSContext *cx)
 {
+    MOZ_ASSERT(hasArrayView());
+
     // Content JS should not be able to run (and change heap) from within an
     // interrupt callback, but in case it does, fail to change heap. Otherwise,
     // the heap can change at every single instruction which would prevent
     // future optimizations like heap-base hoisting.
     if (interrupted_)
         return false;
 
     AutoUnprotectCode auc(cx, *this);
--- a/js/src/asmjs/AsmJSModule.h
+++ b/js/src/asmjs/AsmJSModule.h
@@ -825,16 +825,20 @@ class AsmJSModule
     bool                                  loadedFromCache_;
     bool                                  profilingEnabled_;
     bool                                  interrupted_;
 
     // This field is accessed concurrently when requesting an interrupt.
     // Access must be synchronized via the runtime's interrupt lock.
     mutable bool                          codeIsProtected_;
 
+    void restoreHeapToInitialState(ArrayBufferObjectMaybeShared *maybePrevBuffer);
+    void restoreToInitialState(ArrayBufferObjectMaybeShared *maybePrevBuffer, uint8_t *prevCode,
+                               ExclusiveContext *cx);
+
   public:
     explicit AsmJSModule(ScriptSource *scriptSource, uint32_t srcStart, uint32_t srcBodyStart,
                          bool strict, bool canUseSignalHandlers);
     void trace(JSTracer *trc);
     ~AsmJSModule();
 
     // An AsmJSModule transitions monotonically through these states:
     bool isFinishedWithModulePrologue() const { return pod.funcPtrTableAndExitBytes_ != SIZE_MAX; }
@@ -1461,30 +1465,32 @@ class AsmJSModule
         prevLinked_ = &rt->linkedAsmJSModules;
         if (nextLinked_)
             nextLinked_->prevLinked_ = &nextLinked_;
         rt->linkedAsmJSModules = this;
         MOZ_ASSERT(isDynamicallyLinked());
     }
 
     void initHeap(Handle<ArrayBufferObjectMaybeShared*> heap, JSContext *cx);
-    void restoreHeapToInitialState(ArrayBufferObjectMaybeShared *maybePrevBuffer);
-    void restoreToInitialState(ArrayBufferObjectMaybeShared *maybePrevBuffer,
-                               uint8_t *prevCode,
-                               ExclusiveContext *cx);
+    bool changeHeap(Handle<ArrayBufferObject*> newHeap, JSContext *cx);
+    bool detachHeap(JSContext *cx);
+
     bool clone(JSContext *cx, ScopedJSDeletePtr<AsmJSModule> *moduleOut) const;
-    bool changeHeap(Handle<ArrayBufferObject*> newHeap, JSContext *cx);
 
     /*************************************************************************/
     // Functions that can be called after dynamic linking succeeds:
 
     AsmJSModule *nextLinked() const {
         MOZ_ASSERT(isDynamicallyLinked());
         return nextLinked_;
     }
+    bool hasDetachedHeap() const {
+        MOZ_ASSERT(isDynamicallyLinked());
+        return hasArrayView() && !heapDatum();
+    }
     CodePtr entryTrampoline(const ExportedFunction &func) const {
         MOZ_ASSERT(isDynamicallyLinked());
         MOZ_ASSERT(!func.isChangeHeap());
         return JS_DATA_TO_FUNC_PTR(CodePtr, code_ + func.pod.codeOffset_);
     }
     uint8_t *interruptExit() const {
         MOZ_ASSERT(isDynamicallyLinked());
         return interruptExit_;
@@ -1529,16 +1535,20 @@ StoreAsmJSModuleInCache(AsmJSParser &par
 // return value indicates whether or not an error was encountered, not whether
 // there was a cache hit.
 extern bool
 LookupAsmJSModuleInCache(ExclusiveContext *cx,
                          AsmJSParser &parser,
                          ScopedJSDeletePtr<AsmJSModule> *module,
                          ScopedJSFreePtr<char> *compilationTimeReport);
 
+// This function must be called for every detached ArrayBuffer.
+extern bool
+OnDetachAsmJSArrayBuffer(JSContext *cx, Handle<ArrayBufferObject*> buffer);
+
 // An AsmJSModuleObject is an internal implementation object (i.e., not exposed
 // directly to user script) which manages the lifetime of an AsmJSModule. A
 // JSObject is necessary since we want LinkAsmJS/CallAsmJS JSFunctions to be
 // able to point to their module via their extended slots.
 class AsmJSModuleObject : public NativeObject
 {
     static const unsigned MODULE_SLOT = 0;
 
--- a/js/src/jit-test/tests/asm.js/testNeuter.js
+++ b/js/src/jit-test/tests/asm.js/testNeuter.js
@@ -24,17 +24,17 @@ if (isAsmJSCompilationAvailable())
     assertEq(isAsmJSFunction(get) && isAsmJSFunction(set), true);
 
 set(4, 42);
 assertEq(get(4), 42);
 
 neuter(buffer, "change-data");
 neuter(buffer, "same-data");
 
-// These operations may throw internal errors
+// These operations may throw errors
 try {
     assertEq(get(4), 0);
     set(0, 42);
     assertEq(get(0), 0);
 } catch (e) {
     assertEq(String(e).indexOf("InternalError"), 0);
 }
 
--- a/js/src/jit-test/tests/asm.js/testResize.js
+++ b/js/src/jit-test/tests/asm.js/testResize.js
@@ -231,26 +231,31 @@ set(2*BUF_CHANGE_MIN, 262);
 assertEq(get(2*BUF_CHANGE_MIN), 0);
 changeHeap(buf1);
 assertEq(get(0), 42);
 assertEq(get(4), 13);
 set(BUF_CHANGE_MIN, 262);
 assertEq(get(BUF_CHANGE_MIN), 0);
 
 var buf1 = new ArrayBuffer(BUF_CHANGE_MIN);
-new Int32Array(buf1)[0] = 13;
 var buf2 = new ArrayBuffer(BUF_CHANGE_MIN);
-new Int32Array(buf2)[0] = 42;
-
 var m = asmCompile('glob', 'ffis', 'b', USE_ASM +
                    `var len=glob.byteLength;
                     function ch(b2) { if(len(b2) & 0xffffff || len(b2) <= 0xffffff || len(b2) > 0x80000000) return false; b=b2; return true }
                     return ch`);
 var changeHeap = asmLink(m, this, null, buf1);
-changeHeap(buf2);
+assertEq(changeHeap(buf2), true);
+neuter(buf2, "change-data");
+assertEq(changeHeap(buf1), true);
+neuter(buf1, "change-data");
+
+var buf1 = new ArrayBuffer(BUF_CHANGE_MIN);
+new Int32Array(buf1)[0] = 13;
+var buf2 = new ArrayBuffer(BUF_CHANGE_MIN);
+new Int32Array(buf2)[0] = 42;
 
 // Tests for changing heap during an FFI:
 
 // Set the warmup to '2' so we can hit both interp and ion FFI exits
 setJitCompilerOption("ion.warmup.trigger", 2);
 setJitCompilerOption("baseline.warmup.trigger", 0);
 setJitCompilerOption("offthread-compilation.enable", 0);
 
--- a/js/src/vm/ArrayBufferObject.cpp
+++ b/js/src/vm/ArrayBufferObject.cpp
@@ -272,42 +272,32 @@ AllocateArrayBufferContents(JSContext *c
 {
     uint8_t *p = cx->runtime()->pod_callocCanGC<uint8_t>(nbytes);
     if (!p)
         js_ReportOutOfMemory(cx);
 
     return ArrayBufferObject::BufferContents::create<ArrayBufferObject::PLAIN_BUFFER>(p);
 }
 
-bool
-ArrayBufferObject::canNeuter(JSContext *cx)
-{
-    if (isAsmJSArrayBuffer()) {
-        if (!ArrayBufferObject::canNeuterAsmJSArrayBuffer(cx, *this))
-            return false;
-    }
-
-    return true;
-}
-
 void
 ArrayBufferObject::neuterView(JSContext *cx, ArrayBufferViewObject *view,
                               BufferContents newContents)
 {
     view->neuter(newContents.data());
 
     // Notify compiled jit code that the base pointer has moved.
     MarkObjectStateChange(cx, view);
 }
 
-/* static */ void
+/* static */ bool
 ArrayBufferObject::neuter(JSContext *cx, Handle<ArrayBufferObject*> buffer,
                           BufferContents newContents)
 {
-    MOZ_ASSERT(buffer->canNeuter(cx));
+    if (buffer->isAsmJSArrayBuffer() && !OnDetachAsmJSArrayBuffer(cx, buffer))
+        return false;
 
     // Neuter all views on the buffer, clear out the list of views and the
     // buffer's data.
 
     if (InnerViewTable::ViewVector *views = cx->compartment()->innerViews.maybeViewsUnbarriered(buffer)) {
         for (size_t i = 0; i < views->length(); i++)
             buffer->neuterView(cx, (*views)[i], newContents);
         cx->compartment()->innerViews.removeViews(buffer);
@@ -316,16 +306,17 @@ ArrayBufferObject::neuter(JSContext *cx,
         buffer->neuterView(cx, buffer->firstView(), newContents);
     buffer->setFirstView(nullptr);
 
     if (newContents.data() != buffer->dataPointer())
         buffer->setNewOwnedData(cx->runtime()->defaultFreeOp(), newContents);
 
     buffer->setByteLength(0);
     buffer->setIsNeutered();
+    return true;
 }
 
 void
 ArrayBufferObject::setNewOwnedData(FreeOp* fop, BufferContents newContents)
 {
     MOZ_ASSERT(!isAsmJSArrayBuffer());
 
     if (ownsData()) {
@@ -490,30 +481,16 @@ ArrayBufferObject::prepareForAsmJS(JSCon
 void
 ArrayBufferObject::releaseAsmJSArray(FreeOp *fop)
 {
     // See comment above.
     releaseAsmJSArrayNoSignals(fop);
 }
 #endif
 
-bool
-ArrayBufferObject::canNeuterAsmJSArrayBuffer(JSContext *cx, ArrayBufferObject &buffer)
-{
-    AsmJSActivation *act = cx->mainThread().asmJSActivationStack();
-    for (; act; act = act->prevAsmJS()) {
-        if (act->module().maybeHeapBufferObject() == &buffer)
-            break;
-    }
-    if (!act)
-        return true;
-
-    return false;
-}
-
 ArrayBufferObject::BufferContents
 ArrayBufferObject::createMappedContents(int fd, size_t offset, size_t length)
 {
     void *data = AllocateMappedContent(fd, offset, length, ARRAY_BUFFER_ALIGNMENT);
     return BufferContents::create<MAPPED_BUFFER>(data);
 }
 
 void
@@ -703,39 +680,40 @@ ArrayBufferObject::createDataViewForThis
 }
 
 /* static */ ArrayBufferObject::BufferContents
 ArrayBufferObject::stealContents(JSContext *cx, Handle<ArrayBufferObject*> buffer,
                                  bool hasStealableContents)
 {
     MOZ_ASSERT_IF(hasStealableContents, buffer->hasStealableContents());
 
-    if (!buffer->canNeuter(cx)) {
-        js_ReportOverRecursed(cx);
-        return BufferContents::createUnowned(nullptr);
-    }
-
     BufferContents oldContents(buffer->dataPointer(), buffer->bufferKind());
     BufferContents newContents = AllocateArrayBufferContents(cx, buffer->byteLength());
     if (!newContents)
         return BufferContents::createUnowned(nullptr);
 
     if (hasStealableContents) {
         // Return the old contents and give the neutered buffer a pointer to
         // freshly allocated memory that we will never write to and should
         // never get committed.
         buffer->setOwnsData(DoesntOwnData);
-        ArrayBufferObject::neuter(cx, buffer, newContents);
+        if (!ArrayBufferObject::neuter(cx, buffer, newContents)) {
+            js_free(newContents.data());
+            return BufferContents::createUnowned(nullptr);
+        }
         return oldContents;
     }
 
     // Create a new chunk of memory to return since we cannot steal the
     // existing contents away from the buffer.
     memcpy(newContents.data(), oldContents.data(), buffer->byteLength());
-    ArrayBufferObject::neuter(cx, buffer, oldContents);
+    if (!ArrayBufferObject::neuter(cx, buffer, oldContents)) {
+        js_free(newContents.data());
+        return BufferContents::createUnowned(nullptr);
+    }
     return newContents;
 }
 
 /* static */ void
 ArrayBufferObject::addSizeOfExcludingThis(JSObject *obj, mozilla::MallocSizeOf mallocSizeOf,
                                           JS::ClassInfo *info)
 {
     ArrayBufferObject &buffer = AsArrayBuffer(obj);
@@ -1063,29 +1041,28 @@ JS_NeuterArrayBuffer(JSContext *cx, Hand
 {
     if (!obj->is<ArrayBufferObject>()) {
         JS_ReportError(cx, "ArrayBuffer object required");
         return false;
     }
 
     Rooted<ArrayBufferObject*> buffer(cx, &obj->as<ArrayBufferObject>());
 
-    if (!buffer->canNeuter(cx)) {
-        js_ReportOverRecursed(cx);
-        return false;
-    }
-
     if (changeData == ChangeData && buffer->hasStealableContents()) {
         ArrayBufferObject::BufferContents newContents =
             AllocateArrayBufferContents(cx, buffer->byteLength());
         if (!newContents)
             return false;
-        ArrayBufferObject::neuter(cx, buffer, newContents);
+        if (!ArrayBufferObject::neuter(cx, buffer, newContents)) {
+            js_free(newContents.data());
+            return false;
+        }
     } else {
-        ArrayBufferObject::neuter(cx, buffer, buffer->contents());
+        if (!ArrayBufferObject::neuter(cx, buffer, buffer->contents()))
+            return false;
     }
 
     return true;
 }
 
 JS_FRIEND_API(bool)
 JS_IsNeuteredArrayBufferObject(JSObject *obj)
 {
--- a/js/src/vm/ArrayBufferObject.h
+++ b/js/src/vm/ArrayBufferObject.h
@@ -232,20 +232,19 @@ class ArrayBufferObject : public ArrayBu
     void changeContents(JSContext *cx, BufferContents newContents);
 
     /*
      * Ensure data is not stored inline in the object. Used when handing back a
      * GC-safe pointer.
      */
     static bool ensureNonInline(JSContext *cx, Handle<ArrayBufferObject*> buffer);
 
-    bool canNeuter(JSContext *cx);
-
     /* Neuter this buffer and all its views. */
-    static void neuter(JSContext *cx, Handle<ArrayBufferObject*> buffer, BufferContents newContents);
+    static MOZ_WARN_UNUSED_RESULT bool
+    neuter(JSContext *cx, Handle<ArrayBufferObject*> buffer, BufferContents newContents);
 
   private:
     void neuterView(JSContext *cx, ArrayBufferViewObject *view,
                     BufferContents newContents);
     void changeViewContents(JSContext *cx, ArrayBufferViewObject *view,
                             uint8_t *oldDataPointer, BufferContents newContents);
     void setFirstView(ArrayBufferViewObject *view);
 
@@ -274,17 +273,16 @@ class ArrayBufferObject : public ArrayBu
     BufferKind bufferKind() const { return BufferKind(flags() & BUFFER_KIND_MASK); }
     bool isAsmJSArrayBuffer() const { return flags() & ASMJS_BUFFER; }
     bool isMappedArrayBuffer() const { return flags() & MAPPED_BUFFER; }
     bool isNeutered() const { return flags() & NEUTERED_BUFFER; }
 
     static bool prepareForAsmJS(JSContext *cx, Handle<ArrayBufferObject*> buffer,
                                 bool usesSignalHandlers);
     static bool prepareForAsmJSNoSignals(JSContext *cx, Handle<ArrayBufferObject*> buffer);
-    static bool canNeuterAsmJSArrayBuffer(JSContext *cx, ArrayBufferObject &buffer);
 
     static void finalize(FreeOp *fop, JSObject *obj);
 
     static BufferContents createMappedContents(int fd, size_t offset, size_t length);
 
     static size_t offsetOfFlagsSlot() {
         return getFixedSlotOffset(FLAGS_SLOT);
     }