Bug 1500120 - fix compiler state for struct.narrow. r=bbouvier
authorLars T Hansen <lhansen@mozilla.com>
Thu, 18 Oct 2018 17:15:19 +0200
changeset 490262 a25a1dd8a9f58ee74fe2922c0611d3013be00091
parent 490261 fe962bfc351a0f198e3fa990693973eee5fbcf81
child 490263 ab27299789d3c74d58140d9d6651bc1102e439ed
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersbbouvier
bugs1500120
milestone64.0a1
Bug 1500120 - fix compiler state for struct.narrow. r=bbouvier This makes sure that the compiler's view of the stack matches the run-time stack when we generate code for struct.narrow. The bug crept in here because I insisted on generating an in-line test for null before calling out to the instance. But this is unnecessary; the code in the instance can do this just fine, and null is not a common case here. (And struct.narrow is extremely slow in any case, as it's just prototype code) So, move the null check to C++ and generate straight-line code in Rabaldr.
js/src/wasm/WasmBaselineCompile.cpp
js/src/wasm/WasmInstance.cpp
js/src/wasm/WasmInstance.h
--- a/js/src/wasm/WasmBaselineCompile.cpp
+++ b/js/src/wasm/WasmBaselineCompile.cpp
@@ -9234,18 +9234,25 @@ BaseCompiler::emitInstanceCall(uint32_t 
         }
         passArg(t, peek(numArgs - i), &baselineCall);
     }
     builtinInstanceMethodCall(builtin, instanceArg, baselineCall);
     endCall(baselineCall, stackSpace);
 
     popValueStackBy(numArgs);
 
-    // Note, a number of clients of emitInstanceCall currently assume that the
-    // following operation does not destroy ReturnReg.
+    // Note, many clients of emitInstanceCall currently assume that pushing the
+    // result here does not destroy ReturnReg.
+    //
+    // Furthermore, clients assume that even if retType == ExprType::Void, the
+    // callee may have returned a status result and left it in ReturnReg for us
+    // to find, and that that register will not be destroyed here (or above).
+    // In this case the callee will have a C++ declaration stating that there is
+    // a return value.  Examples include memory and table operations that are
+    // implemented as callouts.
 
     pushReturnedIfNonVoid(baselineCall, retType);
 }
 
 bool
 BaseCompiler::emitGrowMemory()
 {
     uint32_t lineOrBytecode = readCallSiteLineOrBytecode();
@@ -10075,42 +10082,31 @@ BaseCompiler::emitStructNarrow()
     }
 
     // AnyRef -> AnyRef is a no-op, just leave the value on the stack.
 
     if (inputType == ValType::AnyRef && outputType == ValType::AnyRef) {
         return true;
     }
 
-    // Null pointers are just passed through.
-
-    Label done;
-    Label doTest;
     RegPtr rp = popRef();
-    masm.branchTestPtr(Assembler::NonZero, rp, rp, &doTest);
-    pushRef(NULLREF_VALUE);
-    masm.jump(&done);
 
     // AnyRef -> (ref T) must first unbox; leaves rp or null
 
     bool mustUnboxAnyref = inputType == ValType::AnyRef;
 
     // Dynamic downcast (ref T) -> (ref U), leaves rp or null
 
     const StructType& outputStruct = env_.types[outputType.refTypeIndex()].structType();
 
-    masm.bind(&doTest);
-
     pushI32(mustUnboxAnyref);
     pushI32(outputStruct.moduleIndex_);
     pushRef(rp);
     emitInstanceCall(lineOrBytecode, SigPIIP_, ExprType::AnyRef, SymbolicAddress::StructNarrow);
 
-    masm.bind(&done);
-
     return true;
 }
 
 bool
 BaseCompiler::emitBody()
 {
     if (!iter_.readFunctionStart(funcType().ret())) {
         return false;
--- a/js/src/wasm/WasmInstance.cpp
+++ b/js/src/wasm/WasmInstance.cpp
@@ -709,23 +709,28 @@ Instance::structNew(Instance* instance, 
 {
     JSContext* cx = TlsContext.get();
     Rooted<TypeDescr*> typeDescr(cx, instance->structTypeDescrs_[typeIndex]);
     return TypedObject::createZeroed(cx, typeDescr);
 }
 
 /* static */ void*
 Instance::structNarrow(Instance* instance, uint32_t mustUnboxAnyref, uint32_t outputTypeIndex,
-                       void* nonnullPtr)
+                       void* maybeNullPtr)
 {
     JSContext* cx = TlsContext.get();
 
     Rooted<TypedObject*> obj(cx);
     Rooted<StructTypeDescr*> typeDescr(cx);
 
+    if (maybeNullPtr == nullptr) {
+        return maybeNullPtr;
+    }
+
+    void* nonnullPtr = maybeNullPtr;
     if (mustUnboxAnyref) {
         Rooted<NativeObject*> no(cx, static_cast<NativeObject*>(nonnullPtr));
         if (!no->is<TypedObject>()) {
             return nullptr;
         }
         obj = &no->as<TypedObject>();
         Rooted<TypeDescr*> td(cx, &obj->typeDescr());
         if (td->kind() != type::Struct) {
--- a/js/src/wasm/WasmInstance.h
+++ b/js/src/wasm/WasmInstance.h
@@ -187,17 +187,18 @@ class Instance
     static int32_t memInit(Instance* instance, uint32_t dstOffset,
                            uint32_t srcOffset, uint32_t len, uint32_t segIndex);
     static int32_t tableCopy(Instance* instance, uint32_t dstOffset, uint32_t srcOffset, uint32_t len);
     static int32_t tableDrop(Instance* instance, uint32_t segIndex);
     static int32_t tableInit(Instance* instance, uint32_t dstOffset,
                              uint32_t srcOffset, uint32_t len, uint32_t segIndex);
     static void postBarrier(Instance* instance, gc::Cell** location);
     static void* structNew(Instance* instance, uint32_t typeIndex);
-    static void* structNarrow(Instance* instance, uint32_t mustUnboxAnyref, uint32_t outputTypeIndex, void* ptr);
+    static void* structNarrow(Instance* instance, uint32_t mustUnboxAnyref, uint32_t outputTypeIndex,
+                              void* maybeNullPtr);
 };
 
 typedef UniquePtr<Instance> UniqueInstance;
 
 } // namespace wasm
 } // namespace js
 
 #endif // wasm_instance_h