Bug 1253137 - Baldr: switch from expression-count to function body byte size (r=sunfish)

MozReview-Commit-ID: KcbWjViZAM6

js/src/asmjs/AsmJS.cpp

@@ -3494,7 +3494,7 @@ SetLocal(FunctionValidator& f, NumLit lit)
 }
 
 static bool
-CheckVariable(FunctionValidator& f, ParseNode* var, uint32_t* numStmts)
+CheckVariable(FunctionValidator& f, ParseNode* var)
 {
     if (!IsDefinition(var))
         return f.fail(var, "local variable names must not restate argument names");
@@ -3516,7 +3516,6 @@ CheckVariable(FunctionValidator& f, ParseNode* var, uint32_t* numStmts)
         return f.failName(var, "var '%s' initializer out of range", name);
 
     if (!lit.isZeroBits()) {
-        ++*numStmts;
         if (!SetLocal(f, lit))
             return false;
     }
@@ -3525,13 +3524,13 @@ CheckVariable(FunctionValidator& f, ParseNode* var, uint32_t* numStmts)
 }
 
 static bool
-CheckVariables(FunctionValidator& f, ParseNode** stmtIter, uint32_t* numStmts)
+CheckVariables(FunctionValidator& f, ParseNode** stmtIter)
 {
     ParseNode* stmt = *stmtIter;
 
     for (; stmt && stmt->isKind(PNK_VAR); stmt = NextNonEmptyStatement(stmt)) {
         for (ParseNode* var = VarListHead(stmt); var; var = NextNode(var)) {
-            if (!CheckVariable(f, var, numStmts))
+            if (!CheckVariable(f, var))
                 return false;
         }
     }
@@ -6732,18 +6731,11 @@ CheckFunction(ModuleValidator& m)
     if (!CheckArguments(f, &stmtIter, &args))
         return false;
 
-    uint32_t numStmts = 0;
-
-    size_t numStmtsAt;
-    if (!f.encoder().writePatchableVarU32(&numStmtsAt))
-        return false;
-
-    if (!CheckVariables(f, &stmtIter, &numStmts))
+    if (!CheckVariables(f, &stmtIter))
         return false;
 
     ParseNode* lastNonEmptyStmt = nullptr;
     for (; stmtIter; stmtIter = NextNonEmptyStatement(stmtIter)) {
-        numStmts++;
         lastNonEmptyStmt = stmtIter;
         if (!CheckStatement(f, stmtIter))
             return false;
@@ -6752,8 +6744,6 @@ CheckFunction(ModuleValidator& m)
     if (!CheckFinalReturn(f, lastNonEmptyStmt))
         return false;
 
-    f.encoder().patchVarU32(numStmtsAt, numStmts);
-
     ModuleValidator::Func* func = nullptr;
     if (!CheckFunctionSignature(m, fn, Sig(Move(args), f.returnedType()), FunctionName(fn), &func))
         return false;

js/src/asmjs/Wasm.cpp

@@ -1262,25 +1262,31 @@ DecodeFunctionBody(JSContext* cx, Decoder& d, ModuleGenerator& mg, uint32_t func
             return false;
     }
 
-    const uint8_t* bodyBegin = d.currentPosition();
-
     FunctionDecoder f(cx, d, mg, fg, funcIndex);
 
-    uint32_t numExprs;
-    if (!d.readVarU32(&numExprs))
-        return Fail(cx, d, "expected number of function body expressions");
+    uint32_t numBytes;
+    if (!d.readVarU32(&numBytes))
+        return Fail(cx, d, "expected number of function body bytes");
+
+    if (d.bytesRemain() < numBytes)
+        return Fail(cx, d, "function body length too big");
+
+    const uint8_t* bodyBegin = d.currentPosition();
+    const uint8_t* bodyEnd = bodyBegin + numBytes;
 
     ExprType type = ExprType::Void;
 
-    for (size_t i = 0; i < numExprs; i++) {
+    while (d.currentPosition() < bodyEnd) {
         if (!DecodeExpr(f, &type))
             return false;
     }
 
+    if (d.currentPosition() != bodyEnd)
+        return Fail(cx, d, "function body length mismatch");
+
     if (!CheckType(f, type, f.ret()))
         return false;
 
-    const uint8_t* bodyEnd = d.currentPosition();
     uintptr_t bodyLength = bodyEnd - bodyBegin;
     if (!fg.bytecode().resize(bodyLength))
         return false;

js/src/asmjs/WasmBinary.h

@@ -384,8 +384,8 @@ class Encoder
         MOZ_ASSERT(empty());
     }
 
-    size_t bytecodeOffset() const { return bytecode_.length(); }
-    bool empty() const { return bytecodeOffset() == 0; }
+    size_t currentOffset() const { return bytecode_.length(); }
+    bool empty() const { return currentOffset() == 0; }
 
     // Fixed-size encoding operations simply copy the literal bytes (without
     // attempting to align).
@@ -424,6 +424,32 @@ class Encoder
         return writeEnum(type);
     }
 
+    // Variable-length encodings that allow back-patching.
+
+    MOZ_WARN_UNUSED_RESULT bool writePatchableVarU32(size_t* offset) {
+        *offset = bytecode_.length();
+        return writeVarU32(UINT32_MAX);
+    }
+    void patchVarU32(size_t offset, uint32_t patchBits) {
+        return patchVarU32(offset, patchBits, UINT32_MAX);
+    }
+
+    MOZ_WARN_UNUSED_RESULT bool writePatchableVarU8(size_t* offset) {
+        *offset = bytecode_.length();
+        return writeU8(UINT8_MAX);
+    }
+    void patchVarU8(size_t offset, uint8_t patchBits) {
+        MOZ_ASSERT(patchBits < 0x80);
+        return patchU8(offset, patchBits);
+    }
+
+    MOZ_WARN_UNUSED_RESULT bool writePatchableExpr(size_t* offset) {
+        return writePatchableEnum<Expr>(offset);
+    }
+    void patchExpr(size_t offset, Expr expr) {
+        patchEnum(offset, expr);
+    }
+
     // C-strings are written in UTF8 and null-terminated while raw data can
     // contain nulls and instead has an explicit byte length.
 
@@ -452,34 +478,6 @@ class Encoder
         return patchVarU32(offset, bytecode_.length() - offset - varU32ByteLength(offset));
     }
 
-    // Patching is necessary due to the combination of a preorder encoding and a
-    // single-pass algorithm that only knows the precise opcode after visiting
-    // children. Switching to a postorder encoding will remove these methods:
-
-    MOZ_WARN_UNUSED_RESULT bool writePatchableVarU32(size_t* offset) {
-        *offset = bytecode_.length();
-        return writeVarU32(UINT32_MAX);
-    }
-    void patchVarU32(size_t offset, uint32_t patchBits) {
-        return patchVarU32(offset, patchBits, UINT32_MAX);
-    }
-
-    MOZ_WARN_UNUSED_RESULT bool writePatchableVarU8(size_t* offset) {
-        *offset = bytecode_.length();
-        return writeU8(UINT8_MAX);
-    }
-    void patchVarU8(size_t offset, uint8_t patchBits) {
-        MOZ_ASSERT(patchBits < 0x80);
-        return patchU8(offset, patchBits);
-    }
-
-    MOZ_WARN_UNUSED_RESULT bool writePatchableExpr(size_t* offset) {
-        return writePatchableEnum<Expr>(offset);
-    }
-    void patchExpr(size_t offset, Expr expr) {
-        patchEnum(offset, expr);
-    }
-
     // Temporary encoding forms which should be removed as part of the
     // conversion to wasm:
 
@@ -502,11 +500,6 @@ class Decoder
     const uint8_t* const end_;
     const uint8_t* cur_;
 
-    uintptr_t bytesRemain() const {
-        MOZ_ASSERT(end_ >= cur_);
-        return uintptr_t(end_ - cur_);
-    }
-
     template <class T>
     MOZ_WARN_UNUSED_RESULT bool read(T* out) {
         if (bytesRemain() < sizeof(T))
@@ -588,6 +581,10 @@ class Decoder
         return cur_ == end_;
     }
 
+    uintptr_t bytesRemain() const {
+        MOZ_ASSERT(end_ >= cur_);
+        return uintptr_t(end_ - cur_);
+    }
     const uint8_t* currentPosition() const {
         return cur_;
     }

js/src/asmjs/WasmIonCompile.cpp

@@ -3061,20 +3061,12 @@ wasm::IonCompileFunction(IonCompileTask* task)
         if (!f.init())
             return false;
 
-        MDefinition* last = nullptr;
-        if (uint32_t numExprs = f.readVarU32()) {
-            for (uint32_t i = 0; i < numExprs - 1; i++) {
-                if (!EmitExpr(f, &last))
-                    return false;
-            }
-
+        MDefinition* last;
+        while (!f.done()) {
             if (!EmitExpr(f, &last))
                 return false;
         }
 
-        MOZ_ASSERT(f.done());
-        MOZ_ASSERT(IsVoid(f.sig().ret()) || f.inDeadCode() || last);
-
         if (IsVoid(f.sig().ret()))
             f.returnVoid();
         else

js/src/asmjs/WasmText.cpp

@@ -3927,14 +3927,18 @@ EncodeFunctionBody(Encoder& e, WasmAstFunc& func)
             return false;
     }
 
-    if (!e.writeVarU32(func.body().length()))
+    size_t bodySizeAt;
+    if (!e.writePatchableVarU32(&bodySizeAt))
         return false;
 
+    size_t beforeBody = e.currentOffset();
+
     for (WasmAstExpr* expr : func.body()) {
         if (!EncodeExpr(e, *expr))
             return false;
     }
 
+    e.patchVarU32(bodySizeAt, e.currentOffset() - beforeBody);
     return true;
 }
 

js/src/jit-test/tests/wasm/basic-control-flow.js

@@ -140,7 +140,9 @@ assertEq(wasmEvalText('(module (func (result i32) (return (i32.const 1))) (expor
 assertEq(wasmEvalText('(module (func (if (return) (i32.const 0))) (export "" 0))')(), undefined);
 assertErrorMessage(() => wasmEvalText('(module (func (result f32) (return (i32.const 1))) (export "" 0))'), TypeError, mismatchError("i32", "f32"));
 assertThrowsInstanceOf(() => wasmEvalText('(module (func (result i32) (return)) (export "" 0))'), TypeError);
-assertThrowsInstanceOf(() => wasmEvalText('(module (func (return (i32.const 1))) (export "" 0))'), TypeError);
+
+// TODO: Reenable when syntactic arities are added for returns
+//assertThrowsInstanceOf(() => wasmEvalText('(module (func (return (i32.const 1))) (export "" 0))'), TypeError);
 
 // TODO: convert these to wasmEval and assert some results once they are implemented
 

js/src/jit-test/tests/wasm/basic.js

@@ -289,7 +289,9 @@ assertEq(wasmEvalText('(module (func (result i32) (local i32) (set_local 0 (i32.
 // ----------------------------------------------------------------------------
 // calls
 
-assertThrowsInstanceOf(() => wasmEvalText('(module (func (nop)) (func (call 0 (i32.const 0))))'), TypeError);
+// TODO: Reenable when syntactic arities are added for calls
+//assertThrowsInstanceOf(() => wasmEvalText('(module (func (nop)) (func (call 0 (i32.const 0))))'), TypeError);
+
 assertThrowsInstanceOf(() => wasmEvalText('(module (func (param i32) (nop)) (func (call 0)))'), TypeError);
 assertThrowsInstanceOf(() => wasmEvalText('(module (func (param f32) (nop)) (func (call 0 (i32.const 0))))'), TypeError);
 assertErrorMessage(() => wasmEvalText('(module (func (nop)) (func (call 3)))'), TypeError, /callee index out of range/);
@@ -301,7 +303,9 @@ assertThrowsInstanceOf(() => wasmEvalText('(module (func (call 1)) (func (call 0
 wasmEvalText('(module (func (param i32 f32)) (func (call 0 (i32.const 0) (f32.const nan))))');
 assertErrorMessage(() => wasmEvalText('(module (func (param i32 f32)) (func (call 0 (i32.const 0) (i32.const 0))))'), TypeError, mismatchError("i32", "f32"));
 
-assertThrowsInstanceOf(() => wasmEvalText('(module (import "a" "") (func (call_import 0 (i32.const 0))))', {a:()=>{}}), TypeError);
+// TODO: Reenable when syntactic arities are added for calls
+//assertThrowsInstanceOf(() => wasmEvalText('(module (import "a" "") (func (call_import 0 (i32.const 0))))', {a:()=>{}}), TypeError);
+
 assertThrowsInstanceOf(() => wasmEvalText('(module (import "a" "" (param i32)) (func (call_import 0)))', {a:()=>{}}), TypeError);
 assertThrowsInstanceOf(() => wasmEvalText('(module (import "a" "" (param f32)) (func (call_import 0 (i32.const 0))))', {a:()=>{}}), TypeError);
 assertErrorMessage(() => wasmEvalText('(module (import "a" "") (func (call_import 1)))'), TypeError, /import index out of range/);

js/src/jsapi-tests/testWasmLEB128.cpp

@@ -32,7 +32,7 @@ static bool WriteValidBytes(js::wasm::Encoder& encoder, bool* passed)
 
     if (encoder.empty())
         return true;
-    if (encoder.bytecodeOffset() != 7)
+    if (encoder.currentOffset() != 7)
         return true;
     *passed = true;
     return true;