Bug 1231624 - properly gate SAB+atomics in asm.js. r=luke
authorLars T Hansen <lhansen@mozilla.com>
Thu, 10 Dec 2015 10:46:53 -0500
changeset 276110 5b395639df4504eaf3305c2169b9e8394ac4dd09
parent 276109 7959433e79fd98b761d5fed5d5b894200f869848
child 276111 79dd74fd5b0655e84cb369b7be59fa18158465a5
push id69046
push userlhansen@mozilla.com
push dateThu, 10 Dec 2015 20:25:04 +0000
treeherdermozilla-inbound@5b395639df45 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs1231624
milestone45.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 1231624 - properly gate SAB+atomics in asm.js. r=luke
js/src/asmjs/AsmJSLink.cpp
js/src/asmjs/AsmJSModule.h
js/src/asmjs/AsmJSValidate.cpp
js/src/jit-test/tests/asm.js/gating.js
js/src/jit-test/tests/basic/bug984766.js
--- a/js/src/asmjs/AsmJSLink.cpp
+++ b/js/src/asmjs/AsmJSLink.cpp
@@ -548,17 +548,16 @@ DynamicallyLinkModule(JSContext* cx, con
             if (!ValidateGlobalVariable(cx, module, global, importVal))
                 return false;
             break;
           case AsmJSModule::Global::FFI:
             if (!ValidateFFI(cx, global, importVal, &ffis))
                 return false;
             break;
           case AsmJSModule::Global::ArrayView:
-          case AsmJSModule::Global::SharedArrayView:
           case AsmJSModule::Global::ArrayViewCtor:
             if (!ValidateArrayView(cx, global, globalVal))
                 return false;
             break;
           case AsmJSModule::Global::ByteLength:
             if (!ValidateByteLength(cx, globalVal))
                 return false;
             break;
--- a/js/src/asmjs/AsmJSModule.h
+++ b/js/src/asmjs/AsmJSModule.h
@@ -95,17 +95,17 @@ enum AsmJSSimdOperation
 //
 // NB: this means that AsmJSModule must be GC-safe.
 class AsmJSModule
 {
   public:
     class Global
     {
       public:
-        enum Which { Variable, FFI, ArrayView, ArrayViewCtor, SharedArrayView, MathBuiltinFunction,
+        enum Which { Variable, FFI, ArrayView, ArrayViewCtor, MathBuiltinFunction,
                      AtomicsBuiltinFunction, Constant, SimdCtor, SimdOperation, ByteLength };
         enum VarInitKind { InitConstant, InitImport };
         enum ConstantKind { GlobalConstant, MathConstant };
 
       private:
         struct Pod {
             Which which_;
             union {
@@ -184,27 +184,23 @@ class AsmJSModule
             MOZ_ASSERT(pod.which_ == FFI);
             return pod.u.ffiIndex_;
         }
         // When a view is created from an imported constructor:
         //   var I32 = stdlib.Int32Array;
         //   var i32 = new I32(buffer);
         // the second import has nothing to validate and thus has a null field.
         PropertyName* maybeViewName() const {
-            MOZ_ASSERT(pod.which_ == ArrayView || pod.which_ == SharedArrayView || pod.which_ == ArrayViewCtor);
+            MOZ_ASSERT(pod.which_ == ArrayView || pod.which_ == ArrayViewCtor);
             return name_;
         }
         Scalar::Type viewType() const {
-            MOZ_ASSERT(pod.which_ == ArrayView || pod.which_ == SharedArrayView || pod.which_ == ArrayViewCtor);
+            MOZ_ASSERT(pod.which_ == ArrayView || pod.which_ == ArrayViewCtor);
             return pod.u.viewType_;
         }
-        void makeViewShared() {
-            MOZ_ASSERT(pod.which_ == ArrayView);
-            pod.which_ = SharedArrayView;
-        }
         PropertyName* mathName() const {
             MOZ_ASSERT(pod.which_ == MathBuiltinFunction);
             return name_;
         }
         PropertyName* atomicsName() const {
             MOZ_ASSERT(pod.which_ == AtomicsBuiltinFunction);
             return name_;
         }
@@ -905,30 +901,28 @@ class AsmJSModule
     bool addFFI(PropertyName* field, uint32_t* ffiIndex) {
         MOZ_ASSERT(!isFinished());
         if (pod.numFFIs_ == UINT32_MAX)
             return false;
         Global g(Global::FFI, field);
         g.pod.u.ffiIndex_ = *ffiIndex = pod.numFFIs_++;
         return globals_.append(g);
     }
-    bool addArrayView(Scalar::Type vt, PropertyName* maybeField, bool isSharedView) {
+    bool addArrayView(Scalar::Type vt, PropertyName* maybeField) {
         MOZ_ASSERT(!isFinished());
-        MOZ_ASSERT(!pod.hasArrayView_ || (pod.isSharedView_ == isSharedView));
         pod.hasArrayView_ = true;
-        pod.isSharedView_ = isSharedView;
+        pod.isSharedView_ = false;
         Global g(Global::ArrayView, maybeField);
         g.pod.u.viewType_ = vt;
         return globals_.append(g);
     }
-    bool addArrayViewCtor(Scalar::Type vt, PropertyName* field, bool isSharedView) {
+    bool addArrayViewCtor(Scalar::Type vt, PropertyName* field) {
         MOZ_ASSERT(!isFinished());
         MOZ_ASSERT(field);
-        MOZ_ASSERT(!pod.isSharedView_ || isSharedView);
-        pod.isSharedView_ = isSharedView;
+        pod.isSharedView_ = false;
         Global g(Global::ArrayViewCtor, field);
         g.pod.u.viewType_ = vt;
         return globals_.append(g);
     }
     bool addByteLength() {
         MOZ_ASSERT(!isFinished());
         Global g(Global::ByteLength, nullptr);
         return globals_.append(g);
@@ -973,29 +967,19 @@ class AsmJSModule
         return globals_.append(g);
     }
     unsigned numGlobals() const {
         return globals_.length();
     }
     Global& global(unsigned i) {
         return globals_[i];
     }
-    bool isValidViewSharedness(bool shared) const {
-        if (pod.hasArrayView_)
-            return pod.isSharedView_ == shared;
-        return !pod.isSharedView_ || shared;
-    }
     void setViewsAreShared() {
         if (pod.hasArrayView_)
             pod.isSharedView_ = true;
-        for (size_t i=0 ; i < globals_.length() ; i++) {
-            Global& g = globals_[i];
-            if (g.which() == Global::ArrayView)
-                g.makeViewShared();
-        }
     }
 
     /*************************************************************************/
     // These functions are called while parsing/compiling function bodies:
 
     bool hasArrayView() const {
         return pod.hasArrayView_;
     }
--- a/js/src/asmjs/AsmJSValidate.cpp
+++ b/js/src/asmjs/AsmJSValidate.cpp
@@ -1009,17 +1009,16 @@ class MOZ_STACK_CLASS ModuleValidator
                 uint32_t globalDataOffset_;
                 NumLit literalValue_;
             } varOrConst;
             uint32_t funcIndex_;
             uint32_t funcPtrTableIndex_;
             uint32_t ffiIndex_;
             struct {
                 Scalar::Type viewType_;
-                bool isSharedView_;
             } viewInfo;
             AsmJSMathBuiltinFunction mathBuiltinFunc_;
             AsmJSAtomicsBuiltinFunction atomicsBuiltinFunc_;
             AsmJSSimdType simdCtorType_;
             struct {
                 AsmJSSimdType type_;
                 AsmJSSimdOperation which_;
             } simdOp;
@@ -1067,24 +1066,16 @@ class MOZ_STACK_CLASS ModuleValidator
         }
         bool isAnyArrayView() const {
             return which_ == ArrayView || which_ == ArrayViewCtor;
         }
         Scalar::Type viewType() const {
             MOZ_ASSERT(isAnyArrayView());
             return u.viewInfo.viewType_;
         }
-        bool viewIsSharedView() const {
-            MOZ_ASSERT(isAnyArrayView());
-            return u.viewInfo.isSharedView_;
-        }
-        void setViewIsSharedView() {
-            MOZ_ASSERT(isAnyArrayView());
-            u.viewInfo.isSharedView_ = true;
-        }
         bool isMathFunction() const {
             return which_ == MathBuiltinFunction;
         }
         AsmJSMathBuiltinFunction mathBuiltinFunction() const {
             MOZ_ASSERT(which_ == MathBuiltinFunction);
             return u.mathBuiltinFunc_;
         }
         bool isAtomicsFunction() const {
@@ -1397,28 +1388,26 @@ class MOZ_STACK_CLASS ModuleValidator
         Global::Which which = isConst ? Global::ConstantImport : Global::Variable;
         Global* global = validationLifo_.new_<Global>(which);
         if (!global)
             return false;
         global->u.varOrConst.globalDataOffset_ = globalDataOffset;
         global->u.varOrConst.type_ = Type::var(importType).which();
         return globals_.putNew(varName, global);
     }
-    bool addArrayView(PropertyName* varName, Scalar::Type vt, PropertyName* maybeField,
-                      bool isSharedView)
+    bool addArrayView(PropertyName* varName, Scalar::Type vt, PropertyName* maybeField)
     {
         if (!arrayViews_.append(ArrayView(varName, vt)))
             return false;
         Global* global = validationLifo_.new_<Global>(Global::ArrayView);
         if (!global)
             return false;
-        if (!module().addArrayView(vt, maybeField, isSharedView))
+        if (!module().addArrayView(vt, maybeField))
             return false;
         global->u.viewInfo.viewType_ = vt;
-        global->u.viewInfo.isSharedView_ = isSharedView;
         return globals_.putNew(varName, global);
     }
     bool addMathBuiltinFunction(PropertyName* varName, AsmJSMathBuiltinFunction func,
                                 PropertyName* fieldName)
     {
         if (!module().addMathBuiltinFunction(func, fieldName))
             return false;
         Global* global = validationLifo_.new_<Global>(Global::MathBuiltinFunction);
@@ -1492,24 +1481,23 @@ class MOZ_STACK_CLASS ModuleValidator
         module().addChangeHeap(mask, min, max);
         Global* global = validationLifo_.new_<Global>(Global::ChangeHeap);
         if (!global)
             return false;
         global->u.changeHeap.srcBegin_ = fn->pn_pos.begin;
         global->u.changeHeap.srcEnd_ = fn->pn_pos.end;
         return globals_.putNew(name, global);
     }
-    bool addArrayViewCtor(PropertyName* varName, Scalar::Type vt, PropertyName* fieldName, bool isSharedView) {
+    bool addArrayViewCtor(PropertyName* varName, Scalar::Type vt, PropertyName* fieldName) {
         Global* global = validationLifo_.new_<Global>(Global::ArrayViewCtor);
         if (!global)
             return false;
-        if (!module().addArrayViewCtor(vt, fieldName, isSharedView))
+        if (!module().addArrayViewCtor(vt, fieldName))
             return false;
         global->u.viewInfo.viewType_ = vt;
-        global->u.viewInfo.isSharedView_ = isSharedView;
         return globals_.putNew(varName, global);
     }
     bool addFFI(PropertyName* varName, PropertyName* field) {
         Global* global = validationLifo_.new_<Global>(Global::FFI);
         if (!global)
             return false;
         uint32_t index;
         if (!module().addFFI(field, &index))
@@ -1607,16 +1595,20 @@ class MOZ_STACK_CLASS ModuleValidator
     }
     bool tryRequireHeapLengthToBeAtLeast(uint32_t len) {
         return module().tryRequireHeapLengthToBeAtLeast(len);
     }
     uint32_t minHeapLength() const {
         return module().minHeapLength();
     }
 
+    bool usesSharedMemory() const {
+        return atomicsPresent_;
+    }
+
     // Error handling.
     bool hasAlreadyFailed() const {
         return !!errorString_;
     }
 
     bool failOffset(uint32_t offset, const char* str) {
         MOZ_ASSERT(!hasAlreadyFailed());
         MOZ_ASSERT(errorOffset_ == UINT32_MAX);
@@ -1735,24 +1727,18 @@ class MOZ_STACK_CLASS ModuleValidator
         if (SimdOperationNameMap::Ptr p = standardLibrarySimdOpNames_.lookup(name)) {
             *op = p->value();
             return true;
         }
         return false;
     }
 
     void startFunctionBodies() {
-        if (atomicsPresent_) {
-            for (GlobalMap::Range r = globals_.all() ; !r.empty() ; r.popFront()) {
-                Global* g = r.front().value();
-                if (g->isAnyArrayView())
-                    g->setViewIsSharedView();
-            }
+        if (atomicsPresent_)
             module().setViewsAreShared();
-        }
     }
 };
 
 } // namespace
 
 /*****************************************************************************/
 // Numeric literal utilities
 
@@ -2467,20 +2453,19 @@ CheckGlobalVariableInitImport(ModuleVali
     ValType coerceTo;
     ParseNode* coercedExpr;
     if (!CheckTypeAnnotation(m, initNode, &coerceTo, &coercedExpr))
         return false;
     return CheckGlobalVariableImportExpr(m, varName, coerceTo, coercedExpr, isConst);
 }
 
 static bool
-IsArrayViewCtorName(ModuleValidator& m, PropertyName* name, Scalar::Type* type, bool* shared)
+IsArrayViewCtorName(ModuleValidator& m, PropertyName* name, Scalar::Type* type)
 {
     JSAtomState& names = m.cx()->names();
-    *shared = false;
     if (name == names.Int8Array) {
         *type = Scalar::Int8;
     } else if (name == names.Uint8Array) {
         *type = Scalar::Uint8;
     } else if (name == names.Int16Array) {
         *type = Scalar::Int16;
     } else if (name == names.Uint16Array) {
         *type = Scalar::Uint16;
@@ -2521,50 +2506,45 @@ CheckNewArrayView(ModuleValidator& m, Pr
     PropertyName* bufferName = m.module().bufferArgumentName();
     if (!bufferName)
         return m.fail(newExpr, "cannot create array view without an asm.js heap parameter");
 
     ParseNode* ctorExpr = ListHead(newExpr);
 
     PropertyName* field;
     Scalar::Type type;
-    bool shared = false;
     if (ctorExpr->isKind(PNK_DOT)) {
         ParseNode* base = DotBase(ctorExpr);
 
         if (!IsUseOfName(base, globalName))
             return m.failName(base, "expecting '%s.*Array", globalName);
 
         field = DotMember(ctorExpr);
-        if (!IsArrayViewCtorName(m, field, &type, &shared))
+        if (!IsArrayViewCtorName(m, field, &type))
             return m.fail(ctorExpr, "could not match typed array name");
     } else {
         if (!ctorExpr->isKind(PNK_NAME))
             return m.fail(ctorExpr, "expecting name of imported array view constructor");
 
         PropertyName* globalName = ctorExpr->name();
         const ModuleValidator::Global* global = m.lookupGlobal(globalName);
         if (!global)
             return m.failName(ctorExpr, "%s not found in module global scope", globalName);
 
         if (global->which() != ModuleValidator::Global::ArrayViewCtor)
             return m.failName(ctorExpr, "%s must be an imported array view constructor", globalName);
 
         field = nullptr;
         type = global->viewType();
-        shared = global->viewIsSharedView();
     }
 
     if (!CheckNewArrayViewArgs(m, ctorExpr, bufferName))
         return false;
 
-    if (!m.module().isValidViewSharedness(shared))
-        return m.failName(ctorExpr, "%s has different sharedness than previous view constructors", globalName);
-
-    return m.addArrayView(varName, type, field, shared);
+    return m.addArrayView(varName, type, field);
 }
 
 static bool
 IsSimdTypeName(ModuleValidator& m, PropertyName* name, AsmJSSimdType* type)
 {
     if (name == m.cx()->names().int32x4) {
         *type = AsmJSSimdType_int32x4;
         return true;
@@ -2690,22 +2670,18 @@ CheckGlobalDotImport(ModuleValidator& m,
         if (field == m.cx()->names().NaN)
             return m.addGlobalConstant(varName, GenericNaN(), field);
         if (field == m.cx()->names().Infinity)
             return m.addGlobalConstant(varName, PositiveInfinity<double>(), field);
         if (field == m.cx()->names().byteLength)
             return m.addByteLength(varName);
 
         Scalar::Type type;
-        bool shared = false;
-        if (IsArrayViewCtorName(m, field, &type, &shared)) {
-            if (!m.module().isValidViewSharedness(shared))
-                return m.failName(initNode, "'%s' has different sharedness than previous view constructors", field);
-            return m.addArrayViewCtor(varName, type, field, shared);
-        }
+        if (IsArrayViewCtorName(m, field, &type))
+            return m.addArrayViewCtor(varName, type, field);
 
         return m.failName(initNode, "'%s' is not a standard constant or typed array name", field);
     }
 
     if (base->name() == m.module().importArgumentName())
         return m.addFFI(varName, field);
 
     const ModuleValidator::Global* global = m.lookupGlobal(base->name());
@@ -6782,16 +6758,22 @@ CheckModule(ExclusiveContext* cx, AsmJSP
     if (!CheckModuleProcessingDirectives(m))
         return false;
 
     if (!CheckModuleGlobals(m))
         return false;
 
     m.startFunctionBodies();
 
+#if !defined(ENABLE_SHARED_ARRAY_BUFFER)
+    if (m.usesSharedMemory())
+        return m.failOffset(m.parser().tokenStream.currentToken().pos.begin,
+                            "shared memory and atomics not supported by this build");
+#endif
+
     if (!CheckFunctions(m))
         return false;
 
     if (!CheckFuncPtrTables(m))
         return false;
 
     if (!CheckModuleReturn(m))
         return false;
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/asm.js/gating.js
@@ -0,0 +1,33 @@
+// Check gating of shared memory features in asm.js (bug 1171540,
+// bug 1231624).
+//
+// In asm.js, importing any atomic is a signal that shared memory is
+// being used.  If an atomic is imported, and if shared memory is
+// disabled in the build or in the run then a type error should be
+// signaled for the module at the end of the declaration section and
+// the module should not be an asm.js module.
+
+if (!this.SharedArrayBuffer || !isAsmJSCompilationAvailable())
+    quit(0);
+
+// This code is not run, we only care whether it compiles as asm.js.
+
+function module_a(stdlib, foreign, heap) {
+    "use asm";
+
+    var i32a = new stdlib.Int32Array(heap);
+    var ld = stdlib.Atomics.load;
+
+    // There should be a type error around this line if shared memory
+    // is not enabled.
+
+    function do_load() {
+	var v = 0;
+	v = ld(i32a, 0)|0;	// It's not actually necessary to use the atomic op
+	return v|0;
+    }
+
+    return { load: do_load };
+}
+
+assertEq(isAsmJSModule(module_a), !!this.SharedArrayBuffer);
--- a/js/src/jit-test/tests/basic/bug984766.js
+++ b/js/src/jit-test/tests/basic/bug984766.js
@@ -1,15 +1,17 @@
 
 for (var i = 0; i < 10; i++) {
   x = new ArrayBuffer(4)
   x.f = (function() {})
   new Uint16Array(x).set(JSON.parse)
   gcslice()
 }
 
+if (!this.SharedArrayBuffer)
+  quit(0);
 
 for (var i = 0; i < 10; i++) {
   x = new SharedArrayBuffer(4)
   x.f = (function() {})
   new Uint16Array(x).set(JSON.parse)
   gcslice()
 }