Bug 1429028: Add a stack recursion check in the wasm::TextToBinary function; r=luke draft
authorBenjamin Bouvier <benj@benj.me>
Tue, 09 Jan 2018 14:25:03 +0100
changeset 719224 66be4206150f75be6aa941b6625ce18fa3efdd84
parent 719223 5877a68b5d71d095714be9d5910e87e8b116e007
child 745732 042baf72f5bae521090b8456f953a3ee317181ef
push id95186
push userbbouvier@mozilla.com
push dateThu, 11 Jan 2018 17:38:36 +0000
reviewersluke
bugs1429028
milestone59.0a1
Bug 1429028: Add a stack recursion check in the wasm::TextToBinary function; r=luke MozReview-Commit-ID: HPXo0ARuKD6
js/src/builtin/TestingFunctions.cpp
js/src/jit-test/tests/wasm/compiler-frame-depth.js
js/src/wasm/WasmTextToBinary.cpp
js/src/wasm/WasmTextToBinary.h
--- a/js/src/builtin/TestingFunctions.cpp
+++ b/js/src/builtin/TestingFunctions.cpp
@@ -609,19 +609,21 @@ WasmTextToBinary(JSContext* cx, unsigned
 
     if (args.hasDefined(1)) {
         if (!args[1].isString()) {
             ReportUsageErrorASCII(cx, callee, "Second argument, if present, must be a String");
             return false;
         }
     }
 
+    uintptr_t stackLimit = GetNativeStackLimit(cx);
+
     wasm::Bytes bytes;
     UniqueChars error;
-    if (!wasm::TextToBinary(twoByteChars.twoByteChars(), &bytes, &error)) {
+    if (!wasm::TextToBinary(twoByteChars.twoByteChars(), stackLimit, &bytes, &error)) {
         JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_WASM_TEXT_FAIL,
                                   error.get() ? error.get() : "out of memory");
         return false;
     }
 
     RootedObject obj(cx, JS_NewUint8Array(cx, bytes.length()));
     if (!obj)
         return false;
--- a/js/src/jit-test/tests/wasm/compiler-frame-depth.js
+++ b/js/src/jit-test/tests/wasm/compiler-frame-depth.js
@@ -10,9 +10,17 @@ var code = `(module
  (func
   (result f32)
   (param f32)
   ${expr}
  )
  (export "run" 0)
 )`;
 
-wasmFullPass(code, Math.fround(13.37), {}, 13.37);
+try {
+    wasmFullPass(code, Math.fround(13.37), {}, 13.37);
+} catch (e) {
+    // ASAN will fail these tests because its stack frames are much bigger than
+    // usual ones and the parser will bail out during its recursive descent.
+    // Ignore those errors specifically.
+    assertEq(e.message.includes('out of memory'), true);
+    assertEq(getBuildConfiguration().asan, true);
+}
--- a/js/src/wasm/WasmTextToBinary.cpp
+++ b/js/src/wasm/WasmTextToBinary.cpp
@@ -1660,22 +1660,25 @@ WasmTokenStream::next()
 namespace {
 
 struct WasmParseContext
 {
     WasmTokenStream ts;
     LifoAlloc& lifo;
     UniqueChars* error;
     DtoaState* dtoaState;
-
-    WasmParseContext(const char16_t* text, LifoAlloc& lifo, UniqueChars* error)
+    uintptr_t stackLimit;
+
+    WasmParseContext(const char16_t* text, uintptr_t stackLimit, LifoAlloc& lifo,
+                     UniqueChars* error)
       : ts(text, error),
         lifo(lifo),
         error(error),
-        dtoaState(NewDtoaState())
+        dtoaState(NewDtoaState()),
+        stackLimit(stackLimit)
     {}
 
     ~WasmParseContext() {
         DestroyDtoaState(dtoaState);
     }
 };
 
 } // end anonymous namespace
@@ -2864,16 +2867,18 @@ ParseGrowMemory(WasmParseContext& c, boo
         return nullptr;
 
     return new(c.lifo) AstGrowMemory(operand);
 }
 
 static AstExpr*
 ParseExprBody(WasmParseContext& c, WasmToken token, bool inParens)
 {
+    if (!CheckRecursionLimitDontReport(c.stackLimit))
+        return nullptr;
     switch (token.kind()) {
       case WasmToken::Unreachable:
         return new(c.lifo) AstUnreachable;
       case WasmToken::AtomicCmpXchg:
         return ParseAtomicCmpXchg(c, token.threadOp(), inParens);
       case WasmToken::AtomicLoad:
         return ParseAtomicLoad(c, token.threadOp(), inParens);
       case WasmToken::AtomicRMW:
@@ -3723,19 +3728,20 @@ ParseBinaryModule(WasmParseContext& c, A
     auto* data = new(c.lifo) AstDataSegment(nullptr, Move(fragments));
     if (!data || !module->append(data))
         return nullptr;
 
     return module;
 }
 
 static AstModule*
-ParseModule(const char16_t* text, LifoAlloc& lifo, UniqueChars* error, bool* binary)
+ParseModule(const char16_t* text, uintptr_t stackLimit, LifoAlloc& lifo, UniqueChars* error,
+            bool* binary)
 {
-    WasmParseContext c(text, lifo, error);
+    WasmParseContext c(text, stackLimit, lifo, error);
 
     *binary = false;
 
     if (!c.ts.match(WasmToken::OpenParen, c.error))
         return nullptr;
     if (!c.ts.match(WasmToken::Module, c.error))
         return nullptr;
 
@@ -5431,22 +5437,22 @@ EncodeBinaryModule(const AstModule& modu
     }
 
     return true;
 }
 
 /*****************************************************************************/
 
 bool
-wasm::TextToBinary(const char16_t* text, Bytes* bytes, UniqueChars* error)
+wasm::TextToBinary(const char16_t* text, uintptr_t stackLimit, Bytes* bytes, UniqueChars* error)
 {
     LifoAlloc lifo(AST_LIFO_DEFAULT_CHUNK_SIZE);
 
     bool binary = false;
-    AstModule* module = ParseModule(text, lifo, error, &binary);
+    AstModule* module = ParseModule(text, stackLimit, lifo, error, &binary);
     if (!module)
         return false;
 
     if (binary)
         return EncodeBinaryModule(*module, bytes);
 
     if (!ResolveModule(lifo, module, error))
         return false;
--- a/js/src/wasm/WasmTextToBinary.h
+++ b/js/src/wasm/WasmTextToBinary.h
@@ -24,14 +24,14 @@
 namespace js {
 namespace wasm {
 
 // Translate the textual representation of a wasm module (given by a
 // null-terminated char16_t array) into serialized bytes. If there is an error
 // other than out-of-memory an error message string will be stored in 'error'.
 
 extern MOZ_MUST_USE bool
-TextToBinary(const char16_t* text, Bytes* bytes, UniqueChars* error);
+TextToBinary(const char16_t* text, uintptr_t stackLimit, Bytes* bytes, UniqueChars* error);
 
 } // namespace wasm
 } // namespace js
 
 #endif // wasm_text_to_binary_h