Bug 1284156 - Baldr: simplify representation of null table elements (r=bbouvier)

MozReview-Commit-ID: 1hyFX6CCGqR

js/src/asmjs/WasmCode.cpp

@@ -231,7 +231,6 @@ CodeSegment::create(JSContext* cx,
     cs->interruptCode_ = codeBase + linkData.interruptOffset;
     cs->outOfBoundsCode_ = codeBase + linkData.outOfBoundsOffset;
     cs->unalignedAccessCode_ = codeBase + linkData.unalignedAccessOffset;
-    cs->badIndirectCallCode_ = codeBase + linkData.badIndirectCallOffset;
 
     {
         JitContext jcx(CompileRuntime::get(cx->compartment()->runtimeFromAnyThread()));

js/src/asmjs/WasmCode.h

@@ -54,7 +54,6 @@ class CodeSegment
     // These are pointers into code for stubs used for asynchronous
     // signal-handler control-flow transfer.
     uint8_t* interruptCode_;
-    uint8_t* badIndirectCallCode_;
     uint8_t* outOfBoundsCode_;
     uint8_t* unalignedAccessCode_;
 
@@ -84,7 +83,6 @@ class CodeSegment
     uint32_t totalLength() const { return codeLength_ + globalDataLength_; }
 
     uint8_t* interruptCode() const { return interruptCode_; }
-    uint8_t* badIndirectCallCode() const { return badIndirectCallCode_; }
     uint8_t* outOfBoundsCode() const { return outOfBoundsCode_; }
     uint8_t* unalignedAccessCode() const { return unalignedAccessCode_; }
 

js/src/asmjs/WasmGenerator.cpp

@@ -508,7 +508,6 @@ ModuleGenerator::finishCodegen()
     linkData_.interruptOffset = interruptExit.begin;
     linkData_.outOfBoundsOffset = jumpTargets[JumpTarget::OutOfBounds].begin;
     linkData_.unalignedAccessOffset = jumpTargets[JumpTarget::UnalignedAccess].begin;
-    linkData_.badIndirectCallOffset = jumpTargets[JumpTarget::BadIndirectCall].begin;
 
     // Only call convertOutOfRangeBranchesToThunks after all other codegen that may
     // emit new jumps to JumpTargets has finished.

js/src/asmjs/WasmInstance.cpp

@@ -843,7 +843,7 @@ Instance::ensureProfilingState(JSContext* cx, bool newProfilingEnabled)
     }
 
     for (const SharedTable& table : tables_) {
-        if (!table->isTypedFunction() || !table->initialized())
+        if (!table->isTypedFunction())
             continue;
 
         // This logic will have to be generalized to match the import logic
@@ -853,8 +853,10 @@ Instance::ensureProfilingState(JSContext* cx, bool newProfilingEnabled)
 
         void** array = table->internalArray();
         uint32_t length = table->length();
-        for (size_t i = 0; i < length; i++)
-            UpdateEntry(*code_, newProfilingEnabled, &array[i]);
+        for (size_t i = 0; i < length; i++) {
+            if (array[i])
+                UpdateEntry(*code_, newProfilingEnabled, &array[i]);
+        }
     }
 
     return true;

js/src/asmjs/WasmJS.cpp

@@ -1200,24 +1200,19 @@ WasmTableObject::getImpl(JSContext* cx, const CallArgs& args)
     uint32_t index = uint32_t(indexDbl);
     MOZ_ASSERT(double(index) == indexDbl);
 
-    if (!table.initialized()) {
-        args.rval().setNull();
-        return true;
-    }
-
     ExternalTableElem& elem = table.externalArray()[index];
-    Instance& instance = *elem.tls->instance;
-    const CodeRange* codeRange = instance.code().lookupRange(elem.code);
-
-    // A non-function code range means the bad-indirect-call stub, so a null element.
-    if (!codeRange || !codeRange->isFunction()) {
+    if (!elem.code) {
         args.rval().setNull();
         return true;
     }
 
+    Instance& instance = *elem.tls->instance;
+    const CodeRange& codeRange = *instance.code().lookupRange(elem.code);
+    MOZ_ASSERT(codeRange.isFunction());
+
     RootedWasmInstanceObject instanceObj(cx, instance.object());
     RootedFunction fun(cx);
-    if (!instanceObj->getExportedFunction(cx, instanceObj, codeRange->funcDefIndex(), &fun))
+    if (!instanceObj->getExportedFunction(cx, instanceObj, codeRange.funcDefIndex(), &fun))
         return false;
 
     args.rval().setObject(*fun);
@@ -1258,15 +1253,6 @@ WasmTableObject::setImpl(JSContext* cx, const CallArgs& args)
         return false;
     }
 
-    if (!table.initialized()) {
-        if (!value) {
-            args.rval().setUndefined();
-            return true;
-        }
-
-        table.init(ExportedFunctionToInstance(value));
-    }
-
     if (value) {
         RootedWasmInstanceObject instanceObj(cx, ExportedFunctionToInstanceObject(value));
         uint32_t funcDefIndex = ExportedFunctionToDefinitionIndex(value);

js/src/asmjs/WasmModule.cpp

@@ -462,13 +462,6 @@ Module::initSegments(JSContext* cx,
         MOZ_ASSERT(dataSegments_.empty());
     }
 
-    // Ensure all tables are initialized before storing into them.
-
-    for (const SharedTable& table : tables) {
-        if (!table->initialized())
-            table->init(instance);
-    }
-
     // Now that initialization can't fail partway through, write data/elem
     // segments into memories/tables.
 

js/src/asmjs/WasmModule.h

@@ -51,7 +51,6 @@ struct LinkDataCacheablePod
     uint32_t interruptOffset;
     uint32_t outOfBoundsOffset;
     uint32_t unalignedAccessOffset;
-    uint32_t badIndirectCallOffset;
 
     LinkDataCacheablePod() { mozilla::PodZero(this); }
 };

js/src/asmjs/WasmTable.cpp

@@ -48,32 +48,10 @@ Table::create(JSContext* cx, const TableDesc& desc, HandleWasmTableObject maybeO
     table->array_.reset((uint8_t*)array);
     table->kind_ = desc.kind;
     table->length_ = desc.initial;
-    table->initialized_ = false;
     table->external_ = desc.external;
     return table;
 }
 
-void
-Table::init(Instance& instance)
-{
-    MOZ_ASSERT(!initialized());
-    initialized_ = true;
-
-    void* code = instance.codeSegment().badIndirectCallCode();
-    if (external_) {
-        ExternalTableElem* array = externalArray();
-        TlsData* tls = &instance.tlsData();
-        for (uint32_t i = 0; i < length_; i++) {
-            array[i].code = code;
-            array[i].tls = tls;
-        }
-    } else {
-        void** array = internalArray();
-        for (uint32_t i = 0; i < length_; i++)
-            array[i] = code;
-    }
-}
-
 void
 Table::tracePrivate(JSTracer* trc)
 {
@@ -87,12 +65,15 @@ Table::tracePrivate(JSTracer* trc)
         TraceEdge(trc, &maybeObject_, "wasm table object");
     }
 
-    if (!initialized_ || !external_)
-        return;
-
-    ExternalTableElem* array = externalArray();
-    for (uint32_t i = 0; i < length_; i++)
-        array[i].tls->instance->trace(trc);
+    if (external_) {
+        ExternalTableElem* array = externalArray();
+        for (uint32_t i = 0; i < length_; i++) {
+            if (array[i].tls)
+                array[i].tls->instance->trace(trc);
+            else
+                MOZ_ASSERT(!array[i].code);
+        }
+    }
 }
 
 void
@@ -112,7 +93,6 @@ Table::trace(JSTracer* trc)
 void**
 Table::internalArray() const
 {
-    MOZ_ASSERT(initialized_);
     MOZ_ASSERT(!external_);
     return (void**)array_.get();
 }
@@ -120,7 +100,6 @@ Table::internalArray() const
 ExternalTableElem*
 Table::externalArray() const
 {
-    MOZ_ASSERT(initialized_);
     MOZ_ASSERT(external_);
     return (ExternalTableElem*)array_.get();
 }
@@ -142,7 +121,8 @@ Table::setNull(uint32_t index)
 {
     // Only external tables can set elements to null after initialization.
     ExternalTableElem& elem = externalArray()[index];
-    elem.code = elem.tls->instance->codeSegment().badIndirectCallCode();
+    elem.code = nullptr;
+    elem.tls = nullptr;
 }
 
 size_t

js/src/asmjs/WasmTable.h

@@ -36,7 +36,6 @@ class Table : public ShareableBase<Table>
     UniqueByteArray              array_;
     TableKind                    kind_;
     uint32_t                     length_;
-    bool                         initialized_;
     bool                         external_;
 
     void tracePrivate(JSTracer* trc);
@@ -47,21 +46,14 @@ class Table : public ShareableBase<Table>
                                 HandleWasmTableObject maybeObject);
     void trace(JSTracer* trc);
 
-    // These accessors may be used before initialization.
-
     bool external() const { return external_; }
     bool isTypedFunction() const { return kind_ == TableKind::TypedFunction; }
     uint32_t length() const { return length_; }
     uint8_t* base() const { return array_.get(); }
 
-    // A Table must be initialized before any dependent instance can execute.
-
-    bool initialized() const { return initialized_; }
-    void init(Instance& instance);
-
-    // After initialization, elements must be accessed. All updates must go
-    // through a set() function with the exception of (profiling) updates to the
-    // callee pointer that do not change which logical function is being called.
+    // All updates must go through a set() function with the exception of
+    // (profiling) updates to the callee pointer that do not change which
+    // logical function is being called.
 
     void** internalArray() const;
     ExternalTableElem* externalArray() const;

js/src/jit-test/tests/wasm/table-gc.js

@@ -79,8 +79,7 @@ t = null;
 gc();
 assertEq(finalizeCount(), 4);
 
-// The bad-indirect-call stub should (currently, could be changed later) keep
-// the instance containing that stub alive.
+// Null elements shouldn't keep anything alive.
 resetFinalizeCount();
 var i = evalText(`(module (table (resizable 2)) (export "tbl" table) ${caller})`);
 var e = i.exports;
@@ -96,7 +95,7 @@ gc();
 assertEq(finalizeCount(), 1);
 i = null;
 gc();
-assertEq(finalizeCount(), 1);
+assertEq(finalizeCount(), 2);
 t = null;
 gc();
 assertEq(finalizeCount(), 3);
@@ -144,14 +143,13 @@ assertEq(finalizeCount(), 2);
 t.set(0, null);
 assertEq(t.get(0), null);
 gc();
-assertEq(finalizeCount(), 2);
+assertEq(finalizeCount(), 3);
 t = null;
 gc();
 assertEq(finalizeCount(), 4);
 
-// Once all of an instance's elements in a Table (including the
-// bad-indirect-call stub) have been clobbered, the Instance should not be
-// rooted.
+// Once all of an instance's elements in a Table have been clobbered, the
+// Instance should not be reachable.
 resetFinalizeCount();
 var i1 = evalText(`(module (func $f1 (result i32) (i32.const 13)) (export "f1" $f1))`);
 var i2 = evalText(`(module (func $f2 (result i32) (i32.const 42)) (export "f2" $f2))`);

js/src/jit/MacroAssembler.cpp

@@ -2818,11 +2818,14 @@ MacroAssembler::wasmCallIndirect(const wasm::CallSiteDesc& desc, const wasm::Cal
         }
 
         loadPtr(Address(scratch, offsetof(wasm::ExternalTableElem, tls)), WasmTlsReg);
+        branchTest32(Assembler::Zero, WasmTlsReg, WasmTlsReg, wasm::JumpTarget::BadIndirectCall);
+
         loadWasmPinnedRegsFromTls();
 
         loadPtr(Address(scratch, offsetof(wasm::ExternalTableElem, code)), scratch);
     } else {
         loadPtr(BaseIndex(scratch, index, ScalePointer), scratch);
+        branchTest32(Assembler::Zero, scratch, scratch, wasm::JumpTarget::BadIndirectCall);
     }
 
     call(desc, scratch);